Key reassembly buffers by characteristic, validate sequence, drop truncated data

[kmatzen]

Second of four stacked PRs. Base is fix/correctness-and-test-gaps (#87), not main.

Three defects in BLEPacketReconstructor. All of them corrupt camera state silently rather than failing visibly, which is why none showed up as a bug report.

Continuations routed by recency

Continuations went to whichever buffer for that peripheral was touched most recently, because nothing in the packet correlates it to a message.

The root cause is the buffer key. (peripheral, queryID) invents concurrency the protocol does not have, while ignoring the concurrency it does: query responses (0x0077) and settings responses (0x0075) are separate notify characteristics that stream independently, and both fed one buffer set. Buffers are now keyed by (peripheral, characteristic), matching the spec's per-characteristic accumulation, which makes routing deterministic.

Reproduced against the pre-fix code with the real ordering — start query 0x13, start settings 0x12, then 0x13's continuation arrives:

delivered as queryID 0x12
payload contains B's bytes (0xBB): true
payload contains A's bytes (0xDD): true
>>> CONTAMINATED: query 0x13's fragment delivered inside settings 0x12

Sequence counter never validated

The 4-bit counter documented in BLE_PROTOCOL.md:29 was parsed by nobody. A dropped or duplicated continuation produced a byte-shifted payload that still satisfied the length check and decoded as plausible-but-wrong TLV — wrong resolution, FPS, battery. It is now validated, and a gap discards the message in progress.

Worth being explicit: sequence validation alone would not have fixed the routing bug. Two messages interleaved at the same fragment position carry the same counter, so the check cannot tell them apart. Re-keying does that work; the counter catches drops and duplicates.

Truncated buffers broadcast to every camera

checkTimeouts returned (data, queryID) and discarded the peripheral half of the key, so BLEManager applied one camera's partial buffer to every connected camera — which then fed hasSettingsMismatch and could trigger a settings rewrite of the whole group. A dropout on one camera could rewrite the fleet.

Truncated buffers are now discarded. Nothing is delivered on timeout, so the broadcast loop is gone entirely.

Smaller fixes

• clearBuffers(for:) matched peripherals by bare prefix, so clearing p1 also cleared p10.
• A start packet arriving mid-message now discards the superseded one loudly instead of overwriting it silently.
• A negative declared length is rejected.

Testing

128 tests pass, including five new ones covering sequence gaps, duplicates, cross-characteristic isolation, superseded messages, and timeout discard. Mutation-verified — reverting the key to peripheral-only fails the isolation test; disabling sequence validation fails the gap and duplicate tests.

Review notes

The design was checked with a TLA+ model (specs/PacketReassembly.tla, added in #90) before the code was written. Behaviour on real hardware is not verified — in particular, the counter starts at 0 for the first continuation, which matches this repo's existing tests and the Open GoPro convention, but is worth a sanity check against a camera.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AqfipPBp3eErDgZboooqxP