mawei/sunnypilot
1
0
Fork 0
mirror of https://github.com/sunnypilot/sunnypilot.git synced 2026-04-07 04:33:58 +08:00
Code Issues Packages Projects Releases Wiki Activity

WifiManager: fix connect flash while forgetting (#37416)

Browse Source 
* real traces for some tests

combine and new test for low strength/turn off hotspot while connecting

revert wifiui

* stupid llm

* clean up
This commit is contained in:
Shane Smiskol
2026-02-26 01:10:35 -08:00
committed by GitHub
parent 561c490b2a
commit cf5ae3cbca
3 changed files with 191 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
system/ui/lib/networkmanager.py
View File

@@ -26,6 +26,7 @@ class NMDeviceStateReason(IntEnum):
IP_CONFIG_UNAVAILABLE = 5
NO_SECRETS = 7
SUPPLICANT_DISCONNECT = 8
SUPPLICANT_TIMEOUT = 11
CONNECTION_REMOVED = 38
USER_REQUESTED = 39
SSID_NOT_FOUND = 53

197
system/ui/lib/tests/test_handle_state_change.py
View File

@@ -93,16 +93,42 @@ class TestDisconnected:
class TestDeactivating:
def test_deactivating_is_noop(self, mocker):
"""DEACTIVATING is a no-op — DISCONNECTED follows with the correct reason."""
def test_deactivating_noop_for_non_connection_removed(self, mocker):
"""DEACTIVATING with non-CONNECTION_REMOVED reason is a no-op."""
wm = _make_wm(mocker)
wm._wifi_state = WifiState(ssid="Net", status=ConnectStatus.CONNECTED)
fire(wm, NMDeviceState.DEACTIVATING, reason=NMDeviceStateReason.CONNECTION_REMOVED)
fire(wm, NMDeviceState.DEACTIVATING, reason=NMDeviceStateReason.USER_REQUESTED)
assert wm._wifi_state.ssid == "Net"
assert wm._wifi_state.status == ConnectStatus.CONNECTED
@pytest.mark.parametrize("status, expected_clears", [
(ConnectStatus.CONNECTED, True),
(ConnectStatus.CONNECTING, False),
])
def test_deactivating_connection_removed(self, mocker, status, expected_clears):
"""DEACTIVATING(CONNECTION_REMOVED) clears CONNECTED but preserves CONNECTING.
CONNECTED: forgetting the current network. The forgotten callback fires between
DEACTIVATING and DISCONNECTED — must clear here so the UI doesn't flash "connected"
after the eager _network_forgetting flag resets.
CONNECTING: forget A while connecting to B. DEACTIVATING fires for A's removal,
but B's CONNECTING state must be preserved.
"""
wm = _make_wm(mocker, connections={"B": "/path/B"})
wm._wifi_state = WifiState(ssid="B" if status == ConnectStatus.CONNECTING else "A", status=status)
fire(wm, NMDeviceState.DEACTIVATING, reason=NMDeviceStateReason.CONNECTION_REMOVED)
if expected_clears:
assert wm._wifi_state.ssid is None
assert wm._wifi_state.status == ConnectStatus.DISCONNECTED
else:
assert wm._wifi_state.ssid == "B"
assert wm._wifi_state.status == ConnectStatus.CONNECTING
class TestPrepareConfig:
def test_user_initiated_skips_dbus_lookup(self, mocker):
@@ -170,7 +196,17 @@ class TestNeedAuth:
cb.assert_called_once_with("SecNet")
def test_failed_no_secrets_fires_callback(self, mocker):
"""FAILED+NO_SECRETS = wrong password (weak/gone network)."""
"""FAILED+NO_SECRETS = wrong password (weak/gone network).
Confirmed on device: also fires when a hotspot turns off during connection.
NM can't complete the WPA handshake (AP vanished) and reports NO_SECRETS
rather than SSID_NOT_FOUND. The need_auth callback fires, so the UI shows
"wrong password" — a false positive, but same signal path.
Real device sequence (new connection, hotspot turned off immediately):
PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) → PREPARE(NEED_AUTH) → CONFIG
→ NEED_AUTH(CONFIG, NONE) → FAILED(NEED_AUTH, NO_SECRETS) → DISCONNECTED(FAILED, NONE)
"""
wm = _make_wm(mocker)
cb = mocker.MagicMock()
wm.add_callbacks(need_auth=cb)
@@ -304,8 +340,9 @@ class TestActivated:
# and discards the stale update.
# ---------------------------------------------------------------------------
# The deterministic fixes (skip DBus lookup when ssid already set, prev_state guard
# on NEED_AUTH, DEACTIVATING no-op, CONNECTION_REMOVED guard) shrink these race
# windows significantly. The epoch counter closes the remaining gaps.
# on NEED_AUTH, DEACTIVATING clears CONNECTED on CONNECTION_REMOVED, CONNECTION_REMOVED
# guard) shrink these race windows significantly. The epoch counter closes the
# remaining gaps.
class TestThreadRaces:
def test_prepare_race_user_tap_during_dbus(self, mocker):
@@ -410,14 +447,17 @@ class TestFullSequences:
def test_wrong_password_then_retry(self, mocker):
"""Wrong password → NEED_AUTH → FAILED → NM auto-reconnects to saved network.
Real device sequence:
PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) ← WPA handshake
Confirmed on device: wrong password for Shane's iPhone, NM auto-connected to unifi.
Real device sequence (switching from a connected network):
DEACTIVATING(ACTIVATED, NEW_ACTIVATION) → DISCONNECTED(DEACTIVATING, NEW_ACTIVATION)
→ PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) ← WPA handshake
→ PREPARE(NEED_AUTH, NONE) → CONFIG
→ NEED_AUTH(CONFIG, SUPPLICANT_DISCONNECT) ← wrong password
→ FAILED(NEED_AUTH, NO_SECRETS) ← NM gives up
→ DISCONNECTED(FAILED, NONE)
→ PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) → PREPARE ← auto-reconnect
→ CONFIG → IP_CONFIG → ... → ACTIVATED
→ PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) → PREPARE(NEED_AUTH) → CONFIG
→ IP_CONFIG → IP_CHECK → SECONDARIES → ACTIVATED ← auto-reconnect to other saved network
"""
wm = _make_wm(mocker, connections={"Sec": "/path/sec"})
cb = mocker.MagicMock()
@@ -515,6 +555,80 @@ class TestFullSequences:
fire_wpa_connect(wm)
assert wm._wifi_state.status == ConnectStatus.CONNECTED
def test_forget_while_connecting(self, mocker):
"""Forget the network we're currently connecting to (not yet ACTIVATED).
Confirmed on device: connected to unifi, tapped Shane's iPhone, then forgot
Shane's iPhone while at CONFIG. NM auto-connected to unifi afterward.
Real device sequence (switching then forgetting mid-connection):
DEACTIVATING(ACTIVATED, NEW_ACTIVATION) → DISCONNECTED(DEACTIVATING, NEW_ACTIVATION)
→ PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) → PREPARE(NEED_AUTH) → CONFIG
→ DEACTIVATING(CONFIG, CONNECTION_REMOVED) ← forget at CONFIG
→ DISCONNECTED(DEACTIVATING, CONNECTION_REMOVED)
→ PREPARE → CONFIG → ... → ACTIVATED ← NM auto-connects to other saved network
Note: DEACTIVATING fires from CONFIG (not ACTIVATED). wifi_state.status is
CONNECTING, so the DEACTIVATING handler is a no-op. DISCONNECTED clears state
(ssid removed from _connections by ConnectionRemoved), then PREPARE recovers
via DBus lookup for the auto-connect.
"""
wm = _make_wm(mocker, connections={"A": "/path/A", "Other": "/path/other"})
wm._get_active_wifi_connection.return_value = ("/path/other", {})
wm._set_connecting("A")
fire(wm, NMDeviceState.PREPARE)
fire(wm, NMDeviceState.CONFIG)
assert wm._wifi_state.ssid == "A"
assert wm._wifi_state.status == ConnectStatus.CONNECTING
# User forgets A: ConnectionRemoved processed first, then state changes
del wm._connections["A"]
fire(wm, NMDeviceState.DEACTIVATING, prev_state=NMDeviceState.CONFIG,
reason=NMDeviceStateReason.CONNECTION_REMOVED)
assert wm._wifi_state.ssid == "A"
assert wm._wifi_state.status == ConnectStatus.CONNECTING # DEACTIVATING preserves CONNECTING
fire(wm, NMDeviceState.DISCONNECTED, prev_state=NMDeviceState.DEACTIVATING,
reason=NMDeviceStateReason.CONNECTION_REMOVED)
assert wm._wifi_state.ssid is None
assert wm._wifi_state.status == ConnectStatus.DISCONNECTED
# NM auto-connects to another saved network
fire(wm, NMDeviceState.PREPARE)
assert wm._wifi_state.ssid == "Other"
assert wm._wifi_state.status == ConnectStatus.CONNECTING
fire(wm, NMDeviceState.CONFIG)
fire_wpa_connect(wm)
assert wm._wifi_state.status == ConnectStatus.CONNECTED
assert wm._wifi_state.ssid == "Other"
def test_forget_connected_network(self, mocker):
"""Forget the currently connected network (not switching to another).
Real device sequence:
DEACTIVATING(ACTIVATED, CONNECTION_REMOVED) → DISCONNECTED(DEACTIVATING, CONNECTION_REMOVED)
ConnectionRemoved signal may or may not have been processed before state changes.
Either way, state must clear — we're forgetting what we're connected to, not switching.
"""
wm = _make_wm(mocker, connections={"A": "/path/A"})
wm._wifi_state = WifiState(ssid="A", status=ConnectStatus.CONNECTED)
fire(wm, NMDeviceState.DEACTIVATING, prev_state=NMDeviceState.ACTIVATED,
reason=NMDeviceStateReason.CONNECTION_REMOVED)
assert wm._wifi_state.ssid is None
assert wm._wifi_state.status == ConnectStatus.DISCONNECTED
# DISCONNECTED follows — harmless since state is already cleared
fire(wm, NMDeviceState.DISCONNECTED, prev_state=NMDeviceState.DEACTIVATING,
reason=NMDeviceStateReason.CONNECTION_REMOVED)
assert wm._wifi_state.ssid is None
assert wm._wifi_state.status == ConnectStatus.DISCONNECTED
def test_forget_A_connect_B(self, mocker):
"""Forget A while connecting to B: full signal sequence.
@@ -613,16 +727,69 @@ class TestFullSequences:
assert wm._wifi_state.status == ConnectStatus.CONNECTED
assert wm._wifi_state.ssid == "AutoNet"
def test_network_lost_during_connection(self, mocker):
"""Hotspot turned off while connecting (before ACTIVATED).
Confirmed on device: started new connection to Shane's iPhone, immediately
turned off the hotspot. NM can't complete WPA handshake and reports
FAILED(NO_SECRETS) — same signal as wrong password (false positive).
Real device sequence:
PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) → PREPARE(NEED_AUTH) → CONFIG
→ NEED_AUTH(CONFIG, NONE) → FAILED(NEED_AUTH, NO_SECRETS) → DISCONNECTED(FAILED, NONE)
Note: no DEACTIVATING, no SUPPLICANT_DISCONNECT. The NEED_AUTH(CONFIG, NONE) is the
normal WPA handshake (not an error). NM gives up with NO_SECRETS because the AP
vanished mid-handshake.
"""
wm = _make_wm(mocker, connections={"Hotspot": "/path/hs"})
cb = mocker.MagicMock()
wm.add_callbacks(need_auth=cb)
wm._set_connecting("Hotspot")
fire(wm, NMDeviceState.PREPARE)
fire(wm, NMDeviceState.CONFIG)
fire(wm, NMDeviceState.NEED_AUTH) # WPA handshake (reason=NONE)
fire(wm, NMDeviceState.PREPARE, prev_state=NMDeviceState.NEED_AUTH)
fire(wm, NMDeviceState.CONFIG)
assert wm._wifi_state.status == ConnectStatus.CONNECTING
# Second NEED_AUTH(CONFIG, NONE) — NM retries handshake, AP vanishing
fire(wm, NMDeviceState.NEED_AUTH)
assert wm._wifi_state.status == ConnectStatus.CONNECTING
# NM gives up — reports NO_SECRETS (same as wrong password)
fire(wm, NMDeviceState.FAILED, prev_state=NMDeviceState.NEED_AUTH,
reason=NMDeviceStateReason.NO_SECRETS)
assert wm._wifi_state.status == ConnectStatus.DISCONNECTED
assert len(wm._callback_queue) == 1
fire(wm, NMDeviceState.DISCONNECTED, prev_state=NMDeviceState.FAILED)
assert wm._wifi_state.ssid is None
assert wm._wifi_state.status == ConnectStatus.DISCONNECTED
wm.process_callbacks()
cb.assert_called_once_with("Hotspot")
@pytest.mark.xfail(reason="TODO: FAILED(SSID_NOT_FOUND) should emit error for UI")
def test_ssid_not_found(self, mocker):
"""Network drops off after connection starts.
"""Network drops off while connected — hotspot turned off.
NM docs: SSID_NOT_FOUND (53) = "The WiFi network could not be found"
Expected sequence: PREPARE → CONFIG → FAILED(SSID_NOT_FOUND) → DISCONNECTED
NOTE: SSID_NOT_FOUND is rare. On-device testing with a disappearing hotspot typically
produces FAILED(NO_SECRETS) instead. May be driver-specific or require the network
to vanish from scan results mid-connection.
Confirmed on device: connected to Shane's iPhone, then turned off the hotspot.
No DEACTIVATING fires — NM goes straight from ACTIVATED to FAILED(SSID_NOT_FOUND).
NM retries connecting (PREPARE → CONFIG → ... → FAILED(CONFIG, SSID_NOT_FOUND))
before finally giving up with DISCONNECTED.
NOTE: turning off a hotspot during initial connection (before ACTIVATED) typically
produces FAILED(NO_SECRETS) instead of SSID_NOT_FOUND (see test_failed_no_secrets).
Real device sequence (hotspot turned off while connected):
FAILED(ACTIVATED, SSID_NOT_FOUND) → DISCONNECTED(FAILED, NONE)
→ PREPARE → CONFIG → NEED_AUTH(CONFIG, NONE) → PREPARE(NEED_AUTH) → CONFIG
→ NEED_AUTH(CONFIG, NONE) → PREPARE(NEED_AUTH) → CONFIG
→ FAILED(CONFIG, SSID_NOT_FOUND) → DISCONNECTED(FAILED, NONE)
The UI error callback mechanism is intentionally deferred — for now just clear state.
"""

11
system/ui/lib/wifi_manager.py
View File

@@ -394,7 +394,8 @@ class WifiManager:
# on every user action. Handlers snapshot the epoch before their DBus call and compare
# after: if it changed, a user action occurred during the call and the stale result is
# discarded. Combined with deterministic fixes (skip DBus lookup when ssid already set,
# DEACTIVATING no-op, CONNECTION_REMOVED guard), all known race windows are closed.
# DEACTIVATING clears CONNECTED on CONNECTION_REMOVED, CONNECTION_REMOVED guard),
# all known race windows are closed.
# TODO: Handle (FAILED, SSID_NOT_FOUND) and emit for UI to show error
# Happens when network drops off after starting connection
@@ -480,8 +481,12 @@ class WifiManager:
cloudlog.warning(f"Failed to persist connection to disk: {save_reply}")
elif new_state == NMDeviceState.DEACTIVATING:
# no-op — DISCONNECTED always follows with the correct reason
pass
# Must clear state when forgetting the currently connected network so the UI
# doesn't flash "connected" after the eager "forgetting..." state resets
# (the forgotten callback fires between DEACTIVATING and DISCONNECTED).
# Only clear CONNECTED — CONNECTING must be preserved for forget-A-connect-B.
if change_reason == NMDeviceStateReason.CONNECTION_REMOVED and self._wifi_state.status == ConnectStatus.CONNECTED:
self._set_connecting(None)
def _network_scanner(self):
while not self._exit: