Derive register length from its payload class

[bruno-f-cruz]

length and payload_class both describe the extent of a register payload but were declared independently, so a register with a multi-element payload reported no length and was then rejected by its own frames. length is now derived from payload_class.

The register that could not read itself

RegisterBase.parse sizes the payload from the dtype. HarpMessage.decode sizes it from length:

expected = (decoder.length or 1) * np.dtype(decoder.payload_type.value).itemsize

Two routes to one quantity, so a register declaring no length parses a frame fine and is rejected by decode:

DeviceName.length                                  # None
DeviceName.payload_class.payload_dtype.itemsize    # 25

HarpMessage.parse(DeviceName.format("Behavior")).decode(DeviceName)
# HarpParseError: DeviceName reads 1 payload bytes but this message carries 25.

Device.read and Device.write both end in msg.decode(register), so this broke every multi-element register: the struct-typed settings registers on devices such as Quac, which #36 reports, and DeviceName, which every Harp device carries.

Giving the two a shared payload_size on the register was rejected. The extent is a property of the dtype, not of the register declaration, so restating it there is the shape of the bug rather than its fix, a second copy drifting the moment a payload changes without it. parse keeps reading the dtype and length is derived from it, so decode cannot disagree. The checks stay distinct either way, parse asking whether a buffer holds at least one payload and decode whether a frame holds exactly one.

What the extent is measured in

_derive_length divides payload_dtype.itemsize by the width of one payload_type element. That width comes from payload_type, not the payload's own _elem_dtype, since payload_type is what decode measures the frame with and the two need not agree:

AnalogData.payload_type          # PayloadType.S16, so 2 bytes an element
AnalogDataPayload._elem_dtype    # uint8
# 6 bytes is 3 S16 elements, which decode() needs, and 6 uint8 ones, which it does not

A payload that is not a whole number of elements raises rather than truncating.

The declared length

A declared length is checked against the payload but not trusted, and the emitter passes reg.length through so a schema declaration is checked too (this means if a schema is wrong, an error will be raised).

Tests

Fourteen, each regression confirmed to fail with the derivation disabled. test_register.py covers the derivation, the unit it measures in, one-element arrays and their subclasses, a declared 1 normalizing away, and the two rejections. test_emit.py covers the same through a message, the Device.read and Device.write path.

Important note on length=1 => None

@glopesdev

The current schema upstream defines length as not required (however this is different from having a default). The corresponding model in this python client defines length with a default = 1..

This is unfortunate as it prevents us from distinguishing between someone who wrongly adds "1" explicitly (and gets replaced with "None" silently) and someone that genuanly does not define a value and rather has it inferred. I think this should probably be revisit if/when we merge the spec for the variable-length registers. Until then we have two choices:

• Maintain the inference behavior whereas length=1 is converted silently to None
• Change the default in the model to be "None" so we can distinguish the two cases.

Closes #36.

[bruno-f-cruz]

Closed in favor of #44.

Bottom line: after discussion, @glopesdev and I decided to drop length instead of deriving it.