search

ninjasource / embedded-bacnet

A library to read and write bacnet packets for embedded devices
Apache License 2.0
12 stars 2 forks source link

Include reference to raw child packet in DataLink and NPDU. #5

Open mnbbrown opened 2 months ago

mnbbrown commented 2 months ago

Why?

My app implements a state machine to model confirmed requests. To support segmented requests this state machine accumulates the first APDU, and then for every following segment, just the "data".

When all segments have been recieved I combine them all (APDU + data + data) and send them to the application for decoding (and use).

I'd like to use the same state machine for non segmented APDUs.

Ideally I'd be able to easily get a slice of the buffer that represents the APDU during reading, but given ApplicationPdu is implemented as an enum this isn't currently possible.

What?

I've updated NetworkPdu and DataLink to include references to their raw payloads when decoding.

ninjasource commented 2 months ago

I've given this some thought and I believe that the design you have chosen can and should be pushed into your networking layer rather. The decode function on the DataLink takes a Reader which is already a cursor that tells you how many bytes you have consumed out of a buffer. I think that polluting the DataLink with this raw_payload slice is therefore redundant but more-so, it opens the floodgates for it being used to link BACnet packets together which is not it's job (I expect that even more fields will have to be added). I think the real issue is that the DataLink is not an owned data structure which makes it difficult to reference from another data structure. Something that would need to be done if you are to link segments together.

I have actually been working on adding the alloc feature to this library so that DataLink and its children can be owned (no lifetimes). This should make your life a lot easier. However, I am trying to limit the duplication of code as much as possible so I have been experimenting with writing a macro for it (which I am not a big fan of). As a result the codebase is going to be in flux as I play around with different approaches.