diff --git a/Doxyfile b/Doxyfile index e2d892eb44..dc785e3a8f 100644 --- a/Doxyfile +++ b/Doxyfile @@ -48,7 +48,7 @@ PROJECT_NAME = ESPHome # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 2025.8.0b2 +PROJECT_NUMBER = 2025.8.0b3 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/esphome/__main__.py b/esphome/__main__.py index 7cc8296e7e..8e8fc7d5d9 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -476,7 +476,7 @@ def show_logs(config: ConfigType, args: ArgsProtocol, devices: list[str]) -> int from esphome.components.api.client import run_logs return run_logs(config, addresses_to_use) - if get_port_type(port) == "MQTT" and "mqtt" in config: + if get_port_type(port) in ("NETWORK", "MQTT") and "mqtt" in config: from esphome import mqtt return mqtt.show_logs( diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index 347f60c28f..d2cbdeb984 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -133,7 +133,7 @@ void BluetoothConnection::loop() { // Check if we should disable the loop // - For V3_WITH_CACHE: Services are never sent, disable after INIT state - // - For other connections: Disable only after service discovery is complete + // - For V3_WITHOUT_CACHE: Disable only after service discovery is complete // (send_service_ == DONE_SENDING_SERVICES, which is only set after services are sent) if (this->state_ != espbt::ClientState::INIT && (this->connection_type_ == espbt::ConnectionType::V3_WITH_CACHE || this->send_service_ == DONE_SENDING_SERVICES)) { @@ -160,10 +160,7 @@ void BluetoothConnection::send_service_for_discovery_() { if (this->send_service_ >= this->service_count_) { this->send_service_ = DONE_SENDING_SERVICES; this->proxy_->send_gatt_services_done(this->address_); - if (this->connection_type_ == espbt::ConnectionType::V3_WITH_CACHE || - this->connection_type_ == espbt::ConnectionType::V3_WITHOUT_CACHE) { - this->release_services(); - } + this->release_services(); return; } diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index c219b8851a..ac236f4eb3 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -824,8 +824,9 @@ async def to_code(config): cg.set_cpp_standard("gnu++20") cg.add_build_flag("-DUSE_ESP32") cg.add_define("ESPHOME_BOARD", config[CONF_BOARD]) - cg.add_build_flag(f"-DUSE_ESP32_VARIANT_{config[CONF_VARIANT]}") - cg.add_define("ESPHOME_VARIANT", VARIANT_FRIENDLY[config[CONF_VARIANT]]) + variant = config[CONF_VARIANT] + cg.add_build_flag(f"-DUSE_ESP32_VARIANT_{variant}") + cg.add_define("ESPHOME_VARIANT", VARIANT_FRIENDLY[variant]) cg.add_define(ThreadModel.MULTI_ATOMICS) cg.add_platformio_option("lib_ldf_mode", "off") @@ -859,6 +860,7 @@ async def to_code(config): cg.add_platformio_option( "platform_packages", ["espressif/toolchain-esp32ulp@2.35.0-20220830"] ) + add_idf_sdkconfig_option(f"CONFIG_IDF_TARGET_{variant}", True) add_idf_sdkconfig_option( f"CONFIG_ESPTOOLPY_FLASHSIZE_{config[CONF_FLASH_SIZE]}", True ) diff --git a/esphome/components/esp32_ble/__init__.py b/esphome/components/esp32_ble/__init__.py index c758b3ef8f..2edd69c6c0 100644 --- a/esphome/components/esp32_ble/__init__.py +++ b/esphome/components/esp32_ble/__init__.py @@ -294,6 +294,7 @@ async def to_code(config): if config[CONF_ADVERTISING]: cg.add_define("USE_ESP32_BLE_ADVERTISING") + cg.add_define("USE_ESP32_BLE_UUID") @automation.register_condition("ble.enabled", BLEEnabledCondition, cv.Schema({})) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index d1ee7af4ea..e22d43c0cc 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -306,7 +306,7 @@ void ESP32BLE::loop() { case BLEEvent::GATTS: { esp_gatts_cb_event_t event = ble_event->event_.gatts.gatts_event; esp_gatt_if_t gatts_if = ble_event->event_.gatts.gatts_if; - esp_ble_gatts_cb_param_t *param = ble_event->event_.gatts.gatts_param; + esp_ble_gatts_cb_param_t *param = &ble_event->event_.gatts.gatts_param; ESP_LOGV(TAG, "gatts_event [esp_gatt_if: %d] - %d", gatts_if, event); for (auto *gatts_handler : this->gatts_event_handlers_) { gatts_handler->gatts_event_handler(event, gatts_if, param); @@ -316,7 +316,7 @@ void ESP32BLE::loop() { case BLEEvent::GATTC: { esp_gattc_cb_event_t event = ble_event->event_.gattc.gattc_event; esp_gatt_if_t gattc_if = ble_event->event_.gattc.gattc_if; - esp_ble_gattc_cb_param_t *param = ble_event->event_.gattc.gattc_param; + esp_ble_gattc_cb_param_t *param = &ble_event->event_.gattc.gattc_param; ESP_LOGV(TAG, "gattc_event [esp_gatt_if: %d] - %d", gattc_if, event); for (auto *gattc_handler : this->gattc_event_handlers_) { gattc_handler->gattc_event_handler(event, gattc_if, param); diff --git a/esphome/components/esp32_ble/ble_event.h b/esphome/components/esp32_ble/ble_event.h index 884fc9ba65..299fd7705f 100644 --- a/esphome/components/esp32_ble/ble_event.h +++ b/esphome/components/esp32_ble/ble_event.h @@ -3,8 +3,7 @@ #ifdef USE_ESP32 #include // for offsetof -#include - +#include // for memcpy #include #include #include @@ -62,10 +61,24 @@ static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.rssi) == sizeof(es static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.remote_addr) == sizeof(esp_bt_status_t) + sizeof(int8_t), "remote_addr must follow rssi in read_rssi_cmpl"); +// Param struct sizes on ESP32 +static constexpr size_t GATTC_PARAM_SIZE = 28; +static constexpr size_t GATTS_PARAM_SIZE = 32; + +// Maximum size for inline storage of data +// GATTC: 80 - 28 (param) - 8 (other fields) = 44 bytes for data +// GATTS: 80 - 32 (param) - 8 (other fields) = 40 bytes for data +static constexpr size_t GATTC_INLINE_DATA_SIZE = 44; +static constexpr size_t GATTS_INLINE_DATA_SIZE = 40; + +// Verify param struct sizes +static_assert(sizeof(esp_ble_gattc_cb_param_t) == GATTC_PARAM_SIZE, "GATTC param size unexpected"); +static_assert(sizeof(esp_ble_gatts_cb_param_t) == GATTS_PARAM_SIZE, "GATTS param size unexpected"); + // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). // This class stores each event with minimal memory usage. -// GAP events (99% of traffic) don't have the vector overhead. -// GATTC/GATTS events use heap allocation for their param and data. +// GAP events (99% of traffic) don't have the heap allocation overhead. +// GATTC/GATTS events use heap allocation for their param and inline storage for small data. // // Event flow: // 1. ESP-IDF BLE stack calls our static handlers in the BLE task context @@ -112,21 +125,21 @@ class BLEEvent { this->init_gap_data_(e, p); } - // Constructor for GATTC events - uses heap allocation - // IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization. - // The param pointer from ESP-IDF is only valid during the callback execution. - // Since BLE events are processed asynchronously in the main loop, we must create - // our own copy to ensure the data remains valid until the event is processed. + // Constructor for GATTC events - param stored inline, data may use heap + // IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF + // is only valid during the callback execution. Since BLE events are processed + // asynchronously in the main loop, we store our own copy inline to ensure + // the data remains valid until the event is processed. BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { this->type_ = GATTC; this->init_gattc_data_(e, i, p); } - // Constructor for GATTS events - uses heap allocation - // IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization. - // The param pointer from ESP-IDF is only valid during the callback execution. - // Since BLE events are processed asynchronously in the main loop, we must create - // our own copy to ensure the data remains valid until the event is processed. + // Constructor for GATTS events - param stored inline, data may use heap + // IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF + // is only valid during the callback execution. Since BLE events are processed + // asynchronously in the main loop, we store our own copy inline to ensure + // the data remains valid until the event is processed. BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { this->type_ = GATTS; this->init_gatts_data_(e, i, p); @@ -136,25 +149,32 @@ class BLEEvent { ~BLEEvent() { this->release(); } // Default constructor for pre-allocation in pool - BLEEvent() : type_(GAP) {} + BLEEvent() : event_{}, type_(GAP) {} // Invoked on return to EventPool - clean up any heap-allocated data void release() { - if (this->type_ == GAP) { - return; - } - if (this->type_ == GATTC) { - delete this->event_.gattc.gattc_param; - delete this->event_.gattc.data; - this->event_.gattc.gattc_param = nullptr; - this->event_.gattc.data = nullptr; - return; - } - if (this->type_ == GATTS) { - delete this->event_.gatts.gatts_param; - delete this->event_.gatts.data; - this->event_.gatts.gatts_param = nullptr; - this->event_.gatts.data = nullptr; + switch (this->type_) { + case GAP: + // GAP events don't have heap allocations + break; + case GATTC: + // Param is now stored inline, only delete heap data if it was heap-allocated + if (!this->event_.gattc.is_inline && this->event_.gattc.data.heap_data != nullptr) { + delete[] this->event_.gattc.data.heap_data; + } + // Clear critical fields to prevent issues if type changes + this->event_.gattc.is_inline = false; + this->event_.gattc.data.heap_data = nullptr; + break; + case GATTS: + // Param is now stored inline, only delete heap data if it was heap-allocated + if (!this->event_.gatts.is_inline && this->event_.gatts.data.heap_data != nullptr) { + delete[] this->event_.gatts.data.heap_data; + } + // Clear critical fields to prevent issues if type changes + this->event_.gatts.is_inline = false; + this->event_.gatts.data.heap_data = nullptr; + break; } } @@ -206,20 +226,30 @@ class BLEEvent { // NOLINTNEXTLINE(readability-identifier-naming) struct gattc_event { - esp_gattc_cb_event_t gattc_event; - esp_gatt_if_t gattc_if; - esp_ble_gattc_cb_param_t *gattc_param; // Heap-allocated - std::vector *data; // Heap-allocated - } gattc; // 16 bytes (pointers only) + esp_ble_gattc_cb_param_t gattc_param; // Stored inline (28 bytes) + esp_gattc_cb_event_t gattc_event; // 4 bytes + union { + uint8_t *heap_data; // 4 bytes when heap-allocated + uint8_t inline_data[GATTC_INLINE_DATA_SIZE]; // 44 bytes when stored inline + } data; // 44 bytes total + uint16_t data_len; // 2 bytes + esp_gatt_if_t gattc_if; // 1 byte + bool is_inline; // 1 byte - true when data is stored inline + } gattc; // Total: 80 bytes // NOLINTNEXTLINE(readability-identifier-naming) struct gatts_event { - esp_gatts_cb_event_t gatts_event; - esp_gatt_if_t gatts_if; - esp_ble_gatts_cb_param_t *gatts_param; // Heap-allocated - std::vector *data; // Heap-allocated - } gatts; // 16 bytes (pointers only) - } event_; // 80 bytes + esp_ble_gatts_cb_param_t gatts_param; // Stored inline (32 bytes) + esp_gatts_cb_event_t gatts_event; // 4 bytes + union { + uint8_t *heap_data; // 4 bytes when heap-allocated + uint8_t inline_data[GATTS_INLINE_DATA_SIZE]; // 40 bytes when stored inline + } data; // 40 bytes total + uint16_t data_len; // 2 bytes + esp_gatt_if_t gatts_if; // 1 byte + bool is_inline; // 1 byte - true when data is stored inline + } gatts; // Total: 80 bytes + } event_; // 80 bytes ble_event_t type_; @@ -233,6 +263,29 @@ class BLEEvent { const esp_ble_sec_t &security() const { return event_.gap.security; } private: + // Helper to copy data with inline storage optimization + template + void copy_data_with_inline_storage_(EventStruct &event, const uint8_t *src_data, uint16_t len, + uint8_t **param_value_ptr) { + event.data_len = len; + if (len > 0) { + if (len <= InlineSize) { + event.is_inline = true; + memcpy(event.data.inline_data, src_data, len); + *param_value_ptr = event.data.inline_data; + } else { + event.is_inline = false; + event.data.heap_data = new uint8_t[len]; + memcpy(event.data.heap_data, src_data, len); + *param_value_ptr = event.data.heap_data; + } + } else { + event.is_inline = false; + event.data.heap_data = nullptr; + *param_value_ptr = nullptr; + } + } + // Initialize GAP event data void init_gap_data_(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) { this->event_.gap.gap_event = e; @@ -317,35 +370,38 @@ class BLEEvent { this->event_.gattc.gattc_if = i; if (p == nullptr) { - this->event_.gattc.gattc_param = nullptr; - this->event_.gattc.data = nullptr; + // Zero out the param struct when null + memset(&this->event_.gattc.gattc_param, 0, sizeof(this->event_.gattc.gattc_param)); + this->event_.gattc.is_inline = false; + this->event_.gattc.data.heap_data = nullptr; + this->event_.gattc.data_len = 0; return; // Invalid event, but we can't log in header file } - // Heap-allocate param and data - // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) - // while GAP events (99%) are stored inline to minimize memory usage - // IMPORTANT: This heap allocation provides clear ownership semantics: - // - The BLEEvent owns the allocated memory for its lifetime - // - The data remains valid from the BLE callback context until processed in the main loop - // - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory - this->event_.gattc.gattc_param = new esp_ble_gattc_cb_param_t(*p); + // Copy param struct inline (no heap allocation!) + // GATTC/GATTS events are rare (<1% of events) but we can still store them inline + // along with small data payloads, eliminating all heap allocations for typical BLE operations + // CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer + // is only valid during the callback and will be reused/freed after we return + this->event_.gattc.gattc_param = *p; // Copy data for events that need it // The param struct contains pointers (e.g., notify.value) that point to temporary buffers. // We must copy this data to ensure it remains valid when the event is processed later. switch (e) { case ESP_GATTC_NOTIFY_EVT: - this->event_.gattc.data = new std::vector(p->notify.value, p->notify.value + p->notify.value_len); - this->event_.gattc.gattc_param->notify.value = this->event_.gattc.data->data(); + copy_data_with_inline_storage_event_.gattc), GATTC_INLINE_DATA_SIZE>( + this->event_.gattc, p->notify.value, p->notify.value_len, &this->event_.gattc.gattc_param.notify.value); break; case ESP_GATTC_READ_CHAR_EVT: case ESP_GATTC_READ_DESCR_EVT: - this->event_.gattc.data = new std::vector(p->read.value, p->read.value + p->read.value_len); - this->event_.gattc.gattc_param->read.value = this->event_.gattc.data->data(); + copy_data_with_inline_storage_event_.gattc), GATTC_INLINE_DATA_SIZE>( + this->event_.gattc, p->read.value, p->read.value_len, &this->event_.gattc.gattc_param.read.value); break; default: - this->event_.gattc.data = nullptr; + this->event_.gattc.is_inline = false; + this->event_.gattc.data.heap_data = nullptr; + this->event_.gattc.data_len = 0; break; } } @@ -356,30 +412,33 @@ class BLEEvent { this->event_.gatts.gatts_if = i; if (p == nullptr) { - this->event_.gatts.gatts_param = nullptr; - this->event_.gatts.data = nullptr; + // Zero out the param struct when null + memset(&this->event_.gatts.gatts_param, 0, sizeof(this->event_.gatts.gatts_param)); + this->event_.gatts.is_inline = false; + this->event_.gatts.data.heap_data = nullptr; + this->event_.gatts.data_len = 0; return; // Invalid event, but we can't log in header file } - // Heap-allocate param and data - // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) - // while GAP events (99%) are stored inline to minimize memory usage - // IMPORTANT: This heap allocation provides clear ownership semantics: - // - The BLEEvent owns the allocated memory for its lifetime - // - The data remains valid from the BLE callback context until processed in the main loop - // - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory - this->event_.gatts.gatts_param = new esp_ble_gatts_cb_param_t(*p); + // Copy param struct inline (no heap allocation!) + // GATTC/GATTS events are rare (<1% of events) but we can still store them inline + // along with small data payloads, eliminating all heap allocations for typical BLE operations + // CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer + // is only valid during the callback and will be reused/freed after we return + this->event_.gatts.gatts_param = *p; // Copy data for events that need it // The param struct contains pointers (e.g., write.value) that point to temporary buffers. // We must copy this data to ensure it remains valid when the event is processed later. switch (e) { case ESP_GATTS_WRITE_EVT: - this->event_.gatts.data = new std::vector(p->write.value, p->write.value + p->write.len); - this->event_.gatts.gatts_param->write.value = this->event_.gatts.data->data(); + copy_data_with_inline_storage_event_.gatts), GATTS_INLINE_DATA_SIZE>( + this->event_.gatts, p->write.value, p->write.len, &this->event_.gatts.gatts_param.write.value); break; default: - this->event_.gatts.data = nullptr; + this->event_.gatts.is_inline = false; + this->event_.gatts.data.heap_data = nullptr; + this->event_.gatts.data_len = 0; break; } } @@ -389,6 +448,15 @@ class BLEEvent { // The gap member in the union should be 80 bytes (including the gap_event enum) static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)) <= 80, "gap_event struct has grown beyond 80 bytes"); +// Verify GATTC and GATTS structs don't exceed GAP struct size +// This ensures the union size is determined by GAP (the most common event type) +static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gattc)) <= + sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)), + "gattc_event struct exceeds gap_event size - union size would increase"); +static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gatts)) <= + sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)), + "gatts_event struct exceeds gap_event size - union size would increase"); + // Verify esp_ble_sec_t fits within our union static_assert(sizeof(esp_ble_sec_t) <= 73, "esp_ble_sec_t is larger than BLEScanResult"); diff --git a/esphome/components/nextion/nextion.cpp b/esphome/components/nextion/nextion.cpp index 133bd2947c..b348bc9920 100644 --- a/esphome/components/nextion/nextion.cpp +++ b/esphome/components/nextion/nextion.cpp @@ -764,7 +764,8 @@ void Nextion::process_nextion_commands_() { variable_name = to_process.substr(0, index); ++index; - text_value = to_process.substr(index); + // Get variable value without terminating NUL byte. Length check above ensures substr len >= 0. + text_value = to_process.substr(index, to_process_length - index - 1); ESP_LOGN(TAG, "Text sensor: %s='%s'", variable_name.c_str(), text_value.c_str()); diff --git a/esphome/components/senseair/senseair.cpp b/esphome/components/senseair/senseair.cpp index e58ee157f7..84520d407d 100644 --- a/esphome/components/senseair/senseair.cpp +++ b/esphome/components/senseair/senseair.cpp @@ -53,10 +53,14 @@ void SenseAirComponent::update() { this->status_clear_warning(); const uint8_t length = response[2]; - const uint16_t status = (uint16_t(response[3]) << 8) | response[4]; - const int16_t ppm = int16_t((response[length + 1] << 8) | response[length + 2]); + const uint16_t status = encode_uint16(response[3], response[4]); + const uint16_t ppm = encode_uint16(response[length + 1], response[length + 2]); - ESP_LOGD(TAG, "SenseAir Received CO₂=%dppm Status=0x%02X", ppm, status); + ESP_LOGD(TAG, "SenseAir Received CO₂=%uppm Status=0x%02X", ppm, status); + if (ppm == 0 && (status & SenseAirStatus::OUT_OF_RANGE_ERROR) != 0) { + ESP_LOGD(TAG, "Discarding 0 ppm reading with out-of-range status."); + return; + } if (this->co2_sensor_ != nullptr) this->co2_sensor_->publish_state(ppm); } diff --git a/esphome/components/senseair/senseair.h b/esphome/components/senseair/senseair.h index 9f939d5b07..5b66860f1a 100644 --- a/esphome/components/senseair/senseair.h +++ b/esphome/components/senseair/senseair.h @@ -8,6 +8,17 @@ namespace esphome { namespace senseair { +enum SenseAirStatus : uint8_t { + FATAL_ERROR = 1 << 0, + OFFSET_ERROR = 1 << 1, + ALGORITHM_ERROR = 1 << 2, + OUTPUT_ERROR = 1 << 3, + SELF_DIAGNOSTIC_ERROR = 1 << 4, + OUT_OF_RANGE_ERROR = 1 << 5, + MEMORY_ERROR = 1 << 6, + RESERVED = 1 << 7 +}; + class SenseAirComponent : public PollingComponent, public uart::UARTDevice { public: void set_co2_sensor(sensor::Sensor *co2_sensor) { co2_sensor_ = co2_sensor; } diff --git a/esphome/components/web_server/web_server.cpp b/esphome/components/web_server/web_server.cpp index 92c5961f87..399b8785ae 100644 --- a/esphome/components/web_server/web_server.cpp +++ b/esphome/components/web_server/web_server.cpp @@ -813,7 +813,7 @@ std::string WebServer::cover_state_json_generator(WebServer *web_server, void *s return web_server->cover_json((cover::Cover *) (source), DETAIL_STATE); } std::string WebServer::cover_all_json_generator(WebServer *web_server, void *source) { - return web_server->cover_json((cover::Cover *) (source), DETAIL_STATE); + return web_server->cover_json((cover::Cover *) (source), DETAIL_ALL); } std::string WebServer::cover_json(cover::Cover *obj, JsonDetail start_config) { return json::build_json([this, obj, start_config](JsonObject root) { diff --git a/esphome/components/wifi/__init__.py b/esphome/components/wifi/__init__.py index ac002eac53..4013e8f400 100644 --- a/esphome/components/wifi/__init__.py +++ b/esphome/components/wifi/__init__.py @@ -375,11 +375,16 @@ async def to_code(config): var = cg.new_Pvariable(config[CONF_ID]) cg.add(var.set_use_address(config[CONF_USE_ADDRESS])) + # Track if any network uses Enterprise authentication + has_eap = False + def add_sta(ap, network): ip_config = network.get(CONF_MANUAL_IP, config.get(CONF_MANUAL_IP)) cg.add(var.add_sta(wifi_network(network, ap, ip_config))) for network in config.get(CONF_NETWORKS, []): + if CONF_EAP in network: + has_eap = True cg.with_local_variable(network[CONF_ID], WiFiAP(), add_sta, network) if CONF_AP in config: @@ -396,6 +401,10 @@ async def to_code(config): add_idf_sdkconfig_option("CONFIG_ESP_WIFI_SOFTAP_SUPPORT", False) add_idf_sdkconfig_option("CONFIG_LWIP_DHCPS", False) + # Disable Enterprise WiFi support if no EAP is configured + if CORE.is_esp32 and CORE.using_esp_idf and not has_eap: + add_idf_sdkconfig_option("CONFIG_ESP_WIFI_ENTERPRISE_SUPPORT", False) + cg.add(var.set_reboot_timeout(config[CONF_REBOOT_TIMEOUT])) cg.add(var.set_power_save_mode(config[CONF_POWER_SAVE_MODE])) cg.add(var.set_fast_connect(config[CONF_FAST_CONNECT])) diff --git a/esphome/const.py b/esphome/const.py index 3b5365854d..bf0f2eaf8a 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -4,7 +4,7 @@ from enum import Enum from esphome.enum import StrEnum -__version__ = "2025.8.0b2" +__version__ = "2025.8.0b3" ALLOWED_NAME_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789-_" VALID_SUBSTITUTIONS_CHARACTERS = ( diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index 9df5da1c78..8a9630735e 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -803,6 +803,10 @@ class EsphomeCore: raise TypeError( f"Library {library} must be instance of Library, not {type(library)}" ) + + if not library.name: + raise ValueError(f"The library for {library.repository} must have a name") + short_name = ( library.name if "/" not in library.name else library.name.split("/")[-1] ) diff --git a/esphome/core/scheduler.cpp b/esphome/core/scheduler.cpp index 6269a66543..c3ade260ac 100644 --- a/esphome/core/scheduler.cpp +++ b/esphome/core/scheduler.cpp @@ -82,7 +82,13 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type item->set_name(name_cstr, !is_static_string); item->type = type; item->callback = std::move(func); + // Initialize remove to false (though it should already be from constructor) + // Not using mark_item_removed_ helper since we're setting to false, not true +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + item->remove.store(false, std::memory_order_relaxed); +#else item->remove = false; +#endif item->is_retry = is_retry; #ifndef ESPHOME_THREAD_SINGLE @@ -398,6 +404,31 @@ void HOT Scheduler::call(uint32_t now) { this->pop_raw_(); continue; } + + // Check if item is marked for removal + // This handles two cases: + // 1. Item was marked for removal after cleanup_() but before we got here + // 2. Item is marked for removal but wasn't at the front of the heap during cleanup_() +#ifdef ESPHOME_THREAD_MULTI_NO_ATOMICS + // Multi-threaded platforms without atomics: must take lock to safely read remove flag + { + LockGuard guard{this->lock_}; + if (is_item_removed_(item.get())) { + this->pop_raw_(); + this->to_remove_--; + continue; + } + } +#else + // Single-threaded or multi-threaded with atomics: can check without lock + if (is_item_removed_(item.get())) { + LockGuard guard{this->lock_}; + this->pop_raw_(); + this->to_remove_--; + continue; + } +#endif + #ifdef ESPHOME_DEBUG_SCHEDULER const char *item_name = item->get_name(); ESP_LOGV(TAG, "Running %s '%s/%s' with interval=%" PRIu32 " next_execution=%" PRIu64 " (now=%" PRIu64 ")", @@ -518,7 +549,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c if (type == SchedulerItem::TIMEOUT) { for (auto &item : this->defer_queue_) { if (this->matches_item_(item, component, name_cstr, type, match_retry)) { - item->remove = true; + this->mark_item_removed_(item.get()); total_cancelled++; } } @@ -528,7 +559,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c // Cancel items in the main heap for (auto &item : this->items_) { if (this->matches_item_(item, component, name_cstr, type, match_retry)) { - item->remove = true; + this->mark_item_removed_(item.get()); total_cancelled++; this->to_remove_++; // Track removals for heap items } @@ -537,7 +568,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c // Cancel items in to_add_ for (auto &item : this->to_add_) { if (this->matches_item_(item, component, name_cstr, type, match_retry)) { - item->remove = true; + this->mark_item_removed_(item.get()); total_cancelled++; // Don't track removals for to_add_ items } diff --git a/esphome/core/scheduler.h b/esphome/core/scheduler.h index a6092e1b1e..c73bd55d5d 100644 --- a/esphome/core/scheduler.h +++ b/esphome/core/scheduler.h @@ -97,22 +97,42 @@ class Scheduler { std::function callback; - // Bit-packed fields to minimize padding +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + // Multi-threaded with atomics: use atomic for lock-free access + // Place atomic separately since it can't be packed with bit fields + std::atomic remove{false}; + + // Bit-packed fields (3 bits used, 5 bits padding in 1 byte) + enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; + bool name_is_dynamic : 1; // True if name was dynamically allocated (needs delete[]) + bool is_retry : 1; // True if this is a retry timeout + // 5 bits padding +#else + // Single-threaded or multi-threaded without atomics: can pack all fields together + // Bit-packed fields (4 bits used, 4 bits padding in 1 byte) enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; bool remove : 1; bool name_is_dynamic : 1; // True if name was dynamically allocated (needs delete[]) bool is_retry : 1; // True if this is a retry timeout - // 4 bits padding + // 4 bits padding +#endif // Constructor SchedulerItem() : component(nullptr), interval(0), next_execution_(0), +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + // remove is initialized in the member declaration as std::atomic{false} + type(TIMEOUT), + name_is_dynamic(false), + is_retry(false) { +#else type(TIMEOUT), remove(false), name_is_dynamic(false), is_retry(false) { +#endif name_.static_name = nullptr; } @@ -219,6 +239,37 @@ class Scheduler { return item->remove || (item->component != nullptr && item->component->is_failed()); } + // Helper to check if item is marked for removal (platform-specific) + // Returns true if item should be skipped, handles platform-specific synchronization + // For ESPHOME_THREAD_MULTI_NO_ATOMICS platforms, the caller must hold the scheduler lock before calling this + // function. + bool is_item_removed_(SchedulerItem *item) const { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + // Multi-threaded with atomics: use atomic load for lock-free access + return item->remove.load(std::memory_order_acquire); +#else + // Single-threaded (ESPHOME_THREAD_SINGLE) or + // multi-threaded without atomics (ESPHOME_THREAD_MULTI_NO_ATOMICS): direct read + // For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller MUST hold lock! + return item->remove; +#endif + } + + // Helper to mark item for removal (platform-specific) + // For ESPHOME_THREAD_MULTI_NO_ATOMICS platforms, the caller must hold the scheduler lock before calling this + // function. + void mark_item_removed_(SchedulerItem *item) { +#ifdef ESPHOME_THREAD_MULTI_ATOMICS + // Multi-threaded with atomics: use atomic store + item->remove.store(true, std::memory_order_release); +#else + // Single-threaded (ESPHOME_THREAD_SINGLE) or + // multi-threaded without atomics (ESPHOME_THREAD_MULTI_NO_ATOMICS): direct write + // For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller MUST hold lock! + item->remove = true; +#endif + } + // Template helper to check if any item in a container matches our criteria template bool has_cancelled_timeout_in_container_(const Container &container, Component *component, const char *name_cstr, diff --git a/esphome/writer.py b/esphome/writer.py index b5c834722a..4b25a25f7e 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -80,13 +80,16 @@ def replace_file_content(text, pattern, repl): return content_new, count -def storage_should_clean(old: StorageJSON, new: StorageJSON) -> bool: +def storage_should_clean(old: StorageJSON | None, new: StorageJSON) -> bool: if old is None: return True if old.src_version != new.src_version: return True - return old.build_path != new.build_path + if old.build_path != new.build_path: + return True + # Check if any components have been removed + return bool(old.loaded_integrations - new.loaded_integrations) def storage_should_update_cmake_cache(old: StorageJSON, new: StorageJSON) -> bool: @@ -100,7 +103,7 @@ def storage_should_update_cmake_cache(old: StorageJSON, new: StorageJSON) -> boo return False -def update_storage_json(): +def update_storage_json() -> None: path = storage_path() old = StorageJSON.load(path) new = StorageJSON.from_esphome_core(CORE, old) @@ -108,7 +111,14 @@ def update_storage_json(): return if storage_should_clean(old, new): - _LOGGER.info("Core config, version changed, cleaning build files...") + if old is not None and old.loaded_integrations - new.loaded_integrations: + removed = old.loaded_integrations - new.loaded_integrations + _LOGGER.info( + "Components removed (%s), cleaning build files...", + ", ".join(sorted(removed)), + ) + else: + _LOGGER.info("Core config or version changed, cleaning build files...") clean_build() elif storage_should_update_cmake_cache(old, new): _LOGGER.info("Integrations changed, cleaning cmake cache...") diff --git a/tests/integration/fixtures/scheduler_removed_item_race.yaml b/tests/integration/fixtures/scheduler_removed_item_race.yaml new file mode 100644 index 0000000000..2f8a7fb987 --- /dev/null +++ b/tests/integration/fixtures/scheduler_removed_item_race.yaml @@ -0,0 +1,139 @@ +esphome: + name: scheduler-removed-item-race + +host: + +api: + services: + - service: run_test + then: + - script.execute: run_test_script + +logger: + level: DEBUG + +globals: + - id: test_passed + type: bool + initial_value: 'true' + - id: removed_item_executed + type: int + initial_value: '0' + - id: normal_item_executed + type: int + initial_value: '0' + +sensor: + - platform: template + id: test_sensor + name: "Test Sensor" + update_interval: never + lambda: return 0.0; + +script: + - id: run_test_script + then: + - logger.log: "=== Starting Removed Item Race Test ===" + + # This test creates a scenario where: + # 1. First item in heap is NOT cancelled (cleanup stops immediately) + # 2. Items behind it ARE cancelled (remain in heap after cleanup) + # 3. All items execute at the same time, including cancelled ones + + - lambda: |- + // The key to hitting the race: + // 1. Add items in a specific order to control heap structure + // 2. Cancel ONLY items that won't be at the front + // 3. Ensure the first item stays non-cancelled so cleanup_() stops immediately + + // Schedule all items to execute at the SAME time (1ms from now) + // Using 1ms instead of 0 to avoid defer queue on multi-core platforms + // This ensures they'll all be ready together and go through the heap + const uint32_t exec_time = 1; + + // CRITICAL: Add a non-cancellable item FIRST + // This will be at the front of the heap and block cleanup_() + App.scheduler.set_timeout(id(test_sensor), "blocker", exec_time, []() { + ESP_LOGD("test", "Blocker timeout executed (expected) - was at front of heap"); + id(normal_item_executed)++; + }); + + // Now add items that we WILL cancel + // These will be behind the blocker in the heap + App.scheduler.set_timeout(id(test_sensor), "cancel_1", exec_time, []() { + ESP_LOGE("test", "RACE: Cancelled timeout 1 executed after being cancelled!"); + id(removed_item_executed)++; + id(test_passed) = false; + }); + + App.scheduler.set_timeout(id(test_sensor), "cancel_2", exec_time, []() { + ESP_LOGE("test", "RACE: Cancelled timeout 2 executed after being cancelled!"); + id(removed_item_executed)++; + id(test_passed) = false; + }); + + App.scheduler.set_timeout(id(test_sensor), "cancel_3", exec_time, []() { + ESP_LOGE("test", "RACE: Cancelled timeout 3 executed after being cancelled!"); + id(removed_item_executed)++; + id(test_passed) = false; + }); + + // Add some more normal items + App.scheduler.set_timeout(id(test_sensor), "normal_1", exec_time, []() { + ESP_LOGD("test", "Normal timeout 1 executed (expected)"); + id(normal_item_executed)++; + }); + + App.scheduler.set_timeout(id(test_sensor), "normal_2", exec_time, []() { + ESP_LOGD("test", "Normal timeout 2 executed (expected)"); + id(normal_item_executed)++; + }); + + App.scheduler.set_timeout(id(test_sensor), "normal_3", exec_time, []() { + ESP_LOGD("test", "Normal timeout 3 executed (expected)"); + id(normal_item_executed)++; + }); + + // Force items into the heap before cancelling + App.scheduler.process_to_add(); + + // NOW cancel the items - they're behind "blocker" in the heap + // When cleanup_() runs, it will see "blocker" (not removed) at the front + // and stop immediately, leaving cancel_1, cancel_2, cancel_3 in the heap + bool c1 = App.scheduler.cancel_timeout(id(test_sensor), "cancel_1"); + bool c2 = App.scheduler.cancel_timeout(id(test_sensor), "cancel_2"); + bool c3 = App.scheduler.cancel_timeout(id(test_sensor), "cancel_3"); + + ESP_LOGD("test", "Cancelled items (behind blocker): %s, %s, %s", + c1 ? "true" : "false", + c2 ? "true" : "false", + c3 ? "true" : "false"); + + // The heap now has: + // - "blocker" at front (not cancelled) + // - cancelled items behind it (marked remove=true but still in heap) + // - When all execute at once, cleanup_() stops at "blocker" + // - The loop then executes ALL ready items including cancelled ones + + ESP_LOGD("test", "Setup complete. Blocker at front prevents cleanup of cancelled items behind it"); + + # Wait for all timeouts to execute (or not) + - delay: 20ms + + # Check results + - lambda: |- + ESP_LOGI("test", "=== Test Results ==="); + ESP_LOGI("test", "Normal items executed: %d (expected 4)", id(normal_item_executed)); + ESP_LOGI("test", "Removed items executed: %d (expected 0)", id(removed_item_executed)); + + if (id(removed_item_executed) > 0) { + ESP_LOGE("test", "TEST FAILED: %d cancelled items were executed!", id(removed_item_executed)); + id(test_passed) = false; + } else if (id(normal_item_executed) != 4) { + ESP_LOGE("test", "TEST FAILED: Expected 4 normal items, got %d", id(normal_item_executed)); + id(test_passed) = false; + } else { + ESP_LOGI("test", "TEST PASSED: No cancelled items were executed"); + } + + ESP_LOGI("test", "=== Test Complete ==="); diff --git a/tests/integration/test_scheduler_removed_item_race.py b/tests/integration/test_scheduler_removed_item_race.py new file mode 100644 index 0000000000..3e72bacc0d --- /dev/null +++ b/tests/integration/test_scheduler_removed_item_race.py @@ -0,0 +1,102 @@ +"""Test for scheduler race condition where removed items still execute.""" + +import asyncio +import re + +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_scheduler_removed_item_race( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that items marked for removal don't execute. + + This test verifies the fix for a race condition where: + 1. cleanup_() only removes items from the front of the heap + 2. Items in the middle of the heap marked for removal still execute + 3. This causes cancelled timeouts to run when they shouldn't + """ + + loop = asyncio.get_running_loop() + test_complete_future: asyncio.Future[bool] = loop.create_future() + + # Track test results + test_passed = False + removed_executed = 0 + normal_executed = 0 + + # Patterns to match + race_pattern = re.compile(r"RACE: .* executed after being cancelled!") + passed_pattern = re.compile(r"TEST PASSED") + failed_pattern = re.compile(r"TEST FAILED") + complete_pattern = re.compile(r"=== Test Complete ===") + normal_count_pattern = re.compile(r"Normal items executed: (\d+)") + removed_count_pattern = re.compile(r"Removed items executed: (\d+)") + + def check_output(line: str) -> None: + """Check log output for test results.""" + nonlocal test_passed, removed_executed, normal_executed + + if race_pattern.search(line): + # Race condition detected - a cancelled item executed + test_passed = False + + if passed_pattern.search(line): + test_passed = True + elif failed_pattern.search(line): + test_passed = False + + normal_match = normal_count_pattern.search(line) + if normal_match: + normal_executed = int(normal_match.group(1)) + + removed_match = removed_count_pattern.search(line) + if removed_match: + removed_executed = int(removed_match.group(1)) + + if not test_complete_future.done() and complete_pattern.search(line): + test_complete_future.set_result(True) + + async with ( + run_compiled(yaml_config, line_callback=check_output), + api_client_connected() as client, + ): + # Verify we can connect + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "scheduler-removed-item-race" + + # List services + _, services = await asyncio.wait_for( + client.list_entities_services(), timeout=5.0 + ) + + # Find run_test service + run_test_service = next((s for s in services if s.name == "run_test"), None) + assert run_test_service is not None, "run_test service not found" + + # Execute the test + client.execute_service(run_test_service, {}) + + # Wait for test completion + try: + await asyncio.wait_for(test_complete_future, timeout=5.0) + except TimeoutError: + pytest.fail("Test did not complete within timeout") + + # Verify results + assert test_passed, ( + f"Test failed! Removed items executed: {removed_executed}, " + f"Normal items executed: {normal_executed}" + ) + assert removed_executed == 0, ( + f"Cancelled items should not execute, but {removed_executed} did" + ) + assert normal_executed == 4, ( + f"Expected 4 normal items to execute, got {normal_executed}" + ) diff --git a/tests/unit_tests/test_writer.py b/tests/unit_tests/test_writer.py new file mode 100644 index 0000000000..f47947ff37 --- /dev/null +++ b/tests/unit_tests/test_writer.py @@ -0,0 +1,220 @@ +"""Test writer module functionality.""" + +from collections.abc import Callable +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +from esphome.storage_json import StorageJSON +from esphome.writer import storage_should_clean, update_storage_json + + +@pytest.fixture +def create_storage() -> Callable[..., StorageJSON]: + """Factory fixture to create StorageJSON instances.""" + + def _create( + loaded_integrations: list[str] | None = None, **kwargs: Any + ) -> StorageJSON: + return StorageJSON( + storage_version=kwargs.get("storage_version", 1), + name=kwargs.get("name", "test"), + friendly_name=kwargs.get("friendly_name", "Test Device"), + comment=kwargs.get("comment"), + esphome_version=kwargs.get("esphome_version", "2025.1.0"), + src_version=kwargs.get("src_version", 1), + address=kwargs.get("address", "test.local"), + web_port=kwargs.get("web_port", 80), + target_platform=kwargs.get("target_platform", "ESP32"), + build_path=kwargs.get("build_path", "/build"), + firmware_bin_path=kwargs.get("firmware_bin_path", "/firmware.bin"), + loaded_integrations=set(loaded_integrations or []), + loaded_platforms=kwargs.get("loaded_platforms", set()), + no_mdns=kwargs.get("no_mdns", False), + framework=kwargs.get("framework", "arduino"), + core_platform=kwargs.get("core_platform", "esp32"), + ) + + return _create + + +def test_storage_should_clean_when_old_is_none( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is triggered when old storage is None.""" + new = create_storage(loaded_integrations=["api", "wifi"]) + assert storage_should_clean(None, new) is True + + +def test_storage_should_clean_when_src_version_changes( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is triggered when src_version changes.""" + old = create_storage(loaded_integrations=["api", "wifi"], src_version=1) + new = create_storage(loaded_integrations=["api", "wifi"], src_version=2) + assert storage_should_clean(old, new) is True + + +def test_storage_should_clean_when_build_path_changes( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is triggered when build_path changes.""" + old = create_storage(loaded_integrations=["api", "wifi"], build_path="/build1") + new = create_storage(loaded_integrations=["api", "wifi"], build_path="/build2") + assert storage_should_clean(old, new) is True + + +def test_storage_should_clean_when_component_removed( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is triggered when a component is removed.""" + old = create_storage( + loaded_integrations=["api", "wifi", "bluetooth_proxy", "esp32_ble_tracker"] + ) + new = create_storage(loaded_integrations=["api", "wifi", "esp32_ble_tracker"]) + assert storage_should_clean(old, new) is True + + +def test_storage_should_clean_when_multiple_components_removed( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is triggered when multiple components are removed.""" + old = create_storage( + loaded_integrations=["api", "wifi", "ota", "web_server", "logger"] + ) + new = create_storage(loaded_integrations=["api", "wifi", "logger"]) + assert storage_should_clean(old, new) is True + + +def test_storage_should_not_clean_when_nothing_changes( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is not triggered when nothing changes.""" + old = create_storage(loaded_integrations=["api", "wifi", "logger"]) + new = create_storage(loaded_integrations=["api", "wifi", "logger"]) + assert storage_should_clean(old, new) is False + + +def test_storage_should_not_clean_when_component_added( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is not triggered when a component is only added.""" + old = create_storage(loaded_integrations=["api", "wifi"]) + new = create_storage(loaded_integrations=["api", "wifi", "ota"]) + assert storage_should_clean(old, new) is False + + +def test_storage_should_not_clean_when_other_fields_change( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test that clean is not triggered when non-relevant fields change.""" + old = create_storage( + loaded_integrations=["api", "wifi"], + friendly_name="Old Name", + esphome_version="2024.12.0", + ) + new = create_storage( + loaded_integrations=["api", "wifi"], + friendly_name="New Name", + esphome_version="2025.1.0", + ) + assert storage_should_clean(old, new) is False + + +def test_storage_edge_case_empty_integrations( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test edge case when old has integrations but new has none.""" + old = create_storage(loaded_integrations=["api", "wifi"]) + new = create_storage(loaded_integrations=[]) + assert storage_should_clean(old, new) is True + + +def test_storage_edge_case_from_empty_integrations( + create_storage: Callable[..., StorageJSON], +) -> None: + """Test edge case when old has no integrations but new has some.""" + old = create_storage(loaded_integrations=[]) + new = create_storage(loaded_integrations=["api", "wifi"]) + assert storage_should_clean(old, new) is False + + +@patch("esphome.writer.clean_build") +@patch("esphome.writer.StorageJSON") +@patch("esphome.writer.storage_path") +@patch("esphome.writer.CORE") +def test_update_storage_json_logging_when_old_is_none( + mock_core: MagicMock, + mock_storage_path: MagicMock, + mock_storage_json_class: MagicMock, + mock_clean_build: MagicMock, + create_storage: Callable[..., StorageJSON], + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that update_storage_json doesn't crash when old storage is None. + + This is a regression test for the AttributeError that occurred when + old was None and we tried to access old.loaded_integrations. + """ + # Setup mocks + mock_storage_path.return_value = "/test/path" + mock_storage_json_class.load.return_value = None # Old storage is None + + new_storage = create_storage(loaded_integrations=["api", "wifi"]) + new_storage.save = MagicMock() # Mock the save method + mock_storage_json_class.from_esphome_core.return_value = new_storage + + # Call the function - should not raise AttributeError + with caplog.at_level("INFO"): + update_storage_json() + + # Verify clean_build was called + mock_clean_build.assert_called_once() + + # Verify the correct log message was used (not the component removal message) + assert "Core config or version changed, cleaning build files..." in caplog.text + assert "Components removed" not in caplog.text + + # Verify save was called + new_storage.save.assert_called_once_with("/test/path") + + +@patch("esphome.writer.clean_build") +@patch("esphome.writer.StorageJSON") +@patch("esphome.writer.storage_path") +@patch("esphome.writer.CORE") +def test_update_storage_json_logging_components_removed( + mock_core: MagicMock, + mock_storage_path: MagicMock, + mock_storage_json_class: MagicMock, + mock_clean_build: MagicMock, + create_storage: Callable[..., StorageJSON], + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that update_storage_json logs removed components correctly.""" + # Setup mocks + mock_storage_path.return_value = "/test/path" + + old_storage = create_storage(loaded_integrations=["api", "wifi", "bluetooth_proxy"]) + new_storage = create_storage(loaded_integrations=["api", "wifi"]) + new_storage.save = MagicMock() # Mock the save method + + mock_storage_json_class.load.return_value = old_storage + mock_storage_json_class.from_esphome_core.return_value = new_storage + + # Call the function + with caplog.at_level("INFO"): + update_storage_json() + + # Verify clean_build was called + mock_clean_build.assert_called_once() + + # Verify the correct log message was used with component names + assert ( + "Components removed (bluetooth_proxy), cleaning build files..." in caplog.text + ) + assert "Core config or version changed" not in caplog.text + + # Verify save was called + new_storage.save.assert_called_once_with("/test/path")