From e1faf8860776f6ad2bac2f3b06e7160fe00da7df Mon Sep 17 00:00:00 2001
From: huntc <huntchr@gmail.com>
Date: Sun, 9 Oct 2022 13:07:25 +1100
Subject: [PATCH] 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.
---
 embassy-nrf/src/uarte.rs          | 468 +++++++++++-------------------
 examples/nrf/src/bin/uart_idle.rs |   7 +-
 2 files changed, 170 insertions(+), 305 deletions(-)

diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs
index d99599112..636d6c7a3 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,6 +652,117 @@ impl<'a, T: Instance> Drop for UarteRx<'a, 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, 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
+    }
+
+    pub fn blocking_read(&mut self, buffer: &mut [u8]) -> Result<(), Error> {
+        self.ppi_ch1.disable();
+        self.rx.blocking_read(buffer)
+    }
+
+    pub async fn read_until_idle(&mut self, buffer: &mut [u8]) -> Result<usize, Error> {
+        if buffer.len() == 0 {
+            return Err(Error::BufferZeroLength);
+        }
+        if buffer.len() > EASY_DMA_SIZE {
+            return Err(Error::BufferTooLong);
+        }
+
+        let ptr = buffer.as_ptr();
+        let len = buffer.len();
+
+        let r = T::regs();
+        let s = T::state();
+
+        self.ppi_ch1.enable();
+
+        let drop = OnDrop::new(|| {
+            self.timer.stop();
+
+            r.intenclr.write(|w| w.endrx().clear());
+            r.events_rxto.reset();
+            r.tasks_stoprx.write(|w| unsafe { w.bits(1) });
+
+            while r.events_endrx.read().bits() == 0 {}
+        });
+
+        r.rxd.ptr.write(|w| unsafe { w.ptr().bits(ptr as u32) });
+        r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) });
+
+        r.events_endrx.reset();
+        r.intenset.write(|w| w.endrx().set());
+
+        compiler_fence(Ordering::SeqCst);
+
+        r.tasks_startrx.write(|w| unsafe { w.bits(1) });
+
+        poll_fn(|cx| {
+            s.endrx_waker.register(cx.waker());
+            if r.events_endrx.read().bits() != 0 {
+                return Poll::Ready(());
+            }
+            Poll::Pending
+        })
+        .await;
+
+        compiler_fence(Ordering::SeqCst);
+        let n = r.rxd.amount.read().amount().bits() as usize;
+
+        self.timer.stop();
+        r.events_rxstarted.reset();
+
+        drop.defuse();
+
+        Ok(n)
+    }
+
+    pub fn blocking_read_until_idle(&mut self, buffer: &mut [u8]) -> Result<usize, Error> {
+        if buffer.len() == 0 {
+            return Err(Error::BufferZeroLength);
+        }
+        if buffer.len() > EASY_DMA_SIZE {
+            return Err(Error::BufferTooLong);
+        }
+
+        let ptr = buffer.as_ptr();
+        let len = buffer.len();
+
+        let r = T::regs();
+
+        self.ppi_ch1.enable();
+
+        r.rxd.ptr.write(|w| unsafe { w.ptr().bits(ptr as u32) });
+        r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) });
+
+        r.events_endrx.reset();
+        r.intenclr.write(|w| w.endrx().clear());
+
+        compiler_fence(Ordering::SeqCst);
+
+        r.tasks_startrx.write(|w| unsafe { w.bits(1) });
+
+        while r.events_endrx.read().bits() == 0 {}
+
+        compiler_fence(Ordering::SeqCst);
+        let n = r.rxd.amount.read().amount().bits() as usize;
+
+        self.timer.stop();
+        r.events_rxstarted.reset();
+
+        Ok(n)
+    }
+}
+
 #[cfg(not(any(feature = "_nrf9160", feature = "nrf5340")))]
 pub(crate) fn apply_workaround_for_enable_anomaly(_r: &crate::pac::uarte0::RegisterBlock) {
     // Do nothing
@@ -665,270 +831,6 @@ pub(crate) fn drop_tx_rx(r: &pac::uarte0::RegisterBlock, s: &sealed::State) {
     }
 }
 
-/// 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>,
-    ppi_ch1: Ppi<'d, AnyConfigurableChannel, 1, 2>,
-    _ppi_ch2: Ppi<'d, AnyConfigurableChannel, 1, 1>,
-}
-
-impl<'d, U: Instance, T: TimerInstance> UarteRxWithIdle<'d, U, T> {
-    pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> {
-        self.ppi_ch1.disable();
-        self.rx.read(buffer).await
-    }
-
-    pub fn blocking_read(&mut self, buffer: &mut [u8]) -> Result<(), Error> {
-        self.ppi_ch1.disable();
-        self.rx.blocking_read(buffer)
-    }
-
-    pub async fn read_until_idle(&mut self, buffer: &mut [u8]) -> Result<usize, Error> {
-        if buffer.len() == 0 {
-            return Err(Error::BufferZeroLength);
-        }
-        if buffer.len() > EASY_DMA_SIZE {
-            return Err(Error::BufferTooLong);
-        }
-
-        let ptr = buffer.as_ptr();
-        let len = buffer.len();
-
-        let r = U::regs();
-        let s = U::state();
-
-        self.ppi_ch1.enable();
-
-        let drop = OnDrop::new(|| {
-            self.timer.stop();
-
-            r.intenclr.write(|w| w.endrx().clear());
-            r.events_rxto.reset();
-            r.tasks_stoprx.write(|w| unsafe { w.bits(1) });
-
-            while r.events_endrx.read().bits() == 0 {}
-        });
-
-        r.rxd.ptr.write(|w| unsafe { w.ptr().bits(ptr as u32) });
-        r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) });
-
-        r.events_endrx.reset();
-        r.intenset.write(|w| w.endrx().set());
-
-        compiler_fence(Ordering::SeqCst);
-
-        r.tasks_startrx.write(|w| unsafe { w.bits(1) });
-
-        poll_fn(|cx| {
-            s.endrx_waker.register(cx.waker());
-            if r.events_endrx.read().bits() != 0 {
-                return Poll::Ready(());
-            }
-            Poll::Pending
-        })
-        .await;
-
-        compiler_fence(Ordering::SeqCst);
-        let n = r.rxd.amount.read().amount().bits() as usize;
-
-        self.timer.stop();
-        r.events_rxstarted.reset();
-
-        drop.defuse();
-
-        Ok(n)
-    }
-
-    pub fn blocking_read_until_idle(&mut self, buffer: &mut [u8]) -> Result<usize, Error> {
-        if buffer.len() == 0 {
-            return Err(Error::BufferZeroLength);
-        }
-        if buffer.len() > EASY_DMA_SIZE {
-            return Err(Error::BufferTooLong);
-        }
-
-        let ptr = buffer.as_ptr();
-        let len = buffer.len();
-
-        let r = U::regs();
-
-        self.ppi_ch1.enable();
-
-        r.rxd.ptr.write(|w| unsafe { w.ptr().bits(ptr as u32) });
-        r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) });
-
-        r.events_endrx.reset();
-        r.intenclr.write(|w| w.endrx().clear());
-
-        compiler_fence(Ordering::SeqCst);
-
-        r.tasks_startrx.write(|w| unsafe { w.bits(1) });
-
-        while r.events_endrx.read().bits() == 0 {}
-
-        compiler_fence(Ordering::SeqCst);
-        let n = r.rxd.amount.read().amount().bits() as usize;
-
-        self.timer.stop();
-        r.events_rxstarted.reset();
-
-        Ok(n)
-    }
-}
 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 09ec624c0..6af4f7097 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);
     }
 }