docs(openspec): add SDD artifact trails for bt-device-identity and alarm-volume-ramp-restore
In-progress artifact sets from the current SDD cycles: exploration, proposal, spec, design, tasks, and verify reports as produced so far. Also drops a leftover working copy of eq-device-disconnect-revert whose contents were already committed under changes/archive/.
This commit is contained in:
@@ -0,0 +1,70 @@
|
||||
# Design: Alarm Volume Ramp & Device-Volume Immunity
|
||||
|
||||
## Technical Approach
|
||||
|
||||
Three independent, rollback-isolated slices realizing proposal #2302. Kotlin owns the manifest fix and a new ring-scoped `STREAM_MUSIC` override (no Flutter volume plugin exists; `MainActivity` already owns the audio channels). Dart drives lifecycle: it invokes override at ring start and restore from the already-centralized exit points. The existing 5%->`alarma.volumen` Dart player ramp is kept; only the audible fade-in *driver* is deduped at handoff. Normal radio playback and `ServicioAudioSession` ducking (S3-R1) are never touched — the override fires only while a ring is active. Kotlin is code-inspection + mandatory on-device QA (flutter build forbidden); Dart follows strict TDD.
|
||||
|
||||
## Architecture Decisions
|
||||
|
||||
| Decision | Choice | Rejected | Rationale |
|
||||
|---|---|---|---|
|
||||
| Manifest FGS combo | `mediaPlayback\|alarm` + add `FOREGROUND_SERVICE_ALARM`; drop `systemExempted`/`FOREGROUND_SERVICE_SYSTEM_EXEMPTED` | Keep `...\|alarm\|systemExempted` | `alarm` is the correct type for an AlarmManager-broadcast-started FGS on API 34+. `systemExempted` is reserved (Play-policy narrow use) and adds nothing once `alarm` is present; matches already-approved D1.1/S1-R1. |
|
||||
| Runtime `startForeground` type | Change `PluriWaveAlarmService.kt:114-120` to `FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK or FOREGROUND_SERVICE_TYPE_ALARM` | Manifest-only edit | **Load-bearing**: the runtime type is hard-coded and must match the manifest, else `ForegroundServiceTypeException` persists. This is the real fix; the manifest alone would still crash. |
|
||||
| Override ownership & state | New channel methods `overrideMediaVolumeForRing(fraction)` / `restoreMediaVolume()` on `pluriwave/alarm_scheduler`; captured original volume in a `@Volatile` field on `MainActivity` (companion) | Kotlin object singleton; SharedPreferences | Survives across method calls, not process death (documented residual). `MainActivity` already holds the channel + `AudioManager`. |
|
||||
| Stream reference level | Force `STREAM_MUSIC` to a FIXED audible reference = `getStreamMaxVolume(STREAM_MUSIC)`; `alarma.volumen` stays the *player* volume via existing Dart ramp | Map `alarma.volumen` to stream volume | Player ramp already governs perceived loudness (5%->target). Stream must only guarantee audibility at volume 0; max is the safe immunity floor. `fraction` arg reserved for future tuning, default 1.0. |
|
||||
| `setStreamVolume` flags | flag `0` (no `FLAG_SHOW_UI`) | `FLAG_SHOW_UI` | No volume-slider flash during a ring. |
|
||||
| Restore idempotence | `@Volatile var mediaVolumeOverridden` + saved level; restore is guarded no-op when not overridden; both override and restore are once-guards | Unconditional restore | Prevents clobbering user volume on double-restore; safe from any exit path. |
|
||||
| Restore call sites (Dart) | Invoke restore inside `_silenciarAudio()` (covers dismiss `_detener` + snooze `_posponer`, both already funnel here) and again in `dispose()` | New per-path calls | Reuses the existing single teardown seam; idempotent guard tolerates the double call. |
|
||||
| Native backstop | `PluriWaveAlarmService.stopAlarm()` / `onDestroy()` call `MainActivity.restoreMediaVolumeBestEffort()` when engine alive | No backstop | Best-effort recovery if app is killed mid-ring; still leaves the documented process-death gap. |
|
||||
| Override trigger point | Dart calls override in `_prearrancarAudioAlarma()` (app.dart) — the earliest point the Flutter player starts, before the ring screen pushes | Ring-screen `initState` | Override must precede the media-player becoming audible to avoid a volume-0 gap at handoff. |
|
||||
| Fade-in dedup gate | Native ramp owns audio until handoff; Dart player pre-starts at 0.05 but its ramp START defers to the existing `confirmarAudioFlutter` success (`_confirmarAudioFlutterListo`, already fired on `reproduciendo`/fallback) | Kill native ramp early; shared timer | Exactly one audible ramp at any instant, reusing the existing handoff protocol seam. No new IPC. |
|
||||
|
||||
## Data Flow
|
||||
|
||||
ring fires -> PluriWaveAlarmService (USAGE_ALARM, native ramp) --immune--> audible
|
||||
| |
|
||||
app.dart _prearrancarAudioAlarma ---> overrideMediaVolumeForRing() [capture+max STREAM_MUSIC]
|
||||
| |
|
||||
PantallaAlarmaSonando: player pre-starts @0.05, Dart ramp START gated ------- v
|
||||
| reproduciendo
|
||||
└── _confirmarAudioFlutterListo -> confirmFlutterAudio -> service.stop() (native ramp ends)
|
||||
-> Dart ramp begins 0.05->alarma.volumen (sole driver)
|
||||
exit (dismiss/snooze/dispose) -> _silenciarAudio + dispose -> restoreMediaVolume() [idempotent]
|
||||
(backstop) service.stopAlarm/onDestroy -> restoreMediaVolumeBestEffort()
|
||||
|
||||
## File Changes
|
||||
|
||||
| File | Action | Description |
|
||||
|---|---|---|
|
||||
| `android/.../AndroidManifest.xml` | Modify | L57 FGS type -> `mediaPlayback\|alarm`; add `FOREGROUND_SERVICE_ALARM` at L3-16; drop `systemExempted` type + `FOREGROUND_SERVICE_SYSTEM_EXEMPTED` perm |
|
||||
| `android/.../PluriWaveAlarmService.kt` | Modify | L114-120 runtime type -> `MEDIA_PLAYBACK or ALARM`; add best-effort restore in `stopAlarm`/`onDestroy` |
|
||||
| `android/.../MainActivity.kt` | Modify | Add `overrideMediaVolumeForRing`/`restoreMediaVolume` channel cases; `@Volatile` capture state; `AudioManager` `STREAM_MUSIC` capture/set(max, flag 0)/restore |
|
||||
| `lib/servicios/servicio_alarmas_android.dart` | Modify | Add `forzarVolumenMediaParaAlarma(double)` + `restaurarVolumenMedia()` to `PuertoAlarmasAndroid` + impl |
|
||||
| `lib/app.dart` | Modify | Call override in `_prearrancarAudioAlarma` |
|
||||
| `lib/pantallas/pantalla_alarma_sonando.dart` | Modify | Call restore in `_silenciarAudio`+`dispose`; gate Dart ramp START on `_confirmarAudioFlutterListo` |
|
||||
|
||||
## Interfaces / Contracts
|
||||
|
||||
```dart
|
||||
// PuertoAlarmasAndroid additions
|
||||
Future<void> forzarVolumenMediaParaAlarma(double fraccion); // -> overrideMediaVolumeForRing
|
||||
Future<void> restaurarVolumenMedia(); // -> restoreMediaVolume (idempotent)
|
||||
```
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
| Layer | What | Approach |
|
||||
|---|---|---|
|
||||
| Unit (Dart, TDD) | New wrapper methods emit correct channel calls/args | `servicio_alarmas_android_test.dart` mock-channel pattern |
|
||||
| Widget (Dart, TDD) | Restore invoked on dismiss/snooze/dispose exactly once; Dart ramp START deferred until confirm | Extend `FakePuertoAlarmasAndroid` (record override/restore call lists); reuse `dismiss_guard_test` harness |
|
||||
| Kotlin | Manifest+runtime type match; capture/set/restore; idempotence | Code inspection only |
|
||||
| Manual QA (mandatory human gate) | media vol 0 -> alarm audible; vol restored after dismiss/snooze; Android 14+ service starts | On-device checklist (CC-R1/R2) |
|
||||
|
||||
## Migration / Rollout
|
||||
|
||||
No data migration. Per-slice independent rollback: revert manifest+runtime type together; disable the override call in `_prearrancarAudioAlarma` to neutralize Slice 2 (restore guard makes it no-worse-than-today); fade-in gate reverts alone.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- [ ] Confirm with user whether app-quality-and-native-alarms Slice 1 manifest was ever verified on a real Android 14+ device (apply-progress Batch 1 deviation note missing).
|
||||
- [ ] `fraction` param default 1.0 (max) accepted, or expose per-alarm later? (Deferred; not blocking.)
|
||||
@@ -0,0 +1,40 @@
|
||||
# Exploration: alarm-volume-ramp-restore
|
||||
|
||||
## Current State — dual-track handoff architecture (by design)
|
||||
|
||||
Alarm audio is a deliberate handoff (documented in `app-quality-and-native-alarms/design.md` Decision 1.5, `docs/alarmas-pantalla-apagada.md`):
|
||||
|
||||
1. **Native track**: `PluriWaveAlarmReceiver(ACTION_FIRE)` → `PluriWaveAlarmService.kt` plays via `MediaPlayer` with `AudioAttributes.USAGE_ALARM` (L350-354) — Android ALARM stream, immune to media-volume-0 by OS design. Has a working fade-in (`startFadeIn`, `FADE_IN_START_FRACTION = 0.05f`, 250ms steps).
|
||||
2. **Flutter track**: `app.dart` `_prearrancarAudioAlarma()` starts the regular radio player (`PluriWaveAudioHandler`/just_audio) at volume 0.05; `pantalla_alarma_sonando.dart` `_iniciarFadeIn()` ramps it 0.05 → `alarma.volumen`. Uses `AudioSessionConfiguration.music()` — normal media session, fully subject to device media volume.
|
||||
3. **Handoff**: when the Flutter player reaches `reproduciendo`, `confirmarAudioFlutter` → `MainActivity` → `PluriWaveAlarmService.stop()`. Native audio torn down; Flutter media-stream player is the sole source for the rest of the ring.
|
||||
|
||||
Grep-verified: **zero occurrences of `setStreamVolume`/AudioManager volume-override anywhere in the codebase, ever** — the app has never programmatically overridden device stream volume. The "used to ignore device volume" impression comes from the native track's brief immune window, not a regressed capability.
|
||||
|
||||
## Root Causes
|
||||
|
||||
**A — architectural**: steady-state alarm audio is the Flutter media-stream player within 1-3s of firing. Media volume 0 → silent alarm (symptom 2).
|
||||
|
||||
**B — manifest bug (verified)**: `AndroidManifest.xml:55-58` declares `foregroundServiceType="mediaPlayback|systemExempted"` and NO `FOREGROUND_SERVICE_ALARM` permission — contradicting the approved design (`app-quality-and-native-alarms` design.md Decision 1.1, spec S1-R1 CRITICAL, which mandates `mediaPlayback|alarm` + permission to prevent `ForegroundServiceTypeException` on Android 14+ when starting from a broadcast receiver). `apply-progress.md` marks T-S1-03/04 done with "DEVIATION (see below)" but no Batch-1 deviation section exists. On Android 14+ this plausibly kills the native service start silently — removing even the brief volume-immune window.
|
||||
|
||||
**C — double fade-in**: native Kotlin ramp and Dart ramp run independently and can interleave at handoff → audible jump/reset (likely the real cause of "feels broken", symptom 1).
|
||||
|
||||
## Recommendation (sequenced, one change)
|
||||
|
||||
1. **Manifest fix**: restore `mediaPlayback|alarm` + `FOREGROUND_SERVICE_ALARM` per the already-approved design. Near-zero risk.
|
||||
2. **Ring-scoped volume immunity (option c)**: at ring start, capture current media-stream volume; force it to an audible reference; ramp PLAYER volume 5% → `alarma.volumen`; restore original stream volume on stop/snooze/dismiss. Scoped strictly to the ring — must not touch normal radio listening.
|
||||
3. **Fade-in dedup**: gate so only one ramp drives audible volume across the handoff.
|
||||
|
||||
Deferred (flagged as follow-up, not bundled): option (b) — native-only audio for the whole ring (Flutter screen as pure UI). Architecturally purest, but a large rewrite across `pantalla_alarma_sonando.dart`, `PluriWaveAlarmService.kt`, and the MethodChannel surface.
|
||||
|
||||
## Affected Areas
|
||||
- `AndroidManifest.xml:56-58` — FGS type + missing permission (CRITICAL on API 34+)
|
||||
- `PluriWaveAlarmService.kt` — native fade-in OK for its scope; handoff teardown
|
||||
- `lib/servicios/servicio_audio.dart` — steady-state alarm source, no alarm-stream routing
|
||||
- `lib/pantallas/pantalla_alarma_sonando.dart` — Dart ramp; double-ramp at handoff
|
||||
- `lib/app.dart` — `_prearrancarAudioAlarma()` shrinks the immune window
|
||||
- `app-quality-and-native-alarms/apply-progress.md` — missing Batch-1 deviation note (doc gap)
|
||||
|
||||
## Risks
|
||||
- Kotlin/manifest edits cannot be compiled by agents (`flutter build` forbidden) — mandatory on-device verification by the user (this exact gap is how the manifest regression slipped through)
|
||||
- Any audio-session/volume change must be provably scoped to the ring and reverted — must not regress phone-call ducking (`ServicioAudioSession`, S3-R1) or normal listening
|
||||
- Native and Dart fade-ins duplicate the same algorithm/constants — flag for single-sourcing to prevent drift
|
||||
@@ -0,0 +1,66 @@
|
||||
# Proposal: Alarm Volume Ramp & Device-Volume Immunity
|
||||
|
||||
## Intent
|
||||
|
||||
An alarm must ring regardless of device media volume. Today the alarm hands off within 1-3s from the volume-immune native `USAGE_ALARM` player to the Flutter media-stream player, which is fully governed by media volume — so **media volume 0 = silent alarm** for nearly the whole ring. A manifest bug (`systemExempted` instead of `alarm`, no `FOREGROUND_SERVICE_ALARM`) can also silently kill the native start on Android 14+. Device-volume immunity for the full ring was never actually built; it must be built now.
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
- **Manifest fix**: `PluriWaveAlarmService` → `foregroundServiceType="mediaPlayback|alarm"` + declare `FOREGROUND_SERVICE_ALARM`. Restores already-approved design (`app-quality-and-native-alarms` D1.1 / S1-R1).
|
||||
- **Ring-scoped media-volume override (Kotlin-owned)**: on ring start capture `STREAM_MUSIC` volume, force an audible reference level; player ramps 5% → `alarma.volumen` (existing Dart ramp); restore original volume on EVERY exit (dismiss, snooze, dispose, service teardown, best-effort on kill).
|
||||
- **Fade-in dedup**: gate native + Dart ramps so only one drives audible volume across handoff.
|
||||
|
||||
### Out of Scope
|
||||
- Full native-only audio for the whole ring (exploration option b) — **future follow-up**.
|
||||
- Single-sourcing the fade-in constant/curve shared by Kotlin+Dart — noted as debt.
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
- None.
|
||||
|
||||
### Modified Capabilities
|
||||
- `native-alarms`: alarm ring MUST be immune to device media volume for its full duration (not just the pre-handoff window); FGS type/permission corrected; fade-in single-driver across handoff.
|
||||
|
||||
## Approach
|
||||
|
||||
Override lives in **Kotlin** (`MainActivity`/`PluriWaveAlarmService` via a new method on the existing `pluriwave/alarm_scheduler` MethodChannel) because `AudioManager.setStreamVolume` has no Flutter plugin here and MainActivity already owns audio channels. **Dart drives the ring lifecycle**; `pantalla_alarma_sonando.dart` calls `capture+override` at ring start and `restore` from the already-centralized exit points (`_silenciarAudio` → dismiss/snooze; `dispose`). Restore must be **idempotent** and provably scoped: normal radio listening and phone-call ducking (`ServicioAudioSession`, S3-R1) stay untouched — the override only fires while a ring is active. Existing 5%→target Dart ramp is kept; the native ramp is gated to avoid a second audible driver at handoff.
|
||||
|
||||
## Affected Areas
|
||||
|
||||
| Area | Impact | Description |
|
||||
|------|--------|-------------|
|
||||
| `android/.../AndroidManifest.xml:57` | Modified | FGS type `mediaPlayback\|alarm` + `FOREGROUND_SERVICE_ALARM` permission |
|
||||
| `android/.../MainActivity.kt` | Modified | New `alarm_scheduler` methods: capture/override/restore `STREAM_MUSIC` |
|
||||
| `android/.../PluriWaveAlarmService.kt` | Modified | Restore-on-teardown safety net; gate native fade-in vs Dart |
|
||||
| `lib/pantallas/pantalla_alarma_sonando.dart` | Modified | Invoke override at start; restore in `_silenciarAudio`/`dispose` |
|
||||
| `lib/servicios/servicio_alarmas_android.dart` | Modified | Dart wrapper for the new channel methods |
|
||||
| `lib/app.dart` | Modified | Coordinate override with early `_prearrancarAudioAlarma` |
|
||||
|
||||
## Risks
|
||||
|
||||
| Risk | Likelihood | Mitigation |
|
||||
|------|------------|------------|
|
||||
| Kotlin/manifest not compilable by agent | High | On-device verify on Android 14+ is a MANDATORY human gate (CC-R1/R2) |
|
||||
| App killed mid-ring leaves volume overridden | Med | Best-effort restore in service teardown + `onDestroy`; document residual gap |
|
||||
| Override leaks into normal radio playback | Med | Scope strictly to ring; idempotent restore; assert ducking path untouched |
|
||||
| Double fade-in still audible at handoff | Low | Single-driver gate; test the handoff window |
|
||||
|
||||
## Rollback Plan
|
||||
|
||||
Per-slice and independent. Revert the manifest line to restore prior FGS type. Behind a guard so `restore` always runs even if `override` failed (no worse than today). If regressions appear in normal playback, disable the override call in `pantalla_alarma_sonando.dart` — manifest fix stands alone.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Android 14+ device for the mandatory on-device verification of the manifest fix.
|
||||
- Confirm from user whether `app-quality-and-native-alarms` Slice 1 (manifest) was ever verified on a real device (apply-progress deviation note is missing).
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Manifest declares `mediaPlayback|alarm` + `FOREGROUND_SERVICE_ALARM`; native service starts on Android 14+.
|
||||
- [ ] With device media volume at 0, the alarm still rings and ramps 5% → `alarma.volumen`.
|
||||
- [ ] Original media volume is restored after dismiss, snooze, and dispose.
|
||||
- [ ] Normal radio listening and phone-call ducking are unaffected.
|
||||
- [ ] Only one fade-in driver is audible across the native→Flutter handoff.
|
||||
- [ ] No new user-visible strings (no l10n across 13 locales required).
|
||||
@@ -0,0 +1,91 @@
|
||||
# Delta for native-alarms
|
||||
|
||||
## ADDED Requirements
|
||||
|
||||
### Requirement: Manifest declares alarm-eligible foreground service
|
||||
|
||||
`PluriWaveAlarmService` in `AndroidManifest.xml` MUST use `foregroundServiceType="mediaPlayback|alarm"` and the app MUST declare `FOREGROUND_SERVICE_ALARM`, so the service is eligible to start on Android 14+ (API 34+) from a broadcast receiver.
|
||||
|
||||
#### Scenario: Manifest declares required FGS type and permission
|
||||
**Testability**: static/grep-verifiable
|
||||
|
||||
- GIVEN the built `AndroidManifest.xml`
|
||||
- WHEN the `PluriWaveAlarmService` `<service>` element is inspected
|
||||
- THEN `android:foregroundServiceType` MUST contain `alarm`
|
||||
- AND a `<uses-permission android:name="android.permission.FOREGROUND_SERVICE_ALARM"/>` MUST exist
|
||||
|
||||
#### Scenario: Native service starts from broadcast context on Android 14+
|
||||
**Testability**: manual on-device QA (Android 14+; agent cannot install/verify)
|
||||
|
||||
- GIVEN an alarm is scheduled and the app is not foregrounded
|
||||
- WHEN `PluriWaveAlarmReceiver.onReceive(ACTION_FIRE)` starts `PluriWaveAlarmService`
|
||||
- THEN it MUST start without `ForegroundServiceTypeException`
|
||||
- AND the alarm MUST be audible via the native `USAGE_ALARM` player
|
||||
|
||||
### Requirement: Ring-scoped device-volume override
|
||||
|
||||
While an alarm is ringing, the system MUST override `STREAM_MUSIC` to an audible reference level so the alarm is not silenced by device volume 0, and MUST restore the original captured volume when the ring ends through any exit path.
|
||||
|
||||
#### Scenario: Alarm is audible when device media volume is 0
|
||||
**Testability**: manual on-device QA (native volume APIs)
|
||||
|
||||
- GIVEN device `STREAM_MUSIC` volume is 0
|
||||
- WHEN an alarm fires and the override captures/raises `STREAM_MUSIC` to an audible level
|
||||
- THEN the alarm MUST be audible for the full ring duration, not only the pre-handoff window
|
||||
|
||||
#### Scenario: Player fade-in ramps within the overridden level
|
||||
**Testability**: flutter test with fakes
|
||||
|
||||
- GIVEN the override has set an audible `STREAM_MUSIC` reference level
|
||||
- WHEN the ring's fade-in timer runs
|
||||
- THEN player volume MUST ramp 5% -> `alarma.volumen` over `alarma.fadeInSegundos`, unchanged from today
|
||||
|
||||
#### Scenario: Dismiss restores the original captured volume
|
||||
**Testability**: flutter test with fakes (Dart->channel call) + manual on-device QA (native restore)
|
||||
|
||||
- GIVEN the override captured `STREAM_MUSIC` at volume `V`
|
||||
- WHEN the user dismisses the alarm (`_detener()` -> `_silenciarAudio()`)
|
||||
- THEN `STREAM_MUSIC` MUST be restored to exactly `V`
|
||||
|
||||
#### Scenario: Snooze restores the original captured volume
|
||||
**Testability**: flutter test with fakes (Dart->channel call) + manual on-device QA (native restore)
|
||||
|
||||
- GIVEN the override captured `STREAM_MUSIC` at volume `V`
|
||||
- WHEN the user snoozes the alarm (`_posponer()` -> `_silenciarAudio()`)
|
||||
- THEN `STREAM_MUSIC` MUST be restored to exactly `V`
|
||||
|
||||
#### Scenario: Restore is idempotent across double-exit paths
|
||||
**Testability**: flutter test with fakes (single/no-op restore call assertion) + manual on-device QA
|
||||
|
||||
- GIVEN restore already ran once (`_silenciarAudio()` inside `_detener()`)
|
||||
- WHEN a second exit path also runs restore (e.g. `dispose()` firing after)
|
||||
- THEN the second call MUST NOT throw, MUST NOT re-apply a stale value, and MUST leave `STREAM_MUSIC` unchanged
|
||||
|
||||
#### Scenario: Normal radio playback never triggers the override
|
||||
**Testability**: flutter test with fakes (channel never invoked outside a ring) + manual on-device QA
|
||||
|
||||
- GIVEN the user is listening to radio with no alarm ringing
|
||||
- WHEN playback starts, plays, or stops normally
|
||||
- THEN the override MUST NOT be invoked; `STREAM_MUSIC` stays fully governed by device controls
|
||||
|
||||
#### Scenario: App killed mid-ring — best-effort restore only
|
||||
**Testability**: manual on-device QA; accepted residual gap, not required in automated coverage
|
||||
|
||||
- GIVEN an alarm is ringing and the override is active
|
||||
- WHEN the app process is killed before any exit path runs
|
||||
- THEN the system SHOULD best-effort restore from service teardown/`onDestroy`, but a residual overridden-volume state MAY occur and is an accepted known gap
|
||||
|
||||
### Requirement: Single fade-in driver across native-to-Flutter handoff
|
||||
|
||||
During handoff from the native `USAGE_ALARM` player to the Flutter media-stream player, only one fade-in ramp MUST drive audible volume at any instant.
|
||||
|
||||
#### Scenario: No double-ramp interleaving at handoff
|
||||
**Testability**: manual on-device QA (timing-sensitive, cross-process); flutter test with fakes can assert the Dart ramp only runs when native ownership is inactive, but cannot observe native `MediaPlayer.setVolume()` timing
|
||||
|
||||
- GIVEN the native fade-in is ramping the `USAGE_ALARM` player
|
||||
- WHEN the Flutter player becomes ready and handoff occurs (`confirmarAudioFlutter`)
|
||||
- THEN the native fade-in MUST stop driving audible volume once Flutter takes over; both ramps MUST NOT drive audible volume simultaneously
|
||||
|
||||
## Non-Functional Notes
|
||||
|
||||
- No new user-visible strings; no l10n work required across the 13 supported locales.
|
||||
@@ -0,0 +1,89 @@
|
||||
# Tasks: Alarm Volume Ramp & Device-Volume Immunity
|
||||
|
||||
## Review Workload Forecast
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Estimated changed lines | 260-360 (7 files: manifest, 2 Kotlin, 3 Dart, 1 new Dart interface method pair + 3 test files) |
|
||||
| 400-line budget risk | Medium |
|
||||
| Chained PRs recommended | Yes |
|
||||
| Suggested split | PR 1 (Slice 1 manifest+FGS) -> PR 2 (Slice 2 volume override/restore) -> PR 3 (Slice 3 fade-in dedup) |
|
||||
| Delivery strategy | ask-on-risk |
|
||||
| Chain strategy | pending |
|
||||
|
||||
Decision needed before apply: Yes
|
||||
Chained PRs recommended: Yes
|
||||
Chain strategy: pending
|
||||
400-line budget risk: Medium
|
||||
|
||||
### Suggested Work Units
|
||||
|
||||
| Unit | Goal | Likely PR | Notes |
|
||||
|------|------|-----------|-------|
|
||||
| 1 | Fix FGS manifest+runtime type mismatch (Slice 1) | PR 1 | Independent, near-zero risk, own rollback; mandatory Android 14+ device gate |
|
||||
| 2 | Ring-scoped STREAM_MUSIC override + idempotent restore (Slice 2) | PR 2 | Depends on PR 1 landing (FGS must start before override matters); own rollback via guard-disable |
|
||||
| 3 | Dedup fade-in driver at handoff (Slice 3) | PR 3 | Independent of PR 2; can reorder before PR 2 if preferred; own rollback (revert gate) |
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: FGS Manifest + Runtime Type Fix (Slice 1 — code-inspection + manual QA)
|
||||
|
||||
- [ ] 1.1 Edit `android/app/src/main/AndroidManifest.xml:57` — change `PluriWaveAlarmService` `android:foregroundServiceType` from `"mediaPlayback|systemExempted"` to `"mediaPlayback|alarm"`.
|
||||
- [ ] 1.2 Edit `android/app/src/main/AndroidManifest.xml:6` — replace `<uses-permission android:name="android.permission.FOREGROUND_SERVICE_SYSTEM_EXEMPTED"/>` with `<uses-permission android:name="android.permission.FOREGROUND_SERVICE_ALARM"/>`.
|
||||
- [ ] 1.3 Edit `PluriWaveAlarmService.kt:118-119` — change `startForeground` type constants from `FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK or FOREGROUND_SERVICE_TYPE_SYSTEM_EXEMPTED` to `FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK or FOREGROUND_SERVICE_TYPE_ALARM`. 1.1-1.3 are ONE atomic unit — a manifest-only or runtime-only edit leaves the API 34+ exception alive; do not split across commits.
|
||||
- [ ] 1.4 Static check: `rg 'foregroundServiceType' android/app/src/main/AndroidManifest.xml` shows `alarm`, not `systemExempted`, on the `PluriWaveAlarmService` line.
|
||||
- [ ] 1.5 Static check: `rg 'FOREGROUND_SERVICE_ALARM|FOREGROUND_SERVICE_SYSTEM_EXEMPTED' android/app/src/main/AndroidManifest.xml` shows `FOREGROUND_SERVICE_ALARM` present and `FOREGROUND_SERVICE_SYSTEM_EXEMPTED` absent.
|
||||
- [ ] 1.6 Static check: `rg 'FOREGROUND_SERVICE_TYPE_ALARM|FOREGROUND_SERVICE_TYPE_SYSTEM_EXEMPTED' android/.../PluriWaveAlarmService.kt` shows `TYPE_ALARM` present and `TYPE_SYSTEM_EXEMPTED` absent — confirms manifest/runtime match (Requirement: Manifest declares alarm-eligible FGS, Scenario "Manifest declares required FGS type and permission").
|
||||
- [ ] 1.7 `flutter analyze` (no `flutter build`) — confirm no lint regressions from these edits (none expected; XML/Kotlin only).
|
||||
|
||||
## Phase 2: Ring-Scoped Volume Override — Kotlin Channel Methods (Slice 2, code-inspection only)
|
||||
|
||||
- [ ] 2.1 In `MainActivity.kt`, add `@Volatile` companion-scoped state: `mediaVolumeOverridden: Boolean` and `capturedMediaVolume: Int?` to track ring-scoped override without surviving process death (documented residual gap).
|
||||
- [ ] 2.2 In `MainActivity.kt`'s `alarm_scheduler` `when (call.method)` block (near L79-218), add `"overrideMediaVolumeForRing"` case: capture current `AudioManager.STREAM_MUSIC` volume into `capturedMediaVolume` (only if not already overridden — idempotent guard), then `setStreamVolume(STREAM_MUSIC, getStreamMaxVolume(STREAM_MUSIC), 0)` (flag `0` = no `FLAG_SHOW_UI`, no slider flash). Set `mediaVolumeOverridden = true`. `fraction` arg accepted but unused (reserved, default `1.0` = max reference level, per design).
|
||||
- [ ] 2.3 In the same `when` block, add `"restoreMediaVolume"` case: no-op if `mediaVolumeOverridden == false` (idempotent guard); otherwise `setStreamVolume(STREAM_MUSIC, capturedMediaVolume, 0)`, then clear `mediaVolumeOverridden = false` and `capturedMediaVolume = null`.
|
||||
- [ ] 2.4 Add a public `restoreMediaVolumeBestEffort()` method on `MainActivity` (or companion) that `PluriWaveAlarmService` can call as a backstop when the engine is alive.
|
||||
- [ ] 2.5 In `PluriWaveAlarmService.kt`'s `stopAlarm()` (L356-381) and `onDestroy()` (L501-504), call the best-effort restore before/alongside existing teardown, guarded so it never throws if the engine/activity is unavailable.
|
||||
- [ ] 2.6 Static check: `rg 'overrideMediaVolumeForRing|restoreMediaVolume' android/.../MainActivity.kt` shows both channel cases present.
|
||||
- [ ] 2.7 Static check: `rg 'mediaVolumeOverridden' android/.../MainActivity.kt` shows the guard read in BOTH the override and restore branches (idempotence, Requirement: Ring-scoped device-volume override, Scenario "Restore is idempotent across double-exit paths").
|
||||
- [ ] 2.8 Static check: `rg 'restoreMediaVolumeBestEffort' android/.../PluriWaveAlarmService.kt` shows it called from both `stopAlarm` and `onDestroy`.
|
||||
- [ ] 2.9 `flutter analyze` — confirm no Kotlin/lint regressions.
|
||||
|
||||
## Phase 3: Ring-Scoped Volume Override — Dart Port + Wiring (Slice 2, strict TDD)
|
||||
|
||||
- [ ] 3.1 (RED) In `test/servicios/servicio_alarmas_android_test.dart`, add a test asserting `ServicioAlarmasAndroid.forzarVolumenMediaParaAlarma(1.0)` invokes channel method `overrideMediaVolumeForRing` with `{'fraction': 1.0}`, using the existing mock-channel pattern (`MethodChannel('pluriwave/alarm_scheduler')` + `llamadas` list). Run `flutter test` — confirm it fails (method does not exist).
|
||||
- [ ] 3.2 (RED) In the same file, add a test asserting `ServicioAlarmasAndroid.restaurarVolumenMedia()` invokes channel method `restoreMediaVolume` with no args. Run `flutter test` — confirm it fails.
|
||||
- [ ] 3.3 (GREEN) Add `Future<void> forzarVolumenMediaParaAlarma(double fraccion)` and `Future<void> restaurarVolumenMedia()` to `PuertoAlarmasAndroid` (abstract, `lib/servicios/servicio_alarmas_android.dart`) and implement both on `ServicioAlarmasAndroid` using the existing `_logAndInvokeVoid` helper pattern. Run `flutter test` — confirm 3.1-3.2 pass.
|
||||
- [ ] 3.4 (GREEN) Extend `test/helpers/fakes_alarmas.dart`'s `FakePuertoAlarmasAndroid`: implement the two new abstract methods, recording calls into new lists `volumenForzado: List<double>` and `volumenRestaurado: int` (call count) so widget tests can assert invocation order/count.
|
||||
- [ ] 3.5 (RED) In `test/pantallas/pantalla_alarma_sonando_dismiss_guard_test.dart` (or a new focused test file), add a widget test asserting `_silenciarAudio` -> restore is called exactly once on `_detener()` (dismiss) using `env.android.volumenRestaurado`. Run `flutter test` — confirm it fails.
|
||||
- [ ] 3.6 (RED) Add the equivalent test for `_posponer()` (snooze) — restore called exactly once. Run `flutter test` — confirm it fails.
|
||||
- [ ] 3.7 (RED) Add a test asserting restore is called at most once total even when both `_silenciarAudio()` (inside `_detener`) and `dispose()` run in sequence (idempotence at the Dart call-site level — the widget always calls restore in `dispose()` too, per design; assert the FAKE'S restore counter, not double-invocation of the real guard, since idempotence itself lives in Kotlin). Run `flutter test` — confirm it fails.
|
||||
- [ ] 3.8 (GREEN) In `lib/pantallas/pantalla_alarma_sonando.dart`, call `context.read<EstadoAlarmas>().android.restaurarVolumenMedia()` inside `_silenciarAudio()` (L202-213, alongside `_liberarAudioLocal()`/`radio.audio.pausar()`, wrapped in its own try/catch so a failure never blocks dismiss/snooze) AND inside `dispose()` (L238-244). Run `flutter test` — confirm 3.5-3.7 pass.
|
||||
- [ ] 3.9 (RED) In `test/pantallas` (widget test, or a lighter unit-style test on `app.dart`'s ring-start seam if testable in isolation), add a test asserting `forzarVolumenMediaParaAlarma` is invoked when an alarm ring starts, at the TOP of `_prearrancarAudioAlarma` in `lib/app.dart` (BEFORE the `if (emisora == null) return;` early exit at L367) — the override must apply even when the alarm uses the fallback WAV path, not only the station path. Run `flutter test` — confirm it fails.
|
||||
- [ ] 3.10 (GREEN) In `lib/app.dart`, call `context.read<EstadoAlarmas>().android.forzarVolumenMediaParaAlarma(1.0)` as the FIRST statement inside `_prearrancarAudioAlarma` (L365), before the `emisora == null` early return. Run `flutter test` — confirm 3.9 passes.
|
||||
- [ ] 3.11 (RED) Add a test asserting the override/restore channel methods are NEVER invoked during normal radio playback with no alarm ringing (Requirement: Ring-scoped device-volume override, Scenario "Normal radio playback never triggers the override") — assert `env.android.volumenForzado` stays empty across a plain play/pause cycle on `EstadoRadio` outside any alarm flow. Run `flutter test` — confirm it fails or passes vacuously (should already pass since no other code path calls these methods yet — treat as a REGRESSION GUARD, not a RED/GREEN pair, if 3.3-3.10 are already in place).
|
||||
- [ ] 3.12 (REFACTOR) Run `flutter test` for the full suite plus `flutter analyze` — confirm no regressions in existing alarm/radio tests.
|
||||
|
||||
## Phase 4: Fade-In Dedup at Handoff (Slice 3, strict TDD)
|
||||
|
||||
- [ ] 4.1 (RED) In `test/pantallas/pantalla_alarma_sonando_test.dart` (or a new focused fade-in test file), add a widget test asserting the Dart fade-in ramp (observable via `FakeServicioAudio`/fallback player volume changes) does NOT start before `confirmarAudioFlutter` has been invoked on the android port (i.e., before `_confirmarAudioFlutterListo()` runs) — assert `env.android.detenidas` (which `confirmarAudioFlutter` appends to, per `FakePuertoAlarmasAndroid.confirmarAudioFlutter`) is non-empty before any volume-ramp step is observed. Run `flutter test` — confirm it fails (current code starts the ramp at L66 immediately after `radio.reproducir`, before confirmation).
|
||||
- [ ] 4.2 (RED) Add a test asserting the fade-in DOES start once `_confirmarAudioFlutterListo()` has run (radio path via `estadoStream` emitting `reproduciendo`, or fallback path via `_iniciarFallback`) — the ramp must still function end-to-end after the gate. Run `flutter test` — confirm it fails or is trivially satisfied depending on 4.1's fixture; treat 4.1+4.2 as one RED pair validating gate correctness both ways.
|
||||
- [ ] 4.3 (GREEN) In `lib/pantallas/pantalla_alarma_sonando.dart`, remove the `_iniciarFadeIn()` call at L66 (radio path, currently fires immediately after `radio.reproducir(emisora)`) and the one at L96 (fallback path, currently fires before `_confirmarAudioFlutterListo()` at L97); move the single `_iniciarFadeIn()` invocation INTO `_confirmarAudioFlutterListo()` (L133-139) so it fires exactly once, after the `_audioFlutterConfirmado` guard, for both the radio and fallback paths. Run `flutter test` — confirm 4.1-4.2 pass.
|
||||
- [ ] 4.4 (REFACTOR) Re-run the full `pantalla_alarma_sonando_test.dart` and `pantalla_alarma_sonando_dismiss_guard_test.dart` suites — confirm no existing fade/dismiss/snooze assertions regressed from moving the ramp start point.
|
||||
- [ ] 4.5 `flutter analyze` and `dart format .` — confirm clean formatting/lint state for all Slice 3 edits.
|
||||
|
||||
## Phase 5: Manual/On-Device QA (mandatory human gate — Android 14+ physical or emulator device)
|
||||
|
||||
- [ ] 5.1 Set device media (`STREAM_MUSIC`) volume to 0. Trigger an alarm. Confirm the alarm rings audibly for its full duration, not just the pre-handoff native window (Scenario "Alarm is audible when device media volume is 0").
|
||||
- [ ] 5.2 Confirm `PluriWaveAlarmService` starts without `ForegroundServiceTypeException` on an Android 14+ (API 34+) device when the alarm fires from a background/killed-app broadcast context (Scenario "Native service starts from broadcast context on Android 14+").
|
||||
- [ ] 5.3 Note the original device media volume before triggering the alarm; dismiss the alarm; confirm media volume is restored to exactly the pre-alarm value (Scenario "Dismiss restores the original captured volume").
|
||||
- [ ] 5.4 Repeat 5.3 for snooze instead of dismiss (Scenario "Snooze restores the original captured volume").
|
||||
- [ ] 5.5 Listen across the native-to-Flutter handoff window; confirm there is no audible double-ramp/volume jump at the moment `confirmarAudioFlutter` fires (Scenario "No double-ramp interleaving at handoff").
|
||||
- [ ] 5.6 With no alarm ringing, play/pause normal radio repeatedly and confirm device volume controls behave exactly as before this change (no override side effects) — Scenario "Normal radio playback never triggers the override".
|
||||
- [ ] 5.7 Optional/best-effort: force-kill the app mid-ring and confirm the residual-volume-override gap is no worse than documented (Scenario "App killed mid-ring — best-effort restore only"; known accepted gap, not a blocking QA failure).
|
||||
- [ ] 5.8 Record QA sign-off (device model, Android version, pass/fail per scenario) before `sdd-verify`/merge.
|
||||
|
||||
## Phase 6: Final Static Sweep
|
||||
|
||||
- [ ] 6.1 Full-repo `rg 'systemExempted|SYSTEM_EXEMPTED'` across `android/app/src/main` — confirm zero remaining references (fully dropped, not kept alongside `alarm`).
|
||||
- [ ] 6.2 `flutter test` (full suite) and `flutter analyze` — final clean run before requesting review.
|
||||
- [ ] 6.3 `dart format .` — confirm no formatting diffs remain uncommitted.
|
||||
@@ -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,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,224 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user