summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-10-20 23:55:00 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-10-20 23:55:00 +0000
commit8a344b9779e535b293f414dedd5c8a7947004a58 (patch)
treec59587ded7534a32bdba5cbaf9e066335f63524d
parent37136b152e923f784ad4056f9deddfab1d7f1e8c (diff)
Jump 25 seconds, the forward button's amount, not 45
The "this is an ad" press jumped 45 seconds, a number argued from ad-unit arithmetic rather than from the controls that already exist. Peter: "the jump should stay 25 seconds. Don't go inventing new timing." Skipping forward is already what you do to an advertisement, so the button goes the same distance and additionally says why. One number to know, and a second press goes the same distance again. The amount had three definitions already -- the player's increments, the episode screen's transport buttons, and the notification caption's hardcoded "Forward 25 seconds" -- and this nearly became a fourth with a different value. They now all read one, and the captions are derived from it so a caption cannot name a number its button does not do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
-rw-r--r--.coverage-baseline2
-rw-r--r--app/src/main/java/org/terst/listen/playback/AdJump.kt17
-rw-r--r--app/src/main/java/org/terst/listen/playback/NotificationControls.kt7
-rw-r--r--app/src/main/java/org/terst/listen/playback/PlaybackService.kt10
-rw-r--r--app/src/main/java/org/terst/listen/playback/SeekAmounts.kt24
-rw-r--r--app/src/main/java/org/terst/listen/ui/Transport.kt8
-rw-r--r--app/src/test/java/org/terst/listen/playback/AdJumpTest.kt4
-rw-r--r--app/src/test/java/org/terst/listen/playback/SeekAmountsTest.kt37
-rw-r--r--docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md22
9 files changed, 102 insertions, 29 deletions
diff --git a/.coverage-baseline b/.coverage-baseline
index e4cbd3e..db882fc 100644
--- a/.coverage-baseline
+++ b/.coverage-baseline
@@ -1 +1 @@
-94.42
+94.43
diff --git a/app/src/main/java/org/terst/listen/playback/AdJump.kt b/app/src/main/java/org/terst/listen/playback/AdJump.kt
index 99622c1..baf09ed 100644
--- a/app/src/main/java/org/terst/listen/playback/AdJump.kt
+++ b/app/src/main/java/org/terst/listen/playback/AdJump.kt
@@ -7,19 +7,18 @@ import org.terst.listen.data.EpisodeMidrollEntity
* How far "this is an ad" jumps.
*
* The press means "get me past this advertisement", so the jump is the best estimate available
- * at that moment rather than a constant. If detection already found a break here, its end is a
- * measured answer and is used. Otherwise the default stands in.
+ * at that moment: if detection already found a break here, its end is a measured answer and is
+ * used.
*
- * The size of that default is the trade between too small -- several presses to clear one pod --
- * and too large, which lands him in the programme and makes him correct constantly. Advertising
- * is sold in roughly 15-second units grouped into pods, and he presses partway through one, so
- * the typical *remaining* duration is what this estimates. Erring small is deliberate: a second
- * press costs one tap and improves the label, while over-shooting costs him content.
+ * With nothing measured it falls back to the forward button's own amount, and deliberately
+ * introduces no timing of its own. Skipping forward is already what the listener does to an
+ * advertisement; this button does the same distance and additionally says why. One number to
+ * know, and pressing again goes the same distance again -- which also extends the label.
*/
object AdJump {
- /** Three 15-second units. See the class comment for why it is not larger. */
- const val DEFAULT_JUMP_MS = 45_000L
+ /** The forward button's amount. Not a constant of this feature's own; see [SeekAmounts]. */
+ const val DEFAULT_JUMP_MS = SeekAmounts.FORWARD_MS
fun targetFor(
ranges: List<EpisodeMidrollEntity>,
diff --git a/app/src/main/java/org/terst/listen/playback/NotificationControls.kt b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt
index 21f3959..1dba3df 100644
--- a/app/src/main/java/org/terst/listen/playback/NotificationControls.kt
+++ b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt
@@ -41,17 +41,18 @@ object NotificationControls {
* Media3 ships numbered icons for 5, 10, 15 and 30 seconds only.
*
* Back is 15, so it gets the numbered icon. Forward is 25 and there is no 25 icon, so it
- * gets the plain arrow rather than one captioned with a number the button doesn't do.
+ * gets the plain arrow rather than one captioned with a number the button doesn't do. The
+ * captions come from [SeekAmounts] for the same reason.
*/
fun mediaButtonPreferences(): List<CommandButton> = listOf(
CommandButton.Builder()
- .setDisplayName("Back 15 seconds")
+ .setDisplayName(SeekAmounts.backLabel)
.setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_back_15)
.setSessionCommand(seekBackCommand)
.setSlots(CommandButton.SLOT_BACK)
.build(),
CommandButton.Builder()
- .setDisplayName("Forward 25 seconds")
+ .setDisplayName(SeekAmounts.forwardLabel)
.setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_forward)
.setSessionCommand(seekForwardCommand)
.setSlots(CommandButton.SLOT_FORWARD)
diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
index 3a2543b..7af4e0a 100644
--- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
+++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
@@ -66,10 +66,10 @@ class PlaybackService : MediaLibraryService() {
/* handleAudioFocus = */ true,
)
.setHandleAudioBecomingNoisy(true)
- // These drive the notification, lock screen, Bluetooth remote and car controls.
- // Asymmetric on purpose: back catches a missed sentence, forward steps over an ad.
- .setSeekBackIncrementMs(SEEK_BACK_MS)
- .setSeekForwardIncrementMs(SEEK_FORWARD_MS)
+ // These drive the notification, lock screen, Bluetooth remote and car controls --
+ // and, through SeekAmounts, the in-app transport and the button captions too.
+ .setSeekBackIncrementMs(SeekAmounts.BACK_MS)
+ .setSeekForwardIncrementMs(SeekAmounts.FORWARD_MS)
.build()
player = exo
@@ -345,8 +345,6 @@ class PlaybackService : MediaLibraryService() {
private companion object {
const val TAG = "ListenPlayback"
const val POSITION_TICK_MS = 2_000L
- const val SEEK_BACK_MS = 15_000L
- const val SEEK_FORWARD_MS = 25_000L
}
}
diff --git a/app/src/main/java/org/terst/listen/playback/SeekAmounts.kt b/app/src/main/java/org/terst/listen/playback/SeekAmounts.kt
new file mode 100644
index 0000000..a02a56b
--- /dev/null
+++ b/app/src/main/java/org/terst/listen/playback/SeekAmounts.kt
@@ -0,0 +1,24 @@
+package org.terst.listen.playback
+
+/**
+ * How far a skip goes. One definition, because there were three.
+ *
+ * The player's increments, the episode screen's transport buttons and the notification button's
+ * caption each carried their own copy of 15 and 25 seconds, and the "this is an ad" jump nearly
+ * became a fourth with a different number again. A listener has one idea of how far a skip goes;
+ * every control that skips uses this.
+ *
+ * Asymmetric on purpose: back catches a missed sentence, forward steps over an advertisement.
+ */
+object SeekAmounts {
+
+ const val BACK_MS = 15_000L
+ const val FORWARD_MS = 25_000L
+
+ /**
+ * Captions, derived rather than written out, so a caption cannot name a number the button
+ * does not do.
+ */
+ val backLabel: String = "Back ${BACK_MS / 1_000} seconds"
+ val forwardLabel: String = "Forward ${FORWARD_MS / 1_000} seconds"
+}
diff --git a/app/src/main/java/org/terst/listen/ui/Transport.kt b/app/src/main/java/org/terst/listen/ui/Transport.kt
index 80c1018..506770d 100644
--- a/app/src/main/java/org/terst/listen/ui/Transport.kt
+++ b/app/src/main/java/org/terst/listen/ui/Transport.kt
@@ -1,10 +1,14 @@
package org.terst.listen.ui
+import org.terst.listen.playback.SeekAmounts
+
/** The episode screen's transport arithmetic, kept out of the composable so it can be tested. */
object Transport {
- const val BACK_MS = 15_000L
- const val FORWARD_MS = 25_000L
+ // The player's own increments, not a second opinion about them: the screen's buttons and
+ // the notification's have to move the playhead by the same amount.
+ const val BACK_MS = SeekAmounts.BACK_MS
+ const val FORWARD_MS = SeekAmounts.FORWARD_MS
/** Where a skip lands, kept inside the episode. */
fun stepped(positionMs: Long, deltaMs: Long, durationMs: Long?): Long {
diff --git a/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt b/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt
index 3421aa2..d2b3138 100644
--- a/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt
+++ b/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt
@@ -87,7 +87,7 @@ class AdJumpTest {
}
@Test
- fun `the default is a plausible remaining advertisement, in whole ad units`() {
- assertEquals(0L, AdJump.DEFAULT_JUMP_MS % 15_000L)
+ fun `the default jump is the forward button's amount, not a number of its own`() {
+ assertEquals(SeekAmounts.FORWARD_MS, AdJump.DEFAULT_JUMP_MS)
}
}
diff --git a/app/src/test/java/org/terst/listen/playback/SeekAmountsTest.kt b/app/src/test/java/org/terst/listen/playback/SeekAmountsTest.kt
new file mode 100644
index 0000000..8d0865c
--- /dev/null
+++ b/app/src/test/java/org/terst/listen/playback/SeekAmountsTest.kt
@@ -0,0 +1,37 @@
+package org.terst.listen.playback
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * One definition of how far a skip goes.
+ *
+ * These were duplicated in three places -- the player's increments, the episode screen's
+ * transport, and the notification button's caption -- and a fourth nearly appeared.
+ */
+class SeekAmountsTest {
+
+ @Test
+ fun `the amounts are what they have always been`() {
+ assertEquals(15_000L, SeekAmounts.BACK_MS)
+ assertEquals(25_000L, SeekAmounts.FORWARD_MS)
+ }
+
+ @Test
+ fun `the episode screen skips by the same amounts as the player`() {
+ assertEquals(SeekAmounts.BACK_MS, org.terst.listen.ui.Transport.BACK_MS)
+ assertEquals(SeekAmounts.FORWARD_MS, org.terst.listen.ui.Transport.FORWARD_MS)
+ }
+
+ @Test
+ fun `a notification caption states the amount it actually seeks`() {
+ assertEquals("Back 15 seconds", SeekAmounts.backLabel)
+ assertEquals("Forward 25 seconds", SeekAmounts.forwardLabel)
+ }
+
+ @Test
+ fun `back is shorter than forward -- back catches a missed sentence, forward steps over an ad`() {
+ assertTrue(SeekAmounts.BACK_MS < SeekAmounts.FORWARD_MS)
+ }
+}
diff --git a/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md b/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md
index cd79f1e..7ec6c52 100644
--- a/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md
+++ b/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md
@@ -91,8 +91,11 @@ window. Headphone/Bluetooth transport keys still seek: `KEYCODE_MEDIA_NEXT` +25s
**Android Auto is NOT verified** — it needs the real car. The risk is low (player commands,
browse tree and custom actions are all unchanged or strictly added to) but it is not zero.
-**What the default jump is.** 45s, three 15-second units, when nothing better is known; the end
-of the detected break when the press lands inside one. See `AdJump`.
+**What the default jump is.** The forward button's own 25s when nothing better is known; the end
+of the detected break when the press lands inside one. Peter, 2026-10-20, on an earlier 45s:
+*"the jump should stay 25 seconds. Don't go inventing new timing."* The amount now has one
+definition (`SeekAmounts`) that the player's increments, the episode screen's transport and the
+notification captions all read, so there is no second place for a number to be invented.
**Why this button matters more than the popup.** They catch opposite errors:
@@ -129,7 +132,7 @@ The two back cases are told apart by *where he lands*, not by guessing: inside t
a correction to the end, before the original press is a rejection. Both are unambiguous from the
playhead alone.
-**How far to jump is an ergonomics question, and probably not a constant.**
+**How far to jump — settled, after two wrong answers.**
An earlier draft of this plan argued the jump must be a fixed size and deliberately unequal to the
25s forward-seek, so that a back-press would be "unambiguous". Peter challenged it and it does not
@@ -142,9 +145,16 @@ hold, in either half:
the correction is read from **where he lands** against a span the system recorded exactly. That
works for any jump size; he never needs to know the number.
-What actually sets the size is the trade between **too small** (several presses to clear one pod)
-and **too large** (routinely landing in content, so he corrects constantly). It should sit near the
-typical *remaining* advertisement duration from a mid-advertisement press.
+**Settled: it is the forward button's 25s, and nothing new.** The first build shipped 45s, argued
+from ad-unit arithmetic. Peter, 2026-10-20: *"the jump should stay 25 seconds. Don't go inventing
+new timing."* Skipping forward is already what he does to an advertisement; this button goes the
+same distance and additionally says why, so there is one number to know rather than two, and a
+second press goes the same distance again.
+
+That is twice this one question has been got wrong by reasoning about it from first principles
+instead of from the controls that already exist. The amount now has a single definition
+(`SeekAmounts`) that the player's increments, the episode screen's transport and the notification
+captions all read, so there is no second place for a number to be invented.
And it need not be fixed: if a detected break is nearby, jump to its end — that position is already
known. If the episode's declared-versus-served gap says minutes remain, that is informative too.