Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Fixed len size on FD #29

Open
David-OConnor opened this issue Jun 1, 2023 · 1 comment
Open

Proposal: Fixed len size on FD #29

David-OConnor opened this issue Jun 1, 2023 · 1 comment

Comments

@David-OConnor
Copy link

David-OConnor commented Jun 1, 2023

Proposal: In FD mode, All variable-length arrays are prepended with a 1-byte length field

Current behavior: They're prepended with a bit-aligned field that changes length depending on maximum array length.

Why:
Variable-length arrays are common in DroneCAN messages, including fundamental types defined in uavcan.protocol. These introduce bit-level alignment. Bit-level alignment, vice byte is not user-friendly to develop for for these reasons:

  • Computer registers are byte-aligned
  • Programming languages like C and Rust have good byte-level APIs, but poor or non-existant bit-level APIs
  • ICs and wire protocols (other than DC's) almost always use byte-level alignment
  • Common message view protocols, like in DroneCAN GUI, print bytes.

Overall, implementing and debugging byte-aligned protocols is much easier than bit-aligned ones.

Bit-level alignment can save bandwidth in some cases, but often these arrays use byte-aligned variables, like uint8.

This doesn't come up in many cases in non-FD mode, since variable-length arrays are often placed at the end of payloads. In non-FD Dronecan, the len field is removed in this cases due to tail optimization. In FD mode, these len fields introduce bit-level alignment, even when the message is otherwise byte-aligned.

Is this a breaking change?

Maybe. I don't think that means we should dismiss it; rather now is a good time to implement this. Here's why:

  • Few devices currently support FD mode. This is likely to change in the near future
  • Ardupilot currently doesn't support FD mode in official releases; this will change in the next release if I understand correctly
  • FD mode's implementation currently is already a breaking change by removing tail optimization
  • The Dronecan spec page currently doesn't discuss FD implementation details like the removed TCO

For basic/BX CAN, I don't think we should make this change, because it would break many devices, and is not worth forcing the change. For FD, I think this is an ideal time to do it, with a narrowing window.

@tridge
Copy link
Member

tridge commented Jun 2, 2023

I'd like @bugobliterator to comment as well, but I don't think we should be changing the CANFD format now. It is already working well and well tested

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

No branches or pull requests

2 participants