Add CFG_TUH_HID_SET_PROTOCOL_ON_ENUM configuration option#3558
Add CFG_TUH_HID_SET_PROTOCOL_ON_ENUM configuration option#3558
CFG_TUH_HID_SET_PROTOCOL_ON_ENUM configuration option#3558Conversation
hathach
commented
Mar 17, 2026
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e14abdc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/class/hid/hid_host.c
Outdated
| #if CFG_TUH_HID_SET_PROTOCOL_ON_ENUM | ||
| hidh_set_protocol(daddr, p_hid->itf_num, _hidh_default_protocol, process_set_config, CONFIG_GET_REPORT_DESC); | ||
| break; | ||
| #else | ||
| TU_ATTR_FALLTHROUGH; | ||
| #endif |
There was a problem hiding this comment.
Sync protocol_mode when skipping SET_PROTOCOL on enum
With CFG_TUH_HID_SET_PROTOCOL_ON_ENUM set to 0, this branch falls through to CONFIG_GET_REPORT_DESC without sending SET_PROTOCOL, but the interface state still keeps the preloaded _hidh_default_protocol value (default Boot). For Boot-subclass devices this causes tuh_hid_get_protocol() to report Boot while the device is actually still in Report protocol, so applications that rely on the reported mode can take the wrong path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a new HID host configuration option to control whether SET_PROTOCOL is issued during enumeration, allowing users to skip that step for devices that misbehave.
Changes:
- Introduced
CFG_TUH_HID_SET_PROTOCOL_ON_ENUM(default1) in the HID host public header. - Updated HID host enumeration state machine to optionally skip SET_PROTOCOL and fall through to report descriptor retrieval.
- Minor internal renames/cleanup (
_hidh_set_*->hidh_set_*) and small comment/formatting tweaks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/class/hid/hid_host.h |
Defines the new CFG_TUH_HID_SET_PROTOCOL_ON_ENUM configuration macro (default enabled). |
src/class/hid/hid_host.c |
Gates the enumeration-time SET_PROTOCOL step behind the new macro (with fallthrough to continue enumeration). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
src/class/hid/hid_host.c
Outdated
| #if CFG_TUH_HID_SET_PROTOCOL_ON_ENUM | ||
| hidh_set_protocol(daddr, p_hid->itf_num, _hidh_default_protocol, process_set_config, CONFIG_GET_REPORT_DESC); | ||
| break; | ||
| #else |
src/class/hid/hid_host.c
Outdated
| #if CFG_TUH_HID_SET_PROTOCOL_ON_ENUM | ||
| hidh_set_protocol(daddr, p_hid->itf_num, _hidh_default_protocol, process_set_config, CONFIG_GET_REPORT_DESC); | ||
| break; | ||
| #else | ||
| TU_ATTR_FALLTHROUGH; | ||
| #endif |
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in size
Changes <1% in sizeNo entries. No changes
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| lpcxpresso1769/hid_controller | 14,132 → 14,168 (+36) | — | — | — | 14,140 → 14,176 (+36) | +0.3% |
| ch32v203c_r0_1v0/hid_controller | 18,144 → 18,188 (+44) | — | — | — | 18,396 → 18,440 (+44) | +0.2% |
| ea4088_quickstart/hid_controller | 14,244 → 14,276 (+32) | — | — | — | 14,260 → 14,292 (+32) | +0.2% |
| metro_m4_express/hid_controller | 14,892 → 14,924 (+32) | — | — | — | 14,900 → 14,932 (+32) | +0.2% |
| lpcxpresso18s37/hid_controller | 16,672 → 16,708 (+36) | — | — | — | 17,110 → 17,146 (+36) | +0.2% |
| portenta_c33/hid_controller | 17,216 → 17,252 (+36) | — | — | — | 17,216 → 17,252 (+36) | +0.2% |
| metro_m7_1011/hid_controller | 19,312 → 19,348 (+36) | — | — | — | 19,320 → 19,356 (+36) | +0.2% |
| double_m33_express/hid_controller | 19,488 → 19,524 (+36) | — | — | — | 19,496 → 19,532 (+36) | +0.2% |
| ea4357/hid_controller | 17,920 → 17,952 (+32) | — | — | — | 18,366 → 18,398 (+32) | +0.2% |
| msp_exp432e401y/hid_controller | 18,820 → 18,852 (+32) | — | — | — | 18,840 → 18,872 (+32) | +0.2% |
…CONFIG_GET_REPORT_DESC response