From 78a451a1c0e9805dcc9354f151d872c47616eeba Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 8 Aug 2026 21:39:48 -0500 Subject: [PATCH] Apply the simplify findings --- .../ble_device_base/ble_client_state.h | 4 + .../bluetooth_connection_bluedroid.cpp | 213 ++++++++---------- .../bluetooth_connection_bluedroid.h | 25 +- 3 files changed, 110 insertions(+), 132 deletions(-) diff --git a/esphome/components/ble_device_base/ble_client_state.h b/esphome/components/ble_device_base/ble_client_state.h index b0c91397fc..7a0a8a3e40 100644 --- a/esphome/components/ble_device_base/ble_client_state.h +++ b/esphome/components/ble_device_base/ble_client_state.h @@ -15,6 +15,10 @@ namespace esphome::ble_device_base { /// ATT code range so they cannot be mistaken for spec errors. -1 is /// understood by API clients as "not connected". Shared by every GATT /// client backend. +/// Safety net shared by every GATT backend: force IDLE when the stack never +/// delivers its disconnect completion. +static constexpr uint32_t GATT_DISCONNECT_TIMEOUT_MS = 10000; + static constexpr int GATT_ERR_NOT_CONNECTED = -1; static constexpr int GATT_ERR_NO_MEMORY = -2; diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index 8fa622b034..0f7011c0c4 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -1,6 +1,6 @@ #include "bluetooth_connection_bluedroid.h" -#if defined(USE_ESP32) && defined(USE_BLE_GATT_CLIENT) +#if defined(USE_ESP32_BLE) && defined(USE_BLE_GATT_CLIENT) #include "bluetooth_connection_hub.h" @@ -28,7 +28,6 @@ using esp32_ble_tracker::ClientState; using esp32_ble_tracker::ConnectionType; static constexpr uint16_t UNSET_CONN_ID = 0xFFFF; -static constexpr uint32_t DISCONNECTING_TIMEOUT = 10000; // ---- shim forwarders ---- @@ -40,14 +39,13 @@ void BluedroidTrackerShim::gap_event_handler(esp_gap_ble_cb_event_t event, esp_b this->engine_->handle_gap_event_(event, param); } void BluedroidTrackerShim::connect() { this->engine_->tracker_connect_(); } -void BluedroidTrackerShim::disconnect() { this->engine_->tracker_disconnect_(); } +void BluedroidTrackerShim::disconnect() { this->engine_->disconnect(); } // ---- component ---- void BluedroidGattClient::setup() { static uint8_t connection_index = 0; this->connection_index_ = connection_index++; - this->conn_id_ = UNSET_CONN_ID; } void BluedroidGattClient::loop() { @@ -68,7 +66,8 @@ void BluedroidGattClient::loop() { } else if (st == ClientState::IDLE) { // The loop only drives the bootstrap and the disconnect safety timeout. this->disable_loop(); - } else if (st == ClientState::DISCONNECTING && millis() - this->disconnecting_started_ > DISCONNECTING_TIMEOUT) { + } else if (st == ClientState::DISCONNECTING && + millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { ESP_LOGE(TAG, "[%d] Timeout waiting for CLOSE_EVT, forcing IDLE", this->connection_index_); // Release before idling: unconditional disconnect does not release, and a // lost CLOSE/DISCONNECT would otherwise leak the table and the cache. @@ -88,14 +87,8 @@ void BluedroidGattClient::dump_config() { // ---- contract ops ---- int BluedroidGattClient::connect(uint64_t address, uint8_t addr_type) { - this->address_ = address; - this->remote_bda_[0] = (address >> 40) & 0xFF; - this->remote_bda_[1] = (address >> 32) & 0xFF; - this->remote_bda_[2] = (address >> 24) & 0xFF; - this->remote_bda_[3] = (address >> 16) & 0xFF; - this->remote_bda_[4] = (address >> 8) & 0xFF; - this->remote_bda_[5] = (address >> 0) & 0xFF; - this->remote_addr_type_ = static_cast(addr_type); + ble_device_base::uint64_to_mac_msb_first(address, this->remote_bda_); + this->remote_addr_type_ = addr_type; // Hand the request to the tracker's promote loop: it stops the scan, raises // coex, and calls tracker_connect_() - the tracker owns connect timing here. this->set_state_(ClientState::DISCOVERED); @@ -125,7 +118,8 @@ void BluedroidGattClient::tracker_connect_() { esp_ble_gap_set_prefer_conn_params(this->remote_bda_, MEDIUM_MIN_CONN_INTERVAL, MEDIUM_MAX_CONN_INTERVAL, 0, MEDIUM_CONN_TIMEOUT); } - auto ret = esp_ble_gattc_open(this->gattc_if_, this->remote_bda_, this->remote_addr_type_, true); + auto ret = esp_ble_gattc_open(this->gattc_if_, this->remote_bda_, + static_cast(this->remote_addr_type_), true); if (ret) { this->log_gattc_warning_("esp_ble_gattc_open", ret); // CONNECT_EVT never fired, so conn_id_ is legitimately unset: plain IDLE. @@ -171,12 +165,8 @@ int BluedroidGattClient::discover_services() { if (this->conn_id_ == UNSET_CONN_ID) { return ble_device_base::GATT_ERR_NOT_CONNECTED; } - this->service_count_ = 0; - auto err = esp_ble_gattc_search_service(this->gattc_if_, this->conn_id_, nullptr); - if (err != ESP_OK) { - this->log_gattc_warning_("esp_ble_gattc_search_service", err); - } - return err; + return this->check_and_log_error_("esp_ble_gattc_search_service", + esp_ble_gattc_search_service(this->gattc_if_, this->conn_id_, nullptr)); } int BluedroidGattClient::read_characteristic(uint16_t handle) { @@ -272,11 +262,7 @@ esp_err_t BluedroidGattClient::update_conn_params_(uint16_t min_interval, uint16 conn_params.latency = latency; conn_params.timeout = timeout; ESP_LOGD(TAG, "[%d] %s conn params", this->connection_index_, param_type); - auto err = esp_ble_gap_update_conn_params(&conn_params); - if (err != ESP_OK) { - this->log_gattc_warning_("esp_ble_gap_update_conn_params", err); - } - return err; + return this->check_and_log_error_("esp_ble_gap_update_conn_params", esp_ble_gap_update_conn_params(&conn_params)); } int BluedroidGattClient::check_and_log_error_(const char *operation, esp_err_t err) { @@ -297,116 +283,116 @@ bool BluedroidGattClient::materialize_table_() { ESP_LOGW(TAG, "[%d] Services released, cannot walk the GATT cache", this->connection_index_); return false; } - // Pass 1: exact counts for a single allocation. - uint16_t total_chars = 0; - uint16_t total_descs = 0; - for (uint16_t s = 0; s < this->service_count_; s++) { - esp_gattc_service_elem_t service_result; - uint16_t count = 1; - if (esp_ble_gattc_get_service(this->gattc_if_, this->conn_id_, nullptr, &service_result, &count, s) != - ESP_GATT_OK || - count == 0) { - return false; - } - uint16_t char_count = 0; - if (esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_CHARACTERISTIC, - service_result.start_handle, service_result.end_handle, 0, - &char_count) != ESP_GATT_OK) { - return false; - } - total_chars += char_count; - uint16_t char_offset = 0; - while (char_offset < char_count) { - esp_gattc_char_elem_t char_result; - uint16_t cc = 1; - auto st = esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, service_result.start_handle, - service_result.end_handle, &char_result, &cc, char_offset); - if (st == ESP_GATT_INVALID_OFFSET || st == ESP_GATT_NOT_FOUND || cc == 0) { + // One flat snapshot of Bluedroid's cached database: a single internal + // allocation instead of a per-element copy of the full result set on every + // get_service/get_all_char/get_all_descr call. + uint16_t total = 0; + if (esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_ALL, 0x0001, 0xFFFF, 0, &total) != + ESP_GATT_OK || + total == 0) { + return false; + } + RAMAllocator db_allocator; + esp_gattc_db_elem_t *db = db_allocator.allocate(total); + if (db == nullptr) { + ESP_LOGE(TAG, "[%d] Database snapshot allocation failed", this->connection_index_); + return false; + } + uint16_t count = total; + if (esp_ble_gattc_get_db(this->gattc_if_, this->conn_id_, 0x0001, 0xFFFF, db, &count) != ESP_GATT_OK) { + db_allocator.deallocate(db, total); + return false; + } + + uint16_t n_svc = 0; + uint16_t n_chr = 0; + uint16_t n_dsc = 0; + for (uint16_t i = 0; i < count; i++) { + switch (db[i].type) { + case ESP_GATT_DB_PRIMARY_SERVICE: + case ESP_GATT_DB_SECONDARY_SERVICE: + n_svc++; + break; + case ESP_GATT_DB_CHARACTERISTIC: + n_chr++; + break; + case ESP_GATT_DB_DESCRIPTOR: + n_dsc++; + break; + default: break; - } - if (st != ESP_GATT_OK) { - return false; - } - uint16_t desc_count = 0; - esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_DESCRIPTOR, 0, 0, - char_result.char_handle, &desc_count); - total_descs += desc_count; - char_offset++; } } - const size_t svc_bytes = this->service_count_ * sizeof(ble_device_base::GattService); - const size_t chr_bytes = total_chars * sizeof(ble_device_base::GattCharacteristic); - const size_t dsc_bytes = total_descs * sizeof(ble_device_base::GattDescriptor); + const size_t svc_bytes = n_svc * sizeof(ble_device_base::GattService); + const size_t chr_bytes = n_chr * sizeof(ble_device_base::GattCharacteristic); RAMAllocator allocator; - this->arena_ = allocator.allocate(svc_bytes + chr_bytes + dsc_bytes); + this->arena_ = allocator.allocate(svc_bytes + chr_bytes + n_dsc * sizeof(ble_device_base::GattDescriptor)); if (this->arena_ == nullptr) { ESP_LOGE(TAG, "[%d] Service table allocation failed", this->connection_index_); + db_allocator.deallocate(db, total); return false; } auto *services = reinterpret_cast(this->arena_); auto *chars = reinterpret_cast(this->arena_ + svc_bytes); auto *descs = reinterpret_cast(this->arena_ + svc_bytes + chr_bytes); - // Pass 2: fill. Enumeration stays bounded by the pass-1 totals - a - // misbehaving peripheral can return more entries than it reported. + // The snapshot is handle-ordered: each service is followed by its + // characteristics, each characteristic by its descriptors, so one linear + // walk assigns the index ranges. + ble_device_base::GattService *service = nullptr; + ble_device_base::GattCharacteristic *chr = nullptr; + uint16_t svc_index = 0; uint16_t chr_index = 0; uint16_t dsc_index = 0; - for (uint16_t s = 0; s < this->service_count_; s++) { - esp_gattc_service_elem_t service_result; - uint16_t count = 1; - if (esp_ble_gattc_get_service(this->gattc_if_, this->conn_id_, nullptr, &service_result, &count, s) != - ESP_GATT_OK || - count == 0) { - break; - } - auto &service = services[s]; - service.uuid = esp32_ble_tracker::ESPBTUUID::from_uuid(service_result.uuid); - service.start_handle = service_result.start_handle; - service.end_handle = service_result.end_handle; - service.first_characteristic = chr_index; - service.characteristic_count = 0; - uint16_t char_offset = 0; - esp_gattc_char_elem_t char_result; - while (chr_index < total_chars) { - uint16_t cc = 1; - auto st = esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, service_result.start_handle, - service_result.end_handle, &char_result, &cc, char_offset); - if (st == ESP_GATT_INVALID_OFFSET || st == ESP_GATT_NOT_FOUND || st != ESP_GATT_OK || cc == 0) { + for (uint16_t i = 0; i < count; i++) { + const auto &elem = db[i]; + switch (elem.type) { + case ESP_GATT_DB_PRIMARY_SERVICE: + case ESP_GATT_DB_SECONDARY_SERVICE: { + service = &services[svc_index++]; + service->uuid = esp32_ble_tracker::ESPBTUUID::from_uuid(elem.uuid); + service->start_handle = elem.start_handle; + service->end_handle = elem.end_handle; + service->first_characteristic = chr_index; + service->characteristic_count = 0; + chr = nullptr; break; } - auto &chr = chars[chr_index]; - chr.uuid = esp32_ble_tracker::ESPBTUUID::from_uuid(char_result.uuid); - chr.value_handle = char_result.char_handle; - chr.end_handle = char_result.char_handle; - chr.properties = char_result.properties; - chr.first_descriptor = dsc_index; - chr.descriptor_count = 0; - uint16_t desc_offset = 0; - esp_gattc_descr_elem_t desc_result; - while (dsc_index < total_descs) { - uint16_t dc = 1; - auto dst = esp_ble_gattc_get_all_descr(this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, - &dc, desc_offset); - if (dst == ESP_GATT_INVALID_OFFSET || dst == ESP_GATT_NOT_FOUND || dst != ESP_GATT_OK || dc == 0) { + case ESP_GATT_DB_CHARACTERISTIC: { + if (service == nullptr) { break; } - descs[dsc_index].uuid = esp32_ble_tracker::ESPBTUUID::from_uuid(desc_result.uuid); - descs[dsc_index].handle = desc_result.handle; - dsc_index++; - chr.descriptor_count++; - desc_offset++; + chr = &chars[chr_index++]; + chr->uuid = esp32_ble_tracker::ESPBTUUID::from_uuid(elem.uuid); + chr->value_handle = elem.attribute_handle; + chr->end_handle = elem.attribute_handle; + chr->properties = elem.properties; + chr->first_descriptor = dsc_index; + chr->descriptor_count = 0; + service->characteristic_count++; + break; } - chr_index++; - service.characteristic_count++; - char_offset++; + case ESP_GATT_DB_DESCRIPTOR: { + if (chr == nullptr) { + break; + } + descs[dsc_index].uuid = esp32_ble_tracker::ESPBTUUID::from_uuid(elem.uuid); + descs[dsc_index].handle = elem.attribute_handle; + dsc_index++; + chr->descriptor_count++; + break; + } + default: + break; } } + db_allocator.deallocate(db, total); this->table_.services = services; this->table_.characteristics = chars; this->table_.descriptors = descs; - this->table_.service_count = this->service_count_; + this->table_.service_count = svc_index; this->table_.characteristic_count = chr_index; this->table_.descriptor_count = dsc_index; return true; @@ -424,7 +410,6 @@ void BluedroidGattClient::handle_search_cmpl_() { // ---- events ---- void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) { - this->service_count_ = 0; auto st = this->state_(); if (st == ClientState::IDLE) { // IDF can deliver OPEN_EVT after esp_ble_gattc_open already returned an @@ -543,12 +528,6 @@ bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_ga this->report_connection_state_(false, param->close.reason); break; } - case ESP_GATTC_SEARCH_RES_EVT: { - if (this->conn_id_ != param->search_res.conn_id) - return false; - this->service_count_++; - break; - } case ESP_GATTC_SEARCH_CMPL_EVT: { if (this->conn_id_ != param->search_cmpl.conn_id) return false; @@ -634,4 +613,4 @@ void BluedroidGattClient::handle_gap_event_(esp_gap_ble_cb_event_t event, esp_bl } // namespace esphome::bluetooth_connection -#endif // USE_ESP32 && USE_BLE_GATT_CLIENT +#endif // USE_ESP32_BLE && USE_BLE_GATT_CLIENT diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h index f109f2e23b..1a1a8bc1c1 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h @@ -11,7 +11,7 @@ #include "esphome/core/defines.h" -#if defined(USE_ESP32) && defined(USE_BLE_GATT_CLIENT) +#if defined(USE_ESP32_BLE) && defined(USE_BLE_GATT_CLIENT) #include "esphome/components/ble_device_base/ble_gatt_client.h" #include "esphome/components/esp32_ble_tracker/esp32_ble_tracker.h" @@ -40,7 +40,6 @@ class BluedroidTrackerShim final : public esp32_ble_tracker::ESPBTClient { bool parse_device(const ble_device_base::ESPBTDevice &device) override { return false; } void schedule_disconnect() { this->want_disconnect_ = true; } - bool disconnect_scheduled() const { return this->want_disconnect_; } protected: BluedroidGattClient *engine_; @@ -71,7 +70,7 @@ class BluedroidGattClient final : public Component { void release_services(); void set_connection_type(esp32_ble_tracker::ConnectionType ct) { this->connection_type_ = ct; } - bool disconnect_pending() const { return this->shim_.disconnect_scheduled(); } + bool disconnect_pending() const { return this->shim_.disconnect_pending(); } void cancel_pending_disconnect() { this->shim_.cancel_pending_disconnect(); } protected: @@ -81,7 +80,6 @@ class BluedroidGattClient final : public Component { void set_state_(esp32_ble_tracker::ClientState st) { this->shim_.set_state(st); } bool check_addr_(const esp_bd_addr_t &addr) const; void tracker_connect_(); - void tracker_disconnect_() { this->disconnect(); } bool handle_gattc_event_(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param); void handle_gap_event_(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param); void handle_open_evt_(esp_ble_gattc_cb_param_t *param); @@ -105,25 +103,22 @@ class BluedroidGattClient final : public Component { uint8_t *arena_{nullptr}; ble_device_base::GattServiceTable table_{}; - // Group 2: 8-byte types - uint64_t address_{0}; - - // Group 3: 4-byte types + // Group 2: 4-byte types int gattc_if_{ESP_GATT_IF_NONE}; uint32_t disconnecting_started_{0}; - // Group 4: arrays + // Group 3: arrays esp_bd_addr_t remote_bda_{}; - // Group 5: 2-byte types - uint16_t conn_id_; + // Group 4: 2-byte types + uint16_t conn_id_{0xFFFF}; uint16_t mtu_{23}; - // Group 6: 1-byte types - esp_ble_addr_type_t remote_addr_type_{BLE_ADDR_TYPE_PUBLIC}; + // Group 5: 1-byte types + // Stored narrow (the enum is 4 bytes); widened at the esp_ble_gattc_open call. + uint8_t remote_addr_type_{0}; esp32_ble_tracker::ConnectionType connection_type_{esp32_ble_tracker::ConnectionType::V3_WITHOUT_CACHE}; uint8_t connection_index_; - uint8_t service_count_{0}; // Set only when release_services() cleans the stack's GATT cache, which no // walk may then touch (Bluedroid asserts rather than erroring). bool services_released_{false}; @@ -134,4 +129,4 @@ class BluedroidGattClient final : public Component { } // namespace esphome::bluetooth_connection -#endif // USE_ESP32 && USE_BLE_GATT_CLIENT +#endif // USE_ESP32_BLE && USE_BLE_GATT_CLIENT