Refactor serial read functions to enhance timeout handling and improve test coverage

- Renamed `readUntilNotReady` to `readUntilTimeoutOrFull` for clarity and updated its parameters to include a per-iteration timeout.
- Modified the `serialRead` function to utilize the new read function, allowing for more flexible timeout management based on a multiplier.
- Added a new utility function `drainInput` in tests to facilitate input handling during tests.
- Introduced new test cases to validate timeout behavior when reading from the serial interface, ensuring robustness in scenarios with no data available.

Author: Katze719

src/serial_read.cpp

@@ -41,13 +41,14 @@ auto handleWaitResultForRead(int wait_result, int abort_fd, ErrorCallbackT error
     return std::nullopt;
 }
 
-auto readUntilNotReady(int fd, int abort_fd, unsigned char *buf, int buffer_size, int already_read,
-                       ErrorCallbackT error_callback) -> int
+auto readUntilTimeoutOrFull(int fd, int abort_fd, unsigned char *buf, int buffer_size, int already_read,
+                            int per_iteration_timeout_ms, ErrorCallbackT error_callback) -> int
 {
     int total_read = already_read;
     while (total_read < buffer_size)
     {
-        const int wait_result = cpp_bindings_linux::detail::waitFdReadyOrAbort(fd, abort_fd, 0, true);
+        const int wait_result =
+            cpp_bindings_linux::detail::waitFdReadyOrAbort(fd, abort_fd, per_iteration_timeout_ms, true);
         if (const auto immediate = handleWaitResultForRead(wait_result, abort_fd, error_callback);
             immediate.has_value())
         {
@@ -66,6 +67,7 @@ auto readUntilNotReady(int fd, int abort_fd, unsigned char *buf, int buffer_size
         }
         if (more_bytes == 0)
         {
+            // Driver reported readiness but returned nothing; treat like "no more data".
             return total_read;
         }
         total_read += static_cast<int>(more_bytes);
@@ -76,7 +78,7 @@ auto readUntilNotReady(int fd, int abort_fd, unsigned char *buf, int buffer_size
 
 extern "C"
 {
-    MODULE_API auto serialRead(int64_t handle, void *buffer, int buffer_size, int timeout_ms, int /*multiplier*/,
+    MODULE_API auto serialRead(int64_t handle, void *buffer, int buffer_size, int timeout_ms, int multiplier,
                                ErrorCallbackT error_callback) -> int
     {
         if (buffer == nullptr || buffer_size <= 0)
@@ -127,7 +129,21 @@ extern "C"
             }
         }
 
-        return readUntilNotReady(fd, abort_fd, buf, buffer_size, static_cast<int>(bytes_read), error_callback);
+        const int already_read = static_cast<int>(bytes_read);
+
+        if (multiplier == 0)
+        {
+            return already_read;
+        }
+
+        const int per_byte_timeout_ms = timeout_ms * multiplier;
+        if (per_byte_timeout_ms <= 0)
+        {
+            return readUntilTimeoutOrFull(fd, abort_fd, buf, buffer_size, already_read, 0, error_callback);
+        }
+
+        return readUntilTimeoutOrFull(fd, abort_fd, buf, buffer_size, already_read, per_byte_timeout_ms,
+                                      error_callback);
     }
 
 } // extern "C"

tests/test_serial_arduino.cpp

@@ -20,6 +20,26 @@
 #include <thread>
 #include <unistd.h>
 
+namespace
+{
+auto drainInput(intptr_t handle) -> void
+{
+    std::array<unsigned char, 256> tmp = {};
+    for (;;)
+    {
+        const int res = serialRead(handle, tmp.data(), static_cast<int>(tmp.size()), 10, 0, nullptr);
+        if (res < 0)
+        {
+            FAIL() << "drainInput failed with error " << res;
+        }
+        if (res == 0)
+        {
+            return;
+        }
+    }
+}
+} // namespace
+
 class SerialArduinoTest : public ::testing::Test
 {
   protected:
@@ -108,6 +128,27 @@ TEST_F(SerialArduinoTest, ReadTimeout)
     EXPECT_GE(read_bytes, 0) << "Timeout should return 0, not error";
 }
 
+TEST_F(SerialArduinoTest, Read60BytesWithoutWritingTimesOut)
+{
+    drainInput(handle);
+    std::array<unsigned char, 60> buffer = {};
+    const int read_bytes = serialRead(handle, buffer.data(), static_cast<int>(buffer.size()), 200, 1, nullptr);
+    EXPECT_EQ(read_bytes, 0) << "Expected timeout (0 bytes) when no data is sent";
+}
+
+TEST_F(SerialArduinoTest, Write10BytesRead60Returns10ThenTimesOut)
+{
+    drainInput(handle);
+    const std::array<unsigned char, 10> payload = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9'};
+    const int written = serialWrite(handle, payload.data(), static_cast<int>(payload.size()), 2000, 1, nullptr);
+    ASSERT_EQ(written, static_cast<int>(payload.size()));
+
+    std::array<unsigned char, 60> buffer = {};
+    const int read_bytes = serialRead(handle, buffer.data(), static_cast<int>(buffer.size()), 200, 1, nullptr);
+    EXPECT_EQ(read_bytes, static_cast<int>(payload.size()))
+        << "Expected to read the 10 echoed bytes, then timeout waiting for more";
+}
+
 TEST_F(SerialArduinoTest, AbortRead)
 {
     std::atomic<int> read_result{999};