Skip to content

feat(ble): Phase 2 BLE file transfer protocol#3117

Open
nic-olo wants to merge 6 commits into
devfrom
feat/ble-transfer-phase2
Open

feat(ble): Phase 2 BLE file transfer protocol#3117
nic-olo wants to merge 6 commits into
devfrom
feat/ble-transfer-phase2

Conversation

@nic-olo

@nic-olo nic-olo commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

  • Align pack size to 470B, drop fakeFileSize, and signal flowVersion:2 on pack 0
  • Pipeline up to 4 UART file packets in flight with backoff on BES buffer-full NAKs
  • Replace pre-transfer Thread.sleep with ble_ready_ack handshake (ASG + phone SDK)
  • Remove isBesLie workaround; phone SDK uses real fileSize / packSize math

Commits

  1. feat(asg): raise BLE pack size to 470B and add flowVersion:2 flags
  2. feat(asg): drop fakeFileSize and pipeline UART file packets
  3. feat(asg): replace pre-transfer sleep with ble_ready_ack handshake
  4. feat(sdk): Phase 2 BLE file transfer on phone
  5. test(asg): add BLE transfer mode benchmark tooling for debug builds

Test plan

  • ./gradlew :app:testDebugUnitTest passes
  • Flash matching BES feat/ble-transfer-phase2 firmware
  • Install ASG debug build; run asg_client/scripts/bench-ble-transfer-mode.sh
  • Take photo over BLE with phone connected; confirm ble_ready_ack in logcat
  • Verify ~half the packet count vs Phase 1 for same photo size

Deploy with: mentra-live-bes feat/ble-transfer-phase2 PR — all three sides must ship together.

Made with Cursor


Note

High Risk
Changes the end-to-end BLE photo pipeline and needs a coordinated BES firmware deploy; mismatched versions or ack timeouts can break or degrade transfers.

Overview
Phase 2 BLE photo transfer bumps payloads to ~470B (512 MTU), advertises flowVersion: 2 on packet 0, and stops inflating fileSize for BES. Glasses UART sending is pipelined (up to four in-flight packets) with window shrink/retry on BES NAKs.

Handshake: fixed pre-transfer Thread.sleep is replaced by ble_ready_ack after ble_photo_ready (BlePhotoReadyAck, BleReadyAckCommandHandler); the phone SDK now sends the ack. ble_photo_ready also carries flowVersion.

Phone SDK: drops isBesLie math, forwards full negotiated MTU (no 256 cap), raises default FILE_PACK_SIZE to 470, requests high connection priority and LE 2M PHY on Android, and adds debug BleBandwidthBench plus bench-ble-transfer-mode.sh on ASG.

Requires matching BES feat/ble-transfer-phase2 firmware with ASG and phone app.

Reviewed by Cursor Bugbot for commit 30f1edf. Bugbot is set up for automated code reviews on this repo. Configure here.

nic-olo and others added 5 commits June 9, 2026 17:29
Align BesWireFormat with Phase 2 wire format so BES can use real fileSize
and actual packSize from the first packet header.

Co-authored-by: Cursor <cursoragent@cursor.com>
Send real fileSize with flowVersion flags, keep up to four UART packets
in flight, and shrink the pipeline on BES buffer-full NAKs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Phone sends ble_ready_ack after ble_photo_ready; ASG waits on a latch
instead of a fixed Thread.sleep before starting file packets.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use 470B FILE_PACK_SIZE, remove fakeFileSize/isBesLie workaround, and
emit ble_ready_ack when ble_photo_ready arrives.

Co-authored-by: Cursor <cursoragent@cursor.com>
Register BleTransferModeReceiver (exported in debug manifest) and add a
bench script to compare OPTIMIZED vs LEGACY pre-transfer timing.

Co-authored-by: Cursor <cursoragent@cursor.com>
@nic-olo nic-olo requested a review from a team as a code owner June 9, 2026 09:30
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

📋 PR Review Helper

📱 Mobile App Build

Ready to test! (commit 30f1edf)

📥 Download APK

🕶️ ASG Client Build

Build failed (commit 30f1edf) - View logs


🔀 Test Locally

gh pr checkout 3117

+ currentFileTransfer.currentPacketIndex);

if (state == 1) { // Success (K900 uses state=1 for success)
// CRITICAL: Ignore duplicate ACKs for packets we've already moved past

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pipeline timeout retry stalls

High Severity

After pipelining, a UART ACK timeout still retries via sendNextFilePacket, which bumps nextSendIndex with max and does not clear pendingPackets or rewind the send cursor. With several packets in flight, retries often send nothing and the ACK watchdog is not scheduled again, leaving the transfer hung or failing only after repeated timeouts.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ce19f43. Configure here.

Add BleBandwidthBench for Android and iOS to auto-trigger take_photo after
connect and log transfer throughput (BLE_BANDWIDTH_BENCH). Wired from
MentraLive in the Phase 2 SDK commit.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 30f1edf. Configure here.

+ ackWaitMs
+ "ms, continuing ["
+ BleTransferMode.get()
+ "]");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack wait reuses pre-delay ms

Medium Severity

BlePhotoReadyAck.await uses BleTransferMode.preTransferDelayMs() (75 or 200 ms), which previously gated a local sleep after sending ble_photo_ready. That value does not cover a round-trip ble_ready_ack over BLE, so the wait often times out and file packets start without the handshake the Phase 2 change is meant to enforce.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 30f1edf. Configure here.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/modules/bluetooth-sdk/ios/Source/bench/BleBandwidthBench.swift">

<violation number="1" location="mobile/modules/bluetooth-sdk/ios/Source/bench/BleBandwidthBench.swift:12">
P2: Static mutable state (`succeededThisProcess`, `scheduledThisConnection`, `attemptCount`) is read and written from multiple execution contexts (main queue, BLE callbacks, cleanup) without any synchronization or memory barrier. The Android counterpart uses `volatile` on all three fields to guarantee cross-thread visibility, but the iOS version has no equivalent protection. This can cause stale reads, missed updates, or duplicate scheduling in concurrent scenarios.

Example race: `shouldSchedule()` on the main queue reads `succeededThisProcess` while `markSucceeded()` on a BLE thread writes it — without a memory barrier the main thread may see a stale `false` and schedule an extra bench run.</violation>

<violation number="2" location="mobile/modules/bluetooth-sdk/ios/Source/bench/BleBandwidthBench.swift:86">
P3: Retryable error-code matching does not trim whitespace, so valid `CAMERA_BUSY` responses with padding are missed on iOS.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

static let defaultRetryDelayNanos: UInt64 = 5_000_000_000
static let defaultMaxAttempts = 8

private static var succeededThisProcess = false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Static mutable state (succeededThisProcess, scheduledThisConnection, attemptCount) is read and written from multiple execution contexts (main queue, BLE callbacks, cleanup) without any synchronization or memory barrier. The Android counterpart uses volatile on all three fields to guarantee cross-thread visibility, but the iOS version has no equivalent protection. This can cause stale reads, missed updates, or duplicate scheduling in concurrent scenarios.

Example race: shouldSchedule() on the main queue reads succeededThisProcess while markSucceeded() on a BLE thread writes it — without a memory barrier the main thread may see a stale false and schedule an extra bench run.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/modules/bluetooth-sdk/ios/Source/bench/BleBandwidthBench.swift, line 12:

<comment>Static mutable state (`succeededThisProcess`, `scheduledThisConnection`, `attemptCount`) is read and written from multiple execution contexts (main queue, BLE callbacks, cleanup) without any synchronization or memory barrier. The Android counterpart uses `volatile` on all three fields to guarantee cross-thread visibility, but the iOS version has no equivalent protection. This can cause stale reads, missed updates, or duplicate scheduling in concurrent scenarios.

Example race: `shouldSchedule()` on the main queue reads `succeededThisProcess` while `markSucceeded()` on a BLE thread writes it — without a memory barrier the main thread may see a stale `false` and schedule an extra bench run.</comment>

<file context>
@@ -0,0 +1,131 @@
+    static let defaultRetryDelayNanos: UInt64 = 5_000_000_000
+    static let defaultMaxAttempts = 8
+
+    private static var succeededThisProcess = false
+    private static var scheduledThisConnection = false
+    private static var attemptCount = 0
</file context>


static func isRetryableError(errorCode: String?, errorMessage: String?) -> Bool {
if let errorCode, !errorCode.isEmpty {
if errorCode.uppercased() == "CAMERA_BUSY" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Retryable error-code matching does not trim whitespace, so valid CAMERA_BUSY responses with padding are missed on iOS.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/modules/bluetooth-sdk/ios/Source/bench/BleBandwidthBench.swift, line 86:

<comment>Retryable error-code matching does not trim whitespace, so valid `CAMERA_BUSY` responses with padding are missed on iOS.</comment>

<file context>
@@ -0,0 +1,131 @@
+
+    static func isRetryableError(errorCode: String?, errorMessage: String?) -> Bool {
+        if let errorCode, !errorCode.isEmpty {
+            if errorCode.uppercased() == "CAMERA_BUSY" {
+                return true
+            }
</file context>
Suggested change
if errorCode.uppercased() == "CAMERA_BUSY" {
if errorCode.trimmingCharacters(in: .whitespacesAndNewlines).uppercased() == "CAMERA_BUSY" {

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed

Re-trigger cubic

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