diff --git a/android/app/src/main/res/drawable/ic_auto_eq_preset.xml b/android/app/src/main/res/drawable/ic_auto_eq_preset.xml deleted file mode 100644 index 0ca5263..0000000 --- a/android/app/src/main/res/drawable/ic_auto_eq_preset.xml +++ /dev/null @@ -1,3 +0,0 @@ - - - diff --git a/lib/servicios/servicio_audio.dart b/lib/servicios/servicio_audio.dart index 57ee859..e9ef72b 100644 --- a/lib/servicios/servicio_audio.dart +++ b/lib/servicios/servicio_audio.dart @@ -111,7 +111,6 @@ Emisora emisoraDesdeMediaItem(MediaItem mediaItem) { /// `MediaControl`/`customAction` namespace, never compared against a /// media id). const accionEqToggle = 'eq_toggle'; -const accionEqPresetSiguiente = 'eq_preset_siguiente'; /// Advances to the NEXT factory preset after [actual] in [presets] order /// (Design "EQ custom actions — cycling presets", item 4): wraps around @@ -154,30 +153,33 @@ String nombrePresetVisible(AppLocalizations l10n, String nombre) { } /// Builds the equalizer's custom-action `MediaControl`s for the now-playing -/// screen (Design "EQ custom actions", item 4) — deliberately just 2: an -/// on/off toggle plus a cycling-preset action, NOT one action per preset, -/// since Android Auto only surfaces a limited number of custom actions. -/// Folding "off" into the preset cycle (one button walking -/// off → preset 1 → preset 2 → ... → preset 6 → off) was considered and -/// REJECTED: with 6 presets that can take up to 6 taps to switch the EQ -/// off while driving, which is worse for a driver than a dedicated one-tap -/// toggle. Do not "simplify" this back down to a single action. +/// screen (decision `auto/ecualizador-diseno`) — exactly 1: an on/off +/// toggle. The previous design paired this with a SECOND action that cycled +/// through the six factory presets; that action is REMOVED. On-device +/// feedback: many head units render custom actions icon-first, so two +/// static, non-parametrized glyphs sitting side by side looked identical/ +/// dead even though the toggle's own icon DID change and the cycle action +/// DID work — a monochrome icon simply cannot legibly encode "which of six +/// presets" the way a browsable list's text rows can. Preset selection now +/// lives in the "Ecualizador" browsable folder instead (see +/// [itemsEcualizadorAuto]), which also frees this scarce custom-action +/// slot. Do NOT re-add a preset-cycling custom action; extend the folder +/// instead. /// Empty when [disponible] is false (gate on EQ availability, mirrors the /// existing `debeReaplicarEcualizador`/`_eqDisponible` gate) — a device /// without the native Equalizer effect gets no EQ actions at all, not /// broken ones. /// -/// On-device feedback follow-up: both actions used to reuse the SAME -/// `ic_stat_pluriwave` drawable and were visually indistinguishable on a -/// car head unit, which foregrounds the icon over the label. Each action -/// now gets its own dedicated drawable (`ic_auto_eq_on`/`ic_auto_eq_off`/ -/// `ic_auto_eq_preset`), and the toggle's icon itself reflects [activo] -/// (not just its label) so on/off is legible at a glance. Pure, no +/// On-device feedback follow-up: this action used to reuse the SAME +/// `ic_stat_pluriwave` drawable as everything else and was visually +/// indistinguishable on a car head unit, which foregrounds the icon over +/// the label. It now gets its own dedicated drawables +/// (`ic_auto_eq_on`/`ic_auto_eq_off`), and the icon itself reflects +/// [activo] (not just its label) so on/off is legible at a glance. Pure, no /// handler dependency. List controlesEcualizadorPersonalizados({ required bool disponible, required bool activo, - required PresetEcualizador presetActual, required AppLocalizations l10n, }) { if (!disponible) return const []; @@ -191,13 +193,6 @@ List controlesEcualizadorPersonalizados({ : l10n.eqCustomActionEnableLabel, name: accionEqToggle, ), - MediaControl.custom( - androidIcon: 'drawable/ic_auto_eq_preset', - label: l10n.eqCustomActionPresetLabel( - nombrePresetVisible(l10n, presetActual.nombre), - ), - name: accionEqPresetSiguiente, - ), ]; } @@ -568,7 +563,6 @@ class PluriWaveAudioHandler extends BaseAudioHandler controlesEcualizadorPersonalizados( disponible: _eqDisponible, activo: _ecualizadorActivo, - presetActual: _presetActual, l10n: _textos, ); @@ -1176,14 +1170,17 @@ class PluriWaveAudioHandler extends BaseAudioHandler await _reproducirEntradaCola(anterior.actual); } - /// Dispatches the equalizer's 2 custom actions (item 4, Design "EQ custom - /// actions"): `accionEqToggle` flips on/off, `accionEqPresetSiguiente` - /// cycles to the next factory preset. Both delegate to the existing - /// [setEcualizadorActivo]/[aplicarPreset] — the SAME entry points the + /// Dispatches the equalizer's only custom action (decision + /// `auto/ecualizador-diseno`): `accionEqToggle` flips on/off, delegating + /// to the existing [setEcualizadorActivo] — the SAME entry point the /// phone settings screen uses via `EstadoEcualizador` — so a car tap and a - /// phone tap have identical effects and both refresh the custom action's - /// label via `_actualizarControlesEq()` (already wired into those two - /// methods). Any other [name] is a no-op — never throws. + /// phone tap have identical effects and both refresh the action's label + /// via `_actualizarControlesEq()` (already wired into that method). The + /// preset-cycling action that used to live here is REMOVED — preset + /// selection now goes through the "Ecualizador" browsable folder (see + /// `seleccionarPresetEqPorMediaId` in `navegacion_auto.dart`, dispatched + /// from [playFromMediaId] below). Any other [name] is a no-op — never + /// throws. @override Future customAction( String name, [ @@ -1192,8 +1189,6 @@ class PluriWaveAudioHandler extends BaseAudioHandler switch (name) { case accionEqToggle: await setEcualizadorActivo(!_ecualizadorActivo); - case accionEqPresetSiguiente: - await aplicarPreset(presetSiguiente(_presetActual)); } } diff --git a/test/servicios/servicio_audio_eq_custom_actions_test.dart b/test/servicios/servicio_audio_eq_custom_actions_test.dart index 7c25a57..05460f0 100644 --- a/test/servicios/servicio_audio_eq_custom_actions_test.dart +++ b/test/servicios/servicio_audio_eq_custom_actions_test.dart @@ -20,14 +20,8 @@ void main() { group('presetSiguiente (item 4 — cycling presets)', () { test('advances to the next preset in order', () { - expect( - presetSiguiente(PresetEcualizador.flat), - PresetEcualizador.rock, - ); - expect( - presetSiguiente(PresetEcualizador.rock), - PresetEcualizador.pop, - ); + expect(presetSiguiente(PresetEcualizador.flat), PresetEcualizador.rock); + expect(presetSiguiente(PresetEcualizador.rock), PresetEcualizador.pop); }); test('wraps around after the last preset', () { @@ -37,21 +31,15 @@ void main() { ); }); - test( - 'an unknown/custom preset (e.g. a user-tweaked "Personalizado" band ' - 'set) starts from the FIRST preset instead of throwing', - () { - final personalizado = PresetEcualizador( - nombre: 'Personalizado', - bandas: [1.0, 2.0, 3.0, 4.0, 5.0], - ); + test('an unknown/custom preset (e.g. a user-tweaked "Personalizado" band ' + 'set) starts from the FIRST preset instead of throwing', () { + final personalizado = PresetEcualizador( + nombre: 'Personalizado', + bandas: [1.0, 2.0, 3.0, 4.0, 5.0], + ); - expect( - presetSiguiente(personalizado), - PresetEcualizador.presets.first, - ); - }, - ); + expect(presetSiguiente(personalizado), PresetEcualizador.presets.first); + }); test('respects an injected presets list instead of the default 6', () { final propios = [PresetEcualizador.jazz, PresetEcualizador.voz]; @@ -85,7 +73,10 @@ void main() { }); test('an unrecognized name falls through verbatim', () { - expect(nombrePresetVisible(l10n, 'Mi Preset Guardado'), 'Mi Preset Guardado'); + expect( + nombrePresetVisible(l10n, 'Mi Preset Guardado'), + 'Mi Preset Guardado', + ); }); }); @@ -94,35 +85,32 @@ void main() { final controles = controlesEcualizadorPersonalizados( disponible: false, activo: true, - presetActual: PresetEcualizador.flat, l10n: l10n, ); expect(controles, isEmpty); }); - test( - 'exactly 2 custom actions when available: on/off toggle + preset ' - 'cycle -- Android Auto shows a limited number of custom actions, so ' - 'this is deliberately NOT one action per preset', - () { - final controles = controlesEcualizadorPersonalizados( - disponible: true, - activo: true, - presetActual: PresetEcualizador.rock, - l10n: l10n, - ); + test('exactly 1 custom action when available: the on/off toggle -- ' + 'decision `auto/ecualizador-diseno` REMOVES the preset-cycling ' + 'action that used to sit alongside it; preset selection now lives ' + 'in the "Ecualizador" browsable folder instead (see ' + '`itemsEcualizadorAuto`)', () { + final controles = controlesEcualizadorPersonalizados( + disponible: true, + activo: true, + l10n: l10n, + ); - expect(controles, hasLength(2)); - expect(controles.every((c) => c.action == MediaAction.custom), isTrue); - }, - ); + expect(controles, hasLength(1)); + expect(controles.single.action, MediaAction.custom); + expect(controles.single.customAction?.name, accionEqToggle); + }); test('toggle label reflects ON -> shows "disable" action', () { final controles = controlesEcualizadorPersonalizados( disponible: true, activo: true, - presetActual: PresetEcualizador.flat, l10n: l10n, ); final toggle = controles.firstWhere( @@ -136,7 +124,6 @@ void main() { final controles = controlesEcualizadorPersonalizados( disponible: true, activo: false, - presetActual: PresetEcualizador.flat, l10n: l10n, ); final toggle = controles.firstWhere( @@ -146,111 +133,68 @@ void main() { expect(toggle.label, l10n.eqCustomActionEnableLabel); }); - test('preset-cycle label shows the CURRENT preset localized name', () { - final controles = controlesEcualizadorPersonalizados( + test('toggle icon reflects EQ state: ON uses ic_auto_eq_on, OFF uses ' + 'ic_auto_eq_off -- a car head unit foregrounds the icon over the ' + 'label, so the icon itself must change, not just the text', () { + final activado = controlesEcualizadorPersonalizados( disponible: true, activo: true, - presetActual: PresetEcualizador.jazz, l10n: l10n, - ); - final ciclo = controles.firstWhere( - (c) => c.customAction?.name == accionEqPresetSiguiente, - ); + ).firstWhere((c) => c.customAction?.name == accionEqToggle); + final desactivado = controlesEcualizadorPersonalizados( + disponible: true, + activo: false, + l10n: l10n, + ).firstWhere((c) => c.customAction?.name == accionEqToggle); - expect( - ciclo.label, - l10n.eqCustomActionPresetLabel(l10n.equalizerPresetJazz), - ); - }); - - test( - 'toggle icon reflects EQ state: ON uses ic_auto_eq_on, OFF uses ' - 'ic_auto_eq_off -- a car head unit foregrounds the icon over the ' - 'label, so the icon itself must change, not just the text', - () { - final activado = controlesEcualizadorPersonalizados( - disponible: true, - activo: true, - presetActual: PresetEcualizador.flat, - l10n: l10n, - ).firstWhere((c) => c.customAction?.name == accionEqToggle); - final desactivado = controlesEcualizadorPersonalizados( - disponible: true, - activo: false, - presetActual: PresetEcualizador.flat, - l10n: l10n, - ).firstWhere((c) => c.customAction?.name == accionEqToggle); - - expect(activado.androidIcon, 'drawable/ic_auto_eq_on'); - expect(desactivado.androidIcon, 'drawable/ic_auto_eq_off'); - }, - ); - - test( - 'the toggle and the preset-cycle action never share an androidIcon, ' - 'in either EQ state -- this was the on-device bug: both buttons used ' - 'the same drawable and were visually indistinguishable', - () { - for (final activo in [true, false]) { - final controles = controlesEcualizadorPersonalizados( - disponible: true, - activo: activo, - presetActual: PresetEcualizador.flat, - l10n: l10n, - ); - final iconos = controles.map((c) => c.androidIcon).toSet(); - - expect( - iconos.length, - controles.length, - reason: - 'every custom action must have a distinct androidIcon ' - '(activo=$activo)', - ); - } - }, - ); - }); - - group('action name constants (item 4 — collision-free with car-tree ids)', () { - test('names are non-empty, distinct, and do not collide with any ' - 'existing browse-tree media-id prefix', () { - expect(accionEqToggle, isNotEmpty); - expect(accionEqPresetSiguiente, isNotEmpty); - expect(accionEqToggle, isNot(equals(accionEqPresetSiguiente))); + expect(activado.androidIcon, 'drawable/ic_auto_eq_on'); + expect(desactivado.androidIcon, 'drawable/ic_auto_eq_off'); }); }); group( - 'equalizer drawable assets on disk (on-device feedback follow-up: the ' - 'two custom actions used to share one drawable and were visually ' - 'indistinguishable)', + 'action name constants (item 4 -- collision-free with car-tree ids)', () { - test( - 'ic_auto_eq_on, ic_auto_eq_off and ic_auto_eq_preset all exist under ' - 'android/app/src/main/res/drawable/ -- a missing drawable is not a ' - 'build error, it silently renders blank/default on the head unit, ' - 'so this is the only safety net that would have caught the original ' - 'duplication', - () { - for (final nombre in [ - 'ic_auto_eq_on', - 'ic_auto_eq_off', - 'ic_auto_eq_preset', - ]) { - final archivo = File( - 'android/app/src/main/res/drawable/$nombre.xml', - ); - expect( - archivo.existsSync(), - isTrue, - reason: - '$nombre.xml must exist under ' - 'android/app/src/main/res/drawable/', - ); - } - }, - ); + test('accionEqToggle is non-empty and does not collide with any ' + 'existing browse-tree media-id prefix -- accionEqPresetSiguiente ' + '(decision `auto/ecualizador-diseno`: removed, superseded by the ' + '"Ecualizador" browsable folder) no longer exists as a symbol at ' + 'all, which this file compiling proves on its own', () { + expect(accionEqToggle, isNotEmpty); + }); }, ); + + group('equalizer drawable assets on disk (on-device feedback follow-up: the ' + 'two custom actions used to share one drawable and were visually ' + 'indistinguishable)', () { + test('ic_auto_eq_on and ic_auto_eq_off exist under ' + 'android/app/src/main/res/drawable/ -- a missing drawable is not a ' + 'build error, it silently renders blank/default on the head unit, ' + 'so this is the only safety net that would have caught the ' + 'original duplication. ic_auto_eq_preset is deliberately NOT ' + 'checked here anymore -- decision `auto/ecualizador-diseno` ' + 'removes the preset-cycling action and its drawable', () { + for (final nombre in ['ic_auto_eq_on', 'ic_auto_eq_off']) { + final archivo = File('android/app/src/main/res/drawable/$nombre.xml'); + expect( + archivo.existsSync(), + isTrue, + reason: + '$nombre.xml must exist under ' + 'android/app/src/main/res/drawable/', + ); + } + expect( + File( + 'android/app/src/main/res/drawable/ic_auto_eq_preset.xml', + ).existsSync(), + isFalse, + reason: + 'ic_auto_eq_preset.xml must be REMOVED -- decision ' + '`auto/ecualizador-diseno` retires the preset-cycling ' + 'custom action it belonged to', + ); + }); + }); }