From 7ae3a11d6b4b9fc97dc84406c3fc51360fa8c293 Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Thu, 18 Dec 2025 19:42:47 -0600 Subject: [PATCH 1/3] [esp32_ble, esp32_ble_tracker] Fix crash, error messages when `ble.disable` called during boot (#12560) --- esphome/components/esp32_ble/ble.cpp | 16 ++++++++++++---- esphome/components/esp32_ble/ble.h | 12 +++++++++--- .../esp32_ble_tracker/esp32_ble_tracker.cpp | 5 ++++- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/esphome/components/esp32_ble/ble.cpp b/esphome/components/esp32_ble/ble.cpp index a0ed9ee90c..a279f7d2a4 100644 --- a/esphome/components/esp32_ble/ble.cpp +++ b/esphome/components/esp32_ble/ble.cpp @@ -308,13 +308,21 @@ bool ESP32BLE::ble_setup_() { bool ESP32BLE::ble_dismantle_() { esp_err_t err = esp_bluedroid_disable(); if (err != ESP_OK) { - ESP_LOGE(TAG, "esp_bluedroid_disable failed: %d", err); - return false; + // ESP_ERR_INVALID_STATE means Bluedroid is already disabled, which is fine + if (err != ESP_ERR_INVALID_STATE) { + ESP_LOGE(TAG, "esp_bluedroid_disable failed: %d", err); + return false; + } + ESP_LOGD(TAG, "Already disabled"); } err = esp_bluedroid_deinit(); if (err != ESP_OK) { - ESP_LOGE(TAG, "esp_bluedroid_deinit failed: %d", err); - return false; + // ESP_ERR_INVALID_STATE means Bluedroid is already deinitialized, which is fine + if (err != ESP_ERR_INVALID_STATE) { + ESP_LOGE(TAG, "esp_bluedroid_deinit failed: %d", err); + return false; + } + ESP_LOGD(TAG, "Already deinitialized"); } #ifndef CONFIG_ESP_HOSTED_ENABLE_BT_BLUEDROID diff --git a/esphome/components/esp32_ble/ble.h b/esphome/components/esp32_ble/ble.h index 393ec2e911..1999c870f8 100644 --- a/esphome/components/esp32_ble/ble.h +++ b/esphome/components/esp32_ble/ble.h @@ -212,17 +212,23 @@ extern ESP32BLE *global_ble; template class BLEEnabledCondition : public Condition { public: - bool check(const Ts &...x) override { return global_ble->is_active(); } + bool check(const Ts &...x) override { return global_ble != nullptr && global_ble->is_active(); } }; template class BLEEnableAction : public Action { public: - void play(const Ts &...x) override { global_ble->enable(); } + void play(const Ts &...x) override { + if (global_ble != nullptr) + global_ble->enable(); + } }; template class BLEDisableAction : public Action { public: - void play(const Ts &...x) override { global_ble->disable(); } + void play(const Ts &...x) override { + if (global_ble != nullptr) + global_ble->disable(); + } }; } // namespace esphome::esp32_ble diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index d3c5edfb94..45e343c0d2 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -185,7 +185,10 @@ void ESP32BLETracker::ble_before_disabled_event_handler() { this->stop_scan_(); void ESP32BLETracker::stop_scan_() { if (this->scanner_state_ != ScannerState::RUNNING && this->scanner_state_ != ScannerState::FAILED) { - ESP_LOGE(TAG, "Cannot stop scan: %s", this->scanner_state_to_string_(this->scanner_state_)); + // If scanner is already idle, there's nothing to stop - this is not an error + if (this->scanner_state_ != ScannerState::IDLE) { + ESP_LOGE(TAG, "Cannot stop scan: %s", this->scanner_state_to_string_(this->scanner_state_)); + } return; } // Reset timeout state machine when stopping scan From 38afc5149a023f19d97072632af46c6625f16e6a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 18 Dec 2025 16:18:45 -1000 Subject: [PATCH 2/3] [api] Use union for iterators to reduce APIConnection size by ~16 bytes --- esphome/components/api/api_connection.cpp | 54 +++++++++++++++++++---- esphome/components/api/api_connection.h | 32 +++++++++++--- 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 85f4566f3c..83c22ed67e 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #ifdef USE_ESP8266 #include @@ -93,8 +94,7 @@ static const int CAMERA_STOP_STREAM = 5000; return; #endif // USE_DEVICES -APIConnection::APIConnection(std::unique_ptr sock, APIServer *parent) - : parent_(parent), initial_state_iterator_(this), list_entities_iterator_(this) { +APIConnection::APIConnection(std::unique_ptr sock, APIServer *parent) : parent_(parent) { #if defined(USE_API_PLAINTEXT) && defined(USE_API_NOISE) auto &noise_ctx = parent->get_noise_ctx(); if (noise_ctx.has_psk()) { @@ -133,6 +133,7 @@ void APIConnection::start() { } APIConnection::~APIConnection() { + this->destroy_active_iterator_(); #ifdef USE_BLUETOOTH_PROXY if (bluetooth_proxy::global_bluetooth_proxy->get_api_connection() == this) { bluetooth_proxy::global_bluetooth_proxy->unsubscribe_api_connection(this); @@ -145,6 +146,32 @@ APIConnection::~APIConnection() { #endif } +void APIConnection::destroy_active_iterator_() { + switch (this->active_iterator_) { + case ActiveIterator::LIST_ENTITIES: + this->iterator_storage_.list_entities.~ListEntitiesIterator(); + break; + case ActiveIterator::INITIAL_STATE: + this->iterator_storage_.initial_state.~InitialStateIterator(); + break; + case ActiveIterator::NONE: + break; + } + this->active_iterator_ = ActiveIterator::NONE; +} + +void APIConnection::begin_iterator_(ActiveIterator type) { + this->destroy_active_iterator_(); + this->active_iterator_ = type; + if (type == ActiveIterator::LIST_ENTITIES) { + new (&this->iterator_storage_.list_entities) ListEntitiesIterator(this); + this->iterator_storage_.list_entities.begin(); + } else { + new (&this->iterator_storage_.initial_state) InitialStateIterator(this); + this->iterator_storage_.initial_state.begin(); + } +} + void APIConnection::loop() { if (this->flags_.next_close) { // requested a disconnect @@ -187,13 +214,22 @@ void APIConnection::loop() { this->process_batch_(); } - if (!this->list_entities_iterator_.completed()) { - this->process_iterator_batch_(this->list_entities_iterator_); - } else if (!this->initial_state_iterator_.completed()) { - this->process_iterator_batch_(this->initial_state_iterator_); - - // If we've completed initial states, process any remaining and clear the flag - if (this->initial_state_iterator_.completed()) { + if (this->active_iterator_ == ActiveIterator::LIST_ENTITIES) { + if (!this->iterator_storage_.list_entities.completed()) { + this->process_iterator_batch_(this->iterator_storage_.list_entities); + } else { + // List entities completed, check if we need to start initial state + this->destroy_active_iterator_(); + if (this->flags_.state_subscription) { + this->begin_iterator_(ActiveIterator::INITIAL_STATE); + } + } + } else if (this->active_iterator_ == ActiveIterator::INITIAL_STATE) { + if (!this->iterator_storage_.initial_state.completed()) { + this->process_iterator_batch_(this->iterator_storage_.initial_state); + } else { + // Initial state completed + this->destroy_active_iterator_(); // Process any remaining batched messages immediately if (!this->deferred_batch_.empty()) { this->process_batch_(); diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index b50be5d0d4..d7876bf033 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -203,10 +203,14 @@ class APIConnection final : public APIServerConnection { bool send_disconnect_response(const DisconnectRequest &msg) override; bool send_ping_response(const PingRequest &msg) override; bool send_device_info_response(const DeviceInfoRequest &msg) override; - void list_entities(const ListEntitiesRequest &msg) override { this->list_entities_iterator_.begin(); } + void list_entities(const ListEntitiesRequest &msg) override { this->begin_iterator_(ActiveIterator::LIST_ENTITIES); } void subscribe_states(const SubscribeStatesRequest &msg) override { this->flags_.state_subscription = true; - this->initial_state_iterator_.begin(); + // Start initial state iterator only if no iterator is active + // If list_entities is running, we'll start initial_state when it completes + if (this->active_iterator_ == ActiveIterator::NONE) { + this->begin_iterator_(ActiveIterator::INITIAL_STATE); + } } void subscribe_logs(const SubscribeLogsRequest &msg) override { this->flags_.log_subscription = msg.level; @@ -490,10 +494,22 @@ class APIConnection final : public APIServerConnection { std::unique_ptr helper_; APIServer *parent_; - // Group 2: Larger objects (must be 4-byte aligned) - // These contain vectors/pointers internally, so putting them early ensures good alignment - InitialStateIterator initial_state_iterator_; - ListEntitiesIterator list_entities_iterator_; + // Group 2: Iterator union (saves ~16 bytes vs separate iterators) + // These iterators are never active simultaneously - list_entities runs to completion + // before initial_state begins, so we use a union with explicit construction/destruction. + enum class ActiveIterator : uint8_t { NONE, LIST_ENTITIES, INITIAL_STATE }; + + union IteratorUnion { + ListEntitiesIterator list_entities; + InitialStateIterator initial_state; + // Constructor/destructor do nothing - use placement new/explicit destructor + IteratorUnion() {} + ~IteratorUnion() {} + } iterator_storage_; + + // Helper methods for iterator lifecycle management + void destroy_active_iterator_(); + void begin_iterator_(ActiveIterator type); #ifdef USE_CAMERA std::unique_ptr image_reader_; #endif @@ -608,7 +624,9 @@ class APIConnection final : public APIServerConnection { // 2-byte types immediately after flags_ (no padding between them) uint16_t client_api_version_major_{0}; uint16_t client_api_version_minor_{0}; - // Total: 2 (flags) + 2 + 2 = 6 bytes, then 2 bytes padding to next 4-byte boundary + // 1-byte type to fill padding + ActiveIterator active_iterator_{ActiveIterator::NONE}; + // Total: 2 (flags) + 2 + 2 + 1 = 7 bytes, then 1 byte padding to next 4-byte boundary uint32_t get_batch_delay_ms_() const; // Message will use 8 more bytes than the minimum size, and typical From a7f82f5201dbfc51b503058c96789ad985a17330 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 18 Dec 2025 16:50:41 -1000 Subject: [PATCH 3/3] state machine --- esphome/components/api/api_connection.cpp | 51 ++++++++++++----------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 83c22ed67e..d903269d4e 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -214,32 +214,35 @@ void APIConnection::loop() { this->process_batch_(); } - if (this->active_iterator_ == ActiveIterator::LIST_ENTITIES) { - if (!this->iterator_storage_.list_entities.completed()) { - this->process_iterator_batch_(this->iterator_storage_.list_entities); - } else { - // List entities completed, check if we need to start initial state - this->destroy_active_iterator_(); - if (this->flags_.state_subscription) { - this->begin_iterator_(ActiveIterator::INITIAL_STATE); + switch (this->active_iterator_) { + case ActiveIterator::LIST_ENTITIES: + if (this->iterator_storage_.list_entities.completed()) { + this->destroy_active_iterator_(); + if (this->flags_.state_subscription) { + this->begin_iterator_(ActiveIterator::INITIAL_STATE); + } + } else { + this->process_iterator_batch_(this->iterator_storage_.list_entities); } - } - } else if (this->active_iterator_ == ActiveIterator::INITIAL_STATE) { - if (!this->iterator_storage_.initial_state.completed()) { - this->process_iterator_batch_(this->iterator_storage_.initial_state); - } else { - // Initial state completed - this->destroy_active_iterator_(); - // Process any remaining batched messages immediately - if (!this->deferred_batch_.empty()) { - this->process_batch_(); + break; + case ActiveIterator::INITIAL_STATE: + if (this->iterator_storage_.initial_state.completed()) { + this->destroy_active_iterator_(); + // Process any remaining batched messages immediately + if (!this->deferred_batch_.empty()) { + this->process_batch_(); + } + // Now that everything is sent, enable immediate sending for future state changes + this->flags_.should_try_send_immediately = true; + // Release excess memory from buffers that grew during initial sync + this->deferred_batch_.release_buffer(); + this->helper_->release_buffers(); + } else { + this->process_iterator_batch_(this->iterator_storage_.initial_state); } - // Now that everything is sent, enable immediate sending for future state changes - this->flags_.should_try_send_immediately = true; - // Release excess memory from buffers that grew during initial sync - this->deferred_batch_.release_buffer(); - this->helper_->release_buffers(); - } + break; + case ActiveIterator::NONE: + break; } if (this->flags_.sent_ping) {