esp32/machine_uart: Convert machine.UART objects to static instances.

Makes machine.UART objects static instances (similar to other ports), adds
machine_uart_deinit_all() function for cleanup on soft reset.

- Fixes the case where the OS-level uart_event_task is leaked
  (along with 2KB of heap) when a UART object is garbage collected
  (including after a soft reset).

- Fixes hard fault if a previous UART irq() fires after the UART object is
  garbage collected (including after a soft reset), but before any new UART
  object is instantiated for the same UART number.

- Constructing multiple UART objects for the same UART now returns the
  same instance multiple times, not different instances.

- Was also able to streamline deinit/init to only install the driver once,
  rather than install-with-defaults/uninstall/reinstall.

Alternative would be to add a finaliser, but this is more consistent
with how most other UART objects are implemented.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
This commit is contained in:
Angus Gratton
2025-08-06 17:26:21 +10:00
committed by Damien George
parent a43e38544b
commit 371db85c8e
3 changed files with 137 additions and 91 deletions

View File

@@ -68,6 +68,9 @@ enum {
RXIDLE_ALERT,
};
// RXIDLE irq feature uses this machine.Timer id
#define RXIDLE_TIMER_IDX 0
typedef struct _machine_uart_obj_t {
mp_obj_base_t base;
uart_port_t uart_num;
@@ -96,6 +99,14 @@ typedef struct _machine_uart_obj_t {
static const char *_parity_name[] = {"None", "1", "0"};
static bool uart_is_repl(uart_port_t uart_num) {
#if MICROPY_HW_ENABLE_UART_REPL
return uart_num == MICROPY_HW_UART_REPL;
#else
return false;
#endif
}
/******************************************************************************/
// MicroPython bindings for UART
@@ -245,13 +256,67 @@ static void mp_machine_uart_init_helper(machine_uart_obj_t *self, size_t n_args,
// wait for all data to be transmitted before changing settings
uart_wait_tx_done(self->uart_num, pdMS_TO_TICKS(1000));
if ((args[ARG_txbuf].u_int >= 0 && args[ARG_txbuf].u_int != self->txbuf) || (args[ARG_rxbuf].u_int >= 0 && args[ARG_rxbuf].u_int != self->rxbuf)) {
// must reinitialise driver to change the tx/rx buffer size
#if MICROPY_HW_ENABLE_UART_REPL
if (self->uart_num == MICROPY_HW_UART_REPL) {
mp_raise_ValueError(MP_ERROR_TEXT("UART buffer size is fixed"));
// If UART is being freshly initialised then restore object defaults
if (!uart_is_driver_installed(self->uart_num)) {
self->bits = 8;
self->parity = 0;
self->stop = 1;
self->rts = UART_PIN_NO_CHANGE;
self->cts = UART_PIN_NO_CHANGE;
self->txbuf = 256;
self->rxbuf = 256; // IDF minimum
self->timeout = 0;
self->timeout_char = 0;
self->invert = 0;
self->flowcontrol = 0;
self->uart_event_task = NULL;
self->uart_queue = NULL;
self->rxidle_state = RXIDLE_INACTIVE;
// Set the MicroPython default UART pins. These may be overwritten with
// caller-provided pins, below
switch (self->uart_num) {
case UART_NUM_0:
self->rx = UART_PIN_NO_CHANGE; // GPIO 3
self->tx = UART_PIN_NO_CHANGE; // GPIO 1
break;
case UART_NUM_1:
self->rx = 9;
self->tx = 10;
break;
#if SOC_UART_HP_NUM > 2
case UART_NUM_2:
self->rx = 16;
self->tx = 17;
break;
#endif
#if SOC_UART_LP_NUM >= 1
case LP_UART_NUM_0:
self->rx = 4;
self->tx = 5;
break;
#endif
case UART_NUM_MAX:
assert(0); // Range is checked in mp_machine_uart_make_new, value should be unreachable
}
}
// Default driver parameters, should correspond to values set above
uart_config_t uartcfg = {
.baud_rate = 115200,
.data_bits = UART_DATA_8_BITS,
.parity = UART_PARITY_DISABLE,
.stop_bits = UART_STOP_BITS_1,
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
.rx_flow_ctrl_thresh = 0,
.source_clk = UART_SOURCE_CLK,
};
if ((args[ARG_txbuf].u_int >= 0 && args[ARG_txbuf].u_int != self->txbuf) || (args[ARG_rxbuf].u_int >= 0 && args[ARG_rxbuf].u_int != self->rxbuf)) {
if (uart_is_repl(self->uart_num)) {
mp_raise_ValueError(MP_ERROR_TEXT("REPL UART buffer size is fixed"));
}
#endif
if (args[ARG_txbuf].u_int >= 0) {
self->txbuf = args[ARG_txbuf].u_int;
@@ -259,18 +324,26 @@ static void mp_machine_uart_init_helper(machine_uart_obj_t *self, size_t n_args,
if (args[ARG_rxbuf].u_int >= 0) {
self->rxbuf = args[ARG_rxbuf].u_int;
}
uart_config_t uartcfg = {
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
.rx_flow_ctrl_thresh = 0,
.source_clk = UART_SOURCE_CLK,
};
uint32_t baudrate;
check_esp_err(uart_get_baudrate(self->uart_num, &baudrate));
uartcfg.baud_rate = baudrate;
check_esp_err(uart_get_word_length(self->uart_num, &uartcfg.data_bits));
check_esp_err(uart_get_parity(self->uart_num, &uartcfg.parity));
check_esp_err(uart_get_stop_bits(self->uart_num, &uartcfg.stop_bits));
mp_machine_uart_deinit(self);
// must reinitialise driver to change the tx/rx buffer size
if (uart_is_driver_installed(self->uart_num)) {
// Update uartcfg with the current uart parameters
uint32_t baudrate;
check_esp_err(uart_get_baudrate(self->uart_num, &baudrate));
uartcfg.baud_rate = baudrate;
check_esp_err(uart_get_word_length(self->uart_num, &uartcfg.data_bits));
check_esp_err(uart_get_parity(self->uart_num, &uartcfg.parity));
check_esp_err(uart_get_stop_bits(self->uart_num, &uartcfg.stop_bits));
// De-initialise the driver
mp_machine_uart_deinit(self);
}
}
if (!uart_is_repl(self->uart_num) && !uart_is_driver_installed(self->uart_num)) {
// install the driver if needed - could be uninstalled for multiple reasons:
// 1. First time initialising this UART
// 2. Has been temporarily de-initialised to change the buffer size
// 3. Python code called .deinit() and now .init()
check_esp_err(uart_param_config(self->uart_num, &uartcfg));
check_esp_err(uart_driver_install(self->uart_num, self->rxbuf, self->txbuf, UART_QUEUE_SIZE, &self->uart_queue, 0));
if (self->mp_irq_obj != NULL && self->mp_irq_obj->handler != mp_const_none) {
@@ -399,6 +472,9 @@ static void mp_machine_uart_init_helper(machine_uart_obj_t *self, size_t n_args,
}
uint8_t uart_fifo_len = UART_HW_FIFO_LEN(self->uart_num);
check_esp_err(uart_set_hw_flow_ctrl(self->uart_num, self->flowcontrol, uart_fifo_len - uart_fifo_len / 4));
// discard any input from previous configuration
check_esp_err(uart_flush_input(self->uart_num));
}
static mp_obj_t mp_machine_uart_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) {
@@ -409,73 +485,14 @@ static mp_obj_t mp_machine_uart_make_new(const mp_obj_type_t *type, size_t n_arg
if (uart_num < 0 || uart_num >= UART_NUM_MAX) {
mp_raise_msg_varg(&mp_type_ValueError, MP_ERROR_TEXT("UART(%d) does not exist"), uart_num);
}
_Static_assert(UART_NUM_MAX == SOC_UART_NUM, "SOC and driver constant should match");
// Defaults
uart_config_t uartcfg = {
.baud_rate = 115200,
.data_bits = UART_DATA_8_BITS,
.parity = UART_PARITY_DISABLE,
.stop_bits = UART_STOP_BITS_1,
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
.rx_flow_ctrl_thresh = 0,
.source_clk = UART_SOURCE_CLK,
};
// create instance
machine_uart_obj_t *self = mp_obj_malloc(machine_uart_obj_t, &machine_uart_type);
self->uart_num = uart_num;
self->bits = 8;
self->parity = 0;
self->stop = 1;
self->rts = UART_PIN_NO_CHANGE;
self->cts = UART_PIN_NO_CHANGE;
self->txbuf = 256;
self->rxbuf = 256; // IDF minimum
self->timeout = 0;
self->timeout_char = 0;
self->invert = 0;
self->flowcontrol = 0;
self->uart_event_task = NULL;
self->uart_queue = NULL;
self->rxidle_state = RXIDLE_INACTIVE;
switch (uart_num) {
case UART_NUM_0:
self->rx = UART_PIN_NO_CHANGE; // GPIO 3
self->tx = UART_PIN_NO_CHANGE; // GPIO 1
break;
case UART_NUM_1:
self->rx = 9;
self->tx = 10;
break;
#if SOC_UART_HP_NUM > 2
case UART_NUM_2:
self->rx = 16;
self->tx = 17;
break;
#endif
#if SOC_UART_LP_NUM >= 1
case LP_UART_NUM_0:
self->rx = 4;
self->tx = 5;
#endif
}
#if MICROPY_HW_ENABLE_UART_REPL
// Only reset the driver if it's not the REPL UART.
if (uart_num != MICROPY_HW_UART_REPL)
#endif
{
// Remove any existing configuration
check_esp_err(uart_driver_delete(self->uart_num));
self->uart_queue = NULL;
// init the peripheral
// Setup
check_esp_err(uart_param_config(self->uart_num, &uartcfg));
check_esp_err(uart_driver_install(uart_num, self->rxbuf, self->txbuf, UART_QUEUE_SIZE, &self->uart_queue, 0));
machine_uart_obj_t *self = MP_STATE_PORT(machine_uart_objs)[uart_num];
if (self == NULL) {
// Allocate a new UART object
self = mp_obj_malloc(machine_uart_obj_t, &machine_uart_type);
self->uart_num = uart_num;
// remaining fields are set from inside mp_machine_uart_init_helper()
}
mp_map_t kw_args;
@@ -485,6 +502,8 @@ static mp_obj_t mp_machine_uart_make_new(const mp_obj_type_t *type, size_t n_arg
// Make sure pins are connected.
check_esp_err(uart_set_pin(self->uart_num, self->tx, self->rx, self->rts, self->cts));
MP_STATE_PORT(machine_uart_objs)[uart_num] = self;
return MP_OBJ_FROM_PTR(self);
}
@@ -493,8 +512,31 @@ static void mp_machine_uart_deinit(machine_uart_obj_t *self) {
vTaskDelete(self->uart_event_task);
self->uart_event_task = NULL;
}
check_esp_err(uart_driver_delete(self->uart_num));
self->uart_queue = NULL;
if (self->rxidle_timer != NULL) {
machine_timer_disable(self->rxidle_timer);
if (self->rxidle_state > RXIDLE_STANDBY) {
// Currently deinit(),init() sequence resumes any previously
// configured irqs, and we currently also rely on this when changing
// buffer size - so keep rxidle in standby if it was active
self->rxidle_state = RXIDLE_STANDBY;
}
}
// Only stop the ESP-IDF driver entirely if it's not the REPL,
// (if it's the REPL then it shouldn't have any ISR configured anyway.)
if (!uart_is_repl(self->uart_num)) {
check_esp_err(uart_driver_delete(self->uart_num));
self->uart_queue = NULL;
}
}
void machine_uart_deinit_all(void) {
for (int i = 0; i < UART_NUM_MAX; i++) {
machine_uart_obj_t *uart = MP_STATE_PORT(machine_uart_objs)[i];
if (uart) {
mp_machine_uart_deinit(uart);
MP_STATE_PORT(machine_uart_objs)[i] = NULL;
}
}
}
static mp_int_t mp_machine_uart_any(machine_uart_obj_t *self) {
@@ -574,11 +616,9 @@ static const mp_irq_methods_t uart_irq_methods = {
};
static mp_irq_obj_t *mp_machine_uart_irq(machine_uart_obj_t *self, bool any_args, mp_arg_val_t *args) {
#if MICROPY_HW_ENABLE_UART_REPL
if (self->uart_num == MICROPY_HW_UART_REPL) {
mp_raise_ValueError(MP_ERROR_TEXT("UART does not support IRQs"));
if (uart_is_repl(self->uart_num)) {
mp_raise_ValueError(MP_ERROR_TEXT("REPL UART does not support IRQs"));
}
#endif
if (self->mp_irq_obj == NULL) {
self->mp_irq_trigger = 0;
@@ -605,7 +645,7 @@ static mp_irq_obj_t *mp_machine_uart_irq(machine_uart_obj_t *self, bool any_args
}
self->mp_irq_obj->ishard = false;
self->mp_irq_trigger = trigger;
self->rxidle_timer = machine_timer_create(0);
self->rxidle_timer = machine_timer_create(RXIDLE_TIMER_IDX);
uart_irq_configure_timer(self, trigger);
// Start a task for handling events
@@ -700,3 +740,5 @@ static mp_uint_t mp_machine_uart_ioctl(mp_obj_t self_in, mp_uint_t request, uint
}
return ret;
}
MP_REGISTER_ROOT_POINTER(void *machine_uart_objs[SOC_UART_NUM]);

View File

@@ -180,6 +180,9 @@ soft_reset_exit:
MP_STATE_PORT(espnow_singleton) = NULL;
#endif
// Deinit uart before timers, as esp32 uart
// depends on a timer instance
machine_uart_deinit_all();
machine_timer_deinit_all();
#if MICROPY_PY_ESP32_PCNT

View File

@@ -20,6 +20,7 @@ void machine_pins_deinit(void);
void machine_pwm_deinit_all(void);
// TODO: void machine_rmt_deinit_all(void);
void machine_timer_deinit_all(void);
void machine_uart_deinit_all(void);
void machine_i2s_init0();
#endif // MICROPY_INCLUDED_ESP32_MODMACHINE_H