From 31608543c2581cc032c1a327f87afa59bc2d0341 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 20 Jan 2026 17:50:53 -1000 Subject: [PATCH] [esp32_ble_tracker] Optimize loop with state change tracking for ~85% CPU reduction (#13337) --- .../bluetooth_proxy/bluetooth_connection.cpp | 4 +- .../esp32_ble_client/ble_client_base.cpp | 34 +++++------ .../esp32_ble_client/ble_client_base.h | 2 +- .../esp32_ble_tracker/esp32_ble_tracker.cpp | 42 +++++++++++--- .../esp32_ble_tracker/esp32_ble_tracker.h | 58 +++++++++++++++++-- 5 files changed, 107 insertions(+), 33 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp index 1d6f7e23b3..60f56fda54 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.cpp @@ -135,8 +135,8 @@ void BluetoothConnection::loop() { // - For V3_WITH_CACHE: Services are never sent, disable after INIT state // - 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)) { + if (this->state() != espbt::ClientState::INIT && (this->connection_type_ == espbt::ConnectionType::V3_WITH_CACHE || + this->send_service_ == DONE_SENDING_SERVICES)) { this->disable_loop(); } } diff --git a/esphome/components/esp32_ble_client/ble_client_base.cpp b/esphome/components/esp32_ble_client/ble_client_base.cpp index 01f79156a9..c464c89390 100644 --- a/esphome/components/esp32_ble_client/ble_client_base.cpp +++ b/esphome/components/esp32_ble_client/ble_client_base.cpp @@ -50,7 +50,7 @@ void BLEClientBase::loop() { this->set_state(espbt::ClientState::INIT); return; } - if (this->state_ == espbt::ClientState::INIT) { + if (this->state() == espbt::ClientState::INIT) { auto ret = esp_ble_gattc_app_register(this->app_id); if (ret) { ESP_LOGE(TAG, "gattc app register failed. app_id=%d code=%d", this->app_id, ret); @@ -60,7 +60,7 @@ void BLEClientBase::loop() { } // If idle, we can disable the loop as connect() // will enable it again when a connection is needed. - else if (this->state_ == espbt::ClientState::IDLE) { + else if (this->state() == espbt::ClientState::IDLE) { this->disable_loop(); } } @@ -86,7 +86,7 @@ bool BLEClientBase::parse_device(const espbt::ESPBTDevice &device) { return false; if (this->address_ == 0 || device.address_uint64() != this->address_) return false; - if (this->state_ != espbt::ClientState::IDLE) + if (this->state() != espbt::ClientState::IDLE) return false; this->log_event_("Found device"); @@ -102,10 +102,10 @@ bool BLEClientBase::parse_device(const espbt::ESPBTDevice &device) { void BLEClientBase::connect() { // Prevent duplicate connection attempts - if (this->state_ == espbt::ClientState::CONNECTING || this->state_ == espbt::ClientState::CONNECTED || - this->state_ == espbt::ClientState::ESTABLISHED) { + if (this->state() == espbt::ClientState::CONNECTING || this->state() == espbt::ClientState::CONNECTED || + this->state() == espbt::ClientState::ESTABLISHED) { ESP_LOGW(TAG, "[%d] [%s] Connection already in progress, state=%s", this->connection_index_, this->address_str_, - espbt::client_state_to_string(this->state_)); + espbt::client_state_to_string(this->state())); return; } ESP_LOGI(TAG, "[%d] [%s] 0x%02x Connecting", this->connection_index_, this->address_str_, this->remote_addr_type_); @@ -133,12 +133,12 @@ void BLEClientBase::connect() { esp_err_t BLEClientBase::pair() { return esp_ble_set_encryption(this->remote_bda_, ESP_BLE_SEC_ENCRYPT); } void BLEClientBase::disconnect() { - if (this->state_ == espbt::ClientState::IDLE || this->state_ == espbt::ClientState::DISCONNECTING) { + if (this->state() == espbt::ClientState::IDLE || this->state() == espbt::ClientState::DISCONNECTING) { ESP_LOGI(TAG, "[%d] [%s] Disconnect requested, but already %s", this->connection_index_, this->address_str_, - espbt::client_state_to_string(this->state_)); + espbt::client_state_to_string(this->state())); return; } - if (this->state_ == espbt::ClientState::CONNECTING || this->conn_id_ == UNSET_CONN_ID) { + if (this->state() == espbt::ClientState::CONNECTING || this->conn_id_ == UNSET_CONN_ID) { ESP_LOGD(TAG, "[%d] [%s] Disconnect before connected, disconnect scheduled", this->connection_index_, this->address_str_); this->want_disconnect_ = true; @@ -150,7 +150,7 @@ void BLEClientBase::disconnect() { void BLEClientBase::unconditional_disconnect() { // Disconnect without checking the state. ESP_LOGI(TAG, "[%d] [%s] Disconnecting (conn_id: %d).", this->connection_index_, this->address_str_, this->conn_id_); - if (this->state_ == espbt::ClientState::DISCONNECTING) { + if (this->state() == espbt::ClientState::DISCONNECTING) { this->log_error_("Already disconnecting"); return; } @@ -170,7 +170,7 @@ void BLEClientBase::unconditional_disconnect() { this->log_gattc_warning_("esp_ble_gattc_close", err); } - if (this->state_ == espbt::ClientState::DISCOVERED) { + if (this->state() == espbt::ClientState::DISCOVERED) { this->set_address(0); this->set_state(espbt::ClientState::IDLE); } else { @@ -295,18 +295,18 @@ bool BLEClientBase::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_ // ESP-IDF's BLE stack may send ESP_GATTC_OPEN_EVT after esp_ble_gattc_open() returns an // error, if the error occurred at the BTA/GATT layer. This can result in the event // arriving after we've already transitioned to IDLE state. - if (this->state_ == espbt::ClientState::IDLE) { + if (this->state() == espbt::ClientState::IDLE) { ESP_LOGD(TAG, "[%d] [%s] ESP_GATTC_OPEN_EVT in IDLE state (status=%d), ignoring", this->connection_index_, this->address_str_, param->open.status); break; } - if (this->state_ != espbt::ClientState::CONNECTING) { + if (this->state() != espbt::ClientState::CONNECTING) { // This should not happen but lets log it in case it does // because it means we have a bad assumption about how the // ESP BT stack works. ESP_LOGE(TAG, "[%d] [%s] ESP_GATTC_OPEN_EVT in %s state (status=%d)", this->connection_index_, - this->address_str_, espbt::client_state_to_string(this->state_), param->open.status); + this->address_str_, espbt::client_state_to_string(this->state()), param->open.status); } if (param->open.status != ESP_GATT_OK && param->open.status != ESP_GATT_ALREADY_OPEN) { this->log_gattc_warning_("Connection open", param->open.status); @@ -327,7 +327,7 @@ bool BLEClientBase::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_ if (this->connection_type_ == espbt::ConnectionType::V3_WITH_CACHE) { // Cached connections already connected with medium parameters, no update needed // only set our state, subclients might have more stuff to do yet. - this->state_ = espbt::ClientState::ESTABLISHED; + this->set_state_internal_(espbt::ClientState::ESTABLISHED); break; } // For V3_WITHOUT_CACHE, we already set fast params before connecting @@ -356,7 +356,7 @@ bool BLEClientBase::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_ return false; // Check if we were disconnected while waiting for service discovery if (param->disconnect.reason == ESP_GATT_CONN_TERMINATE_PEER_USER && - this->state_ == espbt::ClientState::CONNECTED) { + this->state() == espbt::ClientState::CONNECTED) { this->log_warning_("Remote closed during discovery"); } else { ESP_LOGD(TAG, "[%d] [%s] ESP_GATTC_DISCONNECT_EVT, reason 0x%02x", this->connection_index_, this->address_str_, @@ -433,7 +433,7 @@ bool BLEClientBase::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_ #endif } ESP_LOGI(TAG, "[%d] [%s] Service discovery complete", this->connection_index_, this->address_str_); - this->state_ = espbt::ClientState::ESTABLISHED; + this->set_state_internal_(espbt::ClientState::ESTABLISHED); break; } case ESP_GATTC_READ_DESCR_EVT: { diff --git a/esphome/components/esp32_ble_client/ble_client_base.h b/esphome/components/esp32_ble_client/ble_client_base.h index c52f0e5d2d..c2336b2349 100644 --- a/esphome/components/esp32_ble_client/ble_client_base.h +++ b/esphome/components/esp32_ble_client/ble_client_base.h @@ -44,7 +44,7 @@ class BLEClientBase : public espbt::ESPBTClient, public Component { void unconditional_disconnect(); void release_services(); - bool connected() { return this->state_ == espbt::ClientState::ESTABLISHED; } + bool connected() { return this->state() == espbt::ClientState::ESTABLISHED; } void set_auto_connect(bool auto_connect) { this->auto_connect_ = auto_connect; } diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 995755ac84..73a298d279 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -105,15 +105,13 @@ void ESP32BLETracker::loop() { } // Check for scan timeout - moved here from scheduler to avoid false reboots - // when the loop is blocked + // when the loop is blocked. This must run every iteration for safety. if (this->scanner_state_ == ScannerState::RUNNING) { switch (this->scan_timeout_state_) { case ScanTimeoutState::MONITORING: { - uint32_t now = App.get_loop_component_start_time(); - uint32_t timeout_ms = this->scan_duration_ * 2000; // Robust time comparison that handles rollover correctly // This works because unsigned arithmetic wraps around predictably - if ((now - this->scan_start_time_) > timeout_ms) { + if ((App.get_loop_component_start_time() - this->scan_start_time_) > this->scan_timeout_ms_) { // First time we've seen the timeout exceeded - wait one more loop iteration // This ensures all components have had a chance to process pending events // This is because esp32_ble may not have run yet and called @@ -128,13 +126,31 @@ void ESP32BLETracker::loop() { ESP_LOGE(TAG, "Scan never terminated, rebooting"); App.reboot(); break; - case ScanTimeoutState::INACTIVE: - // This case should be unreachable - scanner and timeout states are always synchronized break; } } + // Fast path: skip expensive client state counting and processing + // if no state has changed since last loop iteration. + // + // How state changes ensure we reach the code below: + // - handle_scanner_failure_(): scanner_state_ becomes FAILED via set_scanner_state_(), or + // scan_set_param_failed_ requires scanner_state_==RUNNING which can only be reached via + // set_scanner_state_(RUNNING) in gap_scan_start_complete_() (scan params are set during + // STARTING, not RUNNING, so version is always incremented before this condition is true) + // - start_scan_(): scanner_state_ becomes IDLE via set_scanner_state_() in cleanup_scan_state_() + // - try_promote_discovered_clients_(): client enters DISCOVERED via set_state(), or + // connecting client finishes (state change), or scanner reaches RUNNING/IDLE + // + // All conditions that affect the logic below are tied to state changes that increment + // state_version_, so the fast path is safe. + if (this->state_version_ == this->last_processed_version_) { + return; + } + this->last_processed_version_ = this->state_version_; + + // State changed - do full processing ClientStateCounts counts = this->count_client_states_(); if (counts != this->client_state_counts_) { this->client_state_counts_ = counts; @@ -142,6 +158,7 @@ void ESP32BLETracker::loop() { this->client_state_counts_.discovered, this->client_state_counts_.disconnecting); } + // Scanner failure: reached when set_scanner_state_(FAILED) or scan_set_param_failed_ set if (this->scanner_state_ == ScannerState::FAILED || (this->scan_set_param_failed_ && this->scanner_state_ == ScannerState::RUNNING)) { this->handle_scanner_failure_(); @@ -160,6 +177,8 @@ void ESP32BLETracker::loop() { */ + // Start scan: reached when scanner_state_ becomes IDLE (via set_scanner_state_()) and + // all clients are idle (their state changes increment version when they finish) if (this->scanner_state_ == ScannerState::IDLE && !counts.connecting && !counts.disconnecting && !counts.discovered) { #ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE this->update_coex_preference_(false); @@ -168,8 +187,9 @@ void ESP32BLETracker::loop() { this->start_scan_(false); // first = false } } - // If there is a discovered client and no connecting - // clients, then promote the discovered client to ready to connect. + // Promote discovered clients: reached when a client's state becomes DISCOVERED (via set_state()), + // or when a blocking condition clears (connecting client finishes, scanner reaches RUNNING/IDLE). + // All these trigger state_version_ increment, so we'll process and check promotion eligibility. // We check both RUNNING and IDLE states because: // - RUNNING: gap_scan_event_handler initiates stop_scan_() but promotion can happen immediately // - IDLE: Scanner has already stopped (naturally or by gap_scan_event_handler) @@ -236,6 +256,7 @@ void ESP32BLETracker::start_scan_(bool first) { // Start timeout monitoring in loop() instead of using scheduler // This prevents false reboots when the loop is blocked this->scan_start_time_ = App.get_loop_component_start_time(); + this->scan_timeout_ms_ = this->scan_duration_ * 2000; this->scan_timeout_state_ = ScanTimeoutState::MONITORING; esp_err_t err = esp_ble_gap_set_scan_params(&this->scan_params_); @@ -253,6 +274,10 @@ void ESP32BLETracker::start_scan_(bool first) { void ESP32BLETracker::register_client(ESPBTClient *client) { #ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT client->app_id = ++this->app_id_; + // Give client a pointer to our state_version_ so it can notify us of state changes. + // This enables loop() fast-path optimization - we skip expensive work when no state changed. + // Safe because ESP32BLETracker (singleton) outlives all registered clients. + client->set_tracker_state_version(&this->state_version_); this->clients_.push_back(client); this->recalculate_advertisement_parser_types(); #endif @@ -382,6 +407,7 @@ void ESP32BLETracker::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_i void ESP32BLETracker::set_scanner_state_(ScannerState state) { this->scanner_state_ = state; + this->state_version_++; for (auto *listener : this->scanner_state_listeners_) { listener->on_scanner_state(state); } diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index f538a0eddc..fa0cdb6f45 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -216,6 +216,19 @@ enum class ConnectionType : uint8_t { V3_WITHOUT_CACHE }; +/// Base class for BLE GATT clients that connect to remote devices. +/// +/// State Change Tracking Design: +/// ----------------------------- +/// ESP32BLETracker::loop() needs to know when client states change to avoid +/// expensive polling. Rather than checking all clients every iteration (~7000/min), +/// we use a version counter owned by ESP32BLETracker that clients increment on +/// state changes. The tracker compares versions to skip work when nothing changed. +/// +/// Ownership: ESP32BLETracker owns state_version_. Clients hold a non-owning +/// pointer (tracker_state_version_) set during register_client(). Clients +/// increment the counter through this pointer when their state changes. +/// The pointer may be null if the client is not registered with a tracker. class ESPBTClient : public ESPBTDeviceListener { public: virtual bool gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, @@ -225,26 +238,49 @@ class ESPBTClient : public ESPBTDeviceListener { virtual void disconnect() = 0; bool disconnect_pending() const { return this->want_disconnect_; } void cancel_pending_disconnect() { this->want_disconnect_ = false; } + + /// Set the client state with IDLE handling (clears want_disconnect_). + /// Notifies the tracker of state change for loop optimization. virtual void set_state(ClientState st) { - this->state_ = st; + this->set_state_internal_(st); if (st == ClientState::IDLE) { this->want_disconnect_ = false; } } - ClientState state() const { return state_; } + ClientState state() const { return this->state_; } + + /// Called by ESP32BLETracker::register_client() to enable state change notifications. + /// The pointer must remain valid for the lifetime of the client (guaranteed since + /// ESP32BLETracker is a singleton that outlives all clients). + void set_tracker_state_version(uint8_t *version) { this->tracker_state_version_ = version; } // Memory optimized layout uint8_t app_id; // App IDs are small integers assigned sequentially protected: - // Group 1: 1-byte types - ClientState state_{ClientState::INIT}; + /// Set state without IDLE handling - use for direct state transitions. + /// Increments the tracker's state version counter to signal that loop() + /// should do full processing on the next iteration. + void set_state_internal_(ClientState st) { + this->state_ = st; + // Notify tracker that state changed (tracker_state_version_ is owned by ESP32BLETracker) + if (this->tracker_state_version_ != nullptr) { + (*this->tracker_state_version_)++; + } + } + // want_disconnect_ is set to true when a disconnect is requested // while the client is connecting. This is used to disconnect the // client as soon as we get the connection id (conn_id_) from the // ESP_GATTC_OPEN_EVT event. bool want_disconnect_{false}; - // 2 bytes used, 2 bytes padding + + private: + ClientState state_{ClientState::INIT}; + /// Non-owning pointer to ESP32BLETracker::state_version_. When this client's + /// state changes, we increment the tracker's counter to signal that loop() + /// should perform full processing. Null if client not registered with tracker. + uint8_t *tracker_state_version_{nullptr}; }; class ESP32BLETracker : public Component, @@ -380,6 +416,16 @@ class ESP32BLETracker : public Component, // Group 4: 1-byte types (enums, uint8_t, bool) uint8_t app_id_{0}; uint8_t scan_start_fail_count_{0}; + /// Version counter for loop() fast-path optimization. Incremented when: + /// - Scanner state changes (via set_scanner_state_()) + /// - Any registered client's state changes (clients hold pointer to this counter) + /// Owned by this class; clients receive non-owning pointer via register_client(). + /// When loop() sees state_version_ == last_processed_version_, it skips expensive + /// client state counting and takes the fast path (just timeout check + return). + uint8_t state_version_{0}; + /// Last state_version_ value when loop() did full processing. Compared against + /// state_version_ to detect if any state changed since last iteration. + uint8_t last_processed_version_{0}; ScannerState scanner_state_{ScannerState::IDLE}; bool scan_continuous_; bool scan_active_; @@ -396,6 +442,8 @@ class ESP32BLETracker : public Component, EXCEEDED_WAIT, // Timeout exceeded, waiting one loop before reboot }; uint32_t scan_start_time_{0}; + /// Precomputed timeout value: scan_duration_ * 2000 + uint32_t scan_timeout_ms_{0}; ScanTimeoutState scan_timeout_state_{ScanTimeoutState::INACTIVE}; };