mirror/esphome
1
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2026-02-18 23:45:40 -07:00
Code Issues Packages Projects Releases Wiki Activity

[uart] Remove redundant mutex, fix flush race, conditional event queue (#13955)

Browse Source
This commit is contained in:
J. Nick Koston
2026-02-12 10:26:10 -06:00
committed by GitHub
parent 7b251dcc31
commit 9aa98ed6c6
2 changed files with 41 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
esphome/components/uart/uart_component_esp_idf.cpp
View File

@@ -90,7 +90,6 @@ void IDFUARTComponent::setup() {
return;
}
this->uart_num_ = static_cast<uart_port_t>(next_uart_num++);
this->lock_ = xSemaphoreCreateMutex();
#if (SOC_UART_LP_NUM >= 1)
size_t fifo_len = ((this->uart_num_ < SOC_UART_HP_NUM) ? SOC_UART_FIFO_LEN : SOC_LP_UART_FIFO_LEN);
@@ -102,11 +101,7 @@ void IDFUARTComponent::setup() {
this->rx_buffer_size_ = fifo_len * 2;
}
xSemaphoreTake(this->lock_, portMAX_DELAY);
this->load_settings(false);
xSemaphoreGive(this->lock_);
}
void IDFUARTComponent::load_settings(bool dump_config) {
@@ -126,13 +121,20 @@ 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
20, // event queue size/depth
&this->uart_event_queue_, // event queue
0 // Flags used to allocate the interrupt
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
);
if (err != ESP_OK) {
ESP_LOGW(TAG, "uart_driver_install failed: %s", esp_err_to_name(err));
@@ -282,9 +284,7 @@ void IDFUARTComponent::set_rx_timeout(size_t rx_timeout) {
}
void IDFUARTComponent::write_array(const uint8_t *data, size_t len) {
xSemaphoreTake(this->lock_, portMAX_DELAY);
int32_t write_len = uart_write_bytes(this->uart_num_, data, len);
xSemaphoreGive(this->lock_);
if (write_len != (int32_t) len) {
ESP_LOGW(TAG, "uart_write_bytes failed: %d != %zu", write_len, len);
this->mark_failed();
@@ -299,7 +299,6 @@ void IDFUARTComponent::write_array(const uint8_t *data, size_t len) {
bool IDFUARTComponent::peek_byte(uint8_t *data) {
if (!this->check_read_timeout_())
return false;
xSemaphoreTake(this->lock_, portMAX_DELAY);
if (this->has_peek_) {
*data = this->peek_byte_;
} else {
@@ -311,7 +310,6 @@ bool IDFUARTComponent::peek_byte(uint8_t *data) {
this->peek_byte_ = *data;
}
}
xSemaphoreGive(this->lock_);
return true;
}
@@ -320,7 +318,6 @@ bool IDFUARTComponent::read_array(uint8_t *data, size_t len) {
int32_t read_len = 0;
if (!this->check_read_timeout_(len))
return false;
xSemaphoreTake(this->lock_, portMAX_DELAY);
if (this->has_peek_) {
length_to_read--;
*data = this->peek_byte_;
@@ -329,7 +326,6 @@ bool IDFUARTComponent::read_array(uint8_t *data, size_t len) {
}
if (length_to_read > 0)
read_len = uart_read_bytes(this->uart_num_, data, length_to_read, 20 / portTICK_PERIOD_MS);
xSemaphoreGive(this->lock_);
#ifdef USE_UART_DEBUGGER
for (size_t i = 0; i < len; i++) {
this->debug_callback_.call(UART_DIRECTION_RX, data[i]);
@@ -342,9 +338,7 @@ size_t IDFUARTComponent::available() {
size_t available = 0;
esp_err_t err;
xSemaphoreTake(this->lock_, portMAX_DELAY);
err = uart_get_buffered_data_len(this->uart_num_, &available);
xSemaphoreGive(this->lock_);
if (err != ESP_OK) {
ESP_LOGW(TAG, "uart_get_buffered_data_len failed: %s", esp_err_to_name(err));
@@ -358,9 +352,7 @@ size_t IDFUARTComponent::available() {
void IDFUARTComponent::flush() {
ESP_LOGVV(TAG, " Flushing");
xSemaphoreTake(this->lock_, portMAX_DELAY);
uart_wait_tx_done(this->uart_num_, portMAX_DELAY);
xSemaphoreGive(this->lock_);
}
void IDFUARTComponent::check_logger_conflict() {}
@@ -384,6 +376,13 @@ void IDFUARTComponent::start_rx_event_task_() {
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<IDFUARTComponent *>(param);
uart_event_t event;
@@ -405,8 +404,14 @@ void IDFUARTComponent::rx_event_task_func(void *param) {
case UART_FIFO_OVF:
case UART_BUFFER_FULL:
ESP_LOGW(TAG, "FIFO overflow or ring buffer full - clearing");
uart_flush_input(self->uart_num_);
// 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

17
esphome/components/uart/uart_component_esp_idf.h
View File

@@ -8,6 +8,13 @@
namespace esphome::uart {
/// ESP-IDF UART driver wrapper.
///
/// 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().
class IDFUARTComponent : public UARTComponent, public Component {
public:
void setup() override;
@@ -26,7 +33,9 @@ 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.
@@ -46,18 +55,20 @@ class IDFUARTComponent : public UARTComponent, public Component {
protected:
void check_logger_conflict() override;
uart_port_t uart_num_;
QueueHandle_t uart_event_queue_;
uart_config_t get_config_();
SemaphoreHandle_t lock_;
bool has_peek_{false};
uint8_t peek_byte_;
#ifdef USE_UART_WAKE_LOOP_ON_RX
// RX notification support
// 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};
#endif // USE_UART_WAKE_LOOP_ON_RX
};