picoLCD improved key input

Martin Tharby Jones <[email protected]>
Newsgroups gmane.comp.sysutils.lcdproc
Message-ID <[email protected]>
Hello,

I have attached a patch which updates the picoLCD driver to use 
libusb-1.0. The advantage of libusb-1.0 is the asynchronous USB access 
it provides, this removes the need for the key time-out used in 
picoLCD_get_key() which disrupts the main loop timing. Try a screen that 
updates frequently such as the lcdproc CPU screen to see the improvement.

I have changed the get key algorithm to implement key auto repeat and 
report key events when the key is pressed rather than when it is 
released, this is consistent with most key input so what a user expects.

All the changes are conditionally compiled and can be disabled with the 
configuration option --disable-libusb-1-0. You will of course need to 
have libusb-1.0 installed to use the new code.

I have tested my code on my 20x2 display, the only hardware I have. 
Could someone with a 20x4 display confirm that that is OK.

The change to asynchronous USB input also affects the IR data input, I 
haven't got a working LIRC configuration yet but my problems are the 
same before and after my change. Could anyone with a working LIRC see if 
it still works, it should be better, the key reporting is more reliable.

I have put FIXME comments in picoLCD_init(), picoLCD_close() and 
do_mainloop() where further work is required by someone who has a good 
range of hardware to test on. I do not want to make significant changes 
that I am unable to test. The code works as it is but the libusb 
initinalization and deinitialization needs to be somewhere common to 
enable support of multiple usb devices. The patch in the main loop needs 
integrating with the other socket handling.

I hope this is useful to other picoLCD users.

Martin

_______________________________________________
LCDproc mailing list
[email protected]
http://lists.omnipotent.net/mailman/listinfo/lcdproc
libusb-1.0.patch (text/x-patch, 36.5 KB)
Index: configure.in
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/configure.in,v
retrieving revision 1.87
diff -u -r1.87 configure.in
--- configure.in	3 Apr 2011 19:25:08 -0000	1.87
+++ configure.in	7 Aug 2011 16:16:17 -0000
@@ -271,6 +271,29 @@
 AC_SUBST(LIBUSB_CFLAGS)
 
 dnl ######################################################################
+dnl libusb-1.0 support
+dnl ######################################################################
+AC_MSG_CHECKING([if libusb-1-0 support has been enabled]);
+AC_ARG_ENABLE(libusb_1_0,
+	[AS_HELP_STRING([--disable-libusb-1-0],[disable USB support using libusb-1.0])],
+	[ if test "$enableval" != "no"; then
+		enable_libusb_1_0="yes"
+	fi ],
+	[ enable_libusb_1_0=yes ]
+)
+AC_MSG_RESULT($enable_libusb_1_0)
+
+if test "$enable_libusb_1_0" = "yes"; then
+	ifdef([PKG_CHECK_MODULES],
+		[PKG_CHECK_MODULES(LIBUSB_1_0, libusb-1.0 >= 1.0,
+			[AC_DEFINE(HAVE_LIBUSB_1_0, [1], [Define to 1 if you have libusb-1.0])],
+			[ enable_libusb_1_0=no ])],
+		[AC_MSG_WARN([pkg-config not (fully) installed; drivers requiring libusb-1.0 may not be built])])
+fi
+AC_SUBST(LIBUSB_1_0_LIBS)
+AC_SUBST(LIBUSB_1_0_CFLAGS)
+
+dnl ######################################################################
 dnl libftdi support
 dnl ######################################################################
 AC_MSG_CHECKING([if libftdi support has been enabled]);
Index: acinclude.m4
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/acinclude.m4,v
retrieving revision 1.115
diff -u -r1.115 acinclude.m4
--- acinclude.m4	12 Mar 2011 22:36:48 -0000	1.115
+++ acinclude.m4	7 Aug 2011 16:16:17 -0000
@@ -366,6 +366,11 @@
 			if test "$enable_libusb" = yes ; then
 				DRIVERS="$DRIVERS picolcd${SO}"
 				actdrivers=["$actdrivers picolcd"]
+				if test "$enable_libusb_1_0" = yes ; then
+					AC_MSG_RESULT([The picolcd driver is using the libusb-1.0 library.])
+				else
+					AC_MSG_WARN([The picolcd driver is using the libusb-0.1 library.])
+				fi
 			else
 				AC_MSG_WARN([The picolcd driver needs the libusb library.])
 			fi
Index: LCDd.conf
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/LCDd.conf,v
retrieving revision 1.148
diff -u -r1.148 LCDd.conf
--- LCDd.conf	3 Apr 2011 19:25:08 -0000	1.148
+++ LCDd.conf	7 Aug 2011 16:16:16 -0000
@@ -868,6 +868,10 @@
 ## Mini-box.com picoLCD (usblcd) driver ##
 [picolcd]
 
+# KeyTimeout is only used if the picoLCD driver is built with libusb-0.1, when
+# built with libusb-1.0 key and IR data is input asynchronously so there is no
+# need to wait for the USB data thus allowing LCDd to process other inputs at
+# the correct rate. 
 # KeyTimeout is the time in ms that LCDd spends waiting for a key press before
 # cycling through other duties.  Higher values make LCDd use less CPU time and
 # make key presses more detectable.  Lower values make LCDd more responsive
@@ -875,6 +879,19 @@
 # and a balanced value. [default: 500; legal: 0 - 1000]
 KeyTimeout=500
 
+# Key auto repeat is only available if the picoLCD driver is built with libusb-1.0, when
+# built with libusb-0.1 key input blocks all other processing until the key is released. 
+# Time values for key auto repeat are in milliseconds.
+# Use the maximum value 67108863 (0x3FFFFFF) which is over 18 hours to disable auto repeat.
+#
+# Key auto repeat delay (time from first key report to first repeat)
+# [default: 300; legal: 0 - 67108863]
+KeyRepeatDelay=300
+
+# Key auto repeat interval (time between repeat reports)
+# [default: 200; legal: 0 - 67108863]
+KeyRepeatInterval=200
+
 # Sets the initial state of the backlight upon start-up.
 # [default: on; legal: on, off]
 #Backlight=on
Index: server/Makefile.am
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/server/Makefile.am,v
retrieving revision 1.18
diff -u -r1.18 Makefile.am
--- server/Makefile.am	14 Apr 2007 07:54:27 -0000	1.18
+++ server/Makefile.am	7 Aug 2011 16:16:18 -0000
@@ -6,12 +6,12 @@
 
 LCDd_SOURCES= client.c client.h clients.c clients.h input.c input.h main.c main.h menuitem.c menuitem.h menu.c menu.h menuscreens.c menuscreens.h parse.c parse.h render.c render.h screen.c screen.h screenlist.c screenlist.h serverscreens.c serverscreens.h sock.c sock.h widget.c widget.h drivers.c drivers.h driver.c driver.h
 
-LDADD = ../shared/libLCDstuff.a commands/libLCDcommands.a
+LDADD = ../shared/libLCDstuff.a commands/libLCDcommands.a @LIBUSB_1_0_LIBS@
 
 if !DARWIN
 AM_LDFLAGS = -rdynamic -uget_args
 endif
 
-AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared -DSYSCONFDIR=\"$(sysconfdir)\"
+AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/shared @LIBUSB_1_0_CFLAGS@ -DSYSCONFDIR=\"$(sysconfdir)\"
 
 ## EOF
Index: server/main.c
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/server/main.c,v
retrieving revision 1.96
diff -u -r1.96 main.c
--- server/main.c	19 Jan 2011 19:08:37 -0000	1.96
+++ server/main.c	7 Aug 2011 16:16:18 -0000
@@ -51,6 +51,12 @@
 #endif
 /* TODO: fill in what to include otherwise */
 
+#ifdef HAVE_LIBUSB_1_0
+/* N.B. include for libusb-1.0 */
+#  include <libusb.h>
+#endif
+
+
 #include "shared/report.h"
 #include "shared/defines.h"
 
@@ -781,7 +787,7 @@
 			t_diff *= 1e6;
 			t_diff += t.tv_usec - last_t.tv_usec;
 		}
-                process_lag += t_diff;
+		process_lag += t_diff;
 		if (process_lag > 0) {
 			/* Time for a processing stroke */
 			sock_poll_clients();		/* poll clients for input*/
@@ -793,6 +799,23 @@
 			/* Note : this does not make a fixed frequency */
 		}
 
+#ifdef HAVE_LIBUSB_1_0
+		/* FIXME: do_mainloop using libusb-1.0
+		 * This is just a quick hack to try the asynchronous interface provided
+		 * by libusb-1.0. It needs to be restructured on the lines suggested in
+		 * the advanced option described in the libusb-1.0 documentation:
+		 * http://libusb.sourceforge.net/api-1.0/group__poll.html#_details
+		 */
+		{
+			int usb_result;
+			struct timeval t;
+
+			t.tv_sec = 0;
+			t.tv_usec = 0;
+
+			usb_result = libusb_handle_events_timeout(NULL, &t);
+		}
+#endif
 		render_lag += t_diff;
 		if (render_lag > 0) {
 			/* Time for a rendering stroke */
Index: docs/lcdproc-user/drivers/picolcd.docbook
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/docs/lcdproc-user/drivers/picolcd.docbook,v
retrieving revision 1.13
diff -u -r1.13 picolcd.docbook
--- docs/lcdproc-user/drivers/picolcd.docbook	23 Jan 2011 14:07:41 -0000	1.13
+++ docs/lcdproc-user/drivers/picolcd.docbook	7 Aug 2011 16:16:18 -0000
@@ -56,7 +56,10 @@
 <para>
 	Finally, the picoLCD 20x2 (OEM) supports 8 general purpose outputs
 	and 10 custom splash screens.
-	Although these features are not supported by this driver, the
+	When the keypad is connected the outputs control the key LEDs. The
+	output command and KeyLight settings below can be used to control the
+	outputs.
+	Although splash screens are not supported by this driver, the
 	splash screens can be changed using the <command>usblcd</command>
 	tool, that can be built from the Linux SDK available on the picoLCD web page.
 </para>
@@ -243,14 +246,62 @@
     <property>KeyTimeout</property> =
     <parameter><replaceable>DURATION</replaceable></parameter>
   </term>
-  <listitem><para>
+  <listitem>
+  <para>
+    KeyTimeout is only used if the picoLCD driver is built with libusb-0.1, when
+    built with libusb-1.0 key and IR data is input asynchronously so there is no
+    need to wait for the USB data thus allowing LCDd to process other inputs at
+    the correct rate. 
+  </para>
+  <para>
     This value controls how long <application>LCDd</application> waits for a key press when
     get_key() is called.  The value represents milliseconds and the default is <literal>500</literal>
     or .5 seconds.  Lowering this value will make LCDd more responsive but also causes LCDd to use
     more CPU time and, as the timeout grows shorter, key presses become harder to detect.
     Large values make key presses more reliable but may slow down LCDd. Values
     in the range <literal>0</literal>-<literal>1000</literal> (1s) are allowed.
-  </para></listitem>
+  </para>
+  </listitem>
+</varlistentry>
+
+<varlistentry>
+  <term>
+    <property>KeyRepeatDelay</property> =
+    <parameter><replaceable>DURATION</replaceable></parameter>
+  </term>
+  <listitem>
+  <para>
+    KeyRepeatDelay is only used if the picoLCD driver is built with libusb-1.0, when
+    built with libusb-0.1 key input blocks all other processing until the key is released. 
+  </para>
+  <para>
+    This value controls how long <application>LCDd</application> waits from when a key is
+	pressed and reported before generating the first repeat.  The value represents
+	milliseconds and the default is <literal>300</literal> (0.3 second). Use the maximum
+	value 67108863 (0x3FFFFFF) which is over 18 hours to disable auto repeat.
+	Positive integer values are allowed.
+  </para>
+  </listitem>
+</varlistentry>
+
+<varlistentry>
+  <term>
+    <property>KeyRepeatInterval</property> =
+    <parameter><replaceable>DURATION</replaceable></parameter>
+  </term>
+  <listitem>
+  <para>
+    KeyRepeatInterval is only used if the picoLCD driver is built with libusb-1.0, when
+    built with libusb-0.1 key input blocks all other processing until the key is released. 
+  </para>
+  <para>
+    This value controls how long <application>LCDd</application> waits between key reports
+	after generating the first repeat.  The value represents milliseconds and the default
+	is <literal>200</literal> (0.2 second). Use the maximum value 67108863 (0x3FFFFFF)
+	which is over 18 hours to disable auto repeat.
+	Positive integer values are allowed.
+  </para>
+  </listitem>
 </varlistentry>
 
 <varlistentry>
@@ -320,6 +371,10 @@
 	deal with this in a sane way and toss out all key-up events for now.  The hardware is
 	touchy and both combo key-down and key-up actions may be reported as multiple events if
 	the user is more than a tenth of a second (maybe less?) off in motions.
+	The hardware is <emphasis>not</emphasis> "touchy" it reports what it receives. Two key
+	presses or releases may appear simultaneous to a human but they are always some time apart.
+	The hardware probably samples the keys for every USB transfer cycle; that is every 10ms,
+	significantly faster than the typical human response time of a few hundred milliseconds!
 </para>
 
 <sect3 id="picolcd-ir-status">
Index: server/drivers/picolcd.c
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/server/drivers/picolcd.c,v
retrieving revision 1.23
diff -u -r1.23 picolcd.c
--- server/drivers/picolcd.c	23 Jan 2011 13:28:16 -0000	1.23
+++ server/drivers/picolcd.c	7 Aug 2011 16:16:20 -0000
@@ -70,7 +70,6 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
-#include <usb.h>
 
 /* LCDproc includes */
 #include "lcd.h"
@@ -78,17 +77,24 @@
 #include "adv_bignum.h"
 #include "report.h"
 #include "picolcd.h"
+#include "../main.h"
 
 #define NUM_CCs         8 /* max. number of custom characters */
 
-/* PrivateData struct */
+#ifdef HAVE_LIBUSB_1_0
+typedef struct  {		/* for the key press data */
+	unsigned char high_key;
+	unsigned char low_key;
+} keys;
+#endif
+
+/** Private data for the picoLCD driver */
 typedef struct picolcd_private_data {
-	usb_dev_handle *lcd;
+	USB_DEVICE_HANDLE *lcd;
 	int  width;
 	int  height;
 	int  cellwidth;
 	int  cellheight;
-	int  key_timeout;
 	int  contrast;
 	int  backlight;
 	int  brightness;
@@ -111,18 +117,42 @@
 	struct timeval lastmsg;
 	int lastval;
 	int flush_threshold;
+#ifdef HAVE_LIBUSB_1_0
+	/* N.B. Asynchronous USB transfer data (libusb-1.0) */
+	/** data buffer for the asynchronous USB transfer */
+	unsigned char input_buffer[PICOLCD_MAX_DATA_LEN];
+	/** structure for the details of the asynchronous USB transfer */
+	struct libusb_transfer *input_transfer;
+	int libusb_status;
+#define KEY_BUFFER_SIZE 8
+	/** buffer for the key press data */
+	keys key_buffer[KEY_BUFFER_SIZE];
+	int key_read_index;
+	int key_write_index;
+	keys reported_keys;
+	int key_repeat_delay;
+	int key_repeat_interval;
+	int key_delay_count;
+	int key_interval_count;
+#else
+	int  key_timeout;
+#endif
 } PrivateData;
 
 /* Private function definitions */
-static void picolcd_send(usb_dev_handle *lcd, unsigned char *data, int size);
-static void picolcd_20x2_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data);
-static void picolcd_20x4_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data);
-static void picolcd_20x2_set_char(Driver *drvthis, int n, unsigned char *dat);
-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);
+static void picolcd_send(USB_DEVICE_HANDLE *lcd, unsigned char *data, int size);
+static void picolcd_20x2_write(USB_DEVICE_HANDLE *lcd, const int row, const int col, const unsigned char *data);
+static void picolcd_20x4_write(USB_DEVICE_HANDLE *lcd, const int row, const int col, const unsigned char *data);
+static void picolcd_20x2_set_char (Driver *drvthis, int n, unsigned char *dat);
+static void picolcd_20x4_set_char (Driver *drvthis, int n, unsigned char *dat);
+static void get_key_event  (USB_DEVICE_HANDLE *lcd, lcd_packet *packet, int timeout);
+static void set_key_lights (USB_DEVICE_HANDLE *lcd, int keys[], int state);
 static void picolcd_lircsend(Driver *drvthis);
 static void ir_transcode(Driver *drvthis, unsigned char *data, unsigned int cbdata);
+#ifdef HAVE_LIBUSB_1_0
+static void key_buffer_put(Driver *drvthis, unsigned char high_key, unsigned char low_key);
+static void usb_cb_input(struct libusb_transfer *transfer);
+#endif
 
 /**
  * Table describing various features of known picoLCD devices and pointers
@@ -188,8 +218,12 @@
 {
 	PrivateData *p;
 	int x;
+#ifdef HAVE_LIBUSB_1_0
+	int error = 0;
+#else
 	struct usb_bus *bus;
 	struct usb_device *dev;
+#endif
 	const char *lirchost;
 	int lircport;
 	int id;
@@ -202,14 +236,109 @@
 	if (drvthis->store_private_ptr(drvthis, p))
 		return -1;
 
+	p->lcd = NULL;
+	p->device = NULL;
+
+#ifdef HAVE_LIBUSB_1_0
+	/* FIXME: Initialise libusb-1.0
+	 * libusb should be initialised once for all usb drivers
+	 * so to support multiple USB drivers this should be
+	 * somewhere common, before init_drivers() perhaps.
+	 */
+	error = libusb_init(NULL);
+	if (error) {
+		report(RPT_ERR, "libusb_init error %d", error);
+		return -1;
+	}
+	else {
+		/* Set libusb message verbosity.
+		 * 0: no messages ever printed by the library (default)
+		 * 1: error messages are printed to stderr
+		 * 2: warning and error messages are printed to stderr
+		 * 3: informational messages are printed to stdout,
+		 *    warning and error messages are printed to stderr
+		 */
+		int level = 3;
+		libusb_set_debug(NULL, level);
+	}
+
+	/* N.B. init using libusb-1.0 */
+	p->libusb_status = LIBUSB_SUCCESS;
+	p->input_transfer = NULL;
+	p->key_read_index = 0;
+	p->key_write_index = 0;
+	/*
+	 * Try to find picolcd device the new way, this opens the first picoLCD
+	 * found if you need to handle more than one picoLCD you'll need
+	 * something more sophisticated.
+	 */
+	for (id = 0; picolcd_device_ids[id].device_name != NULL; ++id)
+	{
+		report(RPT_INFO, "%s: looking for device %s ",
+			drvthis->name, picolcd_device_ids[id].device_name);
+		if ( (p->lcd = libusb_open_device_with_vid_pid(NULL,
+							picolcd_device_ids[id].vendor_id,
+							picolcd_device_ids[id].device_id)) ) {
+			p->device = &picolcd_device_ids[id];
+			break;
+		}
+	}
+	if (p->lcd != NULL) {
+		debug(RPT_DEBUG, "%s: opening device %s succeeded"
+				, drvthis->name
+				, picolcd_device_ids[id].device_name);
+	}
+	else {
+		report(RPT_ERR, "%s: no device found", drvthis->name);
+		return -1;
+	}
+	if ( libusb_kernel_driver_active(p->lcd, 0)	) {
+		debug(RPT_DEBUG, "%s: libusb_kernel_driver_active returned true", drvthis->name);
+		error = libusb_detach_kernel_driver(p->lcd, 0);
+		if (error) {
+			report(RPT_ERR, "%s: libusb_detach_kernel_driver error %d", drvthis->name, error);
+			return -1;
+		}
+	}
+	else {
+		debug(RPT_DEBUG, "%s: libusb_kernel_driver_active returned false", drvthis->name);
+	}
+	error = libusb_claim_interface(p->lcd, 0);
+	if (error) {
+		report(RPT_ERR, "%s: libusb_claim_interface error %d", drvthis->name, error);
+		return -1;
+	}
+	usleep(100);
+	/* FIXME Is this the libusb-1.0 equivalent to
+	 *	if (usb_set_altinterface(p->lcd, 0) < 0)
+	 *		report(RPT_WARNING, "%s: unable to set alternate configuration", drvthis->name);
+	 * I always get error -5 (LIBUSB_ERROR_NOT_FOUND the requested alternate setting does not exist)
+	 * Is this needed? Has it ever worked?
+	 * lsusb reports one configuration with one interface and no alternate settings.
+	 */
+	error = libusb_set_interface_alt_setting(p->lcd, 1, 0);
+	if (error) {
+		report(RPT_WARNING, "%s: libusb_set_interface_alt_setting error %d", drvthis->name, error);
+	}
+	p->input_transfer = libusb_alloc_transfer(0);
+	if (!p->input_transfer) {
+		report(RPT_ERR, "%s: libusb_alloc_transfer failed", drvthis->name);
+		return -1;
+	}
+	libusb_fill_interrupt_transfer(p->input_transfer, p->lcd, LIBUSB_ENDPOINT_IN + 1, p->input_buffer,
+		sizeof(p->input_buffer), usb_cb_input, (void *)drvthis, 0);
+	error = libusb_submit_transfer(p->input_transfer);
+	if (error) {
+		report(RPT_ERR, "%s: libusb_submit_transfer error %d", drvthis->name, error);
+		libusb_free_transfer(p->input_transfer);
+		return -1;
+	}
+#else
 	/* Try to find picolcd device */
 	usb_init();
 	usb_find_busses();
 	usb_find_devices();
 
-	p->lcd = NULL;
-	p->device = NULL;
-
 	for (id = 0; picolcd_device_ids[id].device_name != NULL; ++id) {
 		report(RPT_INFO, "%s: looking for device %s ",
 			drvthis->name, picolcd_device_ids[id].device_name);
@@ -258,8 +387,9 @@
 		report(RPT_ERR, "%s: no device found", drvthis->name);
 		return -1;
 	}
+#endif
 
-	/* if the device has a init sequence sent it to device */
+	/* if the device has a init sequence send it to device */
 	picolcd_send(p->lcd, p->device->initseq, PICOLCD_MAX_DATA_LEN);
 
 	p->width  = p->device->width;
@@ -308,6 +438,46 @@
 		p->key_light[x] = drvthis->config_get_bool(drvthis->name, configkey, 0, 1);
 	}
 
+#ifdef HAVE_LIBUSB_1_0
+	/* Time values for key auto repeat are in milliseconds in the configuration file
+	 * they are converted here to counts for use in picoLCD_get_key.
+	 */
+	/* Get key auto repeat delay */
+	tmp = drvthis->config_get_int(drvthis->name, "KeyRepeatDelay", 0, DEFAULT_REPEAT_DELAY);
+	if (tmp < 0) {
+		report(RPT_WARNING, "%s: KeyRepeatDelay must be positive; using default %d",
+			drvthis->name, DEFAULT_REPEAT_DELAY);
+		tmp = DEFAULT_REPEAT_DELAY;
+	}
+	if (INT_MAX / PROCESS_FREQ < tmp) {
+		report(RPT_WARNING, "%s: KeyRepeatDelay too large; using maximum %d",
+			drvthis->name, INT_MAX / PROCESS_FREQ);
+		tmp = INT_MAX / PROCESS_FREQ;
+	}
+	p->key_repeat_delay = (tmp * PROCESS_FREQ + 500) / 1000;
+
+	/* Get key auto repeat interval */
+	tmp = drvthis->config_get_int(drvthis->name, "KeyRepeatInterval", 0, DEFAULT_REPEAT_INTERVAL);
+	if (tmp < 0) {
+		report(RPT_WARNING, "%s: KeyRepeatInterval must be positive; using default %d",
+			drvthis->name, DEFAULT_REPEAT_INTERVAL);
+		tmp = DEFAULT_REPEAT_INTERVAL;
+	}
+	if (INT_MAX / PROCESS_FREQ < tmp) {
+		report(RPT_WARNING, "%s: KeyRepeatInterval too large; using maximum %d",
+			drvthis->name, INT_MAX / PROCESS_FREQ);
+		tmp = INT_MAX / PROCESS_FREQ;
+	}
+	p->key_repeat_interval = (tmp * PROCESS_FREQ + 500) / 1000;
+	if (0 == p->key_repeat_interval) p->key_repeat_interval = 1;
+
+	report(RPT_WARNING, "%s: Key repeat counts: delay %d, interval %d",
+			drvthis->name, p->key_repeat_delay, p->key_repeat_interval);
+
+	p->reported_keys.high_key = 0;
+	p->reported_keys.low_key = 0;
+
+#else
 	/* Get Timeout for USB read of key presses */
 	tmp = drvthis->config_get_int(drvthis->name, "KeyTimeout", 0, DEFAULT_TIMEOUT);
 	if ((tmp < 0) || (tmp > 1000)) {
@@ -316,6 +486,7 @@
 		tmp = DEFAULT_TIMEOUT;
 	}
 	p->key_timeout = tmp;
+#endif
 
 	/* Allocate and clear frame buffers */
 	p->framebuf = (unsigned char *) malloc(p->width * p->height + 1);
@@ -415,8 +586,30 @@
 {
 	PrivateData *p = drvthis->private_data;
 
+#ifdef HAVE_LIBUSB_1_0
+	/* N.B. close using libusb-1.0 */
+	int error;
+
+	error = libusb_release_interface(p->lcd, 0);
+	if (error) {
+		report(RPT_ERR, "%s: usb_release_interface error %d", drvthis->name, error);
+	}
+	error = libusb_attach_kernel_driver(p->lcd, 0);
+	if (error) {
+		report(RPT_ERR, "%s: libusb_attach_kernel_driver error %d", drvthis->name, error);
+	}
+	libusb_close(p->lcd);
+	/* FIXME: Deinitialize libusb-1.0
+	 * Should be called after closing all open devices and before the
+	 * application terminates so to support multiple USB drivers this
+	 * should be somewhere common, after drivers_unload_all() perhaps.
+	 */
+	libusb_exit(NULL);
+	sleep(1);
+#else
 	usb_release_interface(p->lcd, 0);
 	usb_close(p->lcd);
+#endif
 
 	debug(RPT_DEBUG, "%s: close complete", drvthis->name);
 }
@@ -672,7 +865,7 @@
 
 		for (i = 1; i <= p->cellwidth; i++) {
 			/* fill pixel columns from left to right. */
-			memset(hBar, 0xFF & ~((1 << (p->cellwidth - i)) - 1), sizeof(hBar));
+			memset(hBar, 0x1F & ~((1 << (p->cellwidth - i)) - 1), sizeof(hBar));
 			picoLCD_set_char(drvthis, i, hBar);
 		}
 	}
@@ -724,17 +917,19 @@
  */
 MODULE_EXPORT int picoLCD_icon (Driver *drvthis, int x, int y, int icon)
 {
+	PrivateData *p = drvthis->private_data;
+
 	static unsigned char heart_open[] =
-	{
-		b_______,
-		b___X_X_,
-		b__X_X_X,
-		b__X___X,
-		b__XX_XX,
-		b___X_X_,
-		b____X__,
-		b_______
-	};
+		{
+			b_______,
+			b___X_X_,
+			b__X_X_X,
+			b__X___X,
+			b__X___X,
+			b___X_X_,
+			b____X__,
+			b_______
+		};
 
 	static unsigned char heart_filled[] =
 	{
@@ -784,10 +979,38 @@
 		b_______
 	};
 
+	/* Icons from the display's character map ROM */
 	switch (icon) {
 		case ICON_BLOCK_FILLED:
 			picoLCD_chr(drvthis, x, y, 255);
-			break;
+			return 0;
+		case ICON_ARROW_LEFT:
+			picoLCD_chr(drvthis, x, y, 127);
+			return 0;
+		case ICON_ARROW_RIGHT:
+			picoLCD_chr(drvthis, x, y, 126);
+			return 0;
+	}
+	/* Icons loaded to the display's custom character RAM.
+	 * Horizontal & vertical bars do not use RAM address zero
+	 * so the heart can be used in these modes. Big numbers use
+	 * all eight locations, big characters are not supported.
+	 */
+	if (	(p->ccmode != custom)
+		&& !(	   ((p->ccmode == hbar) || (p->ccmode == vbar))
+				&& ((icon == ICON_HEART_FILLED) || (icon == ICON_HEART_OPEN))
+			)
+		)
+	{
+		if (p->ccmode != standard) {
+			/* Combined custom character modes not supported */
+			report(RPT_WARNING, "%s: icon: cannot combine two modes using user-defined characters",
+					drvthis->name);
+			return -1;	/* Let the core do the icon */
+		}
+		p->ccmode = custom;
+	}
+	switch (icon) {
 		case ICON_HEART_FILLED:
 			picoLCD_set_char(drvthis, 0, heart_filled);
 			picoLCD_chr(drvthis, x, y, 0);
@@ -796,12 +1019,7 @@
 			picoLCD_set_char(drvthis, 0, heart_open);
 			picoLCD_chr(drvthis, x, y, 0);
 			break;
-		case ICON_ARROW_LEFT:
-			picoLCD_chr(drvthis, x, y, 127);
-			break;
-		case ICON_ARROW_RIGHT:
-			picoLCD_chr(drvthis, x, y, 126);
-			break;
+
 		case ICON_CHECKBOX_GRAY:
 			picoLCD_set_char(drvthis, 5, checkbox_gray);
 			picoLCD_chr(drvthis, x, y, 5);
@@ -832,8 +1050,87 @@
 MODULE_EXPORT char *picoLCD_get_key(Driver *drvthis)
 {
 	PrivateData *p = drvthis->private_data;
-	lcd_packet keydata;
 	char *keystr = NULL;
+
+#ifdef HAVE_LIBUSB_1_0
+	int high_key;
+	int low_key;
+
+	/* N.B. get_key using libusb-1.0
+	 * Read any key events from the buffer and report, do not wait for key up
+	 * events so that the main loop timing is not disrupted; thus the behaviour
+	 * is somewhat different from the previous version.
+	 * TODO report key release.
+	 * Do we want the option to exactly mimic libusb-0.1 behaviour? this did not
+	 * report anything until the keys were released, not the normal way keys are
+	 * reported and thus contrary to a users expectation but with the advantage
+	 * of only reporting a single dual key press if the keys were not pressed or
+	 * released at exactly the same time.
+	 */
+	if (p->key_read_index == p->key_write_index) {
+		/* No new key, check if it is time to repeat a key */
+		if (p->reported_keys.high_key) {
+			if (p->key_delay_count) {
+				p->key_delay_count--;
+				return NULL;
+			}
+			else {
+				if (p->key_interval_count) {
+					p->key_interval_count--;
+					return NULL;
+				}
+				else {
+					high_key = p->reported_keys.high_key;
+					low_key = p->reported_keys.low_key;
+					p->key_interval_count = p->key_repeat_interval;
+				}
+			}
+		}
+		else {
+			return NULL;
+		}
+	}
+	else {
+		/* Get new key data */
+		high_key = p->key_buffer[p->key_read_index].high_key;
+		low_key = p->key_buffer[p->key_read_index].low_key;
+		debug(RPT_DEBUG, "%s: got %d, %d from key_buffer %d"
+				, drvthis->name, high_key, low_key, p->key_read_index);
+		p->key_read_index++;
+		if (KEY_BUFFER_SIZE <= p->key_read_index)
+			p->key_read_index = 0;
+		p->reported_keys.high_key = high_key;
+		p->reported_keys.low_key = low_key;
+		p->key_delay_count = p->key_repeat_delay;
+		p->key_interval_count = 0;
+	}
+	if (low_key) {
+		static char keybuf[2 * KEYPAD_LABEL_MAX + 1];
+		/* N.B. the order here is important for clients that are interested
+		 * in multi-key presses. The key pairs are reported in the opposite
+		 * order to their position in the key-map thus if a client wants to
+		 * be informed when keys F1 & F2 are both pressed it will have to send
+		 * the command "client_add_key [-exclusively|-shared] F2+F1".
+		 * It would be more logical to change the order but this is consistent
+		 * with the previous version.
+		 */
+		sprintf(keybuf, "%s+%s", p->device->keymap[high_key],
+								 p->device->keymap[low_key]);
+		keystr = keybuf;
+	}
+	else {
+		keystr = p->device->keymap[high_key];
+	}
+
+	debug(RPT_DEBUG, "%s: get_key complete (%s)", drvthis->name, keystr);
+
+	if ((keystr != NULL) && (strlen(keystr) > 0))
+		return keystr;
+	else
+		return NULL;
+
+#else
+	lcd_packet keydata;
 	int  keys_read = 0;
 	int  key_pass  = 0;
 	int  two_keys  = 0;
@@ -922,6 +1219,7 @@
  * For keymapping see the picolcd_device structs.
  */
 
+#endif
 }
 
 /* lcd_logical_driver Hardware functions */
@@ -1092,7 +1390,7 @@
  *
  * \param drvthis   Pointer to driver structure [used for debug() and report()].
  * \param data      Buffer of integers to be transcoded.
- * \param cbdata    Buffer of integers to be transcoded.
+ * \param cbdata    Length of data to be transcoded.
  *
  * \note The picoLCD introduces two issues:
  * \note 1. Every read contains a maximum of 10 samples (20 bytes),
@@ -1126,30 +1424,33 @@
 	gettimeofday(&now, 0);
 
 	/* Check for a missing SPACE since the last message */
-	debug(RPT_INFO, "picolcd: last 0x04x first %04x", p->lastval, (-w & 0xFFFF));
+	debug(RPT_INFO, "picolcd: last %04x first %04x", p->lastval, (-w & 0xFFFF));
 	if (((p->lastval & 0x8000) == 0) && ((-w & 0x8000) == 0)) {
 		/* Calculate the time passed from the last ir message to now
 		 * and use that time for the missing space (sync) */
-		int secs = now.tv_sec - p->lastmsg.tv_sec;
+		struct timeval time_gap;
 		int gap = 0x7FFF;
 
+		timersub(&now, &p->lastmsg, &time_gap);
+
 		/* previous message is complete send it, without the added space */
 		debug(RPT_INFO, "picolcd: missing sync detected, flushing queue before adding sync");
 		picolcd_lircsend(drvthis);
 
 		/* Prevent the overflow (2 secs = 32678 jiffies), but allow 2.99 seconds to reach the max */
-		if (secs <= 2) {
+		if (2 <= time_gap.tv_sec) {
 			/* microseconds to jiffies (same as (16384/1000000) but no possible int32 overflow) */
-			gap = ((now.tv_usec - p->lastmsg.tv_usec + secs * 1000000) * 256) / 15625;
-			/* Check overflow */
-			if (gap >= 0x8000) {
-				gap = 0x7FFF;
-			}
+			gap = ((time_gap.tv_sec * 1000000 + time_gap.tv_usec) * 256) / 15625;
+		}
+
+		/* Saturate on 15 bit overflow*/
+		if (gap >= 0x8000) {
+			gap = 0x7FFF;
 		}
 		/* Make it a space */
 		gap |= 0x8000;
 
-		debug(RPT_INFO, "picolcd: injecting space %04hx between %04hx and %04hx",
+		debug(RPT_INFO, "picolcd: injecting space %04x between %04x and %04x",
 			gap, p->lastval, -w & 0xFFFF);
 		*p->resptr++ = (unsigned char)(gap & 0xff);
 		*p->resptr++ = (unsigned char)((gap >> 8) & 0xff);
@@ -1223,7 +1524,7 @@
 				report(RPT_WARNING, "picolcd: failed to send IR data, reason: %s", strerror(errno));
 			}
 		} else {
-			debug(RPT_DEBUG, "picolcd: send %d bytes to lirc(udp)", len);
+			debug(RPT_DEBUG, "picolcd: sent %d bytes to lirc(udp)", len);
 		}
 		p->resptr = p->result;
 	}
@@ -1236,12 +1537,23 @@
  * \param data  pointer to data packet to send
  * \param size  number of bytes to send
  */
-static void picolcd_send(usb_dev_handle *lcd, unsigned char *data, int size)
+static void picolcd_send(USB_DEVICE_HANDLE *lcd, unsigned char *data, int size)
 {
 	if ((lcd == NULL) && (data == NULL))
 		return;
-
+#ifdef HAVE_LIBUSB_1_0
+	/* N.B. send using libusb-1.0 */
+	int error = 0;
+	int transferred = 0;
+	unsigned int timeout = 1000;		/* milliseconds */
+	error = libusb_interrupt_transfer(lcd, LIBUSB_ENDPOINT_OUT + 1, data, size, &transferred, timeout);
+	if (error) {
+		puts ("libusb_interrupt_transfer error %d, sent %d of %d bytes." /* can't use report it needs drvthis,
+				error, transferred, size */);
+	}
+#else
 	usb_interrupt_write(lcd, USB_ENDPOINT_OUT + 1, (char *) data, size, 1000);
+#endif
 }
 
 
@@ -1252,7 +1564,7 @@
  * \param col   ignored
  * \param data  pointer to NUL terminated string
  */
-static void picolcd_20x4_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data)
+static void picolcd_20x4_write(USB_DEVICE_HANDLE *lcd, const int row, const int col, const unsigned char *data)
 {
 	unsigned char packet[64] = { 0x95, 0x01, 0x00, 0x01 };
 	unsigned char lineset[4][6] = {
@@ -1287,10 +1599,10 @@
  * Write function for 20x2 OEM displays
  * \param lcd   pointer to device handle
  * \param row   Row to place the string at
- * \param col   ignored
+ * \param col   Column to place the string at
  * \param data  pointer to NUL terminated string
  */
-static void picolcd_20x2_write(usb_dev_handle *lcd, const int row, const int col, const unsigned char *data)
+static void picolcd_20x2_write(USB_DEVICE_HANDLE *lcd, const int row, const int col, const unsigned char *data)
 {
 	unsigned char packet[64] = { 0x98 };
 	int len = strlen((char *) data);
@@ -1365,7 +1677,7 @@
 	picolcd_send(p->lcd, data, 13);
 }
 
-
+#ifndef HAVE_LIBUSB_1_0
 /**
  * Read a key or IR event from the display into one packet.
  * \param lcd      pointer to device handle
@@ -1373,7 +1685,7 @@
  *                 read from the display
  * \param timeout  Read timeout in ms
  */
-static void get_key_event(usb_dev_handle *lcd, lcd_packet *packet, int timeout)
+static void get_key_event(USB_DEVICE_HANDLE *lcd, lcd_packet *packet, int timeout)
 {
 	int ret;
 
@@ -1396,6 +1708,7 @@
 		}
 	}
 }
+#endif
 
 
 /**
@@ -1405,7 +1718,7 @@
  * \param state  0 to turn all LEDs off, 1 to turn them on according to
  *               values set in 'keys' array
  */
-static void set_key_lights(usb_dev_handle *lcd, int keys[], int state)
+static void set_key_lights(USB_DEVICE_HANDLE *lcd, int keys[], int state)
 {
 	unsigned char packet[2] = { 0x81 }; /* set led */
 	unsigned int leds = 0;
@@ -1428,4 +1741,76 @@
 	picolcd_send(lcd, packet, 2);
 }
 
+
+#ifdef HAVE_LIBUSB_1_0
+/* N.B. private functions for use with libusb-1.0 */
+/**
+ * Store key press and release events in a buffer ready for the get key function.
+ * \param drvthis	Pointer to driver structure.
+ *
+ * Key events come back in such a way as to report up to two simultaneous keys
+ * pressed.  The highest numbered key always comes back as the first key and
+ * the lower numbered key follows.  If only one key was pressed, the second
+ * key is 0.  The picoLCD also sends key-up events (both key parameters zero).
+ *
+ * \param high_key   highest numbered key pressed
+ * \param low_key    the second key if pressed
+ *
+ * If the buffer is full key codes are discarded.
+ */
+static void key_buffer_put(Driver *drvthis, unsigned char high_key, unsigned char low_key)
+{
+	PrivateData *p = drvthis->private_data;
+	int space = ((p->key_read_index > p->key_write_index)
+			? 0 : KEY_BUFFER_SIZE) + p->key_read_index - p->key_write_index;
+	if ((1 < space) || ((1 == space) && (0 == high_key) && (0 == low_key))) {
+		debug(RPT_DEBUG, "%s: key_buffer put %d, %d @ %d"
+				, drvthis->name, high_key, low_key, p->key_write_index);
+		p->key_buffer[p->key_write_index].high_key = high_key;
+		p->key_buffer[p->key_write_index].low_key = low_key;
+		p->key_write_index++;
+		if (KEY_BUFFER_SIZE <= p->key_write_index)
+			p->key_write_index = 0;
+	}
+}
+
+/**
+ * Call-back for USB input
+ * \param transfer    structure containing the USB data
+ */
+static void usb_cb_input(struct libusb_transfer *transfer)
+{
+	Driver *drvthis = (Driver*)transfer->user_data;
+	PrivateData *p = drvthis->private_data;
+
+
+	if (transfer->status != LIBUSB_TRANSFER_COMPLETED) {
+		report(RPT_ERR, "%s: input transfer status %d", drvthis->name, transfer->status);
+		p->libusb_status = transfer->status;
+		libusb_free_transfer(transfer);
+		p->input_transfer = NULL;
+		return;
+	}
+	p->libusb_status = libusb_submit_transfer(p->input_transfer);
+	if (p->libusb_status != LIBUSB_SUCCESS)
+		report(RPT_ERR, "%s: input transfer submit status %d", drvthis->name, p->libusb_status);
+
+	switch (transfer->buffer[0]) {
+		case IN_REPORT_KEY_STATE:
+			debug(RPT_INFO, "%s: USB input call-back key", drvthis->name);
+			key_buffer_put(drvthis, transfer->buffer[1], transfer->buffer[2]);
+			break;
+
+		case IN_REPORT_IR_DATA:
+			debug(RPT_INFO, "%s: USB input call-back IR length %i", drvthis->name, transfer->buffer[1]);
+			if (p->IRenabled)
+				ir_transcode(drvthis, &transfer->buffer[2], transfer->buffer[1]);
+			break;
+
+		default:
+			report(RPT_ERR, "%s: input transfer unexpected data %d", drvthis->name, transfer->buffer[0]);
+	}
+}
+#endif
+
 /* EOF */
Index: server/drivers/Makefile.am
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/server/drivers/Makefile.am,v
retrieving revision 1.98
diff -u -r1.98 Makefile.am
--- server/drivers/Makefile.am	3 Apr 2011 18:07:15 -0000	1.98
+++ server/drivers/Makefile.am	7 Aug 2011 16:16:19 -0000
@@ -30,7 +30,7 @@
 IOWarrior_CFLAGS =   @LIBUSB_CFLAGS@ $(AM_CFLAGS)
 lis_CFLAGS =         @LIBUSB_CFLAGS@ @LIBFTDI_CFLAGS@ $(AM_CFLAGS)
 mdm166a_CFLAGS =     @LIBHID_CFLAGS@ $(AM_CFLAGS)
-picolcd_CFLAGS =     @LIBUSB_CFLAGS@ $(AM_CFLAGS)
+picolcd_CFLAGS =     @LIBUSB_CFLAGS@ @LIBUSB_1_0_CFLAGS@ $(AM_CFLAGS)
 shuttleVFD_CFLAGS =  @LIBUSB_CFLAGS@ $(AM_CFLAGS)
 ula200_CFLAGS =      @LIBFTDI_CFLAGS@ $(AM_CFLAGS)
 xosd_CFLAGS =        @LIBXOSD_CFLAGS@ $(AM_CFLAGS)
@@ -60,7 +60,7 @@
 MtxOrb_LDADD =       libLCD.a libbignum.a
 mx5000_LDADD =       @LIBMX5000@
 NoritakeVFD_LDADD =  libbignum.a
-picolcd_LDADD =      @LIBUSB_LIBS@ libLCD.a libbignum.a
+picolcd_LDADD =      @LIBUSB_LIBS@ @LIBUSB_1_0_LIBS@ libLCD.a libbignum.a
 pyramid_LDADD =      libLCD.a libbignum.a
 serialPOS_LDADD =    libbignum.a
 serialVFD_LDADD =    libLCD.a libbignum.a
Index: server/drivers/picolcd.h
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/server/drivers/picolcd.h,v
retrieving revision 1.13
diff -u -r1.13 picolcd.h
--- server/drivers/picolcd.h	10 Feb 2011 22:45:35 -0000	1.13
+++ server/drivers/picolcd.h	7 Aug 2011 16:16:20 -0000
@@ -14,6 +14,15 @@
 #ifndef PICOLCD_H
 #define PCIOLCD_H
 
+/* N.B. Use libusb-1.0 */
+#ifdef HAVE_LIBUSB_1_0
+#  include <libusb.h>
+#  define USB_DEVICE_HANDLE libusb_device_handle
+#else
+#  include <usb.h>
+#  define USB_DEVICE_HANDLE usb_dev_handle
+#endif
+
 /* 12 keys plus a 0 placeholder */
 #define KEYPAD_MAX		13
 #define KEYPAD_LIGHTS		8
@@ -35,7 +44,8 @@
 #define DEFAULT_BACKLIGHT	1    /* On */
 #define DEFAULT_KEYLIGHTS	1    /* On */
 #define DEFAULT_TIMEOUT		500  /* Half second */
-
+#define DEFAULT_REPEAT_DELAY 300	/* milliseconds */
+#define DEFAULT_REPEAT_INTERVAL 200	/* milliseconds */
 
 typedef struct _lcd_packet {
 	unsigned char data[255];
@@ -56,7 +66,7 @@
 	int width;                  /* width of lcd screen */
 	int height;                 /* height of lcd screen */
 	/* Pointer to function that writes data to the LCD format */
-	void (*write) (usb_dev_handle *lcd, const int row, const int col, const unsigned char *data);
+	void (*write) (USB_DEVICE_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;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.