Enhance error handling and refactor methods in LifxManager and MantleApp

src/action.rs

@@ -41,7 +41,9 @@ impl UserAction {
     pub fn execute(&self, lifx_manager: LifxManager, device: DeviceInfo) {
         match self {
             UserAction::Refresh => {
-                lifx_manager.refresh();
+                if let Err(e) = lifx_manager.refresh() {
+                    log::error!("Failed to refresh: {}", e);
+                }
             }
             UserAction::TogglePower => match device {
                 DeviceInfo::Group(group_info) => {

src/app.rs

@@ -141,7 +141,7 @@ impl MantleApp {
                         ui.label(RichText::new(s).size(16.0).strong());
                     }
                 }
-                Some(self.mgr.avg_group_color(group, bulbs))
+                Some(self.mgr.get_avg_group_color(group, bulbs))
             }
         };
 
@@ -231,7 +231,9 @@ impl MantleApp {
             ui.ctx().send_viewport_cmd(egui::ViewportCommand::Close);
         }
         if ui.input_mut(|i| i.consume_shortcut(&refresh_shortcut)) {
-            self.mgr.refresh();
+            if let Err(e) = self.mgr.refresh() {
+                log::error!("Error refreshing manager: {}", e);
+            }
         }
 
         ui.menu_button("File", |ui| {
@@ -243,7 +245,9 @@ impl MantleApp {
                 )
                 .clicked()
             {
-                self.mgr.refresh();
+                if let Err(e) = self.mgr.refresh() {
+                    log::error!("Error refreshing manager: {}", e);
+                }
                 ui.close_menu();
             }
             if ui.add(egui::Button::new("Settings")).clicked() {
@@ -345,7 +349,9 @@ impl eframe::App for MantleApp {
         {
             self.mgr.discover().expect("Failed to discover bulbs");
         }
-        self.mgr.refresh();
+        if let Err(e) = self.mgr.refresh() {
+            log::error!("Error refreshing manager: {}", e);
+        }
         self.update_ui(_ctx);
         self.show_about_window(_ctx);
         self.settings_ui(_ctx);

src/device_manager.rs

@@ -212,8 +212,9 @@ impl LifxManager {
         }
     }
 
-    pub fn discover(&mut self) -> Result<(), failure::Error> {
+    pub fn discover(&mut self) -> Result<usize, failure::Error> {
         log::debug!("Doing discovery");
+        let mut count = 0;
 
         let opts = BuildOptions {
             source: self.source,
@@ -234,23 +235,25 @@ impl LifxManager {
                 let addr = SocketAddr::new(IpAddr::V4(bcast), 56700);
                 log::debug!("Discovering bulbs on LAN {:?}", addr);
                 self.sock.send_to(&bytes, addr)?;
+                count += 1;
             }
         }
 
         self.last_discovery = Instant::now();
 
-        Ok(())
+        Ok(count)
     }
 
-    pub fn refresh(&self) {
+    pub fn refresh(&self) -> Result<usize, failure::Error> {
+        let mut count = 0;
         if let Ok(mut bulbs) = self.bulbs.lock() {
             let bulbs = bulbs.values_mut();
             for bulb in bulbs {
-                if let Err(e) = bulb.query_for_missing_info(&self.sock) {
-                    log::error!("Error querying bulb: {}", e);
-                }
+                bulb.query_for_missing_info(&self.sock)?;
+                count += 1;
             }
         }
+        Ok(count)
     }
 
     fn send_message(&self, bulb: &&BulbInfo, message: Message) -> Result<usize, std::io::Error> {
@@ -318,14 +321,15 @@ impl LifxManager {
         bulbs: &MutexGuard<HashMap<u64, BulbInfo>>,
         duration: Option<u32>,
     ) -> Result<usize, std::io::Error> {
+        let mut total = 0;
         let bulbs = group.get_bulbs(bulbs);
         for bulb in bulbs {
-            self.set_color(&bulb, color, duration)?;
+            total += self.set_color(&bulb, color, duration)?;
         }
-        Ok(0)
+        Ok(total)
     }
 
-    pub fn avg_group_color(
+    pub fn get_avg_group_color(
         &self,
         group: &GroupInfo,
         bulbs: &MutexGuard<HashMap<u64, BulbInfo>>,
@@ -361,6 +365,7 @@ impl LifxManager {
         device_id: u64,
         avg_color: HSBK,
     ) -> Result<usize, std::io::Error> {
+        // check for bulb
         if let Ok(bulbs) = self.bulbs.lock() {
             if let Some(bulb) = bulbs.get(&device_id) {
                 return self.set_color(&bulb, avg_color, None);
@@ -379,7 +384,8 @@ impl LifxManager {
         Ok(0)
     }
 
-    pub fn toggle_power(&self) {
+    pub fn toggle_power(&self) -> Result<usize, std::io::Error> {
+        let mut total = 0;
         if let Ok(bulbs) = self.bulbs.lock() {
             let bulbs = bulbs.values();
             for bulb in bulbs {
@@ -388,9 +394,10 @@ impl LifxManager {
                 } else {
                     u16::MAX
                 };
-                let _ = self.set_power(&bulb, pwr);
+                total += self.set_power(&bulb, pwr)?;
             }
         }
+        Ok(total)
     }
 
     pub fn set_color_field(
@@ -450,10 +457,11 @@ impl LifxManager {
         value: u16,
         bulbs: &MutexGuard<'_, HashMap<u64, BulbInfo>>,
     ) -> Result<usize, std::io::Error> {
+        let mut total = 0;
         let bulbs = group_info.get_bulbs(bulbs);
         for bulb in bulbs {
-            self.set_color_field(&bulb, field, value)?;
+            total += self.set_color_field(&bulb, field, value)?;
         }
-        Ok(0)
+        Ok(total)
     }
 }

src/shortcut.rs

@@ -126,15 +126,6 @@ impl ShortcutManager {
         }
     }
 
-    pub fn get_active_shortcuts(&self) -> Vec<KeyboardShortcutAction> {
-        if let Ok(shortcuts) = self.shortcuts.lock() {
-            shortcuts.clone()
-        } else {
-            log::error!("Failed to lock shortcuts mutex");
-            Vec::new()
-        }
-    }
-
     pub fn start(&self, lifx_manager: LifxManager) -> std::thread::JoinHandle<()> {
         let input_listener = self.input_listener.clone();
         let shortcuts: Arc<Mutex<Vec<KeyboardShortcutAction>>> = Arc::clone(&self.shortcuts);
@@ -330,10 +321,5 @@ mod tests {
             UserAction::Refresh,
             device.clone(),
         );
-
-        let shortcuts = shortcut_manager.get_active_shortcuts();
-        assert_eq!(shortcuts.len(), 1);
-        assert_eq!(shortcuts[0].shortcut, shortcut);
-        assert_eq!(shortcuts[0].name, "TestAction");
     }
 }