mirror of
https://github.com/esphome/esphome.git
synced 2026-08-24 15:16:20 +00:00
Address review: one overcommit message per platform and a stall breadcrumb
Platforms whose BLE stack owns a connection budget (esp32, rp2) report an overcommit exactly once through that stack; the neutral cap check covers future backend platforms without one. The write action logs unmatched completions while a chain is parked, and the schema-dump gap is documented as deliberate.
This commit is contained in:
@@ -236,6 +236,9 @@ def _gatt_config_schema(platform: str) -> cv.All:
|
||||
@schema_extractor("schema")
|
||||
def _validate_platform(config: ConfigType) -> ConfigType:
|
||||
if config is SCHEMA_EXTRACT:
|
||||
# Deliberate gap (the bluetooth_proxy pattern): the dumper gets only
|
||||
# this shape, so the neutral arm's ble_hub_id is absent from editor
|
||||
# schemas and the esp32-only keys are advertised on every platform.
|
||||
# The language-schema dumper runs without a platform; expose the
|
||||
# esp32 (legacy-engine) shape.
|
||||
return _esp32_config_schema()
|
||||
|
||||
@@ -129,13 +129,21 @@ template<typename... Ts> class BLEClientWriteAction final : public Action<Ts...>
|
||||
}
|
||||
|
||||
void on_write_result(uint16_t handle, int error) override {
|
||||
if (this->resolved_ && handle == this->char_handle_ && this->num_running_ != 0) {
|
||||
if (error != 0) {
|
||||
// Continue the chain (legacy parity) but leave a breadcrumb.
|
||||
esph_log_w(Automation::TAG, "Write completed with status %d", error);
|
||||
}
|
||||
this->ble_client_->run_later([this]() { this->play_next_tuple_(this->var_); });
|
||||
if (this->num_running_ == 0) {
|
||||
return;
|
||||
}
|
||||
if (!this->resolved_ || handle != this->char_handle_) {
|
||||
// A parked chain waiting on a completion that never matches would
|
||||
// otherwise stall silently until disconnect.
|
||||
esph_log_d(Automation::TAG, "Write result for handle 0x%04x ignored, waiting on 0x%04x", handle,
|
||||
this->char_handle_);
|
||||
return;
|
||||
}
|
||||
if (error != 0) {
|
||||
// Continue the chain (legacy parity) but leave a breadcrumb.
|
||||
esph_log_w(Automation::TAG, "Write completed with status %d", error);
|
||||
}
|
||||
this->ble_client_->run_later([this]() { this->play_next_tuple_(this->var_); });
|
||||
}
|
||||
|
||||
private:
|
||||
|
||||
@@ -177,10 +177,10 @@ def consume_gatt_slot(
|
||||
consumer: str, count: int = 1
|
||||
) -> Callable[[ConfigType], ConfigType]:
|
||||
"""Validator claiming GATT connection slots - the one spelling for every
|
||||
claimant. The neutral ledger feeds the hub-platform cap check in
|
||||
FINAL_VALIDATE_SCHEMA; esp32 and rp2 additionally charge their platform
|
||||
stack's connection budget (esp32's cap lives there, not in
|
||||
HUB_MAX_CONNECTIONS)."""
|
||||
claimant. Platforms whose BLE stack owns a connection budget (esp32, rp2)
|
||||
are charged there and their stack's final validation reports an
|
||||
overcommit; the neutral ledger covers any future backend platform without
|
||||
one (the cap check in FINAL_VALIDATE_SCHEMA)."""
|
||||
|
||||
def validator(config: ConfigType) -> ConfigType:
|
||||
_get_data().slot_consumers.extend([consumer] * count)
|
||||
@@ -195,21 +195,23 @@ def consume_gatt_slot(
|
||||
return validator
|
||||
|
||||
|
||||
# Platforms whose BLE stack owns its own connection budget: consume_gatt_slot
|
||||
# charges it there, and the stack's final validation is the one place an
|
||||
# overcommit is reported (never two messages for one misconfiguration).
|
||||
_STACK_BUDGET_PLATFORMS = {PLATFORM_ESP32, PLATFORM_RP2}
|
||||
|
||||
|
||||
def _validate_slot_totals(config: ConfigType) -> ConfigType:
|
||||
# esp32 has its own controller budget (esp32_ble); the hub platforms cap
|
||||
# at the prebuilt stack's client count, and nothing else counts claims
|
||||
# across components (e.g. a proxy plus a radon_eye_rd200 on rp2).
|
||||
# Skipped in testing mode so grouped component builds can co-exist
|
||||
# (mirrors esp32_ble.validate_connection_slots).
|
||||
if CORE.testing_mode:
|
||||
return config
|
||||
if CORE.target_platform in _STACK_BUDGET_PLATFORMS:
|
||||
return config
|
||||
if (cap := HUB_MAX_CONNECTIONS.get(CORE.target_platform)) is None:
|
||||
# esp32's budget lives in esp32_ble; any other registered backend
|
||||
# platform must carry a cap here or fail loudly, never fail open.
|
||||
if (
|
||||
CORE.target_platform != PLATFORM_ESP32
|
||||
and CORE.target_platform in _PLATFORM_BACKENDS
|
||||
):
|
||||
# Any backend platform without a stack budget must carry a cap here
|
||||
# or fail loudly, never fail open.
|
||||
if CORE.target_platform in _PLATFORM_BACKENDS:
|
||||
raise cv.Invalid(
|
||||
f"{CORE.target_platform} has a GATT backend but no slot cap "
|
||||
"in HUB_MAX_CONNECTIONS"
|
||||
|
||||
@@ -8,6 +8,7 @@ from esphome.components import (
|
||||
ble_device_base,
|
||||
bluetooth_connection,
|
||||
bluetooth_proxy,
|
||||
rp2040_ble,
|
||||
)
|
||||
from esphome.const import CONF_MAC_ADDRESS, PlatformFramework
|
||||
from esphome.core import CORE
|
||||
@@ -18,12 +19,14 @@ from ..types import SetCoreConfigCallable
|
||||
def test_gatt_slot_ledger_rejects_overcommit_on_rp2(
|
||||
set_core_config: SetCoreConfigCallable,
|
||||
) -> None:
|
||||
# The cap logic in isolation: hand charges past the cap must trip it.
|
||||
# rp2 owns its budget: the stack's validation reports the overcommit and
|
||||
# the neutral cap check stays silent (one message per misconfiguration).
|
||||
set_core_config(PlatformFramework.RP2_ARDUINO)
|
||||
bluetooth_connection.consume_gatt_slot("bluetooth_proxy", 3)({})
|
||||
bluetooth_connection.consume_gatt_slot("ble_client")({})
|
||||
with pytest.raises(cv.Invalid, match="supports at most 3 GATT client"):
|
||||
bluetooth_connection.FINAL_VALIDATE_SCHEMA({})
|
||||
bluetooth_connection.FINAL_VALIDATE_SCHEMA({})
|
||||
with pytest.raises(cv.Invalid, match="rp2 maximum is 3"):
|
||||
rp2040_ble.validate_connection_slots()
|
||||
|
||||
|
||||
def test_gatt_slot_ledger_skipped_in_testing_mode(
|
||||
@@ -37,6 +40,7 @@ def test_gatt_slot_ledger_skipped_in_testing_mode(
|
||||
CORE.testing_mode = True
|
||||
try:
|
||||
bluetooth_connection.FINAL_VALIDATE_SCHEMA({})
|
||||
rp2040_ble.validate_connection_slots()
|
||||
finally:
|
||||
CORE.testing_mode = False
|
||||
|
||||
@@ -51,10 +55,11 @@ def test_real_validators_charge_the_ledger_on_rp2(
|
||||
CORE.loaded_integrations.add("rp2_ble_tracker")
|
||||
bluetooth_proxy.CONFIG_SCHEMA({})
|
||||
ble_client.CONFIG_SCHEMA({CONF_MAC_ADDRESS: "AA:BB:CC:DD:EE:FF"})
|
||||
# The proxy defaults to 3 slots on rp2; ble_client's claim overcommits.
|
||||
# The proxy defaults to 3 slots on rp2; ble_client's claim overcommits
|
||||
# and rp2's own budget names every claimant.
|
||||
with pytest.raises(
|
||||
cv.Invalid,
|
||||
match="requested by: bluetooth_proxy, bluetooth_proxy, bluetooth_proxy, "
|
||||
match="Components: bluetooth_proxy, bluetooth_proxy, bluetooth_proxy, "
|
||||
"ble_client",
|
||||
):
|
||||
bluetooth_connection.FINAL_VALIDATE_SCHEMA({})
|
||||
rp2040_ble.validate_connection_slots()
|
||||
|
||||
Reference in New Issue
Block a user