fix(eq): stop the enable toggle from landing behind a disk write
cambiarActivo persisted BEFORE telling the audio engine, so two quick taps raced on a SharedPreferences write. When the first write resolved last, the engine received the FIRST tap's value after the second one: the checkbox read enabled while the sound stayed flat, and toggling again could invert it the other way. Reported as the equalizer connecting and disconnecting at random and the checkbox disagreeing with what is audible. Reorder to engine first, disk last. The engine call is now issued before any await, so overlapping taps reach it in tap order and the last tap wins. Each subsequent step re-checks _activo, so a call that a newer tap superseded mid-flight neither applies a preset nor persists a value the user has already changed their mind about. Persisting last also puts what the user HEARS ahead of what is merely stored. The regression test drives two opposite taps through a persistence fake whose FIRST write is the slow one — the exact ordering hazard — and asserts the engine ends matching the state the UI shows. It fails on the previous ordering and passes on this one. An earlier attempt serialized every engine mutation through a shared Future lane. It fixed this case and deadlocked four widget tests: the lane field outlived a tester.runAsync block, so a future created in the real async zone was later chained from the fake-async zone that never advances it. Reverted in favour of the ordering fix, which needs no cross-zone state. Only the enable toggle is addressed here. The other reported symptom — equalization seeming to come and go while playing — is not explained by this race and is still open; the handler rebuilds the whole AndroidEqualizer on every player recreation, which is the next place to look.
This commit is contained in:
@@ -507,14 +507,29 @@ class EstadoEcualizador extends ChangeNotifier {
|
|||||||
return deviceId;
|
return deviceId;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Enables or disables the equalizer.
|
||||||
|
///
|
||||||
|
/// Engine FIRST, disk last. The previous order persisted before telling the
|
||||||
|
/// engine, so two quick taps raced on a disk write: when the first write
|
||||||
|
/// resolved last, the engine received the FIRST tap's value after the second
|
||||||
|
/// one and the checkbox read enabled while the sound stayed flat. Issuing the
|
||||||
|
/// engine call before any `await` means overlapping taps reach the engine in
|
||||||
|
/// tap order, so the last tap always wins.
|
||||||
|
///
|
||||||
|
/// Each step then re-checks [_activo]: a newer tap that landed mid-flight
|
||||||
|
/// owns the outcome, and this superseded call must not apply a preset or
|
||||||
|
/// persist a value the user has already changed their mind about.
|
||||||
Future<void> cambiarActivo(bool activo) async {
|
Future<void> cambiarActivo(bool activo) async {
|
||||||
_activo = activo;
|
_activo = activo;
|
||||||
await servicio.guardarActivo(activo);
|
notifyListeners();
|
||||||
|
|
||||||
await audio.setEcualizadorActivo(activo);
|
await audio.setEcualizadorActivo(activo);
|
||||||
|
if (_activo != activo) return;
|
||||||
if (activo) {
|
if (activo) {
|
||||||
await audio.aplicarPreset(_presetActual);
|
await audio.aplicarPreset(_presetActual);
|
||||||
|
if (_activo != activo) return;
|
||||||
}
|
}
|
||||||
notifyListeners();
|
await servicio.guardarActivo(activo);
|
||||||
}
|
}
|
||||||
|
|
||||||
Future<void> cambiarPreset(
|
Future<void> cambiarPreset(
|
||||||
|
|||||||
@@ -1543,6 +1543,31 @@ void main() {
|
|||||||
// builtin_speaker id collision + device removal
|
// builtin_speaker id collision + device removal
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
group('EstadoEcualizador — cambiarActivo bajo toques rápidos', () {
|
||||||
|
test('el motor acaba en el estado que muestra la UI', () async {
|
||||||
|
// cambiarActivo persists BEFORE telling the engine, so two quick taps
|
||||||
|
// race on a disk write: if the first write resolves last, the engine
|
||||||
|
// receives the FIRST tap's value after the second one — the checkbox
|
||||||
|
// says on while the sound is off.
|
||||||
|
final servicio = _FakeEcualizadorGuardadoLento();
|
||||||
|
final audio = FakeServicioAudio();
|
||||||
|
final eq = EstadoEcualizador(audio: audio, servicio: servicio);
|
||||||
|
await eq.cargarPersistido();
|
||||||
|
audio.cambiosEcualizadorActivo.clear();
|
||||||
|
|
||||||
|
final primero = eq.cambiarActivo(false);
|
||||||
|
final segundo = eq.cambiarActivo(true);
|
||||||
|
await Future.wait([primero, segundo]);
|
||||||
|
|
||||||
|
expect(eq.activo, isTrue);
|
||||||
|
expect(
|
||||||
|
audio.cambiosEcualizadorActivo.last,
|
||||||
|
isTrue,
|
||||||
|
reason: 'the engine must end matching the state the UI shows',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
group('EstadoEcualizador — bonded Bluetooth names', () {
|
group('EstadoEcualizador — bonded Bluetooth names', () {
|
||||||
const deviceId = 'bt_a2dp:AA:BB:CC:DD:EE:FF';
|
const deviceId = 'bt_a2dp:AA:BB:CC:DD:EE:FF';
|
||||||
|
|
||||||
@@ -1704,6 +1729,24 @@ void main() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Fake whose [guardarActivo] stays pending until released, and releases the
|
||||||
|
/// pending writes in REVERSE order — reproducing a disk write that resolves
|
||||||
|
/// out of order between two quick taps.
|
||||||
|
class _FakeEcualizadorGuardadoLento extends FakeServicioEcualizador {
|
||||||
|
int _llamadas = 0;
|
||||||
|
|
||||||
|
@override
|
||||||
|
Future<void> guardarActivo(bool activo) async {
|
||||||
|
// The FIRST write is the slow one: that is the ordering hazard, since an
|
||||||
|
// unserialized second tap overtakes it and the slow write's engine call
|
||||||
|
// lands last.
|
||||||
|
await Future<void>.delayed(
|
||||||
|
_llamadas++ == 0 ? const Duration(milliseconds: 20) : Duration.zero,
|
||||||
|
);
|
||||||
|
await super.guardarActivo(activo);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Fake whose [resubscribir] stays pending until [completarResubscribir]
|
/// Fake whose [resubscribir] stays pending until [completarResubscribir]
|
||||||
/// runs — creates the overlap window for the in-flight-guard test above.
|
/// runs — creates the overlap window for the in-flight-guard test above.
|
||||||
class _FakeDispositivoAudioResubscribirLento
|
class _FakeDispositivoAudioResubscribirLento
|
||||||
|
|||||||
Reference in New Issue
Block a user