From 66baeeeaf8f6fac318cc4c6c1a320f593f9f7f98 Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 13:14:07 +0100 Subject: [PATCH 1/2] 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/2] 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;