update documentation for picolcd driver; cleanup of picolcd driver

CREDITS

@@ -228,7 +228,7 @@ Dave Platt
 	- overhauled CwLnx driver
 
 Nicu Pavel
-	- overhauled picolcd driver
+	- overhauled & extended picolcd driver
 
 Daryl Fonseca-Holt
 	- lis driver

LCDd.conf

@@ -775,9 +775,6 @@ Reboot=no
 
 ## Mini-box.com picoLCD (usblcd) driver ##
 [picolcd]
-# This is for the Mini-Box.com usblcd device that ships with the M300.  It can 
-# also be purchased separately but this driver has only been tested on the M300 
-# setup.
 
 # KeyTimeout is the time that LCDd spends waiting for a key press before cycling 
 # through other duties.  Higher values make LCDd use less CPU time and make 
@@ -786,7 +783,10 @@ Reboot=no
 # balanced value.
 KeyTimeout=500
 
-# Contrast: [default: 1000; legal: 0 - 1000]
+# Set the initial brightness [default: 1000; legal: 0 - 1000]
+Brightness=1000
+
+# Set the initial contrast [default: 1000; legal: 0 - 1000]
 Contrast=1000
 
 # Light the keys? i[default: on; legal: on, off]

docs/LCDd.8.in

@@ -220,7 +220,7 @@ LCD display on the Logitech MX5000 keyboard
 Noritake VFD Device CU20045SCPB-T28A
 .TP
 .B picolcd
-Mini-box.com USB LCD (picoLCD 20x2)
+Mini-box.com USB LCD (PicoLCD 20x4 & picoLCD 20x2)
 .TP
 .B pyramid
 LCD displays from Pyramid (http://www.pyramid.de) 

docs/lcdproc-user/drivers/picolcd.docbook

@@ -1,56 +1,108 @@
 <sect1 id="picolcd">
-<title>The Mini-Box USB LCD (picoLCD 20x2) Driver (picolcd)</title>
+<title>The Mini-Box.com USB LCD picoLCD Driver</title>
 
 <para>
-	This section covers the installation and use of the Mini-Box USB LCD (picoLCD 
+	This section covers the installation and use of the Mini-Box USB LCD displays.
-	20x2) display.
-</para>
-<para>
-	The picoLCD page indicates that the device can be connected via I<superscript>2</superscript>C 
-	or USART.  
-	This LCDproc driver, however, is limited to the USB connected model. 
 </para>
 
-<sect2 id="picolcd-setup">
+<sect2 id="picolcd-displays">
-<title>Setting up the picoLCD</title>
+<title>Displays</title>
+<para>
+	<ulink url="http://www.mini-box.com/">Mini-Box.com</ulink> offers two types of
+	USB LCD displays:
+</para>
+
+<variablelist>
+<varlistentry>
+<term>PicoLCD 4x20-Sideshow</term>
+<listitem>
+<para>
+	<ulink url="http://www.mini-box.com/PicoLCD-4X20-Sideshow">PicoLCD 4x20-Sideshow</ulink>
+	is the desktop variant targeted at end users.
+	It an external USB 2.0 full speed device that comes in a stylish casing and
+	sports a 4 line by 20 character display with white letters
+	on a blue background, a built-in InfraRed receiver as well as an
+	keypad with 8 keys labeled <literal>Escape</literal>, <literal>F1</literal>,
+	<literal>F2</literal>, <literal>F3</literal>, <literal>Home</literal>,
+	<literal>Up</literal>, <literal>Down</literal> and <literal>Enter</literal>.
+</para>
+</listitem>
+</varlistentry>
+
+<varlistentry>
+<term>picoLCD 20x2 (OEM)</term>
+<listitem>
+<para>
+	<ulink url="http://www.mini-box.com/picoLCD-20x2">picoLCD-20x2-OEM</ulink> is
+	the OEM version.
+	It is a 2 line by 20 character display with black letters on a
+	yellow-green background, that can be connected to the system via
+	USB, I<superscript>2</superscript>C or USART (the latter two are
+	not supported by this driver).
+	It  has connectors for an InfraRed receiver, keypad and LEDs.
+</para>
 
 <para>
-	The picoLCD device as used by this driver is USB connected and may be purchased 
+ 	When pre-installed in enclosures like the
-	with an internal USB header connector (5-pin to 5-pin) or an external cable 
+	<ulink url="http://www.mini-box.com/Mini-Box-M300-LCD">Mini-Box M300 LCD</ulink>
-	(Type-A to 5-pin).  See: <ulink url="http://www.mini-box.com/picoLCD-20x2-OEM"/>.
+	it comes equipped with an InfraRed receiver as well as key pad with 
-	It may also come pre-installed on systems like the Mini-Box M300: 
+	12 keys labeled <literal>Plus</literal>, <literal>Minus</literal>,
-	<ulink url="http://www.mini-box.com/Mini-Box-M300-LCD"/>.
+	<literal>F1</literal>, <literal>F2</literal>, <literal>F3</literal>,
+	<literal>F4</literal>, <literal>F5</literal>, <literal>Up</literal>,
+	<literal>Down</literal>, <literal>Left</literal>, <literal>Right</literal>,
+	and <literal>Enter</literal>.
 </para>
+
 <para>
-	Building and using the USB LCD with this driver requires the
+	Finally, the picoLCD 20x2 (OEM) supports 8 general purpose outputs
-	<filename>libusb</filename> library.
+	and 10 custom splash screens.
-	Most distributions do provide this library.
+	Although these features are not supported by this driver, the
-	The only kernel module required is the USB host controller driver 
+	splash screens can be changed using the <command>usblcd</command> 
-	(<filename>uhci_hcd</filename> on the M300) to fire up the USB bus
+	tool, that can be built from the Linux SDK available on the picoLCD web page.
+</para>
+</listitem>
+</varlistentry>
+</variablelist>
+
+</sect2>
+
+
+<sect2 id="picolcd-requirements">
+<title>Requirements</title>
+
+<para>
+	The driver is based on the
+	<ulink url="http://libusb.sourceforge.net/"><filename>libusb</filename></ulink>
+	USB library, which should make it work with Linux, the different BSB variants
+	as well as Darwin/MacOS X.
+
+<note>
+	<para>
+		When using a <filename>libusb</filename> based driver like
+		<code>picolcd</code>, <application>LCDd</application>
+		needs to be started as root.
+	</para>
+</note>
+</para>
+
+<para>
+	On Linux, the only kernel module required is the USB host controller
+	driver 	(<filename>uhci_hcd</filename> on the M300) to fire up the USB bus
 	to which the LCD is attached.
+	For other operating systems, analogous requirements apply.
 </para>
+
 <para>
-	Lastly, the <filename>usbfs</filename> file system must be mounted on
+	Lastly, for <filename>libusb</filename> to work correctly,
+	the <filename>usbfs</filename> file system must be mounted on
 	<filename>/proc/bus/usb</filename>, e.g. using the command
-	<code>mount -t usbfs usbfs /proc/bus/usb</code> or by your disptributions
+	<code>mount -t usbfs usbfs /proc/bus/usb</code> or by your system's
 	default configuration.
 </para>
-<para>
-	You can also change the USB LCD's initial display (what it shows when you
-	initialize the device) by using the <command>usblcd</command> binary's
-	<command>splash</command> command.
-</para>
-<para>
-	To use the picoLCD with LCDproc, add <literal>picolcd</literal>
-	to the LCDproc <code>./configure</code> <code>--drivers</code> option 
-	(or use <literal>all</literal>), set the <code>Driver=</code> to
-	<literal>picolcd</literal> in the <filename>LCDd.conf</filename> config
-	file, and then start <application>LCDd</application> either directly or using startup scripts.
-	The driver has sane default options and the options that are user configurable
-	are limited to mostly simple items, like contrast and which keys to light up.
-</para>
 </sect2>
 
+
+
 <sect2 id="picolcd-config">
 <title>Configuration in LCDd.conf</title>
 
@@ -58,6 +110,37 @@
 <title>[picolcd]</title>
 
 <variablelist>
+<!--
+<varlistentry>
+  <term>
+    <command>Backlight</command> =
+    <arg choice="plain">
+    <group choice="req">
+      <arg choice="plain"><literal><emphasis>yes</emphasis></literal></arg>
+      <arg choice="plain"><literal>no</literal></arg>
+    </group>
+    </arg>
+  </term>
+  <listitem><para>
+    Turn on backlight.
+  </para></listitem>
+</varlistentry>
+-->
+
+<varlistentry>
+  <term>
+    <command>Brightness</command> =
+    <arg choice="plain"><replaceable>BRIGHTNESS</replaceable></arg>
+  </term>
+  <listitem>
+  <para>
+    Set the initial brightness.
+    Legal values are: <literal>0</literal> - <literal>1000</literal>.
+    If not given, it defaults to <literal><emphasis>1000</emphasis></literal>.
+  </para>
+  </listitem>
+</varlistentry>
+
 <varlistentry>
   <term>
     <command>Contrast</command> =
@@ -66,9 +149,10 @@
   <listitem><para>
     Contrast: <literal>0</literal>-<literal>1000</literal>.
     <literal>0</literal> is visible, and <literal>1000</literal> is not on my M300.
-    Default is <literal>0</literal>.			
+    Default is <literal><emphasis>1000</emphasis></literal>.			
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>KeyLights</command> =
@@ -83,6 +167,7 @@
     Allow key LEDs to be turned on or off. Default is <literal>yes</literal>.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>Key0Light</command> =
@@ -98,6 +183,7 @@
     setting this value to <literal>no</literal>.  Default is <literal>yes</literal>.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>Key1Light</command> =
@@ -113,6 +199,7 @@
     to <literal>no</literal>.  Default is <literal>yes</literal>.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>Key2Light</command> =
@@ -128,6 +215,7 @@
     to <literal>no</literal>.  Default is <literal>yes</literal>.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>Key3Light</command> =
@@ -143,6 +231,7 @@
     to <literal>no</literal>.  Default is <literal>yes</literal>.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>Key4Light</command> =
@@ -158,6 +247,7 @@
     to <literal>no</literal>.  Default is <literal>yes</literal>.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>Key5Light</command> =
@@ -173,6 +263,7 @@
     to <literal>no</literal>.  Default is <literal>yes</literal>.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
     <command>KeyTimeout</command> =
@@ -186,9 +277,10 @@
     Larger values make key presses more reliable if somewhat slow to take effect.
   </para></listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
-    <command>LircHost=</command>
+    <command>LircHost</command> =
     <arg choice="plain"><replaceable>HOSTNAME_OR_IP-ADDRESS</replaceable></arg>
   </term>
   <listitem>
@@ -206,9 +298,10 @@
   </para>
   </listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
-    <command>LircPort=</command>
+    <command>LircPort</command> =
     <arg choice="plain"><replaceable>PORTNUM</replaceable></arg>
   </term>
   <listitem>
@@ -218,9 +311,10 @@
   </para>
   </listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
-    <command>LircSync=</command>
+    <command>LircSync</command> =
     <arg choice="plain"><replaceable>DURATION</replaceable></arg>
   </term>
   <listitem>
@@ -232,9 +326,10 @@
   </para>
   </listitem>
 </varlistentry>
+
 <varlistentry>
   <term>
-    <command>LircLength=</command>
+    <command>LircLength</command> =
     <arg choice="plain"><replaceable>DURATION</replaceable></arg>
   </term>
   <listitem>
@@ -251,27 +346,10 @@
 </sect3>
 </sect2>
 
+
 <sect2 id="picolcd-status">
 <title>picolcd driver status</title>
 
-	<para>
-		The lcdproc picolcd driver relies upon Mini-Box's usblcd library to setup the USB/HID 
-		interface.  Unfortunately the usblcd library is very thin and tends to put function 
-		results on stdout/stderr instead of in return values.  They (Mini-Box) really had 
-		scripting interfaces in mind.
-	</para>
-	<para>
-		The primary example of this is that I had to write a replacement (get_key_events) for 
-		usblcd's read_events so that I could get the key presses back to the calling function 
-		instead of on stdout.  usblcd also has a large number of function shells for which no 
-		code exists yet.  I am not an expert on HID and USB coding, but if and as time permits 
-		and hardware is available I will try and improve this drivers access to the hardware in 
-		order to get back data (get_contrast for example) and to potentially drive multiple 
-		pcioLCDs at once (or pick the one we want out of many).
-	</para>
-	<para>
-		To sum up, this driver right now stabs and hopes since response data is lacking.
-	</para>
 <para>
 	The hardware also reports key-up events.  Normally this would be of no issue (they are 
 	usually a 0 or 'no key') except that when keys are used in combonation, the key-up 
@@ -282,8 +360,10 @@
 	touchy and both combo key-down and key-up actions may be reported as mutiple events if 
 	the user is more than a tenth of a second (maybe less?) off in motions.
 </para>
+
 <sect3 id="picolcd-ir-status">
 <title>Infrared sensor status</title>
+
 <para>
 	LIRC expects sensor data that starts with a longish 'sync' space, denoting the start of 
 	a command; followed by the code data, a sequence of mark/space pairs; sometimes followed by 
@@ -295,19 +375,24 @@
 	<emphasis>still</emphasis> had trouble getting <code>irrecord</code> to work; you need at least 
 	to feed it a template configuration containing sync and gap data.  
 </para>
+
 <para>
 	This code could therefore use more testing, more research, and more work.
 </para>  
+
 </sect3>
 </sect2>
 
+
 <sect2 id="picolcd-copy">
 <title>Copyright</title>
+
 <para>
 	The lcdproc picolcd driver originally was written by Gatewood Green (woody@nitrosecurity.com) 
 	or (woody@linif.org) and paid for by NitroSecurity, Inc (nitrosecurity.com),
 	but has been extended with code from various contributors since then.
 </para>
+
 </sect2>
 
 </sect1>

server/drivers/picolcd.c

@@ -49,7 +49,7 @@
 #include <arpa/inet.h>
 
 #define DEFAULT_CONTRAST	1000 /* Full */
-#define DEFAULT_BRIGHTNESS 255  /* Full */
+#define DEFAULT_BRIGHTNESS	1000 /* Full */
 #define DEFAULT_BACKLIGHT	1    /* On */
 #define DEFAULT_KEYLIGHTS	1    /* On */
 #define DEFAULT_TIMEOUT		500  /* Half second */
@@ -98,13 +98,9 @@ typedef struct pd {
 
 /* Private function definitions */
 static void picolcd_send(usb_dev_handle *lcd, unsigned char *data, int size);
-/* Write function for 20x2 OEM displays */
 static void picolcd_20x2_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data);
-/* Write function for 20x4 desktop displays */
 static void picolcd_20x4_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data);
-/* Custom character define function for 20x2 OEM displays */
 static void picolcd_20x2_set_char (Driver *drvthis, int n, unsigned char *dat);
-/* Custom character define function for 20x4 desktop displays */
 static void picolcd_20x4_set_char (Driver *drvthis, int n, unsigned char *dat);
 static void get_key_event  (usb_dev_handle *lcd, lcd_packet *packet, int timeout);
 static void set_key_lights (usb_dev_handle *lcd, int keys[], int state);
@@ -125,6 +121,7 @@ picolcd_device picolcd_device_ids[] = {
 		.height       = 2,
 		.write        = picolcd_20x2_write,
 		.cchar        = picolcd_20x2_set_char,
+		/* all keymap labels must be shorter than KEYPAD_LABEL_MAX */
 		.keymap       = { NULL,  "Plus", "Minus", "F1", "F2", "F3", "F4", "F5",
 				  "Left", "Right", "Up", "Down", "Enter" },
 		.initseq      = {},
@@ -142,11 +139,12 @@ picolcd_device picolcd_device_ids[] = {
 		.height       = 4,
 		.write        = picolcd_20x4_write,
 		.cchar        = picolcd_20x4_set_char,
-                .keymap       = { NULL, "Back", "F1", "F2", "F3", "Home", "Down", 
+		/* all keymap labels must be shorter than KEYPAD_LABEL_MAX */
+		.keymap       = { NULL, "Escape", "F1", "F2", "F3", "Home", "Down", 
 				  "Enter", "Up", "", "", "", ""},
-                .initseq      = { 0x94, 0x00, 0x07, 0x00, 0x32, 0x30, 0x00,0x32, 0x30, 0x00, 0x32,
+		.initseq      = { 0x94, 0x00, 0x07, 0x00, 0x32, 0x30, 0x00, 0x32,
-                                  0x30, 0x00, 0x32, 0x38, 0x00, 0x32, 0x06, 0x00, 0x32, 0x0C, 0x07, 
+				  0x30, 0x00, 0x32, 0x30, 0x00, 0x32, 0x38, 0x00,
-                                  0xD0, 0x01},
+				  0x32, 0x06, 0x00, 0x32, 0x0C, 0x07, 0xD0, 0x01},
 	},
 	{}, /* End list */
 };
@@ -182,14 +180,15 @@ MODULE_EXPORT int  picoLCD_init(Driver *drvthis) {
 	p->lcd = NULL;
 	p->device = NULL;
 
-        for (id = 0; picolcd_device_ids[id].device_name; ++id) {
+	for (id = 0; picolcd_device_ids[id].device_name != NULL; ++id) {
-                report(RPT_INFO, "Looking for device %s ", picolcd_device_ids[id].device_name);
+		report(RPT_INFO, "%s: looking for device %s ",
+			drvthis->name, picolcd_device_ids[id].device_name);
 		for (bus = usb_get_busses(); bus != NULL; bus = bus->next) {
 			for (dev = bus->devices; dev != NULL; dev = dev->next) {
 				if ((dev->descriptor.idVendor == picolcd_device_ids[id].vendor_id) &&
 				     (dev->descriptor.idProduct == picolcd_device_ids[id].device_id)) {
-				
+					report(RPT_INFO, "%s: found %s on bus %s device %s",
-		                      report(RPT_INFO, "Found %s on bus %s device %s", picolcd_device_ids[id].device_name,
+						drvthis->name, picolcd_device_ids[id].device_name,
 						bus->dirname, dev->filename);
 
 					p->lcd = usb_open(dev);
@@ -203,25 +202,29 @@ MODULE_EXPORT int  picoLCD_init(Driver *drvthis) {
 
 	if (p->lcd != NULL) {
 		debug(RPT_DEBUG, "%s: opening device succeeded", drvthis->name);
+
 #ifdef LIBUSB_HAS_GET_DRIVER_NP
 		char driver[1024];
+
 		if (usb_get_driver_np(p->lcd, 0, driver, sizeof(driver)) == 0) {
-                        report(RPT_WARNING, "Interface 0 already claimed by '%s' detaching.", driver);
+			report(RPT_WARNING, "%s: interface 0 already claimed by '%s' - detaching",
+				drvthis->name, driver);
 #ifdef LIBUSB_HAS_DETACH_KERNEL_DRIVER_NP
 			if ((usb_detach_kernel_driver_np(p->lcd, 0) < 0))
-                                report(RPT_ERR, "%s: unable to detach %s driver", driver);
+				report(RPT_ERR, "%s: unable to detach %s driver", drvthis->name, driver);
 #endif
 		}
 #endif
 		if (usb_claim_interface(p->lcd, 0) < 0) {
-			report(RPT_ERR, "Cannot claim interface !", driver);
+			report(RPT_ERR, "%s: cannot claim interface!", drvthis->name);
 			usb_close(p->lcd);
 			return -1;
 		}
 		usleep(100);
 		if (usb_set_altinterface(p->lcd, 0) < 0)
 			report(RPT_WARNING, "%s: unable to set alternate configuration", drvthis->name);
-        } else {
+	}
+	else {
 		report(RPT_ERR, "%s: no device found", drvthis->name);
 		return -1;
 	}		
@@ -236,24 +239,19 @@ MODULE_EXPORT int  picoLCD_init(Driver *drvthis) {
 	p->cellheight = LCD_DEFAULT_CELLHEIGHT;
 	p->ccmode = standard;
 
-	for (x = 0; x < KEYPAD_LIGHTS; x++)
-		p->key_light[x] = 1; /* individual lights on */
-
 	p->contrast     = drvthis->config_get_int(drvthis->name, "Contrast",   0, DEFAULT_CONTRAST);
 	p->brightness   = drvthis->config_get_int(drvthis->name, "Brightness", 0, DEFAULT_BRIGHTNESS);
 	p->backlight    = drvthis->config_get_bool(drvthis->name, "BackLight",  0, DEFAULT_BACKLIGHT);
 	p->keylights    = drvthis->config_get_bool(drvthis->name, "KeyLights",  0, DEFAULT_KEYLIGHTS); /* key lights with LCD Backlight? */
 	p->key_timeout  = drvthis->config_get_int(drvthis->name, "KeyTimeout", 0, DEFAULT_TIMEOUT);
 
-	/* These allow individual lights to be disabled */
+	/* allow individual lights to be set */
-	p->key_light[0] = drvthis->config_get_bool(drvthis->name, "Key0Light",  0, 1); 
+	for (x = 0; x < KEYPAD_LIGHTS; x++) {
-	p->key_light[1] = drvthis->config_get_bool(drvthis->name, "Key1Light",  0, 1); 
+		char configkey[32];
-	p->key_light[2] = drvthis->config_get_bool(drvthis->name, "Key2Light",  0, 1); 
+
-	p->key_light[3] = drvthis->config_get_bool(drvthis->name, "Key3Light",  0, 1); 
+		sprintf(configkey, "Key%dLight", x);
-	p->key_light[4] = drvthis->config_get_bool(drvthis->name, "Key4Light",  0, 1); 
+		p->key_light[x] = drvthis->config_get_bool(drvthis->name, configkey, 0, 1);
-	p->key_light[5] = drvthis->config_get_bool(drvthis->name, "Key5Light",  0, 1); 
+	}	
-	p->key_light[6] = drvthis->config_get_bool(drvthis->name, "Key6Light",  0, 1); 
-	p->key_light[7] = drvthis->config_get_bool(drvthis->name, "Key7Light",  0, 1); 
 
 	p->framebuf = (unsigned char *) malloc(p->width * p->height + 1);
 	if (p->framebuf == NULL) {
@@ -413,7 +411,7 @@ MODULE_EXPORT void picoLCD_string(Driver *drvthis, int x, int y, unsigned char *
 
 	x--; y--; /* Convert 1-based to 0-based */
 	dest = p->framebuf + (y * p->width + x);
-	memcpy(dest, str, len * sizeof(char));
+	memcpy(dest, str, len * sizeof(unsigned char));
 
 	debug(RPT_DEBUG, "%s: string complete (%s)", drvthis->name, str);
 }
@@ -423,6 +421,7 @@ MODULE_EXPORT void picoLCD_chr(Driver *drvthis, int x, int y, unsigned char chr)
 {
 	PrivateData *p = drvthis->private_data;
 	unsigned char *dest;
+
 	debug(RPT_DEBUG, "%s: chr start (%c)", drvthis->name, chr);
 
 	if (y < 1 || y > p->height)
@@ -432,7 +431,8 @@ MODULE_EXPORT void picoLCD_chr(Driver *drvthis, int x, int y, unsigned char chr)
 
 	x--; y--; /* Convert 1-based to 0-based */
 	dest = p->framebuf + (y * p->width + x);
-	memcpy(dest, &chr, sizeof(char));
+	memcpy(dest, &chr, sizeof(unsigned char));
+
 	debug(RPT_DEBUG, "%s: chr complete (%c)", drvthis->name, chr);
 }
 
@@ -602,19 +602,26 @@ MODULE_EXPORT char *picoLCD_get_key(Driver *drvthis) {
 				debug(RPT_DEBUG, "%s: get_key got all clear", drvthis->name);
 				/* Got a <0, 0> key-up event after reading a valid key press event */
 				keys_read++; /* All clear */
-			} else if (! keydata->data[2] && ! two_keys) {
+			}
+			else if (! keydata->data[2] && ! two_keys) {
 				debug(RPT_DEBUG, "%s: get_key got one key", drvthis->name);
 				/* We got one key (but not after a two key event and before and all clear) */
 				keystr = p->device->keymap[keydata->data[1]];
-			} else {
+			}
+			else {
 				/* We got two keys */
+				static char keybuf[2 * KEYPAD_LABEL_MAX + 1];
+
 				debug(RPT_DEBUG, "%s: get_key got two keys", drvthis->name);
 				two_keys++;
-				sprintf(keystr, "%s+%s", p->device->keymap[keydata->data[1]], p->device->keymap[keydata->data[2]]);
+				sprintf(keybuf, "%s+%s", p->device->keymap[keydata->data[1]],
+							 p->device->keymap[keydata->data[2]]);
+				keystr = keybuf;
 			}
 
 			key_pass++; /* This hack allows us to deal with receiving left over <0,0> first */
-		} else if (p->IRenabled && keydata->type == IN_REPORT_IR_DATA) {
+		}
+		else if (p->IRenabled && keydata->type == IN_REPORT_IR_DATA) {
 			int cbres;
 
 			debug(RPT_NOTICE, "%s: get_key irdata, length=%d bytes",
@@ -627,7 +634,8 @@ MODULE_EXPORT char *picoLCD_get_key(Driver *drvthis) {
 			if (cbres < 0) {
 				report(RPT_ERR, "%s: could not transcode buffer, length=%d",
 					drvthis->name, keydata->data[1]);
-			} else {
+			}
+			else {
 				debug(RPT_NOTICE, "%s: sending packet to lirc, length=%d",
 					drvthis->name, cbres);
 			    int ret = sendto(p->lircsock, p->result, cbres, 0, 
@@ -635,14 +643,17 @@ MODULE_EXPORT char *picoLCD_get_key(Driver *drvthis) {
 			    if (ret == -1) {
 			    	report(RPT_ERR, "%s: error sending UDP packet, errno=%d",
 					drvthis->name, errno);
-			    } else if (ret != cbres) {
+			    }
+			    else if (ret != cbres) {
 			    	report(RPT_ERR, "%s: mismatch in number of bytes sent (%d!=%d)",
 					drvthis->name, cbres, ret);
-			    } else {
+			    }
+			    else {
 			    	debug(RPT_NOTICE, "%s: packet sent to lirc.", drvthis->name);
 	   	        }
 	   	    }
-		} else {
+		}
+		else {
 			debug(RPT_DEBUG, "%s: get_key got non-key data or timeout", drvthis->name);
 			/* We got IR or otherwise bad data */
 			free(keydata);
@@ -694,41 +705,43 @@ MODULE_EXPORT char *picoLCD_get_key(Driver *drvthis) {
 /* lcd_logical_driver User-defined character functions */
 
 /* lcd_logical_driver Hardware functions */
-/*MODULE_EXPORT int picoLCD_get_contrast(Driver *drvthis)
+MODULE_EXPORT int picoLCD_get_contrast(Driver *drvthis)
 {
 	PrivateData *p = drvthis->private_data;
 
-}*/
+	return p->contrast;
+}
 
 
-MODULE_EXPORT int  picoLCD_set_contrast(Driver *drvthis, int promille)
+MODULE_EXPORT void picoLCD_set_contrast(Driver *drvthis, int promille)
 {
 	PrivateData *p = drvthis->private_data;
 	int inv; /* The hardware seems to go dark on higher values, so we turn it around */
 	unsigned char packet[2] = { 0x92 }; /* set contrast id */
 	
-	
-	
 	if (promille <= 1000 && promille > 0) {
+		p->contrast = promille;
+
 		if (p->device->contrast_max == 1) 
 			packet[1] = 0x00; /* picoLCD20x4 permits contrast as 0/1 value */
 		else {
 			inv = 1000 - promille;
 			packet[1] =  inv / 1000 * p->device->contrast_max;
 		}
-	} else if (promille > 1000) {
+	}
+	else if (promille > 1000) {
+		p->contrast = 1000;
 		packet[1] = p->device->contrast_min;
-	} else if (promille <= 0) {
+	}
+	else if (promille <= 0) {
+		p->contrast = 0;
 		packet[1] = p->device->contrast_max;
-	} else {
-		return -1;
 	}
 	
 	picolcd_send(p->lcd, packet, 2);
-	
-	return 0;
 }
 
+
 /*MODULE_EXPORT int picoLCD_get_brightness(Driver *drvthis, int state)
 {
 	PrivateData *p = drvthis->private_data;
@@ -760,23 +773,18 @@ MODULE_EXPORT void picoLCD_backlight(Driver *drvthis, int state)
 	if (s > p->device->bklight_max)
 		s = p->device->bklight_max;
 	
-	if (state == 0) {
+	if (state == BACKLIGHT_OFF) {
 		//packet[1] = (unsigned char) p->device->bklight_min; 
 		packet[1] = 0xff;
 		picolcd_send(p->lcd, packet, 2);
 		set_key_lights(p->lcd, p->key_light, state);
-		return;
 	} 
-
+	else if (state == BACKLIGHT_ON) {
-	if (state == 1) {
 		packet[1] = (unsigned char) s;
 		picolcd_send(p->lcd, packet, 2);
 		if (p->keylights)
 			set_key_lights(p->lcd, p->key_light, state);
-		return;
 	}
-
-	return;
 }
 
 /*MODULE_EXPORT int  picoLCD_output(Driver *drvthis, int state) {
@@ -860,7 +868,8 @@ static int ir_transcode(Driver *drvthis, unsigned char *data, unsigned int cbdat
 			//scale: orig is usec, new is jiffy. E.g. 9055usec = 148 jiffy.
 			w = (w * 16384 /1000000) & 0xFFFF;
 			p->gap -= w;
-		} else {
+		}
+		else {
 			//Scale.
 			w = w * 16384 / 1000000;
 			//Set the space bit.
@@ -870,7 +879,8 @@ static int ir_transcode(Driver *drvthis, unsigned char *data, unsigned int cbdat
 		if (resptr + 2 < cbresult) {
 			result[resptr++] = w & 0xFF;
 			result[resptr++] = (w >> 8) & 0xFF;
-		} else {
+		}
+		else {
 			resptr = -1;
 			break;
 		}
@@ -893,17 +903,21 @@ static int ir_transcode(Driver *drvthis, unsigned char *data, unsigned int cbdat
 					p->gap |= 0x8000;
 					result[resptr++] = p->gap & 0xFF;
 					result[resptr++] = (p->gap >>8) & 0xFF;
-				} else {
+				}
+				else {
 					debug(RPT_DEBUG, "%s: terminal space=[%04x]; not appending gap (length=%d jiffies)", (unsigned int)(w & 0xffff), drvthis->name, p->gap);
 				}
-			} else {
+			}
+			else {
 				debug(RPT_DEBUG, "%s: not appending gap because it would be negative (length=%d jiffies)", drvthis->name, p->gap);
 			}
-		} else {
+		}
+		else {
 			//Result buffer would overflow
 			resptr = -1;
 		}
-	} else {
+	}
+	else {
 		debug(RPT_DEBUG, "%s: cIntervals=%d; not appending gap (length=%d jiffies)",
 			drvthis->name, cIntervals, p->gap);
 	}
@@ -919,18 +933,21 @@ static void picolcd_send(usb_dev_handle *lcd, unsigned char *data, int size)
 	usb_interrupt_write(lcd, USB_ENDPOINT_OUT + 1, (char *) data, size, 1000);
 }
 
+
+/* Write function for 20x4 desktop displays */
 static void picolcd_20x4_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data)
 {
-        unsigned char packet[64];
+	unsigned char packet[64] = { 0x95, 0x01, 0x00, 0x01 };
-        
 	unsigned char lineset[4][6] = {
 		{ 0x94, 0x00, 0x01, 0x00, 0x64, 0x80 },
 		{ 0x94, 0x00, 0x01, 0x00, 0x64, 0xC0 },
 		{ 0x94, 0x00, 0x01, 0x00, 0x64, 0x94 },
 		{ 0x94, 0x00, 0x01, 0x00, 0x64, 0xD4 }
 	};
+	int len = strlen((char *) data);
 
-        int len, i;
+	if (len > 20)
+		len = 20;
 
 	switch (row) {
 		case 0:  picolcd_send(lcd, lineset[0], 6);  break;
@@ -940,45 +957,34 @@ static void picolcd_20x4_write(usb_dev_handle *lcd, const int row, const int col
 		default: picolcd_send(lcd, lineset[0], 6);  break;
 	}
 
-        len = strlen((char *)data);
-        if (len > 20) len = 20;
-
-        packet[0] = 0x95;
-        packet[1] = 0x01;
-        packet[2] = 0x00;
-        packet[3] = 0x01;
 	packet[4] = len;
 
-        i = 5;
+	memcpy(packet + 5, data, len);
-        while (len--) {
-                packet[i++] = *data++;
-        }
 
-        picolcd_send(lcd, packet, i);
+	picolcd_send(lcd, packet, 5 + len);
 }
 
 
+/* Write function for 20x2 OEM displays */
 static void picolcd_20x2_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data)
 {
-	unsigned char packet[64];
+	unsigned char packet[64] = { 0x98 };
-        int len, i;
+	int len = strlen((char *) data);
 
-        len = strlen((char *)data);
+	if (len > 20)
-        if (len > 20) len = 20;
+		len = 20;
 
-	packet[0] = 0x98;
 	packet[1] = row;
 	packet[2] = col;
 	packet[3] = len;
 
-	i = 4;
+	memcpy(packet + 4, data, len);
-	while (len--) {
+
-		packet[i++] = *data++;
+	picolcd_send(lcd, packet, 4 + len);
 }
 
-	picolcd_send(lcd, packet, i);
-}
 
+/* Custom character define function for 20x2 OEM displays */
 static void picolcd_20x2_set_char(Driver *drvthis, int n, unsigned char *dat)
 {
 	PrivateData *p = drvthis->private_data;
@@ -998,20 +1004,19 @@ static void picolcd_20x2_set_char (Driver *drvthis, int n, unsigned char *dat)
 	}
 	
 	picolcd_send(p->lcd, packet, 10);
-	
-	return;
 }
 
 
+/* Custom character define function for 20x4 desktop displays */
 static void picolcd_20x4_set_char(Driver *drvthis, int n, unsigned char *dat)
 {
+	PrivateData *p = drvthis->private_data;
 	
 	if ((n < 0) || (n >= NUM_CCs))
 		return;
 	if (!dat)
 		return;
 	
-	PrivateData *p = drvthis->private_data;
 	unsigned char command[6] = { OUT_REPORT_CMD, 0x00, 0x01, 0x00, 0x64, 0x40+8*n }; /* 0x94 */
 	unsigned char data[13] = { OUT_REPORT_DATA, 0x01, 0x00, 0x01, 0x08,
 					dat[0], dat[1], dat[2], dat[3], 
@@ -1019,8 +1024,6 @@ static void picolcd_20x4_set_char (Driver *drvthis, int n, unsigned char *dat)
 	
 	picolcd_send(p->lcd, command, 6);
 	picolcd_send(p->lcd, data, 13);
-	
-	return;
 }
 
 
@@ -1065,10 +1068,11 @@ static void set_key_lights(usb_dev_handle *lcd, int keys[], int state)
 		/* Only LEDs we want on */
 		for (i = 0; i < KEYPAD_LIGHTS; i++) 
 			if(keys[i]) 
-				leds |= 1 << i;
+				leds |= (1 << i);
 			else 
 				leds &= ~ (1 << i);
-	} else {
+	}
+	else {
 		/* All LEDs off */
 		leds = 0;
 	}

server/drivers/picolcd.h

@@ -36,9 +36,7 @@
 /* 12 keys plus a 0 placeholder */
 #define KEYPAD_MAX		13
 #define KEYPAD_LIGHTS		8
-
+#define KEYPAD_LABEL_MAX	25
-#define picoLCD_VENDOR  0x04d8
-#define picoLCD_DEVICE  0x0002
 
 #define IN_REPORT_KEY_STATE	0x11
 #define IN_REPORT_IR_DATA	0x21
@@ -53,16 +51,12 @@
 #define DEFAULT_LENGTH_JIFFY	2048 
 
 
-typedef struct _lcd_packet lcd_packet;
+typedef struct _lcd_packet {
-
-struct _lcd_packet {
 	unsigned char data[255];
 	unsigned int type;
-};
+} lcd_packet;
 
-typedef struct _picolcd_device picolcd_device ;
+typedef struct _picolcd_device {
-
-struct _picolcd_device {
 	char *device_name;          /* Device name */
 	char *description;          /* Device description */
 	unsigned char initseq[PICOLCD_MAX_DATA_LEN];     /* init sequence */
@@ -79,7 +73,8 @@ struct _picolcd_device {
 	void (*write)(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data); 
 	/* Pointer to function that defines a custom character */
 	void (*cchar) (Driver *drvthis, int n, unsigned char *dat);
-};
+} picolcd_device;
+
 
 MODULE_EXPORT int  picoLCD_init(Driver *drvthis);
 MODULE_EXPORT void picoLCD_close(Driver *drvthis);
@@ -98,15 +93,13 @@ MODULE_EXPORT void picoLCD_num(Driver *drvthis, int x, int num);
 MODULE_EXPORT int  picoLCD_icon(Driver *drvthis, int x, int y, int icon);
 MODULE_EXPORT void picoLCD_cursor(Driver *drvthis, int x, int y, int type);
 
-MODULE_EXPORT int  picoLCD_set_contrast(Driver *drvthis, int promille);
+MODULE_EXPORT int  picoLCD_get_contrast(Driver *drvthis);
+MODULE_EXPORT void picoLCD_set_contrast(Driver *drvthis, int promille);
+//MODULE_EXPORT int  picoLCD_get_brightness(Driver *drvthis, int state);
 MODULE_EXPORT void picoLCD_set_brightness(Driver *drvthis, int state, int promille);
 MODULE_EXPORT void picoLCD_backlight(Driver *drvthis, int promille);
+//MODULE_EXPORT void picoLCD_output(Driver *drvthis, int state);
+
 MODULE_EXPORT char *picoLCD_get_info(Driver *drvthis);
 
-//MODULE_EXPORT void picoLCD_output(Driver *drvthis, int state);
-//MODULE_EXPORT int  picoLCD_get_contrast(Driver *drvthis);
-//MODULE_EXPORT int  picoLCD_get_brightness (Driver *drvthis, int state);
-
-
-
 #endif