Repository navigation
feat(android): add settings bluetooth on|off - #3721
mikolajadamowicz wants to merge 6 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 22 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/daemon/src/execution/handlers/__tests__/snapshot-settings-handler.test.ts">
<violation number="1" location="packages/daemon/src/execution/handlers/__tests__/snapshot-settings-handler.test.ts:248">
P2: This test claims the Bluetooth request is refused before dispatch, but it does not assert the typed dispatch disclosure. Add `details.dispatched === 'no'` so the test distinguishes a true admission refusal from an ambiguous dispatch status; keep the empty mutation assertion as the device-side evidence.</violation>
</file>
| if (response && !response.ok) { | ||
| expect(response.error.code).toBe('UNSUPPORTED_OPERATION'); | ||
| expect(response.error.message).toBe('settings bluetooth is not supported on this device'); | ||
| expect(response.error.details?.reason).toBe('unsupported-platform-leaf'); |
There was a problem hiding this comment.
P2: This test claims the Bluetooth request is refused before dispatch, but it does not assert the typed dispatch disclosure. Add details.dispatched === 'no' so the test distinguishes a true admission refusal from an ambiguous dispatch status; keep the empty mutation assertion as the device-side evidence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/daemon/src/execution/handlers/__tests__/snapshot-settings-handler.test.ts, line 248:
<comment>This test claims the Bluetooth request is refused before dispatch, but it does not assert the typed dispatch disclosure. Add `details.dispatched === 'no'` so the test distinguishes a true admission refusal from an ambiguous dispatch status; keep the empty mutation assertion as the device-side evidence.</comment>
<file context>
@@ -227,6 +227,29 @@ test('settings text-size reads the category the owner holds without mutating any
+ if (response && !response.ok) {
+ expect(response.error.code).toBe('UNSUPPORTED_OPERATION');
+ expect(response.error.message).toBe('settings bluetooth is not supported on this device');
+ expect(response.error.details?.reason).toBe('unsupported-platform-leaf');
+ }
+ expect(fixtureSettingsMutations).toHaveLength(0);
</file context>
| expect(response.error.details?.reason).toBe('unsupported-platform-leaf'); | |
| expect(response.error.details?.reason).toBe('unsupported-platform-leaf'); | |
| expect(response.error.details?.dispatched).toBe('no'); |
There was a problem hiding this comment.
Not fixing this one here. The refusal comes from the shared unsupportedOperationResponse in runtime-admission.ts, and it never sets dispatched. So the router marks every mutating command refused at admission as unknown, settings or not. That needs a fix in admission for all routes, which is a separate PR.
|
The Android bluetooth change is not ready to merge at bfd0cfb. I found three problems to fix first. The public Node client type cannot express bluetooth. The catalog entry uses The read-back code in settings-bluetooth.ts#L113 is a near-copy of the mobile-data code in The new device write path is not tested on a device: Not blocking: the iOS refusal test in snapshot-settings-handler.test.ts#L248 could also assert Open review threads: the dispatched assertion thread still applies and is the non-blocking note above. These four no longer apply, so please resolve them. The --app refusal is fixed because the catalog now declares The one failing check is the iOS simulator E2E smoke scenario. It timed out waiting for the deep-link destination. This diff touches no iOS, deep-link or readiness code, so the failure is likely unrelated. I did not read the queued second Smoke Tests run. There are no conflicts. Before merge, add the catalog-derived |
|
Fixed the client type and the shared read-back in 07e3bb1. bluetooth and mobile-data now share Emulator run at 07e3bb1 on Not covered yet. Every image I have locally declares |
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
More emulator runs at 07e3bb1, on AOSP So both assumptions hold on real builds. Below API 27, Limrun isn't run. I don't have Limrun credentials on this machine. |
|
The Android side of this PR now reads well at 8005f9c. The earlier evidence request is met, and the one open P1 thread (the probe timeout) is fixed. I did not run tsc or vitest, and the live emulator transcripts are the author's, not reproduced by me. I have no Limrun Android run. A Limrun image without android.hardware.bluetooth would refuse the toggle instead of switching it. The branch has merge conflicts with main. Smoke Tests is still queued, with no failure shown, and this diff only touches the Android settings write path and contract types, so nothing links it to that route yet. Not blocking, and you can take or leave these: On the open threads, one P2 still applies: the iOS bluetooth refusal test asserts no Please rebase onto main and let Smoke Tests finish. Both are needed before merge. |
Toggle the Android Bluetooth adapter from `settings`, next to wifi and airplane. QA automation for media playback uses it to stand in for a Bluetooth audio device disconnecting mid-session. - Runs `svc bluetooth enable|disable` from Android 8.1 (API 27). From Android 13 AOSP's `svc` forwards that to `cmd bluetooth_manager`, so one command serves every supported level. - The adapter is proven with `pm list features` before the write, so a device or emulator image without android.hardware.bluetooth is refused with UNSUPPORTED_OPERATION and reason android_bluetooth_adapter_missing, unchanged. Builds older than Android 8.1 are refused with android_bluetooth_command_unavailable, and an unreadable API level fails with the getprop output before any write. - `svc bluetooth` exits 0 when the adapter refuses the change, so the write is confirmed by polling `settings get global bluetooth_on` for up to 2 s (host-kit pollUntilSettled). A state that never arrives fails with COMMAND_FAILED and reason bluetooth-not-applied, or bluetooth-unreadable with the stored value; success reports the resulting `bluetooth` state. - Refusals before the write disclose dispatched: no, failures after it dispatched: unknown. A cancelled request stops before the write. - Apple targets refuse the setting at admission through the existing per-owner facts. - `pm list features` now has one parser, androidFeatureListDeclares, which TV discovery and the Bluetooth check share. The API-level read moves out of the permission module into device-sdk-int.ts and keeps its result. The three copies of the underFakeTimers test helper become one in test-utils.
…ctly - bluetooth is device-wide, so the catalog declares it the way mobile-data does: the app rides the request input and a named app is refused with setting_app_not_consumed before dispatch. - readAndroidSdkInt accepts only a whole positive integer. parseInt read `27garbage` as 27, which would let the Bluetooth gate dispatch on an unreadable API level. - The docs and changelog say the old-build refusal applies only once the adapter check passes, and describe bluetooth-unreadable as an unrecognized bluetooth_on value.
settings.ts imported settings-bluetooth.ts statically, so importing mechanics.ts also evaluated settings-bluetooth.ts and the inventory-parsers.ts it pulls in. The eager-closure budget caught the growth (176 to 179 modules). The bluetooth case now imports the module when it runs, the way device-boot and lifecycle load theirs.
…ce-wide client arms
- mobile-data and bluetooth now go through one setAndroidGlobalToggle in
settings-global-toggle.ts: the svc write, the abort check, the
settings-global read-back poll, the typed settle error, and dispatch
disclosure. Each setting only declares its service, row, value map,
reasons, interval and hint. mobile-data gains the exit-code handling
bluetooth already had, and settings-bluetooth.ts keeps only the
adapter and API-level preflight.
- SettingsUpdateOptions derives one arm for every catalog entry with
app: 'input' and a device-level mobile scope, replacing the
hand-written mobile-data arm. client.settings.update({ setting:
'bluetooth', state: 'off' }) now type-checks. A type test asserts every
SettingName has an arm.
pollUntilSettled sleeps up to the deadline and then samples once more. Each `settings get global` read took the deadline's remaining time as its adb timeout, so that last read ran with a 1 ms budget and failed as an adb timeout instead of reaching the typed not-applied reason. Reads now get a fixed 1 s budget; the 2 s settle window still bounds polling.
AndroidGlobalToggle is generic over its stateKey, so setAndroidBluetooth
returns { bluetooth } and the mobile-data toggle { mobileData } again
instead of a string-keyed record.
8005f9c to
8e302d9
Compare
|
Rebased onto main at 77137b0. The new head is 8e302d9 and has no conflicts. I took the typed-result suggestion. I skipped the CHANGELOG clause. Heads-up: main doesn't typecheck right now. |
|
The code is in good shape. The earlier findings from 8005f9c are now fixed at 8e302d9, and I found no new problems in the code. The red checks look unrelated to this PR. Lint/Typecheck, Repo Guards, Integration and the Android catalog smoke fail on On the open review threads: the P2 thread on the dispatched assertion still applies (#3721 (comment)). The empty I did not run tsc or vitest on 8e302d9. The delta is type-only, and the author reports a local typecheck with declarations.ts:558 patched. I did not reproduce the emulator transcripts from the earlier round. There is no Limrun Android run, so an image without android.hardware.bluetooth would refuse the toggle. I did not re-check the mobile-data shipping claim with Nothing in this PR holds up the merge. Once #3740 fixes the typecheck on main, please rerun CI, including the iOS runner preflight smoke. |
This branch has not been deployed
2 participants
Summary
Adds
settings bluetooth on|offfor Android over adb, local and Limrun, next towifi,airplaneandmobile-data. Turning the adapter off lets a test drop a Bluetooth headset mid-playback.svc bluetooth enable|disablefrom Android 8.1, API 27. On Android 13+svcforwards tocmd bluetooth_manager, so one command covers every level.svc bluetoothexits 0 even when the adapter refuses, so the change is confirmed by pollingbluetooth_onfor up to 2 s. If it never settles it fails withCOMMAND_FAILEDand reasonbluetooth-not-appliedorbluetooth-unreadable, like mobile-data.dispatched: no, withoutandroid.hardware.bluetooth, below 8.1, on an unreadable API level, or after cancellation. Other targets refuse it before anything runs.pm list featuresgets one parser, shared with TV discovery. The API-level read moves todevice-sdk-int.ts, and threeunderFakeTimershelpers become one.22 files.
Validation
At
8e302d9e, on77137b01, format, lint, layering, fallow, build, command-docs and related unit tests pass. Typecheck fails only intest/integration/command-coverage/declarations.ts, which is unchanged from main and broken there too. Coverage, integration and smoke run in CI.Unit tests cover each refusal with the adb calls it made, the read-back, dispatch disclosure and cancellation. Reverting each fix fails at least one.
On an API 37 emulator at
07e3bb1f,settings bluetooth offandonreportdisabledandenabled, andbluetooth_onreads 0 then 1. On API 27 and API 26 images withoutandroid.hardware.bluetooth, it refuses withandroid_bluetooth_adapter_missinganddispatched: noand leavesbluetooth_onunchanged. Rawsvc bluetoothexits 0 there without doing anything, which is why the adapter and API checks run first. Not covered: Limrun.