fix: stop stray BLE notifications from desyncing reads and corrupting config

[davelee98]

Problem

The notification queue (BLEConnection._notification_queue) is a plain FIFO of raw frames with no request/response correlation — read_response() returns whatever is at the front. One orphaned frame therefore shifts every subsequent read by one, permanently. drain_notifications() mitigates this before each write, but it only clears what has already arrived.

Two gaps let stray frames through, both observed in the field:

1. Frames survived a disconnect. disconnect() cleared _client and _notification_characteristic but left the queue populated. A BLEConnection is reusable (connect() after a drop keeps the same queue), so residue from a dead link could be returned as the first response on the next link. Neither the stack-signalled drop nor the connect-retry teardown flushed either.

2. A config read absorbed foreign frames silently. strip_command_echo() returns a non-matching frame unchanged, so in interrogate() a frame from another exchange had its 2-byte echo consumed as the chunk-number field and its body appended as config data. The chunk loop only counts bytes, so nothing detected it.

Observed downstream, from a single session's logs:

First chunk: 98 bytes, total length: 707
Received chunk, total: 190/707
...
Received chunk, total: 766/707        <- 59 bytes over
Received complete TLV data: 766 bytes
WARNING Unknown packet type 0x00 at offset 128, skipping

total_length (707) matched the TLV wrapper's own length field; the extra 59 bytes were a stray frame spliced in, and the parser ran off the end of the real config into it. The same root cause surfaces elsewhere as a hard failure, e.g. Firmware version echo mismatch: expected 0x0043, got 0x0040 — a leaked config chunk answering the firmware-version read.

Changes

Flush the queue on every disconnect path — all three, only one of which goes through disconnect():

• _on_disconnect(), the stack-signalled callback, is the only path that runs on an unexpected drop. Flushes before notifying the owner so it still runs if that callback raises.
• disconnect() flushes outside its is_connected branch — the branch is skipped when the link already died, and not all bleak backends invoke disconnected_callback for a caller-initiated disconnect.
• _clear_cache_and_drop() drops the client directly rather than via disconnect(), and a retried connect reuses the queue.

Screen config-read frames by command echo. interrogate() now skips frames belonging to other exchanges rather than splicing them in, bounded by MAX_STRAY_CONFIG_FRAMES so a device streaming unrelated frames fails with a clear error instead of renewing the read timeout indefinitely. The no-config NACK ({0xFF,0x40,...}) is passed through rather than skipped — it answers this command, and discarding it would turn a clear "no stored configuration" error into a timeout.

strip_command_echo() keeps its lenient behavior (callers that cannot receive foreign frames rely on it); the mismatch case is now documented and factored into matches_command_echo() for callers that must screen first.

Name the command when a frame is dropped, at both drop sites. describe_command_code() resolves the ACK high bit and the 0xFF NACK prefix back to the originating command, so the frames most worth diagnosing don't log as unrecognized codes:

Ignoring stray READ_FW_VERSION (0x0043) frame (25 B) while reading config
Discarded 2 stale notification(s) before command: READ_CONFIG (0x0040) (4 B), AUTHENTICATE (0x0050) (3 B)

A bare count reported that a desync happened but not why.

Notes

• No behavior change to strip_command_echo() itself, so no other call site is affected — it has only the two in interrogate().
• Not addressed here: interrogate() still discards the chunk-number field without validating it, and never truncates tlv_data to total_length, so a correctly-echoed duplicate or overshooting final chunk can still get through. Happy to follow up separately.

Test plan

• 9 new unit tests: disconnect/link-drop/connect-retry flush, owner still notified after flush, stray-frame skip, stray bound, error frame after a stray, ACK/NACK/unknown code labelling.
• Full suite: 1028 passed.
• ruff, ruff-format, mypy --strict, pylint clean via prek run --all-files.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!