From 6bdc439c39a0662fba1020db1a769369ccdf1409 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Sun, 20 Apr 2025 23:49:46 +0300 Subject: [PATCH 01/21] dmx: log uart mtbp_min --- components/dmx/include/dmx_uart.h | 8 ++++---- main/dmx_uart.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/dmx/include/dmx_uart.h b/components/dmx/include/dmx_uart.h index 07096f26..994fbc5a 100644 --- a/components/dmx/include/dmx_uart.h +++ b/components/dmx/include/dmx_uart.h @@ -7,17 +7,17 @@ #define DMX_UART_RX_BUFFER_SIZE (512 + 1) #define DMX_UART_TX_BUFFER_SIZE (512 + 1) -#define DMX_UART_MTBP_UNIT (8 * (1000000 / 250000)) -#define DMX_UART_MTBP_MIN (4 * DMX_UART_MTBP_UNIT) +#define DMX_UART_MTBP_UNIT (8 * (1000000 / 250000)) // us per frame +#define DMX_UART_MTBP_MIN (4 * DMX_UART_MTBP_UNIT) // 128us #define DMX_UART_MTBP_MAX (UART_RX_TIMEOUT_MAX * DMX_UART_MTBP_UNIT) // SOC supports mapping IO pins #define DMX_UART_IO_PINS_SUPPORTED UART_IO_PINS_SUPPORTED struct dmx_uart_options { - // buffer RX FIFO until line idle for this many ~8-bit periods + // buffer RX FIFO until line idle for this many us // this must be short enough to trigger in the MTBP, or the final bytes in the packet will be lost... - uint16_t mtbp_min; // use DMX_UART_MTBP_MIN + uint16_t mtbp_min; // us // Acquire mutex before setting dev interrupts SemaphoreHandle_t dev_mutex; diff --git a/main/dmx_uart.c b/main/dmx_uart.c index ac52cf2a..115612bb 100644 --- a/main/dmx_uart.c +++ b/main/dmx_uart.c @@ -82,11 +82,11 @@ int start_dmx_uart() } if (input_enabled && dmx_input_config.mtbp_min) { - LOG_INFO("use uart mtbp_min=%u for dmx-input", dmx_input_config.mtbp_min); - options.mtbp_min = dmx_input_config.mtbp_min; } + LOG_INFO("uart mtbp_min=%uus for dmx-input", options.mtbp_min); + #if DMX_UART_IO_PINS_SUPPORTED if (input_enabled) { options.rx_pin = config->rx_pin; From fa40fdf590857a2811d7694cad6ead76db533311 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 19:40:35 +0300 Subject: [PATCH 02/21] uart: use 0 for xStreamBufferCreate() trigger level --- components/uart/esp32/rx.c | 2 +- components/uart/esp32/tx.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/components/uart/esp32/rx.c b/components/uart/esp32/rx.c index 25f6df66..a09c8dea 100644 --- a/components/uart/esp32/rx.c +++ b/components/uart/esp32/rx.c @@ -15,7 +15,7 @@ int uart_rx_init(struct uart *uart, size_t rx_buffer_size) if (rx_buffer_size == 0) { uart->rx_buffer = NULL; - } else if (!(uart->rx_buffer = xStreamBufferCreate(rx_buffer_size, 1))) { + } else if (!(uart->rx_buffer = xStreamBufferCreate(rx_buffer_size, 0))) { LOG_ERROR("xStreamBufferCreate"); return -1; } diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index 1975c90b..3789e6ce 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -11,7 +11,8 @@ int uart_tx_init(struct uart *uart, size_t tx_buffer_size) if (!tx_buffer_size) { uart->tx_buffer = NULL; - } else if (!(uart->tx_buffer = xStreamBufferCreate(tx_buffer_size, 1))) { + // TODO: use xTriggerLevelBytes=0 + } else if (!(uart->tx_buffer = xStreamBufferCreate(tx_buffer_size, 0))) { LOG_ERROR("xStreamBufferCreate"); return -1; } From 8ff0906d387b1968550f7bc78a9a766a7839c7ff Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 19:53:11 +0300 Subject: [PATCH 03/21] stdio: use uart_write() -> stdio_log_write() with matching len instead of looping on uart write --- components/stdio/log.c | 25 ++++++++++++++----------- components/stdio/log.h | 4 ++-- components/stdio/vfs.c | 20 +++++++++++++------- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/components/stdio/log.c b/components/stdio/log.c index 403a3894..769d9cd4 100644 --- a/components/stdio/log.c +++ b/components/stdio/log.c @@ -89,27 +89,30 @@ int stdio_log_new(struct stdio_log **logp, size_t buffer_size) return err; } -size_t stdio_log_write(struct stdio_log *log, const void *data, size_t len) +void stdio_log_write(struct stdio_log *log, const uint8_t *data, size_t len) { if (!xSemaphoreTakeRecursive(log->mutex, portMAX_DELAY)) { - return 0; + return; } - size_t size = stdio_log_write_size(log); + while (len) { + size_t size = stdio_log_write_size(log); - if (len > size) { - len = size; - } + if (size > len) { + size = len; + } - LOG_BOOT_DEBUG("write=%p count=%u size=%u len=%u", log->write, log->write_count, size, len); + LOG_BOOT_DEBUG("write=%p count=%u size=%u len=%u", log->write, log->write_count, size, len); - memcpy(log->write, data, len); + memcpy(log->write, data, size); - stdio_log_write_len(log, len); + stdio_log_write_len(log, size); - xSemaphoreGiveRecursive(log->mutex); + data += size; + len -= size; + } - return len; + xSemaphoreGiveRecursive(log->mutex); } static size_t stdio_log_read_size(struct stdio_log *log) diff --git a/components/stdio/log.h b/components/stdio/log.h index c34ff2ee..88be74bd 100644 --- a/components/stdio/log.h +++ b/components/stdio/log.h @@ -20,9 +20,9 @@ struct stdio_log { extern struct stdio_log *stderr_log; /* - * Return number of bytes copied. + * Guaranteed to succeeed and write all data. */ -size_t stdio_log_write(struct stdio_log *log, const void *data, size_t len); +void stdio_log_write(struct stdio_log *log, const uint8_t *data, size_t len); /* * Return number of bytes copied. diff --git a/components/stdio/vfs.c b/components/stdio/vfs.c index d43db505..aa517a38 100644 --- a/components/stdio/vfs.c +++ b/components/stdio/vfs.c @@ -18,6 +18,8 @@ ssize_t stdio_vfs_write(int fd, const void *data, size_t size) { + ssize_t ret; + switch(fd) { case STDOUT_FILENO: if (stdio_uart) { @@ -28,15 +30,19 @@ ssize_t stdio_vfs_write(int fd, const void *data, size_t size) } case STDERR_FILENO: - if (stderr_log) { - size = stdio_log_write(stderr_log, data, size); - } - if (!stdio_uart) { os_write(data, size); - } else if (uart_write_all(stdio_uart, data, size)) { - errno = EIO; - return -1; + } else { + if ((ret = uart_write(stdio_uart, data, size)) < 0) { + errno = EIO; + return -1; + } else { + size = ret; + } + } + + if (stderr_log) { + stdio_log_write(stderr_log, data, size); } return size; From 13513feb571b00fcbe6094dde1d97e0029a52aa2 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 19:55:14 +0300 Subject: [PATCH 04/21] uart: remove unused uart_write_buffered() --- components/uart/esp32/tx.c | 23 -------------------- components/uart/include/uart.h | 9 -------- components/uart/uart.c | 39 ---------------------------------- components/uart/uart.h | 1 - 4 files changed, 72 deletions(-) diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index 3789e6ce..98423a0d 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -115,29 +115,6 @@ size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len) return write; } -size_t uart_tx_buffered(struct uart *uart, const uint8_t *buf, size_t len) -{ - size_t write; - - if (!uart->tx_buffer) { - LOG_DEBUG("TX buffer disabled"); - return 0; - } - - // write as many bytes as possible, ensure tx buffer is not empty - write = xStreamBufferSend(uart->tx_buffer, buf, len, 0); - - LOG_ISR_DEBUG("buf len=%u: write=%u", len, write); - - if (write == 0) { - // TX buffer full, enable ISR - uart_ll_set_txfifo_empty_thr(uart->dev, UART_TX_EMPTY_THRD_DEFAULT); - uart_ll_ena_intr_mask(uart->dev, UART_TX_WRITE_INTR_MASK); - } - - return write; -} - size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len) { size_t write; diff --git a/components/uart/include/uart.h b/components/uart/include/uart.h index 51179ac2..87f2da48 100644 --- a/components/uart/include/uart.h +++ b/components/uart/include/uart.h @@ -206,15 +206,6 @@ ssize_t uart_write(struct uart *uart, const void *buf, size_t len); */ int uart_write_all(struct uart *uart, const void *buf, size_t len); -/** - * Write len bytes from buf into the TX buffer. - * - * TX is only started once the TX buffer is full, or uart_flush() is called. - * - * Returns 0, or <0 on error. - */ -ssize_t uart_write_buffered(struct uart *uart, const void *buf, size_t len); - /** * Wait for TX buffer + FIFO to empty. */ diff --git a/components/uart/uart.c b/components/uart/uart.c index 75b347a9..e9da0b0c 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -460,45 +460,6 @@ ssize_t uart_write_all(struct uart *uart, const void *buf, size_t len) return 0; } -ssize_t uart_write_buffered(struct uart *uart, const void *buf, size_t len) -{ - size_t write; - int err; - - if ((err = uart_acquire_tx(uart))) { - return err; - } - - while (len > 0) { - // fastpath via TX buffer, without interrupts - write = uart_tx_buffered(uart, buf, len); - - LOG_DEBUG("tx buf len=%u: write=%u", len, write); - - buf += write; - len -= write; - - if (len > 0) { - // blocking slowpath via buffer + ISR - write = uart_tx_slow(uart, buf, len); - - LOG_DEBUG("tx slow len=%u: write=%u", len, write); - - buf += write; - len -= write; - } - - if (!len) { - LOG_WARN("tx stuck, tx_buffer=%p", uart->tx_buffer); - return -1; - } - } - - uart_release_tx(uart); - - return 0; -} - int uart_flush_write(struct uart *uart) { int err; diff --git a/components/uart/uart.h b/components/uart/uart.h index c7a9758a..7a176d02 100644 --- a/components/uart/uart.h +++ b/components/uart/uart.h @@ -83,7 +83,6 @@ int uart_tx_setup(struct uart *uart, struct uart_options options); int uart_tx_one(struct uart *uart, uint8_t byte); size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len); -size_t uart_tx_buffered(struct uart *uart, const uint8_t *buf, size_t len); size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len); int uart_tx_flush(struct uart *uart); From 24b4f16e77e84f226f6b72593888846f3d36af9e Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 19:59:47 +0300 Subject: [PATCH 05/21] leds interfaces uart: use uart_write() --- components/leds/interfaces/uart/tx.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/components/leds/interfaces/uart/tx.c b/components/leds/interfaces/uart/tx.c index 070fb1f5..a9fc9bf8 100644 --- a/components/leds/interfaces/uart/tx.c +++ b/components/leds/interfaces/uart/tx.c @@ -62,23 +62,40 @@ int leds_interface_uart_init(struct leds_interface_uart *interface, const struct return 0; } +static int leds_interface_uart_write(struct leds_interface_uart *interface, void *buf, size_t len) +{ + const uint8_t *ptr = buf; + int write; + + while (len) { + if ((write = uart_write(interface->uart, ptr, len)) < 0) { + return write; + } + + ptr += write; + len -= write; + } + + return 0; +} + int leds_interface_uart_tx_pixel(struct leds_interface_uart *interface, const struct leds_color *pixels, unsigned index, const struct leds_limit *limit) { switch (interface->mode) { case LEDS_INTERFACE_UART_MODE_24B3I7_0U4_80U: interface->func.uart_mode_24B3I7(interface->buf.uart_mode_24B3I7, pixels, index, limit); - return uart_write_all(interface->uart, interface->buf.uart_mode_24B3I7, sizeof(interface->buf.uart_mode_24B3I7)); + return leds_interface_uart_write(interface, interface->buf.uart_mode_24B3I7, sizeof(interface->buf.uart_mode_24B3I7)); case LEDS_INTERFACE_UART_MODE_24B2I8_0U25_50U: interface->func.uart_mode_24B2I8(interface->buf.uart_mode_24B2I8, pixels, index, limit); - return uart_write_all(interface->uart, interface->buf.uart_mode_24B2I8, sizeof(interface->buf.uart_mode_24B2I8)); + return leds_interface_uart_write(interface, interface->buf.uart_mode_24B2I8, sizeof(interface->buf.uart_mode_24B2I8)); case LEDS_INTERFACE_UART_MODE_32B2I6_0U3_80U: interface->func.uart_mode_32B2I6(interface->buf.uart_mode_32B2I6, pixels, index, limit); - return uart_write_all(interface->uart, interface->buf.uart_mode_32B2I6, sizeof(interface->buf.uart_mode_32B2I6)); + return leds_interface_uart_write(interface, interface->buf.uart_mode_32B2I6, sizeof(interface->buf.uart_mode_32B2I6)); default: LOG_FATAL("invalid mode=%d", interface->mode); From 5a9ffdf7ea32622d2e0358c9a60511b10b2c160f Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 20:00:26 +0300 Subject: [PATCH 06/21] uart: remove uart_write_all() to simplify interface --- components/uart/include/uart.h | 7 ------- components/uart/uart.c | 34 ---------------------------------- 2 files changed, 41 deletions(-) diff --git a/components/uart/include/uart.h b/components/uart/include/uart.h index 87f2da48..675739f2 100644 --- a/components/uart/include/uart.h +++ b/components/uart/include/uart.h @@ -199,13 +199,6 @@ int uart_putc(struct uart *uart, int ch); */ ssize_t uart_write(struct uart *uart, const void *buf, size_t len); -/** - * Write len bytes from buf. Blocks if TX buffer is full. - * - * Returns 0, or <0 on error. - */ -int uart_write_all(struct uart *uart, const void *buf, size_t len); - /** * Wait for TX buffer + FIFO to empty. */ diff --git a/components/uart/uart.c b/components/uart/uart.c index e9da0b0c..aca1a645 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -426,40 +426,6 @@ ssize_t uart_write(struct uart *uart, const void *buf, size_t len) return write; } -ssize_t uart_write_all(struct uart *uart, const void *buf, size_t len) -{ - size_t write; - int err; - - if ((err = uart_acquire_tx(uart))) { - return err; - } - - while (len > 0) { - // fastpath via FIFO queue or TX buffer - write = uart_tx_fast(uart, buf, len); - - LOG_DEBUG("tx fast len=%u: write=%u", len, write); - - buf += write; - len -= write; - - if (len > 0) { - // blocking slowpath via buffer + ISR - write = uart_tx_slow(uart, buf, len); - - LOG_DEBUG("tx slow len=%u: write=%u", len, write); - - buf += write; - len -= write; - } - } - - uart_release_tx(uart); - - return 0; -} - int uart_flush_write(struct uart *uart) { int err; From 55ab3b5dca3192408f5a8d54915a19fefd98ddfa Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 20:08:33 +0300 Subject: [PATCH 07/21] uart: use xTaskNotifyStateClear(), xTaskNotifyWait(), xTaskNotifyFromISR() to ensure existing task notification value does not affect behavior --- components/uart/esp32/intr.c | 2 +- components/uart/esp32/tx.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/components/uart/esp32/intr.c b/components/uart/esp32/intr.c index 08aa5f29..ba8dabb3 100644 --- a/components/uart/esp32/intr.c +++ b/components/uart/esp32/intr.c @@ -162,7 +162,7 @@ void IRAM_ATTR uart_intr_tx_done_handler(struct uart *uart, BaseType_t *task_wok LOG_ISR_DEBUG("notify tx_done_notify_task=%p", uart->tx_done_notify_task); // FIFO is empty, notify task - vTaskNotifyGiveFromISR(uart->tx_done_notify_task, task_woken); + xTaskNotifyFromISR(uart->tx_done_notify_task, 0, eNoAction, task_woken); // only once uart->tx_done_notify_task = NULL; diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index 98423a0d..b7786eea 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -143,6 +143,8 @@ int uart_tx_flush(struct uart *uart) LOG_DEBUG(""); + xTaskNotifyStateClear(task); + taskENTER_CRITICAL(&uart->mux); if (uart->tx_buffer && !xStreamBufferIsEmpty(uart->tx_buffer)) { @@ -174,7 +176,7 @@ int uart_tx_flush(struct uart *uart) LOG_DEBUG("wait done=%d...", done); // wait for tx to complete and break to start - if (!ulTaskNotifyTake(true, portMAX_DELAY)) { + if (!xTaskNotifyWait(0, 0, NULL, portMAX_DELAY)) { LOG_WARN("timeout"); return -1; } From 25ac8ffb9de9433cbb0b7efa0ebefa17441a0b65 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 20:33:23 +0300 Subject: [PATCH 08/21] uart_tx_fast: use critical section, simplify uart_tx_raw() away --- components/uart/esp32/tx.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index b7786eea..89a74ce7 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -75,37 +75,35 @@ int uart_tx_one(struct uart *uart, uint8_t byte) return ret; } -static size_t uart_tx_raw(struct uart *uart, const uint8_t *buf, size_t size) -{ - size_t len = uart_ll_get_txfifo_len(uart->dev); - - if (len > size) { - len = size; - } - - // assume no crtical section required, as uart is locked and ISR should not be running - uart_ll_write_txfifo(uart->dev, buf, len); - - return len; -} - size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len) { size_t write = 0; - // assume no crtical section required, as uart is locked and ISR should not be running - if (!uart->tx_buffer || xStreamBufferIsEmpty(uart->tx_buffer)) { + taskENTER_CRITICAL(&uart->mux); + + if (uart->tx_buffer && !xStreamBufferIsEmpty(uart->tx_buffer)) { + // TX buffer in use + } else if ((write = uart_ll_get_txfifo_len(uart->dev))) { // fastpath via HW FIFO - write = uart_tx_raw(uart, buf, len); + if (write > len) { + write = len; + } LOG_ISR_DEBUG("raw len=%u: write=%u", len, write); + + uart_ll_write_txfifo(uart->dev, buf, write); + + } else { + // FIFO full } + taskEXIT_CRITICAL(&uart->mux); + if (!write && uart->tx_buffer) { // write as many bytes as possible, ensure tx buffer is not empty write = xStreamBufferSend(uart->tx_buffer, buf, len, 0); - LOG_ISR_DEBUG("buf len=%u: write=%u", len, write); + LOG_DEBUG("buf len=%u: write=%u", len, write); // enable ISR to consume stream buffer uart_ll_set_txfifo_empty_thr(uart->dev, UART_TX_EMPTY_THRD_DEFAULT); From 3850c3367f3a30773eb52a21963b7add439a3657 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 20:54:28 +0300 Subject: [PATCH 09/21] uart: timeout parameter for blocking uart_tx_slow() --- components/uart/esp32/tx.c | 8 ++++++-- components/uart/esp8266/tx.c | 4 ++-- components/uart/uart.c | 2 +- components/uart/uart.h | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index 89a74ce7..e240a2d7 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -100,6 +100,8 @@ size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len) taskEXIT_CRITICAL(&uart->mux); if (!write && uart->tx_buffer) { + // does not use a critical section, inter enable racing with stream send / ISR is harmless + // write as many bytes as possible, ensure tx buffer is not empty write = xStreamBufferSend(uart->tx_buffer, buf, len, 0); @@ -113,7 +115,7 @@ size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len) return write; } -size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len) +size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_t timeout) { size_t write; @@ -123,7 +125,9 @@ size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len) } // does not use a critical section, inter enable racing with stream send / ISR is harmless - write = xStreamBufferSend(uart->tx_buffer, buf, len, portMAX_DELAY); + + // assume that the TX interrupt will be enabled, unless this is empty, in which case it will not block + write = xStreamBufferSend(uart->tx_buffer, buf, len, timeout); LOG_ISR_DEBUG("xStreamBufferSend len=%u: write=%u", len, write); diff --git a/components/uart/esp8266/tx.c b/components/uart/esp8266/tx.c index 89041de1..e2a0cdcf 100644 --- a/components/uart/esp8266/tx.c +++ b/components/uart/esp8266/tx.c @@ -122,12 +122,12 @@ size_t uart_tx_buffered(struct uart *uart, const uint8_t *buf, size_t len) return write; } -size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len) +size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_t timeout) { size_t write; // does not use a critical section, inter enable racing with stream send / ISR is harmless - write = xStreamBufferSend(uart->tx_buffer, buf, len, portMAX_DELAY); + write = xStreamBufferSend(uart->tx_buffer, buf, len, timeout); LOG_ISR_DEBUG("xStreamBufferSend len=%u: write=%u", len, write); diff --git a/components/uart/uart.c b/components/uart/uart.c index aca1a645..0386aefb 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -413,7 +413,7 @@ ssize_t uart_write(struct uart *uart, const void *buf, size_t len) if (!write) { // blocking slowpath via buffer + ISR - write = uart_tx_slow(uart, buf, len); + write = uart_tx_slow(uart, buf, len, portMAX_DELAY); LOG_DEBUG("tx slow len=%u: write=%u", len, write); diff --git a/components/uart/uart.h b/components/uart/uart.h index 7a176d02..7950f105 100644 --- a/components/uart/uart.h +++ b/components/uart/uart.h @@ -83,7 +83,7 @@ int uart_tx_setup(struct uart *uart, struct uart_options options); int uart_tx_one(struct uart *uart, uint8_t byte); size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len); -size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len); +size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_t timeout); int uart_tx_flush(struct uart *uart); From 98963b8c29c0ec0878217c25c27e30cae5b52585 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:04:56 +0300 Subject: [PATCH 10/21] uart: fix uart_tx_one() use of critical section and TX buffer --- components/uart/esp32/tx.c | 41 ++++++++++++++++++++++++------------ components/uart/esp32/tx.h | 4 ++-- components/uart/esp8266/tx.c | 4 ++-- components/uart/uart.c | 2 +- components/uart/uart.h | 2 +- 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index e240a2d7..9ef3ca5b 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -42,37 +42,50 @@ int uart_tx_setup(struct uart *uart, struct uart_options options) return 0; } -int uart_tx_one(struct uart *uart, uint8_t byte) +int uart_tx_one(struct uart *uart, uint8_t byte, TickType_t timeout) { - int ret; + size_t write = 0; taskENTER_CRITICAL(&uart->mux); - if (uart_ll_get_txfifo_len(uart->dev) > 0) { - uart_tx_write_txfifo_byte(uart, byte); + if (uart->tx_buffer && !xStreamBufferIsEmpty(uart->tx_buffer)) { + // TX buffer in use + } else if ((write = uart_ll_get_txfifo_len(uart->dev))) { + // fastpath via HW FIFO + uart_tx_fifo_putc(uart, byte); LOG_ISR_DEBUG("tx fifo"); - ret = 0; + write = 1; + } else { + // must buffer + } - } else if (uart->tx_buffer && xStreamBufferSend(uart->tx_buffer, &byte, 1, portMAX_DELAY) > 0) { + taskEXIT_CRITICAL(&uart->mux); + + if (write) { + return 0; + } + + if (!uart->tx_buffer) { + LOG_ISR_DEBUG("TX fifo full"); + return -1; + + // assume that TX ISR will be enabled, unless the TX buffer is empty, in which case this will not block + } else if (xStreamBufferSend(uart->tx_buffer, &byte, 1, timeout) > 0) { LOG_ISR_DEBUG("tx buffer"); // byte was written uart_ll_set_txfifo_empty_thr(uart->dev, UART_TX_EMPTY_THRD_DEFAULT); uart_ll_ena_intr_mask(uart->dev, UART_TX_WRITE_INTR_MASK); - ret = 0; + return 0; } else { - LOG_ISR_DEBUG("failed"); + LOG_ISR_DEBUG("TX buffer full"); - ret = -1; + return -1; } - - taskEXIT_CRITICAL(&uart->mux); - - return ret; } size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len) @@ -126,7 +139,7 @@ size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_ // does not use a critical section, inter enable racing with stream send / ISR is harmless - // assume that the TX interrupt will be enabled, unless this is empty, in which case it will not block + // assume that TX ISR will be enabled, unless the TX buffer is empty, in which case this will not block write = xStreamBufferSend(uart->tx_buffer, buf, len, timeout); LOG_ISR_DEBUG("xStreamBufferSend len=%u: write=%u", len, write); diff --git a/components/uart/esp32/tx.h b/components/uart/esp32/tx.h index e179b6b5..45a7232e 100644 --- a/components/uart/esp32/tx.h +++ b/components/uart/esp32/tx.h @@ -10,8 +10,8 @@ #define UART_TX_WRITE_INTR_MASK (UART_INTR_TXFIFO_EMPTY) -/* Custom, HAL does not provide any method to read a single byte */ -static inline void uart_tx_write_txfifo_byte(struct uart *uart, uint8_t byte) +/* Custom, HAL does not provide any method to write a single byte */ +static inline void uart_tx_fifo_putc(struct uart *uart, uint8_t byte) { WRITE_PERI_REG(UART_FIFO_AHB_REG(uart->port & UART_PORT_MASK), byte); } diff --git a/components/uart/esp8266/tx.c b/components/uart/esp8266/tx.c index e2a0cdcf..ad8397b8 100644 --- a/components/uart/esp8266/tx.c +++ b/components/uart/esp8266/tx.c @@ -34,7 +34,7 @@ int uart_tx_setup(struct uart *uart, struct uart_options options) return 0; } -int uart_tx_one(struct uart *uart, uint8_t byte) +int uart_tx_one(struct uart *uart, uint8_t byte, TickType_t timeout) { int ret; @@ -47,7 +47,7 @@ int uart_tx_one(struct uart *uart, uint8_t byte) ret = 0; - } else if (xStreamBufferSend(uart->tx_buffer, &byte, 1, portMAX_DELAY) > 0) { + } else if (xStreamBufferSend(uart->tx_buffer, &byte, 1, timeout) > 0) { LOG_ISR_DEBUG("tx buffer"); // byte was written diff --git a/components/uart/uart.c b/components/uart/uart.c index 0386aefb..d83c8273 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -382,7 +382,7 @@ int uart_putc(struct uart *uart, int ch) LOG_DEBUG("ch=%#02x", ch); - if ((ret = uart_tx_one(uart, ch))) { + if ((ret = uart_tx_one(uart, ch, portMAX_DELAY))) { goto error; } else { ret = ch; diff --git a/components/uart/uart.h b/components/uart/uart.h index 7950f105..a3cd7e43 100644 --- a/components/uart/uart.h +++ b/components/uart/uart.h @@ -80,7 +80,7 @@ void uart_rx_abort(struct uart *uart); int uart_tx_init(struct uart *uart, size_t tx_buffer_size); int uart_tx_setup(struct uart *uart, struct uart_options options); -int uart_tx_one(struct uart *uart, uint8_t byte); +int uart_tx_one(struct uart *uart, uint8_t byte, TickType_t timeout); size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len); size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_t timeout); From 9ba7eef28c91f8db1403c554b29d21a3966c48f2 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:24:30 +0300 Subject: [PATCH 11/21] uart: uart_read() timeout parameter --- components/dmx/dmx_input.c | 3 ++- components/stdio/vfs.c | 14 +++++--------- components/uart/include/uart.h | 12 +----------- components/uart/uart.c | 22 ++-------------------- components/uart/uart.h | 2 -- 5 files changed, 10 insertions(+), 43 deletions(-) diff --git a/components/dmx/dmx_input.c b/components/dmx/dmx_input.c index 2516817d..6d16e8df 100644 --- a/components/dmx/dmx_input.c +++ b/components/dmx/dmx_input.c @@ -242,7 +242,8 @@ int dmx_input_read (struct dmx_input *in) // read until data -> break while (!in->state_len || read) { WITH_STATS_TIMER(&in->stats.uart_rx) { - if ((read = uart_read(in->uart, buf, sizeof(buf))) < 0) { + // TODO: 1s timeout for most protocol states? + if ((read = uart_read(in->uart, buf, sizeof(buf), portMAX_DELAY)) < 0) { dmx_input_process_error(in, -read); return read; } diff --git a/components/stdio/vfs.c b/components/stdio/vfs.c index aa517a38..d1618ab5 100644 --- a/components/stdio/vfs.c +++ b/components/stdio/vfs.c @@ -16,6 +16,8 @@ #define STDIO_VFS_FD_START 0 #define STDIO_VFS_FD_COUNT 3 +static TickType_t stdin_read_timeout = portMAX_DELAY; + ssize_t stdio_vfs_write(int fd, const void *data, size_t size) { ssize_t ret; @@ -104,7 +106,7 @@ ssize_t stdio_vfs_read(int fd, void *data, size_t size) switch(fd) { case STDIN_FILENO: if (stdio_uart) { - return uart_read(stdio_uart, data, size); + return uart_read(stdio_uart, data, size, stdin_read_timeout); } else { errno = ENODEV; return -1; @@ -131,14 +133,8 @@ int stdio_vfs_fcntl_stdin(int cmd, int arg) return O_RDONLY; case F_SET_READ_TIMEOUT: - if (!stdio_uart) { - errno = ENODEV; - return -1; - } else if (uart_set_read_timeout(stdio_uart, arg ? arg : portMAX_DELAY)) { - errno = EIO; - return -1; - } - + stdin_read_timeout = arg ? arg : portMAX_DELAY; + return 0; default: diff --git a/components/uart/include/uart.h b/components/uart/include/uart.h index 675739f2..dca37417 100644 --- a/components/uart/include/uart.h +++ b/components/uart/include/uart.h @@ -99,9 +99,6 @@ struct uart_options { // invert TX signals uint32_t tx_inverted : 1; - // optional read() timeout, 0 -> portMAX_DELAY - TickType_t read_timeout; - // Acquire mutex before setting dev interrupts SemaphoreHandle_t dev_mutex; @@ -144,13 +141,6 @@ int uart_open(struct uart *uart, struct uart_options options); */ int uart_open_rx(struct uart *uart); -/** - * Set timeout for read(). - * - * @param timeout or portMAX_DELAY to disable - */ -int uart_set_read_timeout(struct uart *uart, TickType_t timeout); - /** * Read data from UART, copying up to size bytes into buf. * @@ -161,7 +151,7 @@ int uart_set_read_timeout(struct uart *uart, TickType_t timeout); * @return -EINTR interrupted using uart_abort_read() * @return -EINVAL RX disabled (rx_buffer_size=0) */ -int uart_read(struct uart *uart, void *buf, size_t size); +int uart_read(struct uart *uart, void *buf, size_t size, TickType_t timeout); /** * Cause the following uart_read() call, or any currently pending call, to return an error. diff --git a/components/uart/uart.c b/components/uart/uart.c index d83c8273..83a285db 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -116,8 +116,6 @@ int uart_setup(struct uart *uart, struct uart_options options) goto error; } - uart->read_timeout = options.read_timeout ? options.read_timeout : portMAX_DELAY; - xSemaphoreGiveRecursive(uart->tx_mutex); xSemaphoreGiveRecursive(uart->rx_mutex); @@ -191,22 +189,6 @@ int uart_open_rx(struct uart *uart) return 1; } -int uart_set_read_timeout(struct uart *uart, TickType_t timeout) -{ - int ret = 0; - - if (!xSemaphoreTakeRecursive(uart->rx_mutex, portMAX_DELAY)) { - LOG_ERROR("xSemaphoreTakeRecursive"); - return -1; - } - - uart->read_timeout = timeout; - - xSemaphoreGiveRecursive(uart->rx_mutex); - - return ret; -} - static int uart_acquire_rx(struct uart *uart) { if (!xSemaphoreTakeRecursive(uart->rx_mutex, portMAX_DELAY)) { @@ -232,7 +214,7 @@ static void uart_release_rx(struct uart *uart) xSemaphoreGiveRecursive(uart->rx_mutex); } -int uart_read(struct uart *uart, void *buf, size_t size) +int uart_read(struct uart *uart, void *buf, size_t size, TickType_t timeout) { int ret = 0; bool read = false; @@ -294,7 +276,7 @@ int uart_read(struct uart *uart, void *buf, size_t size) goto ret; } - ret = uart_rx_read(uart, buf, size, uart->read_timeout); + ret = uart_rx_read(uart, buf, size, timeout); read = true; } diff --git a/components/uart/uart.h b/components/uart/uart.h index a3cd7e43..7a4745b2 100644 --- a/components/uart/uart.h +++ b/components/uart/uart.h @@ -32,8 +32,6 @@ struct uart { StreamBufferHandle_t rx_buffer; bool rx_overflow, rx_break, rx_error, rx_abort; - TickType_t read_timeout; - /* TX */ SemaphoreHandle_t tx_mutex; StreamBufferHandle_t tx_buffer; From b5104a86fbfddcdf6a1962074621aad50fc25a50 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:25:36 +0300 Subject: [PATCH 12/21] cli: fix esp32 console timeout --- components/cli/CMakeLists.txt | 2 +- components/cli/cli.c | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/components/cli/CMakeLists.txt b/components/cli/CMakeLists.txt index bca3fd79..106e3560 100644 --- a/components/cli/CMakeLists.txt +++ b/components/cli/CMakeLists.txt @@ -1,6 +1,6 @@ idf_component_register( SRC_DIRS . INCLUDE_DIRS "include" - REQUIRES cmd + REQUIRES cmd stdio PRIV_REQUIRES logging ) diff --git a/components/cli/cli.c b/components/cli/cli.c index 83e585e4..d8cf1eba 100644 --- a/components/cli/cli.c +++ b/components/cli/cli.c @@ -2,9 +2,12 @@ #include #include -#if CONFIG_NEWLIB_VFS_STDIO -#define HAVE_STDIO_FCNTL 1 -#include +#if CONFIG_IDF_TARGET_ESP8266 && CONFIG_NEWLIB_VFS_STDIO + #define HAVE_STDIO_FCNTL 1 + #include +#elif !CONFIG_IDF_TARGET_ESP8266 && CONFIG_VFS_USE_STDIO + #define HAVE_STDIO_FCNTL 1 + #include #endif #include @@ -93,18 +96,21 @@ static int cli_open(struct cli *cli, TickType_t timeout) #if HAVE_STDIO_FCNTL if (fcntl(STDIN_FILENO, F_SET_READ_TIMEOUT, timeout) < 0) { LOG_WARN("fcntl stdin: %s", strerror(errno)); + } else { + LOG_INFO("fcntl stdin F_SET_READ_TIMEOUT %d", timeout); } #endif printf("! Use [ENTER] to open console\n"); while ((c = fgetc(stdin)) != EOF) { - if (c == '\r') { - continue; - } else if (c == '\n') { - break; - } else { - printf("! Use [ENTER] to open console, ignoring <%02x>\n", c); + switch (c) { + case '\r': + case '\n': + return 0; + + default: + printf("! Use [ENTER] to open console, ignoring <%02x>\n", c); } } From 58b57112ff8aa4a9dce5b7a4f12a6791e70fa1a8 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:25:47 +0300 Subject: [PATCH 13/21] main: flush stdout/stderr on console exit --- main/console.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/main/console.c b/main/console.c index 509df881..33b8e5cb 100644 --- a/main/console.c +++ b/main/console.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -148,6 +149,12 @@ void stop_console_uart() { LOG_INFO("closing console, use short CONFIG button press to re-start"); + fflush(stdout); + fflush(stderr); + + fsync(fileno(stdout)); + fsync(fileno(stderr)); + stdio_detach_uart(); if (uart_teardown(console_uart)) { From 9cf9bd386f9a651b68a163c353b33e3c1b054eb1 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:30:23 +0300 Subject: [PATCH 14/21] uart uart_tx_flush: timeout --- components/uart/esp32/tx.c | 7 +++++-- components/uart/esp8266/tx.c | 8 ++++++-- components/uart/uart.c | 2 +- components/uart/uart.h | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index 9ef3ca5b..c3a7b997 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -151,7 +151,7 @@ size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_ return write; } -int uart_tx_flush(struct uart *uart) +int uart_tx_flush(struct uart *uart, TickType_t timeout) { TaskHandle_t task = xTaskGetCurrentTaskHandle(); bool idle = false, done = false; @@ -191,8 +191,11 @@ int uart_tx_flush(struct uart *uart) LOG_DEBUG("wait done=%d...", done); // wait for tx to complete and break to start - if (!xTaskNotifyWait(0, 0, NULL, portMAX_DELAY)) { + if (!xTaskNotifyWait(0, 0, NULL, timeout)) { + uart->tx_done_notify_task = NULL; + LOG_WARN("timeout"); + return -1; } diff --git a/components/uart/esp8266/tx.c b/components/uart/esp8266/tx.c index ad8397b8..11985527 100644 --- a/components/uart/esp8266/tx.c +++ b/components/uart/esp8266/tx.c @@ -137,10 +137,12 @@ size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_ return write; } -int uart_tx_flush(struct uart *uart) +int uart_tx_flush(struct uart *uart, TickType_t timeout) { TaskHandle_t task = xTaskGetCurrentTaskHandle(); + xTaskNotifyStateClear(task); + taskENTER_CRITICAL(); // notify task once complete @@ -159,7 +161,9 @@ int uart_tx_flush(struct uart *uart) taskEXIT_CRITICAL(); // wait for tx to complete and break to start - if (!ulTaskNotifyTake(true, portMAX_DELAY)) { + if (!xTaskNotifyWait(0, 0, NULL, timeout)) { + uart->tx_done_notify_task = NULL; + LOG_WARN("timeout"); return -1; } diff --git a/components/uart/uart.c b/components/uart/uart.c index 83a285db..abcb6218 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -416,7 +416,7 @@ int uart_flush_write(struct uart *uart) return err; } - err = uart_tx_flush(uart); + err = uart_tx_flush(uart, portMAX_DELAY); uart_release_tx(uart); diff --git a/components/uart/uart.h b/components/uart/uart.h index 7a4745b2..6725d201 100644 --- a/components/uart/uart.h +++ b/components/uart/uart.h @@ -83,7 +83,7 @@ int uart_tx_one(struct uart *uart, uint8_t byte, TickType_t timeout); size_t uart_tx_fast(struct uart *uart, const uint8_t *buf, size_t len); size_t uart_tx_slow(struct uart *uart, const uint8_t *buf, size_t len, TickType_t timeout); -int uart_tx_flush(struct uart *uart); +int uart_tx_flush(struct uart *uart, TickType_t timeout); int uart_tx_break(struct uart *uart, unsigned bits); int uart_tx_mark(struct uart *uart, unsigned bits); From 8527481d7cdea432810f45fa874b318d2ce54a30 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:30:45 +0300 Subject: [PATCH 15/21] uart: write timeouts --- components/uart/include/uart.h | 6 +++--- components/uart/uart.c | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/components/uart/include/uart.h b/components/uart/include/uart.h index dca37417..92839ccf 100644 --- a/components/uart/include/uart.h +++ b/components/uart/include/uart.h @@ -180,19 +180,19 @@ int uart_open_tx(struct uart *uart); * * Returns ch on success, <0 on error. */ -int uart_putc(struct uart *uart, int ch); +int uart_putc(struct uart *uart, int ch, TickType_t timeout); /** * Write up to len bytes from buf. Blocks if TX buffer is full. * * Returns number of bytes written, or <0 on error. */ -ssize_t uart_write(struct uart *uart, const void *buf, size_t len); +ssize_t uart_write(struct uart *uart, const void *buf, size_t len, TickType_t timeout); /** * Wait for TX buffer + FIFO to empty. */ -int uart_flush_write(struct uart *uart); +int uart_flush_write(struct uart *uart, TickType_t timeout); /** * Flush -> idle, hold line low for >= `break_bits` bauds to signal break, and return line high (idle) for >= `mark_bits`. diff --git a/components/uart/uart.c b/components/uart/uart.c index abcb6218..1a190466 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -354,7 +354,7 @@ static void uart_release_tx(struct uart *uart) xSemaphoreGiveRecursive(uart->tx_mutex); } -int uart_putc(struct uart *uart, int ch) +int uart_putc(struct uart *uart, int ch, TickType_t timeout) { int ret; @@ -364,7 +364,7 @@ int uart_putc(struct uart *uart, int ch) LOG_DEBUG("ch=%#02x", ch); - if ((ret = uart_tx_one(uart, ch, portMAX_DELAY))) { + if ((ret = uart_tx_one(uart, ch, timeout))) { goto error; } else { ret = ch; @@ -376,7 +376,7 @@ int uart_putc(struct uart *uart, int ch) return ret; } -ssize_t uart_write(struct uart *uart, const void *buf, size_t len) +ssize_t uart_write(struct uart *uart, const void *buf, size_t len, TickType_t timeout) { size_t write = 0; int err; @@ -393,9 +393,9 @@ ssize_t uart_write(struct uart *uart, const void *buf, size_t len) buf += write; len -= write; - if (!write) { + if (!write && timeout) { // blocking slowpath via buffer + ISR - write = uart_tx_slow(uart, buf, len, portMAX_DELAY); + write = uart_tx_slow(uart, buf, len, timeout); LOG_DEBUG("tx slow len=%u: write=%u", len, write); @@ -408,7 +408,7 @@ ssize_t uart_write(struct uart *uart, const void *buf, size_t len) return write; } -int uart_flush_write(struct uart *uart) +int uart_flush_write(struct uart *uart, TickType_t timeout) { int err; @@ -416,7 +416,7 @@ int uart_flush_write(struct uart *uart) return err; } - err = uart_tx_flush(uart, portMAX_DELAY); + err = uart_tx_flush(uart, timeout); uart_release_tx(uart); From 47736103543b55905d0500414497429f4034fbe0 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:36:28 +0300 Subject: [PATCH 16/21] uart: close/flush timeouts --- components/uart/include/uart.h | 8 ++++---- components/uart/uart.c | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/components/uart/include/uart.h b/components/uart/include/uart.h index 92839ccf..e090c96d 100644 --- a/components/uart/include/uart.h +++ b/components/uart/include/uart.h @@ -202,7 +202,7 @@ int uart_flush_write(struct uart *uart, TickType_t timeout); * * Return <0 on error. */ -int uart_break(struct uart *uart, unsigned break_bits, unsigned mark_bits); +int uart_break(struct uart *uart, unsigned break_bits, unsigned mark_bits, TickType_t timeout); /** * Flush, and hold mark for >= `mark_bits` bauds once TX is complete. @@ -211,19 +211,19 @@ int uart_break(struct uart *uart, unsigned break_bits, unsigned mark_bits); * * Return <0 on error. */ -int uart_mark(struct uart *uart, unsigned mark_bits); +int uart_mark(struct uart *uart, unsigned mark_bits, TickType_t timeout); /** * Flush TX and release TX mutex acquire dusing `uart_open_tx()`. */ -int uart_close_tx(struct uart *uart); +int uart_close_tx(struct uart *uart, TickType_t timeout); /** * Flush TX/RX and release rx/tx mutex acquired using `uart_open()`. * * WARNING: This does not release any intr, dev or pin state acquired by `uart_open()` -> `uart_setup()`! Use `uart_teardown()`! */ -int uart_close(struct uart *uart); +int uart_close(struct uart *uart, TickType_t timeout); /* * Stop UART interrupts and disassocate from UART device. diff --git a/components/uart/uart.c b/components/uart/uart.c index 1a190466..4f5e3968 100644 --- a/components/uart/uart.c +++ b/components/uart/uart.c @@ -100,7 +100,7 @@ int uart_setup(struct uart *uart, struct uart_options options) } // wait TX idle - if ((err = uart_tx_flush(uart))) { + if ((err = uart_tx_flush(uart, portMAX_DELAY))) { LOG_ERROR("uart_tx_flush"); goto error; } @@ -423,7 +423,7 @@ int uart_flush_write(struct uart *uart, TickType_t timeout) return err; } -int uart_break(struct uart *uart, unsigned break_bits, unsigned mark_bits) +int uart_break(struct uart *uart, unsigned break_bits, unsigned mark_bits, TickType_t timeout) { int err; @@ -433,7 +433,7 @@ int uart_break(struct uart *uart, unsigned break_bits, unsigned mark_bits) LOG_DEBUG("break_bits=%u mark_bits=%u", break_bits, mark_bits); - if ((err = uart_tx_flush(uart))) { + if ((err = uart_tx_flush(uart, timeout))) { LOG_ERROR("uart_tx_flush"); goto error; } @@ -464,7 +464,7 @@ int uart_break(struct uart *uart, unsigned break_bits, unsigned mark_bits) return err; } -int uart_mark(struct uart *uart, unsigned mark_bits) +int uart_mark(struct uart *uart, unsigned mark_bits, TickType_t timeout) { int err; @@ -474,7 +474,7 @@ int uart_mark(struct uart *uart, unsigned mark_bits) LOG_DEBUG("mark_bits=%u", mark_bits); - if ((err = uart_tx_flush(uart))) { + if ((err = uart_tx_flush(uart, timeout))) { LOG_ERROR("uart_tx_flush"); goto error; } @@ -492,7 +492,7 @@ int uart_mark(struct uart *uart, unsigned mark_bits) return err; } -int uart_close_tx(struct uart *uart) +int uart_close_tx(struct uart *uart, TickType_t timeout) { int err; @@ -500,7 +500,7 @@ int uart_close_tx(struct uart *uart) return err; } - if ((err = uart_tx_flush(uart))) { + if ((err = uart_tx_flush(uart, timeout))) { LOG_ERROR("uart_tx_flush"); goto error; } @@ -516,7 +516,7 @@ int uart_close_tx(struct uart *uart) } /* setup/open */ -int uart_close(struct uart *uart) +int uart_close(struct uart *uart, TickType_t timeout) { int err; @@ -524,7 +524,7 @@ int uart_close(struct uart *uart) return err; } - if ((err = uart_tx_flush(uart))) { + if ((err = uart_tx_flush(uart, timeout))) { LOG_ERROR("uart_tx_flush"); goto error; } From 09d851691ef9a81f307d06856b56435ddd8366bb Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:36:50 +0300 Subject: [PATCH 17/21] fix uart calls to pass timeout --- components/dmx/dmx_output.c | 10 +++++----- components/leds/interfaces/uart/tx.c | 10 +++++----- components/stdio/vfs.c | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/components/dmx/dmx_output.c b/components/dmx/dmx_output.c index 505c8f92..3c65041c 100644 --- a/components/dmx/dmx_output.c +++ b/components/dmx/dmx_output.c @@ -115,12 +115,12 @@ int dmx_output_write (struct dmx_output *out, enum dmx_cmd cmd, void *data, size WITH_STATS_TIMER(&out->stats.uart_tx) { // send break/mark per spec minimums for transmit; actual timings will vary, these are minimums - if ((err = uart_break(out->uart, DMX_BREAK_BITS, DMX_MARK_AFTER_BREAK_BITS))) { + if ((err = uart_break(out->uart, DMX_BREAK_BITS, DMX_MARK_AFTER_BREAK_BITS, portMAX_DELAY))) { LOG_ERROR("uart1_break"); goto error; } - if ((err = uart_putc(out->uart, cmd)) < 0) { + if ((err = uart_putc(out->uart, cmd, portMAX_DELAY)) < 0) { LOG_ERROR("uart_putc"); goto error; } @@ -128,7 +128,7 @@ int dmx_output_write (struct dmx_output *out, enum dmx_cmd cmd, void *data, size for (uint8_t *ptr = data; len > 0; ) { ssize_t write; - if ((write = uart_write(out->uart, ptr, len)) < 0) { + if ((write = uart_write(out->uart, ptr, len, portMAX_DELAY)) < 0) { LOG_ERROR("uart_write"); err = write; goto error; @@ -138,7 +138,7 @@ int dmx_output_write (struct dmx_output *out, enum dmx_cmd cmd, void *data, size len -= write; } - if ((err = uart_flush_write(out->uart))) { + if ((err = uart_flush_write(out->uart, portMAX_DELAY))) { LOG_ERROR("uart_flush_write"); goto error; } @@ -164,7 +164,7 @@ int dmx_output_close (struct dmx_output *out) gpio_out_clear(out->options.gpio_options); } - if (uart_close_tx(out->uart)) { + if (uart_close_tx(out->uart, portMAX_DELAY)) { LOG_WARN("uart_close_tx"); } diff --git a/components/leds/interfaces/uart/tx.c b/components/leds/interfaces/uart/tx.c index a9fc9bf8..e899036d 100644 --- a/components/leds/interfaces/uart/tx.c +++ b/components/leds/interfaces/uart/tx.c @@ -68,7 +68,7 @@ static int leds_interface_uart_write(struct leds_interface_uart *interface, void int write; while (len) { - if ((write = uart_write(interface->uart, ptr, len)) < 0) { + if ((write = uart_write(interface->uart, ptr, len, portMAX_DELAY)) < 0) { return write; } @@ -106,13 +106,13 @@ int leds_interface_uart_tx_reset(struct leds_interface_uart *interface) { switch (interface->mode) { case LEDS_INTERFACE_UART_MODE_24B3I7_0U4_80U: - return uart_mark(interface->uart, 80); + return uart_mark(interface->uart, 80, portMAX_DELAY); case LEDS_INTERFACE_UART_MODE_24B2I8_0U25_50U: - return uart_mark(interface->uart, 50); + return uart_mark(interface->uart, 50, portMAX_DELAY); case LEDS_INTERFACE_UART_MODE_32B2I6_0U3_80U: - return uart_mark(interface->uart, 80); + return uart_mark(interface->uart, 80, portMAX_DELAY); default: LOG_FATAL("invalid mode=%d", interface->mode); @@ -161,7 +161,7 @@ int leds_interface_uart_tx(struct leds_interface_uart *interface, const struct l leds_gpio_close(&interface->gpio); #endif - if ((err = uart_close(interface->uart))) { + if ((err = uart_close(interface->uart, portMAX_DELAY))) { LOG_ERROR("uart_close"); return err; } diff --git a/components/stdio/vfs.c b/components/stdio/vfs.c index d1618ab5..556b29a4 100644 --- a/components/stdio/vfs.c +++ b/components/stdio/vfs.c @@ -25,7 +25,7 @@ ssize_t stdio_vfs_write(int fd, const void *data, size_t size) switch(fd) { case STDOUT_FILENO: if (stdio_uart) { - return uart_write(stdio_uart, data, size); + return uart_write(stdio_uart, data, size, portMAX_DELAY); } else { os_write(data, size); return size; @@ -35,7 +35,7 @@ ssize_t stdio_vfs_write(int fd, const void *data, size_t size) if (!stdio_uart) { os_write(data, size); } else { - if ((ret = uart_write(stdio_uart, data, size)) < 0) { + if ((ret = uart_write(stdio_uart, data, size, portMAX_DELAY)) < 0) { errno = EIO; return -1; } else { @@ -193,7 +193,7 @@ int stdio_vfs_fsync(int fd) case STDOUT_FILENO: case STDERR_FILENO: if (stdio_uart) { - return uart_flush_write(stdio_uart); + return uart_flush_write(stdio_uart, portMAX_DELAY); } else { errno = ENODEV; return -1; From 0ef5576d1e8efc4e5f9518bcd819752d54e57b5b Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:37:17 +0300 Subject: [PATCH 18/21] stdio: LOG_DEBUG stdin_read_timeout --- components/stdio/vfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/stdio/vfs.c b/components/stdio/vfs.c index 556b29a4..ee43102b 100644 --- a/components/stdio/vfs.c +++ b/components/stdio/vfs.c @@ -134,7 +134,9 @@ int stdio_vfs_fcntl_stdin(int cmd, int arg) case F_SET_READ_TIMEOUT: stdin_read_timeout = arg ? arg : portMAX_DELAY; - + + LOG_DEBUG("stdin_read_timeout = %d", stdin_read_timeout); + return 0; default: From 9cd38339762e62377086c91748e8e7d2a652f06c Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 21:41:01 +0300 Subject: [PATCH 19/21] uart: wrapper func for txd_inv --- components/uart/esp32/tx.c | 10 +++++----- components/uart/esp32/tx.h | 5 +++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/components/uart/esp32/tx.c b/components/uart/esp32/tx.c index c3a7b997..cbe52349 100644 --- a/components/uart/esp32/tx.c +++ b/components/uart/esp32/tx.c @@ -195,7 +195,7 @@ int uart_tx_flush(struct uart *uart, TickType_t timeout) uart->tx_done_notify_task = NULL; LOG_WARN("timeout"); - + return -1; } @@ -222,17 +222,17 @@ static void uart_tx_wait(struct uart *uart, unsigned bits) ets_delay_us(us); } -// ESP-32 txd_brk is tied to TX_DONE, and does nothing if TX idle +// ESP-32 txd_brk is tied to TX_DONE, and does nothing if TX idle? // called after uart_tx_flush() with tx mutex held int uart_tx_break(struct uart *uart, unsigned bits) { LOG_DEBUG("bits=%u", bits); - - uart->dev->conf0.txd_inv = 1; + + uart_tx_inv(uart, 1); uart_tx_wait(uart, bits); - uart->dev->conf0.txd_inv = 0; + uart_tx_inv(uart, 0); LOG_DEBUG("done"); diff --git a/components/uart/esp32/tx.h b/components/uart/esp32/tx.h index 45a7232e..b7557352 100644 --- a/components/uart/esp32/tx.h +++ b/components/uart/esp32/tx.h @@ -15,3 +15,8 @@ static inline void uart_tx_fifo_putc(struct uart *uart, uint8_t byte) { WRITE_PERI_REG(UART_FIFO_AHB_REG(uart->port & UART_PORT_MASK), byte); } + +static inline void uart_tx_inv(struct uart *uart, bool inv) +{ + uart->dev->conf0.txd_inv = inv; +} From 886b3b325803b00febbb65ba78e130ce1ad99167 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 22:09:14 +0300 Subject: [PATCH 20/21] uart: esp8266 fix --- components/uart/esp8266/tx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/uart/esp8266/tx.c b/components/uart/esp8266/tx.c index 11985527..ade9cc1e 100644 --- a/components/uart/esp8266/tx.c +++ b/components/uart/esp8266/tx.c @@ -162,7 +162,7 @@ int uart_tx_flush(struct uart *uart, TickType_t timeout) // wait for tx to complete and break to start if (!xTaskNotifyWait(0, 0, NULL, timeout)) { - uart->tx_done_notify_task = NULL; + uart->txfifo_empty_notify_task = NULL; LOG_WARN("timeout"); return -1; From ec49324319a3574a1c2978446fe30590ce816c3d Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 23 Apr 2025 22:09:18 +0300 Subject: [PATCH 21/21] fix esp8266 build --- main/wifi_http.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main/wifi_http.c b/main/wifi_http.c index 149908ce..f664d5ba 100644 --- a/main/wifi_http.c +++ b/main/wifi_http.c @@ -97,7 +97,9 @@ static int wifi_api_write_ap_sta_object(struct json_writer *w, const wifi_sta_in { return ( JSON_WRITE_MEMBER_BSSID(w, "mac", wifi_sta->mac) || + #if !CONFIG_IDF_TARGET_ESP8266 JSON_WRITE_MEMBER_INT(w, "rssi", wifi_sta->rssi) || + #endif JSON_WRITE_MEMBER_BOOL(w, "phy_11b", wifi_sta->phy_11b) || JSON_WRITE_MEMBER_BOOL(w, "phy_11g", wifi_sta->phy_11g) || JSON_WRITE_MEMBER_BOOL(w, "phy_11n", wifi_sta->phy_11n) ||