Add USB HID example

[TiltMeSenpai]

Requires greatscottgadgets/python-usb-protocol#6

[FFY00]

This is a very opinionated and strict way of handling of handling the report descriptors. HID descriptors are very different from the other USB descriptors. They are extremely dynamic, HID packets have no standard format, the format is given by the report descriptor.

IMO it would be much better to receive the report descriptor in __init__ and not have any opinionated declaration like this. If you want, you could then subclass HIDDevice and define a few useful devices.

Looking at this report descriptor declaration, it seems to me like you are using it to create device with the vendor page (am I right?), in which case, you could create a VendorHIDDevice with almost the exact same code 😊.

[TiltMeSenpai]

I'm actually using the generic desktop controls page with no usage. This causes Linux to create a generic HID input item, and allows users to easily interact with the resulting HID device. My goal here is to create a device that an end user can implement without any knowledge of USB HID, and have it work consistently (ish, Windows is a bit more picky about report formatting). That being said, I'd be plenty happy to change class names or accept changes that allow for more flexibility without requiring HID knowledge to get a device working.

[TiltMeSenpai]

I do suppose an option is to auto-generate a working report descriptor if the descriptor in __init__ is None?

[FFY00]

Yes, that would be great!

Generating a HID report descriptor is a very specific to what you are actually trying to do. Hence my suggestion to subclass HIDDevice. The idea would be to subclass into classes that could generate a correct descriptor for common tasks. This would allow users without HID knowledge to create devices. Does that make sense?

I do not really follow your use-case. What is your goal with it? Just to communicate with the device or to report input data? If you just want to communicate, a better way would be to use the vendor usage page, which is designed for this. The kernel will still export a hidraw device node, which you can interact with.

[TiltMeSenpai]

Re: use cases, I'm currently using python-evdev to poll out register values using this gateware (see https://github.com/TiltMeSenpai/orangecrab_sketches/blob/dev/orangecrab_sketches/usb_hostside.py). This requires Linux to export a /dev/input/event* item, which (as far as I can tell) gets created by the generic HID driver, hence the generic desktop controls page.

Re: HIDDevice subclassing, I think about the only thing a pregenerated report descriptor allows you to do is to use String Index report items to name your input/output fields. Pretty much anything else (such as changing the Collection type) will change the report format, and at that point, I'm not super convinced that a USBSignalInEndpoint will still be the right solution. If we let users define their own interrupt endpoints for input/output, the only thing we're really doing is generating descriptors. Is that worth its own class at that point?

[FFY00]

Well, this is very Linux specific and slightly outside the HID spec. You are using the Generic Desktop usage page with the usage 0, which is undefined.

Generating the report descriptor would be helpful for people wanting to export devices such as a mice, keyboards, joysticks, FIDO keys, vendor protocols, etc. We can generate the report descriptor and give them a way to construct the report by just plugging the field values. For your case, you could create a GenericInputHIDDevice.

I still maintain my position on this being a very specific implementation targeted to your use case, and not something I would want in a generic class such as this 😊

If you do not want a subclass, you could just create a method to generate the report descriptor, but I think subclassing would solve the problem in a nicer way.

[TiltMeSenpai]

Hmm... Could you not create a mouse/keyboard/joystick/vendor-defined with the existing implementation? (I think FIDO requires a computer -> LUNA endpoint, which I currently don't have implemented). The only thing the current implementation actually prescribes is all of the report data is consolidated under a single report ID in an Application collection. You can define global Usage Page/Usage and per-item Usages as is, they just default a Generic Desktop page with all Undefined usages. If we move outside of use cases where a USBSignalInEndpoint is appropriate, the only thing the generic HID class will do is generate/place the HID class/HID report descriptors, which feels like python-usb-protocol's job. If you feel that this is still appropriate, I'm happy to make the change, but I'm hesitant to blur the package purposes like this.

[FFY00]

Hmm... Could you not create a mouse/keyboard/joystick/vendor-defined with the existing implementation?

No, because you are locked to 1 single usage. All you can do is create a device that report a X axis for eg. not a device that export an X an Y axis. You can of course fix this, but if you start trying to fix different use cases things will get out of hand very quickly. You are creating a generalized report descriptor constructor, but HID is very implementation specific.

Hmm... Could you not create a mouse/keyboard/joystick/vendor-defined with the existing implementation? (I think FIDO requires a computer -> LUNA endpoint, which I currently don't have implemented).

Yes, this is an example. But I assume some day this is something we will want to support, at least I have interest in it.

The only thing the current implementation actually prescribes is all of the report data is consolidated under a single report ID in an Application collection.

Hum, what do you mean? You are not using any report ID.

If we move outside of use cases where a USBSignalInEndpoint is appropriate, the only thing the generic HID class will do is generate/place the HID class/HID report descriptors, which feels like python-usb-protocol's job. If you feel that this is still appropriate, I'm happy to make the change, but I'm hesitant to blur the package purposes like this.

Perhaps, this is flexible, I just don't think the current implementation should have such opinionated report descriptor generator.

Anyway, I think this is a question for the maintainers. They are the ones giving support for the software (I am available to comment and help out if they want) at the end of the day... If they think the current approach is fine, then go ahead! I just think if things are done a bit differently, it might avoid some long term pain in the future. When you introduce an API such as this, you are inevitable locked into it, breaking changes are something you want to avoid 😊

[ktemkin]

Generally, the usb/devices/ directory is intended to be a stdlib-esque collection of pre-made devices. I'd prefer to keep it to pre-made "template" devices that are broadly useful to a lot of users.

This PR looks more like it contains a very specific usage of HID for a user use case, rather than something that's a very general-case HID device.

Accordingly, I see three potential paths, here:

• Have it exist outside of the LUNA core as a free-standing bit of code (and as an example users can be pointed to). This is probably the most straightforward option; and it's generally how I'd expect particular user applications to end up.
• Clean up the code into an example, rather than a core device type. I think with some coalescing into a single file, once the python-usb-protocol stuff is merged, it'll make a pretty nice example. I think this is the option I favor, unless you want to put a bunch more work in. :)
• Create a more generic HIDDevice class. This will take a lot more work, as the job of a generic HID class would include squishing together signals to form reports that match the report descriptor. I could see a really nice way for this to work (create a class that allows you to add items consisting of Signals and metadata), but I think this is a lot of work, and probably work for me in the future. :)

Thoughts?

[TiltMeSenpai]

I would agree with option 2 (Clean up the code into an example). HID devices seem to be less complicated from an actual gateware logic side, and the complexity is more in terms of creating descriptors that play nice with operating systems. Additionally, as the descriptors get more complex (as a generic HIDDevice class would need to support), status register type endpoints seem less suited to the task. Super long term, I think a generic HIDDeivce class might be a neat idea if people were interested, but I'm thinking that this becomes a better idea once some sort of host -> device status register endpoint is built out, and I'm pretty likely to run out of attention span before that gets completed.

[nevercast]

Not really sure if at this stage if this example will get me closer to emulating HID controllers on my FPGA, but watching intensely! Really excited to try it out.

[TiltMeSenpai]

@nevercast As this PR stands, you could probably build a working keyboard or something, I've been using this code for a bit as a HID-based datalogger. There may be some breaking changes to the proposed python-usb-protocol API (see greatscottgadgets/python-usb-protocol#6). I'm currently trying to rework that API to maintain similar code patterns to the rest of the code base while avoiding unneeded edge cases or awkward interfaces, and utterly failing, so if you have any ideas there, let me know.

[martinling]

We'd really like to have a HID example in the repository, but this one is now long out of date and doesn't seem like it would be ready to merge in its current form.

So I'm going to close this PR, but feel free to reopen it if updated, or open a new PR for a new version.