Skip to content

HID: Replace manual defines with pluggedEndpoint member #36

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 2 commits into
base: master
Choose a base branch
from

Conversation

forderud
Copy link
Contributor

@forderud forderud commented Jan 11, 2025

Changes to simplify:

  1. Remove unused USB OUTPUT endpoint to simplify
  2. Replace manual defines with pluggedEndpoint member

Motivation for proposing the changes:

@forderud forderud force-pushed the pluggedEndpoint-adopt branch 3 times, most recently from ae7441c to 6057ae9 Compare January 12, 2025 16:57
@forderud
Copy link
Contributor Author

See #39 for an example of how multi-battery setups can be achieved after this PR is merged.

The HIDPowerDevice HID report descriptor only contains INPUT and FEATURE reports, and no OUTPUT reports. It therefore makes little sense to configure a USB input endpoint for INPUT reports.

This also makes the implementation more similar to the upstream Arduino code on https://github.com/arduino/ArduinoCore-avr/blob/master/libraries/HID/src/HID.cpp
Adopt coding style from https://github.com/arduino/ArduinoCore-avr/blob/master/libraries/HID/src/HID.cpp that uses "pluggedEndpoint" from the PluggableUSBModule base-class as endpoint number instead of a hardcoded preprocessor define. This allows removal of several defines in the header that are now unused.

Benefits:
* Closer alignment to the upstream Arduino sources.
* Fewer hardcoded values, which will simplify future scaling to support multiple batteries in a USB composite device.
@forderud forderud force-pushed the pluggedEndpoint-adopt branch from 6057ae9 to 9febd9f Compare January 19, 2025 10:10
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