network applet: unified tunnel section, per-tunnel switches, band/encryption info, and settings (proposal — take any part or none) - #13913
Conversation
Best-practices scannerThis is a regex-based check for API usage that can pose security, performance or This check is not perfect and will not replace a normal review.Found 3 potential issue(s):
|
…h network This is an opinionated follow-up to the correctness fixes, offered so the maintainers can take any part of it or none. Each piece stands alone. Tunnels * One "VPN Connections" section holds every tunnel type, each row naming its kind (WireGuard / OpenVPN / PPTP / ...). NMDeviceWIREGUARD is deleted: WireGuard connections are routed into the VPN category instead, so there is a single code path rather than two that can drift apart. * A switch per tunnel and no section-wide switch. Every other master switch in this applet is either a radio-power toggle (wireless_enabled / wwan_enabled) or the sole device's own switch; with two or more NICs the wired section drops its switch entirely. A "disconnect all" switch on a collection is a different kind of control, and NM has no wireguard_enabled to back it. Panel icon * A tunnel now adds a separate indicator next to the link glyph instead of replacing it with a padlock. The "-secure" wifi variants already mean "this access point is encrypted" in the menu (NMNetworkMenuItem._getIcon), so using the same glyph for "a VPN is up" gave one symbol two meanings. * The base glyph is taken from the physical connection rather than from _mainConnection, so a full-tunnel connection that owns the default route no longer hides the link it runs over. * The panel padlock now reflects access point encryption, matching the menu. Previously an open hotspot and a WPA2 network looked identical in the panel. * A count badge appears once more than one tunnel is up, and the tooltip names the active tunnels. Wireless rows * Show the radio band, including 6 GHz, with the band actually associated to shown in brackets. NM ships utils_wifi_2ghz_freqs()/5ghz_freqs() and nothing for 6 GHz, so the classification is done here. * Optionally show channel width and encryption type. _getApSecurityType() only tests PSK and 802.1X, so WPA3-SAE and OWE fall through it as undefined; apSecurityLabel() classifies from the raw flags and distinguishes personal from enterprise. * The band/strength text never ellipsizes - a long SSID gives way instead, since a clipped name still identifies the network but a clipped signal reading does not. The SSID column is capped so one very long name cannot drag the whole menu wider. * Signal strength is watched on the active access point and redrawn when it crosses a signalToIcon() bucket, rather than only on the 10s timer. Settings * settings-schema.json gains four checkboxes for the wireless rows: radio band, signal percentage, channel width, encryption type. Defaults reproduce the previous look. The padlock is deliberately not switchable.
887ad36 to
1d59a9f
Compare
|
The best-practices scanner findings above are addressed — all of the flagged The scanner comment is stale rather than wrong: the workflow re-run is sitting in |
1 participant
This is an opinionated proposal, offered so you can take any part of it or none. I am not expecting it to be merged as-is, and I won't be offended if the answer is "no thanks" or "just that one bit". #13911 is the minimal, uncontroversial subset and stands alone — if only that lands, this has still been worthwhile.
Cut from
master, so it independently contains equivalents of the #13911 fixes. If #13911 merges first I'll happily rebase this down to just the additions.Tunnels: one section, one code path
NMDeviceWIREGUARDis deleted and WireGuard connections are routed into the VPN category instead, giving one "VPN Connections" section with each row naming its kind (WireGuard / OpenVPN / PPTP / …).The argument for merging rather than fixing both: the collapse in #12178 existed because there were two classes.
NMDeviceVPNhad already been made plural by #12930; the WireGuard copy still had a single slot. Two code paths for the same job drifted apart once and can do so again. Deleting one is a net removal of ~55 lines plus special-casing in five places.The kind label answers the obvious objection — that folding WireGuard into "VPN Connections" hides what kind of tunnel each is.
No section-wide switch
A switch per tunnel, and the section title loses its switch. Every other master switch in this applet is one of two things:
client.wireless_enabled— radio power / rfkillwwan_enabled— radio powerBoth VPN and WireGuard construct
NMWiredSectionTitleMenuItem, the single-device title item, and point it at a collection. There is also nowireguard_enabledin NM for it to reflect. The wired section's own answer for "several members" is to drop the switch, so that is what this does.Panel icon
-securewifi variants already mean "this access point is encrypted" in the menu (NMNetworkMenuItem._getIcon), so using the same glyph for "a VPN is up" gave one symbol two meanings.nm-appletships two distinct glyphs (nm-secure-lock-symbolic,nm-vpn-active-lock-symbolic) for exactly this reason._mainConnection, so a full-tunnel connection that owns the default route no longer hides the link it runs over.Wireless rows
2.4·[5]·6 GHz. NM shipsutils_wifi_2ghz_freqs()/utils_wifi_5ghz_freqs()and nothing for 6 GHz, so the classification is done locally._getApSecurityType()only tests PSK and 802.1X, so WPA3-SAE and OWE fall through it asundefined(see network applet: several small correctness bugs (dead loop, splice truncation, unresolvable icon name, missing return, WPA3 unclassified) #13912 item 5);apSecurityLabel()classifies from the raw flags and distinguishes personal from enterprise.signalToIcon()bucket. Measured here:notify::strengthfires ~8/min on the active AP but crossed a bucket 0 times in 45s stationary, so this is cheaper than the existing 10s poll, not dearer. Watching all APs would have been ~280 events/min with 35 in range — hence active-only.Settings
settings-schema.jsongains four checkboxes: radio band, signal percentage, channel width, encryption type. Defaults reproduce the current look (band and percentage on, the other two off), so nothing changes for anyone who never opens it. The padlock is deliberately not switchable — it answers "is this network open?".Testing
Runtime-tested, not just compiled. Installed as a user override on Cinnamon 6.6.9 (which differs from master only by the four lines of #13847) and exercised against real hardware and real access points:
2.OpenVPN, appearing without an applet reload.WPA2,WPA2/3(a transition-mode AP thatnmclireports as "WPA2 WPA3"), andOpen.Not tested: ethernet link speed (no wired NIC on this machine) and the 6 GHz band display (the radio supports 6E but no 6 GHz APs have been in range). Both are correct by construction and unexercised. Nor has any of this run on a
masterbuild.Happy to split this into separate PRs per section, drop anything, or adjust naming/defaults — whatever is easiest to review.
Disclosure: I worked on this with Claude (Anthropic's Claude Code). It did the bulk of the code reading, diagnosis and patch authoring, and drafted this write-up; I reviewed the changes and ran them on my own machine before filing. The commits carry a
Co-Authored-By: Claudetrailer to the same effect. Flagging it so you can weight the review accordingly.