Skip to content

core: services: wifi: WifiManager: Start without a wlan interface - #4215

Open
patrickelectric wants to merge 7 commits into
bluerobotics:masterfrom
patrickelectric:quiet-missing-wlan
Open

core: services: wifi: WifiManager: Start without a wlan interface#4215
patrickelectric wants to merge 7 commits into
bluerobotics:masterfrom
patrickelectric:quiet-missing-wlan

Conversation

@patrickelectric

Copy link
Copy Markdown
Member

wpa_supplicant fails when there is no wlan adapter.

Fix #4210

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: MINOR SUGGESTIONS ✏️

Makes the wifi service tolerate startup with no wlan interface: the wpa_supplicant WifiManager no longer raises when the UDP fallback socket also fails, instead flagging itself as unavailable and short-circuiting every public method that requires a live socket. The NetworkManager handler mirrors the same state="unavailable" sentinel, and the frontend renders "No wifi adapter" plus backs the status polling off to 30 s while that state is set. Adds a small pytest module for the new guards.

1. Correctness & Implementation Bugs

  • 1.1 [minor] core/services/wifi/wifi_handlers/wpa_supplicant/WifiManager.py:573 — after the change, start() returns normally on UDP-connect failure, but still schedules auto_reconnect(60) and start_hotspot_watchdog() on the loop. Both tasks eventually call methods that end up in WPASupplicant.send_command, which triggers assert self.sock because wpa.run(...) never succeeded; the resulting SockCommError gets re-raised as RuntimeError in enable_saved_networks and propagates out of the task, killing the watchdog silently. Consider skipping (or short-circuiting) the two loop.create_task(...) calls when self.wpa_path is None, so the service degrades cleanly instead of running for 60 s and then dying quietly.
  • 1.2 [minor] core/services/wifi/wifi_handlers/wpa_supplicant/WifiManager.py:97self.wpa_path = str(path) uses str() on either a socket path or a (host, port) tuple. For the UDP fallback that yields the literal string "('127.0.0.1', 6664)", which is fine as a truthy marker but confusing given the attribute is typed Optional[str] and elsewhere is treated as a filesystem path. Consider using a dedicated self._connected: bool (or self.wpa_target: Union[str, Tuple[str, int], None]) so the semantic and the name line up.
  • 1.3 [nit] core/services/wifi/wifi_handlers/wpa_supplicant/WifiManager.py:98 — the added comment reads "run only returns when the socket is connected, before that there is no adapter to talk to". WPASupplicant.run() only guarantees the wpa_supplicant control socket is connected, not that an actual wlan adapter is present (the UDP fallback path succeeds even when wpa_supplicant is talking to nothing on the far side). Reword or drop.

6. Code Quality & Style

  • 6.1 [nit] core/services/wifi/test_wifi_manager.py:46_no_networks(_self: Any) -> list[Any] uses PEP 585 builtin-generics syntax while the rest of the wifi service consistently uses typing.List (List[ScannedWifiNetwork], List[SavedWifiNetwork], etc.). Switch to List[Any] (or list unparameterized) for consistency; also worth typing the return as List[ScannedWifiNetwork] to match what get_wifi_available() actually returns.
  • 6.2 [nit] core/services/wifi/wifi_handlers/wpa_supplicant/WifiManager.py:481set_hotspot_credentials now persists the new ssid/password to SettingsV1 even when there is no adapter, then returns before touching the hotspot. That is likely intentional (settings stick around for when an adapter appears), but is a behavior change worth an inline note or a mention in the PR body.
  • 6.3 [nit] core/frontend/src/components/wifi/WifiUpdater.vue:43 — the two setDelay(...) calls hard-code 30000/5000 ms. Lifting them to const UNAVAILABLE_POLL_MS = 30_000 / const NORMAL_POLL_MS = 5_000 keeps the intent readable next to the equally magic constants in data().

7. Tests

  • 7.1 [minor] core/services/wifi/test_wifi_manager.py:13 — the sys.modules dance (_saved = ...; sys.modules.pop(...)) drops exceptions, settings, typedefs from the cache but, unlike test_hotspot.py, does not stub out commonwealth, pyroute2, or fastapi before importing WifiManager. If pytest collects test_hotspot.py first in the same worker (pytest-xdist can and does batch by module), the previously-stubbed commonwealth MagicMock has already been restored, so the import path likely works — but it is genuinely order-dependent. Consider mirroring the _STUBBED block from test_hotspot.py:15 to make this hermetic.
  • 7.2 [minor] core/services/wifi/test_wifi_manager.py:26test_reports_available_after_connecting only asserts manager.wpa_path is not None at the end. Given the PR's core invariant is that wpa_path is set immediately after a successful wpa.run(...) and cleared only in the UDP-failure branch, it would be worth also asserting the value round-trips (assert manager.wpa_path == str(("127.0.0.1", 6664))) and that a subsequent await manager.status() no longer returns state="unavailable" — the current test would still pass even if a future refactor set wpa_path to a placeholder like "connected".

8. Documentation

  • 8.1 [nit] The PR body ("wpa_supplicant fails when there is no wlan adapter. Fix bug: WIFI_STATUS_FETCH_FAIL spam with no wlan #4210") does not mention the frontend or the NetworkManager handler changes, though both are user-visible. Expanding the description would help reviewers and future git log readers.

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.

@patrickelectric patrickelectric changed the title core: services: wifi: WifiManager: Return empty status if wlan is gone core: services: wifi: WifiManager: Start without a wlan interface Aug 21, 2026
@patrickelectric
patrickelectric requested a review from a team August 21, 2026 02:36
@patrickelectric
patrickelectric force-pushed the quiet-missing-wlan branch 3 times, most recently from 1ad8405 to 01815e3 Compare August 21, 2026 14:57
@patrickelectric

Copy link
Copy Markdown
Member Author

sorry, it should be ready now @bluerobotics/blueos-team

@patrickelectric

Copy link
Copy Markdown
Member Author

ping @bluerobotics/blueos-team

1 similar comment
@patrickelectric

Copy link
Copy Markdown
Member Author

ping @bluerobotics/blueos-team

@patrickelectric patrickelectric added the move-to-stable Needs to be cherry-picked and move to stable label Sep 3, 2026
@joaoantoniocardoso

joaoantoniocardoso commented Sep 3, 2026

Copy link
Copy Markdown
Member

I'm unable to use/test wifi on this branch (from the UI).

@patrickelectric

Copy link
Copy Markdown
Member Author

@joaoantoniocardoso check now

@patrickelectric

Copy link
Copy Markdown
Member Author

I found a problem, let me investigate further

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
…s gone

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
…dapter

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
The attribute was only created inside connect, which never runs when
there is no socket to talk to, so the hotspot property raised
AttributeError instead of building the manager.

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
…ects

It was set before the connection was attempted, so the udp fallback
working was reported to the frontend as no wifi adapter.

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Both handlers use the same state now, the frontend says there is no
adapter on bookworm as well.

Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
@patrickelectric
patrickelectric marked this pull request as ready for review September 11, 2026 10:38
@patrickelectric

Copy link
Copy Markdown
Member Author

@joaoantoniocardoso check now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

move-to-stable Needs to be cherry-picked and move to stable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: WIFI_STATUS_FETCH_FAIL spam with no wlan

2 participants