UART driver: large RX buffer may be ineffective (IDFGH-11220)

[umisef]

Answers checklist.

• I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.1.1

Espressif SoC revision.

ESP32C3

Operating System used.

macOS

How did you build your project?

Command line with Make

If you are using Windows, please specify command line type.

None

Development Kit.

Custom design

Power Supply used.

USB

What is the expected behavior?

Using a large (in our case 180kB) RX buffer with the UART driver should cause no issues while read()ing is permanently running, say, 160kB "behind" the data being received.

What is the actual behavior?

After a while, the UART driver reports "RX Buffer Full", and then stops removing data from the UART's hardware FIFO, leading to reports of FIFO overflow.

The UART RX only resumes once the reader has read through the whole amount it is typically behind.

This is caused by the UART driver using xRingBufferReceive() on a byte ring buffer, which will give ownership of ALL the data currently in the ring buffer, or all the data up to the end of the storage area (if the data wraps), to the UART driver. The driver then parcels this data out a few bytes at a time in read() requests, but has no way of returning no-longer-needed data to the ring buffer until it has used ALL bytes for read()s.

When the reader is running considerably behind the UART RX, this can result in almost all of the RX buffer being owned by the UART, driver, and being held for an unreasonable amount of time.

Steps to reproduce.

1. Initialise UART driver with 100kB RX buffer
2. Have external source continuously send data to the UART
3. In the app's main loop, use uart_get_buffered_data_len() to check how much data is available in the RX buffer
4. Only call uart_read_bytes() whenever at least 80kB are available
5. Note how the UART driver reports "RX buffer full" while uart_get_buffered_data_len() reports just over 80kB used, so almost 20kB unused
6. Note also that the UART driver does not recover, and reception of data stops completely

Debug Logs.

The behaviour is completely fixed with the following modification to the UART driver, which limits the portion of the buffer owned by the driver at any one time to 1/16th:

`diff --git a/components/driver/uart/uart.c b/components/driver/uart/uart.c
index 31dc602919..3e2334a16d 100644
--- a/components/driver/uart/uart.c
+++ b/components/driver/uart/uart.c
@@ -1253,7 +1253,19 @@ int uart_read_bytes(uart_port_t uart_num, void *buf, uint32_t length, TickType_t
     }
     while (length) {
         if (p_uart_obj[uart_num]->rx_cur_remain == 0) {
-            data = (uint8_t *) xRingbufferReceive(p_uart_obj[uart_num]->rx_ring_buf, &size, (TickType_t) ticks_to_wait);
+            // EVERY byte received here is unavailable to the ISR until ALL bytes have been read.
+            // So we need to make sure we don't accidentally lock up most of the rxbuf. Instead,
+            // limit what we claim ownership of to 1/16th of the rx_buffer, or how much the current
+            // read is about to consume, whichever is more.
+            // TODO --- is there ever a reason to ask for more than 'length'? The
+            // xRingbufferReceiveUpTo() is fairly lightweight, so is it really worth avoiding it?
+            // Especially if never asking for more than length would make it possible to avoid
+            // the whole rx_head_ptr/rx_prt/rx_cur_remain overhead in the UART object?
+            size_t max_receive=p_uart_obj[uart_num]->rx_buf_size/16;
+            if (max_receive<length)
+                max_receive=length;
+
+            data = (uint8_t *) xRingbufferReceiveUpTo(p_uart_obj[uart_num]->rx_ring_buf, &size, (TickType_t) ticks_to_wait, max_receive);
             if (data) {
                 p_uart_obj[uart_num]->rx_head_ptr = data;
                 p_uart_obj[uart_num]->rx_ptr = data;
`

More Information.

No response