Skip to content

File upload: send each batch from the latest ack - #18

Merged
robdobsn merged 1 commit into
mainfrom
feature/upload-resend-from-latest-ack
Oct 23, 2026
Merged

robdobsn merged 1 commit into
mainfrom
feature/upload-resend-from-latest-ack

Conversation

@robdobsn

@robdobsn robdobsn commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Problem

PR #17 changed RaftFileHandler to carry on past an ack that covered only part of a batch, and to treat an ack arriving 700 ms or more after the last block as the device's timer ack.

Since RaftCore PR #14, the device acks the first block out of sequence at once and keeps its acks in step with the host's batches. With that firmware the PR #17 rule goes wrong whenever a block is lost mid-batch:

  1. The immediate ack shows progress, for the blocks before the gap, so the handler carries on sending.
  2. The device drops those blocks without acking them.
  3. The handler then waits 1 s for the device's timer ack, once per lost block.

Change

Each batch starts from the latest ack again, as it did before PR #17. PR #17's two fixes stay:

  • Acks that arrive while a batch is going out count. Before PR Feature/upload resend after loss #17 they were ignored, so after any loss every later batch waited for the timer ack.
  • A late ack behind one already received is ignored.

The timing-based timer-ack detection is removed. At most one batch is in flight, as before PR #17. PR #17 allowed two.

Effect

Upload tests: a simulated device with RaftCore's ack rules, 200 blocks at 15 ms each (BLE-like), 6 lost blocks.

Firmware, lost blocks Before PR #17 PR #17 This PR
Current (RaftCore #14), mid-batch 11.2 s, 5 timer acks 12.9 s, 6 timer acks 5.4 s, 0 timer acks, 239 blocks sent
Current, at the start of a batch 12.4 s, 6 timer acks 6.7 s 6.6 s, 251 blocks
Older, mid-batch 26.8 s, 21 timer acks 6.2 s, 309 blocks, 25 in flight 7.4 s, 391 blocks, 10 in flight
Older, at the start of a batch 29.2 s, 23 timer acks 6.0 s, 301 blocks, 20 in flight 7.0 s, 367 blocks, 10 in flight

With older firmware this PR is about 1 s slower than PR #17, but uses the original flow control and is still about 4× faster than before PR #17. RaftCLI's RICREST upload uses the same rule, and its own simulator gives the same block counts.

Tests

  • RaftFileHandler.test.ts now covers current and older firmware, with blocks lost mid-batch and at the start of a batch, plus the last block lost and the late-ack case. It checks timer acks, blocks sent and blocks in flight.
  • 3 of the 8 tests fail with PR Feature/upload resend after loss #17's code.
  • All 237 tests pass; lint and build are clean.
  • Not tested on a device.

🤖 Generated with Claude Code

PR #17 carried on past an ack that covered only part of a batch, and took an ack arriving 700 ms or more after
the last block as the device's timer ack. With current RaftCore (PR #14: the first block out of sequence is
acked at once, and the acks stay in step with the batches) that rule sends blocks the device then drops without
an ack after a lost block mid-batch, and waits 1 s for the timer ack each time.

Each batch now starts from the latest ack again, as before PR #17, keeping its two fixes: acks arriving while a
batch is going out count (they were ignored, so after a loss every later batch waited for the timer ack), and a
late ack behind one already received is ignored. At most one batch is in flight, as before PR #17.

Upload tests (simulated device, 200 blocks at 15 ms each, 6 lost):
- current firmware, lost mid-batch: 12.9 s with 6 timer acks -> 5.4 s with none (239 blocks sent)
- current firmware, lost at the start of a batch: 6.7 s -> 6.6 s (251 blocks)
- older firmware: 6.0-6.2 s -> 7.0-7.4 s (367-391 blocks), against 29 s before PR #17
The tests now cover both firmware versions with blocks lost mid-batch and at the start of a batch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@robdobsn
robdobsn merged commit c7bdf19 into main Oct 23, 2026
1 check passed
@robdobsn
robdobsn deleted the feature/upload-resend-from-latest-ack branch October 23, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant