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.

Author: Youw

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

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));

libusb/hid.c

@@ -1557,11 +1557,20 @@ int HID_API_EXPORT hid_read_timeout(hid_device *dev, unsigned char *data, size_t
 	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;

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,15 +1134,15 @@ 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 {
 			/* Check for errors on the file descriptor. This will
 			   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);
 

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. */

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);