0705e6a2bc
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
243 lines
7.9 KiB
Markdown
243 lines
7.9 KiB
Markdown
# 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)
|