Removes some of the code duplication for UarteWithIdle

This commit removes some of the code duplication for UarteWithIdle at the expense of requiring a split. As the example illustrates though, this expense seems worth the benefit in terms of maintenance, and the avoidance of copying over methods. My main motivation for this commit was actually due to the `event_endtx` method not having been copied across.

2 files changed, 135 insertions, 270 deletions

diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs
index d9959911..636d6c7a 100644
--- a/embassy-nrf/src/uarte.rs
+++ b/embassy-nrf/src/uarte.rs
@@ -173,6 +173,61 @@ impl<'d, T: Instance> Uarte<'d, T> {
         (self.tx, self.rx)
     }
 
+    /// Split the Uarte into a transmitter and receiver that will
+    /// return on idle, which is determined as the time it takes
+    /// for two bytes to be received.
+    pub fn split_with_idle<U: TimerInstance>(
+        self,
+        timer: impl Peripheral<P = U> + 'd,
+        ppi_ch1: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
+        ppi_ch2: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
+    ) -> (UarteTx<'d, T>, UarteRxWithIdle<'d, T, U>) {
+        let mut timer = Timer::new(timer);
+
+        into_ref!(ppi_ch1, ppi_ch2);
+
+        let r = T::regs();
+
+        // BAUDRATE register values are `baudrate * 2^32 / 16000000`
+        // source: https://devzone.nordicsemi.com/f/nordic-q-a/391/uart-baudrate-register-values
+        //
+        // We want to stop RX if line is idle for 2 bytes worth of time
+        // That is 20 bits (each byte is 1 start bit + 8 data bits + 1 stop bit)
+        // This gives us the amount of 16M ticks for 20 bits.
+        let baudrate = r.baudrate.read().baudrate().variant().unwrap();
+        let timeout = 0x8000_0000 / (baudrate as u32 / 40);
+
+        timer.set_frequency(Frequency::F16MHz);
+        timer.cc(0).write(timeout);
+        timer.cc(0).short_compare_clear();
+        timer.cc(0).short_compare_stop();
+
+        let mut ppi_ch1 = Ppi::new_one_to_two(
+            ppi_ch1.map_into(),
+            Event::from_reg(&r.events_rxdrdy),
+            timer.task_clear(),
+            timer.task_start(),
+        );
+        ppi_ch1.enable();
+
+        let mut ppi_ch2 = Ppi::new_one_to_one(
+            ppi_ch2.map_into(),
+            timer.cc(0).event_compare(),
+            Task::from_reg(&r.tasks_stoprx),
+        );
+        ppi_ch2.enable();
+
+        (
+            self.tx,
+            UarteRxWithIdle {
+                rx: self.rx,
+                timer,
+                ppi_ch1: ppi_ch1,
+                _ppi_ch2: ppi_ch2,
+            },
+        )
+    }
+
     /// Return the endtx event for use with PPI
     pub fn event_endtx(&self) -> Event {
         let r = T::regs();
@@ -597,236 +652,14 @@ impl<'a, T: Instance> Drop for UarteRx<'a, T> {
     }
 }
 
-#[cfg(not(any(feature = "_nrf9160", feature = "nrf5340")))]
-pub(crate) fn apply_workaround_for_enable_anomaly(_r: &crate::pac::uarte0::RegisterBlock) {
-    // Do nothing
-}
-
-#[cfg(any(feature = "_nrf9160", feature = "nrf5340"))]
-pub(crate) fn apply_workaround_for_enable_anomaly(r: &crate::pac::uarte0::RegisterBlock) {
-    use core::ops::Deref;
-
-    // Apply workaround for anomalies:
-    // - nRF9160 - anomaly 23
-    // - nRF5340 - anomaly 44
-    let rxenable_reg: *const u32 = ((r.deref() as *const _ as usize) + 0x564) as *const u32;
-    let txenable_reg: *const u32 = ((r.deref() as *const _ as usize) + 0x568) as *const u32;
-
-    // NB Safety: This is taken from Nordic's driver -
-    // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197
-    if unsafe { core::ptr::read_volatile(txenable_reg) } == 1 {
-        r.tasks_stoptx.write(|w| unsafe { w.bits(1) });
-    }
-
-    // NB Safety: This is taken from Nordic's driver -
-    // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197
-    if unsafe { core::ptr::read_volatile(rxenable_reg) } == 1 {
-        r.enable.write(|w| w.enable().enabled());
-        r.tasks_stoprx.write(|w| unsafe { w.bits(1) });
-
-        let mut workaround_succeded = false;
-        // The UARTE is able to receive up to four bytes after the STOPRX task has been triggered.
-        // On lowest supported baud rate (1200 baud), with parity bit and two stop bits configured
-        // (resulting in 12 bits per data byte sent), this may take up to 40 ms.
-        for _ in 0..40000 {
-            // NB Safety: This is taken from Nordic's driver -
-            // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197
-            if unsafe { core::ptr::read_volatile(rxenable_reg) } == 0 {
-                workaround_succeded = true;
-                break;
-            } else {
-                // Need to sleep for 1us here
-            }
-        }
-
-        if !workaround_succeded {
-            panic!("Failed to apply workaround for UART");
-        }
-
-        let errors = r.errorsrc.read().bits();
-        // NB Safety: safe to write back the bits we just read to clear them
-        r.errorsrc.write(|w| unsafe { w.bits(errors) });
-        r.enable.write(|w| w.enable().disabled());
-    }
-}
-
-pub(crate) fn drop_tx_rx(r: &pac::uarte0::RegisterBlock, s: &sealed::State) {
-    if s.tx_rx_refcount.fetch_sub(1, Ordering::Relaxed) == 1 {
-        // Finally we can disable, and we do so for the peripheral
-        // i.e. not just rx concerns.
-        r.enable.write(|w| w.enable().disabled());
-
-        gpio::deconfigure_pin(r.psel.rxd.read().bits());
-        gpio::deconfigure_pin(r.psel.txd.read().bits());
-        gpio::deconfigure_pin(r.psel.rts.read().bits());
-        gpio::deconfigure_pin(r.psel.cts.read().bits());
-
-        trace!("uarte tx and rx drop: done");
-    }
-}
-
-/// Interface to an UARTE peripheral that uses an additional timer and two PPI channels,
-/// allowing it to implement the ReadUntilIdle trait.
-pub struct UarteWithIdle<'d, U: Instance, T: TimerInstance> {
-    tx: UarteTx<'d, U>,
-    rx: UarteRxWithIdle<'d, U, T>,
-}
-
-impl<'d, U: Instance, T: TimerInstance> UarteWithIdle<'d, U, T> {
-    /// Create a new UARTE without hardware flow control
-    pub fn new(
-        uarte: impl Peripheral<P = U> + 'd,
-        timer: impl Peripheral<P = T> + 'd,
-        ppi_ch1: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
-        ppi_ch2: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
-        irq: impl Peripheral<P = U::Interrupt> + 'd,
-        rxd: impl Peripheral<P = impl GpioPin> + 'd,
-        txd: impl Peripheral<P = impl GpioPin> + 'd,
-        config: Config,
-    ) -> Self {
-        into_ref!(rxd, txd);
-        Self::new_inner(
-            uarte,
-            timer,
-            ppi_ch1,
-            ppi_ch2,
-            irq,
-            rxd.map_into(),
-            txd.map_into(),
-            None,
-            None,
-            config,
-        )
-    }
-
-    /// Create a new UARTE with hardware flow control (RTS/CTS)
-    pub fn new_with_rtscts(
-        uarte: impl Peripheral<P = U> + 'd,
-        timer: impl Peripheral<P = T> + 'd,
-        ppi_ch1: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
-        ppi_ch2: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
-        irq: impl Peripheral<P = U::Interrupt> + 'd,
-        rxd: impl Peripheral<P = impl GpioPin> + 'd,
-        txd: impl Peripheral<P = impl GpioPin> + 'd,
-        cts: impl Peripheral<P = impl GpioPin> + 'd,
-        rts: impl Peripheral<P = impl GpioPin> + 'd,
-        config: Config,
-    ) -> Self {
-        into_ref!(rxd, txd, cts, rts);
-        Self::new_inner(
-            uarte,
-            timer,
-            ppi_ch1,
-            ppi_ch2,
-            irq,
-            rxd.map_into(),
-            txd.map_into(),
-            Some(cts.map_into()),
-            Some(rts.map_into()),
-            config,
-        )
-    }
-
-    fn new_inner(
-        uarte: impl Peripheral<P = U> + 'd,
-        timer: impl Peripheral<P = T> + 'd,
-        ppi_ch1: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
-        ppi_ch2: impl Peripheral<P = impl ConfigurableChannel + 'd> + 'd,
-        irq: impl Peripheral<P = U::Interrupt> + 'd,
-        rxd: PeripheralRef<'d, AnyPin>,
-        txd: PeripheralRef<'d, AnyPin>,
-        cts: Option<PeripheralRef<'d, AnyPin>>,
-        rts: Option<PeripheralRef<'d, AnyPin>>,
-        config: Config,
-    ) -> Self {
-        let baudrate = config.baudrate;
-        let (tx, rx) = Uarte::new_inner(uarte, irq, rxd, txd, cts, rts, config).split();
-
-        let mut timer = Timer::new(timer);
-
-        into_ref!(ppi_ch1, ppi_ch2);
-
-        let r = U::regs();
-
-        // BAUDRATE register values are `baudrate * 2^32 / 16000000`
-        // source: https://devzone.nordicsemi.com/f/nordic-q-a/391/uart-baudrate-register-values
-        //
-        // We want to stop RX if line is idle for 2 bytes worth of time
-        // That is 20 bits (each byte is 1 start bit + 8 data bits + 1 stop bit)
-        // This gives us the amount of 16M ticks for 20 bits.
-        let timeout = 0x8000_0000 / (baudrate as u32 / 40);
-
-        timer.set_frequency(Frequency::F16MHz);
-        timer.cc(0).write(timeout);
-        timer.cc(0).short_compare_clear();
-        timer.cc(0).short_compare_stop();
-
-        let mut ppi_ch1 = Ppi::new_one_to_two(
-            ppi_ch1.map_into(),
-            Event::from_reg(&r.events_rxdrdy),
-            timer.task_clear(),
-            timer.task_start(),
-        );
-        ppi_ch1.enable();
-
-        let mut ppi_ch2 = Ppi::new_one_to_one(
-            ppi_ch2.map_into(),
-            timer.cc(0).event_compare(),
-            Task::from_reg(&r.tasks_stoprx),
-        );
-        ppi_ch2.enable();
-
-        Self {
-            tx,
-            rx: UarteRxWithIdle {
-                rx,
-                timer,
-                ppi_ch1: ppi_ch1,
-                _ppi_ch2: ppi_ch2,
-            },
-        }
-    }
-
-    /// Split the Uarte into a transmitter and receiver, which is
-    /// particuarly useful when having two tasks correlating to
-    /// transmitting and receiving.
-    pub fn split(self) -> (UarteTx<'d, U>, UarteRxWithIdle<'d, U, T>) {
-        (self.tx, self.rx)
-    }
-
-    pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> {
-        self.rx.read(buffer).await
-    }
-
-    pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> {
-        self.tx.write(buffer).await
-    }
-
-    pub fn blocking_read(&mut self, buffer: &mut [u8]) -> Result<(), Error> {
-        self.rx.blocking_read(buffer)
-    }
-
-    pub fn blocking_write(&mut self, buffer: &[u8]) -> Result<(), Error> {
-        self.tx.blocking_write(buffer)
-    }
-
-    pub async fn read_until_idle(&mut self, buffer: &mut [u8]) -> Result<usize, Error> {
-        self.rx.read_until_idle(buffer).await
-    }
-
-    pub fn blocking_read_until_idle(&mut self, buffer: &mut [u8]) -> Result<usize, Error> {
-        self.rx.blocking_read_until_idle(buffer)
-    }
-}
-
-pub struct UarteRxWithIdle<'d, U: Instance, T: TimerInstance> {
-    rx: UarteRx<'d, U>,
-    timer: Timer<'d, T>,
+pub struct UarteRxWithIdle<'d, T: Instance, U: TimerInstance> {
+    rx: UarteRx<'d, T>,
+    timer: Timer<'d, U>,
     ppi_ch1: Ppi<'d, AnyConfigurableChannel, 1, 2>,
     _ppi_ch2: Ppi<'d, AnyConfigurableChannel, 1, 1>,
 }
 
-impl<'d, U: Instance, T: TimerInstance> UarteRxWithIdle<'d, U, T> {
+impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> {
     pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> {
         self.ppi_ch1.disable();
         self.rx.read(buffer).await
@@ -848,8 +681,8 @@ impl<'d, U: Instance, T: TimerInstance> UarteRxWithIdle<'d, U, T> {
         let ptr = buffer.as_ptr();
         let len = buffer.len();
 
-        let r = U::regs();
-        let s = U::state();
+        let r = T::regs();
+        let s = T::state();
 
         self.ppi_ch1.enable();
 
@@ -904,7 +737,7 @@ impl<'d, U: Instance, T: TimerInstance> UarteRxWithIdle<'d, U, T> {
         let ptr = buffer.as_ptr();
         let len = buffer.len();
 
-        let r = U::regs();
+        let r = T::regs();
 
         self.ppi_ch1.enable();
 
@@ -929,6 +762,75 @@ impl<'d, U: Instance, T: TimerInstance> UarteRxWithIdle<'d, U, T> {
         Ok(n)
     }
 }
+
+#[cfg(not(any(feature = "_nrf9160", feature = "nrf5340")))]
+pub(crate) fn apply_workaround_for_enable_anomaly(_r: &crate::pac::uarte0::RegisterBlock) {
+    // Do nothing
+}
+
+#[cfg(any(feature = "_nrf9160", feature = "nrf5340"))]
+pub(crate) fn apply_workaround_for_enable_anomaly(r: &crate::pac::uarte0::RegisterBlock) {
+    use core::ops::Deref;
+
+    // Apply workaround for anomalies:
+    // - nRF9160 - anomaly 23
+    // - nRF5340 - anomaly 44
+    let rxenable_reg: *const u32 = ((r.deref() as *const _ as usize) + 0x564) as *const u32;
+    let txenable_reg: *const u32 = ((r.deref() as *const _ as usize) + 0x568) as *const u32;
+
+    // NB Safety: This is taken from Nordic's driver -
+    // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197
+    if unsafe { core::ptr::read_volatile(txenable_reg) } == 1 {
+        r.tasks_stoptx.write(|w| unsafe { w.bits(1) });
+    }
+
+    // NB Safety: This is taken from Nordic's driver -
+    // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197
+    if unsafe { core::ptr::read_volatile(rxenable_reg) } == 1 {
+        r.enable.write(|w| w.enable().enabled());
+        r.tasks_stoprx.write(|w| unsafe { w.bits(1) });
+
+        let mut workaround_succeded = false;
+        // The UARTE is able to receive up to four bytes after the STOPRX task has been triggered.
+        // On lowest supported baud rate (1200 baud), with parity bit and two stop bits configured
+        // (resulting in 12 bits per data byte sent), this may take up to 40 ms.
+        for _ in 0..40000 {
+            // NB Safety: This is taken from Nordic's driver -
+            // https://github.com/NordicSemiconductor/nrfx/blob/master/drivers/src/nrfx_uarte.c#L197
+            if unsafe { core::ptr::read_volatile(rxenable_reg) } == 0 {
+                workaround_succeded = true;
+                break;
+            } else {
+                // Need to sleep for 1us here
+            }
+        }
+
+        if !workaround_succeded {
+            panic!("Failed to apply workaround for UART");
+        }
+
+        let errors = r.errorsrc.read().bits();
+        // NB Safety: safe to write back the bits we just read to clear them
+        r.errorsrc.write(|w| unsafe { w.bits(errors) });
+        r.enable.write(|w| w.enable().disabled());
+    }
+}
+
+pub(crate) fn drop_tx_rx(r: &pac::uarte0::RegisterBlock, s: &sealed::State) {
+    if s.tx_rx_refcount.fetch_sub(1, Ordering::Relaxed) == 1 {
+        // Finally we can disable, and we do so for the peripheral
+        // i.e. not just rx concerns.
+        r.enable.write(|w| w.enable().disabled());
+
+        gpio::deconfigure_pin(r.psel.rxd.read().bits());
+        gpio::deconfigure_pin(r.psel.txd.read().bits());
+        gpio::deconfigure_pin(r.psel.rts.read().bits());
+        gpio::deconfigure_pin(r.psel.cts.read().bits());
+
+        trace!("uarte tx and rx drop: done");
+    }
+}
+
 pub(crate) mod sealed {
     use core::sync::atomic::AtomicU8;
 
@@ -1006,18 +908,6 @@ mod eh02 {
             Ok(())
         }
     }
-
-    impl<'d, U: Instance, T: TimerInstance> embedded_hal_02::blocking::serial::Write<u8> for UarteWithIdle<'d, U, T> {
-        type Error = Error;
-
-        fn bwrite_all(&mut self, buffer: &[u8]) -> Result<(), Self::Error> {
-            self.blocking_write(buffer)
-        }
-
-        fn bflush(&mut self) -> Result<(), Self::Error> {
-            Ok(())
-        }
-    }
 }
 
 #[cfg(feature = "unstable-traits")]
@@ -1067,10 +957,6 @@ mod eh1 {
     impl<'d, T: Instance> embedded_hal_1::serial::ErrorType for UarteRx<'d, T> {
         type Error = Error;
     }
-
-    impl<'d, U: Instance, T: TimerInstance> embedded_hal_1::serial::ErrorType for UarteWithIdle<'d, U, T> {
-        type Error = Error;
-    }
 }
 
 #[cfg(all(
@@ -1126,26 +1012,4 @@ mod eha {
             self.read(buffer)
         }
     }
-
-    impl<'d, U: Instance, T: TimerInstance> embedded_hal_async::serial::Read for UarteWithIdle<'d, U, T> {
-        type ReadFuture<'a> = impl Future<Output = Result<(), Self::Error>> + 'a where Self: 'a;
-
-        fn read<'a>(&'a mut self, buffer: &'a mut [u8]) -> Self::ReadFuture<'a> {
-            self.read(buffer)
-        }
-    }
-
-    impl<'d, U: Instance, T: TimerInstance> embedded_hal_async::serial::Write for UarteWithIdle<'d, U, T> {
-        type WriteFuture<'a> = impl Future<Output = Result<(), Self::Error>> + 'a where Self: 'a;
-
-        fn write<'a>(&'a mut self, buffer: &'a [u8]) -> Self::WriteFuture<'a> {
-            self.write(buffer)
-        }
-
-        type FlushFuture<'a> = impl Future<Output = Result<(), Self::Error>> + 'a where Self: 'a;
-
-        fn flush<'a>(&'a mut self) -> Self::FlushFuture<'a> {
-            async move { Ok(()) }
-        }
-    }
 }
diff --git a/examples/nrf/src/bin/uart_idle.rs b/examples/nrf/src/bin/uart_idle.rs
index 09ec624c..6af4f709 100644
--- a/examples/nrf/src/bin/uart_idle.rs
+++ b/examples/nrf/src/bin/uart_idle.rs
@@ -15,7 +15,8 @@ async fn main(_spawner: Spawner) {
     config.baudrate = uarte::Baudrate::BAUD115200;
 
     let irq = interrupt::take!(UARTE0_UART0);
-    let mut uart = uarte::UarteWithIdle::new(p.UARTE0, p.TIMER0, p.PPI_CH0, p.PPI_CH1, irq, p.P0_08, p.P0_06, config);
+    let uart = uarte::Uarte::new(p.UARTE0, irq, p.P0_08, p.P0_06, config);
+    let (mut tx, mut rx) = uart.split_with_idle(p.TIMER0, p.PPI_CH0, p.PPI_CH1);
 
     info!("uarte initialized!");
 
@@ -23,12 +24,12 @@ async fn main(_spawner: Spawner) {
     let mut buf = [0; 8];
     buf.copy_from_slice(b"Hello!\r\n");
 
-    unwrap!(uart.write(&buf).await);
+    unwrap!(tx.write(&buf).await);
     info!("wrote hello in uart!");
 
     loop {
         info!("reading...");
-        let n = unwrap!(uart.read_until_idle(&mut buf).await);
+        let n = unwrap!(rx.read_until_idle(&mut buf).await);
         info!("got {} bytes", n);
     }
 }