halcyon_symbios: treat timeout as retryable and raise BLE read timeout to 5 s

[mikeller]

The user-supplied log shows a ~3 s gap in BLE notifications mid-transfer
that triggers the read timeout. The most likely cause on iOS is a
transient suspension of BLE notifications (for example during connection
parameter renegotiation), though the exact mechanism is inferred from the
log pattern rather than confirmed.

With the previous code a single DC_STATUS_TIMEOUT in the inner retry loop
of halcyon_symbios_download() was treated as a fatal error and caused an
immediate abort, even though up to MAXRETRIES (3) retries were available
for protocol errors.

Three fixes across two commits:

1. Include DC_STATUS_TIMEOUT in the retryable-error condition (alongside
   DC_STATUS_PROTOCOL) so that a transient notification gap triggers a
   NAK re-transmission instead of aborting the download. This matches
   the pattern used by suunto_common2, divesystem_idive, hw_ostc,
   mares_common, oceanic_vtpro, and other drivers. The existing
   MAXRETRIES limit is still enforced; a persistent timeout still
   terminates the transfer.

2. Raise the BLE read timeout from 3000 ms to 5000 ms in
   halcyon_symbios_device_open(). The 3 s value is tight for iOS BLE;
   other drivers in the same codebase use 4000-5000 ms, giving the
   stack more headroom before a timeout fires.

3. Before sending the NAK on a timeout, sleep 300 ms and purge the
   input queue. If a packet arrived marginally late it may already be
   queued; without the purge the driver would read the stale copy and
   the retransmitted block would corrupt the next read.

Fixes subsurface/subsurface#4929

[Copilot]

🟢 Approval recommended

The changes are small, consistent with existing retry logic (bounded by MAXRETRIES), and align with the stated iOS/CoreBluetooth timeout behavior without introducing unbounded retries.

Pull request overview

This PR improves BLE transfer robustness for the Halcyon Symbios driver by treating transient notification gaps (common on iOS/CoreBluetooth during connection parameter changes) as retryable, and by increasing the receive timeout to reduce unnecessary aborts mid-download.

Changes:

• Treat DC_STATUS_TIMEOUT as retryable in halcyon_symbios_download() (alongside DC_STATUS_PROTOCOL) so the existing NAK retransmission loop can recover from transient gaps.
• Increase the I/O receive timeout in halcyon_symbios_device_open() from 3000 ms to 5000 ms to better accommodate iOS BLE behavior.

File summaries

File	Description
src/halcyon_symbios.c	Makes timeouts retryable during block receive and increases the iostream timeout to reduce spurious BLE download failures.

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.

[jefdriesen]

Where did you find this information about this CoreBluetooth pause issue? If that's indeed the case, then I expect we would run into it with other dive computers too.

I'm not sure this fix will be sufficient. A timeout is a bit different from a device responding with a NAK to indicate some problem. In the latter case, we remain in a know state from the protocol point of view. With a timeout we don't really known what happend. We don't know if the device did not send a packet, the packet was send but got lost somehow, or the packet was send fine and only arrived a bit too late. In that last case, the packet may still arrive and get added to the receive queue. When you request a re-transmission, the device will re-send the packet but we'll read the already queued packet and things will likely go wrong a bit later. To handle that, I usually add an extra delay (dc_iostream_sleep) followed by a dc_iostream_purge call to discard any packets in the receive queue.

[mikeller]

On the source: the CoreBluetooth renegotiation reference was an inference from the log pattern rather than a specific Apple document. The log shows a clean 3 s gap at the transport level with no device-side error, which is consistent with a transient BLE notification suspension, but I cannot point to a definitive Apple documentation entry. I've updated the PR description accordingly.

On the purge point: you're right. I've pushed an additional commit that adds a 300 ms sleep followed by dc_iostream_purge(DC_DIRECTION_INPUT) before the NAK is sent, but only when the error was DC_STATUS_TIMEOUT. A protocol error (DC_STATUS_PROTOCOL) means the device responded with a NAK and the state is known, so the purge is not needed there. The sleep gives a marginally late packet time to arrive before the purge clears it. The pattern follows divesystem_idive, which also sleeps before retrying after a timeout.

On whether this will affect other dive computers: other BLE drivers that use DC_STATUS_TIMEOUT as retryable (e.g. suunto_common2, hw_ostc) go through suunto_common2_transfer, which sends a full command re-request rather than a NAK, so they don't have the same ambiguity about what's in the receive queue. The Halcyon Symbios NAK mechanism is where the risk exists.

[Copilot]

🟡 Changes recommended

The new timeout recovery path ignores dc_iostream_sleep()/dc_iostream_purge() return codes, which can allow continuing after a transport/purge failure and risk stream desynchronization or corrupted transfers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite