Skip to content

fix(mstp): Clause 9 CRCs + USB stream reassembly #464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bbartling wants to merge 3 commits into from
Closed

fix(mstp): Clause 9 CRCs + USB stream reassembly #464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a9912b8
fix(mstp): USB chunk gaps must not trigger T_frame_abort
bbartling Aug 29, 2026
6a70b85
fix(mstp): use BACnet Clause 9 header and data CRCs
bbartling Aug 30, 2026
73a1fd4
refactor(mstp): split frame tests to satisfy 700 LOC cap
bbartling Aug 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions crates/bacnet-transport/src/mstp/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,14 @@ fn calculate_t_turnaround_us(baud_rate: u32) -> u64 {
const N_RETRY_TOKEN: u8 = 1;
/// Maximum frame buffer size: preamble(2) + header(6) + max data(1497) + CRC16(2)
pub(crate) const MSTP_MAX_FRAME_BUF: usize = 1507;
/// Maximum inter-byte gap within a frame before aborting reception.
/// Spec Clause 9.5.5: minimum 60 bit times. Computed per baud rate.
fn calculate_t_frame_abort_us(baud_rate: u32) -> u64 {
// 60 bit times in microseconds, rounded up
60_000_000u64.div_ceil(baud_rate as u64)
/// Host-side stale partial-frame timeout for USB/chunked serial reassembly.
///
/// This is **not** Clause 9 `T_frame_abort` (wire inter-byte silence). Host async reads
/// often arrive with multi-millisecond gaps that would falsely abort mid-frame assembly.
fn calculate_host_stale_partial_timeout_us(baud_rate: u32) -> u64 {
const USB_CHUNK_SLACK_US: u64 = 100_000;
let wire_us = (MSTP_MAX_FRAME_BUF as u64 * 10 * 1_000_000).div_ceil(baud_rate as u64);
wire_us.saturating_add(USB_CHUNK_SLACK_US)
}
/// Maximum number of queued outgoing frames before rejecting new sends.
const MAX_TX_QUEUE_DEPTH: usize = 256;
Expand Down
40 changes: 23 additions & 17 deletions crates/bacnet-transport/src/mstp/port.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ use tokio::sync::{mpsc, oneshot, Mutex};
use tracing::{debug, warn};

use crate::mstp_frame::{
decode_frame, encode_frame, find_preamble, FrameType, MstpFrame, BROADCAST_MAC,
decode_frame_stream, encode_frame, find_preamble, retain_lone_preamble_byte, FrameType,
MstpFrame, StreamDecode, BROADCAST_MAC,
};
use crate::port::{ReceivedNpdu, TransportPort};

use super::{
calculate_t_frame_abort_us, calculate_t_turnaround_us, next_addr, MasterNode, MasterState,
MstpConfig, SerialPort, MSTP_MAX_FRAME_BUF, T_NO_TOKEN_MS, T_REPLY_DELAY_MS,
calculate_host_stale_partial_timeout_us, calculate_t_turnaround_us, next_addr, MasterNode,
MasterState, MstpConfig, SerialPort, MSTP_MAX_FRAME_BUF, T_NO_TOKEN_MS, T_REPLY_DELAY_MS,
T_REPLY_TIMEOUT_MS, T_REPLY_TRANSMIT_MARGIN_MS, T_USAGE_TIMEOUT_MS,
};

Expand Down Expand Up @@ -65,7 +66,9 @@ impl<S: SerialPort> TransportPort for MstpTransport<S> {
let serial = Arc::new(serial);
let serial_clone = serial.clone();
let t_turnaround_us = calculate_t_turnaround_us(self.config.baud_rate);
let t_frame_abort_us = calculate_t_frame_abort_us(self.config.baud_rate);
// Host reassembly policy: tolerate USB read chunk gaps, not wire T_frame_abort.
let host_stale_partial_timeout_us =
calculate_host_stale_partial_timeout_us(self.config.baud_rate);
let reply_decision_delay_ms = T_REPLY_DELAY_MS
.saturating_sub(t_turnaround_us.div_ceil(1_000) + T_REPLY_TRANSMIT_MARGIN_MS);

Expand Down Expand Up @@ -123,14 +126,19 @@ impl<S: SerialPort> TransportPort for MstpTransport<S> {
match result {
Ok(0) => continue,
Ok(n) => {
// T_frame_abort: discard partial frame if inter-byte gap
// exceeds the spec limit (60 bit times).
// Host stale-partial timeout: drop abandoned assembly if no
// bytes arrive for a long host-side gap (USB scheduling, not
// Clause 9 wire T_frame_abort).
let now = tokio::time::Instant::now();
if !frame_buf.is_empty() {
let gap = now.duration_since(last_byte_time);
if gap > tokio::time::Duration::from_micros(t_frame_abort_us) {
if gap
> tokio::time::Duration::from_micros(
host_stale_partial_timeout_us,
)
{
debug!(
"MS/TP: T_frame_abort exceeded ({gap:?}), discarding partial frame"
"MS/TP: host stale partial frame timeout ({gap:?}), discarding partial assembly"
);
frame_buf.clear();
}
Expand Down Expand Up @@ -160,7 +168,7 @@ impl<S: SerialPort> TransportPort for MstpTransport<S> {
let preamble_pos = match find_preamble(&frame_buf) {
Some(pos) => pos,
None => {
frame_buf.clear();
retain_lone_preamble_byte(&mut frame_buf);
break;
}
};
Expand All @@ -170,8 +178,8 @@ impl<S: SerialPort> TransportPort for MstpTransport<S> {
frame_buf.drain(..preamble_pos);
}

match decode_frame(&frame_buf) {
Ok((frame, consumed)) => {
match decode_frame_stream(&frame_buf) {
StreamDecode::Complete { frame, consumed } => {
frame_buf.drain(..consumed);

// Process through state machine — collect
Expand Down Expand Up @@ -274,12 +282,10 @@ impl<S: SerialPort> TransportPort for MstpTransport<S> {
}),
);
}
Err(_) => {
// Incomplete frame or bad CRC — skip first preamble byte
if frame_buf.len() > 2 {
frame_buf.drain(..1);
}
break;
StreamDecode::NeedMore => break,
StreamDecode::Invalid { discard } => {
let discard = discard.min(frame_buf.len()).max(1);
frame_buf.drain(..discard);
}
}
}
Expand Down
Loading
Loading