Merges its delta requirements into the android-auto-media base spec. Completes the 3-phase local-music-in-Android-Auto feature. Standing pre-release gate: on-device/DHU validation of every native surface built across all 4 phases (SAF picking, metadata extraction, art cache, queue/shuffle handler wiring) is still outstanding.
193 lines
11 KiB
Markdown
193 lines
11 KiB
Markdown
# Tasks: Android Auto Local Music — Phase 3 (Folder Queue + Shuffle)
|
|
|
|
## CRITICAL — Testability Gap vs Design (must-read before Phase 2/3)
|
|
|
|
`PluriWaveAudioHandler` **cannot be instantiated in unit tests** (its constructor
|
|
builds a real `just_audio.AudioPlayer` needing platform `MethodChannel`s —
|
|
confirmed by `servicio_audio_eq_reapply_test.dart:12-15` and
|
|
`servicio_audio_source_switch_test.dart:13`; no test in the repo constructs it
|
|
directly). Design's "Handler (integration)" test row ("mocked player/fuente")
|
|
is therefore not achievable as written. Fix: extract the queue-advance
|
|
decisions into a NEW pure module (`lib/servicios/cola_local.dart`), mirroring
|
|
how `ControladorReconexion` was extracted from the same file — pure logic gets
|
|
real unit tests; the actual field mutation on `PluriWaveAudioHandler` is
|
|
**static-review-only**, verified against explicit acceptance criteria below
|
|
(same precedent as the EQ re-apply listener wiring).
|
|
|
|
## Review Workload Forecast
|
|
|
|
| Field | Value |
|
|
|-------|-------|
|
|
| Estimated changed lines | ~850-950 (prod ~440: cola_local.dart ~90, navegacion_auto.dart ~150, servicio_audio.dart ~150; tests ~470) |
|
|
| 400-line budget risk | High |
|
|
| Chained PRs recommended | Yes |
|
|
| Suggested split | PR 1 (queue foundation + sequential play + transport) → PR 2 (shuffle variant) |
|
|
| Delivery strategy | ask-on-risk |
|
|
| Chain strategy | pending (ask user) |
|
|
|
|
Decision needed before apply: Yes
|
|
Chained PRs recommended: Yes
|
|
Chain strategy: pending
|
|
400-line budget risk: High
|
|
|
|
**Split recommendation overrides line-count alone**: even if the line count
|
|
came in lower, split PR 1/PR 2 anyway — PR 1 is the only unit that touches
|
|
`PluriWaveAudioHandler`'s shared error/completion listener; isolating it lets
|
|
a reviewer verify the radio-regression guarantees without shuffle-logic noise.
|
|
|
|
### Suggested Work Units
|
|
|
|
| Unit | Goal | Likely PR | Notes |
|
|
|------|------|-----------|-------|
|
|
| 1 | `ColaLocal` + pure advance-decision + handler isolation/auto-advance/transport wiring + "Reproducir carpeta" + full regression re-run | PR 1 | Gate: all 4 existing suites byte-identical pass count before merge |
|
|
| 2 | Fisher-Yates shuffle + "Reproducir aleatorio" + collision test | PR 2, base = PR 1 branch | No handler changes beyond routing the new prefix through the existing seam |
|
|
|
|
## Phase 0: Regression Baseline (run BEFORE any code change)
|
|
|
|
- [x] 0.1 Run `test/servicios/servicio_audio_reconnect_test.dart`,
|
|
`servicio_audio_session_test.dart`, `servicio_audio_source_switch_test.dart`,
|
|
`servicio_audio_eq_reapply_test.dart`; record exact pass counts per file.
|
|
Baseline: 8 / 5 / 3 / 5 = 21 total, 0 failures.
|
|
- [x] 0.2 Saved the baseline pass counts as the comparison target for Phase 5.4
|
|
(see apply-progress.md).
|
|
|
|
## Phase 1: Pure Foundations (`lib/servicios/cola_local.dart`, new)
|
|
|
|
- [x] 1.1 RED: write `ColaLocal` tests — `conSiguiente`/`conAnterior` on empty,
|
|
single-item, mid-list, and boundary (first/last index) cases.
|
|
GREEN: implement immutable `ColaLocal` per design's contract. REFACTOR.
|
|
`lib/servicios/cola_local.dart`, `test/servicios/cola_local_test.dart`.
|
|
- [x] 1.2 RED: write `decidirAvanceCola` tests — pure function
|
|
`(colaLocal, avanzandoCola, trackCompletado) -> DecisionAvanceCola
|
|
{ninguna|avanzar|desactivar}`. Cover: `colaLocal==null` → `ninguna`
|
|
(radio-isolation proxy, since radio never sets `_colaLocal`);
|
|
`avanzandoCola==true` → `ninguna` (double-advance latch, simulate two
|
|
rapid `trackCompletado` calls); `conSiguiente()==null` → `desactivar`.
|
|
GREEN: implement in `cola_local.dart` (no `just_audio` import — caller
|
|
maps `ProcessingState.completed` to the bool). REFACTOR.
|
|
- [x] 1.3 RED: write `avanceEsValido(colaLocalActual, siguienteEsperado)` tests
|
|
— `identical()` on two structurally-equal-but-distinct `ColaLocal`
|
|
instances must return `false` (guards against an accidental `==` swap
|
|
in the mid-await race guard). GREEN: implement as thin `identical()`
|
|
wrapper. REFACTOR. Gotcha found: `const` instances canonicalize in
|
|
Dart, must use non-const `ColaLocal(...)` for the distinct-instance
|
|
test case (documented in the test and saved to engram).
|
|
|
|
## Phase 2: Pure Foundations — Shuffle & Collision (do NOT block Phase 3-5)
|
|
|
|
- [x] 2.1 RED: `mezclarFisherYates(List<NodoLocal>, Random)` tests —
|
|
determinism under fixed seed (`Random(42)` twice → identical output
|
|
order) and distribution sanity (1000 runs of a 5-item list, assert
|
|
every item appears in every position at least once). GREEN: implement
|
|
Fisher-Yates over the name-sorted list. REFACTOR.
|
|
- [x] 2.2 RED: `pistasEnOrdenAleatorio`/`pistasEnOrdenNombre` tests. GREEN.
|
|
- [x] 2.3 RED: collision-freedom test — for each of `carpeta_local_reproducir:`
|
|
and `carpeta_local_aleatorio:`, assert `startsWith` is false against
|
|
ALL 8 existing prefixes (`emisora:`, `grupo:`, `eq_preset:`,
|
|
`carpeta_local:`, `carpeta_local_pag:`, `carpeta_local_ord:`,
|
|
`carpeta_local_bucket:`, `pista:`) and vice-versa; assert first
|
|
diverging char at index 14 (`r`/`a` vs `p`/`o`/`b`). GREEN: add the two
|
|
`const` prefixes + predicates + strip-by-length codec in
|
|
`navegacion_auto.dart`.
|
|
|
|
## Phase 3: Handler Isolation Wiring — `servicio_audio.dart` (HIGHEST RISK)
|
|
|
|
Static-review-only (no handler instantiation possible). Verify each item by
|
|
reading the diff against its acceptance criterion; do not mark done without
|
|
the read.
|
|
|
|
- [x] 3.1 Split `playMediaItem`: public method clears
|
|
`_colaLocal = null; _avanzandoCola = false;` FIRST, then delegates to
|
|
new private `_encolarCambioFuente(item)` (current body of
|
|
`playMediaItem` minus the two new clear lines).
|
|
**Acceptance**: every existing external call site (`ServicioAudio.reproducir`,
|
|
`reproducirPorMediaId`, `reproducirPistaLocal`, `eq_preset` non-path)
|
|
still routes through the public method unchanged.
|
|
- [x] 3.2 Add `_reproducirEntradaCola(NodoLocal)`: sets/keeps `_colaLocal`,
|
|
calls `_encolarCambioFuente` WITHOUT clearing. **Acceptance**: this is
|
|
the only other caller of `_encolarCambioFuente` besides public
|
|
`playMediaItem`.
|
|
- [x] 3.3 Wire `_manejarFinPista(ProcessingState proc)` as the first line of
|
|
the existing `playerStateStream` listener (`servicio_audio.dart:253`),
|
|
calling `decidirAvanceCola` (1.2) with
|
|
`trackCompletado: proc == ProcessingState.completed`. **Acceptance**:
|
|
for every decision, `avanzar` sets `_avanzandoCola = true` synchronously
|
|
before any `await`; `desactivar` calls `_desactivarCola()` (clears
|
|
`_colaLocal`) + `stop()`; `ninguna` is a no-op — radio's `playing &&
|
|
ready` reset branch (line 256-260) is untouched.
|
|
- [x] 3.4 Wire the advance flow per ADR-3: compute `siguiente =
|
|
cola.conSiguiente()`; capture it; `await uriContenidoDePista`; guard
|
|
with `avanceEsValido(_colaLocal, siguiente)` (1.3) before calling
|
|
`_reproducirEntradaCola`; reset `_avanzandoCola` when the next track
|
|
reaches `playing && ready`.
|
|
- [x] 3.5 `stop()` (`servicio_audio.dart:702`) clears `_colaLocal = null;
|
|
_avanzandoCola = false;` alongside existing reconnect-cancel logic.
|
|
**Acceptance**: `onTaskRemoved` (calls `stop()`) inherits this for free.
|
|
- [x] 3.6 `skipToNext`/`skipToPrevious` overrides: no-op when
|
|
`_colaLocal == null`; otherwise call `conSiguiente`/`conAnterior` (1.1
|
|
already proves clamping at boundaries) and `_reproducirEntradaCola`.
|
|
- [x] 3.7 Gate `controls`/`systemActions` in the `playerStateStream` listener:
|
|
add `MediaControl.skipToPrevious/skipToNext` only when
|
|
`_colaLocal != null`. **Acceptance**: read the diff and confirm the
|
|
`_colaLocal == null` branch is byte-identical to the pre-change
|
|
control/action list (radio regression guard).
|
|
- [x] 3.8 `eq_preset:` branch in `playFromMediaId`
|
|
(`servicio_audio.dart:812-824`) stays FIRST, unconditional `return`,
|
|
untouched — confirm no `_colaLocal` reference was added to it.
|
|
|
|
## Phase 4: Browsable/Playable Action Items — `navegacion_auto.dart`
|
|
|
|
- [x] 4.1 RED: `itemsLocales` page-0 prepend tests — folder with ≥1 direct
|
|
audio child prepends "Reproducir carpeta" + "Reproducir aleatorio"
|
|
(both `playable: true`, prefixed ids); folder with 0 direct audio
|
|
children (only subfolders) prepends neither (empty-folder edge case).
|
|
GREEN: extend the page-0 prepend block (`navegacion_auto.dart:448-457`)
|
|
mirroring `ofreceOrdenCalidad`'s guard shape.
|
|
- [x] 4.2 RED/GREEN: `reproducirCarpetaLocal(id, {aleatorio, fuente, rng,
|
|
iniciarCola})` — fetches `fuente.hijos(docId)`, filters non-directory,
|
|
sorts/shuffles, calls `iniciarCola`; no-op on empty/unresolvable
|
|
folder (fake `FuenteMusicaLocalAuto`, no handler needed).
|
|
- [x] 4.3 Wire the two new prefixes into `playFromMediaId`
|
|
(`servicio_audio.dart:801`) as a THIRD branch (after `eq_preset:` and
|
|
`pista:`, before the station fallthrough), calling
|
|
`reproducirCarpetaLocal` with `iniciarCola` = a private helper that
|
|
sets `_colaLocal` and calls `_reproducirEntradaCola` (3.2).
|
|
Static-review-only for this wiring line.
|
|
|
|
## Phase 5: Open Question + Final Regression
|
|
|
|
- [x] 5.1 Confirm (static read, no code change): `ControladorReconexion`
|
|
(`controlador_reconexion.dart`) has no source-type parameter in
|
|
`registrarFallo` — it structurally cannot special-case a local-track
|
|
error vs a radio error, so leaving it untouched is provably safe.
|
|
Confirmed by direct read; untouched.
|
|
- [x] 5.2 Grep `servicio_audio_reconnect_test.dart` for existing
|
|
`maxReintentos`/`agotado` coverage (confirmed present: lines 115-137,
|
|
using an abbreviated `maxReintentos: 2`). The default-`maxReintentos:
|
|
5` "no 6th retry, no hang/crash" case was missing, so it was ADDED —
|
|
but in a NEW file (`test/servicios/controlador_reconexion_local_test.dart`),
|
|
not inside the protected `servicio_audio_reconnect_test.dart`, so that
|
|
suite's exact pass count (8) stays byte-identical for Phase 5.4's
|
|
regression gate. 3 new tests there prove: 5 failures schedule 5
|
|
backoff retries, the 6th returns `agotado` with no new timer created,
|
|
and all timers end cancelled (no hang).
|
|
- [x] 5.3 Updated design's Open Questions checklist (see design.md edit in
|
|
this same apply pass): both items confirmed — `ControladorReconexion`
|
|
left untouched; deactivate + stop on terminal error/unresolvable URI.
|
|
- [x] 5.4 Re-ran all 4 suites from Phase 0.1; EXACT pass counts match the
|
|
Phase 0.2 baseline byte-for-byte: reconnect 8/8, session 5/5,
|
|
source_switch 3/3, eq_reapply 5/5 — 21/21 total, 0 failures, same
|
|
test names/order. No mismatch.
|
|
- [x] 5.5 Ran the `literal-encoding` corruption scan over all touched/new
|
|
`.dart` files — no `Ã.|â€|�` matches found.
|
|
|
|
## Phase 6: Deviations / Manual Follow-up (not executable here)
|
|
|
|
- [ ] 6.1 `flutter analyze`, `flutter test --coverage`, `flutter gen-l10n` —
|
|
manual follow-up, not run by this agent (per strict-TDD scope: only
|
|
the target test files were run, per instruction).
|
|
- [x] 6.2 Native Kotlin: none touched by this change (confirmed — design's
|
|
File Changes table lists no native/manifest changes; no native file
|
|
was touched during apply) — static-review only, no code change
|
|
needed.
|