[esp32_ble_tracker] Re-register GATT clients after ble.disable and ble.enable (#19068)

esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp

@@ -45,15 +45,7 @@ void BluedroidGattClient::setup() {
 
 void BluedroidGattClient::loop() {
   if (!esp32_ble::global_ble->is_active()) {
-    // Stack down: no CLOSE_EVT will come. Settle a live link so the consumer
-    // frees its slot, then re-register the app on the next enable.
-    auto down_st = this->state();
-    if (down_st != ClientState::IDLE && down_st != ClientState::INIT) {
-      this->release_services();
-      this->set_idle_();
-      this->listener_->on_connection_state(false, 0, ble_device_base::GATT_ERR_NOT_CONNECTED);
-    }
-    this->set_state(ClientState::INIT);
+    // ble_before_disabled_event_handler() settles the slot.
     return;
   }
   auto st = this->state();
@@ -65,7 +57,7 @@ void BluedroidGattClient::loop() {
       ESP_LOGE(TAG, "gattc app register failed: app_id=%d code=%d", this->app_id, ret);
       this->mark_failed();
     }
-    // Do not wait for REG_EVT; a dropped event must not wedge the slot.
+    // Do not wait for REG_EVT; connect() rejects until it lands.
     this->set_idle_();
   } else if (st == ClientState::DISCONNECTING || this->disconnect_pending()) {
     // The one teardown safety net: a lost CLOSE_EVT, or a scheduled
@@ -78,8 +70,8 @@ void BluedroidGattClient::loop() {
       this->listener_->on_connection_state(false, 0, ESP_GATT_CONN_TIMEOUT);
     }
   } else {
-    // The loop stays on while a link exists (stack-down watch, pre-started
-    // search flush); it settles only back at IDLE.
+    // The loop stays on while a link exists (pre-started search flush); it
+    // settles only back at IDLE.
     this->deliver_pending_search_();
     if (this->state() == ClientState::IDLE) {
       this->disable_loop();
@@ -87,6 +79,22 @@ void BluedroidGattClient::loop() {
   }
 }
 
+// Stack down: no CLOSE_EVT will come. Settle a live link so the consumer
+// frees its slot, then register the app again on the next enable.
+void BluedroidGattClient::ble_before_disabled_event_handler() {
+  auto st = this->state();
+  if (st != ClientState::IDLE && st != ClientState::INIT) {
+    this->release_services();
+    this->set_idle_();
+    this->listener_->on_connection_state(false, 0, ble_device_base::GATT_ERR_NOT_CONNECTED);
+  }
+  // The interface belongs to the torn-down stack.
+  this->gattc_if_ = ESP_GATT_IF_NONE;
+  this->set_state(ClientState::INIT);
+  // An idle slot runs no loop; the INIT branch must run to register again.
+  this->enable_loop();
+}
+
 void BluedroidGattClient::dump_config() {
   ESP_LOGCONFIG(TAG, "Bluedroid GATT client %d", this->connection_index_);
   if (this->is_failed()) {
@@ -97,6 +105,11 @@ void BluedroidGattClient::dump_config() {
 // ---- contract ops ----
 
 int BluedroidGattClient::connect(uint64_t address, uint8_t addr_type) {
+  if (this->gattc_if_ == ESP_GATT_IF_NONE) {
+    // Bluedroid drops an open on an unknown interface without any event.
+    ESP_LOGW(TAG, "[%d] Connect rejected, GATT app not registered", this->connection_index_);
+    return ble_device_base::GATT_ERR_NOT_CONNECTED;
+  }
   // Only from idle: clobbering DISCONNECTING would open a new link the
   // stale CLOSE_EVT then tears down.
   if (this->state() != ClientState::IDLE) {

esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h

@@ -56,6 +56,7 @@ class BluedroidGattClient final : public esp32_ble_tracker::ESPBTClient, public
   void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) override;
   void connect() override;
   void disconnect() override;
+  void ble_before_disabled_event_handler() override;
   bool wants_parsed_advertisements() override { return false; }
   void on_scan_end() override {}
   bool parse_device(const ble_device_base::ESPBTDevice &device) override { return false; }

esphome/components/esp32_ble/ble.cpp

@@ -83,18 +83,23 @@ void ESP32BLE::setup() {
   }
 }
 
-void ESP32BLE::enable() {
-  if (this->state_ != BLE_COMPONENT_STATE_DISABLED)
-    return;
-
-  this->state_ = BLE_COMPONENT_STATE_ENABLE;
-}
-
-void ESP32BLE::disable() {
-  if (this->state_ == BLE_COMPONENT_STATE_DISABLED)
-    return;
-
-  this->state_ = BLE_COMPONENT_STATE_DISABLE;
+// Queue the transition for loop(). A pending transition the other way is
+// cancelled instead, since nothing was torn down or brought up yet; any other
+// state is already there or on its way.
+void ESP32BLE::request_state_(bool enable) {
+  if (enable) {
+    if (this->state_ == BLE_COMPONENT_STATE_DISABLED) {
+      this->state_ = BLE_COMPONENT_STATE_ENABLE;
+    } else if (this->state_ == BLE_COMPONENT_STATE_DISABLE) {
+      this->state_ = BLE_COMPONENT_STATE_ACTIVE;
+    }
+  } else {
+    if (this->state_ == BLE_COMPONENT_STATE_ACTIVE) {
+      this->state_ = BLE_COMPONENT_STATE_DISABLE;
+    } else if (this->state_ == BLE_COMPONENT_STATE_ENABLE) {
+      this->state_ = BLE_COMPONENT_STATE_DISABLED;
+    }
+  }
 }
 
 #ifdef USE_ESP32_BLE_ADVERTISING
@@ -580,7 +585,11 @@ void ESP32BLE::loop_handle_state_transition_not_active_() {
       this->mark_failed();
       return;
     }
-    this->state_ = BLE_COMPONENT_STATE_DISABLED;
+    this->drain_ble_events_();
+    // A status callback may have asked for BLE back; the stack is down now, so
+    // that request becomes a bring-up.
+    this->state_ =
+        this->state_ == BLE_COMPONENT_STATE_ACTIVE ? BLE_COMPONENT_STATE_ENABLE : BLE_COMPONENT_STATE_DISABLED;
   } else if (this->state_ == BLE_COMPONENT_STATE_ENABLE) {
     ESP_LOGD(TAG, "Enabling");
     this->state_ = BLE_COMPONENT_STATE_OFF;

esphome/components/esp32_ble/ble.h

@@ -102,8 +102,8 @@ class ESP32BLE final : public Component {
   }
   uint32_t get_advertising_cycle_time() const { return this->advertising_cycle_time_; }
 
-  void enable();
-  void disable();
+  void enable() { this->request_state_(true); }
+  void disable() { this->request_state_(false); }
   ESPHOME_ALWAYS_INLINE bool is_active() { return this->state_ == BLE_COMPONENT_STATE_ACTIVE; }
   void setup() override;
   void loop() override;
@@ -176,6 +176,15 @@ class ESP32BLE final : public Component {
 
   bool ble_setup_();
   bool ble_dismantle_();
+  void request_state_(bool enable);
+  // Drop what the old stack queued; the next stack reuses the same interface ids.
+  void drain_ble_events_() {
+    BLEEvent *ble_event;
+    while ((ble_event = this->ble_events_.pop()) != nullptr) {
+      this->ble_event_pool_.release(ble_event);
+    }
+    this->ble_events_.get_and_reset_dropped_count();
+  }
   bool ble_pre_setup_();
 #ifdef USE_ESP32_BLE_ADVERTISING
   void advertising_init_();

esphome/components/esp32_ble_client/ble_client_base.cpp

@@ -42,7 +42,7 @@ void BLEClientBase::set_state(espbt::ClientState st) {
 
 void BLEClientBase::loop() {
   if (!esp32_ble::global_ble->is_active()) {
-    this->set_state(espbt::ClientState::INIT);
+    // ble_before_disabled_event_handler() resets the client.
     return;
   }
   if (this->state() == espbt::ClientState::INIT) {
@@ -72,6 +72,21 @@ void BLEClientBase::loop() {
 
 float BLEClientBase::get_setup_priority() const { return setup_priority::AFTER_BLUETOOTH; }
 
+void BLEClientBase::ble_before_disabled_event_handler() {
+  auto st = this->state();
+  if (st != espbt::ClientState::IDLE && st != espbt::ClientState::INIT) {
+    // No CLOSE_EVT will come: free the services and settle the link.
+    this->release_services();
+    this->set_idle_();
+    this->on_disconnect_complete(ESP_GATT_CONN_TERMINATE_LOCAL_HOST);
+  }
+  // The interface belongs to the torn-down stack.
+  this->gattc_if_ = ESP_GATT_IF_NONE;
+  this->set_state(espbt::ClientState::INIT);
+  // An idle client runs no loop; the INIT branch must run to register again.
+  this->enable_loop();
+}
+
 void BLEClientBase::dump_config() {
   ESP_LOGCONFIG(TAG,
                 "  Address: %s\n"
@@ -93,6 +108,10 @@ bool BLEClientBase::parse_device(const espbt::ESPBTDevice &device) {
     return false;
   if (this->state() != espbt::ClientState::IDLE)
     return false;
+  // Not registered on this stack yet; promoting now would stop the scan for a
+  // connect that connect() rejects anyway.
+  if (this->gattc_if_ == ESP_GATT_IF_NONE)
+    return false;
 
   this->log_event_("Found device");
   if (ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_DEBUG)
@@ -117,6 +136,15 @@ void BLEClientBase::connect() {
              this->connection_index_, this->address_str_);
     return;
   }
+  if (this->gattc_if_ == ESP_GATT_IF_NONE) {
+    // Bluedroid drops an open on an unknown interface without any event.
+    this->log_warning_("Connect rejected, GATT app not registered");
+    // INIT stays so loop() still registers; only a promoted client goes back.
+    if (this->state() == espbt::ClientState::DISCOVERED) {
+      this->set_state(espbt::ClientState::IDLE);
+    }
+    return;
+  }
   ESP_LOGI(TAG, "[%d] [%s] 0x%02x Connecting", this->connection_index_, this->address_str_, this->remote_addr_type_);
   this->paired_ = false;
   // A registration whose event never arrived must not block this connection's release.
@@ -199,7 +227,10 @@ void BLEClientBase::release_services() {
 #ifndef CONFIG_BT_GATTC_CACHE_NVS_FLASH
   // Only the cache clean makes the stack's database unsafe to walk.
   this->services_released_ = true;
-  esp_ble_gattc_cache_clean(this->remote_bda_);
+  // A stack on its way down frees its own cache.
+  if (esp32_ble::global_ble->is_active()) {
+    esp_ble_gattc_cache_clean(this->remote_bda_);
+  }
 #endif
 }
 

esphome/components/esp32_ble_client/ble_client_base.h

@@ -41,6 +41,7 @@ class BLEClientBase : public espbt::ESPBTClient, public Component {
   void connect() override;
   esp_err_t pair();
   void disconnect() override;
+  void ble_before_disabled_event_handler() override;
   void unconditional_disconnect();
   void release_services();
 
@@ -114,7 +115,7 @@ class BLEClientBase : public espbt::ESPBTClient, public Component {
 #endif
 
   // Group 3: 4-byte types
-  int gattc_if_;
+  int gattc_if_{ESP_GATT_IF_NONE};
   esp_gatt_status_t status_{ESP_GATT_OK};
 
   // Group 4: Arrays
@@ -139,7 +140,7 @@ class BLEClientBase : public espbt::ESPBTClient, public Component {
   uint8_t pending_notify_regs_{0};
   bool auto_connect_{false};
   bool paired_{false};
-  // Set only when release_services() cleans the stack's GATT cache, which no API may then walk
+  // Set by release_services() on RAM-cache builds; the stack's GATT database must not be walked after it
   bool services_released_{false};
   // 8 bytes used, no padding
 
@@ -155,10 +156,11 @@ class BLEClientBase : public espbt::ESPBTClient, public Component {
   void log_connection_params_(const char *param_type);
   void handle_connection_result_(esp_err_t ret);
   /// Hook called once a connection has been fully torn down (after release_services() and
-  /// set_idle_()), from both the CLOSE_EVT handler and the DISCONNECTING safety timeout.
+  /// set_idle_()): CLOSE_EVT, the DISCONNECTING safety timeout, or the BLE stack going down.
   /// Subclasses with extra per-connection accounting (e.g. bluetooth_proxy slot state)
-  /// override this to release that state. `reason` is the controller reason code, or
-  /// ESP_GATT_CONN_TIMEOUT for the safety-timeout path.
+  /// override this to release that state. `reason` is the controller reason code,
+  /// ESP_GATT_CONN_TIMEOUT for the safety timeout, or ESP_GATT_CONN_TERMINATE_LOCAL_HOST
+  /// for the stack going down.
   virtual void on_disconnect_complete(esp_err_t reason) {}
   /// Transition to IDLE and reset conn_id — call when the connection is fully dead.
   void set_idle_() {

esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp

@@ -74,11 +74,11 @@ void ESP32BLETracker::on_ota_global_state(ota::OTAState state, float progress, u
 
 void ESP32BLETracker::loop() {
   if (!this->parent_->is_active()) {
-    this->ble_was_disabled_ = true;
     return;
-  } else if (this->ble_was_disabled_) {
+  }
+  if (this->ble_was_disabled_) {
     this->ble_was_disabled_ = false;
-    // If the BLE stack was disabled, we need to start the scan again.
+    // First start after boot or after the stack came back.
     if (this->scan_continuous_) {
       this->start_scan();
     }
@@ -218,7 +218,27 @@ void ESP32BLETracker::stop_scan() {
   this->stop_scan_();
 }
 
-void ESP32BLETracker::ble_before_disabled_event_handler() { this->stop_scan_(); }
+void ESP32BLETracker::ble_before_disabled_event_handler() {
+  // Tell the controller to stop; a scan still starting has nothing to stop yet.
+  if (this->scanner_state_ == ScannerState::RUNNING || this->scanner_state_ == ScannerState::FAILED) {
+    this->stop_scan_();
+  }
+#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT
+  for (auto *client : this->clients_) {
+    client->ble_before_disabled_event_handler();
+  }
+  this->skip_next_scan_end_ = false;
+#endif
+  // The stop above never completes (stack torn down, events dropped); settle
+  // here so start_scan_() sees IDLE once the stack is back.
+  if (this->scanner_state_ != ScannerState::IDLE) {
+    this->cleanup_scan_state_(true);
+  }
+  // A failure latched by the old stack must not be handled against the next.
+  this->scan_start_failed_ = ESP_BT_STATUS_SUCCESS;
+  this->scan_set_param_failed_ = ESP_BT_STATUS_SUCCESS;
+  this->ble_was_disabled_ = true;
+}
 
 bool ESP32BLETracker::stop_scan_() {
   if (this->scanner_state_ != ScannerState::RUNNING && this->scanner_state_ != ScannerState::FAILED) {

esphome/components/esp32_ble_tracker/esp32_ble_tracker.h

@@ -113,6 +113,9 @@ class ESPBTClient : public ESPBTDeviceListener {
   virtual void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) = 0;
   virtual void connect() = 0;
   virtual void disconnect() = 0;
+  /// Called right before the BLE stack is dismantled. Nothing in flight will
+  /// complete, and the GATT app must register again once the stack is back.
+  virtual void ble_before_disabled_event_handler() {}
   bool disconnect_pending() const { return this->want_disconnect_; }
   void cancel_pending_disconnect() { this->want_disconnect_ = false; }