Address review: fail the empty table before CONNECTED, one slot spelling

- The empty-table bailout ran after state_ went CONNECTED, so its
  teardown's terminal report read as a completed connection and fired
  on_disconnect with no preceding on_connect - the exact trigger
  contract violation the sibling error branch avoids. The check now
  runs first and the teardown resolves through connect_failed; a
  node-less client still reaches CONNECTED through the single
  assignment after the guard
- The esp32 arm claims its slot through consume_gatt_slot like every
  other claimant (behavior-identical: it forwards to the same esp32
  validator), so the ledger's one-spelling contract holds
This commit is contained in:
J. Nick Koston
2026-08-09 18:52:49 -05:00
parent 5ec7047151
commit 87b7076cd1
2 changed files with 19 additions and 18 deletions
+2 -2
View File
@@ -178,7 +178,7 @@ _COMMON_SCHEMA = cv.Schema(
def _esp32_config_schema() -> cv.All:
"""The legacy engine's schema, byte-compatible with what esp32 always had
(including the Bluedroid security triggers)."""
from esphome.components import esp32_ble, esp32_ble_tracker
from esphome.components import esp32_ble_tracker
return cv.All(
_COMMON_SCHEMA.extend(
@@ -213,7 +213,7 @@ def _esp32_config_schema() -> cv.All:
),
}
).extend(esp32_ble_tracker.ESP_BLE_DEVICE_SCHEMA),
esp32_ble.consume_connection_slots(1, "ble_client"),
bluetooth_connection.consume_gatt_slot("ble_client"),
)
@@ -152,33 +152,34 @@ void BLEClient::on_service_discovery_done(int error) {
this->disconnect();
return;
}
// CONNECTED before the fan-out so nodes may consult connected() from
// their own on_connected().
this->state_ = State::CONNECTED;
ble_device_base::GattServiceTable table{};
if (!this->nodes_.empty()) {
// Materialize only when a node will read it: a client with no nodes
// would pay the build/free cycle on every (re)connect for nothing.
auto table = this->backend_->get_service_table();
table = this->backend_->get_service_table();
if (table.service_count == 0) {
// A failed materialization is indistinguishable from a service-less
// peer, and a real GATT peer always exposes at least GAP/GATT: treat
// it as a discovery failure so the connection retries instead of
// sitting inert behind a successful-looking on_connect.
// peer, and a real GATT peer always exposes at least GAP/GATT: fail
// the discovery before CONNECTED so the teardown resolves through
// connect_failed, never a spurious on_disconnect.
ESP_LOGW(TAG, "[%s] Service table is empty; treating as failed discovery", this->address_str_);
this->backend_->release_services();
this->register_failure_();
this->disconnect();
return;
}
for (auto *node : this->nodes_) {
node->on_connected(table);
if (this->state_ != State::CONNECTED || this->cancel_requested_) {
// A node tore the link down mid-fan-out: on_disconnect fires with no
// preceding on_connect, so leave a trace of why.
ESP_LOGW(TAG, "[%s] A node aborted the connection during setup", this->address_str_);
this->backend_->release_services();
return;
}
}
// CONNECTED before the fan-out so nodes may consult connected() from
// their own on_connected().
this->state_ = State::CONNECTED;
for (auto *node : this->nodes_) {
node->on_connected(table);
if (this->state_ != State::CONNECTED || this->cancel_requested_) {
// A node tore the link down mid-fan-out: on_disconnect fires with no
// preceding on_connect, so leave a trace of why.
ESP_LOGW(TAG, "[%s] A node aborted the connection during setup", this->address_str_);
this->backend_->release_services();
return;
}
}
this->backend_->release_services();