diff --git a/BlocksScreen/lib/network/worker.py b/BlocksScreen/lib/network/worker.py index 8ea24744..f01c9030 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 collections.abc 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,39 @@ 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 + ) + # Report the first failure only; each retry would stack another popup. + if delay == _BUS_RETRY_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 +170,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 +180,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 +189,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) @@ -334,14 +385,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 @@ -361,12 +414,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.""" @@ -430,11 +486,21 @@ async def _start_signal_listeners(self) -> None: logger.info("Started %d D-Bus signal listeners", len(self._listener_tasks)) + async def _restart_signal_listeners(self) -> None: + """Cancel the listener tasks and respawn them on the current bus and paths.""" + for task in self._listener_tasks: + task.cancel() + self._listener_tasks.clear() + if self._running: + await self._start_signal_listeners() + 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: @@ -444,19 +510,81 @@ 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) + old = (self._primary_wifi_path, self._primary_wired_path) + self._reset_signal_proxies() + self._primary_wifi_path = "" + self._primary_wifi_iface = "" + self._primary_wired_path = "" + self._primary_wired_iface = "" + await self._detect_interfaces() + self._rediscover_gen += 1 + new = (self._primary_wifi_path, self._primary_wired_path) + if any(new) and new != old: + # Running listeners keep their match rules on the old paths. + self._track_task( + self._asyncio_loop.create_task( + self._restart_signal_listeners(), + name="listener_restart", + ) + ) + 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.""" @@ -653,6 +781,8 @@ async def _ensure_dbus_connection(self) -> bool: try: _ = await self._nm().version self._consecutive_dbus_errors = 0 + # The bus survives an NM restart but device paths do not. + await self._recover_signal_sources() return True except Exception as exc: self._consecutive_dbus_errors += 1 @@ -674,19 +804,9 @@ async def _ensure_dbus_connection(self) -> bool: self._primary_wired_iface = "" self._iface_to_device_path.clear() await self._detect_interfaces() - # Rebuild signal proxies on new bus - self._signal_nm = None - self._signal_wifi = None - self._signal_wired = None - self._signal_settings = None - self._ensure_signal_proxies() - # Cancel stale listener tasks bound to old proxies - # and restart them on the new bus connection. - for task in self._listener_tasks: - if not task.done(): - task.cancel() - self._listener_tasks.clear() - await self._start_signal_listeners() + self._reset_signal_proxies() + # Listener tasks hold old proxies; restart them on the new bus. + await self._restart_signal_listeners() self._consecutive_dbus_errors = 0 logger.info("D-Bus reconnection succeeded") if self._primary_wifi_path or self._primary_wired_path: @@ -739,6 +859,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: @@ -956,14 +1112,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 @@ -977,59 +1126,110 @@ 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 _async_scan_networks(self) -> None: - """Request an NM rescan, parse visible APs, and emit networks_scanned.""" + 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 not self._primary_wifi_path: - self.networks_scanned.emit([]) - return - if not await self._ensure_dbus_connection(): - self.networks_scanned.emit([]) - return + 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 - if not await self._nm().wireless_enabled: - self.networks_scanned.emit([]) - return + async def _async_scan_networks(self) -> None: + """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: + await self._scan_networks_once() + except Exception as exc: + logger.warning("Scan failed (%s), re-detecting interfaces", exc) try: - await self._wifi().request_scan({}) - except Exception as exc: - logger.debug( - "Scan request ignored (already scanning or radio off): %s", exc - ) - - if await self._wifi().last_scan == -1: + 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 - 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()) + 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 - networks: list[NetworkInfo] = [] - seen_ssids: set[str] = set() + if not await self._nm().wireless_enabled: + logger.info("scan: radio disabled") + self.networks_scanned.emit([]) + return - 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) + await self._request_scan_if_allowed() + + if await self._wifi().last_scan == -1: + logger.info("scan: device has never scanned (last_scan=-1)") + self.networks_scanned.emit([]) + return - networks.sort(key=lambda n: (-n.network_status, -n.signal_strength)) - self.networks_scanned.emit(networks) + 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()) + networks: list[NetworkInfo] = [] + seen_ssids: set[str] = set() + + 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: + """Request a rescan; NM itself defers or refuses it depending on device state.""" + try: + await self._wifi().request_scan({}) 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 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.""" @@ -1631,41 +1831,74 @@ 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 - - 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", + if not enabled or await self._wifi_enable_preflight(): + ok = await self._apply_wifi_radio(enabled) + word = "enabled" if enabled else "disabled" + self.connection_result.emit( + ConnectionResult( + ok, + f"Wi-Fi {word}" if ok else f"Wi-Fi could not be {word}", ) - - self.connection_result.emit( - ConnectionResult( - True, - f"Wi-Fi {'enabled' if enabled else 'disabled'}", ) - ) + # Also on a failed preflight, so the toggle the user flipped snaps back. self.state_changed.emit(await self._build_current_state()) except Exception as exc: 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: diff --git a/tests/network/test_sdbus_integration.py b/tests/network/test_sdbus_integration.py index e6c3dc37..149e921c 100644 --- a/tests/network/test_sdbus_integration.py +++ b/tests/network/test_sdbus_integration.py @@ -27,12 +27,13 @@ import asyncio import os from contextlib import contextmanager +from pathlib import Path import pytest from PyQt6.QtCore import Qt # ───────────────────────────────────────────────────────────────────────────── -# Gate — skip entire module when opt-in flag is absent +# Gate: skip entire module when opt-in flag is absent # ───────────────────────────────────────────────────────────────────────────── _ENABLED = os.environ.get("NM_INTEGRATION_TESTS", "0") == "1" _SKIP = pytest.mark.skipif(not _ENABLED, reason="NM_INTEGRATION_TESTS not set") @@ -41,6 +42,28 @@ pytestmark = [_SKIP, pytest.mark.timeout(120)] +def _host_has_wired_nic() -> bool: + """True when sysfs shows a physical ARPHRD_ETHER NIC that is not Wi-Fi.""" + try: + entries = list(Path("/sys/class/net").iterdir()) + except OSError: + return False + for p in entries: + try: + if (p / "type").read_text().strip() != "1": + continue + except OSError: + continue + if not (p / "wireless").is_dir() and (p / "device").exists(): + return True + return False + + +_NEEDS_WIRED = pytest.mark.skipif( + not _host_has_wired_nic(), reason="host has no wired NIC" +) + + # ───────────────────────────────────────────────────────────────────────────── # Signal capture helper # ───────────────────────────────────────────────────────────────────────────── @@ -82,7 +105,6 @@ def real_worker(qapp): """ import sys import threading - from pathlib import Path # Add BlocksScreen/ to sys.path so `import configfile` resolves to # BlocksScreen/configfile.py (worker.py imports it at module level). @@ -107,7 +129,7 @@ def real_worker(qapp): try: from BlocksScreen.lib.network.worker import NetworkManagerWorker except ImportError as exc: - # Real sdbus packages not installed on this host — skip gracefully. + # Real sdbus packages not installed on this host: skip gracefully. sys.modules.update(_saved_stubs) if _path_was_added and sys.path and sys.path[0] == _bs_dir: sys.path.pop(0) @@ -195,6 +217,7 @@ class TestRealInterfaces: def test_wifi_path_detected(self, real_worker): assert real_worker._primary_wifi_path, "No Wi-Fi interface found" + @_NEEDS_WIRED def test_wired_path_detected(self, real_worker): assert real_worker._primary_wired_path, "No wired interface found" @@ -330,7 +353,7 @@ def test_os_fallback_unknown_iface_returns_empty(self, real_worker): # ───────────────────────────────────────────────────────────────────────────── -# Destructive write tests — TEST_-prefixed profiles only +# Destructive write tests: TEST_-prefixed profiles only # ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/network/test_worker_unit.py b/tests/network/test_worker_unit.py index 7bace8e5..bd0abe5d 100644 --- a/tests/network/test_worker_unit.py +++ b/tests/network/test_worker_unit.py @@ -13,6 +13,7 @@ """ import asyncio +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -36,6 +37,22 @@ from tests.network.conftest import AsyncProxyMock, _ProxyFactory, _run +class _PropSeq: + """D-Bus property whose successive reads yield *values*, raising any exception.""" + + def __init__(self, *values): + self._values = iter(values) + + def __await__(self): + return self._next().__await__() + + async def _next(self): + value = next(self._values) + if isinstance(value, BaseException): + raise value + return value + + def _make_worker(qapp, *, running=True, with_wifi=True, with_wired=False): """Create a NetworkManagerWorker WITHOUT starting the asyncio thread. @@ -50,7 +67,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 +94,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 +112,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 +134,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 +148,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 +170,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 @@ -307,6 +339,29 @@ async def test_detect_wired_device(self, qapp): assert w._primary_wired_path == "/dev/eth0" assert w._primary_wired_iface == "eth0" + @pytest.mark.asyncio + async def test_missing_device_reported_once_until_one_returns(self, qapp): + w = _make_worker(qapp, with_wifi=False) + nm_proxy = AsyncProxyMock(get_devices=AsyncMock(return_value=[])) + w._nm = _ProxyFactory(nm_proxy) + w._generic = lambda path: AsyncProxyMock(device_type=2, interface="wlan0") + errors = [] + w.error_occurred.connect(lambda op, _msg: errors.append(op)) + + with patch.object(_worker_mod, "dbus_nm") as mock_dbus: + mock_dbus.enums.DeviceType.WIFI = 2 + mock_dbus.enums.DeviceType.ETHERNET = 1 + await w._detect_interfaces() + await w._detect_interfaces() + assert errors == ["wifi_unavailable"] + nm_proxy.get_devices.return_value = ["/dev/wifi0"] + await w._detect_interfaces() + w._primary_wifi_path = "" + nm_proxy.get_devices.return_value = [] + await w._detect_interfaces() + + assert errors == ["wifi_unavailable", "wifi_unavailable"] + class TestDecodeSSID: def test_bytes_input(self): @@ -701,7 +756,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 @@ -1007,6 +1062,18 @@ async def test_scan_exception_emits_error_and_empty(self, qapp): assert len(errors) == 1 assert received == [[]] + @pytest.mark.asyncio + async def test_scan_recovers_stale_path_and_retries(self, qapp): + w = _make_worker(qapp) + w._scan_networks_once = AsyncMock(side_effect=[OSError("UnknownObject"), None]) + w._recover_signal_sources = AsyncMock() + errors = [] + w.error_occurred.connect(lambda op, _msg: errors.append(op)) + await w._async_scan_networks() + w._recover_signal_sources.assert_awaited_once() + assert w._scan_networks_once.await_count == 2 + assert errors == [] + class TestParseAp: @pytest.mark.asyncio @@ -1638,6 +1705,37 @@ async def test_error_increments_counter(self, qapp): assert result is False assert w._consecutive_dbus_errors == 1 + @pytest.mark.asyncio + async def test_healthy_bus_runs_path_recovery(self, qapp): + w = _make_worker(qapp) + w._nm = _ProxyFactory(AsyncProxyMock(version="1.42.4")) + w._recover_signal_sources = AsyncMock() + assert await w._ensure_dbus_connection() is True + w._recover_signal_sources.assert_awaited_once() + + @pytest.mark.asyncio + async def test_reconnect_restarts_listeners_on_new_bus(self, qapp): + w = _make_worker(qapp) + w._consecutive_dbus_errors = w._MAX_DBUS_ERRORS_BEFORE_RECONNECT - 1 + w._nm = MagicMock( + return_value=SimpleNamespace(version=_PropSeq(OSError("bus closed"))) + ) + w._signal_nm = MagicMock(name="old_proxy") + + async def _detect(): + w._primary_wifi_path = "/org/freedesktop/NetworkManager/Devices/3" + + w._detect_interfaces = AsyncMock(side_effect=_detect) + w._restart_signal_listeners = AsyncMock() + events = [] + w.error_occurred.connect(lambda op, _msg: events.append(op)) + + assert await w._ensure_dbus_connection() is True + assert w._signal_nm is None + w._restart_signal_listeners.assert_awaited_once() + assert w._consecutive_dbus_errors == 0 + assert events == ["device_reconnected"] + class TestShutdown: @pytest.mark.asyncio @@ -2015,13 +2113,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): @@ -2356,6 +2457,36 @@ def test_exception_emits_error(self, qapp): assert len(errors) == 1 assert errors[0][0] == "set_wifi_enabled" + def test_failed_preflight_still_resyncs_toggle(self, qapp): + w = _make(qapp) + w._log_radio_state = AsyncMock() + w._wifi_enable_preflight = AsyncMock(return_value=False) + w._apply_wifi_radio = AsyncMock() + w._build_current_state = AsyncMock(return_value=NetworkState()) + states = [] + w.state_changed.connect(states.append) + + _run(w._async_set_wifi_enabled(True)) + + w._apply_wifi_radio.assert_not_awaited() + assert len(states) == 1 + + def test_radio_timeout_reports_failure(self, qapp): + w = _make(qapp) + _wire(w, nm=AsyncProxyMock(wireless_enabled=False)) + w._log_radio_state = AsyncMock() + w._wifi_enable_preflight = AsyncMock(return_value=True) + w._wait_for_wifi_radio = AsyncMock(return_value=False) + w._build_current_state = AsyncMock(return_value=NetworkState()) + results = [] + w.connection_result.connect(results.append) + + _run(w._async_set_wifi_enabled(True)) + + assert [(r.success, r.message) for r in results] == [ + (False, "Wi-Fi could not be enabled") + ] + class TestDisconnectEthernetAsync: def test_no_wired_path_noop(self, qapp): @@ -3056,3 +3187,331 @@ def test_malformed_entry_skipped_returns_valid_entries(self, qapp): result = _run(w._get_saved_networks_impl()) assert len(result) == 1 assert result[0].ssid == "GoodNet" + + +def _stub_redetect(w, new_paths): + """Make every path look stale; re-detection then finds *new_paths* (wifi, wired).""" + w._primary_paths_alive = AsyncMock(return_value=False) + w._ensure_signal_proxies = MagicMock() + w._restart_signal_listeners = AsyncMock() + + async def _detect(): + await asyncio.sleep(0) + w._primary_wifi_path, w._primary_wired_path = new_paths + + w._detect_interfaces = AsyncMock(side_effect=_detect) + + +class TestPrimaryPathsAlive: + def test_no_paths_is_not_alive(self, qapp): + w = _make(qapp, wifi=False, wired=False) + assert _run(w._primary_paths_alive()) is False + + def test_both_type_probes_answer(self, qapp): + w = _make(qapp) + _wire( + w, + wifi_proxy=AsyncProxyMock(mode=2), + wired_proxy=AsyncProxyMock(speed=1000), + ) + assert _run(w._primary_paths_alive()) is True + + def test_reassigned_wired_path_is_stale(self, qapp): + w = _make(qapp) + _wire(w, wifi_proxy=AsyncProxyMock(mode=2)) + w._wired = MagicMock( + return_value=SimpleNamespace(speed=_PropSeq(OSError("No such interface"))) + ) + assert _run(w._primary_paths_alive()) is False + + def test_stale_warning_once_per_generation(self, qapp): + w = _make(qapp) + with patch.object(_worker_mod, "logger") as log: + w._log_stale("wifi", "/p", OSError("gone")) + w._log_stale("wifi", "/p", OSError("gone")) + w._rediscover_gen += 1 + w._log_stale("wifi", "/p", OSError("gone")) + assert log.warning.call_count == 2 + assert log.debug.call_count == 1 + + +class TestRecoverSignalSources: + @pytest.mark.asyncio + async def test_live_paths_skip_redetect(self, qapp): + w = _make(qapp) + _stub_redetect(w, ("", "")) + w._primary_paths_alive.return_value = True + await w._recover_signal_sources() + w._detect_interfaces.assert_not_awaited() + w._ensure_signal_proxies.assert_called_once() + + @pytest.mark.asyncio + async def test_moved_path_restarts_listeners(self, qapp): + w = _make(qapp, wired=False) + w._asyncio_loop = asyncio.get_running_loop() + _stub_redetect(w, ("/org/freedesktop/NetworkManager/Devices/3", "")) + await w._recover_signal_sources() + await asyncio.sleep(0) + w._restart_signal_listeners.assert_awaited_once() + assert w._rediscover_gen == 1 + + @pytest.mark.asyncio + async def test_unchanged_paths_keep_listeners(self, qapp): + w = _make(qapp) + _stub_redetect(w, (w._primary_wifi_path, w._primary_wired_path)) + await w._recover_signal_sources() + assert w._background_tasks == set() + w._restart_signal_listeners.assert_not_called() + + @pytest.mark.asyncio + async def test_nm_gone_clears_paths_without_restart(self, qapp): + w = _make(qapp) + _stub_redetect(w, ("", "")) + await w._recover_signal_sources() + assert (w._primary_wifi_iface, w._primary_wired_iface) == ("", "") + w._restart_signal_listeners.assert_not_called() + + @pytest.mark.asyncio + async def test_racing_callers_redetect_once(self, qapp): + w = _make(qapp) + _stub_redetect(w, (w._primary_wifi_path, w._primary_wired_path)) + await asyncio.gather(*(w._recover_signal_sources() for _ in range(3))) + w._detect_interfaces.assert_awaited_once() + assert w._rediscover_gen == 1 + + +class TestRestartSignalListeners: + @pytest.mark.asyncio + async def test_cancels_old_tasks_and_respawns(self, qapp): + w = _make(qapp) + w._asyncio_loop = asyncio.get_running_loop() + w._ensure_signal_proxies = MagicMock() + w._resilient_listener = AsyncMock() + old = MagicMock(name="old_task") + w._listener_tasks = [old] + + await w._restart_signal_listeners() + await asyncio.sleep(0) + + old.cancel.assert_called_once() + assert old not in w._listener_tasks + assert len(w._listener_tasks) == 7 + + @pytest.mark.asyncio + async def test_stopped_worker_does_not_respawn(self, qapp): + w = _make(qapp, running=False) + w._start_signal_listeners = AsyncMock() + old = MagicMock(name="old_task") + w._listener_tasks = [old] + await w._restart_signal_listeners() + old.cancel.assert_called_once() + assert w._listener_tasks == [] + w._start_signal_listeners.assert_not_awaited() + + +class TestDevicePathForIface: + @pytest.mark.asyncio + async def test_valid_cache_skips_enumeration(self, qapp): + w = _make_worker(qapp) + w._iface_to_device_path = {"wlan0": "/Devices/2"} + w._generic = lambda _path: AsyncProxyMock(interface="wlan0") + nm_proxy = AsyncProxyMock() + w._nm = _ProxyFactory(nm_proxy) + assert await w._device_path_for_iface("wlan0") == "/Devices/2" + nm_proxy.get_devices.assert_not_awaited() + + @pytest.mark.asyncio + async def test_reassigned_cached_path_is_re_resolved(self, qapp): + w = _make_worker(qapp) + w._iface_to_device_path = {"wlan0": "/Devices/2"} + ifaces = {"/Devices/2": "eth0", "/Devices/3": "wlan0"} + w._generic = lambda path: AsyncProxyMock(interface=ifaces[path]) + w._nm = _ProxyFactory( + AsyncProxyMock(get_devices=AsyncMock(return_value=list(ifaces))) + ) + assert await w._device_path_for_iface("wlan0") == "/Devices/3" + assert w._iface_to_device_path == {"wlan0": "/Devices/3"} + + @pytest.mark.asyncio + async def test_unreadable_cached_path_is_dropped(self, qapp): + w = _make_worker(qapp) + w._iface_to_device_path = {"wlan0": "/Devices/2"} + w._generic = lambda _path: SimpleNamespace( + interface=_PropSeq(OSError("UnknownObject")) + ) + w._nm = _ProxyFactory(AsyncProxyMock(get_devices=AsyncMock(return_value=[]))) + assert await w._device_path_for_iface("wlan0") == "" + assert w._iface_to_device_path == {} + + +class TestEnsureNetworkingEnabled: + def test_already_on_skips_enable(self, qapp): + w = _make(qapp) + nm = AsyncProxyMock(networking_enabled=True) + _wire(w, nm=nm) + assert _run(w._ensure_networking_enabled()) is True + nm.enable.assert_not_awaited() + + def test_unreadable_flag_assumes_on(self, qapp): + w = _make(qapp) + w._nm = MagicMock( + return_value=SimpleNamespace(networking_enabled=_PropSeq(OSError("gone"))) + ) + assert _run(w._ensure_networking_enabled()) is True + + def test_enable_failure_returns_false(self, qapp): + w = _make(qapp) + _wire( + w, + nm=AsyncProxyMock( + networking_enabled=False, + enable=AsyncMock(side_effect=OSError("not authorized")), + ), + ) + assert _run(w._ensure_networking_enabled()) is False + + def test_polls_through_nm_restart_until_enabled(self, qapp): + w = _make(qapp) + nm = SimpleNamespace( + networking_enabled=_PropSeq(False, OSError("restarting"), True), + enable=AsyncMock(), + ) + w._nm = MagicMock(return_value=nm) + with patch.object(_worker_mod.asyncio, "sleep", AsyncMock()): + assert _run(w._ensure_networking_enabled()) is True + nm.enable.assert_awaited_once_with(True) + + def test_times_out_when_flag_stays_off(self, qapp): + w = _make(qapp) + nm = AsyncProxyMock(networking_enabled=False) + _wire(w, nm=nm) + assert _run(w._ensure_networking_enabled(timeout=0)) is False + nm.enable.assert_awaited_once_with(True) + + +class TestWifiEnablePreflight: + def test_rfkill_block_reports_reason(self, qapp): + w = _make(qapp) + _wire(w, nm=AsyncProxyMock(wireless_hardware_enabled=False)) + w._ensure_networking_enabled = AsyncMock() + results = [] + w.connection_result.connect(results.append) + assert _run(w._wifi_enable_preflight()) is False + assert [r.message for r in results] == ["Wi-Fi is blocked by a hardware switch"] + w._ensure_networking_enabled.assert_not_awaited() + + def test_networking_off_reports_reason(self, qapp): + w = _make(qapp) + w._wifi_hardware_enabled = AsyncMock(return_value=True) + w._ensure_networking_enabled = AsyncMock(return_value=False) + results = [] + w.connection_result.connect(results.append) + assert _run(w._wifi_enable_preflight()) is False + assert [r.message for r in results] == ["NetworkManager networking is disabled"] + + def test_clear_path_passes(self, qapp): + w = _make(qapp) + w._wifi_hardware_enabled = AsyncMock(return_value=True) + w._ensure_networking_enabled = AsyncMock(return_value=True) + assert _run(w._wifi_enable_preflight()) is True + + +class TestAsyncBootstrap: + @pytest.mark.asyncio + async def test_retries_with_backoff_and_reports_once(self, qapp): + w = _make(qapp) + w._async_initialize = AsyncMock() + errors = [] + w.error_occurred.connect(lambda op, _msg: errors.append(op)) + bus = MagicMock(name="bus") + sleep = AsyncMock() + with ( + patch.object( + _worker_mod.sdbus, + "sd_bus_open_system", + side_effect=[OSError("no bus")] * 3 + [bus], + ), + patch.object(_worker_mod.asyncio, "sleep", sleep), + ): + await w._async_bootstrap() + assert [c.args[0] for c in sleep.await_args_list] == [1.0, 2.0, 4.0] + assert errors == ["initialize"] + assert w._system_bus is bus + w._async_initialize.assert_awaited_once() + + @pytest.mark.asyncio + async def test_shutdown_during_backoff_exits(self, qapp): + w = _make(qapp) + w._async_initialize = AsyncMock() + + async def _stop(_delay): + w._stopping = True + + with ( + patch.object( + _worker_mod.sdbus, "sd_bus_open_system", side_effect=OSError("no bus") + ), + patch.object(_worker_mod.asyncio, "sleep", side_effect=_stop), + ): + await w._async_bootstrap() + assert w._system_bus is None + w._async_initialize.assert_not_awaited() + + +class TestResilientListener: + @pytest.mark.asyncio + async def test_backoff_doubles_up_to_cap(self, qapp): + w = _make(qapp) + w._asyncio_loop = MagicMock(time=MagicMock(return_value=0.0)) + w._reset_signal_proxies = MagicMock() + w._recover_signal_sources = AsyncMock() + delays = [] + + async def _sleep(delay): + delays.append(delay) + w._running = len(delays) < 7 + + with patch.object(_worker_mod.asyncio, "sleep", side_effect=_sleep): + await w._resilient_listener("x", AsyncMock(side_effect=OSError("bus"))) + + assert delays == [3.0, 6.0, 12.0, 24.0, 48.0, 60.0, 60.0] + assert w._reset_signal_proxies.call_count == 7 + assert w._recover_signal_sources.await_count == 6 + + @pytest.mark.asyncio + async def test_long_run_resets_backoff(self, qapp): + w = _make(qapp) + # (start, end) per pass: long, short, long. + w._asyncio_loop = MagicMock( + time=MagicMock(side_effect=[0.0, 100.0, 200.0, 201.0, 300.0, 400.0]) + ) + w._recover_signal_sources = AsyncMock() + delays = [] + + async def _sleep(delay): + delays.append(delay) + w._running = len(delays) < 3 + + with patch.object(_worker_mod.asyncio, "sleep", side_effect=_sleep): + await w._resilient_listener("x", AsyncMock()) + + assert delays == [3.0, 6.0, 3.0] + + @pytest.mark.asyncio + async def test_cancellation_returns_without_restart(self, qapp): + w = _make(qapp) + w._asyncio_loop = MagicMock(time=MagicMock(return_value=0.0)) + w._recover_signal_sources = AsyncMock() + await w._resilient_listener("x", AsyncMock(side_effect=asyncio.CancelledError)) + w._recover_signal_sources.assert_not_awaited() + + +class TestRequestScanIfAllowed: + def test_nm_refusal_is_swallowed(self, qapp): + w = _make(qapp) + wifi = AsyncProxyMock( + request_scan=AsyncMock(side_effect=OSError("Scanning not allowed")) + ) + _wire(w, wifi_proxy=wifi) + _run(w._request_scan_if_allowed()) + wifi.request_scan.assert_awaited_once_with({})