-
Notifications
You must be signed in to change notification settings - Fork 143
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
[driver] ADC ADS816x #796
[driver] ADC ADS816x #796
Conversation
rleh
commented
Dec 21, 2021
•
edited
Loading
edited
- Driver
- Example
- Test on real hardware
6ded6b1
to
67590d6
Compare
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.
Nice!
2dd9f18
to
b38a815
Compare
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.
Good, just some details to polish…
b38a815
to
177a53b
Compare
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.
Awesome!
using namespace std::chrono_literals; | ||
RF_BEGIN(); | ||
|
||
if (std::popcount(channelsBitmask) > N) { |
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.
Just a side-note, if you are ever implementing the auto conversion mode: The most sensible way to use span here would be a std::span<uint16_t>
with a dynamic size and then have result.size()
here instead of N
. That way it would be a normal runtime parameter for the size. The check is at runtime anyway.
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.
Thanks, I'd be fine with merging it as is.
177a53b
to
f5641c7
Compare
Removed the unnecessary |
f5641c7
to
3ba71c9
Compare