OpenCyphal-Garage / cyphal.rs

Please use Canadensis by Sam Crow instead
https://github.com/samcrow/canadensis
Apache License 2.0
44 stars 5 forks source link

[WIP] Add protocol version 1 #60

Closed kjetilkjeka closed 6 years ago

kjetilkjeka commented 6 years ago

Fixes #56 Fixes #58

pavel-kirienko commented 6 years ago

Make it possible to opt out of tail array optimization when serializing/deserializing.

Why do you want to control that for deserialization? According to the deprecation plan we worked out in the mailing list, while decoding, implementations should always try non-TAO mode first, and then apply TAO only if they run out of data. So the opt-out choice should affect serialization only, shouldn't it?

kjetilkjeka commented 6 years ago

What I meant was making it possible to (de)serialize a data structure without attempting TAO. Earlier the (de)serializer have always done TAO. Now the (de)framer can attempt both.

I edited the text to make this clearer.

According to the deprecation plan we worked out in the mailing list, while decoding, implementations should always try non-TAO mode first, and then apply TAO only if they run out of data

I assumed that with protocol version number we wouldn't need to try both. We would know from the protocol version number which one we should do?

pavel-kirienko commented 6 years ago

I assumed that with protocol version number we wouldn't need to try both. We would know from the protocol version number which one we should do?

If you're referring to the version bit in the CAN ID field, that won't work just yet, because there are fielded implementations that use it as a priority bit. Although in the case of uavcan.rs it might make sense to drop support for the pre-release versions of the protocol and start with UAVCAN v1.0.

kjetilkjeka commented 6 years ago

If you're referring to the version bit in the CAN ID field, that won't work just yet, because there are fielded implementations that use it as a priority bit. Although in the case of uavcan.rs it might make sense to drop support for the pre-release versions of the protocol and start with UAVCAN v1.0.

Since uavcan.rs is not released in any sensible form it doesn't have the risk of breaking anything just by updating to the newest version of the library. This is why i think it makes sense to not do the complete try to deserialize twice dance.

My plan was to do something in between. Support the old protocol version as long as it sends the correct version bit. This way, if nodes with uavcan.rs is going to communicate with older nodes, the older nodes will have to be patched to a 4 bit priority scheme. I think this is acceptable.

pavel-kirienko commented 6 years ago

Yes, seems sensible, thank you for the clarification.

kjetilkjeka commented 6 years ago

Due to change of plans this must be done in a different way,