Improve libusb error logging #10729
Improve libusb error logging #10729
Conversation
…nts_timeout_completed fail
Pokechu22
force-pushed
the
libusb-error-wrap
branch
from
b2bffa6
to
e7f8a16
Compare
Source/Core/Core/LibusbUtils.cpp
Outdated
| @@ -49,22 +49,22 @@ class Context::Impl | |||
|
|
|||
| libusb_context* GetContext() { return m_context; } | |||
|
|
|||
| bool GetDeviceList(GetDeviceListCallback callback) | |||
| int GetDeviceList(GetDeviceListCallback callback) | |||
There was a problem hiding this comment.
Am I missing something or can this function be const?
There was a problem hiding this comment.
If I make the function const I get compile errors when trying to lock on m_device_list_mutex.
There was a problem hiding this comment.
Ah true. Thoughts on making that mutable? I usually make my mutexes mutable in favor of const functions but I see why people would shy away from that...
There was a problem hiding this comment.
I think it could go either way, but it shouldn't be too hard to change. I don't think const has any benefits here, but there isn't really a downside to it either.
| INFO_LOG_FMT(IOS_WIIMOTE, "Sent a reset command to adapter"); | ||
| const int ret = | ||
| libusb_control_transfer(m_handle, REQUEST_TYPE, 0, 0, 0, packet, sizeof(packet), TIMEOUT); | ||
| if (ret < 0) |
There was a problem hiding this comment.
if (ret < LIBUSB_SUCCESS) ?
There was a problem hiding this comment.
Same with the others in this commit
There was a problem hiding this comment.
Note that here, ret indicates the number of bytes transferred if it's nonnegative, but it's probably best to change it to LIBUSB_SUCCESS anyways (the < versus != still applies for that difference).
I can also change a few < 0 comparisons to < LIBUSB_SUCCESS in GCAdapter.cpp.
There was a problem hiding this comment.
Ah ok. I probably could go either way on that too. This works though.
Pokechu22
force-pushed
the
libusb-error-wrap
branch
from
e7f8a16
to
a1fb20f
Compare
Pokechu22
force-pushed
the
libusb-error-wrap
branch
from
a1fb20f
to
27772e0
Compare
|
Apparently this breaks some combinations of Wiimotes on Linux...? https://forums.dolphin-emu.org/Thread-unable-to-pair-remotes-hci-command-timeout |
| libusb_get_string_descriptor_ascii(m_handle, device_descriptor.iManufacturer, manufacturer, | ||
| sizeof(manufacturer)); | ||
| libusb_get_string_descriptor_ascii(m_handle, device_descriptor.iProduct, product, | ||
| sizeof(product)); | ||
| libusb_get_string_descriptor_ascii(m_handle, device_descriptor.iSerialNumber, serial_number, | ||
| sizeof(serial_number)); | ||
| const int manufacturer_ret = libusb_get_string_descriptor_ascii( | ||
| m_handle, device_descriptor.iManufacturer, manufacturer, sizeof(manufacturer)); | ||
| const int product_ret = libusb_get_string_descriptor_ascii( | ||
| m_handle, device_descriptor.iProduct, product, sizeof(product)); | ||
| const int serial_ret = libusb_get_string_descriptor_ascii( | ||
| m_handle, device_descriptor.iSerialNumber, serial_number, sizeof(serial_number)); | ||
| if (manufacturer_ret < LIBUSB_SUCCESS || product_ret < LIBUSB_SUCCESS || | ||
| serial_ret < LIBUSB_SUCCESS) | ||
| { | ||
| ERROR_LOG_FMT(IOS_WIIMOTE, | ||
| "Failed to get descriptor for device {:04x}:{:04x} (rev {:x}): {}/{}/{}", | ||
| device_descriptor.idVendor, device_descriptor.idProduct, | ||
| device_descriptor.bcdDevice, LibusbUtils::ErrorWrap(manufacturer_ret), | ||
| LibusbUtils::ErrorWrap(product_ret), LibusbUtils::ErrorWrap(serial_ret)); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The new error seems to be this:
56:49:662 Core/IOS/USB/Bluetooth/BTReal.cpp:116 E[IOS_WIIMOTE]: Failed to get descriptor for device 057e:0305 (rev 100): Other error (13: **UNKNOWN**)/Other error (8: **UNKNOWN**)/Invalid parameter (-2: LIBUSB_ERROR_INVALID_PARAM)
i.e. libusb_get_string_descriptor_ascii is returning an error code, and now we're treating that as a problem while before we were fine with it.
This PR adds logging when libusb functions fail, using
LibusbUtils::ErrorWrapfrom #10604.I haven't tested this much beyond confirming that it builds. But hopefully adding additional error logging makes debugging easier.