fix(networkmanager): support late module initialisation

For example when a second monitor is connected while Ironbar is already running.

src/clients/networkmanager/event.rs

@@ -1,13 +1,18 @@
 use crate::clients::networkmanager::dbus::{DeviceState, DeviceType};
 
 #[derive(Debug, Clone)]
-pub enum Event {
+pub enum ClientToModuleEvent {
-    DeviceAdded {
-        interface: String,
-    },
     DeviceStateChanged {
         interface: String,
         r#type: DeviceType,
         state: DeviceState,
     },
+    DeviceRemoved {
+        interface: String,
+    },
+}
+
+#[derive(Debug, Clone)]
+pub enum ModuleToClientEvent {
+    NewController,
 }

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<Event>,
+    controller_sender: broadcast::Sender<ClientToModuleEvent>,
+    sender: broadcast::Sender<ModuleToClientEvent>,
 }
 
 impl Client {
     fn new() -> Result<Client> {
-        let (tx, _) = broadcast::channel(64);
+        let (controller_sender, _) = broadcast::channel(64);
-        Ok(Client { tx })
+        let (sender, _) = broadcast::channel(8);
+        Ok(Client {
+            controller_sender,
+            sender,
+        })
     }
 
     fn run(&self) -> Result<()> {
-        let tx = self.tx.clone();
+        let devices: &'static _ = Box::leak(Box::new(RwLock::new(HashSet::<ObjectPath>::new())));
-        spawn(async move {
-            let dbus_connection = Connection::system().await?;
-            let root = DbusProxy::new(&dbus_connection).await?;
 
-            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;
+                let mut devices_changes = root.receive_all_devices_changed().await;
-            while let Some(devices_change) = devices_changes.next().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
+                    // 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 new_devices = devices_change
+                        .get()
+                        .await?
+                        .iter()
+                        .map(ObjectPath::to_owned)
+                        .collect::<HashSet<_>>();
 
-                let added_devices = new_devices.difference(&devices);
+                    // We create a local clone here to avoid holding the lock for too long
-                for added_device in added_devices {
+                    let devices_snapshot = devices.read().await.clone();
-                    spawn(watch_device(added_device.to_owned(), tx.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);
+                Ok(())
-                // TODO: Cook up some way to notify closures for removed devices to exit
+            });
+        }
 
-                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<Event> {
+    pub fn subscribe(&self) -> broadcast::Receiver<ClientToModuleEvent> {
-        self.tx.subscribe()
+        self.controller_sender.subscribe()
+    }
+
+    pub fn get_sender(&self) -> broadcast::Sender<ModuleToClientEvent> {
+        self.sender.clone()
     }
 }
 
@@ -67,19 +124,19 @@ pub fn create_client() -> ClientResult<Client> {
     Ok(client)
 }
 
-async fn watch_device(device_path: ObjectPath<'_>, tx: broadcast::Sender<Event>) -> Result<()> {
+async fn watch_device(
+    device_path: ObjectPath<'_>,
+    controller_sender: broadcast::Sender<ClientToModuleEvent>,
+) -> 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<Event>)
 async fn watch_device_state(
     device_path: ObjectPath<'_>,
     interface: Str<'_>,
-    tx: broadcast::Sender<Event>,
+    controller_sender: broadcast::Sender<ClientToModuleEvent>,
 ) -> 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,

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<gtk::Box> for NetworkManagerModule {
-    type SendMessage = Event;
+    type SendMessage = ClientToModuleEvent;
     type ReceiveMessage = ();
 
     module_impl!("network_manager");
@@ -37,19 +37,24 @@ impl Module<gtk::Box> for NetworkManagerModule {
     fn spawn_controller(
         &self,
         _info: &ModuleInfo,
-        context: &WidgetContext<Event, ()>,
+        context: &WidgetContext<ClientToModuleEvent, ()>,
-        _rx: mpsc::Receiver<()>,
+        _widget_receiver: mpsc::Receiver<()>,
     ) -> Result<()> {
         let client = context.try_client::<Client>()?;
         // Should we be using context.tx with ModuleUpdateEvent::Update instead?
-        let tx = context.update_tx.clone();
+        let widget_sender = 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)?;
-            }
 
+        // 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<gtk::Box> for NetworkManagerModule {
 
     fn into_widget(
         self,
-        context: WidgetContext<Event, ()>,
+        context: WidgetContext<ClientToModuleEvent, ()>,
         _info: &ModuleInfo,
     ) -> Result<ModuleParts<gtk::Box>> {
+        // 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<gtk::Box> for NetworkManagerModule {
 }
 
 async fn handle_update_events(
-    mut rx: broadcast::Receiver<Event>,
+    mut receiver: broadcast::Receiver<ClientToModuleEvent>,
     container: gtk::Box,
     icon_size: i32,
     image_provider: Provider,
-) {
+) -> Result<()> {
     let mut icons = HashMap::<String, Image>::new();
 
-    while let Result::Ok(event) = rx.recv().await {
+    while let Result::Ok(event) = receiver.recv().await {
         match event {
-            Event::DeviceAdded { interface, .. } => {
+            ClientToModuleEvent::DeviceStateChanged {
-                let icon = Image::new();
-                icon.add_class("icon");
-                container.add(&icon);
-                icons.insert(interface, icon);
-            }
-            Event::DeviceStateChanged {
                 interface,
                 r#type,
                 state,
             } => {
-                let icon = icons
+                let icon: &_ = icons.entry(interface).or_insert_with(|| {
-                    .get(&interface)
+                    let icon = Image::new();
-                    .expect("the icon for the interface to be present");
+                    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> {