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

@@ -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
 

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

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<f32>,
 }
 
 #[derive(Clone, Debug, Copy, PartialEq, Eq, Default)]

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<f32> {
     Ok(fs::read_to_string(path)?
         .trim_end()
         .parse::<f32>()
-        .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<Filter>) -> 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<Filter>) -> 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)),
                         });
                     }
                 }

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(),
-                            },
+                            }),
                         });
                     }
                 }

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}");
         }
     }

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

src/utils/logging.rs

@@ -20,9 +20,9 @@ pub static OFFSET: once_cell::sync::Lazy<time::UtcOffset> = 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,25 +43,19 @@ 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 {
-    ($($x:tt)*) => {
-        #[cfg(feature = "logging")]
-        {
-            log::debug!($($x)*)
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! c_error {
+macro_rules! error {
     ($($x:tt)*) => {
         #[cfg(feature = "logging")]
         {
@@ -71,7 +65,17 @@ macro_rules! c_error {
 }
 
 #[macro_export]
-macro_rules! c_info {
+macro_rules! warn {
+    ($($x:tt)*) => {
+        #[cfg(feature = "logging")]
+        {
+            log::warn!($($x)*)
+        }
+    };
+}
+
+#[macro_export]
+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.");
+    }
+}

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<u64>,
     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(),
+        }
     }
 }