From 7ff21e8b8ba3bd03b4317abbc40f0e6a0b02289f Mon Sep 17 00:00:00 2001 From: Justin Beaurivage Date: Tue, 30 Jan 2024 17:06:57 -0500 Subject: [PATCH 1/4] Handle Uarte RX errors --- embassy-nrf/Cargo.toml | 1 + embassy-nrf/src/uarte.rs | 95 +++++++++++++++++++++++++++++++++++----- 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/embassy-nrf/Cargo.toml b/embassy-nrf/Cargo.toml index 10f268b51..580f0e3e6 100644 --- a/embassy-nrf/Cargo.toml +++ b/embassy-nrf/Cargo.toml @@ -133,6 +133,7 @@ embedded-io = { version = "0.6.0" } embedded-io-async = { version = "0.6.1" } defmt = { version = "0.3", optional = true } +bitflags = "2.4.2" log = { version = "0.4.14", optional = true } cortex-m-rt = ">=0.6.15,<0.8" cortex-m = "0.7.6" diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs index 95434e7a7..90d851139 100644 --- a/embassy-nrf/src/uarte.rs +++ b/embassy-nrf/src/uarte.rs @@ -52,6 +52,39 @@ impl Default for Config { } } +bitflags::bitflags! { + /// Error source flags + pub struct ErrorSource: u32 { + /// Buffer overrun + const OVERRUN = 0x01; + /// Parity error + const PARITY = 0x02; + /// Framing error + const FRAMING = 0x04; + /// Break condition + const BREAK = 0x08; + } +} + +impl TryFrom for () { + type Error = Error; + + #[inline] + fn try_from(errors: ErrorSource) -> Result { + if errors.contains(ErrorSource::OVERRUN) { + Err(Error::Overrun) + } else if errors.contains(ErrorSource::PARITY) { + Err(Error::Parity) + } else if errors.contains(ErrorSource::FRAMING) { + Err(Error::Framing) + } else if errors.contains(ErrorSource::BREAK) { + Err(Error::Break) + } else { + Ok(()) + } + } +} + /// UART error. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] @@ -61,6 +94,14 @@ pub enum Error { BufferTooLong, /// The buffer is not in data RAM. It's most likely in flash, and nRF's DMA cannot access flash. BufferNotInRAM, + /// Framing Error + Framing, + /// Parity Error + Parity, + /// Buffer Overrun + Overrun, + /// Break condition + Break, } /// Interrupt handler. @@ -73,9 +114,16 @@ impl interrupt::typelevel::Handler for InterruptHandl let r = T::regs(); let s = T::state(); - if r.events_endrx.read().bits() != 0 { + let endrx = r.events_endrx.read().bits(); + let error = r.events_error.read().bits(); + if endrx != 0 || error != 0 { s.endrx_waker.wake(); - r.intenclr.write(|w| w.endrx().clear()); + if endrx != 0 { + r.intenclr.write(|w| w.endrx().clear()); + } + if error != 0 { + r.intenclr.write(|w| w.error().clear()); + } } if r.events_endtx.read().bits() != 0 { s.endtx_waker.wake(); @@ -486,6 +534,13 @@ impl<'d, T: Instance> UarteRx<'d, T> { Self::new_inner(uarte, rxd.map_into(), Some(rts.map_into()), config) } + fn read_and_clear_errors(&mut self) -> Result<(), Error> { + let r = T::regs(); + let err_bits = r.errorsrc.read().bits(); + r.errorsrc.write(|w| unsafe { w.bits(err_bits) }); + ErrorSource::from_bits_truncate(err_bits).try_into() + } + fn new_inner( uarte: impl Peripheral

+ 'd, rxd: PeripheralRef<'d, AnyPin>, @@ -572,7 +627,7 @@ impl<'d, T: Instance> UarteRx<'d, T> { /// Read bytes until the buffer is filled. pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { - if buffer.len() == 0 { + if buffer.is_empty() { return Ok(()); } if buffer.len() > EASY_DMA_SIZE { @@ -588,8 +643,13 @@ impl<'d, T: Instance> UarteRx<'d, T> { let drop = OnDrop::new(move || { trace!("read drop: stopping"); - r.intenclr.write(|w| w.endrx().clear()); + r.intenclr.write(|w| { + w.endrx().clear(); + w.error().clear() + }); r.events_rxto.reset(); + r.events_error.reset(); + r.errorsrc.reset(); r.tasks_stoprx.write(|w| unsafe { w.bits(1) }); while r.events_endrx.read().bits() == 0 {} @@ -601,17 +661,26 @@ impl<'d, T: Instance> UarteRx<'d, T> { r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) }); r.events_endrx.reset(); - r.intenset.write(|w| w.endrx().set()); + r.events_error.reset(); + r.intenset.write(|w| { + w.endrx().set(); + w.error().set() + }); compiler_fence(Ordering::SeqCst); trace!("startrx"); r.tasks_startrx.write(|w| unsafe { w.bits(1) }); - poll_fn(|cx| { + let result = poll_fn(|cx| { s.endrx_waker.register(cx.waker()); + + let maybe_err = self.read_and_clear_errors(); + if let Err(e) = maybe_err { + return Poll::Ready(Err(e)); + } if r.events_endrx.read().bits() != 0 { - return Poll::Ready(()); + return Poll::Ready(Ok(())); } Poll::Pending }) @@ -621,7 +690,7 @@ impl<'d, T: Instance> UarteRx<'d, T> { r.events_rxstarted.reset(); drop.defuse(); - Ok(()) + result } /// Read bytes until the buffer is filled. @@ -642,19 +711,23 @@ impl<'d, T: Instance> UarteRx<'d, T> { r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) }); r.events_endrx.reset(); - r.intenclr.write(|w| w.endrx().clear()); + r.events_error.reset(); + r.intenclr.write(|w| { + w.endrx().clear(); + w.error().clear() + }); compiler_fence(Ordering::SeqCst); trace!("startrx"); r.tasks_startrx.write(|w| unsafe { w.bits(1) }); - while r.events_endrx.read().bits() == 0 {} + while r.events_endrx.read().bits() == 0 && r.events_error.read().bits() == 0 {} compiler_fence(Ordering::SeqCst); r.events_rxstarted.reset(); - Ok(()) + self.read_and_clear_errors() } } From d364447a34377c708fe6a7ea87aabda3ea1231ba Mon Sep 17 00:00:00 2001 From: Justin Beaurivage Date: Wed, 31 Jan 2024 14:16:58 -0500 Subject: [PATCH 2/4] Add error handling to UarteRxWithIdle --- embassy-nrf/src/uarte.rs | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs index 90d851139..97c887ab2 100644 --- a/embassy-nrf/src/uarte.rs +++ b/embassy-nrf/src/uarte.rs @@ -534,7 +534,8 @@ impl<'d, T: Instance> UarteRx<'d, T> { Self::new_inner(uarte, rxd.map_into(), Some(rts.map_into()), config) } - fn read_and_clear_errors(&mut self) -> Result<(), Error> { + /// Check for errors and clear the error register if an error occured. + fn check_and_clear_errors(&mut self) -> Result<(), Error> { let r = T::regs(); let err_bits = r.errorsrc.read().bits(); r.errorsrc.write(|w| unsafe { w.bits(err_bits) }); @@ -675,8 +676,7 @@ impl<'d, T: Instance> UarteRx<'d, T> { let result = poll_fn(|cx| { s.endrx_waker.register(cx.waker()); - let maybe_err = self.read_and_clear_errors(); - if let Err(e) = maybe_err { + if let Err(e) = self.check_and_clear_errors() { return Poll::Ready(Err(e)); } if r.events_endrx.read().bits() != 0 { @@ -727,7 +727,7 @@ impl<'d, T: Instance> UarteRx<'d, T> { compiler_fence(Ordering::SeqCst); r.events_rxstarted.reset(); - self.read_and_clear_errors() + self.check_and_clear_errors() } } @@ -794,8 +794,12 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { let drop = OnDrop::new(|| { self.timer.stop(); - r.intenclr.write(|w| w.endrx().clear()); + r.intenclr.write(|w| { + w.endrx().clear(); + w.error().clear() + }); r.events_rxto.reset(); + r.events_error.reset(); r.tasks_stoprx.write(|w| unsafe { w.bits(1) }); while r.events_endrx.read().bits() == 0 {} @@ -805,17 +809,23 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) }); r.events_endrx.reset(); - r.intenset.write(|w| w.endrx().set()); + r.events_error.reset(); + r.intenset.write(|w| {w.endrx().set(); w.error().set()}); compiler_fence(Ordering::SeqCst); r.tasks_startrx.write(|w| unsafe { w.bits(1) }); - poll_fn(|cx| { + let result = poll_fn(|cx| { s.endrx_waker.register(cx.waker()); - if r.events_endrx.read().bits() != 0 { - return Poll::Ready(()); + + if let Err(e) = self.rx.check_and_clear_errors() { + return Poll::Ready(Err(e)); } + if r.events_endrx.read().bits() != 0 { + return Poll::Ready(Ok(())); + } + Poll::Pending }) .await; @@ -828,7 +838,7 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { drop.defuse(); - Ok(n) + result.map(|_| n) } /// Read bytes until the buffer is filled, or the line becomes idle. @@ -853,13 +863,14 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) }); r.events_endrx.reset(); - r.intenclr.write(|w| w.endrx().clear()); + r.events_error.reset(); + r.intenclr.write(|w| {w.endrx().clear(); w.error().clear()}); compiler_fence(Ordering::SeqCst); r.tasks_startrx.write(|w| unsafe { w.bits(1) }); - while r.events_endrx.read().bits() == 0 {} + while r.events_endrx.read().bits() == 0 && r.events_error.read().bits() == 0 {} compiler_fence(Ordering::SeqCst); let n = r.rxd.amount.read().amount().bits() as usize; @@ -867,7 +878,7 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { self.timer.stop(); r.events_rxstarted.reset(); - Ok(n) + self.rx.check_and_clear_errors().map(|_| n) } } From cc12eb9680513f380e9a04e76322ae4355c2b6b2 Mon Sep 17 00:00:00 2001 From: Justin Beaurivage Date: Wed, 31 Jan 2024 14:20:06 -0500 Subject: [PATCH 3/4] Rustfmt --- embassy-nrf/src/uarte.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs index 97c887ab2..f29b5061b 100644 --- a/embassy-nrf/src/uarte.rs +++ b/embassy-nrf/src/uarte.rs @@ -676,7 +676,7 @@ impl<'d, T: Instance> UarteRx<'d, T> { let result = poll_fn(|cx| { s.endrx_waker.register(cx.waker()); - if let Err(e) = self.check_and_clear_errors() { + if let Err(e) = self.check_and_clear_errors() { return Poll::Ready(Err(e)); } if r.events_endrx.read().bits() != 0 { @@ -810,7 +810,10 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { r.events_endrx.reset(); r.events_error.reset(); - r.intenset.write(|w| {w.endrx().set(); w.error().set()}); + r.intenset.write(|w| { + w.endrx().set(); + w.error().set() + }); compiler_fence(Ordering::SeqCst); @@ -864,7 +867,10 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { r.events_endrx.reset(); r.events_error.reset(); - r.intenclr.write(|w| {w.endrx().clear(); w.error().clear()}); + r.intenclr.write(|w| { + w.endrx().clear(); + w.error().clear() + }); compiler_fence(Ordering::SeqCst); From 2575f597cc3aafeb9925511b12adf30f6a67bccb Mon Sep 17 00:00:00 2001 From: Justin Beaurivage Date: Mon, 5 Feb 2024 18:05:59 -0500 Subject: [PATCH 4/4] Address @Dirbaio's comments --- embassy-nrf/src/uarte.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs index f29b5061b..de2966ba5 100644 --- a/embassy-nrf/src/uarte.rs +++ b/embassy-nrf/src/uarte.rs @@ -66,18 +66,16 @@ bitflags::bitflags! { } } -impl TryFrom for () { - type Error = Error; - +impl ErrorSource { #[inline] - fn try_from(errors: ErrorSource) -> Result { - if errors.contains(ErrorSource::OVERRUN) { + fn check(self) -> Result<(), Error> { + if self.contains(ErrorSource::OVERRUN) { Err(Error::Overrun) - } else if errors.contains(ErrorSource::PARITY) { + } else if self.contains(ErrorSource::PARITY) { Err(Error::Parity) - } else if errors.contains(ErrorSource::FRAMING) { + } else if self.contains(ErrorSource::FRAMING) { Err(Error::Framing) - } else if errors.contains(ErrorSource::BREAK) { + } else if self.contains(ErrorSource::BREAK) { Err(Error::Break) } else { Ok(()) @@ -539,7 +537,7 @@ impl<'d, T: Instance> UarteRx<'d, T> { let r = T::regs(); let err_bits = r.errorsrc.read().bits(); r.errorsrc.write(|w| unsafe { w.bits(err_bits) }); - ErrorSource::from_bits_truncate(err_bits).try_into() + ErrorSource::from_bits_truncate(err_bits).check() } fn new_inner( @@ -677,6 +675,7 @@ impl<'d, T: Instance> UarteRx<'d, T> { s.endrx_waker.register(cx.waker()); if let Err(e) = self.check_and_clear_errors() { + r.tasks_stoprx.write(|w| unsafe { w.bits(1) }); return Poll::Ready(Err(e)); } if r.events_endrx.read().bits() != 0 { @@ -823,6 +822,7 @@ impl<'d, T: Instance, U: TimerInstance> UarteRxWithIdle<'d, T, U> { s.endrx_waker.register(cx.waker()); if let Err(e) = self.rx.check_and_clear_errors() { + r.tasks_stoprx.write(|w| unsafe { w.bits(1) }); return Poll::Ready(Err(e)); } if r.events_endrx.read().bits() != 0 {