Add Host HID APIs to set/get protocol and and optional device HID changes protocol callback#244
Merged
Conversation
rahmanih
requested changes
Feb 6, 2026
Contributor
rahmanih
left a comment
rahmanih
left a comment
There was a problem hiding this comment.
IIRC, this PR implements 2 features in HID Device and Host Classes.
please split them in separate commits.
Contributor
Author
in the simulator test, i test the callback with new apis |
Contributor
|
the regression test updates can be made in a separate commit too. |
rahmanih
changed the title
rahmanih
requested changes
Feb 24, 2026
ayedm1
force-pushed
the
add_set_get_host_hid_api
branch
4 times, most recently
from
0f90e31 to
b024452
Compare
ayedm1
force-pushed
the
add_set_get_host_hid_api
branch
from
b024452 to
6ea0ac4
Compare
Contributor
Author
|
How to test: 1- Update middleware
|
ayedm1
force-pushed
the
add_set_get_host_hid_api
branch
2 times, most recently
from
ea1f53c to
9032773
Compare
… set/get protocol This pull request adds host HID API functions to get and set the HID protocol (boot vs report mode) and introduces an optional callback for the device side when the protocol changes. - Added new host-side APIs ux_host_class_hid_protocol_set() and ux_host_class_hid_protocol_get() with error checking wrappers. - Added device-side optional callback ux_device_class_hid_set_protocol_callback that is invoked when the protocol changes - Added comprehensive test to verify the new protocol callback functionality
ayedm1
force-pushed
the
add_set_get_host_hid_api
branch
from
9032773 to
63a0d44
Compare
Contributor
|
@rahmanih: Please confirm whether I can merge this or not. Thanks. |
Contributor
|
@ABOUSTM Could you please provide your review. |
Three bugs fixed in the new _ux_host_class_hid_protocol_get/set functions: 1. protocol_get: use a cache-safe allocated buffer for the DMA transfer instead of the caller's USHORT* directly. The caller's stack memory is not guaranteed to be DMA-safe on cache-incoherent embedded targets, and writing 1 byte into a USHORT* yields incorrect results on big-endian platforms. Follows the same pattern as ux_host_class_hid_idle_get. 2. Both functions: acquire ux_device_protection_semaphore in RTOS mode before submitting the transfer, in addition to the HID instance semaphore. All other control transfer functions (idle_get, idle_set) take both semaphores. Missing the device semaphore allowed concurrent callers to corrupt the shared transfer_request fields. 3. Both functions: add proper standalone (UX_HOST_STANDALONE) locking using UX_DISABLE/UX_RESTORE to atomically check and set UX_HOST_CLASS_HID_FLAG_LOCK and UX_DEVICE_FLAG_LOCK, and set UX_TRANSFER_FLAG_AUTO_DEVICE_UNLOCK + UX_TRANSFER_STATE_RESET on the transfer request before initiating. Follows the idle_get inline standalone pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
I am merging into dev now, and this will ship with our Q2 2026 release next week. Please open a new PR if there are remaining issues with the code. |
fdesbiens
added a commit
that referenced
this pull request
Jun 4, 2026
Two issues discovered while reviewing and fixing thread-safety issues in the new ux_host_class_hid_protocol_get/set functions (PR #244). 1. idle_get.c: wrong operator acquires no lock in standalone mode Line 104 used '&= ~UX_HOST_CLASS_HID_FLAG_LOCK' (the release/clear operation) instead of '|= UX_HOST_CLASS_HID_FLAG_LOCK' (acquire/set). The preceding check correctly returns UX_BUSY when the flag is set, but the follow-on line then immediately clears it instead of setting it. The net effect is that the HID instance is never actually locked in UX_HOST_STANDALONE mode, making the mutual-exclusion check a no-op. 2. idle_set.c: standalone path used a blocking spin-loop The standalone branch called _ux_host_class_hid_idle_set_run() in a do/while loop, blocking the caller until the transfer completed. This is inconsistent with every other inline HID control-transfer function (idle_get, report_get, report_set, protocol_get, protocol_set) which all use the direct UX_DISABLE/UX_RESTORE atomic flag pattern and return UX_BUSY if the instance or device endpoint is already locked. Replaced with the same inline standalone locking pattern used by the other functions: atomic HID FLAG_LOCK acquire, atomic DEVICE_FLAG_LOCK acquire with AUTO_DEVICE_UNLOCK + UX_TRANSFER_STATE_RESET, and an AUTO_WAIT check at completion consistent with idle_get behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds host HID API functions to get and set the HID protocol (boot vs report mode) and introduces an optional callback for the device side when the protocol changes.