fix(auto): fall back to brand art and surface quality on dead/missing favicons
Android Auto no longer copies the launcher icon as placeholder art; it rotates through the same 4 on-brand station_art assets the phone UI already uses, keyed by the same per-station hash for visual parity. Malformed or unusable favicon URLs (including a Dart Uri quirk where 'http://' reports hasAuthority=true with an empty host) now fail the validity gate instead of being handed to the OS media browser as-is. Browsable items also show codec/bitrate as a subtitle when known.
This commit is contained in:
Binary file not shown.
|
Before Width: | Height: | Size: 84 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 1.8 MiB |
Binary file not shown.
|
After Width: | Height: | Size: 2.0 MiB |
Binary file not shown.
|
After Width: | Height: | Size: 1.7 MiB |
Binary file not shown.
|
After Width: | Height: | Size: 1.8 MiB |
@@ -9,15 +9,75 @@ import '../modelos/emisora.dart';
|
||||
import 'persistencia_tolerante.dart';
|
||||
import 'servicio_favoritos.dart';
|
||||
|
||||
/// URI of the bundled default station artwork, served from
|
||||
/// `android/app/src/main/res/drawable` via `android.resource://` (Design
|
||||
/// "default artwork delivery" — no per-URI grant needed, works offline, and
|
||||
/// cannot 404 unlike a FileProvider content URI or a remote placeholder).
|
||||
const String _defaultArtUri =
|
||||
'android.resource://es.freetimelab.pluriwave/drawable/default_station_art';
|
||||
|
||||
const _prefijoEmisora = 'emisora:';
|
||||
|
||||
/// Canonical on-brand fallback-art names and rotation order, ported
|
||||
/// **verbatim** (same formula, same order) from
|
||||
/// `lib/widgets/tarjeta_emisora.dart`'s `_fallbackArtFor` (lines 363-367)
|
||||
/// to guarantee phone/car per-station art parity (Design "Fallback-art
|
||||
/// selection"). Keep this list in sync with that one — there is no
|
||||
/// structural enforcement of order, only this comment and the parity test
|
||||
/// in `navegacion_auto_test.dart` (group `parity: phone/auto art order`).
|
||||
const _nombresArte = ['aurora', 'cosmic', 'pulse', 'nova'];
|
||||
|
||||
/// Returns whether [favicon] is usable as a remote `artUri` (Design
|
||||
/// Decision "Case B detection" — static validity gate, zero network):
|
||||
/// non-null, non-blank after trimming, and parses as an absolute
|
||||
/// `http`/`https` URI with a non-empty authority. Does **not** probe
|
||||
/// reachability — the OS art loader fetches `artUri` independently and
|
||||
/// later, so a build-time network check would be racy (TOCTOU); this only
|
||||
/// catches the deterministic malformed/non-http(s) subset (bare hosts,
|
||||
/// wrong scheme, `http://` with no authority, whitespace, unparseable).
|
||||
bool faviconUsable(String? favicon) {
|
||||
final trimmed = favicon?.trim();
|
||||
if (trimmed == null || trimmed.isEmpty) return false;
|
||||
final uri = Uri.tryParse(trimmed);
|
||||
if (uri == null) return false;
|
||||
// `Uri.hasAuthority` is true whenever a `//` authority slot is present,
|
||||
// even with an empty host (e.g. `Uri.parse('http://').hasAuthority` is
|
||||
// `true`) — check `host.isNotEmpty` explicitly to actually require a
|
||||
// non-empty authority host.
|
||||
return (uri.scheme == 'http' || uri.scheme == 'https') &&
|
||||
uri.host.isNotEmpty;
|
||||
}
|
||||
|
||||
/// Deterministic rotation index over the 4 on-brand fallback arts, same
|
||||
/// formula as `tarjeta_emisora.dart`'s `_fallbackArtFor` (Design
|
||||
/// "Fallback-art selection — port verbatim"): `seed` is the station uuid.
|
||||
int indiceArtePara(String seed) =>
|
||||
seed.codeUnits.fold<int>(0, (a, b) => a + b) % _nombresArte.length;
|
||||
|
||||
/// Resolves the `artUri` for [e] (Design "Data Flow"): the favicon when it
|
||||
/// passes [faviconUsable], otherwise a rotating `station_art_<name>`
|
||||
/// drawable URI selected via [indiceArtePara] over `e.uuid` — the same
|
||||
/// on-brand art the phone UI would pick for this station (per-station
|
||||
/// parity), never a launcher-icon lookalike.
|
||||
String artUriPara(Emisora e) => faviconUsable(e.favicon)
|
||||
? e.favicon!
|
||||
: 'android.resource://es.freetimelab.pluriwave/drawable/'
|
||||
'station_art_${_nombresArte[indiceArtePara(e.uuid)]}';
|
||||
|
||||
/// Formats a human-readable audio-quality hint for the browse row's
|
||||
/// `displaySubtitle` (Design Decision "`displaySubtitle` quality format"):
|
||||
/// `"<bitrate> kbps · <CODEC>"` when both are known, just the bitrate or
|
||||
/// just the codec when only one is known, and `null` (never `""`, never a
|
||||
/// string containing the literal `"null"`) when both are unknown. Codec is
|
||||
/// trimmed and upper-cased; blank-after-trim counts as unknown. `bitrate`
|
||||
/// `<= 0` counts as unknown (Radio Browser stores `0` for unknown).
|
||||
String? subtituloCalidad(Emisora e) {
|
||||
final codec = e.codec?.trim();
|
||||
final codecConocido = codec != null && codec.isNotEmpty;
|
||||
final bitrate = e.bitrate;
|
||||
final bitrateConocido = bitrate != null && bitrate > 0;
|
||||
|
||||
if (codecConocido && bitrateConocido) {
|
||||
return '$bitrate kbps · ${codec.toUpperCase()}';
|
||||
}
|
||||
if (bitrateConocido) return '$bitrate kbps';
|
||||
if (codecConocido) return codec.toUpperCase();
|
||||
return null;
|
||||
}
|
||||
|
||||
/// Browse-source abstraction for the Android Auto media tree (Design
|
||||
/// "getChildren data source, cold-start safe"). Kept separate from
|
||||
/// `EstadoRadio` so a headless Auto bind (`main()` runs but the widget tree
|
||||
@@ -97,21 +157,19 @@ class ConstructorArbolAuto {
|
||||
}
|
||||
|
||||
/// Maps a single [Emisora] to a playable `MediaItem`: id `emisora:<uuid>`
|
||||
/// (Design "media-id scheme"), title, and artUri with the default-art
|
||||
/// fallback (Design "default artwork delivery").
|
||||
/// (Design "media-id scheme"), title, on-brand-fallback-aware `artUri`
|
||||
/// (Design "Case B detection" + "Fallback-art selection") and a
|
||||
/// quality-hint `displaySubtitle` (Design "`displaySubtitle` quality
|
||||
/// format") when codec/bitrate are known.
|
||||
MediaItem itemEmisora(Emisora e) => MediaItem(
|
||||
id: '$_prefijoEmisora${e.uuid}',
|
||||
title: e.nombre,
|
||||
playable: true,
|
||||
artUri: Uri.parse(_artUriPara(e)),
|
||||
artUri: Uri.parse(artUriPara(e)),
|
||||
displaySubtitle: subtituloCalidad(e),
|
||||
extras: _contentStyleGrid,
|
||||
);
|
||||
|
||||
String _artUriPara(Emisora e) {
|
||||
final favicon = e.favicon;
|
||||
return (favicon != null && favicon.isNotEmpty) ? favicon : _defaultArtUri;
|
||||
}
|
||||
|
||||
/// Resolves `emisora:<uuid>` ids to the matching [Emisora] in [universo].
|
||||
/// Any other shape (no prefix, empty uuid, unmatched uuid) returns `null`
|
||||
/// instead of throwing (Spec "Media Item Resolution by ID").
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
# Apply Progress: auto-media-art-quality
|
||||
|
||||
**Mode**: Strict TDD
|
||||
**Batch**: 1 of 1 (single delivery, no chaining — forecast was Low risk, ~170-230 est. lines; actual ~347 text lines, still under 400-line budget)
|
||||
|
||||
## Completed Tasks
|
||||
|
||||
All 22 tasks in `tasks.md` are complete and marked `[x]`.
|
||||
|
||||
- [x] 1.1 [RED] `faviconUsable` test matrix
|
||||
- [x] 1.2 [GREEN] Implement `faviconUsable`
|
||||
- [x] 2.1 [RED] `indiceArtePara` formula/order test
|
||||
- [x] 2.2 [GREEN] Implement `indiceArtePara` + `_nombresArte`
|
||||
- [x] 3.1 [RED] `artUriPara` integration test (favicon gate + rotation)
|
||||
- [x] 3.2 [GREEN] Implement `artUriPara`; drop `_defaultArtUri` and old `_artUriPara`
|
||||
- [x] 3.3 [GREEN] Replace obsolete default-art test
|
||||
- [x] 4.1 [RED] `subtituloCalidad` matrix test
|
||||
- [x] 4.2 [GREEN] Implement `subtituloCalidad`
|
||||
- [x] 5.1 [RED] `itemEmisora` sets `displaySubtitle` test
|
||||
- [x] 5.2 [GREEN] Wire `displaySubtitle` into `itemEmisora`
|
||||
- [x] 5.3 [REFACTOR] Cleanup pass, doc comments, dead-reference check
|
||||
- [x] 6.1 [coverage] `Emisora.fromMap` unsanitized-favicon regression test
|
||||
- [x] 6.2 [coverage] Phone/auto art-order parity guard
|
||||
- [x] 7.1 Add 4 native drawable copies (static-review-only)
|
||||
- [x] 7.2 Delete `default_station_art.png` (static-review-only)
|
||||
- [x] 7.3 Verify `applicationId`/authority match (static-review-only)
|
||||
- [x] 8.1 Targeted regression run (`navegacion_auto_test.dart`, full suite deviation documented)
|
||||
- [x] 8.2 `flutter analyze` — DEVIATION, not run (hangs in this environment); manual static review done
|
||||
- [x] 8.3 `flutter build`/`flutter run` — DEVIATION, not run (hangs in this environment)
|
||||
- [x] 8.4 Manual DHU verification note — recorded below (non-blocking, not coded)
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Action | What Was Done |
|
||||
|------|--------|----------------|
|
||||
| `lib/servicios/navegacion_auto.dart` | Modified | Added `faviconUsable`, `indiceArtePara`, `_nombresArte`, `artUriPara`, `subtituloCalidad`; wired `artUri`/`displaySubtitle` into `itemEmisora`; dropped `_defaultArtUri` const and old private `_artUriPara` |
|
||||
| `test/servicios/navegacion_auto_test.dart` | Modified | Added `faviconUsable`, `indiceArtePara`, `artUriPara`, `subtituloCalidad`, `itemEmisora` `displaySubtitle` extension, `Emisora.fromMap` unsanitized-favicon regression, and phone/auto art-order parity groups; replaced the obsolete `_defaultArtUri`-based default-art test; extended `_emisora()` test helper with a `codec` parameter |
|
||||
| `android/app/src/main/res/drawable/station_art_aurora.png` | Created | Verbatim byte-for-byte copy of `assets/images/station_art_aurora.png` (size-verified match) |
|
||||
| `android/app/src/main/res/drawable/station_art_cosmic.png` | Created | Verbatim byte-for-byte copy of `assets/images/station_art_cosmic.png` (size-verified match) |
|
||||
| `android/app/src/main/res/drawable/station_art_pulse.png` | Created | Verbatim byte-for-byte copy of `assets/images/station_art_pulse.png` (size-verified match) |
|
||||
| `android/app/src/main/res/drawable/station_art_nova.png` | Created | Verbatim byte-for-byte copy of `assets/images/station_art_nova.png` (size-verified match) |
|
||||
| `android/app/src/main/res/drawable/default_station_art.png` | Deleted | Retired — rotation is total (uuid always present), launcher-icon-lookalike removed per Design Decision 3 |
|
||||
|
||||
## TDD Cycle Evidence
|
||||
|
||||
| Task | RED | GREEN | REFACTOR |
|
||||
|------|-----|-------|----------|
|
||||
| 1.1/1.2 `faviconUsable` | Confirmed fail: `Method not found: 'faviconUsable'` (compile error across all call sites) | Implemented; all `faviconUsable` cases pass. **Bug found during GREEN**: `Uri.tryParse('http://').hasAuthority` returns `true` even with an empty host — initial implementation used `uri.hasAuthority`, which wrongly accepted `'http://'`. Fixed to check `uri.host.isNotEmpty` explicitly; re-ran, all pass. | Doc comments added explaining the `hasAuthority` gotcha inline |
|
||||
| 2.1/2.2 `indiceArtePara` | Confirmed fail: `Method not found: 'indiceArtePara'` | Implemented verbatim-ported formula; parity assertions pass | `_nombresArte` kept private per design; cross-link comment to `tarjeta_emisora.dart` added |
|
||||
| 3.1/3.2/3.3 `artUriPara` | Confirmed fail: `Method not found: 'artUriPara'` (multiple sites) | Implemented; replaced old `_artUriPara`/`_defaultArtUri`; obsolete default-art test replaced | Grep-confirmed zero remaining `_defaultArtUri`/old `_artUriPara` references anywhere in `lib/`/`android/` |
|
||||
| 4.1/4.2 `subtituloCalidad` | Confirmed fail: `Method not found: 'subtituloCalidad'` | Implemented per design's quality-format table incl. bitrate<=0 and whitespace-codec edge cases; all pass | — |
|
||||
| 5.1/5.2 `itemEmisora` wiring | Confirmed fail: assertion targeted not-yet-set `displaySubtitle` | Wired `displaySubtitle: subtituloCalidad(e)` into `MediaItem(...)`; full group green | 5.3: re-read top to bottom, doc comments added, dead-reference grep clean, full suite (this file) green |
|
||||
| 6.1 `Emisora.fromMap` gap regression | Coverage-only, single-pass assertion (design already characterizes current behavior) | Passed on first run — confirms `artUriPara`'s validity gate catches the `fromMap` unsanitized-favicon gap as designed | — |
|
||||
| 6.2 Phone/auto parity guard | Coverage-only, single-pass assertion | Passed — verified via `artUriPara`/`indiceArtePara` over 4 seeds covering all rotation indices (no public accessor added, keeping `_nombresArte` private per design) | — |
|
||||
|
||||
## Deviations from Design
|
||||
|
||||
- **Test verifies canonical art-name order behaviorally, not via a public constant.** Design specifies `_nombresArte` as a **private** const (task 2.2 says "Private `const _nombresArte`"). Task 2.1/6.2 ask to assert the canonical order directly. Rather than making the list public just for test access (which would contradict the design's explicit privacy choice), the order is verified indirectly through `indiceArtePara` + `artUriPara` behavior using single-character seeds (`'d'`→index 0, `'a'`→1, `'b'`→2, `'c'`→3) that deterministically cover all 4 rotation indices. This satisfies the same intent (pin the order, catch reorder drift) without leaking an implementation-private symbol.
|
||||
- No other deviations — implementation matches design (`faviconUsable`, `indiceArtePara`, `artUriPara`, `subtituloCalidad` signatures match the Design "Interfaces / Contracts" section exactly).
|
||||
|
||||
## Issues Found
|
||||
|
||||
- **Real bug caught by Strict TDD RED→GREEN cycle**: `Uri('http://').hasAuthority` returns `true` in Dart even when `host` is empty — a naive `uri.hasAuthority` check would have let `'http://'` (scheme, no host) pass as a "usable" favicon, contradicting the spec's malformed-URL rejection requirement. Fixed by checking `uri.host.isNotEmpty` explicitly. Documented inline with a code comment to prevent regression.
|
||||
- No other issues.
|
||||
|
||||
## Static Review Notes (Phase 7 — no Android build env available)
|
||||
|
||||
- 4 new drawables (`station_art_aurora/cosmic/pulse/nova.png`) copied byte-for-byte from `assets/images/` — file sizes verified identical to source (1,910,538 / 2,053,431 / 1,879,963 / 1,742,275 bytes respectively).
|
||||
- `default_station_art.png` deleted; `grep -rn default_station_art lib android` returns zero matches (excluding SDD artifact prose in `openspec/changes/auto-media-art-quality/`).
|
||||
- `android/app/build.gradle.kts:34` confirmed unchanged: `applicationId = "es.freetimelab.pluriwave"`, matching the `android.resource://es.freetimelab.pluriwave/drawable/station_art_<name>` authority built in `artUriPara`. No manifest/build.gradle change required.
|
||||
- A real Android build or DHU session is still required before shipping to confirm the OS art loader resolves the 4 new `station_art_*` drawables via `android.resource://` (Design's open question — unresolved, carried forward).
|
||||
|
||||
## Manual DHU Verification Note (task 8.4, non-blocking)
|
||||
|
||||
Recommended before merge/ship: a Desktop Head Unit (or real car) session to visually confirm (a) all 4 rotating arts render correctly per station, (b) `displaySubtitle` quality text renders as expected on the browse row, (c) no stale `default_station_art` reference lingers anywhere in the built APK's resources.
|
||||
|
||||
## Remaining Tasks
|
||||
|
||||
None — all 22 tasks complete.
|
||||
|
||||
## Workload / PR Boundary
|
||||
|
||||
- Mode: single PR (no chaining needed)
|
||||
- Current work unit: entire change (`auto-media-art-quality`), single deliverable
|
||||
- Boundary: starts from the existing `_artUriPara`/`_defaultArtUri` baseline, ends with the full validity-gate + rotation + quality-subtitle feature landed and tested
|
||||
- Estimated review budget impact: ~347 changed text lines (2 files) + 4 binary adds + 1 binary delete — comfortably under the 400-line budget (forecast: Low risk, confirmed)
|
||||
|
||||
## Status
|
||||
|
||||
22/22 tasks complete. Ready for `sdd-verify`.
|
||||
|
||||
Working tree left uncommitted (staged-ready) per instructions — no `git commit` run by this agent.
|
||||
@@ -0,0 +1,96 @@
|
||||
# Design: Android Auto Media Art & Quality Polish
|
||||
|
||||
## Technical Approach
|
||||
|
||||
All logic lands in the **pure** `ConstructorArbolAuto` layer of `navegacion_auto.dart` (folders, leaves, art, subtitle) — never in the handler/browse async path, which per parent WARNING #1 still has zero execution coverage. Three pure additions: a zero-network favicon-validity gate feeding `artUriPara`, a uuid-hash rotation over 4 native drawables ported verbatim from the phone's `_fallbackArtFor`, and a `displaySubtitle` formatter from `Emisora.codec`/`bitrate`. Reuse over rebuild; phone UI and audio pipeline untouched. Realises the extended `android-auto-media` browse-item requirements.
|
||||
|
||||
## Architecture Decisions
|
||||
|
||||
### Decision: Case B (present-but-unreachable favicon) detection — LOAD-BEARING
|
||||
|
||||
**Choice**: Static favicon-URL **validity gate** (no network) + document the genuine live-then-404/host-down case as an accepted known limitation this iteration. `artUriPara` uses the favicon only when it parses as an **absolute http/https URI with an authority**; otherwise it falls open to the rotating on-brand art.
|
||||
|
||||
**Alternatives considered**:
|
||||
|
||||
| Option | Coverage | Cost | Testable (Strict TDD, no net, untested handler) | Correctness risk |
|
||||
|--------|----------|------|--------------------------------------------------|------------------|
|
||||
| (a) eager HTTP HEAD + cache | live-404 too, racy | net latency on every cold tree build; offline + cache-invalidation story | needs HTTP+cache+offline seams injected into the exact zero-coverage path | TOCTOU vs OS's own later fetch; transient offline permanently swaps *good* art |
|
||||
| (b) piggyback phone `errorWidget` signal | partial — car-first stations blind | new persisted cross-isolate shared state | new persistence subsystem to test | staleness; couples car to phone history |
|
||||
| (c) accept whole gap | none of Case B | zero | trivial | ships no A improvement |
|
||||
| (d) lazy per-station HEAD + TTL cache | live-404, deferred | net on first browse-in; TTL staleness | same seam burden as (a) | same TOCTOU/false-negative |
|
||||
| **CHOSEN: static validity gate + documented gap** | malformed/non-http subset, deterministic | **zero network** | pure sync function, no seams | none — only swaps art for a URL that could never render as a remote image |
|
||||
|
||||
**Rationale**: The OS art loader fetches `artUri` **independently and later, outside the Flutter engine** — a build-time probe *predicts* but cannot *bind* that fetch (TOCTOU): "reachable now" can 404 at render, "unreachable now" is often transient offline. So (a)/(d) trade guaranteed cost for a racy guess and can degrade a correct case. Under Strict TDD with the handler path already untested (WARNING #1), adding HTTP+cache+offline seams there is high-risk/low-reward. The **malformed/non-http(s)** subset (bare hosts, wrong scheme, `http://` with no authority, whitespace, unparseable) is a *real* slice of Radio Browser broken-art — and `Emisora.fromMap` (SQLite favorites) does **not** sanitize favicon, so these reach the tree today. We catch them deterministically at zero cost and unit-test them fully. The residual live-404 case stays a documented low-severity limitation — the same gap every mainstream Android Auto music app has. Proposal's Split Judgment explicitly permits this narrowing of A.
|
||||
|
||||
### Decision: Fallback-art selection — port `_fallbackArtFor` verbatim
|
||||
|
||||
**Choice**: Replicate the exact expression `seed.codeUnits.fold<int>(0,(a,b)=>a+b) % 4` (seed = `uuid`) and the **same ordered list** `[aurora, cosmic, pulse, nova]`, mapping the index to `android.resource://…/drawable/station_art_<name>`. **Alternatives**: share one helper across widget+service (rejected — widget returns `assets/…` paths, service needs `android.resource://` URIs; different targets). **Rationale**: Identical formula + identical order = provable phone/car parity. A parity unit test enumerates the canonical order over sample uuids; a code comment cross-links both lists to mitigate reorder drift.
|
||||
|
||||
### Decision: Native drawables + retire `default_station_art.png`
|
||||
|
||||
**Choice**: Add `station_art_{aurora,cosmic,pulse,nova}.png` to `res/drawable` (same `android.resource://` technique as parent). **Retire** `default_station_art.png` and the `_defaultArtUri` const. **Rationale**: `uuid` is always present (empty-uuid → index 0 → aurora), so rotation is **total** — no 5th fallback is reachable. Keeping the launcher-icon-lookalike is exactly the bug WARNING #2 flagged; deleting it removes the smell and dead asset. If a last resort were ever needed it would be one of the 4 arts, never a launcher copy.
|
||||
|
||||
### Decision: `displaySubtitle` quality format
|
||||
|
||||
**Choice**: `MediaItem.displaySubtitle` (the audio_service 0.18.18 field for the browsable row's secondary text — distinct from `artist`/`album`, which the play path already uses semantically). Format `"<bitrate> kbps · <CODEC>"` (codec upper-cased/trimmed, `·` U+00B7).
|
||||
|
||||
| codec | bitrate | subtitle |
|
||||
|-------|---------|----------|
|
||||
| MP3 | 128 | `128 kbps · MP3` |
|
||||
| null | 128 | `128 kbps` |
|
||||
| MP3 | null / ≤0 | `MP3` |
|
||||
| null | null / ≤0 | *omitted (leave null)* |
|
||||
|
||||
**Rationale**: `bitrate ≤ 0` (Radio Browser stores 0) and empty/whitespace codec count as unknown; when both unknown, **omit** the field entirely — never set `""` (some renderers show a blank second line) and never render `"null"`. Applied to the browse leaf `itemEmisora`; now-playing item unchanged (out of scope).
|
||||
|
||||
## Data Flow
|
||||
|
||||
Emisora ──▶ ConstructorArbolAuto.itemEmisora
|
||||
├─ artUriPara(e): faviconUsable? favicon : station_art_<idx(uuid)>
|
||||
└─ subtituloCalidad(e): codec/bitrate → "kbps · CODEC" | null
|
||||
│
|
||||
▼
|
||||
MediaItem(artUri, displaySubtitle) ──▶ OS media browser row
|
||||
|
||||
## File Changes
|
||||
|
||||
| File | Action | Description |
|
||||
|------|--------|-------------|
|
||||
| `lib/servicios/navegacion_auto.dart` | Modify | `artUriPara` validity gate + rotation; `subtituloCalidad`; `itemEmisora` sets `displaySubtitle`; drop `_defaultArtUri` |
|
||||
| `android/app/src/main/res/drawable/station_art_aurora.png` … `_nova.png` | Create | 4 native copies of `assets/images/station_art_*.png` |
|
||||
| `android/app/src/main/res/drawable/default_station_art.png` | Delete | Launcher-icon copy retired (WARNING #2); rotation is total |
|
||||
| `test/servicios/navegacion_auto_test.dart` | Modify | Add validity-gate, rotation-parity, subtitle-matrix, leaf-integration tests; update the old default-art test |
|
||||
|
||||
## Interfaces / Contracts
|
||||
|
||||
```dart
|
||||
// All pure, no platform — unit-testable without net or Android build.
|
||||
String artUriPara(Emisora e); // valid http(s) favicon | rotating drawable URI
|
||||
String? subtituloCalidad(Emisora e); // formatted quality | null when both unknown
|
||||
int indiceArtePara(String seed); // fold-mod index, parity-critical (test seam)
|
||||
bool faviconUsable(String? favicon); // absolute http/https + authority
|
||||
```
|
||||
|
||||
Drawable URI: `android.resource://es.freetimelab.pluriwave/drawable/station_art_<name>`.
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
| Layer | What | Approach |
|
||||
|-------|------|----------|
|
||||
| Unit | Validity gate: http/https+authority passes; null/empty/whitespace/no-scheme/`http://`/relative/non-http → fallback | pure `faviconUsable`/`artUriPara` |
|
||||
| Unit | Rotation parity: index over sample uuids maps to correct drawable in canonical order | pure |
|
||||
| Unit | Subtitle matrix (5 rows incl. bitrate ≤0 and both-null→null) | pure `subtituloCalidad` |
|
||||
| Unit | `itemEmisora`: rotating `artUri` on invalid favicon; `displaySubtitle` set/omitted | builder assertions |
|
||||
| Static | 4 drawables present, authority matches applicationId; `default_station_art` removed | file check (no Android build) |
|
||||
| Manual DHU | Real art + subtitle render in car | user-side |
|
||||
|
||||
Stays entirely in the pure layer — does **not** add I/O to the untested handler path (does not widen WARNING #1).
|
||||
|
||||
## Migration / Rollout
|
||||
|
||||
Additive/reversible. Revert: restore `default_station_art.png`, `_defaultArtUri` and the old `_artUriPara`; delete the 4 drawables; drop `displaySubtitle` and the new tests. Phone UI and audio pipeline untouched — zero residual state.
|
||||
|
||||
## Open Questions
|
||||
|
||||
- [ ] Confirm the OS art loader accepts `android.resource://` for the 4 new drawables (carries over from parent's open Q; low risk — identical mechanism already static-verified for `default_station_art`). Verify in a DHU session.
|
||||
- [x] Live-then-404/host-down favicon detection — resolved: deferred as a documented known limitation this iteration (see Decision 1).
|
||||
@@ -0,0 +1,68 @@
|
||||
# Proposal: Android Auto Media Art & Quality Polish
|
||||
|
||||
## Intent
|
||||
|
||||
Fast-follow to `android-auto-media`, closing 3 gaps that parent's verify-report flagged (PASS WITH WARNINGS) plus one requested capability. In the car head unit today: (1) stations whose favicon URL is present but dead render broken/blank art — Android Auto resolves `artUri` OUTSIDE the Flutter engine, so the phone's `CachedNetworkImage.errorWidget` fallback never runs (only empty-favicon Case A is handled); (2) the fallback art is a byte-for-byte copy of the launcher icon, reading as a bug; (3) browse items expose no audio-quality hint even though `Emisora.codec`/`bitrate` already exist. This picks up the "fallback artwork for stations without favicons" work deferred in engram #2290.
|
||||
|
||||
## Scope
|
||||
|
||||
### In Scope
|
||||
- **A** — Degrade Android Auto art gracefully when favicon is present but unreachable (Case B), not only empty (Case A).
|
||||
- **B** — Replace launcher-icon placeholder with the 4 on-brand `station_art_*` assets, reusing `_fallbackArtFor`'s deterministic per-station hash-by-uuid rotation for phone/car parity. Requires native Android drawable copies of the 4 PNGs (same `android.resource://` technique as existing `default_station_art.png`).
|
||||
- **C** — Surface `codec` + `bitrate` (e.g. "128 kbps · MP3") via `MediaItem.displaySubtitle` on browse items; omit gracefully when unknown (never "null kbps").
|
||||
- Strict-TDD Dart tests for rotation selection, subtitle formatting, and Case-B fallback path.
|
||||
|
||||
### Out of Scope
|
||||
- Commissioning brand-new custom artwork (no image-gen tooling here, #2290; reuse existing approved assets — external follow-up may add custom art later).
|
||||
- iOS CarPlay; voice search; phone-UI art changes (already correct).
|
||||
- `flutter build`/`analyze`/`gen-l10n` (hang in this environment).
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
- None.
|
||||
|
||||
### Modified Capabilities
|
||||
- `android-auto-media`: browse-item requirements extended — artwork MUST degrade gracefully for both empty AND unreachable favicons using on-brand rotating placeholders, and playable items SHOULD carry an audio-quality subtitle.
|
||||
|
||||
## Approach
|
||||
|
||||
Reuse, don't rebuild — mirror the phone UI. Port `_fallbackArtFor`'s uuid-hash rotation into `navegacion_auto.dart` so `_artUriPara()` picks one of 4 native drawables. Add `displaySubtitle` in `itemEmisora()` from existing `codec`/`bitrate`. **Deferred to design.md — the real open question:** HOW to detect Case B (dead URL) with no Flutter image widget in the OS browser loop. Design must weigh proactive reachability check + cache (network cost, staleness, invalidation) vs. piggybacking the phone's `CachedNetworkImage` error signal (misses car-first-open stations) vs. documenting the gap as a known limitation this iteration.
|
||||
|
||||
## Affected Areas
|
||||
|
||||
| Area | Impact | Description |
|
||||
|------|--------|-------------|
|
||||
| `lib/servicios/navegacion_auto.dart` | Modified | `_artUriPara()` rotation + Case-B fallback; `itemEmisora()` subtitle |
|
||||
| `android/app/src/main/res/drawable/station_art_*.png` | New | 4 native copies of brand assets |
|
||||
| `android/app/src/main/res/drawable/default_station_art.png` | Removed/Kept | Retire launcher-icon copy (design confirms) |
|
||||
| `test/` (Dart) | New | Rotation, subtitle, fallback tests |
|
||||
|
||||
## Risks
|
||||
|
||||
| Risk | Likelihood | Mitigation |
|
||||
|------|------------|------------|
|
||||
| Case-B detection adds unbounded network cost | Med | Design phase decides; may accept as documented gap |
|
||||
| Native drawable / asset drift over time | Low | Comment linking copies to `assets/images/` source |
|
||||
| Malformed codec/bitrate → ugly subtitle | Low | Format guard; omit when either is unknown |
|
||||
|
||||
## Split Judgment
|
||||
|
||||
Keep as ONE change: A+B+C all touch `navegacion_auto.dart` browse-item construction and share tests. If design finds Case-B reachability checking too costly/complex, split A into its own follow-up and ship B+C (pure, low-risk parity + subtitle) first.
|
||||
|
||||
## Rollback Plan
|
||||
|
||||
Additive. Revert PR: restore `_artUriPara()`, drop `displaySubtitle`, delete the 4 native drawables (and restore `default_station_art.png` if retired), remove new tests. Phone UI and audio pipeline untouched — zero residual state.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- `audio_service ^0.18.18` (present; `MediaItem.displaySubtitle`).
|
||||
- Existing `assets/images/station_art_*.png` (source for native copies).
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- [ ] Unreachable favicon shows on-brand art in car, not broken/blank (or gap documented).
|
||||
- [ ] Fallback uses rotating `station_art_*`, matching phone per-station selection.
|
||||
- [ ] Launcher-icon-as-art no longer appears.
|
||||
- [ ] Browse items show quality subtitle; unknown quality omits cleanly.
|
||||
- [ ] Dart tests pass under `flutter test`.
|
||||
@@ -0,0 +1,77 @@
|
||||
# Delta for Android Auto Media
|
||||
|
||||
## MODIFIED Requirements
|
||||
|
||||
### Requirement: Playable Item Metadata
|
||||
|
||||
Every playable `MediaItem` (station) MUST include a non-empty `title` and a loadable `artUri`. Stations without a logo MUST fall back to on-brand artwork, and stations whose logo URL cannot actually be loaded MUST degrade to the same on-brand fallback instead of rendering broken or blank art. The fallback MUST be visually consistent with the phone UI's per-station rotation rather than a generic launcher-icon copy.
|
||||
(Previously: fallback only covered a null/empty favicon and pointed at a bundled default-artwork asset that read as a copy of the launcher icon.)
|
||||
|
||||
#### Scenario: Station has a valid, reachable remote logo
|
||||
|
||||
- GIVEN a station has a remote logo URL that resolves to a loadable image
|
||||
- WHEN it is mapped to a `MediaItem`
|
||||
- THEN `title` is the station name and `artUri` is the station's logo URL
|
||||
|
||||
#### Scenario: Station has no logo (Case A)
|
||||
|
||||
- GIVEN a station has no logo (`favicon` is null or empty)
|
||||
- WHEN it is mapped to a `MediaItem`
|
||||
- THEN `artUri` is set to one of the on-brand `station_art_*` fallback assets instead of being empty or null
|
||||
|
||||
#### Scenario: Station's logo URL is present but unreachable (Case B)
|
||||
|
||||
- GIVEN a station has a non-empty `favicon` URL that cannot be loaded (dead link, unreachable host, or non-image response)
|
||||
- WHEN it is mapped to (or resolved as) a `MediaItem` in the Android Auto browse tree
|
||||
- THEN the system SHALL show the same on-brand fallback art used for Case A instead of broken or blank art
|
||||
- AND the car head unit MUST NOT display an empty, broken-image, or indefinitely-loading art tile for that station
|
||||
|
||||
#### Scenario: Fallback art matches phone-UI per-station selection
|
||||
|
||||
- GIVEN a station falls back to on-brand art (Case A or Case B)
|
||||
- WHEN the fallback asset is chosen for that station
|
||||
- THEN the selected asset MUST be one of the same 4 rotating assets used by the phone UI (`station_art_aurora`, `station_art_cosmic`, `station_art_pulse`, `station_art_nova`)
|
||||
- AND the same station MUST deterministically resolve to the same asset on both the phone UI and Android Auto (per-station selection parity), not a random or session-varying choice
|
||||
|
||||
#### Scenario: Fallback art is on-brand, not the launcher icon
|
||||
|
||||
- GIVEN a station requires fallback art (Case A or Case B)
|
||||
- WHEN its `artUri` is resolved
|
||||
- THEN it MUST NOT point at a byte-for-byte copy of the app launcher icon
|
||||
- AND it MUST point at one of the 4 on-brand `station_art_*` assets
|
||||
|
||||
### Requirement: Browsable Media Tree
|
||||
|
||||
`getChildren` MUST return a browsable tree rooted at `AudioService.browsableRootId`, organized into non-playable folders (Favoritos, Todas las emisoras, Mis emisoras) containing playable station items. Playable station items SHOULD carry an audio-quality subtitle when known.
|
||||
(Previously: no subtitle requirement; folders and playable items were otherwise unchanged.)
|
||||
|
||||
#### Scenario: Car requests the root
|
||||
|
||||
- GIVEN the car head unit connects and requests the root (`AudioService.browsableRootId`)
|
||||
- WHEN `getChildren` is called with the root id
|
||||
- THEN it returns three folder `MediaItem`s (Favoritos, Todas las emisoras, Mis emisoras), each with `playable: false`
|
||||
|
||||
#### Scenario: Car requests a folder with no stations
|
||||
|
||||
- GIVEN the user has zero favorite stations
|
||||
- WHEN `getChildren` is called with the Favoritos folder id
|
||||
- THEN it returns an empty list, not an error
|
||||
|
||||
#### Scenario: Browse requested before app state is loaded
|
||||
|
||||
- GIVEN the audio handler starts cold and station/favorites Provider state has not finished loading
|
||||
- WHEN `getChildren` is called (root or any folder)
|
||||
- THEN it returns a valid, possibly empty, list without throwing and without blocking or crashing the service
|
||||
|
||||
#### Scenario: Station has known codec and bitrate
|
||||
|
||||
- GIVEN a station's `Emisora.codec` and `Emisora.bitrate` are both known (non-null)
|
||||
- WHEN it is mapped to a playable `MediaItem`
|
||||
- THEN `displaySubtitle` SHALL contain a human-readable quality hint combining bitrate and codec (e.g. "128 kbps · MP3")
|
||||
|
||||
#### Scenario: Station has unknown codec or bitrate
|
||||
|
||||
- GIVEN a station's `Emisora.codec` or `Emisora.bitrate` (or both) is null/unknown
|
||||
- WHEN it is mapped to a playable `MediaItem`
|
||||
- THEN `displaySubtitle` SHALL omit the quality hint gracefully (no subtitle, or a subtitle with no quality fragment)
|
||||
- AND the subtitle MUST NOT render literal placeholder text such as "null kbps" or "null · null"
|
||||
@@ -0,0 +1,201 @@
|
||||
# Tasks: auto-media-art-quality
|
||||
|
||||
Strict TDD active for Dart layers. Behavioral task = RED (failing test) -> GREEN (minimal impl) -> REFACTOR (cleanup, still green). Native/XML/PNG/build.gradle tasks have no `flutter test` coverage (no Android build env) — marked **[static-review-only]**; `flutter build`/`flutter run`/`flutter analyze` MUST NOT be executed in this environment (they hang). "Parallel" tasks have no file overlap with concurrently-listed siblings; "Sequential" tasks depend on a prior task's output.
|
||||
|
||||
Affected files: `lib/servicios/navegacion_auto.dart`, `android/app/src/main/res/drawable/station_art_aurora.png`, `android/app/src/main/res/drawable/station_art_cosmic.png`, `android/app/src/main/res/drawable/station_art_pulse.png`, `android/app/src/main/res/drawable/station_art_nova.png`, `android/app/src/main/res/drawable/default_station_art.png` (deleted), `test/servicios/navegacion_auto_test.dart`.
|
||||
|
||||
Baseline (verified against live code, not trusted from spec/design prose):
|
||||
- `lib/servicios/navegacion_auto.dart:12-17` — `_defaultArtUri` const, to be dropped.
|
||||
- `lib/servicios/navegacion_auto.dart:102-108` — `itemEmisora()`, sets `artUri` only, no `displaySubtitle`.
|
||||
- `lib/servicios/navegacion_auto.dart:110-113` — `_artUriPara(e)`: null/empty favicon check only, no URL-shape validation.
|
||||
- `lib/widgets/tarjeta_emisora.dart:361-370` — `_fallbackArtFor(seed)`: `[aurora, cosmic, pulse, nova]` order, `seed.codeUnits.fold<int>(0,(a,b)=>a+b) % arts.length`, assets at `assets/images/station_art_<name>.png`.
|
||||
- `lib/modelos/emisora.dart:60-78` — `Emisora.fromMap` (SQLite favorites row) assigns `favicon: map['favicon'] as String?` with **no sanitization** (unlike `Emisora.fromApi:47`, which runs `_nonEmpty`). Confirms design.md's claim: malformed/whitespace favicon values from favorites rows reach the tree unfiltered today.
|
||||
- `android/app/src/main/res/drawable/default_station_art.png` exists (only PNG currently in that directory) — to be deleted.
|
||||
- `assets/images/station_art_{aurora,cosmic,pulse,nova}.png` exist — source images to copy into `android/app/src/main/res/drawable/`.
|
||||
- `android/app/build.gradle.kts:34` — `applicationId = "es.freetimelab.pluriwave"`, matches the existing `android.resource://es.freetimelab.pluriwave/drawable/...` authority used by `_defaultArtUri` and the design's new `station_art_<name>` URIs. No manifest/build.gradle change is needed — adding `res/drawable` PNGs requires no manifest declaration (unlike the parent change's `automotive_app_desc.xml` meta-data, which pointed at an XML resource).
|
||||
- `test/servicios/navegacion_auto_test.dart:45-63` — existing test asserts fallback to `_defaultArtUri` on null/empty favicon; must be replaced (not just extended) since `_defaultArtUri` is retired.
|
||||
- `audio_service-0.18.18` (`lib/audio_service.dart:622`): `MediaItem.displaySubtitle` is `String?` — nullable, safe to pass a nullable `subtituloCalidad(e)` result directly.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — Favicon validity gate (`faviconUsable`)
|
||||
|
||||
### 1.1 [x] [RED] `faviconUsable` rejects malformed/non-http(s) favicons, accepts valid http(s) (Sequential — first task, defines the seam)
|
||||
- Satisfies: Design Decision "Case B detection" (static validity gate); Spec "Playable Item Metadata" / Scenario "Station's logo URL is present but unreachable (Case B)" (malformed subset).
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- New `group('faviconUsable', ...)`. Table-driven cases, each asserting `faviconUsable(input) == expected`:
|
||||
- `null` → `false`
|
||||
- `''` → `false`
|
||||
- `' '` (whitespace only) → `false`
|
||||
- `'ftp://cdn.example.com/logo.png'` (non-http(s) scheme) → `false`
|
||||
- `'cdn.example.com/logo.png'` (no scheme, bare host) → `false`
|
||||
- `'http://'` (scheme present, no authority) → `false`
|
||||
- `'/relative/path/logo.png'` (relative, no scheme) → `false`
|
||||
- `'not a url at all $$$ ///'` (unparseable/garbage) → `false`
|
||||
- `'http://cdn.example.com/logo.png'` → `true`
|
||||
- `'https://cdn.example.com/logo.png'` → `true`
|
||||
- Run: fails (`faviconUsable` not implemented / not exported).
|
||||
|
||||
### 1.2 [x] [GREEN] Implement `faviconUsable` (Sequential — depends on 1.1)
|
||||
- File: `lib/servicios/navegacion_auto.dart`.
|
||||
- Top-level (or static) `bool faviconUsable(String? favicon)`: trim; if null/empty after trim → `false`. `Uri.tryParse(trimmed)`; if parse fails → `false`. Require `scheme == 'http' || scheme == 'https'` AND `uri.hasAuthority` (non-empty host) → `true`, else `false`.
|
||||
- Run: 1.1 passes.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2 — Rotation parity (`indiceArtePara`)
|
||||
|
||||
### 2.1 [x] [RED] `indiceArtePara` matches `tarjeta_emisora.dart`'s `_fallbackArtFor` formula and order (Sequential — depends on Phase 1 file existing, independent logic)
|
||||
- Satisfies: Design Decision "Fallback-art selection — port `_fallbackArtFor` verbatim"; Spec "Fallback art matches phone-UI per-station selection".
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- New `group('indiceArtePara', ...)`. Assert `indiceArtePara(seed)` for a handful of sample uuids reproduces `seed.codeUnits.fold<int>(0,(a,b)=>a+b) % 4` computed inline in the test (do not hardcode indices without showing the formula), covering at least: an empty string (`''` → index 0, since `fold` over no elements is 0), a short seed, and a real-looking uuid.
|
||||
- Assert the canonical order constant (or equivalent) used for mapping index→name is exactly `['aurora', 'cosmic', 'pulse', 'nova']`, matching `tarjeta_emisora.dart:362-367`.
|
||||
- Run: fails.
|
||||
|
||||
### 2.2 [x] [GREEN] Implement `indiceArtePara` + canonical art-name list (Sequential — depends on 2.1)
|
||||
- File: `lib/servicios/navegacion_auto.dart`.
|
||||
- `int indiceArtePara(String seed) => seed.codeUnits.fold<int>(0, (a, b) => a + b) % 4;` — verbatim port, same formula as `tarjeta_emisora.dart:368`.
|
||||
- Private `const _nombresArte = ['aurora', 'cosmic', 'pulse', 'nova']` — same order as `tarjeta_emisora.dart:363-366`. Add a code comment cross-linking to `tarjeta_emisora.dart`'s `_fallbackArtFor` to mitigate reorder drift (per design.md's stated mitigation, since there is no structural enforcement of order — see Phase 6 for the added test-level guard).
|
||||
- Run: 2.1 passes.
|
||||
|
||||
---
|
||||
|
||||
## Phase 3 — `artUriPara` integration (favicon gate + rotating drawable, retires `_defaultArtUri`)
|
||||
|
||||
### 3.1 [x] [RED] `artUriPara` returns the favicon when usable, a rotating `station_art_<name>` drawable URI otherwise (Sequential — depends on 1.2, 2.2)
|
||||
- Satisfies: Design Decision 1 (validity gate) + Decision 2 (rotation) combined at the `artUriPara` seam; Spec "Playable Item Metadata" (all 5 scenarios).
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- New `group('artUriPara', ...)`:
|
||||
- Valid `https://` favicon → `artUriPara(e) == e.favicon`.
|
||||
- `null` favicon → `artUriPara(e) == 'android.resource://es.freetimelab.pluriwave/drawable/station_art_<expected-name-for-uuid>'` (compute expected name via `indiceArtePara(e.uuid)` + canonical order in the test, not a hardcoded guess).
|
||||
- `''` favicon → same rotating-drawable behavior as null.
|
||||
- Malformed favicon (e.g. `'ftp://x/y.png'`, `'not a url'`) → same rotating-drawable behavior (Case B malformed subset).
|
||||
- Two different uuids that hash to different indices → assert their resolved `artUriPara` values differ (parity smoke check, not full enumeration — that's Phase 6).
|
||||
- Same uuid called twice → identical result (determinism).
|
||||
- Run: fails.
|
||||
|
||||
### 3.2 [x] [GREEN] Implement `artUriPara`; drop `_defaultArtUri` and the old `_artUriPara` (Sequential — depends on 3.1)
|
||||
- File: `lib/servicios/navegacion_auto.dart`.
|
||||
- Replace `_artUriPara` (lines 110-113) with `String artUriPara(Emisora e) => faviconUsable(e.favicon) ? e.favicon! : 'android.resource://es.freetimelab.pluriwave/drawable/station_art_${_nombresArte[indiceArtePara(e.uuid)]}';`.
|
||||
- Delete `_defaultArtUri` const (lines 12-17) — dead now that rotation is total (design.md Decision 3: uuid always present, empty-uuid → index 0 → aurora, no 5th fallback reachable).
|
||||
- Update `itemEmisora()` (line 106) call site from `_artUriPara(e)` to `artUriPara(e)`.
|
||||
- Run: 3.1 passes.
|
||||
|
||||
### 3.3 [x] [GREEN] Replace the obsolete default-art test in `itemEmisora` group (Sequential — depends on 3.2)
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- Replace the test at lines 45-63 (`'cae al arte por defecto cuando el favicon es null o vacío'`, which asserts the now-deleted `_defaultArtUri` literal) with an assertion that both null- and empty-favicon cases resolve to the correct rotating `station_art_<name>` URI per `indiceArtePara`, consistent with the Phase 3.1 cases.
|
||||
- Run: full `navegacion_auto_test.dart` green.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4 — Quality subtitle (`subtituloCalidad`)
|
||||
|
||||
### 4.1 [x] [RED] `subtituloCalidad` matrix: both known, codec-only, bitrate-only, both unknown, bitrate<=0 (Sequential — independent of Phase 1-3, same file)
|
||||
- Satisfies: Design Decision "`displaySubtitle` quality format"; Spec "Station has known codec and bitrate" / "Station has unknown codec or bitrate".
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- New `group('subtituloCalidad', ...)`, one case per design.md's table plus the `bitrate <= 0` edge (Radio Browser stores 0 for unknown):
|
||||
- `codec: 'mp3', bitrate: 128` → `'128 kbps · MP3'` (codec upper-cased; `·` is U+00B7).
|
||||
- `codec: null, bitrate: 128` → `'128 kbps'`.
|
||||
- `codec: 'mp3', bitrate: null` → `'MP3'`.
|
||||
- `codec: 'mp3', bitrate: 0` → `'MP3'` (bitrate ≤ 0 treated as unknown).
|
||||
- `codec: null, bitrate: null` → `null` (not `''`, not the literal string `'null'`).
|
||||
- `codec: null, bitrate: 0` → `null`.
|
||||
- `codec: ' '` (whitespace-only codec), `bitrate: null` → `null` (whitespace codec counts as unknown per design.md's "empty/whitespace codec count as unknown").
|
||||
- `codec: ' mp3 '` (whitespace-padded codec), `bitrate: 128` → `'128 kbps · MP3'` (trimmed).
|
||||
- Explicitly assert none of the outputs contain the substring `'null'` (defends the "never render literal null" requirement directly, not just via exact-match).
|
||||
- Run: fails.
|
||||
|
||||
### 4.2 [x] [GREEN] Implement `subtituloCalidad` (Sequential — depends on 4.1)
|
||||
- File: `lib/servicios/navegacion_auto.dart`.
|
||||
- `String? subtituloCalidad(Emisora e)`: normalize codec via `trim().toUpperCase()`, treat empty-after-trim as unknown; treat `bitrate == null || bitrate! <= 0` as unknown.
|
||||
- Both known → `'$bitrate kbps · $codecUpper'`. Bitrate only → `'$bitrate kbps'`. Codec only → `codecUpper`. Both unknown → `null` (never `''`, never a string containing `'null'`).
|
||||
- Run: 4.1 passes.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5 — `itemEmisora` wiring (`displaySubtitle`)
|
||||
|
||||
### 5.1 [x] [RED] `itemEmisora` sets `displaySubtitle` from `subtituloCalidad`; omits it (leaves `null`) when both unknown (Sequential — depends on 3.2, 4.2)
|
||||
- Satisfies: Spec "Browsable Media Tree" / Scenario "Station has known codec and bitrate" + "Station has unknown codec or bitrate", applied at the leaf-building integration point.
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- Extend the existing `group('ConstructorArbolAuto.itemEmisora', ...)`:
|
||||
- `Emisora` with `codec: 'mp3', bitrate: 128` → `item.displaySubtitle == '128 kbps · MP3'`.
|
||||
- `Emisora` with `codec: null, bitrate: null` → `item.displaySubtitle == null`.
|
||||
- Run: fails (current `itemEmisora` at lines 102-108 never sets `displaySubtitle`).
|
||||
|
||||
### 5.2 [x] [GREEN] Wire `displaySubtitle` into `itemEmisora` (Sequential — depends on 5.1)
|
||||
- File: `lib/servicios/navegacion_auto.dart`.
|
||||
- Add `displaySubtitle: subtituloCalidad(e)` to the `MediaItem(...)` construction in `itemEmisora` (lines 102-108), alongside the existing `artUri: Uri.parse(artUriPara(e))`.
|
||||
- Run: 5.1 passes; full `itemEmisora` group green.
|
||||
|
||||
### 5.3 [x] [REFACTOR] Cleanup pass on `navegacion_auto.dart` (Sequential — depends on 3.2, 4.2, 5.2 all green)
|
||||
- Re-read top to bottom: doc-comment `faviconUsable`/`artUriPara`/`indiceArtePara`/`subtituloCalidad` per the Design "Interfaces / Contracts" signatures; confirm no leftover reference to `_defaultArtUri` or the old `_artUriPara` anywhere in the file (grep the file for both identifiers — should return zero matches).
|
||||
- Full `navegacion_auto_test.dart` suite stays green.
|
||||
|
||||
---
|
||||
|
||||
## Phase 6 — Downstream coverage & drift guards (favorites sanitization gap, phone/car order parity)
|
||||
|
||||
### 6.1 [x] [RED→GREEN, coverage-only] Test that `faviconUsable`/`artUriPara` catches the unsanitized `Emisora.fromMap` favicon gap (Sequential — depends on 3.2)
|
||||
- Satisfies: Design Decision 1's explicit callout that `Emisora.fromMap` (SQLite favorites, `lib/modelos/emisora.dart:60-78`) does not sanitize `favicon` (no `_nonEmpty`, unlike `Emisora.fromApi:47`) — confirms the validity gate is the correct place to catch this, rather than silently assuming it.
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- Build an `Emisora` via `Emisora.fromMap({...})` (not the `_emisora()` test helper) with a malformed/whitespace favicon value in the map (e.g. `'favicon': ' '`, or `'favicon': 'not-a-url'`) that `fromMap` passes through unsanitized. Assert `artUriPara(builtEmisora)` resolves to a rotating `station_art_<name>` URI, not the malformed string.
|
||||
- This is a regression-style test, not a design-doc gap-fix: it documents the current architecture's division of responsibility (sanitize-at-read-boundary vs sanitize-at-render-boundary) and pins it in place. If it fails, `fromMap` started sanitizing (fine) or the gate regressed (not fine) — either way the test makes the boundary explicit instead of assumed.
|
||||
- Run: passes on first run once Phase 3 lands (this is a characterization test of already-implemented behavior, so RED/GREEN collapses to a single assertion pass — still write it before considering Phase 3 "done" in spirit, but there is no separate implementation step).
|
||||
|
||||
### 6.2 [x] [RED→GREEN, coverage-only] Parity guard: phone-UI asset order and Android-Auto drawable order stay identical (Sequential — depends on 2.2)
|
||||
- Satisfies: Design.md's explicitly flagged risk — "reorder drift" between `tarjeta_emisora.dart`'s `_fallbackArtFor` asset list and `navegacion_auto.dart`'s `_nombresArte` list, called out as having "no structural enforcement," mitigated only by a code comment + this test.
|
||||
- File: `test/servicios/navegacion_auto_test.dart`.
|
||||
- New `group('parity: phone/auto art order', ...)`. Since `_fallbackArtFor`'s asset list is private to `tarjeta_emisora.dart` and returns `assets/...` paths (not directly importable/comparable in form), this test hardcodes the canonical order `['aurora', 'cosmic', 'pulse', 'nova']` **once**, with a comment explaining it must be kept in sync with `tarjeta_emisora.dart:363-367`'s literal list, and asserts `navegacion_auto.dart`'s exposed art-name list/mapping matches it element-for-element, in order.
|
||||
- This does not eliminate drift risk (design.md accepts that — no structural enforcement chosen) but ensures any future reorder in `navegacion_auto.dart` alone breaks a test immediately; a reorder in `tarjeta_emisora.dart` alone still requires a human to update this test's hardcoded list (documented limitation, same as design.md's own acknowledgment).
|
||||
- Run: passes once Phase 2 lands.
|
||||
|
||||
---
|
||||
|
||||
## Phase 7 — Android native drawables (retiring `default_station_art.png`) [static-review-only — no Android build env, `flutter build`/`flutter run`/`flutter analyze` MUST NOT be executed]
|
||||
|
||||
### 7.1 [x] Add 4 native drawable copies (Parallel with 7.2 — different files within the same directory, no overlapping content)
|
||||
- Satisfies: Spec "Fallback art matches phone-UI per-station selection" + "Fallback art is on-brand, not the launcher icon"; Design "Native drawables + retire `default_station_art.png`".
|
||||
- Files: `android/app/src/main/res/drawable/station_art_aurora.png`, `station_art_cosmic.png`, `station_art_pulse.png`, `station_art_nova.png` (new binary assets).
|
||||
- Copy verbatim from `assets/images/station_art_{aurora,cosmic,pulse,nova}.png` — same technique the parent change used for `default_station_art.png` (Phase 6.3 there).
|
||||
- Static review: filenames match exactly (case-sensitive) the names produced by `_nombresArte`/`indiceArtePara` in `navegacion_auto.dart` (`station_art_<name>`, no extension in the `android.resource://` URI), matching the pattern already verified working for `default_station_art`.
|
||||
|
||||
### 7.2 [x] Delete `default_station_art.png` (Parallel with 7.1 — different file)
|
||||
- Satisfies: Design Decision "Native drawables + retire `default_station_art.png`" — rotation is total (uuid always present, empty-uuid → index 0 → aurora), so this drawable is unreachable dead weight and a byte-for-byte launcher-icon lookalike (parent change's WARNING #2).
|
||||
- File: `android/app/src/main/res/drawable/default_station_art.png` (delete).
|
||||
- Static review: confirm no remaining reference to `default_station_art` anywhere in the codebase after Phase 3.2 removes the Dart-side const (grep `default_station_art` across `lib/` and `android/` — should return zero matches after this task, other than this tasks.md/spec/design history).
|
||||
|
||||
### 7.3 [x] Verify `applicationId`/authority match for the new drawable URIs (Parallel with 7.1/7.2 — read-only verification, no file changes expected)
|
||||
- Satisfies: precedent from parent change's WARNING notes (authority/applicationId must match for `android.resource://` URIs to resolve).
|
||||
- Already verified during task planning: `android/app/build.gradle.kts:34` sets `applicationId = "es.freetimelab.pluriwave"`, matching the authority segment (`es.freetimelab.pluriwave`) used in both the retired `_defaultArtUri` and the new `station_art_<name>` URIs built in `artUriPara` (Phase 3.2). **No manifest or build.gradle change is required** — adding PNGs to `res/drawable` needs no manifest declaration, unlike the parent change's `automotive_app_desc.xml` meta-data entry (a `res/xml` resource that did require a `<meta-data>` reference).
|
||||
- Static review only: re-confirm this holds at apply time (`applicationId` unchanged) before closing Phase 7.
|
||||
|
||||
---
|
||||
|
||||
## Phase 8 — Full regression sweep (Sequential — depends on all prior phases green)
|
||||
|
||||
### 8.1 [x] Run full `flutter test` suite (or the closest safe equivalent in this environment)
|
||||
- Satisfies: Design "Testing Strategy" unit rows; Spec-level success criteria across all modified scenarios.
|
||||
- Run the entire suite to catch cross-file regressions, not just `navegacion_auto_test.dart`.
|
||||
- **Expected deviation (execution constraint, same as parent change)**: if a full-suite run is not safely executable in this environment, run targeted files instead — at minimum `navegacion_auto_test.dart` in full — and record which files were actually exercised. A genuine full-suite run is recommended before merge/CI, same caveat the parent change recorded.
|
||||
|
||||
### 8.2 [x] `flutter analyze` — DO NOT RUN in this environment
|
||||
- **DEVIATION (execution constraint, same as parent change)**: `flutter analyze` hangs in this environment and MUST NOT be executed. Perform manual static review instead: confirm `artUriPara`, `faviconUsable`, `indiceArtePara`, `subtituloCalidad` are used (no unused-symbol risk), confirm no leftover references to `_defaultArtUri`/old `_artUriPara`, confirm all new code compiles implicitly by virtue of the modified file's tests passing in 8.1. A real `flutter analyze` pass is still recommended before merge/CI.
|
||||
|
||||
### 8.3 [x] `flutter build`/`flutter run` — DO NOT RUN in this environment
|
||||
- **DEVIATION (execution constraint, same as parent change)**: these commands hang in this environment and MUST NOT be executed. Phase 7's native drawable work is static-review-only for this reason; a real Android build (or DHU session) is required before shipping to confirm the 4 new `station_art_*` drawables resolve correctly via `android.resource://` (Design's remaining Open Question).
|
||||
|
||||
### 8.4 [x] Manual DHU verification note (optional, non-blocking, not coded)
|
||||
- Satisfies: Design "Testing Strategy" manual row; Design "Open Questions" (confirm OS art loader accepts `android.resource://` for the 4 new drawables).
|
||||
- Record in the commit/PR description that a Desktop Head Unit (or real car) session is recommended before shipping to visually confirm: (a) all 4 rotating arts render correctly per station, (b) `displaySubtitle` quality text renders as expected on the browse row, (c) no stale `default_station_art` reference lingers anywhere in the built APK's resources.
|
||||
|
||||
---
|
||||
|
||||
## Review Workload Forecast
|
||||
|
||||
- Files touched: `lib/servicios/navegacion_auto.dart` (~40-60 changed/added lines: drop `_defaultArtUri` (~6 lines removed), add `faviconUsable`, `indiceArtePara`, `_nombresArte`, `artUriPara`, `subtituloCalidad` (~35-45 lines added), one-line `displaySubtitle` wiring in `itemEmisora`), `test/servicios/navegacion_auto_test.dart` (~130-170 added/changed lines: 6 new test groups — `faviconUsable`, `indiceArtePara`, `artUriPara`, `subtituloCalidad`, `itemEmisora` extension, parity guard — plus replacing the obsolete default-art test), 4 new binary PNGs under `android/app/src/main/res/drawable/` (`station_art_aurora/cosmic/pulse/nova.png`, copied from existing `assets/images/`, zero text-line cost), 1 binary deletion (`default_station_art.png`).
|
||||
- Estimated total changed/added lines (text only, excluding binary PNG bytes): roughly 170-230 lines.
|
||||
- Chained PRs recommended: No — this is well inside the 400-line single-PR budget even with generous test-heavy estimates. No manifest/build.gradle changes are needed (verified: `applicationId` already matches, `res/drawable` PNGs need no manifest declaration), which removes the one file class that made the parent change's Phase 6 riskier.
|
||||
- 400-line budget risk: Low. Total estimate (~170-230 text lines + 4 binary adds + 1 binary delete) sits comfortably under budget as a single commit/PR.
|
||||
- Decision needed before apply: No — proceed as a single delivery. The only structural caveat carried over from the parent change is that Phase 7 (native drawables) remains static-review-only (no Android build env here), same constraint class as the parent change's Phase 6, but it does not change file-count or line-count risk enough to warrant chaining.
|
||||
- Ownership/dependency note: Phase 1 (validity gate) and Phase 2 (rotation) are independent of each other and can proceed in any order — both are prerequisites for Phase 3 (`artUriPara` integration), which retires `_defaultArtUri`. Phase 4 (subtitle) is fully independent of Phases 1-3 (different concern, same file) and could be done in parallel by a second contributor without conflict risk beyond normal same-file merge care. Phase 5 depends on both Phase 3 and Phase 4 landing. Phase 6 is coverage-only and depends on Phase 3 (6.1) and Phase 2 (6.2) respectively but not on each other. Phase 7 has zero Dart dependency and may be done anytime in parallel with all Dart phases (mirrors the parent change's Phase 6 independence). Phase 8 is a hard sequential gate depending on everything above.
|
||||
@@ -0,0 +1,179 @@
|
||||
# Verification Report: auto-media-art-quality
|
||||
|
||||
**Mode**: Strict TDD (adversarial, fresh re-execution — apply-progress claims independently re-verified, not trusted)
|
||||
**Date**: 2026-07-19
|
||||
**Verdict**: PASS WITH WARNINGS
|
||||
|
||||
## Completeness
|
||||
|
||||
| Source | Claim | Verified |
|
||||
|---|---|---|
|
||||
| tasks.md | 22/22 tasks `[x]` | Confirmed by reading tasks.md directly — all checkboxes marked |
|
||||
| apply-progress.md | 22/22 tasks complete, single-batch delivery | Consistent with tasks.md and live code state |
|
||||
|
||||
## Test Execution (independent re-run, not trusted from apply report)
|
||||
|
||||
Command: `flutter test test/servicios/navegacion_auto_test.dart --concurrency=1 --timeout=60s`
|
||||
|
||||
Result: **25/25 passed** (`+0` through `+24`, `All tests passed!`). Matches apply-progress's claimed 25/25 exactly — re-run independently in this session, not copy-pasted from the report.
|
||||
|
||||
## Spec Compliance Matrix
|
||||
|
||||
| Spec Scenario | Covering Test(s) | Status |
|
||||
|---|---|---|
|
||||
| Valid, reachable remote logo | `artUriPara: favicon https valido...`, `itemEmisora: usa el favicon remoto...` | PASS |
|
||||
| No logo (Case A: null/empty) | `artUriPara: favicon null/vacio cae al drawable rotativo` | PASS |
|
||||
| Logo present but unreachable (Case B) | `artUriPara: favicon malformado cae al mismo comportamiento rotativo` | PASS for the **malformed/non-http(s) subset only** — see WARNING-1 |
|
||||
| Fallback art matches phone-UI per-station selection | `indiceArtePara` group, `parity: phone/auto art order` group | PASS — formula/order diff-verified against source, see below |
|
||||
| Fallback art is on-brand, not launcher icon | Static: `default_station_art.png` deleted, 4 `station_art_*` drawables byte-identical to `assets/images/` source | PASS |
|
||||
| Root returns 3 folders, non-playable | `ConstructorArbolAuto.raiz` group (pre-existing, unchanged) | PASS |
|
||||
| Empty folder returns empty list | `ConstructorArbolAuto.hijos: lista vacia...` (pre-existing) | PASS |
|
||||
| Cold-start browse safety | `FuenteEmisorasAutoLocal` try/catch paths (pre-existing, unchanged) | PASS (not touched by this change) |
|
||||
| Known codec+bitrate -> subtitle | `subtituloCalidad` matrix + `itemEmisora: setea displaySubtitle...` | PASS |
|
||||
| Unknown codec/bitrate -> subtitle omitted, never literal "null" | `subtituloCalidad` matrix (incl. bitrate<=0, whitespace-codec) + null-substring test + `itemEmisora: omite displaySubtitle...` | PASS |
|
||||
|
||||
## Adversarial Checks (per orchestrator instructions)
|
||||
|
||||
### 1. Favicon validity gate — edge cases independently re-verified
|
||||
|
||||
Read `lib/servicios/navegacion_auto.dart:31-42`. Independently ran a throwaway Dart script (not trusting the apply report) to confirm the claimed Dart `Uri` quirk:
|
||||
|
||||
Uri.tryParse('http://') -> hasAuthority=true, host="", scheme=http
|
||||
|
||||
This confirms the apply-progress claim that a naive `uri.hasAuthority` check would wrongly accept `'http://'`. The shipped code uses `uri.host.isNotEmpty` (not `hasAuthority`), which correctly rejects it — confirmed both by source inspection and by the passing `faviconUsable` test case (`'http://': false`).
|
||||
|
||||
Also independently re-ran `Uri.tryParse` against the other edge cases in the task list (`cdn.example.com/logo.png`, `/relative/path/logo.png`, `not a url at all $$$ ///`, `ftp://cdn.example.com/logo.png`) — all parse with an empty or non-http(s) scheme, so `scheme == 'http' || scheme == 'https'` alone already rejects them (the `host.isNotEmpty` check specifically fixes the `'http://'` case). All 10 table-driven cases in the `faviconUsable` test pass.
|
||||
|
||||
**Gap (see WARNING-1)**: the gate is a **static URL-shape check only** — it cannot detect a syntactically-valid `https://` URL that 404s or points at a non-image response (spec's Case B literally lists "dead link, unreachable host, or non-image response" as triggers). This is a deliberate, documented design choice (design.md Decision 1), not an implementation bug — but it is a real narrowing of the spec's literal text.
|
||||
|
||||
### 2. Rotation algorithm parity — diffed, not assumed
|
||||
|
||||
Read both implementations side by side.
|
||||
|
||||
`lib/widgets/tarjeta_emisora.dart:361-370` (`_fallbackArtFor`):
|
||||
|
||||
const arts = ['assets/images/station_art_aurora.png', '...cosmic...', '...pulse...', '...nova...'];
|
||||
final index = seed.codeUnits.fold<int>(0, (a, b) => a + b) % arts.length;
|
||||
|
||||
`lib/servicios/navegacion_auto.dart:21,47-48` (`indiceArtePara` + `_nombresArte`):
|
||||
|
||||
const _nombresArte = ['aurora', 'cosmic', 'pulse', 'nova'];
|
||||
int indiceArtePara(String seed) => seed.codeUnits.fold<int>(0, (a, b) => a + b) % _nombresArte.length;
|
||||
|
||||
**Confirmed identical**: same fold expression, same modulus source (4 in both, since both lists have length 4), same element order (aurora, cosmic, pulse, nova). This is a provable, not assumed, match — both files were read and compared directly, not inferred from apply-progress prose.
|
||||
|
||||
### 3. Native drawables — byte-identical, dangling refs checked
|
||||
|
||||
`cmp` run on all 4 pairs — all reported IDENTICAL:
|
||||
|
||||
assets/images/station_art_aurora.png <-> android/.../drawable/station_art_aurora.png
|
||||
assets/images/station_art_cosmic.png <-> android/.../drawable/station_art_cosmic.png
|
||||
assets/images/station_art_pulse.png <-> android/.../drawable/station_art_pulse.png
|
||||
assets/images/station_art_nova.png <-> android/.../drawable/station_art_nova.png
|
||||
|
||||
`android/app/src/main/res/drawable/default_station_art.png` confirmed **absent** (directory listing shows only the 4 new PNGs plus the pre-existing `ic_stat_pluriwave.xml`/`launch_background.xml`).
|
||||
|
||||
Grepped the whole repo for `default_station_art` — 8 hits, **all in SDD artifact prose** (`openspec/changes/auto-media-art-quality/{apply-progress,tasks,design,proposal}.md` and the prior `android-auto-media` change's artifacts). Zero hits in `lib/` or live `android/` resource/manifest files.
|
||||
|
||||
### 4. `displaySubtitle` fallback matrix — assertions inspected, not just names
|
||||
|
||||
Independently re-ran the test file and read the actual assertions (not just test names) in `group('subtituloCalidad', ...)`:
|
||||
|
||||
| Input | Assertion | Result |
|
||||
|---|---|---|
|
||||
| codec=mp3, bitrate=128 | `'128 kbps · MP3'` | matches design table |
|
||||
| codec=null, bitrate=128 | `'128 kbps'` | matches |
|
||||
| codec=mp3, bitrate=null | `'MP3'` | matches |
|
||||
| codec=mp3, bitrate=0 | `'MP3'` (bitrate<=0 -> unknown) | matches design's stated Radio Browser convention |
|
||||
| codec=null, bitrate=null | `isNull` | matches — not `''`, not `'null'` |
|
||||
| codec=null, bitrate=0 | `isNull` | matches |
|
||||
| codec=' ' (whitespace), bitrate=null | `isNull` | matches — whitespace-only codec treated as unknown |
|
||||
| codec=' mp3 ', bitrate=128 | `'128 kbps · MP3'` | matches — trimmed correctly |
|
||||
|
||||
Plus a dedicated test asserting `result?.contains('null') ?? false` is `isFalse` across 4 representative cases — directly defends the "never render literal null" requirement rather than relying only on exact-match equality. All assertions call real production code (`subtituloCalidad`); none are tautological, none are ghost loops.
|
||||
|
||||
### 5. AI attribution / debug prints / dead code
|
||||
|
||||
- `git diff` on both changed files scanned for `print(`, `debugPrint`, `console.log`, `TODO`, `FIXME`, `Anthropic`, `Claude`, `Co-Authored`, `Generated by` — **zero matches**.
|
||||
- Grepped `lib/` for `_defaultArtUri` and `_artUriPara` (the two retired identifiers) — **zero matches** anywhere, confirming full removal, not just from the one call site.
|
||||
|
||||
### 6. Working tree state
|
||||
|
||||
`git status --porcelain` output:
|
||||
|
||||
D android/app/src/main/res/drawable/default_station_art.png
|
||||
M lib/servicios/navegacion_auto.dart
|
||||
M test/servicios/navegacion_auto_test.dart
|
||||
?? android/app/src/main/res/drawable/station_art_aurora.png
|
||||
?? android/app/src/main/res/drawable/station_art_cosmic.png
|
||||
?? android/app/src/main/res/drawable/station_art_nova.png
|
||||
?? android/app/src/main/res/drawable/station_art_pulse.png
|
||||
?? openspec/changes/auto-media-art-quality/
|
||||
|
||||
Confirmed: unstaged modifications/deletion plus untracked new files only. **No commit was made** by the apply agent, matching its claim.
|
||||
|
||||
### 7. Test count cross-check
|
||||
|
||||
Re-ran `navegacion_auto_test.dart` independently in this session (see "Test Execution" above): **25/25 passed**, matching apply-progress's claimed 25/25 exactly. Not trusted from the report — executed fresh in this session.
|
||||
|
||||
## TDD Compliance
|
||||
|
||||
| Check | Result | Details |
|
||||
|---|---|---|
|
||||
| TDD Evidence reported | Yes | "TDD Cycle Evidence" table present in apply-progress.md, one row per phase |
|
||||
| All tasks have tests | Yes | Every behavioral task (1.1-6.2) has a corresponding test group in `navegacion_auto_test.dart`; Phase 7 tasks correctly marked static-review-only |
|
||||
| RED confirmed (tests exist) | Yes | All claimed test groups exist in the live test file |
|
||||
| GREEN confirmed (tests pass) | Yes | 25/25 passing on independent re-run |
|
||||
| Triangulation adequate | Yes | Each behavior has 2+ distinct test cases with varying expected values |
|
||||
| Safety Net for modified files | Yes | `navegacion_auto.dart` modified — full test file (25 tests, incl. 8 pre-existing) re-run and green, no regression |
|
||||
|
||||
**TDD Compliance**: 6/6 checks passed
|
||||
|
||||
### Assertion Quality
|
||||
|
||||
| File | Line(s) | Assertion | Issue | Severity |
|
||||
|---|---|---|---|---|
|
||||
| `test/servicios/navegacion_auto_test.dart` | 120-129 | `if (indiceArtePara(a.uuid) != indiceArtePara(b.uuid)) { expect(...) }` | Conditional/guarded assertion — could execute zero assertions if the two literal seeds hashed to the same index | SUGGESTION |
|
||||
|
||||
Independently computed `indiceArtePara('uuid-a') == 1` and `indiceArtePara('uuid-bbbb') == 0` — confirmed the guard condition is true for the literals actually used today, so the assertion does execute. Minor fragility only (a future seed-literal edit could silently disable the check); the same property is fully and unconditionally enumerated by the `parity: phone/auto art order` test (Phase 6.2), so coverage is not actually at risk.
|
||||
|
||||
No tautologies, no ghost loops over possibly-empty collections, no assertion-free tests found.
|
||||
|
||||
**Assertion quality**: 0 CRITICAL, 0 WARNING, 1 SUGGESTION
|
||||
|
||||
## Design Coherence
|
||||
|
||||
| Design Decision | Code Match |
|
||||
|---|---|
|
||||
| Static favicon validity gate (no network), malformed-subset-only | Matches exactly — `faviconUsable` is a pure sync function, no I/O |
|
||||
| Port `_fallbackArtFor` verbatim (formula + order) | Confirmed identical via direct source diff |
|
||||
| Retire `default_station_art.png` + `_defaultArtUri` (rotation is total) | Confirmed — both removed, zero dangling references |
|
||||
| `displaySubtitle` format, omit when both unknown | Matches design's table exactly, incl. bitrate<=0 and whitespace-codec edges |
|
||||
| `_nombresArte` stays private; order verified indirectly via `artUriPara` | Documented deviation in apply-progress, judged reasonable |
|
||||
|
||||
No design deviations that break a spec requirement.
|
||||
|
||||
## Issues Found
|
||||
|
||||
### CRITICAL
|
||||
|
||||
None.
|
||||
|
||||
### WARNING
|
||||
|
||||
**WARNING-1 — Case B coverage is narrower than the spec's literal text (documented/accepted, flagged for archive sign-off)**
|
||||
|
||||
Spec text (`specs/android-auto-media/spec.md`, "Station's logo URL is present but unreachable (Case B)") lists the triggering conditions as "dead link, unreachable host, or non-image response." The shipped `faviconUsable` gate is a **static URL-shape validator only** — a syntactically valid `https://cdn.example.com/dead-link.png` that returns 404, or a URL resolving to a non-image response, still passes the gate and is handed to the OS art loader as-is, which may still render a broken/blank tile. This contradicts the spec's stated "the car head unit MUST NOT display an empty, broken-image, or indefinitely-loading art tile for that station" for those specific sub-cases.
|
||||
|
||||
This is not an implementation bug — design.md Decision 1 explicitly considered and rejected live-reachability checking (HTTP HEAD, TTL cache, etc.) due to TOCTOU risk and the untested handler-path constraint, and states the proposal's "Split Judgment explicitly permits this narrowing of A." The gap is real and testably confirmed (only the malformed/non-http(s) subset is covered; genuine live-404 or non-image-response cases cannot be tested without network I/O, and are not tested). Recommend explicit archive-time sign-off that this narrowing is accepted, since the literal spec scenario text is broader than what ships.
|
||||
|
||||
### SUGGESTION
|
||||
|
||||
**SUGGESTION-1 — Guarded/conditional assertion in `artUriPara` "uuids distintos" test**
|
||||
|
||||
See Assertion Quality table above. Low priority; the same guarantee is already fully covered by the unconditional `parity: phone/auto art order` test.
|
||||
|
||||
## Next Steps
|
||||
|
||||
- `sdd-archive` is appropriate — no CRITICAL issues block archival. Recommend the archive step (or the user) explicitly acknowledge WARNING-1 (spec's Case B literal scope vs. shipped static-only gate) as an accepted, documented limitation before closing the change.
|
||||
- Phase 7/8's static-review-only items (native drawable resolution via `android.resource://`, real Android build/DHU verification) remain unverified by this report for the same reason apply-progress could not verify them: no Android build environment available in this session. A real device/DHU pass is still recommended before shipping, per both apply-progress and design.md's own open question.
|
||||
@@ -4,6 +4,218 @@ import 'package:pluriwave/modelos/emisora.dart';
|
||||
import 'package:pluriwave/servicios/navegacion_auto.dart';
|
||||
|
||||
void main() {
|
||||
group('faviconUsable', () {
|
||||
test('rechaza favicons malformados o no http(s), acepta http(s) válidos', () {
|
||||
final casos = <String?, bool>{
|
||||
null: false,
|
||||
'': false,
|
||||
' ': false,
|
||||
'ftp://cdn.example.com/logo.png': false,
|
||||
'cdn.example.com/logo.png': false,
|
||||
'http://': false,
|
||||
'/relative/path/logo.png': false,
|
||||
'not a url at all \$\$\$ ///': false,
|
||||
'http://cdn.example.com/logo.png': true,
|
||||
'https://cdn.example.com/logo.png': true,
|
||||
};
|
||||
|
||||
casos.forEach((entrada, esperado) {
|
||||
expect(
|
||||
faviconUsable(entrada),
|
||||
esperado,
|
||||
reason: 'faviconUsable(${entrada.toString()}) debería ser $esperado',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('indiceArtePara', () {
|
||||
test('reproduce la misma fórmula que _fallbackArtFor', () {
|
||||
const seeds = ['', 'a', 'uuid-1234-abcd-real-looking'];
|
||||
|
||||
for (final seed in seeds) {
|
||||
final esperado =
|
||||
seed.codeUnits.fold<int>(0, (a, b) => a + b) % 4;
|
||||
expect(indiceArtePara(seed), esperado);
|
||||
}
|
||||
});
|
||||
|
||||
test('el orden canónico de nombres es aurora, cosmic, pulse, nova '
|
||||
'(verificado vía artUriPara con seeds que cubren los 4 índices)', () {
|
||||
// Seeds de una sola letra cuyo codeUnit % 4 cubre los 4 índices, en
|
||||
// el mismo orden que tarjeta_emisora.dart's _fallbackArtFor:
|
||||
// 'd'(100)->índice 0 aurora, 'a'(97)->índice 1 cosmic,
|
||||
// 'b'(98)->índice 2 pulse, 'c'(99)->índice 3 nova.
|
||||
const indicesEsperados = {'d': 0, 'a': 1, 'b': 2, 'c': 3};
|
||||
const nombresPorIndice = ['aurora', 'cosmic', 'pulse', 'nova'];
|
||||
|
||||
indicesEsperados.forEach((seed, indiceEsperado) {
|
||||
expect(indiceArtePara(seed), indiceEsperado);
|
||||
final emisora = _emisora(uuid: seed, nombre: 'Radio $seed');
|
||||
expect(
|
||||
artUriPara(emisora),
|
||||
'android.resource://es.freetimelab.pluriwave/drawable/station_art_'
|
||||
'${nombresPorIndice[indiceEsperado]}',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('artUriPara', () {
|
||||
String nombreEsperadoPara(String uuid) {
|
||||
final indice = indiceArtePara(uuid);
|
||||
const nombres = ['aurora', 'cosmic', 'pulse', 'nova'];
|
||||
return nombres[indice];
|
||||
}
|
||||
|
||||
test('favicon https válido se usa como artUri', () {
|
||||
final emisora = _emisora(
|
||||
uuid: 'uuid-logo',
|
||||
nombre: 'Radio Con Logo',
|
||||
favicon: 'https://cdn.example.com/logo.png',
|
||||
);
|
||||
|
||||
expect(artUriPara(emisora), emisora.favicon);
|
||||
});
|
||||
|
||||
test('favicon null cae al drawable rotativo correspondiente', () {
|
||||
final emisora = _emisora(uuid: 'uuid-null', nombre: 'Radio Sin Logo');
|
||||
final nombre = nombreEsperadoPara(emisora.uuid);
|
||||
|
||||
expect(
|
||||
artUriPara(emisora),
|
||||
'android.resource://es.freetimelab.pluriwave/drawable/station_art_$nombre',
|
||||
);
|
||||
});
|
||||
|
||||
test('favicon vacío cae al mismo comportamiento rotativo que null', () {
|
||||
final emisora = _emisora(
|
||||
uuid: 'uuid-vacio',
|
||||
nombre: 'Radio Logo Vacio',
|
||||
favicon: '',
|
||||
);
|
||||
final nombre = nombreEsperadoPara(emisora.uuid);
|
||||
|
||||
expect(
|
||||
artUriPara(emisora),
|
||||
'android.resource://es.freetimelab.pluriwave/drawable/station_art_$nombre',
|
||||
);
|
||||
});
|
||||
|
||||
test('favicon malformado cae al mismo comportamiento rotativo', () {
|
||||
final malformados = [
|
||||
_emisora(uuid: 'uuid-m1', nombre: 'M1', favicon: 'ftp://x/y.png'),
|
||||
_emisora(uuid: 'uuid-m2', nombre: 'M2', favicon: 'not a url'),
|
||||
];
|
||||
|
||||
for (final emisora in malformados) {
|
||||
final nombre = nombreEsperadoPara(emisora.uuid);
|
||||
expect(
|
||||
artUriPara(emisora),
|
||||
'android.resource://es.freetimelab.pluriwave/drawable/station_art_$nombre',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('uuids distintos que cayeron en índices distintos resuelven a '
|
||||
'artUriPara distintos', () {
|
||||
final a = _emisora(uuid: 'uuid-a', nombre: 'A');
|
||||
final b = _emisora(uuid: 'uuid-bbbb', nombre: 'B');
|
||||
|
||||
// Solo tiene sentido si de hecho hashean a índices distintos.
|
||||
if (indiceArtePara(a.uuid) != indiceArtePara(b.uuid)) {
|
||||
expect(artUriPara(a), isNot(equals(artUriPara(b))));
|
||||
}
|
||||
});
|
||||
|
||||
test('el mismo uuid llamado dos veces da resultado idéntico', () {
|
||||
final emisora = _emisora(uuid: 'uuid-determinista', nombre: 'Det');
|
||||
|
||||
expect(artUriPara(emisora), artUriPara(emisora));
|
||||
});
|
||||
});
|
||||
|
||||
group('subtituloCalidad', () {
|
||||
test('matriz de codec/bitrate conocidos y desconocidos', () {
|
||||
expect(
|
||||
subtituloCalidad(_emisora(uuid: 'u1', nombre: 'N', codec: 'mp3', bitrate: 128)),
|
||||
'128 kbps · MP3',
|
||||
);
|
||||
expect(
|
||||
subtituloCalidad(_emisora(uuid: 'u2', nombre: 'N', codec: null, bitrate: 128)),
|
||||
'128 kbps',
|
||||
);
|
||||
expect(
|
||||
subtituloCalidad(_emisora(uuid: 'u3', nombre: 'N', codec: 'mp3', bitrate: null)),
|
||||
'MP3',
|
||||
);
|
||||
expect(
|
||||
subtituloCalidad(_emisora(uuid: 'u4', nombre: 'N', codec: 'mp3', bitrate: 0)),
|
||||
'MP3',
|
||||
);
|
||||
expect(
|
||||
subtituloCalidad(_emisora(uuid: 'u5', nombre: 'N', codec: null, bitrate: null)),
|
||||
isNull,
|
||||
);
|
||||
expect(
|
||||
subtituloCalidad(_emisora(uuid: 'u6', nombre: 'N', codec: null, bitrate: 0)),
|
||||
isNull,
|
||||
);
|
||||
expect(
|
||||
subtituloCalidad(
|
||||
_emisora(uuid: 'u7', nombre: 'N', codec: ' ', bitrate: null),
|
||||
),
|
||||
isNull,
|
||||
);
|
||||
expect(
|
||||
subtituloCalidad(
|
||||
_emisora(uuid: 'u8', nombre: 'N', codec: ' mp3 ', bitrate: 128),
|
||||
),
|
||||
'128 kbps · MP3',
|
||||
);
|
||||
});
|
||||
|
||||
test('ningún resultado contiene la subcadena "null"', () {
|
||||
final casos = [
|
||||
_emisora(uuid: 'u1', nombre: 'N', codec: 'mp3', bitrate: 128),
|
||||
_emisora(uuid: 'u2', nombre: 'N', codec: null, bitrate: 128),
|
||||
_emisora(uuid: 'u3', nombre: 'N', codec: 'mp3', bitrate: null),
|
||||
_emisora(uuid: 'u4', nombre: 'N', codec: null, bitrate: null),
|
||||
];
|
||||
|
||||
for (final emisora in casos) {
|
||||
final resultado = subtituloCalidad(emisora);
|
||||
expect(resultado?.contains('null') ?? false, isFalse);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
group('parity: phone/auto art order', () {
|
||||
test('el orden de nombres de arte coincide con tarjeta_emisora.dart', () {
|
||||
// Orden canónico hardcodeado una única vez aquí (mismo patrón que
|
||||
// tarjeta_emisora.dart:363-367's _fallbackArtFor); debe mantenerse
|
||||
// sincronizado manualmente si esa lista cambia. _nombresArte es
|
||||
// privado en navegacion_auto.dart, así que este test verifica el
|
||||
// orden indirectamente vía artUriPara con seeds que cubren los 4
|
||||
// índices (mismo patrón que el segundo test de 'indiceArtePara').
|
||||
const ordenCanonico = ['aurora', 'cosmic', 'pulse', 'nova'];
|
||||
const seedsPorIndice = ['d', 'a', 'b', 'c'];
|
||||
|
||||
for (var indice = 0; indice < ordenCanonico.length; indice++) {
|
||||
final emisora = _emisora(
|
||||
uuid: seedsPorIndice[indice],
|
||||
nombre: 'Radio ${seedsPorIndice[indice]}',
|
||||
);
|
||||
expect(indiceArtePara(emisora.uuid), indice);
|
||||
expect(
|
||||
artUriPara(emisora),
|
||||
'android.resource://es.freetimelab.pluriwave/drawable/station_art_'
|
||||
'${ordenCanonico[indice]}',
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
group('ConstructorArbolAuto.raiz', () {
|
||||
test('devuelve exactamente 3 carpetas no reproducibles con los ids '
|
||||
'esperados', () {
|
||||
@@ -42,7 +254,8 @@ void main() {
|
||||
expect(item.playable, isTrue);
|
||||
});
|
||||
|
||||
test('cae al arte por defecto cuando el favicon es null o vacío', () {
|
||||
test('cae al drawable rotativo correspondiente cuando el favicon es '
|
||||
'null o vacío', () {
|
||||
final sinFavicon = _emisora(
|
||||
uuid: 'uuid-null',
|
||||
nombre: 'Radio Sin Logo',
|
||||
@@ -53,13 +266,67 @@ void main() {
|
||||
nombre: 'Radio Logo Vacio',
|
||||
favicon: '',
|
||||
);
|
||||
const esperado =
|
||||
'android.resource://es.freetimelab.pluriwave/drawable/default_station_art';
|
||||
|
||||
final builder = ConstructorArbolAuto();
|
||||
|
||||
expect(builder.itemEmisora(sinFavicon).artUri.toString(), esperado);
|
||||
expect(builder.itemEmisora(faviconVacio).artUri.toString(), esperado);
|
||||
expect(
|
||||
builder.itemEmisora(sinFavicon).artUri.toString(),
|
||||
artUriPara(sinFavicon),
|
||||
);
|
||||
expect(
|
||||
builder.itemEmisora(faviconVacio).artUri.toString(),
|
||||
artUriPara(faviconVacio),
|
||||
);
|
||||
});
|
||||
|
||||
test('setea displaySubtitle desde subtituloCalidad cuando el codec y '
|
||||
'bitrate son conocidos', () {
|
||||
final emisora = _emisora(
|
||||
uuid: 'uuid-calidad',
|
||||
nombre: 'Radio Calidad',
|
||||
codec: 'mp3',
|
||||
bitrate: 128,
|
||||
);
|
||||
|
||||
final item = ConstructorArbolAuto().itemEmisora(emisora);
|
||||
|
||||
expect(item.displaySubtitle, '128 kbps · MP3');
|
||||
});
|
||||
|
||||
test('omite displaySubtitle (queda null) cuando codec y bitrate son '
|
||||
'ambos desconocidos', () {
|
||||
final emisora = _emisora(
|
||||
uuid: 'uuid-sin-calidad',
|
||||
nombre: 'Radio Sin Calidad',
|
||||
);
|
||||
|
||||
final item = ConstructorArbolAuto().itemEmisora(emisora);
|
||||
|
||||
expect(item.displaySubtitle, isNull);
|
||||
});
|
||||
});
|
||||
|
||||
group('artUriPara: favicon sin sanear desde Emisora.fromMap', () {
|
||||
test('un favicon malformado proveniente de favoritos (SQLite) resuelve '
|
||||
'al drawable rotativo, no al string malformado', () {
|
||||
// Emisora.fromMap (favoritos) no sanea favicon (a diferencia de
|
||||
// Emisora.fromApi), así que valores en blanco/malformados llegan sin
|
||||
// filtrar. Este test fija que la validez-gate de artUriPara es el
|
||||
// punto que efectivamente los atrapa.
|
||||
final desdeMapa = Emisora.fromMap({
|
||||
'uuid': 'uuid-favorito',
|
||||
'nombre': 'Radio Favorita',
|
||||
'url': 'https://stream.demo/radio',
|
||||
'favicon': ' ',
|
||||
});
|
||||
|
||||
final resultado = artUriPara(desdeMapa);
|
||||
|
||||
expect(resultado, isNot(equals(' ')));
|
||||
expect(
|
||||
resultado,
|
||||
startsWith('android.resource://es.freetimelab.pluriwave/drawable/station_art_'),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -188,6 +455,7 @@ Emisora _emisora({
|
||||
required String nombre,
|
||||
String url = 'https://stream.demo/radio',
|
||||
String? favicon,
|
||||
String? codec,
|
||||
int? bitrate,
|
||||
}) {
|
||||
return Emisora(
|
||||
@@ -195,6 +463,7 @@ Emisora _emisora({
|
||||
nombre: nombre,
|
||||
url: url,
|
||||
favicon: favicon,
|
||||
codec: codec,
|
||||
bitrate: bitrate,
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user