refactor: more data conversion cleanup (#1653)

* clean up some battery stuff

* dedupe battery from data conversion

* idk why we had a Value type alias

* clean up dupe load avg, and remove memory use percent from memharvest

* hmm

* nvm

src/app.rs

@@ -762,8 +762,10 @@ impl App {
                         }
                     }
                 }
-                BottomWidgetType::Battery => {
-                    if self.converted_data.battery_data.len() > 1 {
+                BottomWidgetType::Battery =>
+                {
+                    #[cfg(feature = "battery")]
+                    if self.data_collection.battery_harvest.len() > 1 {
                         if let Some(battery_widget_state) = self
                             .states
                             .battery_state
@@ -823,9 +825,11 @@ impl App {
                         }
                     }
                 }
-                BottomWidgetType::Battery => {
-                    if self.converted_data.battery_data.len() > 1 {
-                        let battery_count = self.converted_data.battery_data.len();
+                BottomWidgetType::Battery =>
+                {
+                    #[cfg(feature = "battery")]
+                    if self.data_collection.battery_harvest.len() > 1 {
+                        let battery_count = self.data_collection.battery_harvest.len();
                         if let Some(battery_widget_state) = self
                             .states
                             .battery_state
@@ -2789,6 +2793,7 @@ impl App {
                         }
                     }
                     BottomWidgetType::Battery => {
+                        #[cfg(feature = "battery")]
                         if let Some(battery_widget_state) = self
                             .states
                             .battery_state
@@ -2800,10 +2805,10 @@ impl App {
                                 {
                                     if (x >= *tlc_x && y >= *tlc_y) && (x <= *brc_x && y <= *brc_y)
                                     {
-                                        if itx >= self.converted_data.battery_data.len() {
+                                        if itx >= self.data_collection.battery_harvest.len() {
                                             // range check to keep within current data
                                             battery_widget_state.currently_selected_battery_index =
-                                                self.converted_data.battery_data.len() - 1;
+                                                self.data_collection.battery_harvest.len() - 1;
                                         } else {
                                             battery_widget_state.currently_selected_battery_index =
                                                 itx;

src/app/data_farmer.rs

@@ -28,22 +28,19 @@ use crate::{
     dec_bytes_per_second_string,
 };
 
-pub type Value = f64;
-
 #[derive(Debug, Default, Clone)]
 pub struct TimedData {
-    pub rx_data: Value,
-    pub tx_data: Value,
-    pub cpu_data: Vec<Value>,
-    pub load_avg_data: [f32; 3],
-    pub mem_data: Option<Value>,
+    pub rx_data: f64,
+    pub tx_data: f64,
+    pub cpu_data: Vec<f64>,
+    pub mem_data: Option<f64>,
     #[cfg(not(target_os = "windows"))]
-    pub cache_data: Option<Value>,
-    pub swap_data: Option<Value>,
+    pub cache_data: Option<f64>,
+    pub swap_data: Option<f64>,
     #[cfg(feature = "zfs")]
-    pub arc_data: Option<Value>,
+    pub arc_data: Option<f64>,
     #[cfg(feature = "gpu")]
-    pub gpu_data: Vec<Option<Value>>,
+    pub gpu_data: Vec<Option<f64>>,
 }
 
 #[derive(Clone, Debug, Default)]
@@ -246,7 +243,7 @@ impl DataCollection {
 
         // Load average
         if let Some(load_avg) = harvested_data.load_avg {
-            self.eat_load_avg(load_avg, &mut new_entry);
+            self.eat_load_avg(load_avg);
         }
 
         // Temp
@@ -282,11 +279,8 @@ impl DataCollection {
     fn eat_memory_and_swap(
         &mut self, memory: memory::MemHarvest, swap: memory::MemHarvest, new_entry: &mut TimedData,
     ) {
-        // Memory
-        new_entry.mem_data = memory.use_percent;
-
-        // Swap
-        new_entry.swap_data = swap.use_percent;
+        new_entry.mem_data = memory.checked_percent();
+        new_entry.swap_data = swap.checked_percent();
 
         // In addition copy over latest data for easy reference
         self.memory_harvest = memory;
@@ -295,10 +289,7 @@ impl DataCollection {
 
     #[cfg(not(target_os = "windows"))]
     fn eat_cache(&mut self, cache: memory::MemHarvest, new_entry: &mut TimedData) {
-        // Cache and buffer memory
-        new_entry.cache_data = cache.use_percent;
-
-        // In addition copy over latest data for easy reference
+        new_entry.cache_data = cache.checked_percent();
         self.cache_harvest = cache;
     }
 
@@ -327,9 +318,7 @@ impl DataCollection {
         self.cpu_harvest = cpu;
     }
 
-    fn eat_load_avg(&mut self, load_avg: cpu::LoadAvgHarvest, new_entry: &mut TimedData) {
-        new_entry.load_avg_data = load_avg;
-
+    fn eat_load_avg(&mut self, load_avg: cpu::LoadAvgHarvest) {
         self.load_avg_harvest = load_avg;
     }
 
@@ -457,7 +446,7 @@ impl DataCollection {
 
     #[cfg(feature = "zfs")]
     fn eat_arc(&mut self, arc: memory::MemHarvest, new_entry: &mut TimedData) {
-        new_entry.arc_data = arc.use_percent;
+        new_entry.arc_data = arc.checked_percent();
         self.arc_harvest = arc;
     }
 
@@ -467,7 +456,7 @@ impl DataCollection {
         // within the local copy of gpu_harvest. Since it's all sequential
         // it probably doesn't matter anyways.
         gpu.iter().for_each(|data| {
-            new_entry.gpu_data.push(data.1.use_percent);
+            new_entry.gpu_data.push(data.1.checked_percent());
         });
         self.gpu_harvest = gpu;
     }

src/canvas.rs

@@ -335,7 +335,9 @@ impl Painter {
 
                         self.draw_process(f, app_state, rect[0], widget_id);
                     }
-                    Battery => {
+                    Battery =>
+                    {
+                        #[cfg(feature = "battery")]
                         self.draw_battery(f, app_state, rect[0], app_state.current_widget.widget_id)
                     }
                     _ => {}
@@ -445,7 +447,9 @@ impl Painter {
                             Temp => {
                                 self.draw_temp_table(f, app_state, vertical_chunks[3], widget_id)
                             }
-                            Battery => {
+                            Battery =>
+                            {
+                                #[cfg(feature = "battery")]
                                 self.draw_battery(f, app_state, vertical_chunks[3], widget_id)
                             }
                             _ => {}
@@ -722,7 +726,11 @@ impl Painter {
                     Temp => self.draw_temp_table(f, app_state, *draw_loc, widget.widget_id),
                     Disk => self.draw_disk_table(f, app_state, *draw_loc, widget.widget_id),
                     Proc => self.draw_process(f, app_state, *draw_loc, widget.widget_id),
-                    Battery => self.draw_battery(f, app_state, *draw_loc, widget.widget_id),
+                    Battery =>
+                    {
+                        #[cfg(feature = "battery")]
+                        self.draw_battery(f, app_state, *draw_loc, widget.widget_id)
+                    }
                     _ => {}
                 }
             }

src/canvas/drawing_utils.rs

@@ -1,4 +1,4 @@
-use std::{cmp::min, time::Instant};
+use std::time::Instant;
 
 use tui::{
     layout::Rect,
@@ -7,14 +7,6 @@ use tui::{
 
 use super::SIDE_BORDERS;
 
-/// Calculate how many bars are to be drawn within basic mode's components.
-pub fn calculate_basic_use_bars(use_percentage: f64, num_bars_available: usize) -> usize {
-    min(
-        (num_bars_available as f64 * use_percentage / 100.0).round() as usize,
-        num_bars_available,
-    )
-}
-
 /// Determine whether a graph x-label should be hidden.
 pub fn should_hide_x_label(
     always_hide_time: bool, autohide_time: bool, timer: &mut Option<Instant>, draw_loc: Rect,
@@ -64,20 +56,6 @@ mod test {
 
     use super::*;
 
-    #[test]
-    fn test_calculate_basic_use_bars() {
-        // Testing various breakpoints and edge cases.
-        assert_eq!(calculate_basic_use_bars(0.0, 15), 0);
-        assert_eq!(calculate_basic_use_bars(1.0, 15), 0);
-        assert_eq!(calculate_basic_use_bars(5.0, 15), 1);
-        assert_eq!(calculate_basic_use_bars(10.0, 15), 2);
-        assert_eq!(calculate_basic_use_bars(40.0, 15), 6);
-        assert_eq!(calculate_basic_use_bars(45.0, 15), 7);
-        assert_eq!(calculate_basic_use_bars(50.0, 15), 8);
-        assert_eq!(calculate_basic_use_bars(100.0, 15), 15);
-        assert_eq!(calculate_basic_use_bars(150.0, 15), 15);
-    }
-
     #[test]
     fn test_should_hide_x_label() {
         use std::time::{Duration, Instant};

src/canvas/widgets.rs

@@ -1,4 +1,3 @@
-pub mod battery_display;
 pub mod cpu_basic;
 pub mod cpu_graph;
 pub mod disk_table;
@@ -8,3 +7,6 @@ pub mod network_basic;
 pub mod network_graph;
 pub mod process_table;
 pub mod temperature_table;
+
+#[cfg(feature = "battery")]
+pub mod battery_display;

src/canvas/widgets/battery_display.rs

@@ -1,3 +1,5 @@
+use std::cmp::min;
+
 use tui::{
     layout::{Constraint, Direction, Layout, Rect},
     text::{Line, Span},
@@ -8,14 +10,19 @@ use unicode_width::UnicodeWidthStr;
 
 use crate::{
     app::App,
-    canvas::{
-        drawing_utils::{calculate_basic_use_bars, widget_block},
-        Painter,
-    },
+    canvas::{drawing_utils::widget_block, Painter},
     constants::*,
     data_collection::batteries::BatteryState,
 };
 
+/// Calculate how many bars are to be drawn within basic mode's components.
+fn calculate_basic_use_bars(use_percentage: f64, num_bars_available: usize) -> usize {
+    min(
+        (num_bars_available as f64 * use_percentage / 100.0).round() as usize,
+        num_bars_available,
+    )
+}
+
 impl Painter {
     pub fn draw_battery(
         &self, f: &mut Frame<'_>, app_state: &mut App, draw_loc: Rect, widget_id: u64,
@@ -58,10 +65,10 @@ impl Painter {
                 block
             };
 
-            if app_state.converted_data.battery_data.len() > 1 {
+            if app_state.data_collection.battery_harvest.len() > 1 {
                 let battery_names = app_state
-                    .converted_data
-                    .battery_data
+                    .data_collection
+                    .battery_harvest
                     .iter()
                     .enumerate()
                     .map(|(itx, _)| format!("Battery {itx}"))
@@ -118,8 +125,8 @@ impl Painter {
                 .split(draw_loc)[0];
 
             if let Some(battery_details) = app_state
-                .converted_data
-                .battery_data
+                .data_collection
+                .battery_harvest
                 .get(battery_widget_state.currently_selected_battery_index)
             {
                 let full_width = draw_loc.width.saturating_sub(2);
@@ -195,7 +202,7 @@ impl Painter {
 
                 battery_rows.push(Row::new(["Health", &health]).style(self.styles.text_style));
 
-                let header = if app_state.converted_data.battery_data.len() > 1 {
+                let header = if app_state.data_collection.battery_harvest.len() > 1 {
                     Row::new([""]).bottom_margin(table_gap)
                 } else {
                     Row::default()
@@ -314,4 +321,18 @@ mod tests {
         assert_eq!(short_time(3601), "1h 0m 1s".to_string());
         assert_eq!(short_time(3661), "1h 1m 1s".to_string());
     }
+
+    #[test]
+    fn test_calculate_basic_use_bars() {
+        // Testing various breakpoints and edge cases.
+        assert_eq!(calculate_basic_use_bars(0.0, 15), 0);
+        assert_eq!(calculate_basic_use_bars(1.0, 15), 0);
+        assert_eq!(calculate_basic_use_bars(5.0, 15), 1);
+        assert_eq!(calculate_basic_use_bars(10.0, 15), 2);
+        assert_eq!(calculate_basic_use_bars(40.0, 15), 6);
+        assert_eq!(calculate_basic_use_bars(45.0, 15), 7);
+        assert_eq!(calculate_basic_use_bars(50.0, 15), 8);
+        assert_eq!(calculate_basic_use_bars(100.0, 15), 15);
+        assert_eq!(calculate_basic_use_bars(150.0, 15), 15);
+    }
 }

src/data_collection.rs

@@ -6,6 +6,7 @@ pub mod nvidia;
 #[cfg(all(target_os = "linux", feature = "gpu"))]
 pub mod amd;
 
+#[cfg(feature = "battery")]
 pub mod batteries;
 pub mod cpu;
 pub mod disks;

src/data_collection/amd.rs

@@ -425,11 +425,6 @@ pub fn get_amd_vecs(
                     MemHarvest {
                         total_bytes: mem.total,
                         used_bytes: mem.used,
-                        use_percent: if mem.total == 0 {
-                            None
-                        } else {
-                            Some(mem.used as f64 / mem.total as f64 * 100.0)
-                        },
                     },
                 ));
             }

src/data_collection/batteries.rs

@@ -10,14 +10,13 @@
 //!
 //! For more information, refer to the [starship_battery](https://github.com/starship/rust-battery) repo/docs.
 
-#[cfg(feature = "battery")]
 use starship_battery::{
     units::{power::watt, ratio::percent, time::second},
     Battery, Manager, State,
 };
 
 /// Battery state.
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Clone)]
 pub enum BatteryState {
     Charging {
         /// Time to full in seconds.
@@ -29,7 +28,6 @@ pub enum BatteryState {
     },
     Empty,
     Full,
-    #[default]
     Unknown,
 }
 
@@ -68,7 +66,6 @@ impl BatteryData {
     }
 }
 
-#[cfg(feature = "battery")]
 pub fn refresh_batteries(manager: &Manager, batteries: &mut [Battery]) -> Vec<BatteryData> {
     batteries
         .iter_mut()

src/data_collection/memory.rs

@@ -19,6 +19,18 @@ pub mod arc;
 pub struct MemHarvest {
     pub used_bytes: u64,
     pub total_bytes: u64,
-    pub use_percent: Option<f64>, /* TODO: Might be find to just make this an f64, and any
-                                   * consumer checks NaN. */
+}
+
+impl MemHarvest {
+    /// Return the use percentage. If the total bytes is 0, then this returns `None`.
+    pub fn checked_percent(&self) -> Option<f64> {
+        let used = self.used_bytes as f64;
+        let total = self.total_bytes as f64;
+
+        if total == 0.0 {
+            None
+        } else {
+            Some(used / total * 100.0)
+        }
+    }
 }

src/data_collection/memory/arc.rs

@@ -66,10 +66,5 @@ pub(crate) fn get_arc_usage() -> Option<MemHarvest> {
     Some(MemHarvest {
         total_bytes: mem_total,
         used_bytes: mem_used,
-        use_percent: if mem_total == 0 {
-            None
-        } else {
-            Some(mem_used as f64 / mem_total as f64 * 100.0)
-        },
     })
 }