From 05c858bf96028508037afdabf53303f7d2571d62 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Tue, 11 Jul 2017 00:15:39 +0200 Subject: [PATCH 1/5] Fix compatibility issue with hidapi The hidapi library requires that the first byte contains the report ID. For devices which support only a single report, the report ID byte should be zero. The remaining bytes contain the actual report data. Now, when hidapi uses libusb internally, it strips the zero report ID byte again before passing the data to libusb. Thus in order to remain compatible with the hidapi based implementation, our libusb based implementation should do the same. --- src/usbhid.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/usbhid.c b/src/usbhid.c index 0a93824..b99cf21 100644 --- a/src/usbhid.c +++ b/src/usbhid.c @@ -391,14 +391,32 @@ dc_usbhid_write (dc_usbhid_t *usbhid, const void *data, size_t size, size_t *act goto out_invalidargs; } + if (size == 0) { + goto out; + } + #if defined(HAVE_LIBUSB) && !defined(__APPLE__) - int rc = libusb_interrupt_transfer (usbhid->handle, usbhid->endpoint_out, (void *) data, size, &nbytes, 0); + const unsigned char *buffer = (const unsigned char *) data; + size_t length = size; + + // Skip a report id of zero. + unsigned char report = buffer[0]; + if (report == 0) { + buffer++; + length--; + } + + int rc = libusb_interrupt_transfer (usbhid->handle, usbhid->endpoint_out, (void *) buffer, length, &nbytes, 0); if (rc != LIBUSB_SUCCESS) { ERROR (usbhid->context, "Usb write interrupt transfer failed (%s).", libusb_error_name (rc)); status = syserror (rc); goto out; } + + if (report == 0) { + nbytes++; + } #elif defined(HAVE_HIDAPI) nbytes = hid_write(usbhid->handle, data, size); if (nbytes < 0) { From d251b373beccfe01eac6ba71a7f22a0ee13b09b3 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Tue, 11 Jul 2017 00:17:36 +0200 Subject: [PATCH 2/5] Add a zero report ID to the commands The zero report ID byte is required when using the hidapi library. We just never noticed this problem before, because we use libusb by default, and libusb doesn't need the extra zero byte. --- src/uwatec_g2.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uwatec_g2.c b/src/uwatec_g2.c index 3328b4c..fc32aff 100644 --- a/src/uwatec_g2.c +++ b/src/uwatec_g2.c @@ -110,16 +110,17 @@ uwatec_g2_transfer (uwatec_g2_device_t *device, const unsigned char command[], u dc_status_t status = DC_STATUS_SUCCESS; size_t transferred = 0; - if (csize >= PACKET_SIZE) { + if (csize + 2 > PACKET_SIZE) { ERROR (device->base.context, "command too big (%d)", csize); return DC_STATUS_INVALIDARGS; } HEXDUMP (device->base.context, DC_LOGLEVEL_DEBUG, "cmd", command, csize); - buf[0] = csize; - memcpy(buf + 1, command, csize); - status = dc_usbhid_write (device->usbhid, buf, csize + 1, &transferred); + buf[0] = 0; + buf[1] = csize; + memcpy(buf + 2, command, csize); + status = dc_usbhid_write (device->usbhid, buf, csize + 2, &transferred); if (status != DC_STATUS_SUCCESS) { ERROR (device->base.context, "Failed to send the command."); return status; From b82d5fcfff88c16e3057bfd48cf2c312e1460944 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Tue, 11 Jul 2017 09:52:40 +0200 Subject: [PATCH 3/5] Reset the number of bytes to zero on error The hidapi read and write functions return a negative value if an error occurs. Those negative values should not be returned to the caller as the actual number of bytes (or used in the logging). The value is reset to zero instead. --- src/usbhid.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/usbhid.c b/src/usbhid.c index b99cf21..99df918 100644 --- a/src/usbhid.c +++ b/src/usbhid.c @@ -362,6 +362,7 @@ dc_usbhid_read (dc_usbhid_t *usbhid, void *data, size_t size, size_t *actual) if (nbytes < 0) { ERROR (usbhid->context, "Usb read interrupt transfer failed."); status = DC_STATUS_IO; + nbytes = 0; goto out; } #endif @@ -422,6 +423,7 @@ dc_usbhid_write (dc_usbhid_t *usbhid, const void *data, size_t size, size_t *act if (nbytes < 0) { ERROR (usbhid->context, "Usb write interrupt transfer failed."); status = DC_STATUS_IO; + nbytes = 0; goto out; } #endif From c9ed92d3f55c259931527a27d018eb5791a176dd Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Tue, 11 Jul 2017 11:16:15 +0200 Subject: [PATCH 4/5] Workaround for a Windows hidapi issue The Windows HID api always expects to receive a fixed size buffer (corresponding to the largest report supported by the device). Therefore the hidapi library internally pads the buffer with zeros to the expected size, but apparently it also returns the size of the padded buffer! As a workaround the number of bytes is limited to the actual size. --- src/usbhid.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/usbhid.c b/src/usbhid.c index 99df918..7894f06 100644 --- a/src/usbhid.c +++ b/src/usbhid.c @@ -426,6 +426,12 @@ dc_usbhid_write (dc_usbhid_t *usbhid, const void *data, size_t size, size_t *act nbytes = 0; goto out; } + +#ifdef _WIN32 + if (nbytes > size) { + nbytes = size; + } +#endif #endif out: From eea02126a41edc42324c5dcaa628ea4d1394441a Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Fri, 21 Jul 2017 00:48:04 +0200 Subject: [PATCH 5/5] Use hidapi as the default USB HID library On Windows, the hidapi library uses the standard Microsoft USB HID driver, while libusb requires the installation of a different driver (WinUSB or libusbK). But installing one of the libusb drivers breaks compatibility with other applications using hidapi (Scubapro LogTRAK and Suunto DM5) because only one driver can be active. Switching libdivecomputer to hidapi avoids this problem. On Linux, the hidapi library doesn't seem to offer any advantages over libusb. Most distributions don't even have the hidapi library installed by default. Because there are usually two variants of the hidapi library available on Linux (hidapi-libusb and hidapi-hidraw), the autotools build system won't be able to detect it out-of-the-box, and will automatically fallback to the libusb implementation. On Mac OS X, hidapi is already the default (and also the only option). --- src/descriptor.c | 4 ++-- src/usbhid.c | 42 ++++++++++++++++++++++++------------------ 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/descriptor.c b/src/descriptor.c index 5a0f92e..87311e2 100644 --- a/src/descriptor.c +++ b/src/descriptor.c @@ -23,9 +23,9 @@ #include "config.h" #endif -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(HAVE_HIDAPI) #define USBHID -#elif defined(HAVE_HIDAPI) +#elif defined(HAVE_LIBUSB) && !defined(__APPLE__) #define USBHID #endif diff --git a/src/usbhid.c b/src/usbhid.c index 7894f06..b697f47 100644 --- a/src/usbhid.c +++ b/src/usbhid.c @@ -25,14 +25,20 @@ #include -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(HAVE_HIDAPI) +#define USE_HIDAPI #define USBHID +#elif defined(HAVE_LIBUSB) && !defined(__APPLE__) +#define USE_LIBUSB +#define USBHID +#endif + +#if defined(USE_LIBUSB) #ifdef _WIN32 #define NOGDI #endif #include -#elif defined(HAVE_HIDAPI) -#define USBHID +#elif defined(USE_HIDAPI) #include #endif @@ -44,20 +50,20 @@ struct dc_usbhid_t { /* Library context. */ dc_context_t *context; /* Internal state. */ -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) libusb_context *ctx; libusb_device_handle *handle; int interface; unsigned char endpoint_in; unsigned char endpoint_out; unsigned int timeout; -#elif defined(HAVE_HIDAPI) +#elif defined(USE_HIDAPI) hid_device *handle; int timeout; #endif }; -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) static dc_status_t syserror(int errcode) { @@ -103,7 +109,7 @@ dc_usbhid_open (dc_usbhid_t **out, dc_context_t *context, unsigned int vid, unsi // Library context. usbhid->context = context; -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) struct libusb_device **devices = NULL; struct libusb_config_descriptor *config = NULL; @@ -235,7 +241,7 @@ dc_usbhid_open (dc_usbhid_t **out, dc_context_t *context, unsigned int vid, unsi libusb_free_config_descriptor (config); libusb_free_device_list (devices, 1); -#elif defined(HAVE_HIDAPI) +#elif defined(USE_HIDAPI) // Initialize the hidapi library. rc = hid_init(); @@ -260,7 +266,7 @@ dc_usbhid_open (dc_usbhid_t **out, dc_context_t *context, unsigned int vid, unsi return DC_STATUS_SUCCESS; -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) error_usb_close: libusb_close (usbhid->handle); error_usb_free_config: @@ -269,7 +275,7 @@ error_usb_free_list: libusb_free_device_list (devices, 1); error_usb_exit: libusb_exit (usbhid->ctx); -#elif defined(HAVE_HIDAPI) +#elif defined(USE_HIDAPI) error_hid_exit: hid_exit (); #endif @@ -290,11 +296,11 @@ dc_usbhid_close (dc_usbhid_t *usbhid) if (usbhid == NULL) return DC_STATUS_SUCCESS; -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) libusb_release_interface (usbhid->handle, usbhid->interface); libusb_close (usbhid->handle); libusb_exit (usbhid->ctx); -#elif defined(HAVE_HIDAPI) +#elif defined(USE_HIDAPI) hid_close(usbhid->handle); hid_exit(); #endif @@ -315,7 +321,7 @@ dc_usbhid_set_timeout (dc_usbhid_t *usbhid, int timeout) INFO (usbhid->context, "Timeout: value=%i", timeout); -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) if (timeout < 0) { usbhid->timeout = 0; } else if (timeout == 0) { @@ -323,7 +329,7 @@ dc_usbhid_set_timeout (dc_usbhid_t *usbhid, int timeout) } else { usbhid->timeout = timeout; } -#elif defined(HAVE_HIDAPI) +#elif defined(USE_HIDAPI) if (timeout < 0) { usbhid->timeout = -1; } else { @@ -349,7 +355,7 @@ dc_usbhid_read (dc_usbhid_t *usbhid, void *data, size_t size, size_t *actual) goto out_invalidargs; } -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) int rc = libusb_interrupt_transfer (usbhid->handle, usbhid->endpoint_in, data, size, &nbytes, usbhid->timeout); if (rc != LIBUSB_SUCCESS) { ERROR (usbhid->context, "Usb read interrupt transfer failed (%s).", @@ -357,7 +363,7 @@ dc_usbhid_read (dc_usbhid_t *usbhid, void *data, size_t size, size_t *actual) status = syserror (rc); goto out; } -#elif defined(HAVE_HIDAPI) +#elif defined(USE_HIDAPI) nbytes = hid_read_timeout(usbhid->handle, data, size, usbhid->timeout); if (nbytes < 0) { ERROR (usbhid->context, "Usb read interrupt transfer failed."); @@ -396,7 +402,7 @@ dc_usbhid_write (dc_usbhid_t *usbhid, const void *data, size_t size, size_t *act goto out; } -#if defined(HAVE_LIBUSB) && !defined(__APPLE__) +#if defined(USE_LIBUSB) const unsigned char *buffer = (const unsigned char *) data; size_t length = size; @@ -418,7 +424,7 @@ dc_usbhid_write (dc_usbhid_t *usbhid, const void *data, size_t size, size_t *act if (report == 0) { nbytes++; } -#elif defined(HAVE_HIDAPI) +#elif defined(USE_HIDAPI) nbytes = hid_write(usbhid->handle, data, size); if (nbytes < 0) { ERROR (usbhid->context, "Usb write interrupt transfer failed.");