The hidapi based implementation returns as soon as the first packet is
received, while the libusb based implementation tries to read the
requested number of bytes. That fails with a timeout if the requested
number of bytes is larger than the size of a single packet and no
further packets are received.
Avoid this problem by limiting the size to the maximum packet size.
Comparing signed and unsigned integer expressions can have unexpected
results because the signed integer will get promoted to an unsigned
integer. To avoid the warning, add an explicit cast to the unsigned
type, along with a check to catch negative values.
In one of the Windows system header files, an "interface" macro is
defined as:
#define interface struct
This results in some very strange build errors when also including the
descriptor-private.h header file. That's because the dc_usb_params_t
struct has a member field named "interface":
typedef struct dc_usb_params_t {
unsigned int interface;
unsigned char endpoint_in;
unsigned char endpoint_out;
} dc_usb_params_t;
As a workaround, define the WIN32_LEAN_AND_MEAN macro before including
the windows.h header file. This excludes some less common Windows API
declarations, including the above one.
In commit 57f0ce6d7902117cfc4d0d6b401b516fc93ca488, an extra parameter
was added to the dc_descriptor_filter() function. But I only updated the
libusb code path and forgot to do the same for the hidapi variant.
There is no reason for libusb to ever return a negative number of bytes,
but due to the use of a signed integer, it's technically possible. And
because libdivecomputer returns an unsigned integer, such a negative
number would be reported as a very large size.
Replace the small helper function to retrieve the function pointer and
then call the function, with another helper function to call the filter
function directly. This way the function pointer doesn't need to be
exposed at all.
The set_latency function is the perfect example of a feature that should
be implemented as an ioctl: it's only implemented by a single driver,
and the functionality is also highly platform specific.
This new ioctl function allows to perform I/O stream specific requests
through a generic interface. This provides an easy way to extend the I/O
interface with some driver specific features, without having to modify
the public api.
The Linux implementation is very straighforward and just a lightweight
wrapper around the select function. But the Windows implementation is
much more complex, because the Windows event notification mechanism
behaves very different:
The WaitCommEvent function does not support a timeout and is always a
blocking call. The only way to implement a timeout is to use
asynchronous I/O (or overlapped I/O as it's called in the Windows API),
to run the operation in the background. This requires some additional
book keeping to keep track of the pending background operation.
The event mechanism is also edge triggered instead of level triggered,
and reading the event with the WaitCommEvent function clears the pending
event. Therefore, the state of the input buffer needs to be checked with
the ClearCommError function before and after the WaitCommEvent call.
The check before is necessary in case the event is already cleared by a
previous WaitCommEvent call, while there is still data present in the
input buffer. In this case, WaitCommEvent should not be called at all,
because it would wait until more data arrives.
The check afterwards is necessary in case WaitCommEvent reports a
pending event, while the data in the input buffer has already been
consumed. In this case, the current event must be ignored and
WaitCommEvent needs to be called again, to wait for the next event.
The libusb and hidapi pkg-config files already add the subdirectory to
the header search path with:
-I/usr/include/libusb-1.0
-I/usr/include/hidapi
Therefore, using the subdirectory in the include statement is wrong. In
practice, this usualy works fine by accident, because the base directory
(/usr/include) is typically listed in the default search path of the
compiler. But that's not always true. For example when cross-compiling
or when using the PKG_CONFIG_LIBDIR environment variable.
When two or more identical (or very similar) dive computers are
connected, the USB VID/PID can be ambiguous. That's because the VID/PID
identifies the type of the USB device, and not the individual device.
But each USB HID device descriptor returned by the device discovery
represents a single connected device, and thus guarantees to open the
correct USB device.
To obtain the same behaviour as before, an application can simply open
the first discovered device.
Replace the global USB library context with a reference counted session
to manage the lifetime of the USB library context. For the libusb based
implementation, this is actually a much better match for the underlying
libusb api, and allows to eliminate the global state. For the hidapi
based implementation, the global state is unavoidable because the hidapi
doesn't support multiple sessions. Therefore we use a singleton session.
Add a function to query the underlying transport type. This allows the
dive computer backends to implement transport specific behaviour where
necessary.
For the built-in I/O implementations, the transport type is obviously
always hardcoded, but for a custom I/O implementation the application
needs to provide the correct type. Hence the transport type can't be
hardcoded in the vtable and needs to be passed as a parameter.
The device descriptors are extended with a filter function. During the
device discovery, this filter function is used to return only devices
that match a known dive computer.
The filtering is optional, and can be disabled by passing a NULL pointer
for the device descriptor when creating the iterator with one of the
dc_xxx_iterator_new() functions.
Replacing the callback based interface with an iterator based interface,
results in a more extensible abstraction with a common interface for
each of the built-in I/O implementations (serial, usbhid, irda and
bluetooth).
Perform the initialization inside a critical section.
Unfortunately Windows critical sections, which are the simplest
synchronization mechanism available on Windows, do not support static
initialization. A call to InitializeCriticalSection is required.
Therefore a simple spinlock, with an implementation based on atomic
operations, is used as a workaround.
Initializing the hidapi library more than once is tricky. The hid_init()
function can be called multiple times, but the the hid_exit() function
will free the resources unconditionally, regardless of how many times
the hid_init() function has been called. The consequence is a premature
termination of the library.
To avoid this problem, the calls are reference counted. Note that this
workaround can't protect against calls made outside the libdivecomputer
code!
The libusb library doesn't suffer from this problem, because each
initialization returns a new context pointer. But for consistency, we
now also use a single reference counted libusb context.
When libusb uses the Windows HID api internally, it does automatically
prepend a zero report ID to the data for devices which support only a
single report. But apparently it also returns a size of one byte extra!
As a workaround, the number of bytes is limited to the actual size.
See commit c9ed92d3f55c259931527a27d018eb5791a176dd for a similar issue
in the hidapi 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).
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.
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.
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.
On Mac OS X, libusb doesn't work for USB HID devices. We can use the
hidapi library instead. Although the hidapi library supports Linux and
Windows too, we keep using libusb there to avoid the extra dependency.
