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)
-        },
     })
 }

src/data_collection/memory/sysinfo.rs

@@ -12,11 +12,6 @@ pub(crate) fn get_ram_usage(sys: &System) -> Option<MemHarvest> {
     Some(MemHarvest {
         used_bytes: mem_used,
         total_bytes: mem_total,
-        use_percent: if mem_total == 0 {
-            None
-        } else {
-            Some(mem_used as f64 / mem_total as f64 * 100.0)
-        },
     })
 }
 
@@ -28,11 +23,6 @@ pub(crate) fn get_swap_usage(sys: &System) -> Option<MemHarvest> {
     Some(MemHarvest {
         used_bytes: mem_used,
         total_bytes: mem_total,
-        use_percent: if mem_total == 0 {
-            None
-        } else {
-            Some(mem_used as f64 / mem_total as f64 * 100.0)
-        },
     })
 }
 
@@ -51,10 +41,5 @@ pub(crate) fn get_cache_usage(sys: &System) -> 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)
-        },
     })
 }

src/data_collection/nvidia.rs

@@ -43,11 +43,6 @@ pub fn get_nvidia_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_conversion.rs

@@ -8,13 +8,18 @@ use std::borrow::Cow;
 use crate::{
     app::{data_farmer::DataCollection, AxisScaling},
     canvas::components::time_chart::Point,
-    data_collection::{
-        batteries::BatteryData, cpu::CpuDataType, memory::MemHarvest, temperature::TemperatureType,
-    },
+    data_collection::{cpu::CpuDataType, memory::MemHarvest, temperature::TemperatureType},
     utils::{data_prefixes::*, data_units::DataUnit},
     widgets::{DiskWidgetData, TempWidgetData},
 };
 
+// TODO: [NETWORKING] add min/max/mean of each
+// min_rx : f64,
+// max_rx : f64,
+// mean_rx: f64,
+// min_tx: f64,
+// max_tx: f64,
+// mean_tx: f64,
 #[derive(Default, Debug)]
 pub struct ConvertedNetworkData {
     pub rx: Vec<Point>,
@@ -23,13 +28,6 @@ pub struct ConvertedNetworkData {
     pub tx_display: String,
     pub total_rx_display: Option<String>,
     pub total_tx_display: Option<String>,
-    // TODO: [NETWORKING] add min/max/mean of each
-    // min_rx : f64,
-    // max_rx : f64,
-    // mean_rx: f64,
-    // min_tx: f64,
-    // max_tx: f64,
-    // mean_tx: f64,
 }
 
 #[derive(Clone, Debug)]
@@ -58,6 +56,10 @@ pub struct ConvertedData {
     pub swap_labels: Option<(String, String)>,
 
     // TODO: Switch this and all data points over to a better data structure.
+    //
+    // We can dedupe the f64 for time by storing it alongside this data structure.
+    // We can also just store everything via an references and iterators to avoid
+    // duplicating data, I guess.
     pub mem_data: Vec<Point>,
     #[cfg(not(target_os = "windows"))]
     pub cache_data: Vec<Point>,
@@ -74,7 +76,6 @@ pub struct ConvertedData {
     pub load_avg_data: [f32; 3],
     pub cpu_data: Vec<CpuWidgetData>,
 
-    pub battery_data: Vec<BatteryData>,
     pub disk_data: Vec<DiskWidgetData>,
     pub temp_data: Vec<TempWidgetData>,
 }
@@ -268,8 +269,9 @@ fn get_binary_unit_and_denominator(bytes: u64) -> (&'static str, f64) {
 /// Returns the unit type and denominator for given total amount of memory in
 /// kibibytes.
 pub fn convert_mem_label(harvest: &MemHarvest) -> Option<(String, String)> {
-    if harvest.total_bytes > 0 {
-        Some((format!("{:3.0}%", harvest.use_percent.unwrap_or(0.0)), {
+    (harvest.total_bytes > 0).then(|| {
+        let percentage = harvest.used_bytes as f64 / harvest.total_bytes as f64 * 100.0;
+        (format!("{percentage:3.0}%"), {
             let (unit, denominator) = get_binary_unit_and_denominator(harvest.total_bytes);
 
             format!(
@@ -279,10 +281,8 @@ pub fn convert_mem_label(harvest: &MemHarvest) -> Option<(String, String)> {
                 (harvest.total_bytes as f64 / denominator),
                 unit
             )
-        }))
-    } else {
-        None
-    }
+        })
+    })
 }
 
 pub fn get_network_points(
@@ -549,27 +549,32 @@ pub fn convert_gpu_data(current_data: &DataCollection) -> Option<Vec<ConvertedGp
         .gpu_harvest
         .iter()
         .zip(point_vec)
-        .map(|(gpu, points)| {
-            let short_name = {
-                let last_words = gpu.0.split_whitespace().rev().take(2).collect::<Vec<_>>();
-                let short_name = format!("{} {}", last_words[1], last_words[0]);
-                short_name
-            };
+        .filter_map(|(gpu, points)| {
+            (gpu.1.total_bytes > 0).then(|| {
+                let short_name = {
+                    let last_words = gpu.0.split_whitespace().rev().take(2).collect::<Vec<_>>();
+                    let short_name = format!("{} {}", last_words[1], last_words[0]);
+                    short_name
+                };
 
-            ConvertedGpuData {
-                name: short_name,
-                points,
-                mem_percent: format!("{:3.0}%", gpu.1.use_percent.unwrap_or(0.0)),
-                mem_total: {
-                    let (unit, denominator) = get_binary_unit_and_denominator(gpu.1.total_bytes);
+                let percent = gpu.1.used_bytes as f64 / gpu.1.total_bytes as f64 * 100.0;
 
-                    format!(
-                        "   {:.1}{unit}/{:.1}{unit}",
-                        gpu.1.used_bytes as f64 / denominator,
-                        (gpu.1.total_bytes as f64 / denominator),
-                    )
-                },
-            }
+                ConvertedGpuData {
+                    name: short_name,
+                    points,
+                    mem_percent: format!("{percent:3.0}%"),
+                    mem_total: {
+                        let (unit, denominator) =
+                            get_binary_unit_and_denominator(gpu.1.total_bytes);
+
+                        format!(
+                            "   {:.1}{unit}/{:.1}{unit}",
+                            gpu.1.used_bytes as f64 / denominator,
+                            (gpu.1.total_bytes as f64 / denominator),
+                        )
+                    },
+                }
+            })
         })
         .collect::<Vec<ConvertedGpuData>>();
 

src/lib.rs

@@ -509,14 +509,6 @@ pub fn start_bottom() -> anyhow::Result<()> {
                             }
                         }
 
-                        #[cfg(feature = "battery")]
-                        {
-                            if app.used_widgets.use_battery {
-                                app.converted_data.battery_data =
-                                    app.data_collection.battery_harvest.clone();
-                            }
-                        }
-
                         app.update_data();
                         try_drawing(&mut terminal, &mut app, &mut painter)?;
                     }