From a9912b89d31cd4abebfb520613a3f0b1fbb093c7 Mon Sep 17 00:00:00 2001 From: bartlino Date: Sat, 29 Aug 2026 18:24:57 +0000 Subject: [PATCH 1/3] fix(mstp): USB chunk gaps must not trigger T_frame_abort Host async serial reads arrive in multi-millisecond chunks; treating inter-read gaps as Clause 9 T_frame_abort (~1.56ms @ 38400) discarded partial frames mid-assembly. Add decode_frame_stream (Complete/NeedMore/ Invalid), preserve trailing lone 0x55 on preamble miss, and replace wire T_frame_abort with a host stale-partial timeout based on max frame wire time plus USB scheduling slack. Co-authored-by: Cursor --- crates/bacnet-transport/src/mstp/mod.rs | 13 +- crates/bacnet-transport/src/mstp/port.rs | 40 ++-- crates/bacnet-transport/src/mstp_frame.rs | 241 ++++++++++++++++++++++ 3 files changed, 272 insertions(+), 22 deletions(-) diff --git a/crates/bacnet-transport/src/mstp/mod.rs b/crates/bacnet-transport/src/mstp/mod.rs index 467dc7ab..80a74cc4 100644 --- a/crates/bacnet-transport/src/mstp/mod.rs +++ b/crates/bacnet-transport/src/mstp/mod.rs @@ -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; diff --git a/crates/bacnet-transport/src/mstp/port.rs b/crates/bacnet-transport/src/mstp/port.rs index 6678361e..f175409f 100644 --- a/crates/bacnet-transport/src/mstp/port.rs +++ b/crates/bacnet-transport/src/mstp/port.rs @@ -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, }; @@ -65,7 +66,9 @@ impl TransportPort for MstpTransport { 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); @@ -123,14 +126,19 @@ impl TransportPort for MstpTransport { 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(); } @@ -160,7 +168,7 @@ impl TransportPort for MstpTransport { let preamble_pos = match find_preamble(&frame_buf) { Some(pos) => pos, None => { - frame_buf.clear(); + retain_lone_preamble_byte(&mut frame_buf); break; } }; @@ -170,8 +178,8 @@ impl TransportPort for MstpTransport { 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 @@ -274,12 +282,10 @@ impl TransportPort for MstpTransport { }), ); } - 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); } } } diff --git a/crates/bacnet-transport/src/mstp_frame.rs b/crates/bacnet-transport/src/mstp_frame.rs index 0e11767a..9738e376 100644 --- a/crates/bacnet-transport/src/mstp_frame.rs +++ b/crates/bacnet-transport/src/mstp_frame.rs @@ -246,6 +246,109 @@ pub fn encode_frame( Ok(()) } +/// Result of incremental/streaming MS/TP frame decode. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum StreamDecode { + /// A complete, validated frame was decoded. + Complete { frame: MstpFrame, consumed: usize }, + /// More bytes are required before a decode decision can be made. + NeedMore, + /// The buffer contains invalid data; discard at least `discard` bytes and resync. + Invalid { discard: usize }, +} + +/// Incrementally decode an MS/TP frame from a receive buffer (starting at the preamble). +/// +/// Unlike [`decode_frame`], incomplete input returns [`StreamDecode::NeedMore`] instead of +/// an error. Real corruption (bad CRC, invalid header) returns [`StreamDecode::Invalid`]. +pub fn decode_frame_stream(data: &[u8]) -> StreamDecode { + if data.is_empty() { + return StreamDecode::NeedMore; + } + + if data[0] != PREAMBLE[0] { + return StreamDecode::Invalid { discard: 1 }; + } + if data.len() < 2 { + return StreamDecode::NeedMore; + } + if data[1] != PREAMBLE[1] { + return StreamDecode::Invalid { discard: 1 }; + } + + // Preamble(2) + header fields(5) + header_crc(1) + if data.len() < 2 + HEADER_LENGTH { + return StreamDecode::NeedMore; + } + + if !crc8_valid(&data[2..8]) { + return StreamDecode::Invalid { discard: 1 }; + } + + let frame_type = FrameType::from_raw(data[2]); + let destination = data[3]; + let source = data[4]; + + if source > MAX_MASTER && source != BROADCAST_MAC { + return StreamDecode::Invalid { discard: 1 }; + } + if source == BROADCAST_MAC { + return StreamDecode::Invalid { discard: 1 }; + } + + let data_length = ((data[5] as usize) << 8) | (data[6] as usize); + if data_length > MAX_MPDU_DATA { + return StreamDecode::Invalid { discard: 1 }; + } + + let mut consumed = 2 + HEADER_LENGTH; + + if data_length > 0 { + let needed = consumed + data_length + 2; + if data.len() < needed { + return StreamDecode::NeedMore; + } + + if !crc16_valid(&data[consumed..consumed + data_length + 2]) { + return StreamDecode::Invalid { discard: needed }; + } + + let payload = Bytes::copy_from_slice(&data[consumed..consumed + data_length]); + consumed += data_length + 2; + + StreamDecode::Complete { + frame: MstpFrame { + frame_type, + destination, + source, + data: payload, + }, + consumed, + } + } else { + StreamDecode::Complete { + frame: MstpFrame { + frame_type, + destination, + source, + data: Bytes::new(), + }, + consumed, + } + } +} + +/// When no full preamble is found, retain a trailing lone `0x55` for the next chunk. +pub fn retain_lone_preamble_byte(buf: &mut Vec) { + if buf.last() == Some(&PREAMBLE[0]) { + let lone = PREAMBLE[0]; + buf.clear(); + buf.push(lone); + } else { + buf.clear(); + } +} + /// Decode an MS/TP frame from raw bytes (starting at the preamble). /// /// Returns the decoded frame and the number of bytes consumed. @@ -727,4 +830,142 @@ mod tests { let (decoded, _) = decode_frame(&buf).unwrap(); assert_eq!(decoded.source, MAX_MASTER); } + + // ----------------------------------------------------------------------- + // Streaming decode tests + // ----------------------------------------------------------------------- + + fn encode_token_frame() -> Vec { + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + buf.to_vec() + } + + fn encode_data_frame() -> (MstpFrame, Vec) { + let frame = MstpFrame { + frame_type: FrameType::BACnetDataNotExpectingReply, + destination: 5, + source: 0, + data: Bytes::from_static(&[0x01, 0x02, 0x03, 0x04]), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + (frame, buf.to_vec()) + } + + #[test] + fn stream_decode_token_complete() { + let wire = encode_token_frame(); + assert_eq!( + decode_frame_stream(&wire), + StreamDecode::Complete { + frame: MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }, + consumed: 8, + } + ); + } + + #[test] + fn stream_decode_split_header_then_body() { + let (_frame, wire) = encode_data_frame(); + let split = wire.len() - 3; + + assert_eq!(decode_frame_stream(&wire[..split]), StreamDecode::NeedMore); + + let mut assembled = wire[..split].to_vec(); + assembled.extend_from_slice(&wire[split..]); + let StreamDecode::Complete { frame, consumed } = decode_frame_stream(&assembled) else { + panic!("expected complete frame after reassembly"); + }; + assert_eq!(consumed, wire.len()); + assert_eq!(frame.data.len(), 4); + } + + #[test] + fn stream_decode_preamble_split_across_chunks() { + let wire = encode_token_frame(); + assert_eq!(decode_frame_stream(&wire[..1]), StreamDecode::NeedMore); + + let mut assembled = wire[..1].to_vec(); + assembled.extend_from_slice(&wire[1..]); + assert!(matches!( + decode_frame_stream(&assembled), + StreamDecode::Complete { .. } + )); + } + + #[test] + fn stream_decode_host_gap_simulation_without_clear() { + let wire = encode_token_frame(); + let mut buf = wire[..4].to_vec(); + assert_eq!(decode_frame_stream(&buf), StreamDecode::NeedMore); + + // Simulate a host gap far beyond wire T_frame_abort without clearing the buffer. + buf.extend_from_slice(&wire[4..]); + assert!(matches!( + decode_frame_stream(&buf), + StreamDecode::Complete { .. } + )); + } + + #[test] + fn retain_lone_preamble_byte_preserves_trailing_0x55() { + let mut buf = vec![0x00, 0x12, 0x55]; + retain_lone_preamble_byte(&mut buf); + assert_eq!(buf, vec![0x55]); + + let mut no_preamble = vec![0x00, 0x12, 0x34]; + retain_lone_preamble_byte(&mut no_preamble); + assert!(no_preamble.is_empty()); + } + + #[test] + fn stream_decode_bad_header_crc_invalid() { + let mut wire = encode_token_frame(); + wire[7] ^= 0xFF; + assert_eq!( + decode_frame_stream(&wire), + StreamDecode::Invalid { discard: 1 } + ); + } + + #[test] + fn stream_decode_bad_data_crc_invalid_discards_frame() { + let (_frame, mut wire) = encode_data_frame(); + let last = wire.len() - 1; + wire[last] ^= 0xFF; + assert_eq!( + decode_frame_stream(&wire), + StreamDecode::Invalid { + discard: wire.len() + } + ); + } + + #[test] + fn stream_decode_need_more_on_short_header() { + assert_eq!( + decode_frame_stream(&[0x55, 0xFF, 0x00, 0x01]), + StreamDecode::NeedMore + ); + } + + #[test] + fn stream_decode_invalid_on_bad_second_preamble_byte() { + assert_eq!( + decode_frame_stream(&[0x55, 0x00]), + StreamDecode::Invalid { discard: 1 } + ); + } } From 6a70b85ef23c18ac06900709552113b96c1ea64e Mon Sep 17 00:00:00 2001 From: bartlino Date: Sun, 30 Aug 2026 14:12:41 +0000 Subject: [PATCH 2/3] fix(mstp): use BACnet Clause 9 header and data CRCs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace incorrect reflected polynomials (0xE0 / Modbus 0xA001) with Clause 9.6 values (0x81 / 0x8408). Add literal golden vectors including the live Token 0<-7 frame (CRC 0x37) and data frame 01 00 → 9F 16 so self-round-trip tests cannot mask interoperability failures. Co-authored-by: Cursor --- crates/bacnet-transport/src/mstp_frame.rs | 162 ++++++++++++++++------ 1 file changed, 123 insertions(+), 39 deletions(-) diff --git a/crates/bacnet-transport/src/mstp_frame.rs b/crates/bacnet-transport/src/mstp_frame.rs index 9738e376..2a9b2aa2 100644 --- a/crates/bacnet-transport/src/mstp_frame.rs +++ b/crates/bacnet-transport/src/mstp_frame.rs @@ -111,7 +111,8 @@ pub struct MstpFrame { // CRC-8 (Header CRC) // --------------------------------------------------------------------------- -/// CRC-8 lookup table. +/// BACnet Clause 9.6 Frame Header CRC — reflected poly `G(x)=x^8+x^7+1` → `0x81`. +/// (Prior incorrect table used `0xE0`, which passes self-round-trip but rejects live trunk frames.) const CRC8_TABLE: [u8; 256] = { let mut table = [0u8; 256]; let mut i = 0usize; @@ -120,7 +121,7 @@ const CRC8_TABLE: [u8; 256] = { let mut j = 0; while j < 8 { if crc & 1 != 0 { - crc = (crc >> 1) ^ 0xE0; + crc = (crc >> 1) ^ 0x81; } else { crc >>= 1; } @@ -132,7 +133,10 @@ const CRC8_TABLE: [u8; 256] = { table }; -/// Compute CRC-8 over the given data. Initial value 0xFF, result inverted. +/// Good header-CRC receiver residual (Clause 9.6), including the CRC octet. +pub const HEADER_CRC_RESIDUAL: u8 = 0x55; + +/// Compute CRC-8 over the given data. Initial value 0xFF, result ones-complemented. pub fn crc8(data: &[u8]) -> u8 { let mut crc: u8 = 0xFF; for &b in data { @@ -141,6 +145,15 @@ pub fn crc8(data: &[u8]) -> u8 { !crc } +/// Running header CRC including the transmitted CRC octet (no final invert). +pub fn crc8_accumulate_all(data_with_crc: &[u8]) -> u8 { + let mut crc: u8 = 0xFF; + for &b in data_with_crc { + crc = CRC8_TABLE[(crc ^ b) as usize]; + } + crc +} + /// Verify CRC-8: recomputes CRC over data (excluding last byte) and compares /// to the stored CRC byte. pub fn crc8_valid(data_with_crc: &[u8]) -> bool { @@ -155,7 +168,8 @@ pub fn crc8_valid(data_with_crc: &[u8]) -> bool { // CRC-16 (Data CRC) // --------------------------------------------------------------------------- -/// CRC-16 lookup table. +/// BACnet Clause 9.6 Data CRC — CRC-16-CCITT reflected poly `0x8408`. +/// (Prior incorrect table used Modbus `0xA001`.) const CRC16_TABLE: [u16; 256] = { let mut table = [0u16; 256]; let mut i = 0usize; @@ -164,7 +178,7 @@ const CRC16_TABLE: [u16; 256] = { let mut j = 0; while j < 8 { if crc & 1 != 0 { - crc = (crc >> 1) ^ 0xA001; + crc = (crc >> 1) ^ 0x8408; } else { crc >>= 1; } @@ -176,7 +190,10 @@ const CRC16_TABLE: [u16; 256] = { table }; -/// Compute CRC-16 over the given data. Initial value 0xFFFF, result inverted. +/// Good data-CRC receiver residual (Clause 9.6), including the CRC octets. +pub const DATA_CRC_RESIDUAL: u16 = 0xF0B8; + +/// Compute CRC-16 over the given data. Initial value 0xFFFF, result ones-complemented. pub fn crc16(data: &[u8]) -> u16 { let mut crc: u16 = 0xFFFF; for &b in data { @@ -185,8 +202,17 @@ pub fn crc16(data: &[u8]) -> u16 { !crc } +/// Running data CRC including the transmitted CRC octets (no final invert). +pub fn crc16_accumulate_all(data_with_crc: &[u8]) -> u16 { + let mut crc: u16 = 0xFFFF; + for &b in data_with_crc { + crc = (crc >> 8) ^ CRC16_TABLE[((crc ^ b as u16) & 0xFF) as usize]; + } + crc +} + /// Verify CRC-16: recomputes CRC over data (excluding last 2 bytes) and compares -/// to the stored CRC (little-endian). +/// to the stored CRC (little-endian; LS octet first on the wire). pub fn crc16_valid(data_with_crc: &[u8]) -> bool { if data_with_crc.len() < 3 { return false; @@ -459,46 +485,64 @@ mod tests { use super::*; // ----------------------------------------------------------------------- - // CRC tests + // CRC tests — Clause 9.6 golden vectors (literal expected bytes; not from crc8/crc16) // ----------------------------------------------------------------------- #[test] - fn crc8_known_value() { - // Token frame header: type=0x00, dest=0x01, src=0x00, len=0x0000 - let header = [0x00, 0x01, 0x00, 0x00, 0x00]; - let crc = crc8(&header); - // Verify by appending CRC and checking validity + fn crc8_clause9_header_vectors() { + // [frame_type, dest, src, len_hi, len_lo] -> header CRC + let vectors: &[([u8; 5], u8)] = &[ + ([0x00, 0x00, 0x07, 0x00, 0x00], 0x37), + ([0x00, 0x07, 0x00, 0x00, 0x00], 0x40), + ([0x01, 0x00, 0x07, 0x00, 0x00], 0xB1), + ([0x01, 0x07, 0x00, 0x00, 0x00], 0xC6), + ]; + for (header, expected) in vectors { + assert_eq!(crc8(header), *expected, "header={header:02X?}"); + let mut with_crc = header.to_vec(); + with_crc.push(*expected); + assert!(crc8_valid(&with_crc)); + assert_eq!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); + } + } + + #[test] + fn crc8_one_bit_corruption_rejected() { + let header = [0x00, 0x00, 0x07, 0x00, 0x00]; let mut with_crc = header.to_vec(); - with_crc.push(crc); + with_crc.push(0x37); assert!(crc8_valid(&with_crc)); + with_crc[0] ^= 0x01; + assert!(!crc8_valid(&with_crc)); + assert_ne!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); } #[test] - fn crc8_validate_round_trip() { - let data = [0x05, 0xFF, 0x03, 0x00, 0x0C]; - let crc = crc8(&data); - let mut with_crc = data.to_vec(); - with_crc.push(crc); - assert!(crc8_valid(&with_crc)); + fn crc16_clause9_data_vector_01_00() { + // Data 01 00 → CRC 0x169F, wire order LS first: 9F 16 + assert_eq!(crc16(&[0x01, 0x00]), 0x169F); + let with_crc = [0x01, 0x00, 0x9F, 0x16]; + assert!(crc16_valid(&with_crc)); + assert_eq!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); } #[test] - fn crc8_invalid_detects_corruption() { - let data = [0x05, 0xFF, 0x03, 0x00, 0x0C]; - let crc = crc8(&data); - let mut with_crc = data.to_vec(); - with_crc.push(crc ^ 0x01); // corrupt - assert!(!crc8_valid(&with_crc)); + fn crc16_one_bit_corruption_rejected() { + let mut with_crc = [0x01, 0x00, 0x9F, 0x16]; + assert!(crc16_valid(&with_crc)); + with_crc[0] ^= 0x01; + assert!(!crc16_valid(&with_crc)); + assert_ne!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); } #[test] - fn crc16_known_value() { - let data = [0x01, 0x00, 0x10, 0x02]; - let crc = crc16(&data); + fn crc8_validate_round_trip() { + let data = [0x05, 0xFF, 0x03, 0x00, 0x0C]; + let crc = crc8(&data); let mut with_crc = data.to_vec(); - with_crc.push(crc as u8); - with_crc.push((crc >> 8) as u8); - assert!(crc16_valid(&with_crc)); + with_crc.push(crc); + assert!(crc8_valid(&with_crc)); + assert_eq!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); } #[test] @@ -509,16 +553,56 @@ mod tests { with_crc.push(crc as u8); with_crc.push((crc >> 8) as u8); assert!(crc16_valid(&with_crc)); + assert_eq!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); } #[test] - fn crc16_invalid_detects_corruption() { - let data = [0x01, 0x02, 0x03]; - let crc = crc16(&data); - let mut with_crc = data.to_vec(); - with_crc.push(crc as u8); - with_crc.push((crc >> 8) as u8 ^ 0x01); // corrupt - assert!(!crc16_valid(&with_crc)); + fn literal_token_frame_0_from_7() { + // Live trunk Token: dest BASRT(0) <- FEC(7), header CRC 0x37 + let wire: &[u8] = &[0x55, 0xFF, 0x00, 0x00, 0x07, 0x00, 0x00, 0x37]; + let (frame, consumed) = decode_frame(wire).expect("decode Token 0<-7"); + assert_eq!(consumed, 8); + assert_eq!(frame.frame_type, FrameType::Token); + assert_eq!(frame.destination, 0); + assert_eq!(frame.source, 7); + assert!(frame.data.is_empty()); + + let mut enc = BytesMut::new(); + encode_frame(&mut enc, &frame).unwrap(); + assert_eq!(&enc[..], wire); + } + + #[test] + fn literal_data_not_expecting_reply_frame() { + // Frame type 06, dest 0, src 7, len 2, HDR D9, data 01 00, DCRC 9F 16 + let wire: &[u8] = &[ + 0x55, 0xFF, 0x06, 0x00, 0x07, 0x00, 0x02, 0xD9, 0x01, 0x00, 0x9F, 0x16, + ]; + let (frame, consumed) = decode_frame(wire).expect("decode data frame"); + assert_eq!(consumed, 12); + assert_eq!(frame.frame_type, FrameType::BACnetDataNotExpectingReply); + assert_eq!(frame.destination, 0); + assert_eq!(frame.source, 7); + assert_eq!(&frame.data[..], &[0x01, 0x00]); + + let mut enc = BytesMut::new(); + encode_frame(&mut enc, &frame).unwrap(); + assert_eq!(&enc[..], wire); + } + + #[test] + fn truncated_header_need_more_or_err() { + let partial = [0x55, 0xFF, 0x00, 0x00, 0x07]; + assert!(decode_frame(&partial).is_err()); + assert_eq!(decode_frame_stream(&partial), StreamDecode::NeedMore); + } + + #[test] + fn truncated_data_crc_need_more() { + // Header complete for len=2 but missing data CRC octets + let partial = [0x55, 0xFF, 0x06, 0x00, 0x07, 0x00, 0x02, 0xD9, 0x01, 0x00]; + assert_eq!(decode_frame_stream(&partial), StreamDecode::NeedMore); + assert!(decode_frame(&partial).is_err()); } // ----------------------------------------------------------------------- From 73a1fd41df7df2dfb3fa005cf339f347751f0286 Mon Sep 17 00:00:00 2001 From: bartlino Date: Sun, 30 Aug 2026 14:23:40 +0000 Subject: [PATCH 3/3] refactor(mstp): split frame tests to satisfy 700 LOC cap Move Clause 9 golden / stream tests into mstp_frame_tests.rs so mstp_frame.rs stays under the upstream file-size CI gate. Co-authored-by: Cursor --- crates/bacnet-transport/src/mstp_frame.rs | 574 +----------------- .../bacnet-transport/src/mstp_frame_tests.rs | 570 +++++++++++++++++ 2 files changed, 572 insertions(+), 572 deletions(-) create mode 100644 crates/bacnet-transport/src/mstp_frame_tests.rs diff --git a/crates/bacnet-transport/src/mstp_frame.rs b/crates/bacnet-transport/src/mstp_frame.rs index 2a9b2aa2..78f7b0ab 100644 --- a/crates/bacnet-transport/src/mstp_frame.rs +++ b/crates/bacnet-transport/src/mstp_frame.rs @@ -481,575 +481,5 @@ pub fn find_preamble(data: &[u8]) -> Option { } #[cfg(test)] -mod tests { - use super::*; - - // ----------------------------------------------------------------------- - // CRC tests — Clause 9.6 golden vectors (literal expected bytes; not from crc8/crc16) - // ----------------------------------------------------------------------- - - #[test] - fn crc8_clause9_header_vectors() { - // [frame_type, dest, src, len_hi, len_lo] -> header CRC - let vectors: &[([u8; 5], u8)] = &[ - ([0x00, 0x00, 0x07, 0x00, 0x00], 0x37), - ([0x00, 0x07, 0x00, 0x00, 0x00], 0x40), - ([0x01, 0x00, 0x07, 0x00, 0x00], 0xB1), - ([0x01, 0x07, 0x00, 0x00, 0x00], 0xC6), - ]; - for (header, expected) in vectors { - assert_eq!(crc8(header), *expected, "header={header:02X?}"); - let mut with_crc = header.to_vec(); - with_crc.push(*expected); - assert!(crc8_valid(&with_crc)); - assert_eq!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); - } - } - - #[test] - fn crc8_one_bit_corruption_rejected() { - let header = [0x00, 0x00, 0x07, 0x00, 0x00]; - let mut with_crc = header.to_vec(); - with_crc.push(0x37); - assert!(crc8_valid(&with_crc)); - with_crc[0] ^= 0x01; - assert!(!crc8_valid(&with_crc)); - assert_ne!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); - } - - #[test] - fn crc16_clause9_data_vector_01_00() { - // Data 01 00 → CRC 0x169F, wire order LS first: 9F 16 - assert_eq!(crc16(&[0x01, 0x00]), 0x169F); - let with_crc = [0x01, 0x00, 0x9F, 0x16]; - assert!(crc16_valid(&with_crc)); - assert_eq!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); - } - - #[test] - fn crc16_one_bit_corruption_rejected() { - let mut with_crc = [0x01, 0x00, 0x9F, 0x16]; - assert!(crc16_valid(&with_crc)); - with_crc[0] ^= 0x01; - assert!(!crc16_valid(&with_crc)); - assert_ne!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); - } - - #[test] - fn crc8_validate_round_trip() { - let data = [0x05, 0xFF, 0x03, 0x00, 0x0C]; - let crc = crc8(&data); - let mut with_crc = data.to_vec(); - with_crc.push(crc); - assert!(crc8_valid(&with_crc)); - assert_eq!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); - } - - #[test] - fn crc16_validate_round_trip() { - let data = vec![0xAA; 100]; - let crc = crc16(&data); - let mut with_crc = data; - with_crc.push(crc as u8); - with_crc.push((crc >> 8) as u8); - assert!(crc16_valid(&with_crc)); - assert_eq!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); - } - - #[test] - fn literal_token_frame_0_from_7() { - // Live trunk Token: dest BASRT(0) <- FEC(7), header CRC 0x37 - let wire: &[u8] = &[0x55, 0xFF, 0x00, 0x00, 0x07, 0x00, 0x00, 0x37]; - let (frame, consumed) = decode_frame(wire).expect("decode Token 0<-7"); - assert_eq!(consumed, 8); - assert_eq!(frame.frame_type, FrameType::Token); - assert_eq!(frame.destination, 0); - assert_eq!(frame.source, 7); - assert!(frame.data.is_empty()); - - let mut enc = BytesMut::new(); - encode_frame(&mut enc, &frame).unwrap(); - assert_eq!(&enc[..], wire); - } - - #[test] - fn literal_data_not_expecting_reply_frame() { - // Frame type 06, dest 0, src 7, len 2, HDR D9, data 01 00, DCRC 9F 16 - let wire: &[u8] = &[ - 0x55, 0xFF, 0x06, 0x00, 0x07, 0x00, 0x02, 0xD9, 0x01, 0x00, 0x9F, 0x16, - ]; - let (frame, consumed) = decode_frame(wire).expect("decode data frame"); - assert_eq!(consumed, 12); - assert_eq!(frame.frame_type, FrameType::BACnetDataNotExpectingReply); - assert_eq!(frame.destination, 0); - assert_eq!(frame.source, 7); - assert_eq!(&frame.data[..], &[0x01, 0x00]); - - let mut enc = BytesMut::new(); - encode_frame(&mut enc, &frame).unwrap(); - assert_eq!(&enc[..], wire); - } - - #[test] - fn truncated_header_need_more_or_err() { - let partial = [0x55, 0xFF, 0x00, 0x00, 0x07]; - assert!(decode_frame(&partial).is_err()); - assert_eq!(decode_frame_stream(&partial), StreamDecode::NeedMore); - } - - #[test] - fn truncated_data_crc_need_more() { - // Header complete for len=2 but missing data CRC octets - let partial = [0x55, 0xFF, 0x06, 0x00, 0x07, 0x00, 0x02, 0xD9, 0x01, 0x00]; - assert_eq!(decode_frame_stream(&partial), StreamDecode::NeedMore); - assert!(decode_frame(&partial).is_err()); - } - - // ----------------------------------------------------------------------- - // Frame encode/decode tests - // ----------------------------------------------------------------------- - - #[test] - fn token_frame_round_trip() { - let frame = MstpFrame { - frame_type: FrameType::Token, - destination: 1, - source: 0, - data: Bytes::new(), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - // Token has no data, so: preamble(2) + header(5) + crc(1) = 8 - assert_eq!(buf.len(), 8); - assert_eq!(&buf[..2], &PREAMBLE); - - let (decoded, consumed) = decode_frame(&buf).unwrap(); - assert_eq!(consumed, 8); - assert_eq!(decoded, frame); - } - - #[test] - fn poll_for_master_round_trip() { - let frame = MstpFrame { - frame_type: FrameType::PollForMaster, - destination: 42, - source: 0, - data: Bytes::new(), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - let (decoded, _) = decode_frame(&buf).unwrap(); - assert_eq!(decoded, frame); - } - - #[test] - fn data_expecting_reply_round_trip() { - let npdu = vec![0x01, 0x00, 0x10, 0x02, 0x03, 0x04, 0x05]; - let frame = MstpFrame { - frame_type: FrameType::BACnetDataExpectingReply, - destination: 5, - source: 0, - data: Bytes::from(npdu.clone()), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - // preamble(2) + header(5) + hcrc(1) + data(7) + dcrc(2) = 17 - assert_eq!(buf.len(), 17); - - let (decoded, consumed) = decode_frame(&buf).unwrap(); - assert_eq!(consumed, 17); - assert_eq!(decoded.frame_type, FrameType::BACnetDataExpectingReply); - assert_eq!(decoded.destination, 5); - assert_eq!(decoded.source, 0); - assert_eq!(decoded.data, npdu); - } - - #[test] - fn data_not_expecting_reply_round_trip() { - let npdu = vec![0x01, 0x20, 0xFF, 0xFF, 0x00, 0xFF, 0x10, 0x08]; - let frame = MstpFrame { - frame_type: FrameType::BACnetDataNotExpectingReply, - destination: BROADCAST_MAC, - source: 3, - data: Bytes::from(npdu.clone()), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - let (decoded, _) = decode_frame(&buf).unwrap(); - assert_eq!(decoded, frame); - } - - #[test] - fn broadcast_destination() { - let frame = MstpFrame { - frame_type: FrameType::BACnetDataNotExpectingReply, - destination: BROADCAST_MAC, - source: 10, - data: Bytes::from_static(&[0x01, 0x00]), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - let (decoded, _) = decode_frame(&buf).unwrap(); - assert_eq!(decoded.destination, BROADCAST_MAC); - } - - #[test] - fn reply_to_poll_for_master_round_trip() { - let frame = MstpFrame { - frame_type: FrameType::ReplyToPollForMaster, - destination: 0, - source: 42, - data: Bytes::new(), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - let (decoded, _) = decode_frame(&buf).unwrap(); - assert_eq!(decoded, frame); - } - - #[test] - fn test_request_with_data_round_trip() { - let test_data = vec![0xDE, 0xAD, 0xBE, 0xEF]; - let frame = MstpFrame { - frame_type: FrameType::TestRequest, - destination: 5, - source: 0, - data: Bytes::from(test_data.clone()), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - let (decoded, _) = decode_frame(&buf).unwrap(); - assert_eq!(decoded.data, test_data); - } - - #[test] - fn decode_too_short() { - assert!(decode_frame(&[0x55, 0xFF, 0x00]).is_err()); - } - - #[test] - fn decode_bad_preamble() { - let data = [0x00, 0xFF, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00]; - assert!(decode_frame(&data).is_err()); - } - - #[test] - fn decode_bad_header_crc() { - let mut buf = BytesMut::new(); - let frame = MstpFrame { - frame_type: FrameType::Token, - destination: 1, - source: 0, - data: Bytes::new(), - }; - encode_frame(&mut buf, &frame).unwrap(); - // Corrupt header CRC (byte 7) - buf[7] ^= 0xFF; - assert!(decode_frame(&buf).is_err()); - } - - #[test] - fn decode_bad_data_crc() { - let mut buf = BytesMut::new(); - let frame = MstpFrame { - frame_type: FrameType::BACnetDataNotExpectingReply, - destination: 1, - source: 0, - data: Bytes::from_static(&[0x01, 0x00]), - }; - encode_frame(&mut buf, &frame).unwrap(); - // Corrupt last byte (data CRC high) - let last = buf.len() - 1; - buf[last] ^= 0xFF; - assert!(decode_frame(&buf).is_err()); - } - - #[test] - fn decode_truncated_data() { - let mut buf = BytesMut::new(); - let frame = MstpFrame { - frame_type: FrameType::BACnetDataExpectingReply, - destination: 5, - source: 0, - data: Bytes::from_static(&[0x01, 0x02, 0x03, 0x04]), - }; - encode_frame(&mut buf, &frame).unwrap(); - // Truncate: remove data CRC - buf.truncate(buf.len() - 2); - assert!(decode_frame(&buf).is_err()); - } - - #[test] - fn find_preamble_at_start() { - let data = [0x55, 0xFF, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00]; - assert_eq!(find_preamble(&data), Some(0)); - } - - #[test] - fn find_preamble_with_garbage() { - let data = [0x00, 0x00, 0x12, 0x55, 0xFF, 0x00, 0x01, 0x00]; - assert_eq!(find_preamble(&data), Some(3)); - } - - #[test] - fn find_preamble_none() { - let data = [0x00, 0x55, 0x00, 0xFF, 0x01]; - assert_eq!(find_preamble(&data), None); - } - - #[test] - fn frame_type_round_trip() { - for raw in 0..=0x07 { - let ft = FrameType::from_raw(raw); - assert_eq!(ft.to_raw(), raw); - } - // Unknown type - let ft = FrameType::from_raw(0x42); - assert_eq!(ft.to_raw(), 0x42); - assert_eq!(ft, FrameType::Unknown(0x42)); - } - - #[test] - fn frame_type_has_data() { - assert!(!FrameType::Token.has_data()); - assert!(!FrameType::PollForMaster.has_data()); - assert!(!FrameType::ReplyToPollForMaster.has_data()); - assert!(FrameType::TestRequest.has_data()); - assert!(FrameType::TestResponse.has_data()); - assert!(FrameType::BACnetDataExpectingReply.has_data()); - assert!(FrameType::BACnetDataNotExpectingReply.has_data()); - assert!(!FrameType::ReplyPostponed.has_data()); - } - - #[test] - fn large_data_frame() { - // Near-maximum data size - let npdu = vec![0xAA; 1024]; - let frame = MstpFrame { - frame_type: FrameType::BACnetDataNotExpectingReply, - destination: BROADCAST_MAC, - source: 0, - data: Bytes::from(npdu.clone()), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - let (decoded, _) = decode_frame(&buf).unwrap(); - assert_eq!(decoded.data, npdu); - } - - #[test] - fn encode_oversized_data_returns_error() { - let frame = MstpFrame { - frame_type: FrameType::BACnetDataNotExpectingReply, - destination: 1, - source: 0, - data: Bytes::from_static(&[0xAA; MAX_MPDU_DATA + 1]), - }; - let mut buf = BytesMut::new(); - assert!(encode_frame(&mut buf, &frame).is_err()); - } - - #[test] - fn decode_rejects_source_above_max_master() { - // Encode a valid frame then patch the source to 128 (above MAX_MASTER=127) - let frame = MstpFrame { - frame_type: FrameType::Token, - destination: 1, - source: 0, - data: Bytes::new(), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - // Patch source byte (offset 4) to 128 - buf[4] = 128; - // Recompute header CRC (bytes 2..7, CRC at byte 7) - let header_crc = crc8(&buf[2..7]); - buf[7] = header_crc; - - assert!(decode_frame(&buf).is_err()); - } - - #[test] - fn decode_rejects_broadcast_source() { - // Encode a valid frame then patch the source to BROADCAST_MAC (0xFF) - let frame = MstpFrame { - frame_type: FrameType::Token, - destination: 1, - source: 0, - data: Bytes::new(), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - // Patch source byte to 0xFF - buf[4] = BROADCAST_MAC; - // Recompute header CRC - let header_crc = crc8(&buf[2..7]); - buf[7] = header_crc; - - assert!(decode_frame(&buf).is_err()); - } - - #[test] - fn decode_accepts_max_master_source() { - // Source = MAX_MASTER (127) should be valid - let frame = MstpFrame { - frame_type: FrameType::Token, - destination: 1, - source: MAX_MASTER, - data: Bytes::new(), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - - let (decoded, _) = decode_frame(&buf).unwrap(); - assert_eq!(decoded.source, MAX_MASTER); - } - - // ----------------------------------------------------------------------- - // Streaming decode tests - // ----------------------------------------------------------------------- - - fn encode_token_frame() -> Vec { - let frame = MstpFrame { - frame_type: FrameType::Token, - destination: 1, - source: 0, - data: Bytes::new(), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - buf.to_vec() - } - - fn encode_data_frame() -> (MstpFrame, Vec) { - let frame = MstpFrame { - frame_type: FrameType::BACnetDataNotExpectingReply, - destination: 5, - source: 0, - data: Bytes::from_static(&[0x01, 0x02, 0x03, 0x04]), - }; - let mut buf = BytesMut::new(); - encode_frame(&mut buf, &frame).unwrap(); - (frame, buf.to_vec()) - } - - #[test] - fn stream_decode_token_complete() { - let wire = encode_token_frame(); - assert_eq!( - decode_frame_stream(&wire), - StreamDecode::Complete { - frame: MstpFrame { - frame_type: FrameType::Token, - destination: 1, - source: 0, - data: Bytes::new(), - }, - consumed: 8, - } - ); - } - - #[test] - fn stream_decode_split_header_then_body() { - let (_frame, wire) = encode_data_frame(); - let split = wire.len() - 3; - - assert_eq!(decode_frame_stream(&wire[..split]), StreamDecode::NeedMore); - - let mut assembled = wire[..split].to_vec(); - assembled.extend_from_slice(&wire[split..]); - let StreamDecode::Complete { frame, consumed } = decode_frame_stream(&assembled) else { - panic!("expected complete frame after reassembly"); - }; - assert_eq!(consumed, wire.len()); - assert_eq!(frame.data.len(), 4); - } - - #[test] - fn stream_decode_preamble_split_across_chunks() { - let wire = encode_token_frame(); - assert_eq!(decode_frame_stream(&wire[..1]), StreamDecode::NeedMore); - - let mut assembled = wire[..1].to_vec(); - assembled.extend_from_slice(&wire[1..]); - assert!(matches!( - decode_frame_stream(&assembled), - StreamDecode::Complete { .. } - )); - } - - #[test] - fn stream_decode_host_gap_simulation_without_clear() { - let wire = encode_token_frame(); - let mut buf = wire[..4].to_vec(); - assert_eq!(decode_frame_stream(&buf), StreamDecode::NeedMore); - - // Simulate a host gap far beyond wire T_frame_abort without clearing the buffer. - buf.extend_from_slice(&wire[4..]); - assert!(matches!( - decode_frame_stream(&buf), - StreamDecode::Complete { .. } - )); - } - - #[test] - fn retain_lone_preamble_byte_preserves_trailing_0x55() { - let mut buf = vec![0x00, 0x12, 0x55]; - retain_lone_preamble_byte(&mut buf); - assert_eq!(buf, vec![0x55]); - - let mut no_preamble = vec![0x00, 0x12, 0x34]; - retain_lone_preamble_byte(&mut no_preamble); - assert!(no_preamble.is_empty()); - } - - #[test] - fn stream_decode_bad_header_crc_invalid() { - let mut wire = encode_token_frame(); - wire[7] ^= 0xFF; - assert_eq!( - decode_frame_stream(&wire), - StreamDecode::Invalid { discard: 1 } - ); - } - - #[test] - fn stream_decode_bad_data_crc_invalid_discards_frame() { - let (_frame, mut wire) = encode_data_frame(); - let last = wire.len() - 1; - wire[last] ^= 0xFF; - assert_eq!( - decode_frame_stream(&wire), - StreamDecode::Invalid { - discard: wire.len() - } - ); - } - - #[test] - fn stream_decode_need_more_on_short_header() { - assert_eq!( - decode_frame_stream(&[0x55, 0xFF, 0x00, 0x01]), - StreamDecode::NeedMore - ); - } - - #[test] - fn stream_decode_invalid_on_bad_second_preamble_byte() { - assert_eq!( - decode_frame_stream(&[0x55, 0x00]), - StreamDecode::Invalid { discard: 1 } - ); - } -} +#[path = "mstp_frame_tests.rs"] +mod tests; diff --git a/crates/bacnet-transport/src/mstp_frame_tests.rs b/crates/bacnet-transport/src/mstp_frame_tests.rs new file mode 100644 index 00000000..83df7a38 --- /dev/null +++ b/crates/bacnet-transport/src/mstp_frame_tests.rs @@ -0,0 +1,570 @@ +use super::*; + +// ----------------------------------------------------------------------- +// CRC tests — Clause 9.6 golden vectors (literal expected bytes; not from crc8/crc16) +// ----------------------------------------------------------------------- + +#[test] +fn crc8_clause9_header_vectors() { + // [frame_type, dest, src, len_hi, len_lo] -> header CRC + let vectors: &[([u8; 5], u8)] = &[ + ([0x00, 0x00, 0x07, 0x00, 0x00], 0x37), + ([0x00, 0x07, 0x00, 0x00, 0x00], 0x40), + ([0x01, 0x00, 0x07, 0x00, 0x00], 0xB1), + ([0x01, 0x07, 0x00, 0x00, 0x00], 0xC6), + ]; + for (header, expected) in vectors { + assert_eq!(crc8(header), *expected, "header={header:02X?}"); + let mut with_crc = header.to_vec(); + with_crc.push(*expected); + assert!(crc8_valid(&with_crc)); + assert_eq!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); + } +} + +#[test] +fn crc8_one_bit_corruption_rejected() { + let header = [0x00, 0x00, 0x07, 0x00, 0x00]; + let mut with_crc = header.to_vec(); + with_crc.push(0x37); + assert!(crc8_valid(&with_crc)); + with_crc[0] ^= 0x01; + assert!(!crc8_valid(&with_crc)); + assert_ne!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); +} + +#[test] +fn crc16_clause9_data_vector_01_00() { + // Data 01 00 → CRC 0x169F, wire order LS first: 9F 16 + assert_eq!(crc16(&[0x01, 0x00]), 0x169F); + let with_crc = [0x01, 0x00, 0x9F, 0x16]; + assert!(crc16_valid(&with_crc)); + assert_eq!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); +} + +#[test] +fn crc16_one_bit_corruption_rejected() { + let mut with_crc = [0x01, 0x00, 0x9F, 0x16]; + assert!(crc16_valid(&with_crc)); + with_crc[0] ^= 0x01; + assert!(!crc16_valid(&with_crc)); + assert_ne!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); +} + +#[test] +fn crc8_validate_round_trip() { + let data = [0x05, 0xFF, 0x03, 0x00, 0x0C]; + let crc = crc8(&data); + let mut with_crc = data.to_vec(); + with_crc.push(crc); + assert!(crc8_valid(&with_crc)); + assert_eq!(crc8_accumulate_all(&with_crc), HEADER_CRC_RESIDUAL); +} + +#[test] +fn crc16_validate_round_trip() { + let data = vec![0xAA; 100]; + let crc = crc16(&data); + let mut with_crc = data; + with_crc.push(crc as u8); + with_crc.push((crc >> 8) as u8); + assert!(crc16_valid(&with_crc)); + assert_eq!(crc16_accumulate_all(&with_crc), DATA_CRC_RESIDUAL); +} + +#[test] +fn literal_token_frame_0_from_7() { + // Live trunk Token: dest BASRT(0) <- FEC(7), header CRC 0x37 + let wire: &[u8] = &[0x55, 0xFF, 0x00, 0x00, 0x07, 0x00, 0x00, 0x37]; + let (frame, consumed) = decode_frame(wire).expect("decode Token 0<-7"); + assert_eq!(consumed, 8); + assert_eq!(frame.frame_type, FrameType::Token); + assert_eq!(frame.destination, 0); + assert_eq!(frame.source, 7); + assert!(frame.data.is_empty()); + + let mut enc = BytesMut::new(); + encode_frame(&mut enc, &frame).unwrap(); + assert_eq!(&enc[..], wire); +} + +#[test] +fn literal_data_not_expecting_reply_frame() { + // Frame type 06, dest 0, src 7, len 2, HDR D9, data 01 00, DCRC 9F 16 + let wire: &[u8] = &[ + 0x55, 0xFF, 0x06, 0x00, 0x07, 0x00, 0x02, 0xD9, 0x01, 0x00, 0x9F, 0x16, + ]; + let (frame, consumed) = decode_frame(wire).expect("decode data frame"); + assert_eq!(consumed, 12); + assert_eq!(frame.frame_type, FrameType::BACnetDataNotExpectingReply); + assert_eq!(frame.destination, 0); + assert_eq!(frame.source, 7); + assert_eq!(&frame.data[..], &[0x01, 0x00]); + + let mut enc = BytesMut::new(); + encode_frame(&mut enc, &frame).unwrap(); + assert_eq!(&enc[..], wire); +} + +#[test] +fn truncated_header_need_more_or_err() { + let partial = [0x55, 0xFF, 0x00, 0x00, 0x07]; + assert!(decode_frame(&partial).is_err()); + assert_eq!(decode_frame_stream(&partial), StreamDecode::NeedMore); +} + +#[test] +fn truncated_data_crc_need_more() { + // Header complete for len=2 but missing data CRC octets + let partial = [0x55, 0xFF, 0x06, 0x00, 0x07, 0x00, 0x02, 0xD9, 0x01, 0x00]; + assert_eq!(decode_frame_stream(&partial), StreamDecode::NeedMore); + assert!(decode_frame(&partial).is_err()); +} + +// ----------------------------------------------------------------------- +// Frame encode/decode tests +// ----------------------------------------------------------------------- + +#[test] +fn token_frame_round_trip() { + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + // Token has no data, so: preamble(2) + header(5) + crc(1) = 8 + assert_eq!(buf.len(), 8); + assert_eq!(&buf[..2], &PREAMBLE); + + let (decoded, consumed) = decode_frame(&buf).unwrap(); + assert_eq!(consumed, 8); + assert_eq!(decoded, frame); +} + +#[test] +fn poll_for_master_round_trip() { + let frame = MstpFrame { + frame_type: FrameType::PollForMaster, + destination: 42, + source: 0, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + let (decoded, _) = decode_frame(&buf).unwrap(); + assert_eq!(decoded, frame); +} + +#[test] +fn data_expecting_reply_round_trip() { + let npdu = vec![0x01, 0x00, 0x10, 0x02, 0x03, 0x04, 0x05]; + let frame = MstpFrame { + frame_type: FrameType::BACnetDataExpectingReply, + destination: 5, + source: 0, + data: Bytes::from(npdu.clone()), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + // preamble(2) + header(5) + hcrc(1) + data(7) + dcrc(2) = 17 + assert_eq!(buf.len(), 17); + + let (decoded, consumed) = decode_frame(&buf).unwrap(); + assert_eq!(consumed, 17); + assert_eq!(decoded.frame_type, FrameType::BACnetDataExpectingReply); + assert_eq!(decoded.destination, 5); + assert_eq!(decoded.source, 0); + assert_eq!(decoded.data, npdu); +} + +#[test] +fn data_not_expecting_reply_round_trip() { + let npdu = vec![0x01, 0x20, 0xFF, 0xFF, 0x00, 0xFF, 0x10, 0x08]; + let frame = MstpFrame { + frame_type: FrameType::BACnetDataNotExpectingReply, + destination: BROADCAST_MAC, + source: 3, + data: Bytes::from(npdu.clone()), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + let (decoded, _) = decode_frame(&buf).unwrap(); + assert_eq!(decoded, frame); +} + +#[test] +fn broadcast_destination() { + let frame = MstpFrame { + frame_type: FrameType::BACnetDataNotExpectingReply, + destination: BROADCAST_MAC, + source: 10, + data: Bytes::from_static(&[0x01, 0x00]), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + let (decoded, _) = decode_frame(&buf).unwrap(); + assert_eq!(decoded.destination, BROADCAST_MAC); +} + +#[test] +fn reply_to_poll_for_master_round_trip() { + let frame = MstpFrame { + frame_type: FrameType::ReplyToPollForMaster, + destination: 0, + source: 42, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + let (decoded, _) = decode_frame(&buf).unwrap(); + assert_eq!(decoded, frame); +} + +#[test] +fn test_request_with_data_round_trip() { + let test_data = vec![0xDE, 0xAD, 0xBE, 0xEF]; + let frame = MstpFrame { + frame_type: FrameType::TestRequest, + destination: 5, + source: 0, + data: Bytes::from(test_data.clone()), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + let (decoded, _) = decode_frame(&buf).unwrap(); + assert_eq!(decoded.data, test_data); +} + +#[test] +fn decode_too_short() { + assert!(decode_frame(&[0x55, 0xFF, 0x00]).is_err()); +} + +#[test] +fn decode_bad_preamble() { + let data = [0x00, 0xFF, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00]; + assert!(decode_frame(&data).is_err()); +} + +#[test] +fn decode_bad_header_crc() { + let mut buf = BytesMut::new(); + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }; + encode_frame(&mut buf, &frame).unwrap(); + // Corrupt header CRC (byte 7) + buf[7] ^= 0xFF; + assert!(decode_frame(&buf).is_err()); +} + +#[test] +fn decode_bad_data_crc() { + let mut buf = BytesMut::new(); + let frame = MstpFrame { + frame_type: FrameType::BACnetDataNotExpectingReply, + destination: 1, + source: 0, + data: Bytes::from_static(&[0x01, 0x00]), + }; + encode_frame(&mut buf, &frame).unwrap(); + // Corrupt last byte (data CRC high) + let last = buf.len() - 1; + buf[last] ^= 0xFF; + assert!(decode_frame(&buf).is_err()); +} + +#[test] +fn decode_truncated_data() { + let mut buf = BytesMut::new(); + let frame = MstpFrame { + frame_type: FrameType::BACnetDataExpectingReply, + destination: 5, + source: 0, + data: Bytes::from_static(&[0x01, 0x02, 0x03, 0x04]), + }; + encode_frame(&mut buf, &frame).unwrap(); + // Truncate: remove data CRC + buf.truncate(buf.len() - 2); + assert!(decode_frame(&buf).is_err()); +} + +#[test] +fn find_preamble_at_start() { + let data = [0x55, 0xFF, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00]; + assert_eq!(find_preamble(&data), Some(0)); +} + +#[test] +fn find_preamble_with_garbage() { + let data = [0x00, 0x00, 0x12, 0x55, 0xFF, 0x00, 0x01, 0x00]; + assert_eq!(find_preamble(&data), Some(3)); +} + +#[test] +fn find_preamble_none() { + let data = [0x00, 0x55, 0x00, 0xFF, 0x01]; + assert_eq!(find_preamble(&data), None); +} + +#[test] +fn frame_type_round_trip() { + for raw in 0..=0x07 { + let ft = FrameType::from_raw(raw); + assert_eq!(ft.to_raw(), raw); + } + // Unknown type + let ft = FrameType::from_raw(0x42); + assert_eq!(ft.to_raw(), 0x42); + assert_eq!(ft, FrameType::Unknown(0x42)); +} + +#[test] +fn frame_type_has_data() { + assert!(!FrameType::Token.has_data()); + assert!(!FrameType::PollForMaster.has_data()); + assert!(!FrameType::ReplyToPollForMaster.has_data()); + assert!(FrameType::TestRequest.has_data()); + assert!(FrameType::TestResponse.has_data()); + assert!(FrameType::BACnetDataExpectingReply.has_data()); + assert!(FrameType::BACnetDataNotExpectingReply.has_data()); + assert!(!FrameType::ReplyPostponed.has_data()); +} + +#[test] +fn large_data_frame() { + // Near-maximum data size + let npdu = vec![0xAA; 1024]; + let frame = MstpFrame { + frame_type: FrameType::BACnetDataNotExpectingReply, + destination: BROADCAST_MAC, + source: 0, + data: Bytes::from(npdu.clone()), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + let (decoded, _) = decode_frame(&buf).unwrap(); + assert_eq!(decoded.data, npdu); +} + +#[test] +fn encode_oversized_data_returns_error() { + let frame = MstpFrame { + frame_type: FrameType::BACnetDataNotExpectingReply, + destination: 1, + source: 0, + data: Bytes::from_static(&[0xAA; MAX_MPDU_DATA + 1]), + }; + let mut buf = BytesMut::new(); + assert!(encode_frame(&mut buf, &frame).is_err()); +} + +#[test] +fn decode_rejects_source_above_max_master() { + // Encode a valid frame then patch the source to 128 (above MAX_MASTER=127) + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + // Patch source byte (offset 4) to 128 + buf[4] = 128; + // Recompute header CRC (bytes 2..7, CRC at byte 7) + let header_crc = crc8(&buf[2..7]); + buf[7] = header_crc; + + assert!(decode_frame(&buf).is_err()); +} + +#[test] +fn decode_rejects_broadcast_source() { + // Encode a valid frame then patch the source to BROADCAST_MAC (0xFF) + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + // Patch source byte to 0xFF + buf[4] = BROADCAST_MAC; + // Recompute header CRC + let header_crc = crc8(&buf[2..7]); + buf[7] = header_crc; + + assert!(decode_frame(&buf).is_err()); +} + +#[test] +fn decode_accepts_max_master_source() { + // Source = MAX_MASTER (127) should be valid + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: MAX_MASTER, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + + let (decoded, _) = decode_frame(&buf).unwrap(); + assert_eq!(decoded.source, MAX_MASTER); +} + +// ----------------------------------------------------------------------- +// Streaming decode tests +// ----------------------------------------------------------------------- + +fn encode_token_frame() -> Vec { + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + buf.to_vec() +} + +fn encode_data_frame() -> (MstpFrame, Vec) { + let frame = MstpFrame { + frame_type: FrameType::BACnetDataNotExpectingReply, + destination: 5, + source: 0, + data: Bytes::from_static(&[0x01, 0x02, 0x03, 0x04]), + }; + let mut buf = BytesMut::new(); + encode_frame(&mut buf, &frame).unwrap(); + (frame, buf.to_vec()) +} + +#[test] +fn stream_decode_token_complete() { + let wire = encode_token_frame(); + assert_eq!( + decode_frame_stream(&wire), + StreamDecode::Complete { + frame: MstpFrame { + frame_type: FrameType::Token, + destination: 1, + source: 0, + data: Bytes::new(), + }, + consumed: 8, + } + ); +} + +#[test] +fn stream_decode_split_header_then_body() { + let (_frame, wire) = encode_data_frame(); + let split = wire.len() - 3; + + assert_eq!(decode_frame_stream(&wire[..split]), StreamDecode::NeedMore); + + let mut assembled = wire[..split].to_vec(); + assembled.extend_from_slice(&wire[split..]); + let StreamDecode::Complete { frame, consumed } = decode_frame_stream(&assembled) else { + panic!("expected complete frame after reassembly"); + }; + assert_eq!(consumed, wire.len()); + assert_eq!(frame.data.len(), 4); +} + +#[test] +fn stream_decode_preamble_split_across_chunks() { + let wire = encode_token_frame(); + assert_eq!(decode_frame_stream(&wire[..1]), StreamDecode::NeedMore); + + let mut assembled = wire[..1].to_vec(); + assembled.extend_from_slice(&wire[1..]); + assert!(matches!( + decode_frame_stream(&assembled), + StreamDecode::Complete { .. } + )); +} + +#[test] +fn stream_decode_host_gap_simulation_without_clear() { + let wire = encode_token_frame(); + let mut buf = wire[..4].to_vec(); + assert_eq!(decode_frame_stream(&buf), StreamDecode::NeedMore); + + // Simulate a host gap far beyond wire T_frame_abort without clearing the buffer. + buf.extend_from_slice(&wire[4..]); + assert!(matches!( + decode_frame_stream(&buf), + StreamDecode::Complete { .. } + )); +} + +#[test] +fn retain_lone_preamble_byte_preserves_trailing_0x55() { + let mut buf = vec![0x00, 0x12, 0x55]; + retain_lone_preamble_byte(&mut buf); + assert_eq!(buf, vec![0x55]); + + let mut no_preamble = vec![0x00, 0x12, 0x34]; + retain_lone_preamble_byte(&mut no_preamble); + assert!(no_preamble.is_empty()); +} + +#[test] +fn stream_decode_bad_header_crc_invalid() { + let mut wire = encode_token_frame(); + wire[7] ^= 0xFF; + assert_eq!( + decode_frame_stream(&wire), + StreamDecode::Invalid { discard: 1 } + ); +} + +#[test] +fn stream_decode_bad_data_crc_invalid_discards_frame() { + let (_frame, mut wire) = encode_data_frame(); + let last = wire.len() - 1; + wire[last] ^= 0xFF; + assert_eq!( + decode_frame_stream(&wire), + StreamDecode::Invalid { + discard: wire.len() + } + ); +} + +#[test] +fn stream_decode_need_more_on_short_header() { + assert_eq!( + decode_frame_stream(&[0x55, 0xFF, 0x00, 0x01]), + StreamDecode::NeedMore + ); +} + +#[test] +fn stream_decode_invalid_on_bad_second_preamble_byte() { + assert_eq!( + decode_frame_stream(&[0x55, 0x00]), + StreamDecode::Invalid { discard: 1 } + ); +}