Homepage GitHub

PyDSDL/Nunavut Union offsets seem to be incorrect

I’m trying to implement a template for libcanard (in plain C), and I’m running into troubles implementing UnionTypes. There seem to be two parts to this: the UnionType itself does not seem to set the tag bit length correctly, and then that incorrect value is being given to whatever calculates the offset.

From pydsdl._serializable._composite.UnionType, line 335.

tag_bit_length = 2 ** math.ceil(math.log2(max(8, unaligned_tag_bit_length)))

Which seems to just be calculating the bit length and then dumping it back into number of fields. To me this doesn’t seem to be correct behaviour, but I don’t know the code well enough to say that definitively (which is why I’m not posting this as an issue). I also can’t find any other reference to the tag bit length elsewhere in pydsdl.

If this code is wrong, I believe max(8 and the 2 ** should be removed from that line. Otherwise, I’m not sure how I’m supposed to implement the tag bit width.

Here’s a direct snippet from my UnionType.j2 template:

{%- for field, offset in T.iterate_fields_with_offsets() %}
  case ({{T.full_name | c.macrofy}}_{{field.name | c.macrofy}}_TAG):
    {{ encode.set_size(field, offset) | indent(width=2) }}
    break;
{%- endfor %}

And here is it’s output:

case (SPEAR_DRIVE_DRIVECOMMAND_THING_TAG):
    canardDSDLSetUxx(buf, offset + {8}, data->thing, 8);
    size += 8;
    break;
  case (SPEAR_DRIVE_DRIVECOMMAND_ID_TAG):
    size += uavcan_node_ID_encode(buf, offset + {8}, data->id);
    break;

The offset, according to the spec, should be 1 (instead of 8, as it is right now), as that is the bit length required to represent the two data types.

From this DSDL:

@union
uint8 thing
uavcan.node.ID.1.0 id

The objective of the line of code you referenced is to ensure that the tag length is rounded up to the nearest one of {8, 16, 32, 64} bits. The rationale is explained in Proposed change to variable-length arrays. Unfortunately, the Specification is currently outdated but we will be looking into that shortly (tracking issue: https://github.com/UAVCAN/specification/issues/75).

In the example you provided, the correct tag length is 8 bits. I’m sorry that the specification misguided you but our ability to keep things well-maintained is dependent on the availability of external contributors. If you could help us resolve any of the issues related to the existing implementations documented on GitHub, we would have more time to keep the specification up to date.

Ah, makes sense. Thank you for clarifying.