Repository navigation
Conversation
cpwrs
marked this pull request as ready for review
October 2, 2026 13:35
cpwrs
marked this pull request as draft
October 2, 2026 14:15
cpwrs
commented
Oct 2, 2026
Comment on lines
-240
to
+248
| if (this->mAccessPoints.contains(ap->path())) return; | ||
| this->mAccessPoints.insert(ap->path(), ap); | ||
| auto onDestroyed = [this, ap]() { | ||
| if (this->mAccessPoints.take(ap->path())) { | ||
| const QString path = ap->path(); | ||
| if (this->mAccessPoints.contains(path)) return; | ||
| this->mAccessPoints.insert(path, ap); | ||
| auto onDestroyed = [this, ap, path]() { | ||
| if (this->mAccessPoints.take(path)) { |
Contributor
Author
There was a problem hiding this comment.
This fix is unrelated to the PR
Captures path instead of assuming ap->path() can be accessed after its QObject destroyed signal
cpwrs
commented
Oct 2, 2026
|
|
||
| namespace { | ||
| QS_LOGGING_CATEGORY(logNetworkManager, "quickshell.network.networkmanager", QtWarningMsg); | ||
| const QStringList SUPPORTED_CONNECTION_TYPES {"802-11-wireless", "802-3-ethernet"}; |
Contributor
Author
There was a problem hiding this comment.
I restricted connection types to be the same as supported device types. But we can include all connections here as long as I update settingTypeTo/FromQml.
cpwrs
marked this pull request as ready for review
October 2, 2026 18:24
outfoxxed
force-pushed
the
master
branch
2 times, most recently
from
October 6, 2026 04:21
72a1ce2 to
11ca60b
Compare
Contributor
Author
|
I can easily keep support for the old API and mark it deprecated. Also I think every backend change is justified even without the API change. I'll open a PR with those if I can't get this approved. |
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
1 participant
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.
This PR transitions NMSettings to a backend-agnostic KnownNetwork object and adds a toplevel Networking.knownNetworks list. Also adds a slew of related NM backend changes.
API Changes
NMSettings->KnownNetworkNMSettings.id->KnownNetwork.nameNMSettings.uuidNetworking.knownNetworksNetworkDevice.nmSettings->NetworkDevice.knownNetworksNetworkDevice.connectWithSettings->NetworkDevice.connectToKnownNetworkBackend Changes
NMSettings->NMConnection)NMConnectionManagerwhich handles registration of theNMConnectionsNMDevices useNMConnectionManagerfor references to their available conns instead of registering conns themselvessettingsTypeTo/FromQmlutils to cover an exhaustive list of settings whose conversion isn't automatically supportedWhy?
These API changes were mostly to address having backend-specific objects littering the frontend. This change allows us to support settings schemas for different backends without having to expose properties for each of the hundreds of settings. The class is also a good place to add some common settings as Q_PROPERTYs if we want in the future.
The toplevel list is an easy way to edit any settings, even those of unavailable networks.
This is a prerequisite for #922 which could now look like
Networking.addNetwork(settings).