Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

other: show N/A for Nvidia GPUs if we detect one but can't get temps #1557

Merged
merged 3 commits into from
Aug 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions src/app/filter.rs
Original file line number Diff line number Diff line change
@@ -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<regex::Regex>,
list: Vec<Regex>,
}

impl Filter {
/// Create a new filter.
#[inline]
pub(crate) fn new(ignore_matches: bool, list: Vec<Regex>) -> 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`.
Expand All @@ -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<Self>, entry: &str) -> bool {
filter
.as_ref()
.map(|f| f.should_keep(entry))
.unwrap_or(true)
}
}

#[cfg(test)]
Expand All @@ -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<_>>(),
vec!["wifi_0", "amd gpu"]
);
Expand All @@ -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<_>>(),
vec!["CPU socket temperature", "motherboard temperature"]
);
Expand All @@ -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<_>>(),
vec!["wifi_0", "amd gpu"]
);
Expand All @@ -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<_>>(),
vec!["CPU socket temperature", "motherboard temperature"]
);
Expand Down
33 changes: 9 additions & 24 deletions src/data_collection/disks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,26 +103,26 @@ pub fn keep_disk_entry(
disk_name: &str, mount_point: &str, disk_filter: &Option<Filter>, mount_filter: &Option<Filter>,
) -> 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,
}
}
Expand Down Expand Up @@ -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]);

Expand Down
2 changes: 1 addition & 1 deletion src/data_collection/network/sysinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
17 changes: 14 additions & 3 deletions src/data_collection/nvidia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
};

Expand All @@ -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() {
Expand All @@ -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();

Expand Down Expand Up @@ -130,6 +140,7 @@ pub fn get_nvidia_vecs(
}
}
}

Some(GpusData {
memory: if !mem_vec.is_empty() {
Some(mem_vec)
Expand Down
17 changes: 0 additions & 17 deletions src/data_collection/temperature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -66,21 +64,6 @@ impl TemperatureType {
}
}

pub fn is_temp_filtered(filter: &Option<Filter>, 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;
Expand Down
6 changes: 3 additions & 3 deletions src/data_collection/temperature/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -327,7 +327,7 @@ fn hwmon_temperatures(temp_type: &TemperatureType, filter: &Option<Filter>) -> 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,
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/data_collection/temperature/sysinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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())),
Expand Down
5 changes: 1 addition & 4 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,10 +868,7 @@ fn get_ignore_list(ignore_list: &Option<IgnoreList>) -> OptionResult<Option<Filt

let list = list.map_err(|err| OptionError::config(err.to_string()))?;

Ok(Some(Filter {
list,
is_list_ignored: ignore_list.is_list_ignored,
}))
Ok(Some(Filter::new(ignore_list.is_list_ignored, list)))
} else {
Ok(None)
}
Expand Down