From 7218aa4803a7f38cdf8fc6bc2ea1903ab751ce3f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 8 Aug 2026 09:51:12 -0500 Subject: [PATCH] [bluetooth_connection] Explicit pairing for rp2 (#18166) --- .../ble_device_base/ble_gatt_client.h | 3 + .../bluetooth_connection.h | 15 +++- .../bluetooth_connection_hub.cpp | 11 +++ .../bluetooth_connection_hub.h | 5 ++ .../bluetooth_connection_rp2.cpp | 74 +++++++++++++++++++ .../bluetooth_connection_rp2.h | 5 ++ .../bluetooth_proxy/bluetooth_proxy.cpp | 18 +++-- .../esp32_ble_client/ble_client_base.h | 2 + .../test_gatt_client_contract.cpp | 10 +++ .../bluetooth_connection/__init__.py | 18 +++++ .../test_close_service_batch.cpp | 58 +++++++++++++++ 11 files changed, 209 insertions(+), 10 deletions(-) create mode 100644 tests/components/bluetooth_connection/__init__.py create mode 100644 tests/components/bluetooth_connection/test_close_service_batch.cpp diff --git a/esphome/components/ble_device_base/ble_gatt_client.h b/esphome/components/ble_device_base/ble_gatt_client.h index 1bcfcf99dc..37edc570ec 100644 --- a/esphome/components/ble_device_base/ble_gatt_client.h +++ b/esphome/components/ble_device_base/ble_gatt_client.h @@ -95,6 +95,7 @@ class GattClientEventListener { virtual void on_notify_state(uint16_t handle, bool enabled, int error) = 0; /// Notification/indication data from the peer. data/len valid during the call. virtual void on_notify_data(uint16_t handle, const uint8_t *data, uint16_t len) = 0; + virtual void on_pairing_result(int status) {} }; /// One GATT client connection slot. Operations return 0 when accepted @@ -123,6 +124,8 @@ class BLEGattConnection { /// handle. Local registration only — the CCCD write is the API client's /// responsibility (it arrives as a plain write_descriptor). virtual int notify_characteristic(uint16_t handle, bool enable) = 0; + /// Initiate pairing on the live link. Completion: on_pairing_result(). + virtual int pair() { return GATT_ERR_NOT_CONNECTED; } virtual int update_connection_params(uint16_t min_interval, uint16_t max_interval, uint16_t latency, uint16_t timeout) = 0; diff --git a/esphome/components/bluetooth_connection/bluetooth_connection.h b/esphome/components/bluetooth_connection/bluetooth_connection.h index 712251b157..2125d5b34f 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection.h @@ -48,20 +48,29 @@ static constexpr conn_err_t GATT_NOT_CONNECTED = ble_device_base::GATT_ERR_NOT_C // What the platform's connection backend supports beyond GATT operations; // the proxy derives its feature flags and legacy version from these. -#ifdef USE_ESP32 +#if defined(USE_ESP32) static constexpr bool SUPPORTS_PAIRING = true; static constexpr bool SUPPORTS_CACHE_CLEARING = true; +#elif defined(USE_RP2040_BLE) && defined(USE_BLE_GATT_CLIENT) +// The rp2 BTstack backend pairs (just works + bonding); it has no service +// cache to clear. Keyed on the backend, not the generic client define, so a +// future backend without pairing keeps the stub arm below. +static constexpr bool SUPPORTS_PAIRING = true; +static constexpr bool SUPPORTS_CACHE_CLEARING = false; #else static constexpr bool SUPPORTS_PAIRING = false; static constexpr bool SUPPORTS_CACHE_CLEARING = false; #endif // Address-scoped (not connection-scoped) maintenance requests. -#ifdef USE_ESP32 +#if defined(USE_ESP32) || (defined(USE_RP2040_BLE) && defined(USE_BLE_GATT_CLIENT)) conn_err_t unpair_device(uint64_t address); -conn_err_t clear_gatt_cache(uint64_t address); #else inline conn_err_t unpair_device(uint64_t) { return GATT_NOT_CONNECTED; } +#endif +#ifdef USE_ESP32 +conn_err_t clear_gatt_cache(uint64_t address); +#else inline conn_err_t clear_gatt_cache(uint64_t) { return GATT_NOT_CONNECTED; } #endif diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index 37d6b21dfe..b69a07fc31 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -74,6 +74,16 @@ void BluetoothConnection::check_disconnect_timeout_() { } } +void BluetoothConnection::on_pairing_result(int status) { + if (this->address_ == 0) { + // A drop before completion already answered: reset_connection_slot_ sends + // the connection response, which the client's pair watcher raises on. + return; + } + this->paired_ = status == 0; + this->proxy_->send_device_pairing(this->address_, status == 0, status); +} + void BluetoothConnection::reset_connection_(conn_err_t reason) { if (this->pending_error_ != 0) { reason = this->pending_error_; @@ -81,6 +91,7 @@ void BluetoothConnection::reset_connection_(conn_err_t reason) { } this->state_ = ClientState::IDLE; this->services_discovered_ = false; + this->paired_ = false; this->backend_->release_services(); this->proxy_->reset_connection_slot_(this, reason); } diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h index 83fbd24e4c..34e400ac01 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h @@ -49,6 +49,9 @@ class BluetoothConnection final : public ble_device_base::GattClientEventListene this->start_connect_(); } void disconnect(); + bool is_paired() const { return this->paired_; } + void set_unpaired() { this->paired_ = false; } + conn_err_t pair() { return this->backend_->pair(); } // A backend disconnect() is a single call that also cancels an in-progress // connect; there is no deferred-disconnect state to track. bool disconnect_pending() const { return false; } @@ -87,6 +90,7 @@ class BluetoothConnection final : public ble_device_base::GattClientEventListene void on_write_result(uint16_t handle, int error) override; void on_notify_state(uint16_t handle, bool enabled, int error) override; void on_notify_data(uint16_t handle, const uint8_t *data, uint16_t len) override; + void on_pairing_result(int status) override; protected: friend class bluetooth_proxy::BluetoothProxy; @@ -118,6 +122,7 @@ class BluetoothConnection final : public ble_device_base::GattClientEventListene // Group 5: 1-byte types ClientState state_{ClientState::IDLE}; + bool paired_{false}; ConnectionType connection_type_{ConnectionType::V1}; uint8_t remote_addr_type_{0}; uint8_t connection_index_{0}; diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp index 5eb3da0263..cd7577e7f7 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp @@ -1,4 +1,5 @@ #include "bluetooth_connection_rp2.h" +#include "bluetooth_connection.h" #if defined(USE_RP2040_BLE) && defined(USE_BLE_GATT_CLIENT) @@ -51,6 +52,7 @@ using ble_device_base::MEDIUM_MIN_CONN_INTERVAL; RP2GattClient *RP2GattClient::instances[ESPHOME_BLE_GATT_CLIENT_COUNT] = {}; uint8_t RP2GattClient::instance_count = 0; btstack_packet_callback_registration_t RP2GattClient::hci_event_registration = {}; +btstack_packet_callback_registration_t RP2GattClient::sm_event_registration = {}; // NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables) static ESPBTUUID uuid_from_btstack(uint16_t uuid16, const uint8_t uuid128[16]) { @@ -88,6 +90,8 @@ void RP2GattClient::setup() { if (hci_event_registration.callback == nullptr) { hci_event_registration.callback = &RP2GattClient::hci_packet_handler; hci_add_event_handler(&hci_event_registration); + sm_event_registration.callback = &RP2GattClient::sm_packet_handler; + sm_add_event_handler(&sm_event_registration); } } @@ -157,6 +161,38 @@ void RP2GattClient::hci_packet_handler(uint8_t type, uint16_t channel, uint8_t * } } +void RP2GattClient::sm_packet_handler(uint8_t type, uint16_t channel, uint8_t *packet, uint16_t size) { + if (type != HCI_EVENT_PACKET) { + return; + } + switch (hci_event_packet_get_type(packet)) { + case SM_EVENT_JUST_WORKS_REQUEST: + // Confirming from the SM callback is the intended BTstack pattern. + // Unscoped on purpose: no peripheral role exists in-tree, and scoping + // would drop a request racing the queued CONNECTED event. + sm_just_works_confirm(sm_event_just_works_request_get_handle(packet)); + break; + case SM_EVENT_PAIRING_COMPLETE: { + RP2GattClient *inst = instance_for_con_handle(sm_event_pairing_complete_get_handle(packet)); + if (inst != nullptr) { + inst->enqueue_event_irq_(RP2GattEvent::PAIRING_RESULT, sm_event_pairing_complete_get_status(packet), 0); + } + break; + } + case SM_EVENT_REENCRYPTION_COMPLETE: { + // A bonded peer re-encrypts instead of pairing; BTstack emits only this + // event on that path, so it answers the PAIR request too. + RP2GattClient *inst = instance_for_con_handle(sm_event_reencryption_complete_get_handle(packet)); + if (inst != nullptr) { + inst->enqueue_event_irq_(RP2GattEvent::PAIRING_RESULT, sm_event_reencryption_complete_get_status(packet), 0); + } + break; + } + default: + break; + } +} + void RP2GattClient::gatt_packet_handler(uint8_t type, uint16_t channel, uint8_t *packet, uint16_t size) { if (type != HCI_EVENT_PACKET) { return; @@ -447,6 +483,11 @@ void RP2GattClient::handle_event_(const RP2GattEvent &event) { case RP2GattEvent::WRITE_NO_RSP_DONE: this->finish_write_no_rsp_(event.status); break; + case RP2GattEvent::PAIRING_RESULT: + if (this->listener_ != nullptr) { + this->listener_->on_pairing_result(event.status); + } + break; } } @@ -1019,6 +1060,15 @@ int RP2GattClient::write_descriptor(uint16_t handle, const uint8_t *data, uint16 return 0; } +int RP2GattClient::pair() { + if (this->state_ != EngineState::READY) { + return GATT_ERR_NOT_CONNECTED; + } + BluetoothLock lock; + sm_request_pairing(this->con_handle_); // void API; completion via SM events + return 0; +} + int RP2GattClient::notify_characteristic(uint16_t handle, bool enable) { if (this->state_ != EngineState::READY) { return GATT_ERR_NOT_CONNECTED; @@ -1064,6 +1114,30 @@ int RP2GattClient::update_connection_params(uint16_t min_interval, uint16_t max_ return gap_update_connection_parameters(this->con_handle_, min_interval, max_interval, latency, timeout); } +conn_err_t unpair_device(uint64_t address) { + uint8_t mac[6]; + ble_device_base::uint64_to_mac_msb_first(address, mac); + bool found = false; + BluetoothLock lock; + // Exhaustive: the db keys on (type, address), so stale entries can share + // the same address bytes under different types. + for (int i = 0; i < le_device_db_max_count(); i++) { + int addr_type = 0; + bd_addr_t addr; + le_device_db_info(i, &addr_type, addr, nullptr); + if (addr_type != BD_ADDR_TYPE_UNKNOWN && memcmp(addr, mac, sizeof(bd_addr_t)) == 0) { + le_device_db_remove(i); + found = true; + } + } + if (found) { + return CONN_OK; + } + // No bond for this address; the shared error domain has no closer code + // (esp32 parity: its remove-bond call also errors for an unknown address). + return GATT_NOT_CONNECTED; +} + } // namespace esphome::bluetooth_connection #endif // USE_RP2040_BLE && USE_BLE_GATT_CLIENT diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h index 0c1bc95fe9..1a3671354d 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h @@ -51,6 +51,7 @@ struct RP2GattEvent { MTU_EXCHANGED, // value = negotiated MTU QUERY_COMPLETE, // status = ATT status of the finished query WRITE_NO_RSP_DONE, // status = result of the deferred write + PAIRING_RESULT, // status = SM pairing status (0 = bonded) }; Type type; uint8_t status; @@ -89,6 +90,7 @@ class RP2GattClient final : public Component, int read_descriptor(uint16_t handle) override; int write_descriptor(uint16_t handle, const uint8_t *data, uint16_t len) override; int notify_characteristic(uint16_t handle, bool enable) override; + int pair() override; int update_connection_params(uint16_t min_interval, uint16_t max_interval, uint16_t latency, uint16_t timeout) override; ble_device_base::GattServiceTable get_service_table() override; @@ -120,6 +122,7 @@ class RP2GattClient final : public Component, // BTstack packet handlers (IRQ context: copy-and-enqueue only). static void hci_packet_handler(uint8_t type, uint16_t channel, uint8_t *packet, uint16_t size); static void gatt_packet_handler(uint8_t type, uint16_t channel, uint8_t *packet, uint16_t size); + static void sm_packet_handler(uint8_t type, uint16_t channel, uint8_t *packet, uint16_t size); static RP2GattClient *instance_for_con_handle(hci_con_handle_t con_handle); void handle_gatt_event_irq_(uint8_t event_type, const uint8_t *packet); @@ -205,6 +208,8 @@ class RP2GattClient final : public Component, static uint8_t instance_count; // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static btstack_packet_callback_registration_t hci_event_registration; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + static btstack_packet_callback_registration_t sm_event_registration; }; } // namespace esphome::bluetooth_connection diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp index 19e894600e..56b79fe1b4 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp @@ -311,12 +311,13 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest break; } case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_PAIR: { -#ifdef USE_ESP32 + // Both connection classes expose the same pairing surface; success is + // reported when the platform's pairing completion arrives. auto *connection = this->get_connection_(msg.address, false); if (connection != nullptr) { if (!connection->is_paired()) { auto err = connection->pair(); - if (err != ESP_OK) { + if (err != CONN_OK) { this->send_device_pairing(msg.address, false, err); } } else { @@ -326,15 +327,18 @@ void BluetoothProxy::bluetooth_device_request(const api::BluetoothDeviceRequest // Answer instead of leaving the client to time out. this->send_device_pairing(msg.address, false, GATT_NOT_CONNECTED); } -#else - // Explicit pairing is not offered (FEATURE_PAIRING is not advertised); - // peripheral-initiated security still works through the platform's SM. - this->send_device_pairing(msg.address, false, GATT_NOT_CONNECTED); -#endif break; } case api::enums::BLUETOOTH_DEVICE_REQUEST_TYPE_UNPAIR: { conn_err_t ret = bluetooth_connection::unpair_device(msg.address); + if (ret == CONN_OK) { + // The bond is gone; a live connection must not short-circuit the + // next PAIR as already paired. + auto *connection = this->get_connection_(msg.address, false); + if (connection != nullptr) { + connection->set_unpaired(); + } + } this->send_device_unpairing(msg.address, ret == CONN_OK, ret); break; } diff --git a/esphome/components/esp32_ble_client/ble_client_base.h b/esphome/components/esp32_ble_client/ble_client_base.h index 0902aad924..e4b9cd5100 100644 --- a/esphome/components/esp32_ble_client/ble_client_base.h +++ b/esphome/components/esp32_ble_client/ble_client_base.h @@ -92,6 +92,8 @@ class BLEClientBase : public espbt::ESPBTClient, public Component { uint16_t get_conn_id() const { return this->conn_id_; } uint64_t get_address() const { return this->address_; } bool is_paired() const { return this->paired_; } + // The proxy clears this when a bond is removed while the link is up. + void set_unpaired() { this->paired_ = false; } uint8_t get_connection_index() const { return this->connection_index_; } diff --git a/tests/components/ble_device_base/test_gatt_client_contract.cpp b/tests/components/ble_device_base/test_gatt_client_contract.cpp index fb743b2699..b4491295db 100644 --- a/tests/components/ble_device_base/test_gatt_client_contract.cpp +++ b/tests/components/ble_device_base/test_gatt_client_contract.cpp @@ -46,6 +46,16 @@ class MinimalConnection : public BLEGattConnection { void release_services() override {} }; +TEST(BleGattClientContract, PairingDefaultsAreSafeForNonPairingBackends) { + // pair() defaults to not-connected and on_pairing_result() to a no-op, so + // a backend without pairing still answers the client through the dispatch. + MinimalConnection conn; + RecordingListener listener; + conn.set_listener(&listener); + EXPECT_EQ(conn.pair(), GATT_ERR_NOT_CONNECTED); + listener.on_pairing_result(0); // must not crash: default body +} + TEST(BleGattClientContract, MinimalImplementerCompilesAndRoutesEvents) { MinimalConnection connection; RecordingListener listener; diff --git a/tests/components/bluetooth_connection/__init__.py b/tests/components/bluetooth_connection/__init__.py new file mode 100644 index 0000000000..eae98931ec --- /dev/null +++ b/tests/components/bluetooth_connection/__init__.py @@ -0,0 +1,18 @@ +import esphome.codegen as cg +from tests.testing_helpers import ComponentManifestOverride + + +def override_manifest(manifest: ComponentManifestOverride) -> None: + # close_service_batch compiles only under BLUETOOTH_CONNECTION_HAS_GATT; + # emit the backend define so the host build exercises it. + async def to_code_testing(config): + # These defines are global to the merged host test binary; safe + # because no co-compiled test observes them. + cg.add_define("USE_BLE_GATT_CLIENT") + cg.add_define("USE_BLUETOOTH_PROXY") + cg.add_define("BLUETOOTH_PROXY_ADVERTISEMENT_BATCH_SIZE", 16) + cg.add_define("BLUETOOTH_PROXY_MAX_CONNECTIONS", 1) + + manifest.to_code = to_code_testing + # The batcher sizes api protobuf messages. + manifest.dependencies = manifest.dependencies + ["api"] diff --git a/tests/components/bluetooth_connection/test_close_service_batch.cpp b/tests/components/bluetooth_connection/test_close_service_batch.cpp new file mode 100644 index 0000000000..601ff9202b --- /dev/null +++ b/tests/components/bluetooth_connection/test_close_service_batch.cpp @@ -0,0 +1,58 @@ +#include "esphome/components/bluetooth_connection/bluetooth_connection.h" + +#include + +#include "esphome/components/api/api_pb2.h" + +namespace esphome::bluetooth_connection { + +// The three cursor behaviors: a fitting service advances and continues, an +// overflowing batch with >1 service pops and retries it, and a single +// oversized service is force-advanced so the stream cannot wedge. + +static void add_service(api::BluetoothGATTGetServicesResponse &resp, uint16_t characteristics) { + resp.services.emplace_back(); + auto &svc = resp.services.back(); + svc.handle = resp.services.size(); + svc.uuid = {0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL}; + svc.characteristics.init(characteristics); + for (uint16_t i = 0; i < characteristics; i++) { + auto &chr = svc.characteristics.emplace_back(); + chr.handle = 100 + i; + chr.properties = 0x12; + chr.uuid = {0x1234567890ABCDEFULL, 0xFEDCBA0987654321ULL}; + } +} + +TEST(CloseServiceBatch, FittingServiceAdvancesAndContinues) { + api::BluetoothGATTGetServicesResponse resp; + add_service(resp, 1); + size_t current_size = 0; + int16_t cursor = 0; + EXPECT_EQ(close_service_batch(resp, current_size, cursor, 0, "AA:BB"), BatchClose::CONTINUE); + EXPECT_EQ(cursor, 1); + EXPECT_GT(current_size, 0u); +} + +TEST(CloseServiceBatch, OverflowPopsAndRetriesWithoutAdvancing) { + api::BluetoothGATTGetServicesResponse resp; + add_service(resp, 1); + add_service(resp, 1); + size_t current_size = MAX_PACKET_SIZE - 10; // any service is bigger than 10 bytes + int16_t cursor = 5; + EXPECT_EQ(close_service_batch(resp, current_size, cursor, 0, "AA:BB"), BatchClose::SEND); + EXPECT_EQ(resp.services.size(), 1u); // popped for the next batch + EXPECT_EQ(cursor, 5); // not advanced: retried next batch +} + +TEST(CloseServiceBatch, SingleOversizedServiceForceAdvances) { + api::BluetoothGATTGetServicesResponse resp; + add_service(resp, 60); // ~30 bytes per characteristic, far past the budget + ASSERT_GT(resp.services.back().calculate_size(), MAX_PACKET_SIZE); + size_t current_size = 0; + int16_t cursor = 7; + EXPECT_EQ(close_service_batch(resp, current_size, cursor, 0, "AA:BB"), BatchClose::SEND); + EXPECT_EQ(cursor, 8); // advanced despite not fitting, so the stream moves on +} + +} // namespace esphome::bluetooth_connection