diff --git a/embassy-rp/src/uart/buffered.rs b/embassy-rp/src/uart/buffered.rs index 3fe0c8c4..58fede76 100644 --- a/embassy-rp/src/uart/buffered.rs +++ b/embassy-rp/src/uart/buffered.rs @@ -2,6 +2,7 @@ use core::future::{poll_fn, Future}; use core::slice; use core::task::Poll; +use atomic_polyfill::{AtomicU8, Ordering}; use embassy_cortex_m::interrupt::{Interrupt, InterruptExt}; use embassy_hal_common::atomic_ring_buffer::RingBuffer; use embassy_sync::waitqueue::AtomicWaker; @@ -16,8 +17,15 @@ pub struct State { tx_buf: RingBuffer, rx_waker: AtomicWaker, rx_buf: RingBuffer, + rx_error: AtomicU8, } +// these must match bits 8..11 in UARTDR +const RXE_OVERRUN: u8 = 8; +const RXE_BREAK: u8 = 4; +const RXE_PARITY: u8 = 2; +const RXE_FRAMING: u8 = 1; + impl State { pub const fn new() -> Self { Self { @@ -25,6 +33,7 @@ impl State { tx_buf: RingBuffer::new(), rx_waker: AtomicWaker::new(), tx_waker: AtomicWaker::new(), + rx_error: AtomicU8::new(0), } } } @@ -196,7 +205,25 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> { }) } - fn try_read(buf: &mut [u8]) -> Poll> { + fn get_rx_error() -> Option { + let errs = T::buffered_state().rx_error.swap(0, Ordering::Relaxed); + if errs & RXE_OVERRUN != 0 { + Some(Error::Overrun) + } else if errs & RXE_BREAK != 0 { + Some(Error::Break) + } else if errs & RXE_PARITY != 0 { + Some(Error::Parity) + } else if errs & RXE_FRAMING != 0 { + Some(Error::Framing) + } else { + None + } + } + + fn try_read(buf: &mut [u8]) -> Poll> + where + T: 'd, + { if buf.is_empty() { return Poll::Ready(Ok(0)); } @@ -210,13 +237,16 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> { }); let result = if n == 0 { - return Poll::Pending; + match Self::get_rx_error() { + None => return Poll::Pending, + Some(e) => Err(e), + } } else { Ok(n) }; // (Re-)Enable the interrupt to receive more data in case it was - // disabled because the buffer was full. + // disabled because the buffer was full or errors were detected. let regs = T::regs(); unsafe { regs.uartimsc().write_set(|w| { @@ -237,18 +267,28 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> { } } - fn fill_buf<'a>() -> impl Future> { + fn fill_buf<'a>() -> impl Future> + where + T: 'd, + { poll_fn(move |cx| { let state = T::buffered_state(); let mut rx_reader = unsafe { state.rx_buf.reader() }; let (p, n) = rx_reader.pop_buf(); - if n == 0 { - state.rx_waker.register(cx.waker()); - return Poll::Pending; - } + let result = if n == 0 { + match Self::get_rx_error() { + None => { + state.rx_waker.register(cx.waker()); + return Poll::Pending; + } + Some(e) => Err(e), + } + } else { + let buf = unsafe { slice::from_raw_parts(p, n) }; + Ok(buf) + }; - let buf = unsafe { slice::from_raw_parts(p, n) }; - Poll::Ready(Ok(buf)) + Poll::Ready(result) }) } @@ -258,7 +298,7 @@ impl<'d, T: Instance> BufferedUartRx<'d, T> { rx_reader.pop_done(amt); // (Re-)Enable the interrupt to receive more data in case it was - // disabled because the buffer was full. + // disabled because the buffer was full or errors were detected. let regs = T::regs(); unsafe { regs.uartimsc().write_set(|w| { @@ -478,19 +518,37 @@ pub(crate) unsafe fn on_interrupt(_: *mut ()) { let mut rx_writer = s.rx_buf.writer(); let rx_buf = rx_writer.push_slice(); let mut n_read = 0; + let mut error = false; for rx_byte in rx_buf { if r.uartfr().read().rxfe() { break; } - *rx_byte = r.uartdr().read().data(); + let dr = r.uartdr().read(); + if (dr.0 >> 8) != 0 { + s.rx_error.fetch_or((dr.0 >> 8) as u8, Ordering::Relaxed); + error = true; + // only fill the buffer with valid characters. the current character is fine + // if the error is an overrun, but if we add it to the buffer we'll report + // the overrun one character too late. drop it instead and pretend we were + // a bit slower at draining the rx fifo than we actually were. + // this is consistent with blocking uart error reporting. + break; + } + *rx_byte = dr.data(); n_read += 1; } if n_read > 0 { rx_writer.push_done(n_read); s.rx_waker.wake(); + } else if error { + s.rx_waker.wake(); } - // Disable any further RX interrupts when the buffer becomes full. - if s.rx_buf.is_full() { + // Disable any further RX interrupts when the buffer becomes full or + // errors have occured. this lets us buffer additional errors in the + // fifo without needing more error storage locations, and most applications + // will want to do a full reset of their uart state anyway once an error + // has happened. + if s.rx_buf.is_full() || error { r.uartimsc().write_clear(|w| { w.set_rxim(true); w.set_rtim(true); diff --git a/tests/rp/src/bin/uart_buffered.rs b/tests/rp/src/bin/uart_buffered.rs index a1f0e2bf..1deb22ce 100644 --- a/tests/rp/src/bin/uart_buffered.rs +++ b/tests/rp/src/bin/uart_buffered.rs @@ -2,39 +2,248 @@ #![no_main] #![feature(type_alias_impl_trait)] -use defmt::{assert_eq, *}; +use defmt::{assert_eq, panic, *}; use embassy_executor::Spawner; +use embassy_rp::gpio::{Level, Output}; use embassy_rp::interrupt; -use embassy_rp::uart::{BufferedUart, Config}; -use embedded_io::asynch::{Read, Write}; +use embassy_rp::uart::{BufferedUart, BufferedUartRx, Config, Error, Instance, Parity}; +use embassy_time::{Duration, Timer}; +use embedded_io::asynch::{Read, ReadExactError, Write}; use {defmt_rtt as _, panic_probe as _}; +async fn read(uart: &mut BufferedUart<'_, impl Instance>) -> Result<[u8; N], Error> { + let mut buf = [255; N]; + match uart.read_exact(&mut buf).await { + Ok(()) => Ok(buf), + // we should not ever produce an Eof condition + Err(ReadExactError::UnexpectedEof) => panic!(), + Err(ReadExactError::Other(e)) => Err(e), + } +} + +async fn read1(uart: &mut BufferedUartRx<'_, impl Instance>) -> Result<[u8; N], Error> { + let mut buf = [255; N]; + match uart.read_exact(&mut buf).await { + Ok(()) => Ok(buf), + // we should not ever produce an Eof condition + Err(ReadExactError::UnexpectedEof) => panic!(), + Err(ReadExactError::Other(e)) => Err(e), + } +} + +async fn send(pin: &mut Output<'_, impl embassy_rp::gpio::Pin>, v: u8, parity: Option) { + pin.set_low(); + Timer::after(Duration::from_millis(1)).await; + for i in 0..8 { + if v & (1 << i) == 0 { + pin.set_low(); + } else { + pin.set_high(); + } + Timer::after(Duration::from_millis(1)).await; + } + if let Some(b) = parity { + if b { + pin.set_high(); + } else { + pin.set_low(); + } + Timer::after(Duration::from_millis(1)).await; + } + pin.set_high(); + Timer::after(Duration::from_millis(1)).await; +} + #[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 (mut tx, mut rx, mut uart) = (p.PIN_0, p.PIN_1, p.UART0); + let mut irq = interrupt::take!(UART0_IRQ); - let config = Config::default(); - let irq = interrupt::take!(UART0_IRQ); - let tx_buf = &mut [0u8; 16]; - let rx_buf = &mut [0u8; 16]; - let mut uart = BufferedUart::new(uart, irq, tx, rx, tx_buf, rx_buf, config); + { + let config = Config::default(); + let tx_buf = &mut [0u8; 16]; + let rx_buf = &mut [0u8; 16]; + let mut uart = BufferedUart::new(&mut uart, &mut irq, &mut tx, &mut rx, tx_buf, rx_buf, config); - // Make sure we send more bytes than fits in the FIFO, to test the actual - // bufferedUart. + // Make sure we send more bytes than fits in the FIFO, to test the actual + // bufferedUart. - let data = [ - 1, 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, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, - ]; - uart.write_all(&data).await.unwrap(); - info!("Done writing"); + let data = [ + 1, 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, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, + ]; + uart.write_all(&data).await.unwrap(); + info!("Done writing"); - let mut buf = [0; 48]; - uart.read_exact(&mut buf).await.unwrap(); - assert_eq!(buf, data); + assert_eq!(read(&mut uart).await.unwrap(), data); + } + + info!("test overflow detection"); + { + let config = Config::default(); + let tx_buf = &mut [0u8; 16]; + let rx_buf = &mut [0u8; 16]; + let mut uart = BufferedUart::new(&mut uart, &mut irq, &mut tx, &mut rx, tx_buf, rx_buf, config); + + // Make sure we send more bytes than fits in the FIFO, to test the actual + // bufferedUart. + + let data = [ + 1, 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, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, + ]; + let overflow = [ + 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, + ]; + // give each block time to settle into the fifo. we want the overrun to occur at a well-defined point. + uart.write_all(&data).await.unwrap(); + uart.blocking_flush().unwrap(); + while uart.busy() {} + uart.write_all(&overflow).await.unwrap(); + uart.blocking_flush().unwrap(); + while uart.busy() {} + + // already buffered/fifod prefix is valid + assert_eq!(read(&mut uart).await.unwrap(), data); + // next received character causes overrun error and is discarded + uart.write_all(&[1, 2, 3]).await.unwrap(); + uart.blocking_flush().unwrap(); + assert_eq!(read::<1>(&mut uart).await.unwrap_err(), Error::Overrun); + assert_eq!(read(&mut uart).await.unwrap(), [2, 3]); + } + + info!("test break detection"); + { + let mut config = Config::default(); + config.baudrate = 1000; + let tx_buf = &mut [0u8; 16]; + let rx_buf = &mut [0u8; 16]; + let mut uart = BufferedUart::new(&mut uart, &mut irq, &mut tx, &mut rx, tx_buf, rx_buf, config); + + // break on empty buffer + uart.send_break(20).await; + assert_eq!(read::<1>(&mut uart).await.unwrap_err(), Error::Break); + uart.write_all(&[64]).await.unwrap(); + assert_eq!(read(&mut uart).await.unwrap(), [64]); + + // break on partially filled buffer + uart.write_all(&[65; 2]).await.unwrap(); + uart.send_break(20).await; + uart.write_all(&[66]).await.unwrap(); + assert_eq!(read(&mut uart).await.unwrap(), [65; 2]); + assert_eq!(read::<1>(&mut uart).await.unwrap_err(), Error::Break); + assert_eq!(read(&mut uart).await.unwrap(), [66]); + + // break on full buffer + uart.write_all(&[64; 16]).await.unwrap(); + uart.send_break(20).await; + uart.write_all(&[65]).await.unwrap(); + assert_eq!(read(&mut uart).await.unwrap(), [64; 16]); + assert_eq!(read::<1>(&mut uart).await.unwrap_err(), Error::Break); + assert_eq!(read(&mut uart).await.unwrap(), [65]); + } + + // parity detection. here we bitbang to not require two uarts. + info!("test parity error detection"); + { + let mut pin = Output::new(&mut tx, Level::High); + // choose a very slow baud rate to make tests reliable even with O0 + let mut config = Config::default(); + config.baudrate = 1000; + config.parity = Parity::ParityEven; + let rx_buf = &mut [0u8; 16]; + let mut uart = BufferedUartRx::new(&mut uart, &mut irq, &mut rx, rx_buf, config); + + async fn chr(pin: &mut Output<'_, impl embassy_rp::gpio::Pin>, v: u8, parity: u32) { + send(pin, v, Some(parity != 0)).await; + } + + // first check that we can send correctly + chr(&mut pin, 64, 1).await; + assert_eq!(read1(&mut uart).await.unwrap(), [64]); + + // parity on empty buffer + chr(&mut pin, 64, 0).await; + chr(&mut pin, 4, 1).await; + assert_eq!(read1::<1>(&mut uart).await.unwrap_err(), Error::Parity); + assert_eq!(read1(&mut uart).await.unwrap(), [4]); + + // parity on partially filled buffer + chr(&mut pin, 64, 1).await; + chr(&mut pin, 32, 1).await; + chr(&mut pin, 64, 0).await; + chr(&mut pin, 65, 0).await; + assert_eq!(read1(&mut uart).await.unwrap(), [64, 32]); + assert_eq!(read1::<1>(&mut uart).await.unwrap_err(), Error::Parity); + assert_eq!(read1(&mut uart).await.unwrap(), [65]); + + // parity on full buffer + for i in 0..16 { + chr(&mut pin, i, i.count_ones() % 2).await; + } + chr(&mut pin, 64, 0).await; + chr(&mut pin, 65, 0).await; + assert_eq!( + read1(&mut uart).await.unwrap(), + [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15] + ); + assert_eq!(read1::<1>(&mut uart).await.unwrap_err(), Error::Parity); + assert_eq!(read1(&mut uart).await.unwrap(), [65]); + } + + // framing error detection. here we bitbang because there's no other way. + info!("test framing error detection"); + { + let mut pin = Output::new(&mut tx, Level::High); + // choose a very slow baud rate to make tests reliable even with O0 + let mut config = Config::default(); + config.baudrate = 1000; + let rx_buf = &mut [0u8; 16]; + let mut uart = BufferedUartRx::new(&mut uart, &mut irq, &mut rx, rx_buf, config); + + async fn chr(pin: &mut Output<'_, impl embassy_rp::gpio::Pin>, v: u8, good: bool) { + if good { + send(pin, v, None).await; + } else { + send(pin, v, Some(false)).await; + } + } + + // first check that we can send correctly + chr(&mut pin, 64, true).await; + assert_eq!(read1(&mut uart).await.unwrap(), [64]); + + // framing on empty buffer + chr(&mut pin, 64, false).await; + assert_eq!(read1::<1>(&mut uart).await.unwrap_err(), Error::Framing); + chr(&mut pin, 65, true).await; + assert_eq!(read1(&mut uart).await.unwrap(), [65]); + + // framing on partially filled buffer + chr(&mut pin, 64, true).await; + chr(&mut pin, 32, true).await; + chr(&mut pin, 64, false).await; + chr(&mut pin, 65, true).await; + assert_eq!(read1(&mut uart).await.unwrap(), [64, 32]); + assert_eq!(read1::<1>(&mut uart).await.unwrap_err(), Error::Framing); + assert_eq!(read1(&mut uart).await.unwrap(), [65]); + + // framing on full buffer + for i in 0..16 { + chr(&mut pin, i, true).await; + } + chr(&mut pin, 64, false).await; + chr(&mut pin, 65, true).await; + assert_eq!( + read1(&mut uart).await.unwrap(), + [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15] + ); + assert_eq!(read1::<1>(&mut uart).await.unwrap_err(), Error::Framing); + assert_eq!(read1(&mut uart).await.unwrap(), [65]); + } info!("Test OK"); cortex_m::asm::bkpt();