diff --git a/embassy-rp/src/uart/mod.rs b/embassy-rp/src/uart/mod.rs index f546abe71..d50f5b4d5 100644 --- a/embassy-rp/src/uart/mod.rs +++ b/embassy-rp/src/uart/mod.rs @@ -490,6 +490,36 @@ impl<'d, T: Instance> UartRx<'d, T, Async> { /// * The first call to `read_to_break()` will return `Ok(20)`. /// * The next call to `read_to_break()` will work as expected pub async fn read_to_break(&mut self, buffer: &mut [u8]) -> Result { + self.read_to_break_with_count(buffer, 0).await + } + + /// Read from the UART, waiting for a line break as soon as at least `min_count` bytes have been read. + /// + /// We read until one of the following occurs: + /// + /// * We read `buffer.len()` bytes without a line break + /// * returns `Err(ReadToBreakError::MissingBreak(buffer.len()))` + /// * We read `n > min_count` bytes then a line break occurs + /// * returns `Ok(n)` + /// * We encounter some error OTHER than a line break + /// * returns `Err(ReadToBreakError::Other(error))` + /// + /// If a line break occurs before `min_count` bytes have been read, the break will be ignored and the read will continue + /// + /// **NOTE**: you MUST provide a buffer one byte larger than your largest expected + /// message to reliably detect the framing on one single call to `read_to_break()`. + /// + /// * If you expect a message of 20 bytes + line break, and provide a 20-byte buffer: + /// * The first call to `read_to_break()` will return `Err(ReadToBreakError::MissingBreak(20))` + /// * The next call to `read_to_break()` will immediately return `Ok(0)`, from the "stale" line break + /// * If you expect a message of 20 bytes + line break, and provide a 21-byte buffer: + /// * The first call to `read_to_break()` will return `Ok(20)`. + /// * The next call to `read_to_break()` will work as expected + pub async fn read_to_break_with_count( + &mut self, + buffer: &mut [u8], + min_count: usize, + ) -> Result { // clear error flags before we drain the fifo. errors that have accumulated // in the flags will also be present in the fifo. T::dma_state().rx_errs.store(0, Ordering::Relaxed); @@ -502,7 +532,7 @@ impl<'d, T: Instance> UartRx<'d, T, Async> { // then drain the fifo. we need to read at most 32 bytes. errors that apply // to fifo bytes will be reported directly. - let sbuffer = match { + let mut sbuffer = match { let limit = buffer.len().min(32); self.drain_fifo(&mut buffer[0..limit]) } { @@ -511,7 +541,13 @@ impl<'d, T: Instance> UartRx<'d, T, Async> { // Drained (some/all of the fifo), no room left Ok(len) => return Err(ReadToBreakError::MissingBreak(len)), // We got a break WHILE draining the FIFO, return what we did get before the break - Err((i, Error::Break)) => return Ok(i), + Err((len, Error::Break)) => { + if len < min_count && len < buffer.len() { + &mut buffer[len..] + } else { + return Ok(len); + } + } // Some other error, just return the error Err((_i, e)) => return Err(ReadToBreakError::Other(e)), }; @@ -530,110 +566,118 @@ impl<'d, T: Instance> UartRx<'d, T, Async> { reg.set_rxdmae(true); reg.set_dmaonerr(true); }); - let transfer = unsafe { - // If we don't assign future to a variable, the data register pointer - // is held across an await and makes the future non-Send. - crate::dma::read(&mut ch, T::regs().uartdr().as_ptr() as *const _, sbuffer, T::RX_DREQ) - }; - // wait for either the transfer to complete or an error to happen. - let transfer_result = select( - transfer, - poll_fn(|cx| { - T::dma_state().rx_err_waker.register(cx.waker()); - match T::dma_state().rx_errs.swap(0, Ordering::Relaxed) { - 0 => Poll::Pending, - e => Poll::Ready(Uartris(e as u32)), + loop { + let transfer = unsafe { + // If we don't assign future to a variable, the data register pointer + // is held across an await and makes the future non-Send. + crate::dma::read(&mut ch, T::regs().uartdr().as_ptr() as *const _, sbuffer, T::RX_DREQ) + }; + + // wait for either the transfer to complete or an error to happen. + let transfer_result = select( + transfer, + poll_fn(|cx| { + T::dma_state().rx_err_waker.register(cx.waker()); + match T::dma_state().rx_errs.swap(0, Ordering::Relaxed) { + 0 => Poll::Pending, + e => Poll::Ready(Uartris(e as u32)), + } + }), + ) + .await; + + // Figure out our error state + let errors = match transfer_result { + Either::First(()) => { + // We're here because the DMA finished, BUT if an error occurred on the LAST + // byte, then we may still need to grab the error state! + Uartris(T::dma_state().rx_errs.swap(0, Ordering::Relaxed) as u32) } - }), - ) - .await; - - // Figure out our error state - let errors = match transfer_result { - Either::First(()) => { - // We're here because the DMA finished, BUT if an error occurred on the LAST - // byte, then we may still need to grab the error state! - Uartris(T::dma_state().rx_errs.swap(0, Ordering::Relaxed) as u32) - } - Either::Second(e) => { - // We're here because we errored, which means this is the error that - // was problematic. - e - } - }; - - if errors.0 == 0 { - // No errors? That means we filled the buffer without a line break. - // For THIS function, that's a problem. - return Err(ReadToBreakError::MissingBreak(buffer.len())); - } else if errors.beris() { - // We got a Line Break! By this point, we've finished/aborted the DMA - // transaction, which means that we need to figure out where it left off - // by looking at the write_addr. - // - // First, we do a sanity check to make sure the write value is within the - // range of DMA we just did. - let sval = buffer.as_ptr() as usize; - let eval = sval + buffer.len(); - - // This is the address where the DMA would write to next - let next_addr = ch.regs().write_addr().read() as usize; - - // If we DON'T end up inside the range, something has gone really wrong. - // Note that it's okay that `eval` is one past the end of the slice, as - // this is where the write pointer will end up at the end of a full - // transfer. - if (next_addr < sval) || (next_addr > eval) { - unreachable!("UART DMA reported invalid `write_addr`"); - } - - let regs = T::regs(); - let all_full = next_addr == eval; - - // NOTE: This is off label usage of RSR! See the issue below for - // why I am not checking if there is an "extra" FIFO byte, and why - // I am checking RSR directly (it seems to report the status of the LAST - // POPPED value, rather than the NEXT TO POP value like the datasheet - // suggests!) - // - // issue: https://github.com/raspberrypi/pico-feedback/issues/367 - let last_was_break = regs.uartrsr().read().be(); - - return match (all_full, last_was_break) { - (true, true) | (false, _) => { - // We got less than the full amount + a break, or the full amount - // and the last byte was a break. Subtract the break off by adding one to sval. - Ok(next_addr.saturating_sub(1 + sval)) - } - (true, false) => { - // We finished the whole DMA, and the last DMA'd byte was NOT a break - // character. This is an error. - // - // NOTE: we COULD potentially return Ok(buffer.len()) here, since we - // know a line break occured at SOME POINT after the DMA completed. - // - // However, we have no way of knowing if there was extra data BEFORE - // that line break, so instead return an Err to signal to the caller - // that there are "leftovers", and they'll catch the actual line break - // on the next call. - // - // Doing it like this also avoids racyness: now whether you finished - // the full read BEFORE the line break occurred or AFTER the line break - // occurs, you still get `MissingBreak(buffer.len())` instead of sometimes - // getting `Ok(buffer.len())` if you were "late enough" to observe the - // line break. - Err(ReadToBreakError::MissingBreak(buffer.len())) + Either::Second(e) => { + // We're here because we errored, which means this is the error that + // was problematic. + e } }; - } else if errors.oeris() { - return Err(ReadToBreakError::Other(Error::Overrun)); - } else if errors.peris() { - return Err(ReadToBreakError::Other(Error::Parity)); - } else if errors.feris() { - return Err(ReadToBreakError::Other(Error::Framing)); + + if errors.0 == 0 { + // No errors? That means we filled the buffer without a line break. + // For THIS function, that's a problem. + return Err(ReadToBreakError::MissingBreak(buffer.len())); + } else if errors.beris() { + // We got a Line Break! By this point, we've finished/aborted the DMA + // transaction, which means that we need to figure out where it left off + // by looking at the write_addr. + // + // First, we do a sanity check to make sure the write value is within the + // range of DMA we just did. + let sval = buffer.as_ptr() as usize; + let eval = sval + buffer.len(); + + // This is the address where the DMA would write to next + let next_addr = ch.regs().write_addr().read() as usize; + + // If we DON'T end up inside the range, something has gone really wrong. + // Note that it's okay that `eval` is one past the end of the slice, as + // this is where the write pointer will end up at the end of a full + // transfer. + if (next_addr < sval) || (next_addr > eval) { + unreachable!("UART DMA reported invalid `write_addr`"); + } + + if (next_addr - sval) < min_count { + sbuffer = &mut buffer[(next_addr - sval)..]; + continue; + } + + let regs = T::regs(); + let all_full = next_addr == eval; + + // NOTE: This is off label usage of RSR! See the issue below for + // why I am not checking if there is an "extra" FIFO byte, and why + // I am checking RSR directly (it seems to report the status of the LAST + // POPPED value, rather than the NEXT TO POP value like the datasheet + // suggests!) + // + // issue: https://github.com/raspberrypi/pico-feedback/issues/367 + let last_was_break = regs.uartrsr().read().be(); + + return match (all_full, last_was_break) { + (true, true) | (false, _) => { + // We got less than the full amount + a break, or the full amount + // and the last byte was a break. Subtract the break off by adding one to sval. + Ok(next_addr.saturating_sub(1 + sval)) + } + (true, false) => { + // We finished the whole DMA, and the last DMA'd byte was NOT a break + // character. This is an error. + // + // NOTE: we COULD potentially return Ok(buffer.len()) here, since we + // know a line break occured at SOME POINT after the DMA completed. + // + // However, we have no way of knowing if there was extra data BEFORE + // that line break, so instead return an Err to signal to the caller + // that there are "leftovers", and they'll catch the actual line break + // on the next call. + // + // Doing it like this also avoids racyness: now whether you finished + // the full read BEFORE the line break occurred or AFTER the line break + // occurs, you still get `MissingBreak(buffer.len())` instead of sometimes + // getting `Ok(buffer.len())` if you were "late enough" to observe the + // line break. + Err(ReadToBreakError::MissingBreak(buffer.len())) + } + }; + } else if errors.oeris() { + return Err(ReadToBreakError::Other(Error::Overrun)); + } else if errors.peris() { + return Err(ReadToBreakError::Other(Error::Parity)); + } else if errors.feris() { + return Err(ReadToBreakError::Other(Error::Framing)); + } + unreachable!("unrecognized rx error"); } - unreachable!("unrecognized rx error"); } } @@ -997,6 +1041,17 @@ impl<'d, T: Instance> Uart<'d, T, Async> { pub async fn read_to_break<'a>(&mut self, buf: &'a mut [u8]) -> Result { self.rx.read_to_break(buf).await } + + /// Read until the buffer is full or a line break occurs after at least `min_count` bytes have been read. + /// + /// See [`UartRx::read_to_break_with_count()`] for more details + pub async fn read_to_break_with_count<'a>( + &mut self, + buf: &'a mut [u8], + min_count: usize, + ) -> Result { + self.rx.read_to_break_with_count(buf, min_count).await + } } impl<'d, T: Instance, M: Mode> embedded_hal_02::serial::Read for UartRx<'d, T, M> {