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.
This commit is contained in:
@@ -0,0 +1,227 @@
|
||||
# Archive Report: Canonical Bluetooth Device Identity
|
||||
|
||||
**Change**: bt-device-identity
|
||||
**Archived**: 2026-07-11
|
||||
**Status**: ARCHIVED AND CLOSED
|
||||
**Archive Path**: `openspec/changes/archive/2026-07-11-bt-device-identity/`
|
||||
**Spec Merged To**: `openspec/specs/bt-device-identity/spec.md` (new capability domain)
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The **bt-device-identity** change is now archived with a **PASS WITH WARNINGS** verification verdict. All code-gated implementation work is complete across Phases 1–5 (41 of 57 tasks done, 6 N/A by design). Phase 7 (manual/on-device QA, 10 tasks, human sign-off gate) remains pending as the mandatory pre-shipping gate. The change restores stable Bluetooth device identity by acquiring `BLUETOOTH_CONNECT` at point-of-intent, guarding against the OS placeholder MAC, caching per-device names, and migrating corrupted placeholder-keyed entries. All test suites pass (102/102 tests), `flutter analyze` reports 0 issues, and the re-verify phase closed the single CRITICAL finding (CRITICAL-1: untested "rename survives re-pair" scenario — now covered by new task 3.10 composed regression test that passes against existing code).
|
||||
|
||||
---
|
||||
|
||||
## Artifact Traceability
|
||||
|
||||
| Artifact | Engram Observation ID | Location | Notes |
|
||||
|----------|----------------------|----------|-------|
|
||||
| Proposal | #2299 | sdd/bt-device-identity/proposal | Root cause analysis, scope, approach, risks |
|
||||
| Spec | #2304 | sdd/bt-device-identity/spec; merged to openspec/specs/bt-device-identity/spec.md | 7 requirement groups, 21 scenarios; on-disk version (openspec/changes/bt-device-identity/spec.md) reconciled and merged to new domain |
|
||||
| Design | #2305 | sdd/bt-device-identity/design | 6 ADRs, technical approach, file changes, testing strategy |
|
||||
| Explore | (embedded) | sdd/bt-device-identity/proposal (no separate observe) | Root cause evidence, bug analysis |
|
||||
| Tasks | #2314 | sdd/bt-device-identity/tasks | 57 checkboxes: 41 complete (Phase 1–5), 6 N/A (Phase 6), 10 pending (Phase 7 human QA) |
|
||||
| Apply-Progress | #2328 | sdd/bt-device-identity/apply-progress | Batches 1/2/2b complete (Phases 1–5); Batch 3 (Phase 7) pending; full merged evidence of all 41 tasks |
|
||||
| Verify-Report | #2329 | sdd/bt-device-identity/verify-report | Initial FAIL (CRITICAL-1 untested scenario); re-verify PASS WITH WARNINGS (CRITICAL-1 closed via task 3.10); 15/21 scenarios COMPLIANT, 4/21 DEFERRED (manual QA), 2/21 N/A (l10n justified), 0/21 still UNTESTED |
|
||||
|
||||
---
|
||||
|
||||
## Implementation Completion
|
||||
|
||||
### Phases 1–5: Code-Gated Work — COMPLETE
|
||||
|
||||
| Phase | Description | Tasks | Status | Evidence |
|
||||
|-------|-------------|-------|--------|----------|
|
||||
| 1 | Kotlin permission plumbing (manifest, placeholder guard, composite fallback, permission request, re-emit) | 1.1–1.8 (8) | 8/8 [x] | Code-inspection verified; `flutter analyze` 0 issues |
|
||||
| 2 | Dart permission contract (abstract method, fakes, interface tests) | 2.1–2.5 (5) | 5/5 [x] | All 3 fake implementers updated; contract tests pass |
|
||||
| 3 | Dart platform-name cache + duplicate-entry guard (in-memory map, composites don't auto-create, regression tests) | 3.1–3.10 (10) | 10/10 [x] | 49/49 estado_ecualizador_test.dart pass (incl. post-verify task 3.10) |
|
||||
| 4 | Dart display fix + permission trigger (call sites updated, permission fires on toggle-ON) | 4.1–4.8 (8) | 8/8 [x] | 11/11 pantalla_ajustes_test.dart pass; provider wiring resolved via passthrough |
|
||||
| 5 | Dart one-time migration purge (guarded flag, exact-literal placeholder removal, idempotent) | 5.1–5.10 (10) | 10/10 [x] | 20/20 servicio_ecualizador_test.dart pass; migration isolation verified |
|
||||
|
||||
### Phase 6: Localization — N/A (By Design)
|
||||
|
||||
| Phase | Description | Tasks | Status | Rationale |
|
||||
|-------|-------------|-------|--------|-----------|
|
||||
| 6 | Localization (l10n keys for all 13 locales IF UI copy is shown) | 6.1–6.6 (6) | N/A | No in-app rationale sheet or migration-notice UI exists; OS `BLUETOOTH_CONNECT` dialog only; migration purge entirely silent; Phase 6 condition ("IF UI copy is shown") not met |
|
||||
|
||||
### Phase 7: Manual/On-Device QA — PENDING (Human Gate)
|
||||
|
||||
| Phase | Description | Tasks | Status | Notes |
|
||||
|-------|-------------|-------|--------|-------|
|
||||
| 7 | Manual/on-device QA (permission dialog flow, real MAC after grant, rename survives re-pair, no duplicates, migration idempotency, locale rendering if Phase 6 ships) | 7.1–7.10 (10) | 0/10 [ ] | Mandatory human sign-off gate; requires real/emulated Android 12+ device; Kotlin permission path not unit-testable (code-inspected only); these 10 items are the ONLY remaining work for this change |
|
||||
|
||||
**Task Summary**:
|
||||
- 41 tasks complete (`[x]`) across Phases 1–5
|
||||
- 6 tasks N/A (`[ ]` with justification) in Phase 6
|
||||
- 10 tasks pending (`[ ]`) in Phase 7
|
||||
- **Total: 57 checkboxes, 0 unaccounted**
|
||||
|
||||
---
|
||||
|
||||
## Test Results
|
||||
|
||||
### Unit/Widget Tests
|
||||
|
||||
| Suite | Tests | Result | Notes |
|
||||
|-------|-------|--------|-------|
|
||||
| `estado_ecualizador_test.dart` | 49/49 | PASS | +1 from task 3.10 (was 48/48); Phase D regression group (D.1-D.5) confirmed green |
|
||||
| `pantalla_ajustes_test.dart` | 11/11 | PASS | Display fix + permission trigger scenarios |
|
||||
| `servicio_ecualizador_test.dart` | 20/20 | PASS | Migration tests including matrix-key and near-miss scenarios |
|
||||
| `servicio_dispositivo_audio_real_test.dart` | 10/10 | PASS | Permission contract (MethodChannel invokeMethod) |
|
||||
| `servicio_dispositivo_audio_test.dart` | 7/7 | PASS | Abstract contract tests |
|
||||
| `servicio_dispositivo_audio_toggle_test.dart` | 5/5 | PASS | Ripple-fix (NullServicioDispositivoAudio) included |
|
||||
| **Total** | **102/102** | **PASS** | — |
|
||||
|
||||
### Static Analysis
|
||||
|
||||
| Tool | Result | Coverage |
|
||||
|------|--------|----------|
|
||||
| `flutter analyze` | 0 issues | Whole project |
|
||||
| `rg` (placeholder string checks) | No dead code (`00:00:00:00:00:00` removed completely) | Kotlin, Dart |
|
||||
|
||||
### Code Coverage
|
||||
|
||||
- Exact-line coverage metrics not collected (per verification scope).
|
||||
- Scenario-level mapping: 15/21 spec scenarios COMPLIANT (Dart-testable, all pass); 4/21 DEFERRED (manual QA); 2/21 N/A (l10n justified).
|
||||
- CRITICAL-1 closed by new task 3.10 composed test (spec.md L95-100, rename survives re-pair).
|
||||
|
||||
---
|
||||
|
||||
## Verification Summary
|
||||
|
||||
### Verdict Evolution
|
||||
|
||||
| Pass | Result | Finding |
|
||||
|------|--------|---------|
|
||||
| Initial (2026-07-11 00:44) | **FAIL** | CRITICAL-1: Untested Dart-testable scenario "rename persists after re-pair" (spec.md L95-100) — constituent behaviors tested individually (reconnect-dedup D.4, rename-priority 4.4), composition untested |
|
||||
| Re-verify (2026-07-11 post-verify) | **PASS WITH WARNINGS** | CRITICAL-1 CLOSED: task 3.10 adds composed regression test; passes against existing code (coverage gap confirmed, not functional bug). WARNING-1 spec wording reconciled (composite shape documented as `bt_a2dp:name:<name>` per ADR-3). WARNING-2 cosmetic (arithmetic drift in apply-progress narrative, no code impact) accepted as-is. |
|
||||
|
||||
### Finding Breakdown
|
||||
|
||||
| Severity | Count | Status |
|
||||
|----------|-------|--------|
|
||||
| CRITICAL | 1 | CLOSED (task 3.10 composed regression test) |
|
||||
| WARNING | 2 | CLOSED (WARNING-1, re-verify wording fix); OPEN/ACCEPTED (WARNING-2, cosmetic) |
|
||||
| SUGGESTION | 3 | OPEN (non-blocking: wording nuance SUGGESTION-1, mock call-count pattern SUGGESTION-2, Engram housekeeping SUGGESTION-3) |
|
||||
|
||||
### Compliance Matrix Snapshot
|
||||
|
||||
| Req. | Scenarios | Compliant | Deferred | N/A | Untested |
|
||||
|------|-----------|-----------|----------|-----|----------|
|
||||
| BLUETOOTH_CONNECT at point-of-intent | 2 | 0 | 2 | 0 | 0 |
|
||||
| Placeholder MAC guarded | 5 | 4 | 1 | 0 | 0 |
|
||||
| Platform-name cache feeds display | 3 | 3 | 0 | 0 | 0 |
|
||||
| Rename survives re-pair | 2 | 1 (via 3.10) | 1 | 0 | 0 |
|
||||
| No duplicate entries on transient | 2 | 2 | 0 | 0 | 0 |
|
||||
| One-time guarded migration | 6 | 6 | 0 | 0 | 0 |
|
||||
| Localization (conditional on UI copy) | 2 | 0 | 0 | 2 | 0 |
|
||||
| **Total** | **21** | **15** | **4** | **2** | **0** |
|
||||
|
||||
---
|
||||
|
||||
## File Changes Summary
|
||||
|
||||
**Estimated Changed Lines**: ~450 (Kotlin ~90, Dart prod ~140, Dart tests ~58 post-verify, manifest ~1)
|
||||
**400-Line Budget Risk**: High (mitigated by chained PR strategy; PR 1: Kotlin, PR 2: Dart, PR 3: l10n + manual QA)
|
||||
**Build Execution**: Not run (per verification scope — `flutter build` explicitly excluded)
|
||||
|
||||
### Kotlin (Code-Inspection Verified)
|
||||
- `android/app/src/main/AndroidManifest.xml`: +1 permission declaration
|
||||
- `android/app/.../MainActivity.kt`: ~90 lines (permission request code, placeholder guard, composite fallback, re-emit logic, doc comment updates)
|
||||
|
||||
### Dart Production
|
||||
- `lib/servicios/servicio_dispositivo_audio.dart`: Abstract + real impl of `solicitarPermisoBluetooth()`
|
||||
- `lib/estado/estado_ecualizador.dart`: In-memory `_nombresPlataforma` cache, `nombrePlataforma()` getter, duplicate-entry guard update
|
||||
- `lib/pantallas/pantalla_ajustes.dart`: Two `nombreVisible()` call sites updated, permission trigger added
|
||||
- `lib/servicios/servicio_ecualizador.dart`: Migration constants and `migrarClavesPlaceholder()` function
|
||||
|
||||
### Dart Tests
|
||||
- `test/estado/estado_ecualizador_test.dart`: +58 lines (task 3.10 post-verify regression test)
|
||||
- `test/pantallas/pantalla_ajustes_test.dart`: Display fix + permission trigger tests
|
||||
- `test/servicios/servicio_ecualizador_test.dart`: Migration test suite (exact placeholder, matrix suffix, near-miss, idempotency)
|
||||
- `test/servicios/servicio_dispositivo_audio_real_test.dart`: Permission contract tests
|
||||
- `test/helpers/fakes.dart`: Fake implementers updated (ripple-fix: NullServicioDispositivoAudio also patched)
|
||||
|
||||
### Localization
|
||||
- `lib/l10n/*.arb` (13 files): **0 changes** (Phase 6 N/A — no in-app copy shipped)
|
||||
|
||||
---
|
||||
|
||||
## Deviations from Design
|
||||
|
||||
**Documented and Signed Off**:
|
||||
|
||||
1. **Trigger-Point Deviation (Phase 4, Task 4.7)**: Design specified "on opening device-management UI" (StatefulWidget on-build hook). Actual: permission fires on toggle-turn-ON tap in `_SeccionEcualizadorAvanzado` (StatelessWidget). Rationale: avoids unnecessary StatelessWidget → StatefulWidget conversion; toggle-turn-ON satisfies both spec scenarios. Signed off by orchestrator.
|
||||
|
||||
2. **Provider Wiring Resolution (Phase 4, Task 4.8)**: Design assumed `ServicioDispositivoAudio` would be a top-level Provider. Actual: not registered in `app.dart`. Solution: added `solicitarPermisoBluetooth()` passthrough on `EstadoEcualizador` (delegates to private field). Maintains test isolation, avoids ProviderNotFoundException.
|
||||
|
||||
**No design deviations were introduced in Batch 2b** (task 3.10 is test-only, zero production-code changes).
|
||||
|
||||
---
|
||||
|
||||
## Risks and Mitigations
|
||||
|
||||
| Risk | Likelihood | Status |
|
||||
|------|------------|--------|
|
||||
| Migration deletes legitimate data | Low | Exact-literal `bt_a2dp:02:00:00:00:00:00` match only; all other keys preserved; Task 5.3 near-miss test confirms `02:00:00:00:00:01` is NOT purged |
|
||||
| BT permission denied → unstable ids | Med | Composite `bt_a2dp:name:<productName>` fallback keeps app functional; re-prompt on next settings open; no crash path |
|
||||
| Colon-delimiter key safety broken | Low | Single leading `bt_a2dp:` segment preserved; productName colons sanitized to `-`; `eq_presets_matriz_v1` split-on-first-`:` remains valid (ADR-3, multi-device-eq ADR-3 RFC4122 rationale) |
|
||||
| Kotlin permission flow has no precedent | Med | Copied verbatim from existing `requestPostNotificationsPermission` pattern (L306-318 struct); request codes distinct (4821/4822/4823); human verification in Phase 7 |
|
||||
| No Kotlin test harness | Med | Code-inspection verified against proven pattern; Phase 7 manual QA is the runtime gate |
|
||||
|
||||
---
|
||||
|
||||
## Next Steps
|
||||
|
||||
### Mandatory Before Shipping
|
||||
|
||||
1. **Phase 7 Manual/On-Device QA** (pending, 10 tasks): Requires real or emulated Android 12+ device.
|
||||
- Permission dialog flow (grant/deny, no dialog at launch)
|
||||
- Real MAC after grant (7.3)
|
||||
- Rename survives re-pair without duplicates (7.5, the original bug report symptom)
|
||||
- Migration purge on affected install + idempotency (7.7–7.8)
|
||||
- Locale rendering if Phase 6 ships (7.9; N/A this change)
|
||||
- Sign-off (7.10) before merge
|
||||
|
||||
2. **Chain Strategy** (pending, from tasks.md): Orchestrator should confirm/name the chain strategy if chained PRs are used:
|
||||
- `stacked-to-main`: Each PR merges to main in order (fast, fix on the go)
|
||||
- `feature-branch-chain`: Tracker branch accumulates; PR #1→PR 2→PR 3 (rollback control)
|
||||
|
||||
### Follow-Up Opportunities (Out of Scope)
|
||||
|
||||
- **WARNING-1 (now closed)**: Spec wording reconciled during re-verify. Composite shape (`bt_a2dp:name:<name>`) is now documented in both spec and design.
|
||||
- **WARNING-2 (cosmetic, accepted)**: Arithmetic drift in apply-progress narrative (pre-existing test counts off by 3/2 respectively) — no functional impact. Accepted as-is per task brief.
|
||||
- **SUGGESTION-1/2/3 (non-blocking)**: Wording nuance (composite not single-entry), mock-call-count pattern, Engram conflict housekeeping — all flagged for future review.
|
||||
- **Broader transient-churn dedup** (design Open Question): Deferred; acceptable now that MAC is canonical. Revisit if duplicates persist post-fix.
|
||||
|
||||
---
|
||||
|
||||
## Archive Integrity
|
||||
|
||||
**Moved From**: `openspec/changes/bt-device-identity/` → `openspec/changes/archive/2026-07-11-bt-device-identity/`
|
||||
**Verified Empty**: Source folder no longer exists under `changes/` (not a residual copy, true move)
|
||||
**Spec Merged**: `openspec/specs/bt-device-identity/spec.md` created as new domain (not merged into `multi-device-eq`, which focuses on EQ preset resolution; bt-device-identity is about device identity + persistence)
|
||||
**Artifacts Preserved**: All 6 openspec files (design, explore, proposal, spec, tasks, verify-report) + new state.yaml and this archive-report
|
||||
|
||||
---
|
||||
|
||||
## Sign-Off
|
||||
|
||||
**Archive Ready For**: Production shipping (pending Phase 7 human QA sign-off)
|
||||
**Blocking Issues**: None; Phase 7 is a known human gate, not a blocker
|
||||
**Recommendation**: Proceed to Phase 7 manual QA; no code changes expected; expect 10 pass/fail items to be recorded against real device access
|
||||
|
||||
---
|
||||
|
||||
## Appendix: Observation IDs for Traceability
|
||||
|
||||
- #2299: sdd/bt-device-identity/proposal (root cause, scope, approach)
|
||||
- #2304: sdd/bt-device-identity/spec (21 scenarios, 7 requirement groups)
|
||||
- #2305: sdd/bt-device-identity/design (6 ADRs, technical approach, testing strategy)
|
||||
- #2314: sdd/bt-device-identity/tasks (57 checkboxes, 3 batches, phase status)
|
||||
- #2328: sdd/bt-device-identity/apply-progress (Batches 1/2/2b merged evidence, 41 tasks complete)
|
||||
- #2329: sdd/bt-device-identity/verify-report (PASS WITH WARNINGS, re-verify, CRITICAL-1 closed)
|
||||
|
||||
All artifacts archived and accessible at `openspec/changes/archive/2026-07-11-bt-device-identity/` with full change history preserved for audit trail.
|
||||
@@ -0,0 +1,109 @@
|
||||
# Design: Canonical Bluetooth Device Identity
|
||||
|
||||
## Technical Approach
|
||||
|
||||
Restore stable BT MAC identity by acquiring `BLUETOOTH_CONNECT` at point of intent, and make the id pipeline resilient when the MAC is still the Android placeholder. Four coordinated edits, all following existing patterns: (1) Kotlin `deviceToMap()` gains a placeholder guard + composite fallback and a new `requestBluetoothConnect` MethodChannel call mirroring `requestPostNotificationsPermission`; (2) `EstadoEcualizador` caches per-device platform names in-memory and stops auto-spawning list entries for the placeholder sentinel; (3) the two `pantalla_ajustes.dart` call sites feed the real platform name into `nombreVisible()`; (4) a guarded one-time migration purges the exact placeholder key from the three SP maps. iOS is untouched. Preserves the `bt_a2dp:` id shape and the `eq_presets_matriz_v1` colon-delimiter invariant.
|
||||
|
||||
> **Load-bearing correction**: `multi-device-eq/design.md` ADR-1 (L17) claimed "BT MAC from `AudioManager.getDevices()` requires no extra permission." That is FALSE on API 31+ and is the root of Bug 1. On unpermitted installs `getAddress()` returns the placeholder `02:00:00:00:00:00` (not null/blank), so every BT device collapsed onto `bt_a2dp:02:00:00:00:00:00`.
|
||||
|
||||
## Architecture Decisions
|
||||
|
||||
### ADR-1: When to request BLUETOOTH_CONNECT
|
||||
| Option | Tradeoff | Decision |
|
||||
|--------|----------|----------|
|
||||
| On opening device-management UI (settings), via new `requestBluetoothConnect` MethodChannel call | Point-of-intent = higher grant rate, Play-safe, mirrors POST_NOTIFICATIONS | **Chosen** |
|
||||
| At app launch | Sensitive prompt out of context, lower grant, Play scrutiny | Rejected |
|
||||
|
||||
**Rationale**: Reuses the proven `pluriwave/audio_devices` MethodChannel already handling `getActiveDevice`. Deny degrades gracefully to composite fallback; re-prompted on next settings open.
|
||||
|
||||
### ADR-2: Re-emit device list after grant
|
||||
**Choice**: After a grant result, Kotlin re-runs `getActiveAudioDevice()` and pushes it through `audioDevicesSink`.
|
||||
**Alternatives**: Do nothing (leave pre-grant placeholder cached). **Rationale**: A device connected BEFORE grant carries the placeholder id; without re-enumeration the real MAC never reaches Dart until a reconnect. Re-emission is required for correctness.
|
||||
|
||||
### ADR-3: Placeholder guard + composite fallback id
|
||||
**Choice**: In the `TYPE_BLUETOOTH_A2DP` branch, treat both blank and the literal `02:00:00:00:00:00` as absent. When absent, build a deterministic fallback `"bt_a2dp:name:$safeProductName"` where `safeProductName` sanitizes `:` → `-` (and blank → `unknown`). MAC path unchanged when present.
|
||||
**Alternatives**: Distinct sentinel prefix (breaks matrix delimiter parsing); pass placeholder through (current bug). **Rationale**: Keeps the single leading `bt_a2dp:` segment so `eq_presets_matriz_v1` split-on-first-`:` stays valid (multi-device-eq ADR-3 L35: station UUIDs are RFC 4122, no colons). Sanitizing productName guarantees no additional colons corrupt the matrix key.
|
||||
|
||||
### ADR-4: Per-device name cache — in-memory only
|
||||
**Choice**: `Map<String, String> _nombresPlataforma` in `EstadoEcualizador`, populated on every `_onDispositivoCambiado` and seed; NOT persisted.
|
||||
**Alternatives**: Persist to a new SP key. **Rationale**: Devices re-report their name on every enumeration, so the cache self-heals each session. Persisting adds a key + migration surface AND collides with an existing latent gap (`guardarConfiguracion` never writes `nombresDispositivos` back). In-memory is simpler and sufficient.
|
||||
|
||||
### ADR-5: Placeholder migration location & guard
|
||||
**Choice**: Run once inside `ServicioEcualizador.cargar()` (or a dedicated `migrarClavesPlaceholder()` called there), guarded by a new bool flag key `eq_placeholder_purge_done_v1`. Purge the exact literal `bt_a2dp:02:00:00:00:00:00` from `eq_nombres_dispositivos_v1`, `eq_preset_por_dispositivo_v1`, and (any key ending `:bt_a2dp:02:00:00:00:00:00`) from `eq_presets_matriz_v1`. Set flag true. Idempotent — flag short-circuits re-runs.
|
||||
**Alternatives**: Migration in `EstadoEcualizador.cargarPersistido`. **Rationale**: Service owns SP; keeps state layer clean. Exact-literal match satisfies the "delete only placeholder" risk mitigation.
|
||||
|
||||
### ADR-6: Transient duplicate-entry guard
|
||||
**Choice**: In `_onDispositivoCambiado`, always update `_nombresPlataforma`; skip auto-creating a `_presetsDispositivo` entry when `dispositivo.id` starts with the composite-fallback marker `bt_a2dp:name:` (unknown-MAC device). Stable ids (real MAC, builtin_speaker, wired_headset, usb) still auto-create.
|
||||
**Alternatives**: Dedup by canonical id / suppress non-BT transient types. **Rationale**: The reported "duplicate on rename" is driven by placeholder collision (Bug 1); once MAC is canonical the duplicate disappears. Suppressing legitimate builtin/wired entries would regress their EQ. Broader transient-churn dedup is deferred (Open Question).
|
||||
|
||||
## Data Flow
|
||||
|
||||
Settings UI opens ──► servicioDispositivoAudio.solicitarPermisoBluetooth()
|
||||
│ │ MethodChannel 'requestBluetoothConnect'
|
||||
▼ ▼
|
||||
(Dart) MainActivity.requestBluetoothConnect()
|
||||
│ grant result
|
||||
▼
|
||||
getActiveAudioDevice() ──► deviceToMap() [MAC or composite]
|
||||
│ audioDevicesSink.success(map)
|
||||
▼
|
||||
EstadoEcualizador._onDispositivoCambiado(dispositivo)
|
||||
├─ _nombresPlataforma[id] = dispositivo.nombre (always)
|
||||
└─ if id not placeholder-composite → create/persist preset entry
|
||||
│
|
||||
▼
|
||||
_FilaDispositivo / dialog ─► eq.nombreVisible(id, eq.nombrePlataforma(id))
|
||||
|
||||
## File Changes
|
||||
|
||||
| File | Action | Description |
|
||||
|------|--------|-------------|
|
||||
| `android/app/src/main/AndroidManifest.xml` | Modify | Add `<uses-permission android:name="android.permission.BLUETOOTH_CONNECT"/>` |
|
||||
| `android/app/src/main/kotlin/.../MainActivity.kt` | Modify | Placeholder guard + composite fallback in `deviceToMap()` (L741); `requestBluetoothConnect` MethodChannel case on `audioDevicesChannel` (L638) mirroring `requestPostNotificationsPermission` (L306); new request code; re-emit active device in `onRequestPermissionsResult` (L610) on grant |
|
||||
| `lib/servicios/servicio_dispositivo_audio.dart` | Modify | Add `Future<bool> solicitarPermisoBluetooth()` to abstract + real impl (invokeMethod `requestBluetoothConnect`) |
|
||||
| `lib/estado/estado_ecualizador.dart` | Modify | `_nombresPlataforma` map + `nombrePlataforma(id)` getter; populate in `_onDispositivoCambiado`/seed; guard auto-create against composite sentinel (L216) |
|
||||
| `lib/pantallas/pantalla_ajustes.dart` | Modify | L769 + L846: pass `eq.nombrePlataforma(deviceId)` instead of `''`; trigger `solicitarPermisoBluetooth()` when advanced-EQ section builds/toggles on |
|
||||
| `lib/servicios/servicio_ecualizador.dart` | Modify | `_keyPlaceholderPurgaHecha`; `migrarClavesPlaceholder()` called from `cargar()`; purge literal placeholder from 3 maps |
|
||||
| `lib/l10n/app_*.arb` (13) | Modify | Permission-rationale + migration-notice keys |
|
||||
|
||||
## Interfaces / Contracts
|
||||
|
||||
```dart
|
||||
// ServicioDispositivoAudio (abstract + real): returns true if granted/not-needed
|
||||
Future<bool> solicitarPermisoBluetooth();
|
||||
|
||||
// EstadoEcualizador
|
||||
String nombrePlataforma(String deviceId); // last-seen platform name or ''
|
||||
```
|
||||
|
||||
```kotlin
|
||||
// MainActivity, audioDevicesChannel handler
|
||||
"requestBluetoothConnect" -> result.success(requestBluetoothConnect())
|
||||
// mirrors requestPostNotificationsPermission: SDK<31 → true; granted → true;
|
||||
// else requestPermissions(BLUETOOTH_CONNECT, code) → true
|
||||
```
|
||||
|
||||
Placeholder constant (shared intent, define once per side): `02:00:00:00:00:00`.
|
||||
Composite fallback id shape: `bt_a2dp:name:<sanitized productName>` (colons in name → `-`).
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
Existing fakes: `FakeServicioDispositivoAudio` (has `emitirDispositivo`), `FakeServicioDispositivoAudioThrows`, `FakeServicioEcualizador` (in-memory `ConfiguracionEcualizador`) — all in `test/helpers/fakes.dart`.
|
||||
|
||||
| Layer | What to Test | Approach |
|
||||
|-------|-------------|----------|
|
||||
| Unit (state) | Placeholder-composite id does NOT create a device-list entry; real MAC does; `_nombresPlataforma` populated on event; `nombreVisible` returns platform name when no custom | `test/estado/estado_ecualizador_test.dart` — emit via `FakeServicioDispositivoAudio` |
|
||||
| Unit (service) | `migrarClavesPlaceholder` drops only `bt_a2dp:02:00:00:00:00:00`, keeps stable-MAC entries, is idempotent (flag), matrix suffix variant purged | `test/servicios/servicio_ecualizador_test.dart` — seed SP via `SharedPreferences.setMockInitialValues` |
|
||||
| Unit (device svc) | `solicitarPermisoBluetooth` invokes `requestBluetoothConnect` and returns bool | `test/servicios/servicio_dispositivo_audio_real_test.dart` — mock MethodChannel handler |
|
||||
| Widget | Device row shows platform name (not raw id) when platform name known; permission call fires on section build | `test/pantallas/pantalla_ajustes_test.dart` |
|
||||
|
||||
New fake behavior: add a `permisoBluetoothConcedido` flag + call counter to `FakeServicioDispositivoAudio`, and a helper to emit a placeholder-composite device. Kotlin permission path is not unit-tested (no instrumented tests in repo); covered by the Dart contract test on the channel.
|
||||
|
||||
## Migration / Rollout
|
||||
|
||||
One-time guarded purge in `ServicioEcualizador.cargar()`, flag `eq_placeholder_purge_done_v1`. Deletes only the exact placeholder-keyed entries (unrecoverable regardless). No persistence-key version bump → downgrade clean. Additive otherwise. Surfaces a one-time "rename your Bluetooth devices again" notice only when a placeholder entry was actually removed.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- [ ] Broader transient-churn dedup (builtin/wired appearing mid-handshake) is deferred; acceptable now that MAC is canonical. Revisit if duplicates persist post-fix.
|
||||
- [ ] Rationale-dialog UX (show explanatory sheet before the OS prompt) vs. firing the OS prompt directly — proposal implies a rationale string exists; confirm whether a pre-prompt sheet is in scope for tasks.
|
||||
@@ -0,0 +1,30 @@
|
||||
# Exploration: bt-device-identity
|
||||
|
||||
## Root Cause — two independent, compounding bugs
|
||||
|
||||
### Bug 1 — Missing `BLUETOOTH_CONNECT` permission (Android 12+/API 31+)
|
||||
`AndroidManifest.xml` declares zero Bluetooth permissions. On API 31+, without `BLUETOOTH_CONNECT`, `AudioDeviceInfo.getAddress()` for BT devices returns the fixed placeholder `"02:00:00:00:00:00"` — not null, not empty, no exception. In `MainActivity.kt` `deviceToMap()` (L742), the guard `takeIf { it.isNotBlank() }` lets the placeholder pass through since it is non-blank. Result: every BT A2DP device collapses onto the identical id `bt_a2dp:02:00:00:00:00:00`. No runtime BT permission request flow exists anywhere in the Kotlin code (only `RECORD_AUDIO`/`POST_NOTIFICATIONS` have flows) — completely silent failure. Origin: `multi-device-eq/design.md` L17 claimed "no extra permission needed".
|
||||
|
||||
### Bug 2 — Duplicate device-list entry on any never-before-seen `deviceId`
|
||||
`estado_ecualizador.dart` `_onDispositivoCambiado()` (L210-227) creates a new `_presetsDispositivo` map entry whenever the incoming `dispositivo.id` isn't already a key. The "known devices" list in `pantalla_ajustes.dart` is keyed off this map; the rename overlay (`_nombresDispositivos`) is a SEPARATE map also keyed by `deviceId` — a rename under one id string does nothing for a different id string. This produces the literal "duplicate after re-pairing" symptom (likely via transient builtin_speaker/wired_headset reports during A2DP reconnect handshake) and gets structurally worse once Bug 1 is fixed until placeholder entries are cleaned.
|
||||
|
||||
## Device name (symptoms 2/4) — smaller fix than expected
|
||||
`productName` already flows end-to-end from Kotlin into `DispositivoAudio.nombre`. But `pantalla_ajustes.dart` calls `eq.nombreVisible(deviceId, '')` at both call sites (L769, L846) — always passing an empty platform name — and `EstadoEcualizador` never caches per-device platform names. The fallback chain in `nombreVisible()` is correct; it's just starved of input.
|
||||
|
||||
## iOS is already the reference pattern
|
||||
`AudioDevicesPlugin.swift` `stableKey()` (L153-165) uses `uid` as primary key with `portType+portName` fallback when uid is empty — exactly the pattern Android needs. No iOS changes required.
|
||||
|
||||
## Recommended fix (5 points)
|
||||
1. Declare `BLUETOOTH_CONNECT` in manifest + runtime request flow (mirror existing POST_NOTIFICATIONS pattern)
|
||||
2. Detect the `02:00:00:00:00:00` placeholder in `deviceToMap()` and fall back to type+productName composite id instead
|
||||
3. Cache per-device platform names in `EstadoEcualizador` so `nombreVisible()` gets real input
|
||||
4. MAC remains the canonical id where available (id format unchanged — preserves colon-delimiter key safety from multi-device-eq design L35)
|
||||
5. Migration: detect and DROP placeholder-keyed entries (collision destroyed per-device info at capture time — unrecoverable), keep legitimate stable-MAC entries as-is, one-time "please rename your devices again" notice for affected users only
|
||||
|
||||
## Risks
|
||||
- Migration is destructive by necessity for the placeholder-collision subset
|
||||
- New runtime-request flow has no BT precedent in codebase (copy RECORD_AUDIO/POST_NOTIFICATIONS patterns)
|
||||
- Any id-format change must preserve `eq_presets_matriz_v1` colon-delimiter key safety
|
||||
|
||||
## Files (read during exploration)
|
||||
`dispositivo_audio.dart`, `servicio_dispositivo_audio.dart`, `MainActivity.kt`, `AndroidManifest.xml`, `estado_ecualizador.dart`, `servicio_ecualizador.dart`, `pantalla_ajustes.dart`, `AudioDevicesPlugin.swift`
|
||||
@@ -0,0 +1,71 @@
|
||||
# Proposal: Canonical Bluetooth Device Identity
|
||||
|
||||
## Intent
|
||||
|
||||
Renaming a Bluetooth audio device then re-pairing creates a duplicate device in the multi-device EQ list, and devices display raw ids instead of their own Bluetooth name. Two compounding bugs cause this: (1) on Android 12+ the app never requests `BLUETOOTH_CONNECT`, so `getAddress()` returns the placeholder `02:00:00:00:00:00` for every BT device and they all collapse onto one id `bt_a2dp:02:00:00:00:00:00`; (2) any unseen `deviceId` spawns a new entry, and the rename overlay is keyed separately so a rename never transfers. Fixing identity now unblocks reliable per-device EQ before more devices accumulate corrupted keys.
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
- Declare `BLUETOOTH_CONNECT` in manifest + runtime request flow, mirroring `POST_NOTIFICATIONS`, triggered on opening the device-management UI.
|
||||
- Placeholder detection in `deviceToMap()`: treat `02:00:00:00:00:00` as absent → fall back to `type+productName` composite id (id format unchanged).
|
||||
- Cache per-device platform names in `EstadoEcualizador`; fix both `nombreVisible()` call sites so real BT names feed the fallback chain.
|
||||
- One-time destructive migration dropping placeholder-keyed entries from the three EQ persistence keys; keep stable-MAC entries.
|
||||
- New user-visible strings (permission rationale, migration notice) in all 13 locales.
|
||||
|
||||
### Out of Scope
|
||||
- iOS changes — `stableKey()` already implements uid-primary / type+name fallback.
|
||||
- Reworking the composite-key format or the 4-level EQ resolution hierarchy.
|
||||
- Recovering data lost to the placeholder collision (unrecoverable by design).
|
||||
- Auto-switch / autoswitch-UX behavior (separate change).
|
||||
|
||||
## Capabilities
|
||||
|
||||
> No standing `openspec/specs/*` exists for audio devices yet (multi-device-eq shipped as an archived change). These are new capability specs.
|
||||
|
||||
### New Capabilities
|
||||
- `bt-device-identity`: canonical MAC identity, placeholder detection + composite fallback, `BLUETOOTH_CONNECT` runtime flow, per-device name caching, and placeholder-key migration.
|
||||
|
||||
### Modified Capabilities
|
||||
- None.
|
||||
|
||||
## Approach
|
||||
|
||||
Restore a stable BT MAC by acquiring `BLUETOOTH_CONNECT` at the point of intent (device-management UI), and make the id pipeline resilient when it is still absent. Kotlin `deviceToMap()` gains an explicit placeholder guard so `02:00:00:00:00:00` is treated as blank, producing a deterministic `type:productName` id instead of a colliding one. `EstadoEcualizador` caches each device's reported platform name so `nombreVisible()` prefers the device's own Bluetooth name over the raw id. A guarded one-time migration purges only placeholder-keyed entries from the three SharedPreferences maps and surfaces a rename-again notice to affected users. MAC stays canonical wherever available; the colon-delimited key format is preserved intact.
|
||||
|
||||
## Affected Areas
|
||||
|
||||
| Area | Impact | Description |
|
||||
|------|--------|-------------|
|
||||
| `android/app/src/main/AndroidManifest.xml` | Modified | Add `BLUETOOTH_CONNECT` (maxSdk-scoped as needed). |
|
||||
| `android/app/.../MainActivity.kt` `deviceToMap()` | Modified | Placeholder guard → composite fallback; BT runtime-permission flow (mirror `POST_NOTIFICATIONS`). |
|
||||
| `lib/estado/estado_ecualizador.dart` | Modified | Cache per-device platform names; feed `nombreVisible()`; stop spawning entries for placeholder ids. |
|
||||
| `lib/pantallas/pantalla_ajustes.dart` (L769, L846) | Modified | Pass real platform name to `nombreVisible()` instead of `''`. |
|
||||
| `lib/servicios/servicio_ecualizador.dart` (keys L42-44) | Modified | Migration purging placeholder-keyed entries from 3 maps. |
|
||||
| `lib/l10n/*.arb` (13 files) | Modified | Permission-rationale + migration-notice keys. |
|
||||
|
||||
## Risks
|
||||
|
||||
| Risk | Likelihood | Mitigation |
|
||||
|------|------------|------------|
|
||||
| Migration deletes legitimate data | Low | Match ONLY exact `bt_a2dp:02:00:00:00:00:00`; keep all other keys. |
|
||||
| BT permission denied → unstable ids | Med | Composite `type:productName` fallback keeps app functional; re-prompt on next UI open. |
|
||||
| No BT-permission precedent in codebase | Med | Copy proven `POST_NOTIFICATIONS`/`RECORD_AUDIO` flow verbatim. |
|
||||
| Composite-key colon-delimiter safety broken | Low | Id format unchanged; single leading `type:` segment preserved (ADR-3, multi-device-eq). |
|
||||
|
||||
## Rollback Plan
|
||||
|
||||
Changes are additive/guarded and behind a versioned migration. To revert: restore prior `MainActivity.kt`, `estado_ecualizador.dart`, `pantalla_ajustes.dart`, remove the `BLUETOOTH_CONNECT` declaration and the migration + l10n keys. The migration is one-shot and gated by a run-once flag; already-deleted placeholder-collision entries were unrecoverable regardless, so revert restores behavior, not lost data. No schema/version bump of the persistence keys, so downgrade is clean.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- `permission_handler` capability already used for `POST_NOTIFICATIONS`/`RECORD_AUDIO` (reuse existing pattern; no new package expected).
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] With `BLUETOOTH_CONNECT` granted, distinct BT devices produce distinct MAC-based ids (no `02:00:00:00:00:00`).
|
||||
- [ ] Renaming a device then re-pairing does NOT create a duplicate entry.
|
||||
- [ ] The device list shows each device's own Bluetooth name by default; custom renames still override.
|
||||
- [ ] Migration removes only placeholder-keyed entries; stable-MAC EQ/preset/name data is retained.
|
||||
- [ ] Permission rationale + migration notice render correctly in all 13 locales.
|
||||
- [ ] `flutter analyze` clean; existing `servicio_ecualizador_test.dart` and EQ state tests pass.
|
||||
@@ -0,0 +1,185 @@
|
||||
# Spec: Canonical Bluetooth Device Identity
|
||||
|
||||
## New Capability: bt-device-identity
|
||||
|
||||
### Purpose
|
||||
|
||||
Ensures each physical Bluetooth A2DP device produces a stable, unique `deviceId` and displays its own Bluetooth name by default, by acquiring `BLUETOOTH_CONNECT` at point-of-intent, guarding against the OS placeholder MAC, caching per-device platform names, and migrating away corrupted placeholder-keyed persistence entries.
|
||||
|
||||
### Requirements
|
||||
|
||||
#### Requirement: BLUETOOTH_CONNECT requested at point-of-intent
|
||||
|
||||
On API 31+, the system MUST request `BLUETOOTH_CONNECT` when the device-management UI (Settings → Advanced Equalization) is opened, not at app launch. `AndroidManifest.xml` MUST declare `BLUETOOTH_CONNECT`.
|
||||
|
||||
##### Scenario: permission requested on UI open — manual/on-device QA
|
||||
|
||||
- GIVEN the app has never requested `BLUETOOTH_CONNECT`
|
||||
- WHEN the user opens the device-management screen on API 31+
|
||||
- THEN the system permission dialog MUST appear before any BT device is enumerated for identity purposes
|
||||
|
||||
##### Scenario: permission not requested at app launch — manual/on-device QA
|
||||
|
||||
- GIVEN a fresh install
|
||||
- WHEN the app starts and the user never opens device-management
|
||||
- THEN no BT permission dialog MUST appear
|
||||
|
||||
---
|
||||
|
||||
#### Requirement: Placeholder MAC guarded with composite fallback
|
||||
|
||||
`deviceToMap()` MUST treat the OS placeholder `"02:00:00:00:00:00"` as absent, not as a usable MAC. When the MAC is absent or the placeholder, the id MUST fall back to a deterministic composite id keeping the single leading `bt_a2dp:` segment, with the product name colon-sanitized (implemented shape: `"bt_a2dp:name:<sanitized productName>"`, per design ADR-3 — the fixed `name` marker keeps matrix-key split-on-first-colon safety); the placeholder string itself MUST NOT become a `deviceId`.
|
||||
|
||||
##### Scenario: permission granted yields real MAC id — Dart-testable via fake device stream
|
||||
|
||||
- GIVEN `BLUETOOTH_CONNECT` is granted
|
||||
- WHEN a BT A2DP device with MAC `AA:BB:CC:DD:EE:FF` connects
|
||||
- THEN `deviceId` MUST be `"bt_a2dp:AA:BB:CC:DD:EE:FF"`
|
||||
|
||||
##### Scenario: permission granted yields real MAC id — manual/on-device QA (Kotlin `deviceToMap`)
|
||||
|
||||
- GIVEN `BLUETOOTH_CONNECT` is granted on a real device
|
||||
- WHEN `deviceToMap()` runs for a connected BT A2DP device
|
||||
- THEN `device.address` MUST be a real MAC, not the placeholder
|
||||
- AND the resulting id MUST NOT equal `"bt_a2dp:02:00:00:00:00:00"`
|
||||
|
||||
##### Scenario: permission denied yields composite fallback, no crash — Dart-testable via fake device stream
|
||||
|
||||
- GIVEN `BLUETOOTH_CONNECT` is denied or `device.address` reports the placeholder
|
||||
- WHEN a BT A2DP device with `productName` "AirPods Pro" connects
|
||||
- THEN `deviceId` MUST be a deterministic composite id (implemented shape: `"bt_a2dp:name:AirPods Pro"`), MUST NOT be `"bt_a2dp:02:00:00:00:00:00"`, and app state MUST update without throwing
|
||||
|
||||
##### Scenario: multiple denied-permission devices do not collide — Dart-testable via fake device stream
|
||||
|
||||
- GIVEN `BLUETOOTH_CONNECT` is denied
|
||||
- WHEN two distinct BT A2DP devices with different `productName` values connect in sequence
|
||||
- THEN each MUST produce a distinct composite `deviceId`
|
||||
- AND `presetsDispositivo` MUST NOT collapse them into one entry
|
||||
|
||||
##### Scenario: placeholder never surfaces as a stored id — Dart-testable
|
||||
|
||||
- GIVEN any device change event is processed after the fix
|
||||
- WHEN persistence keys are inspected
|
||||
- THEN no key MUST equal the literal string `"bt_a2dp:02:00:00:00:00:00"`
|
||||
|
||||
---
|
||||
|
||||
#### Requirement: Per-device platform-name cache feeds display
|
||||
|
||||
`EstadoEcualizador` MUST cache the last-seen platform (`productName`) name per `deviceId`. `nombreVisible(deviceId, platformName)` call sites in `pantalla_ajustes.dart` MUST pass the cached platform name, not an empty string.
|
||||
|
||||
##### Scenario: platform name displays with no custom rename — Dart-testable
|
||||
|
||||
- GIVEN a device with id `"bt_a2dp:AA:BB:CC:DD:EE:FF"` and platform name "AirPods Pro" has connected, with no custom rename stored
|
||||
- WHEN the device row is rendered
|
||||
- THEN the displayed name MUST be "AirPods Pro", not the raw `deviceId`
|
||||
|
||||
##### Scenario: custom rename overrides platform name — Dart-testable
|
||||
|
||||
- GIVEN the same device has both a cached platform name "AirPods Pro" and a custom rename "My Headphones"
|
||||
- WHEN the device row is rendered
|
||||
- THEN the displayed name MUST be "My Headphones"
|
||||
|
||||
##### Scenario: no platform name yet falls back to raw id — Dart-testable
|
||||
|
||||
- GIVEN a `deviceId` has no cached platform name and no custom rename
|
||||
- WHEN the device row is rendered
|
||||
- THEN the displayed name MUST be the raw `deviceId` (unchanged legacy behavior)
|
||||
|
||||
---
|
||||
|
||||
#### Requirement: Rename overlay survives re-pair under canonical id
|
||||
|
||||
Once `BLUETOOTH_CONNECT` is granted, a rename stored under a device's canonical MAC-based `deviceId` MUST persist across disconnect/reconnect (re-pair) of the same physical device.
|
||||
|
||||
##### Scenario: rename persists after re-pair — Dart-testable via fake device stream
|
||||
|
||||
- GIVEN a custom rename "My Headphones" is stored for `"bt_a2dp:AA:BB:CC:DD:EE:FF"`
|
||||
- WHEN that device disconnects and reconnects, reporting the same MAC
|
||||
- THEN the device row MUST still display "My Headphones"
|
||||
- AND no second/duplicate entry MUST appear in `presetsDispositivo`
|
||||
|
||||
##### Scenario: rename persists after re-pair — manual/on-device QA
|
||||
|
||||
- GIVEN a real paired BT device is renamed in-app
|
||||
- WHEN the user disconnects and re-pairs the same physical device
|
||||
- THEN the rename MUST still be shown and no duplicate device row MUST appear
|
||||
|
||||
---
|
||||
|
||||
#### Requirement: `_onDispositivoCambiado` does not create duplicate entries for transient reports
|
||||
|
||||
Device-change events reporting an id already present in `presetsDispositivo` MUST NOT create a second entry or overwrite the existing preset with a fresh copy.
|
||||
|
||||
##### Scenario: repeated event for known id is a no-op on preset creation — Dart-testable
|
||||
|
||||
- GIVEN `presetsDispositivo` already contains an entry for `deviceId`
|
||||
- WHEN `_onDispositivoCambiado` fires again for the same `deviceId`
|
||||
- THEN `presetsDispositivo[deviceId]` MUST remain unchanged
|
||||
- AND no new key MUST be added to `presetsDispositivo`
|
||||
|
||||
##### Scenario: transient non-BT id during pairing handshake does not corrupt BT entry — Dart-testable via fake device stream
|
||||
|
||||
- GIVEN a BT device is mid-reconnect and the OS transiently reports `"builtin_speaker"` before A2DP profile attaches
|
||||
- WHEN both the transient and final BT events are processed
|
||||
- THEN the BT device's own entry MUST be keyed only by its BT `deviceId`
|
||||
- AND MUST NOT be merged with or overwritten by the transient `"builtin_speaker"` entry
|
||||
|
||||
---
|
||||
|
||||
#### Requirement: One-time guarded migration purges only exact placeholder-keyed entries
|
||||
|
||||
On first load after this change, the system MUST remove entries whose key is exactly `"bt_a2dp:02:00:00:00:00:00"` from `eq_nombres_dispositivos_v1`, `eq_presets_matriz_v1` (matching the `deviceId` segment), and `eq_preset_por_dispositivo_v1`. All other entries MUST be preserved unchanged. The migration MUST run at most once (idempotent, flagged).
|
||||
|
||||
##### Scenario: migration removes only exact placeholder entries — Dart-testable
|
||||
|
||||
- GIVEN `eq_preset_por_dispositivo_v1` contains both `"bt_a2dp:02:00:00:00:00:00"` and `"bt_a2dp:AA:BB:CC:DD:EE:FF"`
|
||||
- WHEN the migration runs
|
||||
- THEN the placeholder-keyed entry MUST be removed
|
||||
- AND the stable-MAC entry MUST remain byte-for-byte identical
|
||||
|
||||
##### Scenario: matrix keys purge only the placeholder segment — Dart-testable
|
||||
|
||||
- GIVEN `eq_presets_matriz_v1` contains `"station1:bt_a2dp:02:00:00:00:00:00"` and `"station1:bt_a2dp:AA:BB:CC:DD:EE:FF"`
|
||||
- WHEN the migration runs
|
||||
- THEN only the entry with the placeholder `deviceId` segment MUST be removed
|
||||
- AND the other matrix entry MUST be preserved unchanged
|
||||
|
||||
##### Scenario: near-miss keys are preserved — Dart-testable
|
||||
|
||||
- GIVEN a key `"bt_a2dp:02:00:00:00:00:01"` exists (differs by one digit from the placeholder)
|
||||
- WHEN the migration runs
|
||||
- THEN this entry MUST NOT be removed
|
||||
|
||||
##### Scenario: migration runs once — Dart-testable
|
||||
|
||||
- GIVEN the migration has already run once (flag set)
|
||||
- WHEN the app loads again with the same data
|
||||
- THEN the migration MUST NOT execute a second time
|
||||
- AND no additional entries MUST be removed
|
||||
|
||||
##### Scenario: no placeholder entries when BLUETOOTH_CONNECT was never requested — Dart-testable
|
||||
|
||||
- GIVEN no BT permission was ever granted and no placeholder-keyed entries exist
|
||||
- WHEN the migration runs
|
||||
- THEN it MUST be a no-op and MUST NOT alter any persisted keys
|
||||
|
||||
---
|
||||
|
||||
### Localization
|
||||
|
||||
#### Requirement: Permission rationale and migration notice strings exist in all locales
|
||||
|
||||
If UI copy is shown for the `BLUETOOTH_CONNECT` rationale or the migration notice, new l10n keys MUST be added to `lib/l10n/app_en.arb` as the template and translated in all 13 target locales: `ar`, `bn`, `de`, `en`, `es`, `fr`, `hi`, `id`, `it`, `ja`, `pt`, `ru`, `zh`.
|
||||
|
||||
##### Scenario: new keys present in every locale — Dart-testable (arb parity check)
|
||||
|
||||
- GIVEN new keys `btConnectRationale` and `eqDeviceMigrationNotice` (or equivalent) are added to `app_en.arb`
|
||||
- WHEN each of the 13 `app_<locale>.arb` files is parsed
|
||||
- THEN each file MUST contain both keys with non-empty translated values
|
||||
|
||||
##### Scenario: rationale copy renders before the OS dialog — manual/on-device QA
|
||||
|
||||
- GIVEN the device-management UI is opened for the first time
|
||||
- WHEN the in-app rationale (if any) is shown
|
||||
- THEN it MUST render in the user's selected app locale before the OS permission dialog appears
|
||||
@@ -0,0 +1,56 @@
|
||||
change: bt-device-identity
|
||||
archived_date: 2026-07-11
|
||||
status: archived
|
||||
archived_from: openspec/changes/bt-device-identity
|
||||
|
||||
verification_verdict: "PASS WITH WARNINGS"
|
||||
verify_report_observations:
|
||||
- id: 2329
|
||||
type: verify-report
|
||||
critical_issues: 0
|
||||
warnings: 2 (WARNING-1 closed during re-verify; WARNING-2 cosmetic, accepted)
|
||||
suggestions: 3 (non-blocking)
|
||||
|
||||
artifacts_migrated:
|
||||
- proposal.md (obs #2299)
|
||||
- spec.md (obs #2304, on-disk merged to openspec/specs/bt-device-identity/)
|
||||
- design.md (obs #2305)
|
||||
- explore.md
|
||||
- tasks.md (obs #2314, 57 checkboxes: 41 done, 6 N/A Phase 6, 10 pending Phase 7 manual QA)
|
||||
- apply-progress.md (obs #2328, Batches 1/2/2b complete, Batch 3 Phase 7 pending)
|
||||
- verify-report.md (obs #2329, re-verify complete, CRITICAL-1 closed, PASS WITH WARNINGS)
|
||||
|
||||
phase_status:
|
||||
Phase 1 (Kotlin): COMPLETE - 8/8 tasks [x]
|
||||
Phase 2 (Dart Permission Contract): COMPLETE - 5/5 tasks [x]
|
||||
Phase 3 (Platform-Name Cache): COMPLETE - 10/10 tasks [x] (includes task 3.10 post-verify)
|
||||
Phase 4 (Display Fix + Permission Trigger): COMPLETE - 8/8 tasks [x]
|
||||
Phase 5 (Migration Purge): COMPLETE - 10/10 tasks [x]
|
||||
Phase 6 (Localization): N/A - 6 tasks correctly marked N/A (no in-app UI copy shipped)
|
||||
Phase 7 (Manual/On-Device QA): PENDING - 10 tasks [ ] (human gate, code-inspection verified only)
|
||||
|
||||
test_results:
|
||||
total_changed_lines: "~450 (Kotlin 90, Dart prod 140, Dart tests +58 post-verify, manifest 1)"
|
||||
flutter_analyze: "0 issues"
|
||||
targeted_suites:
|
||||
- estado_ecualizador_test.dart: 49/49 (was 48/48, +1 task 3.10)
|
||||
- pantalla_ajustes_test.dart: 11/11
|
||||
- servicio_ecualizador_test.dart: 20/20
|
||||
- servicio_dispositivo_audio_real_test.dart: 10/10
|
||||
- servicio_dispositivo_audio_test.dart: 7/7
|
||||
- servicio_dispositivo_audio_toggle_test.dart: 5/5 (ripple-fix included)
|
||||
total_tests_passing: 102/102
|
||||
|
||||
notes: |
|
||||
Phase 7 (manual/on-device QA) is a mandatory human gate before shipping.
|
||||
10 tasks (7.1-7.10) require real or emulated Android 12+ device access.
|
||||
|
||||
WARNING-2 (cosmetic arithmetic drift in apply-progress narrative) accepted as-is;
|
||||
no corrections made to the artifact per task brief.
|
||||
|
||||
Kotlin work is code-inspection-verified only (no Android test harness in this repo).
|
||||
Real MAC behavior and permission-dialog flow will be validated during Phase 7 manual QA.
|
||||
|
||||
CRITICAL-1 from initial verify (untested Dart-testable spec scenario for rename-survives-re-pair)
|
||||
was closed via new task 3.10 composed regression test in Batch 2b (post-verify).
|
||||
Test passes against existing, unmodified implementation — confirms coverage gap, not functional bug.
|
||||
@@ -0,0 +1,125 @@
|
||||
# Tasks: Canonical Bluetooth Device Identity
|
||||
|
||||
## Review Workload Forecast
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Estimated changed lines | 420–560 (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 1–2) → PR 2: Dart state/display/migration (Phase 3–5) → PR 3: l10n + manual QA sign-off (Phase 6–7) |
|
||||
| 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`
|
||||
@@ -0,0 +1,324 @@
|
||||
# Verification Report
|
||||
|
||||
**Change**: bt-device-identity
|
||||
**Version**: N/A (spec has no version field)
|
||||
**Mode**: Strict TDD (Dart layers, cross-referenced against real `flutter test` runs) / code-inspection only for Kotlin (no Android/Kotlin test harness exists in this repo - confirmed, `test/` tree is Dart-only)
|
||||
**Commits verified**: `aef4e02` (Kotlin plumbing) + `b17c582` (Dart state/display/migration). Working tree clean of `lib/`/`android/` changes at verification time.
|
||||
|
||||
> **Scenario count correction**: the verification brief cited "24 scenarios." Independent counts from both the engram spec artifact (`sdd/bt-device-identity/spec`, obs #2304) and the file `openspec/changes/bt-device-identity/spec.md` (`grep -c "^##### Scenario:"`) agree: **21 scenarios across 7 requirement groups**. This report verifies against the confirmed 21.
|
||||
|
||||
---
|
||||
|
||||
## Completeness
|
||||
|
||||
| Metric | Value |
|
||||
|--------|-------|
|
||||
| Tasks total (tasks.md, grep-verified) | 56 |
|
||||
| Tasks complete `[x]` (Phase 1-5) | 40 |
|
||||
| Tasks N/A with inline justification (Phase 6, l10n) | 6 |
|
||||
| Tasks pending - human sign-off gate (Phase 7, manual/on-device QA) | 10 |
|
||||
| Tasks incomplete/unaccounted | 0 |
|
||||
|
||||
Independently re-counted via `grep -c` against the live `tasks.md` file - matches apply-progress self-reported totals exactly (40 checked, 16 unmarked = 6 N/A + 10 pending).
|
||||
|
||||
---
|
||||
|
||||
## Build & Tests Execution
|
||||
|
||||
**Build**: not run (per verification scope - `flutter build` explicitly excluded).
|
||||
|
||||
**Tests**: independently re-executed in this pass, not taken on the apply-progress report alone.
|
||||
|
||||
```text
|
||||
flutter test test/estado/estado_ecualizador_test.dart test/servicios/servicio_ecualizador_test.dart \
|
||||
test/pantallas/pantalla_ajustes_test.dart test/servicios/servicio_dispositivo_audio_test.dart \
|
||||
test/servicios/servicio_dispositivo_audio_real_test.dart test/servicios/servicio_dispositivo_audio_toggle_test.dart
|
||||
-> 00:11 +101: All tests passed!
|
||||
```
|
||||
|
||||
Per-file breakdown (re-run individually to cross-check against apply-progress claimed counts):
|
||||
|
||||
| File | Actual | Claimed (apply-progress) | Match |
|
||||
|------|--------|---------------------------|-------|
|
||||
| servicio_dispositivo_audio_real_test.dart | 10/10 | 10/10 | Yes |
|
||||
| servicio_dispositivo_audio_test.dart | 7/7 | 7/7 | Yes |
|
||||
| servicio_dispositivo_audio_toggle_test.dart | 5/5 | 5/5 | Yes |
|
||||
| estado_ecualizador_test.dart | 48/48 | 48/48 | Yes |
|
||||
| servicio_ecualizador_test.dart | 20/20 | 20/20 | Yes |
|
||||
| pantalla_ajustes_test.dart | 11/11 | 11/11 | Yes |
|
||||
|
||||
**flutter analyze**: 0 issues, whole project ("No issues found!").
|
||||
|
||||
**Coverage**: not run this pass (line-coverage percent was not part of the verification checklist; scenario-level mapping below is more precise for this purpose).
|
||||
|
||||
---
|
||||
|
||||
## Spec Compliance Matrix
|
||||
|
||||
| # | Requirement | Scenario | Test / Evidence | Result |
|
||||
|---|-------------|----------|------------------|--------|
|
||||
| 1 | BLUETOOTH_CONNECT at point-of-intent | permission requested on UI open (manual QA) | Deferred - tasks.md 7.2 | DEFERRED |
|
||||
| 2 | BLUETOOTH_CONNECT at point-of-intent | permission not requested at app launch (manual QA) | Deferred - tasks.md 7.1 | DEFERRED |
|
||||
| 3 | Placeholder MAC guarded | permission granted yields real MAC id (Dart-testable) | Covered implicitly by extensive real-MAC-id tests (3.1, D.4, 5.5a/b, etc.) - all pass | COMPLIANT |
|
||||
| 4 | Placeholder MAC guarded | permission granted yields real MAC id (manual QA, Kotlin) | Deferred - tasks.md 7.3; code-inspected: guard present, MAC-present path unchanged | DEFERRED (code OK) |
|
||||
| 5 | Placeholder MAC guarded | permission denied yields composite fallback, no crash (Dart-testable) | estado_ecualizador_test.dart 3.4 - PASS | COMPLIANT |
|
||||
| 6 | Placeholder MAC guarded | multiple denied-permission devices do not collide (Dart-testable) | estado_ecualizador_test.dart 3.6 - PASS (see SUGGESTION-1 re: wording nuance) | COMPLIANT |
|
||||
| 7 | Placeholder MAC guarded | placeholder never surfaces as a stored id (Dart-testable) | Structural: Kotlin guard (code-inspected) prevents emission + migration tests 5.1/5.9 purge legacy residue. No single dedicated processing test, but cross-layer enforced | COMPLIANT |
|
||||
| 8 | Platform-name cache feeds display | platform name displays, no custom rename (Dart-testable) | pantalla_ajustes_test.dart 4.1 - PASS | COMPLIANT |
|
||||
| 9 | Platform-name cache feeds display | custom rename overrides platform name (Dart-testable) | pantalla_ajustes_test.dart 4.4 - PASS (self-documented as triangulation, not fail-before RED; still a valid passing assertion) | COMPLIANT |
|
||||
| 10 | Platform-name cache feeds display | no platform name yet falls back to raw id (Dart-testable) | pantalla_ajustes_test.dart 4.5 - PASS | COMPLIANT |
|
||||
| 11 | Rename survives re-pair | rename persists after re-pair (Dart-testable via fake device stream) | No covering test found anywhere in the repo (searched both diffs and full test/ tree) | UNTESTED - CRITICAL |
|
||||
| 12 | Rename survives re-pair | rename persists after re-pair (manual QA) | Deferred - tasks.md 7.5 (correctly scoped, unchecked) | DEFERRED |
|
||||
| 13 | No duplicate entries on transient reports | repeated event for known id is a no-op (Dart-testable) | estado_ecualizador_test.dart 3.7 - PASS | COMPLIANT |
|
||||
| 14 | No duplicate entries on transient reports | transient non-BT id does not corrupt BT entry (Dart-testable) | estado_ecualizador_test.dart 3.8 - PASS | COMPLIANT |
|
||||
| 15 | One-time guarded migration | migration removes only exact placeholder entries (Dart-testable) | servicio_ecualizador_test.dart 5.1 - PASS | COMPLIANT |
|
||||
| 16 | One-time guarded migration | matrix keys purge only placeholder segment (Dart-testable) | servicio_ecualizador_test.dart 5.2 - PASS | COMPLIANT |
|
||||
| 17 | One-time guarded migration | near-miss keys preserved (Dart-testable) | servicio_ecualizador_test.dart 5.3 - PASS | COMPLIANT |
|
||||
| 18 | One-time guarded migration | migration runs once (Dart-testable) | servicio_ecualizador_test.dart 5.4 - PASS (strengthened: re-seed-between-calls proves true flag short-circuit) | COMPLIANT |
|
||||
| 19 | One-time guarded migration | no-op when BLUETOOTH_CONNECT never requested (Dart-testable) | servicio_ecualizador_test.dart 5.5 - PASS | COMPLIANT |
|
||||
| 20 | l10n: rationale/notice strings in all locales | new keys present in every locale (arb parity, Dart-testable) | N/A - verified via diff grep: zero Text()/SnackBar/Dialog/l10n./AppLocalizations additions in either commit; zero .arb files touched | N/A (justified) |
|
||||
| 21 | l10n: rationale/notice strings in all locales | rationale copy renders before OS dialog (manual QA) | N/A - same reason; tasks.md 7.9 correctly marked effectively N/A | N/A (justified) |
|
||||
|
||||
**Compliance summary**: 15/21 COMPLIANT, 4/21 correctly DEFERRED (manual/on-device QA, Phase 7 human gate), 2/21 correctly N/A (l10n, justified), 1/21 UNTESTED (CRITICAL).
|
||||
|
||||
---
|
||||
|
||||
## Correctness (Static Evidence) - Kotlin, code-inspection only
|
||||
|
||||
| Item | Status | Notes |
|
||||
|------|--------|-------|
|
||||
| AndroidManifest.xml declares BLUETOOTH_CONNECT | Implemented | L17, verified live file |
|
||||
| requestBluetoothConnect() mirrors requestPostNotificationsPermission() | Implemented | Structural match: SDK-version gate, checkSelfPermission, requestPermissions, return true |
|
||||
| Placeholder MAC guarded via named constant | Implemented | bluetoothMacPlaceholder = "02:00:00:00:00:00" (L41), used in guard condition, no bare string literal in the check itself |
|
||||
| Composite fallback colon-sanitizes productName | Implemented | .replace(":", "-"), blank becomes "unknown" |
|
||||
| Re-emit after grant (ADR-2) | Implemented | onRequestPermissionsResult branch for bluetoothConnectPermissionRequestCode calls getActiveAudioDevice() + audioDevicesSink success on PERMISSION_GRANTED; no-op on denial |
|
||||
| Old dead "00:00:00:00:00:00" fallback removed | Confirmed removed | Zero occurrences anywhere in MainActivity.kt (grep-verified) - replaced, not extended, as required |
|
||||
| Request codes distinct | Confirmed | 4821 (visualizer) / 4822 (notifications) / 4823 (bluetooth) - no collision |
|
||||
|
||||
---
|
||||
|
||||
## Coherence (Design) - ADR 1-6
|
||||
|
||||
| Decision | Followed? | Notes |
|
||||
|----------|-----------|-------|
|
||||
| ADR-1: request BLUETOOTH_CONNECT on UI open via new channel call | Yes, with a signed-off trigger-point deviation | Fires on toggle-turn-ON tap (_alternarMultiDevice), not a StatefulWidget on-build hook. Explicitly flagged in Batch 1 and re-confirmed in Batch 2; satisfies both spec MUSTs without an unrequested widget-type conversion |
|
||||
| ADR-2: re-emit device list after grant | Yes | Verified in onRequestPermissionsResult (Kotlin) |
|
||||
| ADR-3: placeholder guard + composite fallback bt_a2dp:name:$safeProductName | Yes | Verified in deviceToMap(); Dart's _prefijoPlaceholderCompuesto = 'bt_a2dp:name:' matches exactly, cross-layer consistent. Note: this ADR itself deviates from the spec requirement literal wording, see WARNING-1 |
|
||||
| ADR-4: in-memory-only platform-name cache, not persisted | Yes | _nombresPlataforma is a plain Map, no SharedPreferences key added |
|
||||
| ADR-5: migration location (ServicioEcualizador.cargar()) + guard flag eq_placeholder_purge_done_v1 | Yes | Verified; purges exact literal from all 3 maps as specified |
|
||||
| ADR-6: transient duplicate-entry guard, combined with pre-existing esBase guard | Yes | Verified: !esBase && !esPlaceholderCompuesto && !_presetsDispositivo.containsKey(...) - correctly reads and extends the CURRENT code (post-eq-device-disconnect-revert), not a stale design-time snapshot |
|
||||
|
||||
---
|
||||
|
||||
## TDD Compliance (Strict TDD Mode active)
|
||||
|
||||
| Check | Result | Details |
|
||||
|-------|--------|---------|
|
||||
| TDD Evidence reported | Yes | Full TDD Cycle Evidence table present in apply-progress for all Phase 2-5 Dart tasks. Phase 1 (Kotlin) correctly has none, no test harness exists, explicitly scoped as code-inspection-only in both design and tasks |
|
||||
| All tasks have tests | Yes (Dart scope) | 20 genuinely new tests across 5 files, matching every RED/GREEN task pair in tasks.md Phase 2-5 |
|
||||
| RED confirmed (tests exist) | Yes | All claimed test files and test names verified present in the actual diff/live files |
|
||||
| GREEN confirmed (tests pass) | Yes | 101/101 independently re-executed this pass, not taken on trust |
|
||||
| Triangulation adequate | Yes, with 4 explicitly self-labeled approval/regression-lock tests (3.7, 3.8, 4.4, 4.5) | Correctly and transparently distinguished from true fail-before/pass-after RED in both tasks.md and apply-progress, no overclaiming found |
|
||||
| Safety Net for modified files | Yes, with 2 numeric discrepancies found | See below |
|
||||
|
||||
**TDD Compliance**: 5/6 checks clean, 1 check (Safety Net) has minor reporting-accuracy issues, see WARNING-2.
|
||||
|
||||
**Safety-net cross-check** (pre-existing test counts, verified via `git show <parent>:<file> | grep -c "test("`):
|
||||
|
||||
| File | Apply-progress claim | Actual (git-verified) | Match |
|
||||
|------|----------------------|------------------------|-------|
|
||||
| servicio_dispositivo_audio_test.dart | 6/6 pre-existing | 6 | Yes |
|
||||
| pantalla_ajustes_test.dart | 7/7 pre-existing | 7 | Yes |
|
||||
| servicio_ecualizador_test.dart | 13/13 pre-existing | 13 | Yes |
|
||||
| estado_ecualizador_test.dart | 40/40 pre-existing | 43 | No, off by 3 |
|
||||
| servicio_dispositivo_audio_toggle_test.dart (ripple-fix file) | "7 pre-existing tests" (Test Summary prose) | 5 | No, off by 2 |
|
||||
|
||||
Both discrepancies are narrative/arithmetic only. Actual runtime results (48/48 and 5/5 respectively) were independently re-verified as correct in this pass. See WARNING-2.
|
||||
|
||||
---
|
||||
|
||||
## Test Layer Distribution
|
||||
|
||||
| Layer | New Tests | Files | Tool |
|
||||
|-------|-----------|-------|------|
|
||||
| Unit | 16 | servicio_dispositivo_audio_real_test.dart (3), servicio_dispositivo_audio_test.dart (1), estado_ecualizador_test.dart (5), servicio_ecualizador_test.dart (7) | flutter_test |
|
||||
| Widget | 4 | pantalla_ajustes_test.dart (4) | flutter_test (testWidgets) |
|
||||
| E2E | 0 | - | - |
|
||||
| Total new | 20 | 5 files (+1 file with a 1-method ripple-fix, 0 new tests) | |
|
||||
|
||||
---
|
||||
|
||||
## Assertion Quality
|
||||
|
||||
Reviewed all 20 new tests plus the 1 ripple-fix. No tautologies, no ghost loops, no assertion-free tests found; every test exercises real production code (emitirDispositivo -> _onDispositivoCambiado, migrarClavesPlaceholder(), cargar(), real widget pumps, mocked MethodChannel) and asserts concrete, non-trivial values.
|
||||
|
||||
| File | Line(s) | Assertion | Issue | Severity |
|
||||
|------|---------|-----------|-------|----------|
|
||||
| pantalla_ajustes_test.dart | 4.6 (solicitarPermisoBluetoothCalls counter) | expect(fakeDispositivo.solicitarPermisoBluetoothCalls, equals(0/1/1)) | Mock-call-count pattern (skill's explicit watch-list item) | SUGGESTION (justified, see notes) |
|
||||
|
||||
**Assertion quality**: 0 CRITICAL, 0 WARNING, 1 SUGGESTION (noted for transparency, not blocking, see SUGGESTION-2 for rationale).
|
||||
|
||||
---
|
||||
|
||||
## Localization (Phase 6)
|
||||
|
||||
Confirmed N/A, independently verified (not just taken from apply-progress claim):
|
||||
- `git diff aef4e02~1..b17c582 --stat -- lib/l10n/` returns empty (zero .arb files touched across both commits)
|
||||
- `git diff aef4e02~1..b17c582 -- lib/` piped through a grep for Text(, Snackbar, SnackBar, Dialog, l10n., AppLocalizations additions returns zero matches
|
||||
- tasks.md Phase 6 (6.1-6.6) correctly left as unchecked with inline "N/A" justification, not deleted, not silently marked done
|
||||
|
||||
---
|
||||
|
||||
## Phase 7 (Manual/On-Device QA)
|
||||
|
||||
All 10 tasks (7.1-7.10) correctly unchecked in tasks.md - human sign-off gate, explicitly out of scope for this code-correctness verify pass (no instrumented/emulator harness exists in this repo). Task 7.5 specifically covers the manual-QA half of the CRITICAL finding below (rename survives re-pair, on-device) and is appropriately still pending.
|
||||
|
||||
---
|
||||
|
||||
## Issues Found
|
||||
|
||||
### CRITICAL (1)
|
||||
|
||||
**CRITICAL-1 - Untested Dart-testable spec scenario: "rename persists after re-pair"**
|
||||
|
||||
Requirement group "Rename overlay survives re-pair under canonical id" (spec.md L91-106) has two scenarios: one Dart-testable, one manual/on-device QA. The manual scenario is correctly deferred (tasks.md 7.5). The Dart-testable scenario has no covering test anywhere in the repository. Confirmed by:
|
||||
- Full diff review of test/estado/estado_ecualizador_test.dart and test/pantallas/pantalla_ajustes_test.dart (only 5 + 4 new tests added this batch, none combining rename-set + disconnect + reconnect-same-MAC)
|
||||
- Full-tree search for renombrarDispositivo( call sites in tests - none is followed by a sequence of emitirDispositivo calls simulating disconnect/reconnect
|
||||
- tasks.md Phase 3/4 itself never planned a task for this composed scenario (traces back to design's own Testing Strategy section, which also never named it) - this is a planning-stage gap inherited faithfully through apply, not a deviation introduced during implementation
|
||||
- Mitigating context: the two constituent behaviors are each independently tested. Reconnect-does-not-duplicate-or-corrupt-an-entry is proven generically by the pre-existing D.4 regression test (eq-device-disconnect-revert), and custom-rename-wins-over-platform-name is proven by new test 4.4. Composing them carries low residual risk, but per verification rules a scenario is compliant only when an actual runtime test covers it; code-review confidence is not a substitute.
|
||||
|
||||
**Recommendation**: add one composed test to estado_ecualizador_test.dart (store a custom rename for a real-MAC id, emit disconnect, emit reconnect with the same id, assert the rename is still returned by nombreVisible/obtenerNombreDispositivo AND presetsDispositivo.length did not grow). Estimated effort: small, single test, no production-code changes expected (the underlying mechanism already looks correct by inspection). Route through sdd-apply for this one addition, then re-run sdd-verify before archiving.
|
||||
|
||||
### WARNING (2)
|
||||
|
||||
**WARNING-1 - Composite fallback id shape deviates from the spec requirement literal wording**
|
||||
|
||||
Spec requirement prose (spec.md L31) states the fallback id "MUST fall back to a composite bt_a2dp:<type>:<productName>-shaped id," with its own scenario example showing "bt_a2dp:8:AirPods Pro". The actual design (ADR-3) and implementation instead use "bt_a2dp:name:$safeProductName" (literal "name" segment, not the numeric type). This is a documented, deliberate design choice (ADR-3 rationale: preserves the single-leading-bt_a2dp:-segment invariant for eq_presets_matriz_v1's split-on-first-colon parsing), consistently implemented cross-layer (Kotlin's literal prefix matches Dart's _prefijoPlaceholderCompuesto exactly), and does not violate any scenario's actual testable MUST conditions (deterministic, not equal to the placeholder literal, no crash, all verified passing). Recommend a quick sign-off from the spec owner that this wording gap is accepted, since a literal re-read of the requirement text alone would suggest non-compliance.
|
||||
|
||||
**WARNING-2 - Minor arithmetic/count inaccuracies in apply-progress narrative (self-correcting pattern, no functional impact)**
|
||||
|
||||
Two small discrepancies found between apply-progress prose claims and git-verified reality (in addition to the "63 vs 56 total tasks" discrepancy apply-progress already self-corrected transparently):
|
||||
1. TDD Cycle Evidence table states "40/40 pre-existing" for the estado_ecualizador_test.dart Phase-3 safety-net row; git-verified actual pre-existing count is 43.
|
||||
2. Test Summary prose states "7 pre-existing tests in servicio_dispositivo_audio_toggle_test.dart"; the file actually contains 5.
|
||||
|
||||
Neither affects functional correctness - actual runtime results (48/48 and 5/5 respectively) were independently re-verified as accurate in this pass. Recommend correcting these two numbers in the artifact for future-reader accuracy, no code action needed.
|
||||
|
||||
### SUGGESTION (3)
|
||||
|
||||
**SUGGESTION-1 - "Does not collapse into one entry" wording is satisfied trivially, not by design**
|
||||
|
||||
Scenario "multiple denied-permission devices do not collide" (spec.md L52-57) says presetsDispositivo "MUST NOT collapse them into one entry." ADR-6's actual behavior is to create zero presetsDispositivo entries for ANY composite-placeholder id (not just to avoid collisions between them), confirmed by test 3.6 asserting presetsDispositivo.length == baseline (unchanged) after both devices connect. This technically satisfies the literal wording (zero entries is not "one collapsed entry") but is a broader suppression than a literal reading might imply (some readers might expect 2 distinct persisted entries). Recommend confirming this matches product intent, no code change implied, purely a documentation/interpretation checkpoint.
|
||||
|
||||
**SUGGESTION-2 - Mock-call-count assertion in test 4.6 (pantalla_ajustes_test.dart)**
|
||||
|
||||
expect(fakeDispositivo.solicitarPermisoBluetoothCalls, equals(...)) matches the Strict-TDD skill's explicit "mock call count" watch-list pattern. In this specific case it is justified: solicitarPermisoBluetooth() is a fire-and-forget side effect with no other externally observable consequence inside a widget test (the real effect is an OS permission dialog, unobservable in-process), so the call counter is the most direct available proxy for "did the trigger fire exactly once, and not on toggle-OFF." Flagged for transparency per protocol, not recommended for rework.
|
||||
|
||||
**SUGGESTION-3 - Pending Engram conflict markers on artifact observations**
|
||||
|
||||
Both sdd/bt-device-identity/tasks (obs #2314) and sdd/bt-device-identity/apply-progress (obs #2328) show a pending "contested by #obs-..." marker in mem_search results. Both resolved to their latest, highest-revision content when retrieved via mem_get_observation (Revisions: 3 and 2, respectively) and were used as authoritative for this report. Recommend resolving/judging these pending conflicts before sdd-archive to keep the artifact trail clean - housekeeping only, not a content-accuracy concern for this verification.
|
||||
|
||||
---
|
||||
|
||||
## Verdict
|
||||
|
||||
**FAIL** - blocked by exactly one CRITICAL finding (CRITICAL-1: untested Dart-testable spec scenario for rename-survives-re-pair). All other 20/21 scenarios are verified COMPLIANT or correctly DEFERRED/N/A; all 101 targeted tests independently re-executed and green; flutter analyze clean; all 6 design ADRs followed; l10n N/A correctly justified and independently confirmed; Phase 6/7 task states correctly reflect their true completion status. This is a narrow, well-scoped gap with a small, clearly-specified remediation, not a systemic implementation problem. Recommend one additional composed test via sdd-apply, then re-run sdd-verify before sdd-archive.
|
||||
|
||||
---
|
||||
|
||||
## Re-verify (targeted - 2026-07-11)
|
||||
|
||||
**Scope**: targeted re-verify, not a full re-verification. Confirms closure of CRITICAL-1 and WARNING-1 from the FAIL verdict above, and re-confirms WARNING-2 is still open/cosmetic. The 20/21 scenarios already verified COMPLIANT/DEFERRED/N/A in the original pass above are unchanged and were not individually re-audited in this pass; only the touched artifacts (one new test, one spec-wording edit) were inspected, plus a fresh full run of the one modified test file and a whole-project `flutter analyze`.
|
||||
|
||||
**What changed since the FAIL verdict**:
|
||||
1. New composed regression test `3.10 rename persists after re-pair` added to `test/estado/estado_ecualizador_test.dart` (test-only, +58/-0).
|
||||
2. `openspec/changes/bt-device-identity/spec.md` L31 and L50 rewritten to document the implemented composite shape `bt_a2dp:name:<sanitized productName>` (was `bt_a2dp:<type>:<productName>`, e.g. `bt_a2dp:8:AirPods Pro`).
|
||||
|
||||
**Commit note**: the task brief cited commit ab50df0 for the test addition. `git show ab50df0 --stat` resolves to an author/date/message/diff that is identical to the current HEAD commit 8cca7c3 (message: "test(devices): cover rename surviving a re-pair cycle end to end", 1 file, `test/estado/estado_ecualizador_test.dart`, 58 insertions, 0 deletions). Checked: `git merge-base --is-ancestor ab50df0 HEAD` returns false - ab50df0 is dangling, not reachable from current HEAD, while 8cca7c3 is. Consistent with a version-bump commit (158203f, "chore: bump version to 0.1.74+75") landing between b17c582 and this commit and the branch being rebuilt on top of it after the task brief was written. Content verified byte-identical under both references - this is hash drift, not a content discrepancy.
|
||||
|
||||
---
|
||||
|
||||
### CRITICAL-1 - CLOSED
|
||||
|
||||
Verified independently, not taken on the apply-progress/tasks narrative alone:
|
||||
|
||||
- **Test exists and passes**: `flutter test test/estado/estado_ecualizador_test.dart` (re-run this pass) -> 49/49 pass, including test 46 "3.10 rename persists after re-pair - composed regression (closes bt-device-identity CRITICAL-1)". Full output archived below.
|
||||
- **Sequence matches the spec scenario exactly** (`openspec/changes/bt-device-identity/spec.md` L95-100, Requirement: Rename overlay survives re-pair under canonical id):
|
||||
|
||||
| Spec (L97-100) | Test (`estado_ecualizador_test.dart`, spans L1323-1379) |
|
||||
|---|---|
|
||||
| GIVEN a custom rename "My Headphones" is stored for bt_a2dp:AA:BB:CC:DD:EE:FF | L1328-1332 defines btDevice with that exact id; L1347 connects it via emitirDispositivo(btDevice); L1353 renombrarDispositivo(btDevice.id, 'My Headphones'); asserted at L1354-1357 |
|
||||
| WHEN that device disconnects and reconnects, reporting the same MAC | L1361 emitirDispositivo(builtinSpeaker) (disconnect); L1365 emitirDispositivo(btDevice) again - same id (re-pair) |
|
||||
| THEN the device row MUST still display "My Headphones" | L1373-1376 expect(eq.nombreVisible(btDevice.id, eq.nombrePlataforma(btDevice.id)), equals('My Headphones')) |
|
||||
| AND no second/duplicate entry MUST appear in presetsDispositivo | L1369 expect(eq.presetsDispositivo.length, equals(cantidadTrasRenombrar)) |
|
||||
|
||||
The test additionally asserts the preset entry itself is untouched (L1371, presetsDispositivo[btDevice.id] unchanged), a stronger check than the spec literally requires - good practice, not scope creep.
|
||||
- **Zero production-code changes**: `git show 8cca7c3 --stat` (content-identical to ab50df0 --stat) -> 1 file changed, test/estado/estado_ecualizador_test.dart, 58 insertions(+), 0 deletions(-). No lib/ or android/ diff. Confirms this was a genuine coverage gap, not a latent bug - the mechanism was already correct, composed from already-tested constituents D.4 (reconnect-dedup) and 4.4 (rename-priority).
|
||||
- **Assertion-quality audit (Strict TDD Step 5f)**: read the full test body. 5 expect() calls, all comparing dynamically-captured runtime values (presetTrasConectar, cantidadTrasRenombrar) against fresh reads taken after the disconnect/re-pair cycle - no tautologies, no ghost loops, no assertion-free paths, no mock-call-count coupling. Every assertion exercises real production code (emitirDispositivo -> _onDispositivoCambiado, renombrarDispositivo, nombreVisible, nombrePlataforma). 0 CRITICAL, 0 WARNING.
|
||||
- **TDD classification**: apply-progress transparently labels this "Not RED-first by design" - a coverage-closing composed test run against unmodified, already-correct code, passing on first execution. This is the same category the original report already accepted without penalty for tests 3.7, 3.8, 4.4, 4.5 (approval/regression-lock style, honestly disclosed rather than overclaimed as true fail-before RED). Consistent treatment applied here - no new TDD-process flag.
|
||||
|
||||
**Verdict on CRITICAL-1**: CLOSED. The Dart-testable scenario in requirement group "Rename overlay survives re-pair under canonical id" is now COMPLIANT. (Spec Compliance Matrix row 11 in the original report above is left unedited for audit-trail integrity - read it as superseded by this section, not as still-accurate.)
|
||||
|
||||
---
|
||||
|
||||
### WARNING-1 - CLOSED
|
||||
|
||||
Verified spec vs design vs implementation triangulation across all three layers:
|
||||
|
||||
| Layer | Evidence | Value |
|
||||
|---|---|---|
|
||||
| Spec | spec.md L31 (requirement prose) | "...id MUST fall back to a deterministic composite id keeping the single leading bt_a2dp: segment, with the product name colon-sanitized (implemented shape: bt_a2dp:name:<sanitized productName>, per design ADR-3 ...)" |
|
||||
| Spec | spec.md L50 (scenario) | "THEN deviceId MUST be a deterministic composite id (implemented shape: bt_a2dp:name:AirPods Pro) ..." |
|
||||
| Kotlin | MainActivity.kt L777-784 (guard + fallback block), literal at L783 | "bt_a2dp:name:$safeProductName" |
|
||||
| Dart | estado_ecualizador.dart L33 | static const _prefijoPlaceholderCompuesto = 'bt_a2dp:name:'; |
|
||||
|
||||
All four now agree on the literal "name" marker segment. The spec no longer describes a shape (bt_a2dp:<type>:<productName>, e.g. bt_a2dp:8:AirPods Pro) that the implementation never produced. This was a documentation-only fix - no code changed: the same commit-stat evidence above shows zero lib/ or android/ diff, and openspec/changes/bt-device-identity/ is entirely untracked in git (status shows it as untracked), consistent with this being a working-tree file edit rather than a commit.
|
||||
|
||||
**Verdict on WARNING-1**: CLOSED.
|
||||
|
||||
---
|
||||
|
||||
### WARNING-2 - still open, accepted as cosmetic (no action taken, per task brief)
|
||||
|
||||
Unchanged from the original report: the two arithmetic discrepancies (40 vs. git-verified 43 pre-existing estado_ecualizador_test.dart tests; "7 pre-existing" vs. actual 5 in servicio_dispositivo_audio_toggle_test.dart) live only in the apply-progress engram narrative, not in the spec, code, or tests. Actual runtime results were independently re-verified as correct both in the original pass and again in this pass (49/49, up from 48/48 - exactly +1 for the one new test, arithmetically consistent with the claimed 48-pre-existing baseline for task 3.10 specifically, so no new discrepancy was introduced by this batch). Zero functional impact. Left genuinely open per the task brief ("no action taken") rather than silently marked closed; see final verdict rationale below for why this keeps the verdict at PASS WITH WARNINGS rather than a bare PASS.
|
||||
|
||||
---
|
||||
|
||||
### Independently re-executed evidence (this pass)
|
||||
|
||||
```text
|
||||
$ flutter test test/estado/estado_ecualizador_test.dart
|
||||
...
|
||||
00:01 +41: platform-name cache (bt-device-identity Phase 3) 3.1 platform name is cached from a device-change event
|
||||
00:01 +42: platform-name cache (bt-device-identity Phase 3) 3.4 composite-placeholder sentinel does not create device-list entry
|
||||
00:01 +43: platform-name cache (bt-device-identity Phase 3) 3.6 multiple denied-permission devices do not collide
|
||||
00:01 +44: platform-name cache (bt-device-identity Phase 3) 3.7 repeated event for known id is a no-op on preset creation (regression-lock)
|
||||
00:01 +45: platform-name cache (bt-device-identity Phase 3) 3.8 transient non-BT id during pairing handshake does not corrupt the BT entry (regression-lock)
|
||||
00:01 +46: platform-name cache (bt-device-identity Phase 3) 3.10 rename persists after re-pair - composed regression (closes bt-device-identity CRITICAL-1)
|
||||
00:01 +47: FakeServicioEcualizador - nombresDispositivos preservation (WARNING-3) guardarPrincipal preserves nombresDispositivos
|
||||
00:01 +48: FakeServicioEcualizador - nombresDispositivos preservation (WARNING-3) guardarActivo preserves nombresDispositivos
|
||||
00:02 +49: All tests passed!
|
||||
```
|
||||
|
||||
```text
|
||||
$ flutter analyze
|
||||
Analyzing pluriwave...
|
||||
No issues found! (ran in 5.4s)
|
||||
```
|
||||
|
||||
flutter build was NOT run, per task constraints (consistent with the original pass's scope exclusion).
|
||||
|
||||
---
|
||||
|
||||
### Housekeeping note (SUGGESTION-3, still open)
|
||||
|
||||
The pending Engram conflict marker flagged in SUGGESTION-3 of the original report (on sdd/bt-device-identity/tasks obs #2314 and sdd/bt-device-identity/apply-progress obs #2328) is still present, and this verify-report itself (obs #2329) now also shows a pending "contested by" marker as of this session's mem_search. All three resolved to their latest/highest-revision content when fetched via mem_get_observation and were used as authoritative for this re-verify, same as before. Still recommended: resolve/judge these pending conflicts before sdd-archive - housekeeping only, not a content-accuracy concern.
|
||||
|
||||
---
|
||||
|
||||
## Updated Verdict
|
||||
|
||||
**PASS WITH WARNINGS**
|
||||
|
||||
CRITICAL-1 is closed with real, independently-executed, assertion-quality-audited test evidence composing the exact spec scenario (spec.md L95-100), against zero production-code changes (confirming a coverage gap, not a functional bug). WARNING-1 is closed - spec, design (ADR-3), Kotlin, and Dart now agree byte-for-byte on the bt_a2dp:name:<sanitized productName> composite shape. WARNING-2 remains open by the task's own account (cosmetic arithmetic drift in an internal apply-progress narrative, zero functional/spec/code impact, independently reconfirmed accurate at the runtime-result level in both verify passes) - kept open rather than silently closed, since no correction was actually made to the artifact. Zero CRITICAL findings remain. All 3 original SUGGESTIONs remain open/non-blocking and unchanged; one housekeeping note (pending Engram conflict markers, now also touching this report) is carried forward.
|
||||
|
||||
**Recommendation**: proceed to sdd-archive. Phase 7 (manual/on-device QA, tasks 7.1-7.10 in tasks.md) remains genuinely pending - it always has been, is a human-owned gate with no CI/emulator harness in this repo, and was correctly treated as DEFERRED (not blocking) in the original pass. It stays orthogonal to this code-correctness verdict; the maintainer should track it separately (e.g., as an archive follow-up or explicit sign-off note) rather than treat it as a re-verify blocker.
|
||||
Reference in New Issue
Block a user