process: Make API less fiddly

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<HardwareCurrentlySupported> {
     // 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(
+    let res = script_exit_code("/usr/bin/jupiter-check-support", &[] as &[String; 0]).await?;
-        "check hardware support",
-        "/usr/bin/jupiter-check-support",
-        &[""],
-    )
-    .await?;
 
     Ok(match res {
-        true => HardwareCurrentlySupported::Supported,
+        0 => HardwareCurrentlySupported::Supported,
-        false => HardwareCurrentlySupported::Unsupported,
+        _ => HardwareCurrentlySupported::Unsupported,
     })
 }

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<Connection> {
     let manager = manager::SteamOSManager::new().await?;
 

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
+        .await;
-        .unwrap_or(false);
         match res {
-            true => PrepareFactoryReset::RebootRequired as u32,
+            Ok(_) => PrepareFactoryReset::RebootRequired as u32,
-            false => PrepareFactoryReset::Unknown 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;
+        .await
-
+        .map_err(anyhow_to_zbus)
-        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())),
-        }
     }
 
     #[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;
+        .await
-
+        .map_err(anyhow_to_zbus)
-        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}"))),
-        }
     }
 
     #[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;
+        .await
-
+        .map_err(anyhow_to_zbus_fdo)
-        match res {
-            Ok(_) => Ok(()),
-            Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())),
-        }
     }
 
     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;
+        .await
-
+        .map_err(anyhow_to_zbus_fdo)
-        match res {
-            Ok(_) => Ok(()),
-            Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())),
-        }
     }
 
     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;
+        .await
-
+        .map_err(anyhow_to_zbus_fdo)
-        match res {
-            Ok(_) => Ok(()),
-            Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())),
-        }
     }
 
-    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;
+        .await
-
+        .map_err(anyhow_to_zbus_fdo)
-        match res {
-            Ok(_) => Ok(()),
-            Err(e) => Err(zbus::fdo::Error::Failed(e.to_string())),
-        }
     }
 
     #[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))
             }
         }
     }

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<OsStr>]) -> Result<bool> {
+pub async fn script_exit_code(executable: &str, args: &[impl AsRef<OsStr>]) -> Result<i32> {
-    // Run given script and return true on success
+    // 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<OsStr>]) -> Result<bool> {
+pub async fn run_script(name: &str, executable: &str, args: &[impl AsRef<OsStr>]) -> Result<()> {
     // Run given script to get exit code and return true on success.
-    // Return false on failure, but also print an error if needed
+    // Return Err on failure, but also print an error if needed
-    script_exit_code(executable, args)
+    match script_exit_code(executable, args).await {
-        .await
+        Ok(0) => Ok(()),
-        .inspect_err(|message| warn!("Error running {name} {message}"))
+        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<OsStr>]) -> Result<String> {

src/wifi.rs

@@ -106,19 +106,13 @@ pub async fn setup_iwd_config(want_override: bool) -> std::io::Result<()> {
     }
 }
 
-async fn restart_iwd() -> Result<bool> {
+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) => {
+        Ok(_) => {
-            if value {
+            // worked, now restart iwd
-                // worked, now restart iwd
+            run_script("restart iwd", SYSTEMCTL_PATH, &["restart", "iwd"]).await
-                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)
-            }
         }
         Err(message) => {
             error!("restart_iwd: reload systemd got an error: {message}");
@@ -127,7 +121,7 @@ async fn restart_iwd() -> Result<bool> {
     }
 }
 
-async fn stop_tracing() -> Result<bool> {
+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<bool> {
     .await
 }
 
-async fn start_tracing(buffer_size: u32) -> Result<bool> {
+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 {
+                if let Err(message) = stop_tracing().await {
-                    Ok(result) => result,
+                    bail!("stop_tracing command got an error: {message}");
-                    Err(message) => 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
+            // setup_iwd_config false worked
-            ensure!(value, "restart_iwd failed, check log above");
+            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 {
+            if let Err(message) = restart_iwd().await {
-                Ok(value) => value,
+                bail!("restart_iwd got an error: {message}");
-                Err(message) => 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 {
+                if let Err(message) = start_tracing(buffer_size).await {
-                    Ok(value) => value,
+                    bail!("start_tracing got an error: {message}");
-                    Err(message) => bail!("start_tracing got an error: {message}"),
                 };
-                ensure!(value, "start_tracing failed");
             }
         }
         mode => {