From 3d33a16b71a97e9cae4c525ddcb2bc60c43c7ea1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Aug 2026 12:06:44 -0500 Subject: [PATCH] Return the wrapper to 48 bytes - exact per-slot RAM parity with dev The measured layout showed 56+48 = 104 B/slot vs dev's 96. Two changes that only pay together (the wrapper is 8-aligned, so nothing under a full 8 helps): - The tail packs into 2 bytes of bitfields; remote_addr_type_ becomes a start_connect_ parameter (written and read on adjacent lines only) - The wrapper's duplicate 10 s teardown timer is gone: the backend owns the whole safety window. Its timer now also arms for a teardown scheduled during CONNECTING (the one case the wrapper covered alone), a late OPEN_EVT on a given-up slot closes the link instead of resurrecting it, and the wrapper's transient-refusal branch collapses because both backends return nonzero only when already idle 48 + 48 = 96 B/slot, the split at dev parity. --- .../bluetooth_connection_bluedroid.cpp | 24 +++++++---- .../bluetooth_connection_hub.cpp | 41 ++++--------------- .../bluetooth_connection_hub.h | 40 +++++++----------- 3 files changed, 40 insertions(+), 65 deletions(-) diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index 6139da41e1..7c0212dbfc 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -72,7 +72,18 @@ void BluedroidGattClient::loop() { } // Do not wait for REG_EVT; a dropped event must not wedge the slot. this->set_state(ClientState::IDLE); - } else if (st != ClientState::DISCONNECTING) { + } else if (st == ClientState::DISCONNECTING || this->want_disconnect_) { + // The one teardown safety net (the wrapper has no timer): covers a lost + // CLOSE_EVT and a scheduled teardown whose OPEN_EVT never arrives. + if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { + ESP_LOGE(TAG, "[%d] Timeout waiting for teardown, forcing IDLE", this->connection_index_); + // Release before idling: unconditional disconnect does not release, + // and a lost completion would otherwise leak the table and the cache. + this->release_services(); + this->set_idle_(); // also clears want_disconnect_ + this->listener_->on_connection_state(false, 0, ESP_GATT_CONN_TIMEOUT); + } + } else { // While a link exists the loop stays on watching for a stack-down (the // settle above needs a tick to run) and flushing a pre-started search // claimed outside an event drain; it settles only back at IDLE. @@ -80,13 +91,6 @@ void BluedroidGattClient::loop() { if (this->state() == ClientState::IDLE) { this->disable_loop(); } - } else if (millis() - this->disconnecting_started_ > ble_device_base::GATT_DISCONNECT_TIMEOUT_MS) { - ESP_LOGE(TAG, "[%d] Timeout waiting for CLOSE_EVT, forcing IDLE", this->connection_index_); - // Release before idling: unconditional disconnect does not release, and a - // lost CLOSE/DISCONNECT would otherwise leak the table and the cache. - this->release_services(); - this->set_idle_(); - this->listener_->on_connection_state(false, 0, ESP_GATT_CONN_TIMEOUT); } } @@ -171,6 +175,10 @@ int BluedroidGattClient::gatt_disconnect() { if (st == ClientState::CONNECTING || this->conn_id_ == UNSET_CONN_ID) { ESP_LOGD(TAG, "[%d] Disconnect scheduled", this->connection_index_); this->want_disconnect_ = true; + // The backend owns the whole safety window (the wrapper has no timer): + // a lost OPEN_EVT must not leak the scheduled teardown. + this->disconnecting_started_ = millis(); + this->enable_loop(); return 0; } this->unconditional_disconnect_(); diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp index 062e32b474..20e05e8b73 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.cpp @@ -26,11 +26,11 @@ void BluetoothConnection::set_address(uint64_t address) { format_mac_addr_upper(mac, this->address_str_); } -void BluetoothConnection::start_connect_() { +void BluetoothConnection::start_connect_(uint8_t address_type) { // 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_); + int err = this->backend_->connect(this->address_, address_type); if (err != 0) { ESP_LOGW(TAG, "[%d] [%s] connect failed, err=%d", this->connection_index_, this->address_str_, err); this->reset_connection_(err); @@ -45,33 +45,16 @@ void BluetoothConnection::disconnect() { return; } 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_); + if (err != 0) { + // Both backends return nonzero only when there is nothing to tear down + // (already idle): free the slot so the client is not stuck. Accepted + // teardowns always reach a terminal report - the backend owns the + // safety timer on every path. + ESP_LOGW(TAG, "[%d] [%s] disconnect while backend idle, err=%d", this->connection_index_, this->address_str_, err); this->reset_connection_(err); return; } - if (err != 0) { - // Transient refusal: stay DISCONNECTING and let the safety timeout - // arbitrate rather than freeing a slot whose teardown is unresolved. - // Latch the refusal unless a GATT cause is already recorded (first wins). - ESP_LOGW(TAG, "[%d] [%s] disconnect failed, err=%d", this->connection_index_, this->address_str_, err); - if (this->pending_error_ == 0) { - this->pending_error_ = err; - } - } this->state_ = ClientState::DISCONNECTING; - this->disconnecting_started_ = millis(); -} - -void BluetoothConnection::check_disconnect_timeout_() { - // 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); - } } void BluetoothConnection::on_pairing_result(int status) { @@ -113,15 +96,9 @@ void BluetoothConnection::on_connection_state(bool connected, uint16_t mtu, int // 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_->gatt_disconnect(); - // Fresh teardown attempt: give it the full safety window. - this->disconnecting_started_ = millis(); - if (err == GATT_NOT_CONNECTED) { + if (err != 0) { // Nothing left to tear down after all. this->reset_connection_(err); - } else if (err != 0) { - // Transient refusal while the link is up: keep DISCONNECTING and let - // the safety timeout arbitrate (same policy as disconnect()). - ESP_LOGW(TAG, "[%d] [%s] teardown disconnect failed, err=%d", this->connection_index_, this->address_str_, err); } return; } diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h index 93722380a6..163311acc1 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_hub.h +++ b/esphome/components/bluetooth_connection/bluetooth_connection_hub.h @@ -52,13 +52,10 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { this->disconnect(); } - /// Start connecting: record the API address type (BLE_ADDR_TYPE_* code - /// space) and open the connection through the backend. Failures report - /// through the same reset path a failed open takes. - void initiate_connection(uint8_t address_type) { - this->remote_addr_type_ = address_type; - this->start_connect_(); - } + /// Start connecting with the API address type (BLE_ADDR_TYPE_* code + /// space). Failures report through the same reset path a failed open + /// takes. + void initiate_connection(uint8_t address_type) { this->start_connect_(address_type); } void disconnect(); bool is_paired() const { return this->paired_; } void set_unpaired() { this->paired_ = false; } @@ -84,18 +81,13 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { // 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 — the wrapper has no - /// Component loop of its own. + /// Stream any pending service-discovery batch. Called from the proxy's + /// loop — the wrapper has no Component loop of its own. The disconnect + /// safety timer lives in the backend, which owns every teardown path. void process_pending_services() { if (this->send_service_ >= 0) { this->stream_pending_(this->backend_); } - // 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 listener (called directly by the backend, main loop) ---- @@ -112,7 +104,7 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { // The Bluedroid backend streams services in place from its stack cache. friend class BluedroidGattClient; - void start_connect_(); + void start_connect_(uint8_t address_type); // A backend providing its own streamer (see the contract doc) builds the // response in place from its stack cache; the rest use the table streamer. // Template so the discarded branch is not odr-checked against backends @@ -125,7 +117,6 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { } } void send_service_for_discovery_(); - void check_disconnect_timeout_(); void reset_connection_(conn_err_t reason); conn_err_t check_connected_op_(const char *action, const char *type) const; void log_gatt_operation_error_(const char *operation, uint16_t handle, int status); @@ -141,19 +132,18 @@ class BluetoothConnection final : public ble_device_base::GattClientListener { // Group 3: 8-byte and 4-byte types uint64_t address_{0}; - uint32_t disconnecting_started_{0}; conn_err_t pending_error_{0}; // Group 4: Arrays char address_str_[MAC_ADDRESS_PRETTY_BUFFER_SIZE]{}; - // 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}; - bool services_discovered_{false}; + // Group 5: bit-packed tail. address_ makes the object 8-aligned, so this + // group must stay within 2 bytes to keep the wrapper at 48 (dev parity). + ClientState state_ : 3 {ClientState::IDLE}; + bool paired_ : 1 {false}; + ConnectionType connection_type_ : 2 {ConnectionType::V1}; + uint8_t connection_index_ : 4 {0}; + bool services_discovered_ : 1 {false}; }; } // namespace esphome::bluetooth_connection