Skip to content

Conversation

@cpwrs
Copy link

@cpwrs cpwrs commented Jul 6, 2025

Adds basic support for listing wifi devices and networks and attempting a connection. Missing support for connections that require an agent.

Closes #74.

@outfoxxed
Copy link
Member

Why are we linking nm? Should be entirely over dbus

cpwrs added 7 commits July 9, 2025 14:57
NMWireless will be a property of Device (if DeviceType==Wifi). In the
future if many org.freedesktop.networkmanager.Device.* are implemented,
we can use the factory pattern to create an NMDevice with its subdevice
properties and methods.
@outfoxxed
Copy link
Member

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)

@outfoxxed outfoxxed force-pushed the master branch 2 times, most recently from 9d41148 to 4b35d7b Compare July 10, 2025 08:51
@cpwrs cpwrs changed the title NetworkManager Service Quickshell.Network NetworkManager backend Jul 11, 2025
@outfoxxed outfoxxed force-pushed the master branch 2 times, most recently from 525a933 to bb206e3 Compare July 13, 2025 05:02
Copy link
Member

@outfoxxed outfoxxed left a comment

Choose a reason for hiding this comment

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

Sorry its taken so long but here's an initial review.

I'd appreciate if auto was used in more places, as well as whitespace following the existing code-style more as well.

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.

- Prefer bindables over connections for synced properties
- Remove lambdas from ready and disappeared signal connections
- Add namespaces to Q_PROPERTY definitions
- Remove Network.wifi.defaultDevice (easy to add in userspace)
- Add all devices to a ListView in network.qml
@cpwrs
Copy link
Author

cpwrs commented Sep 5, 2025

@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

@cpwrs cpwrs requested a review from outfoxxed September 5, 2025 18:58
@cpwrs cpwrs changed the title Quickshell.Network NetworkManager backend network: network integration with NetworkManager backend Sep 25, 2025
@cpwrs cpwrs changed the title network: network integration with NetworkManager backend network: add network integration with NetworkManager backend Sep 25, 2025
@outfoxxed outfoxxed force-pushed the master branch 3 times, most recently from a92879d to 9bb2c04 Compare October 4, 2025 20:00
@cpwrs
Copy link
Author

cpwrs commented Oct 8, 2025

@bbedward Would love your feedback on this API. Besides the obvious missing features, does the design make anything difficult for DMS?

@bbedward
Copy link
Contributor

bbedward commented Oct 8, 2025

@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)

Copy link
Member

@outfoxxed outfoxxed left a comment

Choose a reason for hiding this comment

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

Sorry I wasn't able to do a full review today, but I think I got most of the interface. These changes don't necessarily have to be made if you've got a good reason not to though.

@cpwrs
Copy link
Author

cpwrs commented Oct 16, 2025

Resolved all but scan

A few changes to the frontend Network API:
- Expose all devices at the toplevel
- Move wifi rfkill options to the toplevel
- Add WifiSecurityType (backend agnostic)
- Move WifiNetwork signal strength to a qreal from 0.0 to 1.0
- Add extensible BaseNetwork class
@cpwrs
Copy link
Author

cpwrs commented Oct 22, 2025

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.

- Add WifiScanner to replace requestScan in API
- Implement NM scanner that avoids rate limit
- Add conditional visibility to networks based on scanner state
- Add gates to backend-specific props in the manual test
@cpwrs
Copy link
Author

cpwrs commented Oct 23, 2025

I think it's more useful to expose each available connection like KDE, networks are more of a connection variant.

Gonna make that change and set up the API to be extensible to other wifi device modes (station, ap, adhoc) and their associated connections (networks, hotspots, peers)

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.

NetworkManager service

4 participants