diff --git a/docs/plans/2026-03-11-playback-and-ui-fixes-design.md b/docs/plans/2026-03-11-playback-and-ui-fixes-design.md new file mode 100644 index 0000000..129c991 --- /dev/null +++ b/docs/plans/2026-03-11-playback-and-ui-fixes-design.md @@ -0,0 +1,242 @@ +# Playback and UI Fixes Design + +## Scope + +Fixes 1–6 from the root cause analysis. Non-MP3 stream support (Fix 7) is deferred. + +| # | Bug | Fix | +|---|-----|-----| +| 1 | Cleartext HTTP blocked | `network_security_config.xml` + manifest attribute | +| 2 | No URL fallback | `startEngine` iterates resolved URL list | +| 3 | Stuck `Playing` state on failure | Full state machine refactor of `RadioPlaybackService` | +| 4 | Miniplayer overlaps nav bar | Navigation bar inset padding in `MiniPlayer` | +| 5 | No per-station quality/SSL preference | `StationPreference` table + long-press quality dialog | +| 6 | Playlist management gaps | Import naming dialog, tab rename, tab pin/unpin, drag-to-reorder | + +## Approach + +Full state machine refactor (Approach B). The service layer will grow to support recording and clipping in the future; a proper state machine prevents the class of bugs where state gets stuck or transitions are missed. + +--- + +## 1. State Machine + +### States + +``` +Idle +Connecting(station, urls: List, currentUrlIndex: Int) +Playing(station, metadata?, streamInfo?, sessionStartedAt, connectionStartedAt) +Paused(station, metadata?, sessionStartedAt, streamInfo?) +Reconnecting(station, metadata?, sessionStartedAt, attempt) +``` + +`Playing` is designed to gain an optional `recording: RecordingState?` field later without changing the state machine. + +### Transitions + +``` +Idle ──[play]──► Connecting(station, urls[], urlIndex=0) + +Connecting ──[engine Started]──► Playing +Connecting ──[URL failed, more URLs]──► Connecting(urlIndex + 1) +Connecting ──[all URLs failed, stayConnected]──► Reconnecting(attempt=1) +Connecting ──[all URLs failed, !stayConnected]──► Idle + stopSelf + +Playing ──[user pause]──► Paused +Playing ──[connection lost, stayConnected]──► Reconnecting(attempt=1) +Playing ──[connection lost, !stayConnected]──► Idle + stopSelf + +Paused ──[user resume]──► Connecting(station, urls[], urlIndex=0) + +Reconnecting ──[delay / network available]──► Connecting(station, urls[], urlIndex=0) +Reconnecting ──[user stop]──► Idle + stopSelf + +Any ──[user stop]──► Idle + stopSelf +``` + +### Key invariants + +- `Playing` is only entered after `AudioEngineEvent.Started` — never before the engine connects. +- `stayConnected` is a configuration flag (read from preferences), not part of the state. +- All transitions go through a single `transition()` function that validates and updates `RadioController.state`. + +--- + +## 2. RadioPlaybackService Refactor + +### `transition()` function + +All state changes route through one function. No more scattered `controller.updateState()` calls. + +### `startEngine(station, urls)` + +Loops over the URL list: + +```kotlin +for ((index, url) in urls.withIndex()) { + transition(Connecting(station, urls, index)) + try { + engine = AudioEngine(url, bufferMs) + engine.start() + awaitEngineResult(engine, station) + return + } catch (e: ConnectionFailedException) { + continue + } +} +throw AllUrlsExhaustedException() +``` + +### `handlePlay()` finally block + +Explicitly handles all states — no silent fall-through: + +```kotlin +finally { + endConnectionSpan() + when { + state is Paused -> updateNotification(paused) + playJob == coroutineContext[Job] -> { + transition(Idle) + endListeningSession() + cleanupResources() + stopForeground(STOP_FOREGROUND_REMOVE) + stopSelf() + } + // else: new play job took over + } +} +``` + +### stopSelf race fix + +`playJob == coroutineContext[Job]` guards all cleanup, not just the `Idle` branch. If a new play was launched, the old job exits without calling `stopSelf`. + +### handlePause / handleStop + +Remain thin. `RadioController.pause()` sets the `Paused` state optimistically from the UI side. The service's `finally` block respects it. + +--- + +## 3. Cleartext HTTP + +New file `app/src/main/res/xml/network_security_config.xml`: + +```xml + + + +``` + +In `AndroidManifest.xml` `` tag: + +```xml +android:networkSecurityConfig="@xml/network_security_config" +``` + +--- + +## 4. Miniplayer Navigation Bar Insets + +In `MiniPlayer.kt`, add bottom padding for the navigation bar: + +```kotlin +val navBarPadding = WindowInsets.navigationBars.asPaddingValues().calculateBottomPadding() +// Apply as additional bottom padding inside the Surface +``` + +Works on both FireOS and stock Android (gesture and 3-button nav). + +--- + +## 5. Per-Station Quality Preference + +### Data layer + +New `StationPreference` entity (separate table, extensible for future per-station settings like recording prefs): + +- `stationId: Long` (FK to Station, unique) +- `qualityOverride: String?` +- Future columns: `bufferOverrideMs`, `autoRecord`, etc. + +New `StationPreferenceDao`: `getByStationId()`, `upsert()`. + +Room migration adds the table. + +### StreamResolver + +Checks `StationPreference.qualityOverride` for the station before falling back to the global `preferences.qualityPreference`. + +### UI + +- Long-press station → context menu gains "Quality" (only for stations with `station_streams` entries) +- `QualityOverrideDialog`: shows available stream qualities, drag-to-reorder (matches Settings pattern), "Use default" to clear +- `StationListViewModel.setQualityOverride(stationId, override)` + +--- + +## 6. Playlist Management + +### Import naming + +Split `importFile()` into parse → `NamePlaylistDialog` (pre-filled with filename) → insert. Simple text field + OK/Cancel. + +### Tab rename + +Long-press non-built-in tab → "Rename" in context menu → `RenamePlaylistDialog` (pre-filled with current name) → `PlaylistDao.rename()`. + +### Tab pin/unpin + +- `Playlist` gains a `pinned: Boolean` column (default: `true` for built-in tabs SomaFM and My Stations/Favorites, `false` for custom playlists) +- Long-press any tab → "Pin" / "Unpin" toggle in context menu +- Tab row renders two groups: pinned tabs (left), unpinned tabs (right) + +### Tab drag-to-reorder + +- Both groups are independently drag-reorderable: + - **Pinned group (left):** reorderable among pinned tabs only + - **Unpinned group (right):** reorderable among unpinned tabs only +- Dragging does not move tabs between groups — use Pin/Unpin to change groups +- On drop, update `Playlist.sortOrder` for affected playlists via `PlaylistDao.updateSortOrder()` + +--- + +## 7. Testing + +### Service state machine (unit tests, MockK) + +- `Idle → Connecting → Playing` (happy path) +- `Connecting → Connecting` (URL fallback: first URL fails, second succeeds) +- `Connecting → Reconnecting → Connecting → Playing` (all URLs fail, reconnect, succeed) +- `Playing → Reconnecting → Playing` (connection lost, reconnect) +- `Playing → Paused → Connecting → Playing` (pause/resume) +- Stop during `Reconnecting` → `Idle` + `stopSelf` + session ended +- Concurrent play: Station A → Station B → old job cancelled, no `stopSelf` race +- Network callback: `onAvailable` → immediate retry (skip backoff) + +### URL fallback (unit tests) + +- Mock engine fails on URL 0, succeeds on URL 1. Verify transitions and correct URL used. + +### DAO tests (Room in-memory DB) + +- `StationPreferenceDaoTest`: upsert, query quality override +- `PlaylistDaoTest`: sort order updates, pin/unpin + +### Compose UI tests + +- `QualityOverrideDialogTest`: renders stream list, selection, save callback +- `RenamePlaylistDialogTest`: pre-filled name, validates non-empty +- `MiniPlayerTest`: bottom padding includes navigation bar inset + +--- + +## Priority Order + +1. **Cleartext HTTP** — trivial, unblocks HTTP streams immediately +2. **State machine refactor + stuck state fix** — most disruptive UX bug +3. **URL fallback** — natural part of the state machine refactor +4. **Miniplayer insets** — quick UI fix +5. **Per-station quality UI** — feature addition +6. **Playlist management** — feature addition (import naming → rename → pin/unpin → drag reorder)