From 0d46e45a1f72ee61a153c41aaaf6c63818ccffb0 Mon Sep 17 00:00:00 2001
From: Hans Maerki <buhtig.hans.maerki@ergoinfo.ch>
Date: Sun, 26 Jan 2025 18:00:38 +0100
Subject: [PATCH] tools/mpremote: Avoid initial blocking read in read_until().

If the target does not return any data then `read_until()` will block
indefinitely.  Fix this by making the initial read part of the general read
look, which always checks `inWaiting() > 0` before reading from the serial
device.

Also added the UART timeout to the constructor.  This is not currently used
but may be used as an additional safeguard.

Signed-off-by: Hans Maerki <buhtig.hans.maerki@ergoinfo.ch>
---
 tools/mpremote/mpremote/transport_serial.py | 32 ++++++++++++++-------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/tools/mpremote/mpremote/transport_serial.py b/tools/mpremote/mpremote/transport_serial.py
index 28ccaf6d8..449708a5e 100644
--- a/tools/mpremote/mpremote/transport_serial.py
+++ b/tools/mpremote/mpremote/transport_serial.py
@@ -42,7 +42,7 @@ from .transport import TransportError, TransportExecError, Transport
 
 
 class SerialTransport(Transport):
-    def __init__(self, device, baudrate=115200, wait=0, exclusive=True):
+    def __init__(self, device, baudrate=115200, wait=0, exclusive=True, timeout=None):
         self.in_raw_repl = False
         self.use_raw_paste = True
         self.device_name = device
@@ -52,7 +52,11 @@ class SerialTransport(Transport):
         import serial.tools.list_ports
 
         # Set options, and exclusive if pyserial supports it
-        serial_kwargs = {"baudrate": baudrate, "interCharTimeout": 1}
+        serial_kwargs = {
+            "baudrate": baudrate,
+            "timeout": timeout,
+            "interCharTimeout": 1,
+        }
         if serial.__version__ >= "3.3":
             serial_kwargs["exclusive"] = exclusive
 
@@ -95,13 +99,20 @@ class SerialTransport(Transport):
         self.serial.close()
 
     def read_until(self, min_num_bytes, ending, timeout=10, data_consumer=None):
-        # if data_consumer is used then data is not accumulated and the ending must be 1 byte long
-        assert data_consumer is None or len(ending) == 1
+        """
+        min_num_bytes: Obsolete.
+        ending: Return if 'ending' matches.
+        timeout [s]: Return if timeout between characters. None: Infinite timeout.
+        data_consumer: Use callback for incoming characters.
+            If data_consumer is used then data is not accumulated and the ending must be 1 byte long
 
-        data = self.serial.read(min_num_bytes)
-        if data_consumer:
-            data_consumer(data)
-        timeout_count = 0
+        It is not visible to the caller why the function returned. It could be ending or timeout.
+        """
+        assert data_consumer is None or len(ending) == 1
+        assert isinstance(timeout, (type(None), int, float))
+
+        data = b""
+        begin_char_s = time.monotonic()
         while True:
             if data.endswith(ending):
                 break
@@ -112,10 +123,9 @@ class SerialTransport(Transport):
                     data = new_data
                 else:
                     data = data + new_data
-                timeout_count = 0
+                begin_char_s = time.monotonic()
             else:
-                timeout_count += 1
-                if timeout is not None and timeout_count >= 100 * timeout:
+                if timeout is not None and time.monotonic() >= begin_char_s + timeout:
                     break
                 time.sleep(0.01)
         return data