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.
This commit is contained in:
J. Nick Koston
2026-08-09 12:06:44 -05:00
parent e3aa8773e6
commit 3d33a16b71
3 changed files with 40 additions and 65 deletions
@@ -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_();
@@ -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;
}
@@ -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