From 8011a30bd02b08470d91580148e90625edd63c02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?D=C3=A1niel=20Buga?= <bugadani@gmail.com>
Date: Mon, 5 May 2025 15:41:07 +0200
Subject: [PATCH] Fix write_str returning early (#3452)

---
 esp-hal/CHANGELOG.md         |  1 +
 esp-hal/src/uart.rs          | 14 +++++---
 hil-test/tests/uart_async.rs | 67 ++++++++++++++++++++++++++++++++++--
 3 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md
index a0be12a55..843489b40 100644
--- a/esp-hal/CHANGELOG.md
+++ b/esp-hal/CHANGELOG.md
@@ -86,6 +86,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `OneShot` timer now returns an InvalidTimeout from `schedule` instead of panicking (#3433)
 - GPIO interrupt handling no longer causes infinite looping if a task at higher priority is awaiting on a pin event (#3408)
 - `esp_hal::gpio::Input::is_interrupt_set` can now return true (#3408)
+- `Uart::write_str` (both core::fmt and uWrite implementations) no longer stops writing when the internal buffer fills up (#3452)
 
 ### Removed
 
diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs
index 35945cbac..ba95a3d88 100644
--- a/esp-hal/src/uart.rs
+++ b/esp-hal/src/uart.rs
@@ -747,6 +747,14 @@ where
         self.uart.info().write(data)
     }
 
+    fn write_all(&mut self, mut data: &[u8]) -> Result<(), TxError> {
+        while !data.is_empty() {
+            let bytes_written = self.write(data)?;
+            data = &data[bytes_written..];
+        }
+        Ok(())
+    }
+
     /// Flush the transmit buffer.
     ///
     /// This function blocks until all data in the TX FIFO has been
@@ -1737,8 +1745,7 @@ where
 
     #[inline]
     fn write_str(&mut self, s: &str) -> Result<(), Self::Error> {
-        self.write(s.as_bytes())?;
-        Ok(())
+        self.write_all(s.as_bytes())
     }
 }
 
@@ -1758,8 +1765,7 @@ where
 {
     #[inline]
     fn write_str(&mut self, s: &str) -> core::fmt::Result {
-        self.write(s.as_bytes()).map_err(|_| core::fmt::Error)?;
-        Ok(())
+        self.write_all(s.as_bytes()).map_err(|_| core::fmt::Error)
     }
 }
 
diff --git a/hil-test/tests/uart_async.rs b/hil-test/tests/uart_async.rs
index abe90505f..8b7a1289d 100644
--- a/hil-test/tests/uart_async.rs
+++ b/hil-test/tests/uart_async.rs
@@ -6,19 +6,52 @@
 #![no_std]
 #![no_main]
 
+use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, signal::Signal};
 use esp_hal::{
     Async,
-    uart::{self, Uart},
+    Blocking,
+    interrupt::{
+        Priority,
+        software::{SoftwareInterrupt, SoftwareInterruptControl},
+    },
+    uart::{self, Uart, UartRx},
 };
-use hil_test as _;
+use esp_hal_embassy::InterruptExecutor;
+use hil_test::mk_static;
 
 struct Context {
+    interrupt: SoftwareInterrupt<'static, 1>,
     uart: Uart<'static, Async>,
 }
 
+const LONG_TEST_STRING: &str = "This is a very long string that will be printed to the serial console. First line of the string. Second line of the string. Third line of the string. Fourth line of the string. Fifth line of the string. Sixth line of the string.";
+
+#[embassy_executor::task]
+async fn long_string_reader(
+    uart: UartRx<'static, Blocking>,
+    signal: &'static Signal<CriticalSectionRawMutex, ()>,
+) {
+    let mut uart = uart.into_async();
+    let mut received = 0;
+
+    let mut buf = [0u8; 128];
+    loop {
+        let len = uart.read_async(&mut buf[..]).await.unwrap();
+        received += len;
+        assert!(received <= LONG_TEST_STRING.len());
+        if received == LONG_TEST_STRING.len() {
+            break;
+        }
+    }
+    signal.signal(());
+}
+
 #[cfg(test)]
 #[embedded_test::tests(default_timeout = 3, executor = hil_test::Executor::new())]
 mod tests {
+
+    use core::fmt::Write;
+
     use super::*;
 
     #[init]
@@ -33,7 +66,11 @@ mod tests {
             .with_rx(rx)
             .into_async();
 
-        Context { uart }
+        let sw_ints = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT);
+        Context {
+            interrupt: sw_ints.software_interrupt1,
+            uart,
+        }
     }
 
     #[test]
@@ -48,4 +85,28 @@ mod tests {
         ctx.uart.read_async(&mut buf[..]).await.unwrap();
         assert_eq!(&buf[..], SEND);
     }
+
+    #[test]
+    async fn test_long_strings(ctx: Context) {
+        // This is a regression test for https://github.com/esp-rs/esp-hal/issues/3450
+        // The issue was that the printed string is longer than the UART buffer so the
+        // end was lost. We can't test the fix in blocking code, only on
+        // dual-cores, but a preemptive interrupt executor should work on all
+        // chips.
+
+        let (rx, mut tx) = ctx.uart.into_blocking().split();
+
+        let interrupt_executor =
+            mk_static!(InterruptExecutor<1>, InterruptExecutor::new(ctx.interrupt));
+
+        let signal = &*mk_static!(Signal<CriticalSectionRawMutex, ()>, Signal::new());
+
+        let spawner = interrupt_executor.start(Priority::Priority3);
+        spawner.must_spawn(long_string_reader(rx, signal));
+
+        tx.write_str(LONG_TEST_STRING).unwrap();
+
+        // Wait for the signal to be sent
+        signal.wait().await;
+    }
 }