Display progress of media downloads#505
Conversation
.../main/java/com/google/android/horologist/mediasample/ui/mapper/DownloadMediaUiModelMapper.kt
Show resolved
Hide resolved
yschimke
left a comment
There was a problem hiding this comment.
LGTM - some questions about implementation choices.
...n/java/com/google/android/horologist/mediasample/data/datasource/PlaylistRemoteDataSource.kt
Show resolved
Hide resolved
...a/com/google/android/horologist/mediasample/data/service/download/DownloadProgressMonitor.kt
Show resolved
Hide resolved
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.launch | ||
|
|
||
| class DownloadProgressMonitor( |
There was a problem hiding this comment.
Is this from a sample somewhere? Curious about the choice of Handler etc, rather than coroutines.
There was a problem hiding this comment.
mainly from here, the choice copy&paste of Handler was from ForegroundNotificationUpdater in that file
| } | ||
|
|
||
| private fun update(downloadManager: DownloadManager) { | ||
| coroutineScope.launch { |
There was a problem hiding this comment.
Is it possible that two cycles of this overlap? Seems like launch would complete instantly, and then the in (running) will execute.
There was a problem hiding this comment.
I think it is possible, that's the reason it is launching the coroutine in a broader scope, so if an overlapping call to update calls removeCallbacksAndMessages then it wouldn't cancel a potential database update that is in progress. Happy to hear any suggestions to improve this.
There was a problem hiding this comment.
What about moving the loop (reposting to the handler) inside the launch?
WHAT
Display progress of media downloads in
PlaylistDownloadScreen.device-2022-08-19-143143.mp4
WHY
In order to align with Figma specs.
HOW
progressandsizetoMediaDownloadEntity;DownloadProgressMonitorto queryDownloadManagerevery specified interval and update the download progress withMediaDownloadLocalDataSource;DownloadManagerListenerto start and stopDownloadProgressMonitor;Checklist 📋