From b2f612cd458d77b725cb0aa717f4a0c1f3bcca3b Mon Sep 17 00:00:00 2001 From: Vicki Pfau Date: Mon, 20 May 2024 20:52:08 -0700 Subject: [PATCH] manager: Slim down root interface, moving most getters directly to the user interface --- src/daemon/root.rs | 2 +- src/manager/root.rs | 229 ++++---------------------------------------- src/manager/user.rs | 77 +++++++++++---- 3 files changed, 79 insertions(+), 229 deletions(-) diff --git a/src/daemon/root.rs b/src/daemon/root.rs index a950833..2b065c7 100644 --- a/src/daemon/root.rs +++ b/src/daemon/root.rs @@ -32,7 +32,7 @@ async fn create_connection() -> Result { pub async fn daemon() -> Result<()> { // This daemon is responsible for creating a dbus api that steam client can use to do various OS - // level things. It implements com.steampowered.SteamOSManager1.Manager interface + // level things. It implements com.steampowered.SteamOSManager1.RootManager interface let stdout_log = fmt::layer(); let subscriber = Registry::default().with(stdout_log); diff --git a/src/manager/root.rs b/src/manager/root.rs index 6d945fe..ab68a3b 100644 --- a/src/manager/root.rs +++ b/src/manager/root.rs @@ -13,15 +13,12 @@ use zbus::zvariant::Fd; use zbus::{fdo, interface, Connection, SignalContext}; use crate::error::{to_zbus_error, to_zbus_fdo_error}; -use crate::hardware::{check_support, variant, FanControl, FanControlState, HardwareVariant}; -use crate::power::{ - get_gpu_clocks, get_gpu_performance_level, get_tdp_limit, set_gpu_clocks, - set_gpu_performance_level, set_tdp_limit, GPUPerformanceLevel, -}; +use crate::hardware::{variant, FanControl, FanControlState, HardwareVariant}; +use crate::power::{set_gpu_clocks, set_gpu_performance_level, set_tdp_limit, GPUPerformanceLevel}; use crate::process::{run_script, script_output, ProcessManager}; use crate::wifi::{ - get_wifi_backend, get_wifi_power_management_state, set_wifi_backend, set_wifi_debug_mode, - set_wifi_power_management_state, WifiBackend, WifiDebugMode, WifiPowerManagement, + set_wifi_backend, set_wifi_debug_mode, set_wifi_power_management_state, WifiBackend, + WifiDebugMode, WifiPowerManagement, }; use crate::API_VERSION; @@ -56,7 +53,7 @@ impl SteamOSManager { const ALS_INTEGRATION_PATH: &str = "/sys/devices/platform/AMDI0010:00/i2c-0/i2c-PRP0001:01/iio:device0/in_illuminance_integration_time"; -#[interface(name = "com.steampowered.SteamOSManager1.Manager")] +#[interface(name = "com.steampowered.SteamOSManager1.RootManager")] impl SteamOSManager { async fn prepare_factory_reset(&self) -> u32 { // Run steamos factory reset script and return true on success @@ -67,23 +64,14 @@ impl SteamOSManager { } } - #[zbus(property(emits_changed_signal = "false"))] - async fn wifi_power_management_state(&self) -> fdo::Result { - match get_wifi_power_management_state().await { - Ok(state) => Ok(state as u32), - Err(e) => Err(to_zbus_fdo_error(e)), - } - } - - #[zbus(property)] - async fn set_wifi_power_management_state(&self, state: u32) -> zbus::Result<()> { + async fn set_wifi_power_management_state(&self, state: u32) -> fdo::Result<()> { let state = match WifiPowerManagement::try_from(state) { Ok(state) => state, - Err(err) => return Err(fdo::Error::InvalidArgs(err.to_string()).into()), + Err(err) => return Err(to_zbus_fdo_error(err)), }; set_wifi_power_management_state(state) .await - .map_err(to_zbus_error) + .map_err(to_zbus_fdo_error) } #[zbus(property(emits_changed_signal = "false"))] @@ -108,14 +96,6 @@ impl SteamOSManager { .map_err(to_zbus_error) } - #[zbus(property(emits_changed_signal = "const"))] - async fn hardware_currently_supported(&self) -> fdo::Result { - match check_support().await { - Ok(res) => Ok(res as u32), - Err(e) => Err(to_zbus_fdo_error(e)), - } - } - #[zbus(property(emits_changed_signal = "false"))] async fn als_calibration_gain(&self) -> f64 { // Run script to get calibration value @@ -193,77 +173,26 @@ impl SteamOSManager { .await } - #[zbus(property(emits_changed_signal = "false"))] - async fn gpu_performance_level(&self) -> fdo::Result { - match get_gpu_performance_level().await { - Ok(level) => Ok(level as u32), - Err(e) => { - error!("Error getting GPU performance level: {e}"); - Err(to_zbus_fdo_error(e)) - } - } - } - - #[zbus(property)] - async fn set_gpu_performance_level(&self, level: u32) -> zbus::Result<()> { + async fn set_gpu_performance_level(&self, level: u32) -> fdo::Result<()> { let level = match GPUPerformanceLevel::try_from(level) { Ok(level) => level, - Err(e) => return Err(zbus::Error::Failure(e.to_string())), + Err(e) => return Err(to_zbus_fdo_error(e)), }; set_gpu_performance_level(level) .await .inspect_err(|message| error!("Error setting GPU performance level: {message}")) - .map_err(to_zbus_error) - } - - #[zbus(property(emits_changed_signal = "false"))] - async fn manual_gpu_clock(&self) -> fdo::Result { - get_gpu_clocks() - .await - .inspect_err(|message| error!("Error getting manual GPU clock: {message}")) .map_err(to_zbus_fdo_error) } - #[zbus(property)] - async fn set_manual_gpu_clock(&self, clocks: u32) -> zbus::Result<()> { + async fn set_manual_gpu_clock(&self, clocks: u32) -> fdo::Result<()> { set_gpu_clocks(clocks) .await .inspect_err(|message| error!("Error setting manual GPU clock: {message}")) - .map_err(to_zbus_error) + .map_err(to_zbus_fdo_error) } - #[zbus(property(emits_changed_signal = "const"))] - async fn manual_gpu_clock_min(&self) -> u32 { - // TODO: Can this be queried from somewhere? - 200 - } - - #[zbus(property(emits_changed_signal = "const"))] - async fn manual_gpu_clock_max(&self) -> u32 { - // TODO: Can this be queried from somewhere? - 1600 - } - - #[zbus(property(emits_changed_signal = "false"))] - async fn tdp_limit(&self) -> fdo::Result { - get_tdp_limit().await.map_err(to_zbus_fdo_error) - } - - #[zbus(property)] - async fn set_tdp_limit(&self, limit: u32) -> zbus::Result<()> { - set_tdp_limit(limit).await.map_err(to_zbus_error) - } - - #[zbus(property(emits_changed_signal = "const"))] - async fn tdp_limit_min(&self) -> u32 { - // TODO: Can this be queried from somewhere? - 3 - } - - #[zbus(property(emits_changed_signal = "const"))] - async fn tdp_limit_max(&self) -> u32 { - // TODO: Can this be queried from somewhere? - 15 + async fn set_tdp_limit(&self, limit: u32) -> fdo::Result<()> { + set_tdp_limit(limit).await.map_err(to_zbus_fdo_error) } #[zbus(property)] @@ -304,16 +233,6 @@ impl SteamOSManager { } } - /// WifiBackend property. - #[zbus(property(emits_changed_signal = "false"))] - async fn wifi_backend(&self) -> fdo::Result { - match get_wifi_backend().await { - Ok(backend) => Ok(backend as u32), - Err(e) => Err(to_zbus_fdo_error(e)), - } - } - - #[zbus(property)] async fn set_wifi_backend(&mut self, backend: u32) -> fdo::Result<()> { if self.wifi_debug_mode == WifiDebugMode::On { return Err(fdo::Error::Failed(String::from( @@ -340,12 +259,10 @@ impl SteamOSManager { #[cfg(test)] mod test { use super::*; - use crate::{power, testing}; - use std::collections::HashMap; - use std::iter::zip; - use tokio::fs::{create_dir_all, read, write}; - use zbus::{Connection, ConnectionBuilder, Interface}; - use zbus_xml::{Method, Node, Property}; + use crate::power::{self, get_gpu_performance_level}; + use crate::testing; + use tokio::fs::{create_dir_all, write}; + use zbus::{Connection, ConnectionBuilder}; struct TestHandle { _handle: testing::TestHandle, @@ -394,15 +311,11 @@ mod test { } #[zbus::proxy( - interface = "com.steampowered.SteamOSManager1.Manager", + interface = "com.steampowered.SteamOSManager1.RootManager", default_service = "com.steampowered.SteamOSManager1.Test.GpuPerformanceLevel", default_path = "/com/steampowered/SteamOSManager1" )] trait GpuPerformanceLevel { - #[zbus(property)] - fn gpu_performance_level(&self) -> zbus::Result; - - #[zbus(property)] fn set_gpu_performance_level(&self, level: u32) -> zbus::Result<()>; } @@ -414,14 +327,6 @@ mod test { let proxy = GpuPerformanceLevelProxy::new(&test.connection) .await .unwrap(); - set_gpu_performance_level(GPUPerformanceLevel::Auto) - .await - .expect("set"); - assert_eq!( - proxy.gpu_performance_level().await.unwrap(), - GPUPerformanceLevel::Auto as u32 - ); - proxy .set_gpu_performance_level(GPUPerformanceLevel::Low as u32) .await @@ -433,15 +338,11 @@ mod test { } #[zbus::proxy( - interface = "com.steampowered.SteamOSManager1.Manager", + interface = "com.steampowered.SteamOSManager1.RootManager", default_service = "com.steampowered.SteamOSManager1.Test.ManualGpuClock", default_path = "/com/steampowered/SteamOSManager1" )] trait ManualGpuClock { - #[zbus(property)] - fn manual_gpu_clock(&self) -> zbus::Result; - - #[zbus(property)] fn set_manual_gpu_clock(&self, clocks: u32) -> zbus::Result<()>; } @@ -452,11 +353,6 @@ mod test { let proxy = ManualGpuClockProxy::new(&test.connection).await.unwrap(); power::test::setup().await; - assert!(proxy.manual_gpu_clock().await.is_err()); - - power::test::write_clocks(1600).await; - assert_eq!(proxy.manual_gpu_clock().await.unwrap(), 1600); - proxy.set_manual_gpu_clock(200).await.expect("proxy_set"); power::test::expect_clocks(200).await; @@ -465,7 +361,7 @@ mod test { } #[zbus::proxy( - interface = "com.steampowered.SteamOSManager1.Manager", + interface = "com.steampowered.SteamOSManager1.RootManager", default_service = "com.steampowered.SteamOSManager1.Test.Version", default_path = "/com/steampowered/SteamOSManager1" )] @@ -480,87 +376,4 @@ mod test { let proxy = VersionProxy::new(&test.connection).await.unwrap(); assert_eq!(proxy.version().await, Ok(API_VERSION)); } - - fn collect_methods<'a>(methods: &'a [Method<'a>]) -> HashMap> { - let mut map = HashMap::new(); - for method in methods.iter() { - map.insert(method.name().to_string(), method); - } - map - } - - fn collect_properties<'a>(props: &'a [Property<'a>]) -> HashMap> { - let mut map = HashMap::new(); - for prop in props.iter() { - map.insert(prop.name().to_string(), prop); - } - map - } - - #[tokio::test] - async fn interface_matches() { - let test = start("Interface").await; - - let manager_ref = test - .connection - .object_server() - .interface::<_, SteamOSManager>("/com/steampowered/SteamOSManager1") - .await - .expect("interface"); - let manager = manager_ref.get().await; - let mut remote_interface_string = String::from( - "", - ); - manager.introspect_to_writer(&mut remote_interface_string, 0); - remote_interface_string.push_str(""); - let remote_interfaces = - Node::from_reader::<&[u8]>(remote_interface_string.as_bytes()).expect("from_reader"); - let remote_interface: Vec<_> = remote_interfaces - .interfaces() - .iter() - .filter(|iface| iface.name() == "com.steampowered.SteamOSManager1.Manager") - .collect(); - assert_eq!(remote_interface.len(), 1); - let remote_interface = remote_interface[0]; - let remote_methods = collect_methods(remote_interface.methods()); - let remote_properties = collect_properties(remote_interface.properties()); - - let local_interface_string = read("com.steampowered.SteamOSManager1.xml") - .await - .expect("read"); - let local_interfaces = - Node::from_reader::<&[u8]>(local_interface_string.as_ref()).expect("from_reader"); - let local_interface: Vec<_> = local_interfaces - .interfaces() - .iter() - .filter(|iface| iface.name() == "com.steampowered.SteamOSManager1.Manager") - .collect(); - assert_eq!(local_interface.len(), 1); - let local_interface = local_interface[0]; - let local_methods = collect_methods(local_interface.methods()); - let local_properties = collect_properties(local_interface.properties()); - - for key in remote_methods.keys() { - let local_method = local_methods.get(key).expect(key); - let remote_method = remote_methods.get(key).expect(key); - - assert_eq!(local_method.name(), remote_method.name()); - assert_eq!(local_method.args().len(), remote_method.args().len()); - for (local_arg, remote_arg) in - zip(local_method.args().iter(), remote_method.args().iter()) - { - assert_eq!(local_arg.direction(), remote_arg.direction()); - assert_eq!(local_arg.ty(), remote_arg.ty()); - } - } - - for key in remote_properties.keys() { - let local_property = local_properties.get(key).expect(key); - let remote_property = remote_properties.get(key).expect(key); - - assert_eq!(local_property.name(), remote_property.name()); - assert_eq!(local_property.ty(), remote_property.ty()); - assert_eq!(local_property.access(), remote_property.access()); - } - } } diff --git a/src/manager/user.rs b/src/manager/user.rs index 929d40f..c60b615 100644 --- a/src/manager/user.rs +++ b/src/manager/user.rs @@ -14,6 +14,9 @@ use zbus::{fdo, interface, Connection, Proxy, SignalContext}; use crate::cec::{HdmiCecControl, HdmiCecState}; use crate::error::{to_zbus_error, to_zbus_fdo_error, zbus_to_zbus_fdo}; +use crate::hardware::check_support; +use crate::power::{get_gpu_clocks, get_gpu_performance_level, get_tdp_limit}; +use crate::wifi::{get_wifi_backend, get_wifi_power_management_state}; use crate::API_VERSION; macro_rules! method { @@ -63,7 +66,7 @@ impl SteamOSManager { proxy: Builder::new(system_conn) .destination("com.steampowered.SteamOSManager1")? .path("/com/steampowered/SteamOSManager1")? - .interface("com.steampowered.SteamOSManager1.Manager")? + .interface("com.steampowered.SteamOSManager1.RootManager")? .cache_properties(zbus::CacheProperties::No) .build() .await?, @@ -105,12 +108,18 @@ impl SteamOSManager { #[zbus(property(emits_changed_signal = "false"))] async fn wifi_power_management_state(&self) -> fdo::Result { - getter!(self, "WifiPowerManagementState") + match get_wifi_power_management_state().await { + Ok(state) => Ok(state as u32), + Err(e) => Err(to_zbus_fdo_error(e)), + } } #[zbus(property)] async fn set_wifi_power_management_state(&self, state: u32) -> zbus::Result<()> { - setter!(self, "WifiPowerManagementState", state) + self.proxy + .call("SetWifiPowerManagementState", &(state)) + .await + .map_err(to_zbus_error) } #[zbus(property(emits_changed_signal = "false"))] @@ -125,7 +134,10 @@ impl SteamOSManager { #[zbus(property(emits_changed_signal = "const"))] async fn hardware_currently_supported(&self) -> fdo::Result { - getter!(self, "HardwareCurrentlySupported") + match check_support().await { + Ok(res) => Ok(res as u32), + Err(e) => Err(to_zbus_fdo_error(e)), + } } #[zbus(property(emits_changed_signal = "false"))] @@ -168,52 +180,71 @@ impl SteamOSManager { #[zbus(property(emits_changed_signal = "false"))] async fn gpu_performance_level(&self) -> fdo::Result { - getter!(self, "GpuPerformanceLevel") + match get_gpu_performance_level().await { + Ok(level) => Ok(level as u32), + Err(e) => { + error!("Error getting GPU performance level: {e}"); + Err(to_zbus_fdo_error(e)) + } + } } #[zbus(property)] async fn set_gpu_performance_level(&self, level: u32) -> zbus::Result<()> { - setter!(self, "GpuPerformanceLevel", level) + self.proxy + .call("SetGpuPerformanceLevel", &(level)) + .await + .map_err(to_zbus_error) } #[zbus(property(emits_changed_signal = "false"))] async fn manual_gpu_clock(&self) -> fdo::Result { - getter!(self, "ManualGpuClock") + get_gpu_clocks() + .await + .inspect_err(|message| error!("Error getting manual GPU clock: {message}")) + .map_err(to_zbus_fdo_error) } #[zbus(property)] async fn set_manual_gpu_clock(&self, clocks: u32) -> zbus::Result<()> { - setter!(self, "ManualGpuClock", clocks) + setter!(self, "SetManualGpuClock", clocks) } #[zbus(property(emits_changed_signal = "const"))] - async fn manual_gpu_clock_min(&self) -> fdo::Result { - getter!(self, "ManualGpuClockMin") + async fn manual_gpu_clock_min(&self) -> u32 { + // TODO: Can this be queried from somewhere? + 200 } #[zbus(property(emits_changed_signal = "const"))] - async fn manual_gpu_clock_max(&self) -> fdo::Result { - getter!(self, "ManualGpuClockMax") + async fn manual_gpu_clock_max(&self) -> u32 { + // TODO: Can this be queried from somewhere? + 1600 } #[zbus(property(emits_changed_signal = "false"))] async fn tdp_limit(&self) -> fdo::Result { - getter!(self, "TdpLimit") + get_tdp_limit().await.map_err(to_zbus_fdo_error) } #[zbus(property)] async fn set_tdp_limit(&self, limit: u32) -> zbus::Result<()> { - setter!(self, "TdpLimit", limit) + self.proxy + .call("SetTdpLimit", &(limit)) + .await + .map_err(to_zbus_error) } #[zbus(property(emits_changed_signal = "const"))] - async fn tdp_limit_min(&self) -> fdo::Result { - getter!(self, "TdpLimitMin") + async fn tdp_limit_min(&self) -> u32 { + // TODO: Can this be queried from somewhere? + 3 } #[zbus(property(emits_changed_signal = "const"))] - async fn tdp_limit_max(&self) -> fdo::Result { - getter!(self, "TdpLimitMax") + async fn tdp_limit_max(&self) -> u32 { + // TODO: Can this be queried from somewhere? + 15 } #[zbus(property)] @@ -236,12 +267,18 @@ impl SteamOSManager { #[zbus(property(emits_changed_signal = "false"))] async fn wifi_backend(&self) -> fdo::Result { - getter!(self, "WifiBackend") + match get_wifi_backend().await { + Ok(backend) => Ok(backend as u32), + Err(e) => Err(to_zbus_fdo_error(e)), + } } #[zbus(property)] async fn set_wifi_backend(&self, backend: u32) -> zbus::Result<()> { - setter!(self, "WifiBackend", backend) + self.proxy + .call("SetWifiBackend", &(backend)) + .await + .map_err(to_zbus_error) } }