From 1b801427c46e305ea4050e56efd2dc7057e02bd5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Aug 2026 14:46:30 -0500 Subject: [PATCH] Dry-pass round 2: comment accuracy polish Round 2 found no code issues - the round-1 mechanisms verify clean on disk (every IDLE transition through the one door, the teardown-guard trio minimal-complete per event, the error latch airtight, the reset lists disjoint by construction). What remained was comment drift: disconnect_pending() spelling, the OPEN_EVT and cached-MTU comments, the contract header re-wrap, the two claim-a-slot docstrings disambiguated, and ragged wraps from earlier text excisions. --- esphome/components/ble_device_base/__init__.py | 5 +++-- .../components/ble_device_base/ble_gatt_client.h | 13 +++++-------- .../bluetooth_connection_bluedroid.cpp | 12 ++++++------ .../bluetooth_connection_bluedroid.h | 2 +- .../bluetooth_connection_hub.cpp | 10 +++++----- 5 files changed, 20 insertions(+), 22 deletions(-) diff --git a/esphome/components/ble_device_base/__init__.py b/esphome/components/ble_device_base/__init__.py index c2520ba0cf..ae03003713 100644 --- a/esphome/components/ble_device_base/__init__.py +++ b/esphome/components/ble_device_base/__init__.py @@ -163,8 +163,9 @@ _request_gatt_connection_slot = cg.slot_counter(GATT_CLIENT_COUNT_DEFINE) def request_gatt_client() -> None: """Compile in the neutral GATT client contract (ble_gatt_client.h) and - claim one connection slot. Called by bluetooth_connection.new_gatt_backend() - once per backend instance.""" + claim one compiled-in client slot (sizes ESPHOME_BLE_GATT_CLIENT_COUNT; + distinct from the proxy's validated connection budget). Called by + bluetooth_connection.new_gatt_backend() once per backend instance.""" cg.add_define("USE_BLE_GATT_CLIENT") _request_gatt_connection_slot() diff --git a/esphome/components/ble_device_base/ble_gatt_client.h b/esphome/components/ble_device_base/ble_gatt_client.h index 0c5e7bea1f..730c2e162c 100644 --- a/esphome/components/ble_device_base/ble_gatt_client.h +++ b/esphome/components/ble_device_base/ble_gatt_client.h @@ -5,14 +5,11 @@ // Exactly one GATT backend exists per build, so BLEGattConnection is a // compile-time alias (bluetooth_connection_gatt_backend.h), not an abstract // interface. -// A consumer - a streaming consumer that forwards the raw database (the hub -// BluetoothConnection wrapper) or a direct consumer owning a dedicated -// backend and resolving handles by UUID - drives it and receives -// completions through the GattClientListener interface (one build can hold -// several consumer types while the backend stays a single non-virtual -// class). All listener -// calls are delivered on the ESPHome main loop; borrowed data pointers are -// valid only for the duration of the call. +// A consumer - the hub wrapper streaming the raw database, or a direct +// consumer owning a dedicated backend and resolving handles by UUID - +// drives it and receives completions through the GattClientListener +// interface. All listener calls are delivered on the ESPHome main loop; +// borrowed data pointers are valid only for the duration of the call. // // Error domain (plain int, forwarded to the API without translation): // 0 success diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index f78c02d744..d4bee3fe3e 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -65,7 +65,7 @@ void BluedroidGattClient::loop() { } // Do not wait for REG_EVT; a dropped event must not wedge the slot. this->set_idle_(); - } else if (st == ClientState::DISCONNECTING || this->want_disconnect_) { + } else if (st == ClientState::DISCONNECTING || this->disconnect_pending()) { // The one teardown safety net: a lost CLOSE_EVT, or a scheduled // teardown whose OPEN_EVT never arrives. if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { @@ -539,7 +539,8 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) { return; } if (this->disconnect_pending()) { - // Earliest point conn_id_ exists; keep it set so CLOSE_EVT still matches. + // Open resolved with a teardown scheduled: close now (conn_id_ stays set + // so CLOSE_EVT still matches). this->unconditional_disconnect_(); return; } @@ -547,11 +548,10 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) { ESP_LOGI(TAG, "[%d] Connection open", this->connection_index_); if (this->connection_type_ == ConnectionType::V3_WITH_CACHE) { this->set_state(ClientState::ESTABLISHED); - // No discovery phase: report immediately; the MTU report below is - // suppressed by seen_mtu_ (HA tolerates a post-connect MTU of 23 here, - // matching the previous esp32 behavior). + // No discovery phase: report immediately with the default MTU. The + // cached path never waits for (or reports) the exchange - seen_mtu_ + // suppresses the CFG_MTU report, matching the previous esp32 behavior. this->seen_mtu_ = true; - // Cached path never exchanged an MTU; HA has always seen the default. this->listener_->on_connection_state(true, ble_device_base::DEFAULT_ATT_MTU, 0); } else { // Discovery-bound connection: start the search now so it overlaps the diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h index 2f0336f2ea..a0ba122be0 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h @@ -129,7 +129,7 @@ class BluedroidGattClient final : public esp32_ble_tracker::ESPBTClient, public // The MTU request was refused at CONNECT_EVT; OPEN_EVT reports instead. bool mtu_failed_ : 1 {false}; // Search issued at OPEN_EVT overlaps the MTU exchange; discover_services() - // completes from it. Reset per attempt and on idle. + // completes from it. Reset by set_idle_(). SearchState search_state_ : 4 {SearchState::NONE}; // esp_gatt_status_t of the completed search, held until claimed. uint8_t search_status_{0}; diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index f972760d14..b913bb9a55 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -27,8 +27,8 @@ void BluetoothConnection::set_address(uint64_t address) { } void BluetoothConnection::initiate_connection(uint8_t address_type) { - // No connect timeout here: the API client's own timeout or - // the api-gone sweep drives disconnect(). + // No connect timeout here: the API client's own timeout or the api-gone + // sweep drives disconnect(). this->state_ = ClientState::CONNECTING; int err = this->backend_->connect(this->address_, address_type); if (err != 0) { @@ -309,9 +309,9 @@ void BluetoothConnection::send_service_for_discovery_() { } // The subscriber vanished mid-stream: park the cursor at done WITHOUT - // sending services-done (a resubscribing client gets silence and its - // 30 s timeout, never an authoritative partial list) and - // free the table; the api-gone sweep tears the connection down anyway. + // sending services-done (a resubscribing client gets silence and its 30 s + // timeout, never an authoritative partial list) and free the table; the + // api-gone sweep tears the connection down anyway. auto *api_conn = this->proxy_->get_api_connection(); if (api_conn == nullptr) { ESP_LOGW(TAG, "[%d] [%s] API connection lost while streaming services", this->connection_index_,