dronecan / DSDL

DSDL Protocol Description files for DroneCAN
MIT License
22 stars 60 forks source link

Variable-length array fields: Make length a single-byte #28

Closed David-OConnor closed 1 year ago

David-OConnor commented 1 year ago

Currently, variable-arrange arrays are difficult to implement and debug. This is due to them having bit-level alignment, and being skipped in some cases per the DSDL spec, eg when a combination of tail array optimization requirements are met per this: https://dronecan.github.io/Specification/3._Data_structure_description_language/. This can change the alignment of the rest of the message, to save between 1 and 7 bits of data. IMO not worth it. Of note, it appears that in some cases like GetNodeInfo responses, tail array optimization is used in legacy CAN packets, but not FD packets, but reasons I'm not clear about.

Recommended fix: Either to be applied to new message types, or with a deprecation to the existing format; I recognize that the former is more realistic. All variable length arrays start with a single-byte length field.

Example of how, in an FD frame, PyDroneCAN (and presumably the spec as is) implements this name length, printed from PyDroneCAN:

name=ArrayValue(type=saturated uint8[<=80], items=[111, 114, 103, 46, 97, 110, 121, 108, 101, 97, 102, 46, 103, 110, 115, 115]))

This is what that section of the message looks like over the wire in FD mode, due to the present alignment handling. Note that every word is bit-shifted by one.

[32, 222, 228, 206, 92, 194, 220, 242, 216, 202, 194, 204, 92, 206, 220, 230, 230]

In legacy mode, the data is passed byte for byte over the wire, ie 111, 114, 103 etc.

In this case, 32 is the name length of 16, bit shifted left by 1. 222 is the first name byte, shifted left by one; etc.

tridge commented 1 year ago

the TAO was indeed a mistake, and TAO is removed with CANFD, but we cannot remove TAO on bxCAN, and certainly don't want different rules for each DSDL file, sorry