manager: Improve error logging

src/manager.rs

@@ -86,12 +86,7 @@ impl SteamOSManager {
 
     async fn prepare_factory_reset(&self) -> u32 {
         // Run steamos factory reset script and return true on success
-        let res = run_script(
+        let res = run_script("/usr/bin/steamos-factory-reset-config", &[""]).await;
-            "factory reset",
-            "/usr/bin/steamos-factory-reset-config",
-            &[""],
-        )
-        .await;
         match res {
             Ok(_) => PrepareFactoryReset::RebootRequired as u32,
             Err(_) => PrepareFactoryReset::Unknown as u32,
@@ -126,13 +121,10 @@ impl SteamOSManager {
             WifiPowerManagement::Enabled => "on",
         };
 
-        run_script(
+        run_script("/usr/bin/iwconfig", &["wlan0", "power", state])
-            "set wifi power management",
+            .await
-            "/usr/bin/iwconfig",
+            .inspect_err(|message| error!("Error setting wifi power management state: {message}"))
-            &["wlan0", "power", state],
+            .map_err(anyhow_to_zbus)
-        )
-        .await
-        .map_err(anyhow_to_zbus)
     }
 
     #[zbus(property(emits_changed_signal = "false"))]
@@ -209,33 +201,33 @@ impl SteamOSManager {
     async fn update_bios(&self) -> zbus::fdo::Result<()> {
         // Update the bios as needed
         run_script(
-            "update bios",
             "/usr/bin/steamos-potlkit-helpers/jupiter-biosupdate",
             &["--auto"],
         )
         .await
+        .inspect_err(|message| error!("Error updating BIOS: {message}"))
         .map_err(anyhow_to_zbus_fdo)
     }
 
     async fn update_dock(&self) -> zbus::fdo::Result<()> {
         // Update the dock firmware as needed
         run_script(
-            "update dock firmware",
             "/usr/bin/steamos-polkit-helpers/jupiter-dock-updater",
             &[""],
         )
         .await
+        .inspect_err(|message| error!("Error updating dock: {message}"))
         .map_err(anyhow_to_zbus_fdo)
     }
 
     async fn trim_devices(&self) -> zbus::fdo::Result<()> {
         // Run steamos-trim-devices script
         run_script(
-            "trim devices",
             "/usr/bin/steamos-polkit-helpers/steamos-trim-devices",
             &[""],
         )
         .await
+        .inspect_err(|message| error!("Error updating trimming devices: {message}"))
         .map_err(anyhow_to_zbus_fdo)
     }
 
@@ -249,20 +241,20 @@ impl SteamOSManager {
         if !validate {
             args.push("--skip-validation");
         }
-        run_script(
+        run_script("/usr/lib/hwsupport/format-device.sh", args.as_ref())
-            "format device",
+            .await
-            "/usr/lib/hwsupport/format-device.sh",
+            .inspect_err(|message| error!("Error formatting {device}: {message}"))
-            args.as_ref(),
+            .map_err(anyhow_to_zbus_fdo)
-        )
-        .await
-        .map_err(anyhow_to_zbus_fdo)
     }
 
     #[zbus(property(emits_changed_signal = "false"), name = "GPUPerformanceLevel")]
     async fn gpu_performance_level(&self) -> zbus::fdo::Result<u32> {
         match get_gpu_performance_level().await {
             Ok(level) => Ok(level as u32),
-            Err(e) => Err(anyhow_to_zbus_fdo(e)),
+            Err(e) => {
+                error!("Error getting GPU performance level: {e}");
+                Err(anyhow_to_zbus_fdo(e))
+            }
         }
     }
 
@@ -274,17 +266,24 @@ impl SteamOSManager {
         };
         set_gpu_performance_level(level)
             .await
+            .inspect_err(|message| error!("Error setting GPU performance level: {message}"))
             .map_err(anyhow_to_zbus)
     }
 
     #[zbus(property(emits_changed_signal = "false"), name = "ManualGPUClock")]
     async fn manual_gpu_clock(&self) -> zbus::fdo::Result<u32> {
-        get_gpu_clocks().await.map_err(anyhow_to_zbus_fdo)
+        get_gpu_clocks()
+            .await
+            .inspect_err(|message| error!("Error getting manual GPU clock: {message}"))
+            .map_err(anyhow_to_zbus_fdo)
     }
 
     #[zbus(property, name = "ManualGPUClock")]
     async fn set_manual_gpu_clock(&self, clocks: u32) -> zbus::Result<()> {
-        set_gpu_clocks(clocks).await.map_err(anyhow_to_zbus)
+        set_gpu_clocks(clocks)
+            .await
+            .inspect_err(|message| error!("Error setting manual GPU clock: {message}"))
+            .map_err(anyhow_to_zbus)
     }
 
     #[zbus(property(emits_changed_signal = "const"), name = "ManualGPUClockMin")]
@@ -364,7 +363,7 @@ impl SteamOSManager {
                 Ok(())
             }
             Err(e) => {
-                error!("Setting wifi debug mode failed: {e}");
+                error!("Error setting wifi debug mode: {e}");
                 Err(anyhow_to_zbus_fdo(e))
             }
         }
@@ -390,7 +389,10 @@ impl SteamOSManager {
             Ok(backend) => backend,
             Err(e) => return Err(zbus::fdo::Error::InvalidArgs(e.to_string())),
         };
-        set_wifi_backend(backend).await.map_err(anyhow_to_zbus_fdo)
+        set_wifi_backend(backend)
+            .await
+            .inspect_err(|message| error!("Error setting wifi backend: {message}"))
+            .map_err(anyhow_to_zbus_fdo)
     }
 
     /// A version property.

src/process.rs

@@ -8,7 +8,6 @@
 use anyhow::{anyhow, Result};
 use std::ffi::OsStr;
 use tokio::process::Command;
-use tracing::warn;
 
 #[cfg(not(test))]
 pub async fn script_exit_code(executable: &str, args: &[impl AsRef<OsStr>]) -> Result<i32> {
@@ -26,19 +25,13 @@ pub async fn script_exit_code(executable: &str, args: &[impl AsRef<OsStr>]) -> R
     cb(executable, args.as_ref()).map(|(res, _)| res)
 }
 
-pub async fn run_script(name: &str, executable: &str, args: &[impl AsRef<OsStr>]) -> Result<()> {
+pub async fn run_script(executable: &str, args: &[impl AsRef<OsStr>]) -> Result<()> {
     // Run given script to get exit code and return true on success.
     // Return Err on failure, but also print an error if needed
     match script_exit_code(executable, args).await {
         Ok(0) => Ok(()),
-        Ok(code) => {
+        Ok(code) => Err(anyhow!("Exited {code}")),
-            warn!("Error running {name}: exited {code}");
+        Err(message) => Err(message),
-            Err(anyhow!("Exited {code}"))
-        }
-        Err(message) => {
-            warn!("Error running {name}: {message}");
-            Err(message)
-        }
     }
 }
 

src/wifi.rs

@@ -156,21 +156,15 @@ async fn restart_iwd(connection: Connection) -> 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?;
+    run_script(TRACE_CMD_PATH, &["stop"]).await?;
     // stop tracing worked
-    run_script(
+    run_script(TRACE_CMD_PATH, &["extract", "-o", OUTPUT_FILE]).await
-        "extract traces",
-        TRACE_CMD_PATH,
-        &["extract", "-o", OUTPUT_FILE],
-    )
-    .await
 }
 
 async fn start_tracing(buffer_size: u32) -> Result<()> {
     // Start tracing
     let size_str = buffer_size.to_string();
     run_script(
-        "start tracing",
         TRACE_CMD_PATH,
         &["start", "-e", "ath11k_wmi_diag", "-b", &size_str],
     )
@@ -242,12 +236,7 @@ pub async fn get_wifi_backend() -> Result<WifiBackend> {
 }
 
 pub async fn set_wifi_backend(backend: WifiBackend) -> Result<()> {
-    run_script(
+    run_script("/usr/bin/steamos-wifi-set-backend", &[backend.to_string()]).await
-        "set wifi backend",
-        "/usr/bin/steamos-wifi-set-backend",
-        &[backend.to_string()],
-    )
-    .await
 }
 
 #[cfg(test)]
@@ -287,9 +276,15 @@ mod test {
             .expect("write");
         assert_eq!(get_wifi_backend().await.unwrap(), WifiBackend::IWD);
 
-        write(path(WIFI_BACKEND_PATH), "[device]\nwifi.backend=wpa_supplicant\n")
+        write(
-            .await
+            path(WIFI_BACKEND_PATH),
-            .expect("write");
+            "[device]\nwifi.backend=wpa_supplicant\n",
-        assert_eq!(get_wifi_backend().await.unwrap(), WifiBackend::WPASupplicant);
+        )
+        .await
+        .expect("write");
+        assert_eq!(
+            get_wifi_backend().await.unwrap(),
+            WifiBackend::WPASupplicant
+        );
     }
 }