Simplify pool wrap init and connect serialization paths

esphome/components/bluetooth_connection/__init__.py

@@ -37,8 +37,8 @@ CODEOWNERS = ["@bdraco", "@jesserockz"]
 bluetooth_connection_ns = cg.esphome_ns.namespace("bluetooth_connection")
 
 # arduino-pico's prebuilt BTstack is compiled with MAX_NR_GATT_CLIENTS 1 and
-# MAX_NR_HCI_CONNECTIONS 2; for more than one slot, btstack_memory_rp2.cpp
-# replaces those pools via linker --wrap (add_btstack_pool_overrides), sized
+# MAX_NR_HCI_CONNECTIONS 2; for more than one backend, btstack_memory_rp2.cpp
+# replaces those pools via linker --wrap (emitted by _rp2_register), sized
 # from ESPHOME_BLE_GATT_CLIENT_COUNT. 3 matches the esp32 default and stays
 # within the controller's resources (MAX_NR_CONTROLLER_ACL_BUFFERS 3).
 RP2_MAX_CONNECTIONS = 3
@@ -56,6 +56,8 @@ BluedroidGattClient = bluetooth_connection_ns.class_(
 
 CONF_BACKEND_ID = "backend_id"
 
+DOMAIN = "bluetooth_connection"
+
 # The four btstack_memory accessors whose static pools are baked into the
 # prebuilt liblwip-bt.a; every internal use crosses an object boundary in the
 # archive, so --wrap intercepts them all (see btstack_memory_rp2.cpp).
@@ -67,15 +69,15 @@ _RP2_BTSTACK_POOL_SYMBOLS = (
 )
 
 
-def add_btstack_pool_overrides(slot_count: int) -> None:
-    """Emit the --wrap flags that swap the prebuilt BTstack pools for the
-    ESPHOME_BLE_GATT_CLIENT_COUNT-sized ones in btstack_memory_rp2.cpp.
-    No-op off rp2 and for single-slot builds, which fit the prebuilt pools
-    (and stay byte-identical to previous releases)."""
-    if not CORE.is_rp2 or slot_count <= 1:
-        return
-    for symbol in _RP2_BTSTACK_POOL_SYMBOLS:
-        cg.add_build_flag(f"-Wl,--wrap={symbol}")
+@dataclass
+class _ConnectionData:
+    rp2_backend_count: int = 0
+
+
+def _get_data() -> _ConnectionData:
+    if DOMAIN not in CORE.data:
+        CORE.data[DOMAIN] = _ConnectionData()
+    return CORE.data[DOMAIN]
 
 
 def _esp32_schema_fragment() -> cv.Schema:
@@ -103,6 +105,16 @@ async def _esp32_register(backend: cg.MockObj, config: ConfigType) -> None:
 async def _rp2_register(backend: cg.MockObj, config: ConfigType) -> None:
     from esphome.components import rp2040_ble
 
+    # More than one backend outgrows the prebuilt BTstack pools: swap them for
+    # the ESPHOME_BLE_GATT_CLIENT_COUNT-sized ones in btstack_memory_rp2.cpp.
+    # Keyed to backend registrations (the same event that grows the count that
+    # sizes the pools), so single-backend builds emit no flags and stay
+    # byte-identical to previous releases.
+    data = _get_data()
+    data.rp2_backend_count += 1
+    if data.rp2_backend_count == 2:
+        for symbol in _RP2_BTSTACK_POOL_SYMBOLS:
+            cg.add_build_flag(f"-Wl,--wrap={symbol}")
     await cg.register_parented(backend, config[rp2040_ble.CONF_RP2040_BLE_ID])
 
 

esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp

@@ -406,7 +406,6 @@ void RP2GattClient::loop() {
       ESP_LOGW(TAG, "Connect timeout (queued)");
       this->fail_connection_(HCI_REASON_CONNECTION_TIMEOUT);
     } else if (int err = this->try_gap_connect_(); err != 0) {
-      ESP_LOGW(TAG, "gap_connect failed, status=0x%02x", err);
       this->fail_connection_(static_cast<uint8_t>(err));
     }
   } else if (this->state_ == EngineState::CONNECTING || this->state_ == EngineState::MTU_EXCHANGE) {
@@ -414,7 +413,6 @@ void RP2GattClient::loop() {
     if (now - this->connect_started_ > CONNECT_TIMEOUT_MS) {
       ESP_LOGW(TAG, "Connect timeout");
       if (this->state_ == EngineState::CONNECTING && this->con_handle_ == HCI_CON_HANDLE_INVALID) {
-        bool cancel_sent = false;
         if (!this->connect_cancel_attempted_) {
           this->connect_cancel_attempted_ = true;
           BluetoothLock lock;
@@ -422,20 +420,17 @@ void RP2GattClient::loop() {
           // create-connection is in flight may issue it.
           if (connect_owner == this) {
             gap_connect_cancel();
-            cancel_sent = true;
+            // The cancel produces a connection-complete event with a failure
+            // status, which drives the normal failure path; restart the timer
+            // so a lost event escalates below instead of wedging here.
+            this->connect_started_ = now;
+            return;
           }
         }
-        if (cancel_sent) {
-          // The cancel produces a connection-complete event with a failure
-          // status, which drives the normal failure path; restart the timer
-          // so a lost event escalates below instead of wedging here.
-          this->connect_started_ = now;
-        } else {
-          // Not the owner (the completion resolved but its event was
-          // dropped), or the cancel's completion never arrived: reclaim the
-          // slot and the scan rather than cancelling forever.
-          this->fail_connection_(HCI_REASON_CONNECTION_TIMEOUT);
-        }
+        // Not the owner (the completion resolved but its event was dropped),
+        // or the cancel's completion never arrived: reclaim the slot and the
+        // scan rather than cancelling forever.
+        this->fail_connection_(HCI_REASON_CONNECTION_TIMEOUT);
       } else {
         // The link is up (MTU exchange stalled): tear it down properly so the
         // controller frees its side; the DISCONNECTING safety net below
@@ -870,7 +865,6 @@ int RP2GattClient::connect(uint64_t address, uint8_t addr_type) {
   // sum via a disconnect request).
   this->connect_started_ = millis();
   if (int err = this->try_gap_connect_(); err != 0) {
-    ESP_LOGW(TAG, "gap_connect failed, status=0x%02x", err);
     this->release_scan_inhibit_();
     return err;
   }
@@ -882,6 +876,13 @@ int RP2GattClient::connect(uint64_t address, uint8_t addr_type) {
 // engine owns it, otherwise park in CONNECT_PENDING for loop() to retry.
 // Returns nonzero only for hard failures (state untouched; caller cleans up).
 int RP2GattClient::try_gap_connect_() {
+  // Unlocked peek: single core, aligned pointer; a stale value costs one loop
+  // pass and the locked re-check below is authoritative. Keeps the per-loop
+  // pending retry from taking BluetoothLock just to find the radio busy.
+  if (connect_owner != nullptr) {
+    this->state_ = EngineState::CONNECT_PENDING;
+    return 0;
+  }
   uint8_t status;
   {
     BluetoothLock lock;
@@ -906,6 +907,7 @@ int RP2GattClient::try_gap_connect_() {
     this->state_ = EngineState::CONNECT_PENDING;
     return 0;
   }
+  ESP_LOGW(TAG, "gap_connect failed, status=0x%02x", status);
   return status;
 }
 
@@ -915,19 +917,10 @@ int RP2GattClient::gatt_disconnect() {
       return GATT_ERR_NOT_CONNECTED;
     case EngineState::DISCONNECTING:
       return 0;  // already on its way down
-    case EngineState::CONNECT_PENDING: {
-      // Nothing issued stack-side; complete via the queue like a refused
-      // gap_disconnect so the listener cannot re-enter disconnect mid-call.
-      {
-        BluetoothLock lock;
-        this->enqueue_event_irq_(RP2GattEvent::DISCONNECTED, HCI_REASON_CONNECTION_TIMEOUT, 0);
-      }
-      this->state_ = EngineState::DISCONNECTING;
-      this->disconnecting_started_ = millis();
-      this->release_scan_inhibit_();
-      this->enable_loop();
-      return 0;
-    }
+    case EngineState::CONNECT_PENDING:
+      // Nothing issued stack-side; the invalid handle takes the refused
+      // path below without touching the stack.
+      break;
     case EngineState::CONNECTING: {
       if (this->con_handle_ == HCI_CON_HANDLE_INVALID) {
         // The cancel can lose the race against a successful connection
@@ -950,20 +943,21 @@ int RP2GattClient::gatt_disconnect() {
     default:
       break;
   }
-  uint8_t status;
-  {
+  uint8_t status = ERROR_CODE_UNKNOWN_CONNECTION_IDENTIFIER;
+  if (this->con_handle_ != HCI_CON_HANDLE_INVALID) {
     BluetoothLock lock;
     status = gap_disconnect(this->con_handle_);
+    if (status != 0) {
+      ESP_LOGW(TAG, "gap_disconnect failed, status=0x%02x", status);
+    }
   }
   if (status != 0) {
-    // Refused (handle already gone): complete via the event queue so the
-    // listener cannot re-enter disconnect() mid-call. BluetoothLock stops
-    // the IRQ producer, so this main-loop push is SPSC-safe.
-    ESP_LOGW(TAG, "gap_disconnect failed, status=0x%02x", status);
-    {
-      BluetoothLock lock;
-      this->enqueue_event_irq_(RP2GattEvent::DISCONNECTED, HCI_REASON_CONNECTION_TIMEOUT, 0);
-    }
+    // Refused (handle already gone) or never issued (CONNECT_PENDING):
+    // complete via the event queue so the listener cannot re-enter
+    // disconnect mid-call. BluetoothLock stops the IRQ producer, so this
+    // main-loop push is SPSC-safe.
+    BluetoothLock lock;
+    this->enqueue_event_irq_(RP2GattEvent::DISCONNECTED, HCI_REASON_CONNECTION_TIMEOUT, 0);
   }
   this->state_ = EngineState::DISCONNECTING;
   this->disconnecting_started_ = millis();

esphome/components/bluetooth_connection/btstack_memory_rp2.cpp

@@ -2,9 +2,9 @@
 // arduino-pico's prebuilt liblwip-bt.a (built with MAX_NR_GATT_CLIENTS 1,
 // MAX_NR_HCI_CONNECTIONS 2) with pools sized from ESPHOME_BLE_GATT_CLIENT_COUNT.
 // bluetooth_connection/__init__.py emits the matching -Wl,--wrap flags only
-// when more than one connection slot is configured; single-slot builds emit no
-// flags and this file compiles to nothing, leaving the prebuilt pools in
-// charge. Layout safety: the framework defines ENABLE_CLASSIC / ENABLE_BLE for
+// when more than one backend registers; single-backend builds emit no flags
+// and this file compiles to nothing, leaving the prebuilt pools in charge.
+// Layout safety: the framework defines ENABLE_CLASSIC / ENABLE_BLE for
 // every user TU whenever PIO_FRAMEWORK_ARDUINO_ENABLE_BLUETOOTH is set
 // (rp2040_ble always sets it), so sizeof() here matches the archive.
 
@@ -19,34 +19,29 @@
 namespace esphome::bluetooth_connection {
 namespace {
 
-// One gatt_client_t per configured connection slot.
-constexpr int GATT_CLIENT_POOL_SIZE = ESPHOME_BLE_GATT_CLIENT_COUNT;
-// An hci_connection_t is held from gap_connect() to DISCONNECTION_COMPLETE
-// (scanning holds none); +1 mirrors the prebuilt library's own headroom
-// (2 connections for 1 GATT client) so a teardown/re-connect overlap can
-// never starve a slot.
+// One gatt_client_t per configured connection slot. An hci_connection_t is
+// held from gap_connect() to DISCONNECTION_COMPLETE (scanning holds none);
+// +1 mirrors the prebuilt library's own headroom (2 connections for 1 GATT
+// client) so a teardown/re-connect overlap can never starve a slot.
 constexpr int HCI_CONNECTION_POOL_SIZE = ESPHOME_BLE_GATT_CLIENT_COUNT + 1;
 
-// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables)
-gatt_client_t gatt_client_storage[GATT_CLIENT_POOL_SIZE];
+// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables,cert-err58-cpp)
+gatt_client_t gatt_client_storage[ESPHOME_BLE_GATT_CLIENT_COUNT];
 btstack_memory_pool_t gatt_client_pool;
 hci_connection_t hci_connection_storage[HCI_CONNECTION_POOL_SIZE];
 btstack_memory_pool_t hci_connection_pool;
-bool pools_ready = false;
-// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables)
 
-// Lazy one-time init instead of a global constructor: every caller (hci.c /
-// gatt_client.c inside the prebuilt archive) runs in BTstack's single
-// serialized context, so a plain flag is race-free and no static-init-order
-// hazard exists.
-void ensure_pools() {
-  if (pools_ready)
-    return;
-  btstack_memory_pool_create(&gatt_client_pool, gatt_client_storage, GATT_CLIENT_POOL_SIZE, sizeof(gatt_client_t));
-  btstack_memory_pool_create(&hci_connection_pool, hci_connection_storage, HCI_CONNECTION_POOL_SIZE,
-                             sizeof(hci_connection_t));
-  pools_ready = true;
-}
+// Static init: pool_create only links a free list through its own storage,
+// and BTstack first allocates long after static construction.
+struct PoolInit {
+  PoolInit() {
+    btstack_memory_pool_create(&gatt_client_pool, gatt_client_storage, ESPHOME_BLE_GATT_CLIENT_COUNT,
+                               sizeof(gatt_client_t));
+    btstack_memory_pool_create(&hci_connection_pool, hci_connection_storage, HCI_CONNECTION_POOL_SIZE,
+                               sizeof(hci_connection_t));
+  }
+} pool_init;
+// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables,cert-err58-cpp)
 
 }  // namespace
 
@@ -56,7 +51,6 @@ void ensure_pools() {
 extern "C" {
 
 gatt_client_t *__wrap_btstack_memory_gatt_client_get(void) {
-  ensure_pools();
   void *buffer = btstack_memory_pool_get(&gatt_client_pool);
   if (buffer != nullptr) {
     memset(buffer, 0, sizeof(gatt_client_t));
@@ -69,7 +63,6 @@ void __wrap_btstack_memory_gatt_client_free(gatt_client_t *gatt_client) {
 }
 
 hci_connection_t *__wrap_btstack_memory_hci_connection_get(void) {
-  ensure_pools();
   void *buffer = btstack_memory_pool_get(&hci_connection_pool);
   if (buffer != nullptr) {
     memset(buffer, 0, sizeof(hci_connection_t));

esphome/components/bluetooth_proxy/__init__.py

@@ -153,8 +153,8 @@ def _validate_no_active(config: ConfigType) -> ConfigType:
 def _rp2_config_schema() -> cv.All:
     """Full proxy on the rp2 BLE hub: active connections through the BTstack
     GATT client backend in bluetooth_connection. Multi-slot builds replace the
-    prebuilt library's one-client BTstack pools via linker --wrap
-    (bluetooth_connection.add_btstack_pool_overrides)."""
+    prebuilt library's one-client BTstack pools via linker --wrap (emitted by
+    bluetooth_connection's backend registration)."""
     connection_schema = bluetooth_connection.hub_connection_schema(PLATFORM_RP2)
 
     def populate_connections(config: ConfigType) -> ConfigType:
@@ -207,8 +207,6 @@ async def _connections_to_code(var: cg.MockObj, config: ConfigType) -> None:
     # this define whenever a proxy is present (zero on advertisement-only
     # hubs); sized here so it can never diverge from the loop below.
     cg.add_define("BLUETOOTH_PROXY_MAX_CONNECTIONS", len(connections))
-    # Multi-slot rp2 builds outgrow the prebuilt BTstack pools; no-op elsewhere.
-    bluetooth_connection.add_btstack_pool_overrides(len(connections))
     for connection_conf in connections:
         backend = await bluetooth_connection.new_gatt_backend(connection_conf)
         connection = cg.new_Pvariable(connection_conf[CONF_ID])