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

WifiManager: don't emit need auth for partially connected networks (#37397)

Browse Source 
* fix a few

* document

* now remove unused prev_ssid

* more
This commit is contained in:
Shane Smiskol
2026-02-25 02:54:15 -08:00
committed by GitHub
parent d9b5a1e30b
commit 72ecc330e2
2 changed files with 12 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -221,17 +221,12 @@ class TestNeedAuth:
assert len(wm._callback_queue) == 0
@pytest.mark.xfail(reason="TODO: interrupted auth (prev=DISCONNECTED) should be ignored")
def test_interrupted_auth_ignored(self, mocker):
"""Switching A->B: NEED_AUTH from A (prev=DISCONNECTED) must not fire callback.
Reproduced on device: rapidly switching between two saved networks can trigger a
rare false "wrong password" dialog for the previous network, even though both have
correct passwords. The stale NEED_AUTH has prev_state=DISCONNECTED (not CONFIG).
Fix: the handler's second param is currently `_` (unused). Rename to `prev_state`
and only fire the NEED_AUTH callback when prev_state indicates a real auth failure
(e.g. prev_state == CONFIG), not a stale signal from a prior connection.
"""
wm = _make_wm(mocker)
cb = mocker.MagicMock()
@@ -246,26 +241,6 @@ class TestNeedAuth:
assert wm._wifi_state.status == ConnectStatus.CONNECTING
assert len(wm._callback_queue) == 0
def test_need_auth_targets_previous_ssid_via_prev_ssid(self, mocker):
"""Switch A→B, late NEED_AUTH arrives: prev_ssid mechanism fires callback for A.
This tests current prev_ssid behavior which we plan to remove.
Migration: (1) add prev_state guard to NEED_AUTH (see test_interrupted_auth_ignored),
(2) remove prev_ssid from WifiState and handler, (3) delete this test — it becomes
redundant with test_interrupted_auth_ignored once prev_state is the guard mechanism.
"""
wm = _make_wm(mocker)
cb = mocker.MagicMock()
wm.add_callbacks(need_auth=cb)
wm._set_connecting("A")
wm._set_connecting("B")
fire(wm, NMDeviceState.NEED_AUTH, reason=NMDeviceStateReason.SUPPLICANT_DISCONNECT)
assert len(wm._callback_queue) == 1
wm.process_callbacks()
cb.assert_called_once_with("A")
class TestPassthroughStates:
"""NEED_AUTH (generic), IP_CONFIG, IP_CHECK, SECONDARIES, FAILED (generic) are no-ops."""
@@ -335,7 +310,6 @@ class TestActivated:
# on NEED_AUTH) also shrink these race windows to near-zero. If races are still
# visible after, make WifiState frozen (replace() + single atomic assignment) and/or
# add a narrow lock around _wifi_state reads/writes (not around DBus calls).
# The NEED_AUTH prev_ssid mutation race is eliminated by removing prev_ssid entirely.
class TestThreadRaces:
@pytest.mark.xfail(reason="TODO: PREPARE overwrites _set_connecting via stale DBus lookup")
@@ -482,7 +456,6 @@ class TestFullSequences:
assert wm._wifi_state.status == ConnectStatus.CONNECTED
assert wm._wifi_state.ssid == "B"
@pytest.mark.xfail(reason="TODO: interrupted auth from switching should not fire need_auth")
def test_rapid_switch_no_false_wrong_password(self, mocker):
"""Switch A→B quickly: A's interrupted NEED_AUTH must NOT show wrong password.

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

@@ -148,7 +148,6 @@ class ConnectStatus(IntEnum):
@dataclass
class WifiState:
ssid: str | None = None
prev_ssid: str | None = None
status: ConnectStatus = ConnectStatus.DISCONNECTED
@@ -292,10 +291,7 @@ class WifiManager:
return self._tethering_password
def _set_connecting(self, ssid: str | None):
# Track prev ssid so late NEED_AUTH signals target the right network
self._wifi_state = WifiState(ssid=ssid,
prev_ssid=self.connecting_to_ssid if ssid is not None else None,
status=ConnectStatus.DISCONNECTED if ssid is None else ConnectStatus.CONNECTING)
self._wifi_state = WifiState(ssid=ssid, status=ConnectStatus.DISCONNECTED if ssid is None else ConnectStatus.CONNECTING)
def _enqueue_callbacks(self, cbs: list[Callable], *args):
for cb in cbs:
@@ -377,7 +373,7 @@ class WifiManager:
self._handle_state_change(new_state, previous_state, change_reason)
def _handle_state_change(self, new_state: int, _: int, change_reason: int):
def _handle_state_change(self, new_state: int, prev_state: int, change_reason: int):
# TODO: known race conditions when switching networks (e.g. forget A, connect to B):
# 1. DEACTIVATING/DISCONNECTED + CONNECTION_REMOVED: fires before NewConnection for B
# arrives, so _set_connecting(None) clears B's CONNECTING state causing UI flicker.
@@ -398,8 +394,6 @@ class WifiManager:
# deterministic fix (skip DBus lookup when ssid is set) also shrinks the race
# window to near-zero since the read/write happen back-to-back under the GIL.
# ACTIVATED: same read-then-write pattern with a DBus call in between.
# NEED_AUTH: mutates _wifi_state.prev_ssid in place, which can corrupt a new
# WifiState created by _set_connecting on the main thread.
# The deterministic fixes (skip DBus lookup when ssid set, prev_state guard) shrink
# the race windows significantly. If still visible, add a narrow lock around
# _wifi_state reads/writes (not around DBus calls, to avoid blocking the UI thread).
@@ -424,17 +418,18 @@ class WifiManager:
self._wifi_state = wifi_state
# BAD PASSWORD - use prev if current has already moved on to a new connection
# BAD PASSWORD
# - strong network rejects with NEED_AUTH+SUPPLICANT_DISCONNECT
# - weak/gone network fails with FAILED+NO_SECRETS
elif ((new_state == NMDeviceState.NEED_AUTH and change_reason == NMDeviceStateReason.SUPPLICANT_DISCONNECT) or
# prev_state guard: real auth failures come from CONFIG (supplicant handshake).
# Stale NEED_AUTH from a prior connection during network switching arrives with
# prev_state=DISCONNECTED and must be ignored to avoid a false wrong-password callback.
elif ((new_state == NMDeviceState.NEED_AUTH and change_reason == NMDeviceStateReason.SUPPLICANT_DISCONNECT
and prev_state == NMDeviceState.CONFIG) or
(new_state == NMDeviceState.FAILED and change_reason == NMDeviceStateReason.NO_SECRETS)):
failed_ssid = self._wifi_state.prev_ssid or self._wifi_state.ssid
if failed_ssid:
self._enqueue_callbacks(self._need_auth, failed_ssid)
self._wifi_state.prev_ssid = None
if self._wifi_state.ssid == failed_ssid:
if self._wifi_state.ssid:
self._enqueue_callbacks(self._need_auth, self._wifi_state.ssid)
self._set_connecting(None)
elif new_state in (NMDeviceState.NEED_AUTH, NMDeviceState.IP_CONFIG, NMDeviceState.IP_CHECK,
@@ -443,7 +438,7 @@ class WifiManager:
elif new_state == NMDeviceState.ACTIVATED:
# Note that IP address from Ip4Config may not be propagated immediately and could take until the next scan results
wifi_state = replace(self._wifi_state, prev_ssid=None, status=ConnectStatus.CONNECTED)
wifi_state = replace(self._wifi_state, status=ConnectStatus.CONNECTED)
conn_path, _ = self._get_active_wifi_connection(self._conn_monitor)
if conn_path is None: