From 8ae6c985546fcc0a4c81c051b38e2914a7228e1e Mon Sep 17 00:00:00 2001 From: Vicki Pfau Date: Thu, 28 Mar 2024 18:12:21 -0700 Subject: [PATCH] process: Make API less fiddly --- src/hardware.rs | 13 +++----- src/main.rs | 8 +++++ src/manager.rs | 87 ++++++++++++++++++------------------------------- src/process.rs | 26 ++++++++++----- src/wifi.rs | 49 +++++++++------------------- 5 files changed, 76 insertions(+), 107 deletions(-) diff --git a/src/hardware.rs b/src/hardware.rs index 7c07912..fc1ef1f 100644 --- a/src/hardware.rs +++ b/src/hardware.rs @@ -11,7 +11,7 @@ use std::str::FromStr; use tokio::fs; use crate::path; -use crate::process::run_script; +use crate::process::script_exit_code; const BOARD_VENDOR_PATH: &str = "/sys/class/dmi/id/board_vendor"; const BOARD_NAME_PATH: &str = "/sys/class/dmi/id/board_name"; @@ -123,15 +123,10 @@ mod test { pub async fn check_support() -> Result { // Run jupiter-check-support note this script does exit 1 for "Support: No" case // so no need to parse output, etc. - let res = run_script( - "check hardware support", - "/usr/bin/jupiter-check-support", - &[""], - ) - .await?; + let res = script_exit_code("/usr/bin/jupiter-check-support", &[] as &[String; 0]).await?; Ok(match res { - true => HardwareCurrentlySupported::Supported, - false => HardwareCurrentlySupported::Unsupported, + 0 => HardwareCurrentlySupported::Supported, + _ => HardwareCurrentlySupported::Unsupported, }) } diff --git a/src/main.rs b/src/main.rs index 01b6d20..714f7d3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -123,6 +123,14 @@ async fn reload() -> Result<()> { } } +pub fn anyhow_to_zbus(error: Error) -> zbus::Error { + zbus::Error::Failure(error.to_string()) +} + +pub fn anyhow_to_zbus_fdo(error: Error) -> zbus::fdo::Error { + zbus::fdo::Error::Failed(error.to_string()) +} + async fn create_connection() -> Result { let manager = manager::SteamOSManager::new().await?; diff --git a/src/manager.rs b/src/manager.rs index bb86c17..817d12b 100644 --- a/src/manager.rs +++ b/src/manager.rs @@ -18,6 +18,7 @@ use crate::power::{ }; use crate::process::{run_script, script_output, SYSTEMCTL_PATH}; use crate::wifi::{set_wifi_debug_mode, WifiDebugMode, WifiPowerManagement}; +use crate::{anyhow_to_zbus, anyhow_to_zbus_fdo}; #[derive(PartialEq, Debug, Copy, Clone)] #[repr(u32)] @@ -85,11 +86,10 @@ impl SteamOSManager { "/usr/bin/steamos-factory-reset-config", &[""], ) - .await - .unwrap_or(false); + .await; match res { - true => PrepareFactoryReset::RebootRequired as u32, - false => PrepareFactoryReset::Unknown as u32, + Ok(_) => PrepareFactoryReset::RebootRequired as u32, + Err(_) => PrepareFactoryReset::Unknown as u32, } } @@ -115,20 +115,13 @@ impl SteamOSManager { } }; - let res = run_script( + run_script( "set wifi power management", "/usr/bin/iwconfig", &["wlan0", "power", state], ) - .await; - - match res { - Ok(true) => Ok(()), - Ok(false) => Err(zbus::Error::Failure(String::from( - "iwconfig returned non-zero", - ))), - Err(e) => Err(zbus::Error::Failure(e.to_string())), - } + .await + .map_err(anyhow_to_zbus) } #[zbus(property)] @@ -154,20 +147,13 @@ impl SteamOSManager { }; // Run what steamos-polkit-helpers/jupiter-fan-control does - let res = run_script( + run_script( "enable fan control", SYSTEMCTL_PATH, &[state, "jupiter-fan-control-service"], ) - .await; - - match res { - Ok(true) => Ok(()), - Ok(false) => Err(zbus::Error::Failure(String::from( - "systemctl returned non-zero", - ))), - Err(e) => Err(zbus::Error::Failure(format!("{e}"))), - } + .await + .map_err(anyhow_to_zbus) } #[zbus(property)] @@ -209,65 +195,54 @@ impl SteamOSManager { async fn update_bios(&self) -> Result<(), zbus::fdo::Error> { // Update the bios as needed - let res = run_script( + run_script( "update bios", "/usr/bin/steamos-potlkit-helpers/jupiter-biosupdate", &["--auto"], ) - .await; - - match res { - Ok(_) => Ok(()), - Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())), - } + .await + .map_err(anyhow_to_zbus_fdo) } async fn update_dock(&self) -> Result<(), zbus::fdo::Error> { // Update the dock firmware as needed - let res = run_script( + run_script( "update dock firmware", "/usr/bin/steamos-polkit-helpers/jupiter-dock-updater", &[""], ) - .await; - - match res { - Ok(_) => Ok(()), - Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())), - } + .await + .map_err(anyhow_to_zbus_fdo) } async fn trim_devices(&self) -> Result<(), zbus::fdo::Error> { // Run steamos-trim-devices script - let res = run_script( + run_script( "trim devices", "/usr/bin/steamos-polkit-helpers/steamos-trim-devices", &[""], ) - .await; - - match res { - Ok(_) => Ok(()), - Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())), - } + .await + .map_err(anyhow_to_zbus_fdo) } - async fn format_device(&self, device: &str, label: &str, validate: bool) -> Result<(), zbus::fdo::Error> { + async fn format_device( + &self, + device: &str, + label: &str, + validate: bool, + ) -> Result<(), zbus::fdo::Error> { let mut args = vec!["--label", label, "--device", device]; if !validate { args.push("--skip-validation"); } - let res = run_script( + run_script( "format device", "/usr/lib/hwsupport/format-device.sh", - args.as_ref() + args.as_ref(), ) - .await; - - match res { - Ok(_) => Ok(()), - Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())), - } + .await + .map_err(anyhow_to_zbus_fdo) } #[zbus(property)] @@ -286,7 +261,7 @@ impl SteamOSManager { }; set_gpu_performance_level(level) .await - .map_err(|e| zbus::Error::Failure(e.to_string())) + .map_err(anyhow_to_zbus) } async fn set_gpu_clocks(&self, clocks: i32) -> bool { @@ -326,7 +301,7 @@ impl SteamOSManager { } Err(e) => { error!("Setting wifi debug mode failed: {e}"); - Err(zbus::fdo::Error::Failed(e.to_string())) + Err(anyhow_to_zbus_fdo(e)) } } } diff --git a/src/process.rs b/src/process.rs index e38f15c..0398ad1 100644 --- a/src/process.rs +++ b/src/process.rs @@ -5,26 +5,34 @@ * SPDX-License-Identifier: MIT */ -use anyhow::Result; +use anyhow::{anyhow, Result}; use std::ffi::OsStr; use tokio::process::Command; use tracing::warn; pub const SYSTEMCTL_PATH: &str = "/usr/bin/systemctl"; -pub async fn script_exit_code(executable: &str, args: &[impl AsRef]) -> Result { - // Run given script and return true on success +pub async fn script_exit_code(executable: &str, args: &[impl AsRef]) -> Result { + // Run given script and return the exit code let mut child = Command::new(executable).args(args).spawn()?; let status = child.wait().await?; - Ok(status.success()) + status.code().ok_or(anyhow!("Killed by signal")) } -pub async fn run_script(name: &str, executable: &str, args: &[impl AsRef]) -> Result { +pub async fn run_script(name: &str, executable: &str, args: &[impl AsRef]) -> Result<()> { // Run given script to get exit code and return true on success. - // Return false on failure, but also print an error if needed - script_exit_code(executable, args) - .await - .inspect_err(|message| warn!("Error running {name} {message}")) + // Return Err on failure, but also print an error if needed + match script_exit_code(executable, args).await { + Ok(0) => Ok(()), + Ok(code) => { + warn!("Error running {name}: exited {code}"); + Err(anyhow!("Exited {code}")) + } + Err(message) => { + warn!("Error running {name}: {message}"); + Err(message) + } + } } pub async fn script_output(executable: &str, args: &[impl AsRef]) -> Result { diff --git a/src/wifi.rs b/src/wifi.rs index 210b13f..c87df5b 100644 --- a/src/wifi.rs +++ b/src/wifi.rs @@ -106,19 +106,13 @@ pub async fn setup_iwd_config(want_override: bool) -> std::io::Result<()> { } } -async fn restart_iwd() -> Result { +async fn restart_iwd() -> Result<()> { // First reload systemd since we modified the config most likely // otherwise we wouldn't be restarting iwd. match run_script("reload systemd", SYSTEMCTL_PATH, &["daemon-reload"]).await { - Ok(value) => { - if value { - // worked, now restart iwd - run_script("restart iwd", SYSTEMCTL_PATH, &["restart", "iwd"]).await - } else { - // reload failed - error!("restart_iwd: reload systemd failed with non-zero exit code"); - Ok(false) - } + Ok(_) => { + // worked, now restart iwd + run_script("restart iwd", SYSTEMCTL_PATH, &["restart", "iwd"]).await } Err(message) => { error!("restart_iwd: reload systemd got an error: {message}"); @@ -127,7 +121,7 @@ async fn restart_iwd() -> Result { } } -async fn stop_tracing() -> Result { +async fn stop_tracing() -> Result<()> { // Stop tracing and extract ring buffer to disk for capture run_script("stop tracing", TRACE_CMD_PATH, &["stop"]).await?; // stop tracing worked @@ -139,7 +133,7 @@ async fn stop_tracing() -> Result { .await } -async fn start_tracing(buffer_size: u32) -> Result { +async fn start_tracing(buffer_size: u32) -> Result<()> { // Start tracing let size_str = format!("{}", buffer_size); run_script( @@ -164,25 +158,18 @@ pub async fn set_wifi_debug_mode( // If mode is 0 disable wifi debug mode // Stop any existing trace and flush to disk. if should_trace { - let result = match stop_tracing().await { - Ok(result) => result, - Err(message) => bail!("stop_tracing command got an error: {message}"), + if let Err(message) = stop_tracing().await { + bail!("stop_tracing command got an error: {message}"); }; - ensure!(result, "stop_tracing command returned non-zero"); } // Stop_tracing was successful if let Err(message) = setup_iwd_config(false).await { bail!("setup_iwd_config false got an error: {message}"); - } - // setup_iwd_config false worked - let value = match restart_iwd().await { - Ok(value) => value, - Err(message) => { - bail!("restart_iwd got an error: {message}"); - } }; - // restart_iwd failed - ensure!(value, "restart_iwd failed, check log above"); + // setup_iwd_config false worked + if let Err(message) = restart_iwd().await { + bail!("restart_iwd got an error: {message}"); + }; } WifiDebugMode::On => { ensure!(buffer_size > MIN_BUFFER_SIZE, "Buffer size too small"); @@ -191,18 +178,14 @@ pub async fn set_wifi_debug_mode( bail!("setup_iwd_config true got an error: {message}"); } // setup_iwd_config worked - let value = match restart_iwd().await { - Ok(value) => value, - Err(message) => bail!("restart_iwd got an error: {message}"), + if let Err(message) = restart_iwd().await { + bail!("restart_iwd got an error: {message}"); }; - ensure!(value, "restart_iwd failed"); // restart_iwd worked if should_trace { - let value = match start_tracing(buffer_size).await { - Ok(value) => value, - Err(message) => bail!("start_tracing got an error: {message}"), + if let Err(message) = start_tracing(buffer_size).await { + bail!("start_tracing got an error: {message}"); }; - ensure!(value, "start_tracing failed"); } } mode => {