From 984793d445ab22712d4003341fc09a1f2325c5a3 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 07:27:19 +0300 Subject: [PATCH 01/13] daemon: historical interval blending --- src/daemon.rs | 70 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 11 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 9881110..f7ef7cb 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -60,7 +60,7 @@ fn idle_multiplier(idle_secs: u64) -> f32 { } /// Calculate optimal polling interval based on system conditions and history -fn compute_new(params: &IntervalParams) -> u64 { +fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { // Start with base interval let mut adjusted_interval = params.base_interval; @@ -118,7 +118,19 @@ fn compute_new(params: &IntervalParams) -> u64 { } // Ensure interval stays within configured bounds - adjusted_interval.clamp(params.min_interval, params.max_interval) + let new_interval = adjusted_interval.clamp(params.min_interval, params.max_interval); + + // Blend the new interval with the cached value if available + let blended_interval = if let Some(cached) = system_history.last_computed_interval { + // Use a weighted average: 70% previous value, 30% new value + // This smooths out drastic changes in polling frequency + (cached as f32).mul_add(0.7, new_interval as f32 * 0.3).round() as u64 + } else { + new_interval + }; + + // Blended result still needs to respect the configured bounds + blended_interval.clamp(params.min_interval, params.max_interval) } /// Tracks historical system data for "advanced" adaptive polling @@ -142,6 +154,12 @@ struct SystemHistory { last_state_change: Instant, /// Current system state current_state: SystemState, + /// Last computed optimal polling interval + last_computed_interval: Option, + /// Last measured CPU volatility value + last_cpu_volatility: f32, + /// Last measured idle time in seconds + last_idle_secs: u64, } impl Default for SystemHistory { @@ -156,6 +174,9 @@ impl Default for SystemHistory { state_durations: std::collections::HashMap::new(), last_state_change: Instant::now(), current_state: SystemState::default(), + last_computed_interval: None, + last_cpu_volatility: 0.0, + last_idle_secs: 0, } } } @@ -338,7 +359,7 @@ impl SystemHistory { on_battery, }; - compute_new(¶ms) + compute_new(¶ms, self) } } @@ -491,15 +512,42 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> // Calculate optimal polling interval if adaptive polling is enabled if config.daemon.adaptive_interval { - let optimal_interval = - system_history.calculate_optimal_interval(&config, on_battery); + let current_cpu_volatility = system_history.get_cpu_volatility(); + let current_idle_secs = system_history.last_user_activity.elapsed().as_secs(); - // Don't change the interval too dramatically at once - if optimal_interval > current_poll_interval { - current_poll_interval = (current_poll_interval + optimal_interval) / 2; - } else if optimal_interval < current_poll_interval { - current_poll_interval = current_poll_interval - - ((current_poll_interval - optimal_interval) / 2).max(1); + // Only recalculate if there's a significant change in system metrics + // or if we haven't computed an interval yet + if system_history.last_computed_interval.is_none() + || (system_history.last_cpu_volatility - current_cpu_volatility).abs() > 1.0 + || (system_history.last_idle_secs as i64 - current_idle_secs as i64).abs() + > 10 + { + let optimal_interval = + system_history.calculate_optimal_interval(&config, on_battery); + + // Store the new interval and update cached metrics + system_history.last_computed_interval = Some(optimal_interval); + system_history.last_cpu_volatility = current_cpu_volatility; + system_history.last_idle_secs = current_idle_secs; + + debug!( + "Recalculated optimal interval: {optimal_interval}s (significant system change detected)" + ); + + // Don't change the interval too dramatically at once + if optimal_interval > current_poll_interval { + current_poll_interval = (current_poll_interval + optimal_interval) / 2; + } else if optimal_interval < current_poll_interval { + current_poll_interval = current_poll_interval + - ((current_poll_interval - optimal_interval) / 2).max(1); + } + } else { + debug!( + "Using cached optimal interval: {}s (no significant system change)", + system_history + .last_computed_interval + .unwrap_or(current_poll_interval) + ); } // Make sure that we respect the (user) configured min and max limits From ad70a8542655338cd385bea65d7eccf9bc987a07 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 07:43:01 +0300 Subject: [PATCH 02/13] daemon: use integer arithmetic to avoid precision loss --- src/daemon.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/daemon.rs b/src/daemon.rs index f7ef7cb..16b3548 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -124,7 +124,10 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { let blended_interval = if let Some(cached) = system_history.last_computed_interval { // Use a weighted average: 70% previous value, 30% new value // This smooths out drastic changes in polling frequency - (cached as f32).mul_add(0.7, new_interval as f32 * 0.3).round() as u64 + // We use integer arithmetic to avoid precision loss with large interval values + // I think Clippy warned me about this at some point and I ignored it. Now I come + // crawling back to it... + (cached * 7 + new_interval * 3) / 10 } else { new_interval }; From 8c462868b6c50ae2e5e4234a3dae12af83f18e09 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 07:52:32 +0300 Subject: [PATCH 03/13] daemon: use `abs_diff` to remove lossy casts --- src/daemon.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 16b3548..b26acf3 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -522,8 +522,7 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> // or if we haven't computed an interval yet if system_history.last_computed_interval.is_none() || (system_history.last_cpu_volatility - current_cpu_volatility).abs() > 1.0 - || (system_history.last_idle_secs as i64 - current_idle_secs as i64).abs() - > 10 + || u64::abs_diff(system_history.last_idle_secs, current_idle_secs) > 10 { let optimal_interval = system_history.calculate_optimal_interval(&config, on_battery); From 34862b28e10819b2c47bb0512b98b424fd39adef Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 08:11:46 +0300 Subject: [PATCH 04/13] daemon: enforce >= 1s minimum interval in clamping --- src/daemon.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index b26acf3..71edc06 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -118,22 +118,23 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { } // Ensure interval stays within configured bounds - let new_interval = adjusted_interval.clamp(params.min_interval, params.max_interval); + // Enforce a minimum of 1 second to prevent busy loops, regardless of params.min_interval + let min_safe_interval = params.min_interval.max(1); + let new_interval = adjusted_interval.clamp(min_safe_interval, params.max_interval); // Blend the new interval with the cached value if available let blended_interval = if let Some(cached) = system_history.last_computed_interval { // Use a weighted average: 70% previous value, 30% new value // This smooths out drastic changes in polling frequency - // We use integer arithmetic to avoid precision loss with large interval values - // I think Clippy warned me about this at some point and I ignored it. Now I come - // crawling back to it... + // Use integer arithmetic to avoid precision loss with large interval values (cached * 7 + new_interval * 3) / 10 } else { new_interval }; // Blended result still needs to respect the configured bounds - blended_interval.clamp(params.min_interval, params.max_interval) + // Again enforce minimum of 1 second regardless of params.min_interval + blended_interval.clamp(min_safe_interval, params.max_interval) } /// Tracks historical system data for "advanced" adaptive polling From 2efc0e1a6b518bb0ce7ce482b68e8978ba9549a7 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 08:27:53 +0300 Subject: [PATCH 05/13] daemon: use u128 arithmetic to prevent overflow in polling interval calc Overflows are unlikely to happen, but easy to prevent. Let's fix it while we're at it. --- src/daemon.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 71edc06..630bcbe 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -126,8 +126,10 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { let blended_interval = if let Some(cached) = system_history.last_computed_interval { // Use a weighted average: 70% previous value, 30% new value // This smooths out drastic changes in polling frequency - // Use integer arithmetic to avoid precision loss with large interval values - (cached * 7 + new_interval * 3) / 10 + // XXX: Use u128 arithmetic to avoid overflow with large interval values + let result = (cached as u128 * 7 + new_interval as u128 * 3) / 10; + + result as u64 } else { new_interval }; @@ -538,11 +540,18 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> ); // Don't change the interval too dramatically at once - if optimal_interval > current_poll_interval { - current_poll_interval = (current_poll_interval + optimal_interval) / 2; - } else if optimal_interval < current_poll_interval { - current_poll_interval = current_poll_interval - - ((current_poll_interval - optimal_interval) / 2).max(1); + match optimal_interval.cmp(¤t_poll_interval) { + std::cmp::Ordering::Greater => { + current_poll_interval = + (current_poll_interval + optimal_interval) / 2; + } + std::cmp::Ordering::Less => { + current_poll_interval = current_poll_interval + - ((current_poll_interval - optimal_interval) / 2).max(1); + } + std::cmp::Ordering::Equal => { + // No change needed when they're equal + } } } else { debug!( From 63cab6e631afa81c580e2a163c2d19c83eac99c6 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 08:36:51 +0300 Subject: [PATCH 06/13] daemon: clean up weighted average calculation --- src/daemon.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/daemon.rs b/src/daemon.rs index 630bcbe..3b91bb5 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -126,8 +126,14 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { let blended_interval = if let Some(cached) = system_history.last_computed_interval { // Use a weighted average: 70% previous value, 30% new value // This smooths out drastic changes in polling frequency + const PREVIOUS_VALUE_WEIGHT: u128 = 7; // 70% + const NEW_VALUE_WEIGHT: u128 = 3; // 30% + const TOTAL_WEIGHT: u128 = PREVIOUS_VALUE_WEIGHT + NEW_VALUE_WEIGHT; // 10 + // XXX: Use u128 arithmetic to avoid overflow with large interval values - let result = (cached as u128 * 7 + new_interval as u128 * 3) / 10; + let result = (cached as u128 * PREVIOUS_VALUE_WEIGHT + + new_interval as u128 * NEW_VALUE_WEIGHT) + / TOTAL_WEIGHT; result as u64 } else { From fe93a50f9e298b8434f4d71a68824db887477be9 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 11:23:37 +0300 Subject: [PATCH 07/13] treewide: suppress clippy lint about `Error` suffix --- src/battery.rs | 2 +- src/config/load.rs | 6 +++--- src/config/types.rs | 22 +++++++++++----------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/battery.rs b/src/battery.rs index 9beb05e..8fe75dd 100644 --- a/src/battery.rs +++ b/src/battery.rs @@ -74,7 +74,7 @@ pub fn set_battery_charge_thresholds(start_threshold: u8, stop_threshold: u8) -> // Validate thresholds using `BatteryChargeThresholds` let thresholds = BatteryChargeThresholds::new(start_threshold, stop_threshold).map_err(|e| match e { - crate::config::types::ConfigError::ValidationError(msg) => { + crate::config::types::ConfigError::Validation(msg) => { ControlError::InvalidValueError(msg) } _ => ControlError::InvalidValueError(format!("Invalid battery threshold values: {e}")), diff --git a/src/config/load.rs b/src/config/load.rs index 3ac5ca1..b374a32 100644 --- a/src/config/load.rs +++ b/src/config/load.rs @@ -26,7 +26,7 @@ pub fn load_config_from_path(specific_path: Option<&str>) -> Result) -> Result Result { - let contents = fs::read_to_string(path).map_err(ConfigError::IoError)?; + let contents = fs::read_to_string(path).map_err(ConfigError::Io)?; let toml_app_config = - toml::from_str::(&contents).map_err(ConfigError::TomlError)?; + toml::from_str::(&contents).map_err(ConfigError::Toml)?; // Handle inheritance of values from global to profile configs let mut charger_profile = toml_app_config.charger.clone(); diff --git a/src/config/types.rs b/src/config/types.rs index 03711c1..b2f6091 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -23,17 +23,17 @@ pub struct BatteryChargeThresholds { impl BatteryChargeThresholds { pub fn new(start: u8, stop: u8) -> Result { if stop == 0 { - return Err(ConfigError::ValidationError( + return Err(ConfigError::Validation( "Stop threshold must be greater than 0%".to_string(), )); } if start >= stop { - return Err(ConfigError::ValidationError(format!( + return Err(ConfigError::Validation(format!( "Start threshold ({start}) must be less than stop threshold ({stop})" ))); } if stop > 100 { - return Err(ConfigError::ValidationError(format!( + return Err(ConfigError::Validation(format!( "Stop threshold ({stop}) cannot exceed 100%" ))); } @@ -100,29 +100,29 @@ pub struct AppConfig { // Error type for config loading #[derive(Debug)] pub enum ConfigError { - IoError(std::io::Error), - TomlError(toml::de::Error), - ValidationError(String), + Io(std::io::Error), + Toml(toml::de::Error), + Validation(String), } impl From for ConfigError { fn from(err: std::io::Error) -> Self { - Self::IoError(err) + Self::Io(err) } } impl From for ConfigError { fn from(err: toml::de::Error) -> Self { - Self::TomlError(err) + Self::Toml(err) } } impl std::fmt::Display for ConfigError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::IoError(e) => write!(f, "I/O error: {e}"), - Self::TomlError(e) => write!(f, "TOML parsing error: {e}"), - Self::ValidationError(s) => write!(f, "Configuration validation error: {s}"), + Self::Io(e) => write!(f, "I/O error: {e}"), + Self::Toml(e) => write!(f, "TOML parsing error: {e}"), + Self::Validation(s) => write!(f, "Configuration validation error: {s}"), } } } From ee5bd8f80ff67f1249785d2aa72bab1e6b41aa4e Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 12:13:42 +0300 Subject: [PATCH 08/13] daemon: handle invalid config values gracefully --- src/config/load.rs | 3 +- src/daemon.rs | 111 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 87 insertions(+), 27 deletions(-) diff --git a/src/config/load.rs b/src/config/load.rs index b374a32..51f7e22 100644 --- a/src/config/load.rs +++ b/src/config/load.rs @@ -82,8 +82,7 @@ pub fn load_config_from_path(specific_path: Option<&str>) -> Result Result { let contents = fs::read_to_string(path).map_err(ConfigError::Io)?; - let toml_app_config = - toml::from_str::(&contents).map_err(ConfigError::Toml)?; + let toml_app_config = toml::from_str::(&contents).map_err(ConfigError::Toml)?; // Handle inheritance of values from global to profile configs let mut charger_profile = toml_app_config.charger.clone(); diff --git a/src/daemon.rs b/src/daemon.rs index 3b91bb5..b686076 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -60,7 +60,25 @@ fn idle_multiplier(idle_secs: u64) -> f32 { } /// Calculate optimal polling interval based on system conditions and history -fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { +/// +/// Returns Ok with the calculated interval, or Err if the configuration is invalid +fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Result { + // Catch invalid configurations during development + debug_assert!( + params.max_interval >= params.min_interval, + "Configuration error: max_interval ({}) < min_interval ({})", + params.max_interval, + params.min_interval + ); + + // Return an error on invalid configuration + if !validate_poll_intervals(params.min_interval, params.max_interval) { + return Err(format!( + "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", + params.max_interval, params.min_interval + )); + } + // Start with base interval let mut adjusted_interval = params.base_interval; @@ -117,7 +135,6 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { adjusted_interval = (adjusted_interval / 2).max(1); } - // Ensure interval stays within configured bounds // Enforce a minimum of 1 second to prevent busy loops, regardless of params.min_interval let min_safe_interval = params.min_interval.max(1); let new_interval = adjusted_interval.clamp(min_safe_interval, params.max_interval); @@ -142,7 +159,7 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> u64 { // Blended result still needs to respect the configured bounds // Again enforce minimum of 1 second regardless of params.min_interval - blended_interval.clamp(min_safe_interval, params.max_interval) + Ok(blended_interval.clamp(min_safe_interval, params.max_interval)) } /// Tracks historical system data for "advanced" adaptive polling @@ -358,7 +375,11 @@ impl SystemHistory { } /// Calculate optimal polling interval based on system conditions - fn calculate_optimal_interval(&self, config: &AppConfig, on_battery: bool) -> u64 { + fn calculate_optimal_interval( + &self, + config: &AppConfig, + on_battery: bool, + ) -> Result { let params = IntervalParams { base_interval: config.daemon.poll_interval_sec, min_interval: config.daemon.min_poll_interval_sec, @@ -375,6 +396,12 @@ impl SystemHistory { } } +/// Validates that poll interval configuration is consistent +/// Returns true if configuration is valid, false if invalid +fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> bool { + max_interval >= min_interval +} + /// Run the daemon pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> { // Set effective log level based on config and verbose flag @@ -397,6 +424,17 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> info!("Starting superfreq daemon..."); + // Validate critical configuration values before proceeding + if !validate_poll_intervals( + config.daemon.min_poll_interval_sec, + config.daemon.max_poll_interval_sec, + ) { + return Err(AppError::Generic(format!( + "Invalid configuration: max_poll_interval_sec ({}) is less than min_poll_interval_sec ({}). Please fix your configuration.", + config.daemon.max_poll_interval_sec, config.daemon.min_poll_interval_sec + ))); + } + // Create a flag that will be set to true when a signal is received let running = Arc::new(AtomicBool::new(true)); let r = running.clone(); @@ -465,6 +503,21 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> match config_result { Ok(new_config) => { info!("Config file changed, updating configuration"); + + // Validate min/max intervals in the new configuration + if !validate_poll_intervals( + new_config.daemon.min_poll_interval_sec, + new_config.daemon.max_poll_interval_sec, + ) { + error!( + "Invalid configuration loaded: max_poll_interval_sec ({}) is less than min_poll_interval_sec ({}). Keeping previous configuration.", + new_config.daemon.max_poll_interval_sec, + new_config.daemon.min_poll_interval_sec + ); + // Continue with the current configuration + continue; + } + config = new_config; // Reset polling interval after config change current_poll_interval = config.daemon.poll_interval_sec.max(1); @@ -533,30 +586,38 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> || (system_history.last_cpu_volatility - current_cpu_volatility).abs() > 1.0 || u64::abs_diff(system_history.last_idle_secs, current_idle_secs) > 10 { - let optimal_interval = - system_history.calculate_optimal_interval(&config, on_battery); + match system_history.calculate_optimal_interval(&config, on_battery) { + Ok(optimal_interval) => { + // Store the new interval and update cached metrics + system_history.last_computed_interval = Some(optimal_interval); + system_history.last_cpu_volatility = current_cpu_volatility; + system_history.last_idle_secs = current_idle_secs; - // Store the new interval and update cached metrics - system_history.last_computed_interval = Some(optimal_interval); - system_history.last_cpu_volatility = current_cpu_volatility; - system_history.last_idle_secs = current_idle_secs; + debug!( + "Recalculated optimal interval: {optimal_interval}s (significant system change detected)" + ); - debug!( - "Recalculated optimal interval: {optimal_interval}s (significant system change detected)" - ); - - // Don't change the interval too dramatically at once - match optimal_interval.cmp(¤t_poll_interval) { - std::cmp::Ordering::Greater => { - current_poll_interval = - (current_poll_interval + optimal_interval) / 2; + // Don't change the interval too dramatically at once + match optimal_interval.cmp(¤t_poll_interval) { + std::cmp::Ordering::Greater => { + current_poll_interval = + (current_poll_interval + optimal_interval) / 2; + } + std::cmp::Ordering::Less => { + current_poll_interval = current_poll_interval + - ((current_poll_interval - optimal_interval) / 2) + .max(1); + } + std::cmp::Ordering::Equal => { + // No change needed when they're equal + } + } } - std::cmp::Ordering::Less => { - current_poll_interval = current_poll_interval - - ((current_poll_interval - optimal_interval) / 2).max(1); - } - std::cmp::Ordering::Equal => { - // No change needed when they're equal + Err(e) => { + // Log the error and stop the daemon when an invalid configuration is detected + error!("Critical configuration error: {e}"); + running.store(false, Ordering::SeqCst); + break; } } } else { From 58273d89f4e4b2fbed7b8080e82287b0ecaa061f Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 13:27:18 +0300 Subject: [PATCH 09/13] daemon: get rid of config hot-reloading --- Cargo.lock | 167 ++---------------------------------------- Cargo.toml | 1 - src/config/mod.rs | 1 - src/config/watcher.rs | 130 -------------------------------- src/daemon.rs | 148 +++++++------------------------------ 5 files changed, 32 insertions(+), 415 deletions(-) delete mode 100644 src/config/watcher.rs diff --git a/Cargo.lock b/Cargo.lock index 94b1a9b..8233108 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -62,7 +62,7 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79947af37f4177cfead1110013d678905c37501914fba0efea834c3fe9a8d60c" dependencies = [ - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -73,7 +73,7 @@ checksum = "ca3534e77181a9cc07539ad51f2141fe32f6c3ffd4df76db8ad92346b003ae4e" dependencies = [ "anstyle", "once_cell", - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -82,12 +82,6 @@ version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26" -[[package]] -name = "bitflags" -version = "1.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" - [[package]] name = "bitflags" version = "2.9.0" @@ -194,7 +188,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46f93780a459b7d656ef7f071fe699c4d3d2cb201c4b24d085b6ddc505276e73" dependencies = [ "nix", - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -215,7 +209,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.59.0", + "windows-sys", ] [[package]] @@ -247,27 +241,6 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" -[[package]] -name = "filetime" -version = "0.2.25" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35c0522e981e68cbfa8c3f978441a5f34b30b96e146b33cd3359176b50fe8586" -dependencies = [ - "cfg-if", - "libc", - "libredox", - "windows-sys 0.59.0", -] - -[[package]] -name = "fsevent-sys" -version = "4.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76ee7a02da4d231650c7cea31349b889be2f45ddb3ef3032d2ec8185f6313fd2" -dependencies = [ - "libc", -] - [[package]] name = "getrandom" version = "0.2.16" @@ -331,26 +304,6 @@ dependencies = [ "hashbrown", ] -[[package]] -name = "inotify" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f37dccff2791ab604f9babef0ba14fbe0be30bd368dc541e2b08d07c8aa908f3" -dependencies = [ - "bitflags 2.9.0", - "inotify-sys", - "libc", -] - -[[package]] -name = "inotify-sys" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e05c02b5e89bff3b946cedeca278abc628fe811e604f027c45a8aa3cf793d0eb" -dependencies = [ - "libc", -] - [[package]] name = "is_terminal_polyfill" version = "1.70.1" @@ -391,26 +344,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "kqueue" -version = "1.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eac30106d7dce88daf4a3fcb4879ea939476d5074a9b7ddd0fb97fa4bed5596a" -dependencies = [ - "kqueue-sys", - "libc", -] - -[[package]] -name = "kqueue-sys" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed9625ffda8729b85e45cf04090035ac368927b8cebc34898e7c120f52e4838b" -dependencies = [ - "bitflags 1.3.2", - "libc", -] - [[package]] name = "libc" version = "0.2.172" @@ -423,9 +356,8 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ - "bitflags 2.9.0", + "bitflags", "libc", - "redox_syscall", ] [[package]] @@ -440,58 +372,18 @@ version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" -[[package]] -name = "mio" -version = "1.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2886843bf800fba2e3377cff24abf6379b4c4d5c6681eaf9ea5b0d15090450bd" -dependencies = [ - "libc", - "log", - "wasi", - "windows-sys 0.52.0", -] - [[package]] name = "nix" version = "0.30.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" dependencies = [ - "bitflags 2.9.0", + "bitflags", "cfg-if", "cfg_aliases", "libc", ] -[[package]] -name = "notify" -version = "8.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2fee8403b3d66ac7b26aee6e40a897d85dc5ce26f44da36b8b73e987cc52e943" -dependencies = [ - "bitflags 2.9.0", - "filetime", - "fsevent-sys", - "inotify", - "kqueue", - "libc", - "log", - "mio", - "notify-types", - "walkdir", - "windows-sys 0.59.0", -] - -[[package]] -name = "notify-types" -version = "2.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e0826a989adedc2a244799e823aece04662b66609d96af8dff7ac6df9a8925d" -dependencies = [ - "serde", -] - [[package]] name = "num-traits" version = "0.2.19" @@ -556,15 +448,6 @@ dependencies = [ "proc-macro2", ] -[[package]] -name = "redox_syscall" -version = "0.5.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "928fca9cf2aa042393a8325b9ead81d2f0df4cb12e1e24cef072922ccd99c5af" -dependencies = [ - "bitflags 2.9.0", -] - [[package]] name = "redox_users" version = "0.5.0" @@ -611,15 +494,6 @@ version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eded382c5f5f786b989652c49544c4877d9f015cc22e145a5ea8ea66c2921cd2" -[[package]] -name = "same-file" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" -dependencies = [ - "winapi-util", -] - [[package]] name = "serde" version = "1.0.219" @@ -671,7 +545,6 @@ dependencies = [ "dirs", "env_logger", "log", - "notify", "num_cpus", "serde", "toml", @@ -761,16 +634,6 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" -[[package]] -name = "walkdir" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29790946404f91d9c5d06f9874efddea1dc06c5efe94541a7d6863108e3a5e4b" -dependencies = [ - "same-file", - "winapi-util", -] - [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" @@ -835,15 +698,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "winapi-util" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" -dependencies = [ - "windows-sys 0.59.0", -] - [[package]] name = "windows-core" version = "0.61.0" @@ -903,15 +757,6 @@ dependencies = [ "windows-link", ] -[[package]] -name = "windows-sys" -version = "0.52.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" -dependencies = [ - "windows-targets", -] - [[package]] name = "windows-sys" version = "0.59.0" diff --git a/Cargo.toml b/Cargo.toml index 3605118..7740ebd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,6 @@ dirs = "6.0" clap = { version = "4.0", features = ["derive"] } num_cpus = "1.16" ctrlc = "3.4" -notify = { version = "8.0.0", features = ["serde"] } chrono = "0.4" log = "0.4" env_logger = "0.11" diff --git a/src/config/mod.rs b/src/config/mod.rs index 0a20a83..c2f3076 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,6 +1,5 @@ pub mod load; pub mod types; -pub mod watcher; pub use load::*; pub use types::*; diff --git a/src/config/watcher.rs b/src/config/watcher.rs deleted file mode 100644 index c362e0d..0000000 --- a/src/config/watcher.rs +++ /dev/null @@ -1,130 +0,0 @@ -use log::{debug, error, warn}; -use notify::{Config, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher}; -use std::error::Error; -use std::path::Path; -use std::sync::mpsc::{Receiver, TryRecvError, channel}; -use std::thread; -use std::time::Duration; - -use crate::config::{AppConfig, load_config_from_path}; - -/// Watches a configuration file for changes and reloads it when modified -pub struct ConfigWatcher { - rx: Receiver>, - _watcher: RecommendedWatcher, // keep watcher alive while watching - config_path: String, - last_event_time: std::time::Instant, -} - -impl ConfigWatcher { - /// Initialize a new config watcher for the given path - pub fn new(config_path: &str) -> Result { - let (tx, rx) = channel(); - - // Create a watcher with default config - let mut watcher = RecommendedWatcher::new(tx, Config::default())?; - - // Start watching the config file - watcher.watch(Path::new(config_path), RecursiveMode::NonRecursive)?; - - debug!("Started watching config file: {config_path}"); - - Ok(Self { - rx, - _watcher: watcher, - config_path: config_path.to_string(), - last_event_time: std::time::Instant::now(), - }) - } - - /// Check for config file changes and reload if necessary - /// - /// # Returns - /// - /// `Some(AppConfig)` if the config was reloaded, `None` otherwise - pub fn check_for_changes(&mut self) -> Option>> { - // Process all pending events before deciding to reload - let mut should_reload = false; - - loop { - match self.rx.try_recv() { - Ok(Ok(event)) => { - // Process various file events that might indicate configuration changes - match event.kind { - EventKind::Modify(_) => { - debug!("Detected modification to config file: {}", self.config_path); - should_reload = true; - } - EventKind::Create(_) => { - debug!("Detected recreation of config file: {}", self.config_path); - should_reload = true; - } - EventKind::Remove(_) => { - // Some editors delete then recreate the file when saving - // Just log this event and wait for the create event - debug!( - "Detected removal of config file: {} - waiting for recreation", - self.config_path - ); - } - _ => {} // Ignore other event types - } - - if should_reload { - self.last_event_time = std::time::Instant::now(); - } - } - Ok(Err(e)) => { - // File watcher error, log but continue - warn!("Error watching config file: {e}"); - } - Err(TryRecvError::Empty) => { - // No more events - break; - } - Err(TryRecvError::Disconnected) => { - // Channel disconnected, watcher is dead - error!("Config watcher channel disconnected"); - return None; - } - } - } - - // Debounce rapid file changes (e.g., from editors that write multiple times) - if should_reload { - // Wait to ensure file writing is complete - let debounce_time = Duration::from_millis(250); - let time_since_last_event = self.last_event_time.elapsed(); - - if time_since_last_event < debounce_time { - thread::sleep(debounce_time - time_since_last_event); - } - - // Ensure the file exists before attempting to reload - let config_path = Path::new(&self.config_path); - if !config_path.exists() { - warn!( - "Config file does not exist after change events: {}", - self.config_path - ); - return None; - } - - debug!("Reloading configuration from {}", self.config_path); - - // Attempt to reload the config from the specific path being watched - match load_config_from_path(Some(&self.config_path)) { - Ok(config) => { - debug!("Successfully reloaded configuration"); - Some(Ok(config)) - } - Err(e) => { - error!("Failed to reload configuration: {e}"); - Some(Err(Box::new(e))) - } - } - } else { - None - } - } -} diff --git a/src/daemon.rs b/src/daemon.rs index b686076..27748fd 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1,4 +1,3 @@ -use crate::config::watcher::ConfigWatcher; use crate::config::{AppConfig, LogLevel}; use crate::core::SystemReport; use crate::engine; @@ -71,14 +70,6 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Resul params.min_interval ); - // Return an error on invalid configuration - if !validate_poll_intervals(params.min_interval, params.max_interval) { - return Err(format!( - "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", - params.max_interval, params.min_interval - )); - } - // Start with base interval let mut adjusted_interval = params.base_interval; @@ -403,7 +394,7 @@ fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> bool { } /// Run the daemon -pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> { +pub fn run_daemon(config: AppConfig, verbose: bool) -> Result<(), AppError> { // Set effective log level based on config and verbose flag let effective_log_level = if verbose { LogLevel::Debug @@ -456,35 +447,6 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> info!("Stats will be written to: {stats_path}"); } - // Initialize config file watcher if a path is available - let config_file_path = if let Ok(path) = std::env::var("SUPERFREQ_CONFIG") { - Some(path) - } else { - // Check standard config paths - let default_paths = ["/etc/xdg/superfreq/config.toml", "/etc/superfreq.toml"]; - - default_paths - .iter() - .find(|&path| std::path::Path::new(path).exists()) - .map(|path| (*path).to_string()) - }; - - let mut config_watcher = if let Some(path) = config_file_path { - match ConfigWatcher::new(&path) { - Ok(watcher) => { - info!("Watching config file: {path}"); - Some(watcher) - } - Err(e) => { - warn!("Failed to initialize config file watcher: {e}"); - None - } - } - } else { - warn!("No config file found to watch for changes."); - None - }; - // Variables for adaptive polling // Make sure that the poll interval is *never* zero to prevent a busy loop let mut current_poll_interval = config.daemon.poll_interval_sec.max(1); @@ -497,46 +459,6 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> while running.load(Ordering::SeqCst) { let start_time = Instant::now(); - // Check for configuration changes - if let Some(watcher) = &mut config_watcher { - if let Some(config_result) = watcher.check_for_changes() { - match config_result { - Ok(new_config) => { - info!("Config file changed, updating configuration"); - - // Validate min/max intervals in the new configuration - if !validate_poll_intervals( - new_config.daemon.min_poll_interval_sec, - new_config.daemon.max_poll_interval_sec, - ) { - error!( - "Invalid configuration loaded: max_poll_interval_sec ({}) is less than min_poll_interval_sec ({}). Keeping previous configuration.", - new_config.daemon.max_poll_interval_sec, - new_config.daemon.min_poll_interval_sec - ); - // Continue with the current configuration - continue; - } - - config = new_config; - // Reset polling interval after config change - current_poll_interval = config.daemon.poll_interval_sec.max(1); - if config.daemon.poll_interval_sec == 0 { - warn!( - "Poll interval is set to zero in updated config, using 1s minimum to prevent a busy loop" - ); - } - // Mark this as a system event for adaptive polling - system_history.last_user_activity = Instant::now(); - } - Err(e) => { - error!("Error loading new configuration: {e}"); - // Continue with existing config - } - } - } - } - match monitor::collect_system_report(&config) { Ok(report) => { debug!("Collected system report, applying settings..."); @@ -580,53 +502,36 @@ pub fn run_daemon(mut config: AppConfig, verbose: bool) -> Result<(), AppError> let current_cpu_volatility = system_history.get_cpu_volatility(); let current_idle_secs = system_history.last_user_activity.elapsed().as_secs(); - // Only recalculate if there's a significant change in system metrics - // or if we haven't computed an interval yet - if system_history.last_computed_interval.is_none() - || (system_history.last_cpu_volatility - current_cpu_volatility).abs() > 1.0 - || u64::abs_diff(system_history.last_idle_secs, current_idle_secs) > 10 - { - match system_history.calculate_optimal_interval(&config, on_battery) { - Ok(optimal_interval) => { - // Store the new interval and update cached metrics - system_history.last_computed_interval = Some(optimal_interval); - system_history.last_cpu_volatility = current_cpu_volatility; - system_history.last_idle_secs = current_idle_secs; + match system_history.calculate_optimal_interval(&config, on_battery) { + Ok(optimal_interval) => { + // Store the new interval and update cached metrics + system_history.last_computed_interval = Some(optimal_interval); + system_history.last_cpu_volatility = current_cpu_volatility; + system_history.last_idle_secs = current_idle_secs; - debug!( - "Recalculated optimal interval: {optimal_interval}s (significant system change detected)" - ); + debug!("Recalculated optimal interval: {optimal_interval}s"); - // Don't change the interval too dramatically at once - match optimal_interval.cmp(¤t_poll_interval) { - std::cmp::Ordering::Greater => { - current_poll_interval = - (current_poll_interval + optimal_interval) / 2; - } - std::cmp::Ordering::Less => { - current_poll_interval = current_poll_interval - - ((current_poll_interval - optimal_interval) / 2) - .max(1); - } - std::cmp::Ordering::Equal => { - // No change needed when they're equal - } + // Don't change the interval too dramatically at once + match optimal_interval.cmp(¤t_poll_interval) { + std::cmp::Ordering::Greater => { + current_poll_interval = + (current_poll_interval + optimal_interval) / 2; + } + std::cmp::Ordering::Less => { + current_poll_interval = current_poll_interval + - ((current_poll_interval - optimal_interval) / 2).max(1); + } + std::cmp::Ordering::Equal => { + // No change needed when they're equal } } - Err(e) => { - // Log the error and stop the daemon when an invalid configuration is detected - error!("Critical configuration error: {e}"); - running.store(false, Ordering::SeqCst); - break; - } } - } else { - debug!( - "Using cached optimal interval: {}s (no significant system change)", - system_history - .last_computed_interval - .unwrap_or(current_poll_interval) - ); + Err(e) => { + // Log the error and stop the daemon when an invalid configuration is detected + error!("Critical configuration error: {e}"); + running.store(false, Ordering::SeqCst); + break; + } } // Make sure that we respect the (user) configured min and max limits @@ -687,7 +592,6 @@ fn write_stats_file(path: &str, report: &SystemReport) -> Result<(), std::io::Er // CPU info writeln!(file, "governor={:?}", report.cpu_global.current_governor)?; writeln!(file, "turbo={:?}", report.cpu_global.turbo_status)?; - if let Some(temp) = report.cpu_global.average_temperature_celsius { writeln!(file, "cpu_temp={temp:.1}")?; } From 747362b88be9ecbf1e1b3440932bf277be315ac3 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 13:38:33 +0300 Subject: [PATCH 10/13] daemon: replace `debug_assert` with an actual error --- src/daemon.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 27748fd..9ce40a0 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -62,13 +62,13 @@ fn idle_multiplier(idle_secs: u64) -> f32 { /// /// Returns Ok with the calculated interval, or Err if the configuration is invalid fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Result { - // Catch invalid configurations during development - debug_assert!( - params.max_interval >= params.min_interval, - "Configuration error: max_interval ({}) < min_interval ({})", - params.max_interval, - params.min_interval - ); + // Validate interval configuration + if params.max_interval < params.min_interval { + return Err(format!( + "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", + params.max_interval, params.min_interval + )); + } // Start with base interval let mut adjusted_interval = params.base_interval; From 38960ce8bc476ec889d56bff4e2d4bcab172b11d Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 14:08:52 +0300 Subject: [PATCH 11/13] daemon: don't store unused fields --- src/daemon.rs | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 9ce40a0..df99009 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -62,13 +62,8 @@ fn idle_multiplier(idle_secs: u64) -> f32 { /// /// Returns Ok with the calculated interval, or Err if the configuration is invalid fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Result { - // Validate interval configuration - if params.max_interval < params.min_interval { - return Err(format!( - "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", - params.max_interval, params.min_interval - )); - } + // Use the centralized validation function + validate_poll_intervals(params.min_interval, params.max_interval)?; // Start with base interval let mut adjusted_interval = params.base_interval; @@ -176,10 +171,6 @@ struct SystemHistory { current_state: SystemState, /// Last computed optimal polling interval last_computed_interval: Option, - /// Last measured CPU volatility value - last_cpu_volatility: f32, - /// Last measured idle time in seconds - last_idle_secs: u64, } impl Default for SystemHistory { @@ -195,8 +186,6 @@ impl Default for SystemHistory { last_state_change: Instant::now(), current_state: SystemState::default(), last_computed_interval: None, - last_cpu_volatility: 0.0, - last_idle_secs: 0, } } } @@ -388,9 +377,16 @@ impl SystemHistory { } /// Validates that poll interval configuration is consistent -/// Returns true if configuration is valid, false if invalid -fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> bool { - max_interval >= min_interval +/// Returns Ok if configuration is valid, Err with a descriptive message if invalid +fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> Result<(), String> { + if max_interval >= min_interval { + Ok(()) + } else { + Err(format!( + "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", + max_interval, min_interval + )) + } } /// Run the daemon @@ -416,13 +412,13 @@ pub fn run_daemon(config: AppConfig, verbose: bool) -> Result<(), AppError> { info!("Starting superfreq daemon..."); // Validate critical configuration values before proceeding - if !validate_poll_intervals( + if let Err(err) = validate_poll_intervals( config.daemon.min_poll_interval_sec, config.daemon.max_poll_interval_sec, ) { return Err(AppError::Generic(format!( - "Invalid configuration: max_poll_interval_sec ({}) is less than min_poll_interval_sec ({}). Please fix your configuration.", - config.daemon.max_poll_interval_sec, config.daemon.min_poll_interval_sec + "Invalid configuration: {}. Please fix your configuration.", + err ))); } @@ -499,15 +495,10 @@ pub fn run_daemon(config: AppConfig, verbose: bool) -> Result<(), AppError> { // Calculate optimal polling interval if adaptive polling is enabled if config.daemon.adaptive_interval { - let current_cpu_volatility = system_history.get_cpu_volatility(); - let current_idle_secs = system_history.last_user_activity.elapsed().as_secs(); - match system_history.calculate_optimal_interval(&config, on_battery) { Ok(optimal_interval) => { - // Store the new interval and update cached metrics + // Store the new interval system_history.last_computed_interval = Some(optimal_interval); - system_history.last_cpu_volatility = current_cpu_volatility; - system_history.last_idle_secs = current_idle_secs; debug!("Recalculated optimal interval: {optimal_interval}s"); From 99fbfc0ea710894d8407fc3914de2641e1394dde Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 14:26:42 +0300 Subject: [PATCH 12/13] daemon: use internal error types --- src/daemon.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index df99009..21c1425 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -2,7 +2,7 @@ use crate::config::{AppConfig, LogLevel}; use crate::core::SystemReport; use crate::engine; use crate::monitor; -use crate::util::error::AppError; +use crate::util::error::{AppError, ControlError}; use log::{LevelFilter, debug, error, info, warn}; use std::collections::VecDeque; use std::fs::File; @@ -61,7 +61,7 @@ fn idle_multiplier(idle_secs: u64) -> f32 { /// Calculate optimal polling interval based on system conditions and history /// /// Returns Ok with the calculated interval, or Err if the configuration is invalid -fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Result { +fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Result { // Use the centralized validation function validate_poll_intervals(params.min_interval, params.max_interval)?; @@ -359,7 +359,7 @@ impl SystemHistory { &self, config: &AppConfig, on_battery: bool, - ) -> Result { + ) -> Result { let params = IntervalParams { base_interval: config.daemon.poll_interval_sec, min_interval: config.daemon.min_poll_interval_sec, @@ -378,14 +378,24 @@ impl SystemHistory { /// Validates that poll interval configuration is consistent /// Returns Ok if configuration is valid, Err with a descriptive message if invalid -fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> Result<(), String> { +fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> Result<(), ControlError> { + if min_interval < 1 { + return Err(ControlError::InvalidValueError( + "min_interval must be ≥ 1".to_string() + )); + } + if max_interval < 1 { + return Err(ControlError::InvalidValueError( + "max_interval must be ≥ 1".to_string() + )); + } if max_interval >= min_interval { Ok(()) } else { - Err(format!( + Err(ControlError::InvalidValueError(format!( "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", max_interval, min_interval - )) + ))) } } @@ -416,10 +426,7 @@ pub fn run_daemon(config: AppConfig, verbose: bool) -> Result<(), AppError> { config.daemon.min_poll_interval_sec, config.daemon.max_poll_interval_sec, ) { - return Err(AppError::Generic(format!( - "Invalid configuration: {}. Please fix your configuration.", - err - ))); + return Err(AppError::Control(err)); } // Create a flag that will be set to true when a signal is received From 32422f2b4ff19cd1773ca25a381dd573c45423d6 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Sun, 18 May 2025 15:39:49 +0300 Subject: [PATCH 13/13] treewide: streamline error handling; leverage thiserror and anyhow --- Cargo.lock | 8 ++ Cargo.toml | 2 + src/config/types.rs | 35 ++----- src/cpu.rs | 4 +- src/daemon.rs | 16 ++-- src/engine.rs | 2 +- src/util/error.rs | 216 +++++++++++--------------------------------- 7 files changed, 81 insertions(+), 202 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8233108..2602893 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -76,6 +76,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "anyhow" +version = "1.0.98" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487" + [[package]] name = "autocfg" version = "1.4.0" @@ -539,6 +545,7 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" name = "superfreq" version = "0.3.1" dependencies = [ + "anyhow", "chrono", "clap", "ctrlc", @@ -547,6 +554,7 @@ dependencies = [ "log", "num_cpus", "serde", + "thiserror", "toml", ] diff --git a/Cargo.toml b/Cargo.toml index 7740ebd..25f79e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,3 +16,5 @@ ctrlc = "3.4" chrono = "0.4" log = "0.4" env_logger = "0.11" +thiserror = "2.0" +anyhow = "1.0" diff --git a/src/config/types.rs b/src/config/types.rs index b2f6091..eb9ce7f 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -98,37 +98,18 @@ pub struct AppConfig { } // Error type for config loading -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ConfigError { - Io(std::io::Error), - Toml(toml::de::Error), + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), + + #[error("TOML parsing error: {0}")] + Toml(#[from] toml::de::Error), + + #[error("Configuration validation error: {0}")] Validation(String), } -impl From for ConfigError { - fn from(err: std::io::Error) -> Self { - Self::Io(err) - } -} - -impl From for ConfigError { - fn from(err: toml::de::Error) -> Self { - Self::Toml(err) - } -} - -impl std::fmt::Display for ConfigError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Io(e) => write!(f, "I/O error: {e}"), - Self::Toml(e) => write!(f, "TOML parsing error: {e}"), - Self::Validation(s) => write!(f, "Configuration validation error: {s}"), - } - } -} - -impl std::error::Error for ConfigError {} - // Intermediate structs for TOML parsing #[derive(Deserialize, Serialize, Debug, Clone)] pub struct ProfileConfigToml { diff --git a/src/cpu.rs b/src/cpu.rs index cbd37f8..5629df3 100644 --- a/src/cpu.rs +++ b/src/cpu.rs @@ -109,7 +109,7 @@ pub fn set_governor(governor: &str, core_id: Option) -> Result<()> { let (is_valid, available_governors) = is_governor_valid(governor)?; if !is_valid { - return Err(ControlError::InvalidValueError(format!( + return Err(ControlError::InvalidGovernor(format!( "Governor '{}' is not available on this system. Valid governors: {}", governor, available_governors.join(", ") @@ -432,7 +432,7 @@ fn read_sysfs_value_as_u32(path: &str) -> Result { content .trim() .parse::() - .map_err(|e| ControlError::ReadError(format!("Failed to parse value from {path}: {e}"))) + .map_err(|e| ControlError::ParseError(format!("Failed to parse value from {path}: {e}"))) } fn validate_min_frequency(core_id: u32, new_min_freq_mhz: u32) -> Result<()> { diff --git a/src/daemon.rs b/src/daemon.rs index 21c1425..dd90884 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -61,7 +61,10 @@ fn idle_multiplier(idle_secs: u64) -> f32 { /// Calculate optimal polling interval based on system conditions and history /// /// Returns Ok with the calculated interval, or Err if the configuration is invalid -fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Result { +fn compute_new( + params: &IntervalParams, + system_history: &SystemHistory, +) -> Result { // Use the centralized validation function validate_poll_intervals(params.min_interval, params.max_interval)?; @@ -134,8 +137,8 @@ fn compute_new(params: &IntervalParams, system_history: &SystemHistory) -> Resul const TOTAL_WEIGHT: u128 = PREVIOUS_VALUE_WEIGHT + NEW_VALUE_WEIGHT; // 10 // XXX: Use u128 arithmetic to avoid overflow with large interval values - let result = (cached as u128 * PREVIOUS_VALUE_WEIGHT - + new_interval as u128 * NEW_VALUE_WEIGHT) + let result = (u128::from(cached) * PREVIOUS_VALUE_WEIGHT + + u128::from(new_interval) * NEW_VALUE_WEIGHT) / TOTAL_WEIGHT; result as u64 @@ -381,20 +384,19 @@ impl SystemHistory { fn validate_poll_intervals(min_interval: u64, max_interval: u64) -> Result<(), ControlError> { if min_interval < 1 { return Err(ControlError::InvalidValueError( - "min_interval must be ≥ 1".to_string() + "min_interval must be ≥ 1".to_string(), )); } if max_interval < 1 { return Err(ControlError::InvalidValueError( - "max_interval must be ≥ 1".to_string() + "max_interval must be ≥ 1".to_string(), )); } if max_interval >= min_interval { Ok(()) } else { Err(ControlError::InvalidValueError(format!( - "Invalid interval configuration: max_interval ({}) is less than min_interval ({})", - max_interval, min_interval + "Invalid interval configuration: max_interval ({max_interval}) is less than min_interval ({min_interval})" ))) } } diff --git a/src/engine.rs b/src/engine.rs index 966cc20..bbefc86 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -207,7 +207,7 @@ pub fn determine_and_apply_settings( // Let set_governor handle the validation if let Err(e) = cpu::set_governor(governor, None) { // If the governor is not available, log a warning - if matches!(e, ControlError::InvalidValueError(_)) + if matches!(e, ControlError::InvalidGovernor(_)) || matches!(e, ControlError::NotSupported(_)) { warn!( diff --git a/src/util/error.rs b/src/util/error.rs index 8724d62..b91081f 100644 --- a/src/util/error.rs +++ b/src/util/error.rs @@ -1,194 +1,80 @@ use std::io; -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ControlError { - Io(io::Error), + #[error("I/O error: {0}")] + Io(#[from] io::Error), + + #[error("Failed to write to sysfs path: {0}")] WriteError(String), + + #[error("Failed to read sysfs path: {0}")] ReadError(String), + + #[error("Invalid value for setting: {0}")] InvalidValueError(String), + + #[error("Control action not supported: {0}")] NotSupported(String), + + #[error("Permission denied: {0}. Try running with sudo.")] PermissionDenied(String), + + #[error("Invalid platform control profile {0} supplied, please provide a valid one.")] InvalidProfile(String), + + #[error("Invalid governor: {0}")] InvalidGovernor(String), + + #[error("Failed to parse value: {0}")] ParseError(String), + + #[error("Path missing: {0}")] PathMissing(String), } -impl From for ControlError { - fn from(err: io::Error) -> Self { - match err.kind() { - io::ErrorKind::PermissionDenied => Self::PermissionDenied(err.to_string()), - _ => Self::Io(err), - } - } -} - -impl std::fmt::Display for ControlError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Io(e) => write!(f, "I/O error: {e}"), - Self::WriteError(s) => write!(f, "Failed to write to sysfs path: {s}"), - Self::ReadError(s) => write!(f, "Failed to read sysfs path: {s}"), - Self::InvalidValueError(s) => write!(f, "Invalid value for setting: {s}"), - Self::NotSupported(s) => write!(f, "Control action not supported: {s}"), - Self::PermissionDenied(s) => { - write!(f, "Permission denied: {s}. Try running with sudo.") - } - Self::InvalidProfile(s) => { - write!( - f, - "Invalid platform control profile {s} supplied, please provide a valid one." - ) - } - Self::InvalidGovernor(s) => { - write!(f, "Invalid governor: {s}") - } - Self::ParseError(s) => { - write!(f, "Failed to parse value: {s}") - } - Self::PathMissing(s) => { - write!(f, "Path missing: {s}") - } - } - } -} - -impl std::error::Error for ControlError {} - -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum SysMonitorError { - Io(io::Error), + #[error("I/O error: {0}")] + Io(#[from] io::Error), + + #[error("Failed to read sysfs path: {0}")] ReadError(String), + + #[error("Failed to parse value: {0}")] ParseError(String), + + #[error("Failed to parse /proc/stat: {0}")] ProcStatParseError(String), } -impl From for SysMonitorError { - fn from(err: io::Error) -> Self { - Self::Io(err) - } -} - -impl std::fmt::Display for SysMonitorError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Io(e) => write!(f, "I/O error: {e}"), - Self::ReadError(s) => write!(f, "Failed to read sysfs path: {s}"), - Self::ParseError(s) => write!(f, "Failed to parse value: {s}"), - Self::ProcStatParseError(s) => { - write!(f, "Failed to parse /proc/stat: {s}") - } - } - } -} - -impl std::error::Error for SysMonitorError {} - -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum EngineError { - ControlError(ControlError), + #[error("CPU control error: {0}")] + ControlError(#[from] ControlError), + + #[error("Configuration error: {0}")] ConfigurationError(String), } -impl From for EngineError { - fn from(err: ControlError) -> Self { - Self::ControlError(err) - } -} - -impl std::fmt::Display for EngineError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::ControlError(e) => write!(f, "CPU control error: {e}"), - Self::ConfigurationError(s) => write!(f, "Configuration error: {s}"), - } - } -} - -impl std::error::Error for EngineError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::ControlError(e) => Some(e), - Self::ConfigurationError(_) => None, - } - } -} - // A unified error type for the entire application -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum AppError { - Control(ControlError), - Monitor(SysMonitorError), - Engine(EngineError), - Config(crate::config::ConfigError), + #[error("{0}")] + Control(#[from] ControlError), + + #[error("{0}")] + Monitor(#[from] SysMonitorError), + + #[error("{0}")] + Engine(#[from] EngineError), + + #[error("{0}")] + Config(#[from] crate::config::ConfigError), + + #[error("{0}")] Generic(String), - Io(io::Error), -} -impl From for AppError { - fn from(err: ControlError) -> Self { - Self::Control(err) - } -} - -impl From for AppError { - fn from(err: SysMonitorError) -> Self { - Self::Monitor(err) - } -} - -impl From for AppError { - fn from(err: EngineError) -> Self { - Self::Engine(err) - } -} - -impl From for AppError { - fn from(err: crate::config::ConfigError) -> Self { - Self::Config(err) - } -} - -impl From for AppError { - fn from(err: io::Error) -> Self { - Self::Io(err) - } -} - -impl From for AppError { - fn from(err: String) -> Self { - Self::Generic(err) - } -} - -impl From<&str> for AppError { - fn from(err: &str) -> Self { - Self::Generic(err.to_string()) - } -} - -impl std::fmt::Display for AppError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Control(e) => write!(f, "{e}"), - Self::Monitor(e) => write!(f, "{e}"), - Self::Engine(e) => write!(f, "{e}"), - Self::Config(e) => write!(f, "{e}"), - Self::Generic(s) => write!(f, "{s}"), - Self::Io(e) => write!(f, "I/O error: {e}"), - } - } -} - -impl std::error::Error for AppError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::Control(e) => Some(e), - Self::Monitor(e) => Some(e), - Self::Engine(e) => Some(e), - Self::Config(e) => Some(e), - Self::Generic(_) => None, - Self::Io(e) => Some(e), - } - } + #[error("I/O error: {0}")] + Io(#[from] io::Error), }