-
Notifications
You must be signed in to change notification settings - Fork 139
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
Differential ADC #233
base: master
Are you sure you want to change the base?
Differential ADC #233
Conversation
Looks like this needs a rebase |
Rebasing gave me some trouble. I hope merging is okay (I don't know what policy we have). If not, we can squash maybe. I was using this code since Sept/Oct for inductive battery charging and nothing exploded so far :P Sure it could use some extensions but I think the additional functionality warrants a merge :) |
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! Finally got around to reviewing this.
Saadc { saadc } | ||
} | ||
|
||
pub fn channel(&mut self, n: usize) -> Channel { |
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.
pub fn channel(&mut self, n: usize) -> Channel { | |
pub fn channel(&mut self, n: usize) -> Channel<'_> { | |
assert!(n < 8); |
} | ||
|
||
/// Sample channel `PIN` for the configured ADC acquisition time in differential input mode. | ||
/// Note that this is a blocking operation. |
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.
It should probably not return nb::Result
if it ends up blocking regardless
Variant::Val(Resolution::_8BIT) => 256, | ||
Variant::Val(Resolution::_10BIT) => 1024, | ||
Variant::Val(Resolution::_12BIT) => 4096, | ||
Variant::Val(Resolution::_14BIT) => 16384, |
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.
Variant::Val(Resolution::_8BIT) => 256, | |
Variant::Val(Resolution::_10BIT) => 1024, | |
Variant::Val(Resolution::_12BIT) => 4096, | |
Variant::Val(Resolution::_14BIT) => 16384, | |
Variant::Val(Resolution::_8BIT) => 1 << 8, | |
Variant::Val(Resolution::_10BIT) => 1 << 10, | |
Variant::Val(Resolution::_12BIT) => 1 << 12, | |
Variant::Val(Resolution::_14BIT) => 1 << 14, |
Ok(val) | ||
} | ||
|
||
pub fn as_micros(&self, value: i16) -> i32 { |
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.
Since we use raw integers everywhere (and not something like uom
), let's clarify that this returns microvolts and has nothing to do with Duration::as_micros
pub fn as_micros(&self, value: i16) -> i32 { | |
/// Converts a raw reading from this channel to µV. | |
pub fn to_microvolts(&self, value: i16) -> i32 { |
(value as i64 * 1000 * 1000 / lsbs * gain.1 * reference.0 / gain.0 / reference.1) as i32 | ||
} | ||
|
||
pub fn as_millis(&self, value: i16) -> i32 { |
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.
pub fn as_millis(&self, value: i16) -> i32 { | |
pub fn to_millivolts(&self, value: i16) -> i32 { |
} | ||
|
||
impl<'a> Channel<'a> { | ||
pub fn configure( |
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.
Does this have to be called for every channel that is used? What's the default state that would be used by the existing impls if this isn't called?
macro_rules! psel_mappings { | ||
( $($psel:ident => $pin:ident,)*) => { | ||
$( | ||
impl<STATE> Into<Pseln> for crate::gpio::p0::$pin<STATE> { |
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.
Is it okay to use pins in any state here, even Disconnected
?
Would be a very welcome addition.. Is this change still alive? |
I kind of forgot about this. Mostly because I do not work on this paid anymore and my sparetime is already full with other stuff. If you want to you are more than welcome to take this over :) |
This is just a preview on what the API could be like.
I use this successfully on my hardware I and I tested all of it (including the conversion methods) on a nRF52811 (Which is a mix of the 52810 and 52840 feature wise :)). Surely there will be corner cases as usual :/
IF you folks like it, I will try and clean it up a little.
Also, it would be cool if anyone has an idea how to integrate the
OneShot
trait.Currently the trait does only allow for a single input pin and you have to pass it everytime you start a read transaction, even tho that is not required.
This is why I have made a custom channel config API so you can configure the channel once and then just read.
I will extend this further such that the ADC multiplexer & sequencer can be used properly, even with multiple channels at once :)
Excited to hear your feedback :)