From ce784299d8d3d451dcae69638741ed9a04c3a260 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:23:58 -0500 Subject: [PATCH 01/26] wip --- esphome/components/usb_host/usb_host.h | 41 ++++++- .../components/usb_host/usb_host_client.cpp | 104 ++++++++++++++++-- esphome/components/usb_uart/usb_uart.cpp | 80 +++++++++++--- esphome/components/usb_uart/usb_uart.h | 31 +++++- 4 files changed, 229 insertions(+), 27 deletions(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index c5466eb1f0..3625100e4a 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -5,7 +5,10 @@ #include "esphome/core/component.h" #include #include "usb/usb_host.h" - +#include +#include +#include +#include #include namespace esphome { @@ -13,6 +16,10 @@ namespace usb_host { static const char *const TAG = "usb_host"; +// Forward declarations +struct TransferRequest; +class USBClient; + // constants for setup packet type static const uint8_t USB_RECIP_DEVICE = 0; static const uint8_t USB_RECIP_INTERFACE = 1; @@ -49,6 +56,30 @@ struct TransferRequest { USBClient *client; }; +// Lightweight event types for queue +enum EventType { + EVENT_DEVICE_NEW, + EVENT_DEVICE_GONE, + EVENT_TRANSFER_COMPLETE, + EVENT_CONTROL_COMPLETE, +}; + +struct UsbEvent { + EventType type; + union { + struct { + uint8_t address; + } device_new; + struct { + usb_device_handle_t handle; + } device_gone; + struct { + TransferRequest *trq; + bool callback_executed; // Flag to indicate callback was already executed in USB task + } transfer; + } data; +}; + // callback function type. enum ClientState { @@ -83,6 +114,7 @@ class USBClient : public Component { void release_trq(TransferRequest *trq); bool control_transfer(uint8_t type, uint8_t request, uint16_t value, uint16_t index, const transfer_cb_t &callback, const std::vector &data = {}); + QueueHandle_t get_event_queue() { return event_queue_; } protected: bool register_(); @@ -91,6 +123,13 @@ class USBClient : public Component { virtual void on_connected() {} virtual void on_disconnected() { this->init_pool(); } + // USB task management + static void usb_task_fn(void *arg); + void usb_task_loop(); + + TaskHandle_t usb_task_handle_{nullptr}; + QueueHandle_t event_queue_{nullptr}; // Queue of UsbEvent structs + usb_host_client_handle_t handle_{}; usb_device_handle_t device_handle_{}; int device_addr_{-1}; diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index 4c0c12fa18..98a1f6178b 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -139,18 +139,25 @@ static std::string get_descriptor_string(const usb_str_desc_t *desc) { return {buffer}; } +// CALLBACK CONTEXT: USB task (called from usb_host_client_handle_events in USB task) static void client_event_cb(const usb_host_client_event_msg_t *event_msg, void *ptr) { auto *client = static_cast(ptr); + UsbEvent event; + + // Queue events to be processed in main loop switch (event_msg->event) { case USB_HOST_CLIENT_EVENT_NEW_DEV: { - auto addr = event_msg->new_dev.address; ESP_LOGD(TAG, "New device %d", event_msg->new_dev.address); - client->on_opened(addr); + event.type = EVENT_DEVICE_NEW; + event.data.device_new.address = event_msg->new_dev.address; + xQueueSend(client->get_event_queue(), &event, portMAX_DELAY); break; } case USB_HOST_CLIENT_EVENT_DEV_GONE: { - client->on_removed(event_msg->dev_gone.dev_hdl); - ESP_LOGD(TAG, "Device gone %d", event_msg->new_dev.address); + ESP_LOGD(TAG, "Device gone"); + event.type = EVENT_DEVICE_GONE; + event.data.device_gone.handle = event_msg->dev_gone.dev_hdl; + xQueueSend(client->get_event_queue(), &event, portMAX_DELAY); break; } default: @@ -173,9 +180,66 @@ void USBClient::setup() { usb_host_transfer_alloc(64, 0, &trq->transfer); trq->client = this; } + + // Create event queue for communication between USB task and main loop + this->event_queue_ = xQueueCreate(32, sizeof(UsbEvent)); + if (this->event_queue_ == nullptr) { + ESP_LOGE(TAG, "Failed to create event queue"); + this->mark_failed(); + return; + } + + // Create and start USB task + xTaskCreatePinnedToCore(usb_task_fn, "usb_task", + 2048, // Stack size (minimal - just handles USB events) + this, // Task parameter + 5, // Priority (higher than main loop) + &this->usb_task_handle_, + 1 // Core 1 + ); + + if (this->usb_task_handle_ == nullptr) { + ESP_LOGE(TAG, "Failed to create USB task"); + this->mark_failed(); + } +} + +void USBClient::usb_task_fn(void *arg) { + auto *client = static_cast(arg); + client->usb_task_loop(); +} + +void USBClient::usb_task_loop() { + ESP_LOGI(TAG, "USB task started on core %d", xPortGetCoreID()); + + // Run forever - ESPHome reboots rather than shutting down cleanly + while (true) { + // Handle USB events with a timeout to prevent blocking forever + usb_host_client_handle_events(this->handle_, pdMS_TO_TICKS(10)); + } } void USBClient::loop() { + // Process any events from the USB task + UsbEvent event; + while (xQueueReceive(this->event_queue_, &event, 0) == pdTRUE) { + switch (event.type) { + case EVENT_DEVICE_NEW: + this->on_opened(event.data.device_new.address); + break; + case EVENT_DEVICE_GONE: + this->on_removed(event.data.device_gone.handle); + break; + case EVENT_TRANSFER_COMPLETE: + case EVENT_CONTROL_COMPLETE: { + auto *trq = event.data.transfer.trq; + // Callback was already executed in USB task, just cleanup + this->release_trq(trq); + break; + } + } + } + switch (this->state_) { case USB_CLIENT_OPEN: { int err; @@ -228,7 +292,7 @@ void USBClient::loop() { } default: - usb_host_client_handle_events(this->handle_, 0); + // USB events are now handled in the dedicated task break; } } @@ -245,6 +309,7 @@ void USBClient::on_removed(usb_device_handle_t handle) { } } +// CALLBACK CONTEXT: USB task (called from usb_host_client_handle_events in USB task) static void control_callback(const usb_transfer_t *xfer) { auto *trq = static_cast(xfer->context); trq->status.error_code = xfer->status; @@ -252,9 +317,18 @@ static void control_callback(const usb_transfer_t *xfer) { trq->status.endpoint = xfer->bEndpointAddress; trq->status.data = xfer->data_buffer; trq->status.data_len = xfer->actual_num_bytes; - if (trq->callback != nullptr) + + // Execute callback in USB task context + if (trq->callback != nullptr) { trq->callback(trq->status); - trq->client->release_trq(trq); + } + + // Queue cleanup to main loop + UsbEvent event; + event.type = EVENT_CONTROL_COMPLETE; + event.data.transfer.trq = trq; + event.data.transfer.callback_executed = true; + xQueueSend(trq->client->get_event_queue(), &event, portMAX_DELAY); } TransferRequest *USBClient::get_trq_() { @@ -315,6 +389,7 @@ bool USBClient::control_transfer(uint8_t type, uint8_t request, uint16_t value, return true; } +// CALLBACK CONTEXT: USB task (called from usb_host_client_handle_events in USB task) static void transfer_callback(usb_transfer_t *xfer) { auto *trq = static_cast(xfer->context); trq->status.error_code = xfer->status; @@ -322,9 +397,19 @@ static void transfer_callback(usb_transfer_t *xfer) { trq->status.endpoint = xfer->bEndpointAddress; trq->status.data = xfer->data_buffer; trq->status.data_len = xfer->actual_num_bytes; - if (trq->callback != nullptr) + + // Always execute callback in USB task context + // Callbacks should be fast and non-blocking (e.g., copy data to queue) + if (trq->callback != nullptr) { trq->callback(trq->status); - trq->client->release_trq(trq); + } + + // Queue cleanup to main loop + UsbEvent event; + event.type = EVENT_TRANSFER_COMPLETE; + event.data.transfer.trq = trq; + event.data.transfer.callback_executed = true; + xQueueSend(trq->client->get_event_queue(), &event, portMAX_DELAY); } /** * Performs a transfer input operation. @@ -345,6 +430,7 @@ void USBClient::transfer_in(uint8_t ep_address, const transfer_cb_t &callback, u trq->transfer->callback = transfer_callback; trq->transfer->bEndpointAddress = ep_address | USB_DIR_IN; trq->transfer->num_bytes = length; + auto err = usb_host_transfer_submit(trq->transfer); if (err != ESP_OK) { ESP_LOGE(TAG, "Failed to submit transfer, address=%x, length=%d, err=%x", ep_address, length, err); diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index bf1c9086f1..4b2464fd59 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -170,7 +170,37 @@ bool USBUartChannel::read_array(uint8_t *data, size_t len) { return status; } void USBUartComponent::setup() { USBClient::setup(); } -void USBUartComponent::loop() { USBClient::loop(); } +void USBUartComponent::loop() { + USBClient::loop(); + + // Process USB data from the lock-free queue + UsbDataChunk *chunk; + int chunks_processed = 0; + while ((chunk = this->usb_data_queue_.pop()) != nullptr) { + chunks_processed++; + auto *channel = chunk->channel; + +#ifdef USE_UART_DEBUGGER + if (channel->debug_) { + uart::UARTDebug::log_hex(uart::UART_DIRECTION_RX, std::vector(chunk->data, chunk->data + chunk->length), + ','); // NOLINT() + } +#endif + + // Push data to ring buffer (now safe in main loop) + for (size_t i = 0; i < chunk->length; i++) { + channel->input_buffer_.push(chunk->data[i]); + } + + // Return chunk to pool for reuse + this->free_chunks_.push(chunk); + } + + static constexpr int LOG_CHUNK_THRESHOLD = 5; + if (chunks_processed > LOG_CHUNK_THRESHOLD) { + ESP_LOGV(TAG, "Processed %d chunks from USB queue", chunks_processed); + } +} void USBUartComponent::dump_config() { USBClient::dump_config(); for (auto &channel : this->channels_) { @@ -187,31 +217,46 @@ void USBUartComponent::dump_config() { } } void USBUartComponent::start_input(USBUartChannel *channel) { - if (!channel->initialised_ || channel->input_started_ || - channel->input_buffer_.get_free_space() < channel->cdc_dev_.in_ep->wMaxPacketSize) + if (!channel->initialised_ || channel->input_started_) return; + // Note: We no longer check ring buffer space here since this may be called from USB task + // The lock-free queue provides backpressure instead const auto *ep = channel->cdc_dev_.in_ep; + // CALLBACK CONTEXT: This lambda is executed in USB task via transfer_callback auto callback = [this, channel](const usb_host::TransferStatus &status) { ESP_LOGV(TAG, "Transfer result: length: %u; status %X", status.data_len, status.error_code); if (!status.success) { ESP_LOGE(TAG, "Control transfer failed, status=%s", esp_err_to_name(status.error_code)); return; } -#ifdef USE_UART_DEBUGGER - if (channel->debug_) { - uart::UARTDebug::log_hex(uart::UART_DIRECTION_RX, - std::vector(status.data, status.data + status.data_len), ','); // NOLINT() - } -#endif - channel->input_started_ = false; - if (!channel->dummy_receiver_) { - for (size_t i = 0; i != status.data_len; i++) { - channel->input_buffer_.push(status.data[i]); + + if (!channel->dummy_receiver_ && status.data_len > 0) { + // Get a free chunk from the pool + UsbDataChunk *chunk = this->free_chunks_.pop(); + if (chunk == nullptr) { + ESP_LOGW(TAG, "No free chunks available, dropping %u bytes", status.data_len); + // Mark input as not started so we can retry + channel->input_started_ = false; + return; + } + + // Copy data to chunk (this is fast, happens in USB task) + memcpy(chunk->data, status.data, status.data_len); + chunk->length = status.data_len; + chunk->channel = channel; + + // Push to lock-free queue for main loop processing + if (!this->usb_data_queue_.push(chunk)) { + ESP_LOGW(TAG, "USB data queue full, dropping %u bytes", status.data_len); + // Return chunk to pool + this->free_chunks_.push(chunk); } } - if (channel->input_buffer_.get_free_space() >= channel->cdc_dev_.in_ep->wMaxPacketSize) { - this->defer([this, channel] { this->start_input(channel); }); - } + + // Always restart input immediately from USB task + // The lock-free queue will handle backpressure + channel->input_started_ = false; + this->start_input(channel); }; channel->input_started_ = true; this->transfer_in(ep->bEndpointAddress, callback, ep->wMaxPacketSize); @@ -224,9 +269,12 @@ void USBUartComponent::start_output(USBUartChannel *channel) { return; } const auto *ep = channel->cdc_dev_.out_ep; + // CALLBACK CONTEXT: This lambda is stored in TransferRequest and will be executed + // in MAIN LOOP after being queued by transfer_callback in USB task auto callback = [this, channel](const usb_host::TransferStatus &status) { ESP_LOGV(TAG, "Output Transfer result: length: %u; status %X", status.data_len, status.error_code); channel->output_started_ = false; + // DEFERRED CONTEXT: Main loop (restart output in main loop) this->defer([this, channel] { this->start_output(channel); }); }; channel->output_started_ = true; diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index a103c51add..c1affe2bc9 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -5,11 +5,13 @@ #include "esphome/core/helpers.h" #include "esphome/components/uart/uart_component.h" #include "esphome/components/usb_host/usb_host.h" +#include "esphome/core/lock_free_queue.h" namespace esphome { namespace usb_uart { class USBUartTypeCdcAcm; class USBUartComponent; +class USBUartChannel; static const char *const TAG = "usb_uart"; @@ -68,6 +70,14 @@ class RingBuffer { uint8_t *buffer_; }; +// Structure for queuing received USB data chunks +struct UsbDataChunk { + static constexpr size_t MAX_CHUNK_SIZE = 64; // USB packet size + uint8_t data[MAX_CHUNK_SIZE]; + size_t length; + USBUartChannel *channel; +}; + class USBUartChannel : public uart::UARTComponent, public Parented { friend class USBUartComponent; friend class USBUartTypeCdcAcm; @@ -104,7 +114,18 @@ class USBUartChannel : public uart::UARTComponent, public Parenteddata_chunk_pool_[i] = new UsbDataChunk(); + this->free_chunks_.push(this->data_chunk_pool_[i]); + } + } + ~USBUartComponent() { + for (int i = 0; i < MAX_DATA_CHUNKS; i++) { + delete this->data_chunk_pool_[i]; + } + } void setup() override; void loop() override; void dump_config() override; @@ -115,8 +136,16 @@ class USBUartComponent : public usb_host::USBClient { void start_input(USBUartChannel *channel); void start_output(USBUartChannel *channel); + // Lock-free data transfer from USB task to main loop + LockFreeQueue usb_data_queue_; + protected: std::vector channels_{}; + + // Pool of pre-allocated data chunks to avoid dynamic allocation + static constexpr int MAX_DATA_CHUNKS = 32; + UsbDataChunk *data_chunk_pool_[MAX_DATA_CHUNKS]; + LockFreeQueue free_chunks_; }; class USBUartTypeCdcAcm : public USBUartComponent { From 4699e5683250e550b3f6e16228b9261e35dc0738 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:28:14 -0500 Subject: [PATCH 02/26] wip --- esphome/components/usb_uart/usb_uart.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index c1affe2bc9..91a4329f22 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -115,17 +115,12 @@ class USBUartChannel : public uart::UARTComponent, public Parenteddata_chunk_pool_[i] = new UsbDataChunk(); this->free_chunks_.push(this->data_chunk_pool_[i]); } } - ~USBUartComponent() { - for (int i = 0; i < MAX_DATA_CHUNKS; i++) { - delete this->data_chunk_pool_[i]; - } - } void setup() override; void loop() override; void dump_config() override; @@ -137,14 +132,17 @@ class USBUartComponent : public usb_host::USBClient { void start_output(USBUartChannel *channel); // Lock-free data transfer from USB task to main loop - LockFreeQueue usb_data_queue_; + static constexpr int USB_DATA_QUEUE_SIZE = 32; + LockFreeQueue usb_data_queue_; protected: std::vector channels_{}; // Pool of pre-allocated data chunks to avoid dynamic allocation - static constexpr int MAX_DATA_CHUNKS = 32; + static constexpr int MAX_DATA_CHUNKS = 40; UsbDataChunk *data_chunk_pool_[MAX_DATA_CHUNKS]; + // IMPORTANT: This is used bidirectionally (USB task pops, main loop pushes) + // which technically violates SPSC, but works in practice because operations are atomic LockFreeQueue free_chunks_; }; From 0ed6ba9afaf226cd321e2e93337f5014979301bd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:37:07 -0500 Subject: [PATCH 03/26] wip --- esphome/components/usb_uart/usb_uart.cpp | 8 ++++---- esphome/components/usb_uart/usb_uart.h | 26 +++++++++++------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index 4b2464fd59..a872b37fa2 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -193,7 +193,7 @@ void USBUartComponent::loop() { } // Return chunk to pool for reuse - this->free_chunks_.push(chunk); + this->chunk_pool_.release(chunk); } static constexpr int LOG_CHUNK_THRESHOLD = 5; @@ -231,8 +231,8 @@ void USBUartComponent::start_input(USBUartChannel *channel) { } if (!channel->dummy_receiver_ && status.data_len > 0) { - // Get a free chunk from the pool - UsbDataChunk *chunk = this->free_chunks_.pop(); + // Allocate a chunk from the pool + UsbDataChunk *chunk = this->chunk_pool_.allocate(); if (chunk == nullptr) { ESP_LOGW(TAG, "No free chunks available, dropping %u bytes", status.data_len); // Mark input as not started so we can retry @@ -249,7 +249,7 @@ void USBUartComponent::start_input(USBUartChannel *channel) { if (!this->usb_data_queue_.push(chunk)) { ESP_LOGW(TAG, "USB data queue full, dropping %u bytes", status.data_len); // Return chunk to pool - this->free_chunks_.push(chunk); + this->chunk_pool_.release(chunk); } } diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index 91a4329f22..1389ceeaee 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -6,6 +6,7 @@ #include "esphome/components/uart/uart_component.h" #include "esphome/components/usb_host/usb_host.h" #include "esphome/core/lock_free_queue.h" +#include "esphome/core/event_pool.h" namespace esphome { namespace usb_uart { @@ -76,6 +77,12 @@ struct UsbDataChunk { uint8_t data[MAX_CHUNK_SIZE]; size_t length; USBUartChannel *channel; + + // Required for EventPool - reset to clean state + void release() { + this->length = 0; + this->channel = nullptr; + } }; class USBUartChannel : public uart::UARTComponent, public Parented { @@ -114,13 +121,7 @@ class USBUartChannel : public uart::UARTComponent, public Parenteddata_chunk_pool_[i] = new UsbDataChunk(); - this->free_chunks_.push(this->data_chunk_pool_[i]); - } - } + USBUartComponent(uint16_t vid, uint16_t pid) : usb_host::USBClient(vid, pid) {} void setup() override; void loop() override; void dump_config() override; @@ -135,15 +136,12 @@ class USBUartComponent : public usb_host::USBClient { static constexpr int USB_DATA_QUEUE_SIZE = 32; LockFreeQueue usb_data_queue_; + // Pool for allocating data chunks (uses EventPool pattern like BLE) + static constexpr int MAX_DATA_CHUNKS = 40; + EventPool chunk_pool_; + protected: std::vector channels_{}; - - // Pool of pre-allocated data chunks to avoid dynamic allocation - static constexpr int MAX_DATA_CHUNKS = 40; - UsbDataChunk *data_chunk_pool_[MAX_DATA_CHUNKS]; - // IMPORTANT: This is used bidirectionally (USB task pops, main loop pushes) - // which technically violates SPSC, but works in practice because operations are atomic - LockFreeQueue free_chunks_; }; class USBUartTypeCdcAcm : public USBUartComponent { From 0370a3061df5da7fe24dfe1f1513c28ee98f0e5d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:41:48 -0500 Subject: [PATCH 04/26] fix --- esphome/components/usb_uart/usb_uart.cpp | 7 ++----- esphome/components/usb_uart/usb_uart.h | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index a872b37fa2..b485397214 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -246,11 +246,8 @@ void USBUartComponent::start_input(USBUartChannel *channel) { chunk->channel = channel; // Push to lock-free queue for main loop processing - if (!this->usb_data_queue_.push(chunk)) { - ESP_LOGW(TAG, "USB data queue full, dropping %u bytes", status.data_len); - // Return chunk to pool - this->chunk_pool_.release(chunk); - } + // Push always succeeds because pool size == queue size + this->usb_data_queue_.push(chunk); } // Always restart input immediately from USB task diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index 1389ceeaee..aab36c52b5 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -137,8 +137,8 @@ class USBUartComponent : public usb_host::USBClient { LockFreeQueue usb_data_queue_; // Pool for allocating data chunks (uses EventPool pattern like BLE) - static constexpr int MAX_DATA_CHUNKS = 40; - EventPool chunk_pool_; + // MUST be same size as queue to guarantee push always succeeds after allocate + EventPool chunk_pool_; protected: std::vector channels_{}; From 70e89f79dbb0c647dc3b1d264c5a7c35eb05bb05 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:54:33 -0500 Subject: [PATCH 05/26] fix --- esphome/components/usb_host/usb_host_client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index 98a1f6178b..f9ecc89efa 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -191,7 +191,7 @@ void USBClient::setup() { // Create and start USB task xTaskCreatePinnedToCore(usb_task_fn, "usb_task", - 2048, // Stack size (minimal - just handles USB events) + 4096, // Stack size (same as ESP-IDF USB examples) this, // Task parameter 5, // Priority (higher than main loop) &this->usb_task_handle_, From c08c0c111a3cc7ba5ddf456a25708a1dee205378 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:56:22 -0500 Subject: [PATCH 06/26] fix --- esphome/components/usb_host/usb_host.h | 1 - esphome/components/usb_host/usb_host_client.cpp | 3 --- 2 files changed, 4 deletions(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index 3625100e4a..99b8437421 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -75,7 +75,6 @@ struct UsbEvent { } device_gone; struct { TransferRequest *trq; - bool callback_executed; // Flag to indicate callback was already executed in USB task } transfer; } data; }; diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index f9ecc89efa..d294520568 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -212,9 +212,7 @@ void USBClient::usb_task_fn(void *arg) { void USBClient::usb_task_loop() { ESP_LOGI(TAG, "USB task started on core %d", xPortGetCoreID()); - // Run forever - ESPHome reboots rather than shutting down cleanly while (true) { - // Handle USB events with a timeout to prevent blocking forever usb_host_client_handle_events(this->handle_, pdMS_TO_TICKS(10)); } } @@ -327,7 +325,6 @@ static void control_callback(const usb_transfer_t *xfer) { UsbEvent event; event.type = EVENT_CONTROL_COMPLETE; event.data.transfer.trq = trq; - event.data.transfer.callback_executed = true; xQueueSend(trq->client->get_event_queue(), &event, portMAX_DELAY); } From d5ad9dc0fb41beed00580ae6c02d0510d2bb8f9c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:58:01 -0500 Subject: [PATCH 07/26] fix --- .../components/usb_host/usb_host_client.cpp | 19 ++++++++++--------- esphome/components/usb_uart/usb_uart.cpp | 5 ++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index d294520568..1abc260bbf 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -307,6 +307,14 @@ void USBClient::on_removed(usb_device_handle_t handle) { } } +// Helper to queue transfer cleanup to main loop +static void queue_transfer_cleanup(TransferRequest *trq, EventType type) { + UsbEvent event; + event.type = type; + event.data.transfer.trq = trq; + xQueueSend(trq->client->get_event_queue(), &event, portMAX_DELAY); +} + // CALLBACK CONTEXT: USB task (called from usb_host_client_handle_events in USB task) static void control_callback(const usb_transfer_t *xfer) { auto *trq = static_cast(xfer->context); @@ -322,10 +330,7 @@ static void control_callback(const usb_transfer_t *xfer) { } // Queue cleanup to main loop - UsbEvent event; - event.type = EVENT_CONTROL_COMPLETE; - event.data.transfer.trq = trq; - xQueueSend(trq->client->get_event_queue(), &event, portMAX_DELAY); + queue_transfer_cleanup(trq, EVENT_CONTROL_COMPLETE); } TransferRequest *USBClient::get_trq_() { @@ -402,11 +407,7 @@ static void transfer_callback(usb_transfer_t *xfer) { } // Queue cleanup to main loop - UsbEvent event; - event.type = EVENT_TRANSFER_COMPLETE; - event.data.transfer.trq = trq; - event.data.transfer.callback_executed = true; - xQueueSend(trq->client->get_event_queue(), &event, portMAX_DELAY); + queue_transfer_cleanup(trq, EVENT_TRANSFER_COMPLETE); } /** * Performs a transfer input operation. diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index b485397214..5aa9f65f12 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -266,12 +266,11 @@ void USBUartComponent::start_output(USBUartChannel *channel) { return; } const auto *ep = channel->cdc_dev_.out_ep; - // CALLBACK CONTEXT: This lambda is stored in TransferRequest and will be executed - // in MAIN LOOP after being queued by transfer_callback in USB task + // CALLBACK CONTEXT: This lambda is executed in USB task via transfer_callback auto callback = [this, channel](const usb_host::TransferStatus &status) { ESP_LOGV(TAG, "Output Transfer result: length: %u; status %X", status.data_len, status.error_code); channel->output_started_ = false; - // DEFERRED CONTEXT: Main loop (restart output in main loop) + // Defer restart to main loop (defer is thread-safe) this->defer([this, channel] { this->start_output(channel); }); }; channel->output_started_ = true; From fb9334e5bab4907e7f0b3e44d3d2db9ebd27e82b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 21:59:38 -0500 Subject: [PATCH 08/26] fix --- esphome/components/usb_uart/usb_uart.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index 5aa9f65f12..35ebdf7b96 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -188,9 +188,7 @@ void USBUartComponent::loop() { #endif // Push data to ring buffer (now safe in main loop) - for (size_t i = 0; i < chunk->length; i++) { - channel->input_buffer_.push(chunk->data[i]); - } + channel->input_buffer_.push(chunk->data, chunk->length); // Return chunk to pool for reuse this->chunk_pool_.release(chunk); From 02b144c2e50090f81eac61bc33d6c08ae03fc3d9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:02:03 -0500 Subject: [PATCH 09/26] fix --- esphome/components/usb_host/usb_host.h | 2 +- esphome/components/usb_uart/usb_uart.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index 99b8437421..f1600aa0cf 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -57,7 +57,7 @@ struct TransferRequest { }; // Lightweight event types for queue -enum EventType { +enum EventType : uint8_t { EVENT_DEVICE_NEW, EVENT_DEVICE_GONE, EVENT_TRANSFER_COMPLETE, diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index aab36c52b5..0ac710e4c5 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -75,7 +75,7 @@ class RingBuffer { struct UsbDataChunk { static constexpr size_t MAX_CHUNK_SIZE = 64; // USB packet size uint8_t data[MAX_CHUNK_SIZE]; - size_t length; + uint8_t length; // Max 64 bytes, so uint8_t is sufficient USBUartChannel *channel; // Required for EventPool - reset to clean state From 4d64a05334632c473a3c821fd2d54ee62eb80eb3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:12:59 -0500 Subject: [PATCH 10/26] ato --- esphome/components/usb_uart/ch34x.cpp | 6 ++--- esphome/components/usb_uart/cp210x.cpp | 4 +-- esphome/components/usb_uart/usb_uart.cpp | 32 ++++++++++++------------ esphome/components/usb_uart/usb_uart.h | 7 +++--- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/esphome/components/usb_uart/ch34x.cpp b/esphome/components/usb_uart/ch34x.cpp index 74e7933824..601bfe7366 100644 --- a/esphome/components/usb_uart/ch34x.cpp +++ b/esphome/components/usb_uart/ch34x.cpp @@ -16,12 +16,12 @@ using namespace bytebuffer; void USBUartTypeCH34X::enable_channels() { // enable the channels for (auto channel : this->channels_) { - if (!channel->initialised_) + if (!channel->initialised_.load()) continue; usb_host::transfer_cb_t callback = [=](const usb_host::TransferStatus &status) { if (!status.success) { ESP_LOGE(TAG, "Control transfer failed, status=%s", esp_err_to_name(status.error_code)); - channel->initialised_ = false; + channel->initialised_.store(false); } }; @@ -48,7 +48,7 @@ void USBUartTypeCH34X::enable_channels() { auto factor = static_cast(clk / baud_rate); if (factor == 0 || factor == 0xFF) { ESP_LOGE(TAG, "Invalid baud rate %" PRIu32, baud_rate); - channel->initialised_ = false; + channel->initialised_.store(false); continue; } if ((clk / factor - baud_rate) > (baud_rate - clk / (factor + 1))) diff --git a/esphome/components/usb_uart/cp210x.cpp b/esphome/components/usb_uart/cp210x.cpp index f7d60c307a..35834c7529 100644 --- a/esphome/components/usb_uart/cp210x.cpp +++ b/esphome/components/usb_uart/cp210x.cpp @@ -100,12 +100,12 @@ std::vector USBUartTypeCP210X::parse_descriptors(usb_device_handle_t dev void USBUartTypeCP210X::enable_channels() { // enable the channels for (auto channel : this->channels_) { - if (!channel->initialised_) + if (!channel->initialised_.load()) continue; usb_host::transfer_cb_t callback = [=](const usb_host::TransferStatus &status) { if (!status.success) { ESP_LOGE(TAG, "Control transfer failed, status=%s", esp_err_to_name(status.error_code)); - channel->initialised_ = false; + channel->initialised_.store(false); } }; this->control_transfer(USB_VENDOR_IFC | usb_host::USB_DIR_OUT, IFC_ENABLE, 1, channel->index_, callback); diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index 35ebdf7b96..6d3c888bd9 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -130,7 +130,7 @@ size_t RingBuffer::pop(uint8_t *data, size_t len) { return len; } void USBUartChannel::write_array(const uint8_t *data, size_t len) { - if (!this->initialised_) { + if (!this->initialised_.load()) { ESP_LOGV(TAG, "Channel not initialised - write ignored"); return; } @@ -152,7 +152,7 @@ bool USBUartChannel::peek_byte(uint8_t *data) { return true; } bool USBUartChannel::read_array(uint8_t *data, size_t len) { - if (!this->initialised_) { + if (!this->initialised_.load()) { ESP_LOGV(TAG, "Channel not initialised - read ignored"); return false; } @@ -215,7 +215,7 @@ void USBUartComponent::dump_config() { } } void USBUartComponent::start_input(USBUartChannel *channel) { - if (!channel->initialised_ || channel->input_started_) + if (!channel->initialised_.load() || channel->input_started_.load()) return; // Note: We no longer check ring buffer space here since this may be called from USB task // The lock-free queue provides backpressure instead @@ -234,7 +234,7 @@ void USBUartComponent::start_input(USBUartChannel *channel) { if (chunk == nullptr) { ESP_LOGW(TAG, "No free chunks available, dropping %u bytes", status.data_len); // Mark input as not started so we can retry - channel->input_started_ = false; + channel->input_started_.store(false); return; } @@ -250,15 +250,15 @@ void USBUartComponent::start_input(USBUartChannel *channel) { // Always restart input immediately from USB task // The lock-free queue will handle backpressure - channel->input_started_ = false; + channel->input_started_.store(false); this->start_input(channel); }; - channel->input_started_ = true; + channel->input_started_.store(true); this->transfer_in(ep->bEndpointAddress, callback, ep->wMaxPacketSize); } void USBUartComponent::start_output(USBUartChannel *channel) { - if (channel->output_started_) + if (channel->output_started_.load()) return; if (channel->output_buffer_.is_empty()) { return; @@ -267,11 +267,11 @@ void USBUartComponent::start_output(USBUartChannel *channel) { // CALLBACK CONTEXT: This lambda is executed in USB task via transfer_callback auto callback = [this, channel](const usb_host::TransferStatus &status) { ESP_LOGV(TAG, "Output Transfer result: length: %u; status %X", status.data_len, status.error_code); - channel->output_started_ = false; + channel->output_started_.store(false); // Defer restart to main loop (defer is thread-safe) this->defer([this, channel] { this->start_output(channel); }); }; - channel->output_started_ = true; + channel->output_started_.store(true); uint8_t data[ep->wMaxPacketSize]; auto len = channel->output_buffer_.pop(data, ep->wMaxPacketSize); this->transfer_out(ep->bEndpointAddress, callback, data, len); @@ -314,7 +314,7 @@ void USBUartTypeCdcAcm::on_connected() { channel->cdc_dev_ = cdc_devs[i++]; fix_mps(channel->cdc_dev_.in_ep); fix_mps(channel->cdc_dev_.out_ep); - channel->initialised_ = true; + channel->initialised_.store(true); auto err = usb_host_interface_claim(this->handle_, this->device_handle_, channel->cdc_dev_.bulk_interface_number, 0); if (err != ESP_OK) { @@ -343,9 +343,9 @@ void USBUartTypeCdcAcm::on_disconnected() { usb_host_endpoint_flush(this->device_handle_, channel->cdc_dev_.notify_ep->bEndpointAddress); } usb_host_interface_release(this->handle_, this->device_handle_, channel->cdc_dev_.bulk_interface_number); - channel->initialised_ = false; - channel->input_started_ = false; - channel->output_started_ = false; + channel->initialised_.store(false); + channel->input_started_.store(false); + channel->output_started_.store(false); channel->input_buffer_.clear(); channel->output_buffer_.clear(); } @@ -354,10 +354,10 @@ void USBUartTypeCdcAcm::on_disconnected() { void USBUartTypeCdcAcm::enable_channels() { for (auto *channel : this->channels_) { - if (!channel->initialised_) + if (!channel->initialised_.load()) continue; - channel->input_started_ = false; - channel->output_started_ = false; + channel->input_started_.store(false); + channel->output_started_.store(false); this->start_input(channel); } } diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index 0ac710e4c5..ccf998f84e 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -7,6 +7,7 @@ #include "esphome/components/usb_host/usb_host.h" #include "esphome/core/lock_free_queue.h" #include "esphome/core/event_pool.h" +#include namespace esphome { namespace usb_uart { @@ -111,12 +112,12 @@ class USBUartChannel : public uart::UARTComponent, public Parented input_started_{true}; + std::atomic output_started_{true}; CdcEps cdc_dev_{}; bool debug_{}; bool dummy_receiver_{}; - bool initialised_{}; + std::atomic initialised_{false}; }; class USBUartComponent : public usb_host::USBClient { From 0b5964053e28e033ad01b734d8ac3987d888d8d5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:13:42 -0500 Subject: [PATCH 11/26] ato --- esphome/components/usb_uart/usb_uart.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index 6d3c888bd9..04e1abbc50 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -198,6 +198,12 @@ void USBUartComponent::loop() { if (chunks_processed > LOG_CHUNK_THRESHOLD) { ESP_LOGV(TAG, "Processed %d chunks from USB queue", chunks_processed); } + + // Log dropped USB data periodically + uint16_t dropped = this->usb_data_queue_.get_and_reset_dropped_count(); + if (dropped > 0) { + ESP_LOGW(TAG, "Dropped %u USB data chunks due to buffer overflow", dropped); + } } void USBUartComponent::dump_config() { USBClient::dump_config(); @@ -232,7 +238,8 @@ void USBUartComponent::start_input(USBUartChannel *channel) { // Allocate a chunk from the pool UsbDataChunk *chunk = this->chunk_pool_.allocate(); if (chunk == nullptr) { - ESP_LOGW(TAG, "No free chunks available, dropping %u bytes", status.data_len); + // No chunks available - queue is full or we're out of memory + this->usb_data_queue_.increment_dropped_count(); // Mark input as not started so we can retry channel->input_started_.store(false); return; From fdb2e0b247364f82d0e97118ba6a2a97d44eb4e3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:15:36 -0500 Subject: [PATCH 12/26] ato --- esphome/components/usb_host/usb_host.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index f1600aa0cf..183ee73a14 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -32,7 +32,9 @@ static const uint8_t USB_DIR_IN = 1 << 7; static const uint8_t USB_DIR_OUT = 0; static const size_t SETUP_PACKET_SIZE = 8; -static const size_t MAX_REQUESTS = 16; // maximum number of outstanding requests possible. +static const size_t MAX_REQUESTS = 16; // maximum number of outstanding requests possible. +static constexpr size_t USB_EVENT_QUEUE_SIZE = 32; // Size of event queue between USB task and main loop +static constexpr size_t USB_TASK_STACK_SIZE = 4096; // Stack size for USB task (same as ESP-IDF USB examples) // used to report a transfer status struct TransferStatus { From 971931b87784eea3ecd1f359052d795066e84fd0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:15:53 -0500 Subject: [PATCH 13/26] ato --- esphome/components/usb_host/usb_host_client.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index 1abc260bbf..7a3a53cb75 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -182,7 +182,7 @@ void USBClient::setup() { } // Create event queue for communication between USB task and main loop - this->event_queue_ = xQueueCreate(32, sizeof(UsbEvent)); + this->event_queue_ = xQueueCreate(USB_EVENT_QUEUE_SIZE, sizeof(UsbEvent)); if (this->event_queue_ == nullptr) { ESP_LOGE(TAG, "Failed to create event queue"); this->mark_failed(); @@ -191,9 +191,9 @@ void USBClient::setup() { // Create and start USB task xTaskCreatePinnedToCore(usb_task_fn, "usb_task", - 4096, // Stack size (same as ESP-IDF USB examples) - this, // Task parameter - 5, // Priority (higher than main loop) + USB_TASK_STACK_SIZE, // Stack size + this, // Task parameter + 5, // Priority (higher than main loop) &this->usb_task_handle_, 1 // Core 1 ); From efc0d86aa6ffe346cd59aef85720e6e420425890 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:18:43 -0500 Subject: [PATCH 14/26] ato --- esphome/components/usb_uart/usb_uart.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index 04e1abbc50..5ad86037ed 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -223,8 +223,9 @@ void USBUartComponent::dump_config() { void USBUartComponent::start_input(USBUartChannel *channel) { if (!channel->initialised_.load() || channel->input_started_.load()) return; - // Note: We no longer check ring buffer space here since this may be called from USB task - // The lock-free queue provides backpressure instead + // Note: This function is called from both USB task and main loop, so we cannot + // directly check ring buffer space here. Backpressure is handled by the chunk pool: + // when exhausted, USB input stops until chunks are freed by the main loop const auto *ep = channel->cdc_dev_.in_ep; // CALLBACK CONTEXT: This lambda is executed in USB task via transfer_callback auto callback = [this, channel](const usb_host::TransferStatus &status) { From 6ba720d126672937f9c8c4604f073d83a3459d42 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:21:44 -0500 Subject: [PATCH 15/26] ato --- esphome/components/usb_uart/usb_uart.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index 5ad86037ed..c6a0e56adc 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -266,6 +266,9 @@ void USBUartComponent::start_input(USBUartChannel *channel) { } void USBUartComponent::start_output(USBUartChannel *channel) { + // IMPORTANT: This function must only be called from the main loop! + // The output_buffer_ is not thread-safe and can only be accessed from main loop. + // USB callbacks use defer() to ensure this function runs in the correct context. if (channel->output_started_.load()) return; if (channel->output_buffer_.is_empty()) { From 7388a2c9a312ef2b21ae414fbc6d5c4e42ea71ae Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:22:44 -0500 Subject: [PATCH 16/26] ato --- esphome/components/usb_uart/usb_uart.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index ccf998f84e..21e2d34439 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -108,16 +108,20 @@ class USBUartChannel : public uart::UARTComponent, public Parenteddummy_receiver_ = dummy_receiver; } protected: - const uint8_t index_; + // Larger structures first for better alignment RingBuffer input_buffer_; RingBuffer output_buffer_; + CdcEps cdc_dev_{}; + // Enum (likely 4 bytes) UARTParityOptions parity_{UART_CONFIG_PARITY_NONE}; + // Group atomics together (each 1 byte) std::atomic input_started_{true}; std::atomic output_started_{true}; - CdcEps cdc_dev_{}; + std::atomic initialised_{false}; + // Group regular bytes together to minimize padding + const uint8_t index_; bool debug_{}; bool dummy_receiver_{}; - std::atomic initialised_{false}; }; class USBUartComponent : public usb_host::USBClient { From 9f2f33fc89918586045c5a457dcf39bbed602b89 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:36:48 -0500 Subject: [PATCH 17/26] lock free --- esphome/components/usb_host/usb_host.h | 14 ++-- .../components/usb_host/usb_host_client.cpp | 72 ++++++++++++------- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index 183ee73a14..525457fd99 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -7,8 +7,8 @@ #include "usb/usb_host.h" #include #include -#include -#include +#include "esphome/core/lock_free_queue.h" +#include "esphome/core/event_pool.h" #include namespace esphome { @@ -79,6 +79,9 @@ struct UsbEvent { TransferRequest *trq; } transfer; } data; + + // Required for EventPool - no cleanup needed for POD types + void release() {} }; // callback function type. @@ -115,7 +118,11 @@ class USBClient : public Component { void release_trq(TransferRequest *trq); bool control_transfer(uint8_t type, uint8_t request, uint16_t value, uint16_t index, const transfer_cb_t &callback, const std::vector &data = {}); - QueueHandle_t get_event_queue() { return event_queue_; } + + // Lock-free event queue and pool for USB task to main loop communication + // Must be public for access from static callbacks + LockFreeQueue event_queue; + EventPool event_pool; protected: bool register_(); @@ -129,7 +136,6 @@ class USBClient : public Component { void usb_task_loop(); TaskHandle_t usb_task_handle_{nullptr}; - QueueHandle_t event_queue_{nullptr}; // Queue of UsbEvent structs usb_host_client_handle_t handle_{}; usb_device_handle_t device_handle_{}; diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index 7a3a53cb75..77599cb263 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -142,28 +142,37 @@ static std::string get_descriptor_string(const usb_str_desc_t *desc) { // CALLBACK CONTEXT: USB task (called from usb_host_client_handle_events in USB task) static void client_event_cb(const usb_host_client_event_msg_t *event_msg, void *ptr) { auto *client = static_cast(ptr); - UsbEvent event; + + // Allocate event from pool + UsbEvent *event = client->event_pool.allocate(); + if (event == nullptr) { + // No events available - increment counter for periodic logging + client->event_queue.increment_dropped_count(); + return; + } // Queue events to be processed in main loop switch (event_msg->event) { case USB_HOST_CLIENT_EVENT_NEW_DEV: { ESP_LOGD(TAG, "New device %d", event_msg->new_dev.address); - event.type = EVENT_DEVICE_NEW; - event.data.device_new.address = event_msg->new_dev.address; - xQueueSend(client->get_event_queue(), &event, portMAX_DELAY); + event->type = EVENT_DEVICE_NEW; + event->data.device_new.address = event_msg->new_dev.address; break; } case USB_HOST_CLIENT_EVENT_DEV_GONE: { ESP_LOGD(TAG, "Device gone"); - event.type = EVENT_DEVICE_GONE; - event.data.device_gone.handle = event_msg->dev_gone.dev_hdl; - xQueueSend(client->get_event_queue(), &event, portMAX_DELAY); + event->type = EVENT_DEVICE_GONE; + event->data.device_gone.handle = event_msg->dev_gone.dev_hdl; break; } default: ESP_LOGD(TAG, "Unknown event %d", event_msg->event); - break; + client->event_pool.release(event); + return; } + + // Push to lock-free queue (always succeeds since pool size == queue size) + client->event_queue.push(event); } void USBClient::setup() { usb_host_client_config_t config{.is_synchronous = false, @@ -181,14 +190,6 @@ void USBClient::setup() { trq->client = this; } - // Create event queue for communication between USB task and main loop - this->event_queue_ = xQueueCreate(USB_EVENT_QUEUE_SIZE, sizeof(UsbEvent)); - if (this->event_queue_ == nullptr) { - ESP_LOGE(TAG, "Failed to create event queue"); - this->mark_failed(); - return; - } - // Create and start USB task xTaskCreatePinnedToCore(usb_task_fn, "usb_task", USB_TASK_STACK_SIZE, // Stack size @@ -219,23 +220,31 @@ void USBClient::usb_task_loop() { void USBClient::loop() { // Process any events from the USB task - UsbEvent event; - while (xQueueReceive(this->event_queue_, &event, 0) == pdTRUE) { - switch (event.type) { + UsbEvent *event; + while ((event = this->event_queue.pop()) != nullptr) { + switch (event->type) { case EVENT_DEVICE_NEW: - this->on_opened(event.data.device_new.address); + this->on_opened(event->data.device_new.address); break; case EVENT_DEVICE_GONE: - this->on_removed(event.data.device_gone.handle); + this->on_removed(event->data.device_gone.handle); break; case EVENT_TRANSFER_COMPLETE: case EVENT_CONTROL_COMPLETE: { - auto *trq = event.data.transfer.trq; + auto *trq = event->data.transfer.trq; // Callback was already executed in USB task, just cleanup this->release_trq(trq); break; } } + // Return event to pool for reuse + this->event_pool.release(event); + } + + // Log dropped events periodically + uint16_t dropped = this->event_queue.get_and_reset_dropped_count(); + if (dropped > 0) { + ESP_LOGW(TAG, "Dropped %u USB events due to queue overflow", dropped); } switch (this->state_) { @@ -309,10 +318,21 @@ void USBClient::on_removed(usb_device_handle_t handle) { // Helper to queue transfer cleanup to main loop static void queue_transfer_cleanup(TransferRequest *trq, EventType type) { - UsbEvent event; - event.type = type; - event.data.transfer.trq = trq; - xQueueSend(trq->client->get_event_queue(), &event, portMAX_DELAY); + auto *client = trq->client; + + // Allocate event from pool + UsbEvent *event = client->event_pool.allocate(); + if (event == nullptr) { + // No events available - increment counter for periodic logging + client->event_queue.increment_dropped_count(); + return; + } + + event->type = type; + event->data.transfer.trq = trq; + + // Push to lock-free queue (always succeeds since pool size == queue size) + client->event_queue.push(event); } // CALLBACK CONTEXT: USB task (called from usb_host_client_handle_events in USB task) From 5f17a95f2ebfab922744e1086a4d73a02df3f4b9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:37:02 -0500 Subject: [PATCH 18/26] lock free --- esphome/components/usb_uart/usb_uart.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index 21e2d34439..1c4fb34fed 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -79,11 +79,8 @@ struct UsbDataChunk { uint8_t length; // Max 64 bytes, so uint8_t is sufficient USBUartChannel *channel; - // Required for EventPool - reset to clean state - void release() { - this->length = 0; - this->channel = nullptr; - } + // Required for EventPool - no cleanup needed for POD types + void release() {} }; class USBUartChannel : public uart::UARTComponent, public Parented { From 7fbc7e3c37633b94121cd87931534371ecfe1eca Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:41:43 -0500 Subject: [PATCH 19/26] lock free --- esphome/components/usb_host/usb_host_client.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index 77599cb263..caad490124 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -299,7 +299,6 @@ void USBClient::loop() { } default: - // USB events are now handled in the dedicated task break; } } From 1e2785e38721094ff4686048210e6c90e31b7249 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:45:16 -0500 Subject: [PATCH 20/26] Update esphome/components/usb_host/usb_host_client.cpp --- esphome/components/usb_host/usb_host_client.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index caad490124..1abf386b54 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -447,7 +447,6 @@ void USBClient::transfer_in(uint8_t ep_address, const transfer_cb_t &callback, u trq->transfer->callback = transfer_callback; trq->transfer->bEndpointAddress = ep_address | USB_DIR_IN; trq->transfer->num_bytes = length; - auto err = usb_host_transfer_submit(trq->transfer); if (err != ESP_OK) { ESP_LOGE(TAG, "Failed to submit transfer, address=%x, length=%d, err=%x", ep_address, length, err); From 6403c6ee64fea4a0392228eb4f57458e41bf4f86 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:45:30 -0500 Subject: [PATCH 21/26] preen --- esphome/components/usb_host/usb_host.h | 1 - esphome/components/usb_host/usb_host_client.cpp | 3 --- 2 files changed, 4 deletions(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index 525457fd99..afd76595db 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -58,7 +58,6 @@ struct TransferRequest { USBClient *client; }; -// Lightweight event types for queue enum EventType : uint8_t { EVENT_DEVICE_NEW, EVENT_DEVICE_GONE, diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index caad490124..177a90eb52 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -211,8 +211,6 @@ void USBClient::usb_task_fn(void *arg) { } void USBClient::usb_task_loop() { - ESP_LOGI(TAG, "USB task started on core %d", xPortGetCoreID()); - while (true) { usb_host_client_handle_events(this->handle_, pdMS_TO_TICKS(10)); } @@ -232,7 +230,6 @@ void USBClient::loop() { case EVENT_TRANSFER_COMPLETE: case EVENT_CONTROL_COMPLETE: { auto *trq = event->data.transfer.trq; - // Callback was already executed in USB task, just cleanup this->release_trq(trq); break; } From b9a5c57b77bda9c37b9982330b0c017facaa16cb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:47:17 -0500 Subject: [PATCH 22/26] preen --- esphome/components/usb_uart/usb_uart.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.h b/esphome/components/usb_uart/usb_uart.h index 1c4fb34fed..b41e0a52e9 100644 --- a/esphome/components/usb_uart/usb_uart.h +++ b/esphome/components/usb_uart/usb_uart.h @@ -137,9 +137,6 @@ class USBUartComponent : public usb_host::USBClient { // Lock-free data transfer from USB task to main loop static constexpr int USB_DATA_QUEUE_SIZE = 32; LockFreeQueue usb_data_queue_; - - // Pool for allocating data chunks (uses EventPool pattern like BLE) - // MUST be same size as queue to guarantee push always succeeds after allocate EventPool chunk_pool_; protected: From af031530ce03148c891f854790b67771d5d1de8b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 22:48:52 -0500 Subject: [PATCH 23/26] remove debug --- esphome/components/usb_uart/usb_uart.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index c6a0e56adc..aea8234b55 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -175,9 +175,7 @@ void USBUartComponent::loop() { // Process USB data from the lock-free queue UsbDataChunk *chunk; - int chunks_processed = 0; while ((chunk = this->usb_data_queue_.pop()) != nullptr) { - chunks_processed++; auto *channel = chunk->channel; #ifdef USE_UART_DEBUGGER @@ -194,11 +192,6 @@ void USBUartComponent::loop() { this->chunk_pool_.release(chunk); } - static constexpr int LOG_CHUNK_THRESHOLD = 5; - if (chunks_processed > LOG_CHUNK_THRESHOLD) { - ESP_LOGV(TAG, "Processed %d chunks from USB queue", chunks_processed); - } - // Log dropped USB data periodically uint16_t dropped = this->usb_data_queue_.get_and_reset_dropped_count(); if (dropped > 0) { From dca79872bfe32e9511e75982ee60a2eae991d525 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 23:03:25 -0500 Subject: [PATCH 24/26] simplify --- esphome/components/usb_host/usb_host.h | 1 + esphome/components/usb_host/usb_host_client.cpp | 12 +++++------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/esphome/components/usb_host/usb_host.h b/esphome/components/usb_host/usb_host.h index afd76595db..d2ff4da068 100644 --- a/esphome/components/usb_host/usb_host.h +++ b/esphome/components/usb_host/usb_host.h @@ -35,6 +35,7 @@ static const size_t SETUP_PACKET_SIZE = 8; static const size_t MAX_REQUESTS = 16; // maximum number of outstanding requests possible. static constexpr size_t USB_EVENT_QUEUE_SIZE = 32; // Size of event queue between USB task and main loop static constexpr size_t USB_TASK_STACK_SIZE = 4096; // Stack size for USB task (same as ESP-IDF USB examples) +static constexpr UBaseType_t USB_TASK_PRIORITY = 5; // Higher priority than main loop (tskIDLE_PRIORITY + 5) // used to report a transfer status struct TransferStatus { diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index 450bd0a932..d8da440658 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -191,13 +191,11 @@ void USBClient::setup() { } // Create and start USB task - xTaskCreatePinnedToCore(usb_task_fn, "usb_task", - USB_TASK_STACK_SIZE, // Stack size - this, // Task parameter - 5, // Priority (higher than main loop) - &this->usb_task_handle_, - 1 // Core 1 - ); + xTaskCreate(usb_task_fn, "usb_task", + USB_TASK_STACK_SIZE, // Stack size + this, // Task parameter + USB_TASK_PRIORITY, // Priority (higher than main loop) + &this->usb_task_handle_); if (this->usb_task_handle_ == nullptr) { ESP_LOGE(TAG, "Failed to create USB task"); From 07e5ce78ebda7826e7a13e1fb6f88a899b22e2e8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 23:04:20 -0500 Subject: [PATCH 25/26] simplify --- esphome/components/usb_host/usb_host_client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/usb_host/usb_host_client.cpp b/esphome/components/usb_host/usb_host_client.cpp index d8da440658..5c9d56c7f9 100644 --- a/esphome/components/usb_host/usb_host_client.cpp +++ b/esphome/components/usb_host/usb_host_client.cpp @@ -210,7 +210,7 @@ void USBClient::usb_task_fn(void *arg) { void USBClient::usb_task_loop() { while (true) { - usb_host_client_handle_events(this->handle_, pdMS_TO_TICKS(10)); + usb_host_client_handle_events(this->handle_, portMAX_DELAY); } } From 90921348e9e1c157ba7d836f6ac6c14545210086 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 23 Sep 2025 23:08:17 -0500 Subject: [PATCH 26/26] cleanup --- esphome/components/usb_uart/usb_uart.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/esphome/components/usb_uart/usb_uart.cpp b/esphome/components/usb_uart/usb_uart.cpp index aea8234b55..8603e28d62 100644 --- a/esphome/components/usb_uart/usb_uart.cpp +++ b/esphome/components/usb_uart/usb_uart.cpp @@ -225,6 +225,8 @@ void USBUartComponent::start_input(USBUartChannel *channel) { ESP_LOGV(TAG, "Transfer result: length: %u; status %X", status.data_len, status.error_code); if (!status.success) { ESP_LOGE(TAG, "Control transfer failed, status=%s", esp_err_to_name(status.error_code)); + // On failure, don't restart - let next read_array() trigger it + channel->input_started_.store(false); return; } @@ -249,7 +251,7 @@ void USBUartComponent::start_input(USBUartChannel *channel) { this->usb_data_queue_.push(chunk); } - // Always restart input immediately from USB task + // On success, restart input immediately from USB task for performance // The lock-free queue will handle backpressure channel->input_started_.store(false); this->start_input(channel);