hid_error implementation for libusb #698
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Most if not all libusb functions return UTF-8 encoded data but hidapi functions typically take and return wide character strings. Adapt one of the existing algorithms in the code base into a general conversion function.
Returns libusb error names and strings converted to wide character strings.
Store libusb error code so it can be retrieved later. Includes the original error code as well as a context-specific message which the libusb documentation sometimes specifies for each function. Code which uses those functions are meant to set the contextual message whenever possible. The code is initialized to a success state which implies no errors yet. The contextual error message is initialized to NULL and is not freed when the device is closed. It is meant to point at string literals.
Sets all error data, including an optional contextual error message which is supposed to be a non-freeable constant string such as a string literal. Contextual error messages are meant to be used in the cases the libusb documentation goes into detail as to what happened. Passing NULL will produce a message with just the libusb_error_name and the libusb_strerror results. Passing a string literal will produce a message that contains the additional context in addition to the error name and message.
Set error data when send_feature_report fails, including custom messages for the situations especially outlined in the libusb documentation for the libusb_control_transfer function.
Compute a formatted error string containing the libusb error name, the error message as well as any contextual information. Return NULL if there are no errors or if memory allocation failed.
Closed
|
@mcuee if you could test this one as well - would be perfect. |
Yes I will carry out the test later today. |
|
First simple test -- looks good. Tested under Ubuntu Linux 20.04.
|
|
Tested under FreeBSD 14.1 and this PR is good as well. |
|
Anyway, it is good that git main and this PR works fine under FreeBSD 14.1 Release using hidapitester as well. |
|
I think this PR is good to go. |
|
@matheusmoreira would you check as well? |
|
Oh, from the logs I actually see one issue - looks like an error message is cut by one symbol:
I'll double-check this one. |
|
Now we should be good |
Yes it is good now. |
|
It is also good under FreeBSD 14.1 release. |
|
Working for me as well on Arch Linux. The new function that just returns the libusb error code is perfect for my needs. |
|
I assume you've tried the new |
|
Yeah. It's still incomplete but the feature report functions set the libusb error states which are what I need. Would you consider a similar error code based API for hid as well? It's much easier to use than strings. |
Not for v0.x, no.
HIDAPI was never designed to diagnose what went wrong, only an indication that something happened and as string for logging purpose only.
Can you elaborate? I've tried to cover all failure cases. |
|
It's just that I saw some TODO lines in the source code and assumed there were still failure cases left to cover. If they have all been handled then it looks good for merging as far as I can tell. |
|
Ah those - right, still there. For the global state (hid_enumerate/hid_open/etc.). |
|
This PR actually has non-obvious side-effect (related to #688). Because with hid_error implementation, when calling hid_read concurently to other functions (e.g. hid_write) from different threads, in some cases (e.g. device disconnected) the error might be attempted to set in different threads causing a race-condition. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is initial implementation.
hid_libusb_errorto have libusb error code when possible;Closes: #690
Fixes: #684