Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 60395ed

Browse filesBrowse files
SuGliderSuGlider
authored
UART: fixes begin() after a previous begin() :: deleting previous RX/TX buffers and its data (espressif#9095)
* UART: fixes begin() * Typo: fixes typos and some testing left over * feat: fixes end() * feat: adjust internal baurate * feat: Changes CI [HardwareSerial] [HardwareSerial]: Changes CI to match new HardwareSerial begin() and end() * feat: fixes auto_baudrate_test [uart]: fixes end(void) instead of end(bool) * feat: adjust copyright year [fix]: adjust commentary of the copyright year
1 parent 50f436c commit 60395ed
Copy full SHA for 60395ed

File tree

Expand file treeCollapse file tree

5 files changed

+191
-51
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+191
-51
lines changed

‎cores/esp32/HardwareSerial.cpp

Copy file name to clipboardExpand all lines: cores/esp32/HardwareSerial.cpp
+24-19Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ _eventTask(NULL)
114114

115115
HardwareSerial::~HardwareSerial()
116116
{
117-
end(true); // explicit Full UART termination
117+
end(); // explicit Full UART termination
118118
#if !CONFIG_DISABLE_HAL_LOCKS
119119
if(_lock != NULL){
120120
vSemaphoreDelete(_lock);
@@ -329,16 +329,22 @@ void HardwareSerial::begin(unsigned long baud, uint32_t config, int8_t rxPin, in
329329
// map logical pins to GPIO numbers
330330
rxPin = digitalPinToGPIONumber(rxPin);
331331
txPin = digitalPinToGPIONumber(txPin);
332+
// IDF UART driver keeps Pin setting on restarting. Negative Pin number will keep it unmodified.
333+
// it will detach previous UART attached pins
332334

333-
if(_uart) {
334-
// in this case it is a begin() over a previous begin() - maybe to change baud rate
335-
// thus do not disable debug output
336-
end(false); // disables IDF UART driver and UART event Task + sets _uart to NULL
335+
// indicates that uartbegin() has to initilize a new IDF driver
336+
if (_testUartBegin(_uart_nr, baud ? baud : 9600, config, rxPin, txPin, _rxBufferSize, _txBufferSize, invert, rxfifo_full_thrhd)) {
337+
_destroyEventTask(); // when IDF uart driver must be restarted, _eventTask must finish too
337338
}
338339

339340
// IDF UART driver keeps Pin setting on restarting. Negative Pin number will keep it unmodified.
340341
// it will detach previous UART attached pins
341342
_uart = uartBegin(_uart_nr, baud ? baud : 9600, config, rxPin, txPin, _rxBufferSize, _txBufferSize, invert, rxfifo_full_thrhd);
343+
if (_uart == NULL) {
344+
log_e("UART driver failed to start. Please check the logs.");
345+
HSERIAL_MUTEX_UNLOCK();
346+
return;
347+
}
342348
if (!baud) {
343349
// using baud rate as zero, forces it to try to detect the current baud rate in place
344350
uartStartDetectBaudrate(_uart);
@@ -348,11 +354,14 @@ void HardwareSerial::begin(unsigned long baud, uint32_t config, int8_t rxPin, in
348354
yield();
349355
}
350356

351-
end(false); // disables IDF UART driver and UART event Task + sets _uart to NULL
352-
353357
if(detectedBaudRate) {
354358
delay(100); // Give some time...
355359
_uart = uartBegin(_uart_nr, detectedBaudRate, config, rxPin, txPin, _rxBufferSize, _txBufferSize, invert, rxfifo_full_thrhd);
360+
if (_uart == NULL) {
361+
log_e("UART driver failed to start. Please check the logs.");
362+
HSERIAL_MUTEX_UNLOCK();
363+
return;
364+
}
356365
} else {
357366
log_e("Could not detect baudrate. Serial data at the port must be present within the timeout for detection to be possible");
358367
_uart = NULL;
@@ -389,22 +398,17 @@ void HardwareSerial::updateBaudRate(unsigned long baud)
389398
uartSetBaudRate(_uart, baud);
390399
}
391400

392-
void HardwareSerial::end(bool fullyTerminate)
401+
void HardwareSerial::end()
393402
{
394403
// default Serial.end() will completely disable HardwareSerial,
395404
// including any tasks or debug message channel (log_x()) - but not for IDF log messages!
396-
if(fullyTerminate) {
397-
_onReceiveCB = NULL;
398-
_onReceiveErrorCB = NULL;
399-
if (uartGetDebug() == _uart_nr) {
400-
uartSetDebug(0);
401-
}
402-
_rxFIFOFull = 0;
403-
uartEnd(_uart_nr); // fully detach all pins and delete the UART driver
404-
} else {
405-
// do not invalidate callbacks, detach pins, invalidate DBG output
406-
uart_driver_delete(_uart_nr);
405+
_onReceiveCB = NULL;
406+
_onReceiveErrorCB = NULL;
407+
if (uartGetDebug() == _uart_nr) {
408+
uartSetDebug(0);
407409
}
410+
_rxFIFOFull = 0;
411+
uartEnd(_uart_nr); // fully detach all pins and delete the UART driver
408412
_destroyEventTask(); // when IDF uart driver is deleted, _eventTask must finish too
409413
_uart = NULL;
410414
}
@@ -564,3 +568,4 @@ size_t HardwareSerial::setTxBufferSize(size_t new_size) {
564568
_txBufferSize = new_size;
565569
return _txBufferSize;
566570
}
571+

‎cores/esp32/HardwareSerial.h

Copy file name to clipboardExpand all lines: cores/esp32/HardwareSerial.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ class HardwareSerial: public Stream
252252
// invert will invert RX/TX polarity
253253
// rxfifo_full_thrhd if the UART Flow Control Threshold in the UART FIFO (max 127)
254254
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);
255-
void end(bool fullyTerminate = true);
255+
void end(void);
256256
void updateBaudRate(unsigned long baud);
257257
int available(void);
258258
int availableForWrite(void);

‎cores/esp32/esp32-hal-uart.c

Copy file name to clipboardExpand all lines: cores/esp32/esp32-hal-uart.c
+148-27Lines changed: 148 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2015-2023 Espressif Systems (Shanghai) PTE LTD
1+
// Copyright 2015-2024 Espressif Systems (Shanghai) PTE LTD
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -33,19 +33,25 @@
3333
#include "hal/gpio_hal.h"
3434
#include "esp_rom_gpio.h"
3535

36-
static int s_uart_debug_nr = 0;
36+
static int s_uart_debug_nr = 0; // UART number for debug output
3737

3838
struct uart_struct_t {
3939

4040
#if !CONFIG_DISABLE_HAL_LOCKS
41-
SemaphoreHandle_t lock;
41+
SemaphoreHandle_t lock; // UART lock
4242
#endif
4343

44-
uint8_t num;
45-
bool has_peek;
46-
uint8_t peek_byte;
47-
QueueHandle_t uart_event_queue; // export it by some uartGetEventQueue() function
48-
int8_t _rxPin, _txPin, _ctsPin, _rtsPin; // UART GPIOs
44+
uint8_t num; // UART number for IDF driver API
45+
bool has_peek; // flag to indicate that there is a peek byte pending to be read
46+
uint8_t peek_byte; // peek byte that has been read but not consumed
47+
QueueHandle_t uart_event_queue; // export it by some uartGetEventQueue() function
48+
// configuration data:: Arduino API tipical data
49+
int8_t _rxPin, _txPin, _ctsPin, _rtsPin; // UART GPIOs
50+
uint32_t _baudrate, _config; // UART baudrate and config
51+
// UART ESP32 specific data
52+
uint16_t _rx_buffer_size, _tx_buffer_size; // UART RX and TX buffer sizes
53+
bool _inverted; // UART inverted signal
54+
uint8_t _rxfifo_full_thrhd; // UART RX FIFO full threshold
4955
};
5056

5157
#if CONFIG_DISABLE_HAL_LOCKS
@@ -54,12 +60,12 @@ struct uart_struct_t {
5460
#define UART_MUTEX_UNLOCK()
5561

5662
static uart_t _uart_bus_array[] = {
57-
{0, false, 0, NULL, -1, -1, -1, -1},
63+
{0, false, 0, NULL, -1, -1, -1, -1, 0, 0, 0, 0, false, 0},
5864
#if SOC_UART_NUM > 1
59-
{1, false, 0, NULL, -1, -1, -1, -1},
65+
{1, false, 0, NULL, -1, -1, -1, -1, 0, 0, 0, 0, false, 0},
6066
#endif
6167
#if SOC_UART_NUM > 2
62-
{2, false, 0, NULL, -1, -1, -1, -1},
68+
{2, false, 0, NULL, -1, -1, -1, -1, 0, 0, 0, 0, false, 0},
6369
#endif
6470
};
6571

@@ -69,12 +75,12 @@ static uart_t _uart_bus_array[] = {
6975
#define UART_MUTEX_UNLOCK() if(uart->lock != NULL) xSemaphoreGive(uart->lock)
7076

7177
static uart_t _uart_bus_array[] = {
72-
{NULL, 0, false, 0, NULL, -1, -1, -1, -1},
78+
{NULL, 0, false, 0, NULL, -1, -1, -1, -1, 0, 0, 0, 0, false, 0},
7379
#if SOC_UART_NUM > 1
74-
{NULL, 1, false, 0, NULL, -1, -1, -1, -1},
80+
{NULL, 1, false, 0, NULL, -1, -1, -1, -1, 0, 0, 0, 0, false, 0},
7581
#endif
7682
#if SOC_UART_NUM > 2
77-
{NULL, 2, false, 0, NULL, -1, -1, -1, -1},
83+
{NULL, 2, false, 0, NULL, -1, -1, -1, -1, 0, 0, 0, 0, false, 0},
7884
#endif
7985
};
8086

@@ -97,7 +103,12 @@ static bool _uartDetachPins(uint8_t uart_num, int8_t rxPin, int8_t txPin, int8_t
97103
// detaches pins and sets Peripheral Manager and UART information
98104
if (rxPin >= 0 && uart->_rxPin == rxPin && perimanGetPinBusType(rxPin) == ESP32_BUS_TYPE_UART_RX) {
99105
gpio_hal_iomux_func_sel(GPIO_PIN_MUX_REG[rxPin], PIN_FUNC_GPIO);
100-
esp_rom_gpio_connect_in_signal(GPIO_FUNC_IN_LOW, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RX_PIN_IDX), false);
106+
// avoids causing BREAK in the UART line
107+
if (uart->_inverted) {
108+
esp_rom_gpio_connect_in_signal(GPIO_FUNC_IN_LOW, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RX_PIN_IDX), false);
109+
} else {
110+
esp_rom_gpio_connect_in_signal(GPIO_FUNC_IN_HIGH, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RX_PIN_IDX), false);
111+
}
101112
uart->_rxPin = -1; // -1 means unassigned/detached
102113
if (!perimanClearPinBus(rxPin)) {
103114
retCode = false;
@@ -355,28 +366,121 @@ bool uartSetHwFlowCtrlMode(uart_t *uart, uart_hw_flowcontrol_t mode, uint8_t thr
355366
return retCode;
356367
}
357368

358-
uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rxPin, int8_t txPin, uint16_t rx_buffer_size, uint16_t tx_buffer_size, bool inverted, uint8_t rxfifo_full_thrhd)
369+
// This helper function will return true if a new IDF UART driver needs to be restarted and false if the current one can continue its execution
370+
bool _testUartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rxPin, int8_t txPin, uint16_t rx_buffer_size, uint16_t tx_buffer_size, bool inverted, uint8_t rxfifo_full_thrhd)
359371
{
360372
if(uart_nr >= SOC_UART_NUM) {
361-
return NULL;
373+
return false; // no new driver has to be installed
362374
}
363375
uart_t* uart = &_uart_bus_array[uart_nr];
364-
376+
// verify if is necessary to restart the UART driver
365377
if (uart_is_driver_installed(uart_nr)) {
366-
uartEnd(uart_nr);
378+
// some parameters can't be changed unless we end the UART driver
379+
if ( uart->_rx_buffer_size != rx_buffer_size || uart->_tx_buffer_size != tx_buffer_size || uart->_inverted != inverted || uart->_rxfifo_full_thrhd != rxfifo_full_thrhd) {
380+
return true; // the current IDF UART driver must be terminated and a new driver shall be installed
381+
} else {
382+
return false; // The current IDF UART driver can continue its execution
383+
}
384+
} else {
385+
return true; // no IDF UART driver is running and a new driver shall be installed
367386
}
387+
}
388+
389+
uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rxPin, int8_t txPin, uint16_t rx_buffer_size, uint16_t tx_buffer_size, bool inverted, uint8_t rxfifo_full_thrhd)
390+
{
391+
if(uart_nr >= SOC_UART_NUM) {
392+
log_e("UART number is invalid, please use number from 0 to %u", SOC_UART_NUM - 1);
393+
return NULL; // no new driver was installed
394+
}
395+
uart_t* uart = &_uart_bus_array[uart_nr];
396+
log_v("UART%d baud(%ld) Mode(%x) rxPin(%d) txPin(%d)", uart_nr, baudrate, config, rxPin, txPin);
368397

369398
#if !CONFIG_DISABLE_HAL_LOCKS
370399
if(uart->lock == NULL) {
371400
uart->lock = xSemaphoreCreateMutex();
372401
if(uart->lock == NULL) {
373402
log_e("HAL LOCK error.");
374-
return NULL;
403+
return NULL; // no new driver was installed
375404
}
376405
}
377406
#endif
378-
UART_MUTEX_LOCK();
379407

408+
if (uart_is_driver_installed(uart_nr)) {
409+
log_v("UART%d Driver already installed.", uart_nr);
410+
// some parameters can't be changed unless we end the UART driver
411+
if ( uart->_rx_buffer_size != rx_buffer_size || uart->_tx_buffer_size != tx_buffer_size || uart->_inverted != inverted || uart->_rxfifo_full_thrhd != rxfifo_full_thrhd) {
412+
log_v("UART%d changing buffer sizes or inverted signal or rxfifo_full_thrhd. IDF driver will be restarted", uart_nr);
413+
uartEnd(uart_nr);
414+
} else {
415+
bool retCode = true;
416+
UART_MUTEX_LOCK();
417+
//User may just want to change some parameters, such as baudrate, data length, parity, stop bits or pins
418+
if (uart->_baudrate != baudrate) {
419+
if (ESP_OK != uart_set_baudrate(uart_nr, baudrate)) {
420+
log_e("UART%d changing baudrate failed.", uart_nr);
421+
retCode = false;
422+
} else {
423+
log_v("UART%d changed baudrate to %d", uart_nr, baudrate);
424+
uart->_baudrate = baudrate;
425+
}
426+
}
427+
uart_word_length_t data_bits = (config & 0xc) >> 2;
428+
uart_parity_t parity = config & 0x3;
429+
uart_stop_bits_t stop_bits = (config & 0x30) >> 4;
430+
if (retCode && (uart->_config & 0xc) >> 2 != data_bits) {
431+
if (ESP_OK != uart_set_word_length(uart_nr, data_bits)) {
432+
log_e("UART%d changing data length failed.", uart_nr);
433+
retCode = false;
434+
} else {
435+
log_v("UART%d changed data length to %d", uart_nr, data_bits + 5);
436+
}
437+
}
438+
if (retCode && (uart->_config & 0x3) != parity) {
439+
if (ESP_OK != uart_set_parity(uart_nr, parity)) {
440+
log_e("UART%d changing parity failed.", uart_nr);
441+
retCode = false;
442+
} else {
443+
log_v("UART%d changed parity to %s", uart_nr, parity == 0 ? "NONE" : parity == 2 ? "EVEN" : "ODD");
444+
}
445+
}
446+
if (retCode && (uart->_config & 0xc30) >> 4 != stop_bits) {
447+
if (ESP_OK != uart_set_stop_bits(uart_nr, stop_bits)) {
448+
log_e("UART%d changing stop bits failed.", uart_nr);
449+
retCode = false;
450+
} else {
451+
log_v("UART%d changed stop bits to %d", uart_nr, stop_bits == 3 ? 2 : 1);
452+
}
453+
}
454+
if (retCode) uart->_config = config;
455+
if (retCode && rxPin > 0 && uart->_rxPin != rxPin) {
456+
retCode &= _uartDetachPins(uart_nr, uart->_rxPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
457+
retCode &= _uartAttachPins(uart_nr, rxPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
458+
if (!retCode) {
459+
log_e("UART%d changing RX pin failed.", uart_nr);
460+
} else {
461+
log_v("UART%d changed RX pin to %d", uart_nr, rxPin);
462+
}
463+
}
464+
if (retCode && txPin > 0 && uart->_txPin != txPin) {
465+
retCode &= _uartDetachPins(uart_nr, UART_PIN_NO_CHANGE, uart->_txPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
466+
retCode &= _uartAttachPins(uart_nr, UART_PIN_NO_CHANGE, txPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
467+
if (!retCode) {
468+
log_e("UART%d changing TX pin failed.", uart_nr);
469+
} else {
470+
log_v("UART%d changed TX pin to %d", uart_nr, txPin);
471+
}
472+
}
473+
UART_MUTEX_UNLOCK();
474+
if (retCode) {
475+
// UART driver was already working, just return the uart_t structure, syaing that no new driver was installed
476+
return uart;
477+
}
478+
// if we reach this point, it means that we need to restart the UART driver
479+
uartEnd(uart_nr);
480+
}
481+
} else {
482+
log_v("UART%d not installed. Starting installation", uart_nr);
483+
}
380484
uart_config_t uart_config;
381485
uart_config.data_bits = (config & 0xc) >> 2;
382486
uart_config.parity = (config & 0x3);
@@ -386,7 +490,10 @@ uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rx
386490
uart_config.baud_rate = baudrate;
387491
// CLK_APB for ESP32|S2|S3|C3 -- CLK_PLL_F40M for C2 -- CLK_PLL_F48M for H2 -- CLK_PLL_F80M for C6
388492
uart_config.source_clk = UART_SCLK_DEFAULT;
493+
494+
UART_MUTEX_LOCK();
389495
bool retCode = ESP_OK == uart_driver_install(uart_nr, rx_buffer_size, tx_buffer_size, 20, &(uart->uart_event_queue), 0);
496+
390497
if (retCode) retCode &= ESP_OK == uart_param_config(uart_nr, &uart_config);
391498

392499
// Is it right or the idea is to swap rx and tx pins?
@@ -395,19 +502,31 @@ uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rx
395502
retCode &= ESP_OK == uart_set_line_inverse(uart_nr, UART_SIGNAL_TXD_INV | UART_SIGNAL_RXD_INV);
396503
}
397504

505+
if (retCode) {
506+
uart->_baudrate = baudrate;
507+
uart->_config = config;
508+
uart->_inverted = inverted;
509+
uart->_rxfifo_full_thrhd = rxfifo_full_thrhd;
510+
uart->_rx_buffer_size = rx_buffer_size;
511+
uart->_tx_buffer_size = tx_buffer_size;
512+
uart->_ctsPin = -1;
513+
uart->_rtsPin = -1;
514+
uart->has_peek = false;
515+
uart->peek_byte = 0;
516+
}
398517
UART_MUTEX_UNLOCK();
518+
399519
// uartSetPins detaches previous pins if new ones are used over a previous begin()
400520
if (retCode) retCode &= uartSetPins(uart_nr, rxPin, txPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
401-
402-
if (retCode) uartFlush(uart);
403-
else {
521+
if (!retCode) {
404522
uartEnd(uart_nr);
405523
uart = NULL;
406524
log_e("UART%d initialization error.", uart->num);
525+
} else {
526+
uartFlush(uart);
527+
log_v("UART%d initialization done.", uart->num);
407528
}
408-
409-
log_v("UART%d baud(%ld) Mode(%x) rxPin(%d) txPin(%d)", uart_nr, baudrate, config, rxPin, txPin);
410-
return uart;
529+
return uart; // a new driver was installed
411530
}
412531

413532
// This function code is under testing - for now just keep it here
@@ -658,6 +777,7 @@ void uartSetBaudRate(uart_t* uart, uint32_t baud_rate)
658777
if(uart_get_sclk_freq(UART_SCLK_DEFAULT, &sclk_freq) == ESP_OK){
659778
uart_ll_set_baudrate(UART_LL_GET_HW(uart->num), baud_rate, sclk_freq);
660779
}
780+
uart->_baudrate = baud_rate;
661781
UART_MUTEX_UNLOCK();
662782
}
663783

@@ -1025,3 +1145,4 @@ int uart_send_msg_with_break(uint8_t uartNum, uint8_t *msg, size_t msgSize)
10251145
}
10261146

10271147
#endif /* SOC_UART_SUPPORTED */
1148+

‎cores/esp32/esp32-hal-uart.h

Copy file name to clipboardExpand all lines: cores/esp32/esp32-hal-uart.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ extern "C" {
3232
struct uart_struct_t;
3333
typedef struct uart_struct_t uart_t;
3434

35+
bool _testUartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rxPin, int8_t txPin, uint16_t rx_buffer_size, uint16_t tx_buffer_size, bool inverted, uint8_t rxfifo_full_thrhd);
3536
uart_t* uartBegin(uint8_t uart_nr, uint32_t baudrate, uint32_t config, int8_t rxPin, int8_t txPin, uint16_t rx_buffer_size, uint16_t tx_buffer_size, bool inverted, uint8_t rxfifo_full_thrhd);
3637
void uartEnd(uint8_t uart_num);
3738

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.