From d224a82e3835f58191c33e70c288a87ca43b77b4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Aug 2026 02:16:14 -0500 Subject: [PATCH] Fold the tracker shim into the backend and pass the MTU through --- .../ble_device_base/ble_gatt_client.h | 7 +-- .../bluetooth_connection_bluedroid.cpp | 48 ++++++++--------- .../bluetooth_connection_bluedroid.h | 51 ++++++++----------- .../bluetooth_connection_gatt_backend.h | 2 +- .../bluetooth_connection_hub.cpp | 6 +-- .../bluetooth_connection_rp2.cpp | 6 +-- .../bluetooth_connection_rp2.h | 2 +- .../test_gatt_client_contract.cpp | 2 +- 8 files changed, 57 insertions(+), 67 deletions(-) diff --git a/esphome/components/ble_device_base/ble_gatt_client.h b/esphome/components/ble_device_base/ble_gatt_client.h index 327f372f3a..6ba078f2af 100644 --- a/esphome/components/ble_device_base/ble_gatt_client.h +++ b/esphome/components/ble_device_base/ble_gatt_client.h @@ -110,7 +110,8 @@ class GattClientListener { // rejection); one operation may be outstanding at a time. Semantics beyond // the signatures: // - connect: addr_type is a BLE_ADDR_TYPE_* constant (ble_device.h). -// - disconnect: also cancels a connect in progress. +// - gatt_disconnect: also cancels a connect in progress (named to coexist +// with a platform stack's own void disconnect() on one backend class). // - 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, @@ -119,7 +120,7 @@ class GattClientListener { // 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, +// - completions: connect and gatt_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 @@ -128,7 +129,7 @@ template concept BLEGattConnectionContract = requires(T conn, GattClientListener *listener, const uint8_t *data) { conn.set_listener(listener); { conn.connect(uint64_t{}, uint8_t{}) } -> std::same_as; - { conn.disconnect() } -> std::same_as; + { conn.gatt_disconnect() } -> std::same_as; { conn.discover_services() } -> std::same_as; { conn.read_characteristic(uint16_t{}) } -> std::same_as; { conn.write_characteristic(uint16_t{}, data, uint16_t{}, true) } -> std::same_as; diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index 5cf3559611..e3f5a9254f 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -35,17 +35,17 @@ using esp32_ble_tracker::ConnectionType; static constexpr uint16_t UNSET_CONN_ID = 0xFFFF; -// ---- shim forwarders ---- +// ---- tracker surface ---- -bool BluedroidTrackerShim::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, - esp_ble_gattc_cb_param_t *param) { - return this->engine_->handle_gattc_event_(event, gattc_if, param); +bool BluedroidGattClient::gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, + esp_ble_gattc_cb_param_t *param) { + return this->handle_gattc_event_(event, gattc_if, param); } -void BluedroidTrackerShim::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { - this->engine_->handle_gap_event_(event, param); +void BluedroidGattClient::gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) { + this->handle_gap_event_(event, param); } -void BluedroidTrackerShim::connect() { this->engine_->tracker_connect_(); } -void BluedroidTrackerShim::disconnect() { this->engine_->disconnect(); } +void BluedroidGattClient::connect() { this->tracker_connect_(); } +void BluedroidGattClient::disconnect() { this->gatt_disconnect(); } // ---- component ---- @@ -62,9 +62,9 @@ void BluedroidGattClient::loop() { } auto st = this->state_(); if (st == ClientState::INIT) { - auto ret = esp_ble_gattc_app_register(this->shim_.app_id); + auto ret = esp_ble_gattc_app_register(this->app_id); if (ret) { - ESP_LOGE(TAG, "gattc app register failed: app_id=%d code=%d", this->shim_.app_id, ret); + ESP_LOGE(TAG, "gattc app register failed: app_id=%d code=%d", this->app_id, ret); this->mark_failed(); } // Do not wait for REG_EVT; a dropped event must not wedge the slot. @@ -79,7 +79,7 @@ void BluedroidGattClient::loop() { // lost CLOSE/DISCONNECT would otherwise leak the table and the cache. this->release_services(); this->set_idle_(); - this->report_connection_state_(false, ESP_GATT_CONN_TIMEOUT); + this->report_connection_state_(false, 0, ESP_GATT_CONN_TIMEOUT); } } @@ -137,11 +137,11 @@ void BluedroidGattClient::tracker_connect_() { this->log_gattc_warning_("esp_ble_gattc_open", ret); // CONNECT_EVT never fired, so conn_id_ is legitimately unset: plain IDLE. this->set_state_(ClientState::IDLE); - this->report_connection_state_(false, ret); + this->report_connection_state_(false, 0, ret); } } -int BluedroidGattClient::disconnect() { +int BluedroidGattClient::gatt_disconnect() { auto st = this->state_(); if (st == ClientState::DISCONNECTING) { return 0; @@ -158,7 +158,7 @@ int BluedroidGattClient::disconnect() { } if (st == ClientState::CONNECTING || this->conn_id_ == UNSET_CONN_ID) { ESP_LOGD(TAG, "[%d] Disconnect scheduled", this->connection_index_); - this->shim_.schedule_disconnect(); + this->want_disconnect_ = true; return 0; } this->unconditional_disconnect_(); @@ -437,8 +437,8 @@ void BluedroidGattClient::set_disconnecting_() { this->enable_loop(); } -void BluedroidGattClient::report_connection_state_(bool connected, int error) { - this->listener_->on_connection_state(connected, this->mtu_, error); +void BluedroidGattClient::report_connection_state_(bool connected, uint16_t mtu, int error) { + this->listener_->on_connection_state(connected, mtu, error); } esp_err_t BluedroidGattClient::update_conn_params_(uint16_t min_interval, uint16_t max_interval, uint16_t latency, @@ -652,10 +652,10 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) { this->log_gattc_warning_("Connection open", param->open.status); // Never established, CLOSE_EVT may not follow. this->set_idle_(); - this->report_connection_state_(false, param->open.status); + this->report_connection_state_(false, 0, param->open.status); return; } - if (this->shim_.disconnect_pending()) { + if (this->disconnect_pending()) { // Earliest point conn_id_ exists; keep it set so CLOSE_EVT still matches. this->unconditional_disconnect_(); return; @@ -668,7 +668,7 @@ void BluedroidGattClient::handle_open_evt_(esp_ble_gattc_cb_param_t *param) { // suppressed by seen_mtu_ (HA tolerates a post-connect MTU of 23 here, // matching the previous esp32 behavior). this->seen_mtu_ = true; - this->report_connection_state_(true, 0); + this->report_connection_state_(true, 0, 0); // Settled: only the disconnect safety net needs the loop, and // set_disconnecting_() re-enables it. this->disable_loop(); @@ -695,7 +695,7 @@ void BluedroidGattClient::handle_disconnect_evt_(esp_ble_gattc_cb_param_t *param bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_gatt_if_t esp_gattc_if, esp_ble_gattc_cb_param_t *param) { - if (event == ESP_GATTC_REG_EVT && this->shim_.app_id != param->reg.app_id) + if (event == ESP_GATTC_REG_EVT && this->app_id != param->reg.app_id) return false; if (event != ESP_GATTC_REG_EVT && esp_gattc_if != ESP_GATT_IF_NONE && esp_gattc_if != this->gattc_if_) return false; @@ -734,12 +734,12 @@ bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_ga // Warn only; a disconnect will follow if the link is dead. this->log_gattc_warning_("MTU exchange", param->cfg_mtu.status); } else { - this->mtu_ = param->cfg_mtu.mtu; } if (!this->seen_mtu_) { this->seen_mtu_ = true; - // The connected report waited for the MTU so HA never sees 23. - this->report_connection_state_(true, 0); + // The connected report waited for the MTU so HA never sees 23; the + // value is forwarded rather than stored (the consumer keeps it). + this->report_connection_state_(true, param->cfg_mtu.status == ESP_GATT_OK ? param->cfg_mtu.mtu : 23, 0); } break; } @@ -756,7 +756,7 @@ bool BluedroidGattClient::handle_gattc_event_(esp_gattc_cb_event_t event, esp_ga this->set_idle_(); // 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); + this->report_connection_state_(false, 0, param->close.reason); break; } case ESP_GATTC_SEARCH_CMPL_EVT: { diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h index 500c0a3f83..6f1c3d4bc3 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.h @@ -27,11 +27,23 @@ class BluedroidGattClient; class BluetoothConnection; #endif -// The tracker-facing half: owns the ClientState the promote loop reads and -// forwards events/commands to the engine. -class BluedroidTrackerShim final : public esp32_ble_tracker::ESPBTClient { +// One class carries both halves: the tracker's ESPBTClient surface (its +// promote loop owns scan-stop/coex/one-connect-at-a-time and calls the +// virtual connect()/disconnect()) and the neutral contract ops. The +// contract's teardown op is named gatt_disconnect() because the tracker's +// void disconnect() cannot overload with an int-returning twin. +class BluedroidGattClient final : public esp32_ble_tracker::ESPBTClient, public Component { public: - explicit BluedroidTrackerShim(BluedroidGattClient *engine) : engine_(engine) {} + void setup() override; + void loop() override; + void dump_config() override; + float get_setup_priority() const override { return setup_priority::AFTER_BLUETOOTH; } + + // Wired by codegen before setup and invariant for the device lifetime. + void set_listener(ble_device_base::GattClientListener *listener) { this->listener_ = listener; } + esp32_ble_tracker::ESPBTClient *tracker_client() { return this; } + + // ---- esp32_ble_tracker::ESPBTClient ---- bool gattc_event_handler(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param) override; void gap_event_handler(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t *param) override; @@ -41,26 +53,9 @@ class BluedroidTrackerShim final : public esp32_ble_tracker::ESPBTClient { void on_scan_end() override {} bool parse_device(const ble_device_base::ESPBTDevice &device) override { return false; } - void schedule_disconnect() { this->want_disconnect_ = true; } - - protected: - BluedroidGattClient *engine_; -}; - -class BluedroidGattClient final : public Component { - public: - void setup() override; - void loop() override; - void dump_config() override; - float get_setup_priority() const override { return setup_priority::AFTER_BLUETOOTH; } - - // Wired by codegen before setup and invariant for the device lifetime. - void set_listener(ble_device_base::GattClientListener *listener) { this->listener_ = listener; } - esp32_ble_tracker::ESPBTClient *tracker_client() { return &this->shim_; } - // ---- ble_device_base::BLEGattConnection contract ---- int connect(uint64_t address, uint8_t addr_type); - int disconnect(); + int gatt_disconnect(); int discover_services(); int read_characteristic(uint16_t handle); int write_characteristic(uint16_t handle, const uint8_t *data, uint16_t len, bool response); @@ -90,14 +85,10 @@ class BluedroidGattClient final : public Component { #endif 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(); } protected: - friend class BluedroidTrackerShim; - - esp32_ble_tracker::ClientState state_() const { return this->shim_.state(); } - void set_state_(esp32_ble_tracker::ClientState st) { this->shim_.set_state(st); } + esp32_ble_tracker::ClientState state_() const { return this->state(); } + void set_state_(esp32_ble_tracker::ClientState st) { this->set_state(st); } bool check_addr_(const esp_bd_addr_t &addr) const; void tracker_connect_(); bool handle_gattc_event_(esp_gattc_cb_event_t event, esp_gatt_if_t gattc_if, esp_ble_gattc_cb_param_t *param); @@ -108,7 +99,7 @@ class BluedroidGattClient final : public Component { void unconditional_disconnect_(); void set_idle_(); void set_disconnecting_(); - void report_connection_state_(bool connected, int error); + void report_connection_state_(bool connected, uint16_t mtu, int error); esp_err_t update_conn_params_(uint16_t min_interval, uint16_t max_interval, uint16_t latency, uint16_t timeout, const char *param_type); int check_and_log_error_(const char *operation, esp_err_t err); @@ -122,7 +113,6 @@ class BluedroidGattClient final : public Component { #endif // Group 1: pointers / composed objects - BluedroidTrackerShim shim_{this}; ble_device_base::GattClientListener *listener_{nullptr}; #ifdef USE_BLE_GATT_SERVICE_TABLE // One exact-size block carved into the table's three arrays; owned here, @@ -140,7 +130,6 @@ class BluedroidGattClient final : public Component { // Group 4: 2-byte types 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). diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h b/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h index f0615e34f3..7dd79414db 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_gatt_backend.h @@ -28,7 +28,7 @@ class StubGattBackend { public: void set_listener(ble_device_base::GattClientListener *listener) {} 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 gatt_disconnect() { return ble_device_base::GATT_ERR_NOT_CONNECTED; } int discover_services() { return ble_device_base::GATT_ERR_NOT_CONNECTED; } int read_characteristic(uint16_t handle) { return ble_device_base::GATT_ERR_NOT_CONNECTED; } int write_characteristic(uint16_t handle, const uint8_t *data, uint16_t len, bool response) { diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index a0ed5e95be..81dc9040b3 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -44,7 +44,7 @@ void BluetoothConnection::disconnect() { if (this->state_ == ClientState::IDLE || this->state_ == ClientState::DISCONNECTING) { return; } - int err = this->backend_->disconnect(); + int err = this->backend_->gatt_disconnect(); if (err == GATT_NOT_CONNECTED) { // Backend already idle: free the slot so the client is not stuck. ESP_LOGW(TAG, "[%d] [%s] disconnect while backend idle", this->connection_index_, this->address_str_); @@ -102,7 +102,7 @@ void BluetoothConnection::on_connection_state(bool connected, uint16_t mtu, int if (connected && this->address_ == 0) { // Late completion for a slot that was already freed: nothing to report, // and the api-gone sweep or a new reservation owns the slot now. - int err = this->backend_->disconnect(); + int err = this->backend_->gatt_disconnect(); if (err != 0 && err != GATT_NOT_CONNECTED) { // Log only: re-arming a freed slot could clobber a new reservation. ESP_LOGW(TAG, "[%d] freed-slot disconnect refused, err=%d", this->connection_index_, err); @@ -112,7 +112,7 @@ void BluetoothConnection::on_connection_state(bool connected, uint16_t mtu, int if (connected && this->state_ == ClientState::DISCONNECTING) { // The link came up after a disconnect request won the race; finish the // teardown instead of reporting a connection the client no longer wants. - int err = this->backend_->disconnect(); + int err = this->backend_->gatt_disconnect(); // Fresh teardown attempt: give it the full safety window. this->disconnecting_started_ = millis(); if (err == GATT_NOT_CONNECTED) { diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp index 0bd097dc6d..dc77d448a5 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.cpp @@ -393,7 +393,7 @@ void RP2GattClient::loop() { // Control events must not be lost; the connection state is no longer // trustworthy — recover with a forced teardown. ESP_LOGE(TAG, "Dropped %u GATT control events, disconnecting", dropped); - this->disconnect(); + this->gatt_disconnect(); } uint16_t notify_dropped = this->notify_queue_.get_and_reset_dropped_count(); if (notify_dropped > 0) { @@ -424,7 +424,7 @@ void RP2GattClient::loop() { // reclaims state if the disconnection event is lost. Dropping engine // state without gap_disconnect would leak the live link and the // single GATT slot for the rest of the boot. - this->disconnect(); + this->gatt_disconnect(); } } } else if (this->state_ == EngineState::DISCONNECTING) { @@ -856,7 +856,7 @@ int RP2GattClient::connect(uint64_t address, uint8_t addr_type) { return 0; } -int RP2GattClient::disconnect() { +int RP2GattClient::gatt_disconnect() { switch (this->state_) { case EngineState::IDLE: return GATT_ERR_NOT_CONNECTED; diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h index 9b96bad8bf..8aa9f80905 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_rp2.h @@ -82,7 +82,7 @@ class RP2GattClient final : public Component, public Parentedlistener_->on_connection_state(true, 517, 0); return 0; } - int disconnect() { return 0; } + int gatt_disconnect() { return 0; } int discover_services() { this->listener_->on_service_discovery_done(0); return 0;