| Summary: | [PATCH] Only enumerate HID devices that have gamepad HID usages | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Cameron Gutman <cameron.gutman> |
| Component: | joystick | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | HG 2.0 | ||
| Hardware: | x86 | ||
| OS: | Windows 10 | ||
| Attachments: | Only enumerate HID devices on Windows that have gamepad HID usages | ||
|
Description
Cameron Gutman
2020-03-20 21:11:04 UTC
Created attachment 4273 [details]
Only enumerate HID devices on Windows that have gamepad HID usages
There are a number of devices that hang inside HidD_GetPreparsedData(). In addition, checking gamepad HID usages won't catch Xbox controllers or Steam controllers. However, I'm not opposed to the intent of this patch. If those issues can be fixed and it can be applied generally across all the backends, I'd be fine with that. (In reply to Sam Lantinga from comment #2) > There are a number of devices that hang inside HidD_GetPreparsedData(). In > addition, checking gamepad HID usages won't catch Xbox controllers or Steam > controllers. All of the bugs I've seen (the ones mentioned above) are hanging in either HidD_GetManufacturerString(), HidD_GetSerialNumberString(), or HidD_GetProductString(). Those would be fixed by this patch. Hangs in HidD_GetPreparsedData() would still need a blacklist entry, but that should be a much smaller subset of devices. If you have a specific bug in mind, I could take a look there. Checking HID usages does work for Xbox controllers on Windows, because the Windows HID stack creates fake HID descriptors for them even though XInput isn't really HID. You're right that Steam Controller don't seem to have gamepad HID usages, but I don't think this patch changes the existing behavior because SDL_hidapijoystick.c already performs the same check here: https://hg.libsdl.org/SDL/file/82e1e144c03b/src/joystick/hidapi/SDL_hidapijoystick.c#l433 since https://hg.libsdl.org/SDL/rev/a5459597367f I tested this patch successfully against Xbox One (USB and BT), Xbox 360 (USB and wireless adapter), and PS4 (USB and BT) controllers. They all matched the HID usage check as expected. The Steam Controller did not (it has the vendor-defined usage page), but that doesn't seem to be new behavior as it was already filtered out in SDL_hidapijoystick.c. > > However, I'm not opposed to the intent of this patch. If those issues can be > fixed and it can be applied generally across all the backends, I'd be fine > with that. There is already a generic check in SDL_hidapijoystick.c (see above), so I don't think this is needed for all backends individually. I targeted Windows here specifically because that's the platform affected by most (all?) of these hang issues (similar to how we only have hid_blacklist() on Windows). Fair enough. I'll review this, thanks! Patch added, thanks! https://hg.libsdl.org/SDL/rev/25927a14e406 https://hg.libsdl.org/SDL/rev/8a949642b6eb Fixed! Thanks! FYI, in order for https://hg.libsdl.org/SDL/rev/8a949642b6eb to effectively enable Steam Controllers, you'll also need to add that VID exception for Valve devices to the HID usage check in HIDAPI_GetDeviceDriver() too. |