diff --git a/asusd/src/asus_armoury.rs b/asusd/src/asus_armoury.rs index 87bface2..e04cfec4 100644 --- a/asusd/src/asus_armoury.rs +++ b/asusd/src/asus_armoury.rs @@ -65,12 +65,7 @@ impl AsusArmouryAttribute { } fn resolve_i32_value(refreshed: Option, cached: &AttrValue) -> i32 { - refreshed - .or(match cached { - AttrValue::Integer(i) => Some(*i), - _ => None, - }) - .unwrap_or(-1) + refreshed.or_else(|| cached.as_i32()).unwrap_or(-1) } pub async fn emit_limits(&self, connection: &Connection) -> Result<(), RogError> { @@ -291,18 +286,17 @@ impl AsusArmouryAttribute { match self.attr.current_value() { Ok(value) => { self.logged_read_error.store(false, Ordering::Relaxed); - if matches!(value, AttrValue::Integer(_)) { + if value.as_i32().is_some() { attrs.push("current_value".to_string()); } } - Err(e) => { - if !self.logged_read_error.swap(true, Ordering::Relaxed) { - error!( - "Firmware attribute '{}' is not supported or failed to read: {e:?}", - self.attr.name() - ); - } + Err(e) if !self.logged_read_error.swap(true, Ordering::Relaxed) => { + error!( + "Firmware attribute '{}' is not supported or failed to read: {e:?}", + self.attr.name() + ); } + _ => {} } attrs } @@ -332,8 +326,8 @@ impl AsusArmouryAttribute { let mut config = self.config.lock().await; let tuning = config.select_tunings(power_plugged == 1, profile); if let Some(tune) = tuning.group.get_mut(&self.name()) { - if let AttrValue::Integer(i) = self.attr.default_value() { - *tune = *i; + if let Some(i) = self.attr.default_value().as_i32() { + *tune = i; } } if tuning.enabled { @@ -394,8 +388,8 @@ impl AsusArmouryAttribute { return Ok(*tune); } } - if let AttrValue::Integer(i) = self.attr.default_value() { - return Ok(*i); + if let Some(i) = self.attr.default_value().as_i32() { + return Ok(i); } return Err(fdo::Error::Failed( "Could not read current value".to_string(), @@ -415,7 +409,7 @@ impl AsusArmouryAttribute { if let Ok(value) = self.attr.current_value() { self.logged_read_error.store(false, Ordering::Relaxed); - if let AttrValue::Integer(i) = value { + if let Some(i) = value.as_i32() { return Ok(i); } } @@ -673,8 +667,8 @@ pub async fn set_config_or_default( error!("Failed to set {}: {e}", <&str>::from(name)); }) .ok(); - if let AttrValue::Integer(i) = default { - tuning.group.insert(name, *i); + if let Some(i) = default.as_i32() { + tuning.group.insert(name, i); info!( "Set default tuning config for {} = {:?}", <&str>::from(name), diff --git a/rog-aura/src/aura_detection.rs b/rog-aura/src/aura_detection.rs index 81acb13a..e8d8a46d 100644 --- a/rog-aura/src/aura_detection.rs +++ b/rog-aura/src/aura_detection.rs @@ -133,13 +133,18 @@ impl LedSupportFile { if file.is_empty() { warn!("{} is empty", ASUS_LED_MODE_USER_CONF); } else { - if let Ok(mut tmp) = ron::from_str::(&file) { - data.0.append(&mut tmp.0); + match ron::from_str::(&file) { + Ok(mut tmp) => { + data.0.append(&mut tmp.0); + info!( + "Loaded user-defined LED support data from {}", + ASUS_LED_MODE_USER_CONF + ); + } + Err(e) => { + error!("Could not deserialise {}: {}", ASUS_LED_MODE_USER_CONF, e); + } } - info!( - "Loaded user-defined LED support data from {}", - ASUS_LED_MODE_USER_CONF - ); } } // Load and append the default LED support data diff --git a/rog-platform/src/asus_armoury.rs b/rog-platform/src/asus_armoury.rs index 67c56e44..7bde7d7c 100644 --- a/rog-platform/src/asus_armoury.rs +++ b/rog-platform/src/asus_armoury.rs @@ -13,15 +13,9 @@ use crate::error::PlatformError; const BASE_DIR: &str = "/sys/class/firmware-attributes/asus-armoury/attributes/"; fn read_i32(path: &Path) -> Result { - if let Ok(mut f) = File::open(path) { - let mut buf = String::new(); - f.read_to_string(&mut buf)?; - buf.trim() - .parse::() - .map_err(|_| PlatformError::ParseNum) - } else { - Err(PlatformError::ParseNum) - } + read_string(path)? + .parse::() + .map_err(|_| PlatformError::ParseNum) } fn read_string(path: &Path) -> Result { @@ -41,6 +35,22 @@ pub enum AttrValue { None, } +impl AttrValue { + pub fn as_i32(&self) -> Option { + match self { + Self::Integer(val) => Some(*val), + _ => None, + } + } +} + +impl From for AttrValue { + fn from(val: String) -> Self { + val.parse::() + .map(AttrValue::Integer) + .unwrap_or_else(|_| AttrValue::String(val)) + } +} #[derive(Debug, Default, Clone)] pub struct Attribute { name: String, @@ -64,16 +74,7 @@ impl Attribute { /// Read the `current_value` directly from the attribute path pub fn current_value(&self) -> Result { - match read_string(&self.base_path.join("current_value")) { - Ok(val) => { - if let Ok(int) = val.parse::() { - Ok(AttrValue::Integer(int)) - } else { - Ok(AttrValue::String(val)) - } - } - Err(e) => Err(e), - } + read_string(&self.base_path.join("current_value")).map(AttrValue::from) } pub fn base_path_exists(&self) -> bool { @@ -146,27 +147,26 @@ impl Attribute { fn read_base_values( base_path: &Path, ) -> (AttrValue, AttrValue, AttrValue, AttrValue, AttrValue) { - let default_value = match read_string(&base_path.join("default_value")) { - Ok(val) => { - if let Ok(int) = val.parse::() { - AttrValue::Integer(int) - } else { - AttrValue::String(val) - } - } - Err(_) => AttrValue::None, - }; + let default_value = read_string(&base_path.join("default_value")) + .map(AttrValue::from) + .unwrap_or_default(); let possible_values = match read_string(&base_path.join("possible_values")) { - Ok(val) => { - if let Ok(int) = val.parse::() { - AttrValue::Integer(int) - } else if val.contains(';') { - AttrValue::EnumInt(val.split(';').filter_map(|s| s.parse().ok()).collect()) - } else { - AttrValue::EnumStr(val.split(';').map(|s| s.to_string()).collect()) + Ok(val) => match val.parse::() { + Ok(int) => AttrValue::Integer(int), + Err(_) => { + let tokens: Vec<&str> = val.split(';').collect(); + if let Ok(ints) = tokens + .iter() + .map(|s| s.parse::()) + .collect::, _>>() + { + AttrValue::EnumInt(ints) + } else { + AttrValue::EnumStr(tokens.into_iter().map(String::from).collect()) + } } - } + }, Err(_) => AttrValue::None, }; @@ -566,4 +566,77 @@ mod tests { } attr.set_current_value(&val).unwrap(); } + + struct TestDir(PathBuf); + + impl TestDir { + fn new(name: &str) -> Self { + let dir = std::env::temp_dir().join(format!("{name}_{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("failed to create test dir"); + Self(dir) + } + + fn path(&self) -> &Path { + &self.0 + } + + fn join(&self, path: &str) -> PathBuf { + self.0.join(path) + } + } + + impl Drop for TestDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn test_possible_values_parsing() { + let test_dir = TestDir::new("test_possible_values_parsing"); + let possible_path = test_dir.join("possible_values"); + + // 1. All integer tokens + std::fs::write(&possible_path, "0;1;2\n").expect("Failed to write possible_values"); + let (_, possible, _, _, _) = Attribute::read_base_values(test_dir.path()); + assert_eq!(possible, AttrValue::EnumInt(vec![0, 1, 2])); + + // 2. All string tokens + std::fs::write(&possible_path, "Disabled;Enabled\n") + .expect("Failed to write possible_values"); + let (_, possible, _, _, _) = Attribute::read_base_values(test_dir.path()); + assert_eq!( + possible, + AttrValue::EnumStr(vec![ + "Disabled".to_string(), + "Enabled".to_string() + ]) + ); + + // 3. Mixed string and int tokens (preserves all tokens without dropping non-integers) + std::fs::write(&possible_path, "0;Disabled;2\n").expect("Failed to write possible_values"); + let (_, possible, _, _, _) = Attribute::read_base_values(test_dir.path()); + assert_eq!( + possible, + AttrValue::EnumStr(vec![ + "0".to_string(), + "Disabled".to_string(), + "2".to_string() + ]) + ); + + // 4. Single integer token + std::fs::write(&possible_path, "42\n").expect("Failed to write possible_values"); + let (_, possible, _, _, _) = Attribute::read_base_values(test_dir.path()); + assert_eq!(possible, AttrValue::Integer(42)); + + // 5. Single string token + std::fs::write(&possible_path, "Performance\n").expect("Failed to write possible_values"); + let (_, possible, _, _, _) = Attribute::read_base_values(test_dir.path()); + assert_eq!( + possible, + AttrValue::EnumStr(vec!["Performance".to_string()]) + ); + } } diff --git a/rog-platform/src/cpu.rs b/rog-platform/src/cpu.rs index fe211582..6327dbd8 100644 --- a/rog-platform/src/cpu.rs +++ b/rog-platform/src/cpu.rs @@ -6,7 +6,7 @@ use zbus::zvariant::{OwnedValue, Type, Value}; use crate::error::{PlatformError, Result}; use crate::platform::PlatformProfile; -use crate::{read_attr_string, to_device}; +use crate::{read_attr_string, read_sysfs_parsed, to_device}; const ATTR_AVAILABLE_GOVERNORS: &str = "cpufreq/scaling_available_governors"; const ATTR_GOVERNOR: &str = "cpufreq/scaling_governor"; @@ -317,21 +317,16 @@ pub fn get_cpu_temp() -> f32 { if let Ok(name) = std::fs::read_to_string(path.join("name")) { let name = name.trim(); if name == "k10temp" || name == "coretemp" || name == "zenpower" { - if let Ok(temp_str) = std::fs::read_to_string(path.join("temp1_input")) { - if let Ok(temp_val) = temp_str.trim().parse::() { - return temp_val / 1000.0; - } + if let Some(temp_val) = read_sysfs_parsed::(path.join("temp1_input")) { + return temp_val / 1000.0; } } } } } - if let Ok(temp_str) = std::fs::read_to_string("/sys/class/thermal/thermal_zone0/temp") { - if let Ok(temp_val) = temp_str.trim().parse::() { - return temp_val / 1000.0; - } - } - 0.0 + read_sysfs_parsed::("/sys/class/thermal/thermal_zone0/temp") + .map(|t| t / 1000.0) + .unwrap_or(-1.0) } pub fn get_cpu_frequency_mhz() -> f32 { @@ -342,11 +337,9 @@ pub fn get_cpu_frequency_mhz() -> f32 { let name = entry.file_name().to_string_lossy().into_owned(); if name.starts_with("cpu") && name[3..].chars().all(|c| c.is_ascii_digit()) { let freq_path = entry.path().join("cpufreq/scaling_cur_freq"); - if let Ok(freq_str) = std::fs::read_to_string(freq_path) { - if let Ok(freq_khz) = freq_str.trim().parse::() { - total_freq += freq_khz / 1000.0; - count += 1; - } + if let Some(freq_khz) = read_sysfs_parsed::(freq_path) { + total_freq += freq_khz / 1000.0; + count += 1; } } } @@ -368,7 +361,7 @@ pub fn get_cpu_frequency_mhz() -> f32 { if count > 0 { total_freq / count as f32 } else { - 0.0 + -1.0 } } diff --git a/rog-platform/src/gpu_pci.rs b/rog-platform/src/gpu_pci.rs index a1ef4ba0..d26710e2 100644 --- a/rog-platform/src/gpu_pci.rs +++ b/rog-platform/src/gpu_pci.rs @@ -16,6 +16,7 @@ use serde::{Deserialize, Serialize}; use zbus::zvariant::{OwnedValue, Type, Value}; use crate::error::{PlatformError, Result}; +use crate::read_sysfs_parsed; // --- ASUS-specific sysfs paths (reused from rog-platform) --- @@ -527,12 +528,9 @@ pub fn get_igpu_temp() -> f32 { for entry in entries.flatten() { let path = entry.path(); if let Ok(name) = std::fs::read_to_string(path.join("name")) { - let name = name.trim(); - if name == "amdgpu" { - if let Ok(temp_str) = std::fs::read_to_string(path.join("temp1_input")) { - if let Ok(temp_val) = temp_str.trim().parse::() { - return temp_val / 1000.0; - } + if name.trim() == "amdgpu" { + if let Some(temp_val) = read_sysfs_parsed::(path.join("temp1_input")) { + return temp_val / 1000.0; } } } @@ -547,19 +545,14 @@ pub fn get_igpu_usage_pct() -> f32 { let path = entry.path(); let name = path .file_name() - .map(|n| n.to_string_lossy().into_owned()) + .map(|n| n.to_string_lossy()) .unwrap_or_default(); if name.starts_with("card") { - let busy_path = path.join("device/gpu_busy_percent"); - if busy_path.exists() { - if let Ok(vendor_str) = std::fs::read_to_string(path.join("device/vendor")) { - let vendor = vendor_str.trim(); - if vendor == "0x1002" { - if let Ok(val_str) = std::fs::read_to_string(busy_path) { - if let Ok(val) = val_str.trim().parse::() { - return val; - } - } + if let Ok(vendor_str) = std::fs::read_to_string(path.join("device/vendor")) { + if vendor_str.trim() == "0x1002" { + let busy_path = path.join("device/gpu_busy_percent"); + if let Some(val) = read_sysfs_parsed::(busy_path) { + return val; } } } @@ -585,16 +578,14 @@ pub fn get_gpu_temp() -> f32 { if let Ok(name) = std::fs::read_to_string(path.join("name")) { let name = name.trim(); if name == "amdgpu" || name == "nouveau" { - if let Ok(temp_str) = std::fs::read_to_string(path.join("temp1_input")) { - if let Ok(temp_val) = temp_str.trim().parse::() { - return temp_val / 1000.0; - } + if let Some(temp_val) = read_sysfs_parsed::(path.join("temp1_input")) { + return temp_val / 1000.0; } } } } } - 0.0 + -1.0 } pub fn get_gpu_usage_pct() -> f32 { @@ -608,16 +599,12 @@ pub fn get_gpu_usage_pct() -> f32 { if let Ok(entries) = std::fs::read_dir("/sys/class/drm") { for entry in entries.flatten() { let path = entry.path().join("device/gpu_busy_percent"); - if path.exists() { - if let Ok(val_str) = std::fs::read_to_string(path) { - if let Ok(val) = val_str.trim().parse::() { - return val; - } - } + if let Some(val) = read_sysfs_parsed::(path) { + return val; } } } - 0.0 + -1.0 } #[cfg(test)] diff --git a/rog-platform/src/lib.rs b/rog-platform/src/lib.rs index 40d1ee23..a1f8b67b 100644 --- a/rog-platform/src/lib.rs +++ b/rog-platform/src/lib.rs @@ -28,6 +28,10 @@ pub(crate) fn to_device(sys_path: &Path) -> Result { .map_err(|e| PlatformError::Udev("Couldn't transform syspath to device".to_owned(), e)) } +pub(crate) fn read_sysfs_parsed(path: impl AsRef) -> Option { + std::fs::read_to_string(path).ok()?.trim().parse::().ok() +} + pub fn has_attr(device: &Device, attr_name: &str) -> bool { for attr in device.attributes() { if attr.name() == attr_name { diff --git a/rog-platform/src/platform.rs b/rog-platform/src/platform.rs index 07d8760c..a8c33a2f 100644 --- a/rog-platform/src/platform.rs +++ b/rog-platform/src/platform.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use zbus::zvariant::{OwnedValue, Type, Value}; use crate::error::{PlatformError, Result}; -use crate::{attr_string, attr_string_array, to_device}; +use crate::{attr_string, attr_string_array, read_sysfs_parsed, to_device}; /// The "platform" device provides access to things like: /// - `dgpu_disable` @@ -286,15 +286,9 @@ pub fn get_fan_rpms() -> (i32, i32, i32) { let path = entry.path(); if let Ok(name) = std::fs::read_to_string(path.join("name")) { if name.trim() == "asus" { - if let Ok(v) = std::fs::read_to_string(path.join("fan1_input")) { - cpu = v.trim().parse().unwrap_or(0); - } - if let Ok(v) = std::fs::read_to_string(path.join("fan2_input")) { - gpu = v.trim().parse().unwrap_or(0); - } - if let Ok(v) = std::fs::read_to_string(path.join("fan3_input")) { - mid = v.trim().parse().unwrap_or(0); - } + cpu = read_sysfs_parsed(path.join("fan1_input")).unwrap_or(0); + gpu = read_sysfs_parsed(path.join("fan2_input")).unwrap_or(0); + mid = read_sysfs_parsed(path.join("fan3_input")).unwrap_or(0); break; } } diff --git a/rog-platform/src/power.rs b/rog-platform/src/power.rs index 834ffca8..ce0d0745 100644 --- a/rog-platform/src/power.rs +++ b/rog-platform/src/power.rs @@ -123,12 +123,6 @@ impl AsusPower { pub fn get_battery_cycle_count(&self) -> Result { let path = self.battery.join("cycle_count"); - if !path.exists() { - return Err(PlatformError::Read( - path.to_string_lossy().into(), - std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"), - )); - } let content = std::fs::read_to_string(&path) .map_err(|e| PlatformError::Read(path.to_string_lossy().into(), e))?; content.trim().parse::().map_err(|e| { @@ -205,12 +199,6 @@ impl AsusPower { pub fn get_battery_status(&self) -> Result { let path = self.battery.join("status"); - if !path.exists() { - return Err(PlatformError::Read( - path.to_string_lossy().into(), - std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"), - )); - } let content = std::fs::read_to_string(&path) .map_err(|e| PlatformError::Read(path.to_string_lossy().into(), e))?; Ok(content.trim().to_string())