refactor(network): Use nmrs for WiFi connections#1262
refactor(network): Use nmrs for WiFi connections#1262cachebag wants to merge 2 commits intopop-os:masterfrom
Conversation
Replace D-Bus channel flow with direct nmrs API calls for WiFi connections. Provides typed error handling and simplifies code. - Add nmrs 1.3.0 dependency - Use `nmrs.connect()` for open and secured networks - Add `ConnectionError` variants for better error messages - Remove request/sender channel boilerplate
Replace channel-based D-Bus requests with direct nmrs API calls for WiFi enable/disable and network forgetting. Provides better error feedback and waits for operations to complete. - Use `nmrs.set_wifi_enabled()` for WiFi toggle - Use `nmrs.forget()` to remove saved networks - Add user-facing error messages for failures - Auto-refresh UI on successful operations
|
Seems good. I wonder how much of the message handling logic in the applet could be moved into https://github.com/pop-os/cosmic-settings/tree/master/subscriptions/network-manager/ so that the settings application and applet can share the same library for implementing their logic in the UI. Trying to deduplicate logic across the settings app and applets moving forward. |
|
@maria-komarova Do we have any designs for replacing the Advanced Network Configuration app (nm-connection-editor)? This may be a good opportunity for a contributor to help with replacing it since they're already working on nmrs. It may be useful as its own standalone app as well, similar to nmrs-gui and nm-connection-editor. |
We'd definitely prefer to collaborate with a well-typed abstraction over direct DBus calls. network-manager's DBus API is somewhat chaotic. |
|
I think both can share parts quite nicely from looking at the code.
This is helpful to know since I could approach this in a more targeted way. Eventually, I'll need to bump to 2.0 due to a long list of breaking changes needed to make the API more stable. Perhaps that would be a good time to solidify everything (probably 1-1.5 months at the most). |
|
Yes, we had the designs. They are old though so might need a review and some adjustments especially to account for it potentially being a separate app. They were made with being part of Settings in mind. |
|
It could be both. Integrated into settings and available as a separate app. |
|
I might need a few days to review the mockups, make adjustments and clean them up a bit. I'll post the link to them here once they are ready. |
|
Sorry for the delay on this but here are the mockups for network details pages. They are most likely missing some things and might need more revisions but they at least give a sense of the direction. Please let me know if there are any questions or thoughts, I'm ready to look more into this. |
|
Heads up that I will be moving network manager logic to a shared location in the cosmic-settings-daemon soon since that will be providing a varlink service that the applet and settings application will connect to. That way the settings application and applets won't need their implementations to be kept in sync anymore. Or need to be updated to get updates to network manager logic. |
3 participants
More than anything, this is a feeler of sorts to see if I could bring in some value to this initiative. My project, nmrs provides (what I think to be) a pretty nice API for NetworkManager over D-Bus.
Happy to revise, re-approach, etc. where needed or if you guys in fact don't need this and prefer dealing with D-Bus directly, then of course feel free to close this.
In either case, love what you guys are doing!
--
Replace D-Bus channel flow with direct nmrs API calls for WiFi connections. Provides typed error handling and simplifies code.
nmrs.connect()for open and secured networksConnectionErrorvariants for better error messagesAlso replace channel-based D-Bus requests with direct nmrs API calls for
WiFi enable/disable and network forgetting. This provides some better error
feedback and waits for operations to complete.