Stop re-registering the device once per playlist item

PlaylistController.next() asks for a playlist refresh on every item advance, and
requestPlaylistRefresh() emits a full device:register. The server's register handler
runs 7+ statements plus the identity/fingerprint path and rebuilds the playlist
payload, then pushes the whole playlist back down. So a panel showing a 10-second
image re-registered six times a minute, indefinitely, and each reply fed a fresh
playlist into a controller that had to diff it — which is what kept the #234 restart
loop supplied.

It was buying nothing. The heartbeat already refreshes every 4th beat (60s), so the
periodic pull this duplicated happens either way.

Throttled at the single chokepoint rather than by editing callers, because the callers
have genuinely different intents — network-came-back, service-connected, per-item, and
the heartbeat itself — and ranking them would be guesswork. A shared floor keeps every
caller's meaning: recovery paths still refresh, they just cannot stack. The window sits
just under the heartbeat's own 60s so the two interleave instead of the throttle
systematically eating the pull we are relying on.

Measured on the reproduction over 240s: 9 registrations for 9 item plays before, 3 for
the same 9 plays after, with playback unchanged. The saving scales with how short the
items are — a 10s item goes from six refreshes a minute to about one.

Does NOT change what a refresh does, only how often one may be asked for.
This commit is contained in:
ScreenTinker 2026-07-29 14:13:42 -05:00
parent 3a681abda0
commit bc00bc1eb1
3 changed files with 101 additions and 0 deletions

View file

@ -0,0 +1,28 @@
package com.remotedisplay.player.service
/**
* #234 follow-up: how often a device may ask the server to re-send its playlist.
*
* "Refresh" is not cheap. requestPlaylistRefresh() emits a full `device:register`, and the server's
* register handler runs 7+ statements plus the whole fingerprint/identity path and rebuilds the
* playlist payload. PlaylistController.next() was calling it on EVERY item advance, so a panel on a
* 10-second image re-registered six times a minute, forever and each reply pushed a full playlist
* back down, which is what kept feeding the restart loop behind #234.
*
* It was also redundant: the heartbeat already refreshes every 4th beat (60s), so the periodic pull
* this was duplicating exists either way. Throttling at the single chokepoint keeps every caller's
* intent recovery paths still refresh, they just cannot stack up without having to rank them.
*
* Pure so the interval arithmetic is testable without a device or a socket.
*/
object RefreshThrottle {
/** Just under the heartbeat's own 60s pull, so the two interleave instead of cancelling out. */
const val MIN_INTERVAL_MS = 55_000L
fun shouldRefresh(lastAtMs: Long, nowMs: Long): Boolean {
if (lastAtMs <= 0L) return true // never refreshed — always allow the first
val since = nowMs - lastAtMs
if (since < 0L) return true // clock corrected backwards; never wedge on it
return since >= MIN_INTERVAL_MS
}
}

View file

@ -845,8 +845,17 @@ class WebSocketService : Service() {
connect()
}
@Volatile private var lastRefreshAt = 0L
fun requestPlaylistRefresh() {
if (socket?.connected() != true || config.deviceId.isEmpty()) return
// #234 follow-up: this emits a FULL device:register (7+ server statements + the identity
// path + a playlist rebuild), and PlaylistController.next() calls it on every item advance.
// A 10-second image therefore re-registered six times a minute. The heartbeat already pulls
// a fresh playlist every 60s, so the per-item call bought nothing and cost a great deal.
val now = System.currentTimeMillis()
if (!RefreshThrottle.shouldRefresh(lastRefreshAt, now)) return
lastRefreshAt = now
Log.i("WebSocketService", "Requesting playlist refresh")
try {
val data = org.json.JSONObject().apply {

View file

@ -0,0 +1,64 @@
package com.remotedisplay.player.service
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
/**
* #234 follow-up: a playlist refresh emits a full device:register, and PlaylistController.next()
* asked for one on EVERY item advance. On a 10-second image that is six full re-registrations a
* minute, per device, forever each running 7+ server statements and the identity path, and each
* replying with a full playlist push. Measured on the reproduction: 9 item plays, 9 registrations.
*
* The heartbeat already pulls a fresh playlist every 60s, so the per-item call was duplicating a
* refresh that happens anyway.
*/
class RefreshThrottleTest {
private val NOW = 5_000_000L
private val MIN = RefreshThrottle.MIN_INTERVAL_MS
@Test fun the_very_first_refresh_always_goes_through() {
// A device that has never asked must not be held back by an empty timestamp.
assertTrue(RefreshThrottle.shouldRefresh(lastAtMs = 0L, nowMs = NOW))
}
@Test fun THE_BUG_a_second_refresh_moments_later_is_suppressed() {
// Two item advances a few seconds apart: the second must not re-register.
assertFalse(RefreshThrottle.shouldRefresh(NOW - 3_000, NOW))
assertFalse(RefreshThrottle.shouldRefresh(NOW - 10_000, NOW))
}
@Test fun once_the_interval_has_passed_it_refreshes_again() {
assertTrue(RefreshThrottle.shouldRefresh(NOW - MIN, NOW))
assertTrue(RefreshThrottle.shouldRefresh(NOW - (MIN + 1), NOW))
}
@Test fun just_under_the_interval_is_still_suppressed() {
assertFalse(RefreshThrottle.shouldRefresh(NOW - (MIN - 1), NOW))
}
@Test fun it_sits_under_the_heartbeat_pull_so_the_two_interleave() {
// The heartbeat refreshes every 60s. A window at or above that would systematically
// suppress the heartbeat's own pull, which is the one we are relying on to remain.
assertTrue(MIN < 60_000L)
}
@Test fun a_backwards_clock_never_wedges_refreshing() {
// Signage panels correct their clocks. A future 'last' must not disable refresh until the
// clock catches up — that would strand a device on a stale playlist for hours.
assertTrue(RefreshThrottle.shouldRefresh(NOW + 3_600_000, NOW))
}
@Test fun a_ten_second_item_collapses_from_six_refreshes_a_minute_to_about_one() {
// Walk a minute of 10s items and count what actually gets through.
var last = 0L
var allowed = 0
var t = NOW
repeat(6) {
if (RefreshThrottle.shouldRefresh(last, t)) { allowed++; last = t }
t += 10_000
}
assertTrue("expected roughly one refresh per minute, got $allowed", allowed <= 2)
}
}