From 9db31db24d203f9d4f1c02d8dfaea800564a9826 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Aug 2026 00:19:39 -0500 Subject: [PATCH] Multi-consumer sink, table materializer, and review fixes for the backend --- .../ble_device_base/ble_gatt_client.h | 167 +++++++++-- .../bluetooth_connection_bluedroid.cpp | 265 +++++++++++++++--- .../bluetooth_connection_bluedroid.h | 50 +++- .../bluetooth_connection_gatt_backend.h | 9 +- .../bluetooth_connection_hub.cpp | 23 +- .../bluetooth_connection_hub.h | 43 +-- .../bluetooth_connection_rp2.cpp | 48 +--- .../bluetooth_connection_rp2.h | 11 +- .../test_gatt_client_contract.cpp | 79 +++++- 9 files changed, 535 insertions(+), 160 deletions(-) diff --git a/esphome/components/ble_device_base/ble_gatt_client.h b/esphome/components/ble_device_base/ble_gatt_client.h index 81d879a704..97322ec27e 100644 --- a/esphome/components/ble_device_base/ble_gatt_client.h +++ b/esphome/components/ble_device_base/ble_gatt_client.h @@ -5,8 +5,12 @@ // Exactly one GATT backend exists per build, so BLEGattConnection is a // compile-time alias (bluetooth_connection_gatt_backend.h), not an abstract // interface. -// The hub BluetoothConnection wrapper drives it and receives completions -// through its event-sink methods, which the backend calls directly. All sink +// A consumer - a streaming consumer that forwards the raw database (the hub +// BluetoothConnection wrapper) or a direct consumer owning a dedicated +// backend and resolving handles by UUID - drives it and receives +// completions through a GattEventSink — a pointer-sized-entry function table +// rather than a concrete consumer type, because one build can hold several +// consumer types while the backend stays a single non-virtual class. All sink // calls are delivered on the ESPHome main loop; borrowed data pointers are // valid only for the duration of the call. // @@ -78,6 +82,87 @@ struct GattServiceTable { uint16_t descriptor_count{0}; }; +// The event sink the backend calls directly, asserted where each consumer is +// defined: on_connection_state carries the negotiated MTU and an HCI +// status/disconnect reason. The requirements check call validity, not exact +// parameter types; keep sink parameters at the documented widths (uint16_t +// handles and lengths). +template +concept GattClientEventSinkContract = requires(S sink, const uint8_t *data) { + { sink.on_connection_state(true, uint16_t{}, int{}) } -> std::same_as; + { sink.on_service_discovery_done(int{}) } -> std::same_as; + { sink.on_read_result(uint16_t{}, data, uint16_t{}, int{}) } -> std::same_as; + { sink.on_write_result(uint16_t{}, int{}) } -> std::same_as; + { sink.on_notify_state(uint16_t{}, true, int{}) } -> std::same_as; + { sink.on_notify_data(uint16_t{}, data, uint16_t{}) } -> std::same_as; + { sink.on_pairing_result(int{}) } -> std::same_as; +}; + +/// One trampoline per event, shared by every instance of a consumer type. +struct GattEventVTable { + void (*connection_state)(void *, bool, uint16_t, int); + void (*service_discovery_done)(void *, int); + void (*read_result)(void *, uint16_t, const uint8_t *, uint16_t, int); + void (*write_result)(void *, uint16_t, int); + void (*notify_state)(void *, uint16_t, bool, int); + void (*notify_data)(void *, uint16_t, const uint8_t *, uint16_t); + void (*pairing_result)(void *, int); +}; + +// The per-consumer-type table lives in flash (constexpr), so a sink costs +// two pointers of RAM regardless of how many events the surface carries. +template +inline constexpr GattEventVTable GATT_EVENT_VTABLE{ + [](void *p, bool connected, uint16_t mtu, int error) { + static_cast(p)->on_connection_state(connected, mtu, error); + }, + [](void *p, int error) { static_cast(p)->on_service_discovery_done(error); }, + [](void *p, uint16_t handle, const uint8_t *data, uint16_t len, int error) { + static_cast(p)->on_read_result(handle, data, len, error); + }, + [](void *p, uint16_t handle, int error) { static_cast(p)->on_write_result(handle, error); }, + [](void *p, uint16_t handle, bool enabled, int error) { + static_cast(p)->on_notify_state(handle, enabled, error); + }, + [](void *p, uint16_t handle, const uint8_t *data, uint16_t len) { + static_cast(p)->on_notify_data(handle, data, len); + }, + [](void *p, int status) { static_cast(p)->on_pairing_result(status); }, +}; + +/// Type-erased consumer handle a backend delivers events through: an +/// instance pointer plus the consumer type's trampoline table. Built with +/// make_gatt_sink() from any type satisfying GattClientEventSinkContract; +/// call sites read the same as a direct listener call. No virtuals, no heap — +/// the cost of supporting several consumer types in one build is one +/// indirect call per event. Codegen wires the sink before setup(), so +/// backends may call without a null check. +struct GattEventSink { + void *instance{nullptr}; + const GattEventVTable *vtable{nullptr}; + + void on_connection_state(bool connected, uint16_t mtu, int error) const { + this->vtable->connection_state(this->instance, connected, mtu, error); + } + void on_service_discovery_done(int error) const { this->vtable->service_discovery_done(this->instance, error); } + void on_read_result(uint16_t handle, const uint8_t *data, uint16_t len, int error) const { + this->vtable->read_result(this->instance, handle, data, len, error); + } + void on_write_result(uint16_t handle, int error) const { this->vtable->write_result(this->instance, handle, error); } + void on_notify_state(uint16_t handle, bool enabled, int error) const { + this->vtable->notify_state(this->instance, handle, enabled, error); + } + void on_notify_data(uint16_t handle, const uint8_t *data, uint16_t len) const { + this->vtable->notify_data(this->instance, handle, data, len); + } + void on_pairing_result(int status) const { this->vtable->pairing_result(this->instance, status); } +}; + +template GattEventSink make_gatt_sink(T *consumer) { + static_assert(GattClientEventSinkContract, "the consumer is missing part of the event-sink surface"); + return {consumer, &GATT_EVENT_VTABLE}; +} + // The BLEGattConnection op surface, asserted where the alias binds // (bluetooth_connection_gatt_backend.h). Operations return 0 when accepted (completion arrives // through the sink) or a synchronous error (busy, not connected, stack @@ -88,17 +173,19 @@ struct GattServiceTable { // - notify_characteristic: local registration only; the CCCD write is the // API client's responsibility (a plain write_descriptor). // - get_service_table/release_services: backend-owned transient storage, -// released after streaming (release is idempotent). A backend may instead -// provide its own service streamer (stream_service_batch on the concrete -// type, detected by the consumer at compile time) and keep the table empty. +// released after streaming (release is idempotent). A backend may +// additionally provide its own service streamer (stream_service_batch on +// the concrete type, detected by the consumer at compile time) for +// arbitrary-size databases; the table then materializes only for consumers +// that ask for it. // - completions: connect and disconnect land in on_connection_state, // discover_services in on_service_discovery_done, pair in // on_pairing_result, reads in on_read_result, notify_characteristic in // on_notify_state, characteristic writes with response and descriptor // writes in on_write_result. -template -concept BLEGattConnectionContract = requires(T conn, Sink *sink, const uint8_t *data) { - conn.set_listener(sink); +template +concept BLEGattConnectionContract = requires(T conn, GattEventSink sink, const uint8_t *data) { + conn.set_sink(sink); { conn.connect(uint64_t{}, uint8_t{}) } -> std::same_as; { conn.disconnect() } -> std::same_as; { conn.discover_services() } -> std::same_as; @@ -111,23 +198,57 @@ concept BLEGattConnectionContract = requires(T conn, Sink *sink, const uint8_t * { conn.update_connection_params(uint16_t{}, uint16_t{}, uint16_t{}, uint16_t{}) } -> std::same_as; { conn.get_service_table() } -> std::same_as; { conn.release_services() } -> std::same_as; + // Deferred-disconnect visibility and the connection-type hint; backends + // without the underlying state carry inline no-ops. + { conn.disconnect_pending() } -> std::same_as; + { conn.cancel_pending_disconnect() } -> std::same_as; + { conn.set_connection_type(ConnectionType{}) } -> std::same_as; }; -// The event sink the backend calls directly (the hub BluetoothConnection -// wrapper), asserted where the wrapper is defined: on_connection_state -// carries the negotiated MTU and an HCI status/disconnect reason. The -// requirements check call validity, not exact parameter types; keep sink -// parameters at the documented widths (uint16_t handles and lengths). -template -concept GattClientEventSinkContract = requires(S sink, const uint8_t *data) { - { sink.on_connection_state(true, uint16_t{}, int{}) } -> std::same_as; - { sink.on_service_discovery_done(int{}) } -> std::same_as; - { sink.on_read_result(uint16_t{}, data, uint16_t{}, int{}) } -> std::same_as; - { sink.on_write_result(uint16_t{}, int{}) } -> std::same_as; - { sink.on_notify_state(uint16_t{}, true, int{}) } -> std::same_as; - { sink.on_notify_data(uint16_t{}, data, uint16_t{}) } -> std::same_as; - { sink.on_pairing_result(int{}) } -> std::same_as; -}; +// ---- service table lookup helpers ---- +// +// Neutral, bounds-checked walks over a materialized GattServiceTable for +// direct consumers that resolve a known device's handles by UUID (streaming +// consumers forward the raw database and never need these). Linear search: +// the table exists only between discovery and release_services(), for one +// small known device. + +/// Client Characteristic Configuration descriptor UUID (Bluetooth spec). +static constexpr uint16_t CCCD_UUID = 0x2902; + +inline const GattService *find_service(const GattServiceTable &table, const ESPBTUUID &uuid) { + for (uint16_t i = 0; i < table.service_count; i++) { + if (table.services[i].uuid == uuid) + return &table.services[i]; + } + return nullptr; +} + +inline const GattCharacteristic *find_characteristic(const GattServiceTable &table, const GattService &service, + const ESPBTUUID &uuid) { + uint16_t end = service.first_characteristic + service.characteristic_count; + if (end > table.characteristic_count) + return nullptr; + for (uint16_t i = service.first_characteristic; i < end; i++) { + if (table.characteristics[i].uuid == uuid) + return &table.characteristics[i]; + } + return nullptr; +} + +/// Handle of the characteristic's Client Characteristic Configuration +/// descriptor (0x2902), or 0 when it has none. +inline uint16_t find_cccd(const GattServiceTable &table, const GattCharacteristic &characteristic) { + uint16_t end = characteristic.first_descriptor + characteristic.descriptor_count; + if (end > table.descriptor_count) + return 0; + const ESPBTUUID cccd_uuid = ESPBTUUID::from_uint16(CCCD_UUID); + for (uint16_t i = characteristic.first_descriptor; i < end; i++) { + if (table.descriptors[i].uuid == cccd_uuid) + return table.descriptors[i].handle; + } + return 0; +} } // namespace esphome::ble_device_base diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index abd37868d0..7fb051b142 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -2,17 +2,20 @@ #if defined(USE_ESP32_BLE) && defined(USE_BLE_GATT_CLIENT) +// The in-place streamer serves the proxy's service-discovery API; backend-only +// builds compile without the proxy headers or the streamer. +#ifdef USE_BLUETOOTH_PROXY #include "bluetooth_connection.h" #include "bluetooth_connection_hub.h" #include "esphome/components/bluetooth_proxy/bluetooth_proxy.h" +#endif #include "esphome/components/ble_device_base/ble_client_state.h" #include "esphome/core/hal.h" #include "esphome/core/helpers.h" #include "esphome/core/log.h" -#include #include #include @@ -90,6 +93,13 @@ void BluedroidGattClient::dump_config() { // ---- contract ops ---- int BluedroidGattClient::connect(uint64_t address, uint8_t addr_type) { + // Refuse anything but a fully idle slot. Clobbering DISCONNECTING with + // DISCOVERED would let the tracker open a new link while the old one is + // still closing - the stale CLOSE_EVT then tears the new attempt down. + if (this->state_() != ClientState::IDLE) { + ESP_LOGW(TAG, "[%d] Connect rejected, slot busy", this->connection_index_); + return ESP_GATT_BUSY; + } ble_device_base::uint64_to_mac_msb_first(address, this->remote_bda_); this->remote_addr_type_ = addr_type; // Hand the request to the tracker's promote loop: it stops the scan, raises @@ -223,6 +233,9 @@ int BluedroidGattClient::update_connection_params(uint16_t min_interval, uint16_ void BluedroidGattClient::release_services() { this->service_total_ = 0; +#ifdef USE_BLE_GATT_SERVICE_TABLE + this->free_service_table_(); +#endif #ifndef CONFIG_BT_GATTC_CACHE_NVS_FLASH // Only the cache clean makes the stack's database unsafe to walk. this->services_released_ = true; @@ -230,6 +243,180 @@ void BluedroidGattClient::release_services() { #endif } +#ifdef USE_BLE_GATT_SERVICE_TABLE +ble_device_base::GattServiceTable BluedroidGattClient::get_service_table() { + if (this->table_storage_ == nullptr && + (this->services_released_ || this->service_total_ == 0 || !this->build_service_table_())) { + return {}; + } + return this->table_view_(); +} + +// The view is carved from the storage block and the counts on each call +// (a cold path) rather than cached, saving a per-instance table member. +ble_device_base::GattServiceTable BluedroidGattClient::table_view_() const { + size_t svc_bytes = this->service_total_ * sizeof(ble_device_base::GattService); + size_t char_bytes = this->table_char_total_ * sizeof(ble_device_base::GattCharacteristic); + return {reinterpret_cast(this->table_storage_), + reinterpret_cast(this->table_storage_ + svc_bytes), + reinterpret_cast(this->table_storage_ + svc_bytes + char_bytes), + this->service_total_, + this->table_char_total_, + this->table_desc_total_}; +} + +void BluedroidGattClient::free_service_table_() { + if (this->table_storage_ == nullptr) { + return; + } + RAMAllocator allocator(RAMAllocator::ALLOC_INTERNAL); + allocator.deallocate(this->table_storage_, 0); + this->table_storage_ = nullptr; + this->table_char_total_ = 0; + this->table_desc_total_ = 0; +} + +template +bool BluedroidGattClient::walk_database_(ServiceFn &&on_service, CharFn &&on_char, DescFn &&on_desc) { + // Shared enumeration for both table-build passes: an identical walk order + // is what lets the counting pass size the block the filling pass fills. + // INVALID_OFFSET/NOT_FOUND mean end-of-range; anything else is a failure. + for (uint16_t s = 0; s < this->service_total_; s++) { + esp_gattc_service_elem_t svc; + uint16_t svc_count = 1; + if (esp_ble_gattc_get_service(this->gattc_if_, this->conn_id_, nullptr, &svc, &svc_count, s) != ESP_GATT_OK || + svc_count == 0) { + return false; + } + if (!on_service(s, svc)) { + return false; + } + uint16_t svc_chars = 0; + if (esp_ble_gattc_get_attr_count(this->gattc_if_, this->conn_id_, ESP_GATT_DB_CHARACTERISTIC, svc.start_handle, + svc.end_handle, 0, &svc_chars) != ESP_GATT_OK) { + return false; + } + for (uint16_t c = 0; c < svc_chars; c++) { + esp_gattc_char_elem_t chr; + uint16_t char_count = 1; + auto status = esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, svc.start_handle, svc.end_handle, &chr, + &char_count, c); + if (status == ESP_GATT_INVALID_OFFSET || status == ESP_GATT_NOT_FOUND) { + break; + } + if (status != ESP_GATT_OK || char_count == 0) { + return false; + } + if (!on_char(svc, chr)) { + return false; + } + for (uint16_t d = 0;; d++) { + esp_gattc_descr_elem_t desc; + uint16_t desc_count = 1; + auto desc_status = + esp_ble_gattc_get_all_descr(this->gattc_if_, this->conn_id_, chr.char_handle, &desc, &desc_count, d); + if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { + break; + } + if (desc_status != ESP_GATT_OK || desc_count == 0) { + return false; + } + if (!on_desc(chr, desc)) { + return false; + } + } + } + } + return true; +} + +bool BluedroidGattClient::build_service_table_() { + // Pass 1: count, so one exact-size block holds the whole table. + uint16_t char_total = 0; + uint16_t desc_total = 0; + bool counted = this->walk_database_([](uint16_t, const esp_gattc_service_elem_t &) { return true; }, + [&](const esp_gattc_service_elem_t &, const esp_gattc_char_elem_t &) { + char_total++; + return true; + }, + [&](const esp_gattc_char_elem_t &, const esp_gattc_descr_elem_t &) { + desc_total++; + return true; + }); + if (!counted) { + return false; + } + + // The arrays share one block; carving stays aligned because each struct's + // strictest member is the UUID and array sizes are multiples of it. + size_t svc_bytes = this->service_total_ * sizeof(ble_device_base::GattService); + size_t char_bytes = char_total * sizeof(ble_device_base::GattCharacteristic); + size_t total_bytes = svc_bytes + char_bytes + desc_total * sizeof(ble_device_base::GattDescriptor); + RAMAllocator allocator(RAMAllocator::ALLOC_INTERNAL); + this->table_storage_ = allocator.allocate(total_bytes); + if (this->table_storage_ == nullptr) { + ESP_LOGW(TAG, "[%d] Service table allocation failed (%u bytes)", this->connection_index_, + static_cast(total_bytes)); + return false; + } + auto *services = reinterpret_cast(this->table_storage_); + auto *characteristics = reinterpret_cast(this->table_storage_ + svc_bytes); + auto *descriptors = + reinterpret_cast(this->table_storage_ + svc_bytes + char_bytes); + + // Pass 2: fill, bounded by the pass-1 totals. A bound trip or a shortfall + // means the cached database changed between the passes; fail the build + // rather than serve an inconsistent table (the consumer retries). + uint16_t char_index = 0; + uint16_t desc_index = 0; + ble_device_base::GattService *cur_service = nullptr; + ble_device_base::GattCharacteristic *cur_char = nullptr; + bool filled = this->walk_database_( + [&](uint16_t s, const esp_gattc_service_elem_t &svc) { + cur_service = &services[s]; + cur_service->uuid = ble_device_base::ESPBTUUID::from_uuid(svc.uuid); + cur_service->start_handle = svc.start_handle; + cur_service->end_handle = svc.end_handle; + cur_service->first_characteristic = char_index; + cur_service->characteristic_count = 0; + return true; + }, + [&](const esp_gattc_service_elem_t &svc, const esp_gattc_char_elem_t &chr) { + if (char_index >= char_total) { + return false; + } + cur_char = &characteristics[char_index++]; + cur_char->uuid = ble_device_base::ESPBTUUID::from_uuid(chr.uuid); + cur_char->value_handle = chr.char_handle; + // Bluedroid addresses descriptors by characteristic handle, so the + // table's end_handle only needs the service-bounded upper bound. + cur_char->end_handle = svc.end_handle; + cur_char->properties = chr.properties; + cur_char->first_descriptor = desc_index; + cur_char->descriptor_count = 0; + cur_service->characteristic_count++; + return true; + }, + [&](const esp_gattc_char_elem_t &, const esp_gattc_descr_elem_t &desc) { + if (desc_index >= desc_total) { + return false; + } + descriptors[desc_index].uuid = ble_device_base::ESPBTUUID::from_uuid(desc.uuid); + descriptors[desc_index].handle = desc.handle; + desc_index++; + cur_char->descriptor_count++; + return true; + }); + if (!filled || char_index != char_total || desc_index != desc_total) { + this->free_service_table_(); + return false; + } + this->table_char_total_ = char_total; + this->table_desc_total_ = desc_total; + return true; +} +#endif // USE_BLE_GATT_SERVICE_TABLE + // ---- internals ---- bool BluedroidGattClient::check_addr_(const esp_bd_addr_t &addr) const { @@ -249,9 +436,7 @@ void BluedroidGattClient::set_disconnecting_() { } void BluedroidGattClient::report_connection_state_(bool connected, int error) { - if (this->listener_ != nullptr) { - this->listener_->on_connection_state(connected, this->mtu_, error); - } + this->sink_.on_connection_state(connected, this->mtu_, error); } esp_err_t BluedroidGattClient::update_conn_params_(uint16_t min_interval, uint16_t max_interval, uint16_t latency, @@ -293,17 +478,14 @@ void BluedroidGattClient::handle_search_cmpl_() { // clients cache the streamed result permanently. auto status = primary_status != ESP_GATT_OK ? primary_status : secondary_status; this->log_gattc_warning_("esp_ble_gattc_get_attr_count", status); - if (this->listener_ != nullptr) { - this->listener_->on_service_discovery_done(status); - } + this->sink_.on_service_discovery_done(status); return; } this->service_total_ = primary + secondary; - if (this->listener_ != nullptr) { - this->listener_->on_service_discovery_done(0); - } + this->sink_.on_service_discovery_done(0); } +#ifdef USE_BLUETOOTH_PROXY void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { if (this->services_released_ || conn.send_service_ >= this->service_total_) { conn.send_service_ = DONE_SENDING_SERVICES; @@ -357,7 +539,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { resp.services.emplace_back(); auto &service_resp = resp.services.back(); fill_gatt_uuid(service_resp.uuid, service_resp.short_uuid, - esp32_ble_tracker::ESPBTUUID::from_uuid(service_result.uuid), use_efficient_uuids); + ble_device_base::ESPBTUUID::from_uuid(service_result.uuid), use_efficient_uuids); service_resp.handle = service_result.start_handle; if (total_char_count > 0) { @@ -384,7 +566,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { service_resp.characteristics.emplace_back(); auto &characteristic_resp = service_resp.characteristics.back(); fill_gatt_uuid(characteristic_resp.uuid, characteristic_resp.short_uuid, - esp32_ble_tracker::ESPBTUUID::from_uuid(char_result.uuid), use_efficient_uuids); + ble_device_base::ESPBTUUID::from_uuid(char_result.uuid), use_efficient_uuids); characteristic_resp.handle = char_result.char_handle; characteristic_resp.properties = char_result.properties; @@ -420,7 +602,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { characteristic_resp.descriptors.emplace_back(); auto &descriptor_resp = characteristic_resp.descriptors.back(); fill_gatt_uuid(descriptor_resp.uuid, descriptor_resp.short_uuid, - esp32_ble_tracker::ESPBTUUID::from_uuid(desc_result.uuid), use_efficient_uuids); + ble_device_base::ESPBTUUID::from_uuid(desc_result.uuid), use_efficient_uuids); descriptor_resp.handle = desc_result.handle; desc_offset++; } @@ -442,6 +624,7 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { conn.send_service_ = batch_start; } } +#endif // USE_BLUETOOTH_PROXY // ---- events ---- @@ -463,7 +646,7 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) { this->report_connection_state_(false, param->open.status); return; } - if (this->shim_.disconnect_scheduled()) { + if (this->shim_.disconnect_pending()) { // Earliest point conn_id_ exists; keep it set so CLOSE_EVT still matches. this->unconditional_disconnect_(); return; @@ -477,6 +660,9 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) { // matching the previous esp32 behavior). this->seen_mtu_ = true; this->report_connection_state_(true, 0); + // Settled: only the disconnect safety net needs the loop, and + // set_disconnecting_() re-enables it. + this->disable_loop(); } } @@ -490,11 +676,12 @@ void BluedroidGattClient::handle_disconnect_evt_(esp_ble_gattc_cb_param_t *param // Active close delivers CLOSE_EVT first; never walk back to DISCONNECTING. return; } - // Passive disconnect: report now, but wait for CLOSE_EVT before going IDLE - - // reconnecting earlier makes the controller reject with 133 or assert. + // Passive disconnect: wait for CLOSE_EVT before going IDLE (reconnecting + // earlier makes the controller reject with 133 or assert) and before + // reporting - the wrapper frees the slot on the report, and a freed slot + // invites a reconnect into the still-closing link. this->release_services(); this->set_disconnecting_(); - this->report_connection_state_(false, param->disconnect.reason); } bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_gatt_if_t esp_gattc_if, @@ -558,9 +745,8 @@ bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_ga return false; this->release_services(); this->set_idle_(); - // The wrapper frees the slot on this final report; after a passive - // disconnect this is the second connected=false, matching the previous - // esp32 behavior (report at DISCONNECT, slot free at CLOSE). + // The one connected=false report: the wrapper frees the slot on it, + // so it must not fire before the controller finished closing. this->report_connection_state_(false, param->close.reason); break; } @@ -570,52 +756,41 @@ bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_ga ESP_LOGI(TAG, "[%d] Service discovery complete", this->connection_index_); this->set_state_(ClientState::ESTABLISHED); this->handle_search_cmpl_(); + // Settled (see the V3_WITH_CACHE arm in handle_open_evt_). + this->disable_loop(); break; } case ESP_GATTC_READ_CHAR_EVT: case ESP_GATTC_READ_DESCR_EVT: { if (this->conn_id_ != param->read.conn_id) return false; - if (this->listener_ != nullptr) { - bool ok = param->read.status == ESP_GATT_OK; - this->listener_->on_read_result(param->read.handle, ok ? param->read.value : nullptr, - ok ? param->read.value_len : 0, ok ? 0 : param->read.status); - } + bool ok = param->read.status == ESP_GATT_OK; + this->sink_.on_read_result(param->read.handle, ok ? param->read.value : nullptr, ok ? param->read.value_len : 0, + ok ? 0 : param->read.status); break; } case ESP_GATTC_WRITE_CHAR_EVT: case ESP_GATTC_WRITE_DESCR_EVT: { if (this->conn_id_ != param->write.conn_id) return false; - if (this->listener_ != nullptr) { - this->listener_->on_write_result(param->write.handle, - param->write.status == ESP_GATT_OK ? 0 : param->write.status); - } + this->sink_.on_write_result(param->write.handle, param->write.status == ESP_GATT_OK ? 0 : param->write.status); break; } case ESP_GATTC_REG_FOR_NOTIFY_EVT: { - if (this->listener_ != nullptr) { - this->listener_->on_notify_state( - param->reg_for_notify.handle, true, - param->reg_for_notify.status == ESP_GATT_OK ? 0 : param->reg_for_notify.status); - } + this->sink_.on_notify_state(param->reg_for_notify.handle, true, + param->reg_for_notify.status == ESP_GATT_OK ? 0 : param->reg_for_notify.status); break; } case ESP_GATTC_UNREG_FOR_NOTIFY_EVT: { - if (this->listener_ != nullptr) { - this->listener_->on_notify_state( - param->unreg_for_notify.handle, false, - param->unreg_for_notify.status == ESP_GATT_OK ? 0 : param->unreg_for_notify.status); - } + this->sink_.on_notify_state(param->unreg_for_notify.handle, false, + param->unreg_for_notify.status == ESP_GATT_OK ? 0 : param->unreg_for_notify.status); break; } case ESP_GATTC_NOTIFY_EVT: { if (this->conn_id_ != param->notify.conn_id) return false; ESP_LOGV(TAG, "[%d] NOTIFY_EVT handle=0x%2X", this->connection_index_, param->notify.handle); - if (this->listener_ != nullptr) { - this->listener_->on_notify_data(param->notify.handle, param->notify.value, param->notify.value_len); - } + this->sink_.on_notify_data(param->notify.handle, param->notify.value, param->notify.value_len); break; } default: @@ -636,10 +811,8 @@ void BluedroidGattClient::handle_gap_event_(esp_gap_ble_cb_event_t event, esp_bl case ESP_GAP_BLE_AUTH_CMPL_EVT: { if (!this->check_addr_(param->ble_security.auth_cmpl.bd_addr)) break; - if (this->listener_ != nullptr) { - this->listener_->on_pairing_result( - param->ble_security.auth_cmpl.success ? 0 : param->ble_security.auth_cmpl.fail_reason); - } + this->sink_.on_pairing_result(param->ble_security.auth_cmpl.success ? 0 + : param->ble_security.auth_cmpl.fail_reason); break; } default: diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h index 2c50123b8f..0690fb6df3 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h @@ -22,8 +22,10 @@ namespace esphome::bluetooth_connection { -class BluetoothConnection; class BluedroidGattClient; +#ifdef USE_BLUETOOTH_PROXY +class BluetoothConnection; +#endif // The tracker-facing half: owns the ClientState the promote loop reads and // forwards events/commands to the engine. @@ -52,7 +54,8 @@ class BluedroidGattClient final : public Component { void dump_config() override; float get_setup_priority() const override { return setup_priority::AFTER_BLUETOOTH; } - void set_listener(BluetoothConnection *listener) { this->listener_ = listener; } + // Wired by codegen before setup and invariant for the device lifetime. + void set_sink(ble_device_base::GattEventSink sink) { this->sink_ = sink; } esp32_ble_tracker::ESPBTClient *tracker_client() { return &this->shim_; } // ---- ble_device_base::BLEGattConnection contract ---- @@ -66,17 +69,27 @@ class BluedroidGattClient final : public Component { int notify_characteristic(uint16_t handle, bool enable); int pair(); int update_connection_params(uint16_t min_interval, uint16_t max_interval, uint16_t latency, uint16_t timeout); - // Never called: this backend streams in place (stream_service_batch), so - // the table stays empty. Satisfies the contract concept. + // Materialized on demand from Bluedroid's cached database for direct + // consumers that resolve handles by UUID. The streaming consumer (the + // proxy wrapper) never calls this - it uses stream_service_batch - so the + // materializer only compiles when codegen declares a direct consumer + // (USE_BLE_GATT_SERVICE_TABLE) and proxy-only builds keep the old + // footprint; a direct consumer's peak is bounded by its one known device. +#ifdef USE_BLE_GATT_SERVICE_TABLE + ble_device_base::GattServiceTable get_service_table(); +#else ble_device_base::GattServiceTable get_service_table() { return {}; } +#endif void release_services(); - /// In-place service streamer (the wrapper detects and prefers it): builds - /// one api response batch directly from Bluedroid's cached database, so the - /// streaming peak is the response itself - the old esp32 model. +#ifdef USE_BLUETOOTH_PROXY + /// In-place service streamer (the proxy wrapper detects and prefers it): + /// builds one api response batch directly from Bluedroid's cached database, + /// so the streaming peak is the response itself - the old esp32 model. void stream_service_batch(BluetoothConnection &conn); +#endif - void set_connection_type(esp32_ble_tracker::ConnectionType ct) { this->connection_type_ = ct; } + void set_connection_type(ble_device_base::ConnectionType ct) { this->connection_type_ = ct; } bool disconnect_pending() const { return this->shim_.disconnect_pending(); } void cancel_pending_disconnect() { this->shim_.cancel_pending_disconnect(); } @@ -100,10 +113,24 @@ class BluedroidGattClient final : public Component { const char *param_type); int check_and_log_error_(const char *operation, esp_err_t err); void log_gattc_warning_(const char *operation, int code); +#ifdef USE_BLE_GATT_SERVICE_TABLE + template + bool walk_database_(ServiceFn &&on_service, CharFn &&on_char, DescFn &&on_desc); + bool build_service_table_(); + void free_service_table_(); + ble_device_base::GattServiceTable table_view_() const; +#endif // Group 1: pointers / composed objects BluedroidTrackerShim shim_{this}; - BluetoothConnection *listener_{nullptr}; + ble_device_base::GattEventSink sink_; +#ifdef USE_BLE_GATT_SERVICE_TABLE + // One exact-size block carved into the table's three arrays; owned here, + // freed by release_services(). Null when no table is materialized. The + // GattServiceTable view is rebuilt from this pointer and the counts on + // each (cold) get_service_table() call instead of being cached. + uint8_t *table_storage_{nullptr}; +#endif // Group 2: 4-byte types int gattc_if_{ESP_GATT_IF_NONE}; uint32_t disconnecting_started_{0}; @@ -115,6 +142,11 @@ class BluedroidGattClient final : public Component { uint16_t conn_id_{0xFFFF}; uint16_t mtu_{23}; uint16_t service_total_{0}; +#ifdef USE_BLE_GATT_SERVICE_TABLE + // Filled element counts of the materialized table (0 when none). + uint16_t table_char_total_{0}; + uint16_t table_desc_total_{0}; +#endif // Group 5: 1-byte types // Stored narrow (the enum is 4 bytes); widened at the esp_ble_gattc_open call. diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h b/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h index 58102ecfb3..55badf4e60 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h @@ -24,11 +24,9 @@ // build hits the #error below. namespace esphome::bluetooth_connection { -class BluetoothConnection; - class StubGattBackend { public: - void set_listener(BluetoothConnection *listener) {} + void set_sink(ble_device_base::GattEventSink sink) {} int connect(uint64_t address, uint8_t addr_type) { return ble_device_base::GATT_ERR_NOT_CONNECTED; } int disconnect() { return ble_device_base::GATT_ERR_NOT_CONNECTED; } int discover_services() { return ble_device_base::GATT_ERR_NOT_CONNECTED; } @@ -46,6 +44,9 @@ class StubGattBackend { return ble_device_base::GATT_ERR_NOT_CONNECTED; } ble_device_base::GattServiceTable get_service_table() { return {}; } + bool disconnect_pending() const { return false; } + void cancel_pending_disconnect() {} + void set_connection_type(ble_device_base::ConnectionType ct) {} void release_services() {} }; @@ -58,7 +59,7 @@ class StubGattBackend { namespace esphome::ble_device_base { using BLEGattConnection = ESPHOME_BLE_GATT_CONNECTION_TYPE; -static_assert(BLEGattConnectionContract, +static_assert(BLEGattConnectionContract, "The build's GATT backend is missing part of the BLEGattConnection surface (ble_gatt_client.h)"); #undef ESPHOME_BLE_GATT_CONNECTION_TYPE diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index 41e0ce48f1..374db952bf 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -1,7 +1,7 @@ -// Hub-platform connection wrapper (USE_RP2 hub builds today). +// The proxy's per-slot connection wrapper, shared by every platform. #include "bluetooth_connection_hub.h" -#if !defined(USE_ESP32) && defined(USE_BLE_GATT_CLIENT) +#ifdef USE_BLE_GATT_CLIENT #include "esphome/components/api/api_pb2.h" #include "esphome/components/bluetooth_proxy/bluetooth_proxy.h" @@ -27,7 +27,7 @@ void BluetoothConnection::set_address(uint64_t address) { } void BluetoothConnection::start_connect_() { - // No connect timeout here (esp32 parity): the client's own timeout or + // No connect timeout here: the API client's own timeout or // the api-gone sweep drives disconnect(). this->state_ = ClientState::CONNECTING; int err = this->backend_->connect(this->address_, this->remote_addr_type_); @@ -38,7 +38,7 @@ void BluetoothConnection::start_connect_() { } void BluetoothConnection::disconnect() { - // Idempotent like the esp32 class: the proxy's teardown loop calls this + // Idempotent: the proxy's teardown loop calls this // every 100 ms while the API subscriber is gone, and a repeat call must not // reach the backend (whose busy error would free the slot mid-teardown). if (this->state_ == ClientState::IDLE || this->state_ == ClientState::DISCONNECTING) { @@ -65,10 +65,10 @@ void BluetoothConnection::disconnect() { } void BluetoothConnection::check_disconnect_timeout_() { - // Safety net mirroring the esp32 base class: if the backend's disconnect - // completion is lost, force the slot free instead of leaking it. - if (this->state_ == ClientState::DISCONNECTING && - millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { + // Safety net: if the backend's disconnect completion is lost (or a refusal + // left the teardown unresolved), force the slot free instead of leaking it. + // The caller already gates on DISCONNECTING. + if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { ESP_LOGW(TAG, "[%d] [%s] Disconnect timeout, freeing slot", this->connection_index_, this->address_str_); this->reset_connection_(GATT_NOT_CONNECTED); } @@ -130,7 +130,10 @@ void BluetoothConnection::on_connection_state(bool connected, uint16_t mtu, int if (this->connection_type_ == ConnectionType::V3_WITH_CACHE) { // The API client has the services cached; never discover them. No // discovery phase needs the fast interval, so settle straight into the - // shared steady-state parameters (same lifecycle place as esp32). + // shared steady-state parameters. On esp32 the backend already set the + // same values as prefer-params before opening, so this request is + // usually redundant there - kept because rp2 has no prefer-params and + // the explicit update is its only path to the steady-state interval. this->state_ = ClientState::ESTABLISHED; int param_err = this->backend_->update_connection_params(ble_device_base::MEDIUM_MIN_CONN_INTERVAL, ble_device_base::MEDIUM_MAX_CONN_INTERVAL, 0, @@ -433,4 +436,4 @@ void BluetoothConnection::send_service_for_discovery_() { } // namespace esphome::bluetooth_connection -#endif // !USE_ESP32 && USE_BLE_GATT_CLIENT +#endif // USE_BLE_GATT_CLIENT diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h index 95aac31931..269a41f23f 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h @@ -1,14 +1,13 @@ -// Hub-platform BluetoothConnection: drives the build's GATT backend (the +// BluetoothConnection: drives the build's GATT backend (the // ble_device_base::BLEGattConnection alias) and translates its events into -// the same API messages the esp32 class emits. -// Presents the identical method surface, so the proxy's GATT dispatch -// compiles against either class unchanged. +// the proxy's API messages. One wrapper for every platform; per-backend +// differences live behind the alias and the streamer cut-through. #pragma once #include "esphome/core/defines.h" -#if !defined(USE_ESP32) && defined(USE_BLE_GATT_CLIENT) +#ifdef USE_BLE_GATT_CLIENT #include "bluetooth_connection.h" @@ -30,10 +29,10 @@ class BluetoothConnection final { /// Wire the platform backend. Called from codegen before setup. void set_backend(ble_device_base::BLEGattConnection *backend) { this->backend_ = backend; - backend->set_listener(this); + backend->set_sink(ble_device_base::make_gatt_sink(this)); } - // ---- proxy dispatch surface (mirrors the esp32 class) ---- + // ---- proxy dispatch surface ---- conn_err_t read_characteristic(uint16_t handle); conn_err_t write_characteristic(uint16_t handle, const uint8_t *data, size_t length, bool response); conn_err_t read_descriptor(uint16_t handle); @@ -52,10 +51,8 @@ class BluetoothConnection final { 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; } - void cancel_pending_disconnect() {} + bool disconnect_pending() const { return this->backend_->disconnect_pending(); } + void cancel_pending_disconnect() { this->backend_->cancel_pending_disconnect(); } void set_address(uint64_t address); uint64_t get_address() const { return this->address_; } @@ -65,22 +62,30 @@ class BluetoothConnection final { ClientState state() const { return this->state_; } void set_state(ClientState st) { this->state_ = st; } bool connected() const { return this->state_ == ClientState::ESTABLISHED; } - void set_connection_type(ConnectionType ct) { this->connection_type_ = ct; } + void set_connection_type(ConnectionType ct) { + this->connection_type_ = ct; + // The bluedroid backend branches on the type itself (prefer-params and + // the with-cache report at OPEN_EVT); the others ignore it. + this->backend_->set_connection_type(ct); + } // Latched at discovery completion rather than read from the backend table: // streaming frees the table, and this must stay true for the connection's - // lifetime (esp32 parity — a repeat GetServices is silently ignored there, - // never answered with an authoritative empty database). + // lifetime (a repeat GetServices is silently ignored, never answered with + // an authoritative empty database). bool has_gatt_services() const { return this->services_discovered_; } /// Stream any pending service-discovery batch and police the disconnect - /// safety timeout. Called from the proxy's loop — hub connections have no - /// Component loop of their own (the esp32 class streams from its own - /// loop() and has the same 10 s safety net in its base class). + /// safety timeout. Called from the proxy's loop — the wrapper has no + /// Component loop of its own. void process_pending_services() { if (this->send_service_ >= 0) { this->stream_pending_(this->backend_); } - this->check_disconnect_timeout_(); + // Inline state gate: this runs per loop iteration for every slot, and the + // 10 s safety net only matters while DISCONNECTING. + if (this->state_ == ClientState::DISCONNECTING) { + this->check_disconnect_timeout_(); + } } // ---- backend event sink (called directly by the backend, main loop) ---- @@ -146,4 +151,4 @@ static_assert(ble_device_base::GattClientEventSinkContract, } // namespace esphome::bluetooth_connection -#endif // !USE_ESP32 && USE_BLE_GATT_CLIENT +#endif // USE_BLE_GATT_CLIENT diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp index dc730659f5..33181ed748 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp @@ -1,6 +1,5 @@ #include "bluetooth_connection_rp2.h" -#include "bluetooth_connection_hub.h" #include "bluetooth_connection.h" #if defined(USE_RP2040_BLE) && defined(USE_BLE_GATT_CLIENT) @@ -26,7 +25,6 @@ using ble_device_base::GATT_ERR_NO_MEMORY; // and keeps the scan inhibited, so the engine cancels after 20 s. The // disconnect timeout mirrors the esp32 CLOSE_EVT safety net. static constexpr uint32_t CONNECT_TIMEOUT_MS = 20000; -static constexpr uint32_t DISCONNECT_TIMEOUT_MS = 10000; // Can-send windows normally open within a connection interval (tens of ms). static constexpr uint32_t WRITE_NO_RSP_TIMEOUT_MS = 500; @@ -384,8 +382,8 @@ void RP2GattClient::loop() { RP2GattNotifyEvent *notify; while ((notify = this->notify_queue_.pop()) != nullptr) { - if (this->listener_ != nullptr && this->notify_subscribed_(notify->handle)) { - this->listener_->on_notify_data(notify->handle, notify->data, notify->len); + if (this->notify_subscribed_(notify->handle)) { + this->sink_.on_notify_data(notify->handle, notify->data, notify->len); } this->notify_pool_.release(notify); } @@ -430,7 +428,7 @@ void RP2GattClient::loop() { } } } else if (this->state_ == EngineState::DISCONNECTING) { - if (millis() - this->disconnecting_started_ > DISCONNECT_TIMEOUT_MS) { + if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { ESP_LOGW(TAG, "Disconnect timeout, forcing idle"); this->handle_disconnected_(HCI_REASON_CONNECTION_TIMEOUT); } @@ -448,9 +446,7 @@ void RP2GattClient::loop() { } if (timed_out) { ESP_LOGW(TAG, "Deferred write timeout, handle=0x%04x", this->op_handle_); - if (this->listener_ != nullptr) { - this->listener_->on_write_result(this->op_handle_, GATT_CLIENT_BUSY); - } + this->sink_.on_write_result(this->op_handle_, GATT_CLIENT_BUSY); } } else if (this->state_ == EngineState::IDLE || (this->state_ == EngineState::READY && !this->op_in_flight_() && this->event_queue_.empty() && this->notify_queue_.empty())) { @@ -474,9 +470,7 @@ void RP2GattClient::handle_event_(const RP2GattEvent &event) { this->state_ = EngineState::READY; // Scanning resumes and runs alongside the established connection. this->release_scan_inhibit_(); - if (this->listener_ != nullptr) { - this->listener_->on_connection_state(true, this->mtu_, 0); - } + this->sink_.on_connection_state(true, this->mtu_, 0); } break; case RP2GattEvent::QUERY_COMPLETE: @@ -486,9 +480,7 @@ void RP2GattClient::handle_event_(const RP2GattEvent &event) { this->finish_write_no_rsp_(event.status); break; case RP2GattEvent::PAIRING_RESULT: - if (this->listener_ != nullptr) { - this->listener_->on_pairing_result(event.status); - } + this->sink_.on_pairing_result(event.status); break; } } @@ -514,9 +506,7 @@ void RP2GattClient::finish_write_no_rsp_(uint8_t status) { return; } this->op_type_ = OpType::NONE; - if (this->listener_ != nullptr) { - this->listener_->on_write_result(this->op_handle_, status); - } + this->sink_.on_write_result(this->op_handle_, status); } void RP2GattClient::handle_connected_(uint8_t status, uint16_t con_handle) { @@ -576,9 +566,7 @@ void RP2GattClient::fail_connection_(uint8_t reason) { this->cleanup_link_state_(); this->release_scan_inhibit_(); this->state_ = EngineState::IDLE; - if (this->listener_ != nullptr) { - this->listener_->on_connection_state(false, 0, reason); - } + this->sink_.on_connection_state(false, 0, reason); } void RP2GattClient::cleanup_link_state_() { @@ -621,9 +609,6 @@ void RP2GattClient::handle_query_complete_(uint8_t att_status) { if (this->op_type_ != OpType::NONE && this->op_type_ != OpType::WRITE_CHAR_NO_RSP) { OpType op = this->op_type_; this->op_type_ = OpType::NONE; - if (this->listener_ == nullptr) { - return; - } switch (op) { case OpType::READ_CHAR: case OpType::READ_DESC: @@ -634,12 +619,11 @@ void RP2GattClient::handle_query_complete_(uint8_t att_status) { this->op_len_ > 0) { att_status = 0; } - this->listener_->on_read_result(this->op_handle_, this->op_buffer_, att_status == 0 ? this->op_len_ : 0, - att_status); + this->sink_.on_read_result(this->op_handle_, this->op_buffer_, att_status == 0 ? this->op_len_ : 0, att_status); break; case OpType::WRITE_CHAR: case OpType::WRITE_DESC: - this->listener_->on_write_result(this->op_handle_, att_status); + this->sink_.on_write_result(this->op_handle_, att_status); break; default: break; @@ -796,9 +780,7 @@ void RP2GattClient::finish_discovery_(int error) { if (error != 0) { this->release_services(); } - if (this->listener_ != nullptr) { - this->listener_->on_service_discovery_done(error); - } + this->sink_.on_service_discovery_done(error); } ble_device_base::GattServiceTable RP2GattClient::get_service_table() { @@ -990,8 +972,8 @@ int RP2GattClient::write_characteristic(uint16_t handle, const uint8_t *data, ui return 0; } } - if (status == 0 && this->listener_ != nullptr) { - this->listener_->on_write_result(handle, 0); + if (status == 0) { + this->sink_.on_write_result(handle, 0); } return status; } @@ -1092,9 +1074,7 @@ int RP2GattClient::notify_characteristic(uint16_t handle, bool enable) { } } } - if (this->listener_ != nullptr) { - this->listener_->on_notify_state(handle, enable, 0); - } + this->sink_.on_notify_state(handle, enable, 0); return 0; } diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h index d5bf76e6ee..1e782cdd3a 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h @@ -26,8 +26,6 @@ namespace esphome::bluetooth_connection { -class BluetoothConnection; - // Caps for the transient service table. Sized generously for real devices // (typical peripherals expose < 8 services / < 30 characteristics); a peer // exceeding a cap fails discovery with INSUFFICIENT_RESOURCES rather than @@ -80,7 +78,7 @@ class RP2GattClient final : public Component, public Parentedlistener_ = listener; } + void set_sink(ble_device_base::GattEventSink sink) { this->sink_ = sink; } // ---- ble_device_base::BLEGattConnection contract ---- int connect(uint64_t address, uint8_t addr_type); @@ -94,6 +92,11 @@ class RP2GattClient final : public Component, public Parented event_queue_; esphome::EventPool event_pool_; 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 25b6cbf002..0206b6ecce 100644 --- a/tests/components/ble_device_base/test_gatt_client_contract.cpp +++ b/tests/components/ble_device_base/test_gatt_client_contract.cpp @@ -2,7 +2,7 @@ // configured; this TU pins it on the host so the header cannot rot unseen. // The contract is a concept (BLEGattConnection is a per-platform alias), so // the minimal backend here proves the concept stays satisfiable and routes -// events through the duck-typed sink the way a real backend does. +// events through the type-erased GattEventSink the way a real backend does. #define USE_BLE_GATT_CLIENT #include "esphome/components/ble_device_base/ble_gatt_client.h" @@ -15,33 +15,35 @@ struct RecordingSink { void on_connection_state(bool connected, uint16_t mtu, int error) { this->connected_ = connected; } void on_service_discovery_done(int error) { this->discovery_error_ = error; } void on_read_result(uint16_t handle, const uint8_t *data, uint16_t len, int error) {} - void on_write_result(uint16_t handle, int error) {} + void on_write_result(uint16_t handle, int error) { this->write_handle_ = handle; } void on_notify_state(uint16_t handle, bool enabled, int error) {} void on_notify_data(uint16_t handle, const uint8_t *data, uint16_t len) {} void on_pairing_result(int status) {} bool connected_{false}; int discovery_error_{0}; + uint16_t write_handle_{0}; }; static_assert(GattClientEventSinkContract, "the recording sink must cover the full event-sink surface"); class MinimalConnection { public: - void set_listener(RecordingSink *listener) { this->listener_ = listener; } + void set_sink(GattEventSink sink) { this->sink_ = sink; } int connect(uint64_t address, uint8_t addr_type) { - if (this->listener_ != nullptr) - this->listener_->on_connection_state(true, 517, 0); + this->sink_.on_connection_state(true, 517, 0); return 0; } int disconnect() { return 0; } int discover_services() { - if (this->listener_ != nullptr) - this->listener_->on_service_discovery_done(0); + this->sink_.on_service_discovery_done(0); return 0; } int read_characteristic(uint16_t handle) { return GATT_ERR_NOT_CONNECTED; } - int write_characteristic(uint16_t handle, const uint8_t *data, uint16_t len, bool response) { return 0; } + int write_characteristic(uint16_t handle, const uint8_t *data, uint16_t len, bool response) { + this->sink_.on_write_result(handle, 0); + return 0; + } int read_descriptor(uint16_t handle) { return 0; } int write_descriptor(uint16_t handle, const uint8_t *data, uint16_t len) { return 0; } int notify_characteristic(uint16_t handle, bool enable) { return 0; } @@ -51,23 +53,28 @@ class MinimalConnection { } GattServiceTable get_service_table() { return {}; } void release_services() {} + bool disconnect_pending() const { return false; } + void cancel_pending_disconnect() {} + void set_connection_type(ConnectionType ct) {} protected: - RecordingSink *listener_{nullptr}; + GattEventSink sink_; }; -static_assert(BLEGattConnectionContract, +static_assert(BLEGattConnectionContract, "a minimal backend must satisfy the contract the alias asserts"); TEST(BleGattClientContract, MinimalImplementerCompilesAndRoutesEvents) { MinimalConnection connection; RecordingSink listener; - connection.set_listener(&listener); + connection.set_sink(make_gatt_sink(&listener)); EXPECT_EQ(connection.connect(0xAABBCCDDEEFFULL, 0), 0); EXPECT_TRUE(listener.connected_); EXPECT_EQ(connection.discover_services(), 0); EXPECT_EQ(listener.discovery_error_, 0); EXPECT_EQ(connection.read_characteristic(1), GATT_ERR_NOT_CONNECTED); + EXPECT_EQ(connection.write_characteristic(7, nullptr, 0, true), 0); + EXPECT_EQ(listener.write_handle_, 7); // A default table is empty and safe to walk. GattServiceTable table = connection.get_service_table(); @@ -76,4 +83,54 @@ TEST(BleGattClientContract, MinimalImplementerCompilesAndRoutesEvents) { EXPECT_EQ(table.descriptor_count, 0); } +// A radon_eye_rd200-shaped table: two services, the second holding a +// notifying characteristic with a CCCD and a bare write characteristic. +class ServiceTableLookup : public ::testing::Test { + protected: + void SetUp() override { + this->services_[0] = {ESPBTUUID::from_uint16(0x1800), 0x0001, 0x0005, 0, 1}; + this->services_[1] = {ESPBTUUID::from_uint16(0x1523), 0x0010, 0x0020, 1, 2}; + this->characteristics_[0] = {ESPBTUUID::from_uint16(0x2A00), 0x0003, 0x0003, 0x02, 0, 0}; + this->characteristics_[1] = {ESPBTUUID::from_uint16(0x1525), 0x0012, 0x0014, 0x10, 0, 1}; + this->characteristics_[2] = {ESPBTUUID::from_uint16(0x1524), 0x0016, 0x0016, 0x04, 1, 0}; + this->descriptors_[0] = {ESPBTUUID::from_uint16(0x2902), 0x0013}; + this->table_ = {this->services_, this->characteristics_, this->descriptors_, 2, 3, 1}; + } + + GattService services_[2]; + GattCharacteristic characteristics_[3]; + GattDescriptor descriptors_[1]; + GattServiceTable table_; +}; + +TEST_F(ServiceTableLookup, FindsServicesAndCharacteristicsByUuid) { + const GattService *service = find_service(this->table_, ESPBTUUID::from_uint16(0x1523)); + ASSERT_NE(service, nullptr); + EXPECT_EQ(service->start_handle, 0x0010); + EXPECT_EQ(find_service(this->table_, ESPBTUUID::from_uint16(0xFFFF)), nullptr); + + const GattCharacteristic *characteristic = + find_characteristic(this->table_, *service, ESPBTUUID::from_uint16(0x1525)); + ASSERT_NE(characteristic, nullptr); + EXPECT_EQ(characteristic->value_handle, 0x0012); + // The lookup is scoped to the service: 0x2A00 lives in the other service. + EXPECT_EQ(find_characteristic(this->table_, *service, ESPBTUUID::from_uint16(0x2A00)), nullptr); +} + +TEST_F(ServiceTableLookup, FindsTheCccdAndReportsItsAbsence) { + const GattService *service = find_service(this->table_, ESPBTUUID::from_uint16(0x1523)); + const GattCharacteristic *notify_char = find_characteristic(this->table_, *service, ESPBTUUID::from_uint16(0x1525)); + EXPECT_EQ(find_cccd(this->table_, *notify_char), 0x0013); + const GattCharacteristic *write_char = find_characteristic(this->table_, *service, ESPBTUUID::from_uint16(0x1524)); + EXPECT_EQ(find_cccd(this->table_, *write_char), 0); +} + +TEST_F(ServiceTableLookup, RejectsRangesThatOverrunTheTable) { + // A corrupt index range must fail the lookup, not walk out of bounds. + GattService bad_service = {ESPBTUUID::from_uint16(0x1523), 0x0010, 0x0020, 2, 5}; + EXPECT_EQ(find_characteristic(this->table_, bad_service, ESPBTUUID::from_uint16(0x1524)), nullptr); + GattCharacteristic bad_char = {ESPBTUUID::from_uint16(0x1525), 0x0012, 0x0014, 0x10, 0, 9}; + EXPECT_EQ(find_cccd(this->table_, bad_char), 0); +} + } // namespace esphome::ble_device_base::testing