Three small cleanup fixes bundled per proposal §4.1 commit order.
N08 — CONTROL_DATA empty payload guard. The handler reads
`payload = dbuf.read()` then immediately dereferences `payload[0]`
without checking length. A zero-length payload (firmware truncation
or garbled frame) raises IndexError. Pre-F06 the IndexError would
escape; post-F06 it would log and skip the dispatch via the umbrella.
Adding an explicit `if len(payload) == 0: return` after the read
short-circuits the empty case before it touches `payload[0]`, with a
debug log noting the empty payload. The `return` exits handle_rx
cleanly without engaging the F06 umbrella's parse-error path, which
is the correct behavior — an empty CONTROL_DATA frame is not a parse
error, it's an unusable frame.
F12 — print(res) leftover debug. The RAW_DATA handler had a stray
`print(res)` polluting stdout. Replaced with `logger.debug(res)` to
match the surrounding `logger.debug("Received raw data")` line.
N10 — magic numbers 16 and 17. Two `elif packet_type_value == 16/17`
branches hardcoded the integer values for CONTACT_MSG_RECV_V3 and
CHANNEL_MSG_RECV_V3, both already declared in packets.py:94-95.
Replaced with `PacketType.CONTACT_MSG_RECV_V3.value` and
`PacketType.CHANNEL_MSG_RECV_V3.value` to eliminate drift risk if
the enum is ever renumbered.
Findings: N08 (Info), F12 (Info), N10 (Info)
File: src/meshcore/reader.py
Why: parse_status with offset=8 reads up through data[56:60]
(the rx_airtime field), so a full STATUS_RESPONSE push frame is
60 bytes: 1 type + 1 reserved + 6 pubkey + 52 status fields. The
push handler in handle_rx previously called parse_status with no
length check at all, so a short frame would slice through empty
data and silently produce zeros for every missing field. HA sensor
telemetry would silently report all-zero status — same class as N07.
The BINARY_RESPONSE STATUS path at the bottom of handle_rx already
gates parse_status with `len(response_data) >= 52` on its
offset-stripped buffer; this commit adds the equivalent gate for
the push path: `if len(data) < 60: log + return`. The `return`
short-circuits cleanly out of the umbrella try block without
dispatching a STATUS_RESPONSE event for the bogus parse.
Refs: Forensics report finding NEW-C (S3)
Why: The BATTERY handler previously gated the used_kb / total_kb
reads on `len(data) > 3`, which is wrong. The full
RESP_CODE_BATT_AND_STORAGE frame is 11 bytes (1 type + 2 level +
4 used_kb + 4 total_kb), so a 4-10 byte truncated frame would pass
the guard, and io.BytesIO.read(4) silently returns short bytes
instead of raising. int.from_bytes(b"", ...) returns 0, so HA
sensor telemetry silently reports zero storage on a truncated frame.
Tighten the guard to `len(data) >= 11` so the storage fields are
only parsed when the full frame is present. Inline comment added
to document the expected frame layout.
Note: the unconditional 2-byte `level` read at the top of the
handler has the same class of issue (no guard, silent zero on a
1-byte frame). That is out of scope for finding N07 and has been
logged in issues_log.md as a separate item.
Refs: Forensics report finding N07 (S3)
Why: The ALLOWED_REPEAT_FREQ branch in handle_rx had `except e:` —
syntactically valid Python only if `e` happens to be bound to an
exception class, which it isn't. The first time the inner read loop
actually raised, the except clause itself would raise NameError
("name 'e' is not defined") and propagate out of the handler. The
proposal correctly notes this is unreachable in practice today
because `int.from_bytes(b"", ...)` returns 0 so the loop terminates
cleanly, but it is a latent footgun. Replace with the standard
`except Exception as e:` form and swap the `print(e)` for a proper
`logger.warning(...)` call to match the rest of the file (which uses
the module logger, not stdout).
Refs: Forensics report finding F11 (S3)
Why: The LOGIN_FAILED handler in handle_rx referenced an undefined
identifier `pbuf` instead of the local BytesIO `dbuf`. Firmware emits
PUSH_CODE_LOGIN_FAIL as a fixed 8-byte frame, which trivially
satisfies the `len(data) > 7` guard, so every remote auth failure
raised NameError. The sibling LOGIN_SUCCESS handler a few lines above
already uses `dbuf.read(6).hex()` correctly; this commit aligns the
LOGIN_FAILED branch with the same pattern.
Refs: Forensics report finding F10 (S1)
Why: handle_rx is invoked from a detached task in MessageReader, so any
exception escaping its ~850-line if/elif dispatch is silently swallowed
by asyncio as "Task exception was never retrieved." The only crash
guard previously was a single try/except IndexError around the first
byte read; everything past line 73 was unguarded. This commit adds an
umbrella try: ... except Exception as e: around the entire dispatch
body that logs the exception class, message, raw frame hex, and full
traceback via logger.error. The umbrella neutralizes the crash surface
of F10, F11, N07, N08, R01, NEW-B, and NEW-C, which the next commits
will then fix individually now that they are observable.
Refs: Forensics report finding F06 (umbrella crash protection)
