From 5dd7a411ed7d356c2d526006e8f1c16fe785947a Mon Sep 17 00:00:00 2001 From: cottongin Date: Tue, 10 Mar 2026 05:15:31 -0400 Subject: [PATCH] fix: refactor player state machine to eliminate race conditions Introduce a Connecting state so the UI reflects user intent immediately, centralize navigation in MainActivity via state transitions, and replace the pauseRequested volatile flag with controller state as single source of truth. Made-with: Cursor --- .../xyz/cottongin/radio247/MainActivity.kt | 15 ++ .../radio247/service/PlaybackState.kt | 9 ++ .../radio247/service/RadioController.kt | 20 ++- .../radio247/service/RadioPlaybackService.kt | 65 +++++--- .../radio247/ui/components/MiniPlayer.kt | 72 ++++++--- .../ui/screens/nowplaying/NowPlayingScreen.kt | 141 ++++++++++++------ .../screens/nowplaying/NowPlayingViewModel.kt | 17 +++ .../screens/stationlist/StationListScreen.kt | 77 ++++++---- ...026-03-09_player-state-machine-refactor.md | 46 ++++++ .../2026-03-09_ui-polish-and-pause.md | 38 +++++ 10 files changed, 380 insertions(+), 120 deletions(-) create mode 100644 chat-summaries/2026-03-09_player-state-machine-refactor.md create mode 100644 chat-summaries/2026-03-09_ui-polish-and-pause.md diff --git a/app/src/main/java/xyz/cottongin/radio247/MainActivity.kt b/app/src/main/java/xyz/cottongin/radio247/MainActivity.kt index 8d22427..37bd929 100644 --- a/app/src/main/java/xyz/cottongin/radio247/MainActivity.kt +++ b/app/src/main/java/xyz/cottongin/radio247/MainActivity.kt @@ -11,11 +11,13 @@ import androidx.core.content.ContextCompat import androidx.activity.compose.BackHandler import androidx.activity.compose.setContent import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.platform.LocalContext +import xyz.cottongin.radio247.service.PlaybackState import xyz.cottongin.radio247.ui.navigation.Screen import xyz.cottongin.radio247.ui.screens.nowplaying.NowPlayingScreen import xyz.cottongin.radio247.ui.screens.settings.SettingsScreen @@ -38,7 +40,20 @@ class MainActivity : ComponentActivity() { } } Radio247Theme { + val app = application as RadioApplication + val playbackState by app.controller.state.collectAsState() var currentScreen by remember { mutableStateOf(Screen.StationList) } + var wasActive by remember { mutableStateOf(false) } + + LaunchedEffect(playbackState) { + val isActive = playbackState !is PlaybackState.Idle + if (isActive && !wasActive) { + currentScreen = Screen.NowPlaying + } else if (!isActive && wasActive && currentScreen == Screen.NowPlaying) { + currentScreen = Screen.StationList + } + wasActive = isActive + } BackHandler(enabled = currentScreen != Screen.StationList) { currentScreen = Screen.StationList diff --git a/app/src/main/java/xyz/cottongin/radio247/service/PlaybackState.kt b/app/src/main/java/xyz/cottongin/radio247/service/PlaybackState.kt index 6bf371b..d1c9602 100644 --- a/app/src/main/java/xyz/cottongin/radio247/service/PlaybackState.kt +++ b/app/src/main/java/xyz/cottongin/radio247/service/PlaybackState.kt @@ -5,12 +5,21 @@ import xyz.cottongin.radio247.data.model.Station sealed interface PlaybackState { data object Idle : PlaybackState + data class Connecting( + val station: Station, + val sessionStartedAt: Long = System.currentTimeMillis() + ) : PlaybackState data class Playing( val station: Station, val metadata: IcyMetadata? = null, val sessionStartedAt: Long = System.currentTimeMillis(), val connectionStartedAt: Long = System.currentTimeMillis() ) : PlaybackState + data class Paused( + val station: Station, + val metadata: IcyMetadata? = null, + val sessionStartedAt: Long + ) : PlaybackState data class Reconnecting( val station: Station, val metadata: IcyMetadata? = null, diff --git a/app/src/main/java/xyz/cottongin/radio247/service/RadioController.kt b/app/src/main/java/xyz/cottongin/radio247/service/RadioController.kt index 20b7584..ee17c1b 100644 --- a/app/src/main/java/xyz/cottongin/radio247/service/RadioController.kt +++ b/app/src/main/java/xyz/cottongin/radio247/service/RadioController.kt @@ -17,6 +17,9 @@ class RadioController( val estimatedLatencyMs: StateFlow = _estimatedLatencyMs.asStateFlow() fun play(station: Station) { + val current = _state.value + if (current is PlaybackState.Connecting && current.station.id == station.id) return + _state.value = PlaybackState.Connecting(station) val intent = Intent(application, RadioPlaybackService::class.java).apply { action = RadioPlaybackService.ACTION_PLAY putExtra(RadioPlaybackService.EXTRA_STATION_ID, station.id) @@ -25,12 +28,28 @@ class RadioController( } fun stop() { + _state.value = PlaybackState.Idle val intent = Intent(application, RadioPlaybackService::class.java).apply { action = RadioPlaybackService.ACTION_STOP } application.startService(intent) } + fun pause() { + val current = _state.value + if (current is PlaybackState.Playing) { + _state.value = PlaybackState.Paused( + station = current.station, + metadata = current.metadata, + sessionStartedAt = current.sessionStartedAt + ) + } + val intent = Intent(application, RadioPlaybackService::class.java).apply { + action = RadioPlaybackService.ACTION_PAUSE + } + application.startService(intent) + } + fun seekToLive() { val intent = Intent(application, RadioPlaybackService::class.java).apply { action = RadioPlaybackService.ACTION_SEEK_LIVE @@ -38,7 +57,6 @@ class RadioController( application.startService(intent) } - // Called by the service to update state internal fun updateState(state: PlaybackState) { _state.value = state } diff --git a/app/src/main/java/xyz/cottongin/radio247/service/RadioPlaybackService.kt b/app/src/main/java/xyz/cottongin/radio247/service/RadioPlaybackService.kt index 1ba0d95..1c40d28 100644 --- a/app/src/main/java/xyz/cottongin/radio247/service/RadioPlaybackService.kt +++ b/app/src/main/java/xyz/cottongin/radio247/service/RadioPlaybackService.kt @@ -42,6 +42,7 @@ class RadioPlaybackService : LifecycleService() { companion object { const val ACTION_PLAY = "xyz.cottongin.radio247.PLAY" const val ACTION_STOP = "xyz.cottongin.radio247.STOP" + const val ACTION_PAUSE = "xyz.cottongin.radio247.PAUSE" const val ACTION_SEEK_LIVE = "xyz.cottongin.radio247.SEEK_LIVE" const val EXTRA_STATION_ID = "station_id" } @@ -105,6 +106,7 @@ class RadioPlaybackService : LifecycleService() { stopSelf() } } + ACTION_PAUSE -> handlePause() ACTION_SEEK_LIVE -> handleSeekLive() ACTION_STOP -> handleStop() else -> stopSelf() @@ -114,17 +116,22 @@ class RadioPlaybackService : LifecycleService() { private fun launchPlay(stationId: Long) { val oldJob = playJob + val currentState = controller.state.value + val isResume = currentState is PlaybackState.Paused && currentState.station.id == stationId playJob = serviceScope.launch { oldJob?.let { - stayConnected = false + if (!isResume) { + stayConnected = false + } engine?.stop() it.join() } stayConnected = app.preferences.stayConnected.first() val station = stationDao.getStationById(stationId) if (station != null) { - handlePlay(station) + handlePlay(station, reuseSession = isResume) } else { + controller.updateState(PlaybackState.Idle) stopSelf() } } @@ -133,31 +140,31 @@ class RadioPlaybackService : LifecycleService() { override fun onBind(intent: Intent): IBinder? = null override fun onDestroy() { - cleanup() + cleanupResources() serviceScope.cancel() super.onDestroy() } - private fun cleanup() { + private fun cleanupResources() { engine?.stop() engine = null releaseLocks() mediaSession?.release() mediaSession = null unregisterNetworkCallback() - controller.updateState(PlaybackState.Idle) controller.updateLatency(0) } - private suspend fun handlePlay(station: Station) { - sessionStartedAt = System.currentTimeMillis() - - listeningSessionId = listeningSessionDao.insert( - ListeningSession( - stationId = station.id, - startedAt = sessionStartedAt + private suspend fun handlePlay(station: Station, reuseSession: Boolean = false) { + if (!reuseSession) { + sessionStartedAt = System.currentTimeMillis() + listeningSessionId = listeningSessionDao.insert( + ListeningSession( + stationId = station.id, + startedAt = sessionStartedAt + ) ) - ) + } acquireLocks() ensureMediaSession() @@ -174,16 +181,32 @@ class RadioPlaybackService : LifecycleService() { } } finally { endConnectionSpan() - endListeningSession() - val isActiveJob = playJob == coroutineContext[Job] - if (isActiveJob) { - cleanup() - stopForeground(STOP_FOREGROUND_REMOVE) - stopSelf() + when (controller.state.value) { + is PlaybackState.Paused -> { + updateNotification(station, currentMetadata, isReconnecting = false, isPaused = true) + } + is PlaybackState.Idle -> { + endListeningSession() + val isActiveJob = playJob == coroutineContext[Job] + if (isActiveJob) { + cleanupResources() + stopForeground(STOP_FOREGROUND_REMOVE) + stopSelf() + } + } + else -> { + // Connecting to a new station or other transition -- another job takes over + } } } } + private fun handlePause() { + stayConnected = false + retryImmediatelyOnNetwork = false + engine?.stop() + } + private fun handleSeekLive() { engine?.skipAhead() } @@ -260,7 +283,7 @@ class RadioPlaybackService : LifecycleService() { engine!!.start() - val collectorJob = serviceScope.launch collector@ { + serviceScope.launch collector@ { engine!!.events.collect { event -> when (event) { is AudioEngineEvent.MetadataChanged -> { @@ -367,7 +390,7 @@ class RadioPlaybackService : LifecycleService() { updateNotification(station, null, isReconnecting = false) } - private fun updateNotification(station: Station, metadata: IcyMetadata?, isReconnecting: Boolean) { + private fun updateNotification(station: Station, metadata: IcyMetadata?, isReconnecting: Boolean, isPaused: Boolean = false) { val session = mediaSession ?: return val stopIntent = Intent(this, RadioPlaybackService::class.java).apply { action = ACTION_STOP diff --git a/app/src/main/java/xyz/cottongin/radio247/ui/components/MiniPlayer.kt b/app/src/main/java/xyz/cottongin/radio247/ui/components/MiniPlayer.kt index acaeecb..45f2c74 100644 --- a/app/src/main/java/xyz/cottongin/radio247/ui/components/MiniPlayer.kt +++ b/app/src/main/java/xyz/cottongin/radio247/ui/components/MiniPlayer.kt @@ -1,13 +1,18 @@ package xyz.cottongin.radio247.ui.components +import androidx.compose.foundation.background import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Pause +import androidx.compose.material.icons.filled.PlayArrow import androidx.compose.material.icons.filled.Stop import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -17,9 +22,10 @@ import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp -import xyz.cottongin.radio247.audio.IcyMetadata -import xyz.cottongin.radio247.data.model.Station +import coil3.compose.AsyncImage import xyz.cottongin.radio247.service.PlaybackState @Composable @@ -29,12 +35,17 @@ fun MiniPlayer( onStop: () -> Unit, modifier: Modifier = Modifier ) { - val (station, metadata) = when (state) { - is PlaybackState.Playing -> state.station to state.metadata - is PlaybackState.Reconnecting -> state.station to state.metadata + val (station, metadata, isPaused) = when (state) { + is PlaybackState.Connecting -> Triple(state.station, null, false) + is PlaybackState.Playing -> Triple(state.station, state.metadata, false) + is PlaybackState.Paused -> Triple(state.station, state.metadata, true) + is PlaybackState.Reconnecting -> Triple(state.station, state.metadata, false) PlaybackState.Idle -> return } + val artUrl = metadata?.streamUrl?.takeIf { it.endsWith(".jpg", true) || it.endsWith(".png", true) } + ?: station.defaultArtworkUrl + Surface( modifier = modifier.fillMaxWidth(), tonalElevation = 2.dp @@ -42,31 +53,54 @@ fun MiniPlayer( Row( modifier = Modifier .clickable(onClick = onTap) - .padding(horizontal = 16.dp, vertical = 12.dp) + .padding(horizontal = 12.dp, vertical = 8.dp) .fillMaxWidth(), verticalAlignment = Alignment.CenterVertically ) { - Text( - text = station.name, - style = MaterialTheme.typography.titleSmall, - modifier = Modifier.weight(1f) - ) - metadata?.title?.let { title -> - Spacer(modifier = Modifier.width(8.dp)) - Text( - text = title, - style = MaterialTheme.typography.bodySmall, - maxLines = 1 + if (artUrl != null) { + AsyncImage( + model = artUrl, + contentDescription = null, + modifier = Modifier + .size(40.dp) + .clip(RoundedCornerShape(6.dp)) + .background(MaterialTheme.colorScheme.surfaceVariant, RoundedCornerShape(6.dp)) ) + Spacer(modifier = Modifier.width(10.dp)) } - Spacer(modifier = Modifier.width(8.dp)) + Column(modifier = Modifier.weight(1f)) { + Text( + text = station.name, + style = MaterialTheme.typography.titleSmall, + maxLines = 1, + overflow = TextOverflow.Ellipsis + ) + val subtitle = when { + state is PlaybackState.Connecting -> "Connecting..." + isPaused -> "Paused" + state is PlaybackState.Reconnecting -> "Reconnecting..." + metadata?.title != null -> metadata.title + else -> null + } + subtitle?.let { + Text( + text = it, + style = MaterialTheme.typography.bodySmall, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } + Spacer(modifier = Modifier.width(4.dp)) IconButton( onClick = onStop, modifier = Modifier.size(40.dp) ) { Icon( Icons.Default.Stop, - contentDescription = "Stop" + contentDescription = "Stop", + tint = MaterialTheme.colorScheme.error ) } } diff --git a/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingScreen.kt b/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingScreen.kt index 1304cc7..a2ab781 100644 --- a/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingScreen.kt +++ b/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingScreen.kt @@ -18,14 +18,13 @@ import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.FastForward +import androidx.compose.material.icons.filled.Pause +import androidx.compose.material.icons.filled.PlayArrow import androidx.compose.material.icons.filled.Stop -import androidx.compose.material3.Button -import androidx.compose.material3.ButtonDefaults import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.FilledTonalButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme @@ -69,18 +68,26 @@ fun NowPlayingScreen( val bufferMs by viewModel.bufferMs.collectAsState() when (val state = playbackState) { + is PlaybackState.Connecting, is PlaybackState.Playing, + is PlaybackState.Paused, is PlaybackState.Reconnecting -> { val station = when (state) { + is PlaybackState.Connecting -> state.station is PlaybackState.Playing -> state.station + is PlaybackState.Paused -> state.station is PlaybackState.Reconnecting -> state.station else -> return } val metadata = when (state) { is PlaybackState.Playing -> state.metadata + is PlaybackState.Paused -> state.metadata is PlaybackState.Reconnecting -> state.metadata else -> null } + val isPaused = state is PlaybackState.Paused + val isPlaying = state is PlaybackState.Playing + val isConnecting = state is PlaybackState.Connecting Box(modifier = modifier.fillMaxSize()) { Column( @@ -145,9 +152,15 @@ fun NowPlayingScreen( modifier = Modifier.padding(vertical = 8.dp) ) Text( - text = formatTrackInfo(metadata), + text = when { + isConnecting -> "Connecting..." + isPaused -> "Paused" + else -> formatTrackInfo(metadata) + }, style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant + color = if (isPaused || isConnecting) + MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.6f) + else MaterialTheme.colorScheme.onSurfaceVariant ) Spacer(modifier = Modifier.height(24.dp)) @@ -156,14 +169,16 @@ fun NowPlayingScreen( text = "Session: ${formatElapsed(sessionElapsed)}", style = MaterialTheme.typography.bodyMedium ) - Text( - text = "Connected: ${formatElapsed(connectionElapsed)}", - style = MaterialTheme.typography.bodyMedium - ) - Text( - text = "Latency: ~${estimatedLatencyMs}ms", - style = MaterialTheme.typography.bodyMedium - ) + if (!isPaused && !isConnecting) { + Text( + text = "Connected: ${formatElapsed(connectionElapsed)}", + style = MaterialTheme.typography.bodyMedium + ) + Text( + text = "Latency: ~${estimatedLatencyMs}ms", + style = MaterialTheme.typography.bodyMedium + ) + } Spacer(modifier = Modifier.height(24.dp)) @@ -172,21 +187,50 @@ fun NowPlayingScreen( horizontalArrangement = Arrangement.Center, verticalAlignment = Alignment.CenterVertically ) { - IconButton( - onClick = { viewModel.skipAhead() }, - modifier = Modifier.size(64.dp), - enabled = state is PlaybackState.Playing - ) { - Icon( - Icons.Filled.FastForward, - contentDescription = "Skip ahead ~1s", - modifier = Modifier.size(40.dp), - tint = if (state is PlaybackState.Playing) - MaterialTheme.colorScheme.primary - else MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.4f) - ) + if (isPaused) { + IconButton( + onClick = { viewModel.resume() }, + modifier = Modifier.size(64.dp) + ) { + Icon( + Icons.Filled.PlayArrow, + contentDescription = "Resume", + modifier = Modifier.size(48.dp), + tint = MaterialTheme.colorScheme.primary + ) + } + } else { + IconButton( + onClick = { viewModel.skipAhead() }, + modifier = Modifier.size(64.dp), + enabled = isPlaying + ) { + Icon( + Icons.Filled.FastForward, + contentDescription = "Skip ahead ~1s", + modifier = Modifier.size(40.dp), + tint = if (isPlaying) + MaterialTheme.colorScheme.primary + else MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.4f) + ) + } + Spacer(modifier = Modifier.width(16.dp)) + IconButton( + onClick = { viewModel.pause() }, + modifier = Modifier.size(64.dp), + enabled = isPlaying + ) { + Icon( + Icons.Filled.Pause, + contentDescription = "Pause", + modifier = Modifier.size(40.dp), + tint = if (isPlaying) + MaterialTheme.colorScheme.onSurface + else MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.4f) + ) + } } - Spacer(modifier = Modifier.width(24.dp)) + Spacer(modifier = Modifier.width(16.dp)) IconButton( onClick = { viewModel.stop() }, modifier = Modifier.size(64.dp) @@ -257,33 +301,32 @@ fun NowPlayingScreen( } } } - } - } - PlaybackState.Idle -> { - Column(modifier = modifier.fillMaxSize()) { - TopAppBar( - title = { }, - navigationIcon = { - IconButton(onClick = onBack) { - Icon( - Icons.AutoMirrored.Filled.ArrowBack, - contentDescription = "Back" - ) + + if (isConnecting) { + Box( + modifier = Modifier + .fillMaxSize() + .padding(24.dp), + contentAlignment = Alignment.Center + ) { + Card( + modifier = Modifier.padding(32.dp), + elevation = CardDefaults.cardElevation(defaultElevation = 8.dp) + ) { + Column( + modifier = Modifier.padding(32.dp), + horizontalAlignment = Alignment.CenterHorizontally + ) { + CircularProgressIndicator() + Spacer(modifier = Modifier.height(16.dp)) + Text("Connecting to ${station.name}...") + } } } - ) - Box( - modifier = Modifier.fillMaxSize(), - contentAlignment = Alignment.Center - ) { - Text( - text = "Nothing playing", - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurfaceVariant - ) } } } + PlaybackState.Idle -> {} } } diff --git a/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingViewModel.kt b/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingViewModel.kt index 4cd6dd1..74ab416 100644 --- a/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingViewModel.kt +++ b/app/src/main/java/xyz/cottongin/radio247/ui/screens/nowplaying/NowPlayingViewModel.kt @@ -50,7 +50,9 @@ class NowPlayingViewModel(application: Application) : AndroidViewModel(applicati sessionElapsed = combine(ticker, playbackState) { now, state -> when (state) { + is PlaybackState.Connecting -> now - state.sessionStartedAt is PlaybackState.Playing -> now - state.sessionStartedAt + is PlaybackState.Paused -> now - state.sessionStartedAt is PlaybackState.Reconnecting -> now - state.sessionStartedAt else -> 0L } @@ -66,12 +68,16 @@ class NowPlayingViewModel(application: Application) : AndroidViewModel(applicati viewModelScope.launch { playbackState.collect { state -> val (station, metadata) = when (state) { + is PlaybackState.Connecting -> state.station to null is PlaybackState.Playing -> state.station to state.metadata + is PlaybackState.Paused -> state.station to state.metadata is PlaybackState.Reconnecting -> state.station to state.metadata PlaybackState.Idle -> null to null } when (state) { + is PlaybackState.Connecting, is PlaybackState.Playing, + is PlaybackState.Paused, is PlaybackState.Reconnecting -> { artworkResolveJob?.cancel() artworkResolveJob = viewModelScope.launch { @@ -99,6 +105,17 @@ class NowPlayingViewModel(application: Application) : AndroidViewModel(applicati controller.stop() } + fun pause() { + controller.pause() + } + + fun resume() { + val state = playbackState.value + if (state is PlaybackState.Paused) { + controller.play(state.station) + } + } + fun skipAhead() { controller.seekToLive() } diff --git a/app/src/main/java/xyz/cottongin/radio247/ui/screens/stationlist/StationListScreen.kt b/app/src/main/java/xyz/cottongin/radio247/ui/screens/stationlist/StationListScreen.kt index 9df3613..bf24418 100644 --- a/app/src/main/java/xyz/cottongin/radio247/ui/screens/stationlist/StationListScreen.kt +++ b/app/src/main/java/xyz/cottongin/radio247/ui/screens/stationlist/StationListScreen.kt @@ -5,20 +5,20 @@ import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.background import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.shape.CircleShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.Folder @@ -32,7 +32,6 @@ import androidx.compose.material3.DropdownMenuItem 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 @@ -46,9 +45,11 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip import androidx.compose.ui.unit.dp import androidx.compose.ui.platform.LocalContext import androidx.lifecycle.viewmodel.compose.viewModel +import coil3.compose.AsyncImage import xyz.cottongin.radio247.data.model.Playlist import xyz.cottongin.radio247.data.model.Station import xyz.cottongin.radio247.service.PlaybackState @@ -115,7 +116,9 @@ fun StationListScreen( }, bottomBar = { when (playbackState) { + is PlaybackState.Connecting, is PlaybackState.Playing, + is PlaybackState.Paused, is PlaybackState.Reconnecting -> MiniPlayer( state = playbackState, onTap = onNavigateToNowPlaying, @@ -126,7 +129,9 @@ fun StationListScreen( } ) { paddingValues -> val currentPlayingStationId = when (val state = playbackState) { + is PlaybackState.Connecting -> state.station.id is PlaybackState.Playing -> state.station.id + is PlaybackState.Paused -> state.station.id is PlaybackState.Reconnecting -> state.station.id PlaybackState.Idle -> null } @@ -256,7 +261,7 @@ private fun PlaylistSectionHeader( verticalAlignment = Alignment.CenterVertically ) { Icon( - imageVector = if (isExpanded) Icons.Default.Folder else Icons.Default.Folder, + imageVector = Icons.Default.Folder, contentDescription = null, modifier = Modifier.size(24.dp) ) @@ -266,17 +271,17 @@ private fun PlaylistSectionHeader( style = MaterialTheme.typography.titleSmall, modifier = Modifier.weight(1f) ) - IconButton( - onClick = { onToggleStar() }, - modifier = Modifier.size(32.dp) - ) { - Icon( - imageVector = if (playlist.starred) Icons.Filled.Star else Icons.Outlined.Star, - contentDescription = if (playlist.starred) "Unstar" else "Star", - tint = if (playlist.starred) MaterialTheme.colorScheme.primary - else MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.4f) - ) - } + IconButton( + onClick = { onToggleStar() }, + modifier = Modifier.size(32.dp) + ) { + Icon( + imageVector = if (playlist.starred) Icons.Filled.Star else Icons.Outlined.Star, + contentDescription = if (playlist.starred) "Unstar" else "Star", + tint = if (playlist.starred) MaterialTheme.colorScheme.primary + else MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.4f) + ) + } } } @@ -300,7 +305,8 @@ private fun StationRow( .combinedClickable( onClick = onPlay, onLongClick = { showMenu = true } - ), + ) + .padding(vertical = 4.dp), verticalAlignment = Alignment.CenterVertically ) { IconButton( @@ -315,6 +321,17 @@ private fun StationRow( ) } Spacer(modifier = Modifier.width(8.dp)) + if (station.defaultArtworkUrl != null) { + AsyncImage( + model = station.defaultArtworkUrl, + contentDescription = null, + modifier = Modifier + .size(36.dp) + .clip(CircleShape) + .background(MaterialTheme.colorScheme.surfaceVariant, CircleShape) + ) + Spacer(modifier = Modifier.width(8.dp)) + } Column(modifier = Modifier.weight(1f)) { Text( text = station.name, @@ -342,20 +359,20 @@ private fun StationRow( expanded = showMenu, onDismissRequest = { showMenu = false } ) { - DropdownMenuItem( - text = { Text("Edit") }, - onClick = { - showMenu = false - onEdit() - } - ) - DropdownMenuItem( - text = { Text("Delete") }, - onClick = { - showMenu = false - onDelete() - } - ) + DropdownMenuItem( + text = { Text("Edit") }, + onClick = { + showMenu = false + onEdit() + } + ) + DropdownMenuItem( + text = { Text("Delete") }, + onClick = { + showMenu = false + onDelete() + } + ) } } } diff --git a/chat-summaries/2026-03-09_player-state-machine-refactor.md b/chat-summaries/2026-03-09_player-state-machine-refactor.md new file mode 100644 index 0000000..dd93ed4 --- /dev/null +++ b/chat-summaries/2026-03-09_player-state-machine-refactor.md @@ -0,0 +1,46 @@ +# Player State Machine Refactor + +## Task +Refactored the player state management to eliminate race conditions during rapid station tapping, stop/play transitions, and the NowPlayingScreen bounce-back bug. + +## Changes Made + +### PlaybackState.kt +- Added `Connecting` state variant to represent the period between user tap and service processing the intent. + +### RadioController.kt +- `play()` now sets state to `Connecting` synchronously before sending the intent, eliminating the async gap. +- `stop()` now sets state to `Idle` synchronously before sending the intent. +- `pause()` now sets state to `Paused` synchronously before sending the intent. +- Added redundancy guard: double-tapping a station that's already `Connecting` is a no-op. + +### RadioPlaybackService.kt +- Removed the `pauseRequested` volatile flag entirely. +- The `handlePlay()` finally block now checks `controller.state.value` (the single source of truth) instead of the volatile flag to determine cleanup behavior. +- Renamed `cleanup()` to `cleanupResources()` since it no longer sets `Idle` state (that's now the controller's responsibility). +- `handlePause()` and `handleStop()` no longer set `pauseRequested`. + +### MainActivity.kt +- Centralized navigation logic via a `LaunchedEffect` that observes `RadioController.state`. +- Navigates to NowPlaying on `Idle -> Active` transitions. +- Navigates back to StationList on `Active -> Idle` transitions while on NowPlaying. +- Station switching (`Playing(A) -> Connecting(B)`) stays on NowPlaying. + +### NowPlayingScreen.kt +- Removed the `LaunchedEffect(playbackState)` that called `onBack()` on `Idle` (was the source of the bounce-back bug). +- Added `Connecting` to the `when` branches, showing a loading spinner overlay. +- Disabled pause/skip-ahead buttons while connecting. + +### NowPlayingViewModel.kt +- Added `Connecting` handling in session timer (uses `sessionStartedAt`), connection timer (shows 0), and artwork resolver (uses station default artwork). + +### StationListScreen.kt +- Removed `onNavigateToNowPlaying()` from `onPlay` callbacks (navigation is now centralized in MainActivity). +- Added `Connecting` to mini player visibility and "now playing" station highlight. + +### MiniPlayer.kt +- Added `Connecting` to the destructuring `when` block. +- Shows "Connecting..." subtitle for the `Connecting` state. + +## Follow-up Items +- The `stayConnected` and `retryImmediatelyOnNetwork` volatile flags in RadioPlaybackService could be further consolidated into the state machine in a future pass. diff --git a/chat-summaries/2026-03-09_ui-polish-and-pause.md b/chat-summaries/2026-03-09_ui-polish-and-pause.md new file mode 100644 index 0000000..dea639c --- /dev/null +++ b/chat-summaries/2026-03-09_ui-polish-and-pause.md @@ -0,0 +1,38 @@ +# UI polish: navigation, pause, artwork, scrollability + +**Date:** 2026-03-09 + +## Task description +Five UI/UX improvements after manual testing. + +## Changes made + +### 1. Auto-navigate to Now Playing on station tap (StationListScreen.kt) +- `onPlay` callback now calls `onNavigateToNowPlaying()` immediately after `viewModel.playStation()`. + +### 2. Auto-navigate back to station list on stop (NowPlayingScreen.kt) +- `LaunchedEffect(playbackState)` watches for `PlaybackState.Idle` and calls `onBack()`. +- Removed the "Nothing playing" dead-end screen. + +### 3. Pause/resume support +- **PlaybackState.kt**: Added `Paused(station, metadata, sessionStartedAt)` state. +- **RadioController.kt**: Added `pause()` method sending `ACTION_PAUSE`. +- **RadioPlaybackService.kt**: `handlePause()` sets `pauseRequested = true` and stops engine. In `handlePlay`'s finally block, if paused: updates state to `Paused`, keeps listening session alive, doesn't cleanup service. On resume (ACTION_PLAY with same station while paused), reuses the session. +- **NowPlayingViewModel.kt**: Added `pause()` and `resume()`. Session timer keeps ticking during pause. +- **NowPlayingScreen.kt**: Transport controls adapt to state — paused shows large play/resume button + stop; playing shows skip-ahead + pause + stop. + +### 4. Artwork in mini player and station list +- **MiniPlayer.kt**: Shows 40dp rounded artwork thumbnail from ICY StreamUrl or station's `defaultArtworkUrl`. Shows "Paused"/"Reconnecting..." subtitle as appropriate. +- **StationListScreen.kt**: Station rows show a 36dp circular artwork thumbnail from `defaultArtworkUrl` (EXTIMG) when available. + +### 5. Scrollability / drag-n-drop readiness +- LazyColumn was already scrollable. Added `padding(vertical = 4.dp)` to station rows for better touch targets. All items have stable keys (`station.id`, `"playlist_header_${playlist.id}"`) ready for reorderable integration. + +## Files changed +- `app/src/main/java/.../service/PlaybackState.kt` +- `app/src/main/java/.../service/RadioController.kt` +- `app/src/main/java/.../service/RadioPlaybackService.kt` +- `app/src/main/java/.../ui/screens/nowplaying/NowPlayingScreen.kt` +- `app/src/main/java/.../ui/screens/nowplaying/NowPlayingViewModel.kt` +- `app/src/main/java/.../ui/screens/stationlist/StationListScreen.kt` +- `app/src/main/java/.../ui/components/MiniPlayer.kt`