refactor(script-rs): idiomatic Rust, better error handling, doc comments, SIGINT cleanup (#24)

* refactor(script-rs): idiomatic Rust, error handling, and doc comments
* fix: call unforward_connection on SIGINT to clean up port forward
* fix: remove redundant unforward call; signal thread returns after SIGINT with JoinHandle

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: rayworks <1329281+rayworks@users.noreply.github.com>

Author: Copilot

script-rs/Cargo.toml

@@ -2,10 +2,9 @@
 name = "script-rs"
 version = "0.1.0"
 edition = "2021"
-
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+description = "CLI helper script for DroidCast – forwards ADB connections and opens the live-screen URL in a browser."
+repository = "https://github.com/rayworks/DroidCast"
 
 [dependencies]
 webbrowser = "1.0.3"
-
 signal-hook = "0.3.17"

script-rs/src/main.rs

@@ -3,13 +3,9 @@ use std::process::Command;
 
 use std::{error::Error, thread};
 use std::time::Duration;
-use signal_hook::{iterator::Signals, consts::SIGINT};
-
-use webbrowser;
+use signal_hook::{iterator::{Handle, Signals}, consts::SIGINT};
 
 fn main() {
-    setup_signal_handler().expect("Failed to set up signal handler");
-
     let args: Vec<String> = env::args().collect();
     println!(">>> args {:?}", args);
 
@@ -18,140 +14,203 @@ fn main() {
         return;
     }
 
-    let port = args[1].to_string();
-    port.trim().parse::<u32>().expect("The port must be a number.");
+    let port = args[1].trim();
+    port.parse::<u32>().expect("The port must be a number.");
+
+    // Optional device serial number supplied as the third argument.
+    let serial: Option<&str> = args.get(2).map(|s| s.as_str());
 
     // adb devices
     let dev_cnt = count_connected_devices();
     if dev_cnt < 2 {
         println!("Make sure your device is connected");
         return;
-    } else if dev_cnt > 2 {
-        if args.len() < 3 {
-            println!("Multiple devices connected, please specify the target device serial number");
-            return;
-        }
+    } else if dev_cnt > 2 && serial.is_none() {
+        println!("Multiple devices connected, please specify the target device serial number");
+        return;
     }
 
     // apk path
-    let full_path = locate_apk_path();
-    if full_path.is_empty() {
-        return;
-    }
+    let full_path = match locate_apk_path(serial) {
+        Some(p) => p,
+        None => return,
+    };
 
     // forward
-    forward_connection(&port);
-
-    let handle = thread::spawn(|| {
+    forward_connection(port, serial);
+
+    // Register the SIGINT handler after the forward is established so that
+    // Ctrl-C always tears down the port forward before the process exits.
+    let (signal_handle, signals_close) =
+        setup_signal_handler(port.to_string(), serial.map(str::to_string))
+            .expect("Failed to set up signal handler");
+
+    // Clone owned values for the worker thread.
+    let port_owned = port.to_string();
+    let serial_owned = serial.map(str::to_string);
+    let handle = thread::spawn(move || {
         thread::sleep(Duration::from_secs(2));
 
         println!("Open the browser on the worker thread");
-        open_browser();
+        open_browser(&port_owned, serial_owned.as_deref());
     });
 
-    startup_service_and_wait(&port, full_path);
+    startup_service_and_wait(port, full_path, serial);
 
-    // unforward
-    unforward_connection(&port);
+    // On a normal (non-SIGINT) exit, close the signals iterator so the handler
+    // thread can return without waiting for another signal.
+    signals_close.close();
+    signal_handle.join().expect("Signal handler thread panicked");
 
-    handle.join().unwrap();
+    handle.join().expect("Browser thread panicked");
     println!("About to quit the app");
 }
 
-fn serial_checked_command() -> Command {
+/// Returns an `adb` [`Command`] pre-populated with the `-s <serial>` flag when
+/// a device serial number is provided.
+fn serial_checked_command(serial: Option<&str>) -> Command {
     let mut cmd = Command::new("adb");
-    let args: Vec<String> = env::args().collect();
-    if args.len() >= 3 {
-        cmd.args(vec!["-s", &args[2]]);
+    if let Some(s) = serial {
+        cmd.args(["-s", s]);
     }
     cmd
 }
 
+/// Runs `adb devices` and returns the number of lines that contain the word
+/// `"device"` (including the header), which is a proxy for the number of
+/// recognised entries.
 fn count_connected_devices() -> usize {
-    let devices_result = Command::new("adb").arg("devices").output();
-    let s = devices_result.unwrap().stdout;
-    let devices_out = std::str::from_utf8(&s).unwrap();
-
-    println!("{}", format!("\nDevices info : {}", devices_out));
+    let output = Command::new("adb")
+        .arg("devices")
+        .output()
+        .expect("Failed to run 'adb devices'");
+    let devices_out =
+        std::str::from_utf8(&output.stdout).expect("'adb devices' output is not valid UTF-8");
+
+    println!("\nDevices info : {}", devices_out);
     devices_out.matches("device").count()
 }
 
-fn locate_apk_path() -> String {
-    let params = vec!["shell", "pm", "path", "com.rayworks.droidcast"];
-    let path_result = serial_checked_command().args(params).output();
-    let path = path_result.unwrap().stdout;
-
-    let mut raw_path = std::str::from_utf8(&path).unwrap();
-    raw_path = raw_path.split(":").last().unwrap().trim();
-    if raw_path.is_empty() {
+/// Queries the package manager on the connected device for the APK path of
+/// `com.rayworks.droidcast`.  Returns `Some(classpath_string)` on success, or
+/// `None` (after printing an error) when the package is not found.
+fn locate_apk_path(serial: Option<&str>) -> Option<String> {
+    let params = ["shell", "pm", "path", "com.rayworks.droidcast"];
+    let output = serial_checked_command(serial)
+        .args(params)
+        .output()
+        .expect("Failed to run 'pm path'");
+
+    let raw =
+        std::str::from_utf8(&output.stdout).expect("'pm path' output is not valid UTF-8");
+
+    // `pm path` returns a line like `package:/data/app/…/base.apk`; we only
+    // need the part after the first colon.
+    let apk_path = raw
+        .split_once(':')
+        .map(|(_, after)| after.trim())
+        .unwrap_or("");
+
+    if apk_path.is_empty() {
         eprintln!("Apk not found, have you installed it successfully?");
-        return "".to_string();
+        return None;
     }
-    let full_path = String::from("CLASSPATH=") + &(raw_path.to_string());
+
+    let full_path = format!("CLASSPATH={}", apk_path);
     println!("Path {}", full_path);
 
-    full_path
+    Some(full_path)
 }
 
-fn startup_service_and_wait(port: &String, full_path: String) {
-    // app_process
-    let port_param = String::from("--port=") + &port;
-    let params = vec![
+/// Spawns `app_process` via `adb shell` with the DroidCast main class and
+/// waits for the process to finish.
+fn startup_service_and_wait(port: &str, full_path: String, serial: Option<&str>) {
+    let port_param = format!("--port={}", port);
+    let params = [
         "shell",
         full_path.trim(),
         "app_process",
         "/",
         "com.rayworks.droidcast.Main",
-        port_param.as_str().trim(),
+        port_param.trim(),
     ];
     println!("Params -> {:?}", params);
 
-    let result = serial_checked_command().args(params)
+    let result = serial_checked_command(serial)
+        .args(params)
         .spawn()
-        .unwrap()
+        .expect("Failed to spawn app_process")
         .wait_with_output()
-        .expect("Failed to wait for a Child Process");
+        .expect("Failed to wait for app_process");
 
     println!("status: {}", result.status);
 }
 
-fn forward_connection(port: &String) {
-    let grp = String::from("tcp:") + &port;
-    let params_fwd = vec!["forward", &grp.trim(), &grp.trim()];
+/// Forwards the local TCP port to the same port on the device using `adb forward`.
+fn forward_connection(port: &str, serial: Option<&str>) {
+    let grp = format!("tcp:{}", port);
+    let params_fwd = ["forward", grp.trim(), grp.trim()];
     println!("Params -> {:?}", params_fwd);
 
-    serial_checked_command().args(params_fwd).output().expect("Failed to forward the tcp connection");
+    serial_checked_command(serial)
+        .args(params_fwd)
+        .output()
+        .expect("Failed to forward the tcp connection");
 }
 
-fn unforward_connection(port: &String) {
-    let tcp = format!("tcp:{}", &port);
-    let params_fwd = vec!["forward", "--remove", &tcp];
+/// Removes the previously established port forward via `adb forward --remove`.
+fn unforward_connection(port: &str, serial: Option<&str>) {
+    let tcp = format!("tcp:{}", port);
+    let params_fwd = ["forward", "--remove", &tcp];
 
-    let status = serial_checked_command().args(params_fwd).output().unwrap().status;
+    let status = serial_checked_command(serial)
+        .args(params_fwd)
+        .output()
+        .expect("Failed to run 'adb forward --remove'")
+        .status;
     println!("adb unforward action status : {:?}", status);
 }
 
-fn setup_signal_handler() -> Result<(), Box<dyn Error>> {
-    let mut signals = Signals::new(&[SIGINT])?;
-
-    thread::spawn(move || {
-        for sig in signals.forever() {
+/// Installs a `SIGINT` handler that removes the active port forward when the
+/// user presses Ctrl-C.  Returns the [`thread::JoinHandle`] for the background
+/// signal-handling thread together with a [`Handle`] that can be used to close
+/// the signals iterator (and thus allow the thread to return) on a clean exit.
+/// `port` and `serial` are moved into the thread so `unforward_connection` can
+/// be called from within the handler.
+fn setup_signal_handler(
+    port: String,
+    serial: Option<String>,
+) -> Result<(thread::JoinHandle<()>, Handle), Box<dyn Error>> {
+    let mut signals = Signals::new([SIGINT])?;
+    let handle = signals.handle();
+
+    let join_handle = thread::spawn(move || {
+        if let Some(sig) = signals.forever().next() {
             println!("\nReceived signal {:?}", sig);
+            unforward_connection(&port, serial.as_deref());
         }
     });
 
-    Ok(())
+    Ok((join_handle, handle))
 }
 
-fn open_browser() {
-    let args: Vec<String> = env::args().collect();
-
-    let ip_param = vec!["shell", "ip route | awk '/wlan*/{ print $9 }'| tr -d '\n'"];
-    let ip = serial_checked_command().args(ip_param).output().unwrap().stdout;
-    let ip = std::str::from_utf8(&ip).unwrap().to_string();
-    println!(">>> Share the url 'http://{}:{}/screenshot' to see the live screen", ip, args[1]);
-
-    let url = format!("http://localhost:{}/screenshot", args[1]);
+/// Retrieves the device's WLAN IP address, prints a shareable URL, and opens
+/// a local screenshot URL in the default browser.
+fn open_browser(port: &str, serial: Option<&str>) {
+    let ip_param = ["shell", "ip route | awk '/wlan*/{ print $9 }'| tr -d '\n'"];
+    let ip_bytes = serial_checked_command(serial)
+        .args(ip_param)
+        .output()
+        .expect("Failed to retrieve device IP address")
+        .stdout;
+    let ip = std::str::from_utf8(&ip_bytes)
+        .expect("Device IP output is not valid UTF-8");
+    println!(
+        ">>> Share the url 'http://{}:{}/screenshot' to see the live screen",
+        ip, port
+    );
+
+    let url = format!("http://localhost:{}/screenshot", port);
     if webbrowser::open(&url).is_err() {
         println!("Failed to open browser");
     }