Apply review: settle refused teardowns, per-platform schema cache, ledger tests

This commit is contained in:
J. Nick Koston
2026-08-09 02:50:01 -05:00
parent 399dc26236
commit f9dbd24a2a
4 changed files with 40 additions and 11 deletions
+5 -4
View File
@@ -208,12 +208,13 @@ def _esp32_config_schema() -> cv.All:
@functools.cache
def _gatt_config_schema() -> cv.All:
def _gatt_config_schema(platform: str) -> cv.All:
"""The neutral engine's schema: the shared keys plus the hub reference
(parsed-advertisement sightings) and the GATT backend declaration."""
(parsed-advertisement sightings) and the GATT backend declaration.
Keyed by platform - the backend fragment differs per platform."""
return cv.All(
_COMMON_SCHEMA.extend(ble_device_base.BLE_DEVICE_SCHEMA).extend(
bluetooth_connection.gatt_client_schema()
bluetooth_connection.gatt_client_schema(platform)
),
bluetooth_connection.consume_gatt_slot("ble_client"),
)
@@ -228,7 +229,7 @@ def _validate_platform(config: ConfigType) -> ConfigType:
if CORE.is_esp32:
return _esp32_config_schema()(config)
if CORE.target_platform in bluetooth_connection.GATT_CLIENT_PLATFORMS:
return _gatt_config_schema()(config)
return _gatt_config_schema(CORE.target_platform)(config)
raise cv.Invalid(f"ble_client is not supported on {CORE.target_platform}")
@@ -48,9 +48,9 @@ bool BLEClient::parse_device(const ble_device_base::ESPBTDevice &device) {
void BLEClient::connect() {
if (this->state_ != State::IDLE)
return;
// Without a fresh sighting each attempt can inhibit scanning for the
// backend's full connect timeout (20 s on rp2) if the peer is absent.
ESP_LOGW(TAG, "[%s] Connecting without a recent advertisement", this->address_str_);
// 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_);
this->attempt_connect_();
}
@@ -74,7 +74,11 @@ void BLEClient::disconnect() {
return;
// A deliberate teardown's failure report must not feed the backoff.
this->cancel_requested_ = true;
this->backend_->gatt_disconnect();
if (this->backend_->gatt_disconnect() != 0) {
// Refused synchronously: the backend is already down and no report will
// come; settle through the normal path so waiters resolve.
this->on_connection_state(false, 0, 0);
}
}
void BLEClient::register_failure_() {
@@ -91,7 +95,7 @@ void BLEClient::on_connection_state(bool connected, uint16_t mtu, int error) {
this->state_ = State::DISCOVERING;
if (this->backend_->discover_services() != 0) {
// Synchronous refusal: no discovery completion will follow.
this->backend_->gatt_disconnect();
this->disconnect();
}
return;
}
@@ -121,7 +125,9 @@ void BLEClient::on_service_discovery_done(int error) {
if (error != 0) {
ESP_LOGW(TAG, "[%s] Service discovery failed, status=%d", this->address_str_, error);
this->register_failure_();
this->backend_->gatt_disconnect();
// The teardown is deliberate: do not charge the backoff again for its
// connection report.
this->disconnect();
return;
}
auto table = this->backend_->get_service_table();
@@ -20,7 +20,9 @@
// backend is wired by codegen (one slot per connection). This is the single
// spelling of that predicate - the hub wrapper, the connection-aware API
// request handlers, and the Bluedroid in-place streamer all gate on it.
// Advertisement-only builds get the clean-error handlers; address-scoped
// Builds without a compiled backend get the clean-error handlers (a
// passive proxy alongside a backend consumer compiles the real ones);
// address-scoped
// maintenance (unpair, cache clear) still works there through the
// per-platform free functions below. Backend-only builds (a dedicated-backend
// consumer without bluetooth_proxy) compile none of this API surface.
@@ -96,3 +96,23 @@ def test_on_notify_implies_notify() -> None:
def test_notify_unchanged_without_on_notify() -> None:
config: ConfigType = {CONF_NOTIFY: False}
assert notify_from_on_notify(config)[CONF_NOTIFY] is False
def test_gatt_slot_ledger_rejects_overcommit_on_rp2() -> None:
# Suggestion 4: the cross-component cap must reject two claims on rp2.
from esphome.components import bluetooth_connection
CORE.data.setdefault(KEY_CORE, {})[KEY_TARGET_PLATFORM] = "rp2"
bluetooth_connection.consume_gatt_slot("bluetooth_proxy")({})
bluetooth_connection.consume_gatt_slot("ble_client")({})
with pytest.raises(cv.Invalid, match="supports at most 1 GATT client"):
bluetooth_connection.FINAL_VALIDATE_SCHEMA({})
def test_legacy_node_choke_point_rejects_other_platforms() -> None:
from esphome.components import ble_client
from esphome.core import ID
CORE.data.setdefault(KEY_CORE, {})[KEY_TARGET_PLATFORM] = "rp2"
with pytest.raises(cv.Invalid, match="not been migrated"):
ble_client._legacy_engine_only(ID("x"))