Skip to content

Fix WiFi detection when wired connection is primary#2466

Open
CrazyTodd-one wants to merge 1 commit intoborgbase:masterfrom
CrazyTodd-one:fix-wifi-check-all-networks
Open

Fix WiFi detection when wired connection is primary#2466
CrazyTodd-one wants to merge 1 commit intoborgbase:masterfrom
CrazyTodd-one:fix-wifi-check-all-networks

Conversation

@CrazyTodd-one
Copy link
Copy Markdown

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 one
  • Added get_active_connections_paths() to the DBus adapter, reading the ActiveConnections property
  • Per-connection DBus failures are handled gracefully (logged and skipped, other connections still checked)

Test plan

  • Added test_get_current_wifi_with_wired_primary_and_wifi_active — simulates the docking station scenario (wired primary + WiFi active), verifies WiFi SSID is found
  • Added test_get_current_wifi_skips_failed_connection — verifies that if one connection throws DBusException, remaining connections are still checked
  • Updated existing tests to use get_active_connections_paths mock
  • Fixed incorrect 802-11-ethernet type string to 802-3-ethernet in parametrized test

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>
Copy link
Copy Markdown
Contributor

@m3nu m3nu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 DBusException handling 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-ethernet802-3-ethernet typo 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expand WIFI check to all active networks.

2 participants