From 59de2d83aac36ac772c009e9f4eb9ad8df21199b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 15 Aug 2024 10:18:32 +0200 Subject: [PATCH 1/4] Simplify I2S async test --- hil-test/tests/i2s_async.rs | 97 +++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 41 deletions(-) diff --git a/hil-test/tests/i2s_async.rs b/hil-test/tests/i2s_async.rs index bc6b816cec1..31728be901a 100644 --- a/hil-test/tests/i2s_async.rs +++ b/hil-test/tests/i2s_async.rs @@ -16,34 +16,63 @@ use esp_hal::{ clock::ClockControl, dma::{Dma, DmaChannel0, DmaPriority}, gpio::Io, - i2s::{asynch::*, DataFormat, I2s, Standard}, + i2s::{asynch::*, DataFormat, I2s, I2sTx, Standard}, peripheral::Peripheral, - peripherals::Peripherals, + peripherals::{I2S0, Peripherals}, prelude::*, system::SystemControl, + Async, }; -// choose values which DON'T restart on every descriptor buffer's start -const ADD: u8 = 5; -const CUT_OFF: u8 = 113; +const BUFFER_SIZE: usize = 2000; + +#[derive(Clone)] +struct SampleSource { + i: u8, +} + +impl SampleSource { + // choose values which DON'T restart on every descriptor buffer's start + const ADD: u8 = 5; + const CUT_OFF: u8 = 113; + + fn new() -> Self { + Self { i: 0 } + } +} + +impl Iterator for SampleSource { + type Item = u8; + + fn next(&mut self) -> Option { + let i = self.i; + self.i = (i + Self::ADD) % Self::CUT_OFF; + Some(i) + } +} #[embassy_executor::task] async fn writer( - i: u8, - mut transfer: I2sWriteDmaTransferAsync< + tx_buffer: &'static mut [u8], + i2s_tx: I2sTx< 'static, - esp_hal::peripherals::I2S0, + I2S0, DmaChannel0, - &'static mut [u8; 2000], + Async >, ) { - let mut i = i; + let mut samples = SampleSource::new(); + for b in tx_buffer.iter_mut() { + *b = samples.next().unwrap(); + } + + let mut tx_transfer = i2s_tx.write_dma_circular_async(tx_buffer).unwrap(); + loop { - transfer + tx_transfer .push_with(|buffer| { for b in buffer.iter_mut() { - *b = i; - i = (i + ADD) % CUT_OFF; + *b = samples.next().unwrap(); } buffer.len() }) @@ -73,9 +102,8 @@ mod tests { let dma = Dma::new(peripherals.DMA); let dma_channel = dma.channel0; - #[allow(non_upper_case_globals)] let (tx_buffer, tx_descriptors, rx_buffer, rx_descriptors) = - esp_hal::dma_circular_buffers!(2000, 2000); + esp_hal::dma_circular_buffers!(BUFFER_SIZE, BUFFER_SIZE); let i2s = I2s::new( peripherals.I2S0, @@ -92,7 +120,7 @@ mod tests { .i2s_tx .with_bclk(unsafe { io.pins.gpio0.clone_unchecked() }) .with_ws(unsafe { io.pins.gpio1.clone_unchecked() }) - .with_dout(unsafe { io.pins.gpio2.clone_unchecked() }) + .with_dout(io.pins.gpio2) .build(); let i2s_rx = i2s @@ -116,38 +144,25 @@ mod tests { i2s.rx_conf().modify(|_, w| w.rx_update().set_bit()); } - let mut iteration = 0; - let mut failed = false; - let mut check_i: u8 = 0; - let mut i = 0; - for b in tx_buffer.iter_mut() { - *b = i; - i = (i + ADD) % CUT_OFF; - } - - let mut rcv = [0u8; 2000]; - + spawner.must_spawn(writer(tx_buffer, i2s_tx)); let mut rx_transfer = i2s_rx.read_dma_circular_async(rx_buffer).unwrap(); - let tx_transfer = i2s_tx.write_dma_circular_async(tx_buffer).unwrap(); - - spawner.must_spawn(writer(i, tx_transfer)); - 'outer: loop { + let mut rcv = [0u8; BUFFER_SIZE]; + let mut sample_idx = 0; + let mut samples = SampleSource::new(); + let mut success = true; + for _ in 0..30 { let len = rx_transfer.pop(&mut rcv).await.unwrap(); for &b in &rcv[..len] { - if b != check_i { - failed = true; - break 'outer; + let expected = samples.next().unwrap(); + if b != expected { + success = false; + defmt::error!("Sample #{} does not match ({} != {})", sample_idx, b, expected); } - check_i = (check_i + ADD) % CUT_OFF; - } - iteration += 1; - - if iteration > 30 { - break; + sample_idx += 1; } } - assert!(!failed); + assert!(success); } } From 529ee744aac24135d317525765e34e0deb96420e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 15 Aug 2024 10:39:54 +0200 Subject: [PATCH 2/4] Allow running tests repeatedly --- hil-test/.cargo/config.toml | 4 ++-- xtask/src/cargo.rs | 2 +- xtask/src/lib.rs | 12 +++++++++--- xtask/src/main.rs | 32 +++++++++++++++++++++++++++----- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/hil-test/.cargo/config.toml b/hil-test/.cargo/config.toml index 782ecfe704f..26eefc108e8 100644 --- a/hil-test/.cargo/config.toml +++ b/hil-test/.cargo/config.toml @@ -1,5 +1,5 @@ [target.'cfg(target_arch = "riscv32")'] -runner = "probe-rs run" +runner = "probe-rs run --preverify" rustflags = [ "-C", "link-arg=-Tlinkall.x", "-C", "link-arg=-Tembedded-test.x", @@ -8,7 +8,7 @@ rustflags = [ ] [target.'cfg(target_arch = "xtensa")'] -runner = "probe-rs run" +runner = "probe-rs run --preverify" rustflags = [ "-C", "link-arg=-nostartfiles", "-C", "link-arg=-Wl,-Tlinkall.x", diff --git a/xtask/src/cargo.rs b/xtask/src/cargo.rs index 74d12fa2442..8f1051faab3 100644 --- a/xtask/src/cargo.rs +++ b/xtask/src/cargo.rs @@ -9,7 +9,7 @@ use anyhow::{bail, Result}; use crate::windows_safe_path; -#[derive(Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum CargoAction { Build, Run, diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index ee2ec036df1..c2349a71710 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -201,7 +201,8 @@ pub fn execute_app( chip: Chip, target: &str, app: &Metadata, - action: &CargoAction, + action: CargoAction, + mut repeat: usize, ) -> Result<()> { log::info!( "Building example '{}' for '{}'", @@ -214,7 +215,8 @@ pub fn execute_app( let package = app.example_path().strip_prefix(package_path)?; log::info!("Package: {:?}", package); - let (bin, subcommand) = if action == &CargoAction::Build { + let (bin, subcommand) = if action == CargoAction::Build { + repeat = 1; // Do not repeat builds in a loop let bin = if package.starts_with("src/bin") { format!("--bin={}", app.name()) } else if package.starts_with("tests") { @@ -254,7 +256,11 @@ pub fn execute_app( let args = builder.build(); log::debug!("{args:#?}"); - cargo::run(&args, package_path) + for _ in 0..repeat { + cargo::run(&args, package_path)?; + } + + Ok(()) } /// Build the specified package, using the given toolchain/target/features if diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 371ac743deb..db10638f789 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -68,6 +68,9 @@ struct TestArgs { /// Optional test to act on (all tests used if omitted) #[arg(short = 't', long)] test: Option, + /// Repeat the tests for a specific number of times. + #[arg(long)] + repeat: Option, } #[derive(Debug, Args)] @@ -231,7 +234,8 @@ fn build_examples(args: ExampleArgs, examples: Vec, package_path: &Pat args.chip, target, example, - &CargoAction::Build, + CargoAction::Build, + 1, ) } else if args.example.is_some() { // An invalid argument was provided: @@ -244,7 +248,8 @@ fn build_examples(args: ExampleArgs, examples: Vec, package_path: &Pat args.chip, target, example, - &CargoAction::Build, + CargoAction::Build, + 1, ) }) } @@ -262,7 +267,8 @@ fn run_example(args: ExampleArgs, examples: Vec, package_path: &Path) args.chip, target, &example, - &CargoAction::Run, + CargoAction::Run, + 1, ) } else { bail!("Example not found or unsupported for the given chip") @@ -287,13 +293,29 @@ fn tests(workspace: &Path, args: TestArgs, action: CargoAction) -> Result<()> { // Execute the specified action: if let Some(test) = tests.iter().find(|test| Some(test.name()) == args.test) { - xtask::execute_app(&package_path, args.chip, target, &test, &action) + xtask::execute_app( + &package_path, + args.chip, + target, + &test, + action, + args.repeat.unwrap_or(1), + ) } else if args.test.is_some() { bail!("Test not found or unsupported for the given chip") } else { let mut failed = Vec::new(); for test in tests { - if xtask::execute_app(&package_path, args.chip, target, &test, &action).is_err() { + if xtask::execute_app( + &package_path, + args.chip, + target, + &test, + action, + args.repeat.unwrap_or(1), + ) + .is_err() + { failed.push(test.name()); } } From c7568f351dd1f6f5abf58751b9102cae8bd2f124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 15 Aug 2024 10:43:18 +0200 Subject: [PATCH 3/4] Fail at the first mismatch --- hil-test/tests/i2s_async.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/hil-test/tests/i2s_async.rs b/hil-test/tests/i2s_async.rs index 31728be901a..9addb294519 100644 --- a/hil-test/tests/i2s_async.rs +++ b/hil-test/tests/i2s_async.rs @@ -150,19 +150,13 @@ mod tests { let mut rcv = [0u8; BUFFER_SIZE]; let mut sample_idx = 0; let mut samples = SampleSource::new(); - let mut success = true; for _ in 0..30 { let len = rx_transfer.pop(&mut rcv).await.unwrap(); for &b in &rcv[..len] { let expected = samples.next().unwrap(); - if b != expected { - success = false; - defmt::error!("Sample #{} does not match ({} != {})", sample_idx, b, expected); - } + assert_eq!(b, expected, "Sample #{} does not match ({} != {})", sample_idx, b, expected); sample_idx += 1; } } - - assert!(success); } } From 70da1650b2dd15206e0cd8171c20c9328a720219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 15 Aug 2024 11:06:16 +0200 Subject: [PATCH 4/4] Clean up --- esp-hal/src/i2s.rs | 6 +----- hil-test/tests/i2s_async.rs | 20 ++++++++------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/esp-hal/src/i2s.rs b/esp-hal/src/i2s.rs index 74c7a09fe10..d39c3a0c778 100644 --- a/esp-hal/src/i2s.rs +++ b/esp-hal/src/i2s.rs @@ -2251,8 +2251,6 @@ pub mod asynch { } /// An in-progress async circular DMA write transfer. - #[non_exhaustive] - pub struct I2sWriteDmaTransferAsync<'d, T, CH, BUFFER> where T: RegisterAccess, @@ -2313,7 +2311,7 @@ pub mod asynch { /// One-shot read I2S. async fn read_dma_async(&mut self, words: &mut [u8]) -> Result<(), Error>; - /// Continuously read frm I2S. Returns [I2sReadDmaTransferAsync] + /// Continuously read from I2S. Returns [I2sReadDmaTransferAsync] fn read_dma_circular_async( self, words: RXBUF, @@ -2407,8 +2405,6 @@ pub mod asynch { } /// An in-progress async circular DMA read transfer. - #[non_exhaustive] - pub struct I2sReadDmaTransferAsync<'d, T, CH, BUFFER> where T: RegisterAccess, diff --git a/hil-test/tests/i2s_async.rs b/hil-test/tests/i2s_async.rs index 9addb294519..991f32f5a5b 100644 --- a/hil-test/tests/i2s_async.rs +++ b/hil-test/tests/i2s_async.rs @@ -18,7 +18,7 @@ use esp_hal::{ gpio::Io, i2s::{asynch::*, DataFormat, I2s, I2sTx, Standard}, peripheral::Peripheral, - peripherals::{I2S0, Peripherals}, + peripherals::{Peripherals, I2S0}, prelude::*, system::SystemControl, Async, @@ -52,15 +52,7 @@ impl Iterator for SampleSource { } #[embassy_executor::task] -async fn writer( - tx_buffer: &'static mut [u8], - i2s_tx: I2sTx< - 'static, - I2S0, - DmaChannel0, - Async - >, -) { +async fn writer(tx_buffer: &'static mut [u8], i2s_tx: I2sTx<'static, I2S0, DmaChannel0, Async>) { let mut samples = SampleSource::new(); for b in tx_buffer.iter_mut() { *b = samples.next().unwrap(); @@ -144,8 +136,8 @@ mod tests { i2s.rx_conf().modify(|_, w| w.rx_update().set_bit()); } - spawner.must_spawn(writer(tx_buffer, i2s_tx)); let mut rx_transfer = i2s_rx.read_dma_circular_async(rx_buffer).unwrap(); + spawner.must_spawn(writer(tx_buffer, i2s_tx)); let mut rcv = [0u8; BUFFER_SIZE]; let mut sample_idx = 0; @@ -154,7 +146,11 @@ mod tests { let len = rx_transfer.pop(&mut rcv).await.unwrap(); for &b in &rcv[..len] { let expected = samples.next().unwrap(); - assert_eq!(b, expected, "Sample #{} does not match ({} != {})", sample_idx, b, expected); + assert_eq!( + b, expected, + "Sample #{} does not match ({} != {})", + sample_idx, b, expected + ); sample_idx += 1; } }