feat: rework networkmanager module #1233

Rodrigodd · 2025-11-04T21:30:27Z

Allow a dynamic number of devices.
Show WiFi connection strength and update in real time.
Allow configuring the icons.
Show network info as tool-tip.
Allow whitelist/blacklisting device names/types.

I only have a WiFi and Wireguard interfaces, so I was unable to test other device types, but I tried to preserve the existing functionality as much as I could.

Screenshot

#148

JakeStanger · 2025-11-07T11:34:59Z

Does this superseed #690?

Rodrigodd · 2025-11-08T22:15:19Z

Oh, I guess so. I didn’t notice that @Zedfrigg was already working on a reword of his network manager module implementation.

Compared with #690, my PR appears to have more features. The only area I think my PR is lacking is in how I’m passing events between the Client and the Module. My PR sends the entire list of devices whenever a device changes, and future events refer to devices by index, whereas #690 uses an ID system.

Zedfrigg · 2025-11-09T12:58:52Z

Hi there! This seems pretty in line with the features I implemented or plan to implement in my PR. I'm away from home at the moment and don't have the opportunity to read through your code in detail until a couple of days from now, so instead I'll leave you with some of the considerations I've encountered while implementing this myself:

Handling initialisation of more module instances (e.g. new monitor plugged in) correctly when already running
Handling the addition and removal of network devices (e.g. usb ethernet adapters)
Doing all of this without leaking memory or leaving unused tasks running

JakeStanger

Thanks for the hard work.

I think we'll go with this over #690, pending a review from @Zedfrigg - I'd want to make sure we don't lose anything from there.

There's some initial things I've spotted in review, if you can have a look at those for me please.

docs/modules/Networkmanager.md

JakeStanger · 2025-11-07T11:40:41Z

docs/modules/Networkmanager.md

+| `icons.wired.connected`       | `string`   | `icon:network-wired-symbolic`                         | Icon for connected wired device                                                                                    |
+| `icons.wired.acquiring`       | `string`   | `icon:network-wired-acquiring-symbolic`               | Icon for acquiring wired device                                                                                    |
+| `icons.wired.disconnected`    | `string`   | `""`                                                  | Icon for disconnected wired device                                                                                 |
+| `icons.wifi.levels`           | `string[]` | `["icon:network-wireless-signal-none-symbolic", ...]` | Icon for each strengh level of a connected wifi connection, from lowest to highest. The default contains 5 levels. |


Levels as an array may be more complex than necessary. It also adds a third approach to handling levels compared to other modules (battery, volume). A low/medium/high approach may be more accessible.

At the very least the full list of icons does need to be included in the docs. See how it's being handled in the inhibit module as example:

https://github.com/JakeStanger/ironbar/pull/1190/files#diff-20bab0fe763db2e74cc41455d0d53cdeaa00c9707c367e2c4fac3a1bde131bcfR13

An array seemed to me to be the simplest option. GTK icon themes have five levels of wireless signal strength, and I would like to have all of them available. Do you prefer that I make a struct with five fields: none, weak, ok, good and excellent?
There’s also the option of making a map of thresholds like in the battery module, but that would be even more complex, and the current mapping is non-linear.

Please let me know which approach you prefer.

But, for now, I’ve moved the default array out of the table.

I'll get back to you on this one. #1235 is proposing yet another syntax so this needs standardising and I need to have a think about the best approach.

docs/modules/Networkmanager.md

src/modules/networkmanager/config.rs

docs/modules/Networkmanager.md

src/modules/networkmanager.rs

- Allow a dynamic number of devices. - Show WiFi connection strength and update in real time. - Allow configuring the icons. - Show network info as tool-tip. - Allow whitelist/blacklisting device names/types.

So they don't merge into doc-comments, making them harder to read.

docs/modules/Network-Manager.md

src/clients/networkmanager/state.rs

src/clients/networkmanager/mod.rs

src/modules/networkmanager.rs

docs/modules/Network-Manager.md

src/modules/networkmanager.rs

JakeStanger

Few last minor mop-up bits and then, pending a quick test, I think we're there. Thank you

docs/modules/Network-Manager.md

src/clients/networkmanager/mod.rs

JakeStanger

Cool all LGTM, pending a decision around the icon config syntax. I'll try and have a think about that and get back to you soon.

JakeStanger changed the title ~~feat: reword networkmanager module~~ feat: rework networkmanager module Nov 7, 2025

JakeStanger added this to the 0.19.0 milestone Nov 7, 2025

JakeStanger requested changes Nov 11, 2025

View reviewed changes

Rodrigodd force-pushed the feat-network-manager-rework branch from f1d2919 to 574d343 Compare November 11, 2025 19:46

Rodrigodd added 12 commits November 11, 2025 17:22

feat: reword networkmanager module

db03af3

- Allow a dynamic number of devices. - Show WiFi connection strength and update in real time. - Allow configuring the icons. - Show network info as tool-tip. - Allow whitelist/blacklisting device names/types.

refactor: rename 'IconsConfig' to 'Icons'

cded882

refactor: update DeviceType documentation

c517617

refactor: replace mutually exclusive bools with enum

152e907

refactor: move module_impl! to top of impl

d1fb624

docs(network-manager): update docs

f07439b

fix(networkmanager): fix panic when number of wifi levels is 0 or 1

0d476fc

refactor: simplify use of #[serder(default)]

b376f3f

docs: fix typo

d632b81

style: move comments of unmapped dbus properties

5822a86

So they don't merge into doc-comments, making them harder to read.

chore: remove futures-signals

15d7f82

docs: add list of device types to documentation.

11bccfd

Rodrigodd force-pushed the feat-network-manager-rework branch from 6aec31c to 11bccfd Compare November 11, 2025 20:24

Rodrigodd requested a review from JakeStanger November 11, 2025 20:25

JakeStanger requested changes Nov 13, 2025

View reviewed changes

Rodrigodd added 5 commits November 13, 2025 14:33

fix: fix typos in docs

f15f523

style: add space and header for unmapped properties comment

99c3c3f

refactor: break up unwieldy expression

657f2af

refactor: replace unwrap with expect

978f86f

fix: fix todo in strenght_to_level

5ddf6c4

Rodrigodd force-pushed the feat-network-manager-rework branch from e2df1e3 to 5ddf6c4 Compare November 13, 2025 17:33

Rodrigodd requested a review from JakeStanger November 13, 2025 17:49

JakeStanger requested changes Nov 19, 2025

View reviewed changes

docs/modules/Network-Manager.md Outdated Show resolved Hide resolved

src/modules/networkmanager.rs Outdated Show resolved Hide resolved

Rodrigodd added 3 commits November 19, 2025 15:21

fix: keep module_impl!("network_manager");

849d620

refactor: alias tokio::sync::RwLock to AsyncRwLock

b45ddb5

fix: fix example in docs

c6ed09a

Rodrigodd force-pushed the feat-network-manager-rework branch from 275282e to c6ed09a Compare November 19, 2025 18:23

Rodrigodd requested a review from JakeStanger November 19, 2025 18:35

JakeStanger requested changes Nov 20, 2025

View reviewed changes

docs/modules/Network-Manager.md Outdated Show resolved Hide resolved

docs/modules/Network-Manager.md Outdated Show resolved Hide resolved

src/clients/networkmanager/mod.rs Outdated Show resolved Hide resolved

src/clients/networkmanager/mod.rs Outdated Show resolved Hide resolved

Rodrigodd added 3 commits November 20, 2025 13:10

style: don't prefix used variable with underscore

842aa82

fix: fix typo in log

bcd32ca

doc: replace networkmanager with network_manager

9bb55cf

Rodrigodd requested a review from JakeStanger November 20, 2025 16:16

JakeStanger reviewed Nov 20, 2025

View reviewed changes

JakeStanger added the Z:Review Required Pull request pending review label Dec 6, 2025

JakeStanger mentioned this pull request Dec 7, 2025

wip centralised thresholds system #1274

Draft

feat: rework networkmanager module #1233

Are you sure you want to change the base?

feat: rework networkmanager module #1233

Uh oh!

Conversation

Rodrigodd commented Nov 4, 2025 • edited by JakeStanger Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshot

Uh oh!

JakeStanger commented Nov 7, 2025

Uh oh!

Rodrigodd commented Nov 8, 2025

Uh oh!

Zedfrigg commented Nov 9, 2025

Uh oh!

JakeStanger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JakeStanger Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Rodrigodd Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

JakeStanger Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JakeStanger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JakeStanger left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rodrigodd commented Nov 4, 2025 •

edited by JakeStanger

Loading