refactor: migrate network collection to sysinfo (#1041)

* refactor: migrate network collection to sysinfo

* remove net feature from heim

* comments and changelog

CHANGELOG.md

@@ -23,7 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#1025](https://github.com/ClementTsang/bottom/pull/1025): Officially support M1 macOS.
 - [#1035](https://github.com/ClementTsang/bottom/pull/1035): Migrate away from heim for CPU information.
 - [#1036](https://github.com/ClementTsang/bottom/pull/1036): Migrate away from heim for memory information; Linux
-  platforms will now try to use `MemAvailable` to determine used memory if supported.
+  platforms will also now try to use `MemAvailable` to determine used memory if supported.
+- [#1041](https://github.com/ClementTsang/bottom/pull/1041): Migrate away from heim for network information.
 
 ## Other
 

Cargo.lock

@@ -820,7 +820,6 @@ checksum = "b8a653442b9bdd11a77d3753a60443c60c4437d3acac8e6c3d4a6a9acd7cceed"
 dependencies = [
  "heim-common",
  "heim-disk",
- "heim-net",
  "heim-runtime",
  "heim-sensors",
 ]
@@ -861,21 +860,6 @@ dependencies = [
  "winapi",
 ]
 
-[[package]]
-name = "heim-net"
-version = "0.1.0-rc.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d13afa5e9b71c813c1e087bb27f51ae87d3a6d68a2bdd045bae4322dfae4948b"
-dependencies = [
- "bitflags",
- "cfg-if",
- "heim-common",
- "heim-runtime",
- "libc",
- "macaddr",
- "nix 0.19.1",
-]
-
 [[package]]
 name = "heim-runtime"
 version = "0.1.0-rc.1"
@@ -1042,12 +1026,6 @@ dependencies = [
  "cfg-if",
 ]
 
-[[package]]
-name = "macaddr"
-version = "1.0.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "baee0bbc17ce759db233beb01648088061bf678383130602a298e6998eedb2d8"
-
 [[package]]
 name = "mach"
 version = "0.3.2"

Cargo.toml

@@ -108,11 +108,11 @@ unicode-width = "0.1.10"
 libc = "0.2.124"
 
 [target.'cfg(target_os = "linux")'.dependencies]
-heim = { version = "0.1.0-rc.1", features = ["disk", "net", "sensors"] }
+heim = { version = "0.1.0-rc.1", features = ["disk", "sensors"] }
 procfs = { version = "0.15.1", default-features = false }
 
 [target.'cfg(target_os = "macos")'.dependencies]
-heim = { version = "0.1.0-rc.1", features = ["disk", "net"] }
+heim = { version = "0.1.0-rc.1", features = ["disk"] }
 mach2 = "0.4.1"
 
 [target.'cfg(target_os = "windows")'.dependencies]

src/app.rs

@@ -7,6 +7,7 @@ use std::{
 use concat_string::concat_string;
 use data_farmer::*;
 use data_harvester::temperature;
+use filter::*;
 use layout_manager::*;
 pub use states::*;
 use typed_builder::*;
@@ -23,6 +24,7 @@ use crate::{
 
 pub mod data_farmer;
 pub mod data_harvester;
+pub mod filter;
 pub mod frozen_state;
 pub mod layout_manager;
 mod process_killer;
@@ -81,12 +83,6 @@ pub struct DataFilters {
     pub net_filter: Option<Filter>,
 }
 
-#[derive(Debug, Clone)]
-pub struct Filter {
-    pub is_list_ignored: bool,
-    pub list: Vec<regex::Regex>,
-}
-
 #[derive(TypedBuilder)]
 pub struct App {
     #[builder(default = false, setter(skip))]

src/app/data_harvester.rs

@@ -160,19 +160,25 @@ impl DataCollector {
         self.sys.refresh_memory();
         self.mem_total_kb = self.sys.total_memory();
 
+        // Refresh network list once at the start.
+        // TODO: may be worth refreshing every once in a while (maybe on a separate timer).
+        if self.widgets_to_harvest.use_net {
+            self.sys.refresh_networks_list();
+        }
+
+        if self.widgets_to_harvest.use_proc || self.widgets_to_harvest.use_cpu {
+            self.sys.refresh_cpu();
+        }
+
         #[cfg(not(target_os = "linux"))]
         {
             // TODO: Would be good to get this and network list running on a timer instead...?
+
             // Refresh components list once...
             if self.widgets_to_harvest.use_temp {
                 self.sys.refresh_components_list();
             }
 
-            // Refresh network list once...
-            if cfg!(target_os = "windows") && self.widgets_to_harvest.use_net {
-                self.sys.refresh_networks_list();
-            }
-
             if cfg!(target_os = "windows") && self.widgets_to_harvest.use_proc {
                 self.sys.refresh_users_list();
             }
@@ -183,10 +189,6 @@ impl DataCollector {
             }
         }
 
-        if self.widgets_to_harvest.use_proc || self.widgets_to_harvest.use_cpu {
-            self.sys.refresh_cpu();
-        }
-
         #[cfg(feature = "battery")]
         {
             if self.widgets_to_harvest.use_battery {
@@ -238,6 +240,10 @@ impl DataCollector {
             self.sys.refresh_memory();
         }
 
+        if self.widgets_to_harvest.use_net {
+            self.sys.refresh_networks();
+        }
+
         #[cfg(not(target_os = "linux"))]
         {
             if self.widgets_to_harvest.use_proc {
@@ -247,13 +253,6 @@ impl DataCollector {
                 self.sys.refresh_components();
             }
 
-            #[cfg(target_os = "windows")]
-            {
-                if self.widgets_to_harvest.use_net {
-                    self.sys.refresh_networks();
-                }
-            }
-
             #[cfg(target_os = "freebsd")]
             {
                 if self.widgets_to_harvest.use_disk {
@@ -392,31 +391,20 @@ impl DataCollector {
             }
         }
 
-        let network_data_fut = {
+        if self.widgets_to_harvest.use_net {
-            #[cfg(any(target_os = "windows", target_os = "freebsd"))]
+            let net_data = network::get_network_data(
-            {
-                network::get_network_data(
                 &self.sys,
                 self.last_collection_time,
                 &mut self.total_rx,
                 &mut self.total_tx,
                 current_instant,
-                    self.widgets_to_harvest.use_net,
                 &self.filters.net_filter,
-                )
+            );
+
+            self.total_rx = net_data.total_rx;
+            self.total_tx = net_data.total_tx;
+            self.data.network = Some(net_data);
         }
-            #[cfg(not(any(target_os = "windows", target_os = "freebsd")))]
-            {
-                network::get_network_data(
-                    self.last_collection_time,
-                    &mut self.total_rx,
-                    &mut self.total_tx,
-                    current_instant,
-                    self.widgets_to_harvest.use_net,
-                    &self.filters.net_filter,
-                )
-            }
-        };
 
         let disk_data_fut = disks::get_disk_usage(
             self.widgets_to_harvest.use_disk,
@@ -425,16 +413,7 @@ impl DataCollector {
         );
         let disk_io_usage_fut = disks::get_io_usage(self.widgets_to_harvest.use_disk);
 
-        let (net_data, disk_res, io_res) =
+        let (disk_res, io_res) = join!(disk_data_fut, disk_io_usage_fut,);
-            join!(network_data_fut, disk_data_fut, disk_io_usage_fut,);
-
-        if let Ok(net_data) = net_data {
-            if let Some(net_data) = &net_data {
-                self.total_rx = net_data.total_rx;
-                self.total_tx = net_data.total_tx;
-            }
-            self.data.network = net_data;
-        }
 
         if let Ok(disks) = disk_res {
             self.data.disks = disks;

src/app/data_harvester/network.rs

@@ -1,17 +1,7 @@
 //! Data collection for network usage/IO.
-//!
-//! For Linux and macOS, this is handled by Heim.
-//! For Windows, this is handled by sysinfo.
 
-cfg_if::cfg_if! {
+pub mod sysinfo;
-    if #[cfg(any(target_os = "linux", target_os = "macos"))] {
+pub use self::sysinfo::*;
-        pub mod heim;
-        pub use self::heim::*;
-    } else if #[cfg(any(target_os = "freebsd", target_os = "windows"))] {
-        pub mod sysinfo;
-        pub use self::sysinfo::*;
-    }
-}
 
 #[derive(Default, Clone, Debug)]
 /// All units in bits.

src/app/data_harvester/network/heim.rs

@@ -1,67 +0,0 @@
-//! Gets network data via heim.
-
-use std::time::Instant;
-
-use super::NetworkHarvest;
-
-// TODO: Eventually make it so that this thing also takes individual usage into account, so we can show per-interface!
-pub async fn get_network_data(
-    prev_net_access_time: Instant, prev_net_rx: &mut u64, prev_net_tx: &mut u64,
-    curr_time: Instant, actually_get: bool, filter: &Option<crate::app::Filter>,
-) -> crate::utils::error::Result<Option<NetworkHarvest>> {
-    use futures::StreamExt;
-
-    if !actually_get {
-        return Ok(None);
-    }
-
-    let io_data = heim::net::io_counters().await?;
-    futures::pin_mut!(io_data);
-    let mut total_rx: u64 = 0;
-    let mut total_tx: u64 = 0;
-
-    while let Some(io) = io_data.next().await {
-        if let Ok(io) = io {
-            let to_keep = if let Some(filter) = filter {
-                let mut ret = filter.is_list_ignored;
-                for r in &filter.list {
-                    if r.is_match(io.interface()) {
-                        ret = !filter.is_list_ignored;
-                        break;
-                    }
-                }
-                ret
-            } else {
-                true
-            };
-            if to_keep {
-                // TODO: Use bytes as the default instead, perhaps?
-                // Since you might have to do a double conversion (bytes -> bits -> bytes) in some cases;
-                // but if you stick to bytes, then in the bytes, case, you do no conversion, and in the bits case,
-                // you only do one conversion...
-                total_rx += io.bytes_recv().get::<heim::units::information::bit>();
-                total_tx += io.bytes_sent().get::<heim::units::information::bit>();
-            }
-        }
-    }
-
-    let elapsed_time = curr_time.duration_since(prev_net_access_time).as_secs_f64();
-
-    let (rx, tx) = if elapsed_time == 0.0 {
-        (0, 0)
-    } else {
-        (
-            ((total_rx.saturating_sub(*prev_net_rx)) as f64 / elapsed_time) as u64,
-            ((total_tx.saturating_sub(*prev_net_tx)) as f64 / elapsed_time) as u64,
-        )
-    };
-
-    *prev_net_rx = total_rx;
-    *prev_net_tx = total_tx;
-    Ok(Some(NetworkHarvest {
-        rx,
-        tx,
-        total_rx,
-        total_tx,
-    }))
-}

src/app/data_harvester/network/sysinfo.rs

@@ -2,33 +2,24 @@
 
 use std::time::Instant;
 
+use crate::app::Filter;
+
 use super::NetworkHarvest;
 
-pub async fn get_network_data(
+// TODO: Eventually make it so that this thing also takes individual usage into account, so we can show per-interface!
+pub fn get_network_data(
     sys: &sysinfo::System, prev_net_access_time: Instant, prev_net_rx: &mut u64,
-    prev_net_tx: &mut u64, curr_time: Instant, actually_get: bool,
+    prev_net_tx: &mut u64, curr_time: Instant, filter: &Option<Filter>,
-    filter: &Option<crate::app::Filter>,
+) -> NetworkHarvest {
-) -> crate::utils::error::Result<Option<NetworkHarvest>> {
     use sysinfo::{NetworkExt, SystemExt};
 
-    if !actually_get {
-        return Ok(None);
-    }
-
     let mut total_rx: u64 = 0;
     let mut total_tx: u64 = 0;
 
     let networks = sys.networks();
     for (name, network) in networks {
         let to_keep = if let Some(filter) = filter {
-            let mut ret = filter.is_list_ignored;
+            filter.keep_entry(name)
-            for r in &filter.list {
-                if r.is_match(name) {
-                    ret = !filter.is_list_ignored;
-                    break;
-                }
-            }
-            ret
         } else {
             true
         };
@@ -52,10 +43,10 @@ pub async fn get_network_data(
 
     *prev_net_rx = total_rx;
     *prev_net_tx = total_tx;
-    Ok(Some(NetworkHarvest {
+    NetworkHarvest {
         rx,
         tx,
         total_rx,
         total_tx,
-    }))
+    }
 }

src/app/filter.rs

@@ -0,0 +1,62 @@
+#[derive(Debug, Clone)]
+pub struct Filter {
+    // TODO: Maybe change to "ignore_matches"?
+    pub is_list_ignored: bool,
+    pub list: Vec<regex::Regex>,
+}
+
+impl Filter {
+    /// Whether the filter should keep the entry or reject it.
+    #[inline]
+    pub(crate) fn keep_entry(&self, value: &str) -> bool {
+        self.list
+            .iter()
+            .find(|regex| regex.is_match(value))
+            .map(|_| !self.is_list_ignored)
+            .unwrap_or(self.is_list_ignored)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use regex::Regex;
+
+    use super::*;
+
+    /// Test based on the issue in <https://github.com/ClementTsang/bottom/pull/1037>.
+    #[test]
+    fn filter_is_list_ignored() {
+        let results = [
+            "CPU socket temperature",
+            "wifi_0",
+            "motherboard temperature",
+            "amd gpu",
+        ];
+
+        let ignore_true = Filter {
+            is_list_ignored: true,
+            list: vec![Regex::new("temperature").unwrap()],
+        };
+
+        assert_eq!(
+            results
+                .into_iter()
+                .filter(|r| ignore_true.keep_entry(r))
+                .collect::<Vec<_>>(),
+            vec!["wifi_0", "amd gpu"]
+        );
+
+        let ignore_false = Filter {
+            is_list_ignored: false,
+            list: vec![Regex::new("temperature").unwrap()],
+        };
+
+        assert_eq!(
+            results
+                .into_iter()
+                .filter(|r| ignore_false.keep_entry(r))
+                .collect::<Vec<_>>(),
+            vec!["CPU socket temperature", "motherboard temperature"]
+        );
+    }
+}

src/options.rs

@@ -17,7 +17,7 @@ use starship_battery::Manager;
 use typed_builder::*;
 
 use crate::{
-    app::{layout_manager::*, *},
+    app::{filter::Filter, layout_manager::*, *},
     canvas::{canvas_styling::CanvasColours, ColourScheme},
     constants::*,
     units::data_units::DataUnit,