diff --git a/asusd/src/asus_armoury.rs b/asusd/src/asus_armoury.rs index 96dfbf7e..3fafed92 100644 --- a/asusd/src/asus_armoury.rs +++ b/asusd/src/asus_armoury.rs @@ -27,6 +27,28 @@ fn dbus_path_for_attr(attr_name: &str) -> OwnedObjectPath { ObjectPath::from_str_unchecked(&format!("{ASUS_ZBUS_PATH}/{MOD_NAME}/{attr_name}")).into() } +// Helper: return true when the attribute is effectively unsupported for +// the current power mode/device. We consider an attribute unsupported when +// its resolved min and max are equal (no available range), which indicates +// writes would be invalid. When true, callers should avoid attempting writes. +fn attr_unsupported(attr: &Attribute) -> bool { + let min = attr + .refresh_min_value() + .or(match attr.min_value() { + AttrValue::Integer(i) => Some(*i), + _ => None, + }) + .unwrap_or(-1); + let max = attr + .refresh_max_value() + .or(match attr.max_value() { + AttrValue::Integer(i) => Some(*i), + _ => None, + }) + .unwrap_or(-2); // different default so equality is false unless both present + min == max +} + #[derive(Clone)] pub struct AsusArmouryAttribute { attr: Attribute, @@ -204,14 +226,24 @@ impl crate::Reloadable for AsusArmouryAttribute { }; if let Some(tune) = apply_value { - self.attr - .set_current_value(&AttrValue::Integer(tune)) - .map_err(|e| { - error!("Could not set {} value: {e:?}", self.attr.name()); - self.attr.base_path_exists(); - e - })?; - info!("Set {} to {:?}", self.attr.name(), tune); + // Don't attempt writes for attributes that report min==0 and max==0 + // (commonly means not supported in this power mode/device); skip + // applying stored tune in that case. + if self.attr.base_path_exists() && !attr_unsupported(&self.attr) { + self.attr + .set_current_value(&AttrValue::Integer(tune)) + .map_err(|e| { + error!("Could not set {} value: {e:?}", self.attr.name()); + self.attr.base_path_exists(); + e + })?; + info!("Set {} to {:?}", self.attr.name(), tune); + } else { + debug!( + "Skipping apply for {} because attribute missing or unsupported (min==max)", + self.attr.name() + ); + } } } else { // Handle non-PPT attributes (boolean and other settings) @@ -442,13 +474,23 @@ impl AsusArmouryAttribute { .unwrap_or_default() != 0; - if power_plugged == on_ac { - self.attr - .set_current_value(&AttrValue::Integer(value)) - .map_err(|e| { - error!("Could not set value: {e:?}"); - e - })?; + // Don't attempt writes for attributes that report min==0 and max==0 + // (commonly means not supported in this power mode/device); skip + // applying stored value in that case. + if self.attr.base_path_exists() && !attr_unsupported(&self.attr) { + if power_plugged == on_ac { + self.attr + .set_current_value(&AttrValue::Integer(value)) + .map_err(|e| { + error!("Could not set value: {e:?}"); + e + })?; + } + } else { + debug!( + "Skipping immediate apply for {} because attribute missing or unsupported (min==max)", + self.attr.name() + ); } } @@ -478,12 +520,22 @@ impl AsusArmouryAttribute { debug!("Store tuning config for {} = {:?}", self.attr.name(), value); } if tuning.enabled { - self.attr - .set_current_value(&AttrValue::Integer(value)) - .map_err(|e| { - error!("Could not set value: {e:?}"); - e - })?; + // Don't attempt writes for attributes that report min==0 and max==0 + // (commonly means not supported in this power mode/device); skip + // applying stored value in that case. + if self.attr.base_path_exists() && !attr_unsupported(&self.attr) { + self.attr + .set_current_value(&AttrValue::Integer(value)) + .map_err(|e| { + error!("Could not set value: {e:?}"); + e + })?; + } else { + debug!( + "Skipping apply for {} on set_current_value because attribute missing or unsupported (min==max)", + self.attr.name() + ); + } } } else { self.attr @@ -676,28 +728,44 @@ pub async fn set_config_or_default( debug!("Tuning group is not enabled, skipping"); return; } - + // Determine once whether attribute is present and supports a writable range + let supported = attr.base_path_exists() && !attr_unsupported(attr); if let Some(tune) = tuning.group.get(&name) { - attr.set_current_value(&AttrValue::Integer(*tune)) - .map_err(|e| { - error!("Failed to set {}: {e}", <&str>::from(name)); - }) - .ok(); - } else { - let default = attr.default_value(); - attr.set_current_value(default) - .map_err(|e| { - error!("Failed to set {}: {e}", <&str>::from(name)); - }) - .ok(); - if let AttrValue::Integer(i) = default { - tuning.group.insert(name, *i); - info!( - "Set default tuning config for {} = {:?}", - <&str>::from(name), - i + if supported { + attr.set_current_value(&AttrValue::Integer(*tune)) + .map_err(|e| { + error!("Failed to set {}: {e}", <&str>::from(name)); + }) + .ok(); + } else { + debug!( + "Skipping apply for {} in set_config_or_default because attribute missing or unsupported", + <&str>::from(name) + ); + } + } else { + // Only attempt to apply defaults when the attribute supports a range + if supported { + let default = attr.default_value(); + attr.set_current_value(default) + .map_err(|e| { + error!("Failed to set {}: {e}", <&str>::from(name)); + }) + .ok(); + if let AttrValue::Integer(i) = default { + tuning.group.insert(name, *i); + info!( + "Set default tuning config for {} = {:?}", + <&str>::from(name), + i + ); + // config.write(); + } + } else { + debug!( + "Skipping default apply for {} in set_config_or_default because attribute missing or unsupported", + <&str>::from(name) ); - // config.write(); } } } else { diff --git a/asusd/tests/attr_unsupported_min_eq_max.rs b/asusd/tests/attr_unsupported_min_eq_max.rs new file mode 100644 index 00000000..2f7174dc --- /dev/null +++ b/asusd/tests/attr_unsupported_min_eq_max.rs @@ -0,0 +1,68 @@ +use std::fs::{create_dir_all, File}; +use std::io::Write; +use std::path::PathBuf; + +use tempfile::tempdir; + +use asusd::asus_armoury::set_config_or_default; +use asusd::config::Config; +use rog_platform::asus_armoury::FirmwareAttributes; +use rog_platform::platform::PlatformProfile; + +fn write_attr_dir_with_min_max(base: &PathBuf, name: &str, default: &str, min: &str, max: &str) { + let attr_dir = base.join(name); + create_dir_all(&attr_dir).unwrap(); + + let mut f = File::create(attr_dir.join("default_value")).unwrap(); + write!(f, "{}", default).unwrap(); + let mut f = File::create(attr_dir.join("display_name")).unwrap(); + write!(f, "{}", name).unwrap(); + // create current_value file so set_current_value can open for write + let mut f = File::create(attr_dir.join("current_value")).unwrap(); + write!(f, "{}", default).unwrap(); + // write explicit min and max + let mut f = File::create(attr_dir.join("min_value")).unwrap(); + write!(f, "{}", min).unwrap(); + let mut f = File::create(attr_dir.join("max_value")).unwrap(); + write!(f, "{}", max).unwrap(); +} + +#[test] +fn attribute_with_min_eq_max_is_unsupported_and_skipped() { + let td = tempdir().unwrap(); + let base = td.path().join("attributes"); + create_dir_all(&base).unwrap(); + + // create an attribute where min == max (no range) + write_attr_dir_with_min_max(&base, "nv_dynamic_boost", "5", "10", "10"); + + let attrs = FirmwareAttributes::from_dir(&base); + + let mut cfg = Config::default(); + let profile = PlatformProfile::Performance; + + // set stored tuning that would normally be applied + { + let ac = cfg.select_tunings(true, profile); + ac.enabled = true; + ac.group.insert( + rog_platform::asus_armoury::FirmwareAttribute::NvDynamicBoost, + 9, + ); + } + + let rt = tokio::runtime::Runtime::new().unwrap(); + + // apply AC + rt.block_on(async { + set_config_or_default(&attrs, &mut cfg, true, profile).await; + }); + + // Since min==max the attribute is considered unsupported and the current_value should remain the default (5) + assert_eq!( + std::fs::read_to_string(base.join("nv_dynamic_boost").join("current_value")) + .unwrap() + .trim(), + "5" + ); +} diff --git a/rog-control-center/src/main.rs b/rog-control-center/src/main.rs index 7e8556b7..7b22e529 100644 --- a/rog-control-center/src/main.rs +++ b/rog-control-center/src/main.rs @@ -153,6 +153,9 @@ async fn main() -> Result<()> { let startup_in_background = config.startup_in_background; let config = Arc::new(Mutex::new(config)); + // shared weak handle to the UI so other threads can request UI updates + let ui_handle: Arc>>> = Arc::new(Mutex::new(None)); + start_notifications(config.clone(), &rt)?; if enable_tray_icon { @@ -177,12 +180,14 @@ async fn main() -> Result<()> { slint::init_translations!(concat!(env!("CARGO_MANIFEST_DIR"), "/translations/")); } + let config_for_ui = config.clone(); + let ui_handle_for_thread = ui_handle.clone(); thread::spawn(move || { let mut state = AppState::StartingUp; loop { if is_rog_ally { - let config_copy_2 = config.clone(); - let newui = setup_window(config.clone()); + let config_copy_2 = config_for_ui.clone(); + let newui = setup_window(config_for_ui.clone()); newui.window().on_close_requested(move || { exit(0); }); @@ -218,13 +223,19 @@ async fn main() -> Result<()> { *app_state = AppState::MainWindowOpen; } - let config_copy = config.clone(); + let config_copy = config_for_ui.clone(); let app_state_copy = app_state.clone(); + let ui_handle_for_ui = ui_handle_for_thread.clone(); slint::invoke_from_event_loop(move || { + let ui_handle_for_ui = ui_handle_for_ui.clone(); UI.with(|ui| { let app_state_copy = app_state_copy.clone(); let mut ui = ui.borrow_mut(); if let Some(ui) = ui.as_mut() { + // store weak handle so other threads can update UI globals + if let Ok(mut h) = ui_handle_for_ui.lock() { + *h = Some(ui.as_weak()); + } ui.window().show().unwrap(); ui.window().on_close_requested(move || { if let Ok(mut app_state) = app_state_copy.lock() { @@ -235,6 +246,10 @@ async fn main() -> Result<()> { } else { let config_copy_2 = config_copy.clone(); let newui = setup_window(config_copy); + // store weak handle for the newly created UI + if let Ok(mut h) = ui_handle_for_ui.lock() { + *h = Some(newui.as_weak()); + } newui.window().on_close_requested(move || { if let Ok(mut app_state) = app_state_copy.lock() { *app_state = AppState::MainWindowClosed; @@ -270,8 +285,8 @@ async fn main() -> Result<()> { slint::quit_event_loop().unwrap(); exit(0); } else if state != AppState::MainWindowOpen { - if let Ok(config) = config.lock() { - if !config.run_in_background { + if let Ok(cfg) = config_for_ui.lock() { + if !cfg.run_in_background { slint::quit_event_loop().unwrap(); exit(0); } @@ -281,11 +296,67 @@ async fn main() -> Result<()> { } }); + // start config watcher to pick up external edits + spawn_config_watcher(config.clone(), ui_handle.clone()); + slint::run_event_loop_until_quit().unwrap(); rt.shutdown_background(); Ok(()) } +// Spawn a watcher thread that polls the config file and reloads it when modified. +// This keeps the running rogcc instance in sync with manual edits to the config file. +fn spawn_config_watcher( + config: Arc>, + ui_handle: Arc>>>, +) { + std::thread::spawn(move || { + use std::time::SystemTime; + let cfg_path = Config::config_dir().join(Config::new().file_name()); + let mut last_mtime: Option = None; + loop { + if let Ok(meta) = std::fs::metadata(&cfg_path) { + if let Ok(m) = meta.modified() { + if last_mtime.is_none() { + last_mtime = Some(m); + } else if last_mtime.is_some_and(|t| t < m) { + // file changed, reload + last_mtime = Some(m); + let new_cfg = Config::new().load(); + if let Ok(mut lock) = config.lock() { + *lock = new_cfg.clone(); + } + + // Update UI app settings globals if UI is present + if let Ok(maybe_weak) = ui_handle.lock() { + if let Some(weak) = maybe_weak.clone() { + let config_for_ui = config.clone(); + weak.upgrade_in_event_loop(move |w| { + if let Ok(lock) = config_for_ui.lock() { + let cfg = lock.clone(); + w.global::() + .set_run_in_background(cfg.run_in_background); + w.global::() + .set_startup_in_background(cfg.startup_in_background); + w.global::() + .set_enable_tray_icon(cfg.enable_tray_icon); + w.global::() + .set_enable_dgpu_notifications( + cfg.notifications.enabled, + ); + } + }) + .ok(); + } + } + } + } + } + std::thread::sleep(std::time::Duration::from_secs(2)); + } + }); +} + fn do_cli_help(parsed: &CliStart) -> bool { if parsed.help { println!("{}", CliStart::usage());