Derive the restart from the programmed window; share the scan-end sweep; fold validation loop; parametrize codegen tests

This commit is contained in:
J. Nick Koston
2026-08-21 18:34:54 -05:00
parent 311104efd8
commit 8886ff5ba6
4 changed files with 69 additions and 99 deletions
+7 -11
View File
@@ -206,21 +206,17 @@ def validate_scan_parameters(config: ConfigType) -> ConfigType:
interval = config[CONF_INTERVAL]
window = config[CONF_WINDOW]
if window > interval:
raise cv.Invalid(
f"Scan window ({window}) needs to be smaller than scan interval ({interval})"
)
# Labels name the YAML key the user typed so the errors are greppable.
# Labels are reused in every error below; the optional one names its key.
windows = [("Scan window", window)]
if (connection_window := config.get(CONF_CONNECTION_SCAN_WINDOW)) is not None:
if connection_window > interval:
raise cv.Invalid(
f"{CONF_CONNECTION_SCAN_WINDOW} ({connection_window}) needs to be "
f"smaller than scan interval ({interval})"
)
windows.append((CONF_CONNECTION_SCAN_WINDOW, connection_window))
for name, value in windows:
if value > interval:
raise cv.Invalid(
f"{name} ({value}) needs to be smaller than scan interval ({interval})"
)
# 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.
@@ -122,8 +122,8 @@ 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
// - connection-window restart: scan_params_ 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
@@ -149,13 +149,14 @@ void ESP32BLETracker::loop() {
}
#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT
// Last connection dropped: restart so the configured window returns now
// The programmed window no longer matches the connection state (typically
// the last connection dropped): restart so the right window applies now
// instead of at the end of the scan period. Continuous only (a user-started
// scan would not restart); !disconnecting matches the restart gate below.
if (this->using_connection_window_ && !counts.active && !counts.disconnecting && this->scan_continuous_ &&
this->scanner_state_ == ScannerState::RUNNING) {
// Same logical scan period continues: cleanup_scan_state_ and start_scan_
// both skip their on_scan_end sweep. Only armed when the stop was issued.
if (this->scanner_state_ == ScannerState::RUNNING && this->scan_continuous_ && !counts.disconnecting &&
this->scan_params_.scan_window != this->desired_scan_window_(counts.active)) {
// Same logical scan period continues: no on_scan_end sweeps for this
// restart. Only armed when the stop was issued.
this->skip_next_scan_end_ = this->stop_scan_();
}
#endif
@@ -250,24 +251,11 @@ void ESP32BLETracker::start_scan_(bool first) {
}
this->set_scanner_state_(ScannerState::STARTING);
ESP_LOGV(TAG, "Starting scan, set scanner state to STARTING.");
bool notify_scan_end = !first;
if (!first)
this->notify_scan_end_();
#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT
if (this->skip_next_scan_end_) {
// Window-change restart continues the same scan period.
this->skip_next_scan_end_ = false;
notify_scan_end = false;
}
this->skip_next_scan_end_ = false;
#endif
if (notify_scan_end) {
#ifdef ESPHOME_ESP32_BLE_TRACKER_LISTENER_COUNT
for (auto *listener : this->listeners_)
listener->on_scan_end();
#endif
#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT
for (auto *listener : this->neutral_listeners_)
listener->on_scan_end();
#endif
}
#ifdef USE_ESP32_BLE_DEVICE
this->discovered_log_.clear();
#endif
@@ -277,15 +265,11 @@ void ESP32BLETracker::start_scan_(bool first) {
this->scan_params_.scan_interval = this->scan_interval_;
uint32_t window = this->scan_window_;
#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT
// Count fresh: an automation can start a scan before loop() refreshes the
// counts. An equal window changes nothing and must not arm the restart.
this->using_connection_window_ = this->connection_scan_window_ != 0 &&
this->connection_scan_window_ != this->scan_window_ &&
this->count_client_states_().active > 0;
if (this->using_connection_window_) {
// Count fresh: an automation can start a scan before loop() refreshes the counts.
window = this->desired_scan_window_(this->count_client_states_().active);
if (window != this->scan_window_) {
// Guarantee the connection airtime instead of scanning wall to wall.
ESP_LOGV(TAG, "Connection active, using %" PRIu32 " unit scan window", this->connection_scan_window_);
window = this->connection_scan_window_;
ESP_LOGV(TAG, "Connection active, using %" PRIu32 " unit scan window", window);
}
#endif
this->scan_params_.scan_window = window;
@@ -533,26 +517,28 @@ void ESP32BLETracker::cleanup_scan_state_(bool is_stop_complete) {
// Reset timeout state machine instead of cancelling scheduler timeout
this->scan_timeout_state_ = ScanTimeoutState::INACTIVE;
bool notify_scan_end = true;
#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT
// Window-change restart continues the scan period: no on_scan_end here.
// The flag stays set so start_scan_ skips its sweep too.
notify_scan_end = !this->skip_next_scan_end_;
#endif
if (notify_scan_end) {
#ifdef ESPHOME_ESP32_BLE_TRACKER_LISTENER_COUNT
for (auto *listener : this->listeners_)
listener->on_scan_end();
#endif
#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT
for (auto *listener : this->neutral_listeners_)
listener->on_scan_end();
#endif
}
this->notify_scan_end_();
this->set_scanner_state_(ScannerState::IDLE);
}
void ESP32BLETracker::notify_scan_end_() {
#ifdef ESPHOME_ESP32_BLE_TRACKER_CLIENT_COUNT
// Window-change restart continues the same scan period; the flag stays set
// across the stop and is cleared by the restart in start_scan_.
if (this->skip_next_scan_end_)
return;
#endif
#ifdef ESPHOME_ESP32_BLE_TRACKER_LISTENER_COUNT
for (auto *listener : this->listeners_)
listener->on_scan_end();
#endif
#ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT
for (auto *listener : this->neutral_listeners_)
listener->on_scan_end();
#endif
}
void ESP32BLETracker::handle_scanner_failure_() {
this->stop_scan_();
if (this->scan_start_fail_count_ == std::numeric_limits<uint8_t>::max()) {
@@ -231,6 +231,8 @@ class ESP32BLETracker final : public Component,
protected:
/// Returns true when a stop was issued to the controller.
bool stop_scan_();
/// Fire on_scan_end on every listener unless a window-change restart suppressed it.
void notify_scan_end_();
/// Start a single scan by setting up the parameters and doing some esp-idf calls.
void start_scan_(bool first);
/// Called when a `ESP_GAP_BLE_SCAN_RESULT_EVT` event is received.
@@ -321,6 +323,10 @@ class ESP32BLETracker final : public Component,
/// 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};
/// The window to scan at for the given number of active GATT connections.
uint32_t desired_scan_window_(uint8_t active) const {
return (this->connection_scan_window_ != 0 && active > 0) ? this->connection_scan_window_ : this->scan_window_;
}
#endif
esp_bt_status_t scan_start_failed_{ESP_BT_STATUS_SUCCESS};
esp_bt_status_t scan_set_param_failed_{ESP_BT_STATUS_SUCCESS};
@@ -348,9 +354,6 @@ class ESP32BLETracker final : public Component,
bool ble_was_disabled_ : 1 {true};
bool parse_advertisements_ : 1 {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 using_connection_window_ : 1 {false};
/// Suppress the window-change restart's on_scan_end sweeps (stop and start).
bool skip_next_scan_end_ : 1 {false};
#endif
@@ -363,10 +366,10 @@ class ESP32BLETracker final : public Component,
MONITORING, // Actively monitoring for timeout
EXCEEDED_WAIT, // Timeout exceeded, waiting one loop before reboot
};
ScanTimeoutState scan_timeout_state_{ScanTimeoutState::INACTIVE};
uint32_t scan_start_time_{0};
/// Precomputed timeout value: scan_duration_ * 2000
uint32_t scan_timeout_ms_{0};
ScanTimeoutState scan_timeout_state_{ScanTimeoutState::INACTIVE};
};
// NOLINTNEXTLINE
@@ -195,44 +195,29 @@ def test_connection_scan_window_truncation_collapse_rejected(
)
def test_codegen_connection_window_when_raised(
generate_main: Callable[[str | Path], str],
component_config_path: Callable[[str], Path],
) -> None:
main_cpp = generate_main(component_config_path("scan_window_raised.yaml"))
assert "set_scan_window(512)" in main_cpp
assert "set_connection_scan_window(48)" in main_cpp
def test_codegen_no_connection_window_for_explicit_window(
generate_main: Callable[[str | Path], str],
component_config_path: Callable[[str], Path],
) -> None:
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
def test_user_set_connection_window_warns_without_gatt_clients(
@pytest.mark.parametrize(
("config_file", "window_call", "connection_call", "warns"),
[
# Raised window with GATT clients: the injected fallback is emitted.
("scan_window_raised.yaml", "set_scan_window(512)", True, False),
# Explicit window: nothing injected.
("scan_window_explicit.yaml", "set_scan_window(48)", False, False),
# Scan-only build compiles the path out: the injected default is
# dropped silently, a user-set value warns.
("scan_window_scan_only.yaml", "set_scan_window(512)", False, False),
("scan_window_user_set_scan_only.yaml", "set_scan_window(512)", False, True),
],
)
def test_connection_scan_window_codegen(
generate_main: Callable[[str | Path], str],
component_config_path: Callable[[str], Path],
caplog: pytest.LogCaptureFixture,
config_file: str,
window_call: str,
connection_call: bool,
warns: bool,
) -> None:
"""A user-set value that cannot take effect warns; the injected default
(previous test) is dropped silently."""
main_cpp = generate_main(
component_config_path("scan_window_user_set_scan_only.yaml")
)
assert "set_connection_scan_window" not in main_cpp
assert "'connection_scan_window' has no effect" in caplog.text
main_cpp = generate_main(component_config_path(config_file))
assert window_call in main_cpp
assert ("set_connection_scan_window(48)" in main_cpp) == connection_call
assert ("'connection_scan_window' has no effect" in caplog.text) == warns