From 3d75161535b6a47429294c2b3210bc203bc4ae70 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Fri, 1 Mar 2024 17:49:04 -0300 Subject: [PATCH 1/3] feat(uart): setBufferSize It makes sure that setting TX buffer size will match availableForWrite() response. It also sets the buffer to the minimum instead of doing nothing and returning an error. For RX Buffer, it sets the minimum and also don't return an error. This makes the APIs better and easy to understand its results. --- cores/esp32/HardwareSerial.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index 10a6bc1ca1d..eac47473f6e 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -537,35 +537,38 @@ bool HardwareSerial::setMode(SerialMode mode) return uartSetMode(_uart, mode); } +// minimum total RX Buffer size is the UART FIFO space (128 bytes for most SoC) + 1. IDF imposition. size_t HardwareSerial::setRxBufferSize(size_t new_size) { if (_uart) { - log_e("RX Buffer can't be resized when Serial is already running.\n"); + log_e("RX Buffer can't be resized when Serial is already running. Set it before calling begin()."); return 0; } if (new_size <= SOC_UART_FIFO_LEN) { - log_e("RX Buffer must be higher than %d.\n", SOC_UART_FIFO_LEN); // ESP32, S2, S3 and C3 means higher than 128 - return 0; + log_w("RX Buffer set to minimum value: %d.", SOC_UART_FIFO_LEN + 1); // ESP32, S2, S3 and C3 means higher than 128 + new_size = SOC_UART_FIFO_LEN + 1; } _rxBufferSize = new_size; return _rxBufferSize; } +// minimum total TX Buffer size is the UART FIFO space (128 bytes for most SoC). size_t HardwareSerial::setTxBufferSize(size_t new_size) { if (_uart) { - log_e("TX Buffer can't be resized when Serial is already running.\n"); + log_e("TX Buffer can't be resized when Serial is already running. Set it before calling begin()."); return 0; } - if (new_size <= SOC_UART_FIFO_LEN) { - log_e("TX Buffer must be higher than %d.\n", SOC_UART_FIFO_LEN); // ESP32, S2, S3 and C3 means higher than 128 - return 0; + if (new_size < SOC_UART_FIFO_LEN) { + log_w("TX Buffer set to minimum value: %d.", SOC_UART_FIFO_LEN); // ESP32, S2, S3 and C3 means higher than 128 + _txBufferSize = 0; // it will use just UART FIFO with SOC_UART_FIFO_LEN bytes (128 for most SoC) + return SOC_UART_FIFO_LEN; } - - _txBufferSize = new_size; - return _txBufferSize; + // if new_size is SOC_UART_FIFO_LEN, _txBufferSize will be zero - just use the UART FIFO space + _txBufferSize = new_size - SOC_UART_FIFO_LEN; // for total correct report from "availableForWrite()" that matches a call to this function + return new_size; } From c9b1407526820105cff5deaae875e56d51a37d5f Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Fri, 1 Mar 2024 19:08:18 -0300 Subject: [PATCH 2/3] feat: sets TX buffer This will allow to set TX buffer to a minimum with no error message. It also makes it works as in the Arduino API specification that is to return the buffer available space. In ESP32 case it will be the minmum the HW TX FIFO size of 128 bytes. --- cores/esp32/HardwareSerial.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index eac47473f6e..3f3714d3573 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -562,13 +562,12 @@ size_t HardwareSerial::setTxBufferSize(size_t new_size) { return 0; } - if (new_size < SOC_UART_FIFO_LEN) { + if (new_size <= SOC_UART_FIFO_LEN) { log_w("TX Buffer set to minimum value: %d.", SOC_UART_FIFO_LEN); // ESP32, S2, S3 and C3 means higher than 128 _txBufferSize = 0; // it will use just UART FIFO with SOC_UART_FIFO_LEN bytes (128 for most SoC) return SOC_UART_FIFO_LEN; } - // if new_size is SOC_UART_FIFO_LEN, _txBufferSize will be zero - just use the UART FIFO space - _txBufferSize = new_size - SOC_UART_FIFO_LEN; // for total correct report from "availableForWrite()" that matches a call to this function + // if new_size is higher than SOC_UART_FIFO_LEN, TX Ringbuffer will be active and it will be used to report back "availableToWrite()" + _txBufferSize = new_size; return new_size; } - From a79e2c7594d821ce7928c00bed5d7299a92604c7 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Fri, 1 Mar 2024 19:13:07 -0300 Subject: [PATCH 3/3] feat: adjust availableForWrite This change will make sure that if no TX Ringbuffer is used, it will return the UART FIFO available space. Otherwise, it will return the Ringbuffer available space, as defined in the Arduino especification. --- cores/esp32/esp32-hal-uart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/esp32-hal-uart.c b/cores/esp32/esp32-hal-uart.c index b22267fbc68..34cf17ee0ea 100644 --- a/cores/esp32/esp32-hal-uart.c +++ b/cores/esp32/esp32-hal-uart.c @@ -642,7 +642,7 @@ uint32_t uartAvailableForWrite(uart_t* uart) uint32_t available = uart_ll_get_txfifo_len(UART_LL_GET_HW(uart->num)); size_t txRingBufferAvailable = 0; if (ESP_OK == uart_get_tx_buffer_free_size(uart->num, &txRingBufferAvailable)) { - available += txRingBufferAvailable; + available = txRingBufferAvailable == 0 ? available : txRingBufferAvailable; } UART_MUTEX_UNLOCK(); return available;