Skip to content

Fix decoding of multi-element register payloads - #44

Merged
glopesdev merged 3 commits into
mainfrom
fix-register-length
Aug 25, 2026
Merged

Fix decoding of multi-element register payloads#44
glopesdev merged 3 commits into
mainfrom
fix-register-length

Conversation

@glopesdev

@glopesdev glopesdev commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

HarpMessage.decode sized the payload from the register length while RegisterBase.parse sized it from payload_class. Only array registers declare a length, so for every other kind the first calculation assumed a single element and rejected any longer frame. Device.read and Device.write both end in decode, so this broke every register whose payload spans more than one element, DeviceName on every Harp device included.

decode now reads payload_class.payload_dtype.itemsize, the same quantity parse uses, so there is one derivation and the two cannot disagree. PayloadDecoder declares payload_class in place of length, since every register has a payload class and only an array register has a length.

What a declared length does

Two further defects surfaced in the same area, neither of them filed.

An array register can be declared two ways, by calling a base with an address and a length, or by subclassing with both in the class body. Only the first sized the payload, so the second would inherit a one-element payload and read the first element while discarding the rest. device.olfactometer and device.soundcard both carry registers in that shape. Sizing moved into the metaclass __init__, which runs for either form, and re-declaring a length on a subclass now raises rather than nesting one sub-array inside another.

The schema model defaulted length to 1, so an absent length and a declared length: 1 arrived as the same value and both emitted a scalar. Both model fields now default to None with a minimum of 1, and the emitter branches on absence, so a declared 1 emits a one-element array. A length: 0 is rejected for a register and for a payload member, which the generator reads as absent.

RegisterBase.length keeps its type and its meaning, and no generated output changes. The emitter was checked against generator output for a schema covering an absent length, a declared 1, a declared 3 and struct payloads at both counts, comparing base class, payload type, length, dtype, field names and address, and decoding a frame built by one path through the other.

Where length lives

RegisterBase no longer declares length. It moves to the array register metaclass, so an array register reports its element count and every other kind carries none.

This follows from the decode change above. Once the payload class is the only thing that decides an extent, a length on a scalar or struct register is always None while the payload spans several elements, which invites exactly the mistake this pull request fixes.

Closes #36

@glopesdev
glopesdev requested a review from bruno-f-cruz August 24, 2026 13:24
@glopesdev glopesdev added the fix Pull request that fixes an issue label Aug 24, 2026
decode now sizes the payload from payload_class rather than from the
register length, so a register whose payload spans several elements no
longer rejects its own frames. Only an array register declares a length,
so this reached every struct, string and mask register, including
DeviceName.

An array register declaring length in its class body now sizes its
payload, where before only the call form did, so it would read the
first element and discard the rest. A declared length of 1 is now
distinct from an absent one, so such a register emits a one-element
array rather than a scalar.

Closes #36
length is no longer a member of every register. It moves to the array
register metaclass, so an array register reports its element count and
every other kind carries no length at all.

@bruno-f-cruz bruno-f-cruz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small clarification.

Comment thread src/packages/harp-device/src/harp/device/schema/_emit.py Outdated
ConverterContext.length is now the
effective element count and never zero, so span is a plain multiply and
no emitted payload or converter changes.
@glopesdev
glopesdev requested a review from bruno-f-cruz August 24, 2026 20:02
@glopesdev
glopesdev merged commit fec46b0 into main Aug 25, 2026
14 checks passed
@glopesdev
glopesdev deleted the fix-register-length branch August 25, 2026 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull request that fixes an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_build_register should be updated to work with registers with length > 1

2 participants