add optional device hid set protocol callback #225

ayedm1 · 2025-11-30T00:06:39Z

This callback invoked to notify the application side when host send to device a hid request to set protcol report/boot.

ux_device_class_hid_set_protocol_callback to allow applications to respond to protocol changes
Validates that protocol values must be 0 (boot) or 1 (report).

ayedm1 · 2025-12-07T20:22:01Z

Copilot

Pull request overview

This PR adds an optional callback mechanism to notify applications when the USB host sends a HID SET_PROTOCOL request to switch between boot protocol (keyboard/mouse simplified mode) and report protocol (full HID descriptor mode). The PR also adds validation to ensure GET_PROTOCOL and SET_PROTOCOL requests are only handled for boot subclass devices (bInterfaceSubClass = 0x01), as required by the HID specification.

Key changes:

Adds ux_device_class_hid_set_protocol_callback to allow applications to respond to protocol changes
Implements validation for GET_PROTOCOL and SET_PROTOCOL to ensure they only work for boot subclass devices
Validates that protocol values must be 0 (boot) or 1 (report)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
common/usbx_device_classes/inc/ux_device_class_hid.h	Adds callback function pointer declarations to both the parameter structure and HID class structure, with inline comment documenting the callback's purpose
common/usbx_device_classes/src/ux_device_class_hid_initialize.c	Stores the callback pointer from initialization parameters into the HID instance; also includes whitespace cleanup (trailing space removal)
common/usbx_device_classes/src/ux_device_class_hid_control_request.c	Implements validation for GET_PROTOCOL and SET_PROTOCOL requests, invokes the optional callback when protocol changes, and updates protocol state; includes whitespace cleanup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

common/usbx_device_classes/inc/ux_device_class_hid.h

Copilot · 2026-01-06T16:28:32Z

common/usbx_device_classes/src/ux_device_class_hid_control_request.c

+            /* Only boot subclass devices should respond to GET_PROTOCOL.  */
+            if ((hid->ux_slave_class_hid_interface == UX_NULL) ||
+                (hid->ux_slave_class_hid_interface->ux_slave_interface_descriptor.bInterfaceSubClass != 0x01))
+            {
+                return (UX_ERROR);
+            }


This PR adds validation for GET_PROTOCOL to ensure it only responds for boot subclass devices (bInterfaceSubClass = 0x01). While this is correct per the HID specification, it's not mentioned in the PR description which only describes adding a callback for SET_PROTOCOL. This is an additional functional change that should be documented in the PR description.

Copilot · 2026-01-06T16:28:33Z

common/usbx_device_classes/src/ux_device_class_hid_control_request.c

+            /* If there is a callback defined by the application, send the protocol to it.  */
+            if (hid -> ux_device_class_hid_set_protocol_callback != UX_NULL)
+                hid -> ux_device_class_hid_set_protocol_callback(hid, request_value);
+
            /* Accept the protocol.  */
            hid -> ux_device_class_hid_protocol = request_value;
+


The callback is invoked before the protocol value is stored in the hid structure. This means when the application callback executes, hid->ux_device_class_hid_protocol still contains the old protocol value, not the new one being set. This creates an inconsistent state where the parameter passed to the callback (request_value) differs from what the application would read from the hid structure.

The protocol should be stored before invoking the callback, so the application sees a consistent state. Move line 242 to before line 238.

Suggested change

/* If there is a callback defined by the application, send the protocol to it. */

if (hid -> ux_device_class_hid_set_protocol_callback != UX_NULL)

hid -> ux_device_class_hid_set_protocol_callback(hid, request_value);

/* Accept the protocol. */

hid -> ux_device_class_hid_protocol = request_value;

/* Accept the protocol. */

hid -> ux_device_class_hid_protocol = request_value;

/* If there is a callback defined by the application, send the protocol to it. */

if (hid -> ux_device_class_hid_set_protocol_callback != UX_NULL)

hid -> ux_device_class_hid_set_protocol_callback(hid, request_value);

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-08T12:30:55Z

common/usbx_device_classes/src/ux_device_class_hid_control_request.c

+            /* If there is a callback defined by the application, send the protocol to it.  */
+            if (hid -> ux_device_class_hid_set_protocol_callback != UX_NULL)
+                hid -> ux_device_class_hid_set_protocol_callback(hid, request_value);


The new callback invocation lacks test coverage. The existing test file does not set up or verify the ux_device_class_hid_set_protocol_callback. Consider adding test coverage to verify that the callback is properly invoked when a SET_PROTOCOL request is received.

Copilot · 2026-01-08T12:30:55Z

common/usbx_device_classes/src/ux_device_class_hid_control_request.c

+            /* Only boot subclass devices should respond to GET_PROTOCOL.  */
+            if ((hid->ux_slave_class_hid_interface == UX_NULL) ||
+                (hid->ux_slave_class_hid_interface->ux_slave_interface_descriptor.bInterfaceSubClass != 0x01))
+            {
+                return (UX_ERROR);
+            }


The new validation for GET_PROTOCOL will break existing tests. The test file usbx_ux_device_class_hid_control_request_test.c uses a device with bInterfaceSubClass set to 0x00 (not 0x01 for boot subclass), which means GET_PROTOCOL requests will now return UX_ERROR and fail the test. The test at lines 558-579 expects GET_PROTOCOL to succeed. Either the test needs to be updated to use a boot subclass device (change bInterfaceSubClass to 0x01 in the device framework), or the validation logic needs to be reconsidered if non-boot devices should also support these commands.

Copilot · 2026-01-08T12:30:55Z

common/usbx_device_classes/src/ux_device_class_hid_control_request.c

+            /* Only boot subclass devices are expected to handle SET_PROTOCOL.  */
+            if ((hid->ux_slave_class_hid_interface == UX_NULL) ||
+                (hid->ux_slave_class_hid_interface->ux_slave_interface_descriptor.bInterfaceSubClass != 0x01))
+            {
+                /* Not a boot subclass: not handled. */
+                return (UX_ERROR);
+            }


The new validation for SET_PROTOCOL will break existing tests. The test file usbx_ux_device_class_hid_control_request_test.c uses a device with bInterfaceSubClass set to 0x00 (not 0x01 for boot subclass), which means SET_PROTOCOL requests will now return UX_ERROR and fail the test. The test at lines 585-609 expects SET_PROTOCOL to succeed. Either the test needs to be updated to use a boot subclass device (change bInterfaceSubClass to 0x01 in the device framework), or the validation logic needs to be reconsidered if non-boot devices should also support these commands.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-08T12:41:47Z

common/usbx_device_classes/src/ux_device_class_hid_control_request.c

+            /* Check protocol must be 0 (Boot) or 1 (Report).  */
+            if ((request_value != UX_DEVICE_CLASS_HID_PROTOCOL_BOOT) &&
+                (request_value != UX_DEVICE_CLASS_HID_PROTOCOL_REPORT))
+            {
+                /* Invalid value: not handled. */
+                return (UX_ERROR);
+            }
+
            /* Accept the protocol.  */
            hid -> ux_device_class_hid_protocol = request_value;
+
+            /* If there is a callback defined by the application, send the protocol to it.  */
+            if (hid -> ux_device_class_hid_set_protocol_callback != UX_NULL)
+                hid -> ux_device_class_hid_set_protocol_callback(hid, request_value);


The new callback functionality for set protocol requests lacks test coverage. The existing test file "usbx_ux_device_class_hid_control_request_test.c" tests SET_PROTOCOL requests but does not verify that the new callback is invoked correctly when set. Consider adding test cases that:

Verify the callback is called with the correct protocol value (both boot and report)

Verify the callback receives the correct HID instance pointer

Verify that when the callback is NULL, the code still functions correctly (this is already handled but should be tested)

ayedm1 changed the title ~~add optional hid set protocol callback~~ add optional device hid set protocol callback Nov 30, 2025

ayedm1 marked this pull request as ready for review November 30, 2025 00:27

ayedm1 force-pushed the add_set_protocol_callback branch 6 times, most recently from 58d7d83 to 69f7857 Compare December 2, 2025 00:57

fdesbiens requested a review from rahmanih December 2, 2025 16:45

fdesbiens added this to ThreadX Roadmap Dec 2, 2025

fdesbiens moved this to In review in ThreadX Roadmap Dec 2, 2025

ayedm1 requested a review from Copilot January 6, 2026 16:24

Copilot started reviewing on behalf of ayedm1 January 6, 2026 16:24 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

ayedm1 requested a review from ABOUSTM January 7, 2026 10:00

ayedm1 force-pushed the add_set_protocol_callback branch from 69f7857 to 14f1281 Compare January 8, 2026 12:26

ayedm1 requested a review from Copilot January 8, 2026 12:27

Copilot started reviewing on behalf of ayedm1 January 8, 2026 12:27 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

add optional hid set protocol callback

cd5f682

ayedm1 force-pushed the add_set_protocol_callback branch from 14f1281 to cd5f682 Compare January 8, 2026 12:38

ayedm1 requested a review from Copilot January 8, 2026 12:39

Copilot started reviewing on behalf of ayedm1 January 8, 2026 12:39 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

ayedm1 marked this pull request as draft January 8, 2026 20:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

add optional device hid set protocol callback #225

add optional device hid set protocol callback #225

ayedm1 commented Nov 30, 2025 •

edited

Loading

Uh oh!

ayedm1 commented Dec 7, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

add optional device hid set protocol callback #225

Are you sure you want to change the base?

add optional device hid set protocol callback #225

Conversation

ayedm1 commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ayedm1 commented Dec 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ayedm1 commented Nov 30, 2025 •

edited

Loading