WiFi scan, optimistic disconnect, and cursor-restore fixes

modalapi/wifi/commands.py

@@ -27,12 +27,8 @@
 
 
 class Command(ABC, Generic[T]):
-    """A unit of serialized work. Subclasses carry their args as fields.
-
-    `run(wm)` does the blocking work on a worker thread. `key()` is the dedup
-    identity — if a command with the same key is already pending or in-flight,
-    a fresh submission is silently dropped.
-    """
+    """A unit of serialized work. Deduped by key() — if a command with the
+    same key is pending or in-flight, a fresh submission is dropped."""
 
     @abstractmethod
     def run(self, wm: Any) -> T: ...
@@ -119,6 +115,8 @@ def key(self) -> str:
 
 @dataclass
 class ScanCmd(Command[list]):
+    """Trigger a fresh scan and return the results. Blocks for seconds."""
+
     def run(self, wm: "WifiManager") -> list:
         return wm.scan_networks()
 
@@ -130,13 +128,8 @@ def key(self) -> str:
 
 
 class CommandQueue:
-    """Serialized executor over a WifiManager. Drains submitted Commands on a
-    single daemon worker; delivers results on the main thread via poll().
-
-    Dedupes by Command.key(): a submission whose key matches a queued or
-    in-flight item is silently dropped. State-changing submissions bump
-    pending_op_count; scan submissions do not.
-    """
+    """Serialized executor over a WifiManager. Worker thread runs Commands;
+    results are delivered on the main thread via poll(). Dedupes by key()."""
 
     def __init__(self, wm: "WifiManager") -> None:
         self._wm = wm
@@ -180,6 +173,12 @@ def _drain(self) -> None:
                 self._pending_keys.discard(cmd.key())
                 if bumps_pending:
                     self._pending_op_count -= 1
+            if bumps_pending:
+                # Nudge the poller for fresh status — don't wait out the 5s tick.
+                try:
+                    self._wm.request_refresh()
+                except Exception:
+                    logging.exception("Status refresh request failed")
             self._result_queue.put((on_done, result))
 
     def poll(self) -> None:

modalapi/wifi/manager.py

@@ -25,8 +25,6 @@
 
 
 class WifiManager:
-    # Hard-wired wifi interface to avoid scrubbing sysfs.
-    # Hotspot state is read from / mutates via NetworkManager directly.
     def __init__(self, ifname: str = "wlan0", on_status_change: Optional[Callable[[WifiStatus], None]] = None) -> None:
         self.iface_name: str = ifname
         self.lock: threading.Lock = threading.Lock()
@@ -35,6 +33,8 @@ def __init__(self, ifname: str = "wlan0", on_status_change: Optional[Callable[[W
         self.changed: bool = False
         self.on_status_change: Optional[Callable[[WifiStatus], None]] = on_status_change
         self.stop: threading.Event = threading.Event()
+        self._wake: threading.Event = threading.Event()
+        self._force_publish: bool = False
         self.wireless_supported: bool = False
         self.wireless_file: str = os.path.join(os.sep, "sys", "class", "net", self.iface_name, "wireless")
         self.operstate_file: str = os.path.join(os.sep, "sys", "class", "net", self.iface_name, "operstate")
@@ -46,8 +46,25 @@ def __del__(self) -> None:
         logging.info("Wifi monitor cleanup")
         self.shutdown()
 
+    def set_status(self, status: WifiStatus) -> None:
+        """Optimistically overwrite cached status and publish it now. The next
+        poll reconciles against truth."""
+        with self.lock:
+            self.last_status = status
+            self.changed = False
+        if self.on_status_change is not None:
+            self.on_status_change(status)
+        self.request_refresh()
+
+    def request_refresh(self) -> None:
+        """Poll status now and publish even if unchanged."""
+        with self.lock:
+            self._force_publish = True
+        self._wake.set()
+
     def shutdown(self) -> None:
         self.stop.set()
+        self._wake.set()
         try:
             self.queue.shutdown()
         except Exception:
@@ -69,7 +86,6 @@ def _is_wifi_connected(self) -> bool:
             return False
 
     def _get_wpa_status(self, status: WifiStatus) -> None:
-        # `device show` rejects per-setting fields; fetch SSID/mode via `connection show` below.
         stdout, err = nmcli(
             ["device", "show", self.iface_name],
             terse_fields=["GENERAL.STATE", "GENERAL.CONNECTION", "IP4.ADDRESS"],
@@ -96,12 +112,15 @@ def _get_wpa_status(self, status: WifiStatus) -> None:
 
     def _polling_thread(self) -> None:
         while True:
+            # Claimed up front: a refresh requested mid-poll refers to state we
+            # haven't read yet, so it survives into the next pass.
+            with self.lock:
+                forced = self._force_publish
+                self._force_publish = False
+
             new_status: WifiStatus = {}
             supported = new_status["wifi_supported"] = self._is_wifi_supported()
             connected = new_status["wifi_connected"] = self._is_wifi_connected()
-            # Default false; _get_wpa_status flips it when the active wlan0
-            # connection has mode=ap. operstate is "up" in both client and AP
-            # modes, so `connected` covers both cases.
             new_status["hotspot_active"] = False
             if supported and connected:
                 self._get_wpa_status(new_status)
@@ -110,17 +129,22 @@ def _polling_thread(self) -> None:
 
             with self.lock:
                 self._cached_saved = saved
-                if new_status != self.last_status:
+                if forced or new_status != self.last_status:
                     logging.debug("Wifi status changed:" + str(new_status))
                     self.last_status = new_status
                     self.changed = True
 
-            if self.stop.wait(5.0):
+            if self._wait_next_poll(5.0):
                 break
 
+    def _wait_next_poll(self, timeout: float) -> bool:
+        """Sleep out the poll interval, cut short by request_refresh()."""
+        self._wake.wait(timeout)
+        self._wake.clear()
+        return self.stop.is_set()
+
     def poll(self) -> None:
-        """Main-thread tick. Drains write-op callbacks and fires
-        on_status_change when the polling thread has new status."""
+        """Main-thread tick: drain callbacks and publish status changes."""
         self.queue.poll()
         update: Optional[WifiStatus] = None
         with self.lock:

modalapi/wifi/ops.py

@@ -67,11 +67,12 @@ def wifi_profile_ssid_mode(uuid: str) -> tuple[str, str]:
 
 
 def scan_networks(iface_name: str) -> list[ScannedNetwork]:
-    """Return visible nearby networks, deduplicated by SSID (strongest wins), sorted by signal desc."""
+    """Scan and return visible networks, deduplicated by SSID (strongest wins)."""
     stdout, err = nmcli(
         ["dev", "wifi", "list", "--rescan", "yes", "ifname", iface_name],
+        sudo=True,
         terse_fields=["IN-USE", "SSID", "SIGNAL", "SECURITY"],
-        timeout=15,
+        timeout=30,
     )
     if err is not None or stdout is None:
         logging.error("nmcli dev wifi list failed: " + (err or b"").decode("utf-8", "replace"))
@@ -118,11 +119,7 @@ def delete_connection(name: str) -> Optional[bytes]:
 
 
 def connect_scanned(iface_name: str, ssid: str, security: str, psk: Optional[str] = None) -> Optional[bytes]:
-    """Create a profile for an SSID and activate it; on failure the new profile is deleted.
-
-    Activating a client profile via `nmcli connection up` on wlan0 implicitly
-    deactivates whatever is currently active on the device — including a
-    running AP — so no explicit hotspot teardown is needed first."""
+    """Create a profile for an SSID and activate it; on failure the new profile is deleted."""
     try:
         km = KeyMgmt.from_scan_security(security)
     except ValueError as e:
@@ -147,10 +144,11 @@ def connect_scanned(iface_name: str, ssid: str, security: str, psk: Optional[str
         "connection.autoconnect",
         "yes",
     ]
-    # nmcli treats wifi-sec.key-mgmt=none as WEP. For a genuinely open AP,
-    # the wifi-sec section must be omitted entirely.
+    # nmcli treats wifi-sec.key-mgmt=none as WEP; omit the section for open APs.
     if km != KeyMgmt.NONE:
         add_args += ["wifi-sec.key-mgmt", km]
+        if km == KeyMgmt.WPA_PSK:
+            add_args += ["wifi-sec.pmf", "optional"]
     if km in (KeyMgmt.WPA_PSK, KeyMgmt.SAE) and psk:
         add_args += ["wifi-sec.psk", psk]
     _, err = nmcli(add_args, sudo=True, timeout=20)
@@ -184,11 +182,7 @@ def is_profile_activated(name: str) -> bool:
 
 
 def connect_saved(name: str, wait: bool = True, reconnect: bool = False) -> Optional[bytes]:
-    """Activate a saved profile; with wait=False, NM keeps retrying in the background.
-
-    `nmcli connection up` on a wifi profile implicitly deactivates whatever is
-    currently active on wlan0 — including an AP-mode hotspot — so no explicit
-    teardown is needed first."""
+    """Activate a saved profile; with wait=False, NM keeps retrying in the background."""
     if not reconnect and is_profile_activated(name):
         return None
     args = ["--wait", "0", "connection", "up", name] if not wait else ["connection", "up", name]
@@ -236,11 +230,7 @@ def replace_psk(name: str, psk: str) -> Optional[bytes]:
 
 
 def find_hotspot_profile() -> Optional[str]:
-    """Return the name of an existing AP-mode wifi profile, or None.
-
-    The boot-fallback `wifi-hotspot.service` (shipped by both pi-gen-pistomp
-    and pistomp-arch) typically creates `pistomp-hotspot` on first run. Older
-    images may name it `Hotspot`. We accept whichever AP-mode profile exists."""
+    """Return the name of an existing AP-mode wifi profile, or None."""
     stdout, err = nmcli(
         ["connection", "show"],
         terse_fields=["NAME", "UUID", "TYPE"],
@@ -294,11 +284,7 @@ def _create_default_hotspot_profile(iface_name: str) -> Optional[bytes]:
 
 
 def enable_hotspot(iface_name: str) -> Optional[bytes]:
-    """Activate the AP-mode profile on wlan0, creating it if missing.
-
-    Drives NetworkManager directly — does not touch the systemd unit. NM will
-    deactivate any currently-active client connection on the device as a side
-    effect of bringing the AP up."""
+    """Activate the AP-mode profile on wlan0, creating it if missing."""
     name = find_hotspot_profile()
     if name is None:
         err = _create_default_hotspot_profile(iface_name)
@@ -310,11 +296,7 @@ def enable_hotspot(iface_name: str) -> Optional[bytes]:
 
 
 def disable_hotspot(iface_name: str) -> Optional[bytes]:
-    """Tear down the AP and reactivate the most-recent saved client profile.
-
-    NM doesn't autoconnect a client profile after AP teardown, so we drive
-    the reconnect explicitly. Returns None on success or when no saved
-    profile is available; nmcli stderr bytes otherwise."""
+    """Tear down the AP and reactivate the most-recent saved client profile."""
     name = find_hotspot_profile()
     if name is not None:
         _, err = nmcli(["connection", "down", name], sudo=True, timeout=20)

plugins/layouts/mode_selector.py

@@ -84,7 +84,7 @@ def _open_dialog(self) -> None:
         for label, value in self._param.get_enum_value_list():
             selected = value == current_value
             if selected:
-                default_item = f"\u2714 {label}"
+                default_item = label
             items.append((label, self._commit_value, value, selected))
         title = f"{self._param.instance_id}:{self._param.name}"
         lcd.draw_selection_menu(items, title, auto_dismiss=True, default_item=default_item)

tests/test_command_queue.py

@@ -3,6 +3,7 @@
 import threading
 import time
 from dataclasses import dataclass, field
+from unittest.mock import patch
 
 import pytest
 
@@ -16,6 +17,11 @@ class _Ctx:
     lock: threading.Lock = field(default_factory=threading.Lock)
     started: list = field(default_factory=list)
     finished: list = field(default_factory=list)
+    refreshes: int = 0
+
+    def request_refresh(self) -> None:
+        with self.lock:
+            self.refreshes += 1
 
 
 @dataclass
@@ -145,6 +151,35 @@ def call_poll():
     assert len(err) == 1
 
 
+def test_write_op_requests_status_refresh_but_scan_does_not(queue, ctx):
+    """A write op may have changed connectivity, so the poller is nudged for
+    fresh status. A scan changes nothing — nudging on every 2s scan would spin
+    the poller for no reason."""
+    results: list = []
+    queue.submit(SleepCmd("write", delay=0.0), results.append)
+    _drain(queue, results, 1)
+    assert ctx.refreshes == 1
+
+    queue.submit_scan(SleepCmd("scan", delay=0.0), results.append)
+    _drain(queue, results, 2)
+    assert ctx.refreshes == 1
+
+
+def test_worker_survives_a_failing_refresh(queue, ctx):
+    """A blowup in the post-op refresh must not kill the worker — that would
+    silently wedge every later wifi command."""
+    with patch.object(_Ctx, "request_refresh", side_effect=RuntimeError("nope")):
+        results: list = []
+        queue.submit(SleepCmd("A", delay=0.0), results.append)
+        _drain(queue, results, 1)
+        assert results == ["A"]
+
+    results2: list = []
+    queue.submit(SleepCmd("B", delay=0.0), results2.append)
+    _drain(queue, results2, 1)
+    assert results2 == ["B"]
+
+
 def test_shutdown_joins_worker(ctx):
     """shutdown() returns promptly even with no work pending."""
     q = CommandQueue(ctx)