-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add no_alloc compatibility #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Looks good, just one minor suggestion.
Seems QA action pipeline is currently not triggered by PR.
Could you please adjust
rt-PCA9539/.github/workflows/qa.yaml
Line 3 in 366bc6e
on: |
on:
push:
pull_request:
src/expander.rs
Outdated
), | ||
Bank::Bank0 => self | ||
.bus | ||
.write(self.address, &[COMMAND_CONF_0, self.configuration_0.as_value().clone()]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As u8 implements the Copy trait, no cloning is required, as also suggested by Clippy.
Please try to dereference/copy instead:
*self.configuration_0.as_value()
This also applies to lines 342, 351, 354, 363 and 366.
Thanks for the feedback! Let me know if any further changes are needed |
LGTM |
Should fix #2 -- I haven't run into any issues yet in my usage over the past month.