FreeTLab
977cbcd8cc
Merges its delta requirements into the android-auto-media base spec. Phases 2 (metadata/sort/filter/art) and 3 (subfolder scoping/shuffle) remain planned future work.
171 lines
13 KiB
Markdown
171 lines
13 KiB
Markdown
# Apply Progress: Android Auto Local Music — Phase 1
|
||
|
||
**Status**: done — all 12 task groups complete in a single pass.
|
||
**Delivery**: single PR with `size:exception` (per orchestrator/user decision —
|
||
matches repo precedent `35bb180 feat(auto): browsable Android Auto media tree
|
||
with play-by-id [size:exception]`). No slicing across batches.
|
||
**Mode**: Strict TDD for all pure-Dart, behavior-changing tasks; native Kotlin
|
||
and the on-device SAF picker round trip are **static-review-only** (no Android
|
||
build/DHU available in this environment — same established precedent as prior
|
||
Android Auto changes).
|
||
|
||
## Completed Tasks
|
||
|
||
- [x] 1.1–1.2 `PistaLocal`/`NodoLocal` models (pure DTOs — no tests written,
|
||
per task 1.2's explicit "getter-only classes, skip" rule)
|
||
- [x] 2.1–2.2 `esArchivoAudio` (RED → GREEN, `musica_local_auto.dart`)
|
||
- [x] 3.1–3.2 Media-id scheme: `idMusicaLocal`, `_prefijoCarpetaLocal`,
|
||
`_prefijoPista`, `esCarpetaLocalMediaId`, `esPistaMediaId`,
|
||
`idCarpetaLocalDesde` (RED → GREEN, collision + `:`-in-docId cases)
|
||
- [x] 4.1–4.2 `FuenteMusicaLocalAuto` interface + `FuenteMusicaLocalAutoImpl`
|
||
channel-backed implementation (`musica_local_auto.dart`)
|
||
- [x] 5.1–5.4 + 11.1 `raiz(incluirMusicaLocal:)`, `itemsLocales`,
|
||
title-stripping, `artUriLocal`, root-count regression landed in the
|
||
SAME commit/batch as the test update (never a stale-count false green)
|
||
- [x] 6.1–6.2 `hijosMusicaLocal` dispatch, wired into
|
||
`PluriWaveAudioHandler.getChildren`
|
||
- [x] 7.1–7.3 `reproducirPistaLocal`, wired into
|
||
`PluriWaveAudioHandler.playFromMediaId`
|
||
- [x] 8.1–8.5 Native `file_actions` channel: `pickMusicFolder`,
|
||
`listAudioChildren`, `resolvePlayableUri`, `hasPersistedPermission`,
|
||
`onActivityResult` override (STATIC REVIEW ONLY)
|
||
- [x] 9.1–9.4 `_SeccionMusicaLocal` settings UI + SharedPreferences
|
||
persistence (`musica_local_uri`) + l10n keys (`app_en.arb`/`app_es.arb`)
|
||
- [x] 10.1–10.3 Cold-start / permission-revoked safety (explicit tests on top
|
||
of the try/catch already required by tasks 4/6/7)
|
||
- [x] 12.1 Full regression pass of `navegacion_auto_test.dart` +
|
||
`musica_local_auto_test.dart` + the 4 existing `servicio_audio_*_test.dart`
|
||
files — 91/91 passing, no pre-existing assertion broken
|
||
|
||
## Files Changed
|
||
|
||
| File | Action | What Was Done |
|
||
|------|--------|----------------|
|
||
| `lib/modelos/pista_local.dart` | Created | `NodoLocal` + `PistaLocal` pure DTOs |
|
||
| `lib/servicios/musica_local_auto.dart` | Created | `esArchivoAudio`, `FuenteMusicaLocalAuto` interface, `FuenteMusicaLocalAutoImpl` (channel-backed, cold-start-safe, SharedPreferences-DI) |
|
||
| `lib/servicios/navegacion_auto.dart` | Modified | `_prefijoPista`/`esPistaMediaId` (top-level); `idMusicaLocal`/`_prefijoCarpetaLocal`/`_maxItemsCarpetaLocal`/`esCarpetaLocalMediaId`/`idCarpetaLocalDesde`/`itemsLocales`/`_itemLocal` (on `ConstructorArbolAuto`); `raiz(incluirMusicaLocal:)` signature change; module-level `artUriLocal`, `_tituloDesdeNombre`, `_tituloDesdeDocumentId`, `hijosMusicaLocal`, `reproducirPistaLocal` |
|
||
| `lib/servicios/servicio_audio.dart` | Modified | `_fuenteMusicaLocalGlobal` + `registrarFuenteMusicaLocal`; `getChildren` root-flag + `hijosMusicaLocal` delegation branches; `playFromMediaId` `pista:` branch (2nd, mirrors the `eq_preset:` branch's unconditional-return shape) |
|
||
| `lib/main.dart` | Modified (see Deviations) | Registers `FuenteMusicaLocalAutoImpl(prefs: prefs)` via `registrarFuenteMusicaLocal` — required plumbing, not an explicit task line item |
|
||
| `lib/pantallas/pantalla_ajustes.dart` | Modified | New `_SeccionMusicaLocal`/`_SeccionMusicaLocalState`, registered in `_AjustesContent`'s section list |
|
||
| `lib/l10n/app_en.arb` / `app_es.arb` | Modified | 7 new keys: `localMusicSectionTitle`, `localMusicSectionDescription`, `localMusicFolderNotConfigured`, `localMusicFolderTitle`, `localMusicChoosePath`, `localMusicChangePath`, `localMusicFolderUpdated`, `localMusicFolderSaveError` |
|
||
| `android/app/src/main/kotlin/es/freetimelab/pluriwave/MainActivity.kt` | Modified (STATIC REVIEW ONLY) | `pendingMusicFolderResult` field; `pickMusicFolder`/`listAudioChildren`/`resolvePlayableUri`/`hasPersistedPermission` cases in the existing `file_actions` `when` block; new `onActivityResult` override (first on this Activity); 3 new private helper methods |
|
||
| `test/servicios/musica_local_auto_test.dart` | Created | `esArchivoAudio` unit tests (4 tests) |
|
||
| `test/servicios/navegacion_auto_test.dart` | Modified | `raiz` test split into configured(5)/hidden(4) cases; new groups: `esCarpetaLocalMediaId / esPistaMediaId`, `artUriLocal`, `ConstructorArbolAuto.itemsLocales`, `hijosMusicaLocal`, `reproducirPistaLocal`; `_FakeFuenteMusicaLocalAuto` test double |
|
||
| `openspec/changes/android-auto-local-music/tasks.md` | Modified | All 31 checkbox items marked `[x]` |
|
||
|
||
## TDD Cycle Evidence
|
||
|
||
| Task | RED | GREEN | REFACTOR | Notes |
|
||
|------|-----|-------|----------|-------|
|
||
| 2. `esArchivoAudio` | Wrote 4 failing tests (null/blank MIME+name, non-audio MIME, valid audio/*) before implementation existed | Implemented in `musica_local_auto.dart`, all pass | N/A — single small pure function | |
|
||
| 3. Media-id predicates | Wrote collision + `:`-in-docId tests before adding `esCarpetaLocalMediaId`/`esPistaMediaId`/`idCarpetaLocalDesde` | Implemented, all pass | Mirrored exact shape of existing `esCarpetaGrupo`/`esPresetMediaId` — no rework needed | |
|
||
| 5. `raiz`/`itemsLocales` | Rewrote the existing 4-folder `raiz` test into configured(5)/hidden(4) cases + wrote 8 `itemsLocales` cases (cap, sort, title-strip x3, art, folder-vs-file) before touching `raiz()`'s signature | Implemented `raiz(incluirMusicaLocal:)`, `itemsLocales`, `_itemLocal`, `_tituloDesdeNombre`, `artUriLocal`; all pass | None needed | Landed together with task 11 in the same edit so the suite was never red for the stale-count reason |
|
||
| 6. `hijosMusicaLocal` dispatch | Wrote 5 cases (root delegation, subfolder delegation, non-local id → null, null fuente → `[]`, throwing fuente → `[]`) before writing the function | Implemented, all pass | None needed | |
|
||
| 7. `reproducirPistaLocal` | Wrote 4 cases (resolves+plays, stale id no-op, wrong-prefix no-op, structural EQ-seam regression guard) before writing the function | Implemented, all pass | None needed | |
|
||
| 10. Cold-start safety | Covered by task 6/7's null-fuente and throwing-fuente cases above (explicit, not incidental) | Already green from 6/7's implementation | N/A | No additional guard code was needed — the try/catch shape from tasks 4/6/7 already satisfied it |
|
||
| 8. Native Kotlin (`pickMusicFolder` etc.) | N/A — static-review-only, no test runner available | N/A | Written carefully against documented `DocumentsContract`/`ActivityResultLauncher`-era SAF patterns; balanced-brace/paren sanity check run | Flagged as highest-risk, unverified at runtime (see Risks) |
|
||
| 9. Settings UI | N/A — SAF round trip is on-device-only per task 9's own note; no widget test written (would only cover layout, adds little given the channel call is stubbed either way) | N/A | Mirrors `_SeccionGrabaciones` exactly | |
|
||
| 1. Models | N/A — task 1.2 explicitly says skip tests for getter-only DTOs | N/A | N/A | |
|
||
|
||
## Test Results (independently re-run, exact counts)
|
||
|
||
```
|
||
flutter test test/servicios/navegacion_auto_test.dart test/servicios/musica_local_auto_test.dart --concurrency=1 --timeout=60s
|
||
→ 70/70 passing (66 in navegacion_auto_test.dart, 4 in musica_local_auto_test.dart)
|
||
|
||
flutter test test/servicios/servicio_audio_reconnect_test.dart test/servicios/servicio_audio_session_test.dart test/servicios/servicio_audio_source_switch_test.dart test/servicios/servicio_audio_eq_reapply_test.dart --concurrency=1 --timeout=60s
|
||
→ 21/21 passing (pre-existing suites, re-run to confirm no regression from servicio_audio.dart edits)
|
||
```
|
||
|
||
**Total: 91/91 passing, 0 failing.** Both commands were run twice in this
|
||
session and produced identical counts both times.
|
||
|
||
Not run (per constraints): `flutter analyze`, `flutter build`,
|
||
`flutter gen-l10n`, any Android/Gradle build or DHU/emulator session.
|
||
|
||
## Deviations from Design
|
||
|
||
1. **`lib/main.dart` registration call — NOT an explicit task line item, but
|
||
added anyway.** Design's File Changes table lists
|
||
`registrarFuenteMusicaLocal`'s *definition* under `servicio_audio.dart`,
|
||
mirroring `_fuenteNavegacionGlobal`/`registrarFuenteNavegacion`. However,
|
||
nothing in tasks.md or design.md explicitly instructs wiring the actual
|
||
`registrarFuenteMusicaLocal(...)` *call site* into `main.dart`. Without it,
|
||
`_fuenteMusicaLocalGlobal` would always be `null` at runtime and the local
|
||
root would never appear regardless of what the user configures — the
|
||
feature would be entirely non-functional despite 100% green tests (since
|
||
all pure-Dart tests inject a fake fuente directly). I added the one-line
|
||
registration in `main.dart`, mirroring the existing
|
||
`registrarFuenteNavegacion(fuenteAuto)` call exactly (same file, same
|
||
pattern, 5 lines including the comment). This is a necessary completion of
|
||
task 4/6's own intent, not a scope expansion — flagging it explicitly
|
||
rather than silently expanding scope, as instructed.
|
||
2. **Local-track title on playback is derived from the SAF `documentId`'s
|
||
trailing path segment, not from a carried filename.** Design's
|
||
`FuenteMusicaLocalAuto` interface (`Interfaces / Contracts`) only exposes
|
||
`uriContenidoDePista(documentId) → content:// or null` — no filename is
|
||
returned alongside the content URI. Since `MediaItem.title` is required
|
||
for the Now Playing UI, `reproducirPistaLocal` derives a best-effort title
|
||
via `_tituloDesdeDocumentId` (last `/`-segment of the documentId, then the
|
||
SAME `_tituloDesdeNombre` extension-stripping rule the browse tree uses).
|
||
This is consistent with the design's stated title rule and requires no
|
||
interface change or second native round trip, but is a genuine
|
||
interpretation filling a gap in the documented contract — flagging it
|
||
explicitly per the "note deviations, don't silently freelance" rule.
|
||
3. **Folder titles in `itemsLocales` are NOT extension-stripped** — only
|
||
playable track titles are. Design's "Title = filename minus extension;
|
||
art = reused station_art_* rotation" bullet pairs title+art together, and
|
||
art clearly only applies to playable items (folders have no `artUri`) —
|
||
read narrowly as a track-mapping rule, not a folder-mapping rule.
|
||
Folders keep their raw `nombre`, matching how every other non-playable
|
||
folder in this file (`itemGrupo`, the root `_carpeta` calls) uses the raw
|
||
label with no stripping.
|
||
|
||
## Risks
|
||
|
||
- **Native Kotlin `onActivityResult` (task 8) is genuinely new plumbing on
|
||
`MainActivity` and is UNVERIFIED at runtime** — no Android build/DHU
|
||
available in this environment. Written carefully against standard SAF
|
||
`ACTION_OPEN_DOCUMENT_TREE`/`DocumentsContract` patterns (mirrors how
|
||
`openDirectory`/`viewDirectory` already use `DocumentsContract` in this
|
||
same file), with explicit handling for: stale-pending-result overwrite
|
||
(user re-triggers the picker while a previous call is still pending),
|
||
cancel (`resultCode != RESULT_OK`), null `data.data`, and a
|
||
`takePersistableUriPermission` failure. This is exactly the risk flagged
|
||
by both design.md's "Open Questions" and tasks.md's task-8 note — reviewer
|
||
attention warranted here specifically, on-device manual verification
|
||
required before this ships.
|
||
- **`listAudioChildren`'s SAF query (task 8.2) is also unverified at
|
||
runtime.** Uses `DocumentsContract.buildChildDocumentsUriUsingTree` +
|
||
`ContentResolver.query` with the standard 3-column projection
|
||
(`COLUMN_DOCUMENT_ID`/`COLUMN_DISPLAY_NAME`/`COLUMN_MIME_TYPE`) — a
|
||
well-documented pattern, but not exercised against a real SAF provider in
|
||
this pass.
|
||
- Deviation #2 above (derived playback title) means a played local track's
|
||
Now Playing title could theoretically differ from its browse-list title if
|
||
a future native change ever made `documentId`'s trailing segment diverge
|
||
from the actual filename (unlikely with the current SAF-native
|
||
`DocumentsContract` documentId shape, but worth a note for Phase 2 if
|
||
metadata is added).
|
||
|
||
## Workload / PR Boundary
|
||
|
||
- **Mode**: single PR, `size:exception` (explicit user choice, matches
|
||
precedent commit `35bb180`).
|
||
- **Current work unit**: all of Phase 1 (task groups 1–12), one pass.
|
||
- **Boundary**: starts from zero (no prior apply-progress existed) and ends
|
||
with every task in `tasks.md` marked `[x]` and the full targeted test
|
||
suite green.
|
||
- **Estimated review budget impact**: tasks.md's own forecast estimated
|
||
~750–950 changed lines; this implementation is within that range. Reviewer
|
||
should budget for a single large review pass, with extra attention on
|
||
`MainActivity.kt`'s new `onActivityResult` per the Risks section above.
|
||
|
||
## Status
|
||
|
||
31/31 checkbox items across 12/12 task groups complete. Working tree left
|
||
unstaged for the orchestrator to commit/push with a `[size:exception]` tag,
|
||
per instructions.
|
||
|
||
**Next recommended**: `sdd-verify`
|