quickshell-mirror
network: add network integration with NetworkManager backend
Adds basic support for listing wifi devices and networks and attempting a connection. Missing support for connections that require an agent.
Closes #74.
I'm not sure what stage of development you're at here and haven't checked code yet, but just so we're clear I'd like to expose this under Quickshell.Network and keep the API clean of NM specific things (an IWD backend has been requested as well).
I don't think I've made that clear so far. (If you've got any questions, @ me here or in the matrix/discord)
Still missing a secret agent for networks that require passwords. I've tested with nm-applet as the agent and everything seems to work.
I completely separated the frontend and backend objects, with some nm-specific members in the frontend. Happy to do multi-backend in a different style though.
I think this is enough features (+ agent) for most WiFi use cases, but there's a ton more functionality to add in the future:
- Connection manager
- VPN connections
- Wired/ethernet
- Device speed
@outfoxxed Thanks for the feedback! I think this is ready for another review.
There are a couple bits in here that look completely insane, but judging by the fact that they're on the NM api surface I'm going to assume it's NM's fault.
Yep. Unfortunately the api doesn't have any concept of wifi networks. Had to grab access points from one interface, "active" and "saved" connections from another, and then group them by SSID. It also doesn't give any useful security info, hence the copied code from nmcli.
Other clients do something similar: https://invent.kde.org/plasma/plasma-nm/-/blob/master/libs/models/networkmodel.cpp?ref_type=heads https://invent.kde.org/frameworks/networkmanager-qt/-/blob/master/src/wirelessnetwork.cpp?ref_type=heads
@bbedward Would love your feedback on this API. Besides the obvious missing features, does the design make anything difficult for DMS?
@bbedward Would love your feedback on this API. Besides the obvious missing features, does the design make anything difficult for DMS?
It looks pretty consistent with quickshell conventions to me. I'd like to see the agent/connection manager, but it's not necessary for the initial integration IMO (the bluez implementation doesn't have an agent either)
Made an attempt at WifiScanner following the KDE implementation. Requires deprecated header <time.h> for CLOCK_BOOTTIME -> QDateTime. I like the scanner but the conditional network visibility is inherently hacky because NM. Kept it in one list, networks are visible if scanning or connected or known.
@outfoxxed Yep!
I decided to keep WifiNetwork as a group of all access points and connection settings for a given SSID. KDE shows a network item PER conn settings, which makes sense because its only backend (NM) is connection centric. But I think 99% of use is connecting to a network which only has one conn settings. IWD doesn't even allow multiple. Right now, if theres multiple it attempts a connection using the best settings (highest security).
Eventually we'd want to expose the AccessPoint(s) and ConnSetting(s) (only one for IWD) under these networks and expand the connect() function to optionally take a specific AP or settings. This gives feature parity with the NM API for the 1% who actually use multiple settings.
A connection editor and these extensions to WifiNetwork would be a large feature so I was hoping to get the basics merged before that. The biggest issue with not including ConnSetting initially is losing a forget() function that removes the setting. Does a forget() at the WifiNetwork level that deletes ALL its associated settings suffice for now?
Would it make sense to reuse the KF6 NetworkManagerQt bindings?
https://api.kde.org/networkmanager-qt-index.html
It's a tier 1 API so the dependencies are very minor, same for eventual ModemManager. Take a look at Plasma-NM for inspiration.
Would it make sense to reuse the KF6 NetworkManagerQt bindings?
It's not permitted here, see #74. I don't think there's a good argument to use it over the qt_add_dbus_interface pattern that the other services use.
Carson Powers @.***> writes:
Would it make sense to reuse the KF6 NetworkManagerQt bindings?
It's not permitted here, see #74. I don't think there's a good argument to use it over the qt_add_dbus_interface pattern that the other services use.
OK that's disappointing. I don't think it's good to reinvent the wheel here.
In any case I think there is a reason that most projects end of wrapping libnm, i.e. to reduce the overhead on maintaining their NetworkManager client API.
The NetworkManager documentation recommends to use LibNM for more complex cases such as this one: https://www.networkmanager.dev/docs/libnm/latest/usage.html
On another does his have to be added as internal module? Maybe it's better if this lives as an external/separately packaged plugin.
@Thaodan
OK that's disappointing. I don't think it's good to reinvent the wheel here.
In any case I think there is a reason that most projects end of wrapping libnm, i.e. to reduce the overhead on maintaining their NetworkManager client API.
The NetworkManager documentation recommends to use LibNM for more complex cases such as this one: https://www.networkmanager.dev/docs/libnm/latest/usage.html
On another does his have to be added as internal module? Maybe it's better if this lives as an external/separately packaged plugin.
Eventually a lot of the project should probably be split into separately installable modules, however that requires some work I haven't had any time to do. If it were more modular I wouldn't have a problem with an nm dep for nm, though my general policy against KDE deps remains due to kde/distro release cycles, some distros effectively forcing that pulling in one kde dep pulls in all of kde.
@cpwrs I will get to this this weekend. I've had a lot going on and haven't had time but I want to apologize because the review cycle for this has been far too long.
outfoxxed @.***> writes:
outfoxxed left a comment (quickshell-mirror/quickshell#96)
@Thaodan
OK that's disappointing. I don't think it's good to reinvent the wheel here.
In any case I think there is a reason that most projects end of wrapping libnm, i.e. to reduce the overhead on maintaining their NetworkManager client API.
The NetworkManager documentation recommends to use LibNM for more complex cases such as this one: https://www.networkmanager.dev/docs/libnm/latest/usage.html
On another does his have to be added as internal module? Maybe it's better if this lives as an external/separately packaged plugin.
Eventually a lot of the project should probably be split into separately installable modules, however that requires some work I haven't had any time to do. If it were more modular I wouldn't have a problem with an nm dep for nm, though my general policy against KDE deps remains due to kde/distro release cycles, some distros effectively forcing that pulling in one kde dep pulls in all of kde.
OK thank you for explaining. Minor comment: I don't think this kind of issue exist these days anymore at least not since the KF5 days. I'm not aware of any distributions that have such bad packaging practices left. There are a few non-KDE Qt programs who have picked up KDE Frameworks dependencies without those issues. I.e. for example LxQt.
Anyway thanks for your work.
@outfoxxed Thanks for the reviews! It's my first time working with Qt or DBus so I really appreciate the feedback and opportunity.
WARN quickshell.network.networkmanager: Sent removal signal for "/org/freedesktop/NetworkManager/Devices/11" which is not registered.
This was true but not useful. Only warns on device types we support now. Also fixed a bug that was setting security to Unknown when there was a better match.
There are a couple more options id like to see in network configuration (e.g. per network autoconnect) but we can leave those till this is merged.
I have a big list of stuff that's trivial to add to NM, and a good idea of how IWD will look. I can make an issue with some TODOs.
Also have you tested this on a non-open unknown wifi network? It doesn't seem to be able to request a password, at least with the tester.
Yep! The device waits at NMDeviceState.NeedAuth when there's an NM secret agent registered. I've been using nm-applet to test. A few security types have static passwords that can be provided without an agent - adding this to the list.