From 226b32ce6a674c32ee94da0361e82a98d9c01ecb Mon Sep 17 00:00:00 2001 From: Reinout Meliesie Date: Tue, 2 Sep 2025 20:42:26 +0200 Subject: [PATCH] fix(networkmanager): support late module initialisation For example when a second monitor is connected while Ironbar is already running. --- src/clients/networkmanager/event.rs | 13 ++- src/clients/networkmanager/mod.rs | 123 ++++++++++++++++++++-------- src/modules/networkmanager.rs | 65 ++++++++------- 3 files changed, 135 insertions(+), 66 deletions(-) diff --git a/src/clients/networkmanager/event.rs b/src/clients/networkmanager/event.rs index 4963f6e..034d6e6 100644 --- a/src/clients/networkmanager/event.rs +++ b/src/clients/networkmanager/event.rs @@ -1,13 +1,18 @@ use crate::clients::networkmanager::dbus::{DeviceState, DeviceType}; #[derive(Debug, Clone)] -pub enum Event { - DeviceAdded { - interface: String, - }, +pub enum ClientToModuleEvent { DeviceStateChanged { interface: String, r#type: DeviceType, state: DeviceState, }, + DeviceRemoved { + interface: String, + }, +} + +#[derive(Debug, Clone)] +pub enum ModuleToClientEvent { + NewController, } diff --git a/src/clients/networkmanager/mod.rs b/src/clients/networkmanager/mod.rs index 94cb82d..52af9e7 100644 --- a/src/clients/networkmanager/mod.rs +++ b/src/clients/networkmanager/mod.rs @@ -3,13 +3,13 @@ use color_eyre::eyre::Ok; use futures_lite::StreamExt; use std::collections::HashSet; use std::sync::Arc; -use tokio::sync::broadcast; +use tokio::sync::{RwLock, broadcast}; use zbus::Connection; use zbus::zvariant::{ObjectPath, Str}; use crate::clients::ClientResult; use crate::clients::networkmanager::dbus::{DbusProxy, DeviceDbusProxy}; -use crate::clients::networkmanager::event::Event; +use crate::clients::networkmanager::event::{ClientToModuleEvent, ModuleToClientEvent}; use crate::{register_fallible_client, spawn}; pub mod dbus; @@ -17,47 +17,104 @@ pub mod event; #[derive(Debug)] pub struct Client { - tx: broadcast::Sender, + controller_sender: broadcast::Sender, + sender: broadcast::Sender, } impl Client { fn new() -> Result { - let (tx, _) = broadcast::channel(64); - Ok(Client { tx }) + let (controller_sender, _) = broadcast::channel(64); + let (sender, _) = broadcast::channel(8); + Ok(Client { + controller_sender, + sender, + }) } fn run(&self) -> Result<()> { - let tx = self.tx.clone(); - spawn(async move { - let dbus_connection = Connection::system().await?; - let root = DbusProxy::new(&dbus_connection).await?; + let devices: &'static _ = Box::leak(Box::new(RwLock::new(HashSet::::new()))); - let mut devices = HashSet::new(); + { + let controller_sender = self.controller_sender.clone(); + spawn(async move { + let dbus_connection = Connection::system().await?; + let root = DbusProxy::new(&dbus_connection).await?; - let mut devices_changes = root.receive_all_devices_changed().await; - while let Some(devices_change) = devices_changes.next().await { - // The new list of devices from dbus, not to be confused with the added devices below - let new_devices = HashSet::from_iter(devices_change.get().await?); + let mut devices_changes = root.receive_all_devices_changed().await; + while let Some(devices_change) = devices_changes.next().await { + // The new list of devices from dbus, not to be confused with the added devices below + let new_devices = devices_change + .get() + .await? + .iter() + .map(ObjectPath::to_owned) + .collect::>(); - let added_devices = new_devices.difference(&devices); - for added_device in added_devices { - spawn(watch_device(added_device.to_owned(), tx.clone())); + // We create a local clone here to avoid holding the lock for too long + let devices_snapshot = devices.read().await.clone(); + + let added_devices = new_devices.difference(&devices_snapshot); + for added_device in added_devices { + spawn(watch_device( + added_device.to_owned(), + controller_sender.clone(), + )); + } + + let _removed_devices = devices_snapshot.difference(&new_devices); + // TODO: Cook up some way to notify closures for removed devices to exit + + *devices.write().await = new_devices; } - let _removed_devices = devices.difference(&new_devices); - // TODO: Cook up some way to notify closures for removed devices to exit + Ok(()) + }); + } - devices = new_devices; - } + { + let controller_sender = self.controller_sender.clone(); + let mut receiver = self.sender.subscribe(); + spawn(async move { + while let Result::Ok(event) = receiver.recv().await { + match event { + ModuleToClientEvent::NewController => { + // We create a local clone here to avoid holding the lock for too long + let devices_snapshot = devices.read().await.clone(); - Ok(()) - }); + for device_path in devices_snapshot { + let dbus_connection = Connection::system().await?; + let device = + DeviceDbusProxy::new(&dbus_connection, device_path).await?; + + // TODO: Create DeviceDbusProxy -> DeviceStateChanged function and use it in the watcher as well + let interface = device.interface().await?.to_string(); + let r#type = device.device_type().await?; + let state = device.state().await?; + controller_sender.send( + ClientToModuleEvent::DeviceStateChanged { + interface, + r#type, + state, + }, + )?; + } + } + } + } + + Ok(()) + }); + } Ok(()) } - pub fn subscribe(&self) -> broadcast::Receiver { - self.tx.subscribe() + pub fn subscribe(&self) -> broadcast::Receiver { + self.controller_sender.subscribe() + } + + pub fn get_sender(&self) -> broadcast::Sender { + self.sender.clone() } } @@ -67,19 +124,19 @@ pub fn create_client() -> ClientResult { Ok(client) } -async fn watch_device(device_path: ObjectPath<'_>, tx: broadcast::Sender) -> Result<()> { +async fn watch_device( + device_path: ObjectPath<'_>, + controller_sender: broadcast::Sender, +) -> Result<()> { let dbus_connection = Connection::system().await?; let device = DeviceDbusProxy::new(&dbus_connection, device_path.to_owned()).await?; let interface = device.interface().await?; - tx.send(Event::DeviceAdded { - interface: interface.to_string(), - })?; spawn(watch_device_state( device_path.to_owned(), interface.to_owned(), - tx.clone(), + controller_sender.clone(), )); Ok(()) @@ -88,7 +145,7 @@ async fn watch_device(device_path: ObjectPath<'_>, tx: broadcast::Sender) async fn watch_device_state( device_path: ObjectPath<'_>, interface: Str<'_>, - tx: broadcast::Sender, + controller_sender: broadcast::Sender, ) -> Result<()> { let dbus_connection = Connection::system().await?; let device = DeviceDbusProxy::new(&dbus_connection, &device_path).await?; @@ -96,7 +153,7 @@ async fn watch_device_state( // Send an event communicating the initial state let state = device.state().await?; - tx.send(Event::DeviceStateChanged { + controller_sender.send(ClientToModuleEvent::DeviceStateChanged { interface: interface.to_string(), r#type: r#type.clone(), state, @@ -105,7 +162,7 @@ async fn watch_device_state( let mut state_changes = device.receive_state_changed().await; while let Some(state_change) = state_changes.next().await { let state = state_change.get().await?; - tx.send(Event::DeviceStateChanged { + controller_sender.send(ClientToModuleEvent::DeviceStateChanged { interface: interface.to_string(), r#type: r#type.clone(), state, diff --git a/src/modules/networkmanager.rs b/src/modules/networkmanager.rs index ea8b8d9..4f464f0 100644 --- a/src/modules/networkmanager.rs +++ b/src/modules/networkmanager.rs @@ -1,6 +1,6 @@ use crate::clients::networkmanager::Client; use crate::clients::networkmanager::dbus::{DeviceState, DeviceType}; -use crate::clients::networkmanager::event::Event; +use crate::clients::networkmanager::event::{ClientToModuleEvent, ModuleToClientEvent}; use crate::config::CommonConfig; use crate::gtk_helpers::IronbarGtkExt; use crate::image::Provider; @@ -29,7 +29,7 @@ const fn default_icon_size() -> i32 { } impl Module for NetworkManagerModule { - type SendMessage = Event; + type SendMessage = ClientToModuleEvent; type ReceiveMessage = (); module_impl!("network_manager"); @@ -37,19 +37,24 @@ impl Module for NetworkManagerModule { fn spawn_controller( &self, _info: &ModuleInfo, - context: &WidgetContext, - _rx: mpsc::Receiver<()>, + context: &WidgetContext, + _widget_receiver: mpsc::Receiver<()>, ) -> Result<()> { let client = context.try_client::()?; // Should we be using context.tx with ModuleUpdateEvent::Update instead? - let tx = context.update_tx.clone(); - // Must be done here synchronously to avoid race condition - let mut client_rx = client.subscribe(); - spawn(async move { - while let Result::Ok(event) = client_rx.recv().await { - tx.send(event)?; - } + let widget_sender = context.update_tx.clone(); + // Must be done here otherwise we miss the response to our `NewController` event + let mut client_receiver = client.subscribe(); + + client + .get_sender() + .send(ModuleToClientEvent::NewController)?; + + spawn(async move { + while let Result::Ok(event) = client_receiver.recv().await { + widget_sender.send(event)?; + } Ok(()) }); @@ -58,16 +63,17 @@ impl Module for NetworkManagerModule { fn into_widget( self, - context: WidgetContext, + context: WidgetContext, _info: &ModuleInfo, ) -> Result> { + // Must be done here otherwise we miss the response to our `NewController` event + let receiver = context.subscribe(); + let container = gtk::Box::new(Orientation::Horizontal, 0); - // Must be done here synchronously to avoid race condition - let rx = context.subscribe(); // We cannot use recv_glib_async here because the lifetimes don't work out spawn_future_local(handle_update_events( - rx, + receiver, container.clone(), self.icon_size, context.ironbar.image_provider(), @@ -78,29 +84,27 @@ impl Module for NetworkManagerModule { } async fn handle_update_events( - mut rx: broadcast::Receiver, + mut receiver: broadcast::Receiver, container: gtk::Box, icon_size: i32, image_provider: Provider, -) { +) -> Result<()> { let mut icons = HashMap::::new(); - while let Result::Ok(event) = rx.recv().await { + while let Result::Ok(event) = receiver.recv().await { match event { - Event::DeviceAdded { interface, .. } => { - let icon = Image::new(); - icon.add_class("icon"); - container.add(&icon); - icons.insert(interface, icon); - } - Event::DeviceStateChanged { + ClientToModuleEvent::DeviceStateChanged { interface, r#type, state, } => { - let icon = icons - .get(&interface) - .expect("the icon for the interface to be present"); + let icon: &_ = icons.entry(interface).or_insert_with(|| { + let icon = Image::new(); + icon.add_class("icon"); + container.add(&icon); + icon + }); + // TODO: Make this configurable at runtime let icon_name = get_icon_for_device_state(&r#type, &state); match icon_name { @@ -115,8 +119,11 @@ async fn handle_update_events( } } } - }; + ClientToModuleEvent::DeviceRemoved { .. } => {} + } } + + Ok(()) } fn get_icon_for_device_state(r#type: &DeviceType, state: &DeviceState) -> Option<&'static str> {