fix: decode trailing wire-format fields the reader drops (AUTOADD_CONFIG, LOGIN_SUCCESS, ACK, DEFAULT_FLOOD_SCOPE)#87
Conversation
|
Hi @mwolter805 |
Wire-format parity fixes in reader.py for trailing fields the
companion-radio firmware emits but the SDK decoder was dropping, plus an
over-read guard on DEFAULT_FLOOD_SCOPE. Each fix uses the existing
BATT_AND_STORAGE defensive-read pattern (up-front minimum-length check
plus per-field cumulative-length gates), so older firmware that doesn't
emit the trailing fields keeps decoding without raising. Every fix ships
a legacy-frame and modern-frame unit test pair.
AUTOADD_CONFIG (PacketType 25): companion-v1.14.0 firmware emits a
trailing max_hops byte (firmware commit 00566741); the SDK dropped it.
Adds a defensive `if len(data) >= 3` read. Pre-v1.14.0 frames decode to
`{"config": ...}` with no max_hops key.
LOGIN_SUCCESS (PacketType 0x85): the new-style RESP_SERVER_LOGIN_OK path
emits three trailing fields the SDK was dropping — server_timestamp (4B,
firmware commit 0e90b731), acl_permissions (1B, 7947e8a2), and
fw_ver_level (1B, 418ae08b), all first shipped in companion-v1.10.0.
Adds per-field length gates (>=12, >=13, >=14). Legacy 8-byte "OK"-path
frames decode unchanged.
ACK (PacketType 0x82): firmware emits a trailing 4-byte trip_time
(round-trip latency in ms) since companion-v1.0.0a (firmware commit
d9dc76f1, Jan 2025) — on the wire ~16 months but never surfaced by the
SDK. Adds an `if len(data) >= 9` read. Legacy 5-byte ACK frames decode
unchanged.
DEFAULT_FLOOD_SCOPE (PacketType 28): firmware emits a 48-byte populated
frame or a 1-byte sentinel when no scope is set. The SDK unconditionally
read 31+16 bytes, over-reading 47 bytes past the end of the sentinel
frame and dispatching `{"scope_name": "", "scope_key": ""}`. Adds an
`if len(data) >= 48` guard so the sentinel dispatches `{}`. Consumers
detecting "no scope" via `payload["scope_name"] == ""` should switch to
a key-presence check.
RAW_DATA: the payload-framing fix this branch originally carried landed
upstream first in PR meshcore-dev#86 (commit 44b21be), with identical decode logic
(discard the reserved 0xFF byte, then read the remaining variable-length
payload). That redundant change is dropped here; this commit retains only
its regression test (test_raw_data_realistic_frame), which exercises the
upstream fix end-to-end — the upstream change shipped without a test.
CHANGELOG:
- Decode max_hops trailing byte in AUTOADD_CONFIG (companion-v1.14.0+).
- Decode server_timestamp / acl_permissions / fw_ver_level trailing
fields in LOGIN_SUCCESS (companion-v1.10.0+).
- Decode trip_time trailing field in ACK (companion-v1.0.0a+).
- DEFAULT_FLOOD_SCOPE dispatches `{}` on the 1-byte sentinel frame
instead of `{scope_name: "", scope_key: ""}`. Detect "no scope" via
key presence.
Tests: 10 tests in tests/unit/test_protocol_surface_gaps.py (legacy +
modern frame pair per finding, including a RAW_DATA regression test for
the upstream fix); all 10 pass on the rebased upstream base.
Why: companion-radio firmware has been emitting these fields on the wire
for between 2 and 16 months; SDK consumers (integrations, bots,
dashboards) lose access to data that is on the wire today.
mwolter805
force-pushed
the
fix/wire-format-parity-bundle
branch
from
8052908 to
46288e4
Compare
mwolter805
changed the title
|
Rebased onto current The conflict was entirely from #86: your One unrelated heads-up while I was here: |
|
Thanks, I'll merge For the payload size, I just checked on the firmware code and it drops payloads < 4 bytes. Thanks for pointing this and go ahead for the fix ;) |
2 participants
Background
This started while wiring the
AUTOADD_CONFIGresponse into a Home Assistant MeshCore integration built on this SDK — the auto-add config dialog was missingmax_hopsbecause the reader stopped before that trailing byte. Rather than patch the one field downstream, I diffed every frame the companion-radio firmware emits against whatreader.pyactually reads, which surfaced several more frames with the same class of gap.Rebased onto current
mainRebased after #86 and #88 merged — thanks for those. Two consequences:
RAW_DATAis now handled upstream. Sending and receiving raw_data not working as expected #86 (44b21be) landed the exact RAW_DATA payload-framing fix this PR originally carried — identical decode logic (discard the reserved0xFFbyte, then read the remaining variable-length payload). I dropped the now-redundant code change and kept only the regression test for it (test_raw_data_realistic_frame), since Sending and receiving raw_data not working as expected #86 shipped without one. This PR no longer touches the RAW_DATA decode line.CHANNEL_DATA_RECV(feat: add CHANNEL_DATA_RECV (RESP_CODE 27) packet type and handler #88) merged cleanly; the test-file overlap is resolved with both test sets retained.Summary
Four wire-format parity fixes in
reader.pyfor trailing fields the companion-radio firmware emits but the SDK decoder drops, plus an over-read guard. All use the existingBATT_AND_STORAGEdefensive-read pattern (up-front minimum-length check + per-field cumulative-length gates keyed onlen(data) >= N), so older firmware that doesn't emit the trailing fields keeps decoding without raising. Each ships a legacy-frame + modern-frame test pair, making the firmware-version dependency explicit.Fixes
AUTOADD_CONFIG—max_hopstrailing byte. The firmware grew this response from 1 byte to 2 (config+max_hops) in companion-v1.14.0+ (00566741). The reader read onlyconfig. Adds a length-gated 1-bytemax_hopsread; the legacy 1-byte frame decodes to justconfigwith no exception.LOGIN_SUCCESS— three trailing fields. The new-styleRESP_SERVER_LOGIN_OKpayload carriesserver_timestamp(4B,0e90b731),acl_permissions(1B,7947e8a2), andfw_ver_level(1B,418ae08b) — all first shipped in companion-v1.10.0 — but the reader stopped afterpubkey_prefix. Adds per-field gated reads. The legacy 8-byte"OK"frame is unaffected.ACK—trip_timetrailing field. The firmware emitsPUSH_CODE_SEND_CONFIRMEDas 9 bytes (ack_hash4B +trip_time4B, the round-trip latency in ms) sinced9dc76f1(companion-v1.0.0a, ~16 months on the wire), but the reader read onlyack_hash. Adds a length-gated 4-byte read. The key istrip_time, matching the firmware variable name; happy to rename totrip_time_msif you prefer the unit suffix.DEFAULT_FLOOD_SCOPE— over-read guard. On the 1-byte sentinel frame the handler read 47 bytes past the end. Gates the read onlen(data) >= 48; the sentinel frame now dispatches an empty payload. A consumer treatingpayload["scope_name"] == ""as a sentinel should switch to a key-presence check ("scope_name" in payload).Tests
tests/unit/test_protocol_surface_gaps.py— 10 tests, a legacy-frame + modern-frame pair per fix (and the RAW_DATA regression test covering #86's fix). All pass on the rebased base.