diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-11-03 08:54:29 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-11-03 08:54:29 +0000 |
| commit | 24eeda287300211a519afeb80171cb483f3f8589 (patch) | |
| tree | 4b71d814bccd1d8b311e66dc162ae5caf0521533 | |
| parent | 913ba0e829ca20ccbb1a8db213541760dfc42a4b (diff) | |
Wire up the sleep timer's second mode, and make a duration extend
Peter's call on both, after the view-model work surfaced them as dead code.
SleepTimer was documented as working "in two modes: a countdown, or stop at the end of the current
episode", and armForEndOfEpisode, shouldStopAtEpisodeEnd and extendBy had no caller anywhere. So
"stop at the end of this episode" did not exist in the app, and every duration in the sleep dialog
went to armFor, which resets -- tapping 15m on a timer with thirty seconds left gave 15:00 and
quietly took those thirty seconds away, the one thing extendBy's own comment says not to do.
A duration now adds to a running timer and the buttons say so, "+15m" rather than "15m". The
dialog offers "End of episode", and the overflow entry reads "Sleep timer - end of episode" in
that mode, since there is no countdown to show.
The two modes watch differently on purpose. A countdown needs a clock, so it ticks. Stopping at
the end of an episode needs the moment the episode is done, and a one-second poll means a second
of the next episode playing to somebody who asked to be stopped -- so it follows nowPlaying, which
already republishes every second for the progress bar, and treats the queue advancing as the end
whatever the last position said.
Adding the mode took PlayerViewModel past its function limit, so the whole concern moved to
SleepController -- the answer ListenViewModel got when it outgrew one, rather than raising the
threshold a third time. SleepTimer keeps the arithmetic and stays pure.
ReachabilityTest now covers classes, which is why this was invisible: it scanned `object`
declarations only, and SleepTimer is a class. Classes cannot use the object check's matching --
a call names the instance, not the class -- so they get the DAO check's looser `.name(` matching,
which needed three call shapes before it was honest: the ordinary call, a `::name` reference, and
a trailing-lambda call with no parens at all, which is how OpmlViewModel reaches importFrom.
It found one more thing, allowlisted with the reason rather than hidden: **on-device detection is
dead code.** FingerprintUpdater and MidrollUpdater are never constructed -- every reference from
outside their own files is a KDoc cross-reference -- and through them IntroOutroDetector,
AudioCorrelation, MidrollScanner, MidrollDetector, MidrollClipStore, MediaCodecClipSource,
Decimate, Fft, MonoResampler, PcmCodec, StoredClip and BreakTrack, about two thousand lines
shipped in every APK. The workers went when detection moved to the server; the classes stayed.
Deleting a subsystem is Peter's call, so it is recorded in the allowlist instead.
Two smaller things found on the way. EpisodeDownloader's wait notification went onto ServerAudio,
where the rest of the server's side of a download already lives, rather than becoming a seventh
constructor parameter -- and the first attempt added it *after* the `now` clock, which silently
rebound every existing trailing-lambda call site from the clock to the new hook while still
compiling. The UnusedImports rule enabled earlier today caught a stale import in this very commit.
1057 tests, detekt 0, lint clean, coverage 95.17% -> 95.29%. Checked on the emulator: the dialog
offers End of episode, and arming 15m shows "Sleep timer - 14:52" on the overflow entry.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
14 files changed, 460 insertions, 65 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index e8f178c..621badc 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -95.17 +95.29 diff --git a/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt index 04ca9a3..9aca940 100644 --- a/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt +++ b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt @@ -46,6 +46,16 @@ interface ServerAudio { /** Records the server's offsets, once its audio is actually the file on disk. */ suspend fun record(episode: EpisodeEntity, info: ServerEpisode) + + /** + * Called when the server could not supply this episode yet and it has been left waiting. + * + * Here rather than as another downloader parameter because it is the same subject as the + * rest of this interface: a wait exists only because a server is configured, and what to do + * about one -- ask again shortly rather than at the next feed refresh -- is the server's + * side of the download, not the HTTP's. + */ + suspend fun waiting(episode: EpisodeEntity) {} } class EpisodeDownloader( @@ -110,6 +120,9 @@ class EpisodeDownloader( DownloadState.FAILED } episodes.updateDownload(episode.id, state, null, null) + // Only a wait. A download that failed outright has nothing to come back for -- + // see DownloadDrainer on why a 404 is not retried for ever. + if (state == DownloadState.WAITING_FOR_SERVER) serverAudio?.waiting(episode) }, ) return result.map { it.file } diff --git a/app/src/main/java/org/terst/listen/playback/SleepTimer.kt b/app/src/main/java/org/terst/listen/playback/SleepTimer.kt index d94b371..9cab6cf 100644 --- a/app/src/main/java/org/terst/listen/playback/SleepTimer.kt +++ b/app/src/main/java/org/terst/listen/playback/SleepTimer.kt @@ -65,9 +65,20 @@ class SleepTimer(private val fadeMs: Long = 20_000) { /** Ramps linearly to silence over the final stretch, so the audio recedes rather than stops. */ fun volumeAt(nowMs: Long): Float { val remaining = remainingMs(nowMs) ?: return 1.0f - if (remaining >= fadeMs) return 1.0f + return volumeFor(remaining) + } + + /** + * The same ramp over a stretch somebody else is counting. + * + * End-of-episode mode has no deadline of its own -- what is left comes from the episode, and + * a seek moves it -- so the caller supplies the remaining time and the fade is shared rather + * than written twice. + */ + fun volumeFor(remainingMs: Long): Float { + if (remainingMs >= fadeMs) return 1.0f if (fadeMs <= 0) return 0.0f - return (remaining.toFloat() / fadeMs).coerceIn(0.0f, 1.0f) + return (remainingMs.toFloat() / fadeMs).coerceIn(0.0f, 1.0f) } fun state(nowMs: Long): State? = when { diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt b/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt index 48ab23f..3404589 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeDetailActions.kt @@ -29,6 +29,7 @@ data class EpisodeDetailActions( val reportAd: () -> Unit = {}, /** Stops playback after this many minutes, fading over the last twenty seconds. */ val armSleepTimer: (minutes: Long) -> Unit = {}, + val armSleepTimerAtEndOfEpisode: () -> Unit = {}, val cancelSleepTimer: () -> Unit = {}, /** Opens the episode's words, or the reason there are none. */ val openTranscript: () -> Unit = {}, diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeMenu.kt b/app/src/main/java/org/terst/listen/ui/EpisodeMenu.kt index 93294ba..4ce2f3b 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeMenu.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeMenu.kt @@ -4,6 +4,7 @@ import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.EpisodeTranscriptEntity import org.terst.listen.download.WaitingForServerText import org.terst.listen.feed.Chapter +import org.terst.listen.playback.SleepTimer /** What the overflow menu needs to know about the open episode. */ data class OverflowContent( @@ -12,8 +13,8 @@ data class OverflowContent( val loadChapters: suspend () -> List<Chapter>, val watchLink: String?, val positionMs: Long, - /** Milliseconds left on the sleep timer, or null when none is running. */ - val sleepRemainingMs: Long? = null, + /** What the sleep timer is waiting for, or null when none is running. */ + val sleep: SleepTimer.State? = null, /** What is known about this episode's words, or null when nobody has looked. */ val transcript: EpisodeTranscriptEntity? = null, ) @@ -38,14 +39,14 @@ data class EpisodeMenuItem(val action: EpisodeMenuAction, val label: String) object EpisodeMenu { /** - * [sleepRemainingMs] is null when no sleep timer is running. It is shown on the entry so the - * remaining time can be read without opening anything -- the one question anyone has about a - * timer they set in the dark. + * [sleep] is null when no sleep timer is running. What it is waiting for is shown on the + * entry so it can be read without opening anything -- the one question anyone has about a + * timer they set in the dark. End-of-episode mode has no countdown, so it names itself. */ fun items( episode: EpisodeEntity, watchLink: String?, - sleepRemainingMs: Long? = null, + sleep: SleepTimer.State? = null, transcript: EpisodeTranscriptEntity? = null, ): List<EpisodeMenuItem> = buildList { add(EpisodeMenuItem(EpisodeMenuAction.QUEUE, "Queue")) @@ -72,7 +73,11 @@ object EpisodeMenu { add( EpisodeMenuItem( EpisodeMenuAction.SLEEP, - if (sleepRemainingMs == null) "Sleep timer" else "Sleep timer \u00b7 ${formatClock(sleepRemainingMs)}", + when (sleep) { + null -> "Sleep timer" + is SleepTimer.State.Countdown -> "Sleep timer \u00b7 ${formatClock(sleep.remainingMs)}" + SleepTimer.State.EndOfEpisode -> "Sleep timer \u00b7 end of episode" + }, ), ) } diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeOverflow.kt b/app/src/main/java/org/terst/listen/ui/EpisodeOverflow.kt index 0622bb2..b2f8ecb 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeOverflow.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeOverflow.kt @@ -35,6 +35,7 @@ import androidx.core.net.toUri import org.terst.listen.data.EpisodeEntity import org.terst.listen.download.WaitingForServerText import org.terst.listen.feed.Chapter +import org.terst.listen.playback.SleepTimer /** * The top bar's `⋮`: every secondary action, one tap away. What it lists is [EpisodeMenu]'s call. @@ -55,7 +56,7 @@ fun EpisodeOverflow(content: OverflowContent, actions: EpisodeDetailActions) { EpisodeMenu.items( content.episode, content.watchLink, - content.sleepRemainingMs, + content.sleep, content.transcript, ).forEach { item -> DropdownMenuItem( @@ -78,7 +79,7 @@ fun EpisodeOverflow(content: OverflowContent, actions: EpisodeDetailActions) { EpisodeMenuAction.CHAPTERS -> ChaptersDialog(content, actions.seekTo) { dialog = null } EpisodeMenuAction.PUBLISHER_COPY -> PublisherCopyDialog(content.episode, actions.downloadFromPublisher) { dialog = null } - EpisodeMenuAction.SLEEP -> SleepTimerDialog(content.sleepRemainingMs, actions) { dialog = null } + EpisodeMenuAction.SLEEP -> SleepTimerDialog(content.sleep, actions) { dialog = null } else -> Unit } } @@ -102,35 +103,46 @@ private fun perform(action: EpisodeMenuAction, actions: EpisodeDetailActions, wa /** * Set a sleep timer, or cancel the one running. * - * Durations rather than a picker: nobody sleeps in 37 minutes. The remaining time is shown + * Durations rather than a picker: nobody sleeps in 37 minutes. What it is waiting for is shown * because the whole point of opening this a second time is to find out how long is left. + * + * A duration **adds** to a timer already running, and the buttons say so -- "+15m" rather than + * "15m". Resetting silently took the remaining time away, on a control reached for half-asleep. + * "End of episode" is the timer's other mode, which the app supported internally and offered + * nowhere until 2026-11-03. */ @Composable -private fun SleepTimerDialog(remainingMs: Long?, actions: EpisodeDetailActions, dismiss: () -> Unit) { +private fun SleepTimerDialog(sleep: SleepTimer.State?, actions: EpisodeDetailActions, dismiss: () -> Unit) { + val running = sleep != null AlertDialog( onDismissRequest = dismiss, title = { Text("Sleep timer") }, text = { Column { Text( - if (remainingMs == null) { - "Volume fades over the last 20 seconds rather than cutting." - } else { - "Stopping in ${formatClock(remainingMs)}. Volume fades over the last 20 seconds." + when (sleep) { + null -> "Volume fades over the last 20 seconds rather than cutting." + is SleepTimer.State.Countdown -> + "Stopping in ${formatClock(sleep.remainingMs)}. Volume fades over the last 20 seconds." + SleepTimer.State.EndOfEpisode -> + "Stopping when this episode ends. Volume fades over the last 20 seconds." }, style = MaterialTheme.typography.bodyMedium, ) Row(Modifier.padding(top = 12.dp)) { SLEEP_MINUTES.forEach { minutes -> TextButton(onClick = { dismiss(); actions.armSleepTimer(minutes) }) { - Text("${minutes}m") + Text(if (running) "+${minutes}m" else "${minutes}m") } } } + TextButton(onClick = { dismiss(); actions.armSleepTimerAtEndOfEpisode() }) { + Text("End of episode") + } } }, confirmButton = { - if (remainingMs != null) { + if (running) { TextButton(onClick = { dismiss(); actions.cancelSleepTimer() }) { Text("Cancel timer") } } }, diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt index fec8d4e..1e8864f 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt @@ -43,6 +43,7 @@ import org.terst.listen.feed.WatchLink import org.terst.listen.fingerprint.EpisodeSkips import org.terst.listen.fingerprint.SkipSummary import org.terst.listen.playback.NowPlaying +import org.terst.listen.playback.SleepTimer import org.terst.listen.playback.PlaybackSpeed /** @@ -66,7 +67,7 @@ fun EpisodeScreen( var ranges by remember { mutableStateOf<List<EpisodeMidrollEntity>>(emptyList()) } var speed by remember { mutableFloatStateOf(PlaybackSpeed.DEFAULT) } val nowPlaying by player.nowPlaying.collectAsState() - val sleepRemaining by player.sleepRemainingMs.collectAsState() + val sleep by player.sleepState.collectAsState() // What the screen shows, which is the route's episode until the player moves off it. The // route argument cannot be the answer on its own: the queue advances without any navigation, @@ -102,7 +103,7 @@ fun EpisodeScreen( topBar = { EpisodeTopBar(EpisodeTitles.topBar(showTitle), nav.back) { current?.let { shown -> - Overflow(ScreenSource(vm, player), shown, nowPlaying, sleepRemaining, hooks) + Overflow(ScreenSource(vm, player), shown, nowPlaying, sleep, hooks) } } }, @@ -194,6 +195,7 @@ private fun episodeActions( reportAd = player::reportAd, openTranscript = hooks.openTranscript, armSleepTimer = player::armSleepTimer, + armSleepTimerAtEndOfEpisode = player::armSleepTimerAtEndOfEpisode, cancelSleepTimer = player::cancelSleepTimer, setSpeed = { chosen -> hooks.onSpeed(chosen) @@ -355,7 +357,7 @@ private fun Overflow( source: ScreenSource, shown: EpisodeEntity, nowPlaying: NowPlaying, - sleepRemainingMs: Long?, + sleep: SleepTimer.State?, hooks: EpisodeHooks, ) { val positionMs = if (nowPlaying.episodeId == shown.id) nowPlaying.positionMs else shown.positionMs @@ -367,7 +369,7 @@ private fun Overflow( EpisodeOverflow( OverflowContent( shown, chapters, WatchLink.at(shown.mediaUrl, positionMs), positionMs, - sleepRemainingMs, transcript, + sleep, transcript, ), episodeActions(source.vm, source.player, shown, nowPlaying, hooks), ) diff --git a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt index 4552211..7ff96b4 100644 --- a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt @@ -13,13 +13,13 @@ import org.terst.listen.data.Graph import org.terst.listen.feed.Chapter import org.terst.listen.playback.NowPlaying import org.terst.listen.playback.PlaybackSpeed +import org.terst.listen.playback.SleepTimer import org.terst.listen.data.UiGraph import org.terst.listen.playback.PlaybackControls import org.terst.listen.playback.PlayerConnection import org.terst.listen.playback.StartOutcome import org.terst.listen.playback.BedtimeAction import org.terst.listen.playback.BedtimePolicy -import org.terst.listen.playback.SleepTimer import org.terst.listen.playback.toMediaItem /** @@ -187,18 +187,25 @@ class PlayerViewModel( // --- sleep timer --------------------------------------------------------------------- - private val sleepTimer = SleepTimer() - private val _sleepRemainingMs = MutableStateFlow<Long?>(null) - val sleepRemainingMs: StateFlow<Long?> = _sleepRemainingMs.asStateFlow() + /** The timer and the two ways of watching for the end; see [SleepController]. */ + private val sleep = SleepController( + scope = viewModelScope, + connection = connection, + onFinished = { + bedtimeArmedTimer = false + _banner.value = Banner("Sleep timer finished") + }, + ) - fun armSleepTimer(minutes: Long) { - sleepTimer.armFor(minutes * MS_PER_MINUTE, System.currentTimeMillis()) - watchSleepTimer() - } + val sleepRemainingMs: StateFlow<Long?> = sleep.remainingMs + val sleepState: StateFlow<SleepTimer.State?> = sleep.state + + fun armSleepTimer(minutes: Long) = sleep.arm(minutes * MS_PER_MINUTE) + + fun armSleepTimerAtEndOfEpisode() = sleep.armForEndOfEpisode() fun cancelSleepTimer() { - sleepTimer.cancel() - _sleepRemainingMs.value = null + sleep.cancel() bedtimeArmedTimer = false } @@ -212,7 +219,7 @@ class PlayerViewModel( /** Bedtime mode's effect on playback: see [BedtimePolicy] for why each case is what it is. */ fun onBedtimeChanged(entering: Boolean) { val action = if (entering) { - BedtimePolicy.entering(timerAlreadyArmed = sleepTimer.isArmed) + BedtimePolicy.entering(timerAlreadyArmed = sleep.isArmed) } else { BedtimePolicy.leaving(bedtimeArmedIt = bedtimeArmedTimer) } @@ -226,31 +233,6 @@ class PlayerViewModel( } } - /** - * Polls the timer rather than scheduling a single alarm: the fade needs a volume update - * every tick anyway, and a deadline read from the clock survives the process being paused. - */ - private fun watchSleepTimer() { - viewModelScope.launch { - while (sleepTimer.isArmed) { - val now = System.currentTimeMillis() - _sleepRemainingMs.value = sleepTimer.remainingMs(now) - if (sleepTimer.hasExpired(now)) { - connection.setVolume(0f) - connection.pause() - sleepTimer.cancel() - _sleepRemainingMs.value = null - bedtimeArmedTimer = false - _banner.value = Banner("Sleep timer finished") - break - } - connection.setVolume(sleepTimer.volumeAt(now)) - delay(PROGRESS_TICK_MS) - } - } - } - - private companion object { const val PROGRESS_TICK_MS = 1_000L const val MS_PER_MINUTE = 60_000L diff --git a/app/src/main/java/org/terst/listen/ui/SleepController.kt b/app/src/main/java/org/terst/listen/ui/SleepController.kt new file mode 100644 index 0000000..7565f30 --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/SleepController.kt @@ -0,0 +1,142 @@ +package org.terst.listen.ui + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import org.terst.listen.playback.PlaybackControls +import org.terst.listen.playback.SleepTimer + +/** + * Running the sleep timer against what is playing. + * + * Split out of [PlayerViewModel] when adding the second mode took that class past its function + * limit -- the same answer `ListenViewModel` got when it outgrew one, and for the same reason: + * the cluster that moved is a whole concern, not an arbitrary few methods. [SleepTimer] owns the + * arithmetic and is pure; this owns the two ways of watching for the end, which need a scope and + * the player. + * + * The modes watch differently on purpose. A countdown needs a clock, so it ticks. Stopping at the + * end of an episode needs to know the moment the episode is done, and a one-second poll means a + * second of the next episode playing to somebody who asked to be stopped -- so it follows + * `nowPlaying`, which already republishes every second for the progress bar. + */ +class SleepController( + private val scope: CoroutineScope, + private val connection: PlaybackControls, + private val onFinished: () -> Unit, + private val timer: SleepTimer = SleepTimer(), +) { + + private val _remainingMs = MutableStateFlow<Long?>(null) + val remainingMs: StateFlow<Long?> = _remainingMs.asStateFlow() + + private val _state = MutableStateFlow<SleepTimer.State?>(null) + val state: StateFlow<SleepTimer.State?> = _state.asStateFlow() + + val isArmed: Boolean get() = timer.isArmed + + /** The episode end-of-episode mode is waiting for, so the queue moving on counts as the end. */ + private var stopAfterEpisodeId: String? = null + private var endOfEpisodeWatch: Job? = null + + /** One loop however many times the timer is armed; extending makes re-arming routine. */ + private var ticking = false + + /** + * Adds to a running timer rather than replacing it. + * + * Every duration in the dialog used to reset: 15m on a timer with thirty seconds left gave + * 15:00 and quietly took those thirty seconds away. [SleepTimer.extendBy] was written for + * exactly this, with the reason in its own comment, and had no caller. + */ + fun arm(durationMs: Long) { + val now = System.currentTimeMillis() + if (timer.isArmed) timer.extendBy(durationMs, now) else timer.armFor(durationMs, now) + stopAfterEpisodeId = null + endOfEpisodeWatch?.cancel() + // At once rather than on the next tick: the dialog reads this the moment it reopens, and + // extending showed the figure from before the extension for up to a second. + publish(now) + tick() + } + + /** Stop when the current episode finishes. The timer's second mode, offered nowhere until now. */ + fun armForEndOfEpisode() { + timer.armForEndOfEpisode() + stopAfterEpisodeId = connection.nowPlaying.value.episodeId + _remainingMs.value = null + _state.value = SleepTimer.State.EndOfEpisode + watchEndOfEpisode() + } + + fun cancel() { + timer.cancel() + clear() + } + + private fun publish(nowMs: Long) { + _state.value = timer.state(nowMs) + _remainingMs.value = timer.remainingMs(nowMs) + } + + private fun tick() { + if (ticking) return + ticking = true + scope.launch { + try { + while (timer.isArmed && !timer.shouldStopAtEpisodeEnd) { + val now = System.currentTimeMillis() + publish(now) + if (timer.hasExpired(now)) { + finish() + break + } + connection.setVolume(timer.volumeAt(now)) + delay(TICK_MS) + } + } finally { + ticking = false + } + } + } + + private fun watchEndOfEpisode() { + endOfEpisodeWatch?.cancel() + endOfEpisodeWatch = scope.launch { + connection.nowPlaying.collect { playing -> + if (!timer.shouldStopAtEpisodeEnd) return@collect + // The queue moving on is the end of it, whatever the last position said. + if (playing.episodeId != stopAfterEpisodeId) { + finish() + return@collect + } + val duration = playing.durationMs ?: return@collect + val left = (duration - playing.positionMs).coerceAtLeast(0) + if (left <= 0) finish() else connection.setVolume(timer.volumeFor(left)) + } + } + } + + private fun finish() { + connection.setVolume(0f) + connection.pause() + timer.cancel() + clear() + onFinished() + } + + private fun clear() { + _remainingMs.value = null + _state.value = null + stopAfterEpisodeId = null + endOfEpisodeWatch?.cancel() + } + + private companion object { + const val TICK_MS = 1_000L + } +} diff --git a/app/src/test/java/org/terst/listen/ReachabilityTest.kt b/app/src/test/java/org/terst/listen/ReachabilityTest.kt index e1e93b3..10d0409 100644 --- a/app/src/test/java/org/terst/listen/ReachabilityTest.kt +++ b/app/src/test/java/org/terst/listen/ReachabilityTest.kt @@ -12,8 +12,9 @@ import java.io.File * exception. Tests passing said nothing, because a tested function with no call site is still * dead code. * - * Scoped to `object` declarations on purpose: that is where pure helpers live here, and it is - * the shape where unreachable code hides without a compiler warning. + * Scoped to `object` declarations: a call on one names the object, so a caller can be matched + * exactly. Classes are checked separately and more loosely -- see the class test below for why + * they cannot use this one. */ class ReachabilityTest { @@ -160,6 +161,96 @@ class ReachabilityTest { private val allowedViewModelFunctions = emptySet<String>() + /** + * Methods on classes, which the object check structurally cannot see. + * + * The blind spot cost something real. SleepTimer is a class, and `armForEndOfEpisode`, + * `shouldStopAtEpisodeEnd` and `extendBy` sat written, documented and unit-tested with no + * caller anywhere -- so "stop at the end of this episode" did not exist in the app, and every + * duration in the sleep dialog reset the timer rather than extending it, which is the one + * thing `extendBy`'s own comment says not to do. Tests passing said nothing. + * + * Matched on `.name(` across all of main with declarations stripped, the same way the DAO + * check works and for the same reason: a call on a class names the *instance*, not the class, + * so `player.armSleepTimer(` cannot be found by looking for `PlayerViewModel.armSleepTimer(`. + * That makes this looser than the object check -- two classes with a same-named method alias + * each other -- and still worth having, because the names that hide are the distinctive ones. + * + * `override` members are excluded by [memberFun]: Kotlin requires the keyword, so anything + * reached through an interface is not reported here. + */ + @Test + fun `every class method has a caller`() { + val sources = mainDir.walkTopDown().filter { it.extension == "kt" }.toList() + assertTrue("could not find sources under ${mainDir.absolutePath}", sources.isNotEmpty()) + + val corpus = sources.joinToString("\n") { anyDeclaration.replace(it.readText(), "DECL(") } + + val classDecl = Regex( + """^(?:internal |abstract |sealed |open )*class (\w+)""", + RegexOption.MULTILINE, + ) + + val ownBodies = mutableMapOf<String, String>() + val declared = sources.flatMap { file -> + val body = file.readText() + classDecl.findAll(body).flatMap { declaration -> + val name = declaration.groupValues[1] + val slice = body.substring(declaration.range.first).let { rest -> + val next = nextTopLevel.find(rest, startIndex = 1)?.range?.first ?: rest.length + rest.substring(0, next) + } + ownBodies[name] = anyDeclaration.replace(slice, "DECL(") + memberFun.findAll(slice).map { "$name.${it.groupValues[1]}" } + } + } + + val dead = declared + .distinct() + .filterNot { it in allowedClassMethods } + .filterNot { qualified -> + val bare = qualified.substringAfter(".") + // Three shapes, all of them real in this codebase and all found the hard way. + // `.name(` is the ordinary call on an instance. `::name` is a method reference, + // which has no parens. `name {` is a trailing-lambda call with no parens either, + // which is how OpmlViewModel reaches importFrom and exportTo. + corpus.contains(".$bare(") || corpus.contains("::$bare") || corpus.contains(".$bare {") || + // A method called by its own class's entry point, with no receiver at all -- + // RetentionPolicy.shouldDelete, reached as `filter { shouldDelete(it, now) }`. + ownBodies[qualified.substringBefore(".")] + ?.let { it.contains("$bare(") || it.contains("$bare {") } == true + } + + assertTrue( + "No production caller for: ${dead.sorted()}. Either wire it up, delete it, or add " + + "it to allowedClassMethods with a reason.", + dead.isEmpty(), + ) + } + + /** + * Class methods deliberately not called from production. Every entry needs a reason. + * + * Both entries are the same fact: **on-device detection is dead code.** It moved to the + * server (see CLAUDE.md -- "FingerprintWorker and MidrollWorker are gone from the app"), and + * the workers went while the classes they drove stayed. Nothing constructs either updater; + * every reference to them from outside their own files is a KDoc cross-reference. + * + * That reaches further than these two methods. IntroOutroDetector, AudioCorrelation, + * MidrollScanner, MidrollDetector, MidrollClipStore, MediaCodecClipSource, Decimate, Fft, + * MonoResampler, PcmCodec, StoredClip and BreakTrack are all only reachable through them -- + * roughly two thousand lines of app code and fourteen test files, shipped in every APK. + * SkipSummary and BreakPopup are the only things in `fingerprint/` still live. + * + * Allowlisted rather than deleted because deleting a subsystem is the owner's call, and the + * audio-fingerprint work cost weeks and may yet be wanted back. Recorded here so it cannot + * be lost again: this is the entry to delete when that decision is made. + */ + private val allowedClassMethods = setOf( + "FingerprintUpdater.process", + "MidrollUpdater.processShow", + ) + @Test fun `every helper object function has a caller`() { val sources = mainDir.walkTopDown().filter { it.extension == "kt" }.toList() diff --git a/app/src/test/java/org/terst/listen/download/EpisodeDownloaderTest.kt b/app/src/test/java/org/terst/listen/download/EpisodeDownloaderTest.kt index bbb29af..f615eec 100644 --- a/app/src/test/java/org/terst/listen/download/EpisodeDownloaderTest.kt +++ b/app/src/test/java/org/terst/listen/download/EpisodeDownloaderTest.kt @@ -189,6 +189,9 @@ class EpisodeDownloaderTest { override suspend fun record(episode: EpisodeEntity, info: ServerEpisode) { recorded = episode.id to info } + + var onWaiting: () -> Unit = {} + override suspend fun waiting(episode: EpisodeEntity) = onWaiting() } private fun withServer(audio: ServerAudio?) = @@ -283,6 +286,48 @@ class EpisodeDownloaderTest { assertEquals("nothing should be left in the library", 0, library.listFiles()!!.size) } + /* + A wait asks to be looked at again, rather than waiting for the next refresh. + + The server starts fetching the moment it is asked and its on-demand mirrors complete in + seconds -- 183 of them over the last thirty days. The phone was retrying only on a feed + refresh, which is the 5am alarm, a launch or the button, so an episode the server would have + within the minute read "waiting for server" for hours. That gap, not anything on the server, + is what makes "not checked yet" a state a listener sees. + + Only on a wait. A download that succeeded or failed outright has nothing to come back for. + */ + @Test + fun `an episode left waiting asks for a retry`() = runTest { + var retries = 0 + val ep = episode(url = server.url("/original.mp3").toString()).also { dao.rows += it } + + val audio = FakeServerAudio(pending()).also { it.onWaiting = { retries++ } } + + val result = withServer(audio).download(ep, SubscriptionKind.PODCAST) + + assertWaiting(result) + assertEquals("a wait must schedule a look back", 1, retries) + } + + @Test + fun `a download that worked asks for nothing`() = runTest { + server.enqueue(MockResponse().setBody("server-bytes")) + var retries = 0 + val ready = ServerEpisode( + ready = true, audioUrl = server.url("/audio/abc?token=t").toString(), + durationMs = 1, introMs = null, outroMs = null, detected = true, + ) + val ep = episode(url = "https://publisher.example/original.mp3").also { dao.rows += it } + + val audio = FakeServerAudio(ready).also { it.onWaiting = { retries++ } } + + withServer(audio).download(ep, SubscriptionKind.PODCAST) + + assertEquals("nothing to come back for", 0, retries) + } + + @Test fun `a pending server copy waits and never touches the publisher`() = runTest { val ep = episode(url = server.url("/original.mp3").toString()).also { dao.rows += it } diff --git a/app/src/test/java/org/terst/listen/ui/EpisodeMenuTest.kt b/app/src/test/java/org/terst/listen/ui/EpisodeMenuTest.kt index f81d6d0..4a54a4e 100644 --- a/app/src/test/java/org/terst/listen/ui/EpisodeMenuTest.kt +++ b/app/src/test/java/org/terst/listen/ui/EpisodeMenuTest.kt @@ -7,6 +7,7 @@ import org.junit.Test import org.terst.listen.data.DownloadState import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.EpisodeTranscriptEntity +import org.terst.listen.playback.SleepTimer import org.terst.listen.ui.EpisodeMenuAction.CHAPTERS import org.terst.listen.ui.EpisodeMenuAction.DOWNLOAD import org.terst.listen.ui.EpisodeMenuAction.MARK_OLDER_PLAYED @@ -103,21 +104,30 @@ class EpisodeMenuTest { */ @Test fun `the sleep timer is offered while something is playing`() { - assertTrue(EpisodeMenu.items(episode(), null, sleepRemainingMs = null).map { it.action } + assertTrue(EpisodeMenu.items(episode(), null, sleep = null).map { it.action } .contains(EpisodeMenuAction.SLEEP)) } @Test fun `an unarmed timer offers to start one`() { - val item = EpisodeMenu.items(episode(), null, sleepRemainingMs = null) + val item = EpisodeMenu.items(episode(), null, sleep = null) .single { it.action == EpisodeMenuAction.SLEEP } assertEquals("Sleep timer", item.label) } + /* The mode has no countdown, so the entry says which mode rather than a time. */ + @Test + fun `a timer set for the end of the episode says so`() { + val item = EpisodeMenu.items(episode(), null, sleep = SleepTimer.State.EndOfEpisode) + .single { it.action == EpisodeMenuAction.SLEEP } + + assertEquals("Sleep timer \u00b7 end of episode", item.label) + } + @Test fun `an armed timer says how long is left, so it can be checked without opening anything`() { - val item = EpisodeMenu.items(episode(), null, sleepRemainingMs = 12 * 60_000 + 34_000) + val item = EpisodeMenu.items(episode(), null, sleep = SleepTimer.State.Countdown(12 * 60_000 + 34_000)) .single { it.action == EpisodeMenuAction.SLEEP } assertEquals("Sleep timer \u00b7 12:34", item.label) diff --git a/app/src/test/java/org/terst/listen/ui/FakePlaybackControls.kt b/app/src/test/java/org/terst/listen/ui/FakePlaybackControls.kt index 1a29593..b61deb8 100644 --- a/app/src/test/java/org/terst/listen/ui/FakePlaybackControls.kt +++ b/app/src/test/java/org/terst/listen/ui/FakePlaybackControls.kt @@ -46,6 +46,16 @@ class FakePlaybackControls : PlaybackControls { _nowPlaying.value = _nowPlaying.value.copy(episodeId = episodeId, isPlaying = episodeId != null) } + /** As [playing], with the position and length the end-of-episode sleep mode reads. */ + fun playingAt(episodeId: String?, positionMs: Long, durationMs: Long?) { + _nowPlaying.value = _nowPlaying.value.copy( + episodeId = episodeId, + isPlaying = episodeId != null, + positionMs = positionMs, + durationMs = durationMs, + ) + } + override fun connect() { connects++ } diff --git a/app/src/test/java/org/terst/listen/ui/PlayerViewModelTest.kt b/app/src/test/java/org/terst/listen/ui/PlayerViewModelTest.kt index 844b90f..3195f46 100644 --- a/app/src/test/java/org/terst/listen/ui/PlayerViewModelTest.kt +++ b/app/src/test/java/org/terst/listen/ui/PlayerViewModelTest.kt @@ -15,6 +15,7 @@ import org.terst.listen.data.SubscriptionEntity import org.terst.listen.data.SubscriptionKind import org.terst.listen.playback.BedtimePolicy import org.terst.listen.playback.PlaybackSpeed +import org.terst.listen.playback.SleepTimer /** * The controls that act on whatever is playing. @@ -310,6 +311,76 @@ class PlayerViewModelTest { // --- bedtime mode ------------------------------------------------------------------- + /* + Tapping a duration on a running timer adds to it. + + SleepTimer.extendBy was written for this, documented with the reason -- "+5 minutes on a timer + with 30 seconds to run means five and a half minutes ... a control people reach for half-asleep + without watching the result" -- and then never called. Every button in the dialog went to + armFor, which resets, so 15m on a timer with 30 seconds left gave 15:00 and silently took those + 30 seconds away. + */ + @Test + fun `tapping a duration while a timer runs adds to what is left`() { + vm.armSleepTimer(5) + + vm.armSleepTimer(15) + + val remaining = vm.sleepRemainingMs.value!! + assertTrue( + "15m on top of 5m should be about 20m, got ${remaining / MINUTE}m", + remaining in 19 * MINUTE..20 * MINUTE, + ) + } + + /* Nothing running: the first tap sets the timer rather than adding to zero. */ + @Test + fun `tapping a duration with no timer running sets it`() { + vm.armSleepTimer(15) + + val remaining = vm.sleepRemainingMs.value!! + assertTrue(remaining in 14 * MINUTE..15 * MINUTE) + } + + /* + The second mode the app never offered. + + SleepTimer has supported stopping at the end of the episode since it was written, and nothing + called armForEndOfEpisode or read shouldStopAtEpisodeEnd. ReachabilityTest did not catch it + because it only scans object declarations. + */ + @Test + fun `stopping at the end of the episode is a mode the timer reports`() { + controls.playing("e1") + + vm.armSleepTimerAtEndOfEpisode() + + assertEquals(SleepTimer.State.EndOfEpisode, vm.sleepState.value) + assertNull("there is no countdown to show in this mode", vm.sleepRemainingMs.value) + } + + @Test + fun `the end of the episode stops playback`() { + controls.playingAt("e1", positionMs = 3_600_000, durationMs = 3_600_000) + + vm.armSleepTimerAtEndOfEpisode() + + assertTrue("reaching the end must pause", controls.pauses >= 1) + assertNull(vm.sleepState.value) + } + + /* The queue moving on is the end of the episode, whatever the position said. */ + @Test + fun `the episode being replaced stops playback`() { + controls.playingAt("e1", positionMs = 10_000, durationMs = 3_600_000) + vm.armSleepTimerAtEndOfEpisode() + assertEquals(SleepTimer.State.EndOfEpisode, vm.sleepState.value) + + controls.playingAt("e2", positionMs = 0, durationMs = 3_600_000) + + assertTrue("advancing to another episode must pause", controls.pauses >= 1) + } + @Test fun `entering bedtime mode arms the timer`() { vm.onBedtimeChanged(entering = true) |
