cvuchener / hidpp

Collection of HID++ tools
GNU General Public License v3.0
89 stars 23 forks source link

Some HID++ devices not recognized #12

Closed mrubli2 closed 3 years ago

mrubli2 commented 3 years ago

Currently parts of the HID report descriptor are hardcoded with, for example, the two recognized variants of the long report being:

0x06, 0x00, 0xFF,  // Usage Page (Vendor Defined 0xFF00)
0x09, 0x02,        // Usage (0x02)
0xA1, 0x01,        // Collection (Application)
0x85, 0x11,        //   Report ID (17)
0x75, 0x08,        //   Report Size (8)     ←
0x95, 0x13,        //   Report Count (19)   ←
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0xFF, 0x00,  //   Logical Maximum (255)
0x09, 0x02,        //   Usage (0x02)
0x81, 0x00,        //   Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x02,        //   Usage (0x02)
0x91, 0x00,        //   Output (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
0xC0,              // End Collection
0x06, 0x00, 0xFF,  // Usage Page (Vendor Defined 0xFF00)
0x09, 0x02,        // Usage (0x02)
0xA1, 0x01,        // Collection (Application)
0x85, 0x11,        //   Report ID (17)
0x95, 0x13,        //   Report Count (19)   ←
0x75, 0x08,        //   Report Size (8)     ←
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0xFF, 0x00,  //   Logical Maximum (255)
0x09, 0x02,        //   Usage (0x02)
0x81, 0x00,        //   Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x02,        //   Usage (0x02)
0x91, 0x00,        //   Output (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
0xC0,              // End Collection

Many (newer) devices, e.g. the ~Yeti X WoW Edition~ G213 support HID++ 2.0 through different reports like this one (full report descriptor here):

0x06, 0x43, 0xFF,  // Usage Page (Vendor Defined 0xFF43)   ←
0x0A, 0x02, 0x06,  // Usage (0x0602)                       ←
0xA1, 0x01,        // Collection (Application)
0x85, 0x11,        //   Report ID (17)
0x75, 0x08,        //   Report Size (8)
0x95, 0x13,        //   Report Count (19)
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0xFF, 0x00,  //   Logical Maximum (255)
0x09, 0x02,        //   Usage (0x02)
0x81, 0x00,        //   Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x02,        //   Usage (0x02)
0x91, 0x00,        //   Output (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
0xC0,              // End Collection

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.

cvuchener commented 3 years ago

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?

mrubli2 commented 3 years ago

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.

mrubli2 commented 3 years ago

It turns out the descriptor I posted originally was from a G213 keyboard. The Yeti X WoW Edition does have all three HID++ reports:

0x06, 0x43, 0xFF,  // Usage Page (Vendor Defined 0xFF43)
0x0A, 0x01, 0x07,  // Usage (0x0701)
0xA1, 0x01,        // Collection (Application)
0x85, 0x10,        //   Report ID (16)
0x75, 0x08,        //   Report Size (8)
0x95, 0x06,        //   Report Count (6)
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0xFF, 0x00,  //   Logical Maximum (255)
0x09, 0x01,        //   Usage (0x01)
0x81, 0x00,        //   Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x01,        //   Usage (0x01)
0x91, 0x00,        //   Output (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
0xC0,              // End Collection

0x06, 0x43, 0xFF,  // Usage Page (Vendor Defined 0xFF43)
0x0A, 0x02, 0x07,  // Usage (0x0702)
0xA1, 0x01,        // Collection (Application)
0x85, 0x11,        //   Report ID (17)
0x75, 0x08,        //   Report Size (8)
0x95, 0x13,        //   Report Count (19)
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0xFF, 0x00,  //   Logical Maximum (255)
0x09, 0x02,        //   Usage (0x02)
0x81, 0x00,        //   Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x02,        //   Usage (0x02)
0x91, 0x00,        //   Output (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
0xC0,              // End Collection

0x06, 0x43, 0xFF,  // Usage Page (Vendor Defined 0xFF43)
0x0A, 0x04, 0x07,  // Usage (0x0704)
0xA1, 0x01,        // Collection (Application)
0x85, 0x12,        //   Report ID (18)
0x75, 0x08,        //   Report Size (8)
0x95, 0x3F,        //   Report Count (63)
0x15, 0x00,        //   Logical Minimum (0)
0x26, 0xFF, 0x00,  //   Logical Maximum (255)
0x09, 0x04,        //   Usage (0x04)
0x81, 0x00,        //   Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x04,        //   Usage (0x04)
0x91, 0x00,        //   Output (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
0xC0,              // End Collection

Here are both complete descriptors: g213.txt yetixwow.txt

They both use the 0xFF43 usage page, though they do use different usages, something I have yet to investigate.

cvuchener commented 3 years ago

So what are the rules for recognizing a HID++ device? Report ID/Size/Count + device Vendor ID?

mrubli2 commented 3 years ago

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.

cvuchener commented 3 years ago

Thank you for the explanation.

mrubli2 commented 3 years ago

We've made the documentation of the collection usage schemes available in the hidpp20 public folder. The file is called logitech_hidpp_hid_vendor_collection_usages.pdf.

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.

mrubli2 commented 3 years ago

@cvuchener Here's a first version where the hardcoded descriptor fragments have been replaced with a parser https://github.com/mrubli2/hidpp/tree/feature/hid-parser

Before:

$ hidpp-list-devices
/dev/hidraw5: Logitech USB Receiver (046d:c53d) HID++ 1.0
/dev/hidraw5 (device 1): G613 Prodigy (046d:4065) HID++ 2.0
/dev/hidraw6: Logitech USB Receiver (046d:c52b) HID++ 1.0
[error] Error while querying /dev/hidraw6 wireless device 1: Resource error
/dev/hidraw6 (device 3): MX Ergo (046d:406f) HID++ 4.5

After:

$ hidpp-list-devices
/dev/hidraw9: Blue Microphones Yeti X WoW Edition (046d:0ac6) HID++ 4.2
/dev/hidraw9 (device 0): Blue Microphones Yeti X WoW Edition (046d:0ac6) HID++ 4.2
/dev/hidraw5: Logitech USB Receiver (046d:c53d) HID++ 1.0
/dev/hidraw5 (device 1): G613 Prodigy (046d:4065) HID++ 2.0
/dev/hidraw6: Logitech USB Receiver (046d:c52b) HID++ 1.0
[error] Error while querying /dev/hidraw6 wireless device 1: Resource error
/dev/hidraw6 (device 3): MX Ergo (046d:406f) HID++ 4.5

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 /dev/usb/hiddev* and /dev/hidraw* nodes. Looking into that could be a good next step, so please let me know your thoughts on that. I am simultaneously working on improving the Windows build and backend as well.

cvuchener commented 3 years ago

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 portability

I 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.

mrubli2 commented 3 years ago

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.

cvuchener commented 3 years ago

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.

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.

  • Support for returning error data along with HID++ exceptions.
  • Allow HID++ errors that aren't of the "long" type. (Errors can be "short" as well.)

I don't know anything about error length, is it something I missed?

  • Some build and code changes to allow building on Windows, including static linking

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?

mrubli2 commented 3 years ago

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.

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.

  • Allow HID++ errors that aren't of the "long" type. (Errors can be "short" as well.)

I don't know anything about error length, is it something I missed?

It's this small change here: https://github.com/mrubli2/hidpp/commit/139b1cec8229151e7e2e87c8617171b3c18a2357 Errors don't have to be long HID++ reports. They can be short or very long as well. Having that condition there can make clients hang because they neither get a successful response, nor do they get the error response if the device sent it over a short report.

  • Some build and code changes to allow building on Windows, including static linking

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?

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 CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS but I couldn't make it work, at least not when using libhidpp as a Meson subproject.)

The problem then with static linking was that the library is explicitly marked as SHARED. The fix is simple: https://github.com/mrubli2/hidpp/commit/48bf50e7e3f8be2709cf11cec2a73cdf5f7a190e

Feel free to check out my current working branch: https://github.com/mrubli2/hidpp/commits/develop As you can see some of the stuff is still WIP because I'm not happy with the naming/code around the "supported report schemes". At some point I'd value your input on how to make that nicer. In the meantime, if you spot any uncontroversial fixes feel free to cherry-pick.

cvuchener commented 3 years ago

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.

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.

It's this small change here: mrubli2@139b1ce

I forgot I wrote that. It makes sense that the size can vary.

mrubli2 commented 3 years ago

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.

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.

cvuchener commented 3 years ago

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.

mrubli2 commented 3 years ago

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 HIDP_CAPS is the right thing to do. You could check if there's only a single input and output descriptor in there for extra safety.

Here's an example of a HID++ device that implements all three report lengths:

> HidEnum.exe -d
  1: \\?\hid#dellabce#3&10865f38&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}
  2: \\?\hid#vid_046d&pid_0ac6&mi_03&col01#7&14e16a93&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030} (Yeti X WoW Edition)
  3: \\?\hid#vid_046d&pid_0ac6&mi_03&col02#7&14e16a93&0&0001#{4d1e55b2-f16f-11cf-88cb-001111000030} (Yeti X WoW Edition)
  4: \\?\hid#vid_046d&pid_0ac6&mi_03&col03#7&14e16a93&0&0002#{4d1e55b2-f16f-11cf-88cb-001111000030} (Yeti X WoW Edition)
  5: \\?\hid#vid_046d&pid_0ac6&mi_03&col04#7&14e16a93&0&0003#{4d1e55b2-f16f-11cf-88cb-001111000030} (Yeti X WoW Edition)

> HidEnum.exe -d 2 -c
\\?\hid#vid_046d&pid_0ac6&mi_03&col01#7&14e16a93&0&0000#{4d1e55b2-f16f-11cf-88cb-001111000030}
Collection (Usage ID: 1793, Usage page: 0xff43): Vendor-defined
  Input report length:   7 bytes
  Output report length:  7 bytes
  Linked collection nodes: 1
    Collection (#Children: 0, Usage page: 0xff43, Usage ID: 0x0701)
  Input button (Report ID: 16 (0x10), Usage page: 0xff43, Usage ID: 0x0001) Vendor-defined, <Unknown usage>
  Output button (Report ID: 16 (0x10), Usage page: 0xff43, Usage ID: 0x0001) Vendor-defined, <Unknown usage>

> HidEnum.exe -d 3 -c
\\?\hid#vid_046d&pid_0ac6&mi_03&col02#7&14e16a93&0&0001#{4d1e55b2-f16f-11cf-88cb-001111000030}
Collection (Usage ID: 1794, Usage page: 0xff43): Vendor-defined
  Input report length:   20 bytes
  Output report length:  20 bytes
  Linked collection nodes: 1
    Collection (#Children: 0, Usage page: 0xff43, Usage ID: 0x0702)
  Input button (Report ID: 17 (0x11), Usage page: 0xff43, Usage ID: 0x0002) Vendor-defined, <Unknown usage>
  Output button (Report ID: 17 (0x11), Usage page: 0xff43, Usage ID: 0x0002) Vendor-defined, <Unknown usage>

> HidEnum.exe -d 4 -c
\\?\hid#vid_046d&pid_0ac6&mi_03&col03#7&14e16a93&0&0002#{4d1e55b2-f16f-11cf-88cb-001111000030}
Collection (Usage ID: 1796, Usage page: 0xff43): Vendor-defined
  Input report length:   64 bytes
  Output report length:  64 bytes
  Linked collection nodes: 1
    Collection (#Children: 0, Usage page: 0xff43, Usage ID: 0x0704)
  Input button (Report ID: 18 (0x12), Usage page: 0xff43, Usage ID: 0x0004) Vendor-defined, <Unknown usage>
  Output button (Report ID: 18 (0x12), Usage page: 0xff43, Usage ID: 0x0004) Vendor-defined, <Unknown usage>
mcuee commented 3 years ago

Interesting discussions. We have similar but probably broader discussions here in HIDAPI project. It is challenging under Windows. https://github.com/libusb/hidapi/issues/249

cvuchener commented 3 years ago

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.

mrubli2 commented 3 years ago

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.

mcuee commented 3 years ago

I am involved in the hidapi project discussions here ( https://github.com/libusb/hidapi/issues/249 ). For Windows. It is tough based on my testing. The best efforts seem to be HIDSharp and https://github.com/libusb/hidapi/pull/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.

mcuee commented 3 years ago

I am trying to test https://github.com/libusb/hidapi/pull/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.

mcuee commented 3 years ago

@mrubli2 Just wondering where you get the HidEnum.exe? Thanks.

mrubli2 commented 3 years ago

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.

mcuee commented 3 years ago

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). https://github.com/libusb/hidapi/pull/306