Fix persistence bugs with notification

Fix issues where the notification for Auxio will show up incomplete during the restore process.

app/src/main/java/org/oxycblt/auxio/MainFragment.kt

@@ -12,7 +12,6 @@ import androidx.fragment.app.Fragment
 import androidx.fragment.app.activityViewModels
 import androidx.navigation.NavController
 import androidx.navigation.NavOptions
-import androidx.navigation.findNavController
 import androidx.navigation.fragment.NavHostFragment
 import androidx.navigation.fragment.findNavController
 import androidx.navigation.ui.NavigationUI
@@ -50,8 +49,8 @@ class MainFragment : Fragment() {
             getInactiveAlpha(accent.first)
         )
 
-        // Set up the tints for the navigation icon
+        // Set up the tints for the navigation icons + text
-        val navIconTints = ColorStateList(
+        val navTints = ColorStateList(
             arrayOf(
                 intArrayOf(-android.R.attr.state_checked),
                 intArrayOf(android.R.attr.state_checked)
@@ -68,8 +67,8 @@ class MainFragment : Fragment() {
 
         binding.lifecycleOwner = this
 
-        binding.navBar.itemIconTintList = navIconTints
+        binding.navBar.itemIconTintList = navTints
-        binding.navBar.itemTextColor = navIconTints
+        binding.navBar.itemTextColor = navTints
 
         navController?.let {
             binding.navBar.setOnNavigationItemSelectedListener { item ->
@@ -93,6 +92,8 @@ class MainFragment : Fragment() {
             }
         }
 
+        playbackModel.restorePlaybackIfNeeded(requireContext())
+
         Log.d(this::class.simpleName, "Fragment Created.")
 
         return binding.root

app/src/main/java/org/oxycblt/auxio/music/Models.kt

@@ -4,14 +4,27 @@ import android.net.Uri
 
 // --- MUSIC MODELS ---
 
-// The base model for all music
+/**
-// This is used in a lot of general functions in order to have generic utilities
+ * The base data object for all music.
+ * @property id The ID that is assigned to this object
+ * @property name The name of this object (Such as a song title)
+ * @author OxygenCobalt
+ */
 sealed class BaseModel {
     abstract val id: Long
     abstract val name: String
 }
 
-// Song
+/**
+ * The data object for a song. Inherits [BaseModel].
+ * @property albumId  The Song's Album ID. Never use this outside of when attaching a song to its album.
+ * @property track    The Song's Track number
+ * @property duration The duration of the song, in millis.
+ * @property album    The Song's parent album. Use this instead of [albumId].
+ * @property seconds  The Song's duration in seconds
+ * @property formattedDuration The Song's duration as a duration string.
+ * @author OxygenCobalt
+ */
 data class Song(
     override val id: Long = -1,
     override var name: String,
@@ -25,7 +38,17 @@ data class Song(
     val formattedDuration: String = seconds.toDuration()
 }
 
-// Album
+/**
+ * The data object for an album. Inherits [BaseModel].
+ * @property artistId The Album's parent artist ID. Do not use this outside of attaching an album to its parent artist.
+ * @property coverUri The [Uri] for the album's cover. **Load this using Coil.**
+ * @property year     The year this album was released. 0 if there is none in the metadata.
+ * @property artist   The Album's parent [Artist]. use this instead of [artistId]
+ * @property songs    The Album's child [Song]s.
+ * @property numSongs The amount of songs in an Album.
+ * @property totalDuration The combined duration of all of the album's child songs, formatted.
+ * @author OxygenCobalt
+ */
 data class Album(
     override val id: Long = -1,
     override val name: String,
@@ -46,7 +69,10 @@ data class Album(
     }
 }
 
-// Artist
+/**
+ * The data object for an artist. Inherits [BaseModel]
+ * @author OxygenCobalt
+ */
 data class Artist(
     override val id: Long = -1,
     override var name: String
@@ -71,7 +97,12 @@ data class Artist(
     }
 }
 
-// Genre
+/**
+ * The data object for a genre. Inherits [BaseModel]
+ * @property artists The list of all [Artist]s in this genre
+ * @property songs   The list of all [Song]s in this genre.
+ * @author OxygenCobalt
+ */
 data class Genre(
     override val id: Long = -1,
     override var name: String,
@@ -102,7 +133,9 @@ data class Genre(
     }
 }
 
-// Header [Used for search, nothing else]
+/**
+ * A data object used solely for the "Header" UI element. Inherits [BaseModel].
+ */
 data class Header(
     override val id: Long = -1,
     override var name: String = "",

app/src/main/java/org/oxycblt/auxio/playback/NotificationUtils.kt

@@ -19,8 +19,6 @@ import org.oxycblt.auxio.playback.state.PlaybackMode
 import org.oxycblt.auxio.playback.state.PlaybackStateManager
 
 object NotificationUtils {
-    val DO_COMPAT_SUBTEXT = Build.VERSION.SDK_INT < Build.VERSION_CODES.O
-
     const val CHANNEL_ID = "CHANNEL_AUXIO_PLAYBACK"
     const val NOTIFICATION_ID = 0xA0A0
     const val REQUEST_CODE = 0xA0C0
@@ -81,6 +79,13 @@ fun NotificationManager.createMediaNotification(
         .setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
 }
 
+/**
+ * Set the current metadata of a media notification.
+ * @param song The [Song] that the notification should reflect
+ * @param context The [Context] needed to load the cover bitmap
+ * @param onDone A callback for when the process is finished
+ * @author OxygenCobalt
+ */
 fun NotificationCompat.Builder.setMetadata(song: Song, context: Context, onDone: () -> Unit) {
     setContentTitle(song.name)
     setContentText(
@@ -89,7 +94,7 @@ fun NotificationCompat.Builder.setMetadata(song: Song, context: Context, onDone:
 
     // On older versions of android [API <26], show the song's album on the subtext instead of
     // the current mode, as that makes more sense for the old style of media notifications.
-    if (NotificationUtils.DO_COMPAT_SUBTEXT) {
+    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
         setSubText(song.album.name)
     }
 
@@ -102,7 +107,7 @@ fun NotificationCompat.Builder.setMetadata(song: Song, context: Context, onDone:
     }
 }
 
-// I have no idea how to update actions on the fly so I have to use these restricted APIs.
+// I have no idea how to update a specific action on the fly so I have to use these restricted APIs
 @SuppressLint("RestrictedApi")
 fun NotificationCompat.Builder.updatePlaying(context: Context) {
     mActions[2] = newAction(NotificationUtils.ACTION_PLAY_PAUSE, context)
@@ -114,7 +119,7 @@ fun NotificationCompat.Builder.updateLoop(context: Context) {
 }
 
 fun NotificationCompat.Builder.updateMode(context: Context) {
-    if (!NotificationUtils.DO_COMPAT_SUBTEXT) {
+    if (Build.VERSION.SDK_INT > Build.VERSION_CODES.O) {
         val playbackManager = PlaybackStateManager.getInstance()
 
         // If playing from all songs, set the subtext as that, otherwise the currently played parent.
@@ -126,6 +131,11 @@ fun NotificationCompat.Builder.updateMode(context: Context) {
     }
 }
 
+/**
+ * Create a new [NotificationCompat.Action].
+ * @param action The action that the notification action should represent
+ * @param context The [Context] needed to create the action
+ */
 private fun newAction(action: String, context: Context): NotificationCompat.Action {
     val playbackManager = PlaybackStateManager.getInstance()
 

app/src/main/java/org/oxycblt/auxio/playback/PlaybackService.kt

@@ -145,6 +145,19 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
 
         if (playbackManager.song != null) {
             restorePlayer()
+            notification.updateLoop(this)
+            notification.updateMode(this)
+            notification.updatePlaying(this)
+
+            playbackManager.song?.let {
+                notification.setMetadata(it, this) {
+                    if (playbackManager.isPlaying) {
+                        startForegroundOrNotify("Restore")
+                    } else {
+                        stopForegroundAndNotification()
+                    }
+                }
+            }
         }
     }
 
@@ -223,8 +236,13 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
             }
 
             uploadMetadataToSession(it)
-            notification.setMetadata(playbackManager.song!!, this) {
+
-                startForegroundOrNotify()
+            if (playbackManager.isRestored) {
+                notification.setMetadata(playbackManager.song!!, this) {
+                    startForegroundOrNotify("Song")
+                }
+            } else {
+                notification.setMetadata(playbackManager.song!!, this) {}
             }
 
             return
@@ -238,7 +256,7 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
     override fun onModeUpdate(mode: PlaybackMode) {
         notification.updateMode(this)
 
-        startForegroundOrNotify()
+        startForegroundOrNotify("Mode")
     }
 
     override fun onPlayingUpdate(isPlaying: Boolean) {
@@ -247,13 +265,13 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
         if (isPlaying && !player.isPlaying) {
             player.play()
             notification.updatePlaying(this)
-            startForegroundOrNotify()
+            startForegroundOrNotify("Play")
 
             startPollingPosition()
         } else {
             player.pause()
             notification.updatePlaying(this)
-            startForegroundOrNotify()
+            startForegroundOrNotify("Pause")
         }
     }
 
@@ -270,7 +288,7 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
         }
 
         notification.updateLoop(this)
-        startForegroundOrNotify()
+        startForegroundOrNotify("Loop")
     }
 
     override fun onSeekConfirm(position: Long) {
@@ -279,20 +297,6 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
         player.seekTo(position)
     }
 
-    override fun onNeedContextToRestoreState() {
-        Log.d(this::class.simpleName, "Giving context to PlaybackStateManager")
-
-        serviceScope.launch {
-            playbackManager.getStateFromDatabase(this@PlaybackService)
-        }
-
-        Log.d(this::class.simpleName, "notification restore")
-
-        // FIXME: Current position just will not show on notification when state is restored
-        restorePlayer()
-        restoreNotification()
-    }
-
     // --- OTHER FUNCTIONS ---
 
     private fun restorePlayer() {
@@ -301,27 +305,13 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
             player.setMediaItem(item)
             player.prepare()
             player.seekTo(playbackManager.position)
-
-            notification.setMetadata(it, this) {
-                startForegroundOrNotify()
-            }
         }
     }
 
-    private fun restoreNotification() {
+    override fun onRestoreFinish() {
-        playbackManager.song?.let {
+        Log.d(this::class.simpleName, "Restore done")
-            uploadMetadataToSession(it)
-            notification.updateLoop(this)
-            notification.updateMode(this)
-            notification.updatePlaying(this)
-            notification.setMetadata(it, this) {
-                startForegroundOrNotify()
-            }
 
-            return
+        restorePlayer()
-        }
-
-        stopForegroundAndNotification()
     }
 
     private fun uploadMetadataToSession(song: Song) {
@@ -356,19 +346,23 @@ class PlaybackService : Service(), Player.EventListener, PlaybackStateManager.Ca
         }
     }
 
-    private fun startForegroundOrNotify() {
+    private fun startForegroundOrNotify(reason: String) {
         // Start the service in the foreground if haven't already.
-        if (!isForeground) {
+        if (playbackManager.isRestored) {
-            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
+            Log.d(this::class.simpleName, "Starting foreground because of $reason")
-                startForeground(
+
-                    NotificationUtils.NOTIFICATION_ID, notification.build(),
+            if (!isForeground) {
-                    ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK
+                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
-                )
+                    startForeground(
+                        NotificationUtils.NOTIFICATION_ID, notification.build(),
+                        ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK
+                    )
+                } else {
+                    startForeground(NotificationUtils.NOTIFICATION_ID, notification.build())
+                }
             } else {
-                startForeground(NotificationUtils.NOTIFICATION_ID, notification.build())
+                notificationManager.notify(NotificationUtils.NOTIFICATION_ID, notification.build())
             }
-        } else {
-            notificationManager.notify(NotificationUtils.NOTIFICATION_ID, notification.build())
         }
     }
 

app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt

@@ -1,10 +1,13 @@
 package org.oxycblt.auxio.playback
 
+import android.content.Context
 import android.util.Log
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.Transformations
 import androidx.lifecycle.ViewModel
+import androidx.lifecycle.viewModelScope
+import kotlinx.coroutines.launch
 import org.oxycblt.auxio.music.Album
 import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.BaseModel
@@ -78,14 +81,10 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback {
 
         // If the PlaybackViewModel was cleared [Signified by PlaybackStateManager still being
         // around & the fact that we are in the init function], then attempt to restore the
-        // viewmodel state.
+        // viewmodel state. If it isn't, then wait for MainFragment to give the command to restore
+        // PlaybackStateManager.
         if (playbackManager.isRestored) {
             restorePlaybackState()
-        } else {
-            // If [PlaybackStateManger] was also cleared [Due to Auxio's process dying], then
-            // attempt to restore it [Albeit PlaybackService will need to do the job as it
-            // has a context while PlaybackViewModel doesn't].
-            playbackManager.needContextToRestoreState()
         }
     }
 
@@ -249,6 +248,14 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback {
         mIsSeeking.value = value
     }
 
+    fun restorePlaybackIfNeeded(context: Context) {
+        if (!playbackManager.isRestored) {
+            viewModelScope.launch {
+                playbackManager.getStateFromDatabase(context)
+            }
+        }
+    }
+
     // --- OVERRIDES ---
 
     override fun onCleared() {

app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt

@@ -24,7 +24,6 @@ import kotlin.random.Random
  *
  * All instantiation should be done with [PlaybackStateManager.from()].
  * @author OxygenCobalt
- * FIXME: Add started variable instead of relying on mSong being null
  */
 class PlaybackStateManager private constructor() {
     // Playback
@@ -455,10 +454,6 @@ class PlaybackStateManager private constructor() {
     // TODO: Persist queue edits?
     // FIXME: Shuffling w/o knowing the original queue edit from keepSong will cause issues
 
-    fun needContextToRestoreState() {
-        callbacks.forEach { it.onNeedContextToRestoreState() }
-    }
-
     suspend fun saveStateToDatabase(context: Context) {
         Log.d(this::class.simpleName, "Saving state to DB.")
 
@@ -483,16 +478,19 @@ class PlaybackStateManager private constructor() {
             return@withContext states
         }
 
-        mIsRestored = true
-
         if (states.isEmpty()) {
             Log.d(this::class.simpleName, "Nothing here. Not restoring.")
+
+            mIsRestored = true
+
             return
         }
 
         Log.d(this::class.simpleName, "Old state found, ${states[0]}")
 
         unpackFromPlaybackState(states[0])
+
+        mIsRestored = true
     }
 
     private fun packToPlaybackState(): PlaybackState {
@@ -580,6 +578,7 @@ class PlaybackStateManager private constructor() {
         callbacks.forEach {
             it.onSeekConfirm(mPosition)
             it.onModeUpdate(mMode)
+            it.onRestoreFinish()
         }
     }
 
@@ -600,7 +599,7 @@ class PlaybackStateManager private constructor() {
         fun onShuffleUpdate(isShuffling: Boolean) {}
         fun onLoopUpdate(mode: LoopMode) {}
         fun onSeekConfirm(position: Long) {}
-        fun onNeedContextToRestoreState() {}
+        fun onRestoreFinish() {}
     }
 
     companion object {