ports: Support UART.irq for the RP2, ESP32, MIMXRT, NRF and SAMD ports.

[robert-hh]

Adds UART IRQ functionality for 4 additional ports: RP2, ESP32, MIMXRT and SAMD. Available triggers:

IRQ_RX: called every time when a byte is received. Not useful for high data rates (like > 9600 baud). IRQ_RXIDLE: called when the receive line gets silent after a burst of data. Very useful! IRQ_TXIDLE: called when a block of data has been sent.

Not all ports provide all triggers:

• rp2: IRQ_RXIDLE, IRQ_TXIDLE and IRQ_BREAK. IRQ_RXIDLE works as it should. IRQ_TXIDLE is only called when the data length is mor than 5 bytes, and it triggers when still 4-5 bytes are to be sent.
• MIMXRT: IRQ_RXIDLE and IRQ_TXIDLE. Work as they should with a fix to the NXP UART driver. which is included in this PR.
• SAMD: IRQ_RX and IRQ_TXIDLE. The SAMD port's hardware does not support IRQ_RXIDLE. Based on the IRQ_RX trigger a simple class can be made which implements a IRQ_RXIDLE handler call. IRQ_TXIDLE triggers while the last byte is sent.
• ESP32: IRQ_RX and IRQ_BREAK: The ESP32 does not support IRQ_RXIDLE. The option hard=True is not supported.
• NRF: IRQ_RX and IRQ_TXIDLE. Enabled for boards with nrf52480. I could not test other boards.

Tested with RP2040, ESP32. ESP32-S2, ESP32-S3, ESP32-C3, SAMD21, SAMD51, NRF52840, and all supported MIMXRT MCUs except i.MX RT 1064 (I do not have access to a board).

Test observations for other boards:

• STM32: IRQ_RXIDLE. For F4xx MCU (e.g. Pyboard), the RXIDLE interrupt triggers twice with every message longer than 1 byte starting with the second message after UART init. The first trigger happens at the first byte of the message, the second after the message. 1 Byte messages get only one trigger, but with different delays after the message. Tests with F7xx and H7xx boards showed proper behavior.
• Renesas-RA: Trigger IRQ_RX.
• CC2300: Trigger RX_ANY. IRQ_RX was added as alias in this PR.

[dpgeorge]

Thanks for this, it's a great addition! Definitely needed.

By chance I was working on the docs side of this today: #14040.

[robert-hh]

@dpgeorge I extended the documentation by adding the commit from your PR #14040.

[robert-hh]

Discussion topics:

• Some alternative names for the triggers: IRQ_RXANY instead of IRQ_RX IRQ_TXDONE instead of IRQ_TXIDLE

• The trigger IRQ_TXIDLE for the nrf port seems of less value, since uart.write() is synchronous and waits anyhow, until all data has been sent. That's because the nrf implementation does not use separate buffers for writing. But that synchronous mode can be changed easily, such that uart.write() returns immediately. The drawback then is, that the data which was sent must not be changed until the write is finished, which can be enforced with uart.flush(), tested with uart.txdone() or released by a IRQ_TXIDLE callback.

[github-actions[bot]]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +8 +0.002% PYBV10
     mimxrt:  +408 +0.112% TEENSY40
        rp2:  +632 +0.070% RPI_PICO_W[incl +8(bss)]
       samd:  +736 +0.276% ADAFRUIT_ITSYBITSY_M4_EXPRESS

[dpgeorge]

Thanks again for working on this. After thinking more about it here are my general comments.

I don't think it makes sense to have both IRQ_RX and IRQ_RXIDLE, especially because it seems that the ports either implement one or the other, but never both.

We need to make it as easy as possible to write portable code that runs across all the ports, and having different IRQs, or the same IRQ but with slightly different semantics, makes that hard.

So I suggest to not have IRQ_RX at all, and make IRQ_RXIDLE mandatory. Users should be able to rely on thisIRQ to have the following semantics:

IRQ_RXIDLE: every character received by the UART will have this interrupt
triggered at some point in time after that character is received

The idea is that if you only ever do uart.read() within the IRQ_RXIDLE callback, then you should always be able to (eventually) read all available characters. And ideally in a timely manner.

Even if the hardware/SDK doesn't support this concept we should still be able to synthesize it. Eg if the hardware supports IRQ_RX then in combination with a timer we can implement IRQ_RXIDLE.

Having IRQ_BREAK (when the hardware supports it) makes sense, the semantics for that seem obvious.

For IRQ_TXIDLE/IRQ_TXDONE: as with IRQ_RXIDLE the semantics of this need to be well specified, and then all ports should implement it.

[robert-hh]

So you suggest that IRQ_RX is renamed to IRQ_RXIDLE for those ports that do not support a true IRQ_RXIDLE, and for those that do so IRQ_RX is dropped?

B.t.w: I made two Python classes as example that implement IRQ_RXIDLE based on an existing IRQ_RX. The simpler one is below with uart.irq() being a method. The other implements uart.irq() as a class. I considered implementing that in the machine_uart module itself, but dealing with the timer is easier in Python.

#
# Simple wrapper class for UART, adding the trigger IRQ_RXIDLE for port
# which just support IRQ_RX
# Missing: Handling of the irq.xxx methods in this class.
#
import machine

class UART(machine.UART):
    def __init__(self, id, *args, **kwargs):
        self.irq_timeout_count = 0
        self.irq_rx_count = 0
        self.timeout_idle = 5
        self.irq_timer = None
        self.irq_handler = None
        self.IRQ_RXIDLE = 4096

    def irq(self, handler=None, trigger=None, hard=False, period=5):
        if handler is not None:
            if trigger == self.IRQ_RXIDLE:
                self.irq_handler = handler
                self.irq_timer = machine.Timer(callback=self.irq_timer_callback,
                    mode=machine.Timer.PERIODIC, period=period)
                return super().irq(handler=self.irq_rx_callback, trigger=self.IRQ_RX, hard=True)
            else:
                if self.irq_timer is not None:
                    self.irq_timer.deinit()
                self.irq_handler = None
                return super().irq(handler=handler, trigger=trigger, hard=hard)
        else:
            if self.irq_timer is not None:
                self.irq_timer.deinit()
            self.irq_handler = None
            return super().irq(handler=None, trigger=0, hard=False)

    def irq_rx_callback(self, uart):
        self.irq_rx_count += 1
        self.irq_timeout_count = 0

    def irq_timer_callback(self, timer):
        if self.irq_rx_count > 0:
            self.irq_timeout_count += 1
            if self.irq_timeout_count == 2:
                if self.irq_handler is not None:
                    self.irq_handler(self)
                self.irq_rx_count = 0
                self.irq_timeout_count = 0

[robert-hh]

@dpgeorge Thinking about further, I prefer to support the behaviour with proper names. Looking at the list of ports, three of them support true IRQ_RXIDLE, and eight support IRQ_RX. Both the rp2040 and the MIMXRT port can support IRQ_RX mode. The rp2040 commit would get much simpler and more close to the existing state. Without support for IRQ_RXIDLE, the MIMXRT port would not need a change to the NXP UART driver. So I suggest to have IRQ_RX as the common method and have support for IRQ_RXIDLE as a derived class method.

[dpgeorge]

So I suggest to have IRQ_RX as the common method and have support for IRQ_RXIDLE as a derived class method.

I understand it's simpler to implement what the hardware provides. But:

• IRQ_RX is not that useful for large data rates (as you already mentioned). So it will be impossible to implement IRQ_RXIDLE behaviour in Python using IRQ_RX (because IRQ_RX will be called too fast for Python to keep up with it). Even if the implementation does work, it won't be efficient.
• We need to provide something portable that works the same across all ports. Eg so the network.PPP driver can work and use the UART IRQ.

[robert-hh]

For the existing PR is see then two options: a) Drop the changes for IRQ_RX and keep only the changes for RP2040 and MIMXRT for IRQ_RXIDLE. More boards may be added later it a IRQ_RXIDLE is implemented based on an internal IRQ_RX and a timer. b) Rename IRQ_RX to IRQ_RXIDLE and have a note in the documentation about the difference to real IRQ_RXIDLE.

Obviously the behavior of RXIDLE can as well be implemented in Python with a timer interrupt, which checks if uart.any() values stall at a value > 0.

[dpgeorge]

After discussion with @jimmo and @projectgus we came to the conclusion that it makes sense to expose multiple UART IRQs, and then the user can decide which is best for their application. But we do need to have consistence across ports for the behaviour of the interrupts.

• IRQ_RX: all ports will be able to implement this in hardware (I assume?) and it's a pretty useful interrupt to have access to, although only for slow baud rates.
• IRQ_RXFIFO: for when the RX input buffer is half full. This can be easily synthesized (in C) as long as IRQ_RX is available. Note that we don't need to implement this one yet but there is scope to add it in the future.
• IRQ_RXIDLE: when the RX line remains high for whole character period after receiving at least one character. This can be synthesized (in C) for ports that don't have hardware support for it.
• IRQ_BREAK: useful and obvious semantics.
• IRQ_TXIDLE: not so sure about this one, it's a bit harder to define and we don't yet have consistency across ports for TX, whether it's blocking or runs in the background.

So in the end, based on the above points, I think the PR as it stands is in pretty good shape! The main thing is to have good documentation that specifies the behaviour of each IRQ well, and makes sure the implementations for each port match that behaviour so it can be relied on.

[robert-hh]

Thank you for the confirmation. Some notes:

IRQ_RX: all ports will be able to implement this in hardware

Actually, at the RP2040 and MIMXRT port one has to chose between IRQ_RX and IRQ_RXIDLE. For the RP2040, it's the way interrupts are created and the FIFO is working. Unfortunately it is not possible to tell how many bytes are in the FIFO. And IRQ_TXIDLE happens only if at least one byte is in the FIFO. For that reason, the FIFO full IRQ is set to 75% (24 Byte), and in an interrupt not more than 23 bytes are removed from the FIFO. read() takes all data from the FIFO. but with an IRQ_RX at 24Byte this is not so useful. The MIMXRT UART driver has no IRQ_RX event, since that is all handled in the driver itself. But I can check again if some of the other Event match.

IRQ_RXFIFO: for when the RX input buffer is half full.

That looks doable to some extend for most ports, if the RX ring buffer is taken as reference.

IRQ_RXIDLE: when the RX line remains high for whole character period after receiving at least one character. This can be synthesized (in C) for ports that don't have hardware support for it.

Principally yes. The challenge will be to use the right timer mechanism with little overhead. The time the RX line has to stay high is 30 bit times for RP2040, which is hardwired, and for MIMXRT I have configured it to 4 byte times starting from the most recent start bit, resulting in about 30 bit times after the last byte. If I recall right, the STM32 timing is similar. The Python code above works well for slow communication like that from a GPS receiver or other sensors. So it has it's use case.

IRQ_BREAK: useful and obvious semantics.

I looked at the datasheets for all eight ports. For only two ports I found a hardware interrupt or signalling mechanism for break. I'm still wondering why this is not available at the MIMXRT or STM32 port.

IRQ_TXIDLE: not so sure about this one, it's a bit harder to define and we don't yet have consistency across ports for TX, whether it's blocking or runs in the background.

Yes. This one should happen when all data of a "message" is sent. But yes, if the driver buffers internally without being able to tell, that could be tricky. And we have already uart.flush() and uart.txdone(), which could be used for that purpose, with the same limitation.

I updated the documentation according to Angus' comments.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.43%. Comparing base (fd03a05) to head (09d070a). Report is 16 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #14041   +/-   ##
=======================================
  Coverage   98.43%   98.43%           
=======================================
  Files         163      163           
  Lines       21295    21295           
=======================================
  Hits        20961    20961           
  Misses        334      334           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[robert-hh]

The PR is now updated with a sample implementation of IRQ_RXIDLE for the SAMD port using softtimer. The same approach can be used for the Renesas port. For that purpose, the softtimer data structure is extended by an additional pointer, which can port at context data for the timer interrupt. In this case it's the respective UART data structure. That allows to have a single timer callback and not a separate callback for each UART. At the moment that does not interfere with any other use of softtimer. To make is consistent, the function softtimer_static_init() may get another argument, the context pointer. That requires to change all call of that function. Not a big thing, but I wanted to wait until the approach of a context pointer is accepted.

[dpgeorge]

@robert-hh I have added tests for all the IRQs added in this PR. I pushed those tests to your branch. Feel free to modify the tests, eg add more configurations for other ports.

Each test must have configuration for a given port, eg the UART id and UART pins. Some tests can run without extra hardware connections, but others require connecting TX and RX. The tests are documented at their top to say what they need.

I ran the tests on a few different ports (stm32, rp2, esp32, samd). Not all of them pass. It would be great if all tests could pass on all ports (or skip if the port doesn't support that IRQ), but I know that could be difficult to achieve. If you have some time to look into making them pass that would be fantastic!

---

For example, running the tests on various hardware:

ESP32

$ ./run-tests.py --target minimal --device /dev/ttyUSB0 extmod/machine_uart*.py extmod_hardware/machine_uart*.py
skip  extmod/machine_uart_irq_txidle.py
skip  extmod/machine_uart_tx.py
FAIL  extmod_hardware/machine_uart_irq_break.py
pass  extmod_hardware/machine_uart_irq_rxidle.py
pass  extmod_hardware/machine_uart_irq_rx.py
3 tests performed (36 individual testcases)
2 tests passed
2 tests skipped: machine_uart_irq_txidle machine_uart_tx
1 tests failed: machine_uart_irq_break

SAMD21

$ ./run-tests.py --target minimal extmod/machine_uart*.py extmod_hardware/machine_uart*.py             
FAIL  extmod/machine_uart_irq_txidle.py
skip  extmod/machine_uart_tx.py
skip  extmod_hardware/machine_uart_irq_break.py
pass  extmod_hardware/machine_uart_irq_rxidle.py
FAIL  extmod_hardware/machine_uart_irq_rx.py
3 tests performed (30 individual testcases)
1 tests passed
2 tests skipped: machine_uart_irq_break machine_uart_tx
2 tests failed: machine_uart_irq_rx machine_uart_irq_txidle

rp2 (Pico)

$ ./run-tests.py --target minimal extmod/machine_uart_irq*.py extmod_hardware/machine_uart_irq*.py 
FAIL  extmod/machine_uart_irq_txidle.py
pass  extmod_hardware/machine_uart_irq_break.py
pass  extmod_hardware/machine_uart_irq_rxidle.py
skip  extmod_hardware/machine_uart_irq_rx.py
3 tests performed (33 individual testcases)
2 tests passed
1 tests skipped: machine_uart_irq_rx
1 tests failed: machine_uart_irq_txidle

stm32 (PYBv1.0)

$ ./run-tests.py --target minimal extmod/machine_uart_irq*.py extmod_hardware/machine_uart_irq*.py
skip  extmod/machine_uart_irq_txidle.py
skip  extmod_hardware/machine_uart_irq_break.py
FAIL  extmod_hardware/machine_uart_irq_rxidle.py
pass  extmod_hardware/machine_uart_irq_rx.py
2 tests performed (21 individual testcases)
1 tests passed
2 tests skipped: machine_uart_irq_break machine_uart_irq_txidle
1 tests failed: machine_uart_irq_rxidle

[robert-hh]

I'll see what can be done for testing. In my tests I used two modules. Using a single device with a kind of loopback was sometimes not possible.. Edit: The test must be configured for a specific board. The port alone is not sufficient for some of them. For instance SAMD or MIMXRT or NRF board provide different pins for UART. Similar for ESP32. It seems that you used a SEEED XIAO SAM21 for the SAMD test, and using a Teensy 4.0 for MIMXRT would be an good choice. What should I take for the NRF port, what for Renesas RA? And for SAMD I would prefer the ADAFRUIT_ITSYBITSY_Mx_EXPRESS boards.

[robert-hh]

The test scripts are now reworked to some extend. Tests pass for almost all supported boards. Some boards cannot pass the test even if the feature works:

• ESP32S2 and ESP32C3 have no UART2 and cannot run the IRQ_BREAK test. IRQ_BREAK cannot be performed in a UART loopback mode. IRQ_BREAK works and can be triggered with a GPIO pulse, but then there is no serial data as expected by the test.
• STM32 (PyBoard) fails at the IRQ_RXIDLE test even if the mechanism works. It just triggers more than one IRQ, and then the state of the flag and the expected data mismatch. So I made the test to check for a proper value in the callback only if the IRQ_RXIDLE flag is set. The behavior of the STM32 port can be addressed separately.
• CC3200 is too slow for the IRQ_RX test, and it locks up in the callback when attempting to allocate RAM. Given that the CC3200 is legacy, the test support is not included.

[dpgeorge]

The test scripts are now reworked to some extend. Tests pass for almost all supported boards.

Excellent, thanks for working on that! I will test them myself.

[robert-hh]

Re-testing this morning when running all tests in a row with the loopback jumper attached I see a few hiccups:

• the NRF port locks up when trying to run the second test. It seems to require a hard reset between tests.
• SAMD fails the IRQ_TXIDLE test with the loopback jumper attached. In that case, the IRQ callback is called twice instead of once. There is a similar multiple call situation with STM32 and IRQ_RXIDLE.

[robert-hh]

I updated the SAMD port such that the double callbacks are gone.

[Gadgetoid]

Looking at RP2 (regular Pico), trying to send 512 bytes I get multiple IRQs firing but only 288 bytes of data in the first, and nothing in the subsequent IRQ(s). Is this normal, related to my issues with LTE/PPP?

from machine import UART
import random
import time, sys


def irq(u):
    if u.irq().flags() & u.IRQ_RXIDLE:
        data = u.read()
        if data:
            print("IRQ_RXIDLE:", bool(u.irq().flags() & u.IRQ_RXIDLE), "data:", data, len(data))
        else:
            print("IRQ_RXIDLE: NO DATA!?")


BUF_LEN = 512
text = "".join(chr(random.randint(ord('A'), ord('z'))) for _ in range(BUF_LEN))
 
uart = UART(0, 115200, tx=0, rx=1, timeout=1)
uart.irq(irq, uart.IRQ_RXIDLE)

while True:
    print(f"\nWriting {len(text)} bytes...")
    uart.write(text)
    time.sleep_ms(5000)

eg:

Writing 512 bytes...
IRQ_RXIDLE: True data: b'b]Ubf_s`zOGscbZvdifV^FFsZyxK^gXh_yOjxHTYwOJOCiPyYfV^RYKx`ZPN[eBbDACGjumymFqvLi^X[swHFBGucRZjUkdNuiDixdpidqTDyD_W^rPngQIkkMFmJwycUdgpzgMwWW]LSMYtzgGpjCznIhP[Kpg_]Qa]TLxwmcRDDRjDJqkCVhUZ]AijGRuWwY_iUHqOimvXdbqEfgKHwvDlWpvsWFuhBwZsTV]vGycxutlZXZI\\HQe\\UmAbFeefjZZPmONI[ai]ImmDXWyCpxXedF]rZkHG' 288
IRQ_RXIDLE: NO DATA!?
IRQ_RXIDLE: NO DATA!?

Writing 512 bytes...
IRQ_RXIDLE: True data: b'b]Ubf_s`zOGscbZvdifV^FFsZyxK^gXh_yOjxHTYwOJOCiPyYfV^RYKx`ZPN[eBbDACGjumymFqvLi^X[swHFBGucRZjUkdNuiDixdpidqTDyD_W^rPngQIkkMFmJwycUdgpzgMwWW]LSMYtzgGpjCznIhP[Kpg_]Qa]TLxwmcRDDRjDJqkCVhUZ]AijGRuWwY_iUHqOimvXdbqEfgKHwvDlWpvsWFuhBwZsTV]vGycxutlZXZI\\HQe\\UmAbFeefjZZPmONI[ai]ImmDXWyCpxXedF]rZkHG' 288
IRQ_RXIDLE: NO DATA!?

[robert-hh]

I'll check. Did you have a loopback connection at UART 0?

[robert-hh]

You have to set rxbuf to a larger value. The default is 256. It should be 512 at least. 288 = 256 (rxbuf) + 32 (FIFO size).

[Gadgetoid]

The default is 256. It should be 512 at least.

Hmm I suppose I should probe deeper into what my LTE module tries to send, since this implies it could be overwhelming the rx buffer and might explain why I must use polling.

That said, a generously large (4k) rxbuf does not appear to help. Which only really suggests this is unrelated.

[robert-hh]

I made all changes, but will a test run with all tests and board again.

[robert-hh]

@dpgeorge Ran the tests and it looks OK so far. I found two hiccups:

• IRQ_TXIDLE: SAMD21 and RP2: fail sometimes the test at 115200 baud when the tx/rx loopback is present. It passes always at 57600 baud. The IRQ is called, but the IRQ_TXIDLE flag is not set. Most likely this happens due to the delayed scheduling of the callback. At that time the IRQ_TXIDLE flag is already cleared. Patched by setting hard=True in these cases.
• The NRF port does not behave well when running a series of tests with run-test.py. It locks up after the first test. Running each test individually works well ~~, if the board (Arduino) is hard reset before each test~~.

[Gadgetoid]

• Patched by setting hard=True in these cases.

AFAIK the PPP module does not set hard=True:

https://github.com/micropython/micropython/blob/654711c699e9f27d37442f10ee01827662c4a2bf/extmod/network_ppp_lwip.c#L230-L235

Should it? Or is the PPP poll() method likely to do something verboten by hardware interrupts?

[robert-hh]

Most likely that is not suitable. In hard irq handlers allocating memory is not possible.

[Gadgetoid]

There's no reference held to the receieve buffer:

https://github.com/micropython/micropython/blob/0d313fb721cdd2516cf3c65333f5d89c6ce632d2/ports/mimxrt/machine_uart.c#L339-L340

So if you add a gc.collect() to the txidle test, it will crash.

Minimal repro for RP2:

# Test machine.UART.IRQ_TXIDLE firing after a transmission.
# Does not require any external connections.

from machine import UART
import time, sys
import gc

# Configure pins based on the target.
hard_irq = False

uart_id = 0
tx_pin = "GPIO0"
rx_pin = "GPIO1"
min_window = 1
hard_irq = True


def irq(u):
    print("IRQ_TXIDLE:", u.irq().flags() == u.IRQ_TXIDLE)


text = "Hello World" * 30

# Test that the IRQ is called after the write has completed.
for bits_per_s in (2400, 9600, 115200):
    uart = UART(uart_id, bits_per_s, tx=tx_pin, rx=rx_pin, rxbuf=512)
    uart.irq(irq, uart.IRQ_TXIDLE, hard=True)
    
    # Comment me in/out for crashy funtimes!
    gc.collect()

    bits_per_char = 10  # 1(startbit) + 8(bits) + 1(stopbit) + 0(parity)
    start_time_us = (len(text) - 10) * bits_per_char * 1_000_000 // bits_per_s
    window_ms = max(min_window, 20 * bits_per_char * 1_000 // bits_per_s + 1)

    print("write", bits_per_s)
    uart.write(text)
    time.sleep_us(start_time_us)
    print("ready")
    time.sleep_ms(window_ms)
    print("done")

Not introduced in this PR, possibly here? https://github.com/micropython/micropython/commit/689476c576dc4ea5e944f396bd4bf2b95e08cf70

[robert-hh]

About the NRF port: testing with a Seeed XIAO NRF52840 instead of the Arduino Nano 33 BLE, test batches started by run-tests.py work, but the test for IRQ_TXIDLE fails if the rx/tx loopback is present.

[robert-hh]

So if you add a gc.collect() to the txidle test, it will crash.

The crash is not related to hard=True. Let's look for a free floating pointer.