jumperless-mcp 0.1.0

//! Analog I/O ToolDefs: `dac_set`, `dac_get`, `adc_get`, `adc_get_stats`
//!
//! Wraps the Jumperless V5 DAC and ADC Python bindings.
//!
//! ## Channel encoding
//! - DAC int channels 0..=3 are passed directly as integers.
//! - DAC string aliases `"DAC0"`, `"DAC1"`, `"TOP_RAIL"`, `"BOTTOM_RAIL"` are
//!   passed as bare identifiers (module-level constants in firmware). Any other
//!   string is rejected with a Protocol error before the device is contacted.
//! - ADC channels 0..=3 only. Channel 4 (5V-tolerant input) is outside the
//!   task scope; a separate tool can be added if needed.
//!
//! ## Voltage range
//! - DAC set range: -8.0 V to +8.0 V (V5 amplified DAC per spec §Power).
//!   Out-of-range voltages are rejected at the tool boundary — before any bytes
//!   are sent to the device — to protect hardware.
//!
//! ## adc_get_stats implementation
//! Firmware does not expose a stats primitive (spec §6.5). This tool implements
//! multi-sample statistics on the MCP side by calling `adc_get(channel)` N times
//! and computing mean / min / max / sample-stddev (÷ n-1) in f64.
//! Default samples = 10 (matching the spec §6.5 example). If samples = 1, stddev
//! is defined as 0.0 (no variance observable from a single reading).

use crate::base::{McpError, ToolDescriptor};
use serde_json::{json, Value};
use std::io::{Read, Write};

use crate::library::exec_with_cleanup;

// ── Channel / voltage helpers ──────────────────────────────────────────────────

/// Accepted DAC string channel aliases (firmware module-level constants).
const DAC_STRING_CHANNELS: &[&str] = &["DAC0", "DAC1", "TOP_RAIL", "BOTTOM_RAIL"];

/// Validate a DAC channel argument and return the Python expression to pass
/// to the firmware call.
///
/// - `Value::Number` integer 0..=3 → `"0"` / `"1"` / `"2"` / `"3"`
/// - `Value::String` matching a known alias → bare identifier (e.g. `"DAC0"`)
///
/// Rejects: floats, out-of-range ints, unrecognised strings, or strings that
/// contain characters outside `[A-Za-z0-9_]` or exceed 16 characters.
fn encode_dac_channel(v: &Value) -> Result<String, McpError> {
    match v {
        Value::Number(n) => {
            let i = n
                .as_i64()
                .ok_or_else(|| McpError::Protocol("dac channel must be an integer 0-3".into()))?;
            if !(0..=3).contains(&i) {
                return Err(McpError::Protocol(format!(
                    "dac channel integer must be 0-3; got {i}"
                )));
            }
            Ok(i.to_string())
        }
        Value::String(s) => {
            // Validate identifier shape first — prevents injection.
            if s.len() > 16 {
                return Err(McpError::Protocol(format!(
                    "dac channel string too long (max 16 chars): '{s}'"
                )));
            }
            if !s.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
                return Err(McpError::Protocol(format!(
                    "dac channel string contains invalid characters: '{s}'"
                )));
            }
            // Now check against the known-alias whitelist.
            if DAC_STRING_CHANNELS.contains(&s.as_str()) {
                Ok(s.clone()) // passed bare: `dac_set(TOP_RAIL, 3.3)`
            } else {
                Err(McpError::Protocol(format!(
                    "unknown dac channel '{s}'; valid strings: DAC0, DAC1, TOP_RAIL, BOTTOM_RAIL"
                )))
            }
        }
        other => Err(McpError::Protocol(format!(
            "dac channel must be int 0-3 or string alias; got {other}"
        ))),
    }
}

/// Validate ADC channel: integer 0..=3 only.
fn validate_adc_channel(v: &Value) -> Result<i64, McpError> {
    let i = v
        .as_i64()
        .ok_or_else(|| McpError::Protocol("adc channel must be an integer".into()))?;
    if !(0..=3).contains(&i) {
        return Err(McpError::Protocol(format!(
            "adc channel must be 0-3; got {i}"
        )));
    }
    Ok(i)
}

/// Validate DAC voltage: -8.0 to +8.0, finite (NaN/±inf rejected).
///
/// NaN comparison gotcha: `NaN <= 8.0` and `(-8.0..=8.0).contains(&NaN)` both
/// evaluate to `false`, so a range check ALONE lets NaN through. The `is_finite`
/// guard is the actual NaN/±inf rejection. Without it, Rust's `format!("{nan}")`
/// emits the string "NaN" which MicroPython parses as a NameError on the device —
/// violating the validate-before-touching-the-device contract.
fn validate_dac_voltage(v: &Value) -> Result<f64, McpError> {
    let f = v
        .as_f64()
        .ok_or_else(|| McpError::Protocol("voltage must be a number".into()))?;
    if !f.is_finite() {
        return Err(McpError::Protocol(format!(
            "voltage must be finite (not NaN or infinity); got {f}"
        )));
    }
    if !(-8.0..=8.0).contains(&f) {
        return Err(McpError::Protocol(format!(
            "voltage {f} out of range; must be -8.0 to +8.0 V"
        )));
    }
    Ok(f)
}

// ── ToolDescriptors ───────────────────────────────────────────────────────────

/// Build the `dac_set` [`ToolDescriptor`].
pub fn dac_set_descriptor() -> ToolDescriptor {
    ToolDescriptor::with_timeout(
        "dac_set",
        "Set a DAC output voltage on the Jumperless V5. \
         channel: integer 0-3 OR one of \"DAC0\", \"DAC1\", \"TOP_RAIL\", \"BOTTOM_RAIL\". \
         voltage: float, range -8.0 to +8.0 V (V5 amplified DAC). \
         save: if true (default), persist the new voltage to the active slot. \
         Out-of-range voltages are rejected before contacting the device. \
         Returns {\"set\": true, \"channel\": <as-passed>, \"voltage\": float, \"save\": bool}.",
        json!({
            "type": "object",
            "properties": {
                "channel": {
                    "oneOf": [
                        {
                            "type": "integer",
                            "minimum": 0,
                            "maximum": 3,
                            "description": "DAC channel as integer (0=DAC0, 1=DAC1, 2=TOP_RAIL, 3=BOTTOM_RAIL)"
                        },
                        {
                            "type": "string",
                            "enum": ["DAC0", "DAC1", "TOP_RAIL", "BOTTOM_RAIL"],
                            "description": "DAC channel as named constant"
                        }
                    ]
                },
                "voltage": {
                    "type": "number",
                    "minimum": -8.0,
                    "maximum": 8.0,
                    "description": "Output voltage in Volts. Range: -8.0 to +8.0."
                },
                "save": {
                    "type": "boolean",
                    "description": "Persist to active slot. Default: true."
                }
            },
            "required": ["channel", "voltage"],
            "additionalProperties": false
        }),
        1_500,
    )
}

/// Build the `dac_get` [`ToolDescriptor`].
pub fn dac_get_descriptor() -> ToolDescriptor {
    ToolDescriptor::with_timeout(
        "dac_get",
        "Read the currently set output voltage for a DAC channel on the Jumperless V5. \
         channel: integer 0-3 OR one of \"DAC0\", \"DAC1\", \"TOP_RAIL\", \"BOTTOM_RAIL\". \
         Returns {\"voltage\": float, \"channel\": <as-passed>}.",
        json!({
            "type": "object",
            "properties": {
                "channel": {
                    "oneOf": [
                        {
                            "type": "integer",
                            "minimum": 0,
                            "maximum": 3,
                            "description": "DAC channel as integer (0=DAC0, 1=DAC1, 2=TOP_RAIL, 3=BOTTOM_RAIL)"
                        },
                        {
                            "type": "string",
                            "enum": ["DAC0", "DAC1", "TOP_RAIL", "BOTTOM_RAIL"],
                            "description": "DAC channel as named constant"
                        }
                    ]
                }
            },
            "required": ["channel"],
            "additionalProperties": false
        }),
        1_500,
    )
}

/// Build the `adc_get` [`ToolDescriptor`].
pub fn adc_get_descriptor() -> ToolDescriptor {
    ToolDescriptor::with_timeout(
        "adc_get",
        "Read the current voltage from an ADC channel on the Jumperless V5. \
         channel: integer 0-3. \
         Returns {\"voltage\": float, \"channel\": int}.",
        json!({
            "type": "object",
            "properties": {
                "channel": {
                    "type": "integer",
                    "minimum": 0,
                    "maximum": 3,
                    "description": "ADC channel 0-3."
                }
            },
            "required": ["channel"],
            "additionalProperties": false
        }),
        1_500,
    )
}

/// Build the `adc_get_stats` [`ToolDescriptor`].
pub fn adc_get_stats_descriptor() -> ToolDescriptor {
    ToolDescriptor::with_timeout(
        "adc_get_stats",
        "Read multiple ADC samples and return summary statistics for a channel. \
         Calls adc_get(channel) N times on the MCP side and computes mean, min, max, \
         and sample standard deviation (÷ n-1). \
         channel: integer 0-3. \
         samples: number of readings (default 10, minimum 1). \
         Returns {\"mean\": float, \"min\": float, \"max\": float, \"stddev\": float, \
         \"samples\": int, \"channel\": int}. \
         stddev is 0.0 when samples=1.",
        json!({
            "type": "object",
            "properties": {
                "channel": {
                    "type": "integer",
                    "minimum": 0,
                    "maximum": 3,
                    "description": "ADC channel 0-3."
                },
                "samples": {
                    "type": "integer",
                    "minimum": 1,
                    "description": "Number of samples to collect. Default: 10. Minimum: 1."
                }
            },
            "required": ["channel"],
            "additionalProperties": false
        }),
        3_000,
    )
}

/// Return all four analog ToolDescriptors.
pub fn descriptors() -> Vec<ToolDescriptor> {
    vec![
        dac_set_descriptor(),
        dac_get_descriptor(),
        adc_get_descriptor(),
        adc_get_stats_descriptor(),
    ]
}

// ── Handlers ─────────────────────────────────────────────────────────────────

/// Set a DAC channel voltage.
///
/// Validates voltage range and channel before sending anything to the device.
/// Calls `dac_set(channel, voltage, save)` where channel is either an int
/// literal or a bare identifier constant.
pub fn handle_dac_set<P: Read + Write + ?Sized>(
    port: &mut P,
    args: &Value,
) -> Result<Value, McpError> {
    let channel_val = args
        .get("channel")
        .ok_or_else(|| McpError::Protocol("missing required arg: channel".into()))?;
    let voltage_val = args
        .get("voltage")
        .ok_or_else(|| McpError::Protocol("missing required arg: voltage".into()))?;

    let channel_expr = encode_dac_channel(channel_val)?;
    let voltage = validate_dac_voltage(voltage_val)?;

    // Strict bool validation — non-bool 'save' is an Err, NOT silent default true.
    // Mirrors the pattern in handle_state_set::clear_first and handle_connect::duplicates.
    let save = match args.get("save") {
        Some(v) => v
            .as_bool()
            .ok_or_else(|| McpError::Protocol("dac_set: 'save' must be a boolean".into()))?,
        None => true,
    };

    // Pass save as Python bool literal. Drop the print() wrapper — dac_set() returns
    // None per firmware contract, so we have no stdout to inspect.
    let save_py = if save { "True" } else { "False" };
    let code = format!("dac_set({channel_expr}, {voltage}, {save_py})");

    exec_with_cleanup(port, &code, "dac_set")?;
    // Use 'set: true' for cross-family consistency (gpio_set, oled_print, wavegen_set_*
    // all use 'set'). Previously this returned 'ok: true' — clients normalizing across
    // set-family responses would have hit a key mismatch.
    Ok(json!({
        "set": true,
        "channel": channel_val,
        "voltage": voltage,
        "save": save
    }))
}

/// Read the current DAC voltage for a channel.
///
/// Calls `dac_get(channel)`. Parses the response as f64.
/// Returns `{"voltage": float, "channel": <as-passed>}`.
pub fn handle_dac_get<P: Read + Write + ?Sized>(
    port: &mut P,
    args: &Value,
) -> Result<Value, McpError> {
    let channel_val = args
        .get("channel")
        .ok_or_else(|| McpError::Protocol("missing required arg: channel".into()))?;
    let channel_expr = encode_dac_channel(channel_val)?;
    let code = format!("print(dac_get({channel_expr}))");
    let resp = exec_with_cleanup(port, &code, "dac_get")?;
    let voltage: f64 = resp.stdout.trim().parse().map_err(|_| {
        McpError::Protocol(format!(
            "dac_get: unexpected device response: '{}'",
            resp.stdout.trim()
        ))
    })?;
    Ok(json!({ "voltage": voltage, "channel": channel_val }))
}

/// Read a single ADC measurement.
///
/// Calls `adc_get(channel)`. Parses the response as f64.
/// Returns `{"voltage": float, "channel": int}`.
pub fn handle_adc_get<P: Read + Write + ?Sized>(
    port: &mut P,
    args: &Value,
) -> Result<Value, McpError> {
    let channel_val = args
        .get("channel")
        .ok_or_else(|| McpError::Protocol("missing required arg: channel".into()))?;
    let channel = validate_adc_channel(channel_val)?;
    let code = format!("print(adc_get({channel}))");
    let resp = exec_with_cleanup(port, &code, "adc_get")?;
    let voltage: f64 = resp.stdout.trim().parse().map_err(|_| {
        McpError::Protocol(format!(
            "adc_get: unexpected device response: '{}'",
            resp.stdout.trim()
        ))
    })?;
    Ok(json!({ "voltage": voltage, "channel": channel }))
}

/// Read N ADC samples and return summary statistics.
///
/// Calls `adc_get(channel)` N times on the MCP side. Computes:
/// - mean
/// - min
/// - max
/// - sample stddev (÷ n-1); 0.0 when n=1
///
/// Does not depend on a device-side `adc_get_stats()` — implements per spec §6.5.
pub fn handle_adc_get_stats<P: Read + Write + ?Sized>(
    port: &mut P,
    args: &Value,
) -> Result<Value, McpError> {
    let channel_val = args
        .get("channel")
        .ok_or_else(|| McpError::Protocol("missing required arg: channel".into()))?;
    let channel = validate_adc_channel(channel_val)?;

    let samples = match args.get("samples") {
        Some(v) => {
            let n = v
                .as_i64()
                .ok_or_else(|| McpError::Protocol("samples must be an integer".into()))?;
            if n < 1 {
                return Err(McpError::Protocol(format!("samples must be >= 1; got {n}")));
            }
            n as usize
        }
        None => 10,
    };

    let code = format!("print(adc_get({channel}))");
    let mut readings: Vec<f64> = Vec::with_capacity(samples);

    for i in 0..samples {
        let resp = exec_with_cleanup(port, &code, "adc_get_stats")?;
        let v: f64 = resp.stdout.trim().parse().map_err(|_| {
            McpError::Protocol(format!(
                "adc_get_stats: unexpected device response on sample {i}: '{}'",
                resp.stdout.trim()
            ))
        })?;
        readings.push(v);
    }

    // Statistics — all f64.
    let n = readings.len() as f64;
    let sum: f64 = readings.iter().sum();
    let mean = sum / n;

    let min = readings.iter().cloned().fold(f64::INFINITY, f64::min);
    let max = readings.iter().cloned().fold(f64::NEG_INFINITY, f64::max);

    // Sample stddev: sqrt(Σ(x - mean)² / (n - 1)).
    // Edge case: n = 1 → stddev = 0.0 (no variance observable from one reading).
    let stddev = if readings.len() == 1 {
        0.0
    } else {
        let variance = readings.iter().map(|x| (x - mean).powi(2)).sum::<f64>() / (n - 1.0);
        variance.sqrt()
    };

    Ok(json!({
        "mean": mean,
        "min": min,
        "max": max,
        "stddev": stddev,
        "samples": readings.len(),
        "channel": channel
    }))
}

// ── Tests ─────────────────────────────────────────────────────────────────────

#[cfg(test)]
mod tests {
    use super::*;
    use std::collections::VecDeque;
    use std::io::{self, Read, Write};

    // ── MockPort ──────────────────────────────────────────────────────────────

    struct MockPort {
        read_data: VecDeque<u8>,
        pub write_data: Vec<u8>,
    }

    #[allow(dead_code)]
    impl MockPort {
        fn with_responses(responses: &[&[u8]]) -> Self {
            let mut buf = Vec::new();
            for r in responses {
                buf.extend_from_slice(r);
            }
            MockPort {
                read_data: VecDeque::from(buf),
                write_data: Vec::new(),
            }
        }

        fn ok_frame() -> Vec<u8> {
            b"OK\x04\x04>".to_vec()
        }

        fn ok_with_stdout(line: &str) -> Vec<u8> {
            let mut v = b"OK".to_vec();
            v.extend_from_slice(line.as_bytes());
            v.push(b'\n');
            v.extend_from_slice(b"\x04\x04>");
            v
        }

        fn error_frame(msg: &str) -> Vec<u8> {
            let mut v = b"OK\x04".to_vec();
            v.extend_from_slice(msg.as_bytes());
            v.push(b'\n');
            v.push(b'\x04');
            v.push(b'>');
            v
        }
    }

    impl Read for MockPort {
        fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
            let n = buf.len().min(self.read_data.len());
            if n == 0 {
                return Err(io::Error::new(
                    io::ErrorKind::UnexpectedEof,
                    "MockPort: no more scripted bytes",
                ));
            }
            for (dst, src) in buf[..n].iter_mut().zip(self.read_data.drain(..n)) {
                *dst = src;
            }
            Ok(n)
        }
    }

    impl Write for MockPort {
        fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
            self.write_data.extend_from_slice(buf);
            Ok(buf.len())
        }
        fn flush(&mut self) -> io::Result<()> {
            Ok(())
        }
    }

    // ── Descriptor tests ──────────────────────────────────────────────────────

    #[test]
    fn all_descriptors_have_correct_names() {
        let descs = descriptors();
        let names: Vec<&str> = descs.iter().map(|d| d.name.as_str()).collect();
        assert!(names.contains(&"dac_set"), "missing dac_set");
        assert!(names.contains(&"dac_get"), "missing dac_get");
        assert!(names.contains(&"adc_get"), "missing adc_get");
        assert!(names.contains(&"adc_get_stats"), "missing adc_get_stats");
        assert_eq!(descs.len(), 4);
    }

    #[test]
    fn all_descriptors_have_object_schema_with_additional_properties_false() {
        for d in descriptors() {
            assert!(
                matches!(d.input_schema, Value::Object(_)),
                "descriptor '{}' must have object input_schema",
                d.name
            );
            assert_eq!(
                d.input_schema.get("additionalProperties"),
                Some(&Value::Bool(false)),
                "descriptor '{}' must have additionalProperties=false",
                d.name
            );
        }
    }

    // ── dac_set: int channel ─────────────────────────────────────────────────

    #[test]
    fn dac_set_int_channel_happy() {
        let frame = MockPort::ok_frame();
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": 0, "voltage": 3.3});
        let result = handle_dac_set(&mut port, &args).unwrap();
        assert_eq!(result["set"], true);
        assert_eq!(result["voltage"], 3.3);
        // Command sent to device must contain the int channel and voltage.
        let sent = String::from_utf8_lossy(&port.write_data);
        assert!(
            sent.contains("dac_set(0, 3.3"),
            "expected int channel in command; got: {sent}"
        );
    }

    #[test]
    fn dac_set_int_channel_3() {
        let frame = MockPort::ok_frame();
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": 3, "voltage": -5.0, "save": false});
        let result = handle_dac_set(&mut port, &args).unwrap();
        assert_eq!(result["set"], true);
        let sent = String::from_utf8_lossy(&port.write_data);
        assert!(
            sent.contains("dac_set(3, -5"),
            "expected channel 3; got: {sent}"
        );
        assert!(
            sent.contains("False"),
            "save=false should emit Python False; got: {sent}"
        );
    }

    // ── dac_set: string channel ──────────────────────────────────────────────

    #[test]
    fn dac_set_string_channel_top_rail() {
        let frame = MockPort::ok_frame();
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": "TOP_RAIL", "voltage": 5.0});
        let result = handle_dac_set(&mut port, &args).unwrap();
        assert_eq!(result["set"], true);
        let sent = String::from_utf8_lossy(&port.write_data);
        // Bare identifier — no quotes around TOP_RAIL in the emitted Python.
        assert!(
            sent.contains("dac_set(TOP_RAIL, 5"),
            "expected bare identifier in command; got: {sent}"
        );
    }

    #[test]
    fn dac_set_string_channel_bottom_rail() {
        let frame = MockPort::ok_frame();
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": "BOTTOM_RAIL", "voltage": -3.3});
        let result = handle_dac_set(&mut port, &args).unwrap();
        assert_eq!(result["set"], true);
        let sent = String::from_utf8_lossy(&port.write_data);
        assert!(sent.contains("dac_set(BOTTOM_RAIL,"), "got: {sent}");
    }

    // ── dac_set: voltage range violation ────────────────────────────────────

    #[test]
    fn dac_set_voltage_too_high_rejected_before_device() {
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": 0, "voltage": 9.0});
        let result = handle_dac_set(&mut port, &args);
        assert!(result.is_err(), "voltage > 8.0 must be rejected");
        // No bytes should have been sent to the device.
        assert!(
            port.write_data.is_empty(),
            "no bytes must be sent for out-of-range voltage"
        );
        match result.unwrap_err() {
            McpError::Protocol(msg) => {
                assert!(
                    msg.contains("out of range") || msg.contains("range"),
                    "got: {msg}"
                );
            }
            other => panic!("expected McpError::Protocol, got: {other:?}"),
        }
    }

    #[test]
    fn dac_set_voltage_too_low_rejected_before_device() {
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": 1, "voltage": -8.1});
        let result = handle_dac_set(&mut port, &args);
        assert!(result.is_err(), "voltage < -8.0 must be rejected");
        assert!(
            port.write_data.is_empty(),
            "no bytes sent for out-of-range voltage"
        );
    }

    // NOTE: NaN/±inf tests omitted — serde_json's `Number::from_f64()` rejects
    // those values (returns None), so a JSON-RPC caller cannot inject them through
    // the wire format. The `is_finite()` guard in `validate_dac_voltage` remains
    // as defense-in-depth for any future code path that constructs a Value
    // directly with non-finite floats (none today). Code-reviewer flagged this
    // as CRITICAL (NaN passthrough) 2026-05-12 — the guard is the fix; testing
    // the guard would require bypassing the JSON layer.

    #[test]
    fn dac_set_save_non_bool_rejected() {
        // Post-fix (IMPORTANT from SF-hunter 2026-05-12): non-bool 'save' must
        // be rejected with Err — NOT silently coerced to default true. This
        // restores the consistency that every other optional bool in the sprint
        // (clear_first, duplicates) already follows.
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": 0, "voltage": 3.3, "save": "yes"});
        let result = handle_dac_set(&mut port, &args);
        assert!(result.is_err(), "string 'save' must be rejected");
        assert!(port.write_data.is_empty());
        match result.unwrap_err() {
            McpError::Protocol(msg) => assert!(
                msg.contains("'save'") && msg.contains("boolean"),
                "error must mention save/boolean; got: {msg}"
            ),
            other => panic!("expected Protocol err, got: {other:?}"),
        }
    }

    #[test]
    fn dac_set_save_null_rejected() {
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": 0, "voltage": 3.3, "save": null});
        let result = handle_dac_set(&mut port, &args);
        // null is explicit "I'm passing nothing" but it's also a type error in
        // a strict contract — same handling as other type mismatches.
        assert!(result.is_err(), "null 'save' must be rejected");
    }

    // ── dac_set: invalid channel identifier ─────────────────────────────────

    #[test]
    fn dac_set_invalid_channel_identifier_rejected() {
        // Shell-injection attempt: must be caught by identifier validation.
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": "foo;ls", "voltage": 3.3});
        let result = handle_dac_set(&mut port, &args);
        assert!(result.is_err(), "invalid identifier must be rejected");
        assert!(
            port.write_data.is_empty(),
            "no bytes sent for invalid channel"
        );
        match result.unwrap_err() {
            McpError::Protocol(msg) => {
                assert!(
                    msg.contains("invalid characters") || msg.contains("unknown dac channel"),
                    "error should describe invalid identifier; got: {msg}"
                );
            }
            other => panic!("expected McpError::Protocol, got: {other:?}"),
        }
    }

    #[test]
    fn dac_set_unknown_string_channel_rejected() {
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": "VBUS", "voltage": 3.3});
        let result = handle_dac_set(&mut port, &args);
        assert!(result.is_err(), "unknown channel string must be rejected");
        match result.unwrap_err() {
            McpError::Protocol(msg) => {
                assert!(
                    msg.contains("unknown dac channel") || msg.contains("VBUS"),
                    "got: {msg}"
                );
            }
            other => panic!("expected McpError::Protocol, got: {other:?}"),
        }
    }

    // ── dac_get: happy path ──────────────────────────────────────────────────

    #[test]
    fn dac_get_happy_float_response() {
        let frame = MockPort::ok_with_stdout("3.3");
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": "DAC0"});
        let result = handle_dac_get(&mut port, &args).unwrap();
        assert!((result["voltage"].as_f64().unwrap() - 3.3).abs() < 1e-9);
        assert_eq!(result["channel"], "DAC0");
    }

    #[test]
    fn dac_get_int_channel_returns_channel_as_int() {
        let frame = MockPort::ok_with_stdout("5.0");
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": 2});
        let result = handle_dac_get(&mut port, &args).unwrap();
        assert!((result["voltage"].as_f64().unwrap() - 5.0).abs() < 1e-9);
        assert_eq!(result["channel"], 2);
    }

    #[test]
    fn dac_get_bad_device_response_returns_error() {
        let frame = MockPort::ok_with_stdout("not_a_float");
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": 0});
        let result = handle_dac_get(&mut port, &args);
        assert!(result.is_err());
        match result.unwrap_err() {
            McpError::Protocol(msg) => {
                assert!(msg.contains("unexpected"), "got: {msg}");
            }
            other => panic!("expected McpError::Protocol, got: {other:?}"),
        }
    }

    // ── adc_get: happy path ──────────────────────────────────────────────────

    #[test]
    fn adc_get_happy_float_response() {
        let frame = MockPort::ok_with_stdout("1.65");
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": 1});
        let result = handle_adc_get(&mut port, &args).unwrap();
        assert!((result["voltage"].as_f64().unwrap() - 1.65).abs() < 1e-9);
        assert_eq!(result["channel"], 1);
    }

    #[test]
    fn adc_get_invalid_channel_rejected() {
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": 5});
        let result = handle_adc_get(&mut port, &args);
        assert!(result.is_err(), "channel 5 must be rejected");
        assert!(
            port.write_data.is_empty(),
            "no bytes sent for invalid channel"
        );
        match result.unwrap_err() {
            McpError::Protocol(msg) => {
                assert!(msg.contains("0-3") || msg.contains("channel"), "got: {msg}");
            }
            other => panic!("expected McpError::Protocol, got: {other:?}"),
        }
    }

    #[test]
    fn adc_get_bad_device_response_returns_error() {
        let frame = MockPort::ok_with_stdout("ERROR");
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": 0});
        let result = handle_adc_get(&mut port, &args);
        assert!(result.is_err());
    }

    // ── adc_get_stats: happy path with samples=3 ────────────────────────────

    #[test]
    fn adc_get_stats_samples3_happy() {
        // Three mock readings: 1.0, 2.0, 3.0 — mean=2.0, stddev=1.0
        let f1 = MockPort::ok_with_stdout("1.0");
        let f2 = MockPort::ok_with_stdout("2.0");
        let f3 = MockPort::ok_with_stdout("3.0");
        let mut port = MockPort::with_responses(&[&f1, &f2, &f3]);
        let args = json!({"channel": 0, "samples": 3});
        let result = handle_adc_get_stats(&mut port, &args).unwrap();
        assert_eq!(result["samples"], 3);
        assert_eq!(result["channel"], 0);
        let mean = result["mean"].as_f64().unwrap();
        let min = result["min"].as_f64().unwrap();
        let max = result["max"].as_f64().unwrap();
        let stddev = result["stddev"].as_f64().unwrap();
        assert!((mean - 2.0).abs() < 1e-9, "mean={mean}");
        assert!((min - 1.0).abs() < 1e-9, "min={min}");
        assert!((max - 3.0).abs() < 1e-9, "max={max}");
        // sample stddev for [1,2,3]: variance = ((1+0+1)/2) = 1.0, stddev = 1.0
        assert!((stddev - 1.0).abs() < 1e-9, "stddev={stddev}");
    }

    // ── adc_get_stats: samples=1 edge case ──────────────────────────────────

    #[test]
    fn adc_get_stats_samples1_stddev_is_zero() {
        // Arbitrary single-sample mock voltage. Avoid 3.14 / 2.718 etc. so
        // clippy's approx_constant lint doesn't think we meant a math constant.
        let frame = MockPort::ok_with_stdout("2.5");
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": 2, "samples": 1});
        let result = handle_adc_get_stats(&mut port, &args).unwrap();
        assert_eq!(result["samples"], 1);
        let stddev = result["stddev"].as_f64().unwrap();
        assert_eq!(stddev, 0.0, "stddev must be 0.0 for n=1");
        let mean = result["mean"].as_f64().unwrap();
        assert!((mean - 2.5).abs() < 1e-9, "mean={mean}");
    }

    // ── adc_get_stats: stats correctness ────────────────────────────────────

    #[test]
    fn adc_get_stats_correctness_known_values() {
        // readings: [2.0, 4.0, 4.0, 4.0, 5.0, 5.0, 7.0, 9.0]
        // mean = 5.0
        // sum((x - mean)^2) = 9+1+1+1+0+0+4+16 = 32
        // sample variance = 32 / (n - 1) = 32/7 ≈ 4.571
        // sample stddev = sqrt(32/7) ≈ 2.138089935299395
        // (Population stddev for this dataset is exactly 2.0 — but we use
        // sample stddev per spec §6.5, hence the irrational sqrt(32/7).)
        let readings = ["2.0", "4.0", "4.0", "4.0", "5.0", "5.0", "7.0", "9.0"];
        let frames: Vec<Vec<u8>> = readings
            .iter()
            .map(|s| MockPort::ok_with_stdout(s))
            .collect();
        let frame_refs: Vec<&[u8]> = frames.iter().map(|f| f.as_slice()).collect();
        let mut port = MockPort::with_responses(&frame_refs);
        let args = json!({"channel": 3, "samples": 8});
        let result = handle_adc_get_stats(&mut port, &args).unwrap();
        let mean = result["mean"].as_f64().unwrap();
        let stddev = result["stddev"].as_f64().unwrap();
        let min = result["min"].as_f64().unwrap();
        let max = result["max"].as_f64().unwrap();
        let expected_stddev = (32.0_f64 / 7.0).sqrt();
        assert!((mean - 5.0).abs() < 1e-9, "mean={mean}");
        assert!(
            (stddev - expected_stddev).abs() < 1e-9,
            "stddev={stddev} expected={expected_stddev}"
        );
        assert!((min - 2.0).abs() < 1e-9, "min={min}");
        assert!((max - 9.0).abs() < 1e-9, "max={max}");
    }

    // ── adc_get_stats: invalid channel ───────────────────────────────────────

    #[test]
    fn adc_get_stats_invalid_channel_rejected() {
        let mut port = MockPort::with_responses(&[]);
        let args = json!({"channel": 4, "samples": 5});
        let result = handle_adc_get_stats(&mut port, &args);
        assert!(result.is_err(), "channel 4 must be rejected");
        assert!(port.write_data.is_empty());
    }

    // ── dac_set: save default = true ────────────────────────────────────────

    #[test]
    fn dac_set_save_defaults_to_true() {
        let frame = MockPort::ok_frame();
        let mut port = MockPort::with_responses(&[&frame]);
        let args = json!({"channel": "DAC1", "voltage": 1.8});
        let result = handle_dac_set(&mut port, &args).unwrap();
        assert_eq!(result["save"], true, "save must default to true");
        let sent = String::from_utf8_lossy(&port.write_data);
        assert!(
            sent.contains("True"),
            "default save=True must appear in command; got: {sent}"
        );
    }
}