plan 0008: get BLE download throughput off the floor - #144
Merged
Conversation
Coverage — host-testable units📂 Overall coverage
📄 File coverage
|
A 3.3 MB session downloading at 28.8 KB/s on an iPad, against ~130 KB/s remembered from a bench run. The SD transfer-clock bump (2 -> 8 MHz while parked) turned out to still be wired up correctly, so the regression was elsewhere. Three ceilings, all fixed: - Data Length Extension was requested on connect but never verified. The SoftDevice runs one link-layer control procedure at a time, so the ask fired immediately behind requestPHY(2M) can return NRF_ERROR_BUSY into a return value nobody reads -- and nothing ever called getDataLength() to find out. An un-extended link splits every 244-byte notify into ten 27-byte packets. bleTuneLink() now reads back what negotiated at +500 ms and re-asks with nothing else in flight. - The 7.5 ms connection-interval preference is one Apple centrals are required to reject (15 ms floor), leaving iOS on its own choice, commonly 30 ms. The preference stays as-is so desktop/Android are not slowed; a second, Apple-compliant request is made only when the measured interval is slower than 15 ms. - Every chunk was read straight off SdFat before being notified, putting a disk read in the radio's critical path and driving the card in 244-byte pieces rather than whole sectors. Chunks now stream from a compacting 4 KB read-ahead filled by one aligned multi-sector read. The read-ahead index math lives in a new host-tested ble_stream unit, including a model transfer that reassembles a file and compares it byte-for-byte -- an off-by-one here corrupts a downloaded session. Also in the download path: a mid-file SdFat read error used to land in the same branch as end-of-file and report DONE, handing the app a truncated session with a clean status. It now reports ERROR. And so the next regression is visible without a rebuild, the transfer page shows live KB/s plus the SD clock actually in force, the negotiated link-layer PDU and the ATT payload. sdActiveSpiHz() backs the SD figure with the clock SD.begin() accepted -- the 8 MHz bump falls back to 2 MHz silently, which nothing surfaced before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rc6mSm54pTqmg1eSgD3az2
TheAngryRaven
force-pushed
the
claude/bluetooth-download-speed-iwclyc
branch
from
August 17, 2026 00:08
f2038cf to
5c66db6
Compare
TheAngryRaven
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A 3.3 MB session downloading at 28.8 KB/s on an iPad, against ~130 KB/s remembered from an earlier bench run.
First suspect was the SD card, since the normal clock is a deliberately slow 2 MHz for ignition EMI. That turned out to be fine —
BLE_SETUP()still bumps to 8 MHz andBLE_STOP()still reverts it. ButsdSetSpiClock()silently falls back to 2 MHz when the fast re-init fails and nothing anywhere surfaced which clock a session actually got, so "is it at 8?" was unanswerable from the device. That silence is fixed here too.Reading the rest of the transfer path turned up three real ceilings:
1. Data Length Extension was requested but never verified.
configPrphConn(247, ...)sets the ATT MTU; the link-layer PDU is separate and defaults to 27 bytes, which splits every 244-byte notify into ten packets — roughly a 3–4× tax. The connect callback does callrequestDataLengthUpdate(), but it fires one line behindrequestPHY(2M), and the SoftDevice runs one link-layer control procedure at a time — a collision returnsNRF_ERROR_BUSYinto a return value nobody reads. Nothing retried, and nothing ever calledgetDataLength()to notice. Centrals that initiate DLE themselves (Chrome desktop/Android) paper over it entirely, which is very plausibly why the bench run was fast.bleTuneLink()now reads back what actually negotiated at +500 ms and re-asks with nothing else in flight.2. A connection-interval request iOS is required to reject.
setConnInterval(6, 12)asks for 7.5–15 ms; Apple's accessory rules set a 15 ms floor, so iOS rejects it and keeps its own pick — commonly 30 ms, half the connection events. The preference is deliberately left alone (desktop/Android honour it and must not be slowed); instead an adaptive second ask fires only when the measured interval is slower than 15 ms.3. The SD read sat in the radio's critical path. Each chunk was read off SdFat immediately before being notified, so every packet waited on a disk read — and 244 bytes isn't a sector, so the card ran one single-block command per 512 bytes. Chunks now stream from a compacting 4 KB read-ahead filled by one aligned multi-sector read. This also structurally removes the old
seekCur()rewind-on-failed-notify hazard: with the bytes in RAM there is no file position to unwind.Two things beyond the reported problem:
DONE— it fell into the same "no bytes" branch as end-of-file, handing the app a truncated session file with a clean status. It now sendsERROR.118KB/s 8M 251 244. A27in the third slot means DLE didn't take. The whole reason this took a full session to spot is that the old page showed a percentage and nothing else.Rationale in full:
docs/plans/0008-ble-download-throughput.md.Which lever actually dominated is still a bench measurement — the KB/s readout on the next download is what settles it. Best guess is DLE.
Type of change
Wire protocol is unchanged — same
SIZE:/ chunks /DONEflow, and chunk size was already MTU-dependent. The one addition isERRORon the status characteristic if the card fails mid-read, where a bogusDONEused to go.How it was verified
ctest --test-dir tests/build)clang-tidyclean (new unit added to the job's file list)display_pages.ino/sd_functions.ino) builds clean, and all 6 sim tests pass with golden hashes unchanged, butbluetooth.inois excluded from the sim so the compile-sketch job is the first real check on it.New
ble_streamunit is host-tested including a model transfer that streams a file through the real buffer discipline and compares it byte-for-byte, across sizes chosen around the buffer boundary (0, 1, 243, 244, 4095, 4096, 4097, 100000) and the MTUs a real central hands us (244, 182, 20). An off-by-one there corrupts a downloaded session, so it is checked away from the radio.Checklist
CHANGELOG.mdupdated under[Unreleased]ARCHITECTURE.md/CLAUDE.mdupdated (subsystem 6 + constants table + file map)tests/