Cressi GOA: cap BLE read target to remaining bytes on last packet - #126
Cressi GOA: cap BLE read target to remaining bytes on last packet#126mikeller wants to merge 1 commit into
Conversation
On the final outer iteration of cressi_goa_device_download the remaining data may be fewer than SZ_DATA (512) bytes. The inner BLE accumulation loop was still asking dc_iostream_read for a full SZ_DATA block, so the BLE layer received the 16-byte EOT end-of-transfer GATT notification in response, rejected it because its length exceeded the requested size, and returned DC_STATUS_IO -- aborting the download with zero dives reported even though all payload data had already been received. Fix: compute a target byte count for the inner loop. On the first outer iteration (nbytes == 0) size has not yet been updated from the packet header, so keep target == SZ_DATA. From the second iteration onward, if (size - nbytes) < SZ_DATA, cap target to the exact number of remaining bytes. The inner loop and the dc_iostream_read call both use target instead of SZ_DATA. The EOT end-marker read that follows the outer loop is unaffected: with the capped target the inner loop exits before consuming the EOT packet, so the existing post-loop read at line 312 receives it correctly. Buffer bounds are unchanged: packetsize never exceeds target <= SZ_DATA, so writes into packet + 3 + packetsize stay within the declared packet[3 + SZ_DATA + 2] buffer. Signed-off-by: Michael Keller <github@ike.ch>
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, well-scoped, and correctly prevents over-reading past the final payload into the EOT marker without affecting non-BLE behavior.
Pull request overview
Fixes a BLE download failure in cressi_goa_device_download() when the final payload chunk is smaller than SZ_DATA (512): the inner BLE read loop now requests only the remaining bytes on the last iteration so it doesn’t accidentally consume (or cause rejection of) the 16-byte EOT marker.
Changes:
- Compute a per-iteration
targetread size for BLE transfers. - Use
target(instead ofSZ_DATA) for both the accumulation loop bound and thedc_iostream_read()request size.
File summaries
| File | Description |
|---|---|
| src/cressi_goa.c | Caps BLE read size to remaining payload bytes on the final packet to prevent EOT mis-read/rejection and avoid aborting completed downloads. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dirkhh
left a comment
dirkhh
left a comment
There was a problem hiding this comment.
very nice catch...
it does smell rather LLM written to me.
I kinda like marking things that were mainly written by an LLM as such (and if your style has gotten so LLM-y that this is actually plain old you, then that mostly reinforces my point, I guess...)
On the final outer iteration of cressi_goa_device_download the remaining
data may be fewer than SZ_DATA (512) bytes. The inner BLE accumulation
loop was still asking dc_iostream_read for a full SZ_DATA block, so the
BLE layer received the 16-byte EOT end-of-transfer GATT notification in
response, rejected it because its length exceeded the requested size, and
returned DC_STATUS_IO -- aborting the download with zero dives reported
even though all payload data had already been received.
Fix: compute a target byte count for the inner loop. On the first outer
iteration (nbytes == 0) size has not yet been updated from the packet
header, so keep target == SZ_DATA. From the second iteration onward, if
(size - nbytes) < SZ_DATA, cap target to the exact number of remaining
bytes. The inner loop and the dc_iostream_read call both use target
instead of SZ_DATA.
The EOT end-marker read that follows the outer loop is unaffected: with
the capped target the inner loop exits before consuming the EOT packet,
so the existing post-loop read at line 312 receives it correctly.
Buffer bounds are unchanged: packetsize never exceeds target <= SZ_DATA,
so writes into packet + 3 + packetsize stay within the declared
packet[3 + SZ_DATA + 2] buffer.