8000 Remove begin/end methods to simplify the implementation by forderud · Pull Request #17 · abratchik/HIDPowerDevice · GitHub
[go: up one dir, main page]
Skip to content

Remove begin/end methods to simplify the implementation #17

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Remove begin/end methods to simplify the implementation #17

wants to merge 3 commits into from

Conversation

forderud
Copy link
Contributor
@forderud forderud commented Sep 9, 2024

Proposal to simplify the implementation by avoiding the need for manually calling begin() before using HIDPowerDevice_.

@forderud forderud changed the title Remove begin/end method to simplify the implementation Remove begin/end methods to simplify the implementation Sep 9, 2024
@abratchik
Copy link
Owner

conflict, needs to be resolved. Please note that HID.cpp is a core Arduino module

@forderud
Copy link
Contributor Author

conflict, needs to be resolved.

I've now rebased the PR to resolve the conflict.

Please note that HID.cpp is a core Arduino module

Does this mean that you want me to first submit a PR for updating the HID implementation in https://github.com/arduino/ArduinoCore-avr with the same change?

@forderud
Copy link
Contributor Author

I've proposed the same HID_ class changes to ArduinoCore-avr in arduino/ArduinoCore-avr#570

@abratchik
Copy link
Owner

conflict, needs to be resolved.

I've now rebased the PR to resolve the conflict.

Please note that HID.cpp is a core Arduino module

Does this mean that you want me to first submit a PR for updating the HID implementation in https://github.com/arduino/ArduinoCore-avr with the same change?

Hi Fredrik, what I mean is - it makes no sense to refactor HID.cpp in this repo because this is a core Arduino module. I only took it here because there was no other option to override the implementation of the HID protocol in the official Arduino library. The intent was to make only necessary changes and ensure they are mergeable with the original.
I saw you submitted a PR to ArduinoCore-avr, this is the right approach, I believe. The only problem is - Arduino core team is very conservative on accepting PRs, unfortunately.

@forderud forderud force-pushed the begin-end-rm branch 2 times, most recently from a8e26c1 to 86d976a Compare October 14, 2024 11:02
@forderud
Remove unreferenced and empty HIDPowerDevice_::end
2ce8035 
Proposal to simplify the implementation by removing dead code.
@forderud
8AF6 Remove empty HID_::begin method
6079ef1 
Proposal to simplify the implementation by removing dead code.
@forderud
Move HIDPowerDevice_::begin impl. to constructor
a6816b0 
Proposal to simplify the implementation by avoiding the need for manually calling begin() before using HIDPowerDevice_.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@forderud @abratchik
0