refactor: deduplicate USB/UART protocol framing

- Made protocol_io.rs compile for all boards (not just heltec_v3)
- Renamed FrameAccumulator → CobsDecoder, encode_response → cobs_encode_response
- Refactored USB task to use shared CobsDecoder and route_command from protocol_io
- Removed ~80 lines of duplicated COBS framing and command routing from usb/task.rs
- Single canonical MAX_FRAME definition in protocol_io.rs
This commit is contained in:
swaits committed 2026-04-07 09:24:59 -06:00
1 parent 6e8699610a
commit 8bdaa72bc3
4 files changed
+25 -105

No files matched your search

+1 -1
View File
@@ -24,7 +24,7 @@ mod display;
mod protocol; mod protocol;
#[cfg(not(test))] #[cfg(not(test))]
mod radio; mod radio;
#[cfg(all(not(test), feature = "heltec_v3"))] #[cfg(not(test))]
mod protocol_io; mod protocol_io;
#[cfg(all(not(test), feature = "heltec_v3"))] #[cfg(all(not(test), feature = "heltec_v3"))]
mod uart; mod uart;
+4 -4
View File
@@ -18,13 +18,13 @@ const _: () = assert!(
"MAX_FRAME too small for max payload + COBS overhead" "MAX_FRAME too small for max payload + COBS overhead"
); );
/// Accumulates bytes into complete COBS frames. /// Decodes COBS-framed commands from a byte stream.
pub struct FrameAccumulator { pub struct CobsDecoder {
buf: [u8; MAX_FRAME], buf: [u8; MAX_FRAME],
len: usize, len: usize,
} }
impl FrameAccumulator { impl CobsDecoder {
pub const fn new() -> Self { pub const fn new() -> Self {
Self { Self {
buf: [0u8; MAX_FRAME], buf: [0u8; MAX_FRAME],
@@ -67,7 +67,7 @@ impl FrameAccumulator {
/// COBS-encode a response into `encode_buf` with trailing 0x00 sentinel. /// COBS-encode a response into `encode_buf` with trailing 0x00 sentinel.
/// ///
/// Returns the slice to send (encoded frame + sentinel), or `None` on overflow. /// Returns the slice to send (encoded frame + sentinel), or `None` on overflow.
pub fn encode_response<'a>( pub fn cobs_encode_response<'a>(
response: Response, response: Response,
write_buf: &mut [u8; MAX_FRAME], write_buf: &mut [u8; MAX_FRAME],
encode_buf: &'a mut [u8; MAX_FRAME], encode_buf: &'a mut [u8; MAX_FRAME],
+5 -5
View File
@@ -9,7 +9,7 @@ use embassy_executor::task;
use embedded_io_async::Write; use embedded_io_async::Write;
use crate::channel::{CommandChannel, DisplayCommandChannel, ResponseChannel}; use crate::channel::{CommandChannel, DisplayCommandChannel, ResponseChannel};
use crate::protocol_io::{self, FrameAccumulator, MAX_FRAME}; use crate::protocol_io::{self, CobsDecoder, MAX_FRAME};
#[task] #[task]
pub async fn uart_task( pub async fn uart_task(
@@ -25,7 +25,7 @@ pub async fn uart_task(
let mut read_buf = [0u8; 64]; let mut read_buf = [0u8; 64];
let mut write_buf = [0u8; MAX_FRAME]; let mut write_buf = [0u8; MAX_FRAME];
let mut cobs_encode_buf = [0u8; MAX_FRAME]; let mut cobs_encode_buf = [0u8; MAX_FRAME];
let mut accumulator = FrameAccumulator::new(); let mut decoder = CobsDecoder::new();
loop { loop {
use embassy_futures::select::select; use embassy_futures::select::select;
@@ -42,14 +42,14 @@ pub async fn uart_task(
Ok(0) => continue, Ok(0) => continue,
Ok(n) => n, Ok(n) => n,
Err(_) => { Err(_) => {
accumulator.reset(); decoder.reset();
embassy_time::Timer::after_millis(100).await; embassy_time::Timer::after_millis(100).await;
continue; continue;
} }
}; };
let mut cmds = heapless::Vec::<_, 4>::new(); let mut cmds = heapless::Vec::<_, 4>::new();
accumulator.feed(&read_buf[..n], |cmd| { decoder.feed(&read_buf[..n], |cmd| {
let _ = cmds.push(cmd); let _ = cmds.push(cmd);
}); });
for cmd in cmds { for cmd in cmds {
@@ -61,7 +61,7 @@ pub async fn uart_task(
} }
Either::Second(response) => { Either::Second(response) => {
if let Some(frame) = if let Some(frame) =
protocol_io::encode_response(response, &mut write_buf, &mut cobs_encode_buf) protocol_io::cobs_encode_response(response, &mut write_buf, &mut cobs_encode_buf)
{ {
if tx.write_all(frame).await.is_err() { if tx.write_all(frame).await.is_err() {
warn!("UART write failed, response dropped"); warn!("UART write failed, response dropped");
+15 -95
View File
@@ -11,18 +11,10 @@ use embassy_usb::Builder;
use static_cell::StaticCell; use static_cell::StaticCell;
use crate::channel::{CommandChannel, DisplayCommand, DisplayCommandChannel, ResponseChannel}; use crate::channel::{CommandChannel, DisplayCommand, DisplayCommandChannel, ResponseChannel};
use crate::protocol::{Command, ErrorCode, Response}; use crate::protocol_io::{self, CobsDecoder, MAX_FRAME};
const MAX_FRAME: usize = 512;
const MAX_PACKET: usize = 64; const MAX_PACKET: usize = 64;
// 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"
);
#[task] #[task]
pub async fn usb_task( pub async fn usb_task(
parts: crate::board::UsbParts, parts: crate::board::UsbParts,
@@ -103,11 +95,7 @@ async fn protocol_loop<'d, D: embassy_usb_driver::Driver<'d>>(
let mut read_buf = [0u8; MAX_PACKET]; let mut read_buf = [0u8; MAX_PACKET];
let mut write_buf = [0u8; MAX_FRAME]; let mut write_buf = [0u8; MAX_FRAME];
let mut cobs_encode_buf = [0u8; MAX_FRAME]; let mut cobs_encode_buf = [0u8; MAX_FRAME];
let mut decoder = CobsDecoder::new();
// 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; let mut was_connected = false;
loop { loop {
@@ -123,61 +111,33 @@ async fn protocol_loop<'d, D: embassy_usb_driver::Driver<'d>>(
Ok(0) => continue, Ok(0) => continue,
Ok(n) => n, Ok(n) => n,
Err(_) => { Err(_) => {
frame_len = 0; // reset accumulator decoder.reset();
embassy_time::Timer::after_millis(100).await; embassy_time::Timer::after_millis(100).await;
continue; continue;
} }
}; };
// Feed bytes into the COBS accumulator let mut cmds = heapless::Vec::<_, 4>::new();
for &byte in &read_buf[..n] { decoder.feed(&read_buf[..n], |cmd| {
if byte == 0x00 { let _ = cmds.push(cmd);
// End of COBS frame — decode and parse });
if frame_len > 0 { for cmd in cmds {
let mut decode_buf = [0u8; MAX_FRAME]; protocol_io::route_command(
if let Some(decoded_len) = cmd, commands, responses, display_commands, has_display, mac,
ucobs::decode(&frame_buf[..frame_len], &mut decode_buf) )
{ .await;
if let Some(cmd) = Command::from_bytes(&decode_buf[..decoded_len]) {
route_command(
cmd,
commands,
responses,
display_commands,
has_display,
mac,
)
.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) => { Either3::Second(response) => {
// Serialize response to fixed-size LE bytes, then COBS encode if let Some(frame) =
let raw_len = response.write_to(&mut write_buf); protocol_io::cobs_encode_response(response, &mut write_buf, &mut cobs_encode_buf)
let encoded_len = {
ucobs::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;
let frame = &cobs_encode_buf[..encoded_len + 1];
for chunk in frame.chunks(MAX_PACKET) { for chunk in frame.chunks(MAX_PACKET) {
if sender.write_packet(chunk).await.is_err() { if sender.write_packet(chunk).await.is_err() {
warn!("USB write failed, response dropped"); warn!("USB write failed, response dropped");
break; break;
} }
} }
} else {
warn!("COBS encode buffer overflow");
} }
} }
Either3::Third(()) => { Either3::Third(()) => {
@@ -196,43 +156,3 @@ async fn protocol_loop<'d, D: embassy_usb_driver::Driver<'d>>(
was_connected = connected; was_connected = connected;
} }
} }
/// Route a parsed command to the appropriate handler.
///
/// Display and MAC commands are handled locally (response sent immediately).
/// All other commands are forwarded to the radio task (response sent later).
/// Both paths feed the same `ResponseChannel`, so the one-outstanding-command
/// rule in PROTOCOL.md is required to keep solicited responses ordered.
async fn route_command(
cmd: Command,
commands: &CommandChannel,
responses: &ResponseChannel,
display_commands: &DisplayCommandChannel,
has_display: bool,
mac: [u8; 6],
) {
match cmd {
Command::DisplayOn => {
if has_display {
display_commands.send(DisplayCommand::On).await;
responses.send(Response::Ok).await;
} else {
responses.send(Response::Error(ErrorCode::NoDisplay)).await;
}
}
Command::DisplayOff => {
if has_display {
display_commands.send(DisplayCommand::Off).await;
responses.send(Response::Ok).await;
} else {
responses.send(Response::Error(ErrorCode::NoDisplay)).await;
}
}
Command::GetMac => {
responses.send(Response::MacAddress(mac)).await;
}
other => {
commands.send(other).await;
}
}
}