From 7ed480fc6fa6f8d273ba5bcae7c575985684d665 Mon Sep 17 00:00:00 2001 From: Damien George Date: Sun, 20 Oct 2024 23:12:47 +1100 Subject: [PATCH] Revert "stm32/machine_uart: Allow changing only the baudrate." This reverts commit c94a3205b044fb27fa703d5c280fb02a094f12e3. The idea behind this reverted commit was that it allowed to reconfigure the UART to change only the baudrate, which is important in the context of a PPP connection where the baudrate may be changed as part of the protocol. Also, other ports like the rp2 port have this behaviour, where individual parameters of the UART can be changed with the `.init()` method. But this commit was no good for a few reasons: 1. It's a subtle breaking change to the UART API, because existing code that constructs or initialises a UART with just the baudrate would expect all other parameters to be reset to their defaults. But with this commit those parameters would remain unchanged. 2. Constructing a UART like `UART(1, 9600)` also hits this code path of only changing the baudrate and does not reset other parameters, which is unexpected. 3. It doesn't support setting the baudrate via keyword, eg `UART.init(baudrate=9600)`. 4. The `timeout_char` field is not updated when changing only the baudrate, which can lead to unexpected timeouts when reading/writing. Due to point (4), this commit broke the `tests/ports/stm32/uart.py` test, the `uart.writechar(1)` has a timeout because the `uart.init(2400)` does not set the `timeout_char` for the new baudrate. Points (2)-(4) could be fixed, but point (1) (being a breaking change) would remain as an issue. So the commit is reverted. Signed-off-by: Damien George --- ports/stm32/machine_uart.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ports/stm32/machine_uart.c b/ports/stm32/machine_uart.c index 6be6bcdac..8444b2998 100644 --- a/ports/stm32/machine_uart.c +++ b/ports/stm32/machine_uart.c @@ -154,12 +154,6 @@ static void mp_machine_uart_init_helper(machine_uart_obj_t *self, size_t n_args, { MP_QSTR_read_buf_len, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 64} }, // legacy }; - if (self->is_enabled && n_args == 1 && kw_args->used == 0) { - // Only change the baudrate if that's all that is given. - uart_set_baudrate(self, mp_obj_get_int(pos_args[0])); - return; - } - // parse args struct { mp_arg_val_t baudrate, bits, parity, stop, flow, timeout, timeout_char, rxbuf, read_buf_len;