From d9d9e1df9f0be49c4102ddaf5be6b94f6b658705 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Sun, 11 Aug 2024 21:20:07 +0000 Subject: [PATCH] other: show N/A for Nvidia GPUs if we detect one but can't get temps (#1557) * other: show N/A for Nvidia GPUs if we detect one but can't get the temperature * refactor: driveby refactor of filter system and code for temp * missed one --- src/app/filter.rs | 42 +++++++++++++++++----- src/data_collection/disks.rs | 33 +++++------------ src/data_collection/network/sysinfo.rs | 2 +- src/data_collection/nvidia.rs | 17 +++++++-- src/data_collection/temperature.rs | 17 --------- src/data_collection/temperature/linux.rs | 6 ++-- src/data_collection/temperature/sysinfo.rs | 4 +-- src/options.rs | 5 +-- 8 files changed, 64 insertions(+), 62 deletions(-) diff --git a/src/app/filter.rs b/src/app/filter.rs index c313c0b6..05e37b3e 100644 --- a/src/app/filter.rs +++ b/src/app/filter.rs @@ -1,21 +1,32 @@ +use regex::Regex; + /// Filters used by widgets to filter out certain entries. /// TODO: Move this out maybe? #[derive(Debug, Clone)] pub struct Filter { /// Whether the filter _accepts_ all entries that match `list`, /// or _denies_ any entries that match it. - pub is_list_ignored: bool, // TODO: Maybe change to "ignore_matches"? + is_list_ignored: bool, // TODO: Maybe change to "ignore_matches"? /// The list of regexes to match against. Whether it goes through /// the filter or not depends on `is_list_ignored`. - pub list: Vec, + list: Vec, } impl Filter { + /// Create a new filter. + #[inline] + pub(crate) fn new(ignore_matches: bool, list: Vec) -> Self { + Self { + is_list_ignored: ignore_matches, + list, + } + } + /// Whether the filter should keep the entry or reject it. #[inline] - pub(crate) fn keep_entry(&self, value: &str) -> bool { - if self.has_match(value) { + pub(crate) fn should_keep(&self, entry: &str) -> bool { + if self.has_match(entry) { // If a match is found, then if we wanted to ignore if we match, return false. // If we want to keep if we match, return true. Thus, return the // inverse of `is_list_ignored`. @@ -30,6 +41,21 @@ impl Filter { pub(crate) fn has_match(&self, value: &str) -> bool { self.list.iter().any(|regex| regex.is_match(value)) } + + /// Whether entries matching the list should be ignored or kept. + #[inline] + pub(crate) fn ignore_matches(&self) -> bool { + self.is_list_ignored + } + + /// Check a filter if it exists, otherwise accept if it is [`None`]. + #[inline] + pub(crate) fn optional_should_keep(filter: &Option, entry: &str) -> bool { + filter + .as_ref() + .map(|f| f.should_keep(entry)) + .unwrap_or(true) + } } #[cfg(test)] @@ -56,7 +82,7 @@ mod test { assert_eq!( results .into_iter() - .filter(|r| ignore_true.keep_entry(r)) + .filter(|r| ignore_true.should_keep(r)) .collect::>(), vec!["wifi_0", "amd gpu"] ); @@ -69,7 +95,7 @@ mod test { assert_eq!( results .into_iter() - .filter(|r| ignore_false.keep_entry(r)) + .filter(|r| ignore_false.should_keep(r)) .collect::>(), vec!["CPU socket temperature", "motherboard temperature"] ); @@ -85,7 +111,7 @@ mod test { assert_eq!( results .into_iter() - .filter(|r| multi_true.keep_entry(r)) + .filter(|r| multi_true.should_keep(r)) .collect::>(), vec!["wifi_0", "amd gpu"] ); @@ -101,7 +127,7 @@ mod test { assert_eq!( results .into_iter() - .filter(|r| multi_false.keep_entry(r)) + .filter(|r| multi_false.should_keep(r)) .collect::>(), vec!["CPU socket temperature", "motherboard temperature"] ); diff --git a/src/data_collection/disks.rs b/src/data_collection/disks.rs index 9aba0579..e8748b1c 100644 --- a/src/data_collection/disks.rs +++ b/src/data_collection/disks.rs @@ -103,26 +103,26 @@ pub fn keep_disk_entry( disk_name: &str, mount_point: &str, disk_filter: &Option, mount_filter: &Option, ) -> bool { match (disk_filter, mount_filter) { - (Some(d), Some(m)) => match (d.is_list_ignored, m.is_list_ignored) { + (Some(d), Some(m)) => match (d.ignore_matches(), m.ignore_matches()) { (true, true) => !(d.has_match(disk_name) || m.has_match(mount_point)), (true, false) => { if m.has_match(mount_point) { true } else { - d.keep_entry(disk_name) + d.should_keep(disk_name) } } (false, true) => { if d.has_match(disk_name) { true } else { - m.keep_entry(mount_point) + m.should_keep(mount_point) } } (false, false) => d.has_match(disk_name) || m.has_match(mount_point), }, - (Some(d), None) => d.keep_entry(disk_name), - (None, Some(m)) => m.keep_entry(mount_point), + (Some(d), None) => d.should_keep(disk_name), + (None, Some(m)) => m.should_keep(mount_point), (None, None) => true, } } @@ -158,25 +158,10 @@ mod test { #[test] fn test_keeping_disk_entry() { - let disk_ignore = Some(Filter { - is_list_ignored: true, - list: vec![Regex::new("nvme").unwrap()], - }); - - let disk_keep = Some(Filter { - is_list_ignored: false, - list: vec![Regex::new("nvme").unwrap()], - }); - - let mount_ignore = Some(Filter { - is_list_ignored: true, - list: vec![Regex::new("boot").unwrap()], - }); - - let mount_keep = Some(Filter { - is_list_ignored: false, - list: vec![Regex::new("boot").unwrap()], - }); + let disk_ignore = Some(Filter::new(true, vec![Regex::new("nvme").unwrap()])); + let disk_keep = Some(Filter::new(false, vec![Regex::new("nvme").unwrap()])); + let mount_ignore = Some(Filter::new(true, vec![Regex::new("boot").unwrap()])); + let mount_keep = Some(Filter::new(false, vec![Regex::new("boot").unwrap()])); assert_eq!(run_filter(&None, &None), vec![0, 1, 2, 3, 4]); diff --git a/src/data_collection/network/sysinfo.rs b/src/data_collection/network/sysinfo.rs index f29180b0..2975fe66 100644 --- a/src/data_collection/network/sysinfo.rs +++ b/src/data_collection/network/sysinfo.rs @@ -18,7 +18,7 @@ pub fn get_network_data( for (name, network) in networks { let to_keep = if let Some(filter) = filter { - filter.keep_entry(name) + filter.should_keep(name) } else { true }; diff --git a/src/data_collection/nvidia.rs b/src/data_collection/nvidia.rs index a6fe293e..286fb6d0 100644 --- a/src/data_collection/nvidia.rs +++ b/src/data_collection/nvidia.rs @@ -9,7 +9,7 @@ use crate::{ app::{filter::Filter, layout_manager::UsedWidgets}, data_collection::{ memory::MemHarvest, - temperature::{is_temp_filtered, TempHarvest, TemperatureType}, + temperature::{TempHarvest, TemperatureType}, }, }; @@ -32,6 +32,7 @@ pub fn get_nvidia_vecs( let mut mem_vec = Vec::with_capacity(num_gpu as usize); let mut proc_vec = Vec::with_capacity(num_gpu as usize); let mut total_mem = 0; + for i in 0..num_gpu { if let Ok(device) = nvml.device_by_index(i) { if let Ok(name) = device.name() { @@ -51,17 +52,26 @@ pub fn get_nvidia_vecs( )); } } - if widgets_to_harvest.use_temp && is_temp_filtered(filter, &name) { + + if widgets_to_harvest.use_temp + && Filter::optional_should_keep(filter, &name) + { if let Ok(temperature) = device.temperature(TemperatureSensor::Gpu) { let temperature = temp_type.convert_temp_unit(temperature as f32); temp_vec.push(TempHarvest { - name: name.clone(), + name, temperature: Some(temperature), }); + } else { + temp_vec.push(TempHarvest { + name, + temperature: None, + }); } } } + if widgets_to_harvest.use_proc { let mut procs = HashMap::new(); @@ -130,6 +140,7 @@ pub fn get_nvidia_vecs( } } } + Some(GpusData { memory: if !mem_vec.is_empty() { Some(mem_vec) diff --git a/src/data_collection/temperature.rs b/src/data_collection/temperature.rs index 414765f4..b72ffbfd 100644 --- a/src/data_collection/temperature.rs +++ b/src/data_collection/temperature.rs @@ -15,8 +15,6 @@ cfg_if::cfg_if! { use std::str::FromStr; -use crate::app::filter::Filter; - #[derive(Default, Debug, Clone)] pub struct TempHarvest { pub name: String, @@ -66,21 +64,6 @@ impl TemperatureType { } } -pub fn is_temp_filtered(filter: &Option, text: &str) -> bool { - if let Some(filter) = filter { - let mut ret = filter.is_list_ignored; - for r in &filter.list { - if r.is_match(text) { - ret = !filter.is_list_ignored; - break; - } - } - ret - } else { - true - } -} - #[cfg(test)] mod test { use crate::data_collection::temperature::TemperatureType; diff --git a/src/data_collection/temperature/linux.rs b/src/data_collection/temperature/linux.rs index 3f1ac095..4aaf1f52 100644 --- a/src/data_collection/temperature/linux.rs +++ b/src/data_collection/temperature/linux.rs @@ -8,7 +8,7 @@ use std::{ use anyhow::Result; use hashbrown::{HashMap, HashSet}; -use super::{is_temp_filtered, TempHarvest, TemperatureType}; +use super::{TempHarvest, TemperatureType}; use crate::app::filter::Filter; const EMPTY_NAME: &str = "Unknown"; @@ -327,7 +327,7 @@ fn hwmon_temperatures(temp_type: &TemperatureType, filter: &Option) -> H // TODO: It's possible we may want to move the filter check further up to avoid // probing hwmon if not needed? - if is_temp_filtered(filter, &name) { + if Filter::optional_should_keep(filter, &name) { if let Ok(temp_celsius) = parse_temp(&temp_path) { temperatures.push(TempHarvest { name, @@ -377,7 +377,7 @@ fn add_thermal_zone_temperatures( name }; - if is_temp_filtered(filter, &name) { + if Filter::optional_should_keep(filter, &name) { let temp_path = file_path.join("temp"); if let Ok(temp_celsius) = parse_temp(&temp_path) { let name = counted_name(&mut seen_names, name); diff --git a/src/data_collection/temperature/sysinfo.rs b/src/data_collection/temperature/sysinfo.rs index 9e6e797e..d1a9e634 100644 --- a/src/data_collection/temperature/sysinfo.rs +++ b/src/data_collection/temperature/sysinfo.rs @@ -2,7 +2,7 @@ use anyhow::Result; -use super::{is_temp_filtered, TempHarvest, TemperatureType}; +use super::{TempHarvest, TemperatureType}; use crate::app::filter::Filter; pub fn get_temperature_data( @@ -13,7 +13,7 @@ pub fn get_temperature_data( for component in components { let name = component.label().to_string(); - if is_temp_filtered(filter, &name) { + if Filter::optional_should_keep(filter, &name) { temperature_vec.push(TempHarvest { name, temperature: Some(temp_type.convert_temp_unit(component.temperature())), diff --git a/src/options.rs b/src/options.rs index 24571f1a..4861e069 100644 --- a/src/options.rs +++ b/src/options.rs @@ -868,10 +868,7 @@ fn get_ignore_list(ignore_list: &Option) -> OptionResult