refactor: dedupe helpers, name protocol constants, testable daemon args

5 files changed, 189 insertions, 159 deletions

diff --git a/Cargo.toml b/Cargo.toml
index 62f1e9a..b1157f8 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -2,7 +2,7 @@
 name = "mmx150"
 version = "0.1.0"
 edition = "2021"
-description = "Battery and connection monitoring for the beyerdynamic MMX 150 wireless dongle"
+description = "Connection monitoring for the beyerdynamic MMX 150 wireless dongle"
 license = "MIT OR Apache-2.0"
 
 [lib]
diff --git a/src/bin/probe.rs b/src/bin/probe.rs
index e77e6cd..008565c 100644
--- a/src/bin/probe.rs
+++ b/src/bin/probe.rs
@@ -1,8 +1,8 @@
 //! mmx150-probe — reverse-engineering tool for the MMX 150 dongle protocol.
 //!
-//! Phase 1 workflow: send candidate output reports to the dongle and observe
-//! what input reports come back, correlating fields with headset on/off,
-//! battery charge level and button presses.
+//! Sends candidate output reports to the dongle and prints what input
+//! reports come back. Kept for future protocol work: battery status, for
+//! example, is not exposed by any command found so far.
 
 use std::process::ExitCode;
 use std::time::Duration;
@@ -25,6 +25,22 @@ fn main() -> ExitCode {
     }
 }
 
+/// Find and open the dongle, printing an error and returning `None` if it
+/// is missing or cannot be opened.
+fn open_dongle() -> Option<Hidraw> {
+    let Some(dev) = mmx150::hid::find_mmx150().unwrap_or(None) else {
+        eprintln!("MMX 150 dongle not found");
+        return None;
+    };
+    match Hidraw::open(&dev.devnode) {
+        Ok(h) => Some(h),
+        Err(e) => {
+            eprintln!("open failed: {e}");
+            None
+        }
+    }
+}
+
 fn cmd_list(_rest: &[String]) -> ExitCode {
     match find_devices_by_vendor(BEYERDYNAMIC_VID) {
         Ok(devices) => {
@@ -120,18 +136,9 @@ fn cmd_send(rest: &[String]) -> ExitCode {
         data.extend(bytes);
     }
 
-    let Some(dev) = mmx150::hid::find_mmx150().unwrap_or(None) else {
-        eprintln!("MMX 150 dongle not found");
+    let Some(mut hid) = open_dongle() else {
         return ExitCode::FAILURE;
     };
-    let hid = match Hidraw::open(&dev.devnode) {
-        Ok(h) => h,
-        Err(e) => {
-            eprintln!("open failed: {e}");
-            return ExitCode::FAILURE;
-        }
-    };
-    let mut hid = hid;
 
     println!(
         ">> 0x{report_id:02x} [{}B] {}",
@@ -160,25 +167,12 @@ fn send_and_read(hid: &mut Hidraw, data: &[u8]) -> Option<Vec<u8>> {
         .flatten()
 }
 
-/// Sweep a set of candidate commands against the dongle, printing responses
-/// that differ from the all-zero baseline. Reports only non-trivial responses.
-///
-/// usage: mmx150-probe sweep <report_id> <B> ...
-///   <report_id> is the output report id to use,
-///   <B> is the report payload length (bytes, excluding the id byte).
+/// Send a fixed set of diagnostic commands (one per known report family)
+/// and print their responses side by side.
 fn cmd_status_dump() -> ExitCode {
-    let Some(dev) = mmx150::hid::find_mmx150().unwrap_or(None) else {
-        eprintln!("MMX 150 dongle not found");
+    let Some(mut hid) = open_dongle() else {
         return ExitCode::FAILURE;
     };
-    let hid = match Hidraw::open(&dev.devnode) {
-        Ok(h) => h,
-        Err(e) => {
-            eprintln!("open failed: {e}");
-            return ExitCode::FAILURE;
-        }
-    };
-    let mut hid = hid;
 
     // Each row: label, report id, command payload.
     let cmds: Vec<(&str, u8, Vec<u8>)> = vec![
@@ -252,18 +246,9 @@ fn cmd_sel_enum(rest: &[String]) -> ExitCode {
         .and_then(|s| u8::from_str_radix(s.trim_start_matches("0x"), 16).ok())
         .unwrap_or(0x3a);
 
-    let Some(dev) = mmx150::hid::find_mmx150().unwrap_or(None) else {
-        eprintln!("MMX 150 dongle not found");
+    let Some(mut hid) = open_dongle() else {
         return ExitCode::FAILURE;
     };
-    let hid = match Hidraw::open(&dev.devnode) {
-        Ok(h) => h,
-        Err(e) => {
-            eprintln!("open failed: {e}");
-            return ExitCode::FAILURE;
-        }
-    };
-    let mut hid = hid;
 
     println!("sweeping report 0x{report_id:02x}, selector range 0x00..0xff");
     for sel in 0x00u8..=0xffu8 {
@@ -304,18 +289,9 @@ fn cmd_sweep(rest: &[String]) -> ExitCode {
     };
     let plen: usize = rest.get(1).and_then(|s| s.parse().ok()).unwrap_or(60);
 
-    let Some(dev) = mmx150::hid::find_mmx150().unwrap_or(None) else {
-        eprintln!("MMX 150 dongle not found");
+    let Some(mut hid) = open_dongle() else {
         return ExitCode::FAILURE;
     };
-    let hid = match Hidraw::open(&dev.devnode) {
-        Ok(h) => h,
-        Err(e) => {
-            eprintln!("open failed: {e}");
-            return ExitCode::FAILURE;
-        }
-    };
-    let mut hid = hid;
 
     // Baseline: all-zero command.
     let base = {
diff --git a/src/hid.rs b/src/hid.rs
index 6b09e98..72f67fa 100644
--- a/src/hid.rs
+++ b/src/hid.rs
@@ -63,40 +63,7 @@ pub struct HidDevice {
 /// Enumerate hidraw devices matching `vid`/`pid` by reading the sysfs `uevent`
 /// of each `/sys/class/hidraw/hidrawN`.
 pub fn find_devices(vid: u16, pid: u16) -> io::Result<Vec<HidDevice>> {
-    let mut out = Vec::new();
-    for entry in fs::read_dir(HIDRAW_SYSFS)? {
-        let entry = entry?;
-        let name = entry.file_name();
-        let Some(name) = name.to_str() else {
-            continue;
-        };
-        if !name.starts_with("hidraw") {
-            continue;
-        }
-        let Some(uevent) = read_sysfs(&entry.path().join("device/uevent")) else {
-            continue;
-        };
-        let Some((bus, v, p)) = parse_hid_id(&uevent) else {
-            continue;
-        };
-        if bus != 3 {
-            continue; // USB only
-        }
-        if v == vid && p == pid {
-            let dev_name = uevent
-                .lines()
-                .find(|l| l.starts_with("HID_NAME="))
-                .map(|l| l.trim_start_matches("HID_NAME=").to_string())
-                .unwrap_or_default();
-            out.push(HidDevice {
-                vid: v,
-                pid: p,
-                name: dev_name,
-                devnode: PathBuf::from("/dev").join(name),
-            });
-        }
-    }
-    Ok(out)
+    find_with(|v, p| v == vid && p == pid)
 }
 
 /// Find the first (only) MMX 150 dongle HID device.
@@ -108,6 +75,12 @@ pub fn find_mmx150() -> io::Result<Option<HidDevice>> {
 
 /// Enumerate all hidraw devices belonging to a vendor.
 pub fn find_devices_by_vendor(vid: u16) -> io::Result<Vec<HidDevice>> {
+    find_with(|v, _| v == vid)
+}
+
+/// Collect USB hidraw devices whose `HID_ID` vendor/product pair matches
+/// `keep`. Returns one [`HidDevice`] per match.
+fn find_with(mut keep: impl FnMut(u16, u16) -> bool) -> io::Result<Vec<HidDevice>> {
     let mut out = Vec::new();
     for entry in fs::read_dir(HIDRAW_SYSFS)? {
         let entry = entry?;
@@ -124,8 +97,8 @@ pub fn find_devices_by_vendor(vid: u16) -> io::Result<Vec<HidDevice>> {
         let Some((bus, v, p)) = parse_hid_id(&uevent) else {
             continue;
         };
-        if bus != 3 || v != vid {
-            continue;
+        if bus != 3 || !keep(v, p) {
+            continue; // USB only, filtered by caller
         }
         let dev_name = uevent
             .lines()
@@ -160,11 +133,6 @@ impl Hidraw {
         })
     }
 
-    /// Returns the underlying raw file descriptor.
-    pub fn as_raw_fd(&self) -> std::os::fd::RawFd {
-        self.file.as_raw_fd()
-    }
-
     /// `HIDIOCGFEATURE`: fetch `report_id` into `data` (report length + 1
     /// bytes including the report-id byte). Returns bytes received.
     pub fn get_feature(&self, report_id: u8, len: usize) -> io::Result<Vec<u8>> {
@@ -266,6 +234,12 @@ impl Hidraw {
     }
 }
 
+impl std::os::fd::AsRawFd for Hidraw {
+    fn as_raw_fd(&self) -> std::os::fd::RawFd {
+        self.file.as_raw_fd()
+    }
+}
+
 fn read_sysfs(path: &Path) -> Option<String> {
     fs::read_to_string(path).ok()
 }
diff --git a/src/main.rs b/src/main.rs
index 1a704fc..8582db1 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -12,7 +12,6 @@ fn main() -> ExitCode {
     match args.first().map(String::as_str) {
         Some("status") => cmd_status(),
         Some("daemon") => cmd_daemon(&args[1..]),
-        Some("probe") => cmd_probe(),
         _ => {
             print_usage();
             ExitCode::FAILURE
@@ -46,14 +45,13 @@ fn cmd_status() -> ExitCode {
         eprintln!("dongle not found");
         return ExitCode::from(2);
     };
-    let hid = match Hidraw::open(&dev.devnode) {
+    let mut hid = match Hidraw::open(&dev.devnode) {
         Ok(h) => h,
         Err(e) => {
             eprintln!("error: {e}");
             return ExitCode::from(2);
         }
     };
-    let mut hid = hid;
     match protocol::link_status(&mut hid) {
         Ok(LinkState::Connected) => {
             println!("connected");
@@ -69,47 +67,17 @@ fn cmd_status() -> ExitCode {
         }
     }
 }
-
 fn cmd_daemon(args: &[String]) -> ExitCode {
-    let mut on_connect: Option<String> = None;
-    let mut on_disconnect: Option<String> = None;
-    let mut interval = Duration::from_secs(2);
-    let mut no_initial = false;
-
-    let mut it = args.iter();
-    while let Some(arg) = it.next() {
-        match arg.as_str() {
-            "--on-connect" => match it.next() {
-                Some(c) => on_connect = Some(c.clone()),
-                None => {
-                    eprintln!("--on-connect needs an argument");
-                    return ExitCode::FAILURE;
-                }
-            },
-            "--on-disconnect" => match it.next() {
-                Some(c) => on_disconnect = Some(c.clone()),
-                None => {
-                    eprintln!("--on-disconnect needs an argument");
-                    return ExitCode::FAILURE;
-                }
-            },
-            "--interval" => match it.next().and_then(|s| s.parse::<u64>().ok()) {
-                Some(secs) if secs > 0 => interval = Duration::from_secs(secs),
-                _ => {
-                    eprintln!("--interval needs a number of seconds > 0");
-                    return ExitCode::FAILURE;
-                }
-            },
-            "--no-initial" => no_initial = true,
-            other => {
-                eprintln!("unknown option: {other}");
-                return ExitCode::FAILURE;
-            }
+    let opts = match DaemonArgs::parse(args) {
+        Ok(o) => o,
+        Err(e) => {
+            eprintln!("{e}");
+            return ExitCode::FAILURE;
         }
-    }
+    };
 
     let mut hid: Option<Hidraw> = None;
-    let mut tracker = StateTracker::new(!no_initial);
+    let mut tracker = StateTracker::new(opts.fire_initial);
     let mut children: Vec<Child> = Vec::new();
 
     loop {
@@ -120,13 +88,13 @@ fn cmd_daemon(args: &[String]) -> ExitCode {
         match tracker.update(link_up) {
             Some(Transition::ToConnected { initial }) => {
                 log_state("connected", initial);
-                if let Some(cmd) = &on_connect {
+                if let Some(cmd) = &opts.on_connect {
                     spawn_hook(cmd, &mut children);
                 }
             }
             Some(Transition::ToDisconnected { initial }) => {
                 log_state("disconnected", initial);
-                if let Some(cmd) = &on_disconnect {
+                if let Some(cmd) = &opts.on_disconnect {
                     spawn_hook(cmd, &mut children);
                 }
             }
@@ -134,8 +102,8 @@ fn cmd_daemon(args: &[String]) -> ExitCode {
         }
 
         let elapsed = cycle.elapsed();
-        if elapsed < interval {
-            std::thread::sleep(interval - elapsed);
+        if elapsed < opts.interval {
+            std::thread::sleep(opts.interval - elapsed);
         }
     }
 }
@@ -200,7 +168,106 @@ fn timestamp() -> u64 {
         .unwrap_or(0)
 }
 
-fn cmd_probe() -> ExitCode {
-    eprintln!("use `mmx150-probe` binary instead");
-    ExitCode::FAILURE
+/// Parsed `mmx150 daemon` command line.
+#[derive(Debug, PartialEq)]
+struct DaemonArgs {
+    on_connect: Option<String>,
+    on_disconnect: Option<String>,
+    interval: Duration,
+    fire_initial: bool,
+}
+
+impl DaemonArgs {
+    fn parse(args: &[String]) -> Result<DaemonArgs, String> {
+        let mut on_connect = None;
+        let mut on_disconnect = None;
+        let mut interval = Duration::from_secs(2);
+        let mut fire_initial = true;
+
+        let mut it = args.iter();
+        while let Some(arg) = it.next() {
+            match arg.as_str() {
+                "--on-connect" => {
+                    on_connect = Some(it.next().ok_or("--on-connect needs an argument")?.clone())
+                }
+                "--on-disconnect" => {
+                    on_disconnect = Some(
+                        it.next()
+                            .ok_or("--on-disconnect needs an argument")?
+                            .clone(),
+                    )
+                }
+                "--interval" => {
+                    let secs = it
+                        .next()
+                        .and_then(|s| s.parse::<u64>().ok())
+                        .filter(|&s| s > 0)
+                        .ok_or("--interval needs a number of seconds > 0")?;
+                    interval = Duration::from_secs(secs);
+                }
+                "--no-initial" => fire_initial = false,
+                other => return Err(format!("unknown option: {other}")),
+            }
+        }
+        Ok(DaemonArgs {
+            on_connect,
+            on_disconnect,
+            interval,
+            fire_initial,
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn args(list: &[&str]) -> Vec<String> {
+        list.iter().map(|s| s.to_string()).collect()
+    }
+
+    #[test]
+    fn daemon_args_defaults() {
+        let a = DaemonArgs::parse(&args(&[])).unwrap();
+        assert_eq!(a.interval, Duration::from_secs(2));
+        assert!(a.fire_initial);
+        assert_eq!(a.on_connect, None);
+        assert_eq!(a.on_disconnect, None);
+    }
+
+    #[test]
+    fn daemon_args_full_parse() {
+        let a = DaemonArgs::parse(&args(&[
+            "--on-connect",
+            "echo up",
+            "--on-disconnect",
+            "echo down",
+            "--interval",
+            "5",
+            "--no-initial",
+        ]))
+        .unwrap();
+        assert_eq!(a.on_connect.as_deref(), Some("echo up"));
+        assert_eq!(a.on_disconnect.as_deref(), Some("echo down"));
+        assert_eq!(a.interval, Duration::from_secs(5));
+        assert!(!a.fire_initial);
+    }
+
+    #[test]
+    fn daemon_args_rejects_missing_option_argument() {
+        assert!(DaemonArgs::parse(&args(&["--on-connect"])).is_err());
+        assert!(DaemonArgs::parse(&args(&["--on-disconnect"])).is_err());
+        assert!(DaemonArgs::parse(&args(&["--interval"])).is_err());
+    }
+
+    #[test]
+    fn daemon_args_rejects_bad_interval() {
+        assert!(DaemonArgs::parse(&args(&["--interval", "0"])).is_err());
+        assert!(DaemonArgs::parse(&args(&["--interval", "x"])).is_err());
+    }
+
+    #[test]
+    fn daemon_args_rejects_unknown_option() {
+        assert!(DaemonArgs::parse(&args(&["--wat"])).is_err());
+    }
 }
diff --git a/src/protocol.rs b/src/protocol.rs
index 3a24195..d6fce5f 100644
--- a/src/protocol.rs
+++ b/src/protocol.rs
@@ -36,6 +36,15 @@ pub const LINK_DEBOUNCE_MISSES: u32 = 2;
 /// How long to wait for a link response after sending the query.
 const LINK_RESPONSE_TIMEOUT: Duration = Duration::from_millis(400);
 
+/// Input report id the dongle answers with when the headset is linked.
+const LINK_RESPONSE_REPORT: u8 = 0x3c;
+/// Selector byte we query with; the response echoes it back.
+const LINK_SELECTOR: u8 = 0x01;
+/// Command prefix at the start of the query payload.
+const CMD_PREFIX: [u8; 2] = [0x09, 0x22];
+/// Link query frame size: report id + 60-byte payload.
+const LINK_QUERY_LEN: usize = 61;
+
 /// Headset link state as seen through the dongle.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum LinkState {
@@ -52,20 +61,20 @@ pub enum Transition {
     ToDisconnected { initial: bool },
 }
 
-/// Builds the 61-byte link query output report (report id `0x3a` + payload).
-pub fn link_query_frame() -> Vec<u8> {
-    let mut frame = vec![0u8; 61];
+/// Builds the link query output report (report id `0x3a` + payload).
+pub fn link_query_frame() -> [u8; LINK_QUERY_LEN] {
+    let mut frame = [0u8; LINK_QUERY_LEN];
     frame[0] = reports::OUT_60_C;
-    frame[1] = 0x09;
-    frame[2] = 0x22;
-    frame[3] = 0x01;
+    frame[1] = CMD_PREFIX[0];
+    frame[2] = CMD_PREFIX[1];
+    frame[3] = LINK_SELECTOR;
     frame
 }
 
 /// True if `data` is a link-status response report (`0x3c` echoing the
 /// query selector).
 pub fn is_link_response(data: &[u8]) -> bool {
-    data.len() >= 2 && data[0] == 0x3c && data[1] == 0x01
+    data.len() >= 2 && data[0] == LINK_RESPONSE_REPORT && data[1] == LINK_SELECTOR
 }
 
 /// Queries the dongle once for the headset link state. Returns
@@ -116,25 +125,29 @@ impl StateTracker {
             if self.state == Some(LinkState::Connected) {
                 return None;
             }
-            let initial = self.state.is_none();
-            self.state = Some(LinkState::Connected);
-            if initial && !self.fire_initial {
-                None
-            } else {
-                Some(Transition::ToConnected { initial })
-            }
+            self.settle(LinkState::Connected)
         } else {
             self.misses += 1;
             if self.misses < LINK_DEBOUNCE_MISSES || self.state == Some(LinkState::Disconnected) {
                 return None;
             }
-            let initial = self.state.is_none();
-            self.state = Some(LinkState::Disconnected);
-            if initial && !self.fire_initial {
-                None
-            } else {
-                Some(Transition::ToDisconnected { initial })
-            }
+            self.settle(LinkState::Disconnected)
+        }
+    }
+
+    /// Pin the state, producing the transition unless it is the first
+    /// determination and initial hooks are suppressed.
+    fn settle(&mut self, state: LinkState) -> Option<Transition> {
+        let initial = self.state.is_none();
+        self.state = Some(state);
+        let transition = match state {
+            LinkState::Connected => Transition::ToConnected { initial },
+            LinkState::Disconnected => Transition::ToDisconnected { initial },
+        };
+        if initial && !self.fire_initial {
+            None
+        } else {
+            Some(transition)
         }
     }
 }