fix: connection issues

This commit is contained in:
2026-07-27 00:16:46 -04:00
parent bc9cad78cb
commit f665ac14ae
12 changed files with 448 additions and 92 deletions
@@ -1,7 +1,9 @@
package ca.ksamad.musicremote.mpd package ca.ksamad.musicremote.mpd
import ca.ksamad.musicremote.mpd.model.MpdAlbum
import ca.ksamad.musicremote.mpd.model.MpdSong import ca.ksamad.musicremote.mpd.model.MpdSong
import ca.ksamad.musicremote.mpd.model.MpdStatus import ca.ksamad.musicremote.mpd.model.MpdStatus
import ca.ksamad.musicremote.mpd.model.PlayerState
import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
@@ -9,6 +11,7 @@ import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel import kotlinx.coroutines.cancel
import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.cancelAndJoin
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.isActive import kotlinx.coroutines.isActive
@@ -28,15 +31,15 @@ import java.io.IOException
* can push change notifications. Each `changed:` event triggers a refresh on * can push change notifications. Each `changed:` event triggers a refresh on
* the command connection, which flows out through [status]/[currentSong]. * the command connection, which flows out through [status]/[currentSong].
* *
* Why two connections: `idle` blocks its connection indefinitely, so it can't * **Resilience.** A quiet command connection would otherwise be reaped by MPD's
* also carry commands. Splitting them lets the UI stay live (idle) while still * `connection_timeout`, so a periodic [keepalive] `ping` keeps it warm and
* issuing actions (command) without racing on one socket. * detects death early. Any transport failure triggers a transparent
* [reconnect][triggerReconnect] (staying on the player, not bouncing the user to
* the connect screen); only repeated failures surface as
* [MpdConnectionState.Error]. Server-side `ACK` errors are *not* treated as
* connection failures.
* *
* All blocking socket I/O is dispatched to [ioDispatcher]. Observe the exposed * A [connectionFactory] is injectable so tests can supply fake transports.
* [StateFlow]s from the UI; call [connect]/[disconnect] to manage the link.
*
* A [connectionFactory] is injectable so tests can supply fake transports; it
* defaults to a real TCP [MpdConnection.connect].
*/ */
class MpdClient( class MpdClient(
private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO, private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
@@ -49,11 +52,15 @@ class MpdClient(
private var commandConn: MpdConnection? = null private var commandConn: MpdConnection? = null
private var idleConn: MpdConnection? = null private var idleConn: MpdConnection? = null
private var idleJob: Job? = null private var idleJob: Job? = null
private var keepaliveJob: Job? = null
private var reconnectJob: Job? = null
/** // Remembered so a background reconnect can reopen without UI involvement.
* True while a [disconnect] is in progress, so the idle loop can tell an private var host: String? = null
* intentional socket close apart from a real connection drop. private var port: Int = MpdConnection.DEFAULT_PORT
*/ private var password: String? = null
/** True while an intentional [disconnect]/[shutdown] is in progress. */
@Volatile @Volatile
private var shuttingDown = false private var shuttingDown = false
@@ -70,46 +77,55 @@ class MpdClient(
/** /**
* Open both connections, authenticate, prime the initial state, and start * Open both connections, authenticate, prime the initial state, and start
* the idle loop. Safe to await; failures land in [connectionState] as * the idle + keepalive loops. On failure the state becomes
* [MpdConnectionState.Error] and the call throws. * [MpdConnectionState.Error] and the call throws.
*/ */
suspend fun connect(host: String, port: Int = MpdConnection.DEFAULT_PORT, password: String? = null) { suspend fun connect(host: String, port: Int = MpdConnection.DEFAULT_PORT, password: String? = null) {
disconnect() // ensure a clean slate if re-connecting disconnect() // ensure a clean slate if re-connecting
shuttingDown = false shuttingDown = false
this.host = host
this.port = port
this.password = password
_connectionState.value = MpdConnectionState.Connecting _connectionState.value = MpdConnectionState.Connecting
try { try {
withContext(ioDispatcher) { openConnections()
// Command connection: a finite read timeout so a wedged server
// surfaces as an error instead of hanging a UI action forever.
val command = connectionFactory(host, port, COMMAND_READ_TIMEOUT_MS)
// Idle connection: no read timeout — it must park indefinitely.
val idle = connectionFactory(host, port, 0)
if (password != null) {
command.execute(MpdCommands.password(password))
idle.execute(MpdCommands.password(password))
}
commandConn = command
idleConn = idle
}
refresh() refresh()
_connectionState.value = MpdConnectionState.Connected _connectionState.value = MpdConnectionState.Connected
startIdleLoop() startIdleLoop()
startKeepalive()
} catch (e: IOException) { } catch (e: IOException) {
failAndClose(e.message ?: "connection failed") failAndClose(e.message ?: "connection failed")
throw e throw e
} }
} }
/** Tear down the idle loop and both connections; returns to [MpdConnectionState.Disconnected]. */ /** (Re)open and authenticate both sockets using the remembered credentials. */
private suspend fun openConnections() {
withContext(ioDispatcher) {
val h = host ?: throw MpdConnectionException("no host")
// Command connection: finite read timeout so a wedged server surfaces
// as an error instead of hanging a UI action forever.
val command = connectionFactory(h, port, COMMAND_READ_TIMEOUT_MS)
// Idle connection: no read timeout — it must park indefinitely.
val idle = connectionFactory(h, port, 0)
password?.let { pw ->
command.execute(MpdCommands.password(pw))
idle.execute(MpdCommands.password(pw))
}
commandConn = command
idleConn = idle
}
}
/** Tear everything down; returns to [MpdConnectionState.Disconnected]. */
suspend fun disconnect() { suspend fun disconnect() {
shuttingDown = true shuttingDown = true
stopBackgroundJobs()
// Closing the idle socket unblocks the loop's parked `idle` read. // Closing the idle socket unblocks the loop's parked `idle` read.
withContext(ioDispatcher) { withContext(ioDispatcher) {
idleConn?.close() idleConn?.close()
commandConn?.close() commandConn?.close()
} }
idleJob?.cancelAndJoin()
idleJob = null
idleConn = null idleConn = null
commandConn = null commandConn = null
if (_connectionState.value !is MpdConnectionState.Error) { if (_connectionState.value !is MpdConnectionState.Error) {
@@ -118,22 +134,13 @@ class MpdClient(
shuttingDown = false shuttingDown = false
} }
/** /** Non-suspending teardown for owner destruction (e.g. `ViewModel.onCleared`). */
* Non-suspending teardown for owner destruction (e.g. `ViewModel.onCleared`).
* Cancels the internal scope and drops the sockets without waiting.
*/
fun shutdown() { fun shutdown() {
shuttingDown = true shuttingDown = true
try { idleJob?.cancel()
idleConn?.close() keepaliveJob?.cancel()
} catch (_: IOException) { reconnectJob?.cancel()
} closeConnectionsQuietly()
try {
commandConn?.close()
} catch (_: IOException) {
}
idleConn = null
commandConn = null
scope.cancel() scope.cancel()
_connectionState.value = MpdConnectionState.Disconnected _connectionState.value = MpdConnectionState.Disconnected
} }
@@ -142,8 +149,7 @@ class MpdClient(
// //
// These don't optimistically update the flows: the change they cause makes // These don't optimistically update the flows: the change they cause makes
// the server emit an idle event, which refreshes state through the idle // the server emit an idle event, which refreshes state through the idle
// loop. That keeps the app in lockstep with the server (and with other // loop, keeping us in lockstep with the server (and other clients).
// clients) rather than guessing.
suspend fun play() = run(MpdCommands.play()) suspend fun play() = run(MpdCommands.play())
suspend fun playPos(pos: Int) = run(MpdCommands.playPos(pos)) suspend fun playPos(pos: Int) = run(MpdCommands.playPos(pos))
@@ -153,9 +159,9 @@ class MpdClient(
suspend fun previous() = run(MpdCommands.previous()) suspend fun previous() = run(MpdCommands.previous())
suspend fun pause(paused: Boolean) = run(MpdCommands.pause(paused)) suspend fun pause(paused: Boolean) = run(MpdCommands.pause(paused))
/** Flip play/pause based on the latest known [status]. No-op if state is unknown. */ /** Flip play/pause based on the latest known [status]. */
suspend fun togglePause() { suspend fun togglePause() {
val playing = _status.value?.state == ca.ksamad.musicremote.mpd.model.PlayerState.PLAY val playing = _status.value?.state == PlayerState.PLAY
run(MpdCommands.pause(playing)) run(MpdCommands.pause(playing))
} }
@@ -171,29 +177,51 @@ class MpdClient(
conn.execute(MpdCommands.playlistInfo()).split().mapNotNull { MpdSong.from(it) } conn.execute(MpdCommands.playlistInfo()).split().mapNotNull { MpdSong.from(it) }
} }
/** Every album in the library. */
suspend fun albums(): List<MpdAlbum> = withCommand { conn ->
MpdAlbum.listFrom(conn.execute(MpdCommands.listAlbums()))
}
/** Replace the queue with an album and start playing it. */
suspend fun playAlbum(album: String, albumArtist: String?) = withCommand { conn ->
conn.execute(MpdCommands.clear())
conn.execute(MpdCommands.findAddAlbum(album, albumArtist))
conn.execute(MpdCommands.play())
}
// --- Internals ---------------------------------------------------------- // --- Internals ----------------------------------------------------------
/** Run a fire-and-forget command whose response we don't need. */
private suspend fun run(commandLine: String) { private suspend fun run(commandLine: String) {
withCommand { it.execute(commandLine) } withCommand { it.execute(commandLine) }
} }
/** Serialize access to the command connection and run [block] on the I/O dispatcher. */ /**
* Serialize access to the command connection. A server `ACK` is rethrown as-is
* (the connection is fine); a transport failure triggers a reconnect.
*/
private suspend fun <T> withCommand(block: (MpdConnection) -> T): T = private suspend fun <T> withCommand(block: (MpdConnection) -> T): T =
withContext(ioDispatcher) { withContext(ioDispatcher) {
commandMutex.withLock { commandMutex.withLock {
val conn = commandConn val conn = commandConn ?: throw MpdConnectionException("not connected")
?: throw MpdConnectionException("not connected") try {
block(conn) block(conn)
} catch (e: MpdAckException) {
throw e // server rejected the command; connection is healthy
} catch (e: IOException) {
triggerReconnect("command failed: ${e.message}")
throw e
}
} }
} }
/** Re-read `status` and `currentsong` into the flows. */ /** Re-read `status` and `currentsong` into the flows (single round-trip). */
private suspend fun refresh() { private suspend fun refresh() {
val (status, song) = withCommand { conn -> val (status, song) = withCommand { conn ->
val status = MpdStatus.from(conn.execute(MpdCommands.status()).toMap()) val (statusResp, songResp) = conn.executeList(
val song = MpdSong.from(conn.execute(MpdCommands.currentSong()).toMap()) MpdCommands.status(),
status to song MpdCommands.currentSong(),
)
MpdStatus.from(statusResp.toMap()) to MpdSong.from(songResp.toMap())
} }
_status.value = status _status.value = status
_currentSong.value = song _currentSong.value = song
@@ -204,26 +232,75 @@ class MpdClient(
idleJob = scope.launch { idleJob = scope.launch {
try { try {
while (isActive) { while (isActive) {
// Blocks here until the server reports a change (or we close
// the socket during disconnect, which throws below).
val changed = withContext(ioDispatcher) { val changed = withContext(ioDispatcher) {
idle.execute(MpdCommands.idle()).getAll("changed") idle.execute(MpdCommands.idle()).getAll("changed")
} }
if (changed.isEmpty()) continue if (changed.any { it in REFRESHING_SUBSYSTEMS }) refresh()
// Any of these subsystems affect what we currently show.
if (changed.any { it in REFRESHING_SUBSYSTEMS }) {
refresh()
}
} }
} catch (e: IOException) { } catch (e: IOException) {
if (!shuttingDown) failAndClose(e.message ?: "idle connection lost") if (!shuttingDown) triggerReconnect("idle connection lost: ${e.message}")
} }
} }
} }
/** Record an error state and drop the connections (best-effort). */ /**
private fun failAndClose(message: String) { * Periodic `ping` so MPD's `connection_timeout` never reaps our (often quiet)
_connectionState.value = MpdConnectionState.Error(message) * command connection, and so a dead socket is noticed promptly.
*/
private fun startKeepalive() {
keepaliveJob = scope.launch {
while (isActive) {
delay(KEEPALIVE_INTERVAL_MS)
try {
withCommand { it.execute(MpdCommands.ping()) }
} catch (_: IOException) {
// withCommand already triggered a reconnect on a transport error.
return@launch
}
}
}
}
/** Start a background reconnect unless one is already running or we're shutting down. */
@Synchronized
private fun triggerReconnect(reason: String) {
if (shuttingDown || reconnectJob?.isActive == true) return
reconnectJob = scope.launch { reconnectLoop() }
}
private suspend fun reconnectLoop() {
stopBackgroundJobs()
closeConnectionsQuietly()
_connectionState.value = MpdConnectionState.Connecting
var attempt = 0
while (scope.isActive && !shuttingDown) {
attempt++
try {
openConnections()
refresh()
_connectionState.value = MpdConnectionState.Connected
startIdleLoop()
startKeepalive()
return
} catch (e: IOException) {
if (attempt >= MAX_RECONNECT_ATTEMPTS) {
failAndClose(e.message ?: "reconnect failed")
return
}
delay(RECONNECT_BACKOFF_MS * attempt)
}
}
}
/** Cancel the idle + keepalive loops (but not a running reconnect). */
private suspend fun stopBackgroundJobs() {
idleJob?.cancelAndJoin()
keepaliveJob?.cancelAndJoin()
idleJob = null
keepaliveJob = null
}
private fun closeConnectionsQuietly() {
try { try {
idleConn?.close() idleConn?.close()
} catch (_: IOException) { } catch (_: IOException) {
@@ -236,8 +313,16 @@ class MpdClient(
commandConn = null commandConn = null
} }
private fun failAndClose(message: String) {
_connectionState.value = MpdConnectionState.Error(message)
closeConnectionsQuietly()
}
companion object { companion object {
private const val COMMAND_READ_TIMEOUT_MS = 10_000 private const val COMMAND_READ_TIMEOUT_MS = 10_000
private const val KEEPALIVE_INTERVAL_MS = 25_000L
private const val RECONNECT_BACKOFF_MS = 1_000L
private const val MAX_RECONNECT_ATTEMPTS = 5
/** Idle subsystems that change something [status]/[currentSong] reflects. */ /** Idle subsystems that change something [status]/[currentSong] reflects. */
private val REFRESHING_SUBSYSTEMS = setOf("player", "mixer", "options", "playlist") private val REFRESHING_SUBSYSTEMS = setOf("player", "mixer", "options", "playlist")
@@ -66,4 +66,17 @@ object MpdCommands {
fun clear() = MpdProtocol.command("clear") fun clear() = MpdProtocol.command("clear")
fun add(uri: String) = MpdProtocol.command("add", uri) fun add(uri: String) = MpdProtocol.command("add", uri)
fun deleteId(songId: Int) = MpdProtocol.command("deleteid", songId.toString()) fun deleteId(songId: Int) = MpdProtocol.command("deleteid", songId.toString())
// --- Database / library -------------------------------------------------
/** Every album, grouped by album artist (`Album`/`AlbumArtist` lines). */
fun listAlbums() = MpdProtocol.command("list", "album", "group", "albumartist")
/** Append every track of an album to the queue (optionally scoped to an artist). */
fun findAddAlbum(album: String, albumArtist: String?) =
if (albumArtist.isNullOrEmpty()) {
MpdProtocol.command("findadd", "album", album)
} else {
MpdProtocol.command("findadd", "album", album, "albumartist", albumArtist)
}
} }
@@ -65,6 +65,28 @@ class MpdConnection(
fun execute(name: String, vararg args: String): MpdResponse = fun execute(name: String, vararg args: String): MpdResponse =
execute(MpdProtocol.command(name, *args)) execute(MpdProtocol.command(name, *args))
/**
* Run several commands as a single batch (`command_list_ok_begin`), returning
* one [MpdResponse] per command in order. One network round-trip instead of
* N — worth it on a slow server. Each command's response is delimited by
* `list_OK`, and the batch ends with a final `OK`.
*/
fun executeList(vararg commandLines: String): List<MpdResponse> {
val payload = buildString {
append("command_list_ok_begin")
for (c in commandLines) {
append('\n')
append(c)
}
append('\n')
append("command_list_end")
}
write(payload)
val responses = List(commandLines.size) { readResponse() } // each stops at list_OK
readResponse() // consume the final OK terminating the batch
return responses
}
private fun write(commandLine: String) { private fun write(commandLine: String) {
output.write(commandLine.toByteArray(Charsets.UTF_8)) output.write(commandLine.toByteArray(Charsets.UTF_8))
output.write('\n'.code) output.write('\n'.code)
@@ -78,7 +100,9 @@ class MpdConnection(
val line = readLine() val line = readLine()
?: throw MpdConnectionException("connection closed mid-response") ?: throw MpdConnectionException("connection closed mid-response")
when { when {
line == MpdProtocol.OK -> return MpdResponse(values, binary) // OK ends a command; list_OK ends one command within a command list.
line == MpdProtocol.OK || line == MpdProtocol.LIST_OK ->
return MpdResponse(values, binary)
line.startsWith("ACK ") -> line.startsWith("ACK ") ->
throw MpdAckException.parse(line) throw MpdAckException.parse(line)
?: MpdConnectionException("malformed ACK: $line") ?: MpdConnectionException("malformed ACK: $line")
@@ -0,0 +1,28 @@
package ca.ksamad.musicremote.mpd.model
import ca.ksamad.musicremote.mpd.MpdResponse
/** An album in the library, optionally attributed to an album artist. */
data class MpdAlbum(
val name: String,
val albumArtist: String?,
) {
companion object {
/**
* Parse the response of `list album group albumartist`. MPD emits an
* `AlbumArtist:` line followed by the `Album:` lines belonging to it, so
* we track the current artist and attach it to each album.
*/
fun listFrom(response: MpdResponse): List<MpdAlbum> {
val albums = ArrayList<MpdAlbum>()
var currentArtist: String? = null
for ((key, value) in response.values) {
when (key) {
"AlbumArtist" -> currentArtist = value.ifEmpty { null }
"Album" -> if (value.isNotEmpty()) albums.add(MpdAlbum(value, currentArtist))
}
}
return albums
}
}
}
@@ -5,9 +5,12 @@ import ca.ksamad.musicremote.data.ConnectionSettings
import ca.ksamad.musicremote.data.SettingsRepository import ca.ksamad.musicremote.data.SettingsRepository
import ca.ksamad.musicremote.mpd.MpdClient import ca.ksamad.musicremote.mpd.MpdClient
import ca.ksamad.musicremote.mpd.MpdConnectionState import ca.ksamad.musicremote.mpd.MpdConnectionState
import ca.ksamad.musicremote.mpd.model.MpdAlbum
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
@@ -54,7 +57,19 @@ class MpdConnectionManager(context: Context) {
@Volatile @Volatile
private var pendingVolume: Int? = null private var pendingVolume: Int? = null
// Volume writes are coalesced through a conflated channel + throttle so that
// holding a volume key (or dragging the OS remote-volume slider) sends at
// most one `setvol` per interval with the latest value — a tiny MPD server
// (e.g. a Pi Zero) is easily overwhelmed by a flood of them otherwise.
private val volumeRequests = Channel<Int>(Channel.CONFLATED)
init { init {
scope.launch {
for (target in volumeRequests) {
runCatching { client.setVolume(target) }
delay(VOLUME_THROTTLE_MS)
}
}
// Auto-connect on startup if a server was saved. // Auto-connect on startup if a server was saved.
scope.launch { scope.launch {
val saved = settingsRepo.settingsOrNull.first() val saved = settingsRepo.settingsOrNull.first()
@@ -109,10 +124,18 @@ class MpdConnectionManager(context: Context) {
fun previous() = fire { previous() } fun previous() = fire { previous() }
fun togglePlayPause() = fire { togglePause() } fun togglePlayPause() = fire { togglePause() }
fun seekTo(seconds: Double) = fire { seekCurrent(seconds) } fun seekTo(seconds: Double) = fire { seekCurrent(seconds) }
fun setVolume(volume: Int) = fire { setVolume(volume) } fun setVolume(volume: Int) {
// Coalesced + throttled by the volumeRequests consumer (see init).
volumeRequests.trySend(volume.coerceIn(0, 100))
}
fun setRepeat(on: Boolean) = fire { setRepeat(on) } fun setRepeat(on: Boolean) = fire { setRepeat(on) }
fun setRandom(on: Boolean) = fire { setRandom(on) } fun setRandom(on: Boolean) = fire { setRandom(on) }
fun playAlbum(album: String, albumArtist: String?) = fire { playAlbum(album, albumArtist) }
/** One-shot library fetch; returns empty on any failure. */
suspend fun loadAlbums(): List<MpdAlbum> = runCatching { client.albums() }.getOrDefault(emptyList())
/** /**
* True when we own the volume: connected to a server that exposes a mixer. * True when we own the volume: connected to a server that exposes a mixer.
* Used to decide whether hardware volume keys drive the *server* (silently, * Used to decide whether hardware volume keys drive the *server* (silently,
@@ -135,5 +158,6 @@ class MpdConnectionManager(context: Context) {
private companion object { private companion object {
const val VOLUME_STEP = 5 const val VOLUME_STEP = 5
const val VOLUME_THROTTLE_MS = 120L
} }
} }
@@ -0,0 +1,91 @@
package ca.ksamad.musicremote.ui
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.filled.ArrowBack
import androidx.compose.material.icons.filled.Album
import androidx.compose.material3.CircularProgressIndicator
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton
import androidx.compose.material3.ListItem
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Scaffold
import androidx.compose.material3.Text
import androidx.compose.material3.TopAppBar
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.produceState
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.dp
import ca.ksamad.musicremote.mpd.model.MpdAlbum
/**
* Browse every album on the server. Tapping one replaces the queue with that
* album and starts playing it, then returns to the now-playing screen.
*
* (Album artwork is item #5 — for now each row shows a placeholder icon.)
*/
@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun AlbumsScreen(vm: PlayerViewModel, onBack: () -> Unit, onPlay: () -> Unit) {
// null = still loading.
val albums by produceState<List<MpdAlbum>?>(initialValue = null) {
value = vm.loadAlbums().sortedWith(
compareBy({ it.albumArtist?.lowercase() ?: "" }, { it.name.lowercase() }),
)
}
Scaffold(
topBar = {
TopAppBar(
title = { Text(albums?.let { "Albums (${it.size})" } ?: "Albums") },
navigationIcon = {
IconButton(onClick = onBack) {
Icon(Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Back")
}
},
)
},
) { innerPadding ->
val current = albums
when {
current == null -> Box(
Modifier.fillMaxSize().padding(innerPadding),
contentAlignment = Alignment.Center,
) { CircularProgressIndicator() }
current.isEmpty() -> Box(
Modifier.fillMaxSize().padding(innerPadding),
contentAlignment = Alignment.Center,
) { Text("No albums found", color = MaterialTheme.colorScheme.onSurfaceVariant) }
else -> LazyColumn(modifier = Modifier.padding(innerPadding)) {
items(current) { album ->
ListItem(
modifier = Modifier.clickable {
vm.playAlbum(album.name, album.albumArtist)
onPlay()
},
leadingContent = {
Icon(Icons.Filled.Album, contentDescription = null)
},
headlineContent = {
Text(album.name, maxLines = 1, overflow = TextOverflow.Ellipsis)
},
supportingContent = album.albumArtist?.let {
{ Text(it, maxLines = 1, overflow = TextOverflow.Ellipsis) }
},
)
}
}
}
}
}
@@ -31,26 +31,36 @@ import ca.ksamad.musicremote.data.ConnectionSettings
* brief loading splash while it reads settings / auto-connects, the connect form * brief loading splash while it reads settings / auto-connects, the connect form
* (first run, after disconnect, or on error), or the now-playing screen. * (first run, after disconnect, or on error), or the now-playing screen.
*/ */
/** Sub-screens layered over the player (no nav library needed for this few). */
private enum class PlayerOverlay { None, Settings, Albums }
@Composable @Composable
fun MusicRemoteApp(vm: PlayerViewModel = viewModel()) { fun MusicRemoteApp(vm: PlayerViewModel = viewModel()) {
val screen by vm.screen.collectAsStateWithLifecycle() val screen by vm.screen.collectAsStateWithLifecycle()
var showSettings by rememberSaveable { mutableStateOf(false) } var overlay by rememberSaveable { mutableStateOf(PlayerOverlay.None) }
// Settings is only meaningful over the player; leaving it (e.g. after a // Overlays only make sense over the player; leaving it (e.g. after a
// reset/disconnect) drops us back to the normal screen flow. // reset/disconnect) drops us back to the normal screen flow.
LaunchedEffect(screen) { LaunchedEffect(screen) {
if (screen !is AppScreen.Player) showSettings = false if (screen !is AppScreen.Player) overlay = PlayerOverlay.None
} }
when (val s = screen) { when (val s = screen) {
is AppScreen.Loading -> LoadingScreen() is AppScreen.Loading -> LoadingScreen()
is AppScreen.Connect -> ConnectScreen(vm, s.settings, s.error) is AppScreen.Connect -> ConnectScreen(vm, s.settings, s.error)
is AppScreen.Player -> is AppScreen.Player -> when (overlay) {
if (showSettings) { PlayerOverlay.Settings -> SettingsScreen(vm, onBack = { overlay = PlayerOverlay.None })
SettingsScreen(vm, onBack = { showSettings = false }) PlayerOverlay.Albums -> AlbumsScreen(
} else { vm,
NowPlayingScreen(vm, onOpenSettings = { showSettings = true }) onBack = { overlay = PlayerOverlay.None },
} onPlay = { overlay = PlayerOverlay.None },
)
PlayerOverlay.None -> NowPlayingScreen(
vm,
onOpenSettings = { overlay = PlayerOverlay.Settings },
onOpenLibrary = { overlay = PlayerOverlay.Albums },
)
}
} }
} }
@@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.size
import androidx.compose.material.icons.Icons import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.Cast import androidx.compose.material.icons.filled.Cast
import androidx.compose.material.icons.filled.LibraryMusic
import androidx.compose.material.icons.filled.Pause import androidx.compose.material.icons.filled.Pause
import androidx.compose.material.icons.filled.PlayArrow import androidx.compose.material.icons.filled.PlayArrow
import androidx.compose.material.icons.filled.Repeat import androidx.compose.material.icons.filled.Repeat
@@ -51,7 +52,11 @@ import kotlinx.coroutines.delay
* including changes made from other clients. * including changes made from other clients.
*/ */
@Composable @Composable
fun NowPlayingScreen(vm: PlayerViewModel, onOpenSettings: () -> Unit) { fun NowPlayingScreen(
vm: PlayerViewModel,
onOpenSettings: () -> Unit,
onOpenLibrary: () -> Unit,
) {
val status by vm.status.collectAsStateWithLifecycle() val status by vm.status.collectAsStateWithLifecycle()
val song by vm.currentSong.collectAsStateWithLifecycle() val song by vm.currentSong.collectAsStateWithLifecycle()
val serverHost by vm.serverHost.collectAsStateWithLifecycle() val serverHost by vm.serverHost.collectAsStateWithLifecycle()
@@ -62,8 +67,11 @@ fun NowPlayingScreen(vm: PlayerViewModel, onOpenSettings: () -> Unit) {
.padding(24.dp), .padding(24.dp),
horizontalAlignment = Alignment.CenterHorizontally, horizontalAlignment = Alignment.CenterHorizontally,
) { ) {
// --- Top bar: settings ---------------------------------------------- // --- Top bar: library + settings ------------------------------------
Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.End) { Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.End) {
IconButton(onClick = onOpenLibrary) {
Icon(Icons.Filled.LibraryMusic, contentDescription = "Albums")
}
IconButton(onClick = onOpenSettings) { IconButton(onClick = onOpenSettings) {
Icon(Icons.Filled.Settings, contentDescription = "Settings") Icon(Icons.Filled.Settings, contentDescription = "Settings")
} }
@@ -67,4 +67,7 @@ class PlayerViewModel(application: Application) : AndroidViewModel(application)
fun setVolume(volume: Int) = manager.setVolume(volume) fun setVolume(volume: Int) = manager.setVolume(volume)
fun setRepeat(on: Boolean) = manager.setRepeat(on) fun setRepeat(on: Boolean) = manager.setRepeat(on)
fun setRandom(on: Boolean) = manager.setRandom(on) fun setRandom(on: Boolean) = manager.setRandom(on)
fun playAlbum(album: String, albumArtist: String?) = manager.playAlbum(album, albumArtist)
suspend fun loadAlbums() = manager.loadAlbums()
} }
@@ -80,6 +80,27 @@ class MpdConnectionTest {
assertArrayEquals(payload, resp.binary) assertArrayEquals(payload, resp.binary)
} }
@Test
fun executeList_splitsResponsesOnListOk() {
// command_list_ok_begin: each sub-response ends with list_OK, then final OK.
val server = "OK MPD 0.23.5\n" +
"volume: 50\nstate: play\nlist_OK\n" +
"file: a.mp3\nTitle: A\nlist_OK\n" +
"OK\n"
val out = ByteArrayOutputStream()
val c = conn(server, out)
c.handshake()
val responses = c.executeList("status", "currentsong")
assertEquals(2, responses.size)
assertEquals("50", responses[0]["volume"])
assertEquals("play", responses[0]["state"])
assertEquals("a.mp3", responses[1]["file"])
assertEquals("A", responses[1]["Title"])
assertTrue(out.toString("UTF-8").startsWith("command_list_ok_begin\nstatus\ncurrentsong\ncommand_list_end\n"))
}
@Test @Test
fun execute_throwsOnEofMidResponse() { fun execute_throwsOnEofMidResponse() {
val c = conn("OK MPD 0.23.5\nvolume: 50\n") // no OK terminator val c = conn("OK MPD 0.23.5\nvolume: 50\n") // no OK terminator
@@ -95,6 +95,31 @@ class MpdModelTest {
assertEquals(2, songs[1].id) assertEquals(2, songs[1].id)
} }
@Test
fun albums_parseGroupedByArtist() {
// `list album group albumartist` style output.
val resp = MpdResponse(
listOf(
"AlbumArtist" to "Pink Floyd",
"Album" to "Animals",
"Album" to "The Wall",
"AlbumArtist" to "Daft Punk",
"Album" to "Discovery",
)
)
val albums = MpdAlbum.listFrom(resp)
assertEquals(3, albums.size)
assertEquals(MpdAlbum("Animals", "Pink Floyd"), albums[0])
assertEquals(MpdAlbum("The Wall", "Pink Floyd"), albums[1])
assertEquals(MpdAlbum("Discovery", "Daft Punk"), albums[2])
}
@Test
fun albums_albumWithoutArtistGroupHasNullArtist() {
val resp = MpdResponse(listOf("Album" to "Untitled"))
assertEquals(listOf(MpdAlbum("Untitled", null)), MpdAlbum.listFrom(resp))
}
@Test @Test
fun statistics_parseCounts() { fun statistics_parseCounts() {
val stats = MpdStatistics.from( val stats = MpdStatistics.from(
+36 -12
View File
@@ -68,14 +68,19 @@ Notes: verified on the FiiO via injected key events (a rotary volume knob may no
emit `VOLUME_UP/DOWN` — hardware-dependent). Album art in the notification/session emit `VOLUME_UP/DOWN` — hardware-dependent). Album art in the notification/session
is pending item #5. is pending item #5.
## [ ] 4. Library browse — albums ## [x] 4. Library browse — albums
A screen to browse all albums on the server (artists can come later). Done. `AlbumsScreen` (library icon on the player) lists every album via
`list album group albumartist` (`MpdCommands.listAlbums``MpdAlbum.listFrom`),
sorted by artist then title. Tapping an album replaces the queue and plays it
(`MpdClient.playAlbum` = `clear` + `findadd album … albumartist …` + `play`),
then returns to the now-playing screen.
- MPD commands: `list album group albumartist` (or `list album`), and - Verified live: 149 albums listed and played correctly on the FiiO.
`find album "<name>"` to fetch an album's tracks; add to the queue with - Placeholder disc icon per row (real art is item #5).
`add`/`findadd`. Extend `MpdCommands` + `MpdClient`. - Interaction choice: tap = **replace queue + play** (the direct "play this
- Grid or list of albums → tap to view/queue tracks. album" gesture). Could later add a long-press / menu for "add to queue" and an
album-detail/track view. Artist browse is a future extension.
## [ ] 5. Album / artist images ## [ ] 5. Album / artist images
@@ -88,7 +93,7 @@ Pull artwork for the now-playing track and for the album browse grid.
- Needs an image loader + caching. Coil (`io.coil-kt`) is the standard Compose - Needs an image loader + caching. Coil (`io.coil-kt`) is the standard Compose
choice; a custom `MpdArtFetcher` could feed it. Dependency → `deps.json` regen. choice; a custom `MpdArtFetcher` could feed it. Dependency → `deps.json` regen.
## [ ] 6. BUG: idle connection drops after a few minutes → kicked to connect page ## [x] 6. BUG: idle connection drops after a few minutes → kicked to connect page
After a few minutes idling, the app surfaces **"connection closed mid-response"** After a few minutes idling, the app surfaces **"connection closed mid-response"**
and falls back to the connect screen. MALP does not do this. and falls back to the connect screen. MALP does not do this.
@@ -108,8 +113,27 @@ and falls back to the connect screen. MALP does not do this.
**auto-reconnect transparently** (re-open connections, re-issue `idle`, resync **auto-reconnect transparently** (re-open connections, re-issue `idle`, resync
state) and keep showing the player. Consider a keepalive ping and, for state) and keep showing the player. Consider a keepalive ping and, for
backgrounded playback control, a foreground service / partial wakelock. backgrounded playback control, a foreground service / partial wakelock.
- PARTIALLY ADDRESSED by item #3: the `mediaPlayback` foreground service now - PARTIALLY ADDRESSED by item #3: the `mediaPlayback` foreground service keeps
keeps the process/connection alive in the background, which should stop Doze / the process/connection alive in the background.
wifi power-save from tearing the sockets down (the most likely cause). Still
worth verifying over a long idle, and adding transparent auto-reconnect + **FIXED.** Root cause was two-fold: (a) the quiet command connection was reaped
keepalive ping as defense-in-depth (e.g. against MPD `connection_timeout`). by MPD's `connection_timeout`, and (b) the idle loop treated *any* `IOException`
(including that reap) as fatal → `Error` → connect page. Plus a Pi-Zero overload
angle: volume-key floods + 2-round-trip refreshes. Fixes, all in `MpdClient`
/`MpdConnectionManager`/`MpdConnection`:
- **Keepalive `ping`** every 25s on the command connection — prevents the
`connection_timeout` reap and detects a dead socket early.
- **Transparent auto-reconnect** (`triggerReconnect`/`reconnectLoop`): a transport
drop reopens both sockets in the background (state → `Connecting`, "Connecting…"
splash, player stays), retrying with backoff; only after
`MAX_RECONNECT_ATTEMPTS` does it surface `Error`. `MpdAckException` is
explicitly *not* treated as a connection failure.
- **Volume throttle**: `setvol` writes are coalesced through a conflated channel
(~1 per 120 ms, latest value) — kills the flood from holding volume keys while
keeping accurate accumulation via `pendingVolume`.
- **Command-list refresh**: `status`+`currentsong` fetched in one round-trip
(`MpdConnection.executeList`, `command_list_ok_begin`).
Verified on the FiiO: Wi-Fi drop → "Connecting…" → auto-recovers to the player
(no connect-page bounce); 6 rapid volume presses = exactly 30, throttled.