greatscottgadgets / python-usb-protocol

python library providing utilities, data structures, constants, parsers, and tools for working with USB data
BSD 3-Clause "New" or "Revised" License
55 stars 32 forks source link

Add HID descriptors #6

Closed TiltMeSenpai closed 7 months ago

TiltMeSenpai commented 4 years ago

Add basic emitters for USB HID.

TiltMeSenpai commented 4 years ago
  • It'd be best to treat the different HID descriptors / elements separately in our emitters. I like the idea of these behaving similarly to the other emitters -- e.g. we could have a HIDDescriptor and a HIDReportDescriptor as separate classes, and then have HIDReportDescriptor be capable of stringing together items to make a single report descriptor. I don't see any issues with adding these two descriptors separately to their parent descriptor.

HID does require 2 separate descriptors, but for some reason, the protocol decided to put the report descriptor length in the HID descriptor. This means that the HID descriptor needs to be aware of the length of the HID report, and I've made the decision to tie the two descriptors together. Not saying it's the right decision, I actually think it's pretty ugly and I'd be happy to hear alternate ideas. Additionally, the HID report descriptor doesn't follow the traditional pattern of "second byte is the type." This means that if the user needs to add the HID report descriptor manually, they need to be aware that the descriptor type must be manually set.

  • I'm not sure I understand the logic behind add_input_item -- unless I'm misunderstanding, it seems to create a single-item report descriptor, rather than allowing a collection of items in a single report descriptor.

The INPUT, OUTPUT, and FEATURE items are a bit special in that they (as far as I can see) are the only report items that utilize a bitmap in their data field. add_input_item/add_output_item were created as convenience methods to automatically populate that field, they could be removed but it might make the interface a bit more clunky to use.

jmgurney commented 2 years ago

I did a little work on this myself, but I think I'm going to abandon the work I did. I can't get construct to be bidirectional to deal w/ the fact that input/output report item's data can be 1 or 2 bytes. but in most cases this isn't needed, so if someone wants to rip that code out, and adapt the tests from my change, feel free.

https://github.com/jmgurney/python-usb-protocol/tree/hid

antoinevg commented 7 months ago

I'm going to close this until we have a RFC in place specifying how we're going to add support for USB HID Descriptors.