From 499a0b592436c4d27f8f2cd08c4c265cf64dd17e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 21 Aug 2026 17:08:08 -0500 Subject: [PATCH] Compile the connection window out of scan-only builds; review polish --- .../components/ble_device_base/__init__.py | 17 ++++++------- .../components/esp32_ble_tracker/__init__.py | 20 +++++++++------ .../esp32_ble_tracker/esp32_ble_tracker.cpp | 25 +++++++++++++------ .../esp32_ble_tracker/esp32_ble_tracker.h | 8 +++++- .../config/scan_window_explicit.yaml | 5 ++++ .../config/scan_window_raised.yaml | 5 ++++ .../config/scan_window_scan_only.yaml | 12 +++++++++ .../test_scan_window_default.py | 15 +++++++++-- 8 files changed, 81 insertions(+), 26 deletions(-) create mode 100644 tests/component_tests/esp32_ble_tracker/config/scan_window_scan_only.yaml diff --git a/esphome/components/ble_device_base/__init__.py b/esphome/components/ble_device_base/__init__.py index 14be6c4d29..2f4b1013bc 100644 --- a/esphome/components/ble_device_base/__init__.py +++ b/esphome/components/ble_device_base/__init__.py @@ -211,23 +211,22 @@ def validate_scan_parameters(config: ConfigType) -> ConfigType: f"Scan window ({window}) needs to be smaller than scan interval ({interval})" ) - windows = [("window", window)] + # Labels name the YAML key the user typed so the errors are greppable. + windows = [("Scan window", window)] if (connection_window := config.get(CONF_CONNECTION_SCAN_WINDOW)) is not None: if connection_window > interval: raise cv.Invalid( - f"Connection scan window ({connection_window}) needs to be smaller " - f"than scan interval ({interval})" + f"{CONF_CONNECTION_SCAN_WINDOW} ({connection_window}) needs to be " + f"smaller than scan interval ({interval})" ) - windows.append(("connection window", connection_window)) + windows.append((CONF_CONNECTION_SCAN_WINDOW, connection_window)) # BLE scan interval/window are programmed in 0.625 ms units as a 16-bit value; the # controller only accepts 2.5 ms .. 10240 ms (0x0004 .. 0x4000). Reject out-of-range # values here instead of letting the unit conversion silently overflow. - for name, value in (("interval", interval), *windows): + for name, value in (("Scan interval", interval), *windows): if value.total_microseconds < 2500 or value.total_microseconds > 10_240_000: - raise cv.Invalid( - f"Scan {name} ({value}) must be between 2.5 ms and 10240 ms" - ) + raise cv.Invalid(f"{name} ({value}) must be between 2.5 ms and 10240 ms") # Validate what actually reaches the controller: both values are truncated to # whole 0.625 ms units, so a window/interval pair that differs by less than one @@ -237,7 +236,7 @@ def validate_scan_parameters(config: ConfigType) -> ConfigType: for name, value in windows: if to_ble_units(value) == interval_units and value < interval: raise cv.Invalid( - f"Scan {name} ({value}) and interval ({interval}) both truncate to " + f"{name} ({value}) and interval ({interval}) both truncate to " f"{interval_units} x 0.625 ms, which the controller scans at a 100 % duty " f"cycle. Separate them by at least 0.625 ms." ) diff --git a/esphome/components/esp32_ble_tracker/__init__.py b/esphome/components/esp32_ble_tracker/__init__.py index 8d8688b8f9..0d432826d5 100644 --- a/esphome/components/esp32_ble_tracker/__init__.py +++ b/esphome/components/esp32_ble_tracker/__init__.py @@ -74,8 +74,9 @@ def _get_required_features() -> set[BLEFeatures]: # Slot counters sizing the tracker's StaticVector storage; one request per # registered listener or client. +CLIENT_COUNT_DEFINE = "ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT" _request_listener_slot = cg.slot_counter("ESPHOME_ESP32_BLE_TRACKER_LISTENER_COUNT") -_request_client_slot = cg.slot_counter("ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT") +_request_client_slot = cg.slot_counter(CLIENT_COUNT_DEFINE) def register_ble_features(features: set[BLEFeatures]) -> None: @@ -301,12 +302,17 @@ async def to_code(config: ConfigType) -> None: if (connection_window := params.get(CONF_CONNECTION_SCAN_WINDOW)) is not None: # Set by the user, or defaulted by _raise_defaulted_scan_window when # the window was raised to full duty; while a GATT connection is - # active the scanner falls back to it (see start_scan_). - cg.add( - var.set_connection_scan_window( - ble_device_base.to_ble_units(connection_window) - ) - ) + # active the scanner falls back to it (see start_scan_). Emitted at + # FINAL priority so a scan-only build (no GATT clients registered, + # so the guarded C++ path compiles out) skips the call entirely. + window_units = ble_device_base.to_ble_units(connection_window) + + @coroutine_with_priority(CoroPriority.FINAL) + async def _emit_connection_scan_window() -> None: + if cg.get_slot_count(CLIENT_COUNT_DEFINE): + cg.add(var.set_connection_scan_window(window_units)) + + CORE.add_job(_emit_connection_scan_window) cg.add(var.set_scan_active(params[CONF_ACTIVE])) cg.add(var.set_scan_continuous(params[CONF_CONTINUOUS])) diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index 21053abe85..cfe7d3172e 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -122,6 +122,9 @@ void ESP32BLETracker::loop() { // - start_scan_(): scanner_state_ becomes IDLE via set_scanner_state_() in cleanup_scan_state_() // - try_promote_discovered_clients_(): client enters DISCOVERED via set_state(), or // connecting client finishes (state change), or scanner reaches RUNNING/IDLE + // - connection-window restart: using_connection_window_ is only written in + // start_scan_() (which changes scanner state via set_scanner_state_()), and + // counts.active/disconnecting only change on client state changes // // All conditions that affect the logic below are tied to state changes that increment // state_version_, so the fast path is safe. @@ -145,15 +148,19 @@ void ESP32BLETracker::loop() { this->handle_scanner_failure_(); } +#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT // The window is chosen at scan start, so when the last connection drops - // mid-scan the reduced connection-time window would persist for the rest - // of the scan period (up to scan_duration_); stop the scan so the restart - // below returns to the configured window. Only for continuous scanning: - // a user-started scan would not be restarted. - if (this->scan_window_reduced_ && !counts.active && this->scan_continuous_ && + // mid-scan the connection-time window would persist for the rest of the + // scan period (up to scan_duration_); stop the scan so the restart below + // returns to the configured window. Only for continuous scanning: a + // user-started scan would not be restarted. Waiting for disconnecting to + // reach zero keeps the stop off the controller while a GATT disconnect is + // in flight, matching the restart gate below. + if (this->using_connection_window_ && !counts.active && !counts.disconnecting && this->scan_continuous_ && this->scanner_state_ == ScannerState::RUNNING) { this->stop_scan_(); } +#endif /* Avoid starting the scanner if: @@ -258,16 +265,18 @@ void ESP32BLETracker::start_scan_(bool first) { this->scan_params_.scan_filter_policy = BLE_SCAN_FILTER_ALLOW_ALL; this->scan_params_.scan_interval = this->scan_interval_; uint32_t window = this->scan_window_; +#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT // Count fresh rather than reading the loop() cache: an automation can start // a scan before loop() has refreshed the counts for a new connection. - this->scan_window_reduced_ = this->connection_scan_window_ != 0 && this->count_client_states_().active > 0; - if (this->scan_window_reduced_) { + this->using_connection_window_ = this->connection_scan_window_ != 0 && this->count_client_states_().active > 0; + if (this->using_connection_window_) { // While a GATT connection is active, fall back to the connection scan // window so the connection events get guaranteed airtime instead of // competing with a wall-to-wall scan on the shared radio. ESP_LOGV(TAG, "Connection active, using %" PRIu32 " unit scan window", this->connection_scan_window_); window = this->connection_scan_window_; } +#endif this->scan_params_.scan_window = window; // Start timeout monitoring in loop() instead of using scheduler @@ -429,9 +438,11 @@ void ESP32BLETracker::dump_config() { " Continuous Scanning: %s", this->scan_duration_, this->scan_interval_ * 0.625f, this->scan_window_ * 0.625f, this->scan_active_ ? "ACTIVE" : "PASSIVE", YESNO(this->scan_continuous_)); +#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT if (this->connection_scan_window_ != 0) { ESP_LOGCONFIG(TAG, " Connection Scan Window: %.1f ms", this->connection_scan_window_ * 0.625f); } +#endif ESP_LOGCONFIG(TAG, " Scanner State: %s\n" " Connecting: %d, discovered: %d, disconnecting: %d, active: %d", diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index 2a761172f7..5b4206c668 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -169,7 +169,9 @@ class ESP32BLETracker final : public Component, void set_scan_duration(uint32_t scan_duration) { scan_duration_ = scan_duration; } void set_scan_interval(uint32_t scan_interval) { scan_interval_ = scan_interval; } void set_scan_window(uint32_t scan_window) { scan_window_ = scan_window; } +#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT void set_connection_scan_window(uint32_t scan_window) { connection_scan_window_ = scan_window; } +#endif void set_scan_active(bool scan_active) { scan_active_ = scan_active; } bool get_scan_active() const { return scan_active_; } void set_scan_continuous(bool scan_continuous) { scan_continuous_ = scan_continuous; } @@ -314,9 +316,11 @@ class ESP32BLETracker final : public Component, uint32_t scan_duration_; uint32_t scan_interval_; uint32_t scan_window_; +#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT /// Window used while a GATT connection is active; set by the user, or /// defaulted when the window was raised to full duty (0 = no fallback). uint32_t connection_scan_window_{0}; +#endif esp_bt_status_t scan_start_failed_{ESP_BT_STATUS_SUCCESS}; esp_bt_status_t scan_set_param_failed_{ESP_BT_STATUS_SUCCESS}; @@ -341,9 +345,11 @@ class ESP32BLETracker final : public Component, #endif bool ble_was_disabled_{true}; bool parse_advertisements_{false}; +#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT /// The running scan was started with connection_scan_window_; lets loop() /// restart the scan at the configured window when the last connection drops. - bool scan_window_reduced_{false}; + bool using_connection_window_{false}; +#endif #ifdef USE_ESP32_BLE_SOFTWARE_COEXISTENCE bool coex_prefer_ble_{false}; #endif diff --git a/tests/component_tests/esp32_ble_tracker/config/scan_window_explicit.yaml b/tests/component_tests/esp32_ble_tracker/config/scan_window_explicit.yaml index 6ab1575198..70760fdea6 100644 --- a/tests/component_tests/esp32_ble_tracker/config/scan_window_explicit.yaml +++ b/tests/component_tests/esp32_ble_tracker/config/scan_window_explicit.yaml @@ -12,3 +12,8 @@ wifi: esp32_ble_tracker: scan_parameters: window: 30ms + +bluetooth_proxy: + active: true + +api: diff --git a/tests/component_tests/esp32_ble_tracker/config/scan_window_raised.yaml b/tests/component_tests/esp32_ble_tracker/config/scan_window_raised.yaml index ba57af8750..4febbfcf3b 100644 --- a/tests/component_tests/esp32_ble_tracker/config/scan_window_raised.yaml +++ b/tests/component_tests/esp32_ble_tracker/config/scan_window_raised.yaml @@ -10,3 +10,8 @@ wifi: ssid: MySSID esp32_ble_tracker: + +bluetooth_proxy: + active: true + +api: diff --git a/tests/component_tests/esp32_ble_tracker/config/scan_window_scan_only.yaml b/tests/component_tests/esp32_ble_tracker/config/scan_window_scan_only.yaml new file mode 100644 index 0000000000..5da5601388 --- /dev/null +++ b/tests/component_tests/esp32_ble_tracker/config/scan_window_scan_only.yaml @@ -0,0 +1,12 @@ +esphome: + name: scan-window-scan-only + +esp32: + board: esp32dev + framework: + type: esp-idf + +wifi: + ssid: MySSID + +esp32_ble_tracker: diff --git a/tests/component_tests/esp32_ble_tracker/test_scan_window_default.py b/tests/component_tests/esp32_ble_tracker/test_scan_window_default.py index a441a8337c..806735f184 100644 --- a/tests/component_tests/esp32_ble_tracker/test_scan_window_default.py +++ b/tests/component_tests/esp32_ble_tracker/test_scan_window_default.py @@ -158,7 +158,7 @@ def test_connection_scan_window_above_interval_rejected( ) -> None: stage_esp32("5.5.5", wifi=True) with pytest.raises( - cv.Invalid, match="Connection scan window .* needs to be smaller" + cv.Invalid, match="connection_scan_window .* needs to be smaller" ): _scan_params({"scan_parameters": {"connection_scan_window": "400ms"}}) @@ -169,7 +169,7 @@ def test_connection_scan_window_truncation_collapse_rejected( """A connection window that truncates into the interval's 0.625 ms unit would silently program a full-duty scan during connections.""" stage_esp32("5.5.5", wifi=True) - with pytest.raises(cv.Invalid, match="connection window .* both truncate"): + with pytest.raises(cv.Invalid, match="connection_scan_window .* both truncate"): _scan_params( { "scan_parameters": { @@ -196,3 +196,14 @@ def test_codegen_no_connection_window_for_explicit_window( main_cpp = generate_main(component_config_path("scan_window_explicit.yaml")) assert "set_scan_window(48)" in main_cpp assert "set_connection_scan_window" not in main_cpp + + +def test_codegen_no_connection_window_without_gatt_clients( + generate_main: Callable[[str | Path], str], + component_config_path: Callable[[str], Path], +) -> None: + """Scan-only builds compile the connection-window path out, so the raised + default must not emit a call to the guarded setter.""" + main_cpp = generate_main(component_config_path("scan_window_scan_only.yaml")) + assert "set_scan_window(512)" in main_cpp + assert "set_connection_scan_window" not in main_cpp