Implement implicit field size alignment #77
Conversation
pavel-kirienko
changed the title
This is a tentative fix. A proper long-term solution is described in #68.
pavel-kirienko
changed the title
|
@thirtytwobits the build is green and I think it's ready for a review. |
| @@ -6,18 +6,18 @@ | |||
| # | |||
| # The motivation for the difference between this small-MTU case and the general case is that the full 128-bit | |||
| # unique-ID can't be accommodated in a small-MTU single-frame anonymous message transfer. The solution is to replace | |||
| # the full 128-bit ID with a smaller 55-bit hash of it, and remove support for preferred node-ID value in the | |||
| # the full 128-bit ID with a smaller 48-bit hash of it, and remove support for preferred node-ID value in the | |||
There was a problem hiding this comment.
So we're increasing the chances of collisions just to ensure we can use the length property of the variable length array as a "is present" flag. Why not just add a bool has_valid_id flag and make the id a plan int type. You can keep your 55-bit hash that way.
There was a problem hiding this comment.
But remember, the message shall fit into a single-frame transfer over Classic CAN. With the variable-length array, the bit length set is {56, 72}. The case of 56 bits is used with request transfers so it works. If we remove the variable-length array, we'd have to squeeze both cases into a single-frame transfer, which would leave even fewer bits for the unique ID hash.
With 48-bit, assuming uniformly distributed hashes, the probability of collision for a network with 128 participants is still negligible.
CI will be failing until OpenCyphal/pydsdl#33 is released.
Context: https://forum.uavcan.org/t/proposed-change-to-variable-length-arrays/740