FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix decoding of multi-element register payloads by glopesdev · Pull Request #44 · harp-tech/python · GitHub

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 commented Aug 24, 2026
edited
Loading

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 requested a review from bruno-f-cruz August 24, 2026 13:24
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 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

One small clarification.

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 requested a review from bruno-f-cruz August 24, 2026 20:02
glopesdev merged commit fec46b0 into main Aug 25, 2026
14 checks passed
glopesdev deleted the fix-register-length branch August 25, 2026 07:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL