docs: add playback and UI fixes design
State machine refactor of RadioPlaybackService, cleartext HTTP, URL fallback, miniplayer insets, per-station quality prefs, and playlist management (import naming, rename, pin/unpin, reorder). Made-with: Cursor
This commit is contained in:
cottongin
242
docs/plans/2026-03-11-playback-and-ui-fixes-design.md
Normal file
242
docs/plans/2026-03-11-playback-and-ui-fixes-design.md
Normal file
@@ -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<String>, 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
|
||||||
|
<network-security-config>
|
||||||
|
<base-config cleartextTrafficPermitted="true" />
|
||||||
|
</network-security-config>
|
||||||
|
```
|
||||||
|
|
||||||
|
In `AndroidManifest.xml` `<application>` 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)
|
||||||
Reference in New Issue
Block a user