Skip to content

Implement support for BACnet MS/TP frame encoding/decoding.#11

Open
de-vri-es wants to merge 8 commits intoninjasource:mainfrom
de-vri-es:bacnet-mstp
Open

Implement support for BACnet MS/TP frame encoding/decoding.#11
de-vri-es wants to merge 8 commits intoninjasource:mainfrom
de-vri-es:bacnet-mstp

Conversation

@de-vri-es
Copy link
Copy Markdown
Contributor

@de-vri-es de-vri-es commented Apr 3, 2026

This PR makes a number of changes to support encoding and decoding MS/TP frames, to allow using the library with BACnet over RS-485.

In a nutshell:

  • It turns the device part of the source/destination address of an NPDU into an opaque blob of bytes, with some convenience conversion impls.
    • This is needed because connected BACnet networks may be using different link layer protocols, so even when you are a BACnet/IP device, you may receive a message from a BACnet MS/TP device on an RS-485 networks, and you may need to send a reply back to it. On the link layer, this reply goes to a BACnet router that forwarded the request to your device, which uses the NPDU data to forward the message to the correct device.
    • Note that this is kinda needed even without supporting MS/TP frame encoding/decoding.
  • It renames the existing BACnet/IP modules and types to disambiguate between BACnet/IP and MS/TP frames:
    • data_link module => ip
    • DataLink struct => IpFrame
    • DataLinkFunction enum => BvllFunction
    • There are still deprecated aliases in place for the old names.
  • It adds a network_protocol::mstp module with a MstpFrame struct and an MstpFrameType enum.

(Code written by hand, no AI used.)

@ninjasource
Copy link
Copy Markdown
Owner

ninjasource commented Apr 3, 2026

Amazing, thanks! I will only be able to look at this when I'm back from holiday on the 6th of April.

@de-vri-es
Copy link
Copy Markdown
Contributor Author

de-vri-es commented Apr 3, 2026

Sure, no rush! Enjoy your holiday :)

@de-vri-es de-vri-es force-pushed the bacnet-mstp branch 3 times, most recently from 1faebff to 3cc1ed0 Compare April 3, 2026 11:45
de-vri-es added 4 commits April 3, 2026 13:50
@de-vri-es
Make NPDU device addresses opaque byte blobs.
57b1b36 
You can not assume that the source/target network uses BACnet/IP, even
when you are a BACnet/IP device. Preserving the address as-is is
important to let BACnet routers function properly.
@de-vri-es
Return error when decoding NPDU with the wrong protocol version.
a0e82b4
@de-vri-es
Rename data_link module to ip and DataLink to IpFrame.
305329f 
And rename `DataLinkFunction` to `BvllFunction`.
@de-vri-es
Add MS/TP frame encoder/decoder.
abfb0d3
@de-vri-es
Copy link
Copy Markdown
Contributor Author

de-vri-es commented Apr 6, 2026

Also added support for COBS-encoded MS/TP frames now. For now, it relies on a patched version of corncobs, but I hope it will get merged: cbiffle/corncobs#5

And otherwise, the actual encode/decode functions used are easy to extract, and they're under a compatible license (Mozilla Public License 2.0). So they could be included in the implementation instead of depending on corncobs.

ninjasource
ninjasource reviewed Apr 12, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@ninjasource ninjasource left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at your main branch and that looks good to me. Maybe a smaller change than this the one first and then with corncobs later when your PR gets merged? Otherwise we can just keep this one open until that dependency is sorted.

Comment thread src/network_protocol/network_pdu.rs
@@ -207,7 +207,10 @@ impl<'a> NetworkPdu<'a> {
#[cfg_attr(feature = "alloc", bacnet_macros::remove_lifetimes_from_fn_args)]
pub fn decode(reader: &mut Reader, buf: &'a [u8]) -> Result<Self, Error> {
// ignore version
Copy link
Copy Markdown
Owner

@ninjasource ninjasource Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, you can remove this comment now.

Comment thread Cargo.toml
bacnet-macros = { path = "./bacnet-macros", version = "0.1.0" }

# https://github.com/cbiffle/corncobs/pull/5
corncobs = { git = "https://github.com/de-vri-es/corncobs", rev = "68025686e0547fac483baabb63c8066065023d88" }
Copy link
Copy Markdown
Owner

@ninjasource ninjasource Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be a crates.io dependency rather

@de-vri-es
Copy link
Copy Markdown
Contributor Author

de-vri-es commented Apr 13, 2026

I took a look at your main branch and that looks good to me. Maybe a smaller change than this the one first and then with corncobs later when your PR gets merged? Otherwise we can just keep this one open until that dependency is sorted.

How would you feel about moving the in-place COBS encoder into this crate if the corncobs maintainer is unresponsive? Thanks to the corncobs crate is has received extensive testing and fuzzing already, so I feel pretty confident that it is correct.

Either way, I prefer to wait a bit before removing COBS support from the PR, if that's no problem for you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ninjasource ninjasource ninjasource left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@de-vri-es @ninjasource @LeandroMarceddu