-
Notifications
You must be signed in to change notification settings - Fork 23
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
Some HID++ devices not recognized #12
Comments
I agree that comparing the raw report descriptor is bad and it should be parsed. But yours has a different usage code and that's the most important part for matching the reports. And since it is not a Logitech device, I doubt it is perfectly compatible with a protocol that is not fully publicly documented. It just looks like a vendor specific report that happens to have the same size has Logitech's long one. Do you have evidence it is the same protocol? |
Sorry, I probably should have mentioned that I work for Logitech and Blue microphones are part of Logitech. So yeah, I know for a fact that it's the same protocol. :-) Let me find some other report descriptors that use the same reports. |
It turns out the descriptor I posted originally was from a G213 keyboard. The Yeti X WoW Edition does have all three HID++ reports:
Here are both complete descriptors: They both use the 0xFF43 usage page, though they do use different usages, something I have yet to investigate. |
So what are the rules for recognizing a HID++ device? Report ID/Size/Count + device Vendor ID? |
I would document the exact rules as part of the PR. Basically there are two usage pages, one for the legacy scheme (0xFF00) and one for the modern scheme (0xFF43). In the modern scheme the 16-bit usage is split into an LSB and an MSB. The LSB indicates the report type: short (0x01), long (0x02), or very long (0x04). The MSB is the same for all reports and is a bitmask indicating which report types are supported by the device (combining the same three hex values). Basically in order to detect the reports reliably you only need the usage page and the report ID. But ideally we'll want to have some code to perform sanity checking on the usage and size as well. |
Thank you for the explanation. |
We've made the documentation of the collection usage schemes available in the hidpp20 public folder. The file is called As for the original feature I'll have a PR ready for you soon. I've been dogfooding it for the last few days and it works well. |
…orts (cvuchener#12) Use the HID report descriptor parser to look for HID++ reports instead of hardcoding binary fragments of HID++ reports. This scales better and does not require devices to adhere to a certain way of describing HID++ reports for them to be recognized.
@cvuchener Here's a first version where the hardcoded descriptor fragments have been replaced with a parser Before:
After:
Let me know if you think this is ready for a PR or if you see any problems. One big outstanding question is related to the Windows implementation: In the Windows backend we don't have the raw descriptor today but instead have a list of reports. That is actually a lot easier to handle (and more in the spirit of HID) and we could achieve the same thing on Linux using the HIDDEV API. The challenge there is to make the proper link between |
…orts (cvuchener#12) Use the HID report descriptor parser to look for HID++ reports instead of hardcoding binary fragments of HID++ reports. This scales better and does not require devices to adhere to a certain way of describing HID++ reports for them to be recognized.
Out of curiosity, do you plan to use this in your own projects? I don't want to waste your time on something that don't have a lot of users (I am not very active on it myself). If you want to go forth with it, there is no point in not creating a PR even if it won't be merged in this state. Then you tell me if you want a review on a specific part or the whole PR. Not an in-depth review but a few comment from a quick look at the code.
This is far from a complete review. Don't rush to fix the details. Just keep these comments in mind when rewriting code. About Windows portabilityI wrote the windows port as a kind of challenge to myself and to see if it was worth keeping the unixy details hidden (e.g. not exposing file descriptors in the API). The result is not great. It is not even working correctly with some devices (unifying receivers IIRC). If it gets too much in the way, I don't mind scraping it. If you want to use this library maybe you can tell me if Windows compatibility is important to you. A good solution, in my opinion, would be to have a common structure that could be filled either by the raw descriptor parser or the Windows user space HID API. It would not have to be complete, as long as it contains whatever is needed to find the HID++ reports. |
I'm using the library for some internal test software that needs to run on both Linux and Windows and I'm trying to fix all the issues that block me as I go along. If you're interested in integrating those fixes upstream then I'll happily contribute them but unfortunately my time on that project is limited, so I would hope that the overhead from upstreaming doesn't become too large. If you're in the same situation as me and don't have a lot of spare cycles for this project I don't mind much keeping my fixes on a fork. It would be a shame for some users out there but I totally get that maintaining an open source project takes a lot of energy in the long-term and sometimes life and other projects get in the way. The HID parser actually comes from another project where I was testing it before breaking it out. I wouldn't want to have to rewrite the entire code just to imperfectly imitate the existing coding style. I'll certainly fix the bigger issues or I can even run it through clang-format but I don't want to go through the code and manually put spaces in all function calls. I just don't have the time for that. So let's perhaps quickly see if our mutual expectations and motivations match before we go into the details of the code. Let me also add a list of other things I've come across and changed/fixed so far:
Some of these would still require a bit of work and cleanup before upstreaming. |
It's not really time I'm lacking, but more motivation and perspective. So if you prefer to maintain your own fork, I don't mind. But if you have more remarks like the one that started this thread, I'm still interested even if I don't act on it right away. Or if you think I can help you somehow, you can ask me.
I don't know anything about error length, is it something I missed?
IIRC, you need to export functions explicitly when building dlls with MSVC (which I don't), but what is the issue with static linking? Just adding the option in the cmake script? |
Ok, then let me try to get this issue here to the point where we have an acceptable PR. I'll update my implementation to incorporate your feedback.
It's this small change here:
Yes, that's indeed the problem with shared libraries on Windows and instead of annotating every export I went for the static library. (There is The problem then with static linking was that the library is explicitly marked as Feel free to check out my current working branch: |
Do you mind if I try my own rewrite of the report descriptor parser? I'd like to try something simpler, not much is needed, right? Collection usage, report id, field size, count, usage and flags should be enough.
I forgot I wrote that. It makes sense that the size can vary. |
Not at all, go ahead. My parser was designed to retain as much information as possible for analysis purposes, so I'm sure it could be simplified quite a bit for hidpp's requirements, though perhaps at the cost of robustness. That being said, if you're willing to invest a bit of time then the HIDDEV approach I mentioned earlier (and that I only thought of in hindsight) seems much more promising and the Linux and Windows implementations could be made to match pretty closely. It's been on my to-do list but I haven't got around to it yet. |
I've been looking more closely at Windows API, and there is an issue for checking the sizes. "Buttons" (array or 1-bit variable fields, I guess) don't have the report size or count information. HID++ fields are "buttons" (because they are arrays) and so it is not possible to know their length on Windows. There are report lengths stored in HIDP_CAPS but it is for the top-level collection, not the report. If there are multiple report in a top-level collection, the length is the maximum, I think. What would be the best solution? Ignoring the report/field length and only check the usages? Or checking the report length but on Windows it may be bigger than the actual size, if there are several reports in the collection? Another question: is it necessary to look at the non top-level collections? I don't think there is an issue getting them, but it would greatly simplify the data structure to not have to store a tree of collections. |
The easy one first: You can safely ignore non-top-level collections. We don't use those in our HID++ descriptors. About the report lengths: In my experience with the Windows API each top-level collection shows up as its own device, so using the length information in Here's an example of a HID++ device that implements all three report lengths:
|
Interesting discussions. We have similar but probably broader discussions here in HIDAPI project. It is challenging under Windows. |
Did you try reconstructing the report descriptor for HID++ devices? The reversed-engineered preparsed data from chromium maybe a solution if the report size, count and logical range are still present for every item. This would fix the limitations of HIDP_BUTTON_CAPS and maybe even get the length of each report instead of the maximum length per-collection. |
Because of the way HID++ is specified and used in practice I wouldn't go to the trouble for this particular library. I even checked the library code that we use for some of our production software and it relies on the same Windows HID API that you're using now. |
I am involved in the hidapi project discussions here ( libusb/hidapi#249 ). For Windows. It is tough based on my testing. The best efforts seem to be HIDSharp and libusb/hidapi#262. I am not suggesting hidpp to use HIDAPI since it is just a thin wrapper on top of native HID API (Linux hidraw, Windows and macOS) with the exception of libusb backend (for Linux and FreeBSD). I just find it interesting that the Windows HID API limitation makes many things difficult. |
I am trying to test libusb/hidapi#262 against different HID devices and one of the more difficult device I encountered is the Logitech Unifying Receiver. That leads me to this hidpp project. |
@mrubli2 Just wondering where you get the HidEnum.exe? Thanks. |
I wrote it years ago at work. It doesn't do anything fancy, just using the Windows HID API to enumerate all HID collections and send/receive reports. |
…orts (cvuchener#12) Use the HID report descriptor parser to look for HID++ reports instead of hardcoding binary fragments of HID++ reports. This scales better and does not require devices to adhere to a certain way of describing HID++ reports for them to be recognized.
Just an update, the following hidapi pull request uses the Google Chromium approach with a reengineered struct of the preparsed data returned by HIDP_PREPARSED_DATA and it seems to work pretty well (not perfect yet). |
Currently parts of the HID report descriptor are hardcoded with, for example, the two recognized variants of the long report being:
Many (newer) devices, e.g. the
Yeti X WoW EditionG213 support HID++ 2.0 through different reports like this one (full report descriptor here):Note the different usage page and usage values. Also, this particular device does not have the short report at all, only the long and very long one.
Anyway, hardcoding chunks of the report descriptors doesn't scale well because there's an infinite number of ways to describe the same reports and the HID++ 2.0 specification does not call for a particular sequence of items. A better way of doing things would be to parse the HID report descriptor and simply look for the reports with the right usage page, usage, report ID, and size.
If there is interest I would like to contribute such a parser and hook it up, so that we could replace the hardcoded chunks in question, thereby allowing libhidpp to work with a lot more HID++ devices.
The text was updated successfully, but these errors were encountered: