From 103a3305e2a7bb9ef87d72a1237cbc5237aa8156 Mon Sep 17 00:00:00 2001 From: huntc Date: Tue, 12 Oct 2021 11:24:26 +1100 Subject: [PATCH 01/15] Implements continuous sampling for the nRF SAADC Implements continuous sampling for the nRF SAADC and also renames `OneShot` to `Saadc`. The one-shot behaviour is retained with the `sample` method and a new `run_sampler` method is provided for efficiently (i.e. zero copying) sampler processing. A double buffer is used for continuously sampling, which wlll be swapped once sampling has taken place. A sample frequency is provided and will set the internal timer of the SAADC when there is just the one channel being sampled. Otherwise, PPI will be used to hook up the TIMER peripheral to drive the sampling task. --- embassy-nrf/src/saadc.rs | 145 ++++++++++++++++++++++- examples/nrf/src/bin/saadc.rs | 4 +- examples/nrf/src/bin/saadc_continuous.rs | 49 ++++++++ 3 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 examples/nrf/src/bin/saadc_continuous.rs diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 94744c444..b14e2156d 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -10,6 +10,7 @@ use embassy_hal_common::unborrow; use futures::future::poll_fn; use crate::interrupt; +use crate::ppi::Task; use crate::{pac, peripherals}; use pac::{saadc, SAADC}; @@ -29,7 +30,7 @@ pub use saadc::{ pub enum Error {} /// One-shot saadc. Continuous sample mode TODO. -pub struct OneShot<'d, const N: usize> { +pub struct Saadc<'d, const N: usize> { phantom: PhantomData<&'d mut peripherals::SAADC>, } @@ -50,7 +51,7 @@ impl Default for Config { /// Default configuration for single channel sampling. fn default() -> Self { Self { - resolution: Resolution::_14BIT, + resolution: Resolution::_12BIT, oversample: Oversample::BYPASS, } } @@ -109,7 +110,31 @@ impl<'d> ChannelConfig<'d> { } } -impl<'d, const N: usize> OneShot<'d, N> { +/// A sample rate can be provided for the internal clock of the SAADC when +/// one channel is to be continuously sampled. When there are multiple channels +/// to be continuously sampled then an external source is used to generate +/// TASK_SAMPLE tasks. +pub enum Mode { + /// The internal clock is to be used with a sample rate expressed as a divisor of + /// 16MHz, ranging from 80..2047. For example, 1600 represnts a sample rate of 10KHz + /// given 16_000_000 / 10_000_000 = 1600. + Timers(u16), + /// TASK_SAMPLE tasks are generated outside of the SAADC e.g. via PPI on a + /// timer. + Task, +} + +/// The state of a continuously running sampler. While it reflects +/// the progress of a sampler, it also signals what should be done +/// next. For example, if the sampler has stopped then the Saadc implementation +/// can then tear down its infrastructure. +#[derive(PartialEq)] +pub enum SamplerState { + Sampled, + Stopped, +} + +impl<'d, const N: usize> Saadc<'d, N> { pub fn new( _saadc: impl Unborrow + 'd, irq: impl Unborrow + 'd, @@ -176,12 +201,18 @@ impl<'d, const N: usize> OneShot<'d, N> { r.intenclr.write(|w| w.end().clear()); WAKER.wake(); } + + if r.events_started.read().bits() != 0 { + r.intenclr.write(|w| w.started().clear()); + WAKER.wake(); + } } fn regs() -> &'static saadc::RegisterBlock { unsafe { &*SAADC::ptr() } } + /// One shot sampling. The buffer must be the same size as the number of channels configured. pub async fn sample(&mut self, buf: &mut [i16; N]) { let r = Self::regs(); @@ -219,9 +250,115 @@ impl<'d, const N: usize> OneShot<'d, N> { }) .await; } + + /// Continuous sampling with double buffers. The sample buffers generally + /// should be a multiple of the number of channels configured. + /// + /// A sampler closure is provided that receives the buffer of samples, noting + /// that the size of this buffer can be less than the original buffer's size. + /// A command is return from the closure that indicates whether the sampling + /// should continue or stop. + pub async fn run_sampler( + &mut self, + bufs: &mut [[i16; N0]; 2], + mode: Mode, + sampler: S, + ) where + S: Fn(&[i16]) -> SamplerState, + { + let r = Self::regs(); + + // Establish mode and sample rate + match mode { + Mode::Timers(sample_rate) => r.samplerate.write(|w| { + unsafe { + w.cc().bits(sample_rate); + w.mode().timers(); + } + w + }), + Mode::Task => r.samplerate.write(|w| { + unsafe { + w.cc().bits(0); + w.mode().task(); + } + w + }), + } + + // Set up the initial DMA + r.result + .ptr + .write(|w| unsafe { w.ptr().bits(bufs[0].as_mut_ptr() as u32) }); + r.result + .maxcnt + .write(|w| unsafe { w.maxcnt().bits(N0 as _) }); + + // Reset and enable the events + r.events_end.reset(); + r.intenset.write(|w| w.end().set()); + r.events_started.reset(); + r.intenset.write(|w| w.started().set()); + + // Don't reorder the ADC start event before the previous writes. Hopefully self + // wouldn't happen anyway. + compiler_fence(Ordering::SeqCst); + + r.tasks_start.write(|w| unsafe { w.bits(1) }); + + let mut current_buffer = 0; + + // Wait for events and complete when the sampler indicates it has had enough. + poll_fn(|cx| { + let r = Self::regs(); + + WAKER.register(cx.waker()); + + if r.events_end.read().bits() != 0 { + r.events_end.reset(); + r.intenset.write(|w| w.end().set()); + info!("Ended"); + + if sampler(&bufs[current_buffer][0..r.result.amount.read().bits() as usize]) + == SamplerState::Sampled + { + let next_buffer = 1 - current_buffer; + current_buffer = next_buffer; + r.tasks_start.write(|w| unsafe { w.bits(1) }); + } else { + r.tasks_stop.write(|w| unsafe { w.bits(1) }); + return Poll::Ready(()); + }; + } + + if r.events_started.read().bits() != 0 { + r.events_started.reset(); + r.intenset.write(|w| w.started().set()); + info!("Started"); + + if let Mode::Timers(_) = mode { + r.tasks_sample.write(|w| unsafe { w.bits(1) }); + } + + let next_buffer = 1 - current_buffer; + r.result + .ptr + .write(|w| unsafe { w.ptr().bits(bufs[next_buffer].as_mut_ptr() as u32) }); + } + + Poll::Pending + }) + .await; + } + + /// Return the sample task for use with PPI + pub fn task_sample(&self) -> Task { + let r = Self::regs(); + Task::from_reg(&r.tasks_sample) + } } -impl<'d, const N: usize> Drop for OneShot<'d, N> { +impl<'d, const N: usize> Drop for Saadc<'d, N> { fn drop(&mut self) { let r = Self::regs(); r.enable.write(|w| w.enable().disabled()); diff --git a/examples/nrf/src/bin/saadc.rs b/examples/nrf/src/bin/saadc.rs index d12717c04..c6eac555b 100644 --- a/examples/nrf/src/bin/saadc.rs +++ b/examples/nrf/src/bin/saadc.rs @@ -7,7 +7,7 @@ mod example_common; use defmt::panic; use embassy::executor::Spawner; use embassy::time::{Duration, Timer}; -use embassy_nrf::saadc::{ChannelConfig, Config, OneShot}; +use embassy_nrf::saadc::{ChannelConfig, Config, Saadc}; use embassy_nrf::{interrupt, Peripherals}; use example_common::*; @@ -15,7 +15,7 @@ use example_common::*; async fn main(_spawner: Spawner, mut p: Peripherals) { let config = Config::default(); let channel_config = ChannelConfig::single_ended(&mut p.P0_02); - let mut saadc = OneShot::new(p.SAADC, interrupt::take!(SAADC), config, [channel_config]); + let mut saadc = Saadc::new(p.SAADC, interrupt::take!(SAADC), config, [channel_config]); loop { let mut buf = [0; 1]; diff --git a/examples/nrf/src/bin/saadc_continuous.rs b/examples/nrf/src/bin/saadc_continuous.rs new file mode 100644 index 000000000..fba00ebdf --- /dev/null +++ b/examples/nrf/src/bin/saadc_continuous.rs @@ -0,0 +1,49 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +#[path = "../example_common.rs"] +mod example_common; +use defmt::panic; +use embassy::executor::Spawner; +use embassy_nrf::ppi::Ppi; +use embassy_nrf::saadc::{ChannelConfig, Config, Mode, Saadc, SamplerState}; +use embassy_nrf::timer::{Frequency, Timer}; +use embassy_nrf::{interrupt, Peripherals}; +use example_common::*; + +// Demonstrates both continuous sampling and scanning multiple channels driven by a PPI linked timer + +#[embassy::main] +async fn main(_spawner: Spawner, mut p: Peripherals) { + let config = Config::default(); + let channel_1_config = ChannelConfig::single_ended(&mut p.P0_02); + let channel_2_config = ChannelConfig::single_ended(&mut p.P0_03); + let channel_3_config = ChannelConfig::single_ended(&mut p.P0_04); + let mut saadc = Saadc::new( + p.SAADC, + interrupt::take!(SAADC), + config, + [channel_1_config, channel_2_config, channel_3_config], + ); + + let mut timer = Timer::new(p.TIMER0); + timer.set_frequency(Frequency::F1MHz); + timer.cc(0).write(100); // We want to sample at 10KHz + timer.cc(0).short_compare_clear(); + + let mut ppi = Ppi::new(p.PPI_CH0); + ppi.set_event(timer.cc(0).event_compare()); + ppi.set_task(saadc.task_sample()); + ppi.enable(); + + timer.start(); + + let mut bufs = [[0; 3 * 500]; 2]; // Each buffer of the double buffer has to be large enough for all channels. + saadc + .run_sampler(&mut bufs, Mode::Task, |buf| { + info!("sample len={}", buf.len()); + SamplerState::Sampled + }) + .await; +} From 16b6c78332e7e00bc176480a69d1259f7f0f60f1 Mon Sep 17 00:00:00 2001 From: huntc Date: Thu, 14 Oct 2021 16:55:50 +1100 Subject: [PATCH 02/15] Removed debugging logs --- embassy-nrf/src/saadc.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index b14e2156d..039893d15 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -317,7 +317,6 @@ impl<'d, const N: usize> Saadc<'d, N> { if r.events_end.read().bits() != 0 { r.events_end.reset(); r.intenset.write(|w| w.end().set()); - info!("Ended"); if sampler(&bufs[current_buffer][0..r.result.amount.read().bits() as usize]) == SamplerState::Sampled @@ -334,7 +333,6 @@ impl<'d, const N: usize> Saadc<'d, N> { if r.events_started.read().bits() != 0 { r.events_started.reset(); r.intenset.write(|w| w.started().set()); - info!("Started"); if let Mode::Timers(_) = mode { r.tasks_sample.write(|w| unsafe { w.bits(1) }); From 34e9e8581914af6f91ac21de9f027ddb0b67e9c4 Mon Sep 17 00:00:00 2001 From: huntc Date: Fri, 15 Oct 2021 08:12:13 +1100 Subject: [PATCH 03/15] We can kick start the internal timer outside of the main loop --- embassy-nrf/src/saadc.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 039893d15..2a1059836 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -270,13 +270,16 @@ impl<'d, const N: usize> Saadc<'d, N> { // Establish mode and sample rate match mode { - Mode::Timers(sample_rate) => r.samplerate.write(|w| { - unsafe { - w.cc().bits(sample_rate); - w.mode().timers(); - } - w - }), + Mode::Timers(sample_rate) => { + r.samplerate.write(|w| { + unsafe { + w.cc().bits(sample_rate); + w.mode().timers(); + } + w + }); + r.tasks_sample.write(|w| unsafe { w.bits(1) }); // Need to kick-start the internal timer + } Mode::Task => r.samplerate.write(|w| { unsafe { w.cc().bits(0); @@ -334,10 +337,6 @@ impl<'d, const N: usize> Saadc<'d, N> { r.events_started.reset(); r.intenset.write(|w| w.started().set()); - if let Mode::Timers(_) = mode { - r.tasks_sample.write(|w| unsafe { w.bits(1) }); - } - let next_buffer = 1 - current_buffer; r.result .ptr From 3be274dc2a471bb837e53f06065a20ec48d445d5 Mon Sep 17 00:00:00 2001 From: huntc Date: Fri, 15 Oct 2021 17:44:23 +1100 Subject: [PATCH 04/15] We must allow the run handler to mutate state The handler may well need to close over and mutate state --- embassy-nrf/src/saadc.rs | 4 ++-- examples/nrf/src/bin/saadc_continuous.rs | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 2a1059836..56aa8d48c 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -262,9 +262,9 @@ impl<'d, const N: usize> Saadc<'d, N> { &mut self, bufs: &mut [[i16; N0]; 2], mode: Mode, - sampler: S, + mut sampler: S, ) where - S: Fn(&[i16]) -> SamplerState, + S: FnMut(&[i16]) -> SamplerState, { let r = Self::regs(); diff --git a/examples/nrf/src/bin/saadc_continuous.rs b/examples/nrf/src/bin/saadc_continuous.rs index fba00ebdf..8bbdca665 100644 --- a/examples/nrf/src/bin/saadc_continuous.rs +++ b/examples/nrf/src/bin/saadc_continuous.rs @@ -40,9 +40,24 @@ async fn main(_spawner: Spawner, mut p: Peripherals) { timer.start(); let mut bufs = [[0; 3 * 500]; 2]; // Each buffer of the double buffer has to be large enough for all channels. + + let mut c = 0; + let mut a: i32 = 0; + saadc - .run_sampler(&mut bufs, Mode::Task, |buf| { - info!("sample len={}", buf.len()); + .run_sampler(&mut bufs, Mode::Task, move |buf| { + for (i, b) in buf.iter().enumerate() { + if i % 3 == 0 { + a += *b as i32; + c += 1; + } + } + if c > 10000 { + a = a / c as i32; + info!("sample: {=i32}", a); + c = 0; + a = 0; + } SamplerState::Sampled }) .await; From fa82913bc3f227151c0ea574c135a54de0900279 Mon Sep 17 00:00:00 2001 From: huntc Date: Fri, 15 Oct 2021 18:30:53 +1100 Subject: [PATCH 05/15] We have to reduce the buffer size to cater for the number of channels to scan --- examples/nrf/src/bin/saadc_continuous.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/nrf/src/bin/saadc_continuous.rs b/examples/nrf/src/bin/saadc_continuous.rs index 8bbdca665..dfcb46171 100644 --- a/examples/nrf/src/bin/saadc_continuous.rs +++ b/examples/nrf/src/bin/saadc_continuous.rs @@ -39,7 +39,7 @@ async fn main(_spawner: Spawner, mut p: Peripherals) { timer.start(); - let mut bufs = [[0; 3 * 500]; 2]; // Each buffer of the double buffer has to be large enough for all channels. + let mut bufs = [[0; 3 * 50]; 2]; // Each buffer of the double buffer has to be large enough for all channels. let mut c = 0; let mut a: i32 = 0; @@ -54,7 +54,7 @@ async fn main(_spawner: Spawner, mut p: Peripherals) { } if c > 10000 { a = a / c as i32; - info!("sample: {=i32}", a); + info!("channel 1: {=i32}", a); c = 0; a = 0; } From e37a10ae99e7737fcd5881d5fd13d0c736846eef Mon Sep 17 00:00:00 2001 From: huntc Date: Fri, 15 Oct 2021 18:45:53 +1100 Subject: [PATCH 06/15] Hangover - should have set this to internal for a previous PR --- embassy-nrf/src/saadc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 56aa8d48c..11ed1f952 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -99,7 +99,7 @@ impl<'d> ChannelConfig<'d> { ) -> Self { unborrow!(p_input, n_input); Self { - reference: Reference::VDD1_4, + reference: Reference::INTERNAL, gain: Gain::GAIN1_6, resistor: Resistor::BYPASS, time: Time::_10US, From b6cacb98db9148013fbabc124a2d8e296232b2e0 Mon Sep 17 00:00:00 2001 From: huntc Date: Sun, 17 Oct 2021 06:26:06 +1100 Subject: [PATCH 07/15] Compact expression --- embassy-nrf/src/saadc.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 11ed1f952..3acfa50d7 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -271,20 +271,16 @@ impl<'d, const N: usize> Saadc<'d, N> { // Establish mode and sample rate match mode { Mode::Timers(sample_rate) => { - r.samplerate.write(|w| { - unsafe { - w.cc().bits(sample_rate); - w.mode().timers(); - } + r.samplerate.write(|w| unsafe { + w.cc().bits(sample_rate); + w.mode().timers(); w }); r.tasks_sample.write(|w| unsafe { w.bits(1) }); // Need to kick-start the internal timer } - Mode::Task => r.samplerate.write(|w| { - unsafe { - w.cc().bits(0); - w.mode().task(); - } + Mode::Task => r.samplerate.write(|w| unsafe { + w.cc().bits(0); + w.mode().task(); w }), } From a020b1a404c8e72d7b5404c595e57d8c04ac8701 Mon Sep 17 00:00:00 2001 From: huntc Date: Sun, 17 Oct 2021 06:28:19 +1100 Subject: [PATCH 08/15] Combine intenset calls --- embassy-nrf/src/saadc.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 3acfa50d7..273eff799 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -295,9 +295,12 @@ impl<'d, const N: usize> Saadc<'d, N> { // Reset and enable the events r.events_end.reset(); - r.intenset.write(|w| w.end().set()); r.events_started.reset(); - r.intenset.write(|w| w.started().set()); + r.intenset.write(|w| { + w.end().set(); + w.started().set(); + w + }); // Don't reorder the ADC start event before the previous writes. Hopefully self // wouldn't happen anyway. From cb56f52b9952bea266fdcad3a539b6097ddf5079 Mon Sep 17 00:00:00 2001 From: huntc Date: Sun, 17 Oct 2021 06:56:56 +1100 Subject: [PATCH 09/15] Removed the Mode enum and factored out into two functions so that we can assert channel limits --- embassy-nrf/src/saadc.rs | 73 +++++++++++++++++------- examples/nrf/src/bin/saadc_continuous.rs | 4 +- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 273eff799..af45a8033 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -110,20 +110,6 @@ impl<'d> ChannelConfig<'d> { } } -/// A sample rate can be provided for the internal clock of the SAADC when -/// one channel is to be continuously sampled. When there are multiple channels -/// to be continuously sampled then an external source is used to generate -/// TASK_SAMPLE tasks. -pub enum Mode { - /// The internal clock is to be used with a sample rate expressed as a divisor of - /// 16MHz, ranging from 80..2047. For example, 1600 represnts a sample rate of 10KHz - /// given 16_000_000 / 10_000_000 = 1600. - Timers(u16), - /// TASK_SAMPLE tasks are generated outside of the SAADC e.g. via PPI on a - /// timer. - Task, -} - /// The state of a continuously running sampler. While it reflects /// the progress of a sampler, it also signals what should be done /// next. For example, if the sampler has stopped then the Saadc implementation @@ -254,14 +240,61 @@ impl<'d, const N: usize> Saadc<'d, N> { /// Continuous sampling with double buffers. The sample buffers generally /// should be a multiple of the number of channels configured. /// + /// The internal clock is to be used with a sample rate expressed as a divisor of + /// 16MHz, ranging from 80..2047. For example, 1600 represnts a sample rate of 10KHz + /// given 16_000_000 / 10_000_000 = 1600. + /// /// A sampler closure is provided that receives the buffer of samples, noting /// that the size of this buffer can be less than the original buffer's size. /// A command is return from the closure that indicates whether the sampling /// should continue or stop. - pub async fn run_sampler( + pub async fn run_timer_sampler( &mut self, bufs: &mut [[i16; N0]; 2], - mode: Mode, + sample_rate: u16, + sampler: S, + ) where + S: FnMut(&[i16]) -> SamplerState, + { + assert!( + N == 1, + "The internal timer can only be used with one channel" + ); + assert!( + N0 % N == 0, + "The buffer size must be a multiple of the number of channels" + ); + self.run_sampler(bufs, Some(sample_rate), sampler).await; + } + + /// Continuous sampling with double buffers. The sample buffers generally + /// should be a multiple of the number of channels configured. + /// + /// A task-driven approach to driving TASK_SAMPLE is expected. With a task + /// driven approach, multiple channels can be used. + /// + /// A sampler closure is provided that receives the buffer of samples, noting + /// that the size of this buffer can be less than the original buffer's size. + /// A command is return from the closure that indicates whether the sampling + /// should continue or stop. + pub async fn run_task_sampler( + &mut self, + bufs: &mut [[i16; N0]; 2], + sampler: S, + ) where + S: FnMut(&[i16]) -> SamplerState, + { + assert!( + N0 % N == 0, + "The buffer size must be a multiple of the number of channels" + ); + self.run_sampler(bufs, None, sampler).await; + } + + async fn run_sampler( + &mut self, + bufs: &mut [[i16; N0]; 2], + sample_rate: Option, mut sampler: S, ) where S: FnMut(&[i16]) -> SamplerState, @@ -269,16 +302,16 @@ impl<'d, const N: usize> Saadc<'d, N> { let r = Self::regs(); // Establish mode and sample rate - match mode { - Mode::Timers(sample_rate) => { + match sample_rate { + Some(sr) => { r.samplerate.write(|w| unsafe { - w.cc().bits(sample_rate); + w.cc().bits(sr); w.mode().timers(); w }); r.tasks_sample.write(|w| unsafe { w.bits(1) }); // Need to kick-start the internal timer } - Mode::Task => r.samplerate.write(|w| unsafe { + None => r.samplerate.write(|w| unsafe { w.cc().bits(0); w.mode().task(); w diff --git a/examples/nrf/src/bin/saadc_continuous.rs b/examples/nrf/src/bin/saadc_continuous.rs index dfcb46171..d936a7d21 100644 --- a/examples/nrf/src/bin/saadc_continuous.rs +++ b/examples/nrf/src/bin/saadc_continuous.rs @@ -7,7 +7,7 @@ mod example_common; use defmt::panic; use embassy::executor::Spawner; use embassy_nrf::ppi::Ppi; -use embassy_nrf::saadc::{ChannelConfig, Config, Mode, Saadc, SamplerState}; +use embassy_nrf::saadc::{ChannelConfig, Config, Saadc, SamplerState}; use embassy_nrf::timer::{Frequency, Timer}; use embassy_nrf::{interrupt, Peripherals}; use example_common::*; @@ -45,7 +45,7 @@ async fn main(_spawner: Spawner, mut p: Peripherals) { let mut a: i32 = 0; saadc - .run_sampler(&mut bufs, Mode::Task, move |buf| { + .run_task_sampler(&mut bufs, move |buf| { for (i, b) in buf.iter().enumerate() { if i % 3 == 0 { a += *b as i32; From 0c317a64f6f0f845dd2ca3323c9579fda09be410 Mon Sep 17 00:00:00 2001 From: huntc Date: Sun, 17 Oct 2021 07:02:17 +1100 Subject: [PATCH 10/15] As suggested, use the const param to declare the internal sample for one channel only --- embassy-nrf/src/saadc.rs | 54 ++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index af45a8033..6e0bcc167 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -237,36 +237,6 @@ impl<'d, const N: usize> Saadc<'d, N> { .await; } - /// Continuous sampling with double buffers. The sample buffers generally - /// should be a multiple of the number of channels configured. - /// - /// The internal clock is to be used with a sample rate expressed as a divisor of - /// 16MHz, ranging from 80..2047. For example, 1600 represnts a sample rate of 10KHz - /// given 16_000_000 / 10_000_000 = 1600. - /// - /// A sampler closure is provided that receives the buffer of samples, noting - /// that the size of this buffer can be less than the original buffer's size. - /// A command is return from the closure that indicates whether the sampling - /// should continue or stop. - pub async fn run_timer_sampler( - &mut self, - bufs: &mut [[i16; N0]; 2], - sample_rate: u16, - sampler: S, - ) where - S: FnMut(&[i16]) -> SamplerState, - { - assert!( - N == 1, - "The internal timer can only be used with one channel" - ); - assert!( - N0 % N == 0, - "The buffer size must be a multiple of the number of channels" - ); - self.run_sampler(bufs, Some(sample_rate), sampler).await; - } - /// Continuous sampling with double buffers. The sample buffers generally /// should be a multiple of the number of channels configured. /// @@ -387,6 +357,30 @@ impl<'d, const N: usize> Saadc<'d, N> { } } +impl<'d> Saadc<'d, 1> { + /// Continuous sampling on a single channel with double buffers. The sample + /// buffers generally should be a multiple of the number of channels configured. + /// + /// The internal clock is to be used with a sample rate expressed as a divisor of + /// 16MHz, ranging from 80..2047. For example, 1600 represnts a sample rate of 10KHz + /// given 16_000_000 / 10_000_000 = 1600. + /// + /// A sampler closure is provided that receives the buffer of samples, noting + /// that the size of this buffer can be less than the original buffer's size. + /// A command is return from the closure that indicates whether the sampling + /// should continue or stop. + pub async fn run_timer_sampler( + &mut self, + bufs: &mut [[i16; N0]; 2], + sample_rate: u16, + sampler: S, + ) where + S: FnMut(&[i16]) -> SamplerState, + { + self.run_sampler(bufs, Some(sample_rate), sampler).await; + } +} + impl<'d, const N: usize> Drop for Saadc<'d, N> { fn drop(&mut self) { let r = Self::regs(); From 785030df963c9071a51fbc6e57e545faccc483c1 Mon Sep 17 00:00:00 2001 From: huntc Date: Sun, 17 Oct 2021 07:51:53 +1100 Subject: [PATCH 11/15] Use types to strengthen the buffer dimensioning --- embassy-nrf/src/saadc.rs | 22 +++++++++------------- examples/nrf/src/bin/saadc_continuous.rs | 10 ++++------ 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 6e0bcc167..fd7e64e7b 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -249,25 +249,21 @@ impl<'d, const N: usize> Saadc<'d, N> { /// should continue or stop. pub async fn run_task_sampler( &mut self, - bufs: &mut [[i16; N0]; 2], + bufs: &mut [[[i16; N]; N0]; 2], sampler: S, ) where - S: FnMut(&[i16]) -> SamplerState, + S: FnMut(&[[i16; N]]) -> SamplerState, { - assert!( - N0 % N == 0, - "The buffer size must be a multiple of the number of channels" - ); self.run_sampler(bufs, None, sampler).await; } async fn run_sampler( &mut self, - bufs: &mut [[i16; N0]; 2], + bufs: &mut [[[i16; N]; N0]; 2], sample_rate: Option, mut sampler: S, ) where - S: FnMut(&[i16]) -> SamplerState, + S: FnMut(&[[i16; N]]) -> SamplerState, { let r = Self::regs(); @@ -294,7 +290,7 @@ impl<'d, const N: usize> Saadc<'d, N> { .write(|w| unsafe { w.ptr().bits(bufs[0].as_mut_ptr() as u32) }); r.result .maxcnt - .write(|w| unsafe { w.maxcnt().bits(N0 as _) }); + .write(|w| unsafe { w.maxcnt().bits((N0 * N) as _) }); // Reset and enable the events r.events_end.reset(); @@ -323,7 +319,7 @@ impl<'d, const N: usize> Saadc<'d, N> { r.events_end.reset(); r.intenset.write(|w| w.end().set()); - if sampler(&bufs[current_buffer][0..r.result.amount.read().bits() as usize]) + if sampler(&bufs[current_buffer][0..r.result.amount.read().bits() as usize / N]) == SamplerState::Sampled { let next_buffer = 1 - current_buffer; @@ -358,7 +354,7 @@ impl<'d, const N: usize> Saadc<'d, N> { } impl<'d> Saadc<'d, 1> { - /// Continuous sampling on a single channel with double buffers. The sample + /// Continuous sampling on a single channel with double buffers. The sample /// buffers generally should be a multiple of the number of channels configured. /// /// The internal clock is to be used with a sample rate expressed as a divisor of @@ -371,11 +367,11 @@ impl<'d> Saadc<'d, 1> { /// should continue or stop. pub async fn run_timer_sampler( &mut self, - bufs: &mut [[i16; N0]; 2], + bufs: &mut [[[i16; 1]; N0]; 2], sample_rate: u16, sampler: S, ) where - S: FnMut(&[i16]) -> SamplerState, + S: FnMut(&[[i16; 1]]) -> SamplerState, { self.run_sampler(bufs, Some(sample_rate), sampler).await; } diff --git a/examples/nrf/src/bin/saadc_continuous.rs b/examples/nrf/src/bin/saadc_continuous.rs index d936a7d21..149b9c60c 100644 --- a/examples/nrf/src/bin/saadc_continuous.rs +++ b/examples/nrf/src/bin/saadc_continuous.rs @@ -39,19 +39,17 @@ async fn main(_spawner: Spawner, mut p: Peripherals) { timer.start(); - let mut bufs = [[0; 3 * 50]; 2]; // Each buffer of the double buffer has to be large enough for all channels. + let mut bufs = [[[0; 3]; 50]; 2]; let mut c = 0; let mut a: i32 = 0; saadc .run_task_sampler(&mut bufs, move |buf| { - for (i, b) in buf.iter().enumerate() { - if i % 3 == 0 { - a += *b as i32; - c += 1; - } + for b in buf { + a += b[0] as i32; } + c += buf.len(); if c > 10000 { a = a / c as i32; info!("channel 1: {=i32}", a); From c7e426655dfb82d932d7f63dc73838cd090dbfb5 Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 18 Oct 2021 11:28:43 +1100 Subject: [PATCH 12/15] Ensure the compiler doesn't reorder things before calling the sampler --- embassy-nrf/src/saadc.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index fd7e64e7b..41644120b 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -316,6 +316,8 @@ impl<'d, const N: usize> Saadc<'d, N> { WAKER.register(cx.waker()); if r.events_end.read().bits() != 0 { + compiler_fence(Ordering::SeqCst); + r.events_end.reset(); r.intenset.write(|w| w.end().set()); From 93407ed5e20b77dc7828627e7fe0f0af37435c05 Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 18 Oct 2021 11:31:06 +1100 Subject: [PATCH 13/15] Remove unneeded stop --- embassy-nrf/src/saadc.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 41644120b..26e694916 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -328,7 +328,6 @@ impl<'d, const N: usize> Saadc<'d, N> { current_buffer = next_buffer; r.tasks_start.write(|w| unsafe { w.bits(1) }); } else { - r.tasks_stop.write(|w| unsafe { w.bits(1) }); return Poll::Ready(()); }; } From 6dc0ed53ff9cac8436fdcc767ea8f174c752db6a Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 18 Oct 2021 11:42:30 +1100 Subject: [PATCH 14/15] Renamed sample rate to what it actually is --- embassy-nrf/src/saadc.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 26e694916..978335108 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -260,7 +260,7 @@ impl<'d, const N: usize> Saadc<'d, N> { async fn run_sampler( &mut self, bufs: &mut [[[i16; N]; N0]; 2], - sample_rate: Option, + sample_rate_divisor: Option, mut sampler: S, ) where S: FnMut(&[[i16; N]]) -> SamplerState, @@ -268,7 +268,7 @@ impl<'d, const N: usize> Saadc<'d, N> { let r = Self::regs(); // Establish mode and sample rate - match sample_rate { + match sample_rate_divisor { Some(sr) => { r.samplerate.write(|w| unsafe { w.cc().bits(sr); @@ -369,12 +369,12 @@ impl<'d> Saadc<'d, 1> { pub async fn run_timer_sampler( &mut self, bufs: &mut [[[i16; 1]; N0]; 2], - sample_rate: u16, + sample_rate_divisor: u16, sampler: S, ) where S: FnMut(&[[i16; 1]]) -> SamplerState, { - self.run_sampler(bufs, Some(sample_rate), sampler).await; + self.run_sampler(bufs, Some(sample_rate_divisor), sampler).await; } } From a94d44a689f5832a9eefbaab8f126a388308a08c Mon Sep 17 00:00:00 2001 From: huntc Date: Mon, 18 Oct 2021 11:45:23 +1100 Subject: [PATCH 15/15] Comments corrected --- embassy-nrf/src/saadc.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 978335108..8bf4f7276 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -237,8 +237,7 @@ impl<'d, const N: usize> Saadc<'d, N> { .await; } - /// Continuous sampling with double buffers. The sample buffers generally - /// should be a multiple of the number of channels configured. + /// Continuous sampling with double buffers. /// /// A task-driven approach to driving TASK_SAMPLE is expected. With a task /// driven approach, multiple channels can be used. @@ -355,8 +354,7 @@ impl<'d, const N: usize> Saadc<'d, N> { } impl<'d> Saadc<'d, 1> { - /// Continuous sampling on a single channel with double buffers. The sample - /// buffers generally should be a multiple of the number of channels configured. + /// Continuous sampling on a single channel with double buffers. /// /// The internal clock is to be used with a sample rate expressed as a divisor of /// 16MHz, ranging from 80..2047. For example, 1600 represnts a sample rate of 10KHz @@ -374,7 +372,8 @@ impl<'d> Saadc<'d, 1> { ) where S: FnMut(&[[i16; 1]]) -> SamplerState, { - self.run_sampler(bufs, Some(sample_rate_divisor), sampler).await; + self.run_sampler(bufs, Some(sample_rate_divisor), sampler) + .await; } }