From 118f815328b002fc7650c8e7f4990c9401a2dd98 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Thu, 30 Jun 2022 12:12:11 -0300 Subject: [PATCH 1/8] Changes UART ISR to only trigger on RX FIFO Full and timeout --- cores/esp32/HardwareSerial.cpp | 18 +++++++++++++++--- cores/esp32/HardwareSerial.h | 6 ++++++ cores/esp32/esp32-hal-uart.c | 31 +++++++++++++++++++++++++++++++ cores/esp32/esp32-hal-uart.h | 2 ++ 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index acc1bd2c88b..cd7e7fe8624 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -140,7 +140,7 @@ _txBufferSize(0), _onReceiveCB(NULL), _onReceiveErrorCB(NULL), _onReceiveTimeout(true), -_rxTimeout(10), +_rxTimeout(1), _eventTask(NULL) #if !CONFIG_DISABLE_HAL_LOCKS ,_lock(NULL) @@ -212,6 +212,18 @@ void HardwareSerial::onReceive(OnReceiveCb function, bool onlyOnTimeout) HSERIAL_MUTEX_UNLOCK(); } +// This function allow the user to define how many bytes will trigger an Interrupt that will copy RX FIFO to the internal RX Ringbuffer +// ISR will also move data from FIFO to RX Ringbuffer after a RX Timeout defined in HardwareSerial::setRxTimeout(uint8_t symbols_timeout) +// A low value of FIFO Full bytes will consume more CPU time within the ISR +// A high value of FIFO Full bytes will make the application wait longer to have byte available for the Stkech in a streaming scenario +// Both RX FIFO Full and RX Timeout may affect when onReceive() will be called +void HardwareSerial::setRxFIFOFull(uint8_t fifoBytes) +{ + HSERIAL_MUTEX_LOCK(); + uartSetRxFIFOFull(_uart, fifoBytes); // Set new timeout + HSERIAL_MUTEX_UNLOCK(); +} + // timout is calculates in time to receive UART symbols at the UART baudrate. // the estimation is about 11 bits per symbol (SERIAL_8N1) void HardwareSerial::setRxTimeout(uint8_t symbols_timeout) @@ -223,7 +235,7 @@ void HardwareSerial::setRxTimeout(uint8_t symbols_timeout) _rxTimeout = symbols_timeout; if (!symbols_timeout) _onReceiveTimeout = false; // only when RX timeout is disabled, we also must disable this flag - if(_uart != NULL) uart_set_rx_timeout(_uart_nr, _rxTimeout); // Set new timeout + uartSetRxTimeout(_uart, _rxTimeout); // Set new timeout HSERIAL_MUTEX_UNLOCK(); } @@ -367,7 +379,7 @@ void HardwareSerial::begin(unsigned long baud, uint32_t config, int8_t rxPin, in // Set UART RX timeout if (_uart != NULL) { - uart_set_rx_timeout(_uart_nr, _rxTimeout); + uartSetRxTimeout(_uart, _rxTimeout); } HSERIAL_MUTEX_UNLOCK(); diff --git a/cores/esp32/HardwareSerial.h b/cores/esp32/HardwareSerial.h index b7aafb68520..faf3ca6b405 100644 --- a/cores/esp32/HardwareSerial.h +++ b/cores/esp32/HardwareSerial.h @@ -81,6 +81,12 @@ class HardwareSerial: public Stream // For a baudrate of 9600, SERIAL_8N1 (10 bit symbol) and symbols_timeout = 3, the timeout would be 3 / (9600 / 10) = 3.125 ms void setRxTimeout(uint8_t symbols_timeout); + // setRxFIFOFull(uint8_t fifoBytes) will set the number of bytes that will trigger UART_INTR_RXFIFO_FULL interrupt and fill up RxRingBuffer + // This affects some functions such as Serial::available() and Serial.read() because, in a UART flow of receiving data, Serial internal + // RxRingBuffer will be filled only after these number of bytes arrive or a RX Timeout happens. + // This parameter can be set to 1 in order to receive byte by byte, but it will also consume more CPU time as the ISR will be activates often. + void setRxFIFOFull(uint8_t fifoBytes); + // onReceive will setup a callback that will be called whenever an UART interruption occurs (UART_INTR_RXFIFO_FULL or UART_INTR_RXFIFO_TOUT) // UART_INTR_RXFIFO_FULL interrupt triggers at UART_FULL_THRESH_DEFAULT bytes received (defined as 120 bytes by default in IDF) // UART_INTR_RXFIFO_TOUT interrupt triggers at UART_TOUT_THRESH_DEFAULT symbols passed without any reception (defined as 10 symbos by default in IDF) diff --git a/cores/esp32/esp32-hal-uart.c b/cores/esp32/esp32-hal-uart.c index 09a68b83d48..5b9c9f351a3 100644 --- a/cores/esp32/esp32-hal-uart.c +++ b/cores/esp32/esp32-hal-uart.c @@ -162,6 +162,12 @@ uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rx uart_config.rx_flow_ctrl_thresh = rxfifo_full_thrhd; uart_config.source_clk = UART_SCLK_APB; + uart_intr_config_t uart_intr = { + .intr_enable_mask = UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT, // only these IRQs - no BREAK, PARITY or OVERFLOW + .rx_timeout_thresh = 1, + .txfifo_empty_intr_thresh = 10, + .rxfifo_full_thresh = rxfifo_full_thrhd, + }; ESP_ERROR_CHECK(uart_driver_install(uart_nr, rx_buffer_size, tx_buffer_size, 20, &(uart->uart_event_queue), 0)); ESP_ERROR_CHECK(uart_param_config(uart_nr, &uart_config)); @@ -173,12 +179,37 @@ uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rx ESP_ERROR_CHECK(uart_set_line_inverse(uart_nr, UART_SIGNAL_TXD_INV | UART_SIGNAL_RXD_INV)); } + // override default RX IDF Driver Interrupt - no BREAK, PARITY or OVERFLOW + ESP_ERROR_CHECK(uart_intr_config(uart_nr, &uart_intr)); + UART_MUTEX_UNLOCK(); uartFlush(uart); return uart; } +void uartSetRxTimeout(uart_t* uart, uint8_t numSymbTimeout) +{ + if(uart == NULL) { + return; + } + + UART_MUTEX_LOCK(); + uart_set_rx_timeout(uart->num, numSymbTimeout); + UART_MUTEX_UNLOCK(); +} + +void uartSetRxFIFOFull(uart_t* uart, uint8_t numBytesFIFOFull) +{ + if(uart == NULL) { + return; + } + + UART_MUTEX_LOCK(); + uart_set_rx_full_threshold(uart->num, numBytesFIFOFull); + UART_MUTEX_UNLOCK(); +} + void uartEnd(uart_t* uart) { if(uart == NULL) { diff --git a/cores/esp32/esp32-hal-uart.h b/cores/esp32/esp32-hal-uart.h index ec7912c3d7a..47e310c84d1 100644 --- a/cores/esp32/esp32-hal-uart.h +++ b/cores/esp32/esp32-hal-uart.h @@ -82,6 +82,8 @@ void uartSetBaudRate(uart_t* uart, uint32_t baud_rate); uint32_t uartGetBaudRate(uart_t* uart); void uartSetRxInvert(uart_t* uart, bool invert); +void uartSetRxTimeout(uart_t* uart, uint8_t numSymbTimeout); +void uartSetRxFIFOFull(uart_t* uart, uint8_t numBytesFIFOFull); void uartSetDebug(uart_t* uart); int uartGetDebug(); From b9c86f880da0b840e277a3c4f9c27bff04175be0 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 6 Sep 2022 11:49:06 -0300 Subject: [PATCH 2/8] changes initial RX timeout --- cores/esp32/HardwareSerial.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index 66e49a07a39..ba4735811b1 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -140,7 +140,7 @@ _txBufferSize(0), _onReceiveCB(NULL), _onReceiveErrorCB(NULL), _onReceiveTimeout(true), -_rxTimeout(1), +_rxTimeout(2), _eventTask(NULL) #if !CONFIG_DISABLE_HAL_LOCKS ,_lock(NULL) From 4ea6ed6d31d178c4992028d08e77502b027d897b Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 6 Sep 2022 11:55:02 -0300 Subject: [PATCH 3/8] Eliminates extra testing for _uart != NULL --- cores/esp32/HardwareSerial.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index ba4735811b1..86fa7f998ce 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -378,9 +378,7 @@ void HardwareSerial::begin(unsigned long baud, uint32_t config, int8_t rxPin, in } // Set UART RX timeout - if (_uart != NULL) { - uartSetRxTimeout(_uart, _rxTimeout); - } + uartSetRxTimeout(_uart, _rxTimeout); HSERIAL_MUTEX_UNLOCK(); } From 72ce063c6173fbe84bc9cc291657f825dbf767c8 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 6 Sep 2022 12:05:35 -0300 Subject: [PATCH 4/8] reconfiguration with "uartSetFastReading()" --- cores/esp32/esp32-hal-uart.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/cores/esp32/esp32-hal-uart.c b/cores/esp32/esp32-hal-uart.c index 360882b96de..0f229747315 100644 --- a/cores/esp32/esp32-hal-uart.c +++ b/cores/esp32/esp32-hal-uart.c @@ -162,13 +162,6 @@ uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rx uart_config.rx_flow_ctrl_thresh = rxfifo_full_thrhd; uart_config.source_clk = UART_SCLK_APB; - uart_intr_config_t uart_intr = { - .intr_enable_mask = UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT, // only these IRQs - no BREAK, PARITY or OVERFLOW - .rx_timeout_thresh = 1, - .txfifo_empty_intr_thresh = 10, - .rxfifo_full_thresh = rxfifo_full_thrhd, - }; - ESP_ERROR_CHECK(uart_driver_install(uart_nr, rx_buffer_size, tx_buffer_size, 20, &(uart->uart_event_queue), 0)); ESP_ERROR_CHECK(uart_param_config(uart_nr, &uart_config)); ESP_ERROR_CHECK(uart_set_pin(uart_nr, txPin, rxPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE)); @@ -178,9 +171,6 @@ uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rx // invert signal for both Rx and Tx ESP_ERROR_CHECK(uart_set_line_inverse(uart_nr, UART_SIGNAL_TXD_INV | UART_SIGNAL_RXD_INV)); } - - // override default RX IDF Driver Interrupt - no BREAK, PARITY or OVERFLOW - ESP_ERROR_CHECK(uart_intr_config(uart_nr, &uart_intr)); UART_MUTEX_UNLOCK(); @@ -188,6 +178,27 @@ uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rx return uart; } +// This code is under testing - for now just keep it here +void uartSetFastReading(uart_t* uart) +{ + if(uart == NULL) { + return; + } + + UART_MUTEX_LOCK(); + // override default RX IDF Driver Interrupt - no BREAK, PARITY or OVERFLOW + uart_intr_config_t uart_intr = { + .intr_enable_mask = UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT, // only these IRQs - no BREAK, PARITY or OVERFLOW + .rx_timeout_thresh = 1, + .txfifo_empty_intr_thresh = 10, + .rxfifo_full_thresh = 2, + }; + + ESP_ERROR_CHECK(uart_intr_config(uart->num, &uart_intr)); + UART_MUTEX_UNLOCK(); +} + + void uartSetRxTimeout(uart_t* uart, uint8_t numSymbTimeout) { if(uart == NULL) { From f38bf7095ce3cf17acad2860b784c138df7769e9 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 6 Sep 2022 12:07:17 -0300 Subject: [PATCH 5/8] Adds new function "uartSetFastReading()" --- cores/esp32/esp32-hal-uart.h | 1 + 1 file changed, 1 insertion(+) diff --git a/cores/esp32/esp32-hal-uart.h b/cores/esp32/esp32-hal-uart.h index 47e310c84d1..86d56eb728d 100644 --- a/cores/esp32/esp32-hal-uart.h +++ b/cores/esp32/esp32-hal-uart.h @@ -84,6 +84,7 @@ uint32_t uartGetBaudRate(uart_t* uart); void uartSetRxInvert(uart_t* uart, bool invert); void uartSetRxTimeout(uart_t* uart, uint8_t numSymbTimeout); void uartSetRxFIFOFull(uart_t* uart, uint8_t numBytesFIFOFull); +void uartSetFastReading(uart_t* uart); void uartSetDebug(uart_t* uart); int uartGetDebug(); From 00b486e3d7c2704f4e0e3038828b6d7bc6c6c464 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Thu, 8 Sep 2022 11:39:01 -0300 Subject: [PATCH 6/8] changed default onReceive() behaviour --- cores/esp32/HardwareSerial.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HardwareSerial.h b/cores/esp32/HardwareSerial.h index faf3ca6b405..7189bb92559 100644 --- a/cores/esp32/HardwareSerial.h +++ b/cores/esp32/HardwareSerial.h @@ -97,7 +97,7 @@ class HardwareSerial: public Stream // false -- The callback will be called when FIFO reaches 120 bytes and also on RX Timeout. // The stream of incommig bytes will be "split" into blocks of 120 bytes on each callback. // This option avoid any sort of Rx Overflow, but leaves the UART packet reassembling work to the Application. - void onReceive(OnReceiveCb function, bool onlyOnTimeout = true); + void onReceive(OnReceiveCb function, bool onlyOnTimeout = false); // onReceive will be called on error events (see hardwareSerial_error_t) void onReceiveError(OnReceiveErrorCb function); From ca20ed3bbf4304c33977cc7277f0b9a83b0209a7 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Fri, 9 Sep 2022 10:13:08 -0300 Subject: [PATCH 7/8] forces User callback in case of error --- cores/esp32/HardwareSerial.cpp | 15 ++++++++++----- cores/esp32/HardwareSerial.h | 3 ++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index 86fa7f998ce..0fc16f4cf76 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -262,6 +262,7 @@ void HardwareSerial::_uartEventTask(void *args) for(;;) { //Waiting for UART event. if(xQueueReceive(uartEventQueue, (void * )&event, (portTickType)portMAX_DELAY)) { + hardwareSerial_error_t currentErr = UART_NO_ERROR; switch(event.type) { case UART_DATA: if(uart->_onReceiveCB && uart->available() > 0 && @@ -270,28 +271,32 @@ void HardwareSerial::_uartEventTask(void *args) break; case UART_FIFO_OVF: log_w("UART%d FIFO Overflow. Consider adding Hardware Flow Control to your Application.", uart->_uart_nr); - if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(UART_FIFO_OVF_ERROR); + currentErr = UART_FIFO_OVF_ERROR; break; case UART_BUFFER_FULL: log_w("UART%d Buffer Full. Consider increasing your buffer size of your Application.", uart->_uart_nr); - if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(UART_BUFFER_FULL_ERROR); + currentErr = UART_BUFFER_FULL_ERROR; break; case UART_BREAK: log_w("UART%d RX break.", uart->_uart_nr); - if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(UART_BREAK_ERROR); + currentErr = UART_BREAK_ERROR; break; case UART_PARITY_ERR: log_w("UART%d parity error.", uart->_uart_nr); - if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(UART_PARITY_ERROR); + currentErr = UART_PARITY_ERROR; break; case UART_FRAME_ERR: log_w("UART%d frame error.", uart->_uart_nr); - if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(UART_FRAME_ERROR); + currentErr = UART_FRAME_ERROR; break; default: log_w("UART%d unknown event type %d.", uart->_uart_nr, event.type); break; } + if (currentErr != UART_NO_ERROR) { + if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(currentErr); + if(uart->_onReceiveCB && uart->available() > 0) uart->_onReceiveCB(); // forces User Callback too + } } } } diff --git a/cores/esp32/HardwareSerial.h b/cores/esp32/HardwareSerial.h index 7189bb92559..c548adfe8a5 100644 --- a/cores/esp32/HardwareSerial.h +++ b/cores/esp32/HardwareSerial.h @@ -61,7 +61,8 @@ typedef enum { UART_BUFFER_FULL_ERROR, UART_FIFO_OVF_ERROR, UART_FRAME_ERROR, - UART_PARITY_ERROR + UART_PARITY_ERROR, + UART_NO_ERROR } hardwareSerial_error_t; typedef std::function OnReceiveCb; From 7d3571fe80b1e8dd13478447668d01d15c345daf Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Thu, 15 Sep 2022 08:11:29 -0300 Subject: [PATCH 8/8] Error Code Order Set NO_ERROR as first error code, same as ESP_OK = 0 --- cores/esp32/HardwareSerial.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp32/HardwareSerial.h b/cores/esp32/HardwareSerial.h index c548adfe8a5..c6f36f792d9 100644 --- a/cores/esp32/HardwareSerial.h +++ b/cores/esp32/HardwareSerial.h @@ -57,12 +57,12 @@ #include "freertos/semphr.h" typedef enum { + UART_NO_ERROR, UART_BREAK_ERROR, UART_BUFFER_FULL_ERROR, UART_FIFO_OVF_ERROR, UART_FRAME_ERROR, - UART_PARITY_ERROR, - UART_NO_ERROR + UART_PARITY_ERROR } hardwareSerial_error_t; typedef std::function OnReceiveCb;