From a67da93c5f48b9c85fed0fae5e4c310d6a9361e1 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Wed, 20 Dec 2023 01:36:08 -0500 Subject: other: if in a non-D0 state, short-circuit further logic (#1355) * other: if in a non-D0 state, short-circuit further logic * cleanup * add back an empty name and value * fix for macos/windows * some testing things --- CHANGELOG.md | 1 + src/app/data_harvester/nvidia.rs | 2 +- src/app/data_harvester/temperature.rs | 2 +- src/app/data_harvester/temperature/linux.rs | 77 +++++++++++++++------------ src/app/data_harvester/temperature/sysinfo.rs | 6 +-- src/bin/main.rs | 11 +++- src/data_conversion.rs | 2 +- src/utils/logging.rs | 57 ++++++++++++++------ src/widgets/temperature_table.rs | 20 ++++--- 9 files changed, 112 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ad825dc..2227e9d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug Fixes - [#1314](https://github.com/ClementTsang/bottom/pull/1314): Fix fat32 mounts not showing up in macOS. +- [#1355](https://github.com/ClementTsang/bottom/pull/1355): Reduce chances of non-D0 devices waking up due to temperature checks on Linux. ## [0.9.6] - 2023-08-26 diff --git a/src/app/data_harvester/nvidia.rs b/src/app/data_harvester/nvidia.rs index 848a18f7..964ccc5a 100644 --- a/src/app/data_harvester/nvidia.rs +++ b/src/app/data_harvester/nvidia.rs @@ -54,7 +54,7 @@ pub fn get_nvidia_vecs( temp_vec.push(TempHarvest { name: name.clone(), - temperature, + temperature: Some(temperature), }); } } diff --git a/src/app/data_harvester/temperature.rs b/src/app/data_harvester/temperature.rs index a15eb26a..220190e6 100644 --- a/src/app/data_harvester/temperature.rs +++ b/src/app/data_harvester/temperature.rs @@ -18,7 +18,7 @@ use crate::app::Filter; #[derive(Default, Debug, Clone)] pub struct TempHarvest { pub name: String, - pub temperature: f32, + pub temperature: Option, } #[derive(Clone, Debug, Copy, PartialEq, Eq, Default)] diff --git a/src/app/data_harvester/temperature/linux.rs b/src/app/data_harvester/temperature/linux.rs index a62c52b1..a25dc352 100644 --- a/src/app/data_harvester/temperature/linux.rs +++ b/src/app/data_harvester/temperature/linux.rs @@ -9,7 +9,7 @@ use anyhow::Result; use hashbrown::{HashMap, HashSet}; use super::{is_temp_filtered, TempHarvest, TemperatureType}; -use crate::app::Filter; +use crate::{app::Filter, utils::error::BottomError}; const EMPTY_NAME: &str = "Unknown"; @@ -24,7 +24,7 @@ fn read_temp(path: &Path) -> Result { Ok(fs::read_to_string(path)? .trim_end() .parse::() - .map_err(|e| crate::utils::error::BottomError::ConversionError(e.to_string()))? + .map_err(|e| BottomError::ConversionError(e.to_string()))? / 1_000.0) } @@ -131,8 +131,8 @@ fn finalize_name( None => label, }, (Some(name), None) => name, - (None, None) => match &fallback_sensor_name { - Some(sensor_name) => sensor_name.clone(), + (None, None) => match fallback_sensor_name { + Some(sensor_name) => sensor_name.to_owned(), None => EMPTY_NAME.to_string(), }, }; @@ -140,6 +140,31 @@ fn finalize_name( counted_name(seen_names, candidate_name) } +/// Whether the temperature should *actually* be read during enumeration. +/// Will return false if the state is not D0/unknown, or if it does not support `device/power_state`. +#[inline] +fn is_device_awake(path: &Path) -> bool { + // Whether the temperature should *actually* be read during enumeration. + // Set to false if the device is in ACPI D3cold. + // Documented at https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-power_state + let device = path.join("device"); + let power_state = device.join("power_state"); + if power_state.exists() { + if let Ok(state) = fs::read_to_string(power_state) { + let state = state.trim(); + // The zenpower3 kernel module (incorrectly?) reports "unknown", causing this check + // to fail and temperatures to appear as zero instead of having the file not exist. + // + // Their self-hosted git instance has disabled sign up, so this bug cant be reported either. + state == "D0" || state == "unknown" + } else { + true + } + } else { + true + } +} + /// Get temperature sensors from the linux sysfs interface `/sys/class/hwmon` and /// `/sys/devices/platform/coretemp.*`. It returns all found temperature sensors, and the number /// of checked hwmon directories (not coretemp directories). @@ -178,29 +203,15 @@ fn hwmon_temperatures(temp_type: &TemperatureType, filter: &Option) -> H for file_path in dirs { let sensor_name = read_to_string_lossy(file_path.join("name")); - // Whether the temperature should *actually* be read during enumeration. - // Set to false if the device is in ACPI D3cold. - // - // If it is false, then the temperature will be set to 0.0 later down the line. - let should_read_temp = { - // Documented at https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-power_state - let device = file_path.join("device"); - let power_state = device.join("power_state"); - if power_state.exists() { - if let Ok(state) = fs::read_to_string(power_state) { - let state = state.trim(); - // The zenpower3 kernel module (incorrectly?) reports "unknown", causing this check - // to fail and temperatures to appear as zero instead of having the file not exist. - // - // Their self-hosted git instance has disabled sign up, so this bug cant be reported either. - state == "D0" || state == "unknown" - } else { - true - } - } else { - true + if !is_device_awake(&file_path) { + if let Some(sensor_name) = sensor_name { + temperatures.push(TempHarvest { + name: sensor_name, + temperature: None, + }); } - }; + continue; + } if let Ok(dir_entries) = file_path.read_dir() { // Enumerate the devices temperature sensors @@ -272,19 +283,15 @@ fn hwmon_temperatures(temp_type: &TemperatureType, filter: &Option) -> H let name = finalize_name(hwmon_name, sensor_label, &sensor_name, &mut seen_names); if is_temp_filtered(filter, &name) { - let temp_celsius = if should_read_temp { - if let Ok(temp) = read_temp(&temp_path) { - temp - } else { - continue; - } + let temp_celsius = if let Ok(temp) = read_temp(&temp_path) { + temp } else { - 0.0 + continue; }; temperatures.push(TempHarvest { name, - temperature: temp_type.convert_temp_unit(temp_celsius), + temperature: Some(temp_type.convert_temp_unit(temp_celsius)), }); } } @@ -329,7 +336,7 @@ fn add_thermal_zone_temperatures( temperatures.push(TempHarvest { name, - temperature: temp_type.convert_temp_unit(temp_celsius), + temperature: Some(temp_type.convert_temp_unit(temp_celsius)), }); } } diff --git a/src/app/data_harvester/temperature/sysinfo.rs b/src/app/data_harvester/temperature/sysinfo.rs index cc415164..4e28efa7 100644 --- a/src/app/data_harvester/temperature/sysinfo.rs +++ b/src/app/data_harvester/temperature/sysinfo.rs @@ -19,7 +19,7 @@ pub fn get_temperature_data( if is_temp_filtered(filter, &name) { temperature_vec.push(TempHarvest { name, - temperature: temp_type.convert_temp_unit(component.temperature()), + temperature: Some(temp_type.convert_temp_unit(component.temperature())), }); } } @@ -36,11 +36,11 @@ pub fn get_temperature_data( if let Some(temp) = temp.as_temperature() { temperature_vec.push(TempHarvest { name, - temperature: match temp_type { + temperature: Some(match temp_type { TemperatureType::Celsius => temp.celsius(), TemperatureType::Kelvin => temp.kelvin(), TemperatureType::Fahrenheit => temp.fahrenheit(), - }, + }), }); } } diff --git a/src/bin/main.rs b/src/bin/main.rs index 4d59a8e6..f75b1505 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -21,10 +21,14 @@ use crossterm::{ use tui::{backend::CrosstermBackend, Terminal}; use bottom::{ + args, canvas::{self, canvas_styling::CanvasStyling}, + check_if_terminal, cleanup_terminal, create_collection_thread, create_input_thread, + create_or_get_config, data_conversion::*, + handle_key_event_or_break, handle_mouse_event, options::*, - *, + panic_hook, read_config, try_drawing, update_data, BottomEvent, }; // Used for heap allocation debugging purposes. @@ -38,7 +42,10 @@ fn main() -> Result<()> { #[cfg(feature = "logging")] { - if let Err(err) = init_logger(log::LevelFilter::Debug, std::ffi::OsStr::new("debug.log")) { + if let Err(err) = bottom::init_logger( + log::LevelFilter::Debug, + Some(std::ffi::OsStr::new("debug.log")), + ) { println!("Issue initializing logger: {err}"); } } diff --git a/src/data_conversion.rs b/src/data_conversion.rs index 645e70f3..f7f62eac 100644 --- a/src/data_conversion.rs +++ b/src/data_conversion.rs @@ -132,7 +132,7 @@ impl ConvertedData { data.temp_harvest.iter().for_each(|temp_harvest| { self.temp_data.push(TempWidgetData { sensor: KString::from_ref(&temp_harvest.name), - temperature_value: temp_harvest.temperature.ceil() as u64, + temperature_value: temp_harvest.temperature.map(|temp| temp.ceil() as u64), temperature_type, }); }); diff --git a/src/utils/logging.rs b/src/utils/logging.rs index 3b405ad7..0b56591c 100644 --- a/src/utils/logging.rs +++ b/src/utils/logging.rs @@ -20,9 +20,9 @@ pub static OFFSET: once_cell::sync::Lazy = once_cell::sync::Laz #[cfg(feature = "logging")] pub fn init_logger( - min_level: log::LevelFilter, debug_file_name: &std::ffi::OsStr, + min_level: log::LevelFilter, debug_file_name: Option<&std::ffi::OsStr>, ) -> Result<(), fern::InitError> { - fern::Dispatch::new() + let dispatch = fern::Dispatch::new() .format(|out, message, record| { let offset_time = { let utc = time::OffsetDateTime::now_utc(); @@ -43,35 +43,39 @@ pub fn init_logger( message )) }) - .level(min_level) - .chain(fern::log_file(debug_file_name)?) - .apply()?; + .level(min_level); + + if let Some(debug_file_name) = debug_file_name { + dispatch.chain(fern::log_file(debug_file_name)?).apply()?; + } else { + dispatch.chain(std::io::stdout()).apply()?; + } Ok(()) } #[macro_export] -macro_rules! c_debug { +macro_rules! error { ($($x:tt)*) => { #[cfg(feature = "logging")] { - log::debug!($($x)*) + log::error!($($x)*) } }; } #[macro_export] -macro_rules! c_error { +macro_rules! warn { ($($x:tt)*) => { #[cfg(feature = "logging")] { - log::error!($($x)*) + log::warn!($($x)*) } }; } #[macro_export] -macro_rules! c_info { +macro_rules! info { ($($x:tt)*) => { #[cfg(feature = "logging")] { @@ -81,17 +85,17 @@ macro_rules! c_info { } #[macro_export] -macro_rules! c_log { +macro_rules! debug { ($($x:tt)*) => { #[cfg(feature = "logging")] { - log::log!($($x)*) + log::debug!($($x)*) } }; } #[macro_export] -macro_rules! c_trace { +macro_rules! trace { ($($x:tt)*) => { #[cfg(feature = "logging")] { @@ -101,11 +105,34 @@ macro_rules! c_trace { } #[macro_export] -macro_rules! c_warn { +macro_rules! log { ($($x:tt)*) => { #[cfg(feature = "logging")] { - log::warn!($($x)*) + log::log!(log::Level::Trace, $($x)*) } }; + ($level:expr, $($x:tt)*) => { + #[cfg(feature = "logging")] + { + log::log!($level, $($x)*) + } + }; +} + +#[cfg(test)] +mod test { + + #[cfg(feature = "logging")] + #[test] + fn test_logging_macros() { + super::init_logger(log::LevelFilter::Trace, None) + .expect("initializing the logger should succeed"); + + error!("This is an error."); + warn!("This is a warning."); + info!("This is an info."); + debug!("This is a debug."); + info!("This is a trace."); + } } diff --git a/src/widgets/temperature_table.rs b/src/widgets/temperature_table.rs index 1c8fe67c..3f570c0b 100644 --- a/src/widgets/temperature_table.rs +++ b/src/widgets/temperature_table.rs @@ -17,7 +17,7 @@ use crate::{ #[derive(Clone, Debug)] pub struct TempWidgetData { pub sensor: KString, - pub temperature_value: u64, + pub temperature_value: Option, pub temperature_type: TemperatureType, } @@ -37,13 +37,17 @@ impl ColumnHeader for TempWidgetColumn { impl TempWidgetData { pub fn temperature(&self) -> KString { - let temp_val = self.temperature_value.to_string(); - let temp_type = match self.temperature_type { - TemperatureType::Celsius => "°C", - TemperatureType::Kelvin => "K", - TemperatureType::Fahrenheit => "°F", - }; - concat_string!(temp_val, temp_type).into() + match self.temperature_value { + Some(temp_val) => { + let temp_type = match self.temperature_type { + TemperatureType::Celsius => "°C", + TemperatureType::Kelvin => "K", + TemperatureType::Fahrenheit => "°F", + }; + concat_string!(temp_val.to_string(), temp_type).into() + } + None => "N/A".to_string().into(), + } } } -- cgit v1.2.3