You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There is a C standards conformance problem with the macros in class/hid/hid.h and negative 16-bit numbers. This makes it awkward to naturally specify negative numbers in HID report descriptors using the TinyUSB macros. These negative numbers often appear in the HID report descriptors of interfaces such as mice and joysticks. The existing templates in TinyUSB only use 8-bit values, and encode them as the equivalent positive hexadecimal numbers. I am converting some existing report descriptors that use 16-bit negative numbers.
Currently, using a macro like
HID_LOGICAL_MIN_N(-32767, 2)
in a report descriptor will result in a right shift of a negative number, which is implementation-defined in C. The expansion goes through the macros U16_TO_U8S_LE and TU_U16_HIGH in tusb_common.h, which do not cast their argument to an unsigned type. Interestingly, the TU_U32_BYTE macros do cast their argument to uint32_t.
I'm not currently aware of any compilers that produce incorrect code as a result of that standards provision. I think it's still better to produce an expression that has a uniquely-defined semantic according to the standards.
Describe the solution you'd like
Add casts to ensure that TU_U16_HIGH does not produce a right-shift of a negative number.
I can contribute a PR, but I'd like to know if the preferred solution is adding casts to the tusb_common.h macros, or instead to the class/hid/hid.h macros.
I have checked existing issues, dicussion and documentation
I confirm I have checked existing issues, dicussion and documentation.
The text was updated successfully, but these errors were encountered:
Disable some TInyUSB definitions that we don't use (yet) and/or conflict
with existing definitions in KeyboardioHID.
Also import some other TinyUSB support macros, with minor fixes. The
negative number issue with 16-bit report items is now
hathach/tinyusb#2425
tlyu
added a commit
to tlyu/Kaleidoscope
that referenced
this issue
Jan 24, 2024
Disable some TInyUSB definitions that we don't use (yet) and/or conflict
with existing definitions in KeyboardioHID.
Also import some other TinyUSB support macros, with minor fixes. The
negative number issue with 16-bit report items is now
hathach/tinyusb#2425
Signed-off-by: Taylor Yu <code@argon.blue>
Related area
HID report descriptors
Hardware specification
N/A
Is your feature request related to a problem?
There is a C standards conformance problem with the macros in
class/hid/hid.h
and negative 16-bit numbers. This makes it awkward to naturally specify negative numbers in HID report descriptors using the TinyUSB macros. These negative numbers often appear in the HID report descriptors of interfaces such as mice and joysticks. The existing templates in TinyUSB only use 8-bit values, and encode them as the equivalent positive hexadecimal numbers. I am converting some existing report descriptors that use 16-bit negative numbers.Currently, using a macro like
in a report descriptor will result in a right shift of a negative number, which is implementation-defined in C. The expansion goes through the macros
U16_TO_U8S_LE
andTU_U16_HIGH
intusb_common.h
, which do not cast their argument to an unsigned type. Interestingly, theTU_U32_BYTE
macros do cast their argument touint32_t
.I'm not currently aware of any compilers that produce incorrect code as a result of that standards provision. I think it's still better to produce an expression that has a uniquely-defined semantic according to the standards.
Describe the solution you'd like
Add casts to ensure that
TU_U16_HIGH
does not produce a right-shift of a negative number.I can contribute a PR, but I'd like to know if the preferred solution is adding casts to the
tusb_common.h
macros, or instead to theclass/hid/hid.h
macros.I have checked existing issues, dicussion and documentation
The text was updated successfully, but these errors were encountered: