From cef6158c31794795f16099e3df4c9049b976dae6 Mon Sep 17 00:00:00 2001 From: huntc Date: Thu, 7 Oct 2021 18:00:03 +1100 Subject: [PATCH 1/6] Extend SAADC one shot support One-shot mode now permits the sampling of differential pins, and the sampling of multiple pins simultaneously. A new ChannelConfig structure has been introduced so that multiple channels can be configured individually. Further, the `sample` method now accepts a buffer into which samples are written. Along the way, I've reset some default configuration to align with Nordic's settings in their nrfx saadc driver. Specifically, the channel gain defaults to 6 (from 4) and the time defaults to 10us (from 20us). --- README.md | 2 + embassy-nrf/src/saadc.rs | 177 ++++++++++++++++++++++++++-------- examples/nrf/src/bin/saadc.rs | 10 +- 3 files changed, 145 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 13bcf6e5..f8898dcd 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,8 @@ The `embassy-nrf` crate contains implementations for nRF 52 series SoCs. - `uarte`: UARTE driver implementing `AsyncBufRead` and `AsyncWrite`. - `qspi`: QSPI driver implementing `Flash`. - `gpiote`: GPIOTE driver. Allows `await`ing GPIO pin changes. Great for reading buttons or receiving interrupts from external chips. +- `saadc`: SAADC driver. Provides a full implementation of the one-shot sampling for analog channels. + - `rtc`: RTC driver implementing `Clock` and `Alarm`, for use with `embassy::executor`. ## Examples diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index b6e8f4e4..12c302d5 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -1,3 +1,4 @@ +use core::convert::TryInto; use core::marker::PhantomData; use core::sync::atomic::{compiler_fence, Ordering}; use core::task::Poll; @@ -15,6 +16,7 @@ use pac::{saadc, SAADC}; pub use saadc::{ ch::{ config::{GAIN_A as Gain, REFSEL_A as Reference, RESP_A as Resistor, TACQ_A as Time}, + pseln::PSELN_A as NegativeChannel, pselp::PSELP_A as PositiveChannel, }, oversample::OVERSAMPLE_A as Oversample, @@ -27,7 +29,7 @@ pub use saadc::{ pub enum Error {} /// One-shot saadc. Continuous sample mode TODO. -pub struct OneShot<'d> { +pub struct OneShot<'d, const N: usize> { phantom: PhantomData<&'d mut peripherals::SAADC>, } @@ -36,11 +38,29 @@ static WAKER: AtomicWaker = AtomicWaker::new(); /// Used to configure the SAADC peripheral. /// /// See the `Default` impl for suitable default values. +#[non_exhaustive] pub struct Config { /// Output resolution in bits. pub resolution: Resolution, /// Average 2^`oversample` input samples before transferring the result into memory. pub oversample: Oversample, +} + +impl Default for Config { + /// Default configuration for single channel sampling. + fn default() -> Self { + Self { + resolution: Resolution::_14BIT, + oversample: Oversample::BYPASS, + } + } +} + +/// Used to configure an individual SAADC peripheral channel. +/// +/// See the `Default` impl for suitable default values. +#[non_exhaustive] +pub struct ChannelConfig { /// Reference voltage of the SAADC input. pub reference: Reference, /// Gain used to control the effective input range of the SAADC. @@ -49,26 +69,48 @@ pub struct Config { pub resistor: Resistor, /// Acquisition time in microseconds. pub time: Time, + /// Positive channel to sample + p_channel: PositiveChannel, + /// An optional negative channel to sample + n_channel: Option, } -impl Default for Config { - fn default() -> Self { +impl ChannelConfig { + /// Default configuration for single ended channel sampling. + pub fn single_ended(pin: impl Unborrow) -> Self { + unborrow!(pin); Self { - resolution: Resolution::_14BIT, - oversample: Oversample::OVER8X, - reference: Reference::VDD1_4, - gain: Gain::GAIN1_4, + reference: Reference::INTERNAL, + gain: Gain::GAIN1_6, resistor: Resistor::BYPASS, - time: Time::_20US, + time: Time::_10US, + p_channel: pin.channel(), + n_channel: None, + } + } + /// Default configuration for differential channel sampling. + pub fn differential( + ppin: impl Unborrow, + npin: impl Unborrow, + ) -> Self { + unborrow!(ppin, npin); + Self { + reference: Reference::VDD1_4, + gain: Gain::GAIN1_6, + resistor: Resistor::BYPASS, + time: Time::_10US, + p_channel: ppin.channel(), + n_channel: Some(npin.channel()), } } } -impl<'d> OneShot<'d> { +impl<'d, const N: usize> OneShot<'d, N> { pub fn new( _saadc: impl Unborrow + 'd, irq: impl Unborrow + 'd, config: Config, + channel_configs: [ChannelConfig; N], ) -> Self { unborrow!(irq); @@ -77,31 +119,37 @@ impl<'d> OneShot<'d> { let Config { resolution, oversample, - reference, - gain, - resistor, - time, } = config; - // Configure pins + // Configure channels r.enable.write(|w| w.enable().enabled()); r.resolution.write(|w| w.val().variant(resolution)); r.oversample.write(|w| w.oversample().variant(oversample)); - r.ch[0].config.write(|w| { - w.refsel().variant(reference); - w.gain().variant(gain); - w.tacq().variant(time); - w.mode().se(); - w.resp().variant(resistor); - w.resn().bypass(); - if !matches!(oversample, Oversample::BYPASS) { - w.burst().enabled(); - } else { - w.burst().disabled(); + for (i, cc) in channel_configs.iter().enumerate() { + r.ch[i].pselp.write(|w| w.pselp().variant(cc.p_channel)); + if let Some(npin) = cc.n_channel.as_ref() { + r.ch[i].pseln.write(|w| w.pseln().variant(*npin)); } - w - }); + r.ch[i].config.write(|w| { + w.refsel().variant(cc.reference); + w.gain().variant(cc.gain); + w.tacq().variant(cc.time); + if cc.n_channel.is_none() { + w.mode().se(); + } else { + w.mode().diff(); + } + w.resp().variant(cc.resistor); + w.resn().bypass(); + if !matches!(oversample, Oversample::BYPASS) { + w.burst().enabled(); + } else { + w.burst().disabled(); + } + w + }); + } // Disable all events interrupts r.intenclr.write(|w| unsafe { w.bits(0x003F_FFFF) }); @@ -128,18 +176,16 @@ impl<'d> OneShot<'d> { unsafe { &*SAADC::ptr() } } - pub async fn sample(&mut self, pin: &mut impl PositivePin) -> i16 { + pub async fn sample(&mut self, buf: &mut [i16; N]) { let r = Self::regs(); - // Set positive channel - r.ch[0].pselp.write(|w| w.pselp().variant(pin.channel())); - // Set up the DMA - let mut val: i16 = 0; r.result .ptr - .write(|w| unsafe { w.ptr().bits(((&mut val) as *mut _) as u32) }); - r.result.maxcnt.write(|w| unsafe { w.maxcnt().bits(1) }); + .write(|w| unsafe { w.ptr().bits(buf.as_mut_ptr() as u32) }); + r.result + .maxcnt + .write(|w| unsafe { w.maxcnt().bits(N.try_into().unwrap()) }); // Reset and enable the end event r.events_end.reset(); @@ -166,13 +212,10 @@ impl<'d> OneShot<'d> { Poll::Pending }) .await; - - // The DMA wrote the sampled value to `val`. - val } } -impl<'d> Drop for OneShot<'d> { +impl<'d, const N: usize> Drop for OneShot<'d, N> { fn drop(&mut self) { let r = Self::regs(); r.enable.write(|w| w.enable().disabled()); @@ -180,8 +223,6 @@ impl<'d> Drop for OneShot<'d> { } /// A pin that can be used as the positive end of a ADC differential in the SAADC periperhal. -/// -/// Currently negative is always shorted to ground (0V). pub trait PositivePin { fn channel(&self) -> PositiveChannel; } @@ -198,6 +239,38 @@ macro_rules! positive_pin_mappings { }; } +/// A pin that can be used as the negative end of a ADC differential in the SAADC periperhal. +pub trait NegativePin { + fn channel(&self) -> NegativeChannel; +} + +macro_rules! negative_pin_mappings { + ( $($ch:ident => $pin:ident,)*) => { + $( + impl NegativePin for crate::peripherals::$pin { + fn channel(&self) -> NegativeChannel { + NegativeChannel::$ch + } + } + )* + }; +} + +/// Represents an unconnected pin +pub struct NotConnected {} + +impl PositivePin for NotConnected { + fn channel(&self) -> PositiveChannel { + PositiveChannel::NC + } +} + +impl NegativePin for NotConnected { + fn channel(&self) -> NegativeChannel { + NegativeChannel::NC + } +} + // TODO the variant names are unchecked // the pins are copied from nrf hal #[cfg(feature = "9160")] @@ -223,3 +296,27 @@ positive_pin_mappings! { ANALOGINPUT6 => P0_30, ANALOGINPUT7 => P0_31, } + +#[cfg(feature = "9160")] +negative_pin_mappings! { + ANALOGINPUT0 => P0_13, + ANALOGINPUT1 => P0_14, + ANALOGINPUT2 => P0_15, + ANALOGINPUT3 => P0_16, + ANALOGINPUT4 => P0_17, + ANALOGINPUT5 => P0_18, + ANALOGINPUT6 => P0_19, + ANALOGINPUT7 => P0_20, +} + +#[cfg(not(feature = "9160"))] +negative_pin_mappings! { + ANALOGINPUT0 => P0_02, + ANALOGINPUT1 => P0_03, + ANALOGINPUT2 => P0_04, + ANALOGINPUT3 => P0_05, + ANALOGINPUT4 => P0_28, + ANALOGINPUT5 => P0_29, + ANALOGINPUT6 => P0_30, + ANALOGINPUT7 => P0_31, +} diff --git a/examples/nrf/src/bin/saadc.rs b/examples/nrf/src/bin/saadc.rs index c4d23360..d12717c0 100644 --- a/examples/nrf/src/bin/saadc.rs +++ b/examples/nrf/src/bin/saadc.rs @@ -7,18 +7,20 @@ mod example_common; use defmt::panic; use embassy::executor::Spawner; use embassy::time::{Duration, Timer}; -use embassy_nrf::saadc::{Config, OneShot}; +use embassy_nrf::saadc::{ChannelConfig, Config, OneShot}; use embassy_nrf::{interrupt, Peripherals}; use example_common::*; #[embassy::main] async fn main(_spawner: Spawner, mut p: Peripherals) { let config = Config::default(); - let mut saadc = OneShot::new(p.SAADC, interrupt::take!(SAADC), config); + let channel_config = ChannelConfig::single_ended(&mut p.P0_02); + let mut saadc = OneShot::new(p.SAADC, interrupt::take!(SAADC), config, [channel_config]); loop { - let sample = saadc.sample(&mut p.P0_02).await; - info!("sample: {=i16}", sample); + let mut buf = [0; 1]; + saadc.sample(&mut buf).await; + info!("sample: {=i16}", &buf[0]); Timer::after(Duration::from_millis(100)).await; } } From 5f5470a320c47f0cda207e515aaf36e4f63ac043 Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 11 Oct 2021 08:52:45 +1100 Subject: [PATCH 2/6] Need to borrow the pins for the lifetime of the config, and subsequently the one shot. --- embassy-nrf/src/saadc.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 12c302d5..d7ccd2fc 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -60,7 +60,7 @@ impl Default for Config { /// /// See the `Default` impl for suitable default values. #[non_exhaustive] -pub struct ChannelConfig { +pub struct ChannelConfig<'d> { /// Reference voltage of the SAADC input. pub reference: Reference, /// Gain used to control the effective input range of the SAADC. @@ -73,11 +73,13 @@ pub struct ChannelConfig { p_channel: PositiveChannel, /// An optional negative channel to sample n_channel: Option, + + phantom: PhantomData<&'d ()>, } -impl ChannelConfig { +impl<'d> ChannelConfig<'d> { /// Default configuration for single ended channel sampling. - pub fn single_ended(pin: impl Unborrow) -> Self { + pub fn single_ended(pin: impl Unborrow + 'd) -> Self { unborrow!(pin); Self { reference: Reference::INTERNAL, @@ -86,12 +88,13 @@ impl ChannelConfig { time: Time::_10US, p_channel: pin.channel(), n_channel: None, + phantom: PhantomData, } } /// Default configuration for differential channel sampling. pub fn differential( - ppin: impl Unborrow, - npin: impl Unborrow, + ppin: impl Unborrow + 'd, + npin: impl Unborrow + 'd, ) -> Self { unborrow!(ppin, npin); Self { @@ -101,6 +104,7 @@ impl ChannelConfig { time: Time::_10US, p_channel: ppin.channel(), n_channel: Some(npin.channel()), + phantom: PhantomData, } } } From 617a976e96282a103a30667be402da27dc4bbbb3 Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 11 Oct 2021 08:54:24 +1100 Subject: [PATCH 3/6] No need for unwrap --- embassy-nrf/src/saadc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index d7ccd2fc..d8ed5765 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -1,4 +1,3 @@ -use core::convert::TryInto; use core::marker::PhantomData; use core::sync::atomic::{compiler_fence, Ordering}; use core::task::Poll; @@ -189,7 +188,7 @@ impl<'d, const N: usize> OneShot<'d, N> { .write(|w| unsafe { w.ptr().bits(buf.as_mut_ptr() as u32) }); r.result .maxcnt - .write(|w| unsafe { w.maxcnt().bits(N.try_into().unwrap()) }); + .write(|w| unsafe { w.maxcnt().bits(N as _) }); // Reset and enable the end event r.events_end.reset(); From 25d6a2cd131257d83949424f7eebf47c88d95e0b Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 11 Oct 2021 08:56:53 +1100 Subject: [PATCH 4/6] No use case understood for NotConnected, so hiding it for now --- embassy-nrf/src/saadc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index d8ed5765..481f886e 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -259,8 +259,7 @@ macro_rules! negative_pin_mappings { }; } -/// Represents an unconnected pin -pub struct NotConnected {} +struct NotConnected {} impl PositivePin for NotConnected { fn channel(&self) -> PositiveChannel { From b043778f750f60f1361bb56f4b57e8ad56a13dc3 Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 11 Oct 2021 09:08:58 +1100 Subject: [PATCH 5/6] Removed the NotConnected as it isn't used. --- embassy-nrf/src/saadc.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 481f886e..b09b909d 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -259,20 +259,6 @@ macro_rules! negative_pin_mappings { }; } -struct NotConnected {} - -impl PositivePin for NotConnected { - fn channel(&self) -> PositiveChannel { - PositiveChannel::NC - } -} - -impl NegativePin for NotConnected { - fn channel(&self) -> NegativeChannel { - NegativeChannel::NC - } -} - // TODO the variant names are unchecked // the pins are copied from nrf hal #[cfg(feature = "9160")] From 8c9e50b37871368c31ed23208329fbe28bcc0a7e Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 11 Oct 2021 09:38:35 +1100 Subject: [PATCH 6/6] Conflates the negative and positive types as they are the same, and renames pin to input as they can be more than pins --- embassy-nrf/src/saadc.rs | 92 ++++++++++++---------------------------- 1 file changed, 26 insertions(+), 66 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index b09b909d..96116b36 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -15,8 +15,7 @@ use pac::{saadc, SAADC}; pub use saadc::{ ch::{ config::{GAIN_A as Gain, REFSEL_A as Reference, RESP_A as Resistor, TACQ_A as Time}, - pseln::PSELN_A as NegativeChannel, - pselp::PSELP_A as PositiveChannel, + pselp::PSELP_A as InputChannel, // We treat the positive and negative channels with the same enum values to keep our type tidy and given they are the same }, oversample::OVERSAMPLE_A as Oversample, resolution::VAL_A as Resolution, @@ -69,40 +68,40 @@ pub struct ChannelConfig<'d> { /// Acquisition time in microseconds. pub time: Time, /// Positive channel to sample - p_channel: PositiveChannel, + p_channel: InputChannel, /// An optional negative channel to sample - n_channel: Option, + n_channel: Option, phantom: PhantomData<&'d ()>, } impl<'d> ChannelConfig<'d> { /// Default configuration for single ended channel sampling. - pub fn single_ended(pin: impl Unborrow + 'd) -> Self { - unborrow!(pin); + pub fn single_ended(input: impl Unborrow + 'd) -> Self { + unborrow!(input); Self { reference: Reference::INTERNAL, gain: Gain::GAIN1_6, resistor: Resistor::BYPASS, time: Time::_10US, - p_channel: pin.channel(), + p_channel: input.channel(), n_channel: None, phantom: PhantomData, } } /// Default configuration for differential channel sampling. pub fn differential( - ppin: impl Unborrow + 'd, - npin: impl Unborrow + 'd, + p_input: impl Unborrow + 'd, + n_input: impl Unborrow + 'd, ) -> Self { - unborrow!(ppin, npin); + unborrow!(p_input, n_input); Self { reference: Reference::VDD1_4, gain: Gain::GAIN1_6, resistor: Resistor::BYPASS, time: Time::_10US, - p_channel: ppin.channel(), - n_channel: Some(npin.channel()), + p_channel: p_input.channel(), + n_channel: Some(n_input.channel()), phantom: PhantomData, } } @@ -131,8 +130,10 @@ impl<'d, const N: usize> OneShot<'d, N> { for (i, cc) in channel_configs.iter().enumerate() { r.ch[i].pselp.write(|w| w.pselp().variant(cc.p_channel)); - if let Some(npin) = cc.n_channel.as_ref() { - r.ch[i].pseln.write(|w| w.pseln().variant(*npin)); + if let Some(n_channel) = cc.n_channel { + r.ch[i] + .pseln + .write(|w| unsafe { w.pseln().bits(n_channel as u8) }); } r.ch[i].config.write(|w| { w.refsel().variant(cc.reference); @@ -225,34 +226,17 @@ impl<'d, const N: usize> Drop for OneShot<'d, N> { } } -/// A pin that can be used as the positive end of a ADC differential in the SAADC periperhal. -pub trait PositivePin { - fn channel(&self) -> PositiveChannel; +/// An input that can be used as either or negative end of a ADC differential in the SAADC periperhal. +pub trait Input { + fn channel(&self) -> InputChannel; } -macro_rules! positive_pin_mappings { - ( $($ch:ident => $pin:ident,)*) => { +macro_rules! input_mappings { + ( $($ch:ident => $input:ident,)*) => { $( - impl PositivePin for crate::peripherals::$pin { - fn channel(&self) -> PositiveChannel { - PositiveChannel::$ch - } - } - )* - }; -} - -/// A pin that can be used as the negative end of a ADC differential in the SAADC periperhal. -pub trait NegativePin { - fn channel(&self) -> NegativeChannel; -} - -macro_rules! negative_pin_mappings { - ( $($ch:ident => $pin:ident,)*) => { - $( - impl NegativePin for crate::peripherals::$pin { - fn channel(&self) -> NegativeChannel { - NegativeChannel::$ch + impl Input for crate::peripherals::$input { + fn channel(&self) -> InputChannel { + InputChannel::$ch } } )* @@ -260,9 +244,9 @@ macro_rules! negative_pin_mappings { } // TODO the variant names are unchecked -// the pins are copied from nrf hal +// the inputs are copied from nrf hal #[cfg(feature = "9160")] -positive_pin_mappings! { +input_mappings! { ANALOGINPUT0 => P0_13, ANALOGINPUT1 => P0_14, ANALOGINPUT2 => P0_15, @@ -274,31 +258,7 @@ positive_pin_mappings! { } #[cfg(not(feature = "9160"))] -positive_pin_mappings! { - ANALOGINPUT0 => P0_02, - ANALOGINPUT1 => P0_03, - ANALOGINPUT2 => P0_04, - ANALOGINPUT3 => P0_05, - ANALOGINPUT4 => P0_28, - ANALOGINPUT5 => P0_29, - ANALOGINPUT6 => P0_30, - ANALOGINPUT7 => P0_31, -} - -#[cfg(feature = "9160")] -negative_pin_mappings! { - ANALOGINPUT0 => P0_13, - ANALOGINPUT1 => P0_14, - ANALOGINPUT2 => P0_15, - ANALOGINPUT3 => P0_16, - ANALOGINPUT4 => P0_17, - ANALOGINPUT5 => P0_18, - ANALOGINPUT6 => P0_19, - ANALOGINPUT7 => P0_20, -} - -#[cfg(not(feature = "9160"))] -negative_pin_mappings! { +input_mappings! { ANALOGINPUT0 => P0_02, ANALOGINPUT1 => P0_03, ANALOGINPUT2 => P0_04,