Merge pull request #15 from Eraden/refactor-and-fixes

Refactor and fixes

services/amdfand.service

@@ -1,8 +1,9 @@
 [Unit]
 Description=amdfan controller
-Requires=graphical.target
+After=sysinit.target local-fs.target
 [Service]
-ExecStart=/usr/local/bin/amdfand service
-Restart=always
+Restart=on-failure
+RestartSec=4
+ExecStart=/usr/bin/amdfand service
 [Install]
 WantedBy=multi-user.target

src/config.rs

@@ -1,5 +1,4 @@
 use crate::{AmdFanError, CONFIG_PATH};
-use log::LevelFilter;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::fmt::Formatter;
 use std::io::ErrorKind;
@@ -52,7 +51,7 @@ impl<'de> Deserialize<'de> for Card {
                 E: de::Error,
             {
                 match value.parse::<Card>() {
-                    Ok(card) => Ok(card.0),
+                    Ok(card) => Ok(*card),
                     Err(AmdFanError::InvalidPrefix) => {
                         Err(E::custom(format!("expect cardX but got {}", value)))
                     }
@@ -68,6 +67,7 @@ impl<'de> Deserialize<'de> for Card {
                         "Failed to read vendor file for {}",
                         value
                     ))),
+                    _ => unreachable!(),
                 }
             }
         }
@@ -84,6 +84,14 @@ impl Serialize for Card {
     }
 }
 
+impl std::ops::Deref for Card {
+    type Target = u32;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct MatrixPoint {
     pub temp: f64,
@@ -106,15 +114,15 @@ pub enum LogLevel {
     Trace,
 }
 
-impl From<LogLevel> for LevelFilter {
-    fn from(level: LogLevel) -> Self {
-        match level {
-            LogLevel::Off => LevelFilter::Off,
-            LogLevel::Error => LevelFilter::Error,
-            LogLevel::Warn => LevelFilter::Warn,
-            LogLevel::Info => LevelFilter::Info,
-            LogLevel::Debug => LevelFilter::Debug,
-            LogLevel::Trace => LevelFilter::Trace,
+impl LogLevel {
+    pub fn to_str(&self) -> &str {
+        match self {
+            LogLevel::Off => "OFF",
+            LogLevel::Error => "ERROR",
+            LogLevel::Warn => "WARN",
+            LogLevel::Info => "INFO",
+            LogLevel::Debug => "DEBUG",
+            LogLevel::Trace => "TRACE",
         }
     }
 }
@@ -141,7 +149,13 @@ impl Config {
             return self.max_speed();
         }
 
-        crate::linear_map(temp, self.speed_matrix[idx].temp, self.speed_matrix[idx+1].temp, self.speed_matrix[idx].speed, self.speed_matrix[idx+1].speed)
+        crate::linear_map(
+            temp,
+            self.speed_matrix[idx].temp,
+            self.speed_matrix[idx + 1].temp,
+            self.speed_matrix[idx].speed,
+            self.speed_matrix[idx + 1].speed,
+        )
     }
 
     pub fn log_level(&self) -> LogLevel {
@@ -222,7 +236,10 @@ pub fn load_config() -> std::io::Result<Config> {
             return Err(std::io::Error::from(ErrorKind::InvalidData));
         }
         if matrix_point.speed > 100f64 {
-            log::error!("Fan speed can't be above 100.0 found {}", matrix_point.speed);
+            log::error!(
+                "Fan speed can't be above 100.0 found {}",
+                matrix_point.speed
+            );
             return Err(std::io::Error::from(ErrorKind::InvalidData));
         }
         if let Some(last_point) = last_point {

src/hw_mon.rs

@@ -0,0 +1,209 @@
+use crate::config::Card;
+use crate::io_err::{invalid_input, not_found};
+use crate::{linear_map, AmdFanError, HwMon, HW_MON_DIR, ROOT_DIR};
+
+/// pulse width modulation fan control minimum level (0)
+const PULSE_WIDTH_MODULATION_MIN: &str = "pwm1_min";
+
+/// pulse width modulation fan control maximum level (255)
+const PULSE_WIDTH_MODULATION_MAX: &str = "pwm1_max";
+
+/// pulse width modulation fan level (0-255)
+const PULSE_WIDTH_MODULATION: &str = "pwm1";
+
+/// pulse width modulation fan control method (0: no fan speed control, 1: manual fan speed control using pwm interface, 2: automatic fan speed control)
+const PULSE_WIDTH_MODULATION_MODE: &str = "pwm1_enable";
+
+// static PULSE_WIDTH_MODULATION_DISABLED: &str = "0";
+const PULSE_WIDTH_MODULATION_AUTO: &str = "2";
+const PULSE_WIDTH_MODULATION_MANUAL: &str = "1";
+
+impl HwMon {
+    pub fn new(card: &Card, name: &str) -> Self {
+        Self {
+            card: card.clone(),
+            name: String::from(name),
+            pwm_min: None,
+            pwm_max: None,
+            temp_inputs: load_temp_inputs(&card, name),
+        }
+    }
+
+    pub fn max_gpu_temp(&self) -> std::io::Result<f64> {
+        let mut results = Vec::with_capacity(self.temp_inputs.len());
+        for name in self.temp_inputs.iter() {
+            results.push(self.read_gpu_temp(name).unwrap_or(0));
+        }
+        results.sort();
+        results
+            .last()
+            .copied()
+            .map(|temp| temp as f64 / 1000f64)
+            .ok_or_else(invalid_input)
+    }
+
+    pub fn gpu_temp(&self) -> Vec<(String, std::io::Result<f64>)> {
+        self.temp_inputs
+            .clone()
+            .into_iter()
+            .map(|name| {
+                let temp = self
+                    .read_gpu_temp(name.as_str())
+                    .map(|temp| temp as f64 / 1000f64);
+                (name, temp)
+            })
+            .collect()
+    }
+
+    fn read_gpu_temp(&self, name: &str) -> std::io::Result<u64> {
+        self.read(name)?.parse::<u64>().map_err(|e| {
+            log::warn!("Read from gpu monitor failed. Invalid temperature. {}", e);
+            invalid_input()
+        })
+    }
+
+    #[inline]
+    pub(crate) fn name(&self) -> std::io::Result<String> {
+        self.read("name")
+    }
+
+    pub fn pwm_min(&mut self) -> u32 {
+        if self.pwm_min.is_none() {
+            self.pwm_min = Some(self.value_or(PULSE_WIDTH_MODULATION_MIN, 0));
+        };
+        self.pwm_min.unwrap_or_default()
+    }
+
+    pub fn pwm_max(&mut self) -> u32 {
+        if self.pwm_max.is_none() {
+            self.pwm_max = Some(self.value_or(PULSE_WIDTH_MODULATION_MAX, 255));
+        };
+        self.pwm_max.unwrap_or(255)
+    }
+
+    pub fn pwm(&self) -> std::io::Result<u32> {
+        self.read(PULSE_WIDTH_MODULATION)?.parse().map_err(|_e| {
+            log::warn!("Read from gpu monitor failed. Invalid pwm value");
+            invalid_input()
+        })
+    }
+
+    pub fn is_fan_manual(&self) -> bool {
+        self.read(PULSE_WIDTH_MODULATION_MODE)
+            .map(|s| s.as_str() == PULSE_WIDTH_MODULATION_MANUAL)
+            .unwrap_or_default()
+    }
+
+    pub fn is_fan_automatic(&self) -> bool {
+        self.read(PULSE_WIDTH_MODULATION_MODE)
+            .map(|s| s.as_str() == PULSE_WIDTH_MODULATION_AUTO)
+            .unwrap_or_default()
+    }
+
+    #[inline]
+    pub fn is_amd(&self) -> bool {
+        std::fs::read_to_string(format!("{}/{}/device/vendor", ROOT_DIR, self.card))
+            .map_err(|_| AmdFanError::FailedReadVendor)
+            .map(|vendor| vendor.trim() == "0x1002")
+            .unwrap_or_default()
+    }
+
+    #[inline]
+    pub fn name_is_amd(&self) -> bool {
+        self.name().ok().filter(|s| s.trim() == "amdgpu").is_some()
+    }
+
+    pub fn set_manual(&self) -> std::io::Result<()> {
+        self.write("pwm1_enable", 1)
+    }
+
+    pub fn set_automatic(&self) -> std::io::Result<()> {
+        self.write("pwm1_enable", 2)
+    }
+
+    pub fn set_pwm(&self, value: u64) -> std::io::Result<()> {
+        if self.is_fan_automatic() {
+            self.set_manual()?;
+        }
+        self.write("pwm1", value)
+    }
+
+    pub fn set_speed(&mut self, speed: f64) -> std::io::Result<()> {
+        let min = self.pwm_min() as f64;
+        let max = self.pwm_max() as f64;
+        let pwm = linear_map(speed, 0f64, 100f64, min, max).round() as u64;
+        self.set_pwm(pwm)
+    }
+
+    fn read(&self, name: &str) -> std::io::Result<String> {
+        std::fs::read_to_string(self.path(name)).map(|s| String::from(s.trim()))
+    }
+
+    fn write(&self, name: &str, value: u64) -> std::io::Result<()> {
+        std::fs::write(self.path(name), format!("{}", value))?;
+        Ok(())
+    }
+
+    fn path(&self, name: &str) -> std::path::PathBuf {
+        self.mon_dir().join(name)
+    }
+
+    fn mon_dir(&self) -> std::path::PathBuf {
+        hw_mon_dir_path(&self.card, &self.name)
+    }
+
+    #[inline]
+    fn value_or<R: std::str::FromStr>(&self, name: &str, fallback: R) -> R {
+        self.read(name)
+            .ok()
+            .and_then(|s| s.parse().ok())
+            .unwrap_or(fallback)
+    }
+}
+
+fn load_temp_inputs(card: &Card, name: &str) -> Vec<String> {
+    let dir = match std::fs::read_dir(hw_mon_dir_path(card, name)) {
+        Ok(d) => d,
+        _ => return vec![],
+    };
+    dir.filter_map(|f| f.ok())
+        .filter_map(|f| {
+            f.file_name()
+                .to_str()
+                .filter(|s| s.starts_with("temp") && s.ends_with("_input"))
+                .map(String::from)
+        })
+        .collect()
+}
+
+#[inline]
+fn hw_mon_dirs_path(card: &Card) -> std::path::PathBuf {
+    std::path::PathBuf::new()
+        .join(ROOT_DIR)
+        .join(card.to_string())
+        .join(HW_MON_DIR)
+}
+
+#[inline]
+fn hw_mon_dir_path(card: &Card, name: &str) -> std::path::PathBuf {
+    hw_mon_dirs_path(&card).join(name)
+}
+
+pub(crate) fn open_hw_mon(card: Card) -> std::io::Result<HwMon> {
+    let read_path = hw_mon_dirs_path(&card);
+    let entries = std::fs::read_dir(read_path)?;
+    let name = entries
+        .filter_map(|entry| entry.ok())
+        .filter_map(|entry| {
+            entry
+                .file_name()
+                .as_os_str()
+                .to_str()
+                .filter(|name| name.starts_with("hwmon"))
+                .map(String::from)
+        })
+        .take(1)
+        .last()
+        .ok_or_else(not_found)?;
+    Ok(HwMon::new(&card, &name))
+}

src/io_err.rs

@@ -0,0 +1,14 @@
+use std::io::Error as IoErr;
+use std::io::ErrorKind;
+
+pub fn invalid_data() -> IoErr {
+    IoErr::from(ErrorKind::InvalidData)
+}
+
+pub fn not_found() -> IoErr {
+    IoErr::from(ErrorKind::NotFound)
+}
+
+pub fn invalid_input() -> IoErr {
+    IoErr::from(ErrorKind::NotFound)
+}

src/main.rs

@@ -1,12 +1,18 @@
 mod config;
+mod hw_mon;
+mod io_err;
+mod monitor;
 
 extern crate log;
 
 use std::io::ErrorKind;
 
 use crate::config::{load_config, Card, Config};
+use crate::io_err::{invalid_data, not_found};
 use gumdrop::Options;
+use std::fmt::Formatter;
 
+static CONFIG_DIR: &str = "/etc/amdfand";
 static CONFIG_PATH: &str = "/etc/amdfand/config.toml";
 
 static ROOT_DIR: &str = "/sys/class/drm";
@@ -19,6 +25,24 @@ pub enum AmdFanError {
     InvalidSuffix(String),
     NotAmdCard,
     FailedReadVendor,
+    InvalidMonitorFormat,
+}
+
+impl std::fmt::Display for AmdFanError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            AmdFanError::InvalidPrefix => f.write_str("Card must starts with `card`."),
+            AmdFanError::InputTooShort => f.write_str(
+                "Card must start with `card` and ends with a number. The given name is too short.",
+            ),
+            AmdFanError::InvalidSuffix(s) => {
+                f.write_fmt(format_args!("Value after `card` is invalid {}", s))
+            }
+            AmdFanError::NotAmdCard => f.write_str("Vendor is not AMD"),
+            AmdFanError::FailedReadVendor => f.write_str("Unable to read GPU vendor"),
+            AmdFanError::InvalidMonitorFormat => f.write_str("Monitor format is not valid. Available values are: short, s, long l, verbose and v"),
+        }
+    }
 }
 
 // linear mapping from the xrange to the yrange
@@ -33,160 +57,7 @@ pub struct HwMon {
     name: String,
     pwm_min: Option<u32>,
     pwm_max: Option<u32>,
-}
-
-impl HwMon {
-    pub fn new(card: &Card, name: &str) -> Self {
-        Self {
-            card: card.clone(),
-            name: String::from(name),
-            pwm_min: None,
-            pwm_max: None,
-        }
-    }
-
-    pub fn gpu_temp(&self) -> std::io::Result<f64> {
-        let value = self.read("temp1_input")?.parse::<u64>().map_err(|_| {
-            log::warn!("Read from gpu monitor failed. Invalid temperature");
-            std::io::Error::from(ErrorKind::InvalidInput)
-        })?;
-        Ok(value as f64 / 1000f64)
-    }
-
-    fn name(&self) -> std::io::Result<String> {
-        self.read("name")
-    }
-
-    pub fn pwm_min(&mut self) -> u32 {
-        if self.pwm_min.is_none() {
-            self.pwm_min = Some(
-                self.read("pwm1_min")
-                    .unwrap_or_default()
-                    .parse()
-                    .unwrap_or(0),
-            )
-        };
-        self.pwm_min.unwrap_or_default()
-    }
-
-    pub fn pwm_max(&mut self) -> u32 {
-        if self.pwm_max.is_none() {
-            self.pwm_max = Some(
-                self.read("pwm1_max")
-                    .unwrap_or_default()
-                    .parse()
-                    .unwrap_or(255),
-            )
-        };
-        self.pwm_max.unwrap_or(255)
-    }
-
-    pub fn pwm(&self) -> std::io::Result<u32> {
-        self.read("pwm1")?.parse().map_err(|_e| {
-            log::warn!("Read from gpu monitor failed. Invalid pwm value");
-            std::io::Error::from(ErrorKind::InvalidInput)
-        })
-    }
-
-    pub fn pwm_speed(&self) -> std::io::Result<u64> {
-        self.read("pwm1")?.parse().map_err(|_e| {
-            log::warn!("Read from gpu monitor failed. Invalid fan speed");
-            std::io::Error::from(ErrorKind::InvalidInput)
-        })
-    }
-
-    pub fn is_fan_manual(&self) -> bool {
-        self.read("pwm1_enable")
-            .map(|s| s.as_str() == "1")
-            .unwrap_or_default()
-    }
-
-    pub fn is_fan_automatic(&self) -> bool {
-        self.read("pwm1_enable")
-            .map(|s| s.as_str() == "2")
-            .unwrap_or_default()
-    }
-
-    pub fn is_amd(&self) -> bool {
-        std::fs::read_to_string(format!("{}/{}/device/vendor", ROOT_DIR, self.card))
-            .map_err(|_| AmdFanError::FailedReadVendor)
-            .map(|vendor| vendor.trim() == "0x1002")
-            .unwrap_or_default()
-    }
-
-    pub fn set_manual(&self) -> std::io::Result<()> {
-        self.write("pwm1_enable", 1)
-    }
-
-    pub fn set_automatic(&self) -> std::io::Result<()> {
-        self.write("pwm1_enable", 2)
-    }
-
-    pub fn set_pwm(&self, value: u32) -> std::io::Result<()> {
-        if self.is_fan_automatic() {
-            self.set_manual()?;
-        }
-        self.write("pwm1", value as u64)
-    }
-    
-    pub fn set_speed(&mut self, speed: f64) -> std::io::Result<()> {
-        let min = self.pwm_min() as f64;
-        let max = self.pwm_max() as f64;
-        let pwm = linear_map(speed, 0f64, 100f64, min, max).round() as u32;
-        self.set_pwm(pwm)
-    }
-
-    fn read(&self, name: &str) -> std::io::Result<String> {
-        let read_path = self.path(name);
-        std::fs::read_to_string(read_path).map(|s| s.trim().to_string())
-    }
-
-    fn write(&self, name: &str, value: u64) -> std::io::Result<()> {
-        std::fs::write(self.path(name), format!("{}", value))?;
-        Ok(())
-    }
-
-    fn path(&self, name: &str) -> std::path::PathBuf {
-        self.mon_dir().join(name)
-    }
-
-    fn mon_dir(&self) -> std::path::PathBuf {
-        std::path::Path::new(ROOT_DIR)
-            .join(self.card.to_string())
-            .join(HW_MON_DIR)
-            .join(&self.name)
-    }
-}
-
-pub struct CardController {
-    pub hw_mon: HwMon,
-    pub last_temp: f64,
-}
-
-impl CardController {
-    pub fn new(card: Card) -> std::io::Result<Self> {
-        let name = Self::find_hw_mon(&card)?;
-        Ok(Self {
-            hw_mon: HwMon::new(&card, &name),
-            last_temp: 1_000f64,
-        })
-    }
-
-    fn find_hw_mon(card: &Card) -> std::io::Result<String> {
-        let read_path = format!("{}/{}/{}", ROOT_DIR, card.to_string(), HW_MON_DIR);
-        let entries = std::fs::read_dir(read_path)?;
-        entries
-            .filter_map(|entry| entry.ok())
-            .filter_map(|entry| {
-                entry
-                    .file_name()
-                    .as_os_str()
-                    .to_str()
-                    .map(|s| s.to_string())
-            })
-            .find(|name| name.starts_with("hwmon"))
-            .ok_or_else(|| std::io::Error::from(ErrorKind::NotFound))
-    }
+    temp_inputs: Vec<String>,
 }
 
 pub enum FanMode {
@@ -194,12 +65,6 @@ pub enum FanMode {
     Automatic,
 }
 
-#[derive(Debug, Options)]
-pub struct Monitor {
-    #[options(help = "Help message")]
-    help: bool,
-}
-
 #[derive(Debug, Options)]
 pub struct Service {
     #[options(help = "Help message")]
@@ -223,7 +88,7 @@ pub struct AvailableCards {
 #[derive(Debug, Options)]
 pub enum Command {
     #[options(help = "Print current temp and fan speed")]
-    Monitor(Monitor),
+    Monitor(monitor::Monitor),
     #[options(help = "Set fan speed depends on GPU temperature")]
     Service(Service),
     #[options(help = "Switch to GPU automatic fan speed control")]
@@ -254,14 +119,11 @@ fn read_cards() -> std::io::Result<Vec<Card>> {
                     .file_name()
                     .as_os_str()
                     .to_str()
-                    .map(|s| s.to_string())
-                    .ok_or_else(|| std::io::Error::from(ErrorKind::InvalidData))
+                    .map(String::from)
+                    .ok_or_else(invalid_data)
             })
-            .and_then(|file_name| {
-                file_name
-                    .parse::<Card>()
-                    .map_err(|_| std::io::Error::from(ErrorKind::InvalidData))
-            }) {
+            .and_then(|file_name| file_name.parse::<Card>().map_err(|_| invalid_data()))
+        {
             Ok(card) => {
                 cards.push(card);
             }
@@ -271,47 +133,39 @@ fn read_cards() -> std::io::Result<Vec<Card>> {
     Ok(cards)
 }
 
-fn controllers(config: &Config, filter: bool) -> std::io::Result<Vec<CardController>> {
+fn controllers(config: &Config, filter: bool) -> std::io::Result<Vec<HwMon>> {
     Ok(read_cards()?
         .into_iter()
-        .filter(|card| {
-            !filter
-                || config
-                    .cards()
-                    .iter()
-                    .find(|name| name.0 == card.0)
-                    .is_some()
-        })
-        .map(|card| CardController::new(card).unwrap())
-        .filter(|controller| !filter || controller.hw_mon.is_amd())
-        .filter(|reader| {
-            !filter
-                || reader
-                    .hw_mon
-                    .name()
-                    .ok()
-                    .filter(|s| s.as_str() == "amdgpu")
-                    .is_some()
-        })
+        .filter(|card| !filter || config.cards().iter().find(|name| **name == *card).is_some())
+        .filter_map(|card| hw_mon::open_hw_mon(card).ok())
+        .filter(|hw_mon| !filter || { hw_mon.is_amd() })
+        .filter(|hw_mon| !filter || hw_mon.name_is_amd())
         .collect())
 }
 
 fn service(config: Config) -> std::io::Result<()> {
     let mut controllers = controllers(&config, true)?;
+    if controllers.is_empty() {
+        return Err(not_found());
+    }
+    let mut cache = std::collections::HashMap::new();
     loop {
-        for controller in controllers.iter_mut() {
-            let gpu_temp = controller.hw_mon.gpu_temp().unwrap_or_default();
+        for hw_mon in controllers.iter_mut() {
+            let gpu_temp = hw_mon.max_gpu_temp().unwrap_or_default();
+            log::debug!("Current {} temperature: {}", hw_mon.card, gpu_temp);
+            let last = *cache.entry(*hw_mon.card).or_insert(1_000f64);
 
-            let speed = config.speed_for_temp(gpu_temp);
-            let target_pwm = (speed as f32 * 2.55).round() as u32;
-            if controller.hw_mon.pwm_min() > target_pwm || controller.hw_mon.pwm_max() < target_pwm {
+            if (last - gpu_temp).abs() < 0.001f64 {
+                log::debug!("Temperature didn't change");
                 continue;
-            }
+            };
+            let speed = config.speed_for_temp(gpu_temp);
+            log::debug!("Resolved speed {:.2}", speed);
 
-            if let Err(e) = controller.hw_mon.set_speed(target_pwm as f64) {
-                log::error!("Failed to change speed to {}. {:?}", target_pwm, e);
+            if let Err(e) = hw_mon.set_speed(speed) {
+                log::error!("Failed to change speed to {}. {:?}", speed, e);
             }
-            controller.last_temp = gpu_temp;
+            cache.insert(*hw_mon.card, gpu_temp);
         }
         std::thread::sleep(std::time::Duration::from_secs(4));
     }
@@ -321,28 +175,31 @@ fn change_mode(switcher: Switcher, mode: FanMode, config: Config) -> std::io::Re
     let mut controllers = controllers(&config, true)?;
 
     let cards = match switcher.card {
-        Some(card_id) => match controllers.iter().position(|c| c.hw_mon.card.0 == card_id) {
+        Some(card_id) => match controllers
+            .iter()
+            .position(|hw_mon| *hw_mon.card == card_id)
+        {
             Some(card) => vec![controllers.remove(card)],
             None => {
                 eprintln!("Card does not exists. Available cards: ");
-                for card in controllers {
-                    eprintln!(" * {}", card.hw_mon.card.0);
+                for hw_mon in controllers {
+                    eprintln!(" * {}", *hw_mon.card);
                 }
-                return Err(std::io::Error::from(ErrorKind::NotFound));
+                return Err(not_found());
             }
         },
         None => controllers,
     };
 
-    for card in cards {
+    for hw_mon in cards {
         match mode {
             FanMode::Automatic => {
-                if let Err(e) = card.hw_mon.set_automatic() {
+                if let Err(e) = hw_mon.set_automatic() {
                     log::error!("{:?}", e);
                 }
             }
             FanMode::Manual => {
-                if let Err(e) = card.hw_mon.set_manual() {
+                if let Err(e) = hw_mon.set_manual() {
                     log::error!("{:?}", e);
                 }
             }
@@ -351,37 +208,14 @@ fn change_mode(switcher: Switcher, mode: FanMode, config: Config) -> std::io::Re
     Ok(())
 }
 
-fn monitor_cards(config: Config) -> std::io::Result<()> {
-    let mut controllers = controllers(&config, true)?;
-    loop {
-        print!("{esc}[2J{esc}[1;1H", esc = 27 as char);
-        for card in controllers.iter_mut() {
-            println!(
-                "Card {:3} | Temp     |  RPM |  MIN |  MAX |  PWM |   %",
-                card.hw_mon.card.to_string().replace("card", "")
-            );
-            println!(
-                "         | {:>5.2}    | {:>4} | {:>4} | {:>4} | {:>4} | {:>3}",
-                card.hw_mon.gpu_temp().unwrap_or_default(),
-                card.hw_mon.pwm_speed().unwrap_or_default(),
-                card.hw_mon.pwm_min(),
-                card.hw_mon.pwm_max(),
-                card.hw_mon.pwm_speed().unwrap_or_default(),
-                (card.hw_mon.pwm_speed().unwrap_or_default() as f32 / 2.55).round(),
-            );
-        }
-        std::thread::sleep(std::time::Duration::from_secs(4));
-    }
-}
-
 fn main() -> std::io::Result<()> {
-    if std::fs::read("/etc/amdfand").map_err(|e| e.kind() == ErrorKind::NotFound) == Err(true) {
-        std::fs::create_dir_all("/etc/amdfand")?;
+    if std::fs::read(CONFIG_DIR).map_err(|e| e.kind() == ErrorKind::NotFound) == Err(true) {
+        std::fs::create_dir_all(CONFIG_DIR)?;
     }
 
     let config = load_config()?;
 
-    log::set_max_level(config.log_level().into());
+    std::env::set_var("RUST_LOG", config.log_level().to_str());
     pretty_env_logger::init();
 
     let opts: Opts = Opts::parse_args_default_or_exit();
@@ -393,17 +227,17 @@ fn main() -> std::io::Result<()> {
 
     match opts.command {
         None => service(config),
-        Some(Command::Monitor(_)) => monitor_cards(config),
+        Some(Command::Monitor(monitor)) => monitor::run(monitor, config),
         Some(Command::Service(_)) => service(config),
         Some(Command::SetAutomatic(switcher)) => change_mode(switcher, FanMode::Automatic, config),
         Some(Command::SetManual(switcher)) => change_mode(switcher, FanMode::Manual, config),
         Some(Command::Available(_)) => {
             println!("Available cards");
-            controllers(&config, false)?.into_iter().for_each(|card| {
+            controllers(&config, false)?.into_iter().for_each(|hw_mon| {
                 println!(
                     " * {:6>} - {}",
-                    card.hw_mon.card.to_string(),
-                    card.hw_mon.name().unwrap_or_default()
+                    hw_mon.card,
+                    hw_mon.name().unwrap_or_default()
                 );
             });
             Ok(())

src/monitor.rs

@@ -0,0 +1,101 @@
+use crate::config::Config;
+use crate::{controllers, AmdFanError};
+use std::str::FromStr;
+
+#[derive(Debug)]
+pub enum MonitorFormat {
+    Short,
+    Verbose,
+}
+
+impl Default for MonitorFormat {
+    fn default() -> Self {
+        MonitorFormat::Short
+    }
+}
+
+impl FromStr for MonitorFormat {
+    type Err = AmdFanError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "short" | "s" => Ok(MonitorFormat::Short),
+            "verbose" | "v" | "long" | "l" => Ok(MonitorFormat::Verbose),
+            _ => Err(AmdFanError::InvalidMonitorFormat),
+        }
+    }
+}
+
+#[derive(Debug, gumdrop::Options)]
+pub struct Monitor {
+    #[options(help = "Help message")]
+    help: bool,
+    #[options(help = "Help message")]
+    format: MonitorFormat,
+}
+
+pub fn run(monitor: Monitor, config: Config) -> std::io::Result<()> {
+    match monitor.format {
+        MonitorFormat::Short => short(config),
+        MonitorFormat::Verbose => verbose(config),
+    }
+}
+
+pub fn verbose(config: Config) -> std::io::Result<()> {
+    let mut controllers = controllers(&config, true)?;
+    loop {
+        print!("{esc}[2J{esc}[1;1H", esc = 27 as char);
+        for hw_mon in controllers.iter_mut() {
+            println!("Card {:3}", hw_mon.card.to_string().replace("card", ""));
+            println!("  MIN |  MAX |  PWM   |   %");
+            let min = hw_mon.pwm_min();
+            let max = hw_mon.pwm_max();
+            println!(
+                " {:>4} | {:>4} | {:>6} | {:>3}",
+                min,
+                max,
+                hw_mon
+                    .pwm()
+                    .map_or_else(|_e| String::from("FAILED"), |f| f.to_string()),
+                (crate::linear_map(hw_mon.pwm().unwrap_or_default() as f64, min as f64, max as f64, 0f64, 100f64)).round(),
+            );
+
+            println!();
+            println!("  Current temperature");
+            hw_mon.gpu_temp().into_iter().for_each(|(name, temp)| {
+                println!(
+                    "  {:6} | {:>9.2}",
+                    name.replace("_input", ""),
+                    temp.unwrap_or_default(),
+                );
+            });
+        }
+        println!();
+        println!("> PWM may be 0 even if RPM is higher");
+        std::thread::sleep(std::time::Duration::from_secs(4));
+    }
+}
+
+pub fn short(config: Config) -> std::io::Result<()> {
+    let mut controllers = controllers(&config, true)?;
+    loop {
+        print!("{esc}[2J{esc}[1;1H", esc = 27 as char);
+        for hw_mon in controllers.iter_mut() {
+            println!(
+                "Card {:3} | Temp     |  MIN |  MAX |  PWM |   %",
+                hw_mon.card.to_string().replace("card", "")
+            );
+            let min = hw_mon.pwm_min();
+            let max = hw_mon.pwm_max();
+            println!(
+                "         | {:>5.2}    | {:>4} | {:>4} | {:>4} | {:>3}",
+                hw_mon.max_gpu_temp().unwrap_or_default(),
+                min,
+                max,
+                hw_mon.pwm().unwrap_or_default(),
+                crate::linear_map(hw_mon.pwm().unwrap_or_default() as f64, min as f64, max as f64, 0f64, 100f64).round(),
+            );
+        }
+        std::thread::sleep(std::time::Duration::from_secs(4));
+    }
+}