-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Run tests for AppVeyor too #1360
base: master
Are you sure you want to change the base?
Conversation
@tormodvolden FWIW this won't reproduce on CI since it doesn't have cross-platform emulated devices yet, but this revealed that the new extra checks in
Are there known limitation in regard to |
What is the device caussing the failure under Windows? Maybe you can post the debug log using the example program (xusb -d vid:pid) fo that device. Thanks. |
Also, if I comment out
I'll try to investigate further. |
I don't want to post raw xusb logs since they seem to contain list of all connected devices. |
Test results on my Windows 11 laptop. git master: no issue
This PR: failed
Debug log comparison
|
Hmm, I can not reproduce the issue with my HID devices. Maybe you want to run
|
That's because there was a bug in test where it would quietly continue after a error, see the change in stress_mt.c. |
Previously the loop would continue even if error has already occured since `goto close` alone doesn't stop early. References #1360
Cross-posting from #1372: EDIT: Looking at the mismatch reported above, it is also a HID device... EDIT 2: Ah, _hid_get_device_descriptor() hardcodes bcdUSB, bcdDevice, and bMaxPacketSize0 ... |
Ah that would do it. Would it be fair to call this a bug then? (since it doesn't return real values) Or is that a well-known limitation that is unlikely to go away? |
Is this different error code originating from drivers or from Windows backend of libusb itself? If it's the latter, it would be nice to fix it in the backend instead for consistency with other platforms / backends. |
I don't know enough about HID to tell if it is possibly to get these values correctly. |
Worst-case, it should be possible to get them via control transfer like the test does? |
I originally thought such a control transfer should end up doing _hid_get_device_descriptor() so I cannot comment on this without studying the code closer... EDIT: OK, so the control transfer get the HID backend hard-coded values and they are compared to the "real" device descriptor that is retrieved and cached on enumeration. So I think the HID backend should rather use values from the cached device descriptor instead of the hard-coded values. |
I should probably make a separate merge request for this. Here, it would need partial reverting the previous commit to verify that this works (it seems to do). However I don't want to mess with the HID backend now in the release freeze. |
Instead of filling in the blanks with hard-coded made-up values that are sometimes correct, use the cached descriptor values retrieved during enumeration, which should be a better fallback. References libusb#1360 Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Instead of filling in the blanks with hard-coded made-up values that are sometimes correct, use the cached descriptor values retrieved during enumeration, which should be a better fallback. References libusb#1360 Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
edfee49
to
c6bae8d
Compare
( I removed that HID fix from here, and rebased also.) |
Still the same problem.
|
Hm seems like a different problem, the one above was LIBUSB_ERROR_NOT_SUPPORTED. |
It is the same issue. Devices with unsuported driver under Windows may return the following errors.
You have to skip opening devices with unsupported device driver, as what is done for testlibusb. Please refer to the comments in the test matrix and run log of |
c6bae8d
to
c226c29
Compare
Some device descriptor fields are hard-coded by the HID backend, so they will often not match. It is complicated to narrow this down to HID devices, so we simply ignore these fields on Windows wholesale. Hopefully we will fix the HID backend later, and this workaround can then be reverted. References libusb#1360 Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Your latest commit is good. Now
|
This PR should be good to go. |
Depending on the devices or drivers, open() may return LIBUSB_ERROR_ACCESS LIBUSB_ERROR_NOT_FOUND LIBUSB_ERROR_NOT_SUPPORTED for devices "out of reach" to libusb. References #1360 Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
Some device descriptor fields are hard-coded by the HID backend, so they will often not match. It is complicated to narrow this down to HID devices, so we simply ignore these fields on Windows wholesale. Hopefully we will fix the HID backend later, and this workaround can then be reverted. References #1360 References #1378 Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
c226c29
to
2b727a1
Compare
Follow-up to #1349.