Fix WiFi detection when wired connection is primary#2466
Open
CrazyTodd-one wants to merge 1 commit intoborgbase:masterfrom
Open
Fix WiFi detection when wired connection is primary#2466CrazyTodd-one wants to merge 1 commit intoborgbase:masterfrom
CrazyTodd-one wants to merge 1 commit intoborgbase:masterfrom
Conversation
Check all active NetworkManager connections for WiFi, not just the primary one. When a laptop is docked (wired + WiFi), the primary connection is wired, so the WiFi SSID was never detected and network-based backup restrictions were bypassed. - Add get_active_connections_paths() to iterate ActiveConnections - Handle per-connection DBus failures gracefully - Add tests for docking station scenario and connection failure resilience Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
m3nu
approved these changes
Mar 28, 2026
Contributor
m3nu
left a comment
There was a problem hiding this comment.
Clean, focused fix for a long-standing issue (#1775). The approach of iterating all active connections instead of only the primary is correct and well-implemented.
Strengths:
- Reuses existing
read_dbus_property()helper -- consistent with the adapter pattern - Per-connection
DBusExceptionhandling so one failure doesn't block checking others - Two good new tests covering the wired+wifi scenario and per-connection failure resilience
- Fixes pre-existing
802-11-ethernet→802-3-ethernettypo in test data - API contract preserved (
get_current_wifi() -> str | None)
Note: competing PR #2473 addresses the same issue but has significant problems (debug prints in production, deletes metered-network check, catches bare Exception, no tests, breaks ABC contract). Recommend closing it in favor of this one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1775 — When a laptop is connected via both WiFi and a wired connection (e.g. docking station), the primary connection is wired.
get_current_wifi()only checked the primary connection, so the WiFi SSID was never detected and network-based backup restrictions were silently bypassed.Changes:
get_current_wifi()now iterates all active NetworkManager connections instead of only the primary oneget_active_connections_paths()to the DBus adapter, reading theActiveConnectionspropertyTest plan
test_get_current_wifi_with_wired_primary_and_wifi_active— simulates the docking station scenario (wired primary + WiFi active), verifies WiFi SSID is foundtest_get_current_wifi_skips_failed_connection— verifies that if one connection throws DBusException, remaining connections are still checkedget_active_connections_pathsmock802-11-ethernettype string to802-3-ethernetin parametrized test