Cressi GOA: cap BLE read target to remaining bytes on last packet

[mikeller]

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.

[Copilot]

🟢 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 target read size for BLE transfers.
• Use target (instead of SZ_DATA) for both the accumulation loop bound and the dc_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]

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...)

[jefdriesen]

Do you have a download log from the user? I'm pretty sure this change will cause the downloading to fail. The BLE packets have always been padded with 0xFF bytes to reach 512 bytes. That's why the inner loop always tries to read exactly 512 bytes. Reading less bytes means those 0xFF bytes are now no longer consumed here and will be received when reading the EOT xmodem packet and that check will now fail.

You can clearly see the padding in the log attached to ticket subsurface/subsurface#4930.

[mikeller]

You're right, sorry for the noise. Looking at the log more carefully, the
device always pads each block to exactly 512 bytes with 0xFF, so the inner
loop must consume all 512 bytes on every iteration. Capping the target to
the remaining logical bytes would leave the padding unread and corrupt the
EOT read.

The actual failure on that last dive was a timeout -- the data blocks
arrived fine but the device never sent the EOT notification. I'll close
this PR.