From a1f8cfbbd754188dc8d1cdafb2f0336853245fce Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 1 Mar 2022 19:07:51 -0300 Subject: [PATCH 1/2] Adds C++ std::function to Serial.onReceive() --- cores/esp32/HardwareSerial.cpp | 156 +++++++++++++++++++++++++++++++-- cores/esp32/HardwareSerial.h | 37 ++++++-- cores/esp32/esp32-hal-uart.c | 93 ++++---------------- cores/esp32/esp32-hal-uart.h | 5 +- 4 files changed, 201 insertions(+), 90 deletions(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index 313d0f784ef..9690f99989d 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -7,6 +7,7 @@ #include "HardwareSerial.h" #include "soc/soc_caps.h" #include "driver/uart.h" +#include "freertos/queue.h" #ifndef SOC_RX0 #if CONFIG_IDF_TARGET_ESP32 @@ -115,7 +116,128 @@ void serialEventRun(void) } #endif -HardwareSerial::HardwareSerial(int uart_nr) : _uart_nr(uart_nr), _uart(NULL), _rxBufferSize(256) {} +#if !CONFIG_DISABLE_HAL_LOCKS +#define HSERIAL_MUTEX_LOCK() do {} while (xSemaphoreTake(_lock, portMAX_DELAY) != pdPASS) +#define HSERIAL_MUTEX_UNLOCK() xSemaphoreGive(_lock) +#endif + +HardwareSerial::HardwareSerial(int uart_nr) : +_uart_nr(uart_nr), +_uart(NULL), +_rxBufferSize(256), +_onReceiveCB(NULL), +_onReceiveErrorCB(NULL), +_eventTask(NULL) +#if !CONFIG_DISABLE_HAL_LOCKS + ,_lock(NULL) +#endif +{ +#if !CONFIG_DISABLE_HAL_LOCKS + if(_lock == NULL){ + _lock = xSemaphoreCreateMutex(); + if(_lock == NULL){ + log_e("xSemaphoreCreateMutex failed"); + return; + } + } +#endif +} + +HardwareSerial::~HardwareSerial() +{ + end(); +#if !CONFIG_DISABLE_HAL_LOCKS + if(_lock != NULL){ + vSemaphoreDelete(_lock); + } +#endif +} + + +void HardwareSerial::_createEventTask(void *args) +{ + // Creating UART event Task + xTaskCreate(_uartEventTask, "uart_event_task", 2048, this, configMAX_PRIORITIES - 1, &_eventTask); + if (_eventTask == NULL) { + log_e(" -- UART%d Event Task not Created!", _uart_nr); + } +} + +void HardwareSerial::_destroyEventTask(void) +{ + if (_eventTask != NULL) { + vTaskDelete(_eventTask); + _eventTask = NULL; + } +} + +void HardwareSerial::onReceiveError(OnReceiveErrorCb function) +{ + HSERIAL_MUTEX_LOCK(); + // function may be NULL to cancel onReceive() from its respective task + _onReceiveErrorCB = function; + // this can be called after Serial.begin(), therefore it shall create the event task + if (function != NULL && _uart != NULL && _eventTask == NULL) { + _createEventTask(this); + } + HSERIAL_MUTEX_UNLOCK(); +} + +void HardwareSerial::onReceive(OnReceiveCb function) +{ + HSERIAL_MUTEX_LOCK(); + // function may be NULL to cancel onReceive() from its respective task + _onReceiveCB = function; + // this can be called after Serial.begin(), therefore it shall create the event task + if (function != NULL && _uart != NULL && _eventTask == NULL) { + _createEventTask(this); + } + HSERIAL_MUTEX_UNLOCK(); +} + +void HardwareSerial::_uartEventTask(void *args) +{ + HardwareSerial *uart = (HardwareSerial *)args; + uart_event_t event; + QueueHandle_t uartEventQueue = NULL; + uartGetEventQueue(uart->_uart, &uartEventQueue); + if (uartEventQueue != NULL) { + for(;;) { + //Waiting for UART event. + if(xQueueReceive(uartEventQueue, (void * )&event, (portTickType)portMAX_DELAY)) { + switch(event.type) { + case UART_DATA: + if(uart->_onReceiveCB && uart->available() > 0) uart->_onReceiveCB(); + 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); + break; + case UART_BUFFER_FULL: + log_w("UART%d Buffer Full. Consider encreasing your buffer size of your Application.", uart->_uart_nr); + if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(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); + break; + case UART_PARITY_ERR: + log_w("UART%d parity error.", uart->_uart_nr); + if(uart->_onReceiveErrorCB) uart->_onReceiveErrorCB(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); + break; + default: + log_w("UART%d unknown event type %d.", uart->_uart_nr, event.type); + break; + } + } + } + } + vTaskDelete(NULL); +} void HardwareSerial::begin(unsigned long baud, uint32_t config, int8_t rxPin, int8_t txPin, bool invert, unsigned long timeout_ms, uint8_t rxfifo_full_thrhd) { @@ -124,6 +246,14 @@ void HardwareSerial::begin(unsigned long baud, uint32_t config, int8_t rxPin, in return; } +#if !CONFIG_DISABLE_HAL_LOCKS + if(_lock == NULL){ + log_e("MUTEX Lock failed. Can't begin."); + return; + } +#endif + + HSERIAL_MUTEX_LOCK(); // First Time or after end() --> set default Pins if (!uartIsDriverInstalled(_uart)) { switch (_uart_nr) { @@ -176,11 +306,12 @@ void HardwareSerial::begin(unsigned long baud, uint32_t config, int8_t rxPin, in _uart = NULL; } } -} - -void HardwareSerial::onReceive(void(*function)(void)) -{ - uartOnReceive(_uart, function); + // create a task to deal with Serial Events when, for example, calling begin() twice to change the baudrate, + // or when setting the callback before calling begin() + if (_uart != NULL && (_onReceiveCB != NULL || _onReceiveErrorCB != NULL) && _eventTask == NULL) { + _createEventTask(this); + } + HSERIAL_MUTEX_UNLOCK(); } void HardwareSerial::updateBaudRate(unsigned long baud) @@ -188,14 +319,21 @@ void HardwareSerial::updateBaudRate(unsigned long baud) uartSetBaudRate(_uart, baud); } -void HardwareSerial::end(bool turnOffDebug) +void HardwareSerial::end(bool fullyTerminate) { - if(turnOffDebug && uartGetDebug() == _uart_nr) { - uartSetDebug(0); + // default Serial.end() will completely disable HardwareSerial, + // including any tasks or debug message channel (log_x()) - but not for IDF log messages! + if(fullyTerminate) { + _onReceiveCB = NULL; + _onReceiveErrorCB = NULL; + if (uartGetDebug() == _uart_nr) { + uartSetDebug(0); + } } delay(10); uartEnd(_uart); _uart = 0; + _destroyEventTask(); } void HardwareSerial::setDebugOutput(bool en) diff --git a/cores/esp32/HardwareSerial.h b/cores/esp32/HardwareSerial.h index b1e21bc6b27..7a25b64e37f 100644 --- a/cores/esp32/HardwareSerial.h +++ b/cores/esp32/HardwareSerial.h @@ -46,23 +46,40 @@ #define HardwareSerial_h #include - +#include #include "Stream.h" #include "esp32-hal.h" #include "soc/soc_caps.h" #include "HWCDC.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "freertos/semphr.h" + +typedef enum { + UART_BREAK_ERROR, + UART_BUFFER_FULL_ERROR, + UART_FIFO_OVF_ERROR, + UART_FRAME_ERROR, + UART_PARITY_ERROR +} hardwareSerial_error_t; + +typedef std::function OnReceiveCb; +typedef std::function OnReceiveErrorCb; + class HardwareSerial: public Stream { public: HardwareSerial(int uart_nr); + ~HardwareSerial(); // onReceive will setup a callback for whenever UART data is received - // it will work as UART Rx interrupt - void onReceive(void(*function)(void)); - + // it will work as UART Rx interrupt -- Using C++ 11 std::fuction + void onReceive(OnReceiveCb function); + void onReceiveError(OnReceiveErrorCb function); + void begin(unsigned long baud, uint32_t config=SERIAL_8N1, int8_t rxPin=-1, int8_t txPin=-1, bool invert=false, unsigned long timeout_ms = 20000UL, uint8_t rxfifo_full_thrhd = 112); - void end(bool turnOffDebug = true); + void end(bool fullyTerminate = true); void updateBaudRate(unsigned long baud); int available(void); int availableForWrite(void); @@ -120,6 +137,16 @@ class HardwareSerial: public Stream int _uart_nr; uart_t* _uart; size_t _rxBufferSize; + OnReceiveCb _onReceiveCB; + OnReceiveErrorCb _onReceiveErrorCB; + TaskHandle_t _eventTask; + + void _createEventTask(void *args); + void _destroyEventTask(void); + static void _uartEventTask(void *args); +#if !CONFIG_DISABLE_HAL_LOCKS + SemaphoreHandle_t _lock; +#endif }; extern void serialEventRun(void) __attribute__((weak)); diff --git a/cores/esp32/esp32-hal-uart.c b/cores/esp32/esp32-hal-uart.c index 157c3640830..5983795c262 100644 --- a/cores/esp32/esp32-hal-uart.c +++ b/cores/esp32/esp32-hal-uart.c @@ -34,9 +34,7 @@ struct uart_struct_t { uint8_t num; bool has_peek; uint8_t peek_byte; - QueueHandle_t uart_event_queue; - void (*onReceive)(void); - TaskHandle_t envent_task; + QueueHandle_t uart_event_queue; // export it by some uartGetEventQueue() function }; #if CONFIG_DISABLE_HAL_LOCKS @@ -45,12 +43,12 @@ struct uart_struct_t { #define UART_MUTEX_UNLOCK() static uart_t _uart_bus_array[] = { - {0, false, 0, NULL, NULL, NULL}, + {0, false, 0, NULL}, #if SOC_UART_NUM > 1 - {1, false, 0, NULL, NULL, NULL}, + {1, false, 0, NULL}, #endif #if SOC_UART_NUM > 2 - {2, false, 0, NULL, NULL, NULL}, + {2, false, 0, NULL}, #endif }; @@ -60,12 +58,12 @@ static uart_t _uart_bus_array[] = { #define UART_MUTEX_UNLOCK() xSemaphoreGive(uart->lock) static uart_t _uart_bus_array[] = { - {NULL, 0, false, 0, NULL, NULL, NULL}, + {NULL, 0, false, 0, NULL}, #if SOC_UART_NUM > 1 - {NULL, 1, false, 0, NULL, NULL, NULL}, + {NULL, 1, false, 0, NULL}, #endif #if SOC_UART_NUM > 2 - {NULL, 2, false, 0, NULL, NULL, NULL}, + {NULL, 2, false, 0, NULL}, #endif }; @@ -84,69 +82,25 @@ uint32_t _get_effective_baudrate(uint32_t baudrate) } } - -void uartOnReceive(uart_t* uart, void(*function)(void)) +// Routines that take care of UART events will be in the HardwareSerial Class code +void uartGetEventQueue(uart_t* uart, QueueHandle_t *q) { - if(uart == NULL || function == NULL) { + // passing back NULL for the Queue pointer when UART is not initialized yet + *q = NULL; + if(uart == NULL) { return; } - UART_MUTEX_LOCK(); - uart->onReceive = function; - UART_MUTEX_UNLOCK(); -} - - -static void uart_event_task(void *args) -{ - uart_t* uart = (uart_t *)args; - uart_event_t event; - for(;;) { - //Waiting for UART event. - if(xQueueReceive(uart->uart_event_queue, (void * )&event, (portTickType)portMAX_DELAY)) { - switch(event.type) { - //Event of UART receving data - case UART_DATA: - if(uart->onReceive) uart->onReceive(); - break; - //Event of HW FIFO overflow detected - case UART_FIFO_OVF: - log_w("UART%d FIFO Overflow. Flushing data. Consider adding Flow Control to your Application.", uart->num); - uart_flush_input(uart->num); - xQueueReset(uart->uart_event_queue); - break; - //Event of UART ring buffer full - case UART_BUFFER_FULL: - log_w("UART%d Buffer Full. Flushing data. Consider encreasing your buffer size of your Application.", uart->num); - uart_flush_input(uart->num); - xQueueReset(uart->uart_event_queue); - break; - //Event of UART RX break detected - case UART_BREAK: - log_w("UART%d RX break.", uart->num); - break; - //Event of UART parity check error - case UART_PARITY_ERR: - log_w("UART%d parity error.", uart->num); - break; - //Event of UART frame error - case UART_FRAME_ERR: - log_w("UART%d frame error.", uart->num); - break; - //Others - default: - log_w("UART%d unknown event type %d.", uart->num, event.type); - break; - } - } - } - vTaskDelete(NULL); + // MUTEX will lock for the case that another task is executing begin() on the same UART and has not finished + //UART_MUTEX_LOCK(); + *q = uart->uart_event_queue; + //UART_MUTEX_UNLOCK(); + return; } - bool uartIsDriverInstalled(uart_t* uart) { if(uart == NULL) { - return 0; + return false; } if (uart_is_driver_installed(uart->num)) { @@ -222,12 +176,6 @@ 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)); } - // Creating UART event Task - xTaskCreate(uart_event_task, "uart_event_task", 2048, uart, configMAX_PRIORITIES - 1, &(uart->envent_task)); - if (!uart->envent_task) { - log_e(" -- UART%d Event Task not Created!", uart_nr); - } - UART_MUTEX_UNLOCK(); uartFlush(uart); @@ -242,11 +190,6 @@ void uartEnd(uart_t* uart) UART_MUTEX_LOCK(); uart_driver_delete(uart->num); - if (uart->envent_task) { - vTaskDelete(uart->envent_task); - uart->envent_task = NULL; - uart->onReceive = NULL; - } UART_MUTEX_UNLOCK(); } diff --git a/cores/esp32/esp32-hal-uart.h b/cores/esp32/esp32-hal-uart.h index 22a3a74c86a..25c17ea6ab6 100644 --- a/cores/esp32/esp32-hal-uart.h +++ b/cores/esp32/esp32-hal-uart.h @@ -22,6 +22,8 @@ extern "C" { #include #include #include +#include "freertos/FreeRTOS.h" +#include "freertos/queue.h" #define SERIAL_5N1 0x8000010 #define SERIAL_6N1 0x8000014 @@ -62,7 +64,8 @@ typedef struct uart_struct_t uart_t; uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rxPin, int8_t txPin, uint16_t queueLen, bool inverted, uint8_t rxfifo_full_thrhd); void uartEnd(uart_t* uart); -void uartOnReceive(uart_t* uart, void(*function)(void)); +// This is used to retrieve the Event Queue pointer from a UART IDF Driver in order to allow user to deal with its events +void uartGetEventQueue(uart_t* uart, QueueHandle_t *q); uint32_t uartAvailable(uart_t* uart); uint32_t uartAvailableForWrite(uart_t* uart); From 36221d1f11c3dde6c9cffd9ab06e3be447de02af Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 2 Mar 2022 08:59:41 -0300 Subject: [PATCH 2/2] fixes LOCK macro when disabled --- cores/esp32/HardwareSerial.cpp | 3 +++ cores/esp32/esp32-hal-uart.c | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp index 9690f99989d..61c41cebfb0 100644 --- a/cores/esp32/HardwareSerial.cpp +++ b/cores/esp32/HardwareSerial.cpp @@ -119,6 +119,9 @@ void serialEventRun(void) #if !CONFIG_DISABLE_HAL_LOCKS #define HSERIAL_MUTEX_LOCK() do {} while (xSemaphoreTake(_lock, portMAX_DELAY) != pdPASS) #define HSERIAL_MUTEX_UNLOCK() xSemaphoreGive(_lock) +#else +#define HSERIAL_MUTEX_LOCK() +#define HSERIAL_MUTEX_UNLOCK() #endif HardwareSerial::HardwareSerial(int uart_nr) : diff --git a/cores/esp32/esp32-hal-uart.c b/cores/esp32/esp32-hal-uart.c index 5983795c262..adc5d8c2991 100644 --- a/cores/esp32/esp32-hal-uart.c +++ b/cores/esp32/esp32-hal-uart.c @@ -90,10 +90,7 @@ void uartGetEventQueue(uart_t* uart, QueueHandle_t *q) if(uart == NULL) { return; } - // MUTEX will lock for the case that another task is executing begin() on the same UART and has not finished - //UART_MUTEX_LOCK(); *q = uart->uart_event_queue; - //UART_MUTEX_UNLOCK(); return; }