Gate archival photo capture on the absence of a BLE transfer id - #3616
Gate archival photo capture on the absence of a BLE transfer id#3616PhilippeFerreiraDeSousa wants to merge 1 commit into
Conversation
A take_photo with save=true and no webhookUrl currently always routes to the local-save-only capture path, even when it carries a bleImgId — which means a phone-delivery request that also wants a gallery copy would save on the glasses and never start the BLE transfer, leaving the phone to time out. A request with a bleImgId has a delivery leg, so only treat save-without-webhook as an archival capture when there is also no bleImgId. No behavior change for any shape phones send today (the phone app always supplies a webhookUrl, and archival captures from BT-SDK callers carry no bleImgId).
📋 PR Review Helper📱 Mobile App Build⏳ Waiting for build... 🕶️ ASG Client Build❌ Build failed (commit 🔀 Test Locallygh pr checkout 3616 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15eaf44bf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A request carrying a bleImgId DOES have a delivery leg (BLE to the phone) and | ||
| // must go down the normal pipeline even with save=true and no webhookUrl: that | ||
| // shape is phone-delivery with a kept gallery copy, not an archival capture. | ||
| if (save && webhookUrl.isEmpty() && bleImgId.isEmpty()) { |
There was a problem hiding this comment.
When a current Bluetooth SDK caller uses the supported requestPhoto({save: true, webhookUrl: null, ...}) shape, both Mentra Live serializers still generate a nonempty bleImgId even though no delivery was requested, and transferMethod defaults to auto. This condition therefore stops routing that request to takePhotoForLocalSave; it instead attempts an upload to an empty URL or an untracked BLE transfer because the phone only registers blePhotoTransfers when a webhook is present, causing the request to fail or time out instead of returning the local capture response. Gate this exception on an explicit BLE delivery method/destination rather than on the automatically generated fallback ID alone.
AGENTS.md reference: AGENTS.md:L136-L138
Useful? React with 👍 / 👎.
1 participant
Scope
Part 2a of OS-1796: a one-line routing change in
PhotoCommandHandlersosave=trueand a BLE transfer can coexist.Today
save && webhookUrl.isEmpty()unconditionally routes to the local-save-only capture path. The upcoming phone-delivery destination in the Bluetooth SDK sendstransferMethod: ble+bleImgId+save(for a kept glasses-gallery copy) with nowebhookUrl— on current firmware that shape would archive on the glasses and never start the BLE transfer, leaving the phone to time out. A request carrying ableImgIdhas a delivery leg, so the archival route now additionally requires an emptybleImgId.No behavior change for any request shape phones send today: the phone app always supplies a
webhookUrl, and archival captures from direct BT-SDK callers carry nobleImgId.This needs to roll out via the firmware train before the SDK-side destination change ships (release ordering only; both directions remain compatible in the interim).
Test evidence
:app:testDebugUnitTest --tests "*PhotoCommandHandlerTransferMethod*"— 6 tests, 0 failures, including two new cases: save+bleImgId+no-webhook ridestakePhotoForBleTransfer(and never the local-save path), and save-only without bleImgId still routes totakePhotoForLocalSave.Note
Medium Risk
Changes photo command routing on firmware; wrong gating could break BLE delivery or archival saves, but scope is a single condition and is covered by new unit tests.
Overview
Archival vs BLE routing for
take_photois tightened sosave=truewith an emptywebhookUrlno longer always uses the local-save-only path.The local-save shortcut now requires
bleImgIdto be empty as well. Requests withsave, no webhook, but ableImgId(e.g.transferMethod: blefrom the BT SDK for phone delivery plus a gallery copy) go through the normal capture pipeline andtakePhotoForBleTransferinstead of being treated as glasses-only archival captures that never start BLE.Tests add two unit cases: save +
bleImgId+ no webhook must call BLE transfer and never local save; save with no webhook and nobleImgIdstill uses local save.Reviewed by Cursor Bugbot for commit 15eaf44. Bugbot is set up for automated code reviews on this repo. Configure here.