-
Notifications
You must be signed in to change notification settings - Fork 415
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
Get descriptor (windows fixes) #501
Conversation
@jkcmusork Thank you for the patch! Could you please provide the report descriptor (and the preparsed data dump) of the device where these issues occured. |
My pleasure. I'll need a little more time to get a preparsed data dump, unless you have a quick code snippet for the appropriate way to determine preparsed data size (looks like I'll need to walk it manually) however here are the reconstructed report descriptors before the changes and after the changes (with an xbox one game controller). You'll notice that in the original reconstructed report description the first and second collections are swapped, that there is an extra 32bits of padding in the XY collection, and that there is an extra bit of padding at the end. These are all remedied in the reconstructed report description after the fixes. Original code's reconstructed HID Report Descriptor:
Modified code's reconstructed HID Report Descriptor (fixed version):
|
The tool to save the preparsed data as dump file is in the get_descriptor branch: https://github.com/libusb/hidapi/tree/get-descriptor/windows/pp_data_dump |
Perfect. Sorry for overlooking that tool. Below is the relevant dump file:
|
Thank you! Now the only missing piece is the real HID report descriptor. You can either gather it using another OS like MacOS or Linux. Or by a Windows tool that captures the HID report descriptor using a kernel driver, like the Shareware USBlyzer. |
Sadly for this particular device, the descriptor presents differently on MacOS (and is only readable wirelessly), and under USBlyzer, there is no HID descriptors present only vendor specific USB descriptions that don't really line up. Presumably all of this is because the devices are not actually HID compliant, but drivers translate to support them. Here's what I get from my code reading the report descriptor provided on MacOS when connecting via BlueTooth:
|
USBlyzer might only be able to read descriptors from USB HID devices. |
The Report Descriptor from MacOS doesn't match to the data from Windows. It contains ReportIDs from 1...4, while the data from Windows don't contain a ReportID. |
Correct. The windows driver is returning something totally different, AFAICT. FWIW, The windows driver presents the right joystick axes as Rx and Ry, whereas MacOS presents this as Z and Rz. The windows driver presents the left and right trigger as a combined 16bit Z value, whereas MacOS presents this as individuated 10bit brake and acceleration values, each with 6 bits of padding. The following files are allegedly dumps the report descriptors contained in and presumably provided by xusb22.sys: The preparsed data and reconstructed report descriptor on windows looks awfully close to the following report descriptor, which is what in this circumstance we would hope to reconstruct: Side note: it looks like via the registry, one force it to provide the left and right triggers separately that would correspond to this report descriptor: |
There are some differences, namely in the count on the button page, which has 16 rather than 10 buttons, so likely this uses a different version of the driver. I'll keep poking around on my system to see if I can find it embedded in the appropriate driver. However, upon further review, I think the collection sorting change submitted is appropriate, but the final padding change is not quite right. The 4bits of padding looks more accurate, and perhaps the correct fix needs to add one to the final bit that we are inserting in the main_item_list as well...i.e.
instead of
|
Okay, I searched through xinputhid.sys, which AFAICT is the driver being used for this device, and I found the following report description embedded in it. As best as I can tell, this is what the driver is providing as the report description we are looking at above. The two main differences are both in the final bit padding. 1) the additional off by one issue I mentioned in my previous note (correction now checked in), and 2) an extra 2 bytes of padding. I hope this is helpful to confirm the recommended changes. Let me know if you spot anything amiss, or if you'd like to schedule time to chat via some other medium. Apologies for a bit of noise here as I worked to figure out the windows driver details.
|
For completeness, below is the reconstructed HID descriptor with the latest fix:
|
Sadly, I don't really undestand this implementation, so it is mostly for you @JoergAtGithub to check the PR. |
… the size of the padding is considered anyway
I created jkcmusork#1 with a correction for the padding code. |
Corrected padding of by one issue - no change on output
These changes in this PR are correct:
|
@jkcmusork, lets merge jkcmusork#2 and rebase to latest |
@Youw You can merge this PR as it is. Than I will open a PR, which adds the missing unit test, to be merged in libusb:get-descriptor afterwards. |
Add a Testcase, using the data of the "Xbox One For Windows" controller provided: #501 (comment)
Perfect. Thank you both! |
Hello @JoergAtGithub and @Youw, First off, thank you both for your work on supporting reading/reconstructing report descriptors within this hidapi get-descriptors branch. It has been helpful for a project I've been working on.
I believe I've found a few small errors on windows within the reconstruction process that I'm submitting for your review and consideration merging back to the main hidapi repository. Thank you in advance for your attention.
The first commit is simply a null pointer test, which may be irrelevant now that I'm successfully sorting collection bit ranges, but harmless to include and might future proof other problematic cases.
The second commit contains two changes: