Clean up i2s_async test, add option to repeat (#1951)

* Simplify I2S async test

* Allow running tests repeatedly

* Fail at the first mismatch

* Clean up

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<RXBUF>(
             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,

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",

hil-test/tests/i2s_async.rs

@@ -16,34 +16,55 @@ 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::{Peripherals, I2S0},
     prelude::*,
     system::SystemControl,
+    Async,
 };
 
+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<Self::Item> {
+        let i = self.i;
+        self.i = (i + Self::ADD) % Self::CUT_OFF;
+        Some(i)
+    }
+}
+
 #[embassy_executor::task]
-async fn writer(
+async fn writer(tx_buffer: &'static mut [u8], i2s_tx: I2sTx<'static, I2S0, DmaChannel0, Async>) {
-    i: u8,
+    let mut samples = SampleSource::new();
-    mut transfer: I2sWriteDmaTransferAsync<
+    for b in tx_buffer.iter_mut() {
-        'static,
+        *b = samples.next().unwrap();
-        esp_hal::peripherals::I2S0,
+    }
-        DmaChannel0,
+
-        &'static mut [u8; 2000],
+    let mut tx_transfer = i2s_tx.write_dma_circular_async(tx_buffer).unwrap();
-    >,
+
-) {
-    let mut i = i;
     loop {
-        transfer
+        tx_transfer
             .push_with(|buffer| {
                 for b in buffer.iter_mut() {
-                    *b = i;
+                    *b = samples.next().unwrap();
-                    i = (i + ADD) % CUT_OFF;
                 }
                 buffer.len()
             })
@@ -73,9 +94,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 +112,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 +136,23 @@ 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];
-
         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(tx_buffer, i2s_tx));
 
-        spawner.must_spawn(writer(i, tx_transfer));
+        let mut rcv = [0u8; BUFFER_SIZE];
-
+        let mut sample_idx = 0;
-        'outer: loop {
+        let mut samples = SampleSource::new();
+        for _ in 0..30 {
             let len = rx_transfer.pop(&mut rcv).await.unwrap();
             for &b in &rcv[..len] {
-                if b != check_i {
+                let expected = samples.next().unwrap();
-                    failed = true;
+                assert_eq!(
-                    break 'outer;
+                    b, expected,
-                }
+                    "Sample #{} does not match ({} != {})",
-                check_i = (check_i + ADD) % CUT_OFF;
+                    sample_idx, b, expected
-            }
+                );
-            iteration += 1;
+                sample_idx += 1;
-
-            if iteration > 30 {
-                break;
             }
         }
-
-        assert!(!failed);
     }
 }

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,

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

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<String>,
+    /// Repeat the tests for a specific number of times.
+    #[arg(long)]
+    repeat: Option<usize>,
 }
 
 #[derive(Debug, Args)]
@@ -231,7 +234,8 @@ fn build_examples(args: ExampleArgs, examples: Vec<Metadata>, 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<Metadata>, package_path: &Pat
                 args.chip,
                 target,
                 example,
-                &CargoAction::Build,
+                CargoAction::Build,
+                1,
             )
         })
     }
@@ -262,7 +267,8 @@ fn run_example(args: ExampleArgs, examples: Vec<Metadata>, 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());
             }
         }