chore(python/lorawan): bump device-payload-schema and raise the catalog pin - #94
Open
reissjason wants to merge 4 commits into
Open
Conversation
Every catalog test is parametrized over the generated TDs, so a catalog that shrinks does not fail anything - it runs fewer cases and still reports success. The existing "is not empty" check does not help: one TD satisfies it as happily as two hundred. That matters because the schema submodule will be bumped. A schema rewritten to use a construct the converter does not support stops converting, and nothing here would say so. Measured against the pin as it stands today: bumping to the current device-payload-schema master takes the catalog from 157 TDs to 81, because 76 device schemas gained computed fields or nested objects. The suite stays green throughout. EXPECTED_CATALOG_SIZE is asserted in both directions. A drop names how many schemas stopped converting and points at the skip report; a rise asks for the number to be raised in the same commit, so new coverage is locked in rather than absorbed silently. Either way the number moves by hand, in the commit responsible, where a reviewer sees the cost. Verified in both directions: simulating the submodule bump fails with "device catalog shrank: 81 TDs, expected 157. 76 device schema(s) stopped converting", and an artificially low expectation fails with the grew message.
Three changes to what the binding accepts, and one that stops it emitting a Thing
Description that decodes to the wrong number.
`sensor:` is an annotation naming the channel's sensor, like `semantic` and `ipso`
beside it, and is decoding-neutral - so it joins the ignorable keys. It alone was
excluding 42 device schemas.
`value:` is not a derived value. The reference interpreter ignores it when decoding:
it reads the byte and reports what the payload held, verified against 0x99 arriving
where the schema says 0x11. It fixes the byte only when *encoding*, so the field is an
ordinary scalar carrying `lorav:const`, which the reverse conversion restores rather
than dropping. Every downlink command in the library declares its category and command
bytes that way - another 45 schemas.
`transform` on a wire field is post-processing, not derivation, and this is the change
that matters. Both directions treated any field carrying a derived-value descriptor as
derived, so `air_temperature` - a `u16` with `transform: [{div: 100}, {add: -327.68}]` -
had its wire type replaced by the derived marker and became a zero-byte field. The
generated TD decoded without it entirely, and where a `transform` did survive the field
reported its raw value: 65535 rather than 327.67. A field is derived only when it has
nothing to read, so the rule is now the absence of a readable wire type, in the forward
converter, the reverse converter and `_reject_computed` alike. `transform` rides on a
scalar form and is restored on the way back.
Measured against the corpus this was developed on (device-payload-schema master, 52
commits ahead of the pin here): 81 schemas converted before, 166 now. More to the
point, 95 of the tests failed there before - decode parity, silently wrong output -
and none do now.
The last of those 95 needed a guard rather than a fix. qingping derives its temperature
from `$_temp_raw`, an internal 12-bit range inside a 3-byte byte_group. The forms are
emitted correctly, but rebuilding flattens the group into top-level bit-range fields,
and the reference interpreter decodes a top-level `_`-prefixed field without storing it
as a variable - so the reference resolves to nothing and the TD reported -50 degrees
where the schema says 359.5. A derived field reading a masked internal input is now
skipped: a TD that decodes to the wrong number is worse than one that does not exist,
and the catalog size cannot detect the difference. Plain internal scalars are
unaffected, which matters - rejecting internal references by name alone cost 22 schemas
to prevent this one.
No change at the pin this branch carries: still 157 generated, 574 passed.
…og pin Moves the submodule from 0701777 to 1174141, 54 commits of language work, and raises EXPECTED_CATALOG_SIZE from 157 to 166 in the same commit - which is what the pin is for. The number could not drift here: the suite failed with "device catalog grew: 166 TDs, expected 157" until it was updated deliberately. The corpus goes from 158 device schemas to 218 and the catalog from 157 Thing Descriptions to 166, so this is a coverage gain rather than the loss it would have been a few commits ago. Bumping before the converter learned to keep a wire field carrying a `transform` gave 81 TDs and 95 failing tests, most of them decode parity - Thing Descriptions that were silently wrong rather than missing. Both prerequisites are in this branch's history. 52 schemas still fall outside the convertible subset, led by 37 using `bitfield_string`, which needs a form term rather than a classification fix. 706 passed, 55 skipped.
egekorkan
reviewed
Aug 14, 2026
egekorkan
left a comment
egekorkan
left a comment
Member
There was a problem hiding this comment.
@Hao-Siemens please have a look too
| PRESENCE_BIT: Final = "lorav:presenceBit" # flags bit gating this property | ||
| SWITCH_FIELD: Final = "lorav:switchField" # name of the discriminator property | ||
| SWITCH_VALUE: Final = "lorav:switchValue" # discriminator value selecting this case | ||
| CONST: Final = "lorav:const" # fixed byte value used when encoding a downlink |
Member
There was a problem hiding this comment.
this should not be in the forms though
Contributor
|
Thanks for the PR with the catalog size updating. The same as the PR #93, athis point, this PR is stacked on previous vocab commits, so merging it directly would mix two incompatible directions and make review/bisect harder. See the changes we are planning in #96 I would suggest to supersede this PR for now and I will manually update the cateloge size and device-payload-schema pinning in the current branch. |
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves the submodule from 0701777 to 1174141, 54 commits of language work, and raises
EXPECTED_CATALOG_SIZE from 157 to 166 in the same commit - which is what the pin is for.
The number could not drift here: the suite failed with "device catalog grew: 166 TDs,
expected 157" until it was updated deliberately.
The corpus goes from 158 device schemas to 218 and the catalog from 157 Thing
Descriptions to 166, so this is a coverage gain rather than the loss it would have been
a few commits ago. Bumping before the converter learned to keep a wire field carrying a
transformgave 81 TDs and 95 failing tests, most of them decode parity - ThingDescriptions that were silently wrong rather than missing. Both prerequisites are in
this branch's history.
52 schemas still fall outside the convertible subset, led by 37 using
bitfield_string, which needs a form term rather than a classification fix.706 passed, 55 skipped.
Stacks on #92 and #93. Both are in this branch's history, so the diff here shows their commits too — that is not duplication. Merge #92 and #93 first and this reduces to the submodule move plus one changed number; or merge this alone and it brings all three.
Without #93 the same bump gives 81 Thing Descriptions and 95 failing tests, most of them decode parity — TDs that were silently wrong rather than missing.