Skip to content
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

convert report descriptors to TinyUSB macros #1385

Merged
merged 13 commits into from
Mar 4, 2024
Merged

Conversation

tlyu
Copy link
Collaborator

@tlyu tlyu commented Jan 24, 2024

As part of a project to abstract the HID interface code away from particular device back end drivers, import a limited subset of the TinyUSB headers, and use their much nicer report descriptor macros to convert the existing KeyboardioHID report descriptors.

Other than a separate commit that slightly optimizes the Mouse descriptor, the resulting descriptors are byte-wise identical to the originals.

Import class/hid/hid.h from the TinyUSB included in release 1.6 of the
Adafruit nRF52 core.

At least initially, these macros will be used to make HID report
descriptors a bit easier to read and maintain.

Signed-off-by: Taylor Yu <[email protected]>
Disable some TInyUSB definitions that we don't use (yet) and/or conflict
with existing definitions in KeyboardioHID.

Also import some other TinyUSB support macros, with minor fixes. The
negative number issue with 16-bit report items is now
hathach/tinyusb#2425

Signed-off-by: Taylor Yu <[email protected]>
Signed-off-by: Taylor Yu <[email protected]>
Signed-off-by: Taylor Yu <[email protected]>
Signed-off-by: Taylor Yu <[email protected]>
Signed-off-by: Taylor Yu <[email protected]>
Signed-off-by: Taylor Yu <[email protected]>
Signed-off-by: Taylor Yu <[email protected]>
Save a few bytes by removing a few redundant global settings from the
Mouse report descriptor.

Signed-off-by: Taylor Yu <[email protected]>
Copy link
Member

@obra obra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like extra work that will lead to more extra work later to disable unused tusb definitions. I'd love to hear a bit about why that feels like the right path here.

@tlyu
Copy link
Collaborator Author

tlyu commented Jan 24, 2024

There were conflicts with some KeyboardioHID definitions, so I disabled as much stuff as possible that wasn't directly needed by the report descriptor work. I didn't take detailed notes of what all the conflicts were.

I expect that I'll re-enable some of those definitions later, as I continue to improve abstractions and move away from the existing KeyboardioHID code.

Delete a minimal set of conflicting definitions from KeyboardioHID, and
make a few small adjustments to get things to compile.

Signed-off-by: Taylor Yu <[email protected]>
@tlyu
Copy link
Collaborator Author

tlyu commented Jan 29, 2024

It's not especially clean, but the most recent commit has most of tusb_hid.h re-enabled.

Don't use negative numbers in descriptors, because unmodified TinyUSB
doesn't currently handle them, and even the single-byte ones trigger
some warnings on nRF52, which has stricter warnings.

Signed-off-by: Taylor Yu <[email protected]>
Signed-off-by: Taylor Yu <[email protected]>
@obra
Copy link
Member

obra commented Mar 4, 2024

Thanks so much for this

@obra obra merged commit 5996bd0 into keyboardio:master Mar 4, 2024
8 checks passed
@tlyu tlyu deleted the tusb-desc branch March 22, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants