From bce1ce7dcb7fff272a2658a8dda08b70af679e7b Mon Sep 17 00:00:00 2001 From: Mathias Date: Thu, 16 Feb 2023 08:47:22 +0100 Subject: [PATCH 1/2] Allow upgrading a blocking uart to a BufferedUart, and implement blocking serial traits for BufferedUart --- embassy-rp/src/uart/buffered.rs | 340 +++++++++++++++++++++++++++----- embassy-rp/src/uart/mod.rs | 50 ++++- 2 files changed, 340 insertions(+), 50 deletions(-) diff --git a/embassy-rp/src/uart/buffered.rs b/embassy-rp/src/uart/buffered.rs index 0da5bfca1..32e5ddf14 100644 --- a/embassy-rp/src/uart/buffered.rs +++ b/embassy-rp/src/uart/buffered.rs @@ -28,30 +28,23 @@ impl State { } pub struct BufferedUart<'d, T: Instance> { - rx: BufferedUartRx<'d, T>, - tx: BufferedUartTx<'d, T>, + pub(crate) rx: BufferedUartRx<'d, T>, + pub(crate) tx: BufferedUartTx<'d, T>, } pub struct BufferedUartRx<'d, T: Instance> { - phantom: PhantomData<&'d mut T>, + pub(crate) phantom: PhantomData<&'d mut T>, } pub struct BufferedUartTx<'d, T: Instance> { - phantom: PhantomData<&'d mut T>, + pub(crate) phantom: PhantomData<&'d mut T>, } -fn init<'d, T: Instance + 'd>( +pub(crate) fn init_buffers<'d, T: Instance + 'd>( irq: PeripheralRef<'d, T::Interrupt>, - tx: Option>, - rx: Option>, - rts: Option>, - cts: Option>, tx_buffer: &'d mut [u8], rx_buffer: &'d mut [u8], - config: Config, ) { - super::Uart::<'d, T, Async>::init(tx, rx, rts, cts, config); - let state = T::state(); let len = tx_buffer.len(); unsafe { state.tx_buf.init(tx_buffer.as_mut_ptr(), len) }; @@ -69,7 +62,13 @@ fn init<'d, T: Instance + 'd>( // we clear it after it happens. The downside is that the we manually have // to pend the ISR when we want data transmission to start. let regs = T::regs(); - unsafe { regs.uartimsc().write_set(|w| w.set_txim(true)) }; + unsafe { + regs.uartimsc().write_set(|w| { + w.set_rxim(true); + w.set_rtim(true); + w.set_txim(true); + }); + }; irq.set_handler(on_interrupt::); irq.unpend(); @@ -87,16 +86,10 @@ impl<'d, T: Instance> BufferedUart<'d, T> { config: Config, ) -> Self { into_ref!(irq, tx, rx); - init::( - irq, - Some(tx.map_into()), - Some(rx.map_into()), - None, - None, - tx_buffer, - rx_buffer, - config, - ); + + super::Uart::<'d, T, Async>::init(Some(tx.map_into()), Some(rx.map_into()), None, None, config); + init_buffers::(irq, tx_buffer, rx_buffer); + Self { rx: BufferedUartRx { phantom: PhantomData }, tx: BufferedUartTx { phantom: PhantomData }, @@ -115,22 +108,34 @@ impl<'d, T: Instance> BufferedUart<'d, T> { config: Config, ) -> Self { into_ref!(irq, tx, rx, cts, rts); - init::( - irq, + + super::Uart::<'d, T, Async>::init( Some(tx.map_into()), Some(rx.map_into()), Some(rts.map_into()), Some(cts.map_into()), - tx_buffer, - rx_buffer, config, ); + init_buffers::(irq, tx_buffer, rx_buffer); + Self { rx: BufferedUartRx { phantom: PhantomData }, tx: BufferedUartTx { phantom: PhantomData }, } } + pub fn blocking_write(&mut self, buffer: &[u8]) -> Result<(), Error> { + self.tx.blocking_write(buffer) + } + + pub fn blocking_flush(&mut self) -> Result<(), Error> { + self.tx.blocking_flush() + } + + pub fn blocking_read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { + self.rx.blocking_read(buffer) + } + pub fn split(self) -> (BufferedUartRx<'d, T>, BufferedUartTx<'d, T>) { (self.rx, self.tx) } @@ -145,7 +150,10 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> { config: Config, ) -> Self { into_ref!(irq, rx); - init::(irq, None, Some(rx.map_into()), None, None, &mut [], rx_buffer, config); + + super::Uart::<'d, T, Async>::init(None, Some(rx.map_into()), None, None, config); + init_buffers::(irq, &mut [], rx_buffer); + Self { phantom: PhantomData } } @@ -158,16 +166,10 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> { config: Config, ) -> Self { into_ref!(irq, rx, rts); - init::( - irq, - None, - Some(rx.map_into()), - Some(rts.map_into()), - None, - &mut [], - rx_buffer, - config, - ); + + super::Uart::<'d, T, Async>::init(None, Some(rx.map_into()), Some(rts.map_into()), None, config); + init_buffers::(irq, &mut [], rx_buffer); + Self { phantom: PhantomData } } @@ -199,6 +201,32 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> { }) } + pub fn blocking_read(&mut self, buf: &mut [u8]) -> Result<(), Error> { + loop { + let state = T::state(); + let mut rx_reader = unsafe { state.rx_buf.reader() }; + let n = rx_reader.pop(|data| { + let n = data.len().min(buf.len()); + buf[..n].copy_from_slice(&data[..n]); + n + }); + + if n > 0 { + // (Re-)Enable the interrupt to receive more data in case it was + // disabled because the buffer was full. + let regs = T::regs(); + unsafe { + regs.uartimsc().write_set(|w| { + w.set_rxim(true); + w.set_rtim(true); + }); + } + + return Ok(()); + } + } + } + fn fill_buf<'a>() -> impl Future> { poll_fn(move |cx| { let state = T::state(); @@ -240,7 +268,10 @@ impl<'d, T: Instance> BufferedUartTx<'d, T> { config: Config, ) -> Self { into_ref!(irq, tx); - init::(irq, Some(tx.map_into()), None, None, None, tx_buffer, &mut [], config); + + super::Uart::<'d, T, Async>::init(Some(tx.map_into()), None, None, None, config); + init_buffers::(irq, tx_buffer, &mut []); + Self { phantom: PhantomData } } @@ -253,16 +284,10 @@ impl<'d, T: Instance> BufferedUartTx<'d, T> { config: Config, ) -> Self { into_ref!(irq, tx, cts); - init::( - irq, - Some(tx.map_into()), - None, - None, - Some(cts.map_into()), - tx_buffer, - &mut [], - config, - ); + + super::Uart::<'d, T, Async>::init(Some(tx.map_into()), None, None, Some(cts.map_into()), config); + init_buffers::(irq, tx_buffer, &mut []); + Self { phantom: PhantomData } } @@ -300,6 +325,36 @@ impl<'d, T: Instance> BufferedUartTx<'d, T> { Poll::Ready(Ok(())) }) } + + pub fn blocking_write(&mut self, buf: &[u8]) -> Result<(), Error> { + loop { + let state = T::state(); + let mut tx_writer = unsafe { state.tx_buf.writer() }; + let n = tx_writer.push(|data| { + let n = data.len().min(buf.len()); + data[..n].copy_from_slice(&buf[..n]); + n + }); + + if n != 0 { + // The TX interrupt only triggers when the there was data in the + // FIFO and the number of bytes drops below a threshold. When the + // FIFO was empty we have to manually pend the interrupt to shovel + // TX data from the buffer into the FIFO. + unsafe { T::Interrupt::steal() }.pend(); + return Ok(()); + } + } + } + + pub fn blocking_flush(&mut self) -> Result<(), Error> { + loop { + let state = T::state(); + if state.tx_buf.is_empty() { + return Ok(()); + } + } + } } impl<'d, T: Instance> Drop for BufferedUartRx<'d, T> { @@ -477,3 +532,190 @@ impl<'d, T: Instance + 'd> embedded_io::asynch::Write for BufferedUartTx<'d, T> Self::flush().await } } + +mod eh02 { + use super::*; + + impl<'d, T: Instance> embedded_hal_02::serial::Read for BufferedUartRx<'d, T> { + type Error = Error; + + fn read(&mut self) -> Result> { + let r = T::regs(); + unsafe { + if r.uartfr().read().rxfe() { + return Err(nb::Error::WouldBlock); + } + + let dr = r.uartdr().read(); + + if dr.oe() { + Err(nb::Error::Other(Error::Overrun)) + } else if dr.be() { + Err(nb::Error::Other(Error::Break)) + } else if dr.pe() { + Err(nb::Error::Other(Error::Parity)) + } else if dr.fe() { + Err(nb::Error::Other(Error::Framing)) + } else { + Ok(dr.data()) + } + } + } + } + + impl<'d, T: Instance> embedded_hal_02::blocking::serial::Write for BufferedUartTx<'d, 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> { + self.blocking_flush() + } + } + + impl<'d, T: Instance> embedded_hal_02::serial::Read for BufferedUart<'d, T> { + type Error = Error; + + fn read(&mut self) -> Result> { + embedded_hal_02::serial::Read::read(&mut self.rx) + } + } + + impl<'d, T: Instance> embedded_hal_02::blocking::serial::Write for BufferedUart<'d, 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> { + self.blocking_flush() + } + } +} + +#[cfg(feature = "unstable-traits")] +mod eh1 { + use super::*; + + impl<'d, T: Instance> embedded_hal_1::serial::ErrorType for BufferedUart<'d, T> { + type Error = Error; + } + + impl<'d, T: Instance> embedded_hal_1::serial::ErrorType for BufferedUartTx<'d, T> { + type Error = Error; + } + + impl<'d, T: Instance> embedded_hal_1::serial::ErrorType for BufferedUartRx<'d, T> { + type Error = Error; + } + + impl<'d, T: Instance> embedded_hal_nb::serial::Read for BufferedUartRx<'d, T> { + fn read(&mut self) -> nb::Result { + embedded_hal_02::serial::Read::read(self) + } + } + + impl<'d, T: Instance> embedded_hal_1::serial::Write for BufferedUartTx<'d, T> { + fn write(&mut self, buffer: &[u8]) -> Result<(), Self::Error> { + self.blocking_write(buffer) + } + + fn flush(&mut self) -> Result<(), Self::Error> { + self.blocking_flush() + } + } + + impl<'d, T: Instance> embedded_hal_nb::serial::Write for BufferedUartTx<'d, T> { + fn write(&mut self, char: u8) -> nb::Result<(), Self::Error> { + self.blocking_write(&[char]).map_err(nb::Error::Other) + } + + fn flush(&mut self) -> nb::Result<(), Self::Error> { + self.blocking_flush().map_err(nb::Error::Other) + } + } + + impl<'d, T: Instance> embedded_hal_nb::serial::Read for BufferedUart<'d, T> { + fn read(&mut self) -> Result> { + embedded_hal_02::serial::Read::read(&mut self.rx) + } + } + + impl<'d, T: Instance> embedded_hal_1::serial::Write for BufferedUart<'d, T> { + fn write(&mut self, buffer: &[u8]) -> Result<(), Self::Error> { + self.blocking_write(buffer) + } + + fn flush(&mut self) -> Result<(), Self::Error> { + self.blocking_flush() + } + } + + impl<'d, T: Instance> embedded_hal_nb::serial::Write for BufferedUart<'d, T> { + fn write(&mut self, char: u8) -> nb::Result<(), Self::Error> { + self.blocking_write(&[char]).map_err(nb::Error::Other) + } + + fn flush(&mut self) -> nb::Result<(), Self::Error> { + self.blocking_flush().map_err(nb::Error::Other) + } + } +} + +#[cfg(all( + feature = "unstable-traits", + feature = "nightly", + feature = "_todo_embedded_hal_serial" +))] +mod eha { + use core::future::Future; + + use super::*; + + impl<'d, T: Instance> embedded_hal_async::serial::Write for BufferedUartTx<'d, T> { + type WriteFuture<'a> = impl Future> + 'a where Self: 'a; + + fn write<'a>(&'a mut self, buf: &'a [u8]) -> Self::WriteFuture<'a> { + Self::write(buf) + } + + type FlushFuture<'a> = impl Future> + 'a where Self: 'a; + + fn flush<'a>(&'a mut self) -> Self::FlushFuture<'a> { + Self::flush() + } + } + + impl<'d, T: Instance> embedded_hal_async::serial::Read for BufferedUartRx<'d, T> { + type ReadFuture<'a> = impl Future> + 'a where Self: 'a; + + fn read<'a>(&'a mut self, buf: &'a mut [u8]) -> Self::ReadFuture<'a> { + Self::read(buf) + } + } + + impl<'d, T: Instance> embedded_hal_async::serial::Write for BufferedUart<'d, T> { + type WriteFuture<'a> = impl Future> + 'a where Self: 'a; + + fn write<'a>(&'a mut self, buf: &'a [u8]) -> Self::WriteFuture<'a> { + BufferedUartTx::<'d, T>::write(buf) + } + + type FlushFuture<'a> = impl Future> + 'a where Self: 'a; + + fn flush<'a>(&'a mut self) -> Self::FlushFuture<'a> { + BufferedUartTx::<'d, T>::flush() + } + } + + impl<'d, T: Instance> embedded_hal_async::serial::Read for BufferedUart<'d, T> { + type ReadFuture<'a> = impl Future> + 'a where Self: 'a; + + fn read<'a>(&'a mut self, buf: &'a mut [u8]) -> Self::ReadFuture<'a> { + BufferedUartRx::<'d, T>::read(buf) + } + } +} diff --git a/embassy-rp/src/uart/mod.rs b/embassy-rp/src/uart/mod.rs index 42b3671a0..97f4463e5 100644 --- a/embassy-rp/src/uart/mod.rs +++ b/embassy-rp/src/uart/mod.rs @@ -1,11 +1,12 @@ use core::marker::PhantomData; +use embassy_cortex_m::interrupt::InterruptExt; use embassy_hal_common::{into_ref, PeripheralRef}; use crate::dma::{AnyChannel, Channel}; use crate::gpio::sealed::Pin; use crate::gpio::AnyPin; -use crate::{pac, peripherals, Peripheral}; +use crate::{pac, peripherals, Peripheral, RegExt}; #[cfg(feature = "nightly")] mod buffered; @@ -135,6 +136,21 @@ impl<'d, T: Instance, M: Mode> UartTx<'d, T, M> { } } +impl<'d, T: Instance> UartTx<'d, T, Blocking> { + #[cfg(feature = "nightly")] + pub fn into_buffered( + self, + irq: impl Peripheral

+ 'd, + tx_buffer: &'d mut [u8], + ) -> BufferedUartTx<'d, T> { + into_ref!(irq); + + buffered::init_buffers::(irq, tx_buffer, &mut []); + + BufferedUartTx { phantom: PhantomData } + } +} + impl<'d, T: Instance> UartTx<'d, T, Async> { pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { let ch = self.tx_dma.as_mut().unwrap(); @@ -200,6 +216,21 @@ impl<'d, T: Instance, M: Mode> UartRx<'d, T, M> { } } +impl<'d, T: Instance> UartRx<'d, T, Blocking> { + #[cfg(feature = "nightly")] + pub fn into_buffered( + self, + irq: impl Peripheral

+ 'd, + rx_buffer: &'d mut [u8], + ) -> BufferedUartRx<'d, T> { + into_ref!(irq); + + buffered::init_buffers::(irq, &mut [], rx_buffer); + + BufferedUartRx { phantom: PhantomData } + } +} + impl<'d, T: Instance> UartRx<'d, T, Async> { pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { let ch = self.rx_dma.as_mut().unwrap(); @@ -249,6 +280,23 @@ impl<'d, T: Instance> Uart<'d, T, Blocking> { config, ) } + + #[cfg(feature = "nightly")] + pub fn into_buffered( + self, + irq: impl Peripheral

+ 'd, + tx_buffer: &'d mut [u8], + rx_buffer: &'d mut [u8], + ) -> BufferedUart<'d, T> { + into_ref!(irq); + + buffered::init_buffers::(irq, tx_buffer, rx_buffer); + + BufferedUart { + rx: BufferedUartRx { phantom: PhantomData }, + tx: BufferedUartTx { phantom: PhantomData }, + } + } } impl<'d, T: Instance> Uart<'d, T, Async> { From 89a371d10c14768e45846d09d98682ec4ae7a229 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 14 Mar 2023 10:54:15 +0100 Subject: [PATCH 2/2] Add HIL test for into_buffered uart on embassy-rp --- embassy-rp/src/uart/mod.rs | 3 +- tests/rp/src/bin/uart_upgrade.rs | 54 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 tests/rp/src/bin/uart_upgrade.rs diff --git a/embassy-rp/src/uart/mod.rs b/embassy-rp/src/uart/mod.rs index 97f4463e5..682243a27 100644 --- a/embassy-rp/src/uart/mod.rs +++ b/embassy-rp/src/uart/mod.rs @@ -1,12 +1,11 @@ use core::marker::PhantomData; -use embassy_cortex_m::interrupt::InterruptExt; use embassy_hal_common::{into_ref, PeripheralRef}; use crate::dma::{AnyChannel, Channel}; use crate::gpio::sealed::Pin; use crate::gpio::AnyPin; -use crate::{pac, peripherals, Peripheral, RegExt}; +use crate::{pac, peripherals, Peripheral}; #[cfg(feature = "nightly")] mod buffered; diff --git a/tests/rp/src/bin/uart_upgrade.rs b/tests/rp/src/bin/uart_upgrade.rs new file mode 100644 index 000000000..d8c9aecf6 --- /dev/null +++ b/tests/rp/src/bin/uart_upgrade.rs @@ -0,0 +1,54 @@ +#![no_std] +#![no_main] +#![feature(type_alias_impl_trait)] + +use defmt::{assert_eq, *}; +use embassy_executor::Spawner; +use embassy_rp::interrupt; +use embassy_rp::uart::{Config, Uart}; +use embedded_io::asynch::{Read, Write}; +use {defmt_rtt as _, panic_probe as _}; + +#[embassy_executor::main] +async fn main(_spawner: Spawner) { + let p = embassy_rp::init(Default::default()); + info!("Hello World!"); + + let (tx, rx, uart) = (p.PIN_0, p.PIN_1, p.UART0); + + let config = Config::default(); + let mut uart = Uart::new_blocking(uart, tx, rx, config); + + // We can't send too many bytes, they have to fit in the FIFO. + // This is because we aren't sending+receiving at the same time. + + let data = [0xC0, 0xDE]; + uart.blocking_write(&data).unwrap(); + + let mut buf = [0; 2]; + uart.blocking_read(&mut buf).unwrap(); + assert_eq!(buf, data); + + let irq = interrupt::take!(UART0_IRQ); + let tx_buf = &mut [0u8; 16]; + let rx_buf = &mut [0u8; 16]; + + let mut uart = uart.into_buffered(irq, tx_buf, rx_buf); + + // Make sure we send more bytes than fits in the FIFO, to test the actual + // bufferedUart. + + let data = [ + 1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, + 30, 31, + ]; + uart.write_all(&data).await.unwrap(); + info!("Done writing"); + + let mut buf = [0; 31]; + uart.read_exact(&mut buf).await.unwrap(); + assert_eq!(buf, data); + + info!("Test OK"); + cortex_m::asm::bkpt(); +}