From 138304cfbe42b5e6879d7e8da5847624bb5b2b41 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 8 Aug 2026 23:21:57 -0500 Subject: [PATCH 1/2] Fail discovery on count errors and free idle slots at once --- .../ble_device_base/ble_client_state.h | 6 +-- .../bluetooth_connection_bluedroid.cpp | 42 +++++++++++++++---- .../bluetooth_connection_gatt_backend.h | 2 +- .../bluetooth_connection_hub.cpp | 4 +- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/esphome/components/ble_device_base/ble_client_state.h b/esphome/components/ble_device_base/ble_client_state.h index 7a0a8a3e40..013853b2e2 100644 --- a/esphome/components/ble_device_base/ble_client_state.h +++ b/esphome/components/ble_device_base/ble_client_state.h @@ -15,13 +15,13 @@ 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. +static constexpr int GATT_ERR_NOT_CONNECTED = -1; +static constexpr int GATT_ERR_NO_MEMORY = -2; + /// 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; - // Preferred connection parameters shared by every platform's GATT client so // the backends cannot drift (units: interval 1.25 ms, timeout 10 ms; latency // 0). FAST covers connection setup and service discovery; MEDIUM is the diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index e5bdb7af56..abd37868d0 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -133,13 +133,18 @@ void BluedroidGattClient::tracker_connect_() { int BluedroidGattClient::disconnect() { auto st = this->state_(); - if (st == ClientState::IDLE || st == ClientState::DISCONNECTING) { + if (st == ClientState::DISCONNECTING) { return 0; } + // Nothing was opened, so no completion event will follow: report + // not-connected and the hub frees the slot at once (rp2 convention). + if (st == ClientState::IDLE) { + return ble_device_base::GATT_ERR_NOT_CONNECTED; + } if (st == ClientState::DISCOVERED) { - // Never opened: nothing to close. + // Parked for the tracker promote loop, never opened. this->set_state_(ClientState::IDLE); - return 0; + return ble_device_base::GATT_ERR_NOT_CONNECTED; } if (st == ClientState::CONNECTING || this->conn_id_ == UNSET_CONN_ID) { ESP_LOGD(TAG, "[%d] Disconnect scheduled", this->connection_index_); @@ -279,10 +284,20 @@ void BluedroidGattClient::handle_search_cmpl_() { this->update_conn_params_(MEDIUM_MIN_CONN_INTERVAL, MEDIUM_MAX_CONN_INTERVAL, 0, MEDIUM_CONN_TIMEOUT, "medium"); uint16_t primary = 0; uint16_t secondary = 0; - esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_PRIMARY_SERVICE, 0x0001, 0xFFFF, 0, - &primary); - esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_SECONDARY_SERVICE, 0x0001, 0xFFFF, 0, - &secondary); + auto primary_status = esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_PRIMARY_SERVICE, + 0x0001, 0xFFFF, 0, &primary); + auto secondary_status = esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_SECONDARY_SERVICE, + 0x0001, 0xFFFF, 0, &secondary); + if (primary_status != ESP_GATT_OK || secondary_status != ESP_GATT_OK) { + // A failed count must not become an authoritative empty database - V3 + // clients cache the streamed result permanently. + auto status = primary_status != ESP_GATT_OK ? primary_status : secondary_status; + this->log_gattc_warning_("esp_ble_gattc_get_attr_count", status); + if (this->listener_ != nullptr) { + this->listener_->on_service_discovery_done(status); + } + return; + } this->service_total_ = primary + secondary; if (this->listener_ != nullptr) { this->listener_->on_service_discovery_done(0); @@ -360,6 +375,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { } if (char_status != ESP_GATT_OK || cc == 0) { if (char_status != ESP_GATT_OK) { + this->log_gattc_warning_("esp_ble_gattc_get_all_char", char_status); conn.send_service_ = DONE_SENDING_SERVICES; return; } @@ -373,8 +389,15 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { characteristic_resp.properties = char_result.properties; uint16_t total_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, &total_desc_count); + auto desc_count_status = esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_DESCRIPTOR, + 0, 0, char_result.char_handle, &total_desc_count); + if (desc_count_status != ESP_GATT_OK) { + // Abort rather than stream the characteristic descriptor-less: a + // missing CCCD in a cached database breaks notifications for good. + this->log_gattc_warning_("esp_ble_gattc_get_attr_count", desc_count_status); + conn.send_service_ = DONE_SENDING_SERVICES; + return; + } if (total_desc_count > 0) { characteristic_resp.descriptors.init(total_desc_count); uint16_t desc_offset = 0; @@ -388,6 +411,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { } if (desc_status != ESP_GATT_OK || dc == 0) { if (desc_status != ESP_GATT_OK) { + this->log_gattc_warning_("esp_ble_gattc_get_all_descr", desc_status); conn.send_service_ = DONE_SENDING_SERVICES; return; } diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h b/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h index 32ddf5e577..58102ecfb3 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h @@ -15,7 +15,7 @@ #if defined(USE_RP2040_BLE) #include "bluetooth_connection_rp2.h" #define ESPHOME_BLE_GATT_CONNECTION_TYPE bluetooth_connection::RP2GattClient -#elif defined(USE_ESP32) +#elif defined(USE_ESP32_BLE) #include "bluetooth_connection_bluedroid.h" #define ESPHOME_BLE_GATT_CONNECTION_TYPE bluetooth_connection::BluedroidGattClient #elif defined(USE_BLE_GATT_CLIENT_STUB_BACKEND) diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index ec03f18e1d..41e0ce48f1 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -67,8 +67,8 @@ void BluetoothConnection::disconnect() { void BluetoothConnection::check_disconnect_timeout_() { // Safety net mirroring the esp32 base class: if the backend's disconnect // completion is lost, force the slot free instead of leaking it. - static constexpr uint32_t DISCONNECT_TIMEOUT_MS = 10000; - if (this->state_ == ClientState::DISCONNECTING && millis() - this->disconnecting_started_ > DISCONNECT_TIMEOUT_MS) { + if (this->state_ == ClientState::DISCONNECTING && + millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { ESP_LOGW(TAG, "[%d] [%s] Disconnect timeout, freeing slot", this->connection_index_, this->address_str_); this->reset_connection_(GATT_NOT_CONNECTED); } From d135ac97e5e1675bdd7ff6c21b9ae3d0fe0f841e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 8 Aug 2026 23:27:04 -0500 Subject: [PATCH 2/2] Report teardown once at CLOSE_EVT and refuse connects on a busy slot --- .../ble_device_base/ble_gatt_client.h | 5 +++++ .../bluetooth_connection_bluedroid.cpp | 22 +++++++++++-------- .../bluetooth_connection_esp32.cpp | 3 +-- .../bluetooth_connection_hub.cpp | 5 ++++- .../test_gatt_client_contract.cpp | 3 +++ 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/esphome/components/ble_device_base/ble_gatt_client.h b/esphome/components/ble_device_base/ble_gatt_client.h index 81d879a704..9b55a9fcfd 100644 --- a/esphome/components/ble_device_base/ble_gatt_client.h +++ b/esphome/components/ble_device_base/ble_gatt_client.h @@ -111,6 +111,11 @@ concept BLEGattConnectionContract = requires(T conn, Sink *sink, const uint8_t * { conn.update_connection_params(uint16_t{}, uint16_t{}, uint16_t{}, uint16_t{}) } -> std::same_as; { conn.get_service_table() } -> std::same_as; { conn.release_services() } -> std::same_as; + // Deferred-disconnect visibility and the connection-type hint; backends + // without the underlying state carry inline no-ops. + { conn.disconnect_pending() } -> std::same_as; + { conn.cancel_pending_disconnect() } -> std::same_as; + { conn.set_connection_type(ConnectionType{}) } -> std::same_as; }; // The event sink the backend calls directly (the hub BluetoothConnection diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index 71e5356244..191d1ccba0 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -12,9 +12,6 @@ #include "esphome/core/helpers.h" #include "esphome/core/log.h" -#ifndef CONFIG_ESP_HOSTED_ENABLE_BT_BLUEDROID -#include -#endif #include #include @@ -92,6 +89,13 @@ void BluedroidGattClient::dump_config() { // ---- contract ops ---- int BluedroidGattClient::connect(uint64_t address, uint8_t addr_type) { + // Refuse anything but a fully idle slot. Clobbering DISCONNECTING with + // DISCOVERED would let the tracker open a new link while the old one is + // still closing - the stale CLOSE_EVT then tears the new attempt down. + if (this->state_() != ClientState::IDLE) { + ESP_LOGW(TAG, "[%d] Connect rejected, slot busy", this->connection_index_); + return ESP_GATT_BUSY; + } 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 @@ -488,11 +492,12 @@ void BluedroidGattClient::handle_disconnect_evt_(esp_ble_gattc_cb_param_t *param // Active close delivers CLOSE_EVT first; never walk back to DISCONNECTING. return; } - // Passive disconnect: report now, but wait for CLOSE_EVT before going IDLE - - // reconnecting earlier makes the controller reject with 133 or assert. + // Passive disconnect: wait for CLOSE_EVT before going IDLE (reconnecting + // earlier makes the controller reject with 133 or assert) and before + // reporting - the wrapper frees the slot on the report, and a freed slot + // invites a reconnect into the still-closing link. this->release_services(); this->set_disconnecting_(); - this->report_connection_state_(false, param->disconnect.reason); } bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_gatt_if_t esp_gattc_if, @@ -556,9 +561,8 @@ bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_ga return false; this->release_services(); this->set_idle_(); - // The wrapper frees the slot on this final report; after a passive - // disconnect this is the second connected=false, matching the previous - // esp32 behavior (report at DISCONNECT, slot free at CLOSE). + // The one connected=false report: the wrapper frees the slot on it, + // so it must not fire before the controller finished closing. this->report_connection_state_(false, param->close.reason); break; } diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_esp32.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_esp32.cpp index 142b1b65d9..1c716a5605 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_esp32.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_esp32.cpp @@ -21,8 +21,7 @@ conn_err_t unpair_device(uint64_t address) { conn_err_t clear_gatt_cache(uint64_t address) { esp_bd_addr_t bda; ble_device_base::uint64_to_mac_msb_first(address, bda); - esp_ble_gattc_cache_clean(bda); - return CONN_OK; + return esp_ble_gattc_cache_clean(bda); } } // namespace esphome::bluetooth_connection diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index c2cf849348..dfe7054768 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -130,7 +130,10 @@ void BluetoothConnection::on_connection_state(bool connected, uint16_t mtu, int if (this->connection_type_ == ConnectionType::V3_WITH_CACHE) { // The API client has the services cached; never discover them. No // discovery phase needs the fast interval, so settle straight into the - // shared steady-state parameters (same lifecycle place as esp32). + // shared steady-state parameters. On esp32 the backend already set the + // same values as prefer-params before opening, so this request is + // usually redundant there - kept because rp2 has no prefer-params and + // the explicit update is its only path to the steady-state interval. this->state_ = ClientState::ESTABLISHED; int param_err = this->backend_->update_connection_params(ble_device_base::MEDIUM_MIN_CONN_INTERVAL, ble_device_base::MEDIUM_MAX_CONN_INTERVAL, 0, diff --git a/tests/components/ble_device_base/test_gatt_client_contract.cpp b/tests/components/ble_device_base/test_gatt_client_contract.cpp index 25b6cbf002..d92b5561c0 100644 --- a/tests/components/ble_device_base/test_gatt_client_contract.cpp +++ b/tests/components/ble_device_base/test_gatt_client_contract.cpp @@ -51,6 +51,9 @@ class MinimalConnection { } GattServiceTable get_service_table() { return {}; } void release_services() {} + bool disconnect_pending() const { return false; } + void cancel_pending_disconnect() {} + void set_connection_type(ConnectionType ct) {} protected: RecordingSink *listener_{nullptr};