pw_uart: Add ReadAtLeast and ReadExactly methods

Deprecate the existing Read method in favor of two read variants that
cover more use cases.

ReadExactly maps to the existing Read semantics and blocks until the
buffer is completely filled.

ReadAtLeast allows for blocking until at least the specified bytes are
received, but allows for more to be delivered if available.

ReadAtLeast is useful in cases where you don't know exactly how many
bytes will be returned and don't know exactly when to expect them.

Update mcuxpresso implementation to support this new feature.

Provide default implementations of new variant to ease transition of
existing implementations.

Bug: 368149122
Change-Id: If7674937125ff56e70953ffce3c6e323f721034f
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/236268
Reviewed-by: Edward Shin <[email protected]>
Reviewed-by: Taylor Cramer <[email protected]>
Reviewed-by: Jonathon Reinhart <[email protected]>
Commit-Queue: Austin Foxley <[email protected]>
Lint: Lint 🤖 <[email protected]>
Presubmit-Verified: CQ Bot Account <[email protected]>
Docs-Not-Needed: Taylor Cramer <[email protected]>

pw_uart/public/pw_uart/uart.h

@@ -119,10 +119,10 @@ class Uart {
 
   /// Reads data from the UART into a provided buffer.
   ///
-  /// This function blocks until the entirety of `rx_buffer` is filled with
-  /// data.
+  /// This function blocks until `min_bytes` have been read into `rx_buffer`.
   ///
   /// @param rx_buffer  The buffer to read data into.
+  /// @param min_bytes  The minimum number of bytes to read before returning.
   ///
   /// @returns @rst
   ///
@@ -133,14 +133,114 @@ class Uart {
   /// May return other implementation-specific status codes.
   ///
   /// @endrst
+  StatusWithSize ReadAtLeast(ByteSpan rx_buffer, size_t min_bytes) {
+    return DoTryReadFor(rx_buffer, min_bytes, std::nullopt);
+  }
+
+  /// Reads data from the UART into a provided buffer.
+  ///
+  /// This function blocks until the entire buffer has been filled.
+  ///
+  /// @param rx_buffer  The buffer to read data into.
+  ///
+  /// @returns @rst
+  ///
+  /// .. pw-status-codes::
+  ///
+  ///    OK: The operation was successful.
+  ///
+  /// May return other implementation-specific status codes.
+  ///
+  /// @endrst
+  StatusWithSize ReadExactly(ByteSpan rx_buffer) {
+    return DoTryReadFor(rx_buffer, rx_buffer.size(), std::nullopt);
+  }
+
+  /// Deprecated: Prefer ReadExactly in new code.
+  ///
+  /// Reads data from the UART into a provided buffer.
+  ///
+  /// This function blocks until the entire buffer has been filled.
+  ///
+  /// @param rx_buffer  The buffer to read data into.
+  ///
+  /// @returns @rst
+  ///
+  /// .. pw-status-codes::
+  ///
+  ///    OK: The operation was successful.
+  ///
+  /// May return other implementation-specific status codes.
+  ///
+  /// @endrst
+  // TODO: https://pwbug.dev/368149122 - Remove after transition
   Status Read(ByteSpan rx_buffer) {
     return DoTryReadFor(rx_buffer, std::nullopt).status();
   }
 
   /// Reads data from the UART into a provided buffer.
   ///
-  /// This function blocks until either the entire buffer has been filled with
-  /// data or the specified timeout has elapsed, whichever occurs first.
+  /// This function blocks until either `min_bytes` have been read into buffer
+  /// or the specified timeout has elapsed, whichever occurs first.
+  ///
+  /// @param rx_buffer  The buffer to read data into.
+  /// @param min_bytes  The minimum number of bytes to read before returning.
+  /// @param timeout    The maximum time to wait for data to be read. If zero,
+  ///                   the function will immediately return with at least one
+  ///                   hardware read operation attempt.
+  ///
+  /// @returns @rst
+  ///
+  /// .. pw-status-codes::
+  ///
+  ///    OK: The operation was successful and the entire buffer has been filled
+  ///    with data.
+  ///
+  ///    DEADLINE_EXCEEDED: The operation timed out before the entire buffer
+  ///    could be filled.
+  ///
+  /// May return other implementation-specific status codes.
+  ///
+  /// @endrst
+  StatusWithSize TryReadAtLeastFor(ByteSpan rx_buffer,
+                                   size_t min_bytes,
+                                   chrono::SystemClock::duration timeout) {
+    return DoTryReadFor(rx_buffer, min_bytes, timeout);
+  }
+
+  /// Reads data from the UART into a provided buffer.
+  ///
+  /// This function blocks until either `rx_buffer.size()` bytes have been read
+  /// into buffer or the specified timeout has elapsed, whichever occurs first.
+  ///
+  /// @param rx_buffer  The buffer to read data into.
+  /// @param timeout    The maximum time to wait for data to be read. If zero,
+  ///                   the function will immediately return with at least one
+  ///                   hardware read operation attempt.
+  ///
+  /// @returns @rst
+  ///
+  /// .. pw-status-codes::
+  ///
+  ///    OK: The operation was successful and the entire buffer has been filled
+  ///    with data.
+  ///
+  ///    DEADLINE_EXCEEDED: The operation timed out before the entire buffer
+  ///    could be filled.
+  ///
+  /// May return other implementation-specific status codes.
+  ///
+  /// @endrst
+  StatusWithSize TryReadExactlyFor(ByteSpan rx_buffer,
+                                   chrono::SystemClock::duration timeout) {
+    return DoTryReadFor(rx_buffer, rx_buffer.size(), timeout);
+  }
+
+  /// Deprecated: Prefer TryReadExactlyFor in new code.
+  /// Reads data from the UART into a provided buffer.
+  ///
+  /// This function blocks until either `rx_buffer.size()` bytes have been read
+  /// into buffer or the specified timeout has elapsed, whichever occurs first.
   ///
   /// @param rx_buffer  The buffer to read data into.
   /// @param timeout    The maximum time to wait for data to be read. If zero,
@@ -162,6 +262,7 @@ class Uart {
   /// @endrst
   StatusWithSize TryReadFor(ByteSpan rx_buffer,
                             chrono::SystemClock::duration timeout) {
+    // TODO: https://pwbug.dev/368149122 - Remove after transition
     return DoTryReadFor(rx_buffer, timeout);
   }
 
@@ -273,7 +374,7 @@ class Uart {
   ///                   will block for no longer than this duration. If zero,
   ///                   the function will immediately return with at least one
   ///                   hardware read operation attempt. If not specified, the
-  ///                   function blocks untill the buffer is full.
+  ///                   function blocks until the buffer is full.
   ///
   /// @returns @rst
   ///
@@ -288,9 +389,49 @@ class Uart {
   /// May return other implementation-specific status codes.
   ///
   /// @endrst
+  // TODO: https://pwbug.dev/368149122 - Remove after transition.
   virtual StatusWithSize DoTryReadFor(
       ByteSpan rx_buffer,
-      std::optional<chrono::SystemClock::duration> timeout) = 0;
+      std::optional<chrono::SystemClock::duration> timeout) {
+    return DoTryReadFor(rx_buffer, rx_buffer.size(), timeout);
+  }
+
+  /// Reads data from the UART into a provided buffer with an optional timeout
+  /// provided.
+  ///
+  /// This virtual function attempts to read data into the provided byte buffer
+  /// (`rx_buffer`). The operation will continue until either `min_bytes` have
+  /// been read into the buffer, an error occurs, or the optional timeout
+  /// duration expires.
+  ///
+  /// @param rx_buffer  The buffer to read data into.
+  /// @param min_bytes  The minimum number of bytes to read before returning.
+  /// @param timeout    An optional timeout duration. If specified, the function
+  ///                   will block for no longer than this duration. If zero,
+  ///                   the function will immediately return with at least one
+  ///                   hardware read operation attempt. If not specified, the
+  ///                   function blocks until the buffer is full.
+  ///
+  /// @returns @rst
+  ///
+  /// .. pw-status-codes::
+  ///
+  ///    OK: The operation was successful and the entire buffer has been
+  ///    filled with data.
+  ///
+  ///    DEADLINE_EXCEEDED: The operation timed out before the entire buffer
+  ///    could be filled.
+  ///
+  /// May return other implementation-specific status codes.
+  ///
+  /// @endrst
+  // TODO: https://pwbug.dev/368149122 - Make pure virtual after transition.
+  virtual StatusWithSize DoTryReadFor(
+      ByteSpan /*rx_buffer*/,
+      size_t /*min_bytes*/,
+      std::optional<chrono::SystemClock::duration> /*timeout*/) {
+    return StatusWithSize::Unimplemented();
+  }
 
   /// @brief Writes data from a provided buffer to the UART with an optional
   /// timeout.

pw_uart/uart_test.cc

@@ -29,7 +29,7 @@ class UartStub : public Uart {
  private:
   Status DoEnable(bool) override { return OkStatus(); }
   StatusWithSize DoTryReadFor(
-      ByteSpan, std::optional<chrono::SystemClock::duration>) override {
+      ByteSpan, size_t, std::optional<chrono::SystemClock::duration>) override {
     return StatusWithSize(0);
   }
   StatusWithSize DoTryWriteFor(

pw_uart_mcuxpresso/dma_uart.cc

@@ -461,13 +461,15 @@ Status DmaUartMcuxpresso::DoSetFlowControl(bool enable) {
 // otherwise DoRead calls might fail due to contention for
 // the USART RX channel.
 StatusWithSize DmaUartMcuxpresso::DoTryReadFor(
-    ByteSpan rx_buffer, std::optional<chrono::SystemClock::duration> timeout) {
+    ByteSpan rx_buffer,
+    size_t min_bytes,
+    std::optional<chrono::SystemClock::duration> timeout) {
   if (timeout.has_value() && *timeout < chrono::SystemClock::duration::zero()) {
     return StatusWithSize(Status::InvalidArgument(), 0);
   }
 
   size_t length = rx_buffer.size();
-  if (length == 0) {
+  if (length == 0 || min_bytes > length) {
     return StatusWithSize(Status::InvalidArgument(), 0);
   }
 
@@ -485,8 +487,8 @@ StatusWithSize DmaUartMcuxpresso::DoTryReadFor(
   Status status = OkStatus();
 
   ByteBuilder bb(rx_buffer);
-  size_t bytes_needed = length;
-  while (bytes_needed > 0) {
+  size_t bytes_copied = 0;
+  while (bytes_copied < min_bytes) {
     // Get the number of bytes available to read.
     StatusWithSize rx_count_status = TransferGetReceiveDMACount();
     if (!rx_count_status.ok()) {
@@ -497,22 +499,24 @@ StatusWithSize DmaUartMcuxpresso::DoTryReadFor(
 
     // Copy available bytes out of the ring buffer.
     if (rx_count > 0) {
-      size_t copy_size = MIN(bytes_needed, rx_count);
+      // Allow copying more than min_bytes if they are available.
+      size_t copy_size = std::min(length - bytes_copied, rx_count);
       CopyReceiveData(bb, copy_size);
-      bytes_needed -= copy_size;
+      bytes_copied += copy_size;
     }
 
     // Do we still need more bytes?
     // We need to wait for more DMA bytes if so.
-    if (bytes_needed > 0) {
+    if (bytes_copied < min_bytes) {
       // Check if the timeout has expired, if applicable.
       if (deadline.has_value() && chrono::SystemClock::now() >= *deadline) {
         status.Update(Status::DeadlineExceeded());
         break;
       }
 
       // Wait up to the timeout duration to receive more bytes.
-      Status wait_status = WaitForReceiveBytes(bytes_needed, deadline);
+      Status wait_status =
+          WaitForReceiveBytes(min_bytes - bytes_copied, deadline);
       // Even if we exceeded the deadline, stay in the loop for one more
       // iteration to copy out any final bytes.
       if (!wait_status.ok() && !wait_status.IsDeadlineExceeded()) {
@@ -526,7 +530,7 @@ StatusWithSize DmaUartMcuxpresso::DoTryReadFor(
   }
 
   rx_data_.busy.store(false);
-  return StatusWithSize(status, length - bytes_needed);
+  return StatusWithSize(status, bytes_copied);
 }
 
 // Write data to USART using DMA transactions

pw_uart_mcuxpresso/public/pw_uart_mcuxpresso/dma_uart.h

@@ -104,6 +104,7 @@ class DmaUartMcuxpresso final : public Uart {
   Status DoSetFlowControl(bool enable) override;
   StatusWithSize DoTryReadFor(
       ByteSpan rx_buffer,
+      size_t min_bytes,
       std::optional<chrono::SystemClock::duration> timeout) override;
   StatusWithSize DoTryWriteFor(
       ConstByteSpan tx_buffer,