ExoPlayer assigns a new audio session id after transient audio-focus interruptions (navigation prompts, radar warnings), leaving the AndroidEqualizer attached to the dead session so playback resumed without equalization until the next station switch. The session-id listener now detects genuine rotations through a dedicated guard and re-activates the equalizer with the current preset, gated on EQ availability to stay clear of player teardown/rebuild.
159 lines
12 KiB
Markdown
159 lines
12 KiB
Markdown
# Tasks: EQ Re-Apply After Audio-Focus Ducking
|
|
|
|
Change: `eq-audiofocus-reapply`
|
|
Delivery strategy: `ask-on-risk` (single PR — see Review Workload Forecast below)
|
|
|
|
## Review Workload Forecast
|
|
|
|
| Metric | Value |
|
|
|---|---|
|
|
| Production files touched | 1 (`lib/servicios/servicio_audio.dart`) |
|
|
| New test files | 1 (`test/servicios/servicio_audio_eq_reapply_test.dart`) |
|
|
| Estimated changed lines (production) | ~25-35 |
|
|
| Estimated changed lines (tests) | ~50-70 |
|
|
| Estimated total changed lines | ~75-105 |
|
|
| Chained PRs recommended | No |
|
|
| 400-line budget risk | Low |
|
|
| Decision needed before apply | No — proceed as single PR, no `size:exception` needed |
|
|
|
|
---
|
|
|
|
## 1. RED — Predicate truth table (Strict TDD, unit-testable)
|
|
|
|
Target: `PluriWaveAudioHandler.debeReaplicarEcualizador` (static, `@visibleForTesting`), per design's Interfaces/Contracts section. All cases below MUST be written and MUST FAIL before any production code changes (task 2).
|
|
|
|
- [x] **1.1** Create `test/servicios/servicio_audio_eq_reapply_test.dart` with a doc comment mirroring `servicio_audio_source_switch_test.dart`'s pattern: state which spec requirement this covers and why only the static predicate is tested (handler cannot be instantiated in unit tests — MethodChannels).
|
|
- Satisfies: Spec Testability Matrix (predicate is the only unit-testable surface)
|
|
- Parallel: No (foundation for 1.2-1.6)
|
|
|
|
- [x] **1.2** Test case: rotation while playing → `true`. `debeReaplicarEcualizador(sessionId: 2, ultimaSessionIdEq: 1, eqDisponible: true)` returns `true`.
|
|
- Satisfies: Spec — Requirement "Session Id Rotation Triggers EQ Re-Apply" / Scenario "Session id rotates while playing"
|
|
- Parallel: Yes (independent assertion, same file)
|
|
|
|
- [x] **1.3** Test case: same id re-emitted → `false`. `debeReaplicarEcualizador(sessionId: 1, ultimaSessionIdEq: 1, eqDisponible: true)` returns `false`.
|
|
- Satisfies: Spec — Scenario "Same id re-emitted produces no redundant re-apply"
|
|
- Parallel: Yes
|
|
|
|
- [x] **1.4** Test case: first activation / matching guard → `false` (guards against double-apply after station switch already set the guard). `debeReaplicarEcualizador(sessionId: 1, ultimaSessionIdEq: 1, eqDisponible: true)` — same shape as 1.3, kept as a distinct named case per design's testing table to document intent (first-legitimate-activation, not just "duplicate").
|
|
- Satisfies: Spec — Scenario "First legitimate activation is not double-applied"
|
|
- Parallel: Yes
|
|
|
|
- [x] **1.5** Test case: null id → `false`. `debeReaplicarEcualizador(sessionId: null, ultimaSessionIdEq: 1, eqDisponible: true)` returns `false`.
|
|
- Satisfies: Spec — Requirement "Session Id Rotation Triggers EQ Re-Apply" (non-null precondition)
|
|
- Parallel: Yes
|
|
|
|
- [x] **1.6** Test case: teardown gate → `false`. `debeReaplicarEcualizador(sessionId: 9, ultimaSessionIdEq: 1, eqDisponible: false)` returns `false`.
|
|
- Satisfies: Spec — Requirement "Re-Apply Is Gated On EQ Availability" / Scenario "Rotation during player teardown is safely skipped"
|
|
- Parallel: Yes
|
|
|
|
- [x] **1.7** Run `flutter test test/servicios/servicio_audio_eq_reapply_test.dart` and confirm all 5 cases FAIL (predicate does not exist yet → compile error counts as RED).
|
|
- Satisfies: Strict TDD gate — no production code before confirmed RED
|
|
- Parallel: No (sequential checkpoint after 1.1-1.6)
|
|
|
|
---
|
|
|
|
## 2. GREEN — Implement predicate + wire listener
|
|
|
|
- [x] **2.1** Add `import 'package:flutter/foundation.dart' show visibleForTesting;` to `lib/servicios/servicio_audio.dart` (needed for `@visibleForTesting`; not currently imported).
|
|
- Satisfies: Design — Interfaces/Contracts (testable seam)
|
|
- Parallel: No (must precede 2.2)
|
|
|
|
- [x] **2.2** Add static predicate `debeReaplicarEcualizador({required int? sessionId, required int? ultimaSessionIdEq, required bool eqDisponible})` exactly as specified in design's Interfaces/Contracts, annotated `@visibleForTesting`. Place near the other EQ-related members (e.g., adjacent to `_activarEcualizador()` at current line 526, or near the class's static/testable members — implementer's choice, must not shadow/rename existing symbols).
|
|
- Satisfies: Design — Decision "Re-apply target + current-gains source"; Spec — all 5 scenarios (predicate is the shared decision logic)
|
|
- Parallel: No
|
|
|
|
- [x] **2.3** Run `flutter test test/servicios/servicio_audio_eq_reapply_test.dart` and confirm all 5 cases now PASS (predicate alone, no wiring yet).
|
|
- Satisfies: Strict TDD gate — GREEN checkpoint for the tested unit before touching the listener
|
|
- Parallel: No (sequential checkpoint after 2.2)
|
|
|
|
- [x] **2.4** Add field `int? _ultimaSessionIdEq;` to `PluriWaveAudioHandler`, placed near the existing `_androidAudioSessionId` field (current line ~196 area) per design's "Change-guard field separate from broadcast field" decision. Dedicated field — do NOT reuse `_androidAudioSessionId`.
|
|
- Satisfies: Spec — Requirement "Session Id Rotation Triggers EQ Re-Apply" ("Change tracking MUST use a dedicated field")
|
|
- Parallel: No (must precede 2.5)
|
|
|
|
- [x] **2.5** Extend the existing `_androidAudioSessionIdSub` listener (current lines 258-265) — after the existing broadcast (`_androidAudioSessionIdController.add(sessionId)`, line 263), add: call `debeReaplicarEcualizador(sessionId: sessionId, ultimaSessionIdEq: _ultimaSessionIdEq, eqDisponible: _eqDisponible)`; on `true`, set `_ultimaSessionIdEq = sessionId` then `unawaited(_activarEcualizador())`. Do NOT create a new subscription — this is a conditional addition inside the existing listener body, per design's "Conditional inside EXISTING _androidAudioSessionIdSub listener" decision.
|
|
- Satisfies: Spec — Requirement "Session Id Rotation Triggers EQ Re-Apply" (all 3 scenarios); Design — Data Flow section
|
|
- Parallel: No (depends on 2.2, 2.4)
|
|
|
|
- [x] **2.6** Reset `_ultimaSessionIdEq = null;` inside `_recrearPlayer()` (current lines 488-508), placed alongside the existing `_androidAudioSessionId = null;` reset (current line 504), so a rebuilt player's first session id is treated as fresh, not a rotation. Per design's Race Analysis: this reset plus the pre-existing `_eqDisponible = false` (line 503, set BEFORE the new subscription exists at line 507/`_conectarStreamsPlayer()`) is what closes the race window.
|
|
- Satisfies: Spec — Requirement "Re-Apply Is Gated On EQ Availability" / Scenario "Rotation during player teardown is safely skipped"; Design — Race Analysis
|
|
- Parallel: No (depends on 2.4; logically follows 2.5 but touches a different method)
|
|
|
|
- [x] **2.7** Run `flutter test test/servicios/servicio_audio_eq_reapply_test.dart` again — must still PASS (predicate is unaffected by listener/field wiring since it's called directly by tests, not through the handler instance).
|
|
- Satisfies: Strict TDD gate — confirm wiring did not regress the tested unit
|
|
- Parallel: No (sequential checkpoint after 2.4-2.6)
|
|
|
|
---
|
|
|
|
## 3. REFACTOR
|
|
|
|
- [x] **3.1** Review the added listener block and predicate for clarity/naming consistency with surrounding code (Spanish method/field names per project convention — `debeReaplicarEcualizador` matches `_activarEcualizador`/`aplicarPreset` naming style already in file). No behavior change.
|
|
- Satisfies: Strict TDD REFACTOR step
|
|
- Parallel: No
|
|
|
|
- [x] **3.2** Confirm no dead code / no unused imports introduced (the added `visibleForTesting` import is used; nothing else added should be orphaned).
|
|
- Satisfies: Code hygiene, supports Regression gate (task 4.2)
|
|
- Parallel: No
|
|
|
|
---
|
|
|
|
## 4. Regression + static analysis (full suite, unchanged call sites)
|
|
|
|
- [x] **4.1** Run `flutter analyze --no-fatal-infos` — must be clean (no new warnings/errors introduced by the field, predicate, import, or listener edit).
|
|
- Satisfies: Project convention (AGENTS.md analyzer gate)
|
|
- Parallel: No
|
|
- Result: `No issues found! (ran in 78.2s)`
|
|
|
|
- [x] **4.2** Run full `flutter test` suite — confirm `servicio_audio_source_switch_test.dart`, `servicio_audio_session_test.dart`, `servicio_audio_reconnect_test.dart`, and the new `servicio_audio_eq_reapply_test.dart` all pass. This is the only coverage for "station-switch and manual slider paths unchanged" — no new targeted tests for those paths per spec's Testability Matrix ("Regression only, not targeted unit tests").
|
|
- Satisfies: Spec — Non-Goals (station-switch, manual slider unchanged); Proposal — Success Criteria "Station-switch and manual slider (setBanda) paths unchanged"
|
|
- Parallel: No (run after 4.1)
|
|
|
|
Note: project history flags `flutter test` may hang in this local environment (per `sdd-init/pluriwave`) — if it hangs, isolate by running just the 4 files above explicitly rather than the full suite, and note this in the apply-progress artifact.
|
|
- Result: full `flutter test` hung as predicted (unrelated pre-existing loop in `test/estado/estado_alarmas_ejecuciones_test.dart`, outside this change's scope). Fallback used: ran the 4 files explicitly — all 21 tests passed (`All tests passed!`).
|
|
|
|
---
|
|
|
|
## 5. Manual on-device QA (not unit-testable — real MethodChannels/native EQ)
|
|
|
|
Per spec's Testability Matrix: `PluriWaveAudioHandler` cannot be instantiated in unit tests (real `AudioPlayer` needs platform MethodChannels). These scenarios require a physical/emulated Android device with the app installed and a non-flat preset applied.
|
|
|
|
- [ ] **5.1** QA scenario: play a station with a non-flat EQ preset active. Trigger a real ducking interruption (e.g., a Google Maps voice prompt, or another app that requests transient audio focus). Confirm music resumes EQUALIZED (not flat) after the interruption ends.
|
|
- Satisfies: Spec — Scenario "Session id rotates while playing"; Proposal — Success Criteria "After another app ducks/interrupts, resumed music stays equalized"
|
|
- Parallel: No (device-dependent, sequential QA pass)
|
|
|
|
- [ ] **5.2** QA scenario: repeat 5.1 with `_eqMultiDeviceEnabled` toggled ON, then again toggled OFF (via app UI). Confirm identical re-apply behavior in both states.
|
|
- Satisfies: Spec — Requirement "Re-Apply Is Independent Of Multi-Device Toggle"
|
|
- Parallel: No (sequential, depends on device availability from 5.1)
|
|
|
|
- [ ] **5.3** QA scenario: trigger a rapid station switch DURING an active ducking event (i.e., switch stations while another app still holds transient audio focus). Confirm no crash, no exception, and the new station ends up correctly equalized once playback stabilizes (validates the `_recrearPlayer()` teardown race gate from task 2.6 under real conditions).
|
|
- Satisfies: Spec — Scenario "Rotation during player teardown is safely skipped"; Design — Race Analysis
|
|
- Parallel: No (sequential, depends on device availability from 5.1)
|
|
|
|
- [ ] **5.4** QA scenario: play, apply a preset via the manual slider (`setBanda`), confirm gains apply as before (no regression). Then perform a normal explicit station switch, confirm EQ still activates via the existing station-switch path (line 449) with no double-apply artifacts (e.g., no audible gain "blip" from firing twice).
|
|
- Satisfies: Proposal — Success Criteria "Station-switch and manual slider (setBanda) paths unchanged"; Spec — Scenario "First legitimate activation is not double-applied"
|
|
- Parallel: No (sequential)
|
|
|
|
---
|
|
|
|
## Dependency Summary
|
|
|
|
```
|
|
1.1 -> 1.2..1.6 (parallel) -> 1.7 (RED checkpoint)
|
|
|
|
|
v
|
|
2.1 -> 2.2 -> 2.3 (GREEN checkpoint, predicate only)
|
|
|
|
|
v
|
|
2.4 -> 2.5 -> 2.6 -> 2.7 (GREEN checkpoint, full wiring)
|
|
|
|
|
v
|
|
3.1 -> 3.2 (REFACTOR)
|
|
|
|
|
v
|
|
4.1 -> 4.2 (regression + analyzer)
|
|
|
|
|
v
|
|
5.1 -> 5.2 -> 5.3 -> 5.4 (manual QA, sequential, device-dependent)
|
|
```
|
|
|
|
No task group is safely parallelizable across phases (each phase gates the next per Strict TDD). Within phase 1, test cases 1.2-1.6 can be written in any order/concurrently since they are independent assertions in the same new file.
|