Skip to content

Change SendReport & sendDate "id" values from 16 to 8bits #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

forderud
Copy link
Contributor

@forderud forderud commented Jan 12, 2025

The underlying HID_::sendReport method is anyhow only transmitting the first byte of the "id" field, so the content of the second byte is currently ignored. One can then just as well change the "id" argument type from 16 to 8bits.

Update the HIDPowerDevice_::sendDate the same way, since it's just passing on the "id" argument to HID_::sendReport.

This also makes the HID class more similar to the upstream Arduino implementation on https://github.com/arduino/ArduinoCore-avr/blob/master/libraries/HID/src/HID.h#L96 that is already using uint8_t for HID report IDs. This is consistent with the HID specification, that states that ReportID values are 8bits.

@forderud forderud marked this pull request as draft January 20, 2025 09:37
The underlying HID_::sendReport method is anyhow only transmitting the first byte of the "id" field, so the content of the second byte is currently ignored. One can then just as well change the "id" argument type from 16 to 8bits.

Update the HIDPowerDevice_::sendDate the same way, since it's just passing on the "id" argument to HID_::sendReport.
@forderud forderud changed the title Change HID report "id" values from 16 to 8bits Change SendReport & sendDate "id" values from 16 to 8bits Jan 20, 2025
@forderud forderud marked this pull request as ready for review January 20, 2025 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant