machine.UART: Fix the double timeout with uart.read(), uart.write() and uart.readinto(). #17938

robert-hh · 2025-08-16T13:58:23Z

Summary

When uart.read(), uart.write() and uart_readinto() timed out because less data was transferred than expected, but some, the delay was uart.timeout + uart.timeout.char instead of just uart,timeout_char().

This PR changes the behavior into the expected one. It consists of 3 commits. The first changes uart.read() and uart.write(). The impact of that change is limited to the machine.UART class. The second commit changes stream.readinto() and may affect all uses of that function.
The third commit changes the ESP32's machine_uart.c to deal properly with the timeout_char argument for uart.read().

uart.readline() is unchanged.

This PR addresses issue #17611.

Testing

Tested with STM32(Pyboard V1.1 and PYBD_SF6), RP2 Pico, MIMXRT(Teensy 4.1), SAMD51, ESP32, NRF (nrf52840) and a CC3200 board using strings of varying length. Typical results:

RP2

UART(0, baudrate=38400, bits=8, parity=None, stop=1, tx=0, rx=1, txbuf=256, rxbuf=256, timeout=200, timeout_char=50, invert=None, irq=0)

uart.read() msg=""  timeout=201 ms
uart.read() msg="123"  timeout=52 ms
uart.read() msg="12345"  timeout=1 ms

uart.readline() msg=""  timeout=201 ms
uart.readline() msg="12345"  timeout=202 ms
uart.readline() msg="12345\n"  timeout=2 ms

uart.readinto(bytearray(5)) msg="" timeout=201 ms
uart.readinto(bytearray(5)) msg="123" timeout=52 ms
uart.readinto(bytearray(5)) msg="12345" timeout=1 ms

CC3200

UART(1, pins=(Pin("GP16"), Pin("GP17")))

uart.read() msg=""  timeout=36 ms
uart.read() msg="123"  timeout=38 ms (from 74 ms without the change)
uart.read() msg="12345"  timeout=5 ms

uart.readline() msg=""  timeout=35 ms
uart.readline() msg="12345"  timeout=41 ms
uart.readline() msg="12345\n"  timeout=6 ms

uart.readinto(bytearray(5)) msg="" timeout=35 ms
uart.readinto(bytearray(5)) msg="123" timeout=39 ms
uart.readinto(bytearray(5)) msg="12345" timeout=5 ms

Trade-offs and Alternatives

If the impact of the changes to a stream module seems unsafe, the second commit may be omitted, causing the behavior of uart.readinto() unchanged.

github-actions · 2025-08-16T14:14:36Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:   +16 +0.002% standard
      stm32:   +68 +0.017% PYBV10
     mimxrt:   +64 +0.017% TEENSY40
        rp2:   +32 +0.003% RPI_PICO_W
       samd:   +68 +0.025% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:   +24 +0.005% VIRT_RV32

py/stream.c

codecov · 2025-08-29T14:20:08Z

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.38%. Comparing base (c7c0ad2) to head (601b210).

Files with missing lines	Patch %	Lines
py/stream.c	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #17938      +/-   ##
==========================================
- Coverage   98.38%   98.38%   -0.01%     
==========================================
  Files         171      171              
  Lines       22296    22300       +4     
==========================================
+ Hits        21937    21940       +3     
- Misses        359      360       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dpgeorge · 2025-09-08T03:58:48Z

extmod/machine_uart.c

-    { MP_ROM_QSTR(MP_QSTR_readinto), MP_ROM_PTR(&mp_stream_readinto_obj) },
-    { MP_ROM_QSTR(MP_QSTR_write), MP_ROM_PTR(&mp_stream_write_obj) },
+    { MP_ROM_QSTR(MP_QSTR_readinto), MP_ROM_PTR(&mp_stream_readinto1_obj) },
+    { MP_ROM_QSTR(MP_QSTR_write), MP_ROM_PTR(&mp_stream_write1_obj) },


Unfortunately using write1 here will be a breaking change, because it doesn't accept the additional arguments like write does.

So, we'll need to use the same approach as you did with stream_readinto_generic, to make a stream_write_generic(size_t n_args, const mp_obj_t *args, byte flags).

Which additional arguments are expected in the context of machine_uart's write()? Obviously uart.write() works in the test I made.

The signature is:

stream.write(buf) stream.write(buf, max_len) stream.write(buf, off, max_len)

I don't think this is actually documented anywhere, but it may be relied upon in some code.

Thank you. I did not find any documentation about it. It it safe then to change write1 to accept the additional arguments? The only places where it seems to be referenced were extmod/modtls_mbedtls.c and unix/coverage.c.

It it safe then to change write1 to accept the additional arguments?

Yes, it's safe, because it's an extension of the existing signature.

Ideally we would not support these extra arguments, which are a MicroPython specific extension (extension to CPython). Maybe we can remove them in MicroPython v2.0.

But for now, they are needed. Eg umqtt.simple uses the second argument, eg self.sock.write(pkt, 2).

Change implemented & tested with a RP2 board. Both timeout and additional write() arguments work as expected.

dpgeorge · 2025-09-08T03:59:03Z

ports/cc3200/mods/pybuart.c

@@ -579,13 +579,13 @@ static const mp_rom_map_elem_t pyb_uart_locals_dict_table[] = {
    { MP_ROM_QSTR(MP_QSTR_txdone),      MP_ROM_PTR(&machine_uart_txdone_obj) },

    /// \method read([nbytes])
-    { MP_ROM_QSTR(MP_QSTR_read),        MP_ROM_PTR(&mp_stream_read_obj) },
+    { MP_ROM_QSTR(MP_QSTR_read),        MP_ROM_PTR(&mp_stream_read1_obj) },
    /// \method readline()
    { MP_ROM_QSTR(MP_QSTR_readline),    MP_ROM_PTR(&mp_stream_unbuffered_readline_obj) },
    /// \method readinto(buf[, nbytes])
    { MP_ROM_QSTR(MP_QSTR_readinto),    MP_ROM_PTR(&mp_stream_readinto_obj) },


Should this use readinto1 as well?

Indeed. The CC3200 is always a little bit out of sight.

Changed & tested. The timeout and timeout_char situation have the same timeout. These values are fixed with the CC3200 port and are set to 16 character times, as far as I read the code. What I see in testing is a delay of about 36 character times.

dpgeorge · 2025-09-08T04:00:11Z

py/stream.c

@@ -303,7 +303,12 @@ static mp_obj_t stream_readinto(size_t n_args, const mp_obj_t *args) {
    }

    int error;
-    mp_uint_t out_sz = mp_stream_read_exactly(args[0], bufinfo.buf, len, &error);
+    mp_uint_t out_sz;
+    if (flags & MP_STREAM_RW_ONCE) {


This if can be simplified to a single line:

mp_stream_rw(args[0], bufinfo.buf, len, &error, flags);

and then pass in MP_STREAM_RW_READ as flags from stream_readinto(), and MP_STREAM_RW_READ | MP_STREAM_RW_ONCE from stream_readinto1().

Done, good hint.

Do not try to read/write again after timeout happened once. Signed-off-by: robert-hh <robert@hammelrath.com>

Before, it was ignored. Tested with ESP32, ESP32S3, ESP32C6. Signed-off-by: robert-hh <robert@hammelrath.com>

Avoiding the double timeout when used with the UART class. stream.readinto1() returns after the first timeout. Signed-off-by: robert-hh <robert@hammelrath.com>

As suggested by using a common stream_readinto_generic() function. Reducing the size increase to about 50 bytes. Signed-off-by: robert-hh <robert@hammelrath.com>

As suggested adding a stream_write_generic() function. Tested the different write timeouts with a RP2 using flow control. Tested support of the additional arguments of uart.write(). Signed-off-by: robert-hh <robert@hammelrath.com>

robert-hh force-pushed the uart_timeout branch from 37b0b99 to 688d103 Compare August 17, 2025 08:39

dpgeorge reviewed Aug 28, 2025

View reviewed changes

py/stream.c Outdated Show resolved Hide resolved

robert-hh force-pushed the uart_timeout branch 2 times, most recently from 17a6d11 to 12f7758 Compare August 29, 2025 14:14

dpgeorge added ports Relates to multiple ports, or a new/proposed port extmod Relates to extmod/ directory in source labels Sep 2, 2025

dpgeorge reviewed Sep 8, 2025

View reviewed changes

robert-hh added 4 commits September 8, 2025 10:53

extmod/machine_uart: Return from read()/write() at the first timeout.

6ed7d69

Do not try to read/write again after timeout happened once. Signed-off-by: robert-hh <robert@hammelrath.com>

esp32/machine_uart: Handle properly the timeout_char argument.

1e9b638

Before, it was ignored. Tested with ESP32, ESP32S3, ESP32C6. Signed-off-by: robert-hh <robert@hammelrath.com>

py/stream: Add a stream.readinto1() method for machine.UART.

efd027d

Avoiding the double timeout when used with the UART class. stream.readinto1() returns after the first timeout. Signed-off-by: robert-hh <robert@hammelrath.com>

py/stream: Optimize stream_readinto() and stream_readinto1().

b37d132

As suggested by using a common stream_readinto_generic() function. Reducing the size increase to about 50 bytes. Signed-off-by: robert-hh <robert@hammelrath.com>

Josverl mentioned this pull request Sep 8, 2025

create mpy_shed/reference stubs for MicroPython Buffer protocol Josverl/micropython-stubs#833

Open

robert-hh force-pushed the uart_timeout branch from 4148f99 to 601b210 Compare September 8, 2025 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

machine.UART: Fix the double timeout with uart.read(), uart.write() and uart.readinto(). #17938

machine.UART: Fix the double timeout with uart.read(), uart.write() and uart.readinto(). #17938

robert-hh commented Aug 16, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

codecov bot commented Aug 29, 2025 •

edited

Loading

Uh oh!

dpgeorge Sep 8, 2025

Uh oh!

robert-hh Sep 8, 2025 •

edited

Loading

Uh oh!

dpgeorge Sep 8, 2025

Uh oh!

robert-hh Sep 8, 2025

Uh oh!

dpgeorge Sep 8, 2025

Uh oh!

robert-hh Sep 8, 2025

Uh oh!

dpgeorge Sep 8, 2025

Uh oh!

robert-hh Sep 8, 2025

Uh oh!

robert-hh Sep 8, 2025

Uh oh!

dpgeorge Sep 8, 2025

Uh oh!

robert-hh Sep 8, 2025

Uh oh!

Uh oh!

Uh oh!

machine.UART: Fix the double timeout with uart.read(), uart.write() and uart.readinto(). #17938

Are you sure you want to change the base?

machine.UART: Fix the double timeout with uart.read(), uart.write() and uart.readinto(). #17938

Conversation

robert-hh commented Aug 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Trade-offs and Alternatives

Uh oh!

github-actions bot commented Aug 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robert-hh Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robert-hh commented Aug 16, 2025 •

edited

Loading

github-actions bot commented Aug 16, 2025 •

edited

Loading

codecov bot commented Aug 29, 2025 •

edited

Loading

robert-hh Sep 8, 2025 •

edited

Loading