8000 Support "feature" reports in HID · Issue #5197 · adafruit/circuitpython · GitHub
[go: up one dir, main page]

Skip to content

Support "feature" reports in HID #5197

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

Closed
dhalbert opened this issue Aug 21, 2021 · 5 comments · Fixed by #5481
Closed

Support "feature" reports in HID #5197

dhalbert opened this issue Aug 21, 2021 · 5 comments · Fixed by #5481

Comments

@dhalbert
Copy link
Collaborator

"Feature" reports are not currently supported in HID. They can be bidirectional. I believe this requires some additional callback processing for Feature get and set requests.

Narrows #4868.

@ThomasAtBBTF
Copy link

Today I "stumbled" over this when trying to use an HID descriptor with a feature report...
Only after this test, I became aware of this issue here.

@dhalbert: once you figured out how to support feature reports "long term" in your C code, please check the code in
hid.py | HIDService | _init_devices(self) | line 645 and following.
There is a elif case missing for "tag == _MAIN_ITEM_TAG_FEATURE:"

Without this elif the code runs into an: raise RuntimeError("Unsupported main item in HID descriptor")
with an valid HID descriptor having a "main item" with a \xB" tag. [like. b"\xB1\x02" # feature(Data,var,abs)]

I understand, that this comment is not relevant as of now as feature reports are not jet supported.

@dhalbert
Copy link
Collaborator Author

@ThomasAtBBTF I did a trial implementation of feature reports in https://github.com/dhalbert/circuitpython/tree/hid-boot-protocol, which I have not yet PR'd. If you have some non-BLE way of testing this, I would be very interested. I don't have a device in mind where I know the feature reports are used. The Surface Dial has one, but I am not sure how it's used, and it's hard to spy on the BLE HID traffic to see how.

@ThomasAtBBTF
Copy link

The descriptor for a Braille Display I am having can also work on USB.
The code implementing this Display runs under Linux, but I think I can port it to CP.
(At least the important parts)

@todbot
Copy link
todbot commented Oct 29, 2021

Thank you for this! I just tried new custom HID reports with the blink(1) USB LED report descriptor (which uses FEATURE reports w/ reportIds) and TinyUSB hid_generic_inout (IN/OUT reports). But I get ValueError: usage_page must be 1-255. The HID report descriptors look like the below, using the 16-bit way of specifying usagePage & usage, and I think the current usb_hid assumes usagePage/usage are only 8-bit. I've noticed this 16-bit usagePage/usage specification is used by several "rawhid" implementations (e.g. teensy rawhid, Arduino HID library, TinyUSB hid_generic_inout )

The TinyUSB hid_generic_inout HID Report Descriptor looks like this (one IN report, one OUT report, both 64 bytes, no reportId):

RAWHID_REPORT_DESCRIPTOR = bytes((
    0x06, 0x00, 0xFF,  # Usage Page (Vendor Defined 0xFF00)
    0x09, 0x01,        # Usage (0x01)
    0xA1, 0x01,        # Collection (Application)
    0x09, 0x02,        #   Usage (0x02)
    0x15, 0x00,        #   Logical Minimum (0)
    0x26, 0xFF, 0x00,  #   Logical Maximum (255)
    0x75, 0x08,        #   Report Size (8)
    0x95, 0x40,        #   Report Count (64)
    0x81, 0x02,        #   Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
    0x09, 0x03,        #   Usage (0x03)
    0x15, 0x00,        #   Logical Minimum (0)
    0x26, 0xFF, 0x00,  #   Logical Maximum (255)
    0x75, 0x08,        #   Report Size (8)
    0x95, 0x40,        #   Report Count (64)
    0x91, 0x02,        #   Output (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
    0xC0,              # End Collection
))

The blink(1) HID Report Descriptor looks like this (two FEATURE reports, both with report IDs: reportId 1 is 8 bytes, reportId2 is 60 bytes):

BLINK1_REPORT_DESCRIPTOR = bytes((
    0x06, 0xAB, 0xFF,              # usage page vendor defined 0xFFAB
    0x0A, 0x00, 0x20,              # usage 0x2000
    0xA1, 0x01,                    # COLLECTION (Application)
    0x15, 0x00,                    #   LOGICAL_MINIMUM (0)
    0x26, 0xff, 0x00,              #   LOGICAL_MAXIMUM (255)
    0x75, 0x08,                    #   REPORT_SIZE (8)
    0x85, 1,                       #   REPORT_ID (1)
    0x95, 8,                       #   REPORT_COUNT (8)
    0x09, 0x00,                    #   USAGE (Undefined)
    0xb2, 0x02, 0x01,              #   FEATURE (Data,Var,Abs,Buf)
    0x75, 0x08,                    #   REPORT_SIZE (8)
    0x85, 2,                       #   REPORT_ID (2)
    0x95, 60,                      #   REPORT_COUNT (60)
    0x09, 0x00,                    #   USAGE (Undefined)
    0xb2, 0x02, 0x01,              #   FEATURE (Data,Var,Abs,Buf)
    0xc0,                          # END_COLLECTION
))

@dhalbert
Copy link
Collaborator Author

Thanks for this. I was not aware of >255 Usages and Usage Pages. I opened #5529 for this.

Do you have anything you can test with feature reports that works with the current implementation? If you do, and they don't work, could you open an issue with those results as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
0