From fb124dd2280af102e60eba2d538ebb0900fadd40 Mon Sep 17 00:00:00 2001 From: "Luke D. Jones" Date: Sat, 31 Jul 2021 23:09:00 +1200 Subject: [PATCH 1/3] Fix clippy warnings --- daemon/src/config_aura.rs | 4 ++-- daemon/src/ctrl_gfx/controller.rs | 17 ++++++++--------- daemon/src/ctrl_gfx/system.rs | 4 ++-- daemon/src/ctrl_leds/controller.rs | 4 ++-- daemon/src/ctrl_profiles/zbus.rs | 2 +- rog-aura/src/sequencer.rs | 2 +- 6 files changed, 16 insertions(+), 17 deletions(-) diff --git a/daemon/src/config_aura.rs b/daemon/src/config_aura.rs index d60b2809..4cb8924e 100644 --- a/daemon/src/config_aura.rs +++ b/daemon/src/config_aura.rs @@ -90,7 +90,7 @@ impl AuraConfig { let mut buf = String::new(); if let Ok(read_len) = file.read_to_string(&mut buf) { if read_len == 0 { - return AuraConfig::create_default(&mut file, &supported_led_modes); + return AuraConfig::create_default(&mut file, supported_led_modes); } else { if let Ok(data) = serde_json::from_str(&buf) { return data; @@ -109,7 +109,7 @@ impl AuraConfig { panic!("Please remove {} then restart asusd", AURA_CONFIG_PATH); } } - AuraConfig::create_default(&mut file, &supported_led_modes) + AuraConfig::create_default(&mut file, supported_led_modes) } fn create_default(file: &mut File, support_data: &LaptopLedData) -> Self { diff --git a/daemon/src/ctrl_gfx/controller.rs b/daemon/src/ctrl_gfx/controller.rs index d4d92c3c..6a2f23b2 100644 --- a/daemon/src/ctrl_gfx/controller.rs +++ b/daemon/src/ctrl_gfx/controller.rs @@ -8,7 +8,6 @@ use logind_zbus::{ ManagerProxy, SessionProxy, }; use rog_types::gfx_vendors::{GfxPower, GfxRequiredUserAction, GfxVendors}; -use std::iter::FromIterator; use std::{io::Write, ops::Add, path::Path, time::Instant}; use std::{process::Command, thread::sleep, time::Duration}; use std::{str::FromStr, sync::mpsc}; @@ -79,20 +78,20 @@ impl CtrlGraphics { match dev.vendor()? { 0x1002 => { info!("GFX: {}: AMD graphics", dev.id()); - amd.push(GraphicsDevice::new(dev.id().to_owned(), functions(&dev))); + amd.push(GraphicsDevice::new(dev.id().to_owned(), functions(dev))); } 0x10DE => { info!("GFX: {}: NVIDIA graphics", dev.id()); dev.set_runtime_pm(sysfs_class::RuntimePowerManagement::On)?; - nvidia.push(GraphicsDevice::new(dev.id().to_owned(), functions(&dev))); + nvidia.push(GraphicsDevice::new(dev.id().to_owned(), functions(dev))); } 0x8086 => { info!("GFX: {}: Intel graphics", dev.id()); - intel.push(GraphicsDevice::new(dev.id().to_owned(), functions(&dev))); + intel.push(GraphicsDevice::new(dev.id().to_owned(), functions(dev))); } vendor => { info!("GFX: {}: Other({:X}) graphics", dev.id(), vendor); - other.push(GraphicsDevice::new(dev.id().to_owned(), functions(&dev))); + other.push(GraphicsDevice::new(dev.id().to_owned(), functions(dev))); } } } @@ -265,13 +264,13 @@ impl CtrlGraphics { let unbinds = devices.iter().map(|dev| dev.unbind()); // Remove NVIDIA graphics devices and their functions let removes = devices.iter().map(|dev| dev.remove()); - Result::from_iter(unbinds.chain(removes)) + unbinds.chain(removes).collect::>() .map_err(|err| RogError::Command("device unbind error".into(), err)) } fn unbind_only(devices: &[GraphicsDevice]) -> Result<(), RogError> { let unbinds = devices.iter().map(|dev| dev.unbind()); - Result::from_iter(unbinds) + unbinds.collect::>() .map_err(|err| RogError::Command("device unbind error".into(), err)) } @@ -462,7 +461,7 @@ impl CtrlGraphics { for driver in NVIDIA_DRIVERS.iter() { Self::do_driver_action(driver, "rmmod")?; } - Self::unbind_only(&devices)?; + Self::unbind_only(devices)?; Self::do_driver_action("vfio-pci", "modprobe")?; } else { return Err(GfxError::VfioDisabled.into()); @@ -478,7 +477,7 @@ impl CtrlGraphics { for driver in NVIDIA_DRIVERS.iter() { Self::do_driver_action(driver, "rmmod")?; } - Self::unbind_remove_nvidia(&devices)?; + Self::unbind_remove_nvidia(devices)?; } } Ok(()) diff --git a/daemon/src/ctrl_gfx/system.rs b/daemon/src/ctrl_gfx/system.rs index f730a9ae..ce1ddf67 100644 --- a/daemon/src/ctrl_gfx/system.rs +++ b/daemon/src/ctrl_gfx/system.rs @@ -83,7 +83,7 @@ impl GraphicsDevice { Ok(driver) => { info!("{}: Unbinding {}", driver.id(), func.id()); unsafe { - driver.unbind(&func).map_err(|err| { + driver.unbind(func).map_err(|err| { error!("gfx unbind: {}", err); err })?; @@ -109,7 +109,7 @@ impl GraphicsDevice { Ok(driver) => { info!("{}: Binding {}", driver.id(), func.id()); unsafe { - driver.bind(&func).map_err(|err| { + driver.bind(func).map_err(|err| { error!("gfx bind: {}", err); err })?; diff --git a/daemon/src/ctrl_leds/controller.rs b/daemon/src/ctrl_leds/controller.rs index e37b5fea..d9e0a64f 100644 --- a/daemon/src/ctrl_leds/controller.rs +++ b/daemon/src/ctrl_leds/controller.rs @@ -370,7 +370,7 @@ impl CtrlKbdLed { self.config.read(); if let Some(data) = self.config.builtins.get(&next) { - self.write_mode(&data)?; + self.write_mode(data)?; self.config.current_mode = next; } self.config.write(); @@ -381,7 +381,7 @@ impl CtrlKbdLed { #[inline] fn write_mode(&self, mode: &AuraEffect) -> Result<(), RogError> { - if !self.supported_modes.standard.contains(&mode.mode()) { + if !self.supported_modes.standard.contains(mode.mode()) { return Err(RogError::NotSupported); } let bytes: [u8; LED_MSG_LEN] = mode.into(); diff --git a/daemon/src/ctrl_profiles/zbus.rs b/daemon/src/ctrl_profiles/zbus.rs index 4ae52096..84c7da2b 100644 --- a/daemon/src/ctrl_profiles/zbus.rs +++ b/daemon/src/ctrl_profiles/zbus.rs @@ -139,7 +139,7 @@ impl FanAndCpuZbus { if let Ok(ctrl) = self.inner.try_lock() { if let Ok(cfg) = ctrl.config.clone().try_lock() { if let Some(profile) = cfg.power_profiles.get(&cfg.active_profile) { - self.notify_profile(&profile) + self.notify_profile(profile) .unwrap_or_else(|err| warn!("{}", err)); } } diff --git a/rog-aura/src/sequencer.rs b/rog-aura/src/sequencer.rs index c54f1c61..f3ba5016 100644 --- a/rog-aura/src/sequencer.rs +++ b/rog-aura/src/sequencer.rs @@ -39,7 +39,7 @@ impl Sequences { pub fn iter(&self) -> ActionIterator { ActionIterator { - actions: &self, + actions: self, next_idx: 0, } } From 29c26e8c89ae8761511f52fc6ee73fe93e9b6aa8 Mon Sep 17 00:00:00 2001 From: "Luke D. Jones" Date: Sun, 1 Aug 2021 00:04:16 +1200 Subject: [PATCH 2/3] fix: Profile select by name is correctly choosing Closes #100 #96 --- CHANGELOG.md | 1 + asusctl/src/main.rs | 17 ++++++++++---- daemon/src/ctrl_profiles/controller.rs | 32 +++++++++++++++++++------- daemon/src/error.rs | 2 -- rog-profiles/src/error.rs | 4 ---- 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e8ee97a..6e0d71ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed - Fix missing CLI command help for some supported options +- Fix incorrectly selecting profile by name, where the active profile was being copied to the selected profile # [3.7.1] - 2021-06-11 ### Changed diff --git a/asusctl/src/main.rs b/asusctl/src/main.rs index cac15ad7..d4a0a301 100644 --- a/asusctl/src/main.rs +++ b/asusctl/src/main.rs @@ -489,12 +489,21 @@ fn handle_profile( } let mut set_profile = false; - let mut profile; + let mut profile = Profile::default(); if cmd.create { - profile = Profile::default(); set_profile = true; - } else { - profile = dbus.proxies().profile().active_data()?; + } else if let Some(ref name) = cmd.profile { + let profiles = dbus.proxies().profile().all_profile_data()?; + for p in profiles { + if p.name == *name { + profile = p; + break; + } + } + if profile.name != *name { + println!("The requested profile doesn't exist, you may need to create it"); + std::process::exit(-1); + } } if let Some(turbo) = cmd.turbo { diff --git a/daemon/src/ctrl_profiles/controller.rs b/daemon/src/ctrl_profiles/controller.rs index 12805434..aba0f5f9 100644 --- a/daemon/src/ctrl_profiles/controller.rs +++ b/daemon/src/ctrl_profiles/controller.rs @@ -1,6 +1,7 @@ use crate::error::RogError; use crate::{config::Config, GetSupported}; -use log::info; +use log::{info, warn}; +use rog_profiles::error::ProfileError; use rog_profiles::profiles::Profile; use rog_types::supported::FanCpuSupportedFunctions; use std::sync::Arc; @@ -23,12 +24,12 @@ impl GetSupported for CtrlFanAndCpu { } impl crate::Reloadable for CtrlFanAndCpu { + /// Fetcht he active profile and use that to set all related components up fn reload(&mut self) -> Result<(), RogError> { if let Ok(mut cfg) = self.config.clone().try_lock() { let active = cfg.active_profile.clone(); if let Some(existing) = cfg.power_profiles.get_mut(&active) { existing.set_system_all()?; - cfg.write(); } } Ok(()) @@ -38,31 +39,38 @@ impl crate::Reloadable for CtrlFanAndCpu { impl CtrlFanAndCpu { pub fn new(config: Arc>) -> Result { Profile::get_fan_path()?; - info!("Device has thermal throttle control"); + info!("Device has fan control available"); Ok(CtrlFanAndCpu { config }) } /// Toggle to next profile in list pub(super) fn do_next_profile(&mut self) -> Result<(), RogError> { if let Ok(mut config) = self.config.clone().try_lock() { + // Read first just incase the user has modified the config before calling this config.read(); - let mut i = config + let mut toggle_index = config .toggle_profiles .binary_search(&config.active_profile) .unwrap_or(0) + 1; - if i >= config.toggle_profiles.len() { - i = 0; + if toggle_index >= config.toggle_profiles.len() { + toggle_index = 0; } - let profile = config.toggle_profiles[i].clone(); + let profile = config.toggle_profiles[toggle_index].clone(); if let Some(existing) = config.power_profiles.get(&profile) { existing.set_system_all()?; config.active_profile = existing.name.clone(); config.write(); - info!("Profile was changed to: {}", profile); + info!("Profile was changed to: {}", &profile); + } else { + warn!( + "toggle_profile {} does not exist in power_profiles", + &profile + ); + return Err(RogError::MissingProfile(profile.to_string())); } } Ok(()) @@ -70,17 +78,25 @@ impl CtrlFanAndCpu { pub(super) fn set_active(&mut self, profile: &str) -> Result<(), RogError> { if let Ok(mut config) = self.config.clone().try_lock() { + // Read first just incase the user has modified the config before calling this config.read(); if let Some(existing) = config.power_profiles.get(profile) { existing.set_system_all()?; config.active_profile = existing.name.clone(); config.write(); info!("Profile was changed to: {}", profile); + } else { + warn!( + "toggle_profile {} does not exist in power_profiles", + profile + ); + return Err(RogError::MissingProfile(profile.to_string())); } } Ok(()) } + /// Create a new profile if the requested name doesn't exist, or modify existing pub(super) fn new_or_modify(&mut self, profile: &Profile) -> Result<(), RogError> { if let Ok(mut config) = self.config.clone().try_lock() { config.read(); diff --git a/daemon/src/error.rs b/daemon/src/error.rs index fdb85342..fa61f61e 100644 --- a/daemon/src/error.rs +++ b/daemon/src/error.rs @@ -8,7 +8,6 @@ use crate::ctrl_gfx::error::GfxError; #[derive(Debug)] pub enum RogError { - ParseFanLevel, ParseVendor, ParseLed, MissingProfile(String), @@ -36,7 +35,6 @@ impl fmt::Display for RogError { // This trait requires `fmt` with this exact signature. fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - RogError::ParseFanLevel => write!(f, "Parse profile error"), RogError::ParseVendor => write!(f, "Parse gfx vendor error"), RogError::ParseLed => write!(f, "Parse LED error"), RogError::MissingProfile(profile) => write!(f, "Profile does not exist {}", profile), diff --git a/rog-profiles/src/error.rs b/rog-profiles/src/error.rs index e73538e4..c5dc7c12 100644 --- a/rog-profiles/src/error.rs +++ b/rog-profiles/src/error.rs @@ -6,7 +6,6 @@ use rog_fan_curve::CurveError; #[derive(Debug)] pub enum ProfileError { ParseFanLevel, - MissingProfile(String), Path(String, std::io::Error), Read(String, std::io::Error), Write(String, std::io::Error), @@ -23,9 +22,6 @@ impl fmt::Display for ProfileError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { ProfileError::ParseFanLevel => write!(f, "Parse profile error"), - ProfileError::MissingProfile(profile) => { - write!(f, "Profile does not exist {}", profile) - } ProfileError::Path(path, error) => write!(f, "Path {}: {}", path, error), ProfileError::Read(path, error) => write!(f, "Read {}: {}", path, error), ProfileError::Write(path, error) => write!(f, "Write {}: {}", path, error), From 00b7c6482f67615b5059f9fd11370dae90659b00 Mon Sep 17 00:00:00 2001 From: "Luke D. Jones" Date: Sun, 1 Aug 2021 00:15:38 +1200 Subject: [PATCH 3/3] fix: remove the parsing for root supported functions --- asusctl/src/main.rs | 54 -------------------------- daemon/src/ctrl_profiles/controller.rs | 1 - 2 files changed, 55 deletions(-) diff --git a/asusctl/src/main.rs b/asusctl/src/main.rs index d4a0a301..559eb2ac 100644 --- a/asusctl/src/main.rs +++ b/asusctl/src/main.rs @@ -14,7 +14,6 @@ use rog_types::{ gfx_vendors::{GfxRequiredUserAction, GfxVendors}, supported::{ FanCpuSupportedFunctions, LedSupportedFunctions, RogBiosSupportedFunctions, - SupportedFunctions, }, }; use std::{env::args, path::Path}; @@ -133,12 +132,6 @@ fn main() -> Result<(), Box> { let supported = dbus.proxies().supported().get_supported_functions()?; - if parsed.help { - print_supported_help(&supported, &parsed); - println!("\nSee https://asus-linux.org/faq/ for additional help"); - std::process::exit(1); - } - if parsed.version { println!(" asusctl v{}", env!("CARGO_PKG_VERSION")); println!(" rog-dbus v{}", rog_dbus::VERSION); @@ -234,53 +227,6 @@ fn main() -> Result<(), Box> { Ok(()) } -/// Print the root help for base commands if the commands are supported -fn print_supported_help(supported: &SupportedFunctions, parsed: &CliStart) { - // As help option don't work with `parse_args_default` - // we will call `parse_args_default_or_exit` instead - let usage: Vec = parsed.self_usage().lines().map(|s| s.to_string()).collect(); - for line in usage.iter().filter(|line| { - if line.contains("--fan-mode") && !supported.fan_cpu_ctrl.stock_fan_modes { - return false; - } - if line.contains("--chg-limit") && !supported.charge_ctrl.charge_level_set { - return false; - } - true - }) { - println!("{}", line); - } - - // command strings are in order of the struct - let commands: Vec = CliCommand::usage().lines().map(|s| s.to_string()).collect(); - println!("\nCommands available"); - for line in commands.iter().filter(|line| { - if line.contains("profile") { - return supported.fan_cpu_ctrl.stock_fan_modes || supported.fan_cpu_ctrl.fan_curve_set; - } - if line.contains("led-mode") { - return supported.keyboard_led.stock_led_modes.is_empty(); - } - if line.contains("bios") { - return supported.rog_bios_ctrl.dedicated_gfx_toggle - || supported.rog_bios_ctrl.post_sound_toggle; - } - if line.contains("anime") { - return supported.anime_ctrl.0; - } - false - }) { - println!("{}", line); - } - - if !supported.fan_cpu_ctrl.stock_fan_modes { - println!("Note: Fan mode control is not supported by this laptop"); - } - if !supported.charge_ctrl.charge_level_set { - println!("Note: Charge control is not supported by this laptop"); - } -} - fn do_gfx( dbus: &RogDbusClient, supported: &RogBiosSupportedFunctions, diff --git a/daemon/src/ctrl_profiles/controller.rs b/daemon/src/ctrl_profiles/controller.rs index aba0f5f9..e6cd3791 100644 --- a/daemon/src/ctrl_profiles/controller.rs +++ b/daemon/src/ctrl_profiles/controller.rs @@ -1,7 +1,6 @@ use crate::error::RogError; use crate::{config::Config, GetSupported}; use log::{info, warn}; -use rog_profiles::error::ProfileError; use rog_profiles::profiles::Profile; use rog_types::supported::FanCpuSupportedFunctions; use std::sync::Arc;