add optional hid set protocol callback #3
Conversation
|
update regression test to test host set/get api and device set protocol callback
ayedm1
force-pushed
the
add_set_protocol_callback
branch
from
988c15b to
828a0fb
Compare
…lass invoked when protocol change - add optional device hid callback ux_device_class_hid_parameter_set_protocol_callback invoked when protocol changes (boot/report). - add host hid api to set/get protocol. - update regression test to test host set/get api and device set protocol callback.
…/eclipse_usbx into add_set_protocol_callback
There was a problem hiding this comment.
Pull request overview
This pull request adds support for an optional HID set protocol callback, enabling applications to be notified when the USB HID protocol switches between Boot (0) and Report (1) modes. The implementation includes both host-side API functions for getting/setting the protocol and device-side callback infrastructure.
Changes:
- Added new host class APIs
ux_host_class_hid_protocol_set()andux_host_class_hid_protocol_get()for protocol management - Added optional device-side callback
ux_device_class_hid_parameter_set_protocol_callbackthat applications can register to be notified of protocol changes - Implemented comprehensive test coverage for the new callback functionality including NULL callback scenarios
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/regression/usbxtestcontrol.c | Registered new test case and removed trailing whitespace |
| test/regression/usbx_ux_device_class_hid_set_protocol_callback_test.c | New comprehensive test validating callback invocation and NULL callback handling |
| test/cmake/usbx/regression/CMakeLists.txt | Added new test to build configuration |
| common/usbx_host_classes/src/ux_host_class_hid_protocol_set.c | New host API to set HID protocol (Boot/Report) |
| common/usbx_host_classes/src/ux_host_class_hid_protocol_get.c | New host API to get current HID protocol |
| common/usbx_host_classes/inc/ux_host_class_hid.h | Added function declarations, macros, and protocol constants; fixed formatting |
| common/usbx_host_classes/CMakeLists.txt | Added new source files to build |
| common/usbx_device_classes/src/ux_device_class_hid_initialize.c | Initialize callback pointer from parameters; formatting fixes |
| common/usbx_device_classes/src/ux_device_class_hid_control_request.c | Added protocol validation and callback invocation in SET_PROTOCOL handler |
| common/usbx_device_classes/inc/ux_device_class_hid.h | Added callback field to structures and parameter struct |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| UINT (*ux_device_class_hid_parameter_callback)(struct UX_SLAVE_CLASS_HID_STRUCT *hid, UX_SLAVE_CLASS_HID_EVENT *); | ||
| UINT (*ux_device_class_hid_parameter_get_callback)(struct UX_SLAVE_CLASS_HID_STRUCT *hid, UX_SLAVE_CLASS_HID_EVENT *); | ||
|
|
||
| /* Optional callback invoked when protocol changes (boot/report). */ |
There was a problem hiding this comment.
The comment on line 370 describes the callback as being invoked when protocol changes between boot and report. However, it would be clearer to specify the exact values of the protocol parameter: "protocol is 0 for Boot or 1 for Report protocol" to match the USB HID specification.
| /* Optional callback invoked when protocol changes (boot/report). */ | |
| /* Optional callback invoked when protocol changes; protocol is 0 for Boot or 1 for Report. */ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #endif |
There was a problem hiding this comment.
Duplicate #endif directive at end of file. Line 1180 already closes the include guard, making line 1182 redundant and causing a preprocessing error.
| #endif |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Initialize HID class parameters, including set_protocol callback. */ | ||
| hid_parameter.ux_device_class_hid_parameter_report_address = hid_report_descriptor; | ||
| hid_parameter.ux_device_class_hid_parameter_report_length = HID_REPORT_LENGTH; | ||
| hid_parameter.ux_device_class_hid_parameter_callback = UX_NULL; /* Not used in this test. */ |
There was a problem hiding this comment.
The hid_parameter structure is not fully initialized. The field ux_device_class_hid_parameter_get_callback is not set, which could lead to undefined behavior if the structure is not guaranteed to be zero-initialized. While the global static hid_parameter from the common header will be zero-initialized, it's better practice to explicitly initialize all fields for clarity and maintainability, especially when some fields are being set.
| hid_parameter.ux_device_class_hid_parameter_callback = UX_NULL; /* Not used in this test. */ | |
| hid_parameter.ux_device_class_hid_parameter_callback = UX_NULL; /* Not used in this test. */ | |
| hid_parameter.ux_device_class_hid_parameter_get_callback = UX_NULL; /* Explicitly unused in this test. */ |
…add_set_protocol_callback
No description provided.