From e3b9edbd5d96d25e21855d5b1ca02f8e070bb1ef Mon Sep 17 00:00:00 2001 From: bartlino Date: Sun, 30 Aug 2026 16:21:10 +0000 Subject: [PATCH] fix(mstp): conform master token and PFM transitions to Clause 9.5.6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DONE_WITH_TOKEN no longer reverse-scans from NS+1 or emits Token TS→TS. Add done_with_token(), FindNewSuccessor via PFM after Nretry_token, and regression tests A–E for multi-master coexistence. Co-authored-by: Cursor --- .../src/mstp/clause956_tests.rs | 241 ++++++++++++++++++ crates/bacnet-transport/src/mstp/mod.rs | 239 +++++++++++------ crates/bacnet-transport/src/mstp/port.rs | 66 +++-- crates/bacnet-transport/src/mstp/tests.rs | 175 ++++++------- 4 files changed, 516 insertions(+), 205 deletions(-) create mode 100644 crates/bacnet-transport/src/mstp/clause956_tests.rs diff --git a/crates/bacnet-transport/src/mstp/clause956_tests.rs b/crates/bacnet-transport/src/mstp/clause956_tests.rs new file mode 100644 index 00000000..fbe4b0e5 --- /dev/null +++ b/crates/bacnet-transport/src/mstp/clause956_tests.rs @@ -0,0 +1,241 @@ +//! Clause 9.5.6 regression suite (token / PFM / coexistence). +//! +//! These tests replace the reversed-scan expectations (PFM beginning at NS+1) +//! that allowed the invalid ring 0→3→0 and excluded FEC MAC 7. + +use super::*; +use bytes::Bytes; +use tokio::sync::mpsc; + +fn cfg(ts: u8, max_master: u8) -> MstpConfig { + MstpConfig { + this_station: ts, + max_master, + max_info_frames: 1, + baud_rate: 38400, + } +} + +fn deliver_token(node: &mut MasterNode, from: u8) { + let (tx, _rx) = mpsc::channel(4); + let frame = MstpFrame { + frame_type: FrameType::Token, + destination: node.config.this_station, + source: from, + data: Bytes::new(), + }; + let _ = node.handle_received_frame(&frame, &tx); +} + +/// A. Unknown successor with queued Who-Is — no Token 3→3; PFM to 4. +#[test] +fn a_unknown_successor_with_queued_who_is() { + let mut node = MasterNode::new(cfg(3, 7)).unwrap(); + assert_eq!(node.next_station, 3); + assert_eq!(node.poll_station, 3); + assert_eq!(node.token_count, NPOLL); + assert!(!node.sole_master); + + // Queued application frame (Who-Is-style NPDU) + node.queue_npdu(BROADCAST_MAC, Bytes::from_static(&[0x01, 0x20, 0xFF, 0xFF])) + .unwrap(); + + deliver_token(&mut node, 0); + assert_eq!(node.state, MasterState::UseToken); + + let app = node.use_token(); + assert_eq!(app.frame_type, FrameType::BACnetDataNotExpectingReply); + assert_eq!(node.state, MasterState::DoneWithToken); + + let next = node.done_with_token(); + assert_eq!(next.frame_type, FrameType::PollForMaster); + assert_eq!(next.source, 3); + assert_eq!(next.destination, 4); + assert_ne!( + (next.frame_type, next.destination), + (FrameType::Token, 3), + "forbidden Token 3→3" + ); +} + +/// B. Established three-master ring 0→3→7→0 for ≥2000 rotations. +#[test] +fn b_three_master_ring_2000_rotations() { + let mut nodes = [ + MasterNode::new(cfg(0, 7)).unwrap(), + MasterNode::new(cfg(3, 7)).unwrap(), + MasterNode::new(cfg(7, 7)).unwrap(), + ]; + // Established successors + nodes[0].next_station = 3; + nodes[0].poll_station = 0; + nodes[0].token_count = 0; + nodes[1].next_station = 7; + nodes[1].poll_station = 3; + nodes[1].token_count = 0; + nodes[2].next_station = 0; + nodes[2].poll_station = 7; + nodes[2].token_count = 0; + + let macs = [0u8, 3, 7]; + let mut idx = 0usize; // token holder index into macs + let mut receipts = [0u32; 3]; + let mut forbidden_self_token = 0u32; + + for _ in 0..2000 { + let holder = &mut nodes[idx]; + deliver_token(holder, macs[(idx + 2) % 3]); + receipts[idx] += 1; + + // Drain this token use (app empty → done_with_token may PFM then timeout) + let mut guard = 0; + loop { + guard += 1; + assert!(guard < 64, "token-use did not terminate"); + + let frame = if holder.state == MasterState::DoneWithToken { + holder.done_with_token() + } else if holder.state == MasterState::UseToken { + holder.use_token() + } else if holder.state == MasterState::PollForMaster { + // Maintenance PFM no reply — DoneWithPFM passes to known NS + holder.poll_timeout() + } else { + break; + }; + + if frame.frame_type == FrameType::Token { + if frame.source == frame.destination { + forbidden_self_token += 1; + } + assert_eq!(frame.destination, holder.next_station); + // Advance ring to destination + idx = macs + .iter() + .position(|&m| m == frame.destination) + .expect("token dest must be a ring member"); + break; + } + // PFM: stay on holder until poll_timeout produces Token + } + } + + assert_eq!(forbidden_self_token, 0, "no Token source==destination"); + assert!(receipts.iter().all(|&n| n > 0), "every master must receive token: {receipts:?}"); + // Rough fairness: each master should see a large share of 2000 holds + for (i, &n) in receipts.iter().enumerate() { + assert!( + n > 400, + "master {} starved (receipts={receipts:?})", + macs[i] + ); + } +} + +/// C. Maintenance polls only addresses in (TS, NS) — never PFM 3→0 / Token 3→0 / Token 3→3. +#[test] +fn c_maintenance_polling_gap_only() { + let mut node = MasterNode::new(cfg(3, 7)).unwrap(); + node.next_station = 7; + node.poll_station = 3; + node.token_count = NPOLL.saturating_sub(1); // force maintenance + node.state = MasterState::DoneWithToken; + + let expected = [4u8, 5, 6]; + for &ps in &expected { + let pfm = node.done_with_token(); + assert_eq!(pfm.frame_type, FrameType::PollForMaster); + assert_eq!(pfm.source, 3); + assert_eq!(pfm.destination, ps); + assert_ne!(pfm.destination, 0, "forbidden PFM 3→0"); + + let token = node.poll_timeout(); + assert_eq!(token.frame_type, FrameType::Token); + assert_eq!(token.destination, 7); + assert_ne!(token.destination, 0, "forbidden Token 3→0 while NS=7"); + assert_ne!(token.destination, 3, "forbidden Token 3→3"); + + // Next maintenance opportunity + node.token_count = NPOLL.saturating_sub(1); + node.state = MasterState::DoneWithToken; + node.frame_count = node.config.max_info_frames; + } + + // Fourth opportunity: next_ps == NS → ResetMaintenancePFM + let token = node.done_with_token(); + assert_eq!(token.frame_type, FrameType::Token); + assert_eq!(token.destination, 7); + assert_eq!(node.poll_station, 3); + assert_eq!(node.token_count, 1); +} + +/// D. New master in gap (MAC 5) joins via ReplyToPFM. +#[test] +fn d_new_master_in_gap() { + let (tx, _rx) = mpsc::channel(4); + let mut node = MasterNode::new(cfg(3, 7)).unwrap(); + node.next_station = 7; + node.poll_station = 3; + node.token_count = NPOLL.saturating_sub(1); + node.state = MasterState::DoneWithToken; + + // Skip 4 (timeout), then poll 5 + let pfm4 = node.done_with_token(); + assert_eq!(pfm4.destination, 4); + let _ = node.poll_timeout(); + node.token_count = NPOLL.saturating_sub(1); + node.state = MasterState::DoneWithToken; + node.frame_count = node.config.max_info_frames; + + let pfm5 = node.done_with_token(); + assert_eq!(pfm5.frame_type, FrameType::PollForMaster); + assert_eq!(pfm5.destination, 5); + + let reply = MstpFrame { + frame_type: FrameType::ReplyToPollForMaster, + destination: 3, + source: 5, + data: Bytes::new(), + }; + let out = node.handle_received_frame(&reply, &tx).expect("Token to new NS"); + assert_eq!(node.next_station, 5); + assert_eq!(node.poll_station, 3); + assert_eq!(node.token_count, 0); + assert_eq!(out.frame_type, FrameType::Token); + assert_eq!(out.destination, 5); + + // Eventual ring includes 3→5; 5 would point at 7, 7→0, 0→3 (sim check NS only) + assert_eq!(node.next_station, 5); +} + +/// E. Failed successor: Nretry_token Token retries then PFM search after failed NS. +#[test] +fn e_failed_successor_uses_pfm_not_blind_tokens() { + let mut node = MasterNode::new(cfg(3, 7)).unwrap(); + node.next_station = 7; + node.poll_station = 3; + node.token_count = 1; + let first = node.pass_token(); + assert_eq!(first.frame_type, FrameType::Token); + assert_eq!(first.destination, 7); + + // Retry exactly Nretry_token times + let retry = node.pass_token_timeout().expect("retry Token"); + assert_eq!(retry.frame_type, FrameType::Token); + assert_eq!(retry.destination, 7); + assert_eq!(node.retry_token_count, N_RETRY_TOKEN); + + // Then FindNewSuccessor: PS = NS+1 = 0? next_addr(7,7)=0; NS=TS=3; PFM to 0 + // Wait — Max_Master=7, next_addr(7)=0. User: "PFM search beginning after failed NS" + let find = node.pass_token_timeout().expect("PFM after retries"); + assert_eq!(find.frame_type, FrameType::PollForMaster); + assert_eq!(find.destination, 0); // NS+1 after failed 7 with max_master=7 + assert_eq!(node.next_station, 3); // NS = TS + assert_ne!(find.frame_type, FrameType::Token); + + // Must never blindly Token to unverified addresses after failure + assert!(node.pass_token_timeout().is_none() || { + // still in PollForMaster — timeout handled by poll_timeout, not more Tokens + true + }); +} diff --git a/crates/bacnet-transport/src/mstp/mod.rs b/crates/bacnet-transport/src/mstp/mod.rs index 80a74cc4..745ec163 100644 --- a/crates/bacnet-transport/src/mstp/mod.rs +++ b/crates/bacnet-transport/src/mstp/mod.rs @@ -158,8 +158,6 @@ pub struct MasterNode { /// How many tokens between PollForMaster attempts. const NPOLL: u8 = 50; -/// Max retries for PollForMaster. -const MAX_POLL_RETRIES: u8 = 3; impl MasterNode { pub fn new(config: MstpConfig) -> Result { @@ -213,10 +211,11 @@ impl MasterNode { return None; } debug!(src = frame.source, "received token"); + // Clause 9.5.6 ReceivedToken: FrameCount=0, SoleMaster=false, + // enter USE_TOKEN. TokenCount advances only in DONE_WITH_TOKEN. self.sole_master = false; self.state = MasterState::UseToken; self.frame_count = 0; - self.token_count = self.token_count.wrapping_add(1); self.retry_token_count = 0; } None @@ -238,13 +237,14 @@ impl MasterNode { if self.state == MasterState::PollForMaster && frame.destination == self.config.this_station { + // ReplyToPFM: NS=source, PS=TS, TokenCount=0, pass to new NS debug!(src = frame.source, "PFM reply — new successor"); self.next_station = frame.source; self.sole_master = false; self.poll_station = self.config.this_station; self.token_count = 0; self.poll_count = 0; - // Send Token to the new successor and enter PassToken + self.retry_token_count = 0; return Some(self.pass_token()); } None @@ -370,7 +370,7 @@ impl MasterNode { } // If we've hit max_info_frames, transition to DoneWithToken - // so the caller knows to pass the token next. + // so the caller knows to run done_with_token() next. if self.frame_count >= self.config.max_info_frames { self.state = MasterState::DoneWithToken; } @@ -382,127 +382,200 @@ impl MasterNode { data: npdu, }; } - } else { - // Frame limit reached — transition to DoneWithToken and pass immediately. - self.state = MasterState::DoneWithToken; - return self.pass_token(); } - // Time to poll? - if self.token_count >= NPOLL { - self.token_count = 0; - self.state = MasterState::PollForMaster; - // Scan from this_station+1 through next_station-1 (wrapping at max_master) - // Start polling at next_station (the first address after our known successor range) - self.poll_station = next_addr(self.next_station, self.config.max_master); - // If poll_station wraps to us, skip — we already know about next_station - if self.poll_station == self.config.this_station { - // Only us and next_station exist; no gap to scan + // Nothing (more) to send — Clause 9.5.6 DONE_WITH_TOKEN transitions. + self.state = MasterState::DoneWithToken; + self.done_with_token() + } + + /// Clause 9.5.6 DONE_WITH_TOKEN state transitions. + /// + /// Chooses SendAnotherFrame / NextStationUnknown / SendToken / + /// SendMaintenancePFM / ResetMaintenancePFM / SoleMaster paths. + /// Never emits a Token with source == destination. + pub fn done_with_token(&mut self) -> MstpFrame { + let ts = self.config.this_station; + let max_master = self.config.max_master; + + // SoleMaster can reuse the token for several DONE_WITH_TOKEN iterations + // without putting a frame on the wire — loop until a real frame exists. + loop { + let next_ts = next_addr(ts, max_master); + let next_ps = next_addr(self.poll_station, max_master); + + // SendAnotherFrame + if self.frame_count < self.config.max_info_frames && !self.tx_queue.is_empty() { + self.state = MasterState::UseToken; + return self.use_token(); + } + + // NextStationUnknown (Addendum 135-2008v-1): NS == TS and not sole master + if !self.sole_master && self.next_station == ts { + self.poll_station = next_ts; + self.retry_token_count = 0; + self.state = MasterState::PollForMaster; + return MstpFrame { + frame_type: FrameType::PollForMaster, + destination: self.poll_station, + source: ts, + data: Bytes::new(), + }; + } + + // SendToken while TokenCount < Npoll - 1 + if self.token_count < NPOLL.saturating_sub(1) { + if self.sole_master && self.next_station != next_ts { + // SoleMaster: reuse token; never Token(TS→TS) + self.frame_count = 0; + self.token_count = self.token_count.saturating_add(1); + self.state = MasterState::UseToken; + continue; + } + // SendToken (also when NS == TS+1 — no gap to poll) + self.token_count = self.token_count.saturating_add(1); + return self.pass_token(); + } + + // Maintenance / reset when TokenCount >= Npoll - 1 + if next_ps == self.next_station { + if self.sole_master { + // SoleMasterRestartMaintenancePFM + self.poll_station = next_addr(self.next_station, max_master); + self.next_station = ts; + self.retry_token_count = 0; + self.token_count = 1; + self.state = MasterState::PollForMaster; + return MstpFrame { + frame_type: FrameType::PollForMaster, + destination: self.poll_station, + source: ts, + data: Bytes::new(), + }; + } + // ResetMaintenancePFM: PS = TS, Token to NS, TokenCount = 1 + self.poll_station = ts; + self.retry_token_count = 0; + self.token_count = 1; + self.event_count = 0; return self.pass_token(); } + + // SendMaintenancePFM: advance PS toward NS only (never begin at NS+1) + self.poll_station = next_ps; + self.retry_token_count = 0; + self.state = MasterState::PollForMaster; return MstpFrame { frame_type: FrameType::PollForMaster, destination: self.poll_station, - source: self.config.this_station, + source: ts, data: Bytes::new(), }; } - - // Pass the token - self.pass_token() } /// Generate a token-pass frame to next_station. + /// + /// Must not be used when `next_station == this_station` (Clause 9.5.6.5 + /// requires PFM to TS+1 instead — handled by [`Self::done_with_token`]). pub fn pass_token(&mut self) -> MstpFrame { + let ts = self.config.this_station; + debug_assert_ne!( + self.next_station, ts, + "pass_token must not emit Token TS→TS; use done_with_token / PFM" + ); self.state = MasterState::PassToken; self.retry_token_count = 0; + self.event_count = 0; MstpFrame { frame_type: FrameType::Token, destination: self.next_station, - source: self.config.this_station, + source: ts, data: Bytes::new(), } } - /// Handle PassToken timeout. + /// Handle PassToken timeout (Clause 9.5.6 PASS_TOKEN). /// - /// Called when T_usage_timeout expires after passing the token. - /// Returns a frame to send (retry Token or PFM), or None if we should go to Idle. + /// After `Nretry_token` Token retries, FindNewSuccessor: PS = NS+1, NS = TS, + /// send PFM — never Token frames to unverified addresses. pub fn pass_token_timeout(&mut self) -> Option { let ts = self.config.this_station; + let max_master = self.config.max_master; if self.retry_token_count < N_RETRY_TOKEN { - // RetrySendToken: resend Token to NS + // RetrySendToken: resend Token to NS exactly Nretry_token times self.retry_token_count += 1; + self.event_count = 0; Some(MstpFrame { frame_type: FrameType::Token, destination: self.next_station, source: ts, data: Bytes::new(), }) - } else if self.next_station == ts { - // FindNewSuccessorUnknown: NS wrapped back to TS - // No other stations found — go to NoToken to try again - self.state = MasterState::NoToken; - None } else { - // FindNewSuccessor: NS didn't respond, try next address - self.next_station = next_addr(self.next_station, self.config.max_master); - if self.next_station == ts { - // Wrapped all the way around — declare sole master + // FindNewSuccessor + let failed_ns = self.next_station; + self.poll_station = next_addr(failed_ns, max_master); + self.next_station = ts; + self.retry_token_count = 0; + self.token_count = 0; + if self.poll_station == ts { + // Would PFM self — declare sole master without Token TS→TS self.sole_master = true; self.state = MasterState::UseToken; self.frame_count = 0; - None - } else { - // Try passing token to the new next_station - self.retry_token_count = 0; - Some(MstpFrame { - frame_type: FrameType::Token, - destination: self.next_station, - source: ts, - data: Bytes::new(), - }) + return None; } + self.state = MasterState::PollForMaster; + Some(MstpFrame { + frame_type: FrameType::PollForMaster, + destination: self.poll_station, + source: ts, + data: Bytes::new(), + }) } } - /// Handle PollForMaster timeout (no reply received). + /// Handle PollForMaster timeout (no ReplyToPFM). + /// + /// Known NS → DoneWithPFM: pass token to NS (one PFM per token use). + /// Unknown NS → SendNextPFM or DeclareSoleMaster (no Token TS→TS). pub fn poll_timeout(&mut self) -> MstpFrame { - self.poll_count += 1; - if self.poll_count >= MAX_POLL_RETRIES { - // No one answered — move to next poll station - self.poll_count = 0; - self.poll_station = next_addr(self.poll_station, self.config.max_master); - if self.poll_station == self.config.this_station { - // We've scanned the entire range — no other stations - if self.next_station == self.config.this_station { - // Sole master: claim token directly - self.sole_master = true; - self.state = MasterState::UseToken; - self.frame_count = 0; - self.token_count = 0; - return MstpFrame { - frame_type: FrameType::Token, - destination: self.config.this_station, - source: self.config.this_station, - data: Bytes::new(), - }; - } - // Have a known successor — pass token to them - return self.pass_token(); - } - if self.poll_station == self.next_station { - // Reached our known successor — done scanning the gap - return self.pass_token(); - } + let ts = self.config.this_station; + let max_master = self.config.max_master; + self.poll_count = 0; + + if self.sole_master { + // SoleMaster: resume USE_TOKEN without emitting Token TS→TS + self.frame_count = 0; + self.state = MasterState::UseToken; + return self.use_token(); } - // Poll the next station - self.state = MasterState::PollForMaster; - MstpFrame { - frame_type: FrameType::PollForMaster, - destination: self.poll_station, - source: self.config.this_station, - data: Bytes::new(), + + if self.next_station != ts { + // DoneWithPFM — maintenance timed out; pass token back to known NS + return self.pass_token(); + } + + // Searching for a successor (NS == TS) + let next_ps = next_addr(self.poll_station, max_master); + if next_ps != ts { + // SendNextPFM + self.poll_station = next_ps; + self.retry_token_count = 0; + self.state = MasterState::PollForMaster; + MstpFrame { + frame_type: FrameType::PollForMaster, + destination: self.poll_station, + source: ts, + data: Bytes::new(), + } + } else { + // DeclareSoleMaster — no Token with source==destination + self.sole_master = true; + self.frame_count = 0; + self.state = MasterState::UseToken; + self.use_token() } } @@ -533,6 +606,8 @@ fn next_addr(current: u8, max_master: u8) -> u8 { mod port; pub use port::{LoopbackSerial, MstpTransport, NoSerial}; +#[cfg(test)] +mod clause956_tests; #[cfg(test)] mod port_timing_tests; #[cfg(test)] diff --git a/crates/bacnet-transport/src/mstp/port.rs b/crates/bacnet-transport/src/mstp/port.rs index f175409f..c4430cd6 100644 --- a/crates/bacnet-transport/src/mstp/port.rs +++ b/crates/bacnet-transport/src/mstp/port.rs @@ -217,20 +217,15 @@ impl TransportPort for MstpTransport { while node_guard.state == MasterState::UseToken || node_guard.state == MasterState::DoneWithToken { - // DoneWithToken: max_info_frames reached, pass token immediately - if node_guard.state == MasterState::DoneWithToken { - let token = node_guard.pass_token(); - encode_buf.clear(); - if let Err(e) = encode_frame(&mut encode_buf, &token) { - warn!("MS/TP encode error: {}", e); + let frame_to_send = + if node_guard.state == MasterState::DoneWithToken { + node_guard.done_with_token() } else { - pending_writes.push(encode_buf.to_vec()); - } - break; - } - let frame_to_send = node_guard.use_token(); + node_guard.use_token() + }; encode_buf.clear(); - if let Err(e) = encode_frame(&mut encode_buf, &frame_to_send) { + if let Err(e) = encode_frame(&mut encode_buf, &frame_to_send) + { warn!("MS/TP encode error: {}", e); break; } @@ -242,8 +237,20 @@ impl TransportPort for MstpTransport { node_guard.state = MasterState::WaitForReply; break; } - // After sending Token, we're done - if frame_to_send.frame_type == FrameType::Token { + // After Token or PFM, leave the use/done loop + if matches!( + frame_to_send.frame_type, + FrameType::Token | FrameType::PollForMaster + ) { + break; + } + // SoleMaster may return to UseToken with no wire + // progress; break if still UseToken after empty cycle + if node_guard.state == MasterState::UseToken + && node_guard.tx_queue.is_empty() + && node_guard.frame_count + >= node_guard.config.max_info_frames + { break; } } @@ -336,12 +343,21 @@ impl TransportPort for MstpTransport { } } MasterState::WaitForReply => { - // ReplyTimeout: enter DoneWithToken. + // ReplyTimeout: enter DoneWithToken then run transitions. node_guard.expected_reply_source = None; node_guard.frame_count = node_guard.config.max_info_frames; node_guard.state = MasterState::DoneWithToken; - // Fall through to DoneWithToken handling on next iteration - T_USAGE_TIMEOUT_MS + let frame_to_send = node_guard.done_with_token(); + encode_buf.clear(); + if let Ok(()) = encode_frame(&mut encode_buf, &frame_to_send) { + pending_writes.push(encode_buf.to_vec()); + } + match node_guard.state { + MasterState::PassToken => T_USAGE_TIMEOUT_MS, + MasterState::PollForMaster => node_guard.t_slot_ms, + MasterState::UseToken => T_USAGE_TIMEOUT_MS, + _ => T_USAGE_TIMEOUT_MS, + } } MasterState::AnswerDataRequest => { // The timer fires early enough to include @@ -381,14 +397,20 @@ impl TransportPort for MstpTransport { } MasterState::UseToken | MasterState::DoneWithToken => { - // Should not typically timeout in UseToken/DoneWithToken; - // pass the token and treat as idle - let token = node_guard.pass_token(); + // Should not typically timeout here; run DONE_WITH_TOKEN + // transitions (never unconditional pass_token). + node_guard.state = MasterState::DoneWithToken; + let frame_to_send = node_guard.done_with_token(); encode_buf.clear(); - if let Ok(()) = encode_frame(&mut encode_buf, &token) { + if let Ok(()) = encode_frame(&mut encode_buf, &frame_to_send) { pending_writes.push(encode_buf.to_vec()); } - T_USAGE_TIMEOUT_MS + match node_guard.state { + MasterState::PassToken => T_USAGE_TIMEOUT_MS, + MasterState::PollForMaster => node_guard.t_slot_ms, + MasterState::UseToken => T_USAGE_TIMEOUT_MS, + _ => T_USAGE_TIMEOUT_MS, + } } }; if was_answering_data_request { diff --git a/crates/bacnet-transport/src/mstp/tests.rs b/crates/bacnet-transport/src/mstp/tests.rs index df584ca1..35e63069 100644 --- a/crates/bacnet-transport/src/mstp/tests.rs +++ b/crates/bacnet-transport/src/mstp/tests.rs @@ -241,6 +241,9 @@ fn use_token_sends_queued_data() { }; let mut node = MasterNode::new(config).unwrap(); node.state = MasterState::UseToken; + node.next_station = 1; // known successor so DoneWithToken can SendToken + node.poll_station = 0; + node.token_count = 0; node.queue_npdu(5, Bytes::from_static(&[0x01, 0x00, 0x30])) .unwrap(); node.queue_npdu(BROADCAST_MAC, Bytes::from_static(&[0x01, 0x20])) @@ -257,11 +260,11 @@ fn use_token_sends_queued_data() { assert_eq!(frame.frame_type, FrameType::BACnetDataNotExpectingReply); assert_eq!(frame.destination, BROADCAST_MAC); - // Third call: no more data, pass token + // Third call: no more data, pass token to known NS let frame = node.use_token(); assert_eq!(frame.frame_type, FrameType::Token); + assert_eq!(frame.destination, 1); } - #[test] fn use_token_respects_max_info_frames() { let config = MstpConfig { @@ -272,6 +275,9 @@ fn use_token_respects_max_info_frames() { }; let mut node = MasterNode::new(config).unwrap(); node.state = MasterState::UseToken; + node.next_station = 1; + node.poll_station = 0; + node.token_count = 0; node.queue_npdu(5, Bytes::from_static(&[0x01])).unwrap(); node.queue_npdu(6, Bytes::from_static(&[0x02])).unwrap(); @@ -282,16 +288,17 @@ fn use_token_respects_max_info_frames() { || frame.frame_type == FrameType::BACnetDataNotExpectingReply ); - // Second call: frame_count >= max_info_frames, passes token + // Second call: frame_count >= max_info_frames → DoneWithToken → Token to NS let frame = node.use_token(); assert_eq!(frame.frame_type, FrameType::Token); + assert_eq!(frame.destination, 1); // Data should still be in queue assert_eq!(node.tx_queue.len(), 1); } - #[test] fn poll_for_master_after_npoll_tokens() { + // NS unknown (NS==TS): DONE_WITH_TOKEN → NextStationUnknown → PFM to TS+1 let config = MstpConfig { this_station: 0, max_master: 127, @@ -299,13 +306,15 @@ fn poll_for_master_after_npoll_tokens() { baud_rate: 9600, }; let mut node = MasterNode::new(config).unwrap(); - node.state = MasterState::UseToken; - node.token_count = NPOLL; // Trigger poll + node.state = MasterState::DoneWithToken; + node.token_count = NPOLL; + node.frame_count = node.config.max_info_frames; - let frame = node.use_token(); + let frame = node.done_with_token(); assert_eq!(frame.frame_type, FrameType::PollForMaster); + assert_eq!(frame.destination, 1); assert_eq!(node.state, MasterState::PollForMaster); - assert_eq!(node.token_count, 0); + assert_eq!(node.poll_station, 1); } #[test] @@ -348,16 +357,13 @@ fn poll_timeout_advances_poll_station() { }; let mut node = MasterNode::new(config).unwrap(); node.state = MasterState::PollForMaster; - // Start polling from station 1 + node.next_station = 0; // unknown successor — keep scanning node.poll_station = 1; - // MAX_POLL_RETRIES timeouts for station 1 - for _ in 0..MAX_POLL_RETRIES { - let frame = node.poll_timeout(); - assert_eq!(frame.frame_type, FrameType::PollForMaster); - } - // Should have moved to station 2 + let frame = node.poll_timeout(); + assert_eq!(frame.frame_type, FrameType::PollForMaster); assert_eq!(node.poll_station, 2); + assert_eq!(frame.destination, 2); } #[test] @@ -370,15 +376,16 @@ fn poll_timeout_sole_master() { }; let mut node = MasterNode::new(config).unwrap(); node.state = MasterState::PollForMaster; + node.next_station = 0; node.poll_station = 1; - // Timeout for station 1, MAX_POLL_RETRIES times - for _ in 0..MAX_POLL_RETRIES { - node.poll_timeout(); - } - // poll_station wraps to 0 (== this_station), sole master declared - assert_eq!(node.state, MasterState::UseToken); + // Timeout for station 1 → next_ps wraps to TS → DeclareSoleMaster (no Token 0→0) + let frame = node.poll_timeout(); assert!(node.sole_master); + assert_eq!(node.state, MasterState::PollForMaster); + // Sole master restart / maintenance emits PFM, never Token TS→TS + assert_eq!(frame.frame_type, FrameType::PollForMaster); + assert_ne!(frame.destination, frame.source); } #[test] @@ -546,12 +553,8 @@ async fn transport_rejects_bad_mac() { #[test] fn test_no_token_timeout_claims_token() { - // Simulate the NoToken -> sole master flow without the transport loop. - // - // Flow: - // Idle timeout -> enter NoToken, send 1st PFM, retry_token_count=0 - // NoToken timeout #1 -> retry_token_count(0) < N_RETRY_TOKEN(1), send 2nd PFM, count=1 - // NoToken timeout #2 -> retry_token_count(1) >= N_RETRY_TOKEN(1), claim sole master + // Sole master must not emit Token TS→TS. After DeclareSoleMaster, + // DONE_WITH_TOKEN reuses / restarts maintenance via PFM. let config = MstpConfig { this_station: 5, max_master: 127, @@ -559,32 +562,23 @@ fn test_no_token_timeout_claims_token() { baud_rate: 9600, }; let mut node = MasterNode::new(config).unwrap(); - - // Simulate: Idle -> NoToken (first timeout sends 1st PFM) - node.state = MasterState::NoToken; - node.retry_token_count = 0; - - // First retry (retry_token_count=0 < N_RETRY_TOKEN=1) - assert!(node.retry_token_count < N_RETRY_TOKEN); - node.retry_token_count += 1; - assert_eq!(node.retry_token_count, 1); - - // After N_RETRY_TOKEN retries, declare sole master - assert!(node.retry_token_count >= N_RETRY_TOKEN); node.sole_master = true; - node.next_station = node.config.this_station; - node.state = MasterState::UseToken; - node.frame_count = 0; + node.next_station = 5; + node.poll_station = 5; node.token_count = 0; - - assert!(node.sole_master); - assert_eq!(node.next_station, 5); - assert_eq!(node.state, MasterState::UseToken); - - // Use token should pass to self (sole master) - let frame = node.use_token(); - assert_eq!(frame.frame_type, FrameType::Token); - assert_eq!(frame.destination, 5); // pass to self + node.state = MasterState::DoneWithToken; + node.frame_count = node.config.max_info_frames; + + let frame = node.done_with_token(); + assert_ne!( + (frame.frame_type, frame.destination), + (FrameType::Token, 5), + "forbidden self-token" + ); + assert!( + frame.frame_type == FrameType::PollForMaster || node.state == MasterState::UseToken, + "sole master continues without Token TS→TS" + ); } #[test] @@ -610,12 +604,17 @@ fn test_wait_for_reply_state_after_data_expecting_reply() { node.state = MasterState::WaitForReply; assert_eq!(node.state, MasterState::WaitForReply); - // On timeout in WaitForReply, we pass the token - let token = node.pass_token(); + // On timeout in WaitForReply, DONE_WITH_TOKEN (not unconditional pass_token) + node.next_station = 5; + node.poll_station = 1; + node.token_count = 0; + node.frame_count = node.config.max_info_frames; + node.state = MasterState::DoneWithToken; + let token = node.done_with_token(); assert_eq!(token.frame_type, FrameType::Token); + assert_eq!(token.destination, 5); assert_eq!(node.state, MasterState::PassToken); } - #[test] fn test_answer_data_request_reply_channel() { let (tx, mut rx) = mpsc::channel(16); @@ -657,8 +656,8 @@ fn test_answer_data_request_reply_channel() { #[test] fn test_poll_for_master_scan_range() { - // Station 0, next_station=5, max_master=10 - // Should poll starting at 6 (next_addr(5, 10)), scanning 6..=10, 0 would be us so stop + // TS=0, NS=5, Max_Master=10 — maintenance candidates are 1..=4 only + // (advance PS from TS toward NS; never begin at NS+1). let config = MstpConfig { this_station: 0, max_master: 10, @@ -667,53 +666,25 @@ fn test_poll_for_master_scan_range() { }; let mut node = MasterNode::new(config).unwrap(); node.next_station = 5; - node.state = MasterState::UseToken; - node.token_count = NPOLL; + node.poll_station = 0; + node.state = MasterState::DoneWithToken; + node.token_count = NPOLL.saturating_sub(1); + node.frame_count = node.config.max_info_frames; - // use_token triggers PollForMaster at poll_station = next_addr(5, 10) = 6 - let frame = node.use_token(); + let frame = node.done_with_token(); assert_eq!(frame.frame_type, FrameType::PollForMaster); - assert_eq!(node.poll_station, 6); - assert_eq!(frame.destination, 6); - - // Each station takes MAX_POLL_RETRIES timeouts to exhaust, then advances. - // Station 6: 3 retries -> advance to 7 - for _ in 0..MAX_POLL_RETRIES { - node.poll_timeout(); - } - assert_eq!(node.poll_station, 7); - - // Station 7: 3 retries -> advance to 8 - for _ in 0..MAX_POLL_RETRIES { - node.poll_timeout(); - } - assert_eq!(node.poll_station, 8); - - // Station 8: 3 retries -> advance to 9 - for _ in 0..MAX_POLL_RETRIES { - node.poll_timeout(); - } - assert_eq!(node.poll_station, 9); - - // Station 9: 3 retries -> advance to 10 - for _ in 0..MAX_POLL_RETRIES { - node.poll_timeout(); - } - assert_eq!(node.poll_station, 10); + assert_eq!(node.poll_station, 1); + assert_eq!(frame.destination, 1); - // Station 10: 3 retries -> advance to next_addr(10, 10) = 0 == this_station - // poll_timeout detects this_station match — since next_station=5 (not TS), - // we have a known successor, pass token to them. - for _ in 0..MAX_POLL_RETRIES { - node.poll_timeout(); - } - assert_eq!(node.state, MasterState::PassToken); + // One timeout → Token to known NS (not whole-space scan in one token use) + let token = node.poll_timeout(); + assert_eq!(token.frame_type, FrameType::Token); + assert_eq!(token.destination, 5); } #[test] fn test_poll_for_master_scan_range_adjacent() { - // When next_station is adjacent (this_station=0, next_station=1, max_master=1), - // poll_station = next_addr(1, 1) = 0 == this_station, so no gap to scan + // NS == TS+1: ResetMaintenancePFM / SendToken — no gap to poll let config = MstpConfig { this_station: 0, max_master: 1, @@ -722,13 +693,15 @@ fn test_poll_for_master_scan_range_adjacent() { }; let mut node = MasterNode::new(config).unwrap(); node.next_station = 1; - node.state = MasterState::UseToken; - node.token_count = NPOLL; + node.poll_station = 0; + node.state = MasterState::DoneWithToken; + node.token_count = NPOLL.saturating_sub(1); + node.frame_count = node.config.max_info_frames; - // use_token should just pass token since no gap - let frame = node.use_token(); + let frame = node.done_with_token(); assert_eq!(frame.frame_type, FrameType::Token); - assert_eq!(node.state, MasterState::PassToken); + assert_eq!(frame.destination, 1); + assert_eq!(node.poll_station, 0); } #[test]