RFC: Add HID bus type in hid_device_info #308
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
DJm00n
changed the title
DJm00n
changed the title
mcuee
added
Core
|
Sony DualSense controller example. Before: After: |
|
I didn't test Linux version yet. |
|
Technically I could, but right now I'm a little short on time regarding everything, so I can only ask not to make any expectations when I'll do it |
|
Here is the test results on my Raspberry Pi 400 running 64bit Ubuntu Linux. click to expand for the run log |
|
Under my Windows 10 laptop (Windows 10 Enterprise x64 20H2), it seems to me Microsoft HIDI2C device is not listed correctly. click to expand |
|
@DJm00n For the I2C HID device, you have the following codes for Windows. And I checked the Microsoft I2CHID device in Device Manager "Human Interface Devices" section and indeed it has a "Compatible device id" and "Matching device id" value of "ACPI\PNP0C50". |
|
@DJm00n And for those "\?\hid#converteddevice", I am not exactly sure if 255 is the right one to use. I tend to think they are more like "virtual hid bus type". But I am okay as well if you use 255 if there are no better ways. |
|
@DJm00n From what I see, the following Windows codes are not good enough (just using "Device_InstanceId"). On the other hand, for the purpose of HIDAPI, maybe it is still okay. I do not think we can do much with the "Microsoft I2CHID device" anyway. |
|
@mcuee Thank you for testing it! I don't have such I2C or SPI HID devices in person. So I need some further help. Can you debug the code a bit and look what exact For example you can add: wprintf(L"DEVPKEY_Device_InstanceId: %s\n", device_id);before |
|
@DJm00n This is what I get. click to expand |
|
@DJm00n The Microsoft I2C HID device has the following If I look at the Device Manager "Human Device Interface" entries, Device_InstanceId printed out is the same as what is listed as "Device Instance Path" in Device Manager. The one you want to get here is "Compatible IDs" which has two values, "ACPI\PNP0C50" and "PNP0C50". |
|
@DJm00n The ACPI 5.0 Specification includes support for HID Class Devices. the ACPI definitions for HID I²C are as follows.
|
|
Same for SPI HID device. The ACPI 5.0 Specification includes support for HID Class Devices. the ACPI definitions for HID SPI are as follows.
|
|
Compatible ID should work for your purpose, no matter it is USB, Bluetooth, I2C or SPI. |
|
More info about HID Transport under Windows |
|
Built-in device for my Dell Laptop (without any external USB/Bluetooth devices). click to expand |
|
And it is also good for macOS. |
|
@mcuee thanks! “Microsoft Bluetooth Mouse” - what kind of mouse is that? Looks like bus type is not detected in this case? Does it use some kind of custom driver? |
I have the Logitech K480 bluetooth keyboard and Microsoft Bluetooth Mouse. The MS Bluetooth Mouse seems to use Bluetooth Low Energy. Not so sure if you need to deal with BLE specially. |
|
@mcuee can you modify the code locally a bit, to print |
|
Test results under Linux Ubuntu 20.04 (dual boot with the above Windows 11 machine) and it seems to be fine. |
|
Oh. |
That is exactly the reason. |
As mentioned by @DJm00n, somehow macOS 12.4 comes out with |
|
I have pushed a fix. Just checking for "Bluetooth" prefix now. |
Youw
approved these changes
Jul 16, 2022
Why not match both |
|
@mcuee Current |
|
It would make 3 matches, which will break, once Apple deside to invent somethink like |
|
@Youw or "Bluetooth LE" or "Bluetooth Smart" like it was named before :) |
|
@Youw and @DJm00n Now it is fine under macOS. |
|
One last test result under Windows 10 and it seems to be okay as well. |
|
Just wondering if this can be merged for 0.13 release. Thanks. |
|
Yes, probably. Just need to resolve the conflicts. |
|
@DJm00n |
Youw
reviewed
Aug 16, 2022
# Conflicts: # hidtest/test.c # libusb/hid.c # linux/hid.c
Youw
approved these changes
Sep 10, 2022
|
If I'm not mistaken, this breaks the ABI. I'm in no position to argue whether that's fine or not here, but given #292 (comment),
it might be worth adding a note about this in the 0.13.0 release notes. |
|
I haven't performed any specific tests, but it shouldn't break the ABI, since we added the new field at the end of the struct. |
|
You're right. I wasn't completely sure that the new field being added at the end of the struct was sufficient, and had also hit (but not yet looked into) an invalid read with valgrind when running an executable built against 0.12.0 with 0.13.0. But it turned out that the valgrind error was unrelated to this (see #497). And since we only manipulate Sorry for the noise. |
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.
It happens that Bluetooth HID gamepads are having the same VID/PID as when connected via USB (DualShock4 and DualSense for example). But they have different report descriptors and thus input/output reports. We need a way to differentiate them.
To address this I have added a new
hid_bus_typeenum andhid_bus_type bus_typefield tohid_device_infostruct:Implemented it for Windows, libusb, hidraw and MacOS backends.