Skip to content

Changes UART ISR to only trigger on RX FIFO Full and timeout #6930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Sep 15, 2022
35 changes: 25 additions & 10 deletions cores/esp32/HardwareSerial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ _txBufferSize(0),
_onReceiveCB(NULL),
_onReceiveErrorCB(NULL),
_onReceiveTimeout(true),
_rxTimeout(10),
_rxTimeout(2),
_eventTask(NULL)
#if !CONFIG_DISABLE_HAL_LOCKS
,_lock(NULL)
Expand Down Expand Up @@ -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)
Expand All @@ -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();
}
Expand All @@ -250,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 &&
Expand All @@ -258,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
}
}
}
}
Expand Down Expand Up @@ -366,9 +383,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();
}
Expand Down
11 changes: 9 additions & 2 deletions cores/esp32/HardwareSerial.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that having UART_NO_ERROR be the first item in the enum makes most sense. It will equal to 0, which conforms to most/all error uses in C/C++ (and ESP-IDF)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

} hardwareSerial_error_t;

typedef std::function<void(void)> OnReceiveCb;
Expand All @@ -81,6 +82,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)
Expand All @@ -91,7 +98,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);
Expand Down
46 changes: 44 additions & 2 deletions cores/esp32/esp32-hal-uart.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +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;


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));
Expand All @@ -172,13 +171,56 @@ 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));
}

UART_MUTEX_UNLOCK();

uartFlush(uart);
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) {
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) {
Expand Down
3 changes: 3 additions & 0 deletions cores/esp32/esp32-hal-uart.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ 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 uartSetFastReading(uart_t* uart);

void uartSetDebug(uart_t* uart);
int uartGetDebug();
Expand Down