diff --git a/esphome/components/bluetooth_connection/__init__.py b/esphome/components/bluetooth_connection/__init__.py index 191f833505..6ddd1ab9e1 100644 --- a/esphome/components/bluetooth_connection/__init__.py +++ b/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]) diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp index ebf579033b..c6f9372262 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp +++ b/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(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(); diff --git a/esphome/components/bluetooth_connection/btstack_memory_rp2.cpp b/esphome/components/bluetooth_connection/btstack_memory_rp2.cpp index aa73a1c743..5120fccf59 100644 --- a/esphome/components/bluetooth_connection/btstack_memory_rp2.cpp +++ b/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)); diff --git a/esphome/components/bluetooth_proxy/__init__.py b/esphome/components/bluetooth_proxy/__init__.py index 3a1ce7ff9d..36a4594c33 100644 --- a/esphome/components/bluetooth_proxy/__init__.py +++ b/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])