Feature/converter fixes - cherry pick PR #93 and #94 - #97
Open
Hao-Siemens wants to merge 7 commits into
Open
Conversation
Three places turned a source `length` or lookup key into an integer with a bare `int()`. Each raises ValueError on input the schema library is allowed to write, and nothing catches it, so one unconvertible device stopped the whole catalog from being generated rather than being reported as a skip. Neither input appears in the currently pinned schema library, which is why this has held so far. Both appear in the next one, where the batch script exits with a traceback and writes no Thing Descriptions at all. `length: remaining` (PS-014) is now converted rather than skipped. It looked unrepresentable because a form carries a byte count, but the reference interpreter resolves the keyword and any negative length identically, and lorav:byteLength already documents -1 as that sentinel with `minimum: -1`. The keyword normalises to the number on the way back: both decode the same, the numeric spelling is what the library already writes elsewhere, and it is the one the vocabulary can hold, so downstream sees one spelling rather than two. A `$variable` length is still skipped, now with a reason - no implementation of the specification supports it, so there is nothing to convert to. A lookup table with a non-integer key is skipped with SkipReason.ENUM_TABLE. These are `default` labels covering every value the table does not list (PS-269); lorav:valueMap pairs one wire value with one decoded value and cannot say "anything else", so the field would lose meaning silently. A field consuming the rest of the payload counts as zero bytes wide, in both the cursor that assigns byte offsets and the one that computes `consume`. Its true width is known only at decode time, and a field that eats the remainder has nothing after it to place. The catalog is unchanged at 157 devices from 158 schemas: on the pinned library these paths are unreachable, so this commit adds tests and takes nothing away. Co-authored-by: Jason Reiss, Multi-Tech Systems, Inc.
A field carrying `transform` was treated as a derived value. Derived values read
no payload bytes, so the field was given the derived marker for a wire type and
a width of zero: it disappeared from the generated Thing Description, and every
field after it read from an offset short by its width.
That is a silently wrong conversion rather than a skipped schema, so the catalog
count says nothing about it - the device still converts, it just decodes to the
wrong numbers. decentlab's air_temperature (a u16 with
`transform: [{div: 100}, {add: -327.68}]`) reported 65535 instead of 327.67.
`transform` is not a derived-value descriptor. It post-processes a value that
*was* read from the wire, in the same way mult/div/add do, and the reference
interpreter applies it after reading the bytes. Only ref/polynomial/compute/
guard describe a value computed from other values.
So derivation is now decided by what the field says it reads, not by which
descriptors ride along: a declared wire type means bytes are read, and only the
`number` marker means they are not. A field with no type at all is still derived
if a descriptor says where its value comes from.
The transform stays under `lorav:derived` rather than gaining a term of its own,
because that object already names the operation and the reverse direction reads
it from there. The form schema's rule is relaxed to match: a `lorav:derived`
carrying only a transform may sit on a real wire type, while one carrying
ref/polynomial/compute/guard must still declare `number`.
The regression test asserts the byte offset of the *following* field as well as
the transform itself. The damage showed in the neighbours, so a test that only
checked the transform survived would have passed the whole time.
No change to the catalog, which stays at 157 from 158 schemas: no schema in the
pinned library puts a transform on a wire field. The next one does, 45 times.
Co-authored-by: Jason Reiss, Multi-Tech Systems, Inc.
Unrecognised field keys are rejected rather than ignored, so that a key which changes how bytes decode can never be dropped in silence. `sensor` is not one of those: it records which physical sensor a channel came from, the same kind of annotation as `semantic` and `ipso` already listed beside it, and the reference interpreter never reads it. Rejecting it took 42 devices out of the next schema library, most of them for a battery channel tagged with its source. It is ignored rather than carried through, because nothing downstream reads it and there is no vocabulary term that would give it a meaning. Co-authored-by: Jason Reiss, Multi-Tech Systems, Inc.
`value` was grouped with ref/polynomial/compute/guard as a descriptor of a derived value, which put its field outside the convertible subset and took the whole device with it. That cost 45 devices in the next schema library: nearly every downlink command identifies itself with a fixed category byte and a fixed command byte, and both are written that way. Nothing about it is computed. The reference interpreter never consults `value` while decoding - it reads the byte and reports what the payload actually held - and applies it only when encoding, to fill the byte in. The field is an ordinary scalar that happens to be constrained to one value. TD core already has the word for that, so it is emitted as `const` on the event's data schema and no binding term is introduced. `lorav:const` was the first attempt and is the wrong shape: the binding vocabulary describes how bytes are transferred, while this describes what the value may be, which every consumer of a data schema already knows how to read. The catalog is unchanged at 157 from 158 schemas; the pinned library has no downlink schemas for this to reach. Co-authored-by: Jason Reiss, Multi-Tech Systems, Inc.
A derived value names its inputs with `$name`. Converting back to a payload schema rebuilds the fields from the events alone, so an input that never became an event of its own comes back as nothing and the reference interpreter evaluates the field against zero. qingping's temperature decoded as -50 where its schema says 359.5. That failure is worse than a skip because it is invisible. The device appears in the catalog, the Thing Description validates, and only the number is wrong - nothing in the generator's output hints at it. So the assembled events are checked before being returned, and a device that cannot resolve its own inputs is skipped like any other schema outside the subset. The check is on what the Thing Description actually carries, not on the shape of the name. Rejecting every `_`-prefixed input, which is the obvious reading of "internal scratch value", is far too broad: most of them do get an event and resolve correctly, and doing it that way removed 22 devices to prevent this one. Comparing against the assembled event names costs one. The one remaining case is an internal input that is a bit range. Those forms are emitted correctly, but rebuilding flattens the byte group into top-level bit-range fields, and the interpreter does not store a top-level `_`-prefixed field as a variable - so the name resolves in the Thing Description and not in the schema built from it. Plain internal scalars are unaffected. The catalog is unchanged at 157 from 158 schemas. Co-authored-by: Jason Reiss, Multi-Tech Systems, Inc.
The pinned schema library grows from 158 device schemas to 218, and the four preceding commits are what let the new ones through: without them the same bump generates 82. 157 / 158 before 82 / 218 bump alone 167 / 218 bump with the four fixes EXPECTED_CATALOG_SIZE goes to 167 in this commit, as its own comment requires, so the diff shows what the bump is worth. 51 schemas remain outside the convertible subset, 37 of them for genuinely computed fields. Three test adjustments come with it. A lookup table whose wire values skip 0 used to lose its highest entries: the interpreter applied the table positionally, so one keyed 1..3 had length 3 and value 3 fell outside its guard. That was pinned as a known gap, with a note saying a bump might close it. This one does, so the test is turned around to hold the fix in place rather than the bug. The round-trip projection and the payload-width helper both now normalise `length: remaining` to -1. The new library writes the keyword where it used to write the number, and the conversion keeps the numeric spelling the vocabulary carries; the two are the same instruction to the interpreter, so the round trip should not be judged on which one the source happened to use. The golden snapshot is re-recorded against the larger corpus. Co-authored-by: Jason Reiss, Multi-Tech Systems, Inc.
The preceding commits changed behaviour the README still described the old way, and left one documented gap standing that no longer exists. Derived values were described as occupying zero payload bytes, with lorav:wireType "number" recording that there is nothing to read. That is still true of ref, polynomial, compute and guard, but no longer of transform: a transform post-processes a value that was read from the wire, so it sits beside a real wire type and byte offset. The section now draws that line explicitly, in the same terms as the if/then rule in lorawan-form.schema.json, so a reader writing a Thing Description by hand is told what the form schema will enforce. A byte fixed at one value is now carried as const on the event's data schema rather than as a binding term. That is the sort of thing a reader goes looking for under a lorav: name, so it is listed in the table of things this binding deliberately does not define, next to unit and the valid range. The limitation about lorav:valueMap tables having to start at zero is removed. The reference interpreter used to index the table positionally, so a table keyed 1 to 3 dropped its last entry; taking the schema library up to date closed it, and the test that pinned the gap now asserts every entry resolves. Two limitations are corrected rather than removed. Literal value expressions are converted now, so only formula remains unsupported. And bitfield_string is recorded as a new gap: it renders packed bits as text, has no form term, and 36 Milesight schemas adopted it at the current pin, which is why they are outside the catalog even though their files are still there. Co-authored-by: Jason Reiss, Multi-Tech Systems, Inc.
Hao-Siemens
marked this pull request as ready for review
August 26, 2026 18:23
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
1 participant
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.
Widen the LoRaWAN converter's device coverage and bring the schema library up to date
Supersedes #93 and #94 (both @reissjason) — neither applies cleanly since the vocab got breaking change in 0.3.0. The diagnoses from #93 are adopted here (three with different implementations); we can close #93/#94 as superseded.
From #93:
transform ≠ derived value — reimplemented, unblocks 45 devices
sensor is an annotation, not a decode key — same fix, 42 devices
fixed downlink byte ≠ computation — different spelling, 45 devices
derived value may reference data the TD lacks — narrowed, −1 device
length: remaining — now converted (mapped) instead of skipped, +2 devices
Rejected #93's new lorav:const term in favor of TD core's existing const
Internal-reference guard reworked to check assembled events (cost: 1 device instead of 22)er.
From #94: only the submodule pointer/catalog size; its test infra already landed via #92.