From fdbfac15db79a329629173c1d6ba967f12b43e46 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 28 Feb 2026 14:21:08 -1000 Subject: [PATCH] [uart] Replace wake-on-RX task+queue with direct ISR callback (#14382) --- .../uart/uart_component_esp_idf.cpp | 101 +++--------------- .../components/uart/uart_component_esp_idf.h | 21 ++-- esphome/core/application.h | 10 ++ esphome/core/lwip_fast_select.c | 14 +++ esphome/core/lwip_fast_select.h | 5 + 5 files changed, 52 insertions(+), 99 deletions(-) diff --git a/esphome/components/uart/uart_component_esp_idf.cpp b/esphome/components/uart/uart_component_esp_idf.cpp index 8699d37d7a..631db1e5c7 100644 --- a/esphome/components/uart/uart_component_esp_idf.cpp +++ b/esphome/components/uart/uart_component_esp_idf.cpp @@ -2,7 +2,6 @@ #include "uart_component_esp_idf.h" #include -#include "esphome/core/application.h" #include "esphome/core/defines.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" @@ -10,6 +9,10 @@ #include "driver/gpio.h" #include "soc/gpio_num.h" +#ifdef USE_UART_WAKE_LOOP_ON_RX +#include "esphome/core/application.h" +#endif + #ifdef USE_LOGGER #include "esphome/components/logger/logger.h" #endif @@ -107,12 +110,6 @@ void IDFUARTComponent::load_settings(bool dump_config) { esp_err_t err; if (uart_is_driver_installed(this->uart_num_)) { -#ifdef USE_UART_WAKE_LOOP_ON_RX - if (this->rx_event_task_handle_ != nullptr) { - vTaskDelete(this->rx_event_task_handle_); - this->rx_event_task_handle_ = nullptr; - } -#endif err = uart_driver_delete(this->uart_num_); if (err != ESP_OK) { ESP_LOGW(TAG, "uart_driver_delete failed: %s", esp_err_to_name(err)); @@ -120,20 +117,13 @@ void IDFUARTComponent::load_settings(bool dump_config) { return; } } -#ifdef USE_UART_WAKE_LOOP_ON_RX - constexpr int event_queue_size = 20; - QueueHandle_t *event_queue_ptr = &this->uart_event_queue_; -#else - constexpr int event_queue_size = 0; - QueueHandle_t *event_queue_ptr = nullptr; -#endif err = uart_driver_install(this->uart_num_, // UART number this->rx_buffer_size_, // RX ring buffer size 0, // TX ring buffer size. If zero, driver will not use a TX buffer and TX function will // block task until all data has been sent out - event_queue_size, // event queue size/depth - event_queue_ptr, // event queue - 0 // Flags used to allocate the interrupt + 0, // event queue size/depth + nullptr, // event queue + 0 // Flags used to allocate the interrupt ); if (err != ESP_OK) { ESP_LOGW(TAG, "uart_driver_install failed: %s", esp_err_to_name(err)); @@ -213,8 +203,10 @@ void IDFUARTComponent::load_settings(bool dump_config) { } #ifdef USE_UART_WAKE_LOOP_ON_RX - // Start the RX event task to enable low-latency data notifications - this->start_rx_event_task_(); + // Register ISR callback to wake the main loop when UART data arrives. + // The callback runs in ISR context and uses vTaskNotifyGiveFromISR() to + // wake the main loop task directly — no queue or FreeRTOS task needed. + uart_set_select_notif_callback(this->uart_num_, IDFUARTComponent::uart_rx_isr_callback); #endif // USE_UART_WAKE_LOOP_ON_RX if (dump_config) { @@ -345,71 +337,12 @@ void IDFUARTComponent::flush() { void IDFUARTComponent::check_logger_conflict() {} #ifdef USE_UART_WAKE_LOOP_ON_RX -void IDFUARTComponent::start_rx_event_task_() { - // Create FreeRTOS task to monitor UART events - BaseType_t result = xTaskCreate(rx_event_task_func, // Task function - "uart_rx_evt", // Task name (max 16 chars) - 2240, // Stack size in bytes (~2.2KB); increase if needed for logging - this, // Task parameter (this pointer) - tskIDLE_PRIORITY + 1, // Priority (low, just above idle) - &this->rx_event_task_handle_ // Task handle - ); - - if (result != pdPASS) { - ESP_LOGE(TAG, "Failed to create RX event task"); - return; - } - - ESP_LOGV(TAG, "RX event task started"); -} - -// FreeRTOS task that relays UART ISR events to the main loop. -// This task exists because wake_loop_threadsafe() is not ISR-safe (it uses a -// UDP loopback socket), so we need a task as an ISR-to-main-loop trampoline. -// IMPORTANT: This task must NOT call any UART wrapper methods (read_array, -// write_array, peek_byte, etc.) or touch has_peek_/peek_byte_ — all reading -// is done by the main loop. This task only reads from the event queue and -// calls App.wake_loop_threadsafe(). -void IDFUARTComponent::rx_event_task_func(void *param) { - auto *self = static_cast(param); - uart_event_t event; - - ESP_LOGV(TAG, "RX event task running"); - - // Run forever - task lifecycle matches component lifecycle - while (true) { - // Wait for UART events (blocks efficiently) - if (xQueueReceive(self->uart_event_queue_, &event, portMAX_DELAY) == pdTRUE) { - switch (event.type) { - case UART_DATA: - // Data available in UART RX buffer - wake the main loop - ESP_LOGVV(TAG, "Data event: %d bytes", event.size); -#if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_WAKE_LOOP_THREADSAFE) - App.wake_loop_threadsafe(); -#endif - break; - - case UART_FIFO_OVF: - case UART_BUFFER_FULL: - // Don't call uart_flush_input() here — this task does not own the read side. - // ESP-IDF examples flush on overflow because the same task handles both events - // and reads, so flush and read are serialized. Here, reads happen on the main - // loop, so flushing from this task races with read_array() and can destroy data - // mid-read. The driver self-heals without an explicit flush: uart_read_bytes() - // calls uart_check_buf_full() after each chunk, which moves stashed FIFO bytes - // into the ring buffer and re-enables RX interrupts once space is freed. - ESP_LOGW(TAG, "FIFO overflow or ring buffer full"); -#if defined(USE_SOCKET_SELECT_SUPPORT) && defined(USE_WAKE_LOOP_THREADSAFE) - App.wake_loop_threadsafe(); -#endif - break; - - default: - // Ignore other event types - ESP_LOGVV(TAG, "Event type: %d", event.type); - break; - } - } +// ISR callback invoked by the ESP-IDF UART driver when data arrives. +// Wakes the main loop directly via vTaskNotifyGiveFromISR() — no queue or task needed. +void IRAM_ATTR IDFUARTComponent::uart_rx_isr_callback(uart_port_t uart_num, uart_select_notif_t uart_select_notif, + BaseType_t *task_woken) { + if (uart_select_notif == UART_SELECT_READ_NOTIF) { + Application::wake_loop_isrsafe(task_woken); } } #endif // USE_UART_WAKE_LOOP_ON_RX diff --git a/esphome/components/uart/uart_component_esp_idf.h b/esphome/components/uart/uart_component_esp_idf.h index 1517eab509..631bf54cd5 100644 --- a/esphome/components/uart/uart_component_esp_idf.h +++ b/esphome/components/uart/uart_component_esp_idf.h @@ -5,6 +5,9 @@ #include #include "esphome/core/component.h" #include "uart_component.h" +#ifdef USE_UART_WAKE_LOOP_ON_RX +#include +#endif namespace esphome::uart { @@ -12,9 +15,7 @@ namespace esphome::uart { /// /// Thread safety: All public methods must only be called from the main loop. /// The ESP-IDF UART driver API does not guarantee thread safety, and ESPHome's -/// peek byte state (has_peek_/peek_byte_) is not synchronized. The rx_event_task -/// (when enabled) must not call any of these methods — it communicates with the -/// main loop exclusively via App.wake_loop_threadsafe(). +/// peek byte state (has_peek_/peek_byte_) is not synchronized. class IDFUARTComponent : public UARTComponent, public Component { public: void setup() override; @@ -33,9 +34,6 @@ class IDFUARTComponent : public UARTComponent, public Component { void flush() override; uint8_t get_hw_serial_number() { return this->uart_num_; } -#ifdef USE_UART_WAKE_LOOP_ON_RX - QueueHandle_t *get_uart_event_queue() { return &this->uart_event_queue_; } -#endif /** * Load the UART with the current settings. @@ -61,15 +59,8 @@ class IDFUARTComponent : public UARTComponent, public Component { uint8_t peek_byte_; #ifdef USE_UART_WAKE_LOOP_ON_RX - // RX notification support — runs on a separate FreeRTOS task. - // IMPORTANT: rx_event_task_func must NOT call any UART wrapper methods (read_array, - // write_array, etc.) or touch has_peek_/peek_byte_. It must only read from the - // event queue and call App.wake_loop_threadsafe(). - void start_rx_event_task_(); - static void rx_event_task_func(void *param); - - QueueHandle_t uart_event_queue_; - TaskHandle_t rx_event_task_handle_{nullptr}; + // ISR callback for UART RX data notification — wakes the main loop directly. + static void uart_rx_isr_callback(uart_port_t uart_num, uart_select_notif_t uart_select_notif, BaseType_t *task_woken); #endif // USE_UART_WAKE_LOOP_ON_RX }; diff --git a/esphome/core/application.h b/esphome/core/application.h index ba3ce13291..13e0f63885 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -500,6 +500,16 @@ class Application { /// On other platforms: uses UDP loopback socket void wake_loop_threadsafe(); #endif + +#if defined(USE_WAKE_LOOP_THREADSAFE) && defined(USE_LWIP_FAST_SELECT) + /// Wake the main event loop from an ISR. + /// Uses vTaskNotifyGiveFromISR() — <1 us, ISR-safe. + /// Only available on platforms with fast select (ESP32, LibreTiny). + /// @param px_higher_priority_task_woken Set to pdTRUE if a context switch is needed. + static void IRAM_ATTR wake_loop_isrsafe(int *px_higher_priority_task_woken) { + esphome_lwip_wake_main_loop_from_isr(px_higher_priority_task_woken); + } +#endif #endif protected: diff --git a/esphome/core/lwip_fast_select.c b/esphome/core/lwip_fast_select.c index 88cf23b67e..da0f1f337a 100644 --- a/esphome/core/lwip_fast_select.c +++ b/esphome/core/lwip_fast_select.c @@ -126,6 +126,12 @@ #include +// IRAM_ATTR is defined by esp_attr.h (included via FreeRTOS headers) on ESP32. +// On LibreTiny it's not defined — provide a no-op fallback. +#ifndef IRAM_ATTR +#define IRAM_ATTR +#endif + // Compile-time verification of thread safety assumptions. // On ESP32 (Xtensa/RISC-V) and LibreTiny (ARM Cortex-M), naturally-aligned // reads/writes up to 32 bits are atomic. @@ -225,4 +231,12 @@ void esphome_lwip_wake_main_loop(void) { } } +// Wake the main loop from an ISR. ISR-safe variant. +void IRAM_ATTR esphome_lwip_wake_main_loop_from_isr(int *px_higher_priority_task_woken) { + TaskHandle_t task = s_main_loop_task; + if (task != NULL) { + vTaskNotifyGiveFromISR(task, (BaseType_t *) px_higher_priority_task_woken); + } +} + #endif // defined(USE_ESP32) || defined(USE_LIBRETINY) diff --git a/esphome/core/lwip_fast_select.h b/esphome/core/lwip_fast_select.h index b08c946212..b7a70a8f9f 100644 --- a/esphome/core/lwip_fast_select.h +++ b/esphome/core/lwip_fast_select.h @@ -28,6 +28,11 @@ void esphome_lwip_hook_socket(int fd); /// NOT ISR-safe — must only be called from task context. void esphome_lwip_wake_main_loop(void); +/// Wake the main loop task from an ISR — costs <1 us. +/// ISR-safe variant using vTaskNotifyGiveFromISR(). +/// @param px_higher_priority_task_woken Set to pdTRUE if a context switch is needed. +void esphome_lwip_wake_main_loop_from_isr(int *px_higher_priority_task_woken); + #ifdef __cplusplus } #endif