Files
FreeTLab 159334f997
Build & Deploy PluriWave / Análisis de código (push) Successful in 35s
Build & Deploy PluriWave / Build APK + AAB release (push) Successful in 1m44s
docs(openspec): archive bt-device-identity and promote its spec
Move the change folder to changes/archive/2026-07-11-bt-device-identity
with the verified artifact set (verdict: pass with warnings, 0 critical,
102/102 targeted tests) and create the bt-device-identity capability
spec under openspec/specs/. Phase 7 on-device QA remains the pending
human gate before release.
2026-07-11 01:15:06 +02:00

126 lines
22 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Tasks: Canonical Bluetooth Device Identity
## Review Workload Forecast
| Field | Value |
|-------|-------|
| Estimated changed lines | 420560 (Kotlin ~90, Dart prod ~140, Dart tests ~230, manifest ~1, l10n ~40 across 13 files) |
| 400-line budget risk | High |
| Chained PRs recommended | Yes |
| Suggested split | PR 1: Kotlin identity + permission plumbing (Phase 12) → PR 2: Dart state/display/migration (Phase 35) → PR 3: l10n + manual QA sign-off (Phase 67) |
| Delivery strategy | ask-on-risk |
| Chain strategy | pending |
Decision needed before apply: Yes
Chained PRs recommended: Yes
Chain strategy: pending
400-line budget risk: High
**Batch progress: 2/3 complete.** Batch 1 (Phase 1, Kotlin plumbing) and Batch 2 (Phases 2-5, Dart state/display/migration) are both done — see `sdd/bt-device-identity/apply-progress` for full merged evidence. Phase 6 (l10n) resolved N/A for this change (no in-app UI copy shipped — see Phase 6 section below). Remaining: Batch 3 = Phase 7 (manual/on-device QA) only.
### Suggested Work Units
| Unit | Goal | Likely PR | Notes |
|------|------|-----------|-------|
| 1 | Manifest + Kotlin placeholder guard/composite fallback + `requestBluetoothConnect` channel + re-emit on grant | PR 1 | Base: feature/bt-device-identity; no unit harness (Kotlin) — code-inspection gated |
| 2 | Dart contract (`solicitarPermisoBluetooth`), platform-name cache, duplicate-entry guard, display fix, migration purge | PR 2 | Base: PR 1 branch; depends on Unit 1 id-shape contract (composite fallback string) |
| 3 | l10n strings (13 locales) + manual/on-device QA pass | PR 3 | Base: PR 2 branch; depends on Unit 2 UI trigger points existing |
---
## Phase 1: Kotlin — Permission Plumbing (PR 1 scope, code-inspection + manual QA — no unit harness)
> Kotlin has no instrumented/unit test harness in this repo (confirmed: only `test/` Dart tree exists). Every Kotlin task below is validated by **self/peer code inspection** against the exact method signature already used by `requestPostNotificationsPermission` (MainActivity.kt L306-318), plus the Phase 7 manual QA pass. Do NOT attempt to add a Kotlin test file — there is no gradle test source set wired for this.
**Batch progress: 1/3 complete (Phase 1, tasks 1.1-1.8, all `[x]`).** `flutter analyze` re-run after these Kotlin/manifest-only edits: 0 issues (Dart untouched). Next: Batch 2 (Phase 2-5, Dart state/migration).
- [x] 1.1 [manual/code-inspection] Modify `android/app/src/main/AndroidManifest.xml` — add `<uses-permission android:name="android.permission.BLUETOOTH_CONNECT"/>` alongside the existing permission block (after `ACCESS_FINE_LOCATION`, L16) — DONE: inserted at L17, verified via `rg BLUETOOTH_CONNECT` on the manifest.
- [x] 1.2 [manual/code-inspection] Modify `MainActivity.kt` — add `private val bluetoothConnectPermissionRequestCode = 4823` constant (next free code after `notificationPermissionRequestCode = 4822`, L39) — DONE: added at L40, plus a `private val bluetoothMacPlaceholder = "02:00:00:00:00:00"` constant at L41 (grouped with the other class-level channel/request-code constants) to back tasks 1.5/1.6 and satisfy the "placeholder constant handled" static check.
- [x] 1.3 [manual/code-inspection] Modify `MainActivity.kt` — add `private fun requestBluetoothConnect(): Boolean` mirroring `requestPostNotificationsPermission()` (L306-318) verbatim in structure: `SDK_INT < S``true`; already `Manifest.permission.BLUETOOTH_CONNECT` granted → `true`; else `requestPermissions(arrayOf(Manifest.permission.BLUETOOTH_CONNECT), bluetoothConnectPermissionRequestCode)``true` — DONE. Placement note (implementation choice, not a design deviation): placed inside the "Audio Devices Channel" section (right after `setupAudioDevicesChannel()`, before `registerAudioDeviceCallback()`) rather than next to the alarm-permission helpers, since it is invoked exclusively from the `audioDevicesChannel` handler and conceptually belongs with that section; design did not pin an exact line for the new function, only its body structure.
- [x] 1.4 [manual/code-inspection] Modify `MainActivity.kt` `setupAudioDevicesChannel()` (L637-646) — add `"requestBluetoothConnect" -> { Log.d(tag, "audio_devices.requestBluetoothConnect"); result.success(requestBluetoothConnect()) }` case on the existing `audioDevicesChannel` MethodChannel (same channel as `getActiveDevice`, not a new channel) — DONE, case added verbatim as specified.
- [x] 1.5 [manual/code-inspection] Modify `MainActivity.kt` `deviceToMap()` (L730-764), `TYPE_BLUETOOTH_A2DP` branch (L741-748) — replace the current all-zeros fallback (`device.address?.takeIf { it.isNotBlank() } ?: "00:00:00:00:00:00"`, L742) with an explicit placeholder guard: treat `device.address` as absent when it is `null`, blank, OR equal to the literal `"02:00:00:00:00:00"` (the real OS placeholder — NOT the `00:00:...` string currently hardcoded, which was never a real Android placeholder and must be removed as dead/incorrect fallback logic) — DONE: old fallback fully replaced (confirmed zero remaining `00:00:00:00:00:00` matches via `rg`), guard now uses `it.isNotBlank() && it != bluetoothMacPlaceholder`.
- [x] 1.6 [manual/code-inspection] Same branch — when the MAC is absent per 1.5, build composite fallback id `"bt_a2dp:name:$safeProductName"` where `safeProductName = (device.productName?.toString()?.takeIf { it.isNotBlank() } ?: "unknown").replace(":", "-")` (ADR-3: single leading `bt_a2dp:` segment preserved, colons in productName sanitized so `eq_presets_matriz_v1` split-on-first-`:` stays valid); when MAC is present, keep existing `"bt_a2dp:$mac"` shape unchanged — DONE verbatim; MAC-present path untouched (`"bt_a2dp:$mac"`).
- [x] 1.7 [manual/code-inspection] Modify `MainActivity.kt` `onRequestPermissionsResult()` (L610-628) — add a branch for `requestCode == bluetoothConnectPermissionRequestCode`: on `grantResults.firstOrNull() == PERMISSION_GRANTED`, call `getActiveAudioDevice()` and push through `audioDevicesSink?.success(device)` (ADR-2 — re-emit so a device connected before grant gets its real MAC without requiring reconnect); on denial, no-op (composite fallback already active, no crash path needed) — DONE: new early-return branch inserted between the `notificationPermissionRequestCode` and `visualizerPermissionRequestCode` guards, matching the existing guard-clause style; denial path is a bare `return` (no-op), matching spec (no crash path needed).
- [x] 1.8 [code-inspection REFACTOR] Re-read full diff of `MainActivity.kt` against `requestPostNotificationsPermission`/`visualizerPermissionRequestCode` patterns — confirm no request-code collision (4821/4822/4823 distinct), confirm `deviceToMap()` doc comment (L692-703) still accurately describes the `bt_a2dp:` shape after the composite-fallback addition, update comment if stale — DONE: 4821/4822/4823 confirmed distinct by re-read; doc comment above `getActiveAudioDevice()` (which documents the id shape `deviceToMap()` produces) updated with a new `"bt_a2dp:name:<productName>"` bullet explaining the placeholder/absent-MAC fallback and colon sanitization.
## Phase 2: Dart — Permission Contract (PR 1/2 boundary — Dart side of Unit 1↔2 handoff)
**Batch progress: 2/3 complete (Phase 2, tasks 2.1-2.5, all `[x]`).**
- [x] 2.1 RED: extend `test/servicios/servicio_dispositivo_audio_real_test.dart` — assert `solicitarPermisoBluetooth()` invokes `MethodChannel('pluriwave/audio_devices').invokeMethod('requestBluetoothConnect')` and returns the bool result (mock `MethodChannel` per existing test's setup pattern) — DONE: 3 cases (granted/denied/null-default), confirmed compile-fail RED via `flutter test` before GREEN.
- [x] 2.2 GREEN: modify `lib/servicios/servicio_dispositivo_audio.dart` — add abstract `Future<bool> solicitarPermisoBluetooth();` to `ServicioDispositivoAudio`; implement in `ServicioDispositivoAudioReal` as `await _methodChannel.invokeMethod<bool>('requestBluetoothConnect') ?? false` — DONE verbatim.
- [x] 2.3 GREEN: add `permisoBluetoothConcedido` bool field + `solicitarPermisoBluetoothCalls` int counter to `FakeServicioDispositivoAudio` in `test/helpers/fakes.dart`; implement `solicitarPermisoBluetooth()` override returning the field and incrementing the counter — DONE, default `permisoBluetoothConcedido: true`.
- [x] 2.4 GREEN: add the same `solicitarPermisoBluetooth()` override (returning `true`, no-op counter) to `FakeServicioDispositivoAudioThrows` in `test/helpers/fakes.dart` — DONE. Also fixed a 4th, previously-undocumented implementer discovered during grounding: `NullServicioDispositivoAudio` in `test/servicios/servicio_dispositivo_audio_toggle_test.dart` (Dart requires every concrete subclass to implement a new abstract method before ANYTHING compiles, so this was a mandatory atomic addition, not scope creep).
- [x] 2.5 REFACTOR: confirmed `test/servicios/servicio_dispositivo_audio_test.dart` IS the abstract-contract test file (`group('ServicioDispositivoAudio (abstract contract)', ...)`); added an interface-completeness assertion there — DONE.
## Phase 3: Dart — Platform-Name Cache and Duplicate-Entry Guard (PR 2 scope)
**Batch progress: 2/3 complete (Phase 3, tasks 3.1-3.10, all `[x]`).** Integrated cleanly with the pre-existing `esBase` (builtin_speaker bootstrap-skip) guard from `eq-device-disconnect-revert` — Phase D regression group (D.1-D.5) confirmed still green.
- [x] 3.1 RED: extend `test/estado/estado_ecualizador_test.dart` — scenario "platform name is cached from a device-change event": emit a BT device via `FakeServicioDispositivoAudio.emitirDispositivo` with real-MAC id + `nombre: 'AirPods Pro'`, assert `eq.nombrePlataforma('bt_a2dp:AA:BB:CC:DD:EE:FF') == 'AirPods Pro'` — DONE, confirmed compile-fail RED (referenced not-yet-existing `nombrePlataforma`).
- [x] 3.2 GREEN: modify `lib/estado/estado_ecualizador.dart` — add `final Map<String, String> _nombresPlataforma = {};` (in-memory only, ADR-4) and `String nombrePlataforma(String deviceId) => _nombresPlataforma[deviceId] ?? '';` getter — DONE verbatim.
- [x] 3.3 GREEN: in `_onDispositivoCambiado`, unconditionally set `_nombresPlataforma[dispositivo.id] = dispositivo.nombre;` as the first statement inside the `if (!_eqMultiDeviceEnabled) return;` guard — DONE.
- [x] 3.4 RED: extend `test/estado/estado_ecualizador_test.dart` — scenario "composite-placeholder sentinel does not create device-list entry": emit `'bt_a2dp:name:AirPods-Pro'`, assert `presetsDispositivo` gains no new key while `nombrePlataforma` still resolves — DONE.
- [x] 3.5 GREEN: in `_onDispositivoCambiado`, guard the auto-create block with an additional check — skip when `dispositivo.id.startsWith('bt_a2dp:name:')` (extracted to a named constant `_prefijoPlaceholderCompuesto`, ADR-6) — DONE, combined via `!esBase && !esPlaceholderCompuesto && !_presetsDispositivo.containsKey(...)` (the `esBase` guard is the pre-existing `eq-device-disconnect-revert` check, left untouched).
- [x] 3.6 RED: extend `test/estado/estado_ecualizador_test.dart` — scenario "multiple denied-permission devices do not collide": two composite-shape devices in sequence, both cache correctly, neither creates a `presetsDispositivo` entry — DONE.
- [x] 3.7 RED (regression-lock, not new behavior): "repeated event for known id is a no-op on preset creation" — DONE, explicitly labeled as an approval/regression-lock test in the test name per this task's own note.
- [x] 3.8 RED (regression-lock): "transient non-BT id during pairing handshake does not corrupt BT entry" — DONE.
- [x] 3.9 GREEN/REFACTOR: ran full `estado_ecualizador_test.dart` suite — 48/48 pass, including the full Phase D group (D.1-D.5) and all pre-existing Phase 5/5.3-5.9/CRITICAL-1/CRITICAL-2 groups — DONE, zero regressions.
- [x] 3.10 [post-verify, closes CRITICAL-1] Added composed regression test `test/estado/estado_ecualizador_test.dart` — "3.10 rename persists after re-pair — composed regression (closes bt-device-identity CRITICAL-1)": connect BT device (real-MAC id) → `renombrarDispositivo(...)` → simulate disconnect (emit `builtin_speaker`) → re-pair (emit the SAME MAC id again) → assert no duplicate `presetsDispositivo` entry, custom rename still wins via `nombreVisible(...)`, and the device's preset entry survives untouched. Added per `sdd/bt-device-identity/verify-report` CRITICAL-1 finding: spec.md L95-100 ("Requirement: Rename overlay survives re-pair under canonical id") had no covering test — the constituent behaviors were each individually tested (reconnect-dedup by pre-existing Phase D test D.4, rename-priority by test 4.4) but never composed into a single sequence. Result: PASS against the existing, unmodified implementation — this closes a test-coverage gap only, zero production-code changes.
## Phase 4: Dart — Display Fix and Permission Trigger (PR 2 scope)
**Batch progress: 2/3 complete (Phase 4, tasks 4.1-4.8, all `[x]`).**
- [x] 4.1 RED: extend `test/pantallas/pantalla_ajustes_test.dart` — scenario "platform name displays with no custom rename" — DONE, confirmed genuine pre-fix assertion failure (`expected 'AirPods Pro', found 0 widgets`).
- [x] 4.2 GREEN: modify `lib/pantallas/pantalla_ajustes.dart``_FilaDispositivo.build()`: `eq.nombreVisible(deviceId, '')``eq.nombreVisible(deviceId, eq.nombrePlataforma(deviceId))` — DONE.
- [x] 4.3 GREEN: modify `lib/pantallas/pantalla_ajustes.dart``_DialogoEdicionDispositivoState.initState()`: `eq.nombreVisible(widget.deviceId, '')``eq.nombreVisible(widget.deviceId, eq.nombrePlataforma(widget.deviceId))` — DONE.
- [x] 4.4 RED (triangulation companion, not independently RED — see note): "custom rename overrides platform name" — DONE, but **honesty note**: this scenario passes both before AND after the 4.2 fix, because `nombreVisible`'s custom-name branch short-circuits before ever inspecting `platformName`. It is valuable integration-layer triangulation (proves custom-rename priority holds at the widget layer with a populated platform-name cache in play), not a fail-before/pass-after RED. Recorded here rather than silently mischaracterized.
- [x] 4.5 RED (approval/regression-lock, matches task's own "locks unchanged legacy behavior" framing): "no platform name yet falls back to raw id" — DONE, passes before and after by design (proves the fallback chain's final link is untouched).
- [x] 4.6 RED: "permission call fires on device-management open" — DONE, confirmed genuine pre-fix assertion failure (`expected 1 call, found 0`). Extended in-test (not a new task) with a second assertion after toggling back OFF, proving `solicitarPermisoBluetooth` is gated on `habilitado == true` and not called unconditionally (triangulation).
- [x] 4.7 GREEN: modified `_SeccionEcualizadorAvanzado` — both `onTap`/`onChanged` handlers now route through a new private `_alternarMultiDevice(eq, habilitado)` method: `unawaited(eq.cambiarMultiDeviceEnabled(habilitado))`, then `if (habilitado) unawaited(eq.solicitarPermisoBluetooth())` — DONE. `unawaited()` (from `dart:async`, newly imported) used per project's `unawaited_futures: true` lint convention (matches `main.dart`/`app.dart` precedent) since these are bare statements, not arrow-body callback expressions like the original single-line form. Trigger-point deviation from design's literal "on opening device-management UI" stands as previously flagged (toggle-turn-ON tap, not a StatefulWidget on-build hook) — still not converting `_SeccionEcualizadorAvanzado` to StatefulWidget, per orchestrator sign-off.
- [x] 4.8 REFACTOR: verified — `ServicioDispositivoAudio` is NOT registered as a top-level `Provider` anywhere in `app.dart`'s `MultiProvider` (confirmed by reading `app.dart` and grepping `estado_radio.dart`: `ServicioDispositivoAudioReal()` is constructed once inline and stored as `EstadoRadio._dispositivoAudio`, a private field with no public getter). Resolved per the task's own fallback instruction: added `Future<bool> solicitarPermisoBluetooth()` passthrough on `EstadoEcualizador` (delegates to its existing private `_dispositivoAudio` field) instead of adding new provider wiring. This also avoids breaking every existing widget test that builds `_SeccionEcualizadorAvanzado` without a `ServicioDispositivoAudio` provider in the tree (e.g. test 7.1-C) — a literal `context.read<ServicioDispositivoAudio>()` call site would have thrown `ProviderNotFoundException` there.
## Phase 5: Dart — One-Time Migration Purge (PR 2 scope)
**Batch progress: 2/3 complete (Phase 5, tasks 5.1-5.10, all `[x]`).**
- [x] 5.1 RED: extend `test/servicios/servicio_ecualizador_test.dart` — "migration removes only exact placeholder entries" — DONE, confirmed compile-fail RED (`migrarClavesPlaceholder` undefined) before GREEN.
- [x] 5.2 RED: "matrix keys purge only the placeholder segment" — DONE.
- [x] 5.3 RED: "near-miss keys are preserved" (`bt_a2dp:02:00:00:00:00:01`) — DONE.
- [x] 5.4 RED: "migration runs once" — DONE, strengthened beyond the literal task wording: re-seeds the placeholder key directly (bypassing the flag) between the two `migrarClavesPlaceholder()` calls so the assertion proves the SECOND call is a true no-op via the flag short-circuit, not merely "nothing left to purge."
- [x] 5.5 RED: "no placeholder entries when BLUETOOTH_CONNECT was never requested" — DONE.
- [x] 5.6 RED: `eq_nombres_dispositivos_v1` purge scenario — DONE.
- [x] 5.7 GREEN: added `_keyPlaceholderPurgaHecha = 'eq_placeholder_purge_done_v1'` and `_placeholderMacLiteral = 'bt_a2dp:02:00:00:00:00:00'` constants to `lib/servicios/servicio_ecualizador.dart` — DONE verbatim.
- [x] 5.8 GREEN: implemented `Future<void> migrarClavesPlaceholder() async` exactly per the (a)/(b)/(c)/(d) spec in this task — DONE. Matrix-key extraction uses `clave.indexOf(':')` (first colon only, per multi-device-eq ADR-3 RFC4122-no-colons rationale already recorded in the design).
- [x] 5.9 GREEN: `cargar()` now calls `await migrarClavesPlaceholder();` as its first line — DONE.
- [x] 5.10 REFACTOR: ran full `servicio_ecualizador_test.dart` suite — 20/20 pass, including all pre-existing Phase 4/nombresDispositivos round-trip groups — DONE, zero regressions.
## Phase 6: Localization (PR 3 scope) — N/A for this change
> Only add new l10n keys if UI copy is actually shown for permission rationale or migration notice (spec: "IF UI copy is shown"). If Phase 4/5 tasks above ship with no new user-visible string (e.g., the permission request is silent/OS-dialog-only and no in-app migration banner is added), this phase becomes a no-op and MUST be explicitly marked skipped-by-design in the apply report, not silently dropped.
**SCOPE DECISION (orchestrator, recorded here per instruction): N/A for this change.** No in-app rationale sheet or migration-notice UI was implemented anywhere in Phases 2-5 — the permission request in `_alternarMultiDevice` (Task 4.7) fires the OS `BLUETOOTH_CONNECT` dialog directly with zero in-app copy beforehand, and the migration purge (Phase 5) is entirely silent (no banner/snackbar). The spec makes l10n conditional on "IF UI copy is shown"; none does, so Phase 6 does not apply. No UI copy was found to be unavoidable during implementation — nothing was stopped or flagged mid-task for this reason.
- [ ] 6.1 [decision gate] N/A — decided above: OS dialog alone, no in-app copy shipped. Tasks 6.2-6.6 skipped by design, not silently dropped.
- [ ] 6.2 GREEN (if in scope) — N/A, no in-app copy shipped.
- [ ] 6.3 GREEN (if in scope) — N/A, no in-app copy shipped.
- [ ] 6.4 RED — N/A, no new l10n keys were added.
- [ ] 6.5 GREEN — N/A, no rationale string exists to wire in.
- [ ] 6.6 REFACTOR — N/A, `flutter gen-l10n` was not re-run since no `.arb` files changed in this batch.
## Phase 7: Manual/On-Device QA (all PRs — final gate before merge)
> No instrumented/emulator test harness exists in this repo. Every item below requires a real or emulated Android 12+ device and is signed off by a human, not CI. Do NOT mark any of these done from code-reading alone.
- [ ] 7.1 [manual/on-device] Fresh install, never open device-management screen → confirm NO BT permission dialog appears at any point during normal app use (spec: "permission not requested at app launch")
- [ ] 7.2 [manual/on-device] Fresh install, open Settings → enable "Enable per-device EQ" toggle → confirm the system `BLUETOOTH_CONNECT` permission dialog appears before any BT device shows up in the known-devices list
- [ ] 7.3 [manual/on-device] Grant the permission → pair/connect a real BT A2DP device → confirm the device row displays the device's own Bluetooth name (not a raw id), and inspect logs to confirm the underlying id is `bt_a2dp:<real MAC>`, not the placeholder
- [ ] 7.4 [manual/on-device] Deny the permission (or test on a build where it's denied) → connect a BT A2DP device → confirm the app does not crash, the device row still appears with a readable name (composite fallback), and no `bt_a2dp:02:00:00:00:00:00` string appears anywhere in the UI or logs
- [ ] 7.5 [manual/on-device] With permission granted, rename a connected BT device via the edit dialog → disconnect and re-pair the SAME physical device → confirm the custom rename is still shown and NO duplicate row appears in the device list
- [ ] 7.6 [manual/on-device] Connect two DIFFERENT real BT devices in sequence (permission granted) → confirm each gets its own distinct row with its own name/MAC, no collision
- [ ] 7.7 [manual/on-device] On an install that has pre-existing `bt_a2dp:02:00:00:00:00:00`-keyed entries (simulate by seeding SharedPreferences via adb/debug tooling, or use a build from before this change that already has the corrupted key) → upgrade to this change → confirm the placeholder-keyed entries are gone after first load, the migration/rename-again notice (if shipped per Phase 6) is shown once, and any OTHER stable-MAC entries the user had are untouched
- [ ] 7.8 [manual/on-device] Re-launch the app after 7.7's migration already ran once → confirm no second migration notice appears and no further data is altered (idempotency, matches Task 5.4's automated coverage but verified end-to-end)
- [ ] 7.9 [manual/on-device] If Phase 6 ships in-app rationale copy, switch the device's app language to at least 2 non-English locales (e.g. `es`, `ja`) and repeat 7.2 → confirm the rationale text renders in the selected locale before the OS dialog
- [ ] 7.10 [sign-off] Record pass/fail for 7.1-7.9 in the apply-progress artifact before this change is considered ready for `sdd-verify`