Merge branch 'esp32-proxy-flip' into radon-eye-neutral-gatt

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

esphome/components/ble_device_base/ble_gatt_client.h

@@ -188,6 +188,11 @@ concept BLEGattConnectionContract = requires(T conn, GattEventSink sink, const u
   { conn.update_connection_params(uint16_t{}, uint16_t{}, uint16_t{}, uint16_t{}) } -> std::same_as<int>;
   { conn.get_service_table() } -> std::same_as<GattServiceTable>;
   { conn.release_services() } -> std::same_as<void>;
+  // Deferred-disconnect visibility and the connection-type hint; backends
+  // without the underlying state carry inline no-ops.
+  { conn.disconnect_pending() } -> std::same_as<bool>;
+  { conn.cancel_pending_disconnect() } -> std::same_as<void>;
+  { conn.set_connection_type(ConnectionType{}) } -> std::same_as<void>;
 };
 
 // ---- service table lookup helpers ----

esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp

@@ -16,9 +16,6 @@
 #include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
 
-#ifndef CONFIG_ESP_HOSTED_ENABLE_BT_BLUEDROID
-#include <esp_bt.h>
-#endif
 #include <esp_gatt_common_api.h>
 
 #include <cstring>
@@ -96,6 +93,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
@@ -139,13 +143,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_);
@@ -441,10 +450,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;
   this->sink_.on_service_discovery_done(0);
 }
@@ -521,6 +540,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;
           }
@@ -534,8 +554,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;
@@ -549,6 +576,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;
               }
@@ -628,11 +656,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,
@@ -696,9 +725,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;
     }

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

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)

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);
   }
@@ -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,

tests/components/ble_device_base/test_gatt_client_contract.cpp

@@ -53,6 +53,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:
   GattEventSink sink_;