From 46a8d5a20d4aee71bb96b2e28f9e59a61e7ea095 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Fri, 28 Aug 2026 12:11:56 +0100 Subject: [PATCH 1/2] fix(network): recover stale D-Bus paths and support full radio off --- BlocksScreen/lib/network/worker.py | 481 +++++++++++++++++++++-------- tests/network/test_worker_unit.py | 32 +- 2 files changed, 383 insertions(+), 130 deletions(-) diff --git a/BlocksScreen/lib/network/worker.py b/BlocksScreen/lib/network/worker.py index b983dd87..a0c1632f 100644 --- a/BlocksScreen/lib/network/worker.py +++ b/BlocksScreen/lib/network/worker.py @@ -8,7 +8,7 @@ import socket as _socket import struct import threading -from typing import Callable +from collections.abc import Awaitable, Callable from uuid import uuid4 import sdbus @@ -40,6 +40,13 @@ _DEBOUNCE_DELAY: float = 0.8 # Delay before restarting a failed signal listener (seconds). _LISTENER_RESTART_DELAY: float = 3.0 +# Ceiling for the listener restart back-off (seconds). +_LISTENER_RESTART_MAX_DELAY: float = 60.0 +# Back-off bounds for reopening the system bus when it is not up at boot. +_BUS_RETRY_DELAY: float = 1.0 +_BUS_RETRY_MAX_DELAY: float = 30.0 +# Upper bound on awaiting cancelled tasks during shutdown (seconds). +_SHUTDOWN_DRAIN_TIMEOUT: float = 2.0 # Timeout for _wait_for_connection: must cover 802.11 handshake + DHCP. _WIFI_CONNECT_TIMEOUT: float = 20.0 @@ -76,9 +83,12 @@ def __init__(self) -> None: """ super().__init__() self._running: bool = False + self._stopping: bool = False self._system_bus: sdbus.SdBus | None = None + # Set once no interface was found, so rediscovery does not re-alarm the UI. + self._no_iface_reported: bool = False - # Path strings only — read-proxies are always created fresh. + # Path strings only - read-proxies are always created fresh. self._primary_wifi_path: str = "" self._primary_wifi_iface: str = "" self._primary_wired_path: str = "" @@ -106,9 +116,11 @@ def __init__(self) -> None: # Tracked for cancellation during shutdown. self._listener_tasks: list[asyncio.Task] = [] - # Asyncio loop — created here, driven on the daemon thread. - self.stop_event = asyncio.Event() - self.stop_event.clear() + # Serialises interface rediscovery across the listener tasks. + self._rediscover_lock = asyncio.Lock() + self._rediscover_gen = 0 + self._stale_logged_gen = -1 + self._asyncio_loop: asyncio.AbstractEventLoop = asyncio.new_event_loop() self._asyncio_thread = threading.Thread( target=self._run_asyncio_loop, @@ -118,22 +130,35 @@ def __init__(self) -> None: self._asyncio_thread.start() def _run_asyncio_loop(self) -> None: - """Open the system D-Bus and run the asyncio event loop on this thread.""" + """Run the asyncio event loop on this thread, bootstrapping the bus on it.""" asyncio.set_event_loop(self._asyncio_loop) - try: - self._system_bus = sdbus.sd_bus_open_system() - sdbus.set_default_bus(self._system_bus) - self._track_task( - self._asyncio_loop.create_task(self._async_initialize(), name="nm_init") - ) - logger.debug( - "D-Bus opened on asyncio thread '%s'", - threading.current_thread().name, - ) - except Exception as exc: - logger.error("Failed to open system D-Bus: %s", exc) + self._track_task( + self._asyncio_loop.create_task(self._async_bootstrap(), name="nm_bootstrap") + ) self._asyncio_loop.run_forever() + async def _async_bootstrap(self) -> None: + """Open the system D-Bus with back-off, then initialise; dbus may lag us at boot.""" + delay = _BUS_RETRY_DELAY + while not self._stopping: + try: + self._system_bus = sdbus.sd_bus_open_system() + sdbus.set_default_bus(self._system_bus) + logger.debug( + "D-Bus opened on asyncio thread '%s'", + threading.current_thread().name, + ) + await self._async_initialize() + return + except Exception as exc: + self._system_bus = None + logger.error( + "Failed to open system D-Bus: %s - retrying in %.1f s", exc, delay + ) + self.error_occurred.emit("initialize", f"No D-Bus connection: {exc}") + await asyncio.sleep(delay) + delay = min(delay * 2, _BUS_RETRY_MAX_DELAY) + def _track_task(self, task: asyncio.Task) -> None: """Register a background task so it is cancelled on shutdown.""" self._background_tasks.add(task) @@ -141,13 +166,9 @@ def _track_task(self, task: asyncio.Task) -> None: async def _async_shutdown(self) -> None: """Tear down all async state and stop the event loop.""" + self._stopping = True self._running = False - for task in self._listener_tasks: - if not task.done(): - task.cancel() - self._listener_tasks.clear() - if self._state_debounce_handle: self._state_debounce_handle.cancel() self._state_debounce_handle = None @@ -155,10 +176,7 @@ async def _async_shutdown(self) -> None: self._scan_debounce_handle.cancel() self._scan_debounce_handle = None - self._signal_nm = None - self._signal_wifi = None - self._signal_wired = None - self._signal_settings = None + self._reset_signal_proxies() self._primary_wifi_path = "" self._primary_wifi_iface = "" @@ -167,14 +185,43 @@ async def _async_shutdown(self) -> None: self._iface_to_device_path.clear() self._saved_cache.clear() - for task in list(self._background_tasks): - if not task.done(): - task.cancel() + # Await cancellation: dropping the bus mid-call is what hangs shutdown. + current = asyncio.current_task() + pending = [ + task + for task in {*self._listener_tasks, *self._background_tasks} + if task is not current + and isinstance(task, asyncio.Task) + and not task.done() + ] + for task in pending: + task.cancel() + if pending: + try: + await asyncio.wait_for( + asyncio.gather(*pending, return_exceptions=True), + timeout=_SHUTDOWN_DRAIN_TIMEOUT, + ) + except TimeoutError: + logger.warning( + "%d task(s) did not stop within %.1f s", + sum(1 for t in pending if not t.done()), + _SHUTDOWN_DRAIN_TIMEOUT, + ) + self._listener_tasks.clear() self._background_tasks.clear() + self._system_bus = None logger.info("NetworkManagerWorker async shutdown complete") self._asyncio_loop.call_soon_threadsafe(self._asyncio_loop.stop) + def _reset_signal_proxies(self) -> None: + """Drop the persistent signal proxies so they are rebuilt against a fresh bus.""" + self._signal_nm = None + self._signal_wifi = None + self._signal_wired = None + self._signal_settings = None + def _nm(self) -> dbus_nm.NetworkManager: """Return a fresh NetworkManager root D-Bus proxy.""" return dbus_nm.NetworkManager(bus=self._system_bus) @@ -331,14 +378,16 @@ async def _detect_interfaces(self) -> None: Iterates all NetworkManager devices, maps interface names to D-Bus object paths, and stores the first WIFI and ETHERNET device found as the primary interfaces used for all subsequent operations. Emits - ``error_occurred`` if no interfaces at all are found. + ``error_occurred`` once if no interfaces at all are found. """ try: devices = await self._nm().get_devices() + # NM reuses object paths across restarts; a stale entry gives a wrong IP. + self._iface_to_device_path.clear() for device_path in devices: device = self._generic(device_path) device_type = await device.device_type - iface_name = await self._generic(device_path).interface + iface_name = await device.interface if iface_name: self._iface_to_device_path[iface_name] = device_path @@ -358,12 +407,15 @@ async def _detect_interfaces(self) -> None: logger.error("Failed to detect interfaces: %s", exc) if not self._primary_wifi_path and not self._primary_wired_path: - # Both absent — likely D-Bus not ready yet or no hardware present. logger.warning("No network interfaces detected after scan") - self.error_occurred.emit("wifi_unavailable", "No network device found") - elif not self._primary_wifi_path: - # Ethernet-only or Wi-Fi driver still loading — log but don't alarm. - logger.warning("No Wi-Fi interface detected; ethernet-only mode") + # Emit once: rediscovery reruns this on every listener restart. + if not self._no_iface_reported: + self._no_iface_reported = True + self.error_occurred.emit("wifi_unavailable", "No network device found") + else: + self._no_iface_reported = False + if not self._primary_wifi_path: + logger.warning("No Wi-Fi interface detected; ethernet-only mode") async def _set_wired_profiles_autoconnect(self, enabled: bool) -> None: """Persist autoconnect on every wired profile; Device.Autoconnect dies on NM restart.""" @@ -428,10 +480,12 @@ async def _start_signal_listeners(self) -> None: logger.info("Started %d D-Bus signal listeners", len(self._listener_tasks)) async def _resilient_listener( - self, name: str, listener_fn: "asyncio.coroutines" + self, name: str, listener_fn: Callable[[], Awaitable[None]] ) -> None: - """Wrapper that restarts *listener_fn* on failure with back-off.""" + """Restart *listener_fn* on failure or early return, with back-off.""" + delay = _LISTENER_RESTART_DELAY while self._running: + started = self._asyncio_loop.time() try: await listener_fn() except asyncio.CancelledError: @@ -441,19 +495,69 @@ async def _resilient_listener( if not self._running: return logger.warning( - "Listener '%s' failed: %s — restarting in %.1f s", - name, - exc, - _LISTENER_RESTART_DELAY, + "Listener '%s' failed: %s - restarting in %.1f s", name, exc, delay ) - # Rebuild signal proxies in case the bus was reset - self._signal_nm = None - self._signal_wifi = None - self._signal_wired = None - self._signal_settings = None - await asyncio.sleep(_LISTENER_RESTART_DELAY) - if self._running: - self._ensure_signal_proxies() + self._reset_signal_proxies() + + # Only guaranteed suspension point; also covers the early-return path. + if self._asyncio_loop.time() - started >= _LISTENER_RESTART_DELAY: + delay = _LISTENER_RESTART_DELAY + await asyncio.sleep(delay) + if not self._running: + return + await self._recover_signal_sources() + delay = min(delay * 2, _LISTENER_RESTART_MAX_DELAY) + + async def _primary_paths_alive(self) -> bool: + """False when a cached device path is unset, gone, or now points at another device. + + Probes a type-specific property: NM reuses object paths across restarts, + so the generic Device interface survives even when the path has been + reassigned to a different device. + """ + if not self._primary_wifi_path and not self._primary_wired_path: + return False + if self._primary_wifi_path: + try: + await self._wifi(self._primary_wifi_path).mode + except Exception as exc: + self._log_stale("wifi", self._primary_wifi_path, exc) + return False + if self._primary_wired_path: + try: + await self._wired(self._primary_wired_path).speed + except Exception as exc: + self._log_stale("wired", self._primary_wired_path, exc) + return False + return True + + def _log_stale(self, kind: str, path: str, exc: Exception) -> None: + """Warn once per rediscovery generation; the racing listeners only get debug.""" + if self._stale_logged_gen != self._rediscover_gen: + self._stale_logged_gen = self._rediscover_gen + logger.warning("paths_alive: %s %s stale: %s", kind, path, exc) + else: + logger.debug("paths_alive: %s %s stale (dup): %s", kind, path, exc) + + async def _recover_signal_sources(self) -> None: + """Re-detect interfaces when a path is missing or went stale across an NM restart. + + Every listener task races here after an NM restart; the generation + counter collapses that into a single re-detect. + """ + if not await self._primary_paths_alive(): + gen = self._rediscover_gen + async with self._rediscover_lock: + if gen == self._rediscover_gen: + logger.warning("recover: re-detecting interfaces (gen %d)", gen) + self._reset_signal_proxies() + self._primary_wifi_path = "" + self._primary_wired_path = "" + await self._detect_interfaces() + self._rediscover_gen += 1 + else: + logger.debug("recover: gen %d already handled, skipping", gen) + self._ensure_signal_proxies() async def _listen_nm_state_changed(self) -> None: """React to NetworkManager global state transitions.""" @@ -738,6 +842,42 @@ async def _wait_for_wifi_radio(self, desired: bool, timeout: float = 3.0) -> boo await asyncio.sleep(0.25) return False + async def _wifi_hardware_enabled(self) -> bool: + """False only when an rfkill switch blocks the radio, making soft toggles no-ops.""" + try: + return bool(await self._nm().wireless_hardware_enabled) + except Exception as exc: + logger.debug("Reading wireless_hardware_enabled failed: %s", exc) + return True + + async def _ensure_networking_enabled(self, timeout: float = 8.0) -> bool: + """Flip NM's master networking switch back on if `nmcli networking off` set it.""" + try: + if await self._nm().networking_enabled: + return True + except Exception as exc: + logger.debug("Reading networking_enabled failed: %s", exc) + return True + + logger.warning("NetworkManager networking is off - re-enabling") + try: + await self._nm().enable(True) + except Exception as exc: + logger.error("Enable(true) failed: %s", exc) + return False + + loop = asyncio.get_running_loop() + deadline = loop.time() + timeout + while loop.time() < deadline: + await asyncio.sleep(0.25) + try: + if await self._nm().networking_enabled: + return True + except Exception: # nosec B110 - NM is mid-restart, keep polling + pass + logger.error("networking_enabled stayed false after %.1f s", timeout) + return False + async def _wait_for_wifi_device_ready(self, timeout: float = 8.0) -> bool: """Poll wlan0 device state until it reaches DISCONNECTED (30) or above.""" if not self._primary_wifi_path: @@ -971,14 +1111,7 @@ async def _get_current_ip(self) -> str: async def _get_ip_by_interface(self, interface: str = "wlan0") -> str: """Return the IPv4 address assigned to *interface* via NM's IP4Config D-Bus object.""" try: - device_path = self._iface_to_device_path.get(interface) - if not device_path: - devices = await self._nm().get_devices() - for dp in devices: - if await self._generic(dp).interface == interface: - device_path = dp - self._iface_to_device_path[interface] = dp - break + device_path = await self._device_path_for_iface(interface) if not device_path: return "" ip4_path = await self._generic(device_path).ip4_config @@ -992,59 +1125,118 @@ async def _get_ip_by_interface(self, interface: str = "wlan0") -> str: logger.error("Failed to get IP for %s: %s", interface, exc) return "" + async def _device_path_for_iface(self, interface: str) -> str: + """Return the NM device path for *interface*, refreshing a stale cache entry.""" + device_path = self._iface_to_device_path.get(interface) + if device_path and not await self._cached_path_is_valid(interface, device_path): + device_path = "" + if device_path: + return device_path + + for dp in await self._nm().get_devices(): + if await self._generic(dp).interface == interface: + self._iface_to_device_path[interface] = dp + return dp + return "" + + async def _cached_path_is_valid(self, interface: str, device_path: str) -> bool: + """Check a cached device path still maps to *interface*, dropping it if not.""" + try: + if await self._generic(device_path).interface == interface: + return True + logger.warning( + "ip_by_iface: cached %s -> %s is stale, re-resolving", + interface, + device_path, + ) + except Exception as exc: + logger.debug("ip_by_iface: cached %s unreadable: %s", device_path, exc) + self._iface_to_device_path.pop(interface, None) + return False + async def _async_scan_networks(self) -> None: - """Request an NM rescan, parse visible APs, and emit networks_scanned.""" + """Request an NM rescan, parse visible APs, and emit networks_scanned. + + Retries once after re-detecting interfaces so a stale Wi-Fi path cannot + leave the network page permanently empty. + """ try: - if not self._primary_wifi_path: - self.networks_scanned.emit([]) - return - if not await self._ensure_dbus_connection(): + await self._scan_networks_once() + except Exception as exc: + logger.warning("Scan failed (%s), re-detecting interfaces", exc) + try: + await self._recover_signal_sources() + await self._scan_networks_once() + except Exception as retry_exc: + logger.error("Failed to scan networks: %s", retry_exc) + self.error_occurred.emit("scan_networks", str(retry_exc)) self.networks_scanned.emit([]) - return - if not await self._nm().wireless_enabled: - self.networks_scanned.emit([]) - return + async def _scan_networks_once(self) -> None: + """Single scan attempt; raises so the caller can recover and retry.""" + if not self._primary_wifi_path: + logger.info("scan: no wifi interface") + self.networks_scanned.emit([]) + return + if not await self._ensure_dbus_connection(): + logger.warning("scan: no D-Bus connection") + self.networks_scanned.emit([]) + return - try: - await self._wifi().request_scan({}) - except Exception as exc: - logger.debug( - "Scan request ignored (already scanning or radio off): %s", exc - ) + if not await self._nm().wireless_enabled: + logger.info("scan: radio disabled") + self.networks_scanned.emit([]) + return - if await self._wifi().last_scan == -1: - self.networks_scanned.emit([]) - return + await self._request_scan_if_allowed() - ap_paths = await self._wifi().get_all_access_points() - current_ssid = await self._get_current_ssid() - saved_ssids = set(await self._get_saved_ssid_names_cached()) + if await self._wifi().last_scan == -1: + logger.info("scan: device has never scanned (last_scan=-1)") + self.networks_scanned.emit([]) + return - networks: list[NetworkInfo] = [] - seen_ssids: set[str] = set() + ap_paths = await self._wifi().get_all_access_points() + current_ssid = await self._get_current_ssid() + saved_ssids = set(await self._get_saved_ssid_names_cached()) - for ap_path in ap_paths: - try: - info = await self._parse_ap(ap_path, current_ssid, saved_ssids) - if ( - info - and info.ssid not in seen_ssids - and not is_hidden_ssid(info.ssid) - and (info.signal_strength > 0 or info.is_active) - ): - networks.append(info) - seen_ssids.add(info.ssid) - except Exception as exc: - logger.debug("Failed to parse AP %s: %s", ap_path, exc) + networks: list[NetworkInfo] = [] + seen_ssids: set[str] = set() - networks.sort(key=lambda n: (-n.network_status, -n.signal_strength)) - self.networks_scanned.emit(networks) + parsed = await asyncio.gather( + *(self._parse_ap(p, current_ssid, saved_ssids) for p in ap_paths), + return_exceptions=True, + ) + for ap_path, info in zip(ap_paths, parsed): + if isinstance(info, BaseException): + logger.debug("Failed to parse AP %s: %s", ap_path, info) + continue + if ( + info + and info.ssid not in seen_ssids + and not is_hidden_ssid(info.ssid) + and (info.signal_strength > 0 or info.is_active) + ): + networks.append(info) + seen_ssids.add(info.ssid) + + networks.sort(key=lambda n: (-n.network_status, -n.signal_strength)) + logger.info("scan: %d visible of %d AP(s)", len(networks), len(ap_paths)) + self.networks_scanned.emit(networks) + async def _request_scan_if_allowed(self) -> None: + """Ask NM to rescan, but only from a device state where it accepts the call.""" + try: + state = await self._generic(self._primary_wifi_path).state except Exception as exc: - logger.error("Failed to scan networks: %s", exc) - self.error_occurred.emit("scan_networks", str(exc)) - self.networks_scanned.emit([]) + logger.debug("Scan skipped, device state unreadable: %s", exc) + return + if not 30 <= state <= 100: + logger.debug("Scan skipped, device state %s not ready", state) + return + try: + await self._wifi().request_scan({}) + except Exception as exc: + logger.debug("Scan request ignored: %s", exc) async def _get_all_ap_properties(self, ap_path: str) -> dict[str, object]: """Fetch all D-Bus properties for an AccessPoint in one round-trip.""" @@ -1643,34 +1835,20 @@ async def _async_set_wifi_enabled(self, enabled: bool) -> None: """Enable or disable the Wi-Fi radio. Ethernet is left untouched.""" try: if not self._system_bus: + logger.warning("set_wifi_enabled(%s): no system bus", enabled) return + await self._log_radio_state(enabled) if not enabled: self._is_hotspot_active = False + if enabled and not await self._wifi_enable_preflight(): + return - current = await self._nm().wireless_enabled - if current != enabled: - if not enabled: - if self._primary_wifi_path: - try: - await self._wifi().disconnect() - except Exception as exc: - logger.debug( - "Disconnect before Wi-Fi toggle ignored: %s", exc - ) - await asyncio.sleep(0.5) - - await self._nm().wireless_enabled.set_async(enabled) - - if not await self._wait_for_wifi_radio(enabled, timeout=8.0): - logger.warning( - "Wi-Fi radio did not reach %s within 8 s", - "enabled" if enabled else "disabled", - ) - + ok = await self._apply_wifi_radio(enabled) + word = "enabled" if enabled else "disabled" self.connection_result.emit( ConnectionResult( - True, - f"Wi-Fi {'enabled' if enabled else 'disabled'}", + ok, + f"Wi-Fi {word}" if ok else f"Wi-Fi could not be {word}", ) ) self.state_changed.emit(await self._build_current_state()) @@ -1678,13 +1856,67 @@ async def _async_set_wifi_enabled(self, enabled: bool) -> None: logger.error("Failed to toggle Wi-Fi: %s", exc) self.error_occurred.emit("set_wifi_enabled", str(exc)) + async def _log_radio_state(self, enabled: bool) -> None: + """Log the radio/networking flags; diagnostics only, never aborts the toggle.""" + try: + logger.info( + "set_wifi_enabled(%s): radio=%s networking=%s hw=%s", + enabled, + await self._nm().wireless_enabled, + await self._nm().networking_enabled, + await self._nm().wireless_hardware_enabled, + ) + except Exception as log_exc: + logger.debug("set_wifi_enabled(%s): state log failed: %s", enabled, log_exc) + + async def _wifi_enable_preflight(self) -> bool: + """Check the rfkill switch and NM networking, emitting the reason on failure.""" + if not await self._wifi_hardware_enabled(): + self.connection_result.emit( + ConnectionResult(False, "Wi-Fi is blocked by a hardware switch") + ) + return False + if not await self._ensure_networking_enabled(): + self.connection_result.emit( + ConnectionResult(False, "NetworkManager networking is disabled") + ) + return False + return True + + async def _apply_wifi_radio(self, enabled: bool) -> bool: + """Set the radio flag and wait for it to settle; True if already there or reached.""" + if await self._nm().wireless_enabled == enabled: + return True + + if not enabled and self._primary_wifi_path: + try: + await self._wifi().disconnect() + except Exception as exc: + logger.debug("Disconnect before Wi-Fi toggle ignored: %s", exc) + await asyncio.sleep(0.5) + + await self._nm().wireless_enabled.set_async(enabled) + ok = await self._wait_for_wifi_radio(enabled, timeout=8.0) + if not ok: + logger.warning( + "Wi-Fi radio did not reach %s within 8 s", + "enabled" if enabled else "disabled", + ) + return ok + async def _async_disconnect_ethernet(self) -> None: """Deactivate all VLANs, disconnect ethernet, and wait up to 4 s for teardown.""" if not self._primary_wired_path: return try: await self._deactivate_all_vlans() - await self._wired().disconnect() + try: + await self._wired().disconnect() + except Exception as exc: + # Already inactive is the goal state, not a failure. + if "not active" not in str(exc).lower(): + raise + logger.debug("Ethernet already inactive: %s", exc) loop = asyncio.get_running_loop() deadline = loop.time() + 4.0 while loop.time() < deadline: @@ -1694,6 +1926,9 @@ async def _async_disconnect_ethernet(self) -> None: logger.info("Ethernet disconnected") except Exception as exc: logger.error("Failed to disconnect ethernet: %s", exc) + finally: + # Only user toggles reach here, so record intent even if teardown failed. + await self._set_wired_profiles_autoconnect(False) async def _async_connect_ethernet(self) -> None: """Activate the wired device and restore saved VLANs. diff --git a/tests/network/test_worker_unit.py b/tests/network/test_worker_unit.py index dd09b6ec..b102bcaf 100644 --- a/tests/network/test_worker_unit.py +++ b/tests/network/test_worker_unit.py @@ -50,7 +50,9 @@ def _make_worker(qapp, *, running=True, with_wifi=True, with_wired=False): # Core state — mirrors real __init__ w._running = running + w._stopping = False w._system_bus = MagicMock(name="mock_system_bus") + w._no_iface_reported = False w._primary_wifi_path = ( "/org/freedesktop/NetworkManager/Devices/2" if with_wifi else "" ) @@ -75,6 +77,9 @@ def _make_worker(qapp, *, running=True, with_wifi=True, with_wired=False): w._state_debounce_handle = None w._scan_debounce_handle = None w._listener_tasks = [] + w._rediscover_lock = asyncio.Lock() + w._rediscover_gen = 0 + w._stale_logged_gen = -1 # Stubs for thread-related attrs (never used in async tests) w._asyncio_loop = MagicMock() @@ -90,7 +95,9 @@ def _bare_worker(qapp): ): w = NetworkManagerWorker() w._running = False + w._stopping = False w._system_bus = None + w._no_iface_reported = False w._primary_wifi_path = "" w._primary_wifi_iface = "" w._primary_wired_path = "" @@ -110,6 +117,9 @@ def _bare_worker(qapp): w._state_debounce_handle = None w._scan_debounce_handle = None w._listener_tasks = [] + w._rediscover_lock = asyncio.Lock() + w._rediscover_gen = 0 + w._stale_logged_gen = -1 w._asyncio_loop = MagicMock() w._asyncio_thread = MagicMock() return w @@ -121,7 +131,9 @@ def _make(qapp, *, running=True, wifi=True, wired=True): ): w = NetworkManagerWorker() w._running = running + w._stopping = False w._system_bus = MagicMock(name="mock_bus") + w._no_iface_reported = False w._primary_wifi_path = "/org/freedesktop/NetworkManager/Devices/2" if wifi else "" w._primary_wifi_iface = "wlan0" if wifi else "" w._primary_wired_path = "/org/freedesktop/NetworkManager/Devices/1" if wired else "" @@ -141,6 +153,9 @@ def _make(qapp, *, running=True, wifi=True, wired=True): w._state_debounce_handle = None w._scan_debounce_handle = None w._listener_tasks = [] + w._rediscover_lock = asyncio.Lock() + w._rediscover_gen = 0 + w._stale_logged_gen = -1 w._asyncio_loop = MagicMock() w._asyncio_thread = MagicMock() return w @@ -702,7 +717,7 @@ class TestGetIpByInterface: async def test_cached_path_used(self, qapp): w = _make_worker(qapp) w._iface_to_device_path = {"wlan0": "/dev/wifi0"} - generic_proxy = AsyncProxyMock(ip4_config="/ip4/1") + generic_proxy = AsyncProxyMock(interface="wlan0", ip4_config="/ip4/1") w._generic = lambda path: generic_proxy ipv4_proxy = AsyncProxyMock(address_data=[{"address": ("s", "192.168.1.50")}]) w._ipv4 = lambda path: ipv4_proxy @@ -1873,13 +1888,16 @@ def test_sets_not_running(self, qapp): _run(w._async_shutdown()) assert w._running is False - def test_clears_listener_tasks(self, qapp): + @pytest.mark.asyncio + async def test_clears_listener_tasks(self, qapp): + async def dummy(): + await asyncio.sleep(10) + w = _make(qapp) - mock_task = MagicMock() - mock_task.done.return_value = False - w._listener_tasks = [mock_task] - _run(w._async_shutdown()) - mock_task.cancel.assert_called_once() + task = asyncio.create_task(dummy()) + w._listener_tasks = [task] + await w._async_shutdown() + assert task.cancelled() assert w._listener_tasks == [] def test_cancels_debounce_handles(self, qapp): From 46f40452b76f29b4bdde8bcd176f943d7d81b5f5 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Mon, 28 Sep 2026 10:27:17 +0100 Subject: [PATCH 2/2] fix(network): enforce one active link and persist ethernet-off intent (#310) # Description - [x] Bug fix Stacked on #3, review after it merges. Highest review value in the stack, this is where the field bugs were. Enforces exactly one of Wi-Fi/ethernet/hotspot active at a time, on user toggles only. Background/automatic state handlers never drop a link behind the user's back. - `_set_wired_profiles_autoconnect`, `_ensure_wired_autoconnect` - Exclusivity logic in `manager.py`/`worker.py` - `networkWindow.py` toggle handlers + loading guard armed before link changes # Tests All 5 gates pass standing alone. --- BlocksScreen/lib/network/worker.py | 214 ++++++++------ BlocksScreen/lib/panels/networkWindow.py | 99 +++---- tests/network/test_network_ui.py | 19 +- tests/network/test_worker_unit.py | 339 ++++++++++++++++++++--- 4 files changed, 464 insertions(+), 207 deletions(-) diff --git a/BlocksScreen/lib/network/worker.py b/BlocksScreen/lib/network/worker.py index e8085d2b..8ea24744 100644 --- a/BlocksScreen/lib/network/worker.py +++ b/BlocksScreen/lib/network/worker.py @@ -8,7 +8,7 @@ import socket as _socket import struct import threading -from typing import Callable +from collections.abc import Callable from uuid import uuid4 import sdbus @@ -284,11 +284,13 @@ def hotspot_password(self) -> str: async def _async_initialize(self) -> None: """Bootstrap the worker on the asyncio thread. - Detects network interfaces, enforces the boot-time ethernet/Wi-Fi - mutual exclusion, activates any saved VLANs if ethernet is present, - triggers an initial Wi-Fi scan, and starts all D-Bus signal listeners. - Emits ``initialized`` when done (even on failure, so the manager can - unblock its caller). + Detects network interfaces, activates any saved VLANs if ethernet is + present, triggers an initial Wi-Fi scan, and starts all D-Bus signal + listeners. Emits ``initialized`` when done (even on failure, so the + manager can unblock its caller). + + Wired autoconnect is deliberately not re-armed here: NM's latch is what + persists the user's "ethernet off" choice across reboots. """ try: if not self._system_bus: @@ -297,7 +299,6 @@ async def _async_initialize(self) -> None: self._running = True await self._detect_interfaces() - await self._enforce_boot_mutual_exclusion() if await self._is_ethernet_connected(): await self._activate_saved_vlans() @@ -367,31 +368,39 @@ async def _detect_interfaces(self) -> None: # Ethernet-only or Wi-Fi driver still loading — log but don't alarm. logger.warning("No Wi-Fi interface detected; ethernet-only mode") - async def _enforce_boot_mutual_exclusion(self) -> None: - """Disable Wi-Fi at boot if ethernet is already connected. + async def _set_wired_profiles_autoconnect(self, enabled: bool) -> None: + """Persist autoconnect on every wired profile; Device.Autoconnect dies on NM restart.""" + try: + paths = await self._nm_settings().list_connections() + for path, settings in await self._gather_settings(list(paths)): + conn = settings.get("connection", {}) + if conn.get("type", (None, ""))[1] != "802-3-ethernet": + continue + if bool(conn.get("autoconnect", ("b", True))[1]) == enabled: + continue + props = {k: dict(v) for k, v in settings.items()} + props["connection"]["autoconnect"] = ("b", enabled) + props["connection"].pop("timestamp", None) + await self._conn_settings(path).update(props) + logger.info("Wired profile %s autoconnect -> %s", path, enabled) + except Exception as exc: + logger.warning("Wired profile autoconnect (%s) failed: %s", enabled, exc) + + async def _ensure_wired_autoconnect(self) -> None: + """Re-arm wired autoconnect on both the device and the saved profiles. - Prevents the device from simultaneously using both interfaces at - startup. If ethernet is active and the Wi-Fi radio is on, the Wi-Fi - device is disconnected and the radio is disabled, then we wait up to - 8 s for the radio to confirm it is off. Failures are logged but not - propagated — a non-fatal best-effort action at boot. + Called only when the user asks for ethernet, so autoconnect staying off + keeps meaning "user turned it off". Best-effort: never propagates. """ + if not self._primary_wired_path: + return + await self._set_wired_profiles_autoconnect(True) try: - if not await self._is_ethernet_connected(): - return - if not await self._nm().wireless_enabled: - return - logger.info("Boot: ethernet active + Wi-Fi enabled — disabling Wi-Fi") - if self._primary_wifi_path: - try: - await self._wifi().disconnect() - except Exception as exc: - logger.debug("Pre-radio-disable disconnect ignored: %s", exc) - await self._nm().wireless_enabled.set_async(False) - await self._wait_for_wifi_radio(False, timeout=8.0) - self._is_hotspot_active = False + wired = self._generic(self._primary_wired_path) + if not await wired.autoconnect: + await wired.autoconnect.set_async(True) except Exception as exc: - logger.warning("Boot mutual exclusion failed (non-fatal): %s", exc) + logger.debug("Device autoconnect re-arm ignored: %s", exc) async def _start_signal_listeners(self) -> None: """Create persistent proxies and spawn all D-Bus signal listeners. @@ -749,31 +758,12 @@ async def _wait_for_wifi_device_ready(self, timeout: float = 8.0) -> bool: return False async def _async_get_current_state(self) -> None: - """Rebuild and emit the full NetworkState, enforcing runtime mutual exclusion.""" + """Rebuild and emit the full NetworkState. Read-only: never mutates NM.""" try: if not await self._ensure_dbus_connection(): self.state_changed.emit(NetworkState()) return - state = await self._build_current_state() - if ( - state.ethernet_connected - and state.wifi_enabled - and not state.hotspot_enabled - and not self._is_hotspot_active - ): - logger.info( - "Runtime mutual exclusion: ethernet active + " - "Wi-Fi — disabling Wi-Fi" - ) - if self._primary_wifi_path: - try: - await self._wifi().disconnect() - except Exception as exc: - logger.debug("Disconnect before Wi-Fi disable ignored: %s", exc) - await self._nm().wireless_enabled.set_async(False) - await asyncio.sleep(0.5) - state = await self._build_current_state() - self.state_changed.emit(state) + self.state_changed.emit(await self._build_current_state()) except Exception as exc: logger.error("Failed to get current state: %s", exc) self.error_occurred.emit("get_current_state", str(exc)) @@ -1289,17 +1279,14 @@ async def _add_network_impl( ) -> ConnectionResult: """Scan for the SSID, build a connection profile, add it to NM, and activate it. - Deletes any pre-existing profile for the same SSID before adding. + Any pre-existing profile for the same SSID is backed up before being + replaced, and restored if the new credentials fail to activate. Returns a failed ConnectionResult if the SSID is not visible, the security type is unsupported, or the 20-second activation wait times out. """ if not self._primary_wifi_path or not self._system_bus: return ConnectionResult(False, "No Wi-Fi interface", "no_interface") - if await self._is_known(ssid): - await self._delete_network_impl(ssid) - self._invalidate_saved_cache() - try: await self._wifi().request_scan({}) except Exception as exc: @@ -1320,10 +1307,14 @@ async def _add_network_impl( "unsupported_security", ) + # Drop the old profile only once the new one is buildable, so early exits keep it. + backup = await self._backup_and_drop_existing(ssid) try: nm_settings = self._nm_settings() conn_path = await nm_settings.add_connection(conn_props) except Exception as exc: + if backup: + await self._restore_profile(ssid, backup) err_str = str(exc).lower() if "psk" in err_str and ("invalid" in err_str or "property" in err_str): return ConnectionResult( @@ -1338,15 +1329,7 @@ async def _add_network_impl( try: await self._nm().activate_connection(conn_path) if not await self._wait_for_connection(ssid, timeout=_WIFI_CONNECT_TIMEOUT): - await self._delete_network_impl(ssid) - self._invalidate_saved_cache() - return ConnectionResult( - False, - f"Authentication failed for '{ssid}'.\n" - "The saved profile has been removed.\n" - "Please check the password and try again.", - "auth_failed", - ) + return await self._rollback_failed_add(ssid, backup) return ConnectionResult(True, f"Network '{ssid}' added and connecting") except Exception as act_err: logger.warning("Activate after add failed: %s", act_err) @@ -1361,6 +1344,66 @@ async def _reload_connections(self) -> None: except Exception as reload_err: logger.debug("reload_connections non-fatal: %s", reload_err) + async def _backup_profile(self, ssid: str) -> dict | None: + """Snapshot a saved profile's settings plus secrets so it can be re-added.""" + conn_path = await self._get_connection_path(ssid) + if not conn_path: + return None + try: + cs = self._conn_settings(conn_path) + settings = dict(await cs.get_settings()) + await self._merge_wifi_secrets(cs, settings) + settings.get("connection", {}).pop("timestamp", None) + logger.debug("backup_profile: '%s' sections=%s", ssid, sorted(settings)) + return settings + except Exception as exc: + logger.warning("backup_profile: could not snapshot '%s': %s", ssid, exc) + return None + + async def _restore_profile(self, ssid: str, settings: dict) -> bool: + """Re-add a backed-up profile after a failed replacement; True when restored.""" + try: + await self._nm_settings().add_connection(settings) + self._invalidate_saved_cache() + logger.info("restore_profile: '%s' restored after failed add", ssid) + return True + except Exception as exc: + logger.error("restore_profile: could not restore '%s': %s", ssid, exc) + return False + + async def _rollback_failed_add( + self, ssid: str, backup: dict | None + ) -> ConnectionResult: + """Delete the profile that never activated and restore *backup* if there is one.""" + logger.warning("add_network: '%s' never activated, rolling back", ssid) + await self._delete_network_impl(ssid) + self._invalidate_saved_cache() + if backup and await self._restore_profile(ssid, backup): + return ConnectionResult( + False, + f"Could not connect to '{ssid}'.\n" + "The previously saved password was kept.\n" + "Please check the password and try again.", + "auth_failed", + ) + return ConnectionResult( + False, + f"Authentication failed for '{ssid}'.\n" + "The saved profile has been removed.\n" + "Please check the password and try again.", + "auth_failed", + ) + + async def _backup_and_drop_existing(self, ssid: str) -> dict | None: + """Back up and delete a saved profile for *ssid* so it can be re-added cleanly.""" + if not await self._is_known(ssid): + return None + backup = await self._backup_profile(ssid) + logger.info("add_network: replacing saved '%s' (backup=%s)", ssid, bool(backup)) + await self._delete_network_impl(ssid) + self._invalidate_saved_cache() + return backup + async def _async_connect_network(self, ssid: str) -> None: """Activate an existing saved Wi-Fi profile and emit connection_result.""" try: @@ -1585,16 +1628,13 @@ async def _update_network_impl( return ConnectionResult(False, str(exc), "update_failed") async def _async_set_wifi_enabled(self, enabled: bool) -> None: - """Enable or disable the Wi-Fi radio, handling ethernet mutual exclusion.""" + """Enable or disable the Wi-Fi radio. Ethernet is left untouched.""" try: if not self._system_bus: return if not enabled: self._is_hotspot_active = False - if enabled and await self._is_ethernet_connected(): - await self._async_disconnect_ethernet() - current = await self._nm().wireless_enabled if current != enabled: if not enabled: @@ -1632,7 +1672,13 @@ async def _async_disconnect_ethernet(self) -> None: return try: await self._deactivate_all_vlans() - await self._wired().disconnect() + try: + await self._wired().disconnect() + except Exception as exc: + # Already inactive is the goal state, not a failure. + if "not active" not in str(exc).lower(): + raise + logger.debug("Ethernet already inactive: %s", exc) loop = asyncio.get_running_loop() deadline = loop.time() + 4.0 while loop.time() < deadline: @@ -1642,9 +1688,15 @@ async def _async_disconnect_ethernet(self) -> None: logger.info("Ethernet disconnected") except Exception as exc: logger.error("Failed to disconnect ethernet: %s", exc) + finally: + # Only user toggles reach here, so record intent even if teardown failed. + await self._set_wired_profiles_autoconnect(False) async def _async_connect_ethernet(self) -> None: - """Disable Wi-Fi/hotspot, activate the wired device, and restore saved VLANs.""" + """Activate the wired device and restore saved VLANs. + + Mechanism only: the one-link-at-a-time policy lives in the UI toggles. + """ if not self._primary_wired_path: self.error_occurred.emit("connect_ethernet", "No wired device found") return @@ -1652,16 +1704,7 @@ async def _async_connect_ethernet(self) -> None: if self._is_hotspot_active: await self._async_toggle_hotspot(False) - if self._primary_wifi_path: - try: - await self._wifi().disconnect() - except Exception as exc: - logger.debug("Pre-VLAN disconnect ignored: %s", exc) - await asyncio.sleep(0.5) - - if await self._nm().wireless_enabled: - await self._nm().wireless_enabled.set_async(False) - await self._wait_for_wifi_radio(False, timeout=8.0) + await self._ensure_wired_autoconnect() await self._nm().activate_connection("/", self._primary_wired_path, "/") await asyncio.sleep(1.5) @@ -1695,18 +1738,9 @@ async def _async_create_vlan( if self._is_hotspot_active: await self._async_toggle_hotspot(False) - if self._primary_wifi_path: - try: - await self._wifi().disconnect() - except Exception as exc: - logger.debug("Pre-VLAN disconnect ignored: %s", exc) - await asyncio.sleep(0.5) - - if await self._nm().wireless_enabled: - await self._nm().wireless_enabled.set_async(False) - await self._wait_for_wifi_radio(False, timeout=8.0) - + # A VLAN rides on eth0; Wi-Fi is orthogonal and stays up. if not await self._is_ethernet_connected(): + await self._ensure_wired_autoconnect() await self._nm().activate_connection("/", self._primary_wired_path, "/") await asyncio.sleep(1.5) diff --git a/BlocksScreen/lib/panels/networkWindow.py b/BlocksScreen/lib/panels/networkWindow.py index 7e5b44ff..ef77b14b 100644 --- a/BlocksScreen/lib/panels/networkWindow.py +++ b/BlocksScreen/lib/panels/networkWindow.py @@ -37,6 +37,7 @@ from lib.utils.icon_button import IconButton from lib.utils.list_model import EntryDelegate, EntryListModel, ListItem from PyQt6 import QtCore, QtGui, QtWidgets +from PyQt6.QtCore import QTimer logger = logging.getLogger(__name__) @@ -182,7 +183,6 @@ def _init_instance_variables(self) -> None: self._current_network_is_hidden = False self._is_connecting = False self._target_ssid: str | None = None - self._was_ethernet_connected: bool = False self._initial_priority: ConnectionPriority = ConnectionPriority.MEDIUM self._pending_operation: PendingOperation = PendingOperation.NONE self._pending_expected_ip: str = ( @@ -264,7 +264,7 @@ def _on_reconnect_complete(self) -> None: def _init_timers(self) -> None: """Initialize timers.""" - self._load_timer = QtCore.QTimer(self) + self._load_timer = QTimer(self) self._load_timer.setSingleShot(True) self._load_timer.timeout.connect(self._handle_load_timeout) @@ -302,30 +302,9 @@ def _on_network_state_changed(self, state: NetworkState) -> None: self._handle_first_run(state) self._emit_status_icon(state) self._is_first_run = False - self._was_ethernet_connected = state.ethernet_connected return - # Cable just plugged in while Wi-Fi is active -> disable Wi-Fi - if ( - state.ethernet_connected - and not self._was_ethernet_connected - and state.wifi_enabled - and not self._is_connecting - ): - logger.info("Ethernet connected — turning off Wi-Fi") - self._was_ethernet_connected = True - wifi_btn = self.wifi_button.toggle_button - hotspot_btn = self.hotspot_button.toggle_button - with QtCore.QSignalBlocker(wifi_btn): - wifi_btn.state = wifi_btn.State.OFF - with QtCore.QSignalBlocker(hotspot_btn): - hotspot_btn.state = hotspot_btn.State.OFF - self._nm.set_wifi_enabled(False) - self._sync_ethernet_panel(state) - self._emit_status_icon(state) - return - - self._was_ethernet_connected = state.ethernet_connected + # Exclusivity applies to user toggles only; a cable never kills the radio. # Ethernet panel visibility is pure hardware state (carrier + # connection) and must update even while a loading operation is @@ -419,9 +398,7 @@ def _on_network_state_changed(self, state: NetworkState) -> None: return # Normal (not connecting) display updates. - if state.ethernet_connected: - self._display_connected_state(state) - elif ( + if state.ethernet_connected or ( state.current_ssid and state.current_ip and state.connectivity @@ -573,7 +550,7 @@ def _on_operation_complete(self, result: ConnectionResult) -> None: result.message, ) ssid = self._target_ssid - QtCore.QTimer.singleShot( + QTimer.singleShot( 2000, lambda _ssid=ssid: self._nm.connect_network(_ssid) ) return # Keep loading visible; state machine handles completion @@ -690,8 +667,7 @@ def _handle_first_run(self, state: NetworkState) -> None: hotspot_on = False if state.ethernet_connected: - if state.wifi_enabled: - self._nm.set_wifi_enabled(False) + # Display only: never force the radio off here, it is the recovery path. self._display_connected_state(state) elif state.connectivity == ConnectivityState.FULL and state.current_ssid: wifi_on = True @@ -734,6 +710,7 @@ def _sync_toggle_states(self, state: NetworkState) -> None: wifi_on = False hotspot_on = False + # One link at a time: the cable wins the display, then hotspot, then Wi-Fi. if state.ethernet_connected: pass elif state.hotspot_enabled: @@ -1007,6 +984,15 @@ def _configure_info_box_centered(self) -> None: self.mn_info_box.setWordWrap(True) self.mn_info_box.setAlignment(QtCore.Qt.AlignmentFlag.AlignCenter) + def _claim_link(self, winner) -> None: + """Force the other two link toggles OFF; only one link may be on at a time.""" + for btn in (self.wifi_button, self.hotspot_button, self.ethernet_button): + if btn is winner: + continue + toggle = btn.toggle_button + with QtCore.QSignalBlocker(toggle): + toggle.state = toggle.State.OFF + @QtCore.pyqtSlot(object, name="stateChange") def _on_toggle_state(self, new_state) -> None: """Route a toggle-button state change to the correct handler (Wi-Fi or hotspot).""" @@ -1027,7 +1013,7 @@ def _on_toggle_state(self, new_state) -> None: # when the worker emits the disconnected state. def _handle_wifi_toggle(self, is_on: bool) -> None: - """Enable or disable Wi-Fi, enforcing the ethernet/hotspot mutual-exclusion rule.""" + """Enable or disable Wi-Fi; turning it on drops the hotspot and the cable.""" if not is_on: self._target_ssid = None self._pending_operation = PendingOperation.WIFI_OFF @@ -1035,45 +1021,36 @@ def _handle_wifi_toggle(self, is_on: bool) -> None: self._nm.set_wifi_enabled(False) return - hotspot_btn = self.hotspot_button.toggle_button - eth_btn = self.ethernet_button.toggle_button - with QtCore.QSignalBlocker(hotspot_btn): - hotspot_btn.state = hotspot_btn.State.OFF - with QtCore.QSignalBlocker(eth_btn): - eth_btn.state = eth_btn.State.OFF + # Guard before touching links: a state update mid-teardown bounces the toggle. + self._target_ssid = None + self._pending_operation = PendingOperation.WIFI_ON + self._set_loading_state(True) + self._claim_link(self.wifi_button) + self._nm.disconnect_ethernet() self._nm.set_wifi_enabled(True) - # NOTE: set_wifi_enabled is dispatched to the worker — cached state - # is STALE here (may still show ethernet). Always proceed to the - # saved-network connection path. - saved = self._nm.saved_networks wifi_networks = [n for n in saved if "ap" not in n.mode] if not wifi_networks: + self._clear_loading() self._show_warning_popup("No saved Wi-Fi networks. Please add one first.") self._display_wifi_on_no_connection() return - # Sort by priority descending (highest priority first), - # then by timestamp as tiebreaker — this gives "reconnect to - # highest-priority saved network" behaviour. + # Reconnect to the highest-priority saved network, newest breaking ties. wifi_networks.sort(key=lambda n: (n.priority, n.timestamp), reverse=True) self._target_ssid = wifi_networks[0].ssid - self._pending_operation = PendingOperation.WIFI_ON - self._set_loading_state(True) # Non-blocking: disable hotspot then connect self._nm.toggle_hotspot(False) _ssid_to_connect = self._target_ssid - QtCore.QTimer.singleShot( - 500, lambda: self._nm.connect_network(_ssid_to_connect) - ) + QTimer.singleShot(500, lambda: self._nm.connect_network(_ssid_to_connect)) def _handle_hotspot_toggle(self, is_on: bool) -> None: - """Enable or disable the hotspot, enforcing the ethernet/Wi-Fi mutual-exclusion rule.""" + """Enable or disable the hotspot; turning it on drops Wi-Fi client and the cable.""" if not is_on: self._target_ssid = None self._pending_operation = PendingOperation.HOTSPOT_OFF @@ -1081,17 +1058,13 @@ def _handle_hotspot_toggle(self, is_on: bool) -> None: self._nm.toggle_hotspot(False) return - wifi_btn = self.wifi_button.toggle_button - eth_btn = self.ethernet_button.toggle_button - with QtCore.QSignalBlocker(wifi_btn): - wifi_btn.state = wifi_btn.State.OFF - with QtCore.QSignalBlocker(eth_btn): - eth_btn.state = eth_btn.State.OFF - self._target_ssid = None self._pending_operation = PendingOperation.HOTSPOT_ON self._set_loading_state(True) + self._claim_link(self.hotspot_button) + self._nm.disconnect_ethernet() + hotspot_name = self.hotspot_name_input_field.text() or "" hotspot_pass = self.hotspot_password_input_field.text() or "" hotspot_sec = "wpa-psk" @@ -1100,18 +1073,14 @@ def _handle_hotspot_toggle(self, is_on: bool) -> None: self._nm.create_hotspot(hotspot_name, hotspot_pass, hotspot_sec) def _handle_ethernet_toggle(self, is_on: bool) -> None: - """Handle ethernet toggle with mutual exclusion.""" + """Connect or disconnect the cable; connecting drops Wi-Fi and the hotspot.""" if is_on: - wifi_btn = self.wifi_button.toggle_button - hotspot_btn = self.hotspot_button.toggle_button - with QtCore.QSignalBlocker(wifi_btn): - wifi_btn.state = wifi_btn.State.OFF - with QtCore.QSignalBlocker(hotspot_btn): - hotspot_btn.state = hotspot_btn.State.OFF - self._target_ssid = None self._pending_operation = PendingOperation.ETHERNET_ON self._set_loading_state(True) + + self._claim_link(self.ethernet_button) + self._nm.set_wifi_enabled(False) self._nm.connect_ethernet() return diff --git a/tests/network/test_network_ui.py b/tests/network/test_network_ui.py index 7250aaaa..d740274a 100644 --- a/tests/network/test_network_ui.py +++ b/tests/network/test_network_ui.py @@ -178,11 +178,14 @@ def test_ethernet_connected_shows_connected_state(self, win): assert w.netlist_ssuid.isVisible() assert w.netlist_ssuid.text() == "Ethernet" - def test_ethernet_disables_wifi_if_enabled(self, win): + def test_ethernet_never_kills_radio_at_boot(self, win): + """Boot is display-only: the radio is the recovery path, never touched here.""" w, nm = win state = _eth_state(wifi_enabled=True) w._handle_first_run(state) - nm.set_wifi_enabled.assert_called_once_with(False) + nm.set_wifi_enabled.assert_not_called() + wifi_btn = w.wifi_button.toggle_button + assert wifi_btn.state == wifi_btn.State.OFF def test_ethernet_does_not_disable_wifi_if_already_off(self, win): w, nm = win @@ -490,7 +493,6 @@ def _prime(self, w): """Mark first-run as done so the normal display path runs.""" w._is_first_run = False w._is_connecting = False - w._was_ethernet_connected = False def test_ethernet_shows_connected(self, win): w, _ = win @@ -522,14 +524,13 @@ def test_first_run_flag_cleared_after_first_call(self, win): w._on_network_state_changed(_disconnected_state()) assert w._is_first_run is False - def test_ethernet_plug_disables_wifi(self, win): - """Ethernet cable plugged in during Wi-Fi session -> Wi-Fi disabled.""" + def test_ethernet_plug_keeps_wifi_enabled(self, win): + """Wi-Fi is the recovery path; a plugged cable must never kill the radio.""" w, nm = win self._prime(w) - w._was_ethernet_connected = False state = _eth_state(wifi_enabled=True) w._on_network_state_changed(state) - nm.set_wifi_enabled.assert_called_with(False) + nm.set_wifi_enabled.assert_not_called() # ───────────────────────────────────────────────────────────────────────────── @@ -698,7 +699,7 @@ def test_transient_mismatch_retries(self, win, qapp): message="not compatible with device", error_code="nm_error", ) - with patch("BlocksScreen.lib.panels.networkWindow.QtCore.QTimer") as mock_timer: + with patch("BlocksScreen.lib.panels.networkWindow.QTimer") as mock_timer: w._on_operation_complete(result) mock_timer.singleShot.assert_called_once() # Loading should still be visible — retry is pending @@ -745,7 +746,7 @@ def test_wifi_on_with_saved_networks_starts_connect(self, win): ) ] nm.saved_networks = saved - with patch("BlocksScreen.lib.panels.networkWindow.QtCore.QTimer") as mock_timer: + with patch("BlocksScreen.lib.panels.networkWindow.QTimer") as mock_timer: w._handle_wifi_toggle(True) mock_timer.singleShot.assert_called() assert w._pending_operation == PendingOperation.WIFI_ON diff --git a/tests/network/test_worker_unit.py b/tests/network/test_worker_unit.py index b8a8fa7a..7bace8e5 100644 --- a/tests/network/test_worker_unit.py +++ b/tests/network/test_worker_unit.py @@ -236,7 +236,6 @@ async def test_happy_path_sets_running(self, qapp): w = _make_worker(qapp, running=False) # Mock all async calls in initialize w._detect_interfaces = AsyncMock() - w._enforce_boot_mutual_exclusion = AsyncMock() w._is_ethernet_connected = AsyncMock(return_value=False) w._activate_saved_vlans = AsyncMock() w._start_signal_listeners = AsyncMock() @@ -752,6 +751,23 @@ async def test_exception_emits_error(self, qapp): await w._async_get_current_state() assert len(errors) == 1 + @pytest.mark.asyncio + async def test_cable_with_radio_on_never_kills_radio(self, qapp): + w = _make_worker(qapp) + w._ensure_dbus_connection = AsyncMock(return_value=True) + state = NetworkState(ethernet_connected=True, wifi_enabled=True) + w._build_current_state = AsyncMock(return_value=state) + nm_proxy = AsyncProxyMock(wireless_enabled=True) + wifi_proxy = AsyncProxyMock() + w._nm = _ProxyFactory(nm_proxy) + w._wifi = _ProxyFactory(wifi_proxy) + received = [] + w.state_changed.connect(received.append) + await w._async_get_current_state() + assert received == [state] + nm_proxy.wireless_enabled.set_async.assert_not_awaited() + wifi_proxy.disconnect.assert_not_awaited() + class TestBuildCurrentState: @pytest.mark.asyncio @@ -1730,6 +1746,132 @@ async def test_unsupported_security_returns_error(self, qapp): assert result.error_code == "unsupported_security" +_BACKUP = {"connection": {"id": ("s", "Net")}, "802-11-wireless-security": {}} + + +def _add_ready_worker(qapp, *, backup=_BACKUP): + """Worker whose add path reaches NM, with the backup helpers mocked.""" + w = _make_worker(qapp) + w._wifi = _ProxyFactory(AsyncProxyMock(request_scan=AsyncMock(), interface="wlan0")) + w._find_ap_props = AsyncMock(return_value={"ssid": b"Net"}) + w._build_connection_properties = MagicMock(return_value={"connection": {}}) + w._backup_and_drop_existing = AsyncMock(return_value=backup) + w._restore_profile = AsyncMock(return_value=True) + w._delete_network_impl = AsyncMock() + w._reload_connections = AsyncMock() + w._wait_for_connection = AsyncMock(return_value=True) + w._nm_settings = _ProxyFactory( + AsyncProxyMock(add_connection=AsyncMock(return_value="/conn/9")) + ) + w._nm = _ProxyFactory(AsyncProxyMock(activate_connection=AsyncMock())) + return w + + +class TestAddNetworkBackup: + @pytest.mark.asyncio + async def test_not_found_keeps_saved_profile(self, qapp): + w = _add_ready_worker(qapp) + w._find_ap_props = AsyncMock(return_value=None) + result = await w._add_network_impl("Net", "pass", 0) + assert result.error_code == "not_found" + w._backup_and_drop_existing.assert_not_awaited() + + @pytest.mark.asyncio + async def test_unsupported_security_keeps_saved_profile(self, qapp): + w = _add_ready_worker(qapp) + w._build_connection_properties = MagicMock(return_value={}) + result = await w._add_network_impl("Net", "pass", 0) + assert result.error_code == "unsupported_security" + w._backup_and_drop_existing.assert_not_awaited() + + @pytest.mark.asyncio + async def test_add_failure_restores_backup(self, qapp): + w = _add_ready_worker(qapp) + w._nm_settings().add_connection.side_effect = RuntimeError("boom") + result = await w._add_network_impl("Net", "pass", 0) + assert result.error_code == "add_failed" + w._restore_profile.assert_awaited_once_with("Net", _BACKUP) + + @pytest.mark.asyncio + async def test_add_failure_without_backup_restores_nothing(self, qapp): + w = _add_ready_worker(qapp, backup=None) + w._nm_settings().add_connection.side_effect = RuntimeError("boom") + result = await w._add_network_impl("Net", "pass", 0) + assert result.error_code == "add_failed" + w._restore_profile.assert_not_awaited() + + @pytest.mark.asyncio + async def test_activation_timeout_restores_backup(self, qapp): + w = _add_ready_worker(qapp) + w._wait_for_connection = AsyncMock(return_value=False) + result = await w._add_network_impl("Net", "pass", 0) + assert result.error_code == "auth_failed" + assert "previously saved password was kept" in result.message + w._delete_network_impl.assert_awaited_once_with("Net") + w._restore_profile.assert_awaited_once_with("Net", _BACKUP) + + @pytest.mark.asyncio + async def test_activation_timeout_without_backup_reports_removal(self, qapp): + w = _add_ready_worker(qapp, backup=None) + w._wait_for_connection = AsyncMock(return_value=False) + result = await w._add_network_impl("Net", "pass", 0) + assert result.error_code == "auth_failed" + assert "saved profile has been removed" in result.message + w._restore_profile.assert_not_awaited() + + @pytest.mark.asyncio + async def test_failed_restore_reports_removal(self, qapp): + w = _add_ready_worker(qapp) + w._wait_for_connection = AsyncMock(return_value=False) + w._restore_profile = AsyncMock(return_value=False) + result = await w._add_network_impl("Net", "pass", 0) + assert "saved profile has been removed" in result.message + + @pytest.mark.asyncio + async def test_success_leaves_new_profile(self, qapp): + w = _add_ready_worker(qapp) + result = await w._add_network_impl("Net", "pass", 0) + assert result.success + w._delete_network_impl.assert_not_awaited() + w._restore_profile.assert_not_awaited() + + @pytest.mark.asyncio + async def test_drop_existing_skips_unknown_ssid(self, qapp): + w = _make_worker(qapp) + w._is_known = AsyncMock(return_value=False) + w._delete_network_impl = AsyncMock() + assert await w._backup_and_drop_existing("Net") is None + w._delete_network_impl.assert_not_awaited() + + @pytest.mark.asyncio + async def test_drop_existing_backs_up_then_deletes(self, qapp): + w = _make_worker(qapp) + w._saved_cache_dirty = False + w._is_known = AsyncMock(return_value=True) + w._backup_profile = AsyncMock(return_value=_BACKUP) + w._delete_network_impl = AsyncMock() + assert await w._backup_and_drop_existing("Net") == _BACKUP + w._delete_network_impl.assert_awaited_once_with("Net") + assert w._saved_cache_dirty is True + + @pytest.mark.asyncio + async def test_restore_readds_settings_and_dirties_cache(self, qapp): + w = _make_worker(qapp) + w._saved_cache_dirty = False + add = AsyncMock() + w._nm_settings = _ProxyFactory(AsyncProxyMock(add_connection=add)) + assert await w._restore_profile("Net", _BACKUP) is True + add.assert_awaited_once_with(_BACKUP) + assert w._saved_cache_dirty is True + + @pytest.mark.asyncio + async def test_restore_failure_returns_false(self, qapp): + w = _make_worker(qapp) + add = AsyncMock(side_effect=RuntimeError("nm down")) + w._nm_settings = _ProxyFactory(AsyncProxyMock(add_connection=add)) + assert await w._restore_profile("Net", _BACKUP) is False + + class TestDeleteConnectionsById: @pytest.mark.asyncio async def test_deletes_matching(self, qapp): @@ -2028,44 +2170,113 @@ def test_calls_state_and_connectivity(self, qapp): w._async_load_saved_networks.assert_awaited_once() -class TestEnforceBootMutualExclusion: - def test_no_ethernet_returns_early(self, qapp): - w = _make(qapp) - nm = AsyncProxyMock(wireless_enabled=True) - _wire(w, nm=nm) - w._is_ethernet_connected = AsyncMock(return_value=False) - _run(w._enforce_boot_mutual_exclusion()) - # wireless_enabled.set_async should NOT be called - assert ( - not hasattr(nm.wireless_enabled, "set_async") - or not nm.wireless_enabled.set_async.called +class TestWiredProfilesAutoconnect: + """Device.Autoconnect dies on NM restart; only the profile flag persists.""" + + @staticmethod + def _wire_profiles(w, conn_type="802-3-ethernet", autoconnect=True): + nm_settings_proxy = AsyncProxyMock( + list_connections=AsyncMock(return_value=["/conn/eth"]) ) + w._nm_settings = _ProxyFactory(nm_settings_proxy) + settings = { + "connection": { + "type": ("s", conn_type), + "autoconnect": ("b", autoconnect), + "timestamp": ("t", 123), + }, + "ipv4": {"method": ("s", "auto")}, + } + w._gather_settings = AsyncMock(return_value=[("/conn/eth", settings)]) + conn_proxy = AsyncProxyMock(update=AsyncMock()) + w._conn_settings = lambda path: conn_proxy + return conn_proxy + + @pytest.mark.asyncio + async def test_disables_wired_profile(self, qapp): + w = _make_worker(qapp) + conn = self._wire_profiles(w, autoconnect=True) + await w._set_wired_profiles_autoconnect(False) + props = conn.update.await_args[0][0] + assert props["connection"]["autoconnect"] == ("b", False) + + @pytest.mark.asyncio + async def test_strips_timestamp_nm_will_not_accept(self, qapp): + w = _make_worker(qapp) + conn = self._wire_profiles(w, autoconnect=True) + await w._set_wired_profiles_autoconnect(False) + assert "timestamp" not in conn.update.await_args[0][0]["connection"] - def test_ethernet_active_wifi_on_disables_wifi(self, qapp): + @pytest.mark.asyncio + async def test_reenables_wired_profile(self, qapp): + w = _make_worker(qapp) + conn = self._wire_profiles(w, autoconnect=False) + await w._set_wired_profiles_autoconnect(True) + assert conn.update.await_args[0][0]["connection"]["autoconnect"] == ("b", True) + + @pytest.mark.asyncio + async def test_skips_when_already_correct(self, qapp): + w = _make_worker(qapp) + conn = self._wire_profiles(w, autoconnect=True) + await w._set_wired_profiles_autoconnect(True) + conn.update.assert_not_awaited() + + @pytest.mark.asyncio + async def test_ignores_non_ethernet_profiles(self, qapp): + w = _make_worker(qapp) + conn = self._wire_profiles(w, conn_type="802-11-wireless", autoconnect=True) + await w._set_wired_profiles_autoconnect(False) + conn.update.assert_not_awaited() + + @pytest.mark.asyncio + async def test_exception_is_non_fatal(self, qapp): + w = _make_worker(qapp) + w._nm_settings = MagicMock(side_effect=RuntimeError("boom")) + await w._set_wired_profiles_autoconnect(False) # must not raise + + +class TestEnsureWiredAutoconnect: + def test_no_wired_device_returns_early(self, qapp): + w = _make(qapp, wired=False) + wired = AsyncProxyMock(state=30, autoconnect=False) + _wire(w, wired_proxy=wired) + _run(w._ensure_wired_autoconnect()) + wired.autoconnect.set_async.assert_not_awaited() + + def test_autoconnect_off_is_rearmed(self, qapp): w = _make(qapp) - nm = AsyncProxyMock(wireless_enabled=True) - _wire(w, nm=nm) - wifi = AsyncProxyMock() - wifi.disconnect = AsyncMock() - _wire(w, wifi_proxy=wifi) - w._is_ethernet_connected = AsyncMock(return_value=True) - w._wait_for_wifi_radio = AsyncMock(return_value=True) - _run(w._enforce_boot_mutual_exclusion()) - nm.wireless_enabled.set_async.assert_awaited_once_with(False) - assert w._is_hotspot_active is False + wired = AsyncProxyMock(state=30, autoconnect=False) + _wire(w, wired_proxy=wired) + _run(w._ensure_wired_autoconnect()) + wired.autoconnect.set_async.assert_awaited_once_with(True) - def test_ethernet_active_wifi_off_no_action(self, qapp): + def test_autoconnect_on_is_left_alone(self, qapp): w = _make(qapp) - nm = AsyncProxyMock(wireless_enabled=False) - _wire(w, nm=nm) - w._is_ethernet_connected = AsyncMock(return_value=True) - _run(w._enforce_boot_mutual_exclusion()) - nm.wireless_enabled.set_async.assert_not_awaited() + wired = AsyncProxyMock(state=100, autoconnect=True) + _wire(w, wired_proxy=wired) + _run(w._ensure_wired_autoconnect()) + wired.autoconnect.set_async.assert_not_awaited() + + def test_profiles_are_rearmed_too(self, qapp): + w = _make(qapp) + w._set_wired_profiles_autoconnect = AsyncMock() + _wire(w, wired_proxy=AsyncProxyMock(state=30, autoconnect=False)) + _run(w._ensure_wired_autoconnect()) + w._set_wired_profiles_autoconnect.assert_awaited_once_with(True) def test_exception_is_non_fatal(self, qapp): w = _make(qapp) - w._is_ethernet_connected = AsyncMock(side_effect=RuntimeError("boom")) - _run(w._enforce_boot_mutual_exclusion()) # must not raise + w._generic = MagicMock(side_effect=RuntimeError("boom")) + _run(w._ensure_wired_autoconnect()) # must not raise + + def test_wifi_radio_is_never_touched(self, qapp): + w = _make(qapp) + nm = AsyncProxyMock(wireless_enabled=True) + _wire(w, nm=nm) + wired = AsyncProxyMock(state=30, autoconnect=False) + _wire(w, wired_proxy=wired) + _run(w._ensure_wired_autoconnect()) + nm.wireless_enabled.set_async.assert_not_awaited() class TestWaitForWifiRadio: @@ -2105,17 +2316,15 @@ def test_disable_wifi_happy_path(self, qapp): assert received[0].success is True assert w._is_hotspot_active is False - def test_enable_wifi_disconnects_ethernet(self, qapp): + def test_enable_wifi_leaves_ethernet_untouched(self, qapp): w = _make(qapp) nm = AsyncProxyMock(wireless_enabled=False) _wire(w, nm=nm) w._is_ethernet_connected = AsyncMock(return_value=True) - w._async_disconnect_ethernet = AsyncMock() w._wait_for_wifi_radio = AsyncMock(return_value=True) w._build_current_state = AsyncMock(return_value=NetworkState()) _run(w._async_set_wifi_enabled(True)) - w._async_disconnect_ethernet.assert_awaited_once() nm.wireless_enabled.set_async.assert_awaited_once_with(True) def test_already_matching_skips_toggle(self, qapp): @@ -2164,6 +2373,54 @@ def test_calls_disconnect(self, qapp): wired.disconnect.assert_awaited_once() w._deactivate_all_vlans.assert_awaited_once() + def test_persists_choice_in_the_profile(self, qapp): + w = _make(qapp) + wired = AsyncProxyMock() + wired.disconnect = AsyncMock() + _wire(w, wired_proxy=wired) + w._is_ethernet_connected = AsyncMock(return_value=False) + w._deactivate_all_vlans = AsyncMock() + w._set_wired_profiles_autoconnect = AsyncMock() + _run(w._async_disconnect_ethernet()) + w._set_wired_profiles_autoconnect.assert_awaited_once_with(False) + + def test_already_inactive_is_not_an_error(self, qapp): + w = _make(qapp) + wired = AsyncProxyMock() + wired.disconnect = AsyncMock( + side_effect=RuntimeError("This device is not active") + ) + _wire(w, wired_proxy=wired) + w._is_ethernet_connected = AsyncMock(return_value=False) + w._deactivate_all_vlans = AsyncMock() + with patch.object(_worker_mod.logger, "error") as err: + _run(w._async_disconnect_ethernet()) + err.assert_not_called() + + def test_persists_choice_even_when_already_inactive(self, qapp): + w = _make(qapp) + wired = AsyncProxyMock() + wired.disconnect = AsyncMock( + side_effect=RuntimeError("This device is not active") + ) + _wire(w, wired_proxy=wired) + w._is_ethernet_connected = AsyncMock(return_value=False) + w._deactivate_all_vlans = AsyncMock() + w._set_wired_profiles_autoconnect = AsyncMock() + _run(w._async_disconnect_ethernet()) + w._set_wired_profiles_autoconnect.assert_awaited_once_with(False) + + def test_persists_choice_even_when_teardown_fails(self, qapp): + w = _make(qapp) + wired = AsyncProxyMock() + wired.disconnect = AsyncMock(side_effect=RuntimeError("boom")) + _wire(w, wired_proxy=wired) + w._is_ethernet_connected = AsyncMock(return_value=False) + w._deactivate_all_vlans = AsyncMock() + w._set_wired_profiles_autoconnect = AsyncMock() + _run(w._async_disconnect_ethernet()) + w._set_wired_profiles_autoconnect.assert_awaited_once_with(False) + class TestConnectEthernetAsync: def test_no_wired_path_emits_error(self, qapp): @@ -2178,11 +2435,7 @@ def test_happy_path(self, qapp): nm = AsyncProxyMock(wireless_enabled=True) nm.activate_connection = AsyncMock() _wire(w, nm=nm) - wifi = AsyncProxyMock() - wifi.disconnect = AsyncMock() - _wire(w, wifi_proxy=wifi) - w._is_ethernet_connected = AsyncMock(return_value=False) - w._wait_for_wifi_radio = AsyncMock(return_value=True) + w._ensure_wired_autoconnect = AsyncMock() w._build_current_state = AsyncMock(return_value=NetworkState()) w._activate_saved_vlans = AsyncMock() w._is_hotspot_active = False @@ -2191,15 +2444,17 @@ def test_happy_path(self, qapp): w.connection_result.connect(results.append) _run(w._async_connect_ethernet()) - nm.wireless_enabled.set_async.assert_awaited_once_with(False) + w._ensure_wired_autoconnect.assert_awaited_once() nm.activate_connection.assert_awaited_once() assert len(results) == 1 assert results[0].success is True def test_exception_emits_error_and_state(self, qapp): w = _make(qapp) - nm = AsyncProxyMock(wireless_enabled=AsyncMock(side_effect=RuntimeError("x"))) + nm = AsyncProxyMock(wireless_enabled=True) + nm.activate_connection = AsyncMock(side_effect=RuntimeError("x")) w._nm = _ProxyFactory(nm) + w._ensure_wired_autoconnect = AsyncMock() w._build_current_state = AsyncMock(return_value=NetworkState()) errors = [] @@ -2600,7 +2855,6 @@ class TestAsyncInitializeFull: def test_happy_path_full_init(self, qapp): w = _make(qapp, running=False) w._detect_interfaces = AsyncMock() - w._enforce_boot_mutual_exclusion = AsyncMock() w._is_ethernet_connected = AsyncMock(return_value=False) w._activate_saved_vlans = AsyncMock() w._start_signal_listeners = AsyncMock() @@ -2617,7 +2871,6 @@ def test_happy_path_full_init(self, qapp): assert w._running is True w._detect_interfaces.assert_awaited_once() - w._enforce_boot_mutual_exclusion.assert_awaited_once() w._start_signal_listeners.assert_awaited_once() assert len(init_signals) == 1 assert len(hotspot_info) == 1