tinyusb icon indicating copy to clipboard operation
tinyusb copied to clipboard

Improve HID host error handling

Open harbaum opened this issue 2 months ago • 1 comments

Related area

HID host class driver

Hardware specification

RP2040/PIO_USB

Is your feature request related to a problem?

I am having some trouble with HID host unplug events on RP2040/PIO_USB as described in #3296

Part of the problem is that the HID host driver has quite limited error reporting. Especially, the result code is simply ignored:

https://github.com/hathach/tinyusb/blob/79445c2386adefb207a76a70d87578b53c3e7922/src/class/hid/hid_host.c#L475

As a result, the application cannot distinguish between errors and regular zero-length-packets which some HID devices like the Competition Pro USB Joystick actually sends.

Describe the solution you'd like

I think it'd be quite helpful to be able to distinguish between regular zero-byte-replies and errors to e.g. not flood the stack with further requests in an error state. This in turn contributes to the HUB communication problems in #3296

IMHO, a clean solution would be to return more information, but this would unfortunately break the existing API.

The length is returned through an unsigned value:

https://github.com/hathach/tinyusb/blob/79445c2386adefb207a76a70d87578b53c3e7922/src/class/hid/hid_host.c#L487

As a result, it's not possible to e.g. report errors through negative values.

I'd therefore suggest returning a zero pointer in case of an error like so:

tuh_hid_report_received_cb(daddr, idx, (XFER_RESULT_SUCCESS == result)?epbuf->epin:NULL, (uint16_t) xferred_bytes);

This should not break any existing applications (unless they access the payload, even if they are told there isn't any).

I have checked existing issues, discussion and documentation

  • [x] I confirm I have checked existing issues, discussion and documentation.

harbaum avatar Oct 16 '25 07:10 harbaum

Alternately, one could return a pointer to the result itself in the error case like so:

tuh_hid_report_received_cb(daddr, idx, (XFER_RESULT_SUCCESS == result)?epbuf->epin:&result, (uint16_t) xferred_bytes);

This would allow to forward more information in the error case. The check on application side would then either check for len > 0 or for len == 0 and pointer pointing to XFER_RESULT_SUCCESS. In the long term additional information could be passed through that pointer without breaking the API.

harbaum avatar Oct 20 '25 09:10 harbaum