From 56a602e0a646f58acf6effa400cd41afb31b5a39 Mon Sep 17 00:00:00 2001 From: S4NKALP Date: Thu, 12 Mar 2026 00:42:26 +0545 Subject: [PATCH] refactor(src/commands): simplify validation and file handling - New Features - None - Enhancements - Simplified percent range validation for battery mode. - Combined existence and read checks for sysfs entries, removing redundant branches. - Streamlined systemctl command arguments and removed unnecessary references. - Removed superfluous braces and simplified cooling profile detection logic. - Improved robustness of sysfs parsing for Intel RAPL and AMD hwmon temperature limits. - Bug Fixes - Fixed battery percent validation to use inclusive range (1..=100) instead of incorrect reference comparison. --- src/commands/battery.rs | 7 +++--- src/commands/gpu.rs | 17 +++++++-------- src/commands/power.rs | 19 +++++++---------- src/commands/status.rs | 47 ++++++++++++++++------------------------- 4 files changed, 37 insertions(+), 53 deletions(-) diff --git a/src/commands/battery.rs b/src/commands/battery.rs index 335600f..04d556e 100644 --- a/src/commands/battery.rs +++ b/src/commands/battery.rs @@ -6,7 +6,7 @@ use std::path::Path; pub fn execute(command: &BatteryCommands) { match command { BatteryCommands::Limit { percent } => { - if percent < &1 || percent > &100 { + if !(&1..=&100).contains(&percent) { error!("Invalid percentage. Please provide a value between 1 and 100."); std::process::exit(1); } @@ -119,8 +119,8 @@ pub fn execute(command: &BatteryCommands) { for ideapad_entry in ideapad_entries.flatten() { let conservation_path = ideapad_entry.path().join("conservation_mode"); - if conservation_path.exists() { - if let Ok(mode) = fs::read_to_string(&conservation_path) { + if conservation_path.exists() + && let Ok(mode) = fs::read_to_string(&conservation_path) { if mode.trim() == "1" { println!( " Charge Limit: Conservation Mode (~60%)" @@ -129,7 +129,6 @@ pub fn execute(command: &BatteryCommands) { println!(" Charge Limit: 100%"); } } - } } } } diff --git a/src/commands/gpu.rs b/src/commands/gpu.rs index 173fdd7..d4cb198 100644 --- a/src/commands/gpu.rs +++ b/src/commands/gpu.rs @@ -269,11 +269,10 @@ fn create_cache_file() { let _ = fs::create_dir_all(parent); } - if let Ok(json) = serde_json::to_string_pretty(&cache) { - if fs::write(CACHE_FILE_PATH, json).is_ok() { + if let Ok(json) = serde_json::to_string_pretty(&cache) + && fs::write(CACHE_FILE_PATH, json).is_ok() { log::debug!("Created file {}", CACHE_FILE_PATH); } - } } fn read_cache_file() -> Result { @@ -296,7 +295,7 @@ fn switch_integrated() { let is_debug = log::log_enabled!(log::Level::Debug); let mut dis_cmd = Command::new("systemctl"); - dis_cmd.args(&["disable", "nvidia-persistenced.service"]); + dis_cmd.args(["disable", "nvidia-persistenced.service"]); if !is_debug { dis_cmd.stdout(std::process::Stdio::null()); dis_cmd.stderr(std::process::Stdio::null()); @@ -337,7 +336,7 @@ fn switch_hybrid(rtd3: Option, use_nvidia_current: bool) { // persistenced enable let is_debug = log::log_enabled!(log::Level::Debug); let mut enable_cmd = Command::new("systemctl"); - enable_cmd.args(&["enable", "nvidia-persistenced.service"]); + enable_cmd.args(["enable", "nvidia-persistenced.service"]); if !is_debug { enable_cmd.stdout(std::process::Stdio::null()); enable_cmd.stderr(std::process::Stdio::null()); @@ -390,7 +389,7 @@ fn switch_nvidia( // persistenced enable let is_debug = log::log_enabled!(log::Level::Debug); let mut enable_cmd = Command::new("systemctl"); - enable_cmd.args(&["enable", "nvidia-persistenced.service"]); + enable_cmd.args(["enable", "nvidia-persistenced.service"]); if !is_debug { enable_cmd.stdout(std::process::Stdio::null()); enable_cmd.stderr(std::process::Stdio::null()); @@ -454,7 +453,7 @@ fn switch_nvidia( create_file(EXTRA_XORG_PATH, &extra_xorg, false); } - let display_manager = dm.or_else(|| get_display_manager()); + let display_manager = dm.or_else(get_display_manager); if let Some(cdm) = display_manager { if cdm == "sddm" { @@ -537,7 +536,7 @@ pub fn execute(cmd: &GpuCommands) { GpuCommands::Hybrid { rtd3, use_nvidia_current, - } => switch_hybrid(rtd3.clone(), *use_nvidia_current), + } => switch_hybrid(*rtd3, *use_nvidia_current), GpuCommands::Nvidia { dm, force_comp, @@ -547,7 +546,7 @@ pub fn execute(cmd: &GpuCommands) { } => switch_nvidia( dm.clone(), *force_comp, - coolbits.clone(), + *coolbits, *use_nvidia_current, *wayland, ), diff --git a/src/commands/power.rs b/src/commands/power.rs index 3d5687f..dd0598f 100644 --- a/src/commands/power.rs +++ b/src/commands/power.rs @@ -14,7 +14,7 @@ fn set_cpu_governor(governor: &str) { for entry in entries.flatten() { let path = entry.path(); let name = path.file_name().unwrap_or_default().to_string_lossy(); - if name.starts_with("cpu") && name.chars().nth(3).map_or(false, |c| c.is_ascii_digit()) + if name.starts_with("cpu") && name.chars().nth(3).is_some_and(|c| c.is_ascii_digit()) { let scaling_gov_path = path.join("cpufreq/scaling_governor"); if scaling_gov_path.exists() { @@ -48,7 +48,7 @@ fn set_energy_performance_preference(epp: &str) { for entry in entries.flatten() { let path = entry.path(); let name = path.file_name().unwrap_or_default().to_string_lossy(); - if name.starts_with("cpu") && name.chars().nth(3).map_or(false, |c| c.is_ascii_digit()) + if name.starts_with("cpu") && name.chars().nth(3).is_some_and(|c| c.is_ascii_digit()) { let epp_path = path.join("cpufreq/energy_performance_preference"); if epp_path.exists() { @@ -74,8 +74,8 @@ fn set_intel_rapl_limit(watts: u32) -> bool { // Intel RAPL let rapl_dir = Path::new("/sys/class/powercap/intel-rapl"); - if rapl_dir.exists() { - if let Ok(entries) = fs::read_dir(rapl_dir) { + if rapl_dir.exists() + && let Ok(entries) = fs::read_dir(rapl_dir) { for entry in entries.flatten() { let path = entry.path(); let name = path.file_name().unwrap_or_default().to_string_lossy(); @@ -108,7 +108,6 @@ fn set_intel_rapl_limit(watts: u32) -> bool { } } } - } success } @@ -118,8 +117,8 @@ fn set_amd_hwmon_limit(watts: u32) -> bool { // AMD hwmon (Typically exposed via hwmon platform drivers like k10temp or amd_pmc) let hwmon_dir = Path::new("/sys/class/hwmon"); - if hwmon_dir.exists() { - if let Ok(entries) = fs::read_dir(hwmon_dir) { + if hwmon_dir.exists() + && let Ok(entries) = fs::read_dir(hwmon_dir) { for entry in entries.flatten() { let path = entry.path(); let power1_cap = path.join("power1_cap"); @@ -127,8 +126,8 @@ fn set_amd_hwmon_limit(watts: u32) -> bool { if power1_cap.exists() { // Try to verify this is actually the CPU and not a generic sensor - if let Ok(name) = fs::read_to_string(&name_path) { - if name.trim().contains("amd") { + if let Ok(name) = fs::read_to_string(&name_path) + && name.trim().contains("amd") { match fs::write(&power1_cap, microwatts.to_string()) { Ok(_) => { log::debug!("Set AMD hwmon power1_cap to {}uW", microwatts); @@ -137,11 +136,9 @@ fn set_amd_hwmon_limit(watts: u32) -> bool { Err(e) => log::debug!("Failed to write AMD hwmon: {}", e), } } - } } } } - } success } fn set_platform_profile(profile: &str) { diff --git a/src/commands/status.rs b/src/commands/status.rs index f36b06c..8d04865 100644 --- a/src/commands/status.rs +++ b/src/commands/status.rs @@ -38,15 +38,14 @@ pub fn execute() { { for ideapad_entry in ideapad_entries.flatten() { let conservation_path = ideapad_entry.path().join("conservation_mode"); - if conservation_path.exists() { - if let Ok(mode) = fs::read_to_string(&conservation_path) { + if conservation_path.exists() + && let Ok(mode) = fs::read_to_string(&conservation_path) { if mode.trim() == "1" { println!(" Charge Limit: Conservation Mode (~60%)"); } else { println!(" Charge Limit: 100%"); } } - } } } } @@ -66,8 +65,8 @@ pub fn execute() { if let Ok(ideapad_entries) = fs::read_dir("/sys/bus/platform/drivers/ideapad_acpi") { for ideapad_entry in ideapad_entries.flatten() { let fan_path = ideapad_entry.path().join("fan_mode"); - if fan_path.exists() { - if let Ok(mode) = fs::read_to_string(&fan_path) { + if fan_path.exists() + && let Ok(mode) = fs::read_to_string(&fan_path) { thermal_found = true; let desc = match mode.trim() { "1" => "Performance", @@ -77,13 +76,12 @@ pub fn execute() { }; println!("Cooling Profile: {} (Ideapad)", desc); } - } } } let asus_path = Path::new("/sys/devices/platform/asus-nb-wmi/throttle_thermal_policy"); - if asus_path.exists() { - if let Ok(mode) = fs::read_to_string(asus_path) { + if asus_path.exists() + && let Ok(mode) = fs::read_to_string(asus_path) { thermal_found = true; let desc = match mode.trim() { "1" => "Performance", @@ -93,7 +91,6 @@ pub fn execute() { }; println!("Cooling Profile: {} (ASUS)", desc); } - } if !thermal_found { println!("Cooling Profile: Unknown / Firmware managed"); @@ -104,43 +101,40 @@ pub fn execute() { // Check Intel RAPL let rapl_dir = Path::new("/sys/class/powercap/intel-rapl"); - if rapl_dir.exists() { - if let Ok(entries) = fs::read_dir(rapl_dir) { + if rapl_dir.exists() + && let Ok(entries) = fs::read_dir(rapl_dir) { for entry in entries.flatten() { let path = entry.path(); let name = path.file_name().unwrap_or_default().to_string_lossy(); if name.starts_with("intel-rapl:") { let constraint_0 = path.join("constraint_0_power_limit_uw"); // PL1 - if constraint_0.exists() { - if let Ok(limit_uw) = fs::read_to_string(&constraint_0) { - if let Ok(limit) = limit_uw.trim().parse::() { + if constraint_0.exists() + && let Ok(limit_uw) = fs::read_to_string(&constraint_0) + && let Ok(limit) = limit_uw.trim().parse::() { println!("CPU TDP Limit: {}W (Intel RAPL PL1)", limit / 1_000_000); tdp_found = true; break; } - } - } } } } - } // Check AMD HWMon if !tdp_found { let hwmon_dir = Path::new("/sys/class/hwmon"); - if hwmon_dir.exists() { - if let Ok(entries) = fs::read_dir(hwmon_dir) { + if hwmon_dir.exists() + && let Ok(entries) = fs::read_dir(hwmon_dir) { for entry in entries.flatten() { let path = entry.path(); let power1_cap = path.join("power1_cap"); let name_path = path.join("name"); - if power1_cap.exists() { - if let Ok(name) = fs::read_to_string(&name_path) { - if name.trim().contains("amd") { - if let Ok(limit_uw) = fs::read_to_string(&power1_cap) { - if let Ok(limit) = limit_uw.trim().parse::() { + if power1_cap.exists() + && let Ok(name) = fs::read_to_string(&name_path) + && name.trim().contains("amd") + && let Ok(limit_uw) = fs::read_to_string(&power1_cap) + && let Ok(limit) = limit_uw.trim().parse::() { println!( "CPU TDP Limit: {}W (AMD hwmon)", limit / 1_000_000 @@ -148,13 +142,8 @@ pub fn execute() { tdp_found = true; break; } - } - } - } - } } } - } } if !tdp_found {