From f6158597ac80bb49e1cdb01193e5d8d12ca27ef0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Aug 2026 09:42:50 -0500 Subject: [PATCH] Address review: slot-cap testing mode, derived source filter, breadcrumbs - Skip the hub-platform GATT slot cap in testing mode (mirrors esp32_ble.validate_connection_slots) so grouped builds validate - Derive the neutral engine's source filter from the backend registry so a new platform cannot validate and then fail at link - Log the real code on refused service discovery and cancelled connects; read completions get the same breadcrumb writes have - Comment accuracy: the dumper arm is the legacy shape, not a superset - Ledger tests: real-validator end-to-end path and the testing-mode skip --- esphome/components/ble_client/__init__.py | 16 +++++-- .../components/ble_client/ble_client_gatt.cpp | 13 ++++-- .../bluetooth_connection/__init__.py | 4 ++ .../bluetooth_connection/test_slot_ledger.py | 46 +++++++++++++++++-- 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/esphome/components/ble_client/__init__.py b/esphome/components/ble_client/__init__.py index f994891bd3..b679dece9a 100644 --- a/esphome/components/ble_client/__init__.py +++ b/esphome/components/ble_client/__init__.py @@ -25,6 +25,7 @@ from esphome.const import ( CONF_SERVICE_UUID, CONF_TRIGGER_ID, CONF_VALUE, + PLATFORM_ESP32, PlatformFramework, ) from esphome.core import CORE, ID @@ -56,9 +57,14 @@ FILTER_SOURCE_FILES = filter_source_files_from_platform( PlatformFramework.ESP32_ARDUINO, PlatformFramework.ESP32_IDF, }, - # Every framework of every bluetooth_connection.GATT_CLIENT_PLATFORMS - # entry except esp32; extend when the registry gains a platform. - "ble_client_gatt.cpp": {PlatformFramework.RP2_ARDUINO}, + # Every framework of every non-esp32 registry platform: a platform + # that validates the neutral arm must also compile the neutral engine. + "ble_client_gatt.cpp": { + pf + for pf in PlatformFramework + if pf.value[0] in bluetooth_connection.GATT_CLIENT_PLATFORMS + and pf.value[0] != PLATFORM_ESP32 + }, } ) @@ -227,8 +233,8 @@ def _gatt_config_schema(platform: str) -> cv.All: @schema_extractor("schema") def _validate_platform(config: ConfigType) -> ConfigType: if config is SCHEMA_EXTRACT: - # The language-schema dumper runs without a platform; expose the esp32 - # shape (the superset). + # The language-schema dumper runs without a platform; expose the + # esp32 (legacy-engine) shape. return _esp32_config_schema() if CORE.is_esp32: return _esp32_config_schema()(config) diff --git a/esphome/components/ble_client/ble_client_gatt.cpp b/esphome/components/ble_client/ble_client_gatt.cpp index 045d901489..a0409b1b8a 100644 --- a/esphome/components/ble_client/ble_client_gatt.cpp +++ b/esphome/components/ble_client/ble_client_gatt.cpp @@ -95,9 +95,10 @@ void BLEClient::register_failure_() { void BLEClient::on_connection_state(bool connected, uint16_t mtu, int error) { if (connected) { this->state_ = State::DISCOVERING; - if (this->backend_->discover_services() != 0) { + int discover_err = this->backend_->discover_services(); + if (discover_err != 0) { // Synchronous refusal: no discovery completion will follow. - ESP_LOGW(TAG, "[%s] Service discovery refused", this->address_str_); + ESP_LOGW(TAG, "[%s] Service discovery refused, err=%d", this->address_str_, discover_err); this->register_failure_(); // Deliberate teardown: its report must not charge the backoff again. this->disconnect(); @@ -117,7 +118,9 @@ void BLEClient::on_connection_state(bool connected, uint16_t mtu, int error) { this->defer([this]() { this->disconnect_callbacks_.call(); }); } else { if (cancelled) { - ESP_LOGD(TAG, "[%s] Connect attempt cancelled", this->address_str_); + // status carries the refusal code when the teardown settled + // synchronously; 0 on a backend-completed cancel. + ESP_LOGD(TAG, "[%s] Connect attempt cancelled, status=%d", this->address_str_, error); } else { ESP_LOGW(TAG, "[%s] Connect failed, status=%d", this->address_str_, error); this->register_failure_(); @@ -160,6 +163,10 @@ void BLEClient::on_write_result(uint16_t handle, int error) { } void BLEClient::on_read_result(uint16_t handle, const uint8_t *data, uint16_t len, int error) { + if (error != 0) { + // Breadcrumb even when no node claims the handle. + ESP_LOGD(TAG, "[%s] Read on handle 0x%04x completed with status %d", this->address_str_, handle, error); + } for (auto *node : this->nodes_) { node->on_read_result(handle, data, len, error); } diff --git a/esphome/components/bluetooth_connection/__init__.py b/esphome/components/bluetooth_connection/__init__.py index c5791c16fb..b9d13c24cf 100644 --- a/esphome/components/bluetooth_connection/__init__.py +++ b/esphome/components/bluetooth_connection/__init__.py @@ -177,6 +177,10 @@ 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 (cap := HUB_MAX_CONNECTIONS.get(CORE.target_platform)) is None: return config claimed = _ledger().consumers diff --git a/tests/component_tests/bluetooth_connection/test_slot_ledger.py b/tests/component_tests/bluetooth_connection/test_slot_ledger.py index ad169e5497..3ba3daeec0 100644 --- a/tests/component_tests/bluetooth_connection/test_slot_ledger.py +++ b/tests/component_tests/bluetooth_connection/test_slot_ledger.py @@ -3,15 +3,55 @@ import pytest from esphome import config_validation as cv -from esphome.components import bluetooth_connection -from esphome.const import KEY_CORE, KEY_TARGET_PLATFORM, PLATFORM_RP2 +from esphome.components import ( + ble_client, + ble_device_base, + bluetooth_connection, + bluetooth_proxy, +) +from esphome.const import ( + CONF_MAC_ADDRESS, + KEY_CORE, + KEY_TARGET_PLATFORM, + PLATFORM_RP2, + PlatformFramework, +) from esphome.core import CORE +from ..types import SetCoreConfigCallable + def test_gatt_slot_ledger_rejects_overcommit_on_rp2() -> None: - # The cross-component cap must reject two claims on rp2. + # The cap logic in isolation: two hand charges must trip it. CORE.data.setdefault(KEY_CORE, {})[KEY_TARGET_PLATFORM] = 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_gatt_slot_ledger_skipped_in_testing_mode() -> None: + # Grouped component builds merge fixtures past the cap; the check defers + # to testing mode like esp32_ble.validate_connection_slots. + CORE.data.setdefault(KEY_CORE, {})[KEY_TARGET_PLATFORM] = PLATFORM_RP2 + bluetooth_connection.consume_gatt_slot("bluetooth_proxy")({}) + bluetooth_connection.consume_gatt_slot("ble_client")({}) + CORE.testing_mode = True + try: + bluetooth_connection.FINAL_VALIDATE_SCHEMA({}) + finally: + CORE.testing_mode = False + + +def test_real_validators_charge_the_ledger_on_rp2( + set_core_config: SetCoreConfigCallable, +) -> None: + # End to end through the component CONFIG_SCHEMAs (no hand charges): + # removing either consumer's consume_gatt_slot call fails this test. + set_core_config(PlatformFramework.RP2_ARDUINO) + ble_device_base.register_hub_provider("rp2_ble_tracker") + CORE.loaded_integrations.add("rp2_ble_tracker") + bluetooth_proxy.CONFIG_SCHEMA({}) + ble_client.CONFIG_SCHEMA({CONF_MAC_ADDRESS: "AA:BB:CC:DD:EE:FF"}) + with pytest.raises(cv.Invalid, match="requested by: bluetooth_proxy, ble_client"): + bluetooth_connection.FINAL_VALIDATE_SCHEMA({})