From f54480ec48abaec9218caef3a6b8b8a4086f0e8a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 12 May 2026 15:20:25 -0500 Subject: [PATCH 01/24] [rp2040] Bump arduino-pico framework to 5.6.0 (#16375) --- .clang-tidy.hash | 2 +- esphome/components/rp2040/__init__.py | 6 +++--- esphome/components/rp2040/boards.py | 18 ++++++++++++++++++ platformio.ini | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/.clang-tidy.hash b/.clang-tidy.hash index da2e8632817..77b4f5323fa 100644 --- a/.clang-tidy.hash +++ b/.clang-tidy.hash @@ -1 +1 @@ -96c95feaa60831da5f43e3c6a7c7a3a237e17c5d12995a730dbc3884c8dcd11c +593fd53fa09944a59af3f38521e31d87fe10b60326b8d82bb76413c5149b312c diff --git a/esphome/components/rp2040/__init__.py b/esphome/components/rp2040/__init__.py index 81809c05519..862d532645a 100644 --- a/esphome/components/rp2040/__init__.py +++ b/esphome/components/rp2040/__init__.py @@ -139,7 +139,7 @@ def _parse_platform_version(value): # The default/recommended arduino framework version # - https://github.com/earlephilhower/arduino-pico/releases # - https://api.registry.platformio.org/v3/packages/earlephilhower/tool/framework-arduinopico -RECOMMENDED_ARDUINO_FRAMEWORK_VERSION = cv.Version(5, 5, 1) +RECOMMENDED_ARDUINO_FRAMEWORK_VERSION = cv.Version(5, 6, 0) # The raspberrypi platform version to use for arduino frameworks # - https://github.com/maxgerhardt/platform-raspberrypi/tags @@ -149,8 +149,8 @@ RECOMMENDED_ARDUINO_PLATFORM_VERSION = "v1.4.0-gcc14-arduinopico460" def _arduino_check_versions(value): value = value.copy() lookups = { - "dev": (cv.Version(5, 5, 1), "https://github.com/earlephilhower/arduino-pico"), - "latest": (cv.Version(5, 5, 1), None), + "dev": (cv.Version(5, 6, 0), "https://github.com/earlephilhower/arduino-pico"), + "latest": (cv.Version(5, 6, 0), None), "recommended": (RECOMMENDED_ARDUINO_FRAMEWORK_VERSION, None), } diff --git a/esphome/components/rp2040/boards.py b/esphome/components/rp2040/boards.py index aac12eae5ae..1f2b3a93f43 100644 --- a/esphome/components/rp2040/boards.py +++ b/esphome/components/rp2040/boards.py @@ -457,6 +457,19 @@ RP2040_BOARD_PINS = { "SS": 17, "TX": 12, }, + "challenger_2350_nbiot": { + "LED": 15, + "MISO": 16, + "MOSI": 19, + "RX": 13, + "SCK": 18, + "SCL": 21, + "SCL1": 31, + "SDA": 20, + "SDA1": 31, + "SS": 17, + "TX": 12, + }, "challenger_2350_wifi6_ble5": { "LED": 7, "MISO": 16, @@ -1711,6 +1724,11 @@ BOARDS = { "mcu": "rp2350", "max_pin": 47, }, + "challenger_2350_nbiot": { + "name": "iLabs Challenger 2350 NB-IoT", + "mcu": "rp2350", + "max_pin": 47, + }, "challenger_2350_wifi6_ble5": { "name": "iLabs Challenger 2350 WiFi/BLE", "mcu": "rp2350", diff --git a/platformio.ini b/platformio.ini index 42b74007793..8a89f96b39a 100644 --- a/platformio.ini +++ b/platformio.ini @@ -193,7 +193,7 @@ board_build.filesystem_size = 0.5m platform = https://github.com/maxgerhardt/platform-raspberrypi.git#v1.4.0-gcc14-arduinopico460 platform_packages = ; earlephilhower/framework-arduinopico@~1.20602.0 ; Cannot use the platformio package until old releases stop getting deleted - earlephilhower/framework-arduinopico@https://github.com/earlephilhower/arduino-pico/releases/download/5.5.1/rp2040-5.5.1.zip + earlephilhower/framework-arduinopico@https://github.com/earlephilhower/arduino-pico/releases/download/5.6.0/rp2040-5.6.0.zip framework = arduino lib_deps = From ee72efa76094edfd7a1bc28c2e98aba228282437 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 12 May 2026 16:27:25 -0400 Subject: [PATCH 02/24] [sendspin] Fix client_id MAC mismatch with ethernet (#16331) --- esphome/components/sendspin/sendspin_hub.cpp | 19 +++++++++++++++++-- esphome/components/sendspin/sendspin_hub.h | 8 +++++++- .../sendspin/test-ethernet.esp32-idf.yaml | 9 +++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 tests/components/sendspin/test-ethernet.esp32-idf.yaml diff --git a/esphome/components/sendspin/sendspin_hub.cpp b/esphome/components/sendspin/sendspin_hub.cpp index d27c5672eb6..04426b8b1da 100644 --- a/esphome/components/sendspin/sendspin_hub.cpp +++ b/esphome/components/sendspin/sendspin_hub.cpp @@ -3,6 +3,9 @@ #ifdef USE_ESP32 #include "esphome/components/network/util.h" +#ifdef USE_ETHERNET +#include "esphome/components/ethernet/ethernet_component.h" +#endif #ifdef USE_WIFI #include "esphome/components/wifi/wifi_component.h" #endif @@ -63,7 +66,7 @@ void SendspinHub::dump_config() { "Sendspin Hub:\n" " Client ID: %s\n" " Task stack in PSRAM: %s", - get_mac_address_pretty_into_buffer(mac_buf), YESNO(this->task_stack_in_psram_)); + get_client_id_into_buffer(mac_buf), YESNO(this->task_stack_in_psram_)); } // --- Delegating methods --- @@ -89,11 +92,23 @@ void SendspinHub::update_state(sendspin::SendspinClientState state) { } } +const char *SendspinHub::get_client_id_into_buffer(std::span buf) { + // The server matches client_id against the L2 source MAC of the device's multicast traffic. + // ESP-IDF derives the ethernet MAC as base+3 by default on ESP32-S3, so we cannot use the + // eFuse base MAC when ethernet is the active interface. +#ifdef USE_ETHERNET + if (ethernet::global_eth_component != nullptr) { + return ethernet::global_eth_component->get_eth_mac_address_pretty_into_buffer(buf); + } +#endif + return get_mac_address_pretty_into_buffer(buf); +} + sendspin::SendspinClientConfig SendspinHub::build_client_config_() { sendspin::SendspinClientConfig config; char mac_buf[MAC_ADDRESS_PRETTY_BUFFER_SIZE]; - config.client_id = get_mac_address_pretty_into_buffer(mac_buf); + config.client_id = SendspinHub::get_client_id_into_buffer(mac_buf); config.name = App.get_friendly_name(); config.product_name = App.get_name(); config.manufacturer = "ESPHome"; diff --git a/esphome/components/sendspin/sendspin_hub.h b/esphome/components/sendspin/sendspin_hub.h index 12fbf156ea6..c6b1ed97f74 100644 --- a/esphome/components/sendspin/sendspin_hub.h +++ b/esphome/components/sendspin/sendspin_hub.h @@ -35,7 +35,9 @@ namespace esphome::sendspin_ { /// without each subcomponent having to pick a priority independently. Children run /// one step later than hub so they can assume hub's setup() has already completed. namespace sendspin_priority { -inline constexpr float HUB = esphome::setup_priority::PROCESSOR; +// AFTER_WIFI so the hub runs after the wifi/ethernet drivers are up and we can read the active +// interface's MAC for client_id. +inline constexpr float HUB = esphome::setup_priority::AFTER_WIFI; inline constexpr float CHILD = HUB - 1.0f; } // namespace sendspin_priority @@ -149,6 +151,10 @@ class SendspinHub final : public Component, /// @brief Builds the SendspinClientConfig from ESPHome configuration and platform info. sendspin::SendspinClientConfig build_client_config_(); + /// @brief Writes the active network interface's MAC into @p buf and returns its data pointer. + /// Uses the ethernet MAC if ethernet is configured, otherwise the base MAC (used by wifi). + static const char *get_client_id_into_buffer(std::span buf); + // --- SendspinClientListener overrides --- void on_group_update(const sendspin::GroupUpdateObject &group) override; diff --git a/tests/components/sendspin/test-ethernet.esp32-idf.yaml b/tests/components/sendspin/test-ethernet.esp32-idf.yaml new file mode 100644 index 00000000000..069e397d992 --- /dev/null +++ b/tests/components/sendspin/test-ethernet.esp32-idf.yaml @@ -0,0 +1,9 @@ +ethernet: + type: OPENETH + +psram: + mode: quad + +sendspin: + id: sendspin_hub_id + task_stack_in_psram: true From 66e4a1dfa831ca497028f3cc2a7bddcc6865c740 Mon Sep 17 00:00:00 2001 From: Boris Krivonog Date: Tue, 12 May 2026 22:39:21 +0200 Subject: [PATCH 03/24] [mitsubishi_cn105] Add C++ API for setting/clearing remote room temperature (#15558) Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: J. Nick Koston --- .../components/mitsubishi_cn105/climate.py | 71 ++++++++- .../mitsubishi_cn105/mitsubishi_cn105.cpp | 91 ++++++++--- .../mitsubishi_cn105/mitsubishi_cn105.h | 33 ++-- .../mitsubishi_cn105_climate.h | 22 ++- .../climate/mitsubishi_cn105_tests.cpp | 147 ++++++++++++++++++ tests/components/mitsubishi_cn105/common.h | 3 + tests/components/mitsubishi_cn105/common.yaml | 10 ++ 7 files changed, 338 insertions(+), 39 deletions(-) diff --git a/esphome/components/mitsubishi_cn105/climate.py b/esphome/components/mitsubishi_cn105/climate.py index 7fa6825ea60..cc44494d894 100644 --- a/esphome/components/mitsubishi_cn105/climate.py +++ b/esphome/components/mitsubishi_cn105/climate.py @@ -1,8 +1,11 @@ +from esphome import automation import esphome.codegen as cg from esphome.components import climate, uart import esphome.config_validation as cv -from esphome.const import CONF_UPDATE_INTERVAL -from esphome.types import ConfigType +from esphome.const import CONF_ID, CONF_TEMPERATURE, CONF_UPDATE_INTERVAL +from esphome.core import ID +from esphome.cpp_generator import MockObj +from esphome.types import ConfigType, TemplateArgsType DEPENDENCIES = ["uart"] AUTO_LOAD = ["climate"] @@ -19,6 +22,18 @@ MitsubishiCN105Climate = mitsubishi_ns.class_( uart.UARTDevice, ) +SetRemoteTemperatureAction = mitsubishi_ns.class_( + "SetRemoteTemperatureAction", + automation.Action, + cg.Parented.template(MitsubishiCN105Climate), +) + +ClearRemoteTemperatureAction = mitsubishi_ns.class_( + "ClearRemoteTemperatureAction", + automation.Action, + cg.Parented.template(MitsubishiCN105Climate), +) + CONFIG_SCHEMA = ( climate.climate_schema(MitsubishiCN105Climate) .extend(uart.UART_DEVICE_SCHEMA) @@ -53,3 +68,55 @@ async def to_code(config: ConfigType) -> None: config[CONF_CURRENT_TEMPERATURE_MIN_INTERVAL] ) ) + + +@automation.register_action( + "climate.mitsubishi_cn105.set_remote_temperature", + SetRemoteTemperatureAction, + cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Climate), + cv.Required(CONF_TEMPERATURE): cv.templatable( + cv.All( + cv.temperature, + cv.Range(min=8.0, max=39.5), + ) + ), + } + ), + synchronous=True, +) +async def set_remote_temperature_action_to_code( + config: ConfigType, + action_id: ID, + template_arg: cg.TemplateArguments, + args: TemplateArgsType, +) -> MockObj: + var = cg.new_Pvariable(action_id, template_arg) + await cg.register_parented(var, config[CONF_ID]) + + temperature = await cg.templatable(config[CONF_TEMPERATURE], args, float) + cg.add(var.set_temperature(temperature)) + + return var + + +@automation.register_action( + "climate.mitsubishi_cn105.clear_remote_temperature", + ClearRemoteTemperatureAction, + cv.Schema( + { + cv.Required(CONF_ID): cv.use_id(MitsubishiCN105Climate), + } + ), + synchronous=True, +) +async def clear_remote_temperature_action_to_code( + config: ConfigType, + action_id: ID, + template_arg: cg.TemplateArguments, + args: TemplateArgsType, +) -> MockObj: + var = cg.new_Pvariable(action_id, template_arg) + await cg.register_parented(var, config[CONF_ID]) + return var diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp index f04a5906c16..2a173997f3f 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -92,6 +93,7 @@ void MitsubishiCN105::initialize() { this->set_state_(State::CONNECTING); } bool MitsubishiCN105::update() { if (const auto start = this->status_update_start_ms_) { if (this->pending_updates_.any()) { + this->status_update_wait_credit_ms_ = std::min(this->update_interval_ms_, get_loop_time_ms() - *start); this->cancel_waiting_and_transition_to_(State::APPLYING_SETTINGS); return false; } @@ -105,6 +107,7 @@ bool MitsubishiCN105::update() { if (const auto start = this->write_timeout_start_ms_; start && (get_loop_time_ms() - *start) >= WRITE_TIMEOUT_MS) { this->write_timeout_start_ms_.reset(); this->frame_parser_.reset(); + this->status_update_wait_credit_ms_ = 0; this->set_state_(State::READ_TIMEOUT); return false; } @@ -191,14 +194,14 @@ void MitsubishiCN105::did_transition_(State to) { } case State::SCHEDULE_NEXT_STATUS_UPDATE: - this->status_update_start_ms_ = get_loop_time_ms(); + this->status_update_start_ms_ = get_loop_time_ms() - this->status_update_wait_credit_ms_; + this->status_update_wait_credit_ms_ = 0; this->current_status_msg_type_ = STATUS_MSG_SETTINGS; this->set_state_(State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); break; case State::APPLYING_SETTINGS: this->apply_settings_(); - this->pending_updates_.clear(); break; case State::SETTINGS_APPLIED: @@ -309,21 +312,21 @@ bool MitsubishiCN105::parse_status_settings_(const uint8_t *payload, size_t len) return false; } - if (!this->pending_updates_.has(UpdateFlag::POWER)) { + if (!this->pending_updates_.contains(UpdateFlag::POWER)) { this->status_.power_on = payload[2] != 0; } this->use_temperature_encoding_b_ = payload[10] != 0; - if (!this->pending_updates_.has(UpdateFlag::TEMPERATURE)) { + if (!this->pending_updates_.contains(UpdateFlag::TEMPERATURE)) { this->status_.target_temperature = decode_temperature(-payload[4], payload[10], TARGET_TEMPERATURE_ENC_A_OFFSET); } - if (!this->pending_updates_.has(UpdateFlag::MODE)) { + if (!this->pending_updates_.contains(UpdateFlag::MODE)) { const bool i_see = payload[3] > 0x08; this->status_.mode = lookup(PROTOCOL_MODE_MAP, payload[3] - (i_see ? 0x08 : 0)).value_or(Mode::UNKNOWN); } - if (!this->pending_updates_.has(UpdateFlag::FAN)) { + if (!this->pending_updates_.contains(UpdateFlag::FAN)) { this->status_.fan_mode = lookup(PROTOCOL_FAN_MODE_MAP, payload[5]).value_or(FanMode::UNKNOWN); } @@ -342,6 +345,27 @@ bool MitsubishiCN105::parse_status_room_temperature_(const uint8_t *payload, siz return true; } +void MitsubishiCN105::set_remote_temperature(float temperature) { + if (std::isnan(temperature)) { + ESP_LOGD(TAG, "Ignoring NaN remote temperature"); + return; + } + if (temperature < 8.0f || temperature > 39.5f) { + ESP_LOGD(TAG, "Ignoring out-of-range remote temperature: %.1f", temperature); + return; + } + this->set_remote_temperature_half_deg_(static_cast(std::round(temperature * 2.0f))); +} + +void MitsubishiCN105::clear_remote_temperature() { + this->set_remote_temperature_half_deg_(REMOTE_TEMPERATURE_DISABLED); +} + +void MitsubishiCN105::set_remote_temperature_half_deg_(uint8_t temperature_half_deg) { + this->remote_temperature_half_deg_ = temperature_half_deg; + this->pending_updates_.set(UpdateFlag::REMOTE_TEMPERATURE); +} + void MitsubishiCN105::set_power(bool power_on) { this->status_.power_on = power_on; this->pending_updates_.set(UpdateFlag::POWER); @@ -377,30 +401,47 @@ void MitsubishiCN105::set_fan_mode(FanMode fan_mode) { } void MitsubishiCN105::apply_settings_() { - std::array payload = {0x01}; + std::array payload{}; - if (this->pending_updates_.has(UpdateFlag::POWER)) { - payload[1] |= 0x01; - payload[3] = this->status_.power_on ? 0x01 : 0x00; - } - - if (this->pending_updates_.has(UpdateFlag::TEMPERATURE)) { - payload[1] |= 0x04; - if (this->use_temperature_encoding_b_) { - payload[14] = static_cast(std::round(this->status_.target_temperature * 2.0f) + 128); + // Apply all other pending settings first; handle REMOTE_TEMPERATURE last + if (this->pending_updates_.contains_only(UpdateFlag::REMOTE_TEMPERATURE)) { + payload[0] = 0x07; + if (this->remote_temperature_half_deg_ == REMOTE_TEMPERATURE_DISABLED) { + payload[3] = 0x80; } else { - payload[5] = static_cast(TARGET_TEMPERATURE_ENC_A_OFFSET - std::round(this->status_.target_temperature)); + payload[1] = 0x01; + payload[2] = static_cast(this->remote_temperature_half_deg_ - 16); + payload[3] = static_cast(this->remote_temperature_half_deg_ + 128); + } + this->pending_updates_.clear(UpdateFlag::REMOTE_TEMPERATURE); + } else { + payload[0] = 0x01; + if (this->pending_updates_.contains(UpdateFlag::POWER)) { + payload[1] |= 0x01; + payload[3] = this->status_.power_on ? 0x01 : 0x00; } - } - if (this->pending_updates_.has(UpdateFlag::MODE) && - reverse_lookup(PROTOCOL_MODE_MAP, this->status_.mode, payload[4])) { - payload[1] |= 0x02; - } + if (this->pending_updates_.contains(UpdateFlag::TEMPERATURE)) { + payload[1] |= 0x04; + if (this->use_temperature_encoding_b_) { + payload[14] = static_cast(std::round(this->status_.target_temperature * 2.0f) + 128); + } else { + payload[5] = + static_cast(TARGET_TEMPERATURE_ENC_A_OFFSET - std::round(this->status_.target_temperature)); + } + } - if (this->pending_updates_.has(UpdateFlag::FAN) && - reverse_lookup(PROTOCOL_FAN_MODE_MAP, this->status_.fan_mode, payload[6])) { - payload[1] |= 0x08; + if (this->pending_updates_.contains(UpdateFlag::MODE) && + reverse_lookup(PROTOCOL_MODE_MAP, this->status_.mode, payload[4])) { + payload[1] |= 0x02; + } + + if (this->pending_updates_.contains(UpdateFlag::FAN) && + reverse_lookup(PROTOCOL_FAN_MODE_MAP, this->status_.fan_mode, payload[6])) { + payload[1] |= 0x08; + } + + this->pending_updates_.clear(UpdateFlag::POWER, UpdateFlag::TEMPERATURE, UpdateFlag::MODE, UpdateFlag::FAN); } this->send_packet_(make_packet(PACKET_TYPE_WRITE_SETTINGS_REQUEST, payload)); diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h index 68d98bf6d9d..52b78efccda 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105.h @@ -2,6 +2,7 @@ #include #include "esphome/components/uart/uart.h" +#include "esphome/core/finite_set_mask.h" namespace esphome::mitsubishi_cn105 { @@ -60,6 +61,8 @@ class MitsubishiCN105 { void set_target_temperature(float target_temperature); void set_mode(Mode mode); void set_fan_mode(FanMode fan_mode); + void set_remote_temperature(float temperature); + void clear_remote_temperature(); protected: enum class State : uint8_t { @@ -91,20 +94,25 @@ class MitsubishiCN105 { }; enum class UpdateFlag : uint8_t { - TEMPERATURE = 1 << 0, - POWER = 1 << 1, - MODE = 1 << 2, - FAN = 1 << 3, + TEMPERATURE = 0, + POWER = 1, + MODE = 2, + FAN = 3, + REMOTE_TEMPERATURE = 4, }; struct UpdateFlags { - void set(UpdateFlag f) { flags_ |= static_cast(f); } - void clear() { flags_ = 0; } - bool any() const { return flags_ != 0; } - bool has(UpdateFlag f) const { return (flags_ & static_cast(f)) != 0; } + template void set(Flags... flags) { (this->mask_.insert(flags), ...); } + template void clear(Flags... flags) { (this->mask_.erase(flags), ...); } + bool any() const { return !this->mask_.empty(); } + bool contains(UpdateFlag flag) const { return this->mask_.count(flag); } + bool contains_only(UpdateFlag flag) const { return this->mask_.get_mask() == Mask{flag}.get_mask(); } protected: - uint8_t flags_{0}; + using Mask = + FiniteSetMask(UpdateFlag::REMOTE_TEMPERATURE) + 1>>; + + Mask mask_; }; void set_state_(State new_state); @@ -119,12 +127,14 @@ class MitsubishiCN105 { void cancel_waiting_and_transition_to_(State state); bool should_request_room_temperature_() const; void apply_settings_(); + void set_remote_temperature_half_deg_(uint8_t temperature_half_deg); template void send_packet_(const T &packet) { this->send_packet_(packet.data(), packet.size()); } static bool should_transition(State from, State to); static const LogString *state_to_string(State state); uart::UARTDevice &device_; uint32_t update_interval_ms_{1000}; + uint32_t status_update_wait_credit_ms_{0}; uint32_t room_temperature_min_interval_ms_{60000}; std::optional write_timeout_start_ms_; std::optional status_update_start_ms_; @@ -133,8 +143,11 @@ class MitsubishiCN105 { State state_{State::NOT_CONNECTED}; UpdateFlags pending_updates_; bool use_temperature_encoding_b_{false}; - uint8_t current_status_msg_type_{0}; FrameParser frame_parser_; + uint8_t current_status_msg_type_{0}; + + static constexpr uint8_t REMOTE_TEMPERATURE_DISABLED = 0; + uint8_t remote_temperature_half_deg_{REMOTE_TEMPERATURE_DISABLED}; }; } // namespace esphome::mitsubishi_cn105 diff --git a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h index eee4c209669..e09158bfcff 100644 --- a/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h +++ b/esphome/components/mitsubishi_cn105/mitsubishi_cn105_climate.h @@ -1,5 +1,6 @@ #pragma once +#include "esphome/core/automation.h" #include "esphome/core/component.h" #include "esphome/components/climate/climate.h" #include "esphome/components/uart/uart.h" @@ -18,8 +19,11 @@ class MitsubishiCN105Climate : public climate::Climate, public Component, public climate::ClimateTraits traits() override; void control(const climate::ClimateCall &call) override; - void set_update_interval(uint32_t ms) { hp_.set_update_interval(ms); } - void set_current_temperature_min_interval(uint32_t ms) { hp_.set_room_temperature_min_interval(ms); } + void set_update_interval(uint32_t ms) { this->hp_.set_update_interval(ms); } + void set_current_temperature_min_interval(uint32_t ms) { this->hp_.set_room_temperature_min_interval(ms); } + + void set_remote_temperature(float temperature) { this->hp_.set_remote_temperature(temperature); } + void clear_remote_temperature() { this->hp_.clear_remote_temperature(); } protected: void apply_values_(); @@ -27,4 +31,18 @@ class MitsubishiCN105Climate : public climate::Climate, public Component, public MitsubishiCN105 hp_; }; +template +class SetRemoteTemperatureAction : public Action, public Parented { + public: + TEMPLATABLE_VALUE(float, temperature) + + void play(const Ts &...x) override { this->parent_->set_remote_temperature(this->temperature_.value(x...)); } +}; + +template +class ClearRemoteTemperatureAction : public Action, public Parented { + public: + void play(const Ts &...x) override { this->parent_->clear_remote_temperature(); } +}; + } // namespace esphome::mitsubishi_cn105 diff --git a/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp b/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp index 86faaeac784..7615b62d03f 100644 --- a/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp +++ b/tests/components/mitsubishi_cn105/climate/mitsubishi_cn105_tests.cpp @@ -375,14 +375,22 @@ TEST(MitsubishiCN105Tests, ApplyFanModeSpeed1) { TEST(MitsubishiCN105Tests, WriteInterruptsWaitingForNextStatusUpdate) { auto ctx = TestContext{}; + ctx.sut.set_update_interval(2000); + ctx.sut.set_current_time(5000); + // Waiting for next scheduled status update ctx.sut.state_ = TestableMitsubishiCN105::State::STATUS_UPDATED; ctx.sut.set_state(TestableMitsubishiCN105::State::SCHEDULE_NEXT_STATUS_UPDATE); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); + EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{5000}); + EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); // Nothing to do in update (rx empty, no timeout) + ctx.sut.set_current_time(5500); ASSERT_FALSE(ctx.sut.update()); EXPECT_TRUE(ctx.uart.tx.empty()); + EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{5000}); + EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); // Write new values ctx.sut.use_temperature_encoding_b_ = true; @@ -392,11 +400,52 @@ TEST(MitsubishiCN105Tests, WriteInterruptsWaitingForNextStatusUpdate) { ctx.sut.set_fan_mode(MitsubishiCN105::FanMode::AUTO); // Waiting for next status update must be interrupted and new values send to AC + ctx.sut.set_current_time(6000); ASSERT_FALSE(ctx.sut.update()); + EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); + EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 1000); EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::APPLYING_SETTINGS); EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x41, 0x01, 0x30, 0x10, 0x01, 0x0F, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xB2, 0x00, 0xBB)); + // Write ACK response + ctx.uart.push_rx({0xFC, 0x61, 0x01, 0x30, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x5E}); + ctx.sut.set_current_time(6500); + ASSERT_FALSE(ctx.sut.update()); + EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); + EXPECT_EQ(ctx.sut.status_update_start_ms_, std::optional{6500 - 1000}); + EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); +} + +TEST(MitsubishiCN105Tests, SetAndClearRemoteRoomTemp) { + auto ctx = TestContext{}; + + // Set remote temperature + ctx.sut.set_remote_temperature(28.5f); + + ctx.sut.state_ = TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE; + ctx.sut.set_state(TestableMitsubishiCN105::State::APPLYING_SETTINGS); + + EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x41, 0x01, 0x30, 0x10, 0x07, 0x01, 0x29, 0xB9, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x94)); + + // Write ACK response + ctx.uart.push_rx({0xFC, 0x61, 0x01, 0x30, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x5E}); + ASSERT_FALSE(ctx.sut.update()); + EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); + + ctx.uart.tx.clear(); + + // Clear remote temperature + ctx.sut.clear_remote_temperature(); + + ctx.sut.set_state(TestableMitsubishiCN105::State::APPLYING_SETTINGS); + + EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x41, 0x01, 0x30, 0x10, 0x07, 0x00, 0x00, 0x80, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xF7)); + // Write ACK response ctx.uart.push_rx({0xFC, 0x61, 0x01, 0x30, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x5E}); @@ -404,4 +453,102 @@ TEST(MitsubishiCN105Tests, WriteInterruptsWaitingForNextStatusUpdate) { EXPECT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); } +TEST(MitsubishiCN105Tests, ApplyQueuedSettingsThenRemoteRoomTempInSecondWrite) { + auto ctx = TestContext{}; + + // Queue normal settings plus remote temperature together. + ctx.sut.use_temperature_encoding_b_ = true; + ctx.sut.set_power(false); + ctx.sut.set_target_temperature(25.0f); + ctx.sut.set_mode(MitsubishiCN105::Mode::HEAT); + ctx.sut.set_fan_mode(MitsubishiCN105::FanMode::AUTO); + ctx.sut.set_remote_temperature(28.5f); + + // First apply sends only the normal settings write. + ctx.sut.state_ = TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE; + ctx.sut.set_state(TestableMitsubishiCN105::State::APPLYING_SETTINGS); + + EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x41, 0x01, 0x30, 0x10, 0x01, 0x0F, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xB2, 0x00, 0xBB)); + EXPECT_TRUE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::REMOTE_TEMPERATURE)); + EXPECT_FALSE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::POWER)); + EXPECT_FALSE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::TEMPERATURE)); + EXPECT_FALSE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::MODE)); + EXPECT_FALSE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::FAN)); + + // ACK the first write. Remote temperature should still be pending afterward. + ctx.uart.tx.clear(); + ctx.uart.push_rx({0xFC, 0x61, 0x01, 0x30, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x5E}); + ASSERT_FALSE(ctx.sut.update()); + + EXPECT_TRUE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::REMOTE_TEMPERATURE)); + + // The next apply sends the remote-temperature packet and clears the last pending flag. + ctx.uart.tx.clear(); + ctx.sut.set_state(TestableMitsubishiCN105::State::APPLYING_SETTINGS); + + EXPECT_THAT(ctx.uart.tx, ::testing::ElementsAre(0xFC, 0x41, 0x01, 0x30, 0x10, 0x07, 0x01, 0x29, 0xB9, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x94)); + EXPECT_FALSE(ctx.sut.pending_updates_.any()); +} + +TEST(MitsubishiCN105Tests, WriteTimeoutClearsStatusUpdateWaitCreditOnReconnect) { + auto ctx = TestContext{}; + ctx.sut.set_update_interval(2000); + ctx.sut.set_current_time(5000); + + // Start in the scheduled status update wait state. + ctx.sut.state_ = TestableMitsubishiCN105::State::STATUS_UPDATED; + ctx.sut.set_state(TestableMitsubishiCN105::State::SCHEDULE_NEXT_STATUS_UPDATE); + ASSERT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::WAITING_FOR_SCHEDULED_STATUS_UPDATE); + ASSERT_EQ(ctx.sut.status_update_start_ms_, std::optional{5000}); + ASSERT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); + + // Interrupt that wait with a write so credit is accumulated. + ctx.sut.use_temperature_encoding_b_ = true; + ctx.sut.set_power(false); + ctx.sut.set_target_temperature(25.0f); + ctx.sut.set_mode(MitsubishiCN105::Mode::HEAT); + ctx.sut.set_fan_mode(MitsubishiCN105::FanMode::AUTO); + ctx.sut.set_current_time(6000); + ASSERT_FALSE(ctx.sut.update()); + ASSERT_EQ(ctx.sut.state_, TestableMitsubishiCN105::State::APPLYING_SETTINGS); + ASSERT_FALSE(ctx.sut.status_update_start_ms_.has_value()); + ASSERT_EQ(ctx.sut.status_update_wait_credit_ms_, 1000); + + // Do not ACK the write. Advance time far enough to force timeout/reconnect + // handling and verify that stale wait credit is cleared during recovery. + ctx.sut.set_current_time(36000); + ASSERT_FALSE(ctx.sut.update()); + EXPECT_NE(ctx.sut.state_, TestableMitsubishiCN105::State::APPLYING_SETTINGS); + EXPECT_EQ(ctx.sut.status_update_wait_credit_ms_, 0); + EXPECT_FALSE(ctx.sut.status_update_start_ms_.has_value()); +} + +TEST(MitsubishiCN105Tests, SetOutOfRangeRemoteRoomTempIsIgnored) { + auto ctx = TestContext{}; + + ctx.sut.set_remote_temperature(7.0f); + EXPECT_FALSE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::REMOTE_TEMPERATURE)); + + ctx.sut.set_remote_temperature(40.0f); + EXPECT_FALSE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::REMOTE_TEMPERATURE)); + + ctx.sut.set_remote_temperature(NAN); + EXPECT_FALSE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::REMOTE_TEMPERATURE)); +} + +TEST(MitsubishiCN105Tests, SetMinRemoteRoomTemp) { + auto ctx = TestContext{}; + ctx.sut.set_remote_temperature(8.0f); + EXPECT_TRUE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::REMOTE_TEMPERATURE)); +} + +TEST(MitsubishiCN105Tests, SetMaxRemoteRoomTemp) { + auto ctx = TestContext{}; + ctx.sut.set_remote_temperature(39.5f); + EXPECT_TRUE(ctx.sut.pending_updates_.contains(TestableMitsubishiCN105::UpdateFlag::REMOTE_TEMPERATURE)); +} + } // namespace esphome::mitsubishi_cn105::testing diff --git a/tests/components/mitsubishi_cn105/common.h b/tests/components/mitsubishi_cn105/common.h index 0862d64fa7e..d0fdca1ea5a 100644 --- a/tests/components/mitsubishi_cn105/common.h +++ b/tests/components/mitsubishi_cn105/common.h @@ -42,10 +42,13 @@ class TestableMitsubishiCN105 : public MitsubishiCN105 { public: using MitsubishiCN105::MitsubishiCN105; using MitsubishiCN105::State; + using MitsubishiCN105::UpdateFlag; using MitsubishiCN105::state_; using MitsubishiCN105::write_timeout_start_ms_; using MitsubishiCN105::status_update_start_ms_; using MitsubishiCN105::use_temperature_encoding_b_; + using MitsubishiCN105::status_update_wait_credit_ms_; + using MitsubishiCN105::pending_updates_; void set_state(State s) { this->set_state_(s); } void apply_settings() { this->apply_settings_(); } diff --git a/tests/components/mitsubishi_cn105/common.yaml b/tests/components/mitsubishi_cn105/common.yaml index e885ceef817..4b64f51261f 100644 --- a/tests/components/mitsubishi_cn105/common.yaml +++ b/tests/components/mitsubishi_cn105/common.yaml @@ -1,4 +1,14 @@ climate: - platform: mitsubishi_cn105 + id: ac name: "AC Test" uart_id: uart_bus + +esphome: + on_boot: + then: + - climate.mitsubishi_cn105.set_remote_temperature: + id: ac + temperature: 22.0 + - climate.mitsubishi_cn105.clear_remote_temperature: + id: ac From b512cc42a8cd04492049b87e23bfedb0062e5987 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 12 May 2026 16:42:44 -0400 Subject: [PATCH 04/24] [sendspin] Updates sendspin-cpp to v0.5.0 (#16380) --- esphome/components/sendspin/__init__.py | 7 +++++-- .../sendspin/media_source/sendspin_media_source.cpp | 8 -------- .../sendspin/media_source/sendspin_media_source.h | 3 --- esphome/idf_component.yml | 2 +- 4 files changed, 6 insertions(+), 14 deletions(-) diff --git a/esphome/components/sendspin/__init__.py b/esphome/components/sendspin/__init__.py index 1348bf2bfc3..35280020bab 100644 --- a/esphome/components/sendspin/__init__.py +++ b/esphome/components/sendspin/__init__.py @@ -206,12 +206,15 @@ async def to_code(config: ConfigType) -> None: ) # sendspin-cpp library - esp32.add_idf_component(name="sendspin/sendspin-cpp", ref="0.4.0") + esp32.add_idf_component(name="sendspin/sendspin-cpp", ref="0.5.0") cg.add_define("USE_SENDSPIN", True) # for MDNS data = _get_data() + # The color role is not yet wired up in ESPHome; disable it in the library for now. + esp32.add_idf_sdkconfig_option("CONFIG_SENDSPIN_ENABLE_COLOR", False) + # Configure Sendspin roles based on requested features (ESPHome internally via USE_SENDSPIN_*) # and disable building unused code paths in the sendspin-cpp library (IDF SDKConfig via CONFIG_SENDSPIN_ENABLE_*). if data.artwork_support: @@ -264,7 +267,7 @@ async def to_code(config: ConfigType) -> None: # Library defaults: priority 18 (one above httpd_priority 17 so the decoder is not # starved by the HTTP server during the initial encoded-audio burst at stream start), - # interpolation/decode buffer locations PREFER_EXTERNAL. + # decode buffer location PREFER_EXTERNAL. player_struct_fields = [ ("audio_formats", audio_format_structs), ("audio_buffer_capacity", player_cfg[CONF_BUFFER_SIZE]), diff --git a/esphome/components/sendspin/media_source/sendspin_media_source.cpp b/esphome/components/sendspin/media_source/sendspin_media_source.cpp index 0fdfb01c55f..88ff234e831 100644 --- a/esphome/components/sendspin/media_source/sendspin_media_source.cpp +++ b/esphome/components/sendspin/media_source/sendspin_media_source.cpp @@ -188,14 +188,6 @@ void SendspinMediaSource::on_stream_end() { } } -// THREAD CONTEXT: Main loop (PlayerRoleListener lifecycle callback) -void SendspinMediaSource::on_stream_clear() { - if (this->get_state() != media_source::MediaSourceState::IDLE) { - // Only set to IDLE if we were previously in a non-IDLE state, to avoid duplicate state changes - this->set_state_(media_source::MediaSourceState::IDLE); - } -} - // THREAD CONTEXT: Main loop (PlayerRoleListener callback) void SendspinMediaSource::on_volume_changed(uint8_t volume) { this->request_volume_(volume / 100.0f); } diff --git a/esphome/components/sendspin/media_source/sendspin_media_source.h b/esphome/components/sendspin/media_source/sendspin_media_source.h index 3b31716127c..843578783e0 100644 --- a/esphome/components/sendspin/media_source/sendspin_media_source.h +++ b/esphome/components/sendspin/media_source/sendspin_media_source.h @@ -49,9 +49,6 @@ class SendspinMediaSource : public SendspinChild, /// @brief Called when the audio stream ends (main loop thread). void on_stream_end() override; - /// @brief Called when the audio stream is cleared (main loop thread). - void on_stream_clear() override; - /// @brief Called when volume changes (main loop thread). void on_volume_changed(uint8_t volume) override; diff --git a/esphome/idf_component.yml b/esphome/idf_component.yml index 45aaa827c87..814a4031c12 100644 --- a/esphome/idf_component.yml +++ b/esphome/idf_component.yml @@ -100,6 +100,6 @@ dependencies: esp32async/asynctcp: version: 3.4.91 sendspin/sendspin-cpp: - version: 0.4.0 + version: 0.5.0 lvgl/lvgl: version: 9.5.0 From 76d34334250ea1539bea23d3d2ce15adb29652d3 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Tue, 12 May 2026 14:43:48 -0600 Subject: [PATCH 05/24] [cli] Add config-hash command (#15548) Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston --- esphome/__main__.py | 17 +++++++++++++++++ tests/unit_tests/test_main.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/esphome/__main__.py b/esphome/__main__.py index 561391708ee..bca86729171 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -1415,6 +1415,15 @@ def command_config(args: ArgsProtocol, config: ConfigType) -> int | None: return 0 +def command_config_hash(args: ArgsProtocol, config: ConfigType) -> int | None: + # generating code might modify config, so it must be done in order to generate + # a hash that will match what was generated when compiling and then running + # on the device + generate_cpp_contents(config) + safe_print(f"0x{CORE.config_hash:08x}") + return 0 + + def command_vscode(args: ArgsProtocol) -> int | None: from esphome import vscode @@ -1950,6 +1959,7 @@ PRE_CONFIG_ACTIONS = { POST_CONFIG_ACTIONS = { "config": command_config, + "config-hash": command_config_hash, "compile": command_compile, "upload": command_upload, "logs": command_logs, @@ -2063,6 +2073,13 @@ def parse_args(argv): "--show-secrets", help="Show secrets in output.", action="store_true" ) + parser_config_hash = subparsers.add_parser( + "config-hash", help="Calculate the hash of the configuration." + ) + parser_config_hash.add_argument( + "configuration", help="Your YAML configuration file(s).", nargs="+" + ) + parser_compile = subparsers.add_parser( "compile", help="Read the configuration and compile a program." ) diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 0104854e1f5..6ec0069b3a1 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -29,6 +29,7 @@ from esphome.__main__ import ( command_analyze_memory, command_bundle, command_clean_all, + command_config_hash, command_rename, command_run, command_update_all, @@ -3439,6 +3440,33 @@ def test_command_wizard(tmp_path: Path) -> None: mock_wizard.assert_called_once_with(config_file) +def test_command_config_hash( + tmp_path: Path, + capfd: CaptureFixture[str], +) -> None: + """command_config_hash runs codegen then prints CORE.config_hash. + + The printed format must match `0x{config_hash:08x}` used by + generate_build_info_data_cpp so the value can be compared byte-for-byte + against the ESPHOME_CONFIG_HASH embedded in firmware. + """ + setup_core(tmp_path=tmp_path, config={"esphome": {"name": "test"}}) + args = MockArgs() + + # generate_cpp_contents requires real components to be loaded; mock it out + # so this test isolates the command's output contract. The command must + # still call it (codegen can mutate config, which affects the hash). + with patch("esphome.__main__.generate_cpp_contents") as mock_generate: + result = command_config_hash(args, CORE.config) + + assert result == 0 + mock_generate.assert_called_once_with(CORE.config) + + output = strip_ansi_codes(capfd.readouterr().out).strip() + assert re.fullmatch(r"0x[0-9a-f]{8}", output) + assert output == f"0x{CORE.config_hash:08x}" + + def test_command_rename_invalid_characters( tmp_path: Path, capfd: CaptureFixture[str] ) -> None: From 057fc4c1a8e758622e60376f5fe5b965a2124b95 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Wed, 13 May 2026 09:19:27 +1200 Subject: [PATCH 06/24] Move AI instructions to AGENTS.md (#16382) --- .github/copilot-instructions.md | 2 +- .ai/instructions.md => AGENTS.md | 0 CLAUDE.md | 2 +- GEMINI.md | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename .ai/instructions.md => AGENTS.md (100%) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index a4b2fa310ce..be77ac83a18 120000 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1 +1 @@ -../.ai/instructions.md \ No newline at end of file +../AGENTS.md \ No newline at end of file diff --git a/.ai/instructions.md b/AGENTS.md similarity index 100% rename from .ai/instructions.md rename to AGENTS.md diff --git a/CLAUDE.md b/CLAUDE.md index 49e811ff055..47dc3e3d863 120000 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1 +1 @@ -.ai/instructions.md \ No newline at end of file +AGENTS.md \ No newline at end of file diff --git a/GEMINI.md b/GEMINI.md index 49e811ff055..47dc3e3d863 120000 --- a/GEMINI.md +++ b/GEMINI.md @@ -1 +1 @@ -.ai/instructions.md \ No newline at end of file +AGENTS.md \ No newline at end of file From 907ae46aba7f977e47a2b4f04cf898662081dc9e Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Tue, 12 May 2026 19:13:04 -0400 Subject: [PATCH 07/24] [zigbee] Fix init-order and missing-field warnings on native ESP-IDF (#16389) --- .../zigbee/zigbee_attribute_esp32.cpp | 22 +++++++++---------- .../zigbee/zigbee_attribute_esp32.h | 4 ++-- esphome/components/zigbee/zigbee_esp32.cpp | 7 +++--- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/esphome/components/zigbee/zigbee_attribute_esp32.cpp b/esphome/components/zigbee/zigbee_attribute_esp32.cpp index 4d736001715..0a06792c594 100644 --- a/esphome/components/zigbee/zigbee_attribute_esp32.cpp +++ b/esphome/components/zigbee/zigbee_attribute_esp32.cpp @@ -32,10 +32,9 @@ void ZigbeeAttribute::report_(bool has_lock) { return; } if (has_lock or esp_zb_lock_acquire(10 / portTICK_PERIOD_MS)) { - esp_zb_zcl_report_attr_cmd_t cmd = { - .address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT, - .direction = ESP_ZB_ZCL_CMD_DIRECTION_TO_CLI, - }; + esp_zb_zcl_report_attr_cmd_t cmd = {}; + cmd.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; + cmd.direction = ESP_ZB_ZCL_CMD_DIRECTION_TO_CLI; cmd.zcl_basic_cmd.dst_addr_u.addr_short = 0x0000; cmd.zcl_basic_cmd.dst_endpoint = 1; cmd.zcl_basic_cmd.src_endpoint = this->endpoint_id_; @@ -50,14 +49,13 @@ void ZigbeeAttribute::report_(bool has_lock) { } esp_zb_zcl_reporting_info_t ZigbeeAttribute::get_reporting_info() { - esp_zb_zcl_reporting_info_t reporting_info = { - .direction = ESP_ZB_ZCL_CMD_DIRECTION_TO_SRV, - .ep = this->endpoint_id_, - .cluster_id = this->cluster_id_, - .cluster_role = this->role_, - .attr_id = this->attr_id_, - .manuf_code = ESP_ZB_ZCL_ATTR_NON_MANUFACTURER_SPECIFIC, - }; + esp_zb_zcl_reporting_info_t reporting_info = {}; + reporting_info.direction = ESP_ZB_ZCL_CMD_DIRECTION_TO_SRV; + reporting_info.ep = this->endpoint_id_; + reporting_info.cluster_id = this->cluster_id_; + reporting_info.cluster_role = this->role_; + reporting_info.attr_id = this->attr_id_; + reporting_info.manuf_code = ESP_ZB_ZCL_ATTR_NON_MANUFACTURER_SPECIFIC; reporting_info.dst.profile_id = ESP_ZB_AF_HA_PROFILE_ID; reporting_info.u.send_info.min_interval = 10; /*!< Actual minimum reporting interval */ reporting_info.u.send_info.max_interval = 0; /*!< Actual maximum reporting interval */ diff --git a/esphome/components/zigbee/zigbee_attribute_esp32.h b/esphome/components/zigbee/zigbee_attribute_esp32.h index 90a5cf8ff9b..35aa60848f6 100644 --- a/esphome/components/zigbee/zigbee_attribute_esp32.h +++ b/esphome/components/zigbee/zigbee_attribute_esp32.h @@ -37,8 +37,8 @@ class ZigbeeAttribute : public Component { role_(role), attr_id_(attr_id), attr_type_(attr_type), - scale_(scale), - max_size_(max_size) {} + max_size_(max_size), + scale_(scale) {} void loop() override; template void add_attr(T value); esp_zb_zcl_reporting_info_t get_reporting_info(); diff --git a/esphome/components/zigbee/zigbee_esp32.cpp b/esphome/components/zigbee/zigbee_esp32.cpp index c534a384320..ade9e165720 100644 --- a/esphome/components/zigbee/zigbee_esp32.cpp +++ b/esphome/components/zigbee/zigbee_esp32.cpp @@ -204,10 +204,9 @@ static void esp_zb_task_(void *pvParameters) { void ZigbeeComponent::setup() { global_zigbee = this; - esp_zb_platform_config_t config = { - .radio_config = ESP_ZB_DEFAULT_RADIO_CONFIG(), - .host_config = ESP_ZB_DEFAULT_HOST_CONFIG(), - }; + esp_zb_platform_config_t config = {}; + config.radio_config = ESP_ZB_DEFAULT_RADIO_CONFIG(); + config.host_config = ESP_ZB_DEFAULT_HOST_CONFIG(); #ifdef USE_WIFI if (esp_coex_wifi_i154_enable() != ESP_OK) { this->mark_failed(); From aec48cf2316f3d20c601757401692c8ecf5733e3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 May 2026 23:19:07 +0000 Subject: [PATCH 08/24] Bump aioesphomeapi from 44.24.2 to 45.0.0 (#16391) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index c0350518900..6be6a95fe6b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,7 +12,7 @@ platformio==6.1.19 esptool==5.2.0 click==8.3.3 esphome-dashboard==20260425.0 -aioesphomeapi==44.24.2 +aioesphomeapi==45.0.0 zeroconf==0.148.0 puremagic==1.30 ruamel.yaml==0.19.1 # dashboard_import From 9195b9898ee7b8b31e36d0a19d078d6f15b71466 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Tue, 12 May 2026 19:20:09 -0400 Subject: [PATCH 09/24] [ms8607] Pin humidity i2c_id in test fixture (#16386) --- tests/components/ms8607/common.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/components/ms8607/common.yaml b/tests/components/ms8607/common.yaml index 09a3f5a617a..ad2a34308b6 100644 --- a/tests/components/ms8607/common.yaml +++ b/tests/components/ms8607/common.yaml @@ -4,6 +4,7 @@ sensor: temperature: name: Temperature humidity: + i2c_id: i2c_bus name: Humidity pressure: name: Pressure From 45a8bd49c383bd8ebba205ec25f5eb763437e79c Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 12 May 2026 19:33:03 -0400 Subject: [PATCH 10/24] [audio] Add a RingBufferAudioSource (#16314) --- .../audio/audio_transfer_buffer.cpp | 131 ++++++++++++++++++ .../components/audio/audio_transfer_buffer.h | 74 ++++++++++ 2 files changed, 205 insertions(+) diff --git a/esphome/components/audio/audio_transfer_buffer.cpp b/esphome/components/audio/audio_transfer_buffer.cpp index 6ee9e4d28c0..d9ce8060e26 100644 --- a/esphome/components/audio/audio_transfer_buffer.cpp +++ b/esphome/components/audio/audio_transfer_buffer.cpp @@ -207,6 +207,137 @@ void ConstAudioSourceBuffer::consume(size_t bytes) { this->data_start_ += bytes; } +std::unique_ptr RingBufferAudioSource::create( + std::shared_ptr ring_buffer, size_t max_fill_bytes, uint8_t alignment_bytes) { + if (ring_buffer == nullptr || max_fill_bytes == 0 || alignment_bytes == 0 || alignment_bytes > MAX_ALIGNMENT_BYTES) { + return nullptr; + } + return std::unique_ptr( + new RingBufferAudioSource(std::move(ring_buffer), max_fill_bytes, alignment_bytes)); +} + +RingBufferAudioSource::~RingBufferAudioSource() { + if (this->acquired_item_ != nullptr) { + this->ring_buffer_->receive_release(this->acquired_item_); + this->acquired_item_ = nullptr; + } +} + +void RingBufferAudioSource::release_item_() { + if (this->acquired_item_ == nullptr) { + return; + } + if (this->item_trailing_length_ > 0) { + // Copy the trailing sub-frame bytes into the splice buffer before returning the item; the next + // fill() will complete the frame from the head of the next chunk. + std::memcpy(this->splice_buffer_, this->item_trailing_ptr_, this->item_trailing_length_); + this->splice_length_ = this->item_trailing_length_; + this->item_trailing_ptr_ = nullptr; + this->item_trailing_length_ = 0; + } + this->ring_buffer_->receive_release(this->acquired_item_); + this->acquired_item_ = nullptr; +} + +void RingBufferAudioSource::consume(size_t bytes) { + bytes = std::min(bytes, this->current_available_); + this->current_data_ += bytes; + this->current_available_ -= bytes; + // Promotion of queued data is deferred to fill() so callers see new data as a fresh return value + // rather than appearing silently after consume(). When the held item has nothing left depending + // on it (no exposed bytes and no queued region), release it now so the ring buffer can be + // reclaimed by writers even if fill() is never called again. + if (this->current_available_ == 0 && this->queued_length_ == 0) { + this->release_item_(); + } +} + +bool RingBufferAudioSource::has_buffered_data() const { + // splice_length_ is deliberately not considered here. It holds an incomplete frame whose completion + // bytes must still arrive through the ring buffer, which ring_buffer_->available() already reports. + // Counting it separately would strand a drain loop when a stream ends mid-frame and those completion + // bytes never come. + return (this->current_available_ > 0) || (this->queued_length_ > 0) || (this->ring_buffer_->available() > 0); +} + +size_t RingBufferAudioSource::fill(TickType_t ticks_to_wait, bool /*pre_shift*/) { + if (this->current_available_ > 0) { + // Caller has not finished consuming the current exposure + return 0; + } + + // If a queued region (the aligned remainder of the new chunk after a splice frame) is waiting, + // promote it to the exposed region and report its size as fresh data. + if (this->queued_length_ > 0) { + this->current_data_ = this->queued_data_; + this->current_available_ = this->queued_length_; + this->queued_data_ = nullptr; + this->queued_length_ = 0; + return this->current_available_; + } + + // Nothing exposed and nothing queued: release the previously held item (saving any sub-frame tail + // to splice_buffer_) and acquire a new chunk. + this->release_item_(); + + size_t chunk_length = 0; + void *item = this->ring_buffer_->receive_acquire(chunk_length, this->max_fill_bytes_, ticks_to_wait); + if (item == nullptr) { + return 0; + } + + uint8_t *chunk_data = static_cast(item); + bool exposing_splice_frame = false; + + // Complete any pending splice frame from the head of the new chunk. + if (this->splice_length_ > 0) { + const size_t needed = static_cast(this->alignment_bytes_) - this->splice_length_; + if (chunk_length < needed) { + // Not enough data to complete the spliced frame yet; absorb everything and wait for more. + std::memcpy(this->splice_buffer_ + this->splice_length_, chunk_data, chunk_length); + this->splice_length_ += chunk_length; + this->ring_buffer_->receive_release(item); + return 0; + } + std::memcpy(this->splice_buffer_ + this->splice_length_, chunk_data, needed); + chunk_data += needed; + chunk_length -= needed; + this->splice_length_ = 0; + exposing_splice_frame = true; + } + + this->acquired_item_ = item; + + // Split the remaining chunk into its aligned region and a (possibly zero) sub-frame trailing tail. + const size_t trailing = (this->alignment_bytes_ > 1) ? (chunk_length % this->alignment_bytes_) : 0; + const size_t aligned_bytes = chunk_length - trailing; + if (trailing > 0) { + this->item_trailing_ptr_ = chunk_data + aligned_bytes; + this->item_trailing_length_ = trailing; + } + + if (exposing_splice_frame) { + // Expose the spliced frame from splice_buffer_, queuing the chunk's aligned region for the next + // fill() call. + this->current_data_ = this->splice_buffer_; + this->current_available_ = this->alignment_bytes_; + this->queued_data_ = chunk_data; + this->queued_length_ = aligned_bytes; + return this->alignment_bytes_; + } + + if (aligned_bytes == 0) { + // The entire chunk is a sub-frame tail (only possible when alignment exceeds chunk size). Save it + // to the splice buffer and release the item so the next fill() can complete the frame. + this->release_item_(); + return 0; + } + + this->current_data_ = chunk_data; + this->current_available_ = aligned_bytes; + return aligned_bytes; +} + } // namespace esphome::audio #endif diff --git a/esphome/components/audio/audio_transfer_buffer.h b/esphome/components/audio/audio_transfer_buffer.h index 68151bf4e21..d83032582e6 100644 --- a/esphome/components/audio/audio_transfer_buffer.h +++ b/esphome/components/audio/audio_transfer_buffer.h @@ -214,6 +214,80 @@ class ConstAudioSourceBuffer : public AudioReadableBuffer { size_t length_{0}; }; +/// @brief Zero-copy audio source that reads directly from a ring buffer's internal storage. +/// +/// Optionally enforces a minimum read alignment (e.g. one audio frame). When alignment_bytes > 1, the +/// source transparently stitches frames that straddle the ring buffer's wrap boundary by buffering the +/// trailing partial frame from one chunk and joining it with the head of the next chunk in a small +/// internal splice buffer, so callers always see frame-aligned data. +class RingBufferAudioSource : public AudioReadableBuffer { + public: + /// Maximum supported alignment. Sized to cover 32-bit samples across up to 2 channels (8 bytes). + static constexpr size_t MAX_ALIGNMENT_BYTES = 8; + + /// @brief Creates a new ring-buffer-backed audio source after validating its parameters. + /// @param ring_buffer The ring buffer to read from. Must be non-null. + /// @param max_fill_bytes Soft cap on bytes acquired per fill() call. Must be > 0. + /// @param alignment_bytes Minimum exposed-region alignment in bytes (defaults to 1, i.e. byte-aligned). + /// Pass bytes_per_frame to make every exposed region a whole number of frames. Must be in + /// [1, MAX_ALIGNMENT_BYTES]. + /// @return unique_ptr if parameters are valid, nullptr otherwise + static std::unique_ptr create(std::shared_ptr ring_buffer, + size_t max_fill_bytes, uint8_t alignment_bytes = 1); + + ~RingBufferAudioSource() override; + + // AudioReadableBuffer interface + const uint8_t *data() const override { return this->current_data_; } + size_t available() const override { return this->current_available_; } + void consume(size_t bytes) override; + bool has_buffered_data() const override; + size_t fill(TickType_t ticks_to_wait, bool pre_shift) override; + + /// @brief Returns a mutable pointer to the currently exposed audio data. + /// The pointer may reference the ring buffer's internal storage or, when exposing a stitched frame + /// across a wrap boundary, an internal splice buffer. In either case mutations are safe but data + /// should be discarded after use, since the underlying storage will be reused on the next fill(). + /// Use only when the caller is the sole consumer of this source. + uint8_t *mutable_data() { return this->current_data_; } + + protected: + /// @brief Constructs a new ring-buffer-backed audio source. Use create() instead, which validates + /// arguments before construction. + explicit RingBufferAudioSource(std::shared_ptr ring_buffer, size_t max_fill_bytes, + uint8_t alignment_bytes) + : ring_buffer_(std::move(ring_buffer)), max_fill_bytes_(max_fill_bytes), alignment_bytes_(alignment_bytes) {} + + /// @brief Releases the currently held ring buffer item, first copying any trailing sub-frame bytes + /// into the splice buffer so they can be stitched with the next chunk. + void release_item_(); + + std::shared_ptr ring_buffer_; + size_t max_fill_bytes_; + + void *acquired_item_{nullptr}; + uint8_t *current_data_{nullptr}; + + // Sub-frame trailing bytes inside the held item that will be copied to splice_buffer_ on release. + uint8_t *item_trailing_ptr_{nullptr}; + + // After the currently-exposed splice frame is consumed, fill() will promote this region (the aligned + // remainder of the new chunk) to the exposed region. queued_length_ == 0 when nothing is queued. + uint8_t *queued_data_{nullptr}; + + // Splice buffer holds the start of a partial frame whose remainder lives at the head of the next + // chunk. While splice_length_ > 0, the buffer is incomplete and waiting for completion bytes. + uint8_t splice_buffer_[MAX_ALIGNMENT_BYTES]; + + size_t current_available_{0}; + size_t queued_length_{0}; + + // item_trailing_length_ and splice_length_ are bounded by MAX_ALIGNMENT_BYTES. + uint8_t alignment_bytes_; + uint8_t item_trailing_length_{0}; + uint8_t splice_length_{0}; +}; + } // namespace esphome::audio #endif From cb2dbcd70de6de08196e3e9b53a29b841c6d2444 Mon Sep 17 00:00:00 2001 From: Jesse Hills <3060199+jesserockz@users.noreply.github.com> Date: Wed, 13 May 2026 11:37:33 +1200 Subject: [PATCH 11/24] [ci] Add validate.*.yaml for config-only component tests (#16384) --- .github/workflows/ci.yml | 38 ++++- AGENTS.md | 18 ++- script/analyze_component_buses.py | 9 +- script/ci-custom.py | 7 +- script/determine-jobs.py | 46 +++++- script/helpers.py | 28 +++- script/split_components_for_ci.py | 11 +- script/test_build_components.py | 59 +++++++- tests/script/test_determine_jobs.py | 227 ++++++++++++++++++++++++++++ tests/script/test_helpers.py | 168 ++++++++++++++++++++ 10 files changed, 589 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf9d474d7a2..ad39b3f3467 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -261,6 +261,7 @@ jobs: cpp-unit-tests-run-all: ${{ steps.determine.outputs.cpp-unit-tests-run-all }} cpp-unit-tests-components: ${{ steps.determine.outputs.cpp-unit-tests-components }} component-test-batches: ${{ steps.determine.outputs.component-test-batches }} + validate-only-components: ${{ steps.determine.outputs.validate-only-components }} benchmarks: ${{ steps.determine.outputs.benchmarks }} steps: - name: Check out code from GitHub @@ -305,6 +306,7 @@ jobs: echo "cpp-unit-tests-run-all=$(echo "$output" | jq -r '.cpp_unit_tests_run_all')" >> $GITHUB_OUTPUT echo "cpp-unit-tests-components=$(echo "$output" | jq -c '.cpp_unit_tests_components')" >> $GITHUB_OUTPUT echo "component-test-batches=$(echo "$output" | jq -c '.component_test_batches')" >> $GITHUB_OUTPUT + echo "validate-only-components=$(echo "$output" | jq -c '.validate_only_components')" >> $GITHUB_OUTPUT echo "benchmarks=$(echo "$output" | jq -r '.benchmarks')" >> $GITHUB_OUTPUT - name: Save components graph cache if: github.ref == 'refs/heads/dev' @@ -775,13 +777,45 @@ jobs: echo "Config validation passed! Starting compilation..." echo "" + # Compute the compile-stage component list. Components whose only + # changes are validate.*.yaml files are config-only -- their source + # and test fixtures didn't move, so rebuilding firmware adds no + # signal. Subtract them from this batch before invoking compile. + validate_only_json='${{ needs.determine-jobs.outputs.validate-only-components }}' + if [ -z "$validate_only_json" ]; then + validate_only_json='[]' + fi + if ! validate_only_csv=$(echo "$validate_only_json" | jq -r 'join(",")'); then + echo "::error::Failed to render validate-only-components as CSV from: $validate_only_json" + exit 1 + fi + if [ -z "$validate_only_csv" ]; then + compile_csv="$components_csv" + else + components_sorted=$(echo "$components_csv" | tr ',' '\n' | sort -u) + validate_sorted=$(echo "$validate_only_csv" | tr ',' '\n' | sort -u) + if ! diff_out=$(comm -23 <(echo "$components_sorted") <(echo "$validate_sorted")); then + echo "::error::Failed to compute compile component subset." + exit 1 + fi + compile_csv=$(echo "$diff_out" | paste -sd ',' -) + skipped=$(comm -12 <(echo "$components_sorted") <(echo "$validate_sorted") | paste -sd ',' -) + if [ -n "$skipped" ]; then + echo "Validate-only components in this batch (skipping compile): $skipped" + fi + fi + # Show disk space before compilation echo "Disk space before compilation:" df -h echo "" - # Run compilation with grouping and isolation - python3 script/test_build_components.py -e compile -c "$components_csv" -f --isolate "$directly_changed_csv" + if [ -n "$compile_csv" ]; then + # Run compilation with grouping and isolation + python3 script/test_build_components.py -e compile -c "$compile_csv" -f --isolate "$directly_changed_csv" + else + echo "All components in this batch are validate-only -- skipping compile stage." + fi test-native-idf: name: Test components with native ESP-IDF diff --git a/AGENTS.md b/AGENTS.md index 86f554e9cec..2139a2b796d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -398,13 +398,23 @@ This document provides essential context for AI models interacting with this pro │ ├── i2c/ # I2C bus │ └── spi/ # SPI bus └── components/[component]/ - ├── common.yaml # Component-only config (no bus definitions) - ├── test.esp32-idf.yaml - ├── test.esp8266-ard.yaml - └── test.rp2040-ard.yaml + ├── common.yaml # Component-only config (no bus definitions) + ├── test.esp32-idf.yaml # config + compile + ├── test.esp8266-ard.yaml # config + compile + ├── test-variant.esp32-idf.yaml # variant test, config + compile + ├── validate.esp32-idf.yaml # config-only (never compiled) + └── validate-legacy.esp32-idf.yaml # config-only variant ``` Run them using `script/test_build_components`. Use `-c ` to test specific components and `-t ` for specific platforms. + * **Config-only test files (`validate.*.yaml`):** Use this prefix when a YAML file only needs to exercise schema/validation paths and does not need to be compiled. CI runs `validate.*.yaml` files with `esphome config` only and skips them during compile. The grammar mirrors `test.*.yaml`: + - `validate..yaml` — base config-only test + - `validate-..yaml` — config-only variant + + Use this for things like deprecated-syntax migration tests, schema edge cases, or platform-specific validation branches where building firmware adds no signal. A component may have any mix of `test.*.yaml` and `validate.*.yaml` files. Validate files never participate in bus-grouping; each one runs as its own `esphome config` invocation. + + When a PR's only edits to a component are `validate.*.yaml` files (no source changes, no `test.*.yaml` changes, and the component isn't pulled in as a dependency of another changed component), CI skips the compile stage for that component entirely and only runs config validation. This is decided in `script/determine-jobs.py` via `_component_change_is_validate_only` and surfaced as the `validate_only_components` output that the `test-build-components-split` job consumes. + * **Test Grouping with Packages:** Components that use shared bus packages can be grouped together in CI to reduce build count. **Never define buses (uart, i2c, spi, modbus) directly in test YAML files** — always use packages from `test_build_components/common/`: ```yaml # test.esp32-idf.yaml — use packages for buses diff --git a/script/analyze_component_buses.py b/script/analyze_component_buses.py index 17af7af5771..1d86d5c71ca 100755 --- a/script/analyze_component_buses.py +++ b/script/analyze_component_buses.py @@ -34,7 +34,7 @@ from typing import Any # Add esphome to path sys.path.insert(0, str(Path(__file__).parent.parent)) -from helpers import BASE_BUS_COMPONENTS +from helpers import BASE_BUS_COMPONENTS, is_validate_only_file from esphome import yaml_util from esphome.config_helpers import Extend, Remove @@ -283,6 +283,13 @@ def analyze_component(component_dir: Path) -> tuple[dict[str, list[str]], bool, # Analyze all YAML files in the component directory for yaml_file in component_dir.glob("*.yaml"): + # validate.*.yaml files are config-only -- they don't compile, so + # their contents must not influence compile-time grouping decisions + # (e.g. a !extend used only to exercise schema validation must not + # disqualify the whole component from being grouped). + if is_validate_only_file(yaml_file): + continue + analysis = analyze_yaml_file(yaml_file) # Track if any file uses extend/remove diff --git a/script/ci-custom.py b/script/ci-custom.py index 25db32105c6..56ca0d0355e 100755 --- a/script/ci-custom.py +++ b/script/ci-custom.py @@ -1068,7 +1068,12 @@ PACKAGE_BUS_RE = re.compile( ) -@lint_content_check(include=["tests/components/*/test.*.yaml"]) +@lint_content_check( + include=[ + "tests/components/*/test.*.yaml", + "tests/components/*/validate.*.yaml", + ] +) def lint_test_package_key_matches_bus(fname, content): """Ensure package keys match the common bus directory name. diff --git a/script/determine-jobs.py b/script/determine-jobs.py index b8f324784d8..57b3c6eb88e 100755 --- a/script/determine-jobs.py +++ b/script/determine-jobs.py @@ -29,6 +29,8 @@ The CI workflow uses this information to: - Skip or run downstream esphome/device-builder tests against the PR's Python code - Determine which components to test individually - Decide how to split component tests (if there are many) +- Identify directly-changed components whose only edits are validate.*.yaml files, + so CI can skip the compile stage for them and run config validation only - Run memory impact analysis whenever there are changed components (merged config), and also for core-only changes Usage: @@ -68,6 +70,7 @@ from helpers import ( get_integration_test_files_for_components, get_target_branch, git_ls_files, + is_validate_only_file, parse_test_filename, root_path, ) @@ -600,14 +603,41 @@ def _component_has_tests(component: str) -> bool: """Check if a component has test files. Cached to avoid repeated filesystem operations for the same component. + Validate files (validate.*.yaml) count -- they exercise schema validation + in CI even though they are never compiled. Args: component: Component name to check Returns: - True if the component has test YAML files + True if the component has test or validate YAML files """ - return bool(get_component_test_files(component, all_variants=True)) + return bool( + get_component_test_files(component, all_variants=True, include_validate=True) + ) + + +def _component_change_is_validate_only(component: str, changed: list[str]) -> bool: + """Return True if every changed file for this component is a validate.*.yaml. + + Used to decide whether a directly-changed component can skip the compile + stage in CI. A component qualifies when: + - at least one file under ``tests/components//`` changed, AND + - no source file under ``esphome/components//`` changed, AND + - every changed test file is a ``validate.*.yaml`` or + ``validate-*.yaml`` (i.e. no regular ``test.*.yaml`` was touched). + """ + test_prefix = f"tests/components/{component}/" + src_prefix = f"esphome/components/{component}/" + test_changes: list[Path] = [] + for path in changed: + if path.startswith(src_prefix): + return False + if path.startswith(test_prefix): + test_changes.append(Path(path)) + if not test_changes: + return False + return all(is_validate_only_file(p) for p in test_changes) def _select_platform_by_preference( @@ -977,6 +1007,17 @@ def main() -> None: if component not in directly_changed_components ] + # Components whose only changes are validate.*.yaml files can skip the + # compile stage in CI -- their source and test fixtures didn't move, so + # rebuilding firmware adds no signal. Only directly-changed components + # qualify: a component pulled in transitively (because a dependency + # changed) still needs the compile to catch regressions. + validate_only_components = sorted( + component + for component in directly_changed_with_tests + if _component_change_is_validate_only(component, changed) + ) + # Detect components for memory impact analysis (merged config) memory_impact = detect_memory_impact_config(args.branch) @@ -1073,6 +1114,7 @@ def main() -> None: "cpp_unit_tests_run_all": cpp_run_all, "cpp_unit_tests_components": cpp_components, "component_test_batches": component_test_batches, + "validate_only_components": validate_only_components, "benchmarks": run_benchmarks, } diff --git a/script/helpers.py b/script/helpers.py index 7a6d7ecef68..cf82a89f932 100644 --- a/script/helpers.py +++ b/script/helpers.py @@ -117,7 +117,7 @@ def get_component_from_path(file_path: str) -> str | None: def get_component_test_files( - component: str, *, all_variants: bool = False + component: str, *, all_variants: bool = False, include_validate: bool = False ) -> list[Path]: """Get test files for a component. @@ -126,6 +126,10 @@ def get_component_test_files( all_variants: If True, returns all test files including variants (test-*.yaml). If False, returns only base test files (test.*.yaml). Default is False. + include_validate: If True, also returns config-only files (validate.*.yaml, + and validate-*.yaml when all_variants is True). These files + are validated with `esphome config` but never compiled. + Default is False. Returns: List of test file paths for the component, or empty list if none exist @@ -136,9 +140,27 @@ def get_component_test_files( if all_variants: # Match both test.*.yaml and test-*.yaml patterns - return list(tests_dir.glob("test[.-]*.yaml")) + files = list(tests_dir.glob("test[.-]*.yaml")) + if include_validate: + files.extend(tests_dir.glob("validate[.-]*.yaml")) + return files # Match only test.*.yaml (base tests) - return list(tests_dir.glob("test.*.yaml")) + files = list(tests_dir.glob("test.*.yaml")) + if include_validate: + files.extend(tests_dir.glob("validate.*.yaml")) + return files + + +def is_validate_only_file(test_file: Path) -> bool: + """Return True if the given path is a config-only validate file. + + Validate files follow the same grammar as test files but with a + ``validate`` prefix instead of ``test``: ``validate..yaml`` + or ``validate-..yaml``. They are exercised with + ``esphome config`` only and skipped during compile. + """ + name = test_file.name + return name.startswith("validate.") or name.startswith("validate-") @dataclass(frozen=True) diff --git a/script/split_components_for_ci.py b/script/split_components_for_ci.py index d95cdcbe815..0d10246bb48 100755 --- a/script/split_components_for_ci.py +++ b/script/split_components_for_ci.py @@ -44,14 +44,21 @@ ALL_PLATFORMS = "all" def has_test_files(component_name: str, tests_dir: Path) -> bool: """Check if a component has test files. + Validate files (validate.*.yaml) count -- a component with only config-only + test files still needs a CI runner for schema validation. + Args: component_name: Name of the component tests_dir: Path to tests/components directory (unused, kept for compatibility) Returns: - True if the component has test.*.yaml or test-*.yaml files + True if the component has test.*.yaml, test-*.yaml, or validate.*.yaml files """ - return bool(get_component_test_files(component_name, all_variants=True)) + return bool( + get_component_test_files( + component_name, all_variants=True, include_validate=True + ) + ) def create_intelligent_batches( diff --git a/script/test_build_components.py b/script/test_build_components.py index 10c5e5463fe..43b71004eb8 100755 --- a/script/test_build_components.py +++ b/script/test_build_components.py @@ -39,7 +39,11 @@ from script.analyze_component_buses import ( merge_compatible_bus_groups, uses_local_file_references, ) -from script.helpers import get_component_test_files, split_conflicting_groups +from script.helpers import ( + get_component_test_files, + is_validate_only_file, + split_conflicting_groups, +) from script.merge_component_configs import merge_component_configs @@ -83,7 +87,10 @@ def show_disk_space_if_ci(esphome_command: str) -> None: def find_component_tests( - components_dir: Path, component_pattern: str = "*", base_only: bool = False + components_dir: Path, + component_pattern: str = "*", + base_only: bool = False, + include_validate: bool = False, ) -> dict[str, list[Path]]: """Find all component test files. @@ -91,6 +98,8 @@ def find_component_tests( components_dir: Path to tests/components directory component_pattern: Glob pattern for component names base_only: If True, only find base test files (test.*.yaml), not variant files (test-*.yaml) + include_validate: If True, also include config-only files (validate.*.yaml). + These are run with `esphome config` only and never compiled. Returns: Dictionary mapping component name to list of test files @@ -102,7 +111,11 @@ def find_component_tests( continue # Get test files using helper function - test_files = get_component_test_files(comp_dir.name, all_variants=not base_only) + test_files = get_component_test_files( + comp_dir.name, + all_variants=not base_only, + include_validate=include_validate, + ) if test_files: component_tests[comp_dir.name] = test_files @@ -836,12 +849,25 @@ def run_grouped_component_tests( # With grouping: # - 1 build per group (regardless of how many components) # - Individual components still need all their platform builds + # - Validate files of grouped components still run individually + # (they're config-only and bypass the grouped compile, see + # run_individual_component_test), so each adds one more invocation. individual_test_file_count = sum( len(all_tests[comp]) for comp in individual_tests if comp in all_tests ) + grouped_component_set = {c for _, _, comps in groups_to_test for c in comps} + grouped_validate_file_count = sum( + 1 + for comp in grouped_component_set + for test_file in all_tests.get(comp, []) + if is_validate_only_file(test_file) + ) + total_grouped_components = sum(len(comps) for _, _, comps in groups_to_test) - total_builds_with_grouping = len(groups_to_test) + individual_test_file_count + total_builds_with_grouping = ( + len(groups_to_test) + individual_test_file_count + grouped_validate_file_count + ) builds_saved = total_test_files - total_builds_with_grouping print(f"\n{'=' * 80}") @@ -854,6 +880,10 @@ def run_grouped_component_tests( print( f" • {individual_test_file_count} individual builds ({len(individual_tests)} components)" ) + if grouped_validate_file_count: + print( + f" • {grouped_validate_file_count} validate-only invocations for grouped components" + ) if total_test_files > 0: reduction_pct = (builds_saved / total_test_files) * 100 print(f" • Saves {builds_saved} builds ({reduction_pct:.1f}% reduction)") @@ -937,8 +967,13 @@ def run_individual_component_test( tested_components: Set of already tested components test_results: List to append test results """ - # Skip if already tested in a group - if (component, platform_with_version) in tested_components: + # Validate files (validate.*.yaml) are config-only and never participate + # in compile-time bus grouping, so always run them individually even when + # the (component, platform) pair was covered by a group test. + if ( + not is_validate_only_file(test_file) + and (component, platform_with_version) in tested_components + ): return test_result = run_esphome_test( @@ -992,13 +1027,23 @@ def test_components( # Get platform base files platform_bases = get_platform_base_files(build_components_dir) + # Validate files (validate.*.yaml) are config-only -- they exercise + # schema/validation paths but are never compiled. Include them when running + # `config` or `clean`; exclude them under `compile` so they never reach a + # toolchain build. + include_validate = esphome_command != "compile" + # Find all component tests all_tests = {} for pattern in component_patterns: # Skip empty patterns (happens when components list is empty string) if not pattern: continue - all_tests.update(find_component_tests(tests_dir, pattern, base_only)) + all_tests.update( + find_component_tests( + tests_dir, pattern, base_only, include_validate=include_validate + ) + ) # If no components found, build a reference configuration for baseline comparison # Create a synthetic "empty" component test that will build just the base config diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index cc795bc5532..5e2dd670dc1 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -2215,3 +2215,230 @@ def test_should_run_benchmarks_with_branch() -> None: mock_changed.return_value = [] determine_jobs.should_run_benchmarks("release") mock_changed.assert_called_with("release") + + +# --------------------------------------------------------------------------- +# _component_change_is_validate_only +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("component", "changed", "expected"), + [ + # Only a base validate file changed. + ( + "foo", + ["tests/components/foo/validate.esp32-idf.yaml"], + True, + ), + # Only a validate variant changed. + ( + "foo", + ["tests/components/foo/validate-legacy.esp32-idf.yaml"], + True, + ), + # Multiple validate files (all validate). + ( + "foo", + [ + "tests/components/foo/validate.esp32-idf.yaml", + "tests/components/foo/validate-legacy.esp32-idf.yaml", + ], + True, + ), + # Mixed: validate + regular test must NOT be classified as validate-only. + ( + "foo", + [ + "tests/components/foo/validate.esp32-idf.yaml", + "tests/components/foo/test.esp32-idf.yaml", + ], + False, + ), + # Regular test only. + ( + "foo", + ["tests/components/foo/test.esp32-idf.yaml"], + False, + ), + # Source change disqualifies even if a validate file is also touched. + ( + "foo", + [ + "esphome/components/foo/foo.cpp", + "tests/components/foo/validate.esp32-idf.yaml", + ], + False, + ), + # No matching files at all. + ("foo", ["esphome/core/helpers.cpp"], False), + # Filenames merely starting with "validate" but not following the + # grammar must not match (defensive against accidental classification). + ( + "foo", + ["tests/components/foo/validatesomething.yaml"], + False, + ), + # An unrelated component's validate change doesn't affect this one. + ( + "foo", + ["tests/components/bar/validate.esp32-idf.yaml"], + False, + ), + # common.yaml change in the component dir disqualifies. + ( + "foo", + [ + "tests/components/foo/common.yaml", + "tests/components/foo/validate.esp32-idf.yaml", + ], + False, + ), + ], +) +def test_component_change_is_validate_only( + component: str, changed: list[str], expected: bool +) -> None: + """The validate-only classifier rejects anything beyond validate.* edits.""" + assert ( + determine_jobs._component_change_is_validate_only(component, changed) + is expected + ) + + +def test_main_emits_validate_only_components( + mock_determine_integration_tests: Mock, + mock_should_run_clang_tidy: Mock, + mock_should_run_clang_format: Mock, + mock_should_run_python_linters: Mock, + mock_should_run_import_time: Mock, + mock_should_run_device_builder: Mock, + mock_changed_files: Mock, + mock_determine_cpp_unit_tests: Mock, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Directly-changed components whose only edits are validate.*.yaml are + listed in `validate_only_components` so CI can skip their compile stage. + """ + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + + mock_determine_integration_tests.return_value = (False, []) + mock_should_run_clang_tidy.return_value = False + mock_should_run_clang_format.return_value = False + mock_should_run_python_linters.return_value = False + mock_should_run_import_time.return_value = False + mock_should_run_device_builder.return_value = False + mock_determine_cpp_unit_tests.return_value = (False, []) + + # foo: only validate file changed (qualifies) + # bar: test file changed (does not qualify) + mock_changed_files.return_value = [ + "tests/components/foo/validate.esp32-idf.yaml", + "tests/components/bar/test.esp32-idf.yaml", + ] + + with ( + patch("sys.argv", ["determine-jobs.py"]), + patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False), + patch.object( + determine_jobs, + "get_changed_components", + return_value=["foo", "bar"], + ), + patch.object( + determine_jobs, + "filter_component_and_test_files", + side_effect=lambda f: f.startswith("tests/components/"), + ), + patch.object( + determine_jobs, + "get_components_with_dependencies", + side_effect=lambda files, deps: ["foo", "bar"], + ), + patch.object(determine_jobs, "_component_has_tests", return_value=True), + patch.object( + determine_jobs, + "detect_memory_impact_config", + return_value={"should_run": "false"}, + ), + patch.object( + determine_jobs, + "create_intelligent_batches", + return_value=([["foo", "bar"]], {}), + ), + ): + determine_jobs.main() + + output = json.loads(capsys.readouterr().out) + assert output["validate_only_components"] == ["foo"] + + +def test_main_validate_only_excludes_transitive_components( + mock_determine_integration_tests: Mock, + mock_should_run_clang_tidy: Mock, + mock_should_run_clang_format: Mock, + mock_should_run_python_linters: Mock, + mock_should_run_import_time: Mock, + mock_should_run_device_builder: Mock, + mock_changed_files: Mock, + mock_determine_cpp_unit_tests: Mock, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A component pulled in only as a dependency must NOT be considered + validate-only, even if it has no source changes -- its dependency moved, + so the compile is still required. + """ + monkeypatch.delenv("GITHUB_ACTIONS", raising=False) + + mock_determine_integration_tests.return_value = (False, []) + mock_should_run_clang_tidy.return_value = False + mock_should_run_clang_format.return_value = False + mock_should_run_python_linters.return_value = False + mock_should_run_import_time.return_value = False + mock_should_run_device_builder.return_value = False + mock_determine_cpp_unit_tests.return_value = (False, []) + + # Only foo's validate file changed directly. bar is a transitive dep. + mock_changed_files.return_value = [ + "tests/components/foo/validate.esp32-idf.yaml", + ] + + with ( + patch("sys.argv", ["determine-jobs.py"]), + patch.object(determine_jobs, "_is_clang_tidy_full_scan", return_value=False), + patch.object( + determine_jobs, + "get_changed_components", + return_value=["foo", "bar"], # bar pulled in via dependencies + ), + patch.object( + determine_jobs, + "filter_component_and_test_files", + side_effect=lambda f: f.startswith("tests/components/"), + ), + patch.object( + determine_jobs, + "get_components_with_dependencies", + # deps=False -> directly_changed = [foo]; deps=True -> [foo, bar] + side_effect=lambda files, deps: ["foo", "bar"] if deps else ["foo"], + ), + patch.object(determine_jobs, "_component_has_tests", return_value=True), + patch.object( + determine_jobs, + "detect_memory_impact_config", + return_value={"should_run": "false"}, + ), + patch.object( + determine_jobs, + "create_intelligent_batches", + return_value=([["foo", "bar"]], {}), + ), + ): + determine_jobs.main() + + output = json.loads(capsys.readouterr().out) + # Only foo (directly changed, validate-only). bar is a transitive dep + # and still needs compile despite no source change of its own. + assert output["validate_only_components"] == ["foo"] diff --git a/tests/script/test_helpers.py b/tests/script/test_helpers.py index db0d2908f47..10f258aa830 100644 --- a/tests/script/test_helpers.py +++ b/tests/script/test_helpers.py @@ -1624,3 +1624,171 @@ def test_split_conflicting_groups_preserves_original_signature_for_first_bucket( platform, signature = next(iter(extra)) assert platform == "esp32" assert signature.startswith("i2c__conflict") + + +# --------------------------------------------------------------------------- +# get_component_test_files / is_validate_only_file +# --------------------------------------------------------------------------- + + +@pytest.fixture +def fake_component_tests(tmp_path: Path) -> Path: + """Create a fake tests/components/ tree and return the repo root. + + Layout for component "demo": + test.esp32-idf.yaml + test.esp8266-ard.yaml + test-variant.esp32-idf.yaml + validate.esp32-idf.yaml + validate-legacy.esp32-idf.yaml + + Layout for component "validate_only": + validate.esp32-idf.yaml (only validate files) + + Layout for component "no_tests": + common.yaml (no test/validate files at all) + """ + tests_dir = tmp_path / "tests" / "components" + + demo = tests_dir / "demo" + demo.mkdir(parents=True) + (demo / "test.esp32-idf.yaml").write_text("") + (demo / "test.esp8266-ard.yaml").write_text("") + (demo / "test-variant.esp32-idf.yaml").write_text("") + (demo / "validate.esp32-idf.yaml").write_text("") + (demo / "validate-legacy.esp32-idf.yaml").write_text("") + + validate_only = tests_dir / "validate_only" + validate_only.mkdir(parents=True) + (validate_only / "validate.esp32-idf.yaml").write_text("") + + no_tests = tests_dir / "no_tests" + no_tests.mkdir(parents=True) + (no_tests / "common.yaml").write_text("") + + return tmp_path + + +def _names(paths: list[Path]) -> set[str]: + return {p.name for p in paths} + + +def test_get_component_test_files_default_excludes_validate( + fake_component_tests: Path, monkeypatch: MonkeyPatch +) -> None: + """Default behaviour: only base test.*.yaml; no variants, no validate.""" + monkeypatch.setattr(helpers, "root_path", str(fake_component_tests)) + + files = helpers.get_component_test_files("demo") + + assert _names(files) == {"test.esp32-idf.yaml", "test.esp8266-ard.yaml"} + + +def test_get_component_test_files_all_variants_excludes_validate( + fake_component_tests: Path, monkeypatch: MonkeyPatch +) -> None: + """all_variants=True picks up test variants but still skips validate.""" + monkeypatch.setattr(helpers, "root_path", str(fake_component_tests)) + + files = helpers.get_component_test_files("demo", all_variants=True) + + assert _names(files) == { + "test.esp32-idf.yaml", + "test.esp8266-ard.yaml", + "test-variant.esp32-idf.yaml", + } + + +def test_get_component_test_files_include_validate_base_only( + fake_component_tests: Path, monkeypatch: MonkeyPatch +) -> None: + """include_validate=True with base-only adds validate.*.yaml only.""" + monkeypatch.setattr(helpers, "root_path", str(fake_component_tests)) + + files = helpers.get_component_test_files("demo", include_validate=True) + + assert _names(files) == { + "test.esp32-idf.yaml", + "test.esp8266-ard.yaml", + "validate.esp32-idf.yaml", + } + + +def test_get_component_test_files_include_validate_all_variants( + fake_component_tests: Path, monkeypatch: MonkeyPatch +) -> None: + """include_validate=True with all_variants adds validate variants too.""" + monkeypatch.setattr(helpers, "root_path", str(fake_component_tests)) + + files = helpers.get_component_test_files( + "demo", all_variants=True, include_validate=True + ) + + assert _names(files) == { + "test.esp32-idf.yaml", + "test.esp8266-ard.yaml", + "test-variant.esp32-idf.yaml", + "validate.esp32-idf.yaml", + "validate-legacy.esp32-idf.yaml", + } + + +def test_get_component_test_files_validate_only_component( + fake_component_tests: Path, monkeypatch: MonkeyPatch +) -> None: + """A component with only validate files is invisible without the flag.""" + monkeypatch.setattr(helpers, "root_path", str(fake_component_tests)) + + assert helpers.get_component_test_files("validate_only") == [] + assert helpers.get_component_test_files("validate_only", all_variants=True) == [] + + files = helpers.get_component_test_files( + "validate_only", all_variants=True, include_validate=True + ) + assert _names(files) == {"validate.esp32-idf.yaml"} + + +def test_get_component_test_files_missing_component( + fake_component_tests: Path, monkeypatch: MonkeyPatch +) -> None: + """Unknown components return an empty list, regardless of flags.""" + monkeypatch.setattr(helpers, "root_path", str(fake_component_tests)) + + assert ( + helpers.get_component_test_files( + "does_not_exist", all_variants=True, include_validate=True + ) + == [] + ) + + +def test_get_component_test_files_component_without_tests( + fake_component_tests: Path, monkeypatch: MonkeyPatch +) -> None: + """A component with only common.yaml and no test/validate files returns [].""" + monkeypatch.setattr(helpers, "root_path", str(fake_component_tests)) + + assert ( + helpers.get_component_test_files( + "no_tests", all_variants=True, include_validate=True + ) + == [] + ) + + +@pytest.mark.parametrize( + ("filename", "expected"), + [ + ("validate.esp32-idf.yaml", True), + ("validate-legacy.esp32-idf.yaml", True), + ("validate.host.yaml", True), + ("test.esp32-idf.yaml", False), + ("test-variant.esp32-idf.yaml", False), + ("common.yaml", False), + # Defensive: a hypothetical name starting with "validate" but not + # following the grammar must not be classified as a validate file. + ("validatesomething.yaml", False), + ], +) +def test_is_validate_only_file(filename: str, expected: bool, tmp_path: Path) -> None: + assert helpers.is_validate_only_file(tmp_path / filename) is expected From 531367d7e16b3928e4fd88c2f2a1a04748aef7ed Mon Sep 17 00:00:00 2001 From: George Galt Date: Tue, 12 May 2026 19:47:54 -0400 Subject: [PATCH 12/24] [micro_wake_word] Increase INFERENCE_TASK_STACK_SIZE to 8192 for P4 chip (#16390) --- esphome/components/micro_wake_word/micro_wake_word.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/esphome/components/micro_wake_word/micro_wake_word.cpp b/esphome/components/micro_wake_word/micro_wake_word.cpp index c031a9f269d..6877e9e5df5 100644 --- a/esphome/components/micro_wake_word/micro_wake_word.cpp +++ b/esphome/components/micro_wake_word/micro_wake_word.cpp @@ -23,7 +23,13 @@ static const size_t DATA_TIMEOUT_MS = 50; static const uint32_t RING_BUFFER_DURATION_MS = 120; +#ifdef CONFIG_IDF_TARGET_ESP32P4 +// ESP32-P4 PIE-optimized esp-nn kernels (e.g. depthwise_conv_s8_ch1_pie) require +// significantly more stack than other variants, causing stack protection faults at 3072. +static const uint32_t INFERENCE_TASK_STACK_SIZE = 8192; +#else static const uint32_t INFERENCE_TASK_STACK_SIZE = 3072; +#endif static const UBaseType_t INFERENCE_TASK_PRIORITY = 3; enum EventGroupBits : uint32_t { From 8b6cbc9f2b45cba08efef7c416d6ab0667690f40 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 12 May 2026 19:58:03 -0400 Subject: [PATCH 13/24] [i2s_audio] Speaker uses new RingBufferAudioSource (#16315) Thanks! --- .../i2s_audio/speaker/i2s_audio_spdif.cpp | 34 +++++++++--------- .../speaker/i2s_audio_speaker_standard.cpp | 35 ++++++++++--------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/esphome/components/i2s_audio/speaker/i2s_audio_spdif.cpp b/esphome/components/i2s_audio/speaker/i2s_audio_spdif.cpp index d257dd1d8f4..8f67562a777 100644 --- a/esphome/components/i2s_audio/speaker/i2s_audio_spdif.cpp +++ b/esphome/components/i2s_audio/speaker/i2s_audio_spdif.cpp @@ -143,7 +143,11 @@ void I2SAudioSpeakerSPDIF::run_speaker_task() { const uint32_t ring_buffer_duration = std::max(dma_buffers_duration_ms, this->buffer_duration_ms_); // The DMA buffers may have more bits per sample, so calculate buffer sizes based on the input audio stream info - const size_t ring_buffer_size = this->current_stream_info_.ms_to_bytes(ring_buffer_duration); + const size_t bytes_per_frame = this->current_stream_info_.frames_to_bytes(1); + // Round the ring buffer size down to a multiple of bytes_per_frame so the wrap boundary stays frame-aligned and + // avoids unnecessary single-frame splices. + const size_t ring_buffer_size = + (this->current_stream_info_.ms_to_bytes(ring_buffer_duration) / bytes_per_frame) * bytes_per_frame; // For SPDIF mode, one DMA buffer = one SPDIF block = 192 PCM frames const uint32_t frames_to_fill_single_dma_buffer = SPDIF_BLOCK_SAMPLES; @@ -151,13 +155,13 @@ void I2SAudioSpeakerSPDIF::run_speaker_task() { this->current_stream_info_.frames_to_bytes(frames_to_fill_single_dma_buffer); bool successful_setup = false; - std::unique_ptr transfer_buffer = - audio::AudioSourceTransferBuffer::create(bytes_to_fill_single_dma_buffer); + std::unique_ptr audio_source; - if (transfer_buffer != nullptr) { + { std::shared_ptr temp_ring_buffer = ring_buffer::RingBuffer::create(ring_buffer_size); - if (temp_ring_buffer.use_count() == 1) { - transfer_buffer->set_source(temp_ring_buffer); + audio_source = audio::RingBufferAudioSource::create(temp_ring_buffer, bytes_to_fill_single_dma_buffer, + static_cast(bytes_per_frame)); + if (audio_source != nullptr) { this->audio_ring_buffer_ = temp_ring_buffer; successful_setup = true; } @@ -297,24 +301,24 @@ void I2SAudioSpeakerSPDIF::run_speaker_task() { if (!this->pause_state_) { while (real_frames_in_block < SPDIF_BLOCK_SAMPLES) { - if (transfer_buffer->available() == 0) { - size_t bytes_read = transfer_buffer->transfer_data_from_source(read_timeout_ticks); + if (audio_source->available() == 0) { + size_t bytes_read = audio_source->fill(read_timeout_ticks, false); if (bytes_read == 0) { break; // No upstream data within the read budget; silence-pad the remainder. } - uint8_t *new_data = transfer_buffer->get_buffer_end() - bytes_read; + uint8_t *new_data = audio_source->mutable_data(); this->apply_software_volume_(new_data, bytes_read); this->swap_esp32_mono_samples_(new_data, bytes_read); } const uint32_t frames_still_needed = SPDIF_BLOCK_SAMPLES - real_frames_in_block; const size_t bytes_still_needed = this->current_stream_info_.frames_to_bytes(frames_still_needed); - const size_t bytes_to_feed = std::min(transfer_buffer->available(), bytes_still_needed); + const size_t bytes_to_feed = std::min(audio_source->available(), bytes_still_needed); uint32_t blocks_sent = 0; size_t pcm_consumed = 0; - esp_err_t err = this->spdif_encoder_->write(transfer_buffer->get_buffer_start(), bytes_to_feed, - write_timeout_ticks, &blocks_sent, &pcm_consumed); + esp_err_t err = this->spdif_encoder_->write(audio_source->data(), bytes_to_feed, write_timeout_ticks, + &blocks_sent, &pcm_consumed); if (err != ESP_OK) { // A failed (or timed-out) send leaves an unsent block in the encoder's stitch buffer; // resuming would credit the next iteration's bytes against an old block. Bail and @@ -325,7 +329,7 @@ void I2SAudioSpeakerSPDIF::run_speaker_task() { } if (pcm_consumed > 0) { - transfer_buffer->decrease_buffer_length(pcm_consumed); + audio_source->consume(pcm_consumed); real_frames_in_block += this->current_stream_info_.bytes_to_frames(pcm_consumed); } if (blocks_sent > 0) { @@ -387,9 +391,7 @@ void I2SAudioSpeakerSPDIF::run_speaker_task() { this->spdif_encoder_->reset(); } - if (transfer_buffer != nullptr) { - transfer_buffer.reset(); - } + audio_source.reset(); xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::TASK_STOPPED); diff --git a/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp b/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp index 51f2b225e20..0c8b8be5226 100644 --- a/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp +++ b/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp @@ -44,19 +44,23 @@ void I2SAudioSpeaker::run_speaker_task() { const uint32_t ring_buffer_duration = std::max(dma_buffers_duration_ms, this->buffer_duration_ms_); // The DMA buffers may have more bits per sample, so calculate buffer sizes based on the input audio stream info - const size_t ring_buffer_size = this->current_stream_info_.ms_to_bytes(ring_buffer_duration); + const size_t bytes_per_frame = this->current_stream_info_.frames_to_bytes(1); + // Round the ring buffer size down to a multiple of bytes_per_frame so the wrap boundary stays frame-aligned and + // avoids unnecessary single-frame splices. + const size_t ring_buffer_size = + (this->current_stream_info_.ms_to_bytes(ring_buffer_duration) / bytes_per_frame) * bytes_per_frame; const uint32_t frames_to_fill_single_dma_buffer = this->current_stream_info_.ms_to_frames(DMA_BUFFER_DURATION_MS); const size_t bytes_to_fill_single_dma_buffer = this->current_stream_info_.frames_to_bytes(frames_to_fill_single_dma_buffer); bool successful_setup = false; - std::unique_ptr transfer_buffer = - audio::AudioSourceTransferBuffer::create(bytes_to_fill_single_dma_buffer); + std::unique_ptr audio_source; - if (transfer_buffer != nullptr) { + { std::shared_ptr temp_ring_buffer = ring_buffer::RingBuffer::create(ring_buffer_size); - if (temp_ring_buffer.use_count() == 1) { - transfer_buffer->set_source(temp_ring_buffer); + audio_source = audio::RingBufferAudioSource::create(temp_ring_buffer, bytes_to_fill_single_dma_buffer, + static_cast(bytes_per_frame)); + if (audio_source != nullptr) { this->audio_ring_buffer_ = temp_ring_buffer; successful_setup = true; } @@ -129,15 +133,15 @@ void I2SAudioSpeaker::run_speaker_task() { // The millisecond helper modifies the frames_written variable, so use the microsecond helper and divide by 1000 uint32_t read_delay = (this->current_stream_info_.frames_to_microseconds(frames_written) / 1000) / 2; - size_t bytes_read = transfer_buffer->transfer_data_from_source(pdMS_TO_TICKS(read_delay)); - uint8_t *new_data = transfer_buffer->get_buffer_end() - bytes_read; + size_t bytes_read = audio_source->fill(pdMS_TO_TICKS(read_delay), false); + uint8_t *new_data = audio_source->mutable_data(); if (bytes_read > 0) { this->apply_software_volume_(new_data, bytes_read); this->swap_esp32_mono_samples_(new_data, bytes_read); } - if (transfer_buffer->available() == 0) { + if (audio_source->available() == 0) { if (stop_gracefully && tx_dma_underflow) { break; } @@ -150,18 +154,17 @@ void I2SAudioSpeaker::run_speaker_task() { i2s_channel_disable(this->tx_handle_); const i2s_event_callbacks_t null_callbacks = {.on_sent = nullptr}; i2s_channel_register_event_callback(this->tx_handle_, &null_callbacks, this); - i2s_channel_preload_data(this->tx_handle_, transfer_buffer->get_buffer_start(), transfer_buffer->available(), - &bytes_written); + i2s_channel_preload_data(this->tx_handle_, audio_source->data(), audio_source->available(), &bytes_written); } else { // Audio is already playing, use regular write to add to the DMA buffers - i2s_channel_write(this->tx_handle_, transfer_buffer->get_buffer_start(), transfer_buffer->available(), - &bytes_written, DMA_BUFFER_DURATION_MS); + i2s_channel_write(this->tx_handle_, audio_source->data(), audio_source->available(), &bytes_written, + DMA_BUFFER_DURATION_MS); } if (bytes_written > 0) { last_data_received_time = millis(); frames_written += this->current_stream_info_.bytes_to_frames(bytes_written); - transfer_buffer->decrease_buffer_length(bytes_written); + audio_source->consume(bytes_written); if (tx_dma_underflow) { tx_dma_underflow = false; @@ -178,9 +181,7 @@ void I2SAudioSpeaker::run_speaker_task() { xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::TASK_STOPPING); - if (transfer_buffer != nullptr) { - transfer_buffer.reset(); - } + audio_source.reset(); xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::TASK_STOPPED); From 7f37ee3c53a1e10a0450f4b3a3a2b44b199489f1 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 12 May 2026 19:58:32 -0400 Subject: [PATCH 14/24] [mixer] Use RingBufferAudioSource (#16316) --- .../mixer/speaker/mixer_speaker.cpp | 97 ++++++++++--------- .../components/mixer/speaker/mixer_speaker.h | 14 +-- 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/esphome/components/mixer/speaker/mixer_speaker.cpp b/esphome/components/mixer/speaker/mixer_speaker.cpp index 8dea4560c24..1a995a6edfd 100644 --- a/esphome/components/mixer/speaker/mixer_speaker.cpp +++ b/esphome/components/mixer/speaker/mixer_speaker.cpp @@ -182,7 +182,7 @@ void SourceSpeaker::loop() { break; } case speaker::STATE_RUNNING: - if (!this->transfer_buffer_->has_buffered_data() && + if (!this->audio_source_->has_buffered_data() && (this->pending_playback_frames_.load(std::memory_order_acquire) == 0)) { // No audio data in buffer waiting to get mixed and no frames are pending playback if ((this->timeout_ms_.has_value() && ((millis() - this->last_seen_data_ms_) > this->timeout_ms_.value())) || @@ -254,15 +254,12 @@ void SourceSpeaker::send_command_(uint32_t command_bit, bool wake_loop) { void SourceSpeaker::start() { this->send_command_(SOURCE_SPEAKER_COMMAND_START, true); } esp_err_t SourceSpeaker::start_() { - const size_t ring_buffer_size = this->audio_stream_info_.ms_to_bytes(this->buffer_duration_ms_); - if (this->transfer_buffer_.use_count() == 0) { - this->transfer_buffer_ = - audio::AudioSourceTransferBuffer::create(this->audio_stream_info_.ms_to_bytes(TRANSFER_BUFFER_DURATION_MS)); - - if (this->transfer_buffer_ == nullptr) { - return ESP_ERR_NO_MEM; - } - + const size_t bytes_per_frame = this->audio_stream_info_.frames_to_bytes(1); + // Round the ring buffer size down to a multiple of bytes_per_frame so the wrap boundary stays frame-aligned and + // avoids unnecessary single-frame splices. + const size_t ring_buffer_size = + (this->audio_stream_info_.ms_to_bytes(this->buffer_duration_ms_) / bytes_per_frame) * bytes_per_frame; + if (this->audio_source_.use_count() == 0) { std::shared_ptr temp_ring_buffer = this->ring_buffer_.lock(); if (!temp_ring_buffer) { temp_ring_buffer = ring_buffer::RingBuffer::create(ring_buffer_size); @@ -271,9 +268,15 @@ esp_err_t SourceSpeaker::start_() { if (!temp_ring_buffer) { return ESP_ERR_NO_MEM; - } else { - this->transfer_buffer_->set_source(temp_ring_buffer); } + + std::unique_ptr source = audio::RingBufferAudioSource::create( + temp_ring_buffer, this->audio_stream_info_.ms_to_bytes(TRANSFER_BUFFER_DURATION_MS), + static_cast(bytes_per_frame)); + if (source == nullptr) { + return ESP_ERR_NO_MEM; + } + this->audio_source_ = std::move(source); } return this->parent_->start(this->audio_stream_info_); @@ -284,7 +287,7 @@ void SourceSpeaker::stop() { this->send_command_(SOURCE_SPEAKER_COMMAND_STOP); } void SourceSpeaker::finish() { this->send_command_(SOURCE_SPEAKER_COMMAND_FINISH); } bool SourceSpeaker::has_buffered_data() const { - return ((this->transfer_buffer_.use_count() > 0) && this->transfer_buffer_->has_buffered_data()); + return ((this->audio_source_.use_count() > 0) && this->audio_source_->has_buffered_data()); } void SourceSpeaker::set_mute_state(bool mute_state) { @@ -301,16 +304,18 @@ void SourceSpeaker::set_volume(float volume) { float SourceSpeaker::get_volume() { return this->parent_->get_output_speaker()->get_volume(); } -size_t SourceSpeaker::process_data_from_source(std::shared_ptr &transfer_buffer, +size_t SourceSpeaker::process_data_from_source(std::shared_ptr &audio_source, TickType_t ticks_to_wait) { - // Store current offset, as these samples are already ducked - const size_t current_length = transfer_buffer->available(); + if (audio_source->available() > 0) { + // Existing exposure was ducked when fill() promoted it; do not re-duck on partial-consume re-entry. + return 0; + } - size_t bytes_read = transfer_buffer->transfer_data_from_source(ticks_to_wait); + size_t bytes_read = audio_source->fill(ticks_to_wait, false); uint32_t samples_to_duck = this->audio_stream_info_.bytes_to_samples(bytes_read); if (samples_to_duck > 0) { - int16_t *current_buffer = reinterpret_cast(transfer_buffer->get_buffer_start() + current_length); + int16_t *current_buffer = reinterpret_cast(audio_source->mutable_data()); duck_samples(current_buffer, samples_to_duck, &this->current_ducking_db_reduction_, &this->ducking_transition_samples_remaining_, this->samples_per_ducking_step_, @@ -406,7 +411,7 @@ void SourceSpeaker::duck_samples(int16_t *input_buffer, uint32_t input_samples_t void SourceSpeaker::enter_stopping_state_() { this->state_ = speaker::STATE_STOPPING; this->stopping_start_ms_ = millis(); - this->transfer_buffer_.reset(); + this->audio_source_.reset(); } void MixerSpeaker::dump_config() { @@ -612,9 +617,9 @@ void MixerSpeaker::audio_mixer_task(void *params) { // Pre-allocate vectors to avoid heap allocation in the loop (max 8 source speakers per schema) FixedVector speakers_with_data; - FixedVector> transfer_buffers_with_data; + FixedVector> audio_sources_with_data; speakers_with_data.init(this_mixer->source_speakers_.size()); - transfer_buffers_with_data.init(this_mixer->source_speakers_.size()); + audio_sources_with_data.init(this_mixer->source_speakers_.size()); while (true) { uint32_t event_group_bits = xEventGroupGetBits(this_mixer->event_group_); @@ -629,27 +634,27 @@ void MixerSpeaker::audio_mixer_task(void *params) { this_mixer->audio_stream_info_.value().bytes_to_frames(output_transfer_buffer->free()); speakers_with_data.clear(); - transfer_buffers_with_data.clear(); + audio_sources_with_data.clear(); for (auto &speaker : this_mixer->source_speakers_) { if (speaker->is_running() && !speaker->get_pause_state()) { // Speaker is running and not paused, so it possibly can provide audio data - std::shared_ptr transfer_buffer = speaker->get_transfer_buffer().lock(); - if (transfer_buffer.use_count() == 0) { - // No transfer buffer allocated, so skip processing this speaker + std::shared_ptr audio_source = speaker->get_audio_source().lock(); + if (audio_source.use_count() == 0) { + // No audio source allocated, so skip processing this speaker continue; } - speaker->process_data_from_source(transfer_buffer, 0); // Transfers and ducks audio from source ring buffers + speaker->process_data_from_source(audio_source, 0); // Exposes and ducks audio from source ring buffers - if (transfer_buffer->available() > 0) { - // Store the locked transfer buffers in their own vector to avoid releasing ownership until after the loop - transfer_buffers_with_data.push_back(transfer_buffer); + if (audio_source->available() > 0) { + // Retain shared ownership across the mixing pass so the source isn't released mid-mix + audio_sources_with_data.push_back(audio_source); speakers_with_data.push_back(speaker); } } } - if (transfer_buffers_with_data.empty()) { + if (audio_sources_with_data.empty()) { // No audio available for transferring, block task temporarily delay(TASK_DELAY_MS); continue; @@ -657,7 +662,7 @@ void MixerSpeaker::audio_mixer_task(void *params) { uint32_t frames_to_mix = output_frames_free; - if ((transfer_buffers_with_data.size() == 1) || this_mixer->queue_mode_) { + if ((audio_sources_with_data.size() == 1) || this_mixer->queue_mode_) { // Only one speaker has audio data, just copy samples over audio::AudioStreamInfo active_stream_info = speakers_with_data[0]->get_audio_stream_info(); @@ -667,10 +672,10 @@ void MixerSpeaker::audio_mixer_task(void *params) { // Speaker's sample rate matches the output speaker's, copy directly const uint32_t frames_available_in_buffer = - active_stream_info.bytes_to_frames(transfer_buffers_with_data[0]->available()); + active_stream_info.bytes_to_frames(audio_sources_with_data[0]->available()); frames_to_mix = std::min(frames_to_mix, frames_available_in_buffer); - copy_frames(reinterpret_cast(transfer_buffers_with_data[0]->get_buffer_start()), - active_stream_info, reinterpret_cast(output_transfer_buffer->get_buffer_end()), + copy_frames(reinterpret_cast(audio_sources_with_data[0]->data()), active_stream_info, + reinterpret_cast(output_transfer_buffer->get_buffer_end()), this_mixer->audio_stream_info_.value(), frames_to_mix); // Set playback delay for newly contributing source @@ -682,7 +687,7 @@ void MixerSpeaker::audio_mixer_task(void *params) { // Update source speaker pending frames speakers_with_data[0]->pending_playback_frames_.fetch_add(frames_to_mix, std::memory_order_release); - transfer_buffers_with_data[0]->decrease_buffer_length(active_stream_info.frames_to_bytes(frames_to_mix)); + audio_sources_with_data[0]->consume(active_stream_info.frames_to_bytes(frames_to_mix)); // Update output transfer buffer length and pipeline frame count output_transfer_buffer->increase_buffer_length( @@ -709,25 +714,25 @@ void MixerSpeaker::audio_mixer_task(void *params) { } } else { // Determine how many frames to mix - for (size_t i = 0; i < transfer_buffers_with_data.size(); ++i) { - const uint32_t frames_available_in_buffer = speakers_with_data[i]->get_audio_stream_info().bytes_to_frames( - transfer_buffers_with_data[i]->available()); + for (size_t i = 0; i < audio_sources_with_data.size(); ++i) { + const uint32_t frames_available_in_buffer = + speakers_with_data[i]->get_audio_stream_info().bytes_to_frames(audio_sources_with_data[i]->available()); frames_to_mix = std::min(frames_to_mix, frames_available_in_buffer); } - int16_t *primary_buffer = reinterpret_cast(transfer_buffers_with_data[0]->get_buffer_start()); + const int16_t *primary_buffer = reinterpret_cast(audio_sources_with_data[0]->data()); audio::AudioStreamInfo primary_stream_info = speakers_with_data[0]->get_audio_stream_info(); // Mix two streams together - for (size_t i = 1; i < transfer_buffers_with_data.size(); ++i) { + for (size_t i = 1; i < audio_sources_with_data.size(); ++i) { mix_audio_samples(primary_buffer, primary_stream_info, - reinterpret_cast(transfer_buffers_with_data[i]->get_buffer_start()), + reinterpret_cast(audio_sources_with_data[i]->data()), speakers_with_data[i]->get_audio_stream_info(), reinterpret_cast(output_transfer_buffer->get_buffer_end()), this_mixer->audio_stream_info_.value(), frames_to_mix); - if (i != transfer_buffers_with_data.size() - 1) { + if (i != audio_sources_with_data.size() - 1) { // Need to mix more streams together, point primary buffer and stream info to the already mixed output - primary_buffer = reinterpret_cast(output_transfer_buffer->get_buffer_end()); + primary_buffer = reinterpret_cast(output_transfer_buffer->get_buffer_end()); primary_stream_info = this_mixer->audio_stream_info_.value(); } } @@ -735,8 +740,8 @@ void MixerSpeaker::audio_mixer_task(void *params) { // Get current pipeline depth for delay calculation (before incrementing) uint32_t current_pipeline_frames = this_mixer->frames_in_pipeline_.load(std::memory_order_acquire); - // Update source transfer buffer lengths and add new audio durations to the source speaker pending playbacks - for (size_t i = 0; i < transfer_buffers_with_data.size(); ++i) { + // Update source audio source consumption and add new audio durations to the source speaker pending playbacks + for (size_t i = 0; i < audio_sources_with_data.size(); ++i) { // Set playback delay for newly contributing sources if (!speakers_with_data[i]->has_contributed_.load(std::memory_order_acquire)) { speakers_with_data[i]->playback_delay_frames_.store(current_pipeline_frames, std::memory_order_release); @@ -744,7 +749,7 @@ void MixerSpeaker::audio_mixer_task(void *params) { } speakers_with_data[i]->pending_playback_frames_.fetch_add(frames_to_mix, std::memory_order_release); - transfer_buffers_with_data[i]->decrease_buffer_length( + audio_sources_with_data[i]->consume( speakers_with_data[i]->get_audio_stream_info().frames_to_bytes(frames_to_mix)); } diff --git a/esphome/components/mixer/speaker/mixer_speaker.h b/esphome/components/mixer/speaker/mixer_speaker.h index f392e83081a..f57bead679f 100644 --- a/esphome/components/mixer/speaker/mixer_speaker.h +++ b/esphome/components/mixer/speaker/mixer_speaker.h @@ -67,11 +67,13 @@ class SourceSpeaker : public speaker::Speaker, public Component { void set_pause_state(bool pause_state) override { this->pause_state_ = pause_state; } bool get_pause_state() const override { return this->pause_state_; } - /// @brief Transfers audio from the ring buffer into the transfer buffer. Ducks audio while transferring. - /// @param transfer_buffer Locked shared_ptr to the transfer buffer (must be valid, not null) + /// @brief Exposes the next ring buffer chunk (zero-copy) and ducks the freshly exposed bytes in place. + /// If the source still has bytes from a prior partial consume, this is a no-op (those bytes were already + /// ducked on the fill that exposed them). + /// @param audio_source Locked shared_ptr to the audio source (must be valid, not null) /// @param ticks_to_wait FreeRTOS ticks to wait while waiting to read from the ring buffer. - /// @return Number of bytes transferred from the ring buffer. - size_t process_data_from_source(std::shared_ptr &transfer_buffer, + /// @return Number of bytes newly exposed from the ring buffer. + size_t process_data_from_source(std::shared_ptr &audio_source, TickType_t ticks_to_wait); /// @brief Sets the ducking level for the source speaker. @@ -83,7 +85,7 @@ class SourceSpeaker : public speaker::Speaker, public Component { void set_parent(MixerSpeaker *parent) { this->parent_ = parent; } void set_timeout(uint32_t ms) { this->timeout_ms_ = ms; } - std::weak_ptr get_transfer_buffer() { return this->transfer_buffer_; } + std::weak_ptr get_audio_source() { return this->audio_source_; } protected: friend class MixerSpeaker; @@ -106,7 +108,7 @@ class SourceSpeaker : public speaker::Speaker, public Component { MixerSpeaker *parent_; - std::shared_ptr transfer_buffer_; + std::shared_ptr audio_source_; std::weak_ptr ring_buffer_; uint32_t buffer_duration_ms_; From 1c2043e0544bfa7d1193d3a86144574d7fee0655 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Tue, 12 May 2026 20:04:54 -0400 Subject: [PATCH 15/24] [esp32] Relax -Werror=reorder and -Werror=maybe-uninitialized on native ESP-IDF (#16392) --- esphome/components/esp32/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index bb823937aa0..221c84c1491 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -1754,7 +1754,9 @@ async def to_code(config): ) else: cg.add_build_flag("-Wno-error=format") + cg.add_build_flag("-Wno-error=maybe-uninitialized") cg.add_build_flag("-Wno-error=missing-field-initializers") + cg.add_build_flag("-Wno-error=reorder") cg.add_build_flag("-Wno-error=volatile") cg.set_cpp_standard("gnu++20") From dc95b22c763133354873a57f9c5ba1dfd391d85e Mon Sep 17 00:00:00 2001 From: Mat931 <49403702+Mat931@users.noreply.github.com> Date: Wed, 13 May 2026 00:07:49 +0000 Subject: [PATCH 16/24] [safe_mode] Allow recovering soft-bricked devices via reboot to recovery partition (#16339) Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston --- esphome/components/safe_mode/safe_mode.cpp | 76 +++++++++++++++++++++- esphome/components/safe_mode/safe_mode.h | 5 +- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/esphome/components/safe_mode/safe_mode.cpp b/esphome/components/safe_mode/safe_mode.cpp index bae5e42b9b5..5c0047dca0a 100644 --- a/esphome/components/safe_mode/safe_mode.cpp +++ b/esphome/components/safe_mode/safe_mode.cpp @@ -15,6 +15,7 @@ #elif defined(USE_ESP32) #include #include +#include #endif #endif @@ -22,6 +23,37 @@ namespace esphome::safe_mode { static const char *const TAG = "safe_mode"; +#if defined(USE_ESP32) && defined(USE_OTA_ROLLBACK) && !defined(USE_OTA_PARTITIONS) +// Find a non-running app partition. If verify is true, only returns a partition +// whose image passes verification (expensive: reads flash). Returns nullptr if none found. +static const esp_partition_t *find_alternate_app_partition(bool verify) { + const esp_partition_t *running = esp_ota_get_running_partition(); + const esp_partition_t *result = nullptr; + esp_partition_iterator_t it = esp_partition_find(ESP_PARTITION_TYPE_APP, ESP_PARTITION_SUBTYPE_ANY, nullptr); + while (it != nullptr) { + const esp_partition_t *p = esp_partition_get(it); + if (p->address != running->address) { + if (!verify) { + result = p; + break; + } + esp_image_metadata_t data = {}; + const esp_partition_pos_t part_pos = { + .offset = p->address, + .size = p->size, + }; + if (esp_image_verify(ESP_IMAGE_VERIFY_SILENT, &part_pos, &data) == ESP_OK) { + result = p; + break; + } + } + it = esp_partition_next(it); + } + esp_partition_iterator_release(it); + return result; +} +#endif + void SafeModeComponent::dump_config() { ESP_LOGCONFIG(TAG, "Safe Mode:\n" @@ -34,7 +66,11 @@ void SafeModeComponent::dump_config() { #if defined(USE_ESP32) && defined(USE_OTA_ROLLBACK) const char *state_str; if (this->ota_state_ == ESP_OTA_IMG_NEW) { +#ifdef USE_OTA_PARTITIONS + state_str = "support unknown"; +#else state_str = "not supported"; +#endif } else if (this->ota_state_ == ESP_OTA_IMG_PENDING_VERIFY) { state_str = "supported"; } else { @@ -64,6 +100,18 @@ void SafeModeComponent::dump_config() { " See https://esphome.io/guides/faq.html#brownout-detector-was-triggered"); } } + if (!this->app_ota_possible_) { + ESP_LOGW(TAG, "OTA updates are impossible."); +#ifdef USE_OTA_PARTITIONS + ESP_LOGW(TAG, " OTA partition table update or serial flashing is required."); +#else + if (find_alternate_app_partition(false) != nullptr) { + ESP_LOGW(TAG, " Activate safe mode to reboot to the recovery partition."); + } else { + ESP_LOGE(TAG, " No recovery partition available; serial flashing is required."); + } +#endif + } #endif } @@ -124,8 +172,10 @@ bool SafeModeComponent::should_enter_safe_mode(uint8_t num_attempts, uint32_t en #if defined(USE_ESP32) && defined(USE_OTA_ROLLBACK) // Check partition state to detect if bootloader supports rollback - const esp_partition_t *running = esp_ota_get_running_partition(); - esp_ota_get_state_partition(running, &this->ota_state_); + const esp_partition_t *running_part = esp_ota_get_running_partition(); + esp_ota_get_state_partition(running_part, &this->ota_state_); + const esp_partition_t *next_part = esp_ota_get_next_update_partition(nullptr); + this->app_ota_possible_ = (next_part != nullptr && next_part != running_part); #endif uint32_t rtc_val = this->read_rtc_(); @@ -151,6 +201,28 @@ bool SafeModeComponent::should_enter_safe_mode(uint8_t num_attempts, uint32_t en ESP_LOGE(TAG, "Boot loop detected"); } +#if defined(USE_ESP32) && defined(USE_OTA_ROLLBACK) && !defined(USE_OTA_PARTITIONS) + // Allow recovery of soft-bricked devices + // Instead of starting safe_mode, reboot to the other app partition if all conditions are met: + // - app OTA is impossible (for example because the other app partition has type 'factory') + // - the other app partition contains a valid app (for example Tasmota safeboot image or ESPHome) + // - allow_partition_access is not configured making recovery via partition table update impossible + // Image verification is deferred until here so the cost is only paid when entering safe mode, + // not on every boot. + if (!this->app_ota_possible_) { + const esp_partition_t *rollback_part = find_alternate_app_partition(true); + if (rollback_part != nullptr) { + esp_err_t err = esp_ota_set_boot_partition(rollback_part); + if (err == ESP_OK) { + ESP_LOGW(TAG, "OTA updates are impossible. Rebooting to recovery app."); + App.reboot(); + } else { + ESP_LOGE(TAG, "Failed to set recovery boot partition: %s", esp_err_to_name(err)); + } + } + } +#endif + this->status_set_error(); this->set_timeout(enable_time, []() { ESP_LOGW(TAG, "Timeout, restarting"); diff --git a/esphome/components/safe_mode/safe_mode.h b/esphome/components/safe_mode/safe_mode.h index b458a9a3021..94db4357eb9 100644 --- a/esphome/components/safe_mode/safe_mode.h +++ b/esphome/components/safe_mode/safe_mode.h @@ -48,11 +48,14 @@ class SafeModeComponent final : public Component { uint32_t safe_mode_enable_time_{60000}; ///< The time safe mode should remain active for uint32_t safe_mode_rtc_value_{0}; uint32_t safe_mode_start_time_{0}; ///< stores when safe mode was enabled +#if defined(USE_ESP32) && defined(USE_OTA_ROLLBACK) + esp_ota_img_states_t ota_state_{ESP_OTA_IMG_UNDEFINED}; // 4-byte enum +#endif // Group 1-byte members together to minimize padding bool boot_successful_{false}; ///< set to true after boot is considered successful uint8_t safe_mode_num_attempts_{0}; #if defined(USE_ESP32) && defined(USE_OTA_ROLLBACK) - esp_ota_img_states_t ota_state_{ESP_OTA_IMG_UNDEFINED}; + bool app_ota_possible_{true}; #endif // Larger objects at the end ESPPreferenceObject rtc_; From 3df0527c1fb8676e2eb0997b7c5bd4e8ee073836 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 12 May 2026 20:10:32 -0400 Subject: [PATCH 17/24] [audio] Document ring buffer source thread safety (#16393) --- esphome/components/audio/audio_transfer_buffer.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/esphome/components/audio/audio_transfer_buffer.h b/esphome/components/audio/audio_transfer_buffer.h index d83032582e6..b713326141c 100644 --- a/esphome/components/audio/audio_transfer_buffer.h +++ b/esphome/components/audio/audio_transfer_buffer.h @@ -220,6 +220,10 @@ class ConstAudioSourceBuffer : public AudioReadableBuffer { /// source transparently stitches frames that straddle the ring buffer's wrap boundary by buffering the /// trailing partial frame from one chunk and joining it with the head of the next chunk in a small /// internal splice buffer, so callers always see frame-aligned data. +/// +/// Not thread-safe. The underlying ring_buffer::RingBuffer supports one producer and one consumer +/// running concurrently, but a given RingBufferAudioSource (its acquired item, splice buffer, and +/// queued region) must be used by only one thread, and that thread is the ring buffer's consumer. class RingBufferAudioSource : public AudioReadableBuffer { public: /// Maximum supported alignment. Sized to cover 32-bit samples across up to 2 channels (8 bytes). @@ -242,6 +246,8 @@ class RingBufferAudioSource : public AudioReadableBuffer { size_t available() const override { return this->current_available_; } void consume(size_t bytes) override; bool has_buffered_data() const override; + /// pre_shift is ignored: there is no intermediate transfer buffer to compact, so an unconsumed + /// exposure stays in place and fill() returns 0 until it is fully consumed. size_t fill(TickType_t ticks_to_wait, bool pre_shift) override; /// @brief Returns a mutable pointer to the currently exposed audio data. From 65b53692bdd5ba21d90cfe18d97de4f9102a2b97 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 12 May 2026 21:36:26 -0400 Subject: [PATCH 18/24] [i2s_audio] Properly track DMA input/output (#16317) --- .../i2s_audio/speaker/i2s_audio_speaker.cpp | 2 +- .../i2s_audio/speaker/i2s_audio_speaker.h | 4 +- .../speaker/i2s_audio_speaker_standard.cpp | 252 ++++++++++++------ 3 files changed, 178 insertions(+), 80 deletions(-) diff --git a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp index 27961050e6c..680ca069c0e 100644 --- a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp +++ b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp @@ -99,7 +99,7 @@ void I2SAudioSpeakerBase::loop() { } if (event_group_bits & SpeakerEventGroupBits::ERR_ESP_NO_MEM) { - ESP_LOGE(TAG, "Not enough memory"); + ESP_LOGE(TAG, "Speaker task setup failed (allocation, preload, or channel enable)"); xEventGroupClearBits(this->event_group_, SpeakerEventGroupBits::ERR_ESP_NO_MEM); } diff --git a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.h b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.h index c57af2775b3..20bb05e3226 100644 --- a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.h +++ b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.h @@ -36,9 +36,7 @@ enum SpeakerEventGroupBits : uint32_t { ERR_ESP_NO_MEM = (1 << 19), ERR_DROPPED_EVENT = (1 << 20), // ISR overflowed the event queue, dropping a completion event - ERR_PARTIAL_WRITE = (1 << 21), // a DMA write returned fewer bytes than requested (or the encoder - // failed to commit a complete block), which breaks the lockstep - // invariant for every subsequent event + ERR_PARTIAL_WRITE = (1 << 21), // i2s_channel_write returned fewer bytes than requested ERR_LOCKSTEP_DESYNC = (1 << 22), // i2s_event_queue_ and write_records_queue_ fell out of sync ALL_BITS = 0x00FFFFFF, // All valid FreeRTOS event group bits diff --git a/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp b/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp index 0c8b8be5226..e69601e87ae 100644 --- a/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp +++ b/esphome/components/i2s_audio/speaker/i2s_audio_speaker_standard.cpp @@ -17,7 +17,14 @@ namespace esphome::i2s_audio { static const char *const TAG = "i2s_audio.speaker.std"; static constexpr size_t DMA_BUFFERS_COUNT = 4; -static constexpr size_t I2S_EVENT_QUEUE_COUNT = DMA_BUFFERS_COUNT + 1; +// Sized to comfortably absorb scheduling jitter: at most DMA_BUFFERS_COUNT events can be in flight, +// doubled so that a transient backlog never overruns the queue (which would desync the lockstep +// invariant between i2s_event_queue_ and write_records_queue_). +static constexpr size_t I2S_EVENT_QUEUE_COUNT = DMA_BUFFERS_COUNT * 2; +// Generous timeout for ``i2s_channel_write`` blocking. A buffer frees roughly every +// DMA_BUFFER_DURATION_MS, so a multiple of that gives plenty of slack against scheduling jitter +// without masking real failures. +static constexpr TickType_t WRITE_TIMEOUT_TICKS = pdMS_TO_TICKS(DMA_BUFFER_DURATION_MS * (DMA_BUFFERS_COUNT + 1)); void I2SAudioSpeaker::dump_config() { I2SAudioSpeakerBase::dump_config(); @@ -49,30 +56,73 @@ void I2SAudioSpeaker::run_speaker_task() { // avoids unnecessary single-frame splices. const size_t ring_buffer_size = (this->current_stream_info_.ms_to_bytes(ring_buffer_duration) / bytes_per_frame) * bytes_per_frame; - const uint32_t frames_to_fill_single_dma_buffer = this->current_stream_info_.ms_to_frames(DMA_BUFFER_DURATION_MS); - const size_t bytes_to_fill_single_dma_buffer = - this->current_stream_info_.frames_to_bytes(frames_to_fill_single_dma_buffer); + const uint32_t frames_per_dma_buffer = this->current_stream_info_.ms_to_frames(DMA_BUFFER_DURATION_MS); + const size_t dma_buffer_bytes = this->current_stream_info_.frames_to_bytes(frames_per_dma_buffer); bool successful_setup = false; + std::unique_ptr audio_source; - { + // Pre-zeroed buffer used to silence-pad each DMA descriptor whenever real audio doesn't fully fill it. + RAMAllocator silence_allocator; + uint8_t *silence_buffer = silence_allocator.allocate(dma_buffer_bytes); + + if (silence_buffer != nullptr) { + memset(silence_buffer, 0, dma_buffer_bytes); + std::shared_ptr temp_ring_buffer = ring_buffer::RingBuffer::create(ring_buffer_size); - audio_source = audio::RingBufferAudioSource::create(temp_ring_buffer, bytes_to_fill_single_dma_buffer, - static_cast(bytes_per_frame)); + audio_source = + audio::RingBufferAudioSource::create(temp_ring_buffer, dma_buffer_bytes, static_cast(bytes_per_frame)); + if (audio_source != nullptr) { + // audio_source is nullptr if the ring buffer fails to allocate this->audio_ring_buffer_ = temp_ring_buffer; successful_setup = true; } } + if (successful_setup) { + // Preload every DMA descriptor with silence and push a matching zero-real-frames record per buffer. + // This guarantees that every on_sent event has a corresponding write record from the start, so + // ``i2s_event_queue_`` and ``write_records_queue_`` stay in lockstep for the entire task lifetime. + for (size_t i = 0; i < DMA_BUFFERS_COUNT; i++) { + size_t bytes_loaded = 0; + esp_err_t err = i2s_channel_preload_data(this->tx_handle_, silence_buffer, dma_buffer_bytes, &bytes_loaded); + if (err != ESP_OK || bytes_loaded != dma_buffer_bytes) { + ESP_LOGV(TAG, "Failed to preload silence into DMA buffer %u (err=%d, loaded=%u)", (unsigned) i, (int) err, + (unsigned) bytes_loaded); + successful_setup = false; + break; + } + uint32_t zero_real_frames = 0; + if (xQueueSend(this->write_records_queue_, &zero_real_frames, 0) != pdTRUE) { + // Should never happen: the queue was just reset and is sized for DMA_BUFFERS_COUNT * 2 entries. + ESP_LOGV(TAG, "Failed to push preload write record"); + successful_setup = false; + break; + } + } + } + + if (successful_setup) { + // Register the on_sent callback BEFORE enabling the channel so the very first transmitted buffer + // generates a queued event that pairs with the first preloaded silence record. + const i2s_event_callbacks_t callbacks = {.on_sent = i2s_on_sent_cb}; + i2s_channel_register_event_callback(this->tx_handle_, &callbacks, this); + + if (i2s_channel_enable(this->tx_handle_) != ESP_OK) { + ESP_LOGV(TAG, "Failed to enable I2S channel"); + successful_setup = false; + } + } + if (!successful_setup) { xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::ERR_ESP_NO_MEM); } else { bool stop_gracefully = false; - bool tx_dma_underflow = true; - - uint32_t frames_written = 0; + // Number of records currently in ``write_records_queue_`` that carry real audio. Used by graceful + // stop to wait until every real-audio buffer has been confirmed played by an ISR event. + uint32_t pending_real_buffers = 0; uint32_t last_data_received_time = millis(); xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::TASK_RUNNING); @@ -81,11 +131,21 @@ void I2SAudioSpeaker::run_speaker_task() { // - Paused, OR // - No timeout configured, OR // - Timeout hasn't elapsed since last data + // + // Always-fill model: every iteration writes exactly one DMA buffer's worth, mixing real audio + // and silence padding as needed. The blocking ``i2s_channel_write`` paces the loop at the DMA + // consumption rate, and every buffer write is matched 1:1 with a record on ``write_records_queue_``. + // + // While paused, the real-audio fill is skipped and the entire DMA buffer is filled with silence; + // the same blocking ``i2s_channel_write`` provides natural pacing (one buffer per ~DMA_BUFFER_DURATION_MS), + // so the lockstep invariant is preserved without burning CPU. while (this->pause_state_ || !this->timeout_.has_value() || (millis() - last_data_received_time) <= this->timeout_.value()) { uint32_t event_group_bits = xEventGroupGetBits(this->event_group_); if (event_group_bits & SpeakerEventGroupBits::COMMAND_STOP) { + // COMMAND_STOP is set both by user-initiated stop() and by the ISR when it drops a completion + // event (paired with ERR_DROPPED_EVENT so loop() can distinguish the two cases). xEventGroupClearBits(this->event_group_, SpeakerEventGroupBits::COMMAND_STOP); ESP_LOGV(TAG, "Exiting: COMMAND_STOP received"); break; @@ -101,80 +161,115 @@ void I2SAudioSpeaker::run_speaker_task() { break; } + // Drain ISR-stamped completion events. Each event corresponds 1:1 with a write_records_queue_ + // entry by construction (preloaded records at startup, plus exactly one record pushed per + // iteration alongside exactly one DMA-buffer-sized write). int64_t write_timestamp; + bool lockstep_broken = false; while (xQueueReceive(this->i2s_event_queue_, &write_timestamp, 0)) { - // Receives timing events from the I2S on_sent callback. If actual audio data was sent in this event, it passes - // on the timing info via the audio_output_callback. - uint32_t frames_sent = frames_to_fill_single_dma_buffer; - if (frames_to_fill_single_dma_buffer > frames_written) { - tx_dma_underflow = true; - frames_sent = frames_written; - const uint32_t frames_zeroed = frames_to_fill_single_dma_buffer - frames_written; - write_timestamp -= this->current_stream_info_.frames_to_microseconds(frames_zeroed); - } else { - tx_dma_underflow = false; - } - frames_written -= frames_sent; - - // Standard I2S mode: fire callback immediately for each event - if (frames_sent > 0) { - this->audio_output_callback_(frames_sent, write_timestamp); - } - } - - if (this->pause_state_) { - // Pause state is accessed atomically, so thread safe - // Delay so the task yields, then skip transferring audio data - vTaskDelay(pdMS_TO_TICKS(DMA_BUFFER_DURATION_MS)); - continue; - } - - // Wait half the duration of the data already written to the DMA buffers for new audio data - // The millisecond helper modifies the frames_written variable, so use the microsecond helper and divide by 1000 - uint32_t read_delay = (this->current_stream_info_.frames_to_microseconds(frames_written) / 1000) / 2; - - size_t bytes_read = audio_source->fill(pdMS_TO_TICKS(read_delay), false); - uint8_t *new_data = audio_source->mutable_data(); - - if (bytes_read > 0) { - this->apply_software_volume_(new_data, bytes_read); - this->swap_esp32_mono_samples_(new_data, bytes_read); - } - - if (audio_source->available() == 0) { - if (stop_gracefully && tx_dma_underflow) { + uint32_t real_frames = 0; + if (xQueueReceive(this->write_records_queue_, &real_frames, 0) != pdTRUE) { + // Should never happen: would indicate the lockstep invariant is broken. + ESP_LOGV(TAG, "Event without matching write record"); + xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::ERR_LOCKSTEP_DESYNC); + lockstep_broken = true; break; } - vTaskDelay(pdMS_TO_TICKS(DMA_BUFFER_DURATION_MS / 2)); - } else { - size_t bytes_written = 0; - - if (tx_dma_underflow) { - // Temporarily disable channel and callback to reset the I2S driver's internal DMA buffer queue - i2s_channel_disable(this->tx_handle_); - const i2s_event_callbacks_t null_callbacks = {.on_sent = nullptr}; - i2s_channel_register_event_callback(this->tx_handle_, &null_callbacks, this); - i2s_channel_preload_data(this->tx_handle_, audio_source->data(), audio_source->available(), &bytes_written); - } else { - // Audio is already playing, use regular write to add to the DMA buffers - i2s_channel_write(this->tx_handle_, audio_source->data(), audio_source->available(), &bytes_written, - DMA_BUFFER_DURATION_MS); + if (real_frames > 0) { + pending_real_buffers--; + // Real audio is packed at the start of each DMA buffer with any silence padding on the + // tail, so the real audio finished playing earlier than the buffer-completion timestamp + // by the duration of the trailing zeros. + const uint32_t silence_frames = frames_per_dma_buffer - real_frames; + const int64_t adjusted_ts = + write_timestamp - this->current_stream_info_.frames_to_microseconds(silence_frames); + this->audio_output_callback_(real_frames, adjusted_ts); } + } + if (lockstep_broken) { + break; + } - if (bytes_written > 0) { - last_data_received_time = millis(); - frames_written += this->current_stream_info_.bytes_to_frames(bytes_written); - audio_source->consume(bytes_written); + // Graceful stop: exit only after the source's exposed chunk is drained, the underlying ring + // buffer has nothing left to hand over, and every real-audio buffer we submitted has been + // confirmed played. ``has_buffered_data()`` returns bytes still sitting in the ring buffer + // awaiting fill(). + if (stop_gracefully && audio_source->available() == 0 && !this->has_buffered_data() && + pending_real_buffers == 0) { + ESP_LOGV(TAG, "Exiting: graceful stop complete"); + break; + } - if (tx_dma_underflow) { - tx_dma_underflow = false; - // Enable the on_sent callback and channel after preload - xQueueReset(this->i2s_event_queue_); - const i2s_event_callbacks_t callbacks = {.on_sent = i2s_on_sent_cb}; - i2s_channel_register_event_callback(this->tx_handle_, &callbacks, this); - i2s_channel_enable(this->tx_handle_); + // Compose exactly one DMA buffer's worth: drain as much real audio as the source currently + // exposes (may take multiple fill() calls when crossing a ring buffer wrap), then pad any + // remainder with silence. All writes pack into the next free DMA descriptor in order, so the + // descriptor ends up holding [real audio][silence padding]. + size_t bytes_written_total = 0; + size_t real_bytes_total = 0; + bool partial_write_failure = false; + + if (!this->pause_state_) { + while (bytes_written_total < dma_buffer_bytes) { + size_t bytes_read = audio_source->fill(pdMS_TO_TICKS(DMA_BUFFER_DURATION_MS) / 2, false); + if (bytes_read > 0) { + uint8_t *new_data = audio_source->mutable_data() + audio_source->available() - bytes_read; + this->apply_software_volume_(new_data, bytes_read); + this->swap_esp32_mono_samples_(new_data, bytes_read); } + + const size_t to_write = std::min(audio_source->available(), dma_buffer_bytes - bytes_written_total); + if (to_write == 0) { + // Ring buffer has nothing more to hand over right now; pad the rest of this DMA buffer + // with silence so the lockstep invariant (one write per iteration) is preserved. + break; + } + + size_t bw = 0; + i2s_channel_write(this->tx_handle_, audio_source->data(), to_write, &bw, WRITE_TIMEOUT_TICKS); + if (bw != to_write) { + // A short real-audio write breaks DMA descriptor alignment for every subsequent event; + // the only safe recovery is to restart the task. + ESP_LOGV(TAG, "Partial real audio write: %u of %u bytes", (unsigned) bw, (unsigned) to_write); + xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::ERR_PARTIAL_WRITE); + partial_write_failure = true; + break; + } + audio_source->consume(bw); + bytes_written_total += bw; + real_bytes_total += bw; } + if (real_bytes_total > 0) { + last_data_received_time = millis(); + } + } + + if (partial_write_failure) { + break; + } + + const size_t silence_bytes = dma_buffer_bytes - bytes_written_total; + if (silence_bytes > 0) { + size_t bw = 0; + i2s_channel_write(this->tx_handle_, silence_buffer, silence_bytes, &bw, WRITE_TIMEOUT_TICKS); + if (bw != silence_bytes) { + // Same descriptor-alignment hazard as a partial real-audio write. + ESP_LOGV(TAG, "Partial silence write: %u of %u bytes", (unsigned) bw, (unsigned) silence_bytes); + xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::ERR_PARTIAL_WRITE); + break; + } + } + + const uint32_t real_frames_in_buffer = this->current_stream_info_.bytes_to_frames(real_bytes_total); + // Push the matching write record. Capacity headroom in I2S_EVENT_QUEUE_COUNT guarantees this + // succeeds even with a transient backlog of unprocessed events; if it ever fails the lockstep + // invariant is broken and every subsequent timestamp would be silently wrong, so bail. + if (xQueueSend(this->write_records_queue_, &real_frames_in_buffer, 0) != pdTRUE) { + ESP_LOGV(TAG, "Exiting: write records queue full"); + xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::ERR_LOCKSTEP_DESYNC); + break; + } + if (real_frames_in_buffer > 0) { + pending_real_buffers++; } } } @@ -183,6 +278,11 @@ void I2SAudioSpeaker::run_speaker_task() { audio_source.reset(); + if (silence_buffer != nullptr) { + silence_allocator.deallocate(silence_buffer, dma_buffer_bytes); + silence_buffer = nullptr; + } + xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::TASK_STOPPED); while (true) { @@ -301,7 +401,7 @@ esp_err_t I2SAudioSpeaker::start_i2s_driver(audio::AudioStreamInfo &audio_stream return err; } - i2s_channel_enable(this->tx_handle_); + // The speaker task will enable the channel after preloading. return ESP_OK; } From f94735dc621dc494d7383233cdd904e631fbcdad Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Tue, 12 May 2026 20:38:39 -0500 Subject: [PATCH 19/24] [api][voice_assistant] Add second audio channel for voice_assistant (#16265) Co-authored-by: Kevin Ahrendt Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com> --- esphome/components/api/api.proto | 1 + esphome/components/api/api_pb2.cpp | 7 + esphome/components/api/api_pb2.h | 4 +- esphome/components/api/api_pb2_dump.cpp | 1 + .../components/voice_assistant/__init__.py | 38 +++-- .../voice_assistant/voice_assistant.cpp | 142 +++++++++++++++--- .../voice_assistant/voice_assistant.h | 8 + .../voice_assistant/common-idf.yaml | 14 +- .../voice_assistant/test.esp32-idf.yaml | 1 + 9 files changed, 179 insertions(+), 37 deletions(-) diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 4d72be5407b..f4f15c10428 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -2026,6 +2026,7 @@ message VoiceAssistantAudio { bytes data = 1 [(pointer_to_buffer) = true]; bool end = 2; + bytes data2 = 3 [(pointer_to_buffer) = true]; } enum VoiceAssistantTimerEvent { diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index 68be7550eeb..c711ef167c8 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -2893,6 +2893,11 @@ bool VoiceAssistantAudio::decode_length(uint32_t field_id, ProtoLengthDelimited this->data_len = value.size(); break; } + case 3: { + this->data2 = value.data(); + this->data2_len = value.size(); + break; + } default: return false; } @@ -2902,12 +2907,14 @@ uint8_t *VoiceAssistantAudio::encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG uint8_t *__restrict__ pos = buffer.get_pos(); ProtoEncode::encode_bytes(pos PROTO_ENCODE_DEBUG_ARG, 1, this->data, this->data_len); ProtoEncode::encode_bool(pos PROTO_ENCODE_DEBUG_ARG, 2, this->end); + ProtoEncode::encode_bytes(pos PROTO_ENCODE_DEBUG_ARG, 3, this->data2, this->data2_len); return pos; } uint32_t VoiceAssistantAudio::calculate_size() const { uint32_t size = 0; size += ProtoSize::calc_length(1, this->data_len); size += ProtoSize::calc_bool(1, this->end); + size += ProtoSize::calc_length(1, this->data2_len); return size; } bool VoiceAssistantTimerEventResponse::decode_varint(uint32_t field_id, proto_varint_value_t value) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 7b82f1884d1..7e926ee0d47 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -2436,13 +2436,15 @@ class VoiceAssistantEventResponse final : public ProtoDecodableMessage { class VoiceAssistantAudio final : public ProtoDecodableMessage { public: static constexpr uint8_t MESSAGE_TYPE = 106; - static constexpr uint8_t ESTIMATED_SIZE = 21; + static constexpr uint8_t ESTIMATED_SIZE = 40; #ifdef HAS_PROTO_MESSAGE_DUMP const LogString *message_name() const override { return LOG_STR("voice_assistant_audio"); } #endif const uint8_t *data{nullptr}; uint16_t data_len{0}; bool end{false}; + const uint8_t *data2{nullptr}; + uint16_t data2_len{0}; uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const; uint32_t calculate_size() const; #ifdef HAS_PROTO_MESSAGE_DUMP diff --git a/esphome/components/api/api_pb2_dump.cpp b/esphome/components/api/api_pb2_dump.cpp index 5258b355ceb..850ad37bc96 100644 --- a/esphome/components/api/api_pb2_dump.cpp +++ b/esphome/components/api/api_pb2_dump.cpp @@ -2174,6 +2174,7 @@ const char *VoiceAssistantAudio::dump_to(DumpBuffer &out) const { MessageDumpHelper helper(out, ESPHOME_PSTR("VoiceAssistantAudio")); dump_bytes_field(out, ESPHOME_PSTR("data"), this->data, this->data_len); dump_field(out, ESPHOME_PSTR("end"), this->end); + dump_bytes_field(out, ESPHOME_PSTR("data2"), this->data2, this->data2_len); return out.c_str(); } const char *VoiceAssistantTimerEventResponse::dump_to(DumpBuffer &out) const { diff --git a/esphome/components/voice_assistant/__init__.py b/esphome/components/voice_assistant/__init__.py index 9387797ba2e..958d1cbf91e 100644 --- a/esphome/components/voice_assistant/__init__.py +++ b/esphome/components/voice_assistant/__init__.py @@ -53,6 +53,8 @@ CONF_ON_TIMER_CANCELLED = "on_timer_cancelled" CONF_ON_TIMER_FINISHED = "on_timer_finished" CONF_ON_TIMER_TICK = "on_timer_tick" +MAX_MICROPHONE_SOURCES = 2 + voice_assistant_ns = cg.esphome_ns.namespace("voice_assistant") VoiceAssistant = voice_assistant_ns.class_("VoiceAssistant", cg.Component) @@ -90,13 +92,20 @@ CONFIG_SCHEMA = cv.All( cv.Schema( { cv.GenerateID(): cv.declare_id(VoiceAssistant), - cv.Optional( - CONF_MICROPHONE, default={} - ): microphone.microphone_source_schema( - min_bits_per_sample=16, - max_bits_per_sample=16, - min_channels=1, - max_channels=1, + cv.Optional(CONF_MICROPHONE, default=[{}]): cv.All( + cv.ensure_list( + microphone.microphone_source_schema( + min_bits_per_sample=16, + max_bits_per_sample=16, + min_channels=1, + max_channels=1, + ) + ), + cv.Length( + min=1, + max=MAX_MICROPHONE_SOURCES, + msg=f"Voice Assistant supports at most {MAX_MICROPHONE_SOURCES} microphone sources", + ), ), cv.Exclusive(CONF_MEDIA_PLAYER, "output"): cv.use_id( media_player.MediaPlayer @@ -179,10 +188,10 @@ CONFIG_SCHEMA = cv.All( FINAL_VALIDATE_SCHEMA = cv.All( cv.Schema( { - cv.Optional( - CONF_MICROPHONE - ): microphone.final_validate_microphone_source_schema( - "voice_assistant", sample_rate=16000 + cv.Optional(CONF_MICROPHONE): cv.ensure_list( + microphone.final_validate_microphone_source_schema( + "voice_assistant", sample_rate=16000 + ) ), }, extra=cv.ALLOW_EXTRA, @@ -194,9 +203,14 @@ async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) await cg.register_component(var, config) - mic_source = await microphone.microphone_source_to_code(config[CONF_MICROPHONE]) + mic_sources = config[CONF_MICROPHONE] + mic_source = await microphone.microphone_source_to_code(mic_sources[0]) cg.add(var.set_microphone_source(mic_source)) + if len(mic_sources) > 1: + mic_source2 = await microphone.microphone_source_to_code(mic_sources[1]) + cg.add(var.set_microphone_source2(mic_source2)) + if CONF_MICRO_WAKE_WORD in config: mww = await cg.get_variable(config[CONF_MICRO_WAKE_WORD]) cg.add(var.set_micro_wake_word(mww)) diff --git a/esphome/components/voice_assistant/voice_assistant.cpp b/esphome/components/voice_assistant/voice_assistant.cpp index 50a82652977..286e6645d23 100644 --- a/esphome/components/voice_assistant/voice_assistant.cpp +++ b/esphome/components/voice_assistant/voice_assistant.cpp @@ -31,11 +31,21 @@ VoiceAssistant::VoiceAssistant() { global_voice_assistant = this; } void VoiceAssistant::setup() { this->mic_source_->add_data_callback([this](const std::vector &data) { std::shared_ptr temp_ring_buffer = this->ring_buffer_; - if (this->ring_buffer_.use_count() > 1) { + if (temp_ring_buffer != nullptr) { temp_ring_buffer->write((void *) data.data(), data.size()); } }); + // Second microphone channel + if (this->mic_source2_ != nullptr) { + this->mic_source2_->add_data_callback([this](const std::vector &data) { + std::shared_ptr temp_ring_buffer = this->ring_buffer2_; + if (temp_ring_buffer != nullptr) { + temp_ring_buffer->write((void *) data.data(), data.size()); + } + }); + } + #ifdef USE_MEDIA_PLAYER if (this->media_player_ != nullptr) { this->media_player_->add_on_state_callback([this](media_player::MediaPlayerState state) { @@ -115,9 +125,9 @@ bool VoiceAssistant::allocate_buffers_() { } #endif - if (this->ring_buffer_.use_count() == 0) { + if (this->ring_buffer_ == nullptr) { this->ring_buffer_ = ring_buffer::RingBuffer::create(RING_BUFFER_SIZE); - if (this->ring_buffer_.use_count() == 0) { + if (this->ring_buffer_ == nullptr) { ESP_LOGE(TAG, "Could not allocate ring buffer"); return false; } @@ -132,6 +142,26 @@ bool VoiceAssistant::allocate_buffers_() { } } + // Second microphone channel + if (this->mic_source2_ != nullptr) { + if (this->ring_buffer2_ == nullptr) { + this->ring_buffer2_ = ring_buffer::RingBuffer::create(RING_BUFFER_SIZE); + if (this->ring_buffer2_ == nullptr) { + ESP_LOGE(TAG, "Could not allocate second ring buffer"); + return false; + } + } + + if (this->send_buffer2_ == nullptr) { + RAMAllocator send_allocator; + this->send_buffer2_ = send_allocator.allocate(SEND_BUFFER_SIZE); + if (this->send_buffer2_ == nullptr) { + ESP_LOGW(TAG, "Could not allocate second send buffer"); + return false; + } + } + } + return true; } @@ -144,6 +174,15 @@ void VoiceAssistant::clear_buffers_() { this->ring_buffer_->reset(); } + // Second microphone channel + if (this->send_buffer2_ != nullptr) { + memset(this->send_buffer2_, 0, SEND_BUFFER_SIZE); + } + + if (this->ring_buffer2_ != nullptr) { + this->ring_buffer2_->reset(); + } + #ifdef USE_SPEAKER if ((this->speaker_ != nullptr) && (this->speaker_buffer_ != nullptr)) { memset(this->speaker_buffer_, 0, SPEAKER_BUFFER_SIZE); @@ -162,10 +201,17 @@ void VoiceAssistant::deallocate_buffers_() { this->send_buffer_ = nullptr; } - if (this->ring_buffer_.use_count() > 0) { - this->ring_buffer_.reset(); + this->ring_buffer_.reset(); + + // Second microphone channel + if (this->send_buffer2_ != nullptr) { + RAMAllocator send_deallocator; + send_deallocator.deallocate(this->send_buffer2_, SEND_BUFFER_SIZE); + this->send_buffer2_ = nullptr; } + this->ring_buffer2_.reset(); + #ifdef USE_SPEAKER if ((this->speaker_ != nullptr) && (this->speaker_buffer_ != nullptr)) { RAMAllocator speaker_deallocator; @@ -183,7 +229,8 @@ void VoiceAssistant::reset_conversation_id() { void VoiceAssistant::loop() { if (this->api_client_ == nullptr && this->state_ != State::IDLE && this->state_ != State::STOP_MICROPHONE && this->state_ != State::STOPPING_MICROPHONE) { - if (this->mic_source_->is_running() || this->state_ == State::STARTING_MICROPHONE) { + if (this->mic_source_->is_running() || (this->mic_source2_ && this->mic_source2_->is_running()) || + this->state_ == State::STARTING_MICROPHONE) { this->set_state_(State::STOP_MICROPHONE, State::IDLE); } else { this->set_state_(State::IDLE, State::IDLE); @@ -215,11 +262,14 @@ void VoiceAssistant::loop() { this->clear_buffers_(); this->mic_source_->start(); + if (this->mic_source2_) { + this->mic_source2_->start(); + } this->set_state_(State::STARTING_MICROPHONE); break; } case State::STARTING_MICROPHONE: { - if (this->mic_source_->is_running()) { + if (this->mic_source_->is_running() && (!this->mic_source2_ || this->mic_source2_->is_running())) { this->set_state_(this->desired_state_); } break; @@ -266,15 +316,44 @@ void VoiceAssistant::loop() { break; // State changed when udp server port received } case State::STREAMING_MICROPHONE: { - size_t available = this->ring_buffer_->available(); - while (available >= SEND_BUFFER_SIZE) { - size_t read_bytes = this->ring_buffer_->read((void *) this->send_buffer_, SEND_BUFFER_SIZE, 0); - if (this->audio_mode_ == AUDIO_MODE_API) { + if (this->audio_mode_ == AUDIO_MODE_API) { + // API audio + // Both microphone channels are sent, if configured + bool is_available = this->ring_buffer_->available() >= SEND_BUFFER_SIZE; + bool is_available2 = false; + if (this->mic_source2_) { + is_available2 = this->ring_buffer2_->available() >= SEND_BUFFER_SIZE; + } + + while (is_available || is_available2) { api::VoiceAssistantAudio msg; - msg.data = this->send_buffer_; - msg.data_len = read_bytes; + + if (is_available) { + size_t read_bytes = this->ring_buffer_->read((void *) this->send_buffer_, SEND_BUFFER_SIZE, 0); + msg.data = this->send_buffer_; + msg.data_len = read_bytes; + } + + // Second microphone channel + if (is_available2) { + size_t read_bytes = this->ring_buffer2_->read((void *) this->send_buffer2_, SEND_BUFFER_SIZE, 0); + msg.data2 = this->send_buffer2_; + msg.data2_len = read_bytes; + } + this->api_client_->send_message(msg); - } else { + is_available = this->ring_buffer_->available() >= SEND_BUFFER_SIZE; + if (this->mic_source2_) { + is_available2 = this->ring_buffer2_->available() >= SEND_BUFFER_SIZE; + } else { + is_available2 = false; + } + } + } else { + // UDP (will eventually be deprecated) + // Only the primary microphone channel is used + while (this->ring_buffer_->available() >= SEND_BUFFER_SIZE) { + size_t read_bytes = this->ring_buffer_->read((void *) this->send_buffer_, SEND_BUFFER_SIZE, 0); if (!this->udp_socket_running_) { if (!this->start_udp_socket_()) { this->set_state_(State::STOP_MICROPHONE, State::IDLE); @@ -284,14 +363,23 @@ void VoiceAssistant::loop() { this->socket_->sendto(this->send_buffer_, read_bytes, 0, (struct sockaddr *) &this->dest_addr_, sizeof(this->dest_addr_)); } - available = this->ring_buffer_->available(); - } - + } // audio mode break; } case State::STOP_MICROPHONE: { - if (this->mic_source_->is_running()) { - this->mic_source_->stop(); + // Check both microphone channels + bool is_running = this->mic_source_->is_running(); + bool is_running2 = false; + if (this->mic_source2_) { + is_running2 = this->mic_source2_->is_running(); + } + if (is_running || is_running2) { + if (is_running) { + this->mic_source_->stop(); + } + if (is_running2) { + this->mic_source2_->stop(); + } this->set_state_(State::STOPPING_MICROPHONE); } else { this->set_state_(this->desired_state_); @@ -299,7 +387,13 @@ void VoiceAssistant::loop() { break; } case State::STOPPING_MICROPHONE: { - if (this->mic_source_->is_stopped()) { + // Check both microphone channels + bool is_stopped = this->mic_source_->is_stopped(); + bool is_stopped2 = true; + if (this->mic_source2_) { + is_stopped2 = this->mic_source2_->is_stopped(); + } + if (is_stopped && is_stopped2) { this->set_state_(this->desired_state_); } break; @@ -504,7 +598,8 @@ void VoiceAssistant::start_streaming() { ESP_LOGD(TAG, "Client started, streaming microphone"); this->audio_mode_ = AUDIO_MODE_API; - if (this->mic_source_->is_running()) { + // Both microphone channels + if (this->mic_source_->is_running() && (!this->mic_source2_ || this->mic_source2_->is_running())) { this->set_state_(State::STREAMING_MICROPHONE, State::STREAMING_MICROPHONE); } else { this->set_state_(State::START_MICROPHONE, State::STREAMING_MICROPHONE); @@ -520,6 +615,10 @@ void VoiceAssistant::start_streaming(struct sockaddr_storage *addr, uint16_t por ESP_LOGD(TAG, "Client started, streaming microphone"); this->audio_mode_ = AUDIO_MODE_UDP; + if (this->mic_source2_ != nullptr) { + ESP_LOGW(TAG, "UDP audio mode does not support a second microphone channel; only the primary will be streamed"); + } + memcpy(&this->dest_addr_, addr, sizeof(this->dest_addr_)); if (this->dest_addr_.ss_family == AF_INET) { ((struct sockaddr_in *) &this->dest_addr_)->sin_port = htons(port); @@ -534,6 +633,7 @@ void VoiceAssistant::start_streaming(struct sockaddr_storage *addr, uint16_t por return; } + // Only primary microphone channel over UDP if (this->mic_source_->is_running()) { this->set_state_(State::STREAMING_MICROPHONE, State::STREAMING_MICROPHONE); } else { diff --git a/esphome/components/voice_assistant/voice_assistant.h b/esphome/components/voice_assistant/voice_assistant.h index 3de46730018..c4fa7eb6152 100644 --- a/esphome/components/voice_assistant/voice_assistant.h +++ b/esphome/components/voice_assistant/voice_assistant.h @@ -40,6 +40,7 @@ enum VoiceAssistantFeature : uint32_t { FEATURE_TIMERS = 1 << 3, FEATURE_ANNOUNCE = 1 << 4, FEATURE_START_CONVERSATION = 1 << 5, + FEATURE_MULTI_CHANNEL_AUDIO = 1 << 6, }; enum class State { @@ -120,6 +121,7 @@ class VoiceAssistant : public Component { void failed_to_start(); void set_microphone_source(microphone::MicrophoneSource *mic_source) { this->mic_source_ = mic_source; } + void set_microphone_source2(microphone::MicrophoneSource *mic_source2) { this->mic_source2_ = mic_source2; } #ifdef USE_MICRO_WAKE_WORD void set_micro_wake_word(micro_wake_word::MicroWakeWord *mww) { this->micro_wake_word_ = mww; } #endif @@ -149,6 +151,9 @@ class VoiceAssistant : public Component { uint32_t flags = 0; flags |= VoiceAssistantFeature::FEATURE_VOICE_ASSISTANT; flags |= VoiceAssistantFeature::FEATURE_API_AUDIO; + if (this->mic_source2_ != nullptr) { + flags |= VoiceAssistantFeature::FEATURE_MULTI_CHANNEL_AUDIO; + } #ifdef USE_SPEAKER if (this->speaker_ != nullptr) { flags |= VoiceAssistantFeature::FEATURE_SPEAKER; @@ -276,6 +281,7 @@ class VoiceAssistant : public Component { bool timer_tick_running_{false}; microphone::MicrophoneSource *mic_source_{nullptr}; + microphone::MicrophoneSource *mic_source2_{nullptr}; #ifdef USE_SPEAKER void write_speaker_(); speaker::Speaker *speaker_{nullptr}; @@ -301,6 +307,7 @@ class VoiceAssistant : public Component { std::string wake_word_; std::shared_ptr ring_buffer_; + std::shared_ptr ring_buffer2_; bool use_wake_word_; uint8_t noise_suppression_level_; @@ -309,6 +316,7 @@ class VoiceAssistant : public Component { uint32_t conversation_timeout_; uint8_t *send_buffer_{nullptr}; + uint8_t *send_buffer2_{nullptr}; bool continuous_{false}; bool silence_detection_; diff --git a/tests/components/voice_assistant/common-idf.yaml b/tests/components/voice_assistant/common-idf.yaml index 85656837003..0fa09033700 100644 --- a/tests/components/voice_assistant/common-idf.yaml +++ b/tests/components/voice_assistant/common-idf.yaml @@ -31,6 +31,11 @@ microphone: i2s_din_pin: ${i2s_din_pin} adc_type: external pdm: false + - platform: i2s_audio + id: mic_id_external2 + i2s_din_pin: ${i2s_din_pin2} + adc_type: external + pdm: false speaker: - platform: i2s_audio @@ -40,9 +45,12 @@ speaker: voice_assistant: microphone: - microphone: mic_id_external - gain_factor: 4 - channels: 0 + - microphone: mic_id_external + gain_factor: 4 + channels: 0 + - microphone: mic_id_external2 + gain_factor: 4 + channels: 0 speaker: speaker_id micro_wake_word: mww_id conversation_timeout: 60s diff --git a/tests/components/voice_assistant/test.esp32-idf.yaml b/tests/components/voice_assistant/test.esp32-idf.yaml index 1c5c9ddf994..0cc670a77ef 100644 --- a/tests/components/voice_assistant/test.esp32-idf.yaml +++ b/tests/components/voice_assistant/test.esp32-idf.yaml @@ -3,6 +3,7 @@ substitutions: i2s_bclk_pin: GPIO5 i2s_mclk_pin: GPIO15 i2s_din_pin: GPIO13 + i2s_din_pin2: GPIO14 i2s_dout_pin: GPIO12 <<: !include common-idf.yaml From 1dfd3fe9c26706285cc8fca70acf6acf7591ec4a Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Tue, 12 May 2026 22:52:11 -0400 Subject: [PATCH 20/24] [esp32] Print PlatformIO-format RAM/Flash summary after native ESP-IDF builds (#16394) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- esphome/build_gen/espidf.py | 10 +++ esphome/espidf/runner.py | 9 +++ esphome/espidf/size_summary.py | 111 +++++++++++++++++++++++++++++++++ esphome/espidf/toolchain.py | 9 ++- 4 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 esphome/espidf/size_summary.py diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index b1443edac31..dfe2d72b9de 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -89,6 +89,16 @@ include($ENV{{IDF_PATH}}/tools/cmake/project.cmake) {extra_compile_options} project({CORE.name}) + +# Emit raw JSON size data for ESPHome to read post-build. +add_custom_command( + TARGET ${{CMAKE_PROJECT_NAME}}.elf POST_BUILD + COMMAND ${{PYTHON}} -m esp_idf_size --ng --format=raw + -o ${{CMAKE_BINARY_DIR}}/esp_idf_size.json + ${{CMAKE_PROJECT_NAME}}.map + WORKING_DIRECTORY ${{CMAKE_BINARY_DIR}} + VERBATIM +) """ diff --git a/esphome/espidf/runner.py b/esphome/espidf/runner.py index 34e3e7694b5..65df37c7b24 100644 --- a/esphome/espidf/runner.py +++ b/esphome/espidf/runner.py @@ -57,6 +57,15 @@ FILTER_IDF_LINES: list[str] = [ # line, so a NOTICE often arrives prefixed with ".NOTICE:" or # "...........NOTICE:". r"\.*NOTICE: ", + # ``idf.py size`` prefaces its table with a centered banner; the + # per-region table below already makes the structure obvious. + r"\s*Memory Type Usage Summary", + # Prefix match for esp-idf-size's trailing "Note:" paragraph (no + # upstream flag suppresses it). + r"Note: The reported total sizes may be smaller than those in the", + # Drop the blank line rich emits after the note so the build log + # doesn't end with an orphan gap before ESPHome's own status lines. + r"\s*$", ] diff --git a/esphome/espidf/size_summary.py b/esphome/espidf/size_summary.py new file mode 100644 index 00000000000..9477e664b34 --- /dev/null +++ b/esphome/espidf/size_summary.py @@ -0,0 +1,111 @@ +"""PlatformIO-format RAM/Flash one-liners after a native ESP-IDF build. + +``idf.py size`` (chained onto ``idf.py build`` in +``toolchain.run_compile``) prints the per-region table inline as part +of the build. This module adds two summary lines underneath, +byte-identical to PlatformIO's output: + + RAM: [==== ] 26.5% (used 47932 bytes from 180736 bytes) + Flash: [=== ] 48.4% (used 888511 bytes from 1835008 bytes) + +The format matches ``script/ci_memory_impact_extract.py`` so CI memory +analysis works unchanged on native ESP-IDF builds. RAM total is the +DRAM region size from the linker map; Flash total is taken from +``partitions.csv`` using PlatformIO's rule (first app partition whose +subtype is ``factory`` or ``ota_0``; see +``platform-espressif32/builder/main.py::_update_max_upload_size``). + +Structured size data is produced at link time by a CMake POST_BUILD +custom command (see ``build_gen/espidf.py``) which writes +``esp_idf_size.json`` next to the ELF. We read that file here rather +than re-running ``esp_idf_size`` from Python. +""" + +from __future__ import annotations + +import csv +import json +import logging +from pathlib import Path + +_LOGGER = logging.getLogger(__name__) +_SIZE_SUFFIXES = {"K": 1024, "M": 1024 * 1024} + + +def _parse_size(token: str) -> int: + token = token.strip() + if not token: + return 0 + if token.startswith(("0x", "0X")): + return int(token, 16) + suffix = token[-1].upper() + if suffix in _SIZE_SUFFIXES: + return int(token[:-1]) * _SIZE_SUFFIXES[suffix] + return int(token) + + +def _find_app_partition_size(partitions_csv: Path) -> int: + """Return the size of the firmware's app partition. + + Mirrors PlatformIO's ``platform-espressif32/builder/main.py:: + _update_max_upload_size``: take the first ``app``-type partition + whose subtype is ``factory`` or ``ota_0``. Order matters because + layouts like Adafruit's ``partitions-4MB-tinyuf2.csv`` repurpose + ``factory`` for a UF2 bootloader before the real OTA slot, so a + naive "prefer factory" rule would pick the wrong row. Raises + ``ValueError`` if no qualifying partition is present. + """ + if not partitions_csv.is_file(): + raise ValueError(f"partitions.csv not found at {partitions_csv}") + for row in csv.reader(partitions_csv.read_text().splitlines()): + cells = [c.strip() for c in row] + if not cells or cells[0].startswith("#") or len(cells) < 5: + continue + ptype, psubtype, psize = cells[1], cells[2], cells[4] + if ptype in ("app", "0") and psubtype in ("factory", "ota_0"): + return _parse_size(psize) + raise ValueError(f"No app+factory or app+ota_0 partition in {partitions_csv}") + + +def _format_bar(used: int, total: int) -> str: + """Match PlatformIO's ``_format_availale_bytes`` (pioupload.py) exactly.""" + pct_raw = used / total if total else 0 + blocks = 10 + filled = min(int(round(blocks * pct_raw)), blocks) + progress = "=" * filled + return ( + f"[{progress:<{blocks}}] {pct_raw: 6.1%} " + f"(used {used:d} bytes from {total:d} bytes)" + ) + + +def print_summary(size_json: Path, partitions_csv: Path | None) -> None: + """Print PlatformIO-shaped RAM and Flash one-liners. + + Failures are non-fatal: the build has already succeeded, we just couldn't + summarize. Logs the cause at debug level. + """ + if not size_json.is_file(): + _LOGGER.debug("Skipping size summary: %s not found", size_json) + return + try: + data = json.loads(size_json.read_text()) + except (OSError, json.JSONDecodeError) as e: + _LOGGER.debug("Skipping size summary: %s", e) + return + + dram = data.get("memory_types", {}).get("DRAM") or {} + ram_used = dram.get("used") + ram_total = dram.get("size") + if ram_total and ram_used is not None: + print(f"RAM: {_format_bar(ram_used, ram_total)}") + + image_size = data.get("image_size") + if image_size is None or partitions_csv is None: + return + try: + app_size = _find_app_partition_size(partitions_csv) + except ValueError as e: + _LOGGER.debug("Skipping Flash summary: %s", e) + return + print(f"Flash: {_format_bar(image_size, app_size)}") diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index da6d3a8a37f..ecb759ed10c 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -12,6 +12,7 @@ import subprocess from esphome.components.esp32.const import KEY_ESP32, KEY_FLASH_SIZE, KEY_IDF_VERSION from esphome.core import CORE, EsphomeError from esphome.espidf.framework import check_esp_idf_install, get_framework_env +from esphome.espidf.size_summary import print_summary _LOGGER = logging.getLogger(__name__) @@ -341,8 +342,14 @@ def run_compile(config, verbose: bool) -> int: args.extend(_get_sdkconfig_args()) args.append("build") + args.append("size") - return run_idf_py(*args) + rc = run_idf_py(*args) + if rc == 0: + size_json = CORE.relative_build_path("build", "esp_idf_size.json") + partitions = CORE.relative_build_path("partitions.csv") + print_summary(size_json, partitions if partitions.is_file() else None) + return rc def get_firmware_path() -> Path: From 480c23012c6eefdc9ca647a0a03b48b936b885c3 Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Tue, 12 May 2026 22:13:29 -0500 Subject: [PATCH 21/24] [radio_frequency] Add on_control trigger; ir_rf_proxy driver-agnostic (#16368) Co-authored-by: Claude Sonnet 4.6 --- .../components/ir_rf_proxy/ir_rf_proxy.cpp | 5 +- esphome/components/ir_rf_proxy/ir_rf_proxy.h | 5 +- .../components/ir_rf_proxy/radio_frequency.py | 12 +++-- .../components/radio_frequency/__init__.py | 9 +++- .../radio_frequency/radio_frequency.cpp | 4 ++ .../radio_frequency/radio_frequency.h | 11 ++++ .../components/ir_rf_proxy/common-cc1101.yaml | 50 +++++++++++++++++++ .../ir_rf_proxy/test-cc1101.esp32-idf.yaml | 9 ++++ .../ir_rf_proxy/test-cc1101.esp8266-ard.yaml | 9 ++++ 9 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/components/ir_rf_proxy/common-cc1101.yaml create mode 100644 tests/components/ir_rf_proxy/test-cc1101.esp32-idf.yaml create mode 100644 tests/components/ir_rf_proxy/test-cc1101.esp8266-ard.yaml diff --git a/esphome/components/ir_rf_proxy/ir_rf_proxy.cpp b/esphome/components/ir_rf_proxy/ir_rf_proxy.cpp index 60b0cd513bb..c13c6198cb6 100644 --- a/esphome/components/ir_rf_proxy/ir_rf_proxy.cpp +++ b/esphome/components/ir_rf_proxy/ir_rf_proxy.cpp @@ -106,7 +106,6 @@ void RfProxy::setup() { void RfProxy::dump_config() { ESP_LOGCONFIG(TAG, "RF Proxy '%s'\n" - " Backend: remote_transmitter/receiver\n" " Supports Transmitter: %s\n" " Supports Receiver: %s", this->get_name().c_str(), YESNO(this->traits_.get_supports_transmitter()), @@ -124,7 +123,9 @@ void RfProxy::dump_config() { } void RfProxy::control(const radio_frequency::RadioFrequencyCall &call) { - // RF: no IR carrier modulation + // RF: no IR carrier modulation. Any RF front-end coordination (state turnaround, retuning) + // happens via the radio_frequency entity's on_control trigger and remote_transmitter's + // on_transmit/on_complete triggers — wired up in user YAML. transmit_raw_timings(this->transmitter_, 0, call); } diff --git a/esphome/components/ir_rf_proxy/ir_rf_proxy.h b/esphome/components/ir_rf_proxy/ir_rf_proxy.h index 973e9e20514..d0467e822d1 100644 --- a/esphome/components/ir_rf_proxy/ir_rf_proxy.h +++ b/esphome/components/ir_rf_proxy/ir_rf_proxy.h @@ -43,7 +43,10 @@ class IrRfProxy : public infrared::Infrared { #endif // USE_IR_RF #ifdef USE_RADIO_FREQUENCY -/// RfProxy - Radio Frequency platform implementation using remote_transmitter/receiver as backend +/// RfProxy - Radio Frequency platform implementation using remote_transmitter/receiver as backend. +/// Driver-agnostic: integration with specific RF front-end chips (CC1101, RFM69, etc.) is done +/// in YAML by wiring their actions to `remote_transmitter`'s on_transmit/on_complete triggers and +/// to this entity's on_control trigger (see radio_frequency component docs). class RfProxy : public radio_frequency::RadioFrequency { public: RfProxy() = default; diff --git a/esphome/components/ir_rf_proxy/radio_frequency.py b/esphome/components/ir_rf_proxy/radio_frequency.py index 9982f5e4d10..a243909837f 100644 --- a/esphome/components/ir_rf_proxy/radio_frequency.py +++ b/esphome/components/ir_rf_proxy/radio_frequency.py @@ -35,17 +35,19 @@ def _final_validate(config: ConfigType) -> None: if CONF_REMOTE_TRANSMITTER_ID not in config: return - transmitter_id = config[CONF_REMOTE_TRANSMITTER_ID] full_config = fv.full_config.get() - transmitter_path = full_config.get_path_for_id(transmitter_id)[:-1] + transmitter_path = full_config.get_path_for_id(config[CONF_REMOTE_TRANSMITTER_ID])[ + :-1 + ] transmitter_config = full_config.get_config_for_path(transmitter_path) duty_percent = transmitter_config.get(CONF_CARRIER_DUTY_PERCENT) if duty_percent is not None and duty_percent != 100: raise cv.Invalid( - f"Transmitter '{transmitter_id}' must have '{CONF_CARRIER_DUTY_PERCENT}' " - "set to 100% for RF transmission. Dedicated RF hardware handles modulation; " - "applying a carrier duty cycle would corrupt the signal" + f"Transmitter '{config[CONF_REMOTE_TRANSMITTER_ID]}' must have " + f"'{CONF_CARRIER_DUTY_PERCENT}' set to 100% for RF transmission. " + "Dedicated RF hardware handles modulation; applying a carrier duty cycle " + "would corrupt the signal" ) diff --git a/esphome/components/radio_frequency/__init__.py b/esphome/components/radio_frequency/__init__.py index a54ab6e2492..9fdafe428a6 100644 --- a/esphome/components/radio_frequency/__init__.py +++ b/esphome/components/radio_frequency/__init__.py @@ -8,9 +8,10 @@ breaking changes policy. Use at your own risk. Once the API is considered stable, this warning will be removed. """ +from esphome import automation import esphome.codegen as cg import esphome.config_validation as cv -from esphome.const import CONF_ID +from esphome.const import CONF_ID, CONF_ON_CONTROL from esphome.core import CORE, coroutine_with_priority from esphome.core.entity_helpers import queue_entity_register, setup_entity from esphome.coroutine import CoroPriority @@ -42,6 +43,7 @@ def radio_frequency_schema(class_: type[cg.MockObjClass]) -> cv.Schema: return entity_schema.extend( { cv.GenerateID(): cv.declare_id(class_), + cv.Optional(CONF_ON_CONTROL): automation.validate_automation({}), } ) @@ -59,6 +61,11 @@ async def register_radio_frequency(var: cg.Pvariable, config: ConfigType) -> Non await setup_radio_frequency_core_(var, config) CORE.register_platform_component("radio_frequency", var) + for conf in config.get(CONF_ON_CONTROL, []): + await automation.build_callback_automation( + var, "add_on_control_callback", [(RadioFrequencyCall, "x")], conf + ) + async def new_radio_frequency(config: ConfigType, *args) -> cg.Pvariable: """Create a new RadioFrequency instance. diff --git a/esphome/components/radio_frequency/radio_frequency.cpp b/esphome/components/radio_frequency/radio_frequency.cpp index 3c000ae1cad..3e0a905737e 100644 --- a/esphome/components/radio_frequency/radio_frequency.cpp +++ b/esphome/components/radio_frequency/radio_frequency.cpp @@ -54,6 +54,10 @@ RadioFrequencyCall &RadioFrequencyCall::set_repeat_count(uint32_t count) { void RadioFrequencyCall::perform() { if (this->parent_ != nullptr) { + // Fire any on_control hooks (user-wired automations) before handing off to + // the platform-specific control() — gives users a chance to react to call + // parameters (e.g. retune an external RF front-end based on call.get_frequency()). + this->parent_->control_callback_.call(*this); this->parent_->control(*this); } } diff --git a/esphome/components/radio_frequency/radio_frequency.h b/esphome/components/radio_frequency/radio_frequency.h index db73a844ede..7dfd2dd77e1 100644 --- a/esphome/components/radio_frequency/radio_frequency.h +++ b/esphome/components/radio_frequency/radio_frequency.h @@ -170,6 +170,15 @@ class RadioFrequency : public Component, public EntityBase, public remote_base:: this->receive_callback_.add(std::forward(callback)); } + /// Add a callback to invoke when a transmit call is made on this entity. + /// Fires before the platform-specific control() runs, with the call object + /// (containing frequency, modulation, repeat count, etc.). Used by the + /// `on_control` YAML trigger so users can wire any RF front-end driver + /// (CC1101, RFM69, custom) to react to per-call parameters. + template void add_on_control_callback(F &&callback) { + this->control_callback_.add(std::forward(callback)); + } + protected: friend class RadioFrequencyCall; @@ -182,6 +191,8 @@ class RadioFrequency : public Component, public EntityBase, public remote_base:: // Callback manager for receive events (lazy: saves memory when no callbacks registered) LazyCallbackManager receive_callback_; + // Callback manager for on_control trigger (lazy: same memory savings) + LazyCallbackManager control_callback_; }; } // namespace esphome::radio_frequency diff --git a/tests/components/ir_rf_proxy/common-cc1101.yaml b/tests/components/ir_rf_proxy/common-cc1101.yaml new file mode 100644 index 00000000000..392e6db22ef --- /dev/null +++ b/tests/components/ir_rf_proxy/common-cc1101.yaml @@ -0,0 +1,50 @@ +cc1101: + id: cc1101_radio + cs_pin: ${cs_pin} + frequency: 433.92MHz + modulation_type: ASK/OOK + output_power: 10 + +# Dual-pin wiring (recommended by the CC1101 docs): +# CC1101 GDO0 → ${gdo0_pin} (remote_transmitter) +# CC1101 GDO2 → ${gdo2_pin} (remote_receiver) +remote_transmitter: + id: rf_tx + pin: ${gdo0_pin} + carrier_duty_percent: 100% + # Switch the chip into TX state for the duration of each transmission and back to RX + # afterwards. Driver-agnostic: any RF front-end with begin_tx/begin_rx-style actions + # can be wired this way. + on_transmit: + then: + - cc1101.begin_tx: cc1101_radio + on_complete: + then: + - cc1101.begin_rx: cc1101_radio + +remote_receiver: + id: rf_rx + pin: ${gdo2_pin} + +radio_frequency: + - platform: ir_rf_proxy + id: rf_proxy_cc1101_tx + name: "CC1101 RF Transmitter" + frequency: 433.92MHz + remote_transmitter_id: rf_tx + # Optional: retune the CC1101 per-transmit when the API request specifies a + # different carrier frequency. Demonstrates the on_control trigger. + on_control: + then: + - if: + condition: + lambda: "return x.get_frequency().has_value() && *x.get_frequency() > 0;" + then: + - cc1101.set_frequency: + id: cc1101_radio + value: !lambda "return *x.get_frequency();" + - platform: ir_rf_proxy + id: rf_proxy_cc1101_rx + name: "CC1101 RF Receiver" + frequency: 433.92MHz + remote_receiver_id: rf_rx diff --git a/tests/components/ir_rf_proxy/test-cc1101.esp32-idf.yaml b/tests/components/ir_rf_proxy/test-cc1101.esp32-idf.yaml new file mode 100644 index 00000000000..bf6f3d9815f --- /dev/null +++ b/tests/components/ir_rf_proxy/test-cc1101.esp32-idf.yaml @@ -0,0 +1,9 @@ +substitutions: + cs_pin: GPIO5 + gdo0_pin: GPIO4 + gdo2_pin: GPIO16 + +packages: + common: !include common.yaml + spi: !include ../../test_build_components/common/spi/esp32-idf.yaml + cc1101: !include common-cc1101.yaml diff --git a/tests/components/ir_rf_proxy/test-cc1101.esp8266-ard.yaml b/tests/components/ir_rf_proxy/test-cc1101.esp8266-ard.yaml new file mode 100644 index 00000000000..e25c47ab236 --- /dev/null +++ b/tests/components/ir_rf_proxy/test-cc1101.esp8266-ard.yaml @@ -0,0 +1,9 @@ +substitutions: + cs_pin: GPIO5 + gdo0_pin: GPIO4 + gdo2_pin: GPIO16 + +packages: + common: !include common.yaml + spi: !include ../../test_build_components/common/spi/esp8266-ard.yaml + cc1101: !include common-cc1101.yaml From 65ea29b44ac9f8e3dcb1dd64809b4a6d57dc853c Mon Sep 17 00:00:00 2001 From: Dmitrii Kuminov Date: Tue, 12 May 2026 20:41:30 -0700 Subject: [PATCH 22/24] [core] Fix !include vars not being substituted in !lambda values (#16320) Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston --- esphome/yaml_util.py | 2 +- tests/unit_tests/test_substitutions.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index 3cfc9c4b15d..b153d160a72 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -139,7 +139,7 @@ def add_context(value: Any, context_vars: dict[str, Any] | None) -> Any: value.set_context({**value.vars, **(context_vars or {})}) return value - if context_vars and isinstance(value, (dict, list, str)): + if context_vars and isinstance(value, (dict, list, str, Lambda)): value = add_class_to_obj(value, ConfigContext) value.set_context(context_vars) return value diff --git a/tests/unit_tests/test_substitutions.py b/tests/unit_tests/test_substitutions.py index cf6d4adbf51..4783112578c 100644 --- a/tests/unit_tests/test_substitutions.py +++ b/tests/unit_tests/test_substitutions.py @@ -818,3 +818,23 @@ def test_resolve_include_error_no_expanded_from_for_literal_filename( substitutions.resolve_include(include, [], substitutions.ContextVars()) assert "expanded from" not in str(exc_info.value) + + +def test_include_vars_applied_to_lambda_value(tmp_path: Path) -> None: + """!include vars: must substitute into a top-level !lambda value in the included file. + + Regression test for the case where the included file's root is a Lambda; + add_context() previously only tagged dict/list/str, so the include's vars + never reached the substitution pass for Lambda content. + """ + included = tmp_path / "lambda.yaml" + included.write_text('!lambda |-\n return "${foo}";\n') + + include = yaml_util.IncludeFile( + tmp_path / "main.yaml", "lambda.yaml", {"foo": "bar"}, yaml_util.load_yaml + ) + config = OrderedDict({"value": include.load()}) + result = substitutions.do_substitution_pass(config) + + assert isinstance(result["value"], Lambda) + assert result["value"].value == 'return "bar";' From 386e95fbef2bcb254c1adee59226202640591bf2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 May 2026 00:51:22 -0500 Subject: [PATCH 23/24] [core] Retry PlatformIO downloads on transport-layer errors --- esphome/platformio/runner.py | 12 +++-- tests/unit_tests/test_platformio_toolchain.py | 50 +++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/esphome/platformio/runner.py b/esphome/platformio/runner.py index 976979dc57b..caab47dcc24 100644 --- a/esphome/platformio/runner.py +++ b/esphome/platformio/runner.py @@ -51,9 +51,13 @@ def patch_file_downloader() -> None: """Retry PlatformIO package downloads with exponential backoff. PlatformIO's ``FileDownloader`` uses an ``HTTPSession`` without built-in - retry for 502/503 errors. We wrap ``__init__`` to retry on - ``PackageException`` and close the session between attempts so a new - TCP connection can route to a different CDN edge node. + retry. We wrap ``__init__`` to retry on transient failures and close the + session between attempts so a new TCP connection can route to a different + CDN edge node. We catch both ``PackageException`` (raised when the server + returns a non-200 status such as 502/503) and ``OSError`` -- which covers + ``requests.exceptions.ConnectionError``, ``ReadTimeout``, and + ``ChunkedEncodingError`` (all subclasses of ``OSError``) that get raised + when the connection is aborted before a response is parsed. """ from platformio.package.download import FileDownloader from platformio.package.exception import PackageException @@ -70,7 +74,7 @@ def patch_file_downloader() -> None: try: original_init(self, *args, **kwargs) return - except PackageException as e: + except (PackageException, OSError) as e: if attempt < max_retries - 1: delay = 2 ** (attempt + 1) _LOGGER.warning( diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index f771437dd47..64f82ba74f2 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -867,6 +867,56 @@ def test_patch_file_downloader_closes_session_and_response_between_retries() -> mock_session.close.assert_called_once() +def test_patch_file_downloader_retries_on_connection_error() -> None: + """Test patch_file_downloader retries on transport-layer errors (OSError subclasses). + + ``requests.exceptions.ConnectionError`` and ``ReadTimeout`` subclass + ``OSError`` and are raised when the connection is aborted before any HTTP + response is parsed -- e.g. ``RemoteDisconnected`` mid-download. These must + retry too, not just ``PackageException``. + """ + mock_exception_cls = type("PackageException", (Exception,), {}) + call_count = 0 + + def failing_init(self, *args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count < 3: + raise ConnectionError( + f"Connection aborted attempt {call_count}: RemoteDisconnected" + ) + + with ( + patch.dict( + "sys.modules", + { + "platformio": MagicMock(), + "platformio.package": MagicMock(), + "platformio.package.download": SimpleNamespace( + FileDownloader=type( + "FileDownloader", (), {"__init__": failing_init} + ) + ), + "platformio.package.exception": SimpleNamespace( + PackageException=mock_exception_cls + ), + }, + ), + patch("time.sleep") as mock_sleep, + ): + runner.patch_file_downloader() + + from platformio.package.download import FileDownloader + + instance = object.__new__(FileDownloader) + FileDownloader.__init__(instance, "http://example.com/file.zip") + + assert call_count == 3 + assert mock_sleep.call_count == 2 + mock_sleep.assert_any_call(2) + mock_sleep.assert_any_call(4) + + def test_patch_file_downloader_idempotent() -> None: """Test patch_file_downloader does not stack wrappers when called multiple times.""" mock_exception_cls = type("PackageException", (Exception,), {}) From ac7c572303ce84b3da42586e1d3cae0326d27994 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 May 2026 00:57:20 -0500 Subject: [PATCH 24/24] [core] Add socket-based tests for PlatformIO download retry --- tests/unit_tests/test_platformio_toolchain.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index 64f82ba74f2..c1d16530cbf 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -2,10 +2,13 @@ # pylint: disable=protected-access +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer import json import os from pathlib import Path import shutil +import threading from types import SimpleNamespace from unittest.mock import MagicMock, Mock, call, patch @@ -953,6 +956,74 @@ def test_patch_file_downloader_idempotent() -> None: assert call_count == 1 +@contextmanager +def _flaky_http_server(fail_first_n: int, fail_mode: str): + """Local HTTP server that fails the first ``fail_first_n`` requests. + + ``fail_mode="drop"`` closes the TCP connection without responding, so + the client raises ``RemoteDisconnected`` -- the exact CI failure mode. + ``fail_mode="502"`` returns an HTTP 502, triggering ``PackageException``. + """ + state = {"hits": 0} + + class _Handler(BaseHTTPRequestHandler): + def handle_one_request(self) -> None: + state["hits"] += 1 + if state["hits"] <= fail_first_n and fail_mode == "drop": + return # Skip read+respond → kernel sends FIN → RemoteDisconnected + super().handle_one_request() + + def do_GET(self) -> None: # noqa: N802 + if state["hits"] <= fail_first_n and fail_mode == "502": + self.send_error(502) + return + body = b"esphome-test-payload" + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, format: str, *args: object) -> None: # noqa: A002 + pass # silence default stderr logging + + server = ThreadingHTTPServer(("127.0.0.1", 0), _Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield server.server_address[1], state + finally: + server.shutdown() + server.server_close() + thread.join(timeout=2) + + +@pytest.mark.parametrize("fail_mode", ["drop", "502"]) +def test_patch_file_downloader_recovers_against_real_server( + tmp_path: Path, fail_mode: str +) -> None: + """End-to-end: real PlatformIO ``FileDownloader`` against a local server + that fails twice then succeeds. Exercises the real + requests/urllib3/http.client stack for both failure modes: + + - ``drop``: TCP close mid-request → ``RemoteDisconnected`` → caught as + ``OSError`` by the retry patch (the CI failure path). + - ``502``: HTTP error response → ``PackageException`` (the original path). + """ + runner.patch_file_downloader() + from platformio.package.download import FileDownloader + + with ( + _flaky_http_server(fail_first_n=2, fail_mode=fail_mode) as (port, state), + patch("time.sleep"), + ): + fd = FileDownloader(f"http://127.0.0.1:{port}/payload.bin") + fd.set_destination(str(tmp_path / "out.bin")) + fd.start(with_progress=False, silent=True) + + assert state["hits"] == 3 # 2 failures + 1 success + assert (tmp_path / "out.bin").read_bytes() == b"esphome-test-payload" + + def _filter_through_redirect(line: str) -> str: """Write a line through RedirectText with FILTER_PLATFORMIO_LINES and return what passes.""" import io