[esp32_ble_tracker] Warn when the scan window is above 600ms with wifi (#18725)

esphome/components/esp32_ble_tracker/__init__.py

@@ -143,6 +143,13 @@ def validate_max_connections_deprecated(config: ConfigType) -> ConfigType:
 # BLE uses the airtime wifi does not claim.
 IDF_SCAN_WINDOW_FIX_VERSION = cv.Version(5, 5, 5)
 
+# Above this the scanner holds the shared radio long enough that wifi drops
+# packets and connections on some access points (others cope fine, which is
+# why this is a warning and not an error); old proxy configs with 1100 ms
+# windows are a recurring cause of instability (esphome/esphome#18655). Only
+# wifi shares the radio; long windows are fine on ethernet builds.
+MAX_RECOMMENDED_WIFI_SCAN_WINDOW = TimePeriod(milliseconds=600)
+
 
 @dataclass
 class TrackerData:
@@ -209,6 +216,45 @@ def _raise_defaulted_scan_window(config: ConfigType) -> ConfigType:
     return config
 
 
+def _warn_long_scan_window_with_wifi(config: ConfigType) -> ConfigType:
+    """Warn when the scan window is long enough to starve wifi.
+
+    Runs after _raise_defaulted_scan_window so it sees the final window.
+    software_coexistence is only present when wifi is configured, so ethernet
+    builds never warn: BLE has the radio to itself there. Presence is what
+    matters, not the value; with the arbiter disabled a long window starves
+    wifi outright.
+    """
+    params = config[CONF_SCAN_PARAMETERS]
+    window = params[CONF_WINDOW]
+    if CONF_SOFTWARE_COEXISTENCE not in config:
+        return config
+    if window <= MAX_RECOMMENDED_WIFI_SCAN_WINDOW:
+        return config
+    if _get_data().scan_window_defaulted:
+        # The window was raised to match the interval, so point at the key the
+        # user actually set.
+        _LOGGER.warning(
+            "BLE scan interval of %s sets the scan window to the same value, "
+            "which starves wifi on the same radio and can cause wifi disconnects "
+            "depending on the access point; keep the interval at or below %s "
+            "(for example interval: 320ms). Long windows are only a problem with "
+            "wifi, they are fine on ethernet",
+            params[CONF_INTERVAL],
+            MAX_RECOMMENDED_WIFI_SCAN_WINDOW,
+        )
+        return config
+    _LOGGER.warning(
+        "BLE scan window of %s with wifi on the same radio starves wifi and "
+        "can cause wifi disconnects depending on the access point; keep the "
+        "window at or below %s (for example interval: 320ms, window: 300ms). "
+        "Long windows are only a problem with wifi, they are fine on ethernet",
+        window,
+        MAX_RECOMMENDED_WIFI_SCAN_WINDOW,
+    )
+    return config
+
+
 # 320 ms is the ESP-IDF reference scan interval; the shared schema also
 # tightens validation to the controller's 2.5 ms .. 10240 ms range and rejects
 # window/interval pairs that collapse to the same 0.625 ms unit count.
@@ -271,6 +317,7 @@ CONFIG_SCHEMA = cv.All(
     ).extend(cv.COMPONENT_SCHEMA),
     validate_max_connections_deprecated,
     _raise_defaulted_scan_window,
+    _warn_long_scan_window_with_wifi,
 )
 
 

tests/component_tests/esp32_ble_tracker/test_scan_window_default.py

@@ -12,6 +12,7 @@ arbiter a full-duty scan would starve wifi, so the 30 ms default is kept.
 from __future__ import annotations
 
 from collections.abc import Callable
+import logging
 from pathlib import Path
 
 import pytest
@@ -221,3 +222,54 @@ def test_connection_scan_window_codegen(
     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
+
+
+@pytest.mark.parametrize(
+    ("wifi", "params", "expect_warning"),
+    [
+        (True, {"interval": "1100ms", "window": "1100ms"}, True),
+        (True, {"interval": "1100ms", "window": "601ms"}, True),
+        (True, {"interval": "1100ms", "window": "600ms"}, False),
+        (False, {"interval": "1100ms", "window": "1100ms"}, False),
+    ],
+)
+def test_long_window_with_wifi_warns(
+    stage_esp32: Callable[..., None],
+    caplog: pytest.LogCaptureFixture,
+    wifi: bool,
+    params: ConfigType,
+    expect_warning: bool,
+) -> None:
+    """A scan window above 600 ms warns only when wifi shares the radio."""
+    stage_esp32("5.5.5", wifi=wifi)
+    with caplog.at_level(logging.WARNING):
+        _scan_params({"scan_parameters": params})
+    assert ("starves wifi" in caplog.text) is expect_warning
+
+
+def test_long_window_warns_with_coexistence_disabled(
+    stage_esp32: Callable[..., None],
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """Disabling the arbiter is the worst case for a long window, so it still warns."""
+    stage_esp32("5.5.5", wifi=True)
+    with caplog.at_level(logging.WARNING):
+        _scan_params(
+            {
+                CONF_SOFTWARE_COEXISTENCE: False,
+                "scan_parameters": {"interval": "1100ms", "window": "1100ms"},
+            }
+        )
+    assert "BLE scan window of 1100ms" in caplog.text
+
+
+def test_raised_window_warning_points_at_interval(
+    stage_esp32: Callable[..., None],
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """When the window was raised to a long interval, the warning names the interval."""
+    stage_esp32("5.5.5", wifi=True)
+    with caplog.at_level(logging.WARNING):
+        _scan_params({"scan_parameters": {"interval": "1s"}})
+    assert "BLE scan interval of 1s" in caplog.text
+    assert "BLE scan window of" not in caplog.text