Address review: rejection logs, honest service_table docstring, terse sweep

- connect()/disconnect() log a rejected user action like the legacy
  engine instead of silently ignoring it
- The service_table docstring states the flag is Bluedroid-only forward
  scaffolding and that rp2's proxy hub must keep its materializer
  regardless (it streams through get_service_table())
- Review-round comments tightened to repo style
This commit is contained in:
J. Nick Koston
2026-08-09 12:47:17 -05:00
parent 55d14c6fbf
commit cf5faa454c
3 changed files with 20 additions and 18 deletions
@@ -30,9 +30,8 @@ void BLEClient::set_enabled(bool enabled) {
this->disconnect();
return;
}
// A re-enable is an explicit "try again": clear the backoff so the next
// sighting connects promptly. Enabling does not itself connect (legacy
// parity).
// A re-enable clears the backoff; the next sighting connects (legacy
// parity: enabling does not itself connect).
this->consecutive_failures_ = 0;
this->hold_off_ms_ = 0;
}
@@ -52,8 +51,10 @@ bool BLEClient::parse_device(const ble_device_base::ESPBTDevice &device) {
}
void BLEClient::connect() {
if (this->state_ != State::IDLE)
if (this->state_ != State::IDLE) {
ESP_LOGD(TAG, "[%s] Connect requested while busy, ignoring", this->address_str_);
return;
}
// An absent peer can inhibit scanning for the backend's full connect
// timeout, so this is worth a breadcrumb - but it is a supported action.
ESP_LOGI(TAG, "[%s] Connecting on request", this->address_str_);
@@ -81,15 +82,16 @@ void BLEClient::attempt_connect_() {
}
void BLEClient::disconnect() {
if (this->state_ == State::IDLE)
if (this->state_ == State::IDLE) {
ESP_LOGD(TAG, "[%s] Disconnect requested while idle, ignoring", this->address_str_);
return;
}
// A deliberate teardown's failure report must not feed the backoff.
this->cancel_requested_ = true;
int err = this->backend_->gatt_disconnect();
if (err != 0) {
// Refused synchronously: the backend and the client disagreed about the
// link state. Worth a warning of its own - the settle below then runs
// the same deliberate-cancel path as a clean teardown.
// Refused synchronously: backend and client disagree about the link
// state. Warn, then settle through the deliberate-cancel path.
ESP_LOGW(TAG, "[%s] Disconnect refused, err=%d; settling locally", this->address_str_, err);
this->on_connection_state(false, 0, err);
}
@@ -157,9 +159,8 @@ void BLEClient::on_service_discovery_done(int error) {
for (auto *node : this->nodes_) {
node->on_connected(table);
if (this->state_ != State::CONNECTED) {
// A node tore the link down mid-fan-out; the teardown settled. Release
// the borrowed table here since the normal release below is skipped
// (idempotent when the backend released on teardown already).
// A node tore the link down mid-fan-out; the normal release below is
// skipped (release is idempotent).
this->backend_->release_services();
return;
}
@@ -84,10 +84,9 @@ class BLEClient : public Component,
void run_later(std::function<void()> &&f) { this->defer(std::move(f)); } // NOLINT
// Backend ops for nodes and actions - the frozen node-facing surface.
// Only write_characteristic has an in-tree caller today; the rest exist so
// the first migrated node codes against a complete interface (subscribing
// means notify_characteristic plus a write_descriptor on the CCCD - the
// backend contract keeps the CCCD write the caller's responsibility).
// Only write_characteristic has an in-tree caller; subscribing means
// notify_characteristic plus a CCCD write_descriptor (the caller's job
// per the contract).
int write_characteristic(uint16_t handle, const uint8_t *data, uint16_t len, bool response) {
return this->backend_->write_characteristic(handle, data, len, response);
}
@@ -212,9 +212,11 @@ async def new_gatt_backend(
it with its platform stack. The connection slot is claimed at validation
(the consume_gatt_slot validators), not here.
service_table compiles the on-demand service-table materializer into the
backend; direct consumers need it, the streaming proxy does not, so
proxy-only builds keep the smaller footprint.
service_table is honored by the Bluedroid backend only: forward
scaffolding for the first esp32 direct consumer, load-bearing on no
current build (rp2 ignores the define and always materializes - its
proxy hub streams through get_service_table(), so it must keep the
materializer regardless of the flag).
"""
from esphome.components import ble_device_base