How to abort request from within a tud_hid_get_report_cb callback?

Open Beginner friendly
  • Is something wrong?
#3,814 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
65/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
c
Domain
embedded-iot

Research direction

Read class/hid/hid_device.c alongside the callback contract in hid_device.h, focusing on how GET_REPORT responses handle a requested report ID and a zero callback return. Trace the control-request path and its stall behavior; done means returning zero reliably stalls the request whether or not the host specified a report ID. No test file is named, so first inspect the HID tests or run the relevant existing HID tests.

Written by the indexing model from the issue text.

Description

Feature 💡
Related area

class/hid/hid_device.c

Hardware specification

Probably not, I tested on an nRF52840

Is your feature request related to a problem?

This might be related to #2929.

While working on https://github.com/safijari/openpuck/pull/230 I ran into a problem with how tinyusb handles the tud_hid_get_report_cb callback for GET_REPORT requests.

For my application, I need to be able to fail a get_report request, returning some error to the host and have the host retry the request again a couple milliseconds later.

However, there seems to be no clean way to do that.

In hid_device.h, there's the following comment:

// Invoked when received GET_REPORT control request
// Application must fill buffer report's content and return its length.
// Return zero will cause the stack to STALL request
uint16_t tud_hid_get_report_cb(uint8_t instance, uint8_t report_id, hid_report_type_t report_type, uint8_t* buffer, uint16_t reqlen);

However, this seems to not be entirely correct. Looking at the following code in hid_device.c ...

          uint8_t* report_buf = p_epbuf->ctrl;
          uint16_t req_len = tu_min16(request->wLength, CFG_TUD_HID_EP_BUFSIZE);
          uint16_t xferlen = 0;

          // If host request a specific Report ID, add ID to as 1 byte of response
          if ((report_id != HID_REPORT_TYPE_INVALID) && (req_len > 1)) {
            *report_buf++ = report_id;
            req_len--;
            xferlen++;
          }

          xferlen += tud_hid_get_report_cb(hid_itf, report_id, (hid_report_type_t) report_type, report_buf, req_len);
          TU_ASSERT(xferlen > 0);

If I return 0 from tud_hid_get_report_cb as the comment in the header tells me to, then 0 will be added to xferlen. However, if the host has requested a specific report ID, xferlen has already been set to 1 in the if statement before. Given that the return type of tud_hid_get_report_cb is uint16 and not int16, there seems to be no reliable way to make a GET_REPORT request fail from within the callback function, as the callback function can only ever return positive values (or 0) and thus can never run into the assert if xferlen has been incremented before.

If I return 0 from the function, then the device will respond with a 1-byte data block with just the report ID in it. IMO this (adding the report ID to the response) should only happen if the function didn't return 0.

I would need to check, within my function, if a report ID was provided. If not, I can return 0. If it was, I'd need to return 0xFFFF so that when the code adds 1 to it, it overflows back to 0. Which sounds like a terrible hack.

Describe the solution you'd like

A clear, concise way to stall a GET_REPORT request, like mentioned in the header file, that also works if the host requests a specific report ID.

I have checked existing issues, dicussion and documentation
  • I confirm I have checked existing issues, dicussion and documentation.
Dominant language
C
Stars
7.3k
Forks
1.6k
Avg merge
2d 15h
Merged PRs (30d)
88

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from hathach/tinyusb

All issues in hathach/tinyusb

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.