From 66baeeeaf8f6fac318cc4c6c1a320f593f9f7f98 Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 13:14:07 +0100 Subject: [PATCH 1/4] Test the answer poll: rejected, empty, no answer, read error Camera::answer read the device directly, so no test reached its arms. The poll is now poll_answer, which takes the selector-2 read as a closure and the gap as an argument; Camera::answer passes the device read and the same 50 ms gap, so what is sent and read on the camera is unchanged. Tests script the replies: a rejected answer returns Error::Rejected(command, code) at the first read, an empty answer returns an empty list, a stale answer runs through all 20 reads to NoAnswer, and failed reads are polled again until a value arrives. Fixes #151 Co-Authored-By: Claude Sonnet 5.5 --- crates/obsbot-device/src/family/exchange.rs | 123 ++++++++++++++++++-- 1 file changed, 111 insertions(+), 12 deletions(-) diff --git a/crates/obsbot-device/src/family/exchange.rs b/crates/obsbot-device/src/family/exchange.rs index e7cb5ae..187948f 100644 --- a/crates/obsbot-device/src/family/exchange.rs +++ b/crates/obsbot-device/src/family/exchange.rs @@ -6,7 +6,7 @@ use crate::error::{Error, Result}; use crate::family::frame::{Answer, Sequence, parse_answer, query_frame}; -use crate::transport::uvc::Device; +use crate::transport::uvc::{Device, XU_SIZE}; use std::sync::OnceLock; use std::thread::sleep; use std::time::Duration; @@ -63,18 +63,117 @@ impl Camera { /// Polls selector 2 for the answer to the query with `seq`. pub(crate) fn answer(&self, command: u16, seq: u16) -> Result> { - for _ in 0..REPLY_ATTEMPTS { - sleep(REPLY_GAP); - // a read before the answer is ready can fail; just poll again - if let Ok(reply) = self.dev.xu_get(2) { - match parse_answer(&reply, seq, command) { - Some(Answer::Value(data)) => return Ok(data.to_vec()), - Some(Answer::Empty) => return Ok(Vec::new()), - Some(Answer::Rejected(code)) => return Err(Error::Rejected(command, code)), - None => {} - } + poll_answer(command, seq, REPLY_GAP, || self.dev.xu_get(2)) + } +} + +/// The poll of [`Camera::answer`] on any source of selector-2 reads, so a test can script the +/// replies. Waits `gap` before each of `REPLY_ATTEMPTS` reads. +pub(crate) fn poll_answer( + command: u16, + seq: u16, + gap: Duration, + mut read: impl FnMut() -> Result<[u8; XU_SIZE]>, +) -> Result> { + for _ in 0..REPLY_ATTEMPTS { + sleep(gap); + // a read before the answer is ready can fail; just poll again + if let Ok(reply) = read() { + match parse_answer(&reply, seq, command) { + Some(Answer::Value(data)) => return Ok(data.to_vec()), + Some(Answer::Empty) => return Ok(Vec::new()), + Some(Answer::Rejected(code)) => return Err(Error::Rejected(command, code)), + None => {} } } - Err(Error::NoAnswer(command)) + } + Err(Error::NoAnswer(command)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::transport::uvc::XU_SIZE; + use std::cell::Cell; + + const COMMAND: u16 = 0x1344; + const SEQ: u16 = 7; + + /// A selector-2 reply of the given type for `SEQ`, as `parse_answer` reads it: header + /// length 16, an optional error code in the header extension, an optional data section. + fn reply(kind: u8, command: u16, code: Option, data: &[u8]) -> [u8; XU_SIZE] { + let mut r = [0u8; XU_SIZE]; + r[0] = 0xaa; + r[1] = kind; + r[2..4].copy_from_slice(&SEQ.to_le_bytes()); + r[4..6].copy_from_slice(&16u16.to_le_bytes()); + r[10..12].copy_from_slice(&command.to_le_bytes()); + if let Some(code) = code { + r[12..14].copy_from_slice(&[0x00, 0x02]); + r[14..16].copy_from_slice(&code.to_le_bytes()); + } else { + r[12..14].copy_from_slice(&(data.len() as u16).to_le_bytes()); + } + r[16..16 + data.len()].copy_from_slice(data); + r + } + + /// Polls with scripted reads, no waiting; returns the result and the number of reads taken. + fn poll(script: impl Fn(usize) -> Result<[u8; XU_SIZE]>) -> (Result>, usize) { + let reads = Cell::new(0); + let result = poll_answer(COMMAND, SEQ, Duration::ZERO, || { + let n = reads.get(); + reads.set(n + 1); + script(n) + }); + (result, reads.get()) + } + + #[test] + fn a_rejected_answer_returns_the_cameras_code_at_the_first_poll() { + // a refused command must return the camera's code at once, not poll on until "no answer" + let (result, reads) = poll(|_| Ok(reply(0x89, COMMAND, Some(-3), &[]))); + assert!( + matches!(result, Err(Error::Rejected(COMMAND, -3))), + "{result:?}" + ); + assert_eq!(reads, 1); + } + + #[test] + fn an_empty_answer_is_an_empty_list_not_a_missing_one() { + let (result, reads) = poll(|_| Ok(reply(0x09, COMMAND, None, &[]))); + assert_eq!(result.unwrap(), Vec::::new()); + assert_eq!(reads, 1); + } + + #[test] + fn without_a_matching_answer_the_poll_gives_up_after_all_attempts() { + // stale replies: the answer to another query (sequence) never matches + let (result, reads) = poll(|_| { + let mut stale = reply(0x29, COMMAND, None, &[1]); + stale[2..4].copy_from_slice(&(SEQ + 1).to_le_bytes()); + Ok(stale) + }); + assert!( + matches!(result, Err(Error::NoAnswer(COMMAND))), + "{result:?}" + ); + // 20 reads at 50 ms: the one second a caller waits for a camera that does not answer + assert_eq!(reads, 20); + } + + #[test] + fn a_failed_read_is_polled_again_and_a_later_value_is_returned() { + // the camera is not ready for the first reads; the value that follows is the answer + let (result, reads) = poll(|n| { + if n < 2 { + Err(Error::Invalid("not ready".into())) + } else { + Ok(reply(0x29, COMMAND, None, &[4, 5, 6])) + } + }); + assert_eq!(result.unwrap(), vec![4, 5, 6]); + assert_eq!(reads, 3); } } From a3adac64fcca5a98c282f5aa5f335b5101edb13f Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 13:24:10 +0100 Subject: [PATCH 2/4] Drop a redundant import in the answer poll tests The test module already gets XU_SIZE through use super::*. Co-Authored-By: Claude Sonnet 5.5 --- crates/obsbot-device/src/family/exchange.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/obsbot-device/src/family/exchange.rs b/crates/obsbot-device/src/family/exchange.rs index 187948f..9da70a0 100644 --- a/crates/obsbot-device/src/family/exchange.rs +++ b/crates/obsbot-device/src/family/exchange.rs @@ -93,7 +93,6 @@ pub(crate) fn poll_answer( #[cfg(test)] mod tests { use super::*; - use crate::transport::uvc::XU_SIZE; use std::cell::Cell; const COMMAND: u16 = 0x1344; From 4708542ef2fbcdffc329ac16aa042d00630f9d7a Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 13:21:14 +0100 Subject: [PATCH 3/4] Test that a rejected tracking query gives tracking: null full_status_with reads the device directly, so no test reached the arm that turns a rejected or unanswered tracking query (0x0104) into tracking: None. Tests now run it on a scripted Device (compiled in tests only, answering xu_get, xu_set, get_ctrl and ctrl_range from a table): a rejected tracking query returns Ok with tracking None and the other fields filled, an answered one fills tracking, and a failed write still fails the whole read. No status code and no byte sent to the camera changed. Fixes #152 Co-Authored-By: Claude Sonnet 5.5 --- .../obsbot-device/src/legacy/full_status.rs | 99 ++++++++++++++++ crates/obsbot-device/src/lib.rs | 2 + crates/obsbot-device/src/scripted.rs | 110 ++++++++++++++++++ crates/obsbot-device/src/transport/uvc.rs | 49 +++++++- 4 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 crates/obsbot-device/src/scripted.rs diff --git a/crates/obsbot-device/src/legacy/full_status.rs b/crates/obsbot-device/src/legacy/full_status.rs index 4985c47..031489b 100644 --- a/crates/obsbot-device/src/legacy/full_status.rs +++ b/crates/obsbot-device/src/legacy/full_status.rs @@ -416,3 +416,102 @@ mod full_status_tests { assert!(v["status"].is_object() && v["capabilities"].is_array()); } } + +/// `full_status_with` on a scripted camera (no device is opened). +#[cfg(test)] +mod scripted_camera_tests { + use super::*; + use crate::family::exchange::Camera; + use crate::family::frame::Sequence; + use crate::scripted::{Answer, Scripted}; + use crate::transport::uvc::Device; + use std::sync::OnceLock; + + /// A camera that is awake, with HDR on and the LED in mode 2; it answers the zoom factor + /// (2.0), the live pose (pan 25, tilt 10), the microphone volume (55) and the absolute zoom + /// control (37). The tracking query gets `tracking`; every other query is rejected. + fn camera(tracking: Answer) -> Camera { + let mut status = [0u8; 60]; + status[0x06] = 1; + status[0x21] = 2; + let mut script = Scripted::new(status); + script.answers.insert(0x0104, tracking); + script + .answers + .insert(0x6804, Answer::Value(2.0f32.to_le_bytes().to_vec())); + let mut pose = vec![0u8; 12]; + pose[8..10].copy_from_slice(&(-1000i16).to_le_bytes()); + pose[10..12].copy_from_slice(&2500i16.to_le_bytes()); + script.answers.insert(0x0043, Answer::Value(pose)); + script + .answers + .insert(0x80c2, Answer::Value(55i16.to_le_bytes().to_vec())); + script.ctrls.insert(ZOOM_ABSOLUTE, 37); + static SEQ: OnceLock = OnceLock::new(); + Camera { + dev: Device::scripted(script), + seq: SEQ.get_or_init(Sequence::new), + } + } + + /// The zone readbacks and the identity the daemon keeps, so the test asks only for the rest. + fn known() -> (Option, Option) { + ( + Some(ZoneInfo { + zone: None, + hand: None, + startup: None, + framing: None, + presets: None, + }), + Some(Identity { + firmware: Some("6.2.8.1".into()), + model_info: None, + }), + ) + } + + #[test] + fn a_rejected_tracking_query_gives_tracking_null_and_the_rest_of_the_status() { + // the camera refuses 0x0104: status and the daemon's broadcast must still show + // everything else, with `tracking: null`, not fail as a whole + let (zones, identity) = known(); + let full = camera(Answer::Rejected(-3)) + .full_status_with(zones, identity) + .expect("a rejected tracking query does not fail the read"); + assert!(full.tracking.is_none()); + assert!(full.status.hdr); + assert_eq!(full.status.led, 2); + assert_eq!(full.zoom.factor, Some(2.0)); + assert_eq!(full.zoom.absolute, Some(37)); + let pose = full + .pose + .expect("the live pose is read after the tracking query"); + assert_eq!((pose.pan, pose.tilt), (25.0, 10.0)); + assert_eq!(full.audio_volume, Some(55)); + assert_eq!(full.firmware.as_deref(), Some("6.2.8.1")); + assert_eq!(full.model, Some("Tiny 2 Lite")); + } + + #[test] + fn an_answered_tracking_query_is_in_the_status() { + // the same script with a real answer: tracking is filled, so the test above sees the + // arm and not a camera that never reports tracking + let mut answer = vec![0u8; 12]; + answer[0] = 1; // group + answer[10] = 1; // zone tracking + let (zones, identity) = known(); + let full = camera(Answer::Value(answer)) + .full_status_with(zones, identity) + .unwrap(); + assert!(full.tracking.is_some_and(|t| t.zone_tracking)); + } + + #[test] + fn any_other_tracking_error_fails_the_whole_read() { + // only "no answer" and "rejected" are tolerated; a failed write is a broken camera + let (zones, identity) = known(); + let result = camera(Answer::SendFails).full_status_with(zones, identity); + assert!(matches!(result, Err(Error::Io(_))), "{result:?}"); + } +} diff --git a/crates/obsbot-device/src/lib.rs b/crates/obsbot-device/src/lib.rs index 9c6244f..1421c81 100644 --- a/crates/obsbot-device/src/lib.rs +++ b/crates/obsbot-device/src/lib.rs @@ -8,4 +8,6 @@ pub mod family; pub mod legacy; pub mod models; pub mod registry; +#[cfg(test)] +pub(crate) mod scripted; pub mod transport; diff --git a/crates/obsbot-device/src/scripted.rs b/crates/obsbot-device/src/scripted.rs new file mode 100644 index 0000000..7ef7457 --- /dev/null +++ b/crates/obsbot-device/src/scripted.rs @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +//! A scripted camera for tests: a [`Device`](crate::transport::uvc::Device) that answers from a +//! table instead of ioctls, so the flows of `Camera` run without a camera. It sits behind the +//! same four methods the real device uses (`xu_get`, `xu_set`, `get_ctrl`, `ctrl_range`) and is +//! compiled in tests only. + +use crate::error::{Error, Result}; +use crate::transport::uvc::{CtrlRange, XU_SIZE}; +use std::cell::RefCell; +use std::collections::BTreeMap; + +/// How the scripted camera treats a framed query. +pub(crate) enum Answer { + /// Type 0x29 with this data. + Value(Vec), + /// Type 0x89 with this error code. + Rejected(i16), + /// Sending the query fails (the write to selector 2 returns an error). + SendFails, +} + +/// The table behind a scripted [`Device`]. A query that is not in `answers` is rejected, as a +/// camera rejects a query it does not know; that keeps a test free of waiting for polls. +pub(crate) struct Scripted { + /// What a read of selector 6 (the status buffer) returns. + pub status: [u8; XU_SIZE], + pub answers: BTreeMap, + /// V4L2 controls the camera has, with their current values. + pub ctrls: BTreeMap, + /// The last frame written to selector 2, which the next read of selector 2 answers. + last: RefCell<[u8; XU_SIZE]>, +} + +impl Scripted { + pub fn new(status: [u8; XU_SIZE]) -> Self { + Self { + status, + answers: BTreeMap::new(), + ctrls: BTreeMap::new(), + last: RefCell::new([0; XU_SIZE]), + } + } + + pub fn xu_get(&self, selector: u8) -> Result<[u8; XU_SIZE]> { + match selector { + 6 => Ok(self.status), + 2 => { + let last = *self.last.borrow(); + let command = u16::from_le_bytes([last[10], last[11]]); + Ok(match self.answers.get(&command) { + Some(Answer::Value(data)) => answer(0x29, &last, None, data), + Some(Answer::Rejected(code)) => answer(0x89, &last, Some(*code), &[]), + Some(Answer::SendFails) | None => answer(0x89, &last, Some(-1), &[]), + }) + } + _ => Err(Error::Invalid(format!( + "selector {selector} is not scripted" + ))), + } + } + + pub fn xu_set(&self, selector: u8, payload: &[u8]) -> Result<()> { + if selector == 2 { + let mut frame = [0u8; XU_SIZE]; + frame[..payload.len()].copy_from_slice(payload); + let command = u16::from_le_bytes([frame[10], frame[11]]); + if matches!(self.answers.get(&command), Some(Answer::SendFails)) { + return Err(Error::Io(std::io::Error::other("scripted write failure"))); + } + *self.last.borrow_mut() = frame; + } + Ok(()) + } + + pub fn get_ctrl(&self, id: u32) -> Result { + self.ctrls + .get(&id) + .copied() + .ok_or_else(|| Error::Invalid(format!("control {id:#x} is not scripted"))) + } + + pub fn ctrl_range(&self, id: u32) -> Result> { + Ok(self.ctrls.contains_key(&id).then_some(CtrlRange { + min: 0, + max: 100, + step: 1, + default: 50, + })) + } +} + +/// An answer to the query frame `query`: same sequence number, header length 16, then either +/// the error code of a rejection or the data section, as `parse_answer` reads it. +fn answer(kind: u8, query: &[u8; XU_SIZE], code: Option, data: &[u8]) -> [u8; XU_SIZE] { + let mut r = [0u8; XU_SIZE]; + r[0] = 0xaa; + r[1] = kind; + r[2..4].copy_from_slice(&query[2..4]); + r[4..6].copy_from_slice(&16u16.to_le_bytes()); + r[10..12].copy_from_slice(&query[10..12]); + if let Some(code) = code { + r[12..14].copy_from_slice(&[0x00, 0x02]); + r[14..16].copy_from_slice(&code.to_le_bytes()); + } else { + r[12..14].copy_from_slice(&(data.len() as u16).to_le_bytes()); + } + r[16..16 + data.len()].copy_from_slice(data); + r +} diff --git a/crates/obsbot-device/src/transport/uvc.rs b/crates/obsbot-device/src/transport/uvc.rs index 2384621..206b713 100644 --- a/crates/obsbot-device/src/transport/uvc.rs +++ b/crates/obsbot-device/src/transport/uvc.rs @@ -77,6 +77,9 @@ pub struct CtrlRange { pub struct Device { file: Flock, detected: Detected, + /// Tests only: answers from a table instead of the ioctls. + #[cfg(test)] + script: Option, } /// Takes an exclusive flock on `file`, retrying until `wait` has passed. @@ -112,7 +115,35 @@ impl Device { pub fn open() -> Result { let detected = detect().ok_or(Error::NoCamera)?; let file = lock_exclusive(File::open(&detected.device)?, LOCK_WAIT)?; - Ok(Self { file, detected }) + Ok(Self { + file, + detected, + #[cfg(test)] + script: None, + }) + } + + /// Tests only: a device that answers from `script`. It holds a temporary file for the + /// lock and opens no device node. + #[cfg(test)] + pub(crate) fn scripted(script: crate::scripted::Scripted) -> Self { + use std::sync::atomic::{AtomicU32, Ordering}; + static N: AtomicU32 = AtomicU32::new(0); + let path = std::env::temp_dir().join(format!( + "o4l-scripted-{}-{}", + std::process::id(), + N.fetch_add(1, Ordering::Relaxed) + )); + let file = lock_exclusive(File::create(&path).unwrap(), LOCK_WAIT).unwrap(); + std::fs::remove_file(&path).unwrap(); + Self { + file, + detected: Detected { + model: &crate::registry::MODELS[0], + device: "/nonexistent/obsbot-test-video0".into(), + }, + script: Some(script), + } } pub fn detected(&self) -> &Detected { @@ -139,6 +170,10 @@ impl Device { /// GET_CUR on unit 2, after checking the buffer length like the vendor tools do. pub fn xu_get(&self, selector: u8) -> Result<[u8; XU_SIZE]> { + #[cfg(test)] + if let Some(script) = &self.script { + return script.xu_get(selector); + } check_len(selector, self.xu_len(selector)?)?; let mut data = [0u8; XU_SIZE]; self.xu(selector, UVC_GET_CUR, &mut data)?; @@ -147,6 +182,10 @@ impl Device { /// SET_CUR on unit 2 with a zero-padded 60-byte buffer. pub fn xu_set(&self, selector: u8, payload: &[u8]) -> Result<()> { + #[cfg(test)] + if let Some(script) = &self.script { + return script.xu_set(selector, payload); + } if payload.len() > XU_SIZE { return Err(Error::Invalid("payload longer than 60 bytes".into())); } @@ -157,6 +196,10 @@ impl Device { } pub fn get_ctrl(&self, id: u32) -> Result { + #[cfg(test)] + if let Some(script) = &self.script { + return script.get_ctrl(id); + } let mut c = V4l2Control { id, value: 0 }; unsafe { vidioc_g_ctrl(self.file.as_raw_fd(), &mut c) }?; Ok(c.value) @@ -170,6 +213,10 @@ impl Device { /// Range of a control, or None when the camera does not have it (or it is disabled). pub fn ctrl_range(&self, id: u32) -> Result> { + #[cfg(test)] + if let Some(script) = &self.script { + return script.ctrl_range(id); + } let mut q = V4l2QueryCtrl { id, ..Default::default() From dc20a5a41731589b3cd884c1670e5057f3f25eca Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 13:24:44 +0100 Subject: [PATCH 4/4] Scripted: state only the test reason in the doc comment Do not claim how a camera treats an unknown query; the comment now says what the test double does (type 0x89, code -1) and why. Co-Authored-By: Claude Sonnet 5.5 --- crates/obsbot-device/src/scripted.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/obsbot-device/src/scripted.rs b/crates/obsbot-device/src/scripted.rs index 7ef7457..d733cbe 100644 --- a/crates/obsbot-device/src/scripted.rs +++ b/crates/obsbot-device/src/scripted.rs @@ -20,8 +20,8 @@ pub(crate) enum Answer { SendFails, } -/// The table behind a scripted [`Device`]. A query that is not in `answers` is rejected, as a -/// camera rejects a query it does not know; that keeps a test free of waiting for polls. +/// The table behind a scripted [`Device`]. A query that is not in `answers` is rejected (type +/// 0x89, code -1), so a test never waits out the poll. pub(crate) struct Scripted { /// What a read of selector 6 (the status buffer) returns. pub status: [u8; XU_SIZE],