From 1646564bc14f55dc58ce7b79ee016ab1e71ace5f Mon Sep 17 00:00:00 2001 From: Stephen Waits Date: Sun, 29 Mar 2026 16:54:56 -0600 Subject: [PATCH] refactor: replace postcard/serde with fixed-size LE wire format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop postcard and serde dependencies entirely. All integers are now fixed-width little-endian on the wire — no varints, no zigzag encoding. A developer sending tx_power_dbm=14 sees 0x0E on the wire. No surprises. - RadioConfig: 10 bytes fixed (freq:4 LE, bw:1, sf:1, cr:1, sync:2 LE, pwr:1) - Commands/Responses: tag byte + fixed-size fields - Vec: 2-byte LE length prefix + raw bytes - Option: 1-byte (0=None, 1=Some) + T - COBS framing preserved (just byte-stuffing, no encoding tricks) - Manual COBS decode replaces postcard's CobsAccumulator - Removes ~15KB from firmware binary (no serde monomorphization) --- Cargo.lock | 14 +---- Cargo.toml | 5 +- src/protocol.rs | 163 ++++++++++++++++++++++++++++++++++++++++++------ src/usb/task.rs | 114 ++++++++++++++++++++++++--------- 4 files changed, 233 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d66b9e9a..b553a2b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -336,6 +336,7 @@ name = "donglora" version = "0.1.0" dependencies = [ "cfg-if", + "cobs", "cortex-m-rt", "critical-section", "defmt 0.3.100", @@ -359,8 +360,6 @@ dependencies = [ "minijinja", "owo-colors", "panic-probe", - "postcard", - "serde", "ssd1306", "static_cell", ] @@ -1026,7 +1025,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0bfb9eb618601c89945a70e254898da93b13be0388091d42117462b265bb3fad" dependencies = [ "hash32", - "serde", "stable_deref_trait", ] @@ -1353,16 +1351,6 @@ version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c33a9471896f1c69cecef8d20cbe2f7accd12527ce60845ff44c153bb2a21b49" -[[package]] -name = "postcard" -version = "1.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6764c3b5dd454e283a30e6dfe78e9b31096d9e32036b5d1eaac7a6119ccb9a24" -dependencies = [ - "cobs", - "serde", -] - [[package]] name = "proc-macro-crate" version = "3.2.0" diff --git a/Cargo.toml b/Cargo.toml index 5e7a8118..258a0c93 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,10 +21,9 @@ embassy-sync = "0.6" embassy-time = "0.3" embassy-usb = { version = "0.3", features = ["defmt"] } embassy-usb-driver = "0.1" -heapless = { version = "0.8", features = ["serde"] } +cobs = { version = "0.3", default-features = false } +heapless = "0.8" lora-phy = "3.0" -postcard = { version = "1.0", default-features = false } -serde = { version = "1.0", default-features = false, features = ["derive"] } static_cell = "2.1" # Display rendering diff --git a/src/protocol.rs b/src/protocol.rs index 3e8606c2..84e17b14 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1,11 +1,21 @@ +//! Wire protocol types and fixed-size little-endian serialization. +//! +//! Every integer is fixed-width LE. No varints, no zigzag. +//! See `docs/PROTOCOL.md` for the complete specification. + use heapless::Vec; -use serde::{Deserialize, Serialize}; /// Maximum LoRa payload size in bytes. pub const MAX_PAYLOAD: usize = 256; +/// RadioConfig wire size (fixed). +pub const RADIO_CONFIG_SIZE: usize = 10; + +/// Sentinel value for `tx_power_dbm`: use the board's maximum TX power. +pub const TX_POWER_MAX: i8 = i8::MIN; // -128 on the wire + /// LoRa signal bandwidth. -#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, defmt::Format)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, defmt::Format)] #[repr(u8)] pub enum Bandwidth { Khz7 = 0, @@ -20,11 +30,31 @@ pub enum Bandwidth { Khz500 = 9, } -/// Sentinel value for `tx_power_dbm`: use the board's maximum TX power. -pub const TX_POWER_MAX: i8 = i8::MIN; // -128 on the wire +impl Bandwidth { + fn from_u8(v: u8) -> Option { + match v { + 0 => Some(Self::Khz7), + 1 => Some(Self::Khz10), + 2 => Some(Self::Khz15), + 3 => Some(Self::Khz20), + 4 => Some(Self::Khz31), + 5 => Some(Self::Khz41), + 6 => Some(Self::Khz62), + 7 => Some(Self::Khz125), + 8 => Some(Self::Khz250), + 9 => Some(Self::Khz500), + _ => None, + } + } +} /// Complete LoRa radio configuration. -#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, defmt::Format)] +/// +/// Wire layout (10 bytes, all little-endian): +/// ```text +/// [freq_hz:4] [bw:1] [sf:1] [cr:1] [sync_word:2] [tx_power_dbm:1] +/// ``` +#[derive(Debug, Clone, Copy, PartialEq, Eq, defmt::Format)] pub struct RadioConfig { /// Frequency in Hz (150_000_000 - 960_000_000 for SX1262). pub freq_hz: u32, @@ -35,15 +65,12 @@ pub struct RadioConfig { pub cr: u8, pub sync_word: u16, /// Transmit power in dBm. Set to [`TX_POWER_MAX`] (-128) for the - /// board's maximum. Actual range is board-dependent (e.g. -9 to +22 - /// for SX1262). Values above the board max are clamped by the radio - /// driver; values below the board min are clamped too. + /// board's maximum. pub tx_power_dbm: i8, } impl RadioConfig { - /// Validate fields against hardware limits. `power_range` is the - /// board's (min, max) TX power in dBm. + /// Validate fields against hardware limits. pub fn validate(&self, power_range: (i8, i8)) -> Result<(), &'static str> { if !(150_000_000..=960_000_000).contains(&self.freq_hz) { return Err("frequency out of range (150-960 MHz)"); @@ -54,7 +81,6 @@ impl RadioConfig { if !(5..=8).contains(&self.cr) { return Err("coding rate out of range (5-8)"); } - // TX_POWER_MAX sentinel is always valid (resolved to board max later) if self.tx_power_dbm != TX_POWER_MAX && !(power_range.0..=power_range.1).contains(&self.tx_power_dbm) { @@ -70,12 +96,37 @@ impl RadioConfig { } self } + + /// Serialize to fixed-size LE bytes. Returns number of bytes written (always 10). + pub fn to_bytes(&self, buf: &mut [u8]) -> usize { + buf[0..4].copy_from_slice(&self.freq_hz.to_le_bytes()); + buf[4] = self.bw as u8; + buf[5] = self.sf; + buf[6] = self.cr; + buf[7..9].copy_from_slice(&self.sync_word.to_le_bytes()); + buf[9] = self.tx_power_dbm as u8; + RADIO_CONFIG_SIZE + } + + /// Deserialize from fixed-size LE bytes. + pub fn from_bytes(buf: &[u8]) -> Option { + if buf.len() < RADIO_CONFIG_SIZE { + return None; + } + Some(Self { + freq_hz: u32::from_le_bytes(buf[0..4].try_into().ok()?), + bw: Bandwidth::from_u8(buf[4])?, + sf: buf[5], + cr: buf[6], + sync_word: u16::from_le_bytes(buf[7..9].try_into().ok()?), + tx_power_dbm: buf[9] as i8, + }) + } } /// Host → firmware commands. -// Payload variant is intentionally large (inline heapless::Vec) — no allocator available. #[allow(clippy::large_enum_variant)] -#[derive(Serialize, Deserialize, Debug, PartialEq)] +#[derive(Debug, PartialEq)] pub enum Command { Ping, GetConfig, @@ -90,10 +141,51 @@ pub enum Command { DisplayOff, } +impl Command { + /// Deserialize from a COBS-decoded frame. + pub fn from_bytes(buf: &[u8]) -> Option { + let tag = *buf.first()?; + let rest = &buf[1..]; + match tag { + 0 => Some(Self::Ping), + 1 => Some(Self::GetConfig), + 2 => Some(Self::SetConfig(RadioConfig::from_bytes(rest)?)), + 3 => Some(Self::StartRx), + 4 => Some(Self::StopRx), + 5 => { + // Transmit: has_config:u8 + [RadioConfig if 1] + len:u16 LE + payload + if rest.is_empty() { + return None; + } + let (config, pos) = if rest[0] == 0 { + (None, 1) + } else if rest[0] == 1 && rest.len() >= 1 + RADIO_CONFIG_SIZE { + (Some(RadioConfig::from_bytes(&rest[1..])?), 1 + RADIO_CONFIG_SIZE) + } else { + return None; + }; + if rest.len() < pos + 2 { + return None; + } + let len = u16::from_le_bytes(rest[pos..pos + 2].try_into().ok()?) as usize; + let data_start = pos + 2; + if rest.len() < data_start + len { + return None; + } + let mut payload = Vec::new(); + let _ = payload.extend_from_slice(&rest[data_start..data_start + len]); + Some(Self::Transmit { config, payload }) + } + 6 => Some(Self::DisplayOn), + 7 => Some(Self::DisplayOff), + _ => None, + } + } +} + /// Firmware → host responses. -// Payload variant is intentionally large (inline heapless::Vec) — no allocator available. #[allow(clippy::large_enum_variant)] -#[derive(Serialize, Deserialize, Debug, PartialEq)] +#[derive(Debug, PartialEq)] pub enum Response { Pong, Config(RadioConfig), @@ -107,10 +199,45 @@ pub enum Response { Error(ErrorCode), } +impl Response { + /// Serialize to fixed-size LE bytes. Returns number of bytes written. + pub fn to_bytes(&self, buf: &mut [u8]) -> usize { + match self { + Self::Pong => { + buf[0] = 0; + 1 + } + Self::Config(cfg) => { + buf[0] = 1; + 1 + cfg.to_bytes(&mut buf[1..]) + } + Self::RxPacket { rssi, snr, payload } => { + buf[0] = 2; + buf[1..3].copy_from_slice(&rssi.to_le_bytes()); + buf[3..5].copy_from_slice(&snr.to_le_bytes()); + buf[5..7].copy_from_slice(&(payload.len() as u16).to_le_bytes()); + buf[7..7 + payload.len()].copy_from_slice(payload); + 7 + payload.len() + } + Self::TxDone => { + buf[0] = 3; + 1 + } + Self::Ok => { + buf[0] = 4; + 1 + } + Self::Error(code) => { + buf[0] = 5; + buf[1] = *code as u8; + 2 + } + } + } +} + /// Error codes reported to the host. -/// -/// Variant indices match postcard wire encoding (0-based). -#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, defmt::Format)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, defmt::Format)] #[repr(u8)] pub enum ErrorCode { InvalidConfig = 0, diff --git a/src/usb/task.rs b/src/usb/task.rs index 48bffdfb..03c79321 100644 --- a/src/usb/task.rs +++ b/src/usb/task.rs @@ -1,9 +1,13 @@ +//! USB CDC-ACM task: COBS-framed fixed-size LE command/response protocol. +//! +//! Reads COBS frames from USB, decodes them, parses commands, routes them +//! to the radio or display task, and sends COBS-framed responses back. + use defmt::warn; use embassy_executor::task; use embassy_futures::join::join; use embassy_usb::class::cdc_acm::{CdcAcmClass, Receiver, Sender, State}; use embassy_usb::Builder; -use postcard::accumulator::{CobsAccumulator, FeedResult}; use static_cell::StaticCell; use crate::board::UsbParts; @@ -15,9 +19,8 @@ use crate::protocol::{Command, ErrorCode, Response}; const MAX_FRAME: usize = 512; const MAX_PACKET: usize = 64; -// Ensure MAX_FRAME can hold the largest possible COBS-encoded response. -// Worst case: RxPacket with MAX_PAYLOAD bytes. Postcard overhead ~8 bytes, -// COBS adds ceil(n/254)+1 bytes. 256+8+3 = 267. 512 is comfortably sufficient. +// Worst case: tag(1) + rssi(2) + snr(2) + len(2) + payload(256) = 263 bytes. +// COBS adds ceil(263/254)+1 = 3 bytes. 266 < 512. Plenty of room. const _: () = assert!( MAX_FRAME >= crate::protocol::MAX_PAYLOAD + 64, "MAX_FRAME too small for max payload + COBS overhead" @@ -87,7 +90,12 @@ async fn protocol_loop<'d, D: embassy_usb_driver::Driver<'d>>( let mut read_buf = [0u8; MAX_PACKET]; let mut write_buf = [0u8; MAX_FRAME]; - let mut cobs_buf: CobsAccumulator = CobsAccumulator::new(); + let mut cobs_encode_buf = [0u8; MAX_FRAME]; + + // COBS accumulator: collect bytes until 0x00 sentinel + let mut frame_buf = [0u8; MAX_FRAME]; + let mut frame_len: usize = 0; + let mut was_connected = false; loop { @@ -103,41 +111,55 @@ async fn protocol_loop<'d, D: embassy_usb_driver::Driver<'d>>( Ok(0) => continue, Ok(n) => n, Err(_) => { - cobs_buf = CobsAccumulator::new(); + frame_len = 0; // reset accumulator embassy_time::Timer::after_millis(100).await; continue; } }; - let mut window = &read_buf[..n]; - 'cobs: while !window.is_empty() { - window = match cobs_buf.feed::(window) { - FeedResult::Consumed => break 'cobs, - FeedResult::OverFull(remaining) => remaining, - FeedResult::DeserError(remaining) => remaining, - FeedResult::Success { data: cmd, remaining } => { - route_command( - cmd, commands, responses, display_commands, has_display, - ) - .await; - remaining + // Feed bytes into the COBS accumulator + for &byte in &read_buf[..n] { + if byte == 0x00 { + // End of COBS frame — decode and parse + 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) + { + if let Some(cmd) = Command::from_bytes(&decode_buf[..decoded_len]) { + route_command( + cmd, commands, responses, display_commands, has_display, + ) + .await; + } + } } - }; + frame_len = 0; + } else if frame_len < MAX_FRAME { + frame_buf[frame_len] = byte; + frame_len += 1; + } else { + // Frame too large — discard + frame_len = 0; + } } } Either3::Second(response) => { - match postcard::to_slice_cobs(&response, &mut write_buf) { - Ok(buf) => { - for chunk in buf.chunks(MAX_PACKET) { - if sender.write_packet(chunk).await.is_err() { - warn!("USB write failed, response dropped"); - break; - } + // Serialize response to fixed-size LE bytes, then COBS encode + let raw_len = response.to_bytes(&mut write_buf); + let encoded_len = cobs::encode(&write_buf[..raw_len], &mut cobs_encode_buf); + // Append 0x00 sentinel + if encoded_len < cobs_encode_buf.len() { + cobs_encode_buf[encoded_len] = 0x00; + let frame = &cobs_encode_buf[..encoded_len + 1]; + for chunk in frame.chunks(MAX_PACKET) { + if sender.write_packet(chunk).await.is_err() { + warn!("USB write failed, response dropped"); + break; } } - Err(_) => { - warn!("response serialization failed"); - } + } else { + warn!("COBS encode buffer overflow"); } } Either3::Third(()) => { @@ -186,3 +208,37 @@ 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; + } + } + // Strip trailing zero if present (COBS artifact) + if di > 0 && dest[di - 1] == 0 { + di -= 1; + } + Some(di) +}