follow-up: harden RP2350W BLE notification lifecycle from #3863
Source PR: https://github.com/FastLED/FastLED/pull/3863 Related implementation issue: #3861
Source comments:
- https://github.com/FastLED/FastLED/pull/3863#discussion_r3727787264
- https://github.com/FastLED/FastLED/pull/3863#discussion_r3727787271
- https://github.com/FastLED/FastLED/pull/3863#discussion_r3727787283
Why this matters: the RP2350W transport can corrupt or strand JSON-RPC notifications when responses overlap or a connection drops, and the HIL reader can miss a later valid notification.
CodeRabbit suggestions (verbatim):
Handle each BLE notification independently.
read_lines()yields one notification value per line, but this loop accumulates them. If one complete JSON-RPC line arrives and a laterjson.loads()cannot parse because the next value appends to the previous line, keep parsing completed lines instead of treatingnotification_payloadas a shared fragment buffer.
Queue complete responses before notification scheduling.
response_sinkclears and replaces the singlenotification_payloadwhile an earlier response can still have unsent chunks. If two RPC responses are produced before the first transfer completes, the client receives a truncated first response and loses its remaining bytes.Add an outbound FIFO. Advance to the next response only after all chunks of the current response are sent. Add a regression test that produces two responses before the send-ready callback completes.
Reset notification registration state on disconnect.
onDisconnectedstill leavesstate->notification_scheduledandruntime.notification_callback_pendingset. If a request registers last and the connection drops beforeATT_EVENT_CAN_SEND_NOW, new writes can be blocked and a disconnected callback can hit clearednotification_payload. Set both flags false when disconnecting while a pending registration exists.
Acceptance criteria:
- RED: add a focused transport test that queues two complete responses before the first BTstack send-ready callback finishes and demonstrates that current code truncates/replaces the first response.
- GREEN: introduce a bounded outbound FIFO, preserve response order, and advance only after the final ATT-MTU-sized chunk is accepted.
- RED: add a focused disconnect/reconnect test with a pending notification registration that demonstrates stale pending state blocks the next connection or accesses cleared payload state.
- GREEN: make disconnect/destroy lifecycle generation-safe, clear application pending state without reusing a still-owned BTstack registration unsafely, and prove a subsequent write/notification succeeds.
- RED: add a focused Python test with multiple independent notification values where a valid pong follows another complete or malformed value.
- GREEN: parse each complete notification independently while retaining correct ATT-fragment reassembly at the BleInterface boundary.
- Preserve singleton-owned RP runtime state, bounded queue-full behavior, ESP32 NimBLE behavior, and unsupported-platform stubs.
- Run focused BLE tests, strict lint, RP2350W and ESP32-C6 target builds, then record live GATT discovery/ping plus disconnect/reconnect evidence on the Pico 2 W.
Source: FastLED/FastLED