[bluetooth_proxy] Fix service discovery on disconnect and refactor connection handling (#9697)

esphome/components/bluetooth_proxy/bluetooth_connection.cpp

@@ -13,11 +13,180 @@ namespace bluetooth_proxy {
 
 static const char *const TAG = "bluetooth_proxy.connection";
 
+static std::vector<uint64_t> get_128bit_uuid_vec(esp_bt_uuid_t uuid_source) {
+  esp_bt_uuid_t uuid = espbt::ESPBTUUID::from_uuid(uuid_source).as_128bit().get_uuid();
+  return std::vector<uint64_t>{((uint64_t) uuid.uuid.uuid128[15] << 56) | ((uint64_t) uuid.uuid.uuid128[14] << 48) |
+                                   ((uint64_t) uuid.uuid.uuid128[13] << 40) | ((uint64_t) uuid.uuid.uuid128[12] << 32) |
+                                   ((uint64_t) uuid.uuid.uuid128[11] << 24) | ((uint64_t) uuid.uuid.uuid128[10] << 16) |
+                                   ((uint64_t) uuid.uuid.uuid128[9] << 8) | ((uint64_t) uuid.uuid.uuid128[8]),
+                               ((uint64_t) uuid.uuid.uuid128[7] << 56) | ((uint64_t) uuid.uuid.uuid128[6] << 48) |
+                                   ((uint64_t) uuid.uuid.uuid128[5] << 40) | ((uint64_t) uuid.uuid.uuid128[4] << 32) |
+                                   ((uint64_t) uuid.uuid.uuid128[3] << 24) | ((uint64_t) uuid.uuid.uuid128[2] << 16) |
+                                   ((uint64_t) uuid.uuid.uuid128[1] << 8) | ((uint64_t) uuid.uuid.uuid128[0])};
+}
+
 void BluetoothConnection::dump_config() {
   ESP_LOGCONFIG(TAG, "BLE Connection:");
   BLEClientBase::dump_config();
 }
 
+void BluetoothConnection::loop() {
+  BLEClientBase::loop();
+
+  // Early return if no active connection or not in service discovery phase
+  if (this->address_ == 0 || this->send_service_ < 0 || this->send_service_ > this->service_count_) {
+    return;
+  }
+
+  // Handle service discovery
+  this->send_service_for_discovery_();
+}
+
+void BluetoothConnection::reset_connection_(esp_err_t reason) {
+  // Send disconnection notification
+  this->proxy_->send_device_connection(this->address_, false, 0, reason);
+
+  // Important: If we were in the middle of sending services, we do NOT send
+  // send_gatt_services_done() here. This ensures the client knows that
+  // the service discovery was interrupted and can retry. The client
+  // (aioesphomeapi) implements a 30-second timeout (DEFAULT_BLE_TIMEOUT)
+  // to detect incomplete service discovery rather than relying on us to
+  // tell them about a partial list.
+  this->set_address(0);
+  this->send_service_ = DONE_SENDING_SERVICES;
+  this->proxy_->send_connections_free();
+}
+
+void BluetoothConnection::send_service_for_discovery_() {
+  if (this->send_service_ == this->service_count_) {
+    this->send_service_ = DONE_SENDING_SERVICES;
+    this->proxy_->send_gatt_services_done(this->address_);
+    if (this->connection_type_ == espbt::ConnectionType::V3_WITH_CACHE ||
+        this->connection_type_ == espbt::ConnectionType::V3_WITHOUT_CACHE) {
+      this->release_services();
+    }
+    return;
+  }
+
+  // Early return if no API connection
+  auto *api_conn = this->proxy_->get_api_connection();
+  if (api_conn == nullptr) {
+    return;
+  }
+
+  // Send next service
+  esp_gattc_service_elem_t service_result;
+  uint16_t service_count = 1;
+  esp_gatt_status_t service_status = esp_ble_gattc_get_service(this->gattc_if_, this->conn_id_, nullptr,
+                                                               &service_result, &service_count, this->send_service_);
+  this->send_service_++;
+
+  if (service_status != ESP_GATT_OK) {
+    ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_service error at offset=%d, status=%d", this->connection_index_,
+             this->address_str().c_str(), this->send_service_ - 1, service_status);
+    return;
+  }
+
+  if (service_count == 0) {
+    ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_service missing, service_count=%d", this->connection_index_,
+             this->address_str().c_str(), service_count);
+    return;
+  }
+
+  api::BluetoothGATTGetServicesResponse resp;
+  resp.address = this->address_;
+  resp.services.reserve(1);  // Always one service per response in this implementation
+  api::BluetoothGATTService service_resp;
+  service_resp.uuid = get_128bit_uuid_vec(service_result.uuid);
+  service_resp.handle = service_result.start_handle;
+
+  // Get the number of characteristics directly with one call
+  uint16_t total_char_count = 0;
+  esp_gatt_status_t char_count_status =
+      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, &total_char_count);
+
+  if (char_count_status == ESP_GATT_OK && total_char_count > 0) {
+    // Only reserve if we successfully got a count
+    service_resp.characteristics.reserve(total_char_count);
+  } else if (char_count_status != ESP_GATT_OK) {
+    ESP_LOGW(TAG, "[%d] [%s] Error getting characteristic count, status=%d", this->connection_index_,
+             this->address_str().c_str(), char_count_status);
+  }
+
+  // Now process characteristics
+  uint16_t char_offset = 0;
+  esp_gattc_char_elem_t char_result;
+  while (true) {  // characteristics
+    uint16_t char_count = 1;
+    esp_gatt_status_t char_status =
+        esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, service_result.start_handle,
+                                   service_result.end_handle, &char_result, &char_count, char_offset);
+    if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND) {
+      break;
+    }
+    if (char_status != ESP_GATT_OK) {
+      ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_char error, status=%d", this->connection_index_,
+               this->address_str().c_str(), char_status);
+      break;
+    }
+    if (char_count == 0) {
+      break;
+    }
+
+    api::BluetoothGATTCharacteristic characteristic_resp;
+    characteristic_resp.uuid = get_128bit_uuid_vec(char_result.uuid);
+    characteristic_resp.handle = char_result.char_handle;
+    characteristic_resp.properties = char_result.properties;
+    char_offset++;
+
+    // Get the number of descriptors directly with one call
+    uint16_t total_desc_count = 0;
+    esp_gatt_status_t desc_count_status =
+        esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_DESCRIPTOR, char_result.char_handle,
+                                     service_result.end_handle, 0, &total_desc_count);
+
+    if (desc_count_status == ESP_GATT_OK && total_desc_count > 0) {
+      // Only reserve if we successfully got a count
+      characteristic_resp.descriptors.reserve(total_desc_count);
+    } else if (desc_count_status != ESP_GATT_OK) {
+      ESP_LOGW(TAG, "[%d] [%s] Error getting descriptor count for char handle %d, status=%d", this->connection_index_,
+               this->address_str().c_str(), char_result.char_handle, desc_count_status);
+    }
+
+    // Now process descriptors
+    uint16_t desc_offset = 0;
+    esp_gattc_descr_elem_t desc_result;
+    while (true) {  // descriptors
+      uint16_t desc_count = 1;
+      esp_gatt_status_t desc_status = esp_ble_gattc_get_all_descr(
+          this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &desc_count, desc_offset);
+      if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) {
+        break;
+      }
+      if (desc_status != ESP_GATT_OK) {
+        ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", this->connection_index_,
+                 this->address_str().c_str(), desc_status);
+        break;
+      }
+      if (desc_count == 0) {
+        break;
+      }
+
+      api::BluetoothGATTDescriptor descriptor_resp;
+      descriptor_resp.uuid = get_128bit_uuid_vec(desc_result.uuid);
+      descriptor_resp.handle = desc_result.handle;
+      characteristic_resp.descriptors.push_back(std::move(descriptor_resp));
+      desc_offset++;
+    }
+    service_resp.characteristics.push_back(std::move(characteristic_resp));
+  }
+  resp.services.push_back(std::move(service_resp));
+
+  // Send the message (we already checked api_conn is not null at the beginning)
+  api_conn->send_message(resp, api::BluetoothGATTGetServicesResponse::MESSAGE_TYPE);
+}
+
 bool BluetoothConnection::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if,
                                               esp_ble_gattc_cb_param_t *param) {
   if (!BLEClientBase::gattc_event_handler(event, gattc_if, param))
@@ -25,22 +194,16 @@ bool BluetoothConnection::gattc_event_handler(esp_gattc_cb_event_t event, esp_ga
 
   switch (event) {
     case ESP_GATTC_DISCONNECT_EVT: {
-      this->proxy_->send_device_connection(this->address_, false, 0, param->disconnect.reason);
+      this->reset_connection_(param->disconnect.reason);
-      this->set_address(0);
-      this->proxy_->send_connections_free();
       break;
     }
     case ESP_GATTC_CLOSE_EVT: {
-      this->proxy_->send_device_connection(this->address_, false, 0, param->close.reason);
+      this->reset_connection_(param->close.reason);
-      this->set_address(0);
-      this->proxy_->send_connections_free();
       break;
     }
     case ESP_GATTC_OPEN_EVT: {
       if (param->open.status != ESP_GATT_OK && param->open.status != ESP_GATT_ALREADY_OPEN) {
-        this->proxy_->send_device_connection(this->address_, false, 0, param->open.status);
+        this->reset_connection_(param->open.status);
-        this->set_address(0);
-        this->proxy_->send_connections_free();
       } else if (this->connection_type_ == espbt::ConnectionType::V3_WITH_CACHE) {
         this->proxy_->send_device_connection(this->address_, true, this->mtu_);
         this->proxy_->send_connections_free();

esphome/components/bluetooth_proxy/bluetooth_connection.h

@@ -12,6 +12,7 @@ class BluetoothProxy;
 class BluetoothConnection : public esp32_ble_client::BLEClientBase {
  public:
   void dump_config() override;
+  void loop() override;
   bool gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if,
                            esp_ble_gattc_cb_param_t *param) override;
   void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) override;
@@ -27,6 +28,9 @@ class BluetoothConnection : public esp32_ble_client::BLEClientBase {
  protected:
   friend class BluetoothProxy;
 
+  void send_service_for_discovery_();
+  void reset_connection_(esp_err_t reason);
+
   // Memory optimized layout for 32-bit systems
   // Group 1: Pointers (4 bytes each, naturally aligned)
   BluetoothProxy *proxy_;

esphome/components/bluetooth_proxy/bluetooth_proxy.cpp

@@ -11,19 +11,6 @@ namespace esphome {
 namespace bluetooth_proxy {
 
 static const char *const TAG = "bluetooth_proxy";
-static const int DONE_SENDING_SERVICES = -2;
-
-std::vector<uint64_t> get_128bit_uuid_vec(esp_bt_uuid_t uuid_source) {
-  esp_bt_uuid_t uuid = espbt::ESPBTUUID::from_uuid(uuid_source).as_128bit().get_uuid();
-  return std::vector<uint64_t>{((uint64_t) uuid.uuid.uuid128[15] << 56) | ((uint64_t) uuid.uuid.uuid128[14] << 48) |
-                                   ((uint64_t) uuid.uuid.uuid128[13] << 40) | ((uint64_t) uuid.uuid.uuid128[12] << 32) |
-                                   ((uint64_t) uuid.uuid.uuid128[11] << 24) | ((uint64_t) uuid.uuid.uuid128[10] << 16) |
-                                   ((uint64_t) uuid.uuid.uuid128[9] << 8) | ((uint64_t) uuid.uuid.uuid128[8]),
-                               ((uint64_t) uuid.uuid.uuid128[7] << 56) | ((uint64_t) uuid.uuid.uuid128[6] << 48) |
-                                   ((uint64_t) uuid.uuid.uuid128[5] << 40) | ((uint64_t) uuid.uuid.uuid128[4] << 32) |
-                                   ((uint64_t) uuid.uuid.uuid128[3] << 24) | ((uint64_t) uuid.uuid.uuid128[2] << 16) |
-                                   ((uint64_t) uuid.uuid.uuid128[1] << 8) | ((uint64_t) uuid.uuid.uuid128[0])};
-}
 
 // Batch size for BLE advertisements to maximize WiFi efficiency
 // Each advertisement is up to 80 bytes when packaged (including protocol overhead)
@@ -213,130 +200,12 @@ void BluetoothProxy::loop() {
   }
 
   // Flush any pending BLE advertisements that have been accumulated but not yet sent
-  static uint32_t last_flush_time = 0;
   uint32_t now = App.get_loop_component_start_time();
 
   // Flush accumulated advertisements every 100ms
-  if (now - last_flush_time >= 100) {
+  if (now - this->last_advertisement_flush_time_ >= 100) {
     this->flush_pending_advertisements();
-    last_flush_time = now;
+    this->last_advertisement_flush_time_ = now;
-  }
-  for (auto *connection : this->connections_) {
-    if (connection->send_service_ == connection->service_count_) {
-      connection->send_service_ = DONE_SENDING_SERVICES;
-      this->send_gatt_services_done(connection->get_address());
-      if (connection->connection_type_ == espbt::ConnectionType::V3_WITH_CACHE ||
-          connection->connection_type_ == espbt::ConnectionType::V3_WITHOUT_CACHE) {
-        connection->release_services();
-      }
-    } else if (connection->send_service_ >= 0) {
-      esp_gattc_service_elem_t service_result;
-      uint16_t service_count = 1;
-      esp_gatt_status_t service_status =
-          esp_ble_gattc_get_service(connection->get_gattc_if(), connection->get_conn_id(), nullptr, &service_result,
-                                    &service_count, connection->send_service_);
-      connection->send_service_++;
-      if (service_status != ESP_GATT_OK) {
-        ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_service error at offset=%d, status=%d",
-                 connection->get_connection_index(), connection->address_str().c_str(), connection->send_service_ - 1,
-                 service_status);
-        continue;
-      }
-      if (service_count == 0) {
-        ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_service missing, service_count=%d",
-                 connection->get_connection_index(), connection->address_str().c_str(), service_count);
-        continue;
-      }
-      api::BluetoothGATTGetServicesResponse resp;
-      resp.address = connection->get_address();
-      resp.services.reserve(1);  // Always one service per response in this implementation
-      api::BluetoothGATTService service_resp;
-      service_resp.uuid = get_128bit_uuid_vec(service_result.uuid);
-      service_resp.handle = service_result.start_handle;
-      uint16_t char_offset = 0;
-      esp_gattc_char_elem_t char_result;
-      // Get the number of characteristics directly with one call
-      uint16_t total_char_count = 0;
-      esp_gatt_status_t char_count_status = esp_ble_gattc_get_attr_count(
-          connection->get_gattc_if(), connection->get_conn_id(), ESP_GATT_DB_CHARACTERISTIC,
-          service_result.start_handle, service_result.end_handle, 0, &total_char_count);
-
-      if (char_count_status == ESP_GATT_OK && total_char_count > 0) {
-        // Only reserve if we successfully got a count
-        service_resp.characteristics.reserve(total_char_count);
-      } else if (char_count_status != ESP_GATT_OK) {
-        ESP_LOGW(TAG, "[%d] [%s] Error getting characteristic count, status=%d", connection->get_connection_index(),
-                 connection->address_str().c_str(), char_count_status);
-      }
-
-      // Now process characteristics
-      while (true) {  // characteristics
-        uint16_t char_count = 1;
-        esp_gatt_status_t char_status = esp_ble_gattc_get_all_char(
-            connection->get_gattc_if(), connection->get_conn_id(), service_result.start_handle,
-            service_result.end_handle, &char_result, &char_count, char_offset);
-        if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND) {
-          break;
-        }
-        if (char_status != ESP_GATT_OK) {
-          ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_char error, status=%d", connection->get_connection_index(),
-                   connection->address_str().c_str(), char_status);
-          break;
-        }
-        if (char_count == 0) {
-          break;
-        }
-        api::BluetoothGATTCharacteristic characteristic_resp;
-        characteristic_resp.uuid = get_128bit_uuid_vec(char_result.uuid);
-        characteristic_resp.handle = char_result.char_handle;
-        characteristic_resp.properties = char_result.properties;
-        char_offset++;
-
-        // Get the number of descriptors directly with one call
-        uint16_t total_desc_count = 0;
-        esp_gatt_status_t desc_count_status =
-            esp_ble_gattc_get_attr_count(connection->get_gattc_if(), connection->get_conn_id(), ESP_GATT_DB_DESCRIPTOR,
-                                         char_result.char_handle, service_result.end_handle, 0, &total_desc_count);
-
-        if (desc_count_status == ESP_GATT_OK && total_desc_count > 0) {
-          // Only reserve if we successfully got a count
-          characteristic_resp.descriptors.reserve(total_desc_count);
-        } else if (desc_count_status != ESP_GATT_OK) {
-          ESP_LOGW(TAG, "[%d] [%s] Error getting descriptor count for char handle %d, status=%d",
-                   connection->get_connection_index(), connection->address_str().c_str(), char_result.char_handle,
-                   desc_count_status);
-        }
-
-        // Now process descriptors
-        uint16_t desc_offset = 0;
-        esp_gattc_descr_elem_t desc_result;
-        while (true) {  // descriptors
-          uint16_t desc_count = 1;
-          esp_gatt_status_t desc_status =
-              esp_ble_gattc_get_all_descr(connection->get_gattc_if(), connection->get_conn_id(),
-                                          char_result.char_handle, &desc_result, &desc_count, desc_offset);
-          if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) {
-            break;
-          }
-          if (desc_status != ESP_GATT_OK) {
-            ESP_LOGE(TAG, "[%d] [%s] esp_ble_gattc_get_all_descr error, status=%d", connection->get_connection_index(),
-                     connection->address_str().c_str(), desc_status);
-            break;
-          }
-          if (desc_count == 0) {
-            break;
-          }
-          api::BluetoothGATTDescriptor descriptor_resp;
-          descriptor_resp.uuid = get_128bit_uuid_vec(desc_result.uuid);
-          descriptor_resp.handle = desc_result.handle;
-          characteristic_resp.descriptors.push_back(std::move(descriptor_resp));
-          desc_offset++;
-        }
-        service_resp.characteristics.push_back(std::move(characteristic_resp));
-      }
-      resp.services.push_back(std::move(service_resp));
-      this->api_connection_->send_message(resp, api::BluetoothGATTGetServicesResponse::MESSAGE_TYPE);
-    }
   }
 }
 

esphome/components/bluetooth_proxy/bluetooth_proxy.h

@@ -22,6 +22,7 @@ namespace esphome {
 namespace bluetooth_proxy {
 
 static const esp_err_t ESP_GATT_NOT_CONNECTED = -1;
+static const int DONE_SENDING_SERVICES = -2;
 
 using namespace esp32_ble_client;
 
@@ -149,7 +150,10 @@ class BluetoothProxy : public esp32_ble_tracker::ESPBTDeviceListener, public Com
   std::vector<api::BluetoothLERawAdvertisement> advertisement_pool_;
   std::unique_ptr<api::BluetoothLERawAdvertisementsResponse> response_;
 
-  // Group 3: 1-byte types grouped together
+  // Group 3: 4-byte types
+  uint32_t last_advertisement_flush_time_{0};
+
+  // Group 4: 1-byte types grouped together
   bool active_;
   uint8_t advertisement_count_{0};
   // 2 bytes used, 2 bytes padding