_onDispositivoCambiado() bootstrapped a device-level preset entry for every never-seen device id, including the built-in speaker. That persistent level-3 entry masked later global-preset edits (level 3 beats level 4 on every resolution), so disconnecting a BT device or cold-starting without one could leave the EQ stuck on an outdated copy instead of the current global preset. The base speaker is now excluded from the first-seen bootstrap: disconnect and cold start always resolve through the live hierarchy. BT/wired/USB devices keep their bootstrap behavior unchanged.
77 lines
6.7 KiB
Markdown
77 lines
6.7 KiB
Markdown
# Design: EQ Device Disconnect / Cold-Start Revert
|
|
|
|
## Technical Approach
|
|
|
|
Keep the single handler `_onDispositivoCambiado(DispositivoAudio)` — the platform stream has no add/remove discriminator; it always emits the now-active device (disconnect emits the fallback, e.g. `builtin_speaker`). The handler already sets `_dispositivoActualId`, re-resolves via `_resolverPresetActivo()`, and applies via `audio.aplicarPreset()`. The change is NOT new logic — it is **removing the first-seen bootstrap collision** so revert-to-base resolves live through the 4-level hierarchy instead of being pinned by a forced device-level copy, then locking the corrected behavior with disconnect / cold-start / cycle tests. No parallel resolution path is added; `_resolverPresetActivo()` stays the sole resolver (spec REQ-2, REQ-4).
|
|
|
|
## Architecture Decisions
|
|
|
|
| # | Decision | Options | Choice + Rationale |
|
|
|---|----------|---------|--------------------|
|
|
| 1 | Handler shape | (a) keep single handler, unconditional resolve+apply · (b) split connect/disconnect dispatch · (c) add Kotlin `type` field | **(a)**. Stream carries only the active device; disconnect == "resolve for whatever is now active". Splitting needs a native payload change (out of scope) and buys nothing — resolution is already device-uniform. |
|
|
| 2 | Bootstrap collision (base device) | (a) exclude base type from bootstrap · (b) bootstrap only BT/USB · (c) keep, tolerate redundant entry | **(a)** gate on `dispositivo.tipo != TipoDispositivo.altavozInterno`. `builtin_speaker` must always fall through to global (L4) so a later global-preset edit is not masked by a stale L3 entry. (b) is (a) plus wired-headset ambiguity; wired is legitimately per-device, so only the base speaker is excluded. (c) leaves the persisted mask bug. |
|
|
| 3 | Revert semantics | (a) deterministic re-resolution · (b) snapshot/restore prior preset | **(a)** — locked by exploration. Hierarchy is a pure fn of `(station, device)`; it self-heals and is the ONLY mechanism that also covers cold start (no snapshot exists at boot). |
|
|
| 4 | Apply guarantee | reuse `aplicarPreset()` choke-point vs state-only mutation | Reuse. `_presetActual = resuelto; await audio.aplicarPreset(resuelto)` already present — verified reaching native EQ (REQ-3). Keep idempotent. |
|
|
| 5 | Cold start | trust seed path vs add parallel apply | Trust + test. `cargarPersistido` → `_sembrarDispositivoActual` → `obtenerDispositivoActual` (base fallback) → `_onDispositivoCambiado` already applies base resolution. No new code; add proof tests. |
|
|
| 6 | Toggle-off invariance | — | Early return at L211 (`if (!_eqMultiDeviceEnabled) return;`) is untouched. Subscription is never created when off (`_configurarSuscripcionDispositivo`). Byte-for-byte 2-level preserved. |
|
|
|
|
## Data Flow
|
|
|
|
```
|
|
Disconnect (BT removed) Cold start (no device)
|
|
platform emits builtin_speaker cargarPersistido()
|
|
│ ├─ _resolverPresetActivo() [deviceId null → global]
|
|
▼ ├─ aplicarPreset(base)
|
|
_onDispositivoCambiado(builtin_speaker) └─ _sembrarDispositivoActual()
|
|
├─ guard _eqMultiDeviceEnabled └─ obtenerDispositivoActual() → builtin_speaker
|
|
├─ _dispositivoActualId = builtin_speaker └─ _onDispositivoCambiado(builtin_speaker)
|
|
├─ BOOTSTRAP: SKIP (tipo == altavozInterno) ← the fix
|
|
├─ resuelto = _resolverPresetActivo() [matrix→station→device→GLOBAL]
|
|
├─ _presetActual = resuelto
|
|
└─ audio.aplicarPreset(resuelto) ──────────────► native EQ
|
|
```
|
|
|
|
## File Changes
|
|
|
|
| File | Action | Description |
|
|
|------|--------|-------------|
|
|
| `lib/estado/estado_ecualizador.dart` | Modify | `_onDispositivoCambiado`: gate the first-seen bootstrap (L216-220) on `dispositivo.tipo != TipoDispositivo.altavozInterno`. Resolve+apply stay unconditional and unchanged. |
|
|
| `test/estado/estado_ecualizador_test.dart` | Modify | Add disconnect-revert group (see Testing Strategy). Extend existing fakes only via helpers already present. |
|
|
|
|
## Interfaces / Contracts
|
|
|
|
No new public API, no persistence field, no migration. Bootstrap guard uses the existing `DispositivoAudio.tipo` discriminator:
|
|
|
|
```dart
|
|
// First-seen device bootstrap: skip base speaker so it always resolves
|
|
// live through the hierarchy (never pin a stale device-level entry).
|
|
final esBase = dispositivo.tipo == TipoDispositivo.altavozInterno;
|
|
if (!esBase && !_presetsDispositivo.containsKey(dispositivo.id)) {
|
|
final presetBase = _resolverPresetActivo();
|
|
_presetsDispositivo[dispositivo.id] = presetBase;
|
|
await servicio.guardarPresetDispositivo(dispositivo.id, presetBase);
|
|
}
|
|
```
|
|
|
|
## Testing Strategy
|
|
|
|
Existing fakes suffice — no new fake types. `FakeServicioDispositivoAudio.emitirDispositivo()` drives connect/disconnect; `obtenerDispositivoActual()` returns `builtin_speaker` when nothing emitted (cold-start path). Reuse the base-speaker `DispositivoAudio(id:'builtin_speaker', tipo: altavozInterno)`.
|
|
|
|
| ID | Layer | What to test |
|
|
|----|-------|--------------|
|
|
| D.1 | Unit | Disconnect: BT active → emit `builtin_speaker` → `presetActual` == resolved base (global when no station/matrix); `presetsAplicados.last` == base. |
|
|
| D.2 | Unit | Disconnect does NOT create `presetsDispositivo['builtin_speaker']` (bootstrap skip proven — guards the L3 mask bug). |
|
|
| D.3 | Unit | Cold start, nothing emitted: `cargarPersistido` applies base (global), `dispositivoActualId == 'builtin_speaker'`, and no stale device preset applied. |
|
|
| D.4 | Unit | Cycle: connect BT (device preset) → disconnect (base) → reconnect BT lands on the same device preset each step; 5.5b invariant holds (reconnect never overwrites user preset). |
|
|
| D.5 | Unit | Station-override interaction: station has own preset + BT connected → disconnect → resolution still honors the station preset for `builtin_speaker` (L2 beats L4), proving revert is deterministic, not flatten-to-global. |
|
|
|
|
Consistency: cases 5.1-5.8, 2.1-2.4 stay green unchanged. 5.5a (BT first-seen bootstrap) and 2.2 (BT startup bootstrap) still pass because both use `bluetoothA2dp`, which is NOT excluded by the guard. Full `flutter test` green before/after (Strict TDD, RED→GREEN per new case).
|
|
|
|
## Migration / Rollout
|
|
|
|
No migration. Any `presetsDispositivo['builtin_speaker']` entries already persisted by the old bootstrap remain harmless (L3 still resolves) and are not written going forward; optional one-line cleanup deferred — out of scope.
|
|
|
|
## Open Questions
|
|
|
|
- None blocking. Wired-headset (`auricularesCable`) is intentionally treated as per-device (bootstrap applies) — confirm in tasks if product wants it excluded too; default keeps current per-device behavior.
|