From 65c43cda23b225786f87ed77f0f662008daca14b Mon Sep 17 00:00:00 2001 From: Stephen Waits Date: Sun, 29 Mar 2026 21:28:46 -0600 Subject: [PATCH] feat: donglora-cobs workspace crate with 31 spec-compliance tests Extract COBS encoder/decoder into a separate no_std workspace crate (crates/donglora-cobs) with comprehensive tests against the Cheshire & Baker 1999 IEEE/ACM specification and Wikipedia canonical vectors. Tests cover: empty input, single/multiple zeros, block boundaries at 254/255 bytes, alternating patterns, the Ping regression case (tag 0x00), round-trip verification, and error cases (truncation, unexpected zeros). Replace inline cobs_decode() in usb/task.rs with the tested crate. Replace external cobs dependency with workspace crate. Fix encoder bug at 254-byte block boundary (spurious trailing code byte). --- Cargo.lock | 15 +- Cargo.toml | 5 +- crates/donglora-cobs/Cargo.toml | 9 + crates/donglora-cobs/src/lib.rs | 391 ++++++++++++++++++++++++++++++++ src/usb/task.rs | 162 ++++++++++--- 5 files changed, 542 insertions(+), 40 deletions(-) create mode 100644 crates/donglora-cobs/Cargo.toml create mode 100644 crates/donglora-cobs/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index b553a2b0..f4e238d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -119,15 +119,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "cobs" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fa961b519f0b462e3a3b4a34b64d119eeaca1d59af726fe450bbba07a9fc0a1" -dependencies = [ - "thiserror", -] - [[package]] name = "cortex-m" version = "0.7.7" @@ -336,11 +327,11 @@ name = "donglora" version = "0.1.0" dependencies = [ "cfg-if", - "cobs", "cortex-m-rt", "critical-section", "defmt 0.3.100", "display-interface", + "donglora-cobs", "embassy-embedded-hal", "embassy-executor", "embassy-futures", @@ -364,6 +355,10 @@ dependencies = [ "static_cell", ] +[[package]] +name = "donglora-cobs" +version = "0.1.0" + [[package]] name = "embassy-embedded-hal" version = "0.2.0" diff --git a/Cargo.toml b/Cargo.toml index 258a0c93..e883eb0f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,3 +1,6 @@ +[workspace] +members = [".", "crates/donglora-cobs"] + [package] name = "donglora" version = "0.1.0" @@ -21,7 +24,7 @@ embassy-sync = "0.6" embassy-time = "0.3" embassy-usb = { version = "0.3", features = ["defmt"] } embassy-usb-driver = "0.1" -cobs = { version = "0.3", default-features = false } +donglora-cobs = { path = "crates/donglora-cobs" } heapless = "0.8" lora-phy = "3.0" static_cell = "2.1" diff --git a/crates/donglora-cobs/Cargo.toml b/crates/donglora-cobs/Cargo.toml new file mode 100644 index 00000000..1b053e16 --- /dev/null +++ b/crates/donglora-cobs/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "donglora-cobs" +version = "0.1.0" +edition = "2021" +license = "MIT" +description = "COBS encoder/decoder — no_std, zero-alloc, spec-compliant" + +[features] +default = [] diff --git a/crates/donglora-cobs/src/lib.rs b/crates/donglora-cobs/src/lib.rs new file mode 100644 index 00000000..5a00aa11 --- /dev/null +++ b/crates/donglora-cobs/src/lib.rs @@ -0,0 +1,391 @@ +//! COBS (Consistent Overhead Byte Stuffing) encoder and decoder. +//! +//! Implements the algorithm from Cheshire & Baker, "Consistent Overhead +//! Byte Stuffing," IEEE/ACM Transactions on Networking, Vol. 7, No. 2, +//! April 1999. +//! +//! COBS transforms a byte sequence so that `0x00` never appears in the +//! output, allowing `0x00` to be used as an unambiguous frame delimiter. +//! Overhead is exactly 1 byte per 254 input bytes (worst case). +//! +//! This crate is `no_std` and zero-alloc — suitable for embedded firmware. +//! +//! # Examples +//! +//! ``` +//! let mut buf = [0u8; 16]; +//! +//! // Encode +//! let n = donglora_cobs::encode(&[0x11, 0x00, 0x33], &mut buf).unwrap(); +//! assert_eq!(&buf[..n], &[0x02, 0x11, 0x02, 0x33]); +//! +//! // Decode +//! let mut out = [0u8; 16]; +//! let n = donglora_cobs::decode(&[0x02, 0x11, 0x02, 0x33], &mut out).unwrap(); +//! assert_eq!(&out[..n], &[0x11, 0x00, 0x33]); +//! ``` + +#![no_std] + +/// Encode `src` into `dest` using COBS. Returns the number of bytes written, +/// or `None` if `dest` is too small. +/// +/// The output does NOT include a trailing `0x00` sentinel — the caller +/// should append it as a frame delimiter. +pub fn encode(src: &[u8], dest: &mut [u8]) -> Option { + let mut si = 0; + let mut di = 0; + let mut code_idx = di; + let mut need_final_code = true; + + if di >= dest.len() && !src.is_empty() { + return None; + } + // Reserve space for the first code byte. + di += 1; + let mut code: u8 = 1; + + while si < src.len() { + if src[si] == 0x00 { + if code_idx >= dest.len() { + return None; + } + dest[code_idx] = code; + code_idx = di; + if di >= dest.len() { + return None; + } + di += 1; + code = 1; + si += 1; + need_final_code = true; + } else { + if di >= dest.len() { + return None; + } + dest[di] = src[si]; + di += 1; + si += 1; + code += 1; + if code == 0xFF { + // Block full (254 data bytes) — emit code. + if code_idx >= dest.len() { + return None; + } + dest[code_idx] = code; + + if si < src.len() { + // More input: start a new block. + code_idx = di; + if di >= dest.len() { + return None; + } + di += 1; + code = 1; + need_final_code = true; + } else { + // Input exhausted right at the block boundary. + need_final_code = false; + } + } + } + } + + if need_final_code { + if code_idx >= dest.len() { + return None; + } + dest[code_idx] = code; + } + + Some(di) +} + +/// Decode a COBS-encoded buffer. Returns the number of decoded bytes +/// written to `dest`, or `None` if the input is malformed. +/// +/// `src` should NOT include the trailing `0x00` frame sentinel — strip +/// it before calling this function. +pub fn decode(src: &[u8], dest: &mut [u8]) -> Option { + let mut si = 0; + let mut di = 0; + while si < src.len() { + let code = src[si] as usize; + si += 1; + if code == 0 { + return None; // unexpected zero in COBS data + } + // Copy `code - 1` data bytes verbatim. + for _ in 1..code { + if si >= src.len() || di >= dest.len() { + return None; + } + dest[di] = src[si]; + di += 1; + si += 1; + } + // If code < 0xFF, an implicit zero follows (unless we're at the end). + if code < 0xFF && si < src.len() { + if di >= dest.len() { + return None; + } + dest[di] = 0; + di += 1; + } + } + Some(di) +} + +/// Maximum encoded size for a given source length (excluding sentinel). +pub const fn max_encoded_len(src_len: usize) -> usize { + src_len + (src_len / 254) + 1 +} + +#[cfg(test)] +mod tests { + use super::*; + extern crate alloc; + use alloc::vec; + use alloc::vec::Vec; + + fn dec(encoded: &[u8]) -> Option> { + let mut buf = vec![0u8; 512]; + decode(encoded, &mut buf).map(|n| buf[..n].to_vec()) + } + + fn enc(input: &[u8]) -> Option> { + let mut buf = vec![0u8; max_encoded_len(input.len()) + 1]; + encode(input, &mut buf).map(|n| buf[..n].to_vec()) + } + + // ── Wikipedia canonical decode vectors (Cheshire & Baker 1999) ── + + #[test] + fn decode_empty() { + assert_eq!(dec(&[0x01]), Some(vec![])); + } + + #[test] + fn decode_single_zero() { + assert_eq!(dec(&[0x01, 0x01]), Some(vec![0x00])); + } + + #[test] + fn decode_two_zeros() { + assert_eq!(dec(&[0x01, 0x01, 0x01]), Some(vec![0x00, 0x00])); + } + + #[test] + fn decode_single_nonzero() { + assert_eq!(dec(&[0x02, 0x11]), Some(vec![0x11])); + } + + #[test] + fn decode_zero_delimited() { + assert_eq!( + dec(&[0x01, 0x02, 0x11, 0x01]), + Some(vec![0x00, 0x11, 0x00]) + ); + } + + #[test] + fn decode_mixed() { + assert_eq!( + dec(&[0x03, 0x11, 0x22, 0x02, 0x33]), + Some(vec![0x11, 0x22, 0x00, 0x33]) + ); + } + + #[test] + fn decode_no_zeros() { + assert_eq!( + dec(&[0x05, 0x11, 0x22, 0x33, 0x44]), + Some(vec![0x11, 0x22, 0x33, 0x44]) + ); + } + + #[test] + fn decode_trailing_zeros() { + assert_eq!( + dec(&[0x02, 0x11, 0x01, 0x01, 0x01]), + Some(vec![0x11, 0x00, 0x00, 0x00]) + ); + } + + #[test] + fn decode_all_zeros_4() { + assert_eq!( + dec(&[0x01, 0x01, 0x01, 0x01, 0x01]), + Some(vec![0x00, 0x00, 0x00, 0x00]) + ); + } + + #[test] + fn decode_all_ff_4() { + assert_eq!( + dec(&[0x05, 0xFF, 0xFF, 0xFF, 0xFF]), + Some(vec![0xFF, 0xFF, 0xFF, 0xFF]) + ); + } + + #[test] + fn decode_alternating() { + assert_eq!( + dec(&[0x01, 0x02, 0x01, 0x02, 0x02, 0x02, 0x03]), + Some(vec![0x00, 0x01, 0x00, 0x02, 0x00, 0x03]) + ); + } + + // ── Block boundary tests (254/255 bytes) ──────────────────── + + #[test] + fn decode_254_nonzero() { + let input: Vec = (1..=254).map(|i| i as u8).collect(); + let mut encoded = vec![0xFF]; + encoded.extend(1u8..=254); + assert_eq!(dec(&encoded), Some(input)); + } + + #[test] + fn decode_255_nonzero_split() { + let input: Vec = (1..=255).map(|i| i as u8).collect(); + let mut encoded = vec![0xFF]; + encoded.extend(1u8..=254); + encoded.push(0x02); + encoded.push(0xFF); + assert_eq!(dec(&encoded), Some(input)); + } + + // ── Ping regression (tag byte 0x00 was stripped by old decoder) ── + + #[test] + fn decode_ping_tag_zero() { + assert_eq!(dec(&[0x01, 0x01]), Some(vec![0x00])); + } + + // ── Error cases ───────────────────────────────────────────── + + #[test] + fn decode_unexpected_zero() { + assert_eq!(dec(&[0x00]), None); + } + + #[test] + fn decode_truncated() { + assert_eq!(dec(&[0x04, 0x11]), None); + } + + #[test] + fn decode_empty_input() { + assert_eq!(dec(&[]), Some(vec![])); + } + + // ── Encode tests ──────────────────────────────────────────── + + #[test] + fn encode_empty() { + assert_eq!(enc(&[]), Some(vec![0x01])); + } + + #[test] + fn encode_single_zero() { + assert_eq!(enc(&[0x00]), Some(vec![0x01, 0x01])); + } + + #[test] + fn encode_single_nonzero() { + assert_eq!(enc(&[0x11]), Some(vec![0x02, 0x11])); + } + + #[test] + fn encode_mixed() { + assert_eq!( + enc(&[0x11, 0x22, 0x00, 0x33]), + Some(vec![0x03, 0x11, 0x22, 0x02, 0x33]) + ); + } + + #[test] + fn encode_no_zeros() { + assert_eq!( + enc(&[0x11, 0x22, 0x33, 0x44]), + Some(vec![0x05, 0x11, 0x22, 0x33, 0x44]) + ); + } + + #[test] + fn encode_trailing_zeros() { + assert_eq!( + enc(&[0x11, 0x00, 0x00, 0x00]), + Some(vec![0x02, 0x11, 0x01, 0x01, 0x01]) + ); + } + + #[test] + fn encode_254_nonzero() { + let input: Vec = (1..=254).map(|i| i as u8).collect(); + let mut expected = vec![0xFF]; + expected.extend(1u8..=254); + assert_eq!(enc(&input), Some(expected)); + } + + // ── Round-trip tests ──────────────────────────────────────── + + #[test] + fn roundtrip_empty() { + roundtrip(&[]); + } + + #[test] + fn roundtrip_single_zero() { + roundtrip(&[0x00]); + } + + #[test] + fn roundtrip_ping() { + roundtrip(&[0x00]); // Ping command = tag 0 + } + + #[test] + fn roundtrip_mixed() { + roundtrip(&[0x11, 0x22, 0x00, 0x33, 0x00, 0x00, 0x44]); + } + + #[test] + fn roundtrip_254_nonzero() { + let data: Vec = (1..=254).map(|i| i as u8).collect(); + roundtrip(&data); + } + + #[test] + fn roundtrip_255_nonzero() { + let data: Vec = (0..255).map(|i| (i + 1) as u8).collect(); + roundtrip(&data); + } + + #[test] + fn roundtrip_256_with_zeros() { + let mut data = vec![0u8; 256]; + for (i, b) in data.iter_mut().enumerate() { + *b = i as u8; // 0x00, 0x01, ..., 0xFF + } + roundtrip(&data); + } + + fn roundtrip(input: &[u8]) { + let mut enc_buf = vec![0u8; max_encoded_len(input.len()) + 1]; + let enc_len = encode(input, &mut enc_buf).expect("encode failed"); + let encoded = &enc_buf[..enc_len]; + + // Verify no zero bytes in encoded output. + assert!( + !encoded.contains(&0x00), + "encoded output contains 0x00: {:?}", + encoded + ); + + let mut dec_buf = vec![0u8; input.len() + 1]; + let dec_len = decode(encoded, &mut dec_buf).expect("decode failed"); + assert_eq!(&dec_buf[..dec_len], input, "round-trip mismatch"); + } +} diff --git a/src/usb/task.rs b/src/usb/task.rs index a96175ff..39495684 100644 --- a/src/usb/task.rs +++ b/src/usb/task.rs @@ -124,7 +124,7 @@ async fn protocol_loop<'d, D: embassy_usb_driver::Driver<'d>>( if frame_len > 0 { let mut decode_buf = [0u8; MAX_FRAME]; if let Some(decoded_len) = - cobs_decode(&frame_buf[..frame_len], &mut decode_buf) + donglora_cobs::decode(&frame_buf[..frame_len], &mut decode_buf) { if let Some(cmd) = Command::from_bytes(&decode_buf[..decoded_len]) { route_command( @@ -147,7 +147,8 @@ async fn protocol_loop<'d, D: embassy_usb_driver::Driver<'d>>( Either3::Second(response) => { // Serialize response to fixed-size LE bytes, then COBS encode let raw_len = response.write_to(&mut write_buf); - let encoded_len = cobs::encode(&write_buf[..raw_len], &mut cobs_encode_buf); + let encoded_len = donglora_cobs::encode(&write_buf[..raw_len], &mut cobs_encode_buf) + .unwrap_or(0); // Append 0x00 sentinel if encoded_len < cobs_encode_buf.len() { cobs_encode_buf[encoded_len] = 0x00; @@ -209,32 +210,135 @@ async fn route_command( } } -/// Decode a COBS-encoded buffer (without the trailing 0x00 sentinel). -/// Returns the number of decoded bytes, or None on error. -fn cobs_decode(src: &[u8], dest: &mut [u8]) -> Option { - let mut si = 0; - let mut di = 0; - while si < src.len() { - let code = src[si] as usize; - si += 1; - if code == 0 { - return None; // unexpected zero in COBS data - } - for _ in 1..code { - if si >= src.len() || di >= dest.len() { - return None; - } - dest[di] = src[si]; - di += 1; - si += 1; - } - if code < 0xFF && si < src.len() { - if di >= dest.len() { - return None; - } - dest[di] = 0; - di += 1; - } + +// ── COBS compliance tests (IEEE/ACM Cheshire & Baker 1999) ────── + +#[cfg(test)] +mod tests { + use super::cobs_decode; + + fn decode(encoded: &[u8]) -> Option> { + let mut buf = vec![0u8; 512]; + cobs_decode(encoded, &mut buf).map(|n| buf[..n].to_vec()) + } + + // Wikipedia canonical vectors + #[test] + fn empty_input() { + assert_eq!(decode(&[0x01]), Some(vec![])); + } + + #[test] + fn single_zero() { + assert_eq!(decode(&[0x01, 0x01]), Some(vec![0x00])); + } + + #[test] + fn two_zeros() { + assert_eq!(decode(&[0x01, 0x01, 0x01]), Some(vec![0x00, 0x00])); + } + + #[test] + fn single_nonzero() { + assert_eq!(decode(&[0x02, 0x11]), Some(vec![0x11])); + } + + #[test] + fn zero_delimited() { + assert_eq!( + decode(&[0x01, 0x02, 0x11, 0x01]), + Some(vec![0x00, 0x11, 0x00]) + ); + } + + #[test] + fn mixed_with_zero() { + assert_eq!( + decode(&[0x03, 0x11, 0x22, 0x02, 0x33]), + Some(vec![0x11, 0x22, 0x00, 0x33]) + ); + } + + #[test] + fn no_zeros() { + assert_eq!( + decode(&[0x05, 0x11, 0x22, 0x33, 0x44]), + Some(vec![0x11, 0x22, 0x33, 0x44]) + ); + } + + #[test] + fn nonzero_then_trailing_zeros() { + assert_eq!( + decode(&[0x02, 0x11, 0x01, 0x01, 0x01]), + Some(vec![0x11, 0x00, 0x00, 0x00]) + ); + } + + #[test] + fn all_zeros_4() { + assert_eq!( + decode(&[0x01, 0x01, 0x01, 0x01, 0x01]), + Some(vec![0x00, 0x00, 0x00, 0x00]) + ); + } + + #[test] + fn all_ff_4() { + assert_eq!( + decode(&[0x05, 0xFF, 0xFF, 0xFF, 0xFF]), + Some(vec![0xFF, 0xFF, 0xFF, 0xFF]) + ); + } + + #[test] + fn alternating_zero_nonzero() { + assert_eq!( + decode(&[0x01, 0x02, 0x01, 0x02, 0x02, 0x02, 0x03]), + Some(vec![0x00, 0x01, 0x00, 0x02, 0x00, 0x03]) + ); + } + + // The Ping case that caught the original bug + #[test] + fn ping_command_tag_zero() { + assert_eq!(decode(&[0x01, 0x01]), Some(vec![0x00])); + } + + // 254-byte block boundary (code 0xFF = 254 data bytes, no implicit zero) + #[test] + fn block_254_nonzero() { + let input: Vec = (1..=254).collect(); + let mut encoded = vec![0xFF]; + encoded.extend(1u8..=254); + assert_eq!(decode(&encoded), Some(input)); + } + + // 255 non-zero bytes: first 254 as one block, then 1 leftover + #[test] + fn block_255_nonzero() { + let input: Vec = (1..=255).map(|i| i as u8).collect(); + let mut encoded = vec![0xFF]; + encoded.extend(1u8..=254); + encoded.push(0x02); // code for 1 data byte + encoded.push(0xFF); // the 255th byte + assert_eq!(decode(&encoded), Some(input)); + } + + // Error cases + #[test] + fn unexpected_zero_in_data() { + assert_eq!(decode(&[0x00]), None); + } + + #[test] + fn truncated_input() { + // Code says 3 data bytes follow, but only 1 available + assert_eq!(decode(&[0x04, 0x11]), None); + } + + #[test] + fn empty_encoded() { + assert_eq!(decode(&[]), Some(vec![])); } - Some(di) }