From 95e6b98ce9b42b60db8ccecc69060cf910c1712e Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Tue, 11 Mar 2025 19:26:36 +0200 Subject: [PATCH] Add hid_read_error (#721) hid_read_error is a separate error function, that returns error status of hid_read/hid_read_timeout. hid_read/hid_read_timeout is no longer changes internal buffer used by hid_error and it makes it safe to use hid_read/hid_read_timeout from a separa thread, concurently with other device functions. --- hidapi/hidapi.h | 34 ++++++++++++++++++++++++++++++++-- hidtest/test.c | 9 ++++++++- libusb/hid.c | 9 +++++++++ linux/hid.c | 21 +++++++++++++++------ mac/hid.c | 20 ++++++++++++++++---- netbsd/hid.c | 32 +++++++++++++++++++++++++++----- windows/hid.c | 19 +++++++++++++++---- 7 files changed, 122 insertions(+), 22 deletions(-) diff --git a/hidapi/hidapi.h b/hidapi/hidapi.h index 62ca8a8..75c6245 100644 --- a/hidapi/hidapi.h +++ b/hidapi/hidapi.h @@ -341,9 +341,11 @@ extern "C" { @returns This function returns the actual number of bytes read and -1 on error. - Call hid_error(dev) to get the failure reason. + Call hid_read_error(dev) to get the failure reason. If no packet was available to be read within the timeout period, this function returns 0. + + @note This function doesn't change the buffer returned by the hid_error(dev). */ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char *data, size_t length, int milliseconds); @@ -363,12 +365,40 @@ extern "C" { @returns This function returns the actual number of bytes read and -1 on error. - Call hid_error(dev) to get the failure reason. + Call hid_read_error(dev) to get the failure reason. If no packet was available to be read and the handle is in non-blocking mode, this function returns 0. + + @note This function doesn't change the buffer returned by the hid_error(dev). */ int HID_API_EXPORT HID_API_CALL hid_read(hid_device *dev, unsigned char *data, size_t length); + /** @brief Get a string describing the last error which occurred during hid_read/hid_read_timeout. + + Since version 0.15.0, @ref HID_API_VERSION >= HID_API_MAKE_VERSION(0, 15, 0) + + This function is intended for logging/debugging purposes. + + This function guarantees to never return NULL. + If there was no error in the last call to hid_read/hid_read_error - + the returned string clearly indicates that. + + Any HIDAPI function that can explicitly indicate an execution failure + (e.g. by an error code, or by returning NULL) - may set the error string, + to be returned by this function. + + Strings returned from hid_read_error() must not be freed by the user, + i.e. owned by HIDAPI library. + Device-specific error string may remain allocated at most until hid_close() is called. + + @ingroup API + @param dev A device handle. Shall never be NULL. + + @returns + A string describing the hid_read/hid_read_timeout error (if any). + */ + HID_API_EXPORT const wchar_t* HID_API_CALL hid_read_error(hid_device *dev); + /** @brief Set the device handle to be non-blocking. In non-blocking mode calls to hid_read() will return diff --git a/hidtest/test.c b/hidtest/test.c index cb4a2ce..eb01d88 100644 --- a/hidtest/test.c +++ b/hidtest/test.c @@ -245,6 +245,13 @@ int main(int argc, char* argv[]) // Try to read from the device. There should be no // data here, but execution should not block. res = hid_read(handle, buf, 17); + if (res < 0) { +#if HID_API_VERSION >= HID_API_MAKE_VERSION(0, 15, 0) + printf("Unable to read from device: %ls\n", hid_read_error(handle)); +#else + printf("Unable to read from device: %ls\n", hid_error(handle)); +#endif + } // Send a Feature Report to the device buf[0] = 0x2; @@ -254,7 +261,7 @@ int main(int argc, char* argv[]) buf[4] = 0x00; res = hid_send_feature_report(handle, buf, 17); if (res < 0) { - printf("Unable to send a feature report.\n"); + printf("Unable to send a feature report: %ls\n", hid_error(handle)); } memset(buf,0,sizeof(buf)); diff --git a/libusb/hid.c b/libusb/hid.c index 29df4ce..eb3ebde 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -1557,11 +1557,20 @@ ret: return bytes_read; } + int HID_API_EXPORT hid_read(hid_device *dev, unsigned char *data, size_t length) { return hid_read_timeout(dev, data, length, dev->blocking ? -1 : 0); } + +HID_API_EXPORT const wchar_t * HID_API_CALL hid_read_error(hid_device *dev) +{ + (void)dev; + return L"hid_read_error is not implemented yet"; +} + + int HID_API_EXPORT hid_set_nonblocking(hid_device *dev, int nonblock) { dev->blocking = !nonblock; diff --git a/linux/hid.c b/linux/hid.c index 228fdee..0fc9293 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -75,6 +75,7 @@ struct hid_device_ { int device_handle; int blocking; wchar_t *last_error_str; + wchar_t *last_read_error_str; struct hid_device_info* device_info; }; @@ -97,6 +98,7 @@ static hid_device *new_hid_device(void) dev->device_handle = -1; dev->blocking = 1; dev->last_error_str = NULL; + dev->last_read_error_str = NULL; dev->device_info = NULL; return dev; @@ -1108,7 +1110,7 @@ int HID_API_EXPORT hid_write(hid_device *dev, const unsigned char *data, size_t int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t length, int milliseconds) { /* Set device error to none */ - register_device_error(dev, NULL); + register_error_str(&dev->last_read_error_str, NULL); int bytes_read; @@ -1132,7 +1134,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t } if (ret == -1) { /* Error */ - register_device_error(dev, strerror(errno)); + register_error_str(&dev->last_read_error_str, strerror(errno)); return ret; } else { @@ -1140,7 +1142,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t indicate a device disconnection. */ if (fds.revents & (POLLERR | POLLHUP | POLLNVAL)) { // We cannot use strerror() here as no -1 was returned from poll(). - register_device_error(dev, "hid_read_timeout: unexpected poll error (device disconnected)"); + register_error_str(&dev->last_read_error_str, "hid_read_timeout: unexpected poll error (device disconnected)"); return -1; } } @@ -1151,7 +1153,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t if (errno == EAGAIN || errno == EINPROGRESS) bytes_read = 0; else - register_device_error(dev, strerror(errno)); + register_error_str(&dev->last_read_error_str, strerror(errno)); } return bytes_read; @@ -1162,6 +1164,13 @@ int HID_API_EXPORT hid_read(hid_device *dev, unsigned char *data, size_t length) return hid_read_timeout(dev, data, length, (dev->blocking)? -1: 0); } +HID_API_EXPORT const wchar_t * HID_API_CALL hid_read_error(hid_device *dev) +{ + if (dev->last_read_error_str == NULL) + return L"Success"; + return dev->last_read_error_str; +} + int HID_API_EXPORT hid_set_nonblocking(hid_device *dev, int nonblock) { /* Do all non-blocking in userspace using poll(), since it looks @@ -1232,8 +1241,8 @@ void HID_API_EXPORT hid_close(hid_device *dev) close(dev->device_handle); - /* Free the device error message */ - register_device_error(dev, NULL); + free(dev->last_error_str); + free(dev->last_read_error_str); hid_free_enumeration(dev->device_info); diff --git a/mac/hid.c b/mac/hid.c index 97d511b..5a41ea7 100644 --- a/mac/hid.c +++ b/mac/hid.c @@ -142,6 +142,7 @@ struct hid_device_ { pthread_barrier_t shutdown_barrier; /* Ensures correct shutdown sequence */ int shutdown_thread; wchar_t *last_error_str; + wchar_t *last_read_error_str; }; static hid_device *new_hid_device(void) @@ -163,6 +164,7 @@ static hid_device *new_hid_device(void) dev->device_info = NULL; dev->shutdown_thread = 0; dev->last_error_str = NULL; + dev->last_read_error_str = NULL; /* Thread objects */ pthread_mutex_init(&dev->mutex, NULL); @@ -196,6 +198,7 @@ static void free_hid_device(hid_device *dev) CFRelease(dev->source); free(dev->input_report_buf); free(dev->last_error_str); + free(dev->last_read_error_str); hid_free_enumeration(dev->device_info); /* Clean up the thread objects */ @@ -1244,6 +1247,8 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t { int bytes_read = -1; + register_error_str(&dev->last_read_error_str, NULL); + /* Lock the access to the report list. */ pthread_mutex_lock(&dev->mutex); @@ -1257,7 +1262,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t /* Return if the device has been disconnected. */ if (dev->disconnected) { bytes_read = -1; - register_device_error(dev, "hid_read_timeout: device disconnected"); + register_error_str(&dev->last_read_error_str, "hid_read_timeout: device disconnected"); goto ret; } @@ -1266,7 +1271,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t has been an error. An error code of -1 should be returned. */ bytes_read = -1; - register_device_error(dev, "hid_read_timeout: thread shutdown"); + register_error_str(&dev->last_read_error_str, "hid_read_timeout: thread shutdown"); goto ret; } @@ -1280,7 +1285,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t bytes_read = return_data(dev, data, length); else { /* There was an error, or a device disconnection. */ - register_device_error(dev, "hid_read_timeout: error waiting for more data"); + register_error_str(&dev->last_read_error_str, "hid_read_timeout: error waiting for more data"); bytes_read = -1; } } @@ -1304,7 +1309,7 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t } else if (res == ETIMEDOUT) { bytes_read = 0; } else { - register_device_error(dev, "hid_read_timeout: error waiting for more data"); + register_error_str(&dev->last_read_error_str, "hid_read_timeout: error waiting for more data"); bytes_read = -1; } } @@ -1324,6 +1329,13 @@ int HID_API_EXPORT hid_read(hid_device *dev, unsigned char *data, size_t length) return hid_read_timeout(dev, data, length, (dev->blocking)? -1: 0); } +HID_API_EXPORT const wchar_t * HID_API_CALL hid_read_error(hid_device *dev) +{ + if (dev->last_read_error_str == NULL) + return L"Success"; + return dev->last_read_error_str; +} + int HID_API_EXPORT hid_set_nonblocking(hid_device *dev, int nonblock) { /* All Nonblocking operation is handled by the library. */ diff --git a/netbsd/hid.c b/netbsd/hid.c index 9aed28f..ec5319a 100644 --- a/netbsd/hid.c +++ b/netbsd/hid.c @@ -45,6 +45,7 @@ struct hid_device_ { int device_handle; int blocking; wchar_t *last_error_str; + wchar_t *last_read_error_str; struct hid_device_info *device_info; size_t poll_handles_length; struct pollfd poll_handles[256]; @@ -143,6 +144,18 @@ static void register_device_error_format(hid_device *dev, const char *format, .. va_end(args); } +static void register_device_read_error(hid_device *dev, const char *msg) +{ + register_error_str(&dev->last_read_error_str, msg); +} + +static void register_device_read_error_format(hid_device *dev, const char *format, ...) +{ + va_list args; + va_start(args, format); + register_error_str_vformat(&dev->last_read_error_str, format, args); + va_end(args); +} /* * Gets the size of the HID item at the given position @@ -878,9 +891,11 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char struct pollfd *ph; ssize_t n; + register_device_read_error(dev, NULL); + res = poll(dev->poll_handles, dev->poll_handles_length, milliseconds); if (res == -1) { - register_device_error_format(dev, "error while polling: %s", strerror(errno)); + register_device_read_error_format(dev, "error while polling: %s", strerror(errno)); return -1; } @@ -891,7 +906,7 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char ph = &dev->poll_handles[i]; if (ph->revents & (POLLERR | POLLHUP | POLLNVAL)) { - register_device_error(dev, "device IO error while polling"); + register_device_read_error(dev, "device IO error while polling"); return -1; } @@ -907,7 +922,7 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char if (errno == EAGAIN || errno == EINPROGRESS) n = 0; else - register_device_error_format(dev, "error while reading: %s", strerror(errno)); + register_device_read_error_format(dev, "error while reading: %s", strerror(errno)); } return n; @@ -918,6 +933,13 @@ int HID_API_EXPORT HID_API_CALL hid_read(hid_device *dev, unsigned char *data, s return hid_read_timeout(dev, data, length, (dev->blocking) ? -1 : 0); } +HID_API_EXPORT const wchar_t* HID_API_CALL hid_read_error(hid_device *dev) +{ + if (dev->last_read_error_str == NULL) + return L"Success"; + return dev->last_read_error_str; +} + int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device *dev, int nonblock) { dev->blocking = !nonblock; @@ -949,8 +971,8 @@ void HID_API_EXPORT HID_API_CALL hid_close(hid_device *dev) if (!dev) return; - /* Free the device error message */ - register_device_error(dev, NULL); + free(dev->last_error_str); + free(dev->last_read_error_str); hid_free_enumeration(dev->device_info); diff --git a/windows/hid.c b/windows/hid.c index 05237fc..fe8eb4e 100644 --- a/windows/hid.c +++ b/windows/hid.c @@ -189,6 +189,7 @@ struct hid_device_ { USHORT feature_report_length; unsigned char *feature_buf; wchar_t *last_error_str; + wchar_t *last_read_error_str; BOOL read_pending; char *read_buf; OVERLAPPED ol; @@ -213,6 +214,7 @@ static hid_device *new_hid_device() dev->feature_report_length = 0; dev->feature_buf = NULL; dev->last_error_str = NULL; + dev->last_read_error_str = NULL; dev->read_pending = FALSE; dev->read_buf = NULL; memset(&dev->ol, 0, sizeof(dev->ol)); @@ -231,7 +233,9 @@ static void free_hid_device(hid_device *dev) CloseHandle(dev->write_ol.hEvent); CloseHandle(dev->device_handle); free(dev->last_error_str); + free(dev->last_read_error_str); dev->last_error_str = NULL; + dev->last_read_error_str = NULL; free(dev->write_buf); free(dev->feature_buf); free(dev->read_buf); @@ -1152,11 +1156,11 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char BOOL overlapped = FALSE; if (!data || !length) { - register_string_error(dev, L"Zero buffer/length"); + register_string_error_to_buffer(&dev->last_read_error_str, L"Zero buffer/length"); return -1; } - register_string_error(dev, NULL); + register_string_error_to_buffer(&dev->last_read_error_str, NULL); /* Copy the handle for convenience. */ HANDLE ev = dev->ol.hEvent; @@ -1172,7 +1176,7 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char if (GetLastError() != ERROR_IO_PENDING) { /* ReadFile() has failed. Clean up and return error. */ - register_winapi_error(dev, L"ReadFile"); + register_winapi_error_to_buffer(&dev->last_read_error_str, L"ReadFile"); CancelIo(dev->device_handle); dev->read_pending = FALSE; goto end_of_function; @@ -1219,7 +1223,7 @@ int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device *dev, unsigned char } } if (!res) { - register_winapi_error(dev, L"hid_read_timeout/GetOverlappedResult"); + register_winapi_error_to_buffer(&dev->last_read_error_str, L"hid_read_timeout/GetOverlappedResult"); } end_of_function: @@ -1235,6 +1239,13 @@ int HID_API_EXPORT HID_API_CALL hid_read(hid_device *dev, unsigned char *data, s return hid_read_timeout(dev, data, length, (dev->blocking)? -1: 0); } +HID_API_EXPORT const wchar_t * HID_API_CALL hid_read_error(hid_device *dev) +{ + if (dev->last_read_error_str == NULL) + return L"Success"; + return dev->last_read_error_str; +} + int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device *dev, int nonblock) { dev->blocking = !nonblock;