Skip to content

fix(mstp): Clause 9 CRC, USB stream assembly, and 9.5.6 token/PFM - #467

Merged
jscott3201 merged 8 commits into
jscott3201:devfrom
bbartling:pr/mstp-clause9-interop
Aug 30, 2026
Merged

fix(mstp): Clause 9 CRC, USB stream assembly, and 9.5.6 token/PFM#467
jscott3201 merged 8 commits into
jscott3201:devfrom
bbartling:pr/mstp-clause9-interop

Conversation

@bbartling

Copy link
Copy Markdown
Contributor

Summary

  • Clause 9.6 CRCs — replace incorrect reflected polys (0xE0 / Modbus 0xA001) with BACnet header 0x81 and data 0x8408, plus live golden vectors (Token 0←7 CRC 0x37).
  • USB host gaps ≠ T_frame_abort — async serial reads arrive in multi-ms chunks; treating inter-read gaps as wire T_frame_abort (~1.56 ms @ 38400) discarded partial frames. Stream decoder (Complete / NeedMore / Invalid) + host stale-partial timeout.
  • Clause 9.5.6 token/PFMDONE_WITH_TOKEN no longer reverse-scans from NS+1 or emits Token TS→TS; maintenance PS advances TS→NS only; FindNewSuccessor via PollForMaster. Regressions A–E.
  • Split frame tests into mstp_frame_tests.rs for the 700 LOC file-size gate.

Lab evidence (not claimed as formal conformance)

Validated on a live MS/TP trunk (BASRT MAC 0 + JCI FEC MAC 7 + Waveshare USB RS-485 @ 38400) with JENEsys Workbench: passive token decode, FEC ReadProperty coexistence, and a Rust mini-device (MAC 3) discovered with points Polled {ok}. Details in bbartling playground vibe13 docs.

Limitations (current)

  • Not a conformance claim — Clause 9 checklist items beyond CRC / stream assembly / 9.5.6 master token+PFM are still open.
  • No extended frames — types 32/33, COBS, CRC-32K not implemented; do not claim oversized-frame or router conformance.
  • Gates still open in lab — shared endpoint + soak (vibe13 Gates 5–6) not closed.
  • Host USB timing — stale-partial timeout is host-scheduling aware; wire T_frame_abort is not emulated on chunk boundaries.
  • Windows CI flake — unrelated max_retry_budget 3s wall-clock flake is not in this PR (tracked on fork main only).

Test plan

  • cargo test -p bacnet-transport --features serial
  • Confirm CI green (fmt/clippy/audit/deny/ubuntu/macos/windows)
  • Review regressions in clause956_tests.rs and CRC goldens in mstp_frame_tests.rs

Made with Cursor

bbartling and others added 5 commits August 30, 2026 17:32
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 <cursoragent@cursor.com>
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 <cursoragent@cursor.com>
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 <cursoragent@cursor.com>
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 <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
bbartling added a commit to bbartling/rusty-bacnet that referenced this pull request Aug 30, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@bbartling

Copy link
Copy Markdown
Contributor Author

Follow-up Python surface (separate PR): MS/TP bindings + mstp_mini_device.py — will link once created.

@bbartling

Copy link
Copy Markdown
Contributor Author

Python follow-up (bindings + example, separate from this stack PR): #468 — please merge this MS/TP transport PR first for live trunk coexistence.

@bbartling

Copy link
Copy Markdown
Contributor Author

Note from lab / contributor (bbartling)

This PR closes the live MS/TP interoperability blockers we hit on a real trunk (Clause 9.6 CRCs, USB chunk ≠ T_frame_abort, Clause 9.5.6 DONE_WITH_TOKEN / PFM). With it, a standard-frame MS/TP master device is discoverable in Workbench alongside commercial masters.

Still outstanding on the rusty-bacnet stack side (not solved by this PR — please treat as follow-up work):

  1. APDU segmentation over MS/TP — client TSM has segment machinery, but we are not claiming end-to-end segmented confirmed services on MS/TP. Phase-2 style devices should keep advertising / fitting in a 480-octet Max_APDU_Length_Accepted (standard-frame safe).
  2. Extended MS/TP frames — types 32/33, COBS, CRC-32K are still missing. Do not claim oversized-frame or 135-2020 router conformance until those exist with independent goldens + cross-impl traffic.
  3. Outbound size enforcement — until extended frames land, keep capping outbound MS/TP APDU/NPDU so oversized standard frames cannot be emitted (transport already reports max_apdu_length() == 480).
  4. Broader Clause 9 timing matrix — not fully evidenced at every baud (T_frame_gap, turnaround, etc.); baud sweeps / soak remain lab follow-ups, not blockers for this PR.

Happy to help on follow-up PRs for (1)–(3) once this lands. The Python MS/TP surface is separate: #468 (depends on this).

@jscott3201

Copy link
Copy Markdown
Owner

Review cycle 1 — one major repair required

DF-467-001 (major): oversized outbound NPDUs are rejected too late. The new 501-byte codec cap is correct, but send_unicast / send_broadcast can still queue 502–1497-byte NPDUs; use_token then removes them before encode_frame fails, so the transport only logs and silently loses the queued item. The application-reply path similarly clears reply ownership before a late encode failure.

Required bounded repair: validate the supported standard-frame payload limit at queue/reply ingress before destructive state mutation, return or otherwise surface a deterministic failure, and prove oversized values produce no wire output while preserving queue/state invariants. Keep extended frames and segmentation deferred.

The holistic review otherwise passed; exact-head CI was green.

@jscott3201
jscott3201 merged commit 1d0ea37 into jscott3201:dev Aug 30, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants