From 687466b6cddef38738e9c93422e8ea1fee38ab55 Mon Sep 17 00:00:00 2001 From: tomaszduda23 Date: Sat, 17 Oct 2026 06:14:51 +0200 Subject: [PATCH] [logger][usb_cdc_acm][core] Add CDC uart to NRF52 (#11634) --- esphome/components/logger/__init__.py | 16 +- esphome/components/usb_cdc_acm/__init__.py | 119 +++++++-- .../components/usb_cdc_acm/usb_cdc_acm.cpp | 43 +++- esphome/components/usb_cdc_acm/usb_cdc_acm.h | 70 +++++- .../usb_cdc_acm/usb_cdc_acm_esp32.cpp | 53 +--- .../usb_cdc_acm/usb_cdc_acm_zephyr.cpp | 230 ++++++++++++++++++ esphome/components/zephyr/__init__.py | 67 ++++- .../components/zephyr_mcumgr/ota/__init__.py | 13 +- esphome/core/defines.h | 2 + .../usb_cdc_acm/test.nrf52-adafruit.yaml | 7 + .../usb_cdc_acm/test.nrf52-mcumgr.yaml | 5 + .../usb_cdc_acm/test.nrf52-xiao-ble.yaml | 8 + tests/unit_tests/test_zephyr_cdc_acm.py | 67 +++++ 13 files changed, 605 insertions(+), 95 deletions(-) create mode 100644 esphome/components/usb_cdc_acm/usb_cdc_acm_zephyr.cpp create mode 100644 tests/components/usb_cdc_acm/test.nrf52-adafruit.yaml create mode 100644 tests/components/usb_cdc_acm/test.nrf52-mcumgr.yaml create mode 100644 tests/components/usb_cdc_acm/test.nrf52-xiao-ble.yaml create mode 100644 tests/unit_tests/test_zephyr_cdc_acm.py diff --git a/esphome/components/logger/__init__.py b/esphome/components/logger/__init__.py index 67137214c1e..cb222b39e34 100644 --- a/esphome/components/logger/__init__.py +++ b/esphome/components/logger/__init__.py @@ -31,9 +31,9 @@ from esphome.components.libretiny.const import ( COMPONENT_RTL87XX, ) from esphome.components.zephyr import ( - zephyr_add_cdc_acm, zephyr_add_overlay, zephyr_add_prj_conf, + zephyr_claim_cdc_acm, ) from esphome.config_helpers import filter_source_files_from_platform import esphome.config_validation as cv @@ -360,6 +360,19 @@ CONFIG_SCHEMA = cv.All( ) +def _final_validate(config: ConfigType) -> None: + # Serial logging over USB CDC writes to CDC ACM port 0 with uart_poll_out() + if ( + CORE.is_nrf52 + and config[CONF_BAUD_RATE] != 0 + and config.get(CONF_HARDWARE_UART) == USB_CDC + ): + zephyr_claim_cdc_acm(0, "logger", write_only=True) + + +FINAL_VALIDATE_SCHEMA = _final_validate + + @coroutine_with_priority(CoroPriority.EARLY_INIT) async def to_code(config: ConfigType) -> None: baud_rate: int = config[CONF_BAUD_RATE] @@ -521,7 +534,6 @@ async def _late_logger_init(config: ConfigType) -> None: if config[CONF_HARDWARE_UART] == USB_CDC: cg.add_define("USE_LOGGER_UART_SELECTION_USB_CDC") zephyr_add_prj_conf("UART_LINE_CTRL", True) - zephyr_add_cdc_acm(config, 0) # Register at end for safe mode await cg.register_component(log, config) diff --git a/esphome/components/usb_cdc_acm/__init__.py b/esphome/components/usb_cdc_acm/__init__.py index 08cd13e3d27..bdc1a964ee1 100644 --- a/esphome/components/usb_cdc_acm/__init__.py +++ b/esphome/components/usb_cdc_acm/__init__.py @@ -1,3 +1,5 @@ +from collections.abc import Iterator + import esphome.codegen as cg from esphome.components import esp32, uart from esphome.components.esp32 import ( @@ -8,15 +10,30 @@ from esphome.components.esp32 import ( VARIANT_ESP32S31, add_idf_sdkconfig_option, ) +from esphome.components.uart import debug_to_code, maybe_empty_debug, uart_ns +from esphome.components.zephyr import zephyr_add_prj_conf, zephyr_claim_cdc_acm import esphome.config_validation as cv -from esphome.const import CONF_ID, CONF_RX_BUFFER_SIZE, CONF_TX_BUFFER_SIZE +from esphome.const import ( + CONF_DEBUG, + CONF_DISABLED, + CONF_ID, + CONF_RX_BUFFER_SIZE, + CONF_TX_BUFFER_SIZE, +) +from esphome.core import CORE +from esphome.cpp_generator import MockObj from esphome.types import ConfigType CODEOWNERS = ["@kbx81"] -AUTO_LOAD = ["uart"] -DEPENDENCIES = ["tinyusb"] DOMAIN = "usb_cdc_acm" + +def AUTO_LOAD() -> list[str]: + if CORE.using_zephyr: + return ["uart"] + return ["uart", "tinyusb"] + + CONF_INTERFACES = "interfaces" usb_cdc_acm_ns = cg.esphome_ns.namespace("usb_cdc_acm") @@ -30,9 +47,24 @@ USBCDCACMInstance = usb_cdc_acm_ns.class_( INTERFACE_SCHEMA = cv.Schema( { cv.GenerateID(): cv.All(cv.declare_id(USBCDCACMInstance), uart.mark_unclocked), + cv.Optional(CONF_DEBUG): maybe_empty_debug, + cv.Optional(CONF_DISABLED): cv.All(cv.boolean, cv.only_on_nrf52), } ) + +def _enabled_interfaces(config: ConfigType) -> Iterator[tuple[int, ConfigType]]: + for index, conf in enumerate(config[CONF_INTERFACES]): + if not conf.get(CONF_DISABLED, False): + yield index, conf + + +def _validate_any_interface_enabled(config: ConfigType) -> ConfigType: + if next(_enabled_interfaces(config), None) is None: + raise cv.Invalid("At least one interface must be enabled") + return config + + # Main component schema CONFIG_SCHEMA = cv.All( cv.Schema( @@ -50,37 +82,70 @@ CONFIG_SCHEMA = cv.All( ), } ).extend(cv.COMPONENT_SCHEMA), - esp32.only_on_variant( - supported=[ - VARIANT_ESP32H4, - VARIANT_ESP32P4, - VARIANT_ESP32S2, - VARIANT_ESP32S3, - VARIANT_ESP32S31, - ], + _validate_any_interface_enabled, + cv.Any( + cv.only_on_nrf52, + cv.All( + cv.only_on_esp32, + esp32.only_on_variant( + supported=[ + VARIANT_ESP32H4, + VARIANT_ESP32P4, + VARIANT_ESP32S2, + VARIANT_ESP32S3, + VARIANT_ESP32S31, + ], + ), + ), ), ) +def _final_validate(config: ConfigType) -> None: + if not CORE.using_zephyr: + return + for index, _ in _enabled_interfaces(config): + zephyr_claim_cdc_acm(index, DOMAIN) + + +FINAL_VALIDATE_SCHEMA = _final_validate + + async def to_code(config: ConfigType) -> None: var = cg.new_Pvariable(config[CONF_ID]) await cg.register_component(var, config) - # Create and register interface instances - for interface_index, interface_conf in enumerate(config[CONF_INTERFACES]): - interface = cg.new_Pvariable(interface_conf[CONF_ID]) - await cg.register_parented(interface, var) - cg.add(interface.set_interface_number(interface_index)) - cg.add(var.add_interface(interface)) - - # Configure TinyUSB with the correct number of CDC interfaces num_interfaces = len(config[CONF_INTERFACES]) - add_idf_sdkconfig_option("CONFIG_TINYUSB_CDC_ENABLED", True) - add_idf_sdkconfig_option("CONFIG_TINYUSB_CDC_COUNT", num_interfaces) - add_idf_sdkconfig_option( - "CONFIG_TINYUSB_CDC_RX_BUFSIZE", config[CONF_RX_BUFFER_SIZE] - ) - add_idf_sdkconfig_option( - "CONFIG_TINYUSB_CDC_TX_BUFSIZE", config[CONF_TX_BUFFER_SIZE] - ) cg.add_define("ESPHOME_MAX_USB_CDC_INSTANCES", num_interfaces) + # Create and register interface instances + for index, conf in _enabled_interfaces(config): + args = ( + (MockObj(f"DEVICE_DT_GET_OR_NULL(DT_NODELABEL(cdc_acm_uart{index}))"),) + if CORE.using_zephyr + else () + ) + interface = cg.new_Pvariable(conf[CONF_ID], *args) + await cg.register_parented(interface, var) + cg.add(interface.set_interface_number(index)) + cg.add(var.add_interface(interface)) + if (debug := conf.get(CONF_DEBUG)) is not None: + cg.add_global(uart_ns.using) + await debug_to_code(debug, interface) + if CORE.using_zephyr: + zephyr_add_prj_conf("UART_LINE_CTRL", True) + zephyr_add_prj_conf("CDC_ACM_DTE_RATE_CALLBACK_SUPPORT", True) + # Every CDC ACM instance (the logger's and any the board tree adds) holds a transfer + # slot per endpoint; the default of 4 leaves a third instance without any + zephyr_add_prj_conf("USB_MAX_NUM_TRANSFERS", 8) + cg.add_define("ESPHOME_CDC_RX_RING_BUFFER_SIZE", config[CONF_RX_BUFFER_SIZE]) + cg.add_define("ESPHOME_CDC_TX_RING_BUFFER_SIZE", config[CONF_TX_BUFFER_SIZE]) + else: + # Configure TinyUSB with the correct number of CDC interfaces + add_idf_sdkconfig_option("CONFIG_TINYUSB_CDC_ENABLED", True) + add_idf_sdkconfig_option("CONFIG_TINYUSB_CDC_COUNT", num_interfaces) + add_idf_sdkconfig_option( + "CONFIG_TINYUSB_CDC_RX_BUFSIZE", config[CONF_RX_BUFFER_SIZE] + ) + add_idf_sdkconfig_option( + "CONFIG_TINYUSB_CDC_TX_BUFSIZE", config[CONF_TX_BUFFER_SIZE] + ) diff --git a/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp b/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp index f24339b4bf3..da12a10b16a 100644 --- a/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp +++ b/esphome/components/usb_cdc_acm/usb_cdc_acm.cpp @@ -1,7 +1,8 @@ #if defined(USE_ESP32_VARIANT_ESP32P4) || defined(USE_ESP32_VARIANT_ESP32S2) || defined(USE_ESP32_VARIANT_ESP32S3) || \ - defined(USE_ESP32_VARIANT_ESP32S31) || defined(USE_ESP32_VARIANT_ESP32H4) + defined(USE_ESP32_VARIANT_ESP32S31) || defined(USE_ESP32_VARIANT_ESP32H4) || defined(USE_ZEPHYR) #include "usb_cdc_acm.h" #include "esphome/core/application.h" +#include "esphome/core/hal.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" namespace esphome::usb_cdc_acm { @@ -15,6 +16,7 @@ USBCDCACMComponent *global_usb_cdc_component = nullptr; // NOLINT(cppcoreguidel // USBCDCACMInstance Implementation //============================================================================== +// NOLINTBEGIN(clang-analyzer-unix.Malloc) void USBCDCACMInstance::queue_line_state_event(bool dtr, bool rts) { // Allocate event from pool CDCEvent *event = this->event_pool_.allocate(); @@ -53,6 +55,7 @@ void USBCDCACMInstance::queue_line_coding_event(uint32_t bit_rate, uint8_t stop_ this->event_queue_.push(event); App.wake_loop_threadsafe(); } +// NOLINTEND(clang-analyzer-unix.Malloc) void USBCDCACMInstance::process_events_() { // Process all pending events from the queue @@ -113,6 +116,44 @@ void USBCDCACMInstance::process_events_() { } } +bool USBCDCACMInstance::peek_byte(uint8_t *data) { + if (this->has_peek_) { + *data = this->peek_buffer_; + return true; + } + + if (this->read_byte(&this->peek_buffer_)) { + *data = this->peek_buffer_; + this->has_peek_ = true; + return true; + } + + return false; +} + +bool USBCDCACMInstance::consume_peek_(uint8_t *&data, size_t &len) { + if (!this->has_peek_) { + return false; + } + // No debug callback here: peek_byte() read this byte through read_array(), which already reported it + *data++ = this->peek_buffer_; + this->has_peek_ = false; + return --len == 0; +} + +void USBCDCACMInstance::record_tx_drop_(size_t dropped) { + // During a sustained host stall the buffer stays full, so this runs for every write; throttle the + // warning. The total is never reset, so each line accounts for every byte dropped so far and the + // first drop since boot is logged unthrottled. + const bool first_drop = this->tx_dropped_bytes_ == 0; + this->tx_dropped_bytes_ += dropped; + const uint32_t now = millis(); + if (first_drop || (now - this->tx_dropped_log_ms_) >= LOG_THROTTLE_MS) { + this->tx_dropped_log_ms_ = now; + ESP_LOGW(TAG, "USB TX itf=%d: buffer full, %zu bytes dropped total", this->itf_, this->tx_dropped_bytes_); + } +} + //============================================================================== // USBCDCACMComponent Implementation //============================================================================== diff --git a/esphome/components/usb_cdc_acm/usb_cdc_acm.h b/esphome/components/usb_cdc_acm/usb_cdc_acm.h index 83cb5de89f8..4dc9c54c985 100644 --- a/esphome/components/usb_cdc_acm/usb_cdc_acm.h +++ b/esphome/components/usb_cdc_acm/usb_cdc_acm.h @@ -1,6 +1,6 @@ #pragma once #if defined(USE_ESP32_VARIANT_ESP32P4) || defined(USE_ESP32_VARIANT_ESP32S2) || defined(USE_ESP32_VARIANT_ESP32S3) || \ - defined(USE_ESP32_VARIANT_ESP32S31) || defined(USE_ESP32_VARIANT_ESP32H4) + defined(USE_ESP32_VARIANT_ESP32S31) || defined(USE_ESP32_VARIANT_ESP32H4) || defined(USE_ZEPHYR) #include "esphome/core/component.h" #include "esphome/core/event_pool.h" @@ -11,14 +11,22 @@ #include #include #include +#ifdef USE_ZEPHYR +#include +#include +#else #include "freertos/ringbuf.h" #include "esp_err.h" #include "tinyusb_cdc_acm.h" +#endif namespace esphome::usb_cdc_acm { static const uint8_t EVENT_QUEUE_SIZE = 12; +// Minimum interval between repeated warnings while a host stall persists +static constexpr uint32_t LOG_THROTTLE_MS = 1000; +#ifndef USE_ZEPHYR // Drain up to out_buf_sz bytes from a byte ring buffer, handling FreeRTOS's wrapped // case with a second read. Shared with the cdc_acm_uart bridge platform, whose worker // tasks drain the same ring buffers. @@ -47,6 +55,7 @@ inline esp_err_t ringbuf_read_bytes(RingbufHandle_t ring_buf, uint8_t *out_buf, return ESP_OK; } +#endif // Callback types for line coding and line state changes using LineCodingCallback = std::function; @@ -93,12 +102,19 @@ class USBCDCACMInstance final : public uart::UARTComponent, public Parenteditf_ = itf; } // Get the CDC port number for this instance uint8_t get_itf() const { return this->itf_; } +#ifdef USE_ZEPHYR + explicit USBCDCACMInstance(const device *uart_dev) : uart_dev_(uart_dev) {} +#else + static void usb_tx_task_fn(void *arg); + void usb_tx_task(); + // Ring buffer accessors for bridge components RingbufHandle_t get_tx_ringbuf() const { return this->usb_tx_ringbuf_; } RingbufHandle_t get_rx_ringbuf() const { return this->usb_rx_ringbuf_; } // Task handle accessor for notifying TX task TaskHandle_t get_tx_task_handle() const { return this->usb_tx_task_handle_; } +#endif // Callback registration for line coding and line state changes void set_line_coding_callback(LineCodingCallback callback) { this->line_coding_callback_ = std::move(callback); } @@ -108,9 +124,6 @@ class USBCDCACMInstance final : public uart::UARTComponent, public Parenteddebug_callback_.call(uart::UART_DIRECTION_RX, data[i]); + } +#endif + } + void report_tx_debug_(const uint8_t *data, size_t len) { +#ifdef USE_UART_DEBUGGER + for (size_t i = 0; i < len; i++) { + this->debug_callback_.call(uart::UART_DIRECTION_TX, data[i]); + } +#endif + } +#ifdef USE_ZEPHYR + static void uart_irq_handler(const device *dev, void *instance); + void uart_rx_process_(); + void uart_tx_process_(); + + const device *uart_dev_; + uint32_t last_line_poll_ms_{0}; + ring_buf rx_ringbuf_; + ring_buf tx_ringbuf_; + std::atomic rx_irq_disabled_{false}; + std::atomic tx_irq_disabled_{false}; + // Set by the IRQ handler when our TX ring buffer and the driver's are both empty + std::atomic tx_drained_{false}; + bool dtr_{false}; + bool rts_{false}; + uint8_t rx_ringbuf_data_[ESPHOME_CDC_RX_RING_BUFFER_SIZE]; + uint8_t tx_ringbuf_data_[ESPHOME_CDC_TX_RING_BUFFER_SIZE]; +#else // True while TX bytes are still in the ring buffer or held by the TX task bool tx_pending_(); TaskHandle_t usb_tx_task_handle_{nullptr}; @@ -146,18 +197,19 @@ class USBCDCACMInstance final : public uart::UARTComponent, public Parented ops instead of inlining // them; atomic inlines correctly on all platforms. std::atomic usb_tx_busy_{0}; +#endif // Running total of bytes dropped by write_array() (never reset), and the timestamp // of the last "buffer full" log line (throttled so a sustained host stall doesn't // flood the log). - uint32_t tx_dropped_bytes_{0}; + size_t tx_dropped_bytes_{0}; uint32_t tx_dropped_log_ms_{0}; + // User-registered callbacks (called from main loop) + LineCodingCallback line_coding_callback_{nullptr}; + LineStateCallback line_state_callback_{nullptr}; // RX buffer for peek functionality uint8_t peek_buffer_{0}; bool has_peek_{false}; uint8_t itf_{0}; - // User-registered callbacks (called from main loop) - LineCodingCallback line_coding_callback_{nullptr}; - LineStateCallback line_state_callback_{nullptr}; // Lock-free queue and event pool for cross-task event passing // Pool sized to queue capacity (SIZE-1) because LockFreeQueue is a ring diff --git a/esphome/components/usb_cdc_acm/usb_cdc_acm_esp32.cpp b/esphome/components/usb_cdc_acm/usb_cdc_acm_esp32.cpp index 9ac94802e17..0f02ddf3c7a 100644 --- a/esphome/components/usb_cdc_acm/usb_cdc_acm_esp32.cpp +++ b/esphome/components/usb_cdc_acm/usb_cdc_acm_esp32.cpp @@ -29,9 +29,6 @@ static constexpr size_t USB_TX_TASK_STACK_SIZE_VV = 8192; // the final TinyUSB flush share this budget. static constexpr uint32_t FLUSH_TIMEOUT_MS = 100; -// Minimum interval between repeated warnings while a host stall persists. -static constexpr uint32_t LOG_THROTTLE_MS = 1000; - static USBCDCACMInstance *get_instance_by_itf(int itf) { if (global_usb_cdc_component == nullptr) { return nullptr; @@ -278,41 +275,15 @@ void USBCDCACMInstance::write_array(const uint8_t *data, size_t len) { // Write data to TX ring buffer BaseType_t send_res = xRingbufferSend(this->usb_tx_ringbuf_, data, len, 0); if (send_res != pdTRUE) { - // During a sustained host stall the ring buffer stays full (that is the intended - // backpressure), so this path runs for every write; throttle the warning so the - // log stays readable. The counter is a running total that is never reset: each - // line reports all bytes dropped so far, so bytes dropped in the tail of one - // stall are still accounted for by the next line, whenever that is. It also makes - // the very first drop since boot detectable, which is logged unthrottled. - const bool first_drop = this->tx_dropped_bytes_ == 0; - this->tx_dropped_bytes_ += len; - const uint32_t now = millis(); - if (first_drop || (now - this->tx_dropped_log_ms_) >= LOG_THROTTLE_MS) { - this->tx_dropped_log_ms_ = now; - ESP_LOGW(TAG, "USB TX itf=%d: buffer full, %" PRIu32 " bytes dropped total", this->itf_, this->tx_dropped_bytes_); - } + // A full ring buffer is the intended backpressure during a host stall + this->record_tx_drop_(len); return; } - // Notify TX task that data is available if (this->usb_tx_task_handle_ != nullptr) { xTaskNotifyGive(this->usb_tx_task_handle_); } -} - -bool USBCDCACMInstance::peek_byte(uint8_t *data) { - if (this->has_peek_) { - *data = this->peek_buffer_; - return true; - } - - if (this->read_byte(&this->peek_buffer_)) { - *data = this->peek_buffer_; - this->has_peek_ = true; - return true; - } - - return false; + this->report_tx_debug_(data, len); } bool USBCDCACMInstance::read_array(uint8_t *data, size_t len) { @@ -320,20 +291,12 @@ bool USBCDCACMInstance::read_array(uint8_t *data, size_t len) { return true; } + if (this->consume_peek_(data, len)) { + return true; + } size_t original_len = len; size_t bytes_read = 0; - // First, use the peek buffer if available - if (this->has_peek_) { - data[0] = this->peek_buffer_; - this->has_peek_ = false; - bytes_read = 1; - data++; - if (--len == 0) { // Decrement len first, then check it... - return true; // No more to read - } - } - // Read remaining bytes from RX ring buffer size_t rx_size = 0; uint8_t *buf = static_cast(xRingbufferReceiveUpTo(this->usb_rx_ringbuf_, &rx_size, 0, len)); @@ -343,6 +306,7 @@ bool USBCDCACMInstance::read_array(uint8_t *data, size_t len) { memcpy(data, buf, rx_size); vRingbufferReturnItem(this->usb_rx_ringbuf_, (void *) buf); + this->report_rx_debug_(data, rx_size); bytes_read += rx_size; data += rx_size; len -= rx_size; @@ -358,6 +322,7 @@ bool USBCDCACMInstance::read_array(uint8_t *data, size_t len) { memcpy(data, buf, rx_size); vRingbufferReturnItem(this->usb_rx_ringbuf_, (void *) buf); + this->report_rx_debug_(data, rx_size); bytes_read += rx_size; return bytes_read == original_len; @@ -421,7 +386,5 @@ uart::UARTFlushResult USBCDCACMInstance::flush() { } } -void USBCDCACMInstance::check_logger_conflict() {} - } // namespace esphome::usb_cdc_acm #endif diff --git a/esphome/components/usb_cdc_acm/usb_cdc_acm_zephyr.cpp b/esphome/components/usb_cdc_acm/usb_cdc_acm_zephyr.cpp new file mode 100644 index 00000000000..ab3f0531b88 --- /dev/null +++ b/esphome/components/usb_cdc_acm/usb_cdc_acm_zephyr.cpp @@ -0,0 +1,230 @@ +#if defined(USE_ZEPHYR) +#include "usb_cdc_acm.h" +#include "esphome/core/application.h" +#include "esphome/core/log.h" +#include +#include +#include "esphome/components/zephyr/cdc_acm.h" + +// flush() depends on how the legacy cdc_acm driver reports TX state (see uart_tx_process_()) +#ifdef CONFIG_USB_DEVICE_STACK_NEXT +#error "usb_cdc_acm supports only the legacy USB device stack (CONFIG_USB_DEVICE_STACK_NEXT must be disabled)" +#endif + +//============================================================================== +// USBCDCACMInstance Implementation +//============================================================================== +namespace esphome::usb_cdc_acm { + +ESPHOME_LOG_TAG(TAG, "usb_cdc_acm"); +static constexpr uint32_t FLUSH_TIMEOUT_MS = 500; +static constexpr uint32_t LINE_STATE_POLL_MS = 100; + +void USBCDCACMInstance::uart_tx_process_() { + uint8_t *data; + uint32_t send_len = ring_buf_get_claim(&this->tx_ringbuf_, &data, UINT32_MAX); + if (send_len == 0) { + // Our ring buffer is empty. Here (tx_ready set) cdc_acm reports the free space in its own + // TX ring buffer. tx_ready is cleared before each USB transfer and set again when it + // completes, so a fully free driver buffer means the host has received everything. + if (uart_irq_tx_ready(this->uart_dev_) == CONFIG_USB_CDC_ACM_RINGBUF_SIZE) { + this->tx_drained_ = true; + } + uart_irq_tx_disable(this->uart_dev_); + return; + } + int sent = uart_fifo_fill(this->uart_dev_, data, send_len); + if (sent <= 0) { + // No progress: the USB device is suspended (host asleep, hub power save) or not configured. + // fill() then returns 0 without clearing tx_ready, and irq_update() always returns 1 for + // cdc_acm, so the IRQ handler would loop forever on the cooperative USB work queue and + // block the main loop. Disabling TX ends that loop, because irq_is_pending() only reports + // TX while the TX IRQ is enabled. loop() turns it back on while data is waiting. + ring_buf_get_finish(&this->tx_ringbuf_, 0); + uart_irq_tx_disable(this->uart_dev_); + this->tx_irq_disabled_ = true; + return; + } + ring_buf_get_finish(&this->tx_ringbuf_, sent); +} + +void USBCDCACMInstance::uart_rx_process_() { + uint8_t *data; + uint32_t recv_len = ring_buf_put_claim(&this->rx_ringbuf_, &data, UINT32_MAX); + if (recv_len == 0) { + // Ring buffer full: disable RX IRQ so the ISR stops firing. read_array() re-enables it + // once it has freed space. + uart_irq_rx_disable(this->uart_dev_); + this->rx_irq_disabled_ = true; + return; + } + int rx = uart_fifo_read(this->uart_dev_, data, recv_len); + if (rx < 0) { + ESP_LOGE(TAG, "Failed to read UART FIFO, err %d", rx); + } + ring_buf_put_finish(&this->rx_ringbuf_, rx > 0 ? rx : 0); +} + +void USBCDCACMInstance::uart_irq_handler(const device *dev, void *instance) { + USBCDCACMInstance *thiz = static_cast(instance); + while (uart_irq_update(dev) && uart_irq_is_pending(dev)) { + if (uart_irq_rx_ready(dev)) { + thiz->uart_rx_process_(); + } + + // No early exit here: pending RX data must still be read. The loop ends once neither + // RX nor TX is pending (both process functions disable their IRQ when they cannot progress). + if (uart_irq_tx_ready(dev)) { + thiz->uart_tx_process_(); + } + } +} + +void USBCDCACMInstance::setup() { + if (!device_is_ready(this->uart_dev_)) { + ESP_LOGE(TAG, "UART Bus %s: is not ready.", this->uart_dev_->name); + this->get_parent()->mark_failed(); + return; + } + usb_enable(nullptr); + ring_buf_init(&this->rx_ringbuf_, ESPHOME_CDC_RX_RING_BUFFER_SIZE, this->rx_ringbuf_data_); + ring_buf_init(&this->tx_ringbuf_, ESPHOME_CDC_TX_RING_BUFFER_SIZE, this->tx_ringbuf_data_); + // The driver only reports the bit rate; the frame format is fixed on USB + this->data_bits_ = 8; + this->stop_bits_ = 1; +#if defined(CONFIG_CDC_ACM_DTE_RATE_CALLBACK_SUPPORT) + zephyr::global_cdc_acm->add_on_rate_callback([this](const device *dev, uint32_t bit_rate) { + if (dev == this->uart_dev_) { + // Queue event for processing in main loop + this->queue_line_coding_event(bit_rate, this->stop_bits_, this->parity_, this->data_bits_); + } + }); +#endif + uart_irq_callback_user_data_set(this->uart_dev_, uart_irq_handler, this); + + uart_irq_rx_enable(this->uart_dev_); +} + +void USBCDCACMInstance::loop() { + // Retry TX stopped by uart_tx_process_() (e.g. after the host resumes from suspend) + if (this->tx_irq_disabled_ && !ring_buf_is_empty(&this->tx_ringbuf_)) { + this->tx_irq_disabled_ = false; + uart_irq_tx_enable(this->uart_dev_); + } + // Line state is polled here on the main loop, so it needs no event queue + const uint32_t now = App.get_loop_component_start_time(); + if (now - this->last_line_poll_ms_ >= LINE_STATE_POLL_MS) { + this->last_line_poll_ms_ = now; + uint32_t dtr_val = 0; + uint32_t rts_val = 0; + uart_line_ctrl_get(this->uart_dev_, UART_LINE_CTRL_DTR, &dtr_val); + uart_line_ctrl_get(this->uart_dev_, UART_LINE_CTRL_RTS, &rts_val); + const bool dtr = dtr_val != 0; + const bool rts = rts_val != 0; + if (dtr != this->dtr_) { + ESP_LOGD(TAG, "UART Bus %s: %s", this->uart_dev_->name, + dtr ? LOG_STR_LITERAL("Opened") : LOG_STR_LITERAL("Closed")); + } + if (dtr != this->dtr_ || rts != this->rts_) { + ESP_LOGV(TAG, "Line state device %s: DTR=%d, RTS=%d", this->uart_dev_->name, dtr, rts); + this->dtr_ = dtr; + this->rts_ = rts; + if (this->line_state_callback_ != nullptr) { + this->line_state_callback_(dtr, rts); + } + } + } + // Line coding changes arrive from the USB thread through the event queue + this->process_events_(); +} + +void USBCDCACMInstance::dump_config() { + ESP_LOGCONFIG(TAG, + "UART Bus %s:\n" + " Opened: %s\n" + " Baud Rate: %" PRIu32 " baud\n" + " Data Bits: %u\n" + " Parity: %s\n" + " Stop bits: %u", + this->uart_dev_->name, YESNO(this->dtr_), this->baud_rate_, this->data_bits_, + LOG_STR_ARG(parity_to_str(this->parity_)), this->stop_bits_); +} + +size_t USBCDCACMInstance::available() { + uint32_t size = ring_buf_size_get(&this->rx_ringbuf_) + (this->has_peek_ ? 1 : 0); + ESP_LOGVV(TAG, "UART Bus %s: available %" PRIu32, this->uart_dev_->name, size); + return size; +} + +bool USBCDCACMInstance::read_array(uint8_t *data, size_t len) { + if (len == 0) { + return true; + } + if (this->available() < len) { + return false; + } + + if (this->consume_peek_(data, len)) { + return true; + } + const uint32_t got = ring_buf_get(&this->rx_ringbuf_, data, len); + // Space is free again: resume RX stopped by uart_rx_process_() + if (got > 0 && this->rx_irq_disabled_.exchange(false)) { + uart_irq_rx_enable(this->uart_dev_); + } + if (got != len) { + ESP_LOGE(TAG, "UART CDC unexpected size"); + return false; + } + this->report_rx_debug_(data, len); + return true; +} + +uart::UARTFlushResult USBCDCACMInstance::flush() { + // The driver only moves data while the host reads the port. With no host listening (DTR off) + // the data cannot drain, so do not stall the caller waiting for it. + if (!this->dtr_) { + return uart::UARTFlushResult::UART_FLUSH_RESULT_FAILED; + } + // flush() and write_array() both run on the main loop, so nothing is added while we wait. + // The IRQ handler sets tx_drained_ once both our ring buffer and the driver are empty. + this->tx_drained_ = false; + // A stall may have left the flag set; clear it once so a handler run during the wait is not undone. + // The handler sets it again if the device is still suspended. + this->tx_irq_disabled_ = false; + uint32_t start = millis(); + while (true) { + // Wakes the IRQ handler now if the driver is idle, or after the transfer in progress + uart_irq_tx_enable(this->uart_dev_); + // ring_buf_is_empty() is not enough: it is already true while the IRQ handler holds + // claimed bytes that it has not yet copied into the driver. + if (this->tx_drained_ && ring_buf_space_get(&this->tx_ringbuf_) == ESPHOME_CDC_TX_RING_BUFFER_SIZE) { + return uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS; + } + // USB device suspended or not configured (see uart_tx_process_()): nothing will drain + if (this->tx_irq_disabled_) { + return uart::UARTFlushResult::UART_FLUSH_RESULT_FAILED; + } + if (millis() - start > FLUSH_TIMEOUT_MS) { + ESP_LOGW(TAG, "Flush timeout"); + return uart::UARTFlushResult::UART_FLUSH_RESULT_TIMEOUT; + } + delay(1); + } +} + +void USBCDCACMInstance::write_array(const uint8_t *data, size_t len) { + if (!device_is_ready(this->uart_dev_)) { + return; + } + uint32_t recv_len = ring_buf_put(&this->tx_ringbuf_, data, len); + if (recv_len < len) { + this->record_tx_drop_(len - recv_len); + } + uart_irq_tx_enable(this->uart_dev_); + // Only report the bytes that were queued, not the dropped ones + this->report_tx_debug_(data, recv_len); +} + +} // namespace esphome::usb_cdc_acm +#endif diff --git a/esphome/components/zephyr/__init__.py b/esphome/components/zephyr/__init__.py index 4f69f00339e..1ba3ec098cd 100644 --- a/esphome/components/zephyr/__init__.py +++ b/esphome/components/zephyr/__init__.py @@ -1,7 +1,7 @@ from collections.abc import Callable from pathlib import Path import textwrap -from typing import TypedDict +from typing import NamedTuple, TypedDict import esphome.codegen as cg import esphome.config_validation as cv @@ -68,6 +68,14 @@ class Section: ) +class CdcAcmUser(NamedTuple): + name: str + # Only writes with uart_poll_out() and never sets the IRQ callback (the logger) + write_only: bool + # Can share the port with write-only users (protocols that skip unframed text) + allow_write_only: bool + + class ZephyrData(TypedDict): board: str bootloader: str @@ -78,6 +86,8 @@ class ZephyrData(TypedDict): kconfig: str sysbuild: bool overlay_builder: list[Callable[[], str]] + # CDC ACM port number -> components that use it + cdc_acm_users: dict[int, list[CdcAcmUser]] def zephyr_set_core_data(config: ConfigType) -> None: @@ -89,6 +99,7 @@ def zephyr_set_core_data(config: ConfigType) -> None: "": "", }, # set empty to make sure that overlay is cleared after config change overlay_builder=[], + cdc_acm_users={}, extra_build_files={}, pm_static=[], kconfig="", @@ -177,6 +188,9 @@ def zephyr_to_code(config: ConfigType) -> None: @coroutine_with_priority(CoroPriority.FINAL) async def _cdc_acm_to_code(config: ConfigType) -> None: + # Runs last so these required options override the optional defaults that + # platforms (nrf52) set during their own to_code + _cdc_acm_add_ports() need_cdc_cb = zephyr_data()[KEY_PRJ_CONF][""].get( "CONFIG_CDC_ACM_DTE_RATE_CALLBACK_SUPPORT", (False,) )[0] @@ -205,7 +219,46 @@ def _format_prj_conf_val(value: PrjConfValueType) -> str: raise ValueError -def zephyr_add_cdc_acm(config: ConfigType, id: int) -> None: +def zephyr_claim_cdc_acm( + id: int, + user: str, + *, + write_only: bool = False, + allow_write_only: bool = False, +) -> None: + """Reserve CDC ACM port `id` for `user`. Call from FINAL_VALIDATE_SCHEMA. + + zephyr_to_code() later adds the device tree node and Kconfig options for + every claimed port, so components do not set up the port themselves. + + Components on the same port share one device, and the device has only one + IRQ callback slot: the component set up last takes all received data from + the other. So a port normally has one user. The exception is a `write_only` + user (the logger), which never sets the IRQ callback; it may share the port + with a user that sets `allow_write_only` because its protocol skips text + that is not part of its own frames (mcumgr). + """ + new = CdcAcmUser(user, write_only, allow_write_only) + users = zephyr_data()["cdc_acm_users"].setdefault(id, []) + for other in users: + if other.name == user: + return + if (new.write_only and other.allow_write_only) or ( + other.write_only and new.allow_write_only + ): + continue + raise cv.Invalid( + f"USB CDC ACM port {id} (cdc_acm_uart{id}) is used by both " + f"'{other.name}' and '{user}'. Each port can only be used by one component." + ) + users.append(new) + + +def _cdc_acm_add_ports() -> None: + """Set up every CDC ACM port claimed with zephyr_claim_cdc_acm().""" + users = zephyr_data()["cdc_acm_users"] + if not users: + return framework_ver: cv.Version = CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION] if CORE.is_nrf52 and framework_ver >= cv.Version(3, 2, 0): zephyr_add_prj_conf("CONFIG_USB_DEVICE_STACK_NEXT", False) @@ -217,12 +270,16 @@ def zephyr_add_cdc_acm(config: ConfigType, id: int) -> None: zephyr_add_prj_conf("USB_DEVICE_REMOTE_WAKEUP", False) # prevent logging when buffer is full zephyr_add_prj_conf("USB_CDC_ACM_LOG_LEVEL_WRN", True) - zephyr_add_overlay( + nodes = "".join( f""" - &zephyr_udc0 {{ cdc_acm_uart{id}: cdc_acm_uart{id} {{ compatible = "zephyr,cdc-acm-uart"; - }}; + }};""" + for id in sorted(users) + ) + zephyr_add_overlay( + f""" + &zephyr_udc0 {{{nodes} }}; """ ) diff --git a/esphome/components/zephyr_mcumgr/ota/__init__.py b/esphome/components/zephyr_mcumgr/ota/__init__.py index 44c8be9e374..5b438bbda1c 100644 --- a/esphome/components/zephyr_mcumgr/ota/__init__.py +++ b/esphome/components/zephyr_mcumgr/ota/__init__.py @@ -5,9 +5,9 @@ from esphome.components.nrf52.framework import include_west_project from esphome.components.ota import BASE_OTA_SCHEMA, OTAComponent, ota_to_code from esphome.components.zephyr import ( HexValue, - zephyr_add_cdc_acm, zephyr_add_overlay, zephyr_add_prj_conf, + zephyr_claim_cdc_acm, zephyr_data, ) from esphome.components.zephyr.const import ( @@ -114,6 +114,11 @@ def _validate_bootloader(config: ConfigType) -> None: def _final_validate(config: ConfigType) -> None: _validate_ble_server(config) _validate_bootloader(config) + if (uart := config[CONF_TRANSPORT].get(CONF_HARDWARE_UART)) is not None: + cdc_id = UARTS[uart][1] + if cdc_id >= 0: + # SMP frames are marked, so log text from the logger on the same port is skipped + zephyr_claim_cdc_acm(cdc_id, "zephyr_mcumgr", allow_write_only=True) FINAL_VALIDATE_SCHEMA = _final_validate @@ -160,11 +165,7 @@ async def to_code(config: ConfigType) -> None: request_ble_l2cap_mtu(498) # matches NCS_SAMPLE_MCUMGR_BT_OTA_DFU_SPEEDUP if CONF_HARDWARE_UART in transport: zephyr_add_prj_conf("SERIAL", True) - uart = UARTS[transport[CONF_HARDWARE_UART]] - uart_name = uart[0] - cdc_id = uart[1] - if cdc_id >= 0: - zephyr_add_cdc_acm(config, cdc_id) + uart_name = UARTS[transport[CONF_HARDWARE_UART]][0] zephyr_add_prj_conf("MCUMGR_TRANSPORT_UART", True) zephyr_add_prj_conf("BASE64", True) zephyr_add_prj_conf("CONSOLE", True) diff --git a/esphome/core/defines.h b/esphome/core/defines.h index cd0f0d39103..a5fb354199b 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -761,6 +761,8 @@ // Default counts for static analysis #define ESPHOME_AREA_COUNT 10 +#define ESPHOME_CDC_RX_RING_BUFFER_SIZE 256 +#define ESPHOME_CDC_TX_RING_BUFFER_SIZE 256 #define ESPHOME_COMPONENT_COUNT 50 #define ESPHOME_DEVICE_COUNT 10 #define ESPHOME_ENTITY_ALARM_CONTROL_PANEL_COUNT 1 diff --git a/tests/components/usb_cdc_acm/test.nrf52-adafruit.yaml b/tests/components/usb_cdc_acm/test.nrf52-adafruit.yaml new file mode 100644 index 00000000000..8657c03ecf8 --- /dev/null +++ b/tests/components/usb_cdc_acm/test.nrf52-adafruit.yaml @@ -0,0 +1,7 @@ +# Interface 0 needs CDC ACM port 0, which the logger uses by default on nRF52 +logger: + hardware_uart: UART0 + +usb_cdc_acm: + interfaces: + id: usb_cdc_acm1 diff --git a/tests/components/usb_cdc_acm/test.nrf52-mcumgr.yaml b/tests/components/usb_cdc_acm/test.nrf52-mcumgr.yaml new file mode 100644 index 00000000000..40d58596e59 --- /dev/null +++ b/tests/components/usb_cdc_acm/test.nrf52-mcumgr.yaml @@ -0,0 +1,5 @@ +usb_cdc_acm: + interfaces: + - id: usb_cdc_acm1 + disabled: true + - id: usb_cdc_acm2 diff --git a/tests/components/usb_cdc_acm/test.nrf52-xiao-ble.yaml b/tests/components/usb_cdc_acm/test.nrf52-xiao-ble.yaml new file mode 100644 index 00000000000..871e11705ce --- /dev/null +++ b/tests/components/usb_cdc_acm/test.nrf52-xiao-ble.yaml @@ -0,0 +1,8 @@ +# Interface 0 needs CDC ACM port 0, which the logger uses by default on nRF52 +logger: + baud_rate: 0 + +usb_cdc_acm: + interfaces: + - id: usb_cdc_acm1 + - id: usb_cdc_acm2 diff --git a/tests/unit_tests/test_zephyr_cdc_acm.py b/tests/unit_tests/test_zephyr_cdc_acm.py new file mode 100644 index 00000000000..8d010cd0388 --- /dev/null +++ b/tests/unit_tests/test_zephyr_cdc_acm.py @@ -0,0 +1,67 @@ +"""Tests for CDC ACM port reservation in the Zephyr component.""" + +import pytest + +from esphome.components.zephyr import CdcAcmUser, zephyr_claim_cdc_acm +from esphome.components.zephyr.const import KEY_ZEPHYR +import esphome.config_validation as cv +from esphome.core import CORE + + +@pytest.fixture(autouse=True) +def clean_zephyr_data() -> None: + CORE.data[KEY_ZEPHYR] = {"cdc_acm_users": {}} + + +def _users(id: int) -> list[str]: + return [user.name for user in CORE.data[KEY_ZEPHYR]["cdc_acm_users"][id]] + + +def test_different_ports_can_be_claimed() -> None: + zephyr_claim_cdc_acm(0, "logger", write_only=True) + zephyr_claim_cdc_acm(1, "usb_cdc_acm") + + assert _users(0) == ["logger"] + assert _users(1) == ["usb_cdc_acm"] + + +def test_same_user_can_claim_a_port_again() -> None: + zephyr_claim_cdc_acm(0, "usb_cdc_acm") + zephyr_claim_cdc_acm(0, "usb_cdc_acm") + + assert CORE.data[KEY_ZEPHYR]["cdc_acm_users"][0] == [ + CdcAcmUser("usb_cdc_acm", False, False) + ] + + +LOGGER = ("logger", {"write_only": True}) +RAW = ("usb_cdc_acm", {}) +MCUMGR = ("zephyr_mcumgr", {"allow_write_only": True}) + + +@pytest.mark.parametrize( + ("first", "second", "ok"), + [ + (LOGGER, RAW, False), + (RAW, LOGGER, False), + (LOGGER, MCUMGR, True), + (MCUMGR, LOGGER, True), + ], +) +def test_logger_sharing( + first: tuple[str, dict[str, bool]], second: tuple[str, dict[str, bool]], ok: bool +) -> None: + zephyr_claim_cdc_acm(0, first[0], **first[1]) + if not ok: + with pytest.raises(cv.Invalid, match="is used by both"): + zephyr_claim_cdc_acm(0, second[0], **second[1]) + return + zephyr_claim_cdc_acm(0, second[0], **second[1]) + assert _users(0) == [first[0], second[0]] + + +def test_two_irq_users_are_rejected_even_if_they_allow_the_logger() -> None: + zephyr_claim_cdc_acm(0, "zephyr_mcumgr", allow_write_only=True) + + with pytest.raises(cv.Invalid, match="'zephyr_mcumgr' and 'usb_cdc_acm'"): + zephyr_claim_cdc_acm(0, "usb_cdc_acm")