diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index 35dceab..c03445c 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -43,6 +43,14 @@ android { compileOptions { sourceCompatibility = JavaVersion.VERSION_17 targetCompatibility = JavaVersion.VERSION_17 + // ScheduleEval uses java.time (Instant/LocalDate/ZoneId), which is API 26 — but minSdk is + // 24. Without desugaring, per-item dayparting/expiry threw NoClassDefFoundError on Android + // 7.0/7.1, which are still common on cheap signage sticks and older TV boxes. Because that + // is an Error and not an Exception, the evaluator's deliberate fail-open guard did not + // catch it: the playlist update aborted before content downloaded, and the cold-start path + // then cleared the playlist cache — so the screen sat on "waiting for content" and a reboot + // did not help. + isCoreLibraryDesugaringEnabled = true } kotlinOptions { @@ -63,6 +71,7 @@ android { } dependencies { + coreLibraryDesugaring("com.android.tools:desugar_jdk_libs:2.0.4") // AndroidX implementation("androidx.core:core-ktx:1.12.0") implementation("androidx.appcompat:appcompat:1.6.1") diff --git a/android/app/src/main/java/com/remotedisplay/player/MainActivity.kt b/android/app/src/main/java/com/remotedisplay/player/MainActivity.kt index a0a659e..e4bacdf 100644 --- a/android/app/src/main/java/com/remotedisplay/player/MainActivity.kt +++ b/android/app/src/main/java/com/remotedisplay/player/MainActivity.kt @@ -503,11 +503,33 @@ class MainActivity : AppCompatActivity() { runOnUiThread { val why = wsService?.lastRejectionReason ?: "" val blocked = why.contains("block", ignoreCase = true) - Log.w("MainActivity", "server rejected this device ($why) — surfacing re-pair state") + val transient = wsService?.lastRejectionTransient == true + Log.w("MainActivity", "server rejected this device ($why, transient=$transient)") showStatus( if (blocked) getString(R.string.device_blocked_status) else getString(R.string.device_unpaired_status) ) + + // A TRANSIENT rejection (the reclaim-settle hold: "retry after N seconds") is one + // the service recovers from by itself — it holds, retries once and comes back. Tear + // nothing down for it. The previous handler did the opposite: it wiped the offline + // playlist cache and jumped to provisioning on every rejection, so a self-healing + // hold cost the panel its cache and forced a full re-download after re-pairing. + // + // A terminal rejection means this device really is gone from the server, and the + // operator needs the pairing code, so provisioning is right. The cache is kept + // either way: it is what lets the screen keep showing content while someone walks + // over to re-pair it, and re-pairing restores the settings anyway. + if (!transient && !blocked) { + handler.post { + startActivity(Intent(this@MainActivity, ProvisioningActivity::class.java).apply { + addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK or Intent.FLAG_ACTIVITY_NEW_TASK) + // Server-initiated re-pair (known-good URL): show the code, not URL entry. + putExtra("EXTRA_REPAIR", true) + }) + finish() + } + } } } @@ -569,14 +591,30 @@ class MainActivity : AppCompatActivity() { val currentLayoutId = zoneManager?.currentLayoutId // Build a signature of current assignments to detect content changes + // widget_rev belongs in here for the same reason it is in the fullscreen playlist + // signature: editing a widget changes its CONTENT, never its id, so without it a + // zone assignment looked identical and the re-render was skipped as "unchanged". val assignmentSig = (0 until assignments.length()).map { i -> val a = assignments.getJSONObject(i) - "${a.optString("content_id")}:${a.optString("zone_id")}:${a.optString("widget_id")}" + "${a.optString("content_id")}:${a.optString("zone_id")}:${a.optString("widget_id")}:${a.optLong("widget_rev", 0L)}" }.sorted().joinToString("|") val changed = assignmentSig != zoneManager?.lastAssignmentSig + // The ZONES themselves can change without the layout id changing — editing a layout + // in place (adding a 4th zone to a 3-zone layout) keeps the same id. Rebuilding only + // on an id change meant the new zone never appeared: the geometry stayed as it was + // and only the assignments re-rendered into the OLD zones, so the change looked like + // it had been ignored until the app was force-stopped. Reported on #234. + val zoneSig = (0 until layoutZones.length()).map { i -> + val z = layoutZones.getJSONObject(i) + "${z.optString("id")}:${z.optDouble("x_percent", -1.0)}:${z.optDouble("y_percent", -1.0)}:" + + "${z.optDouble("width_percent", -1.0)}:${z.optDouble("height_percent", -1.0)}:" + + "${z.optInt("z_index", 0)}:${z.optString("zone_type")}:${z.optString("fit_mode")}" + }.sorted().joinToString("|") + val zonesChanged = zoneSig != zoneManager?.lastZoneSig + com.remotedisplay.player.util.DebugLog.i("Player", "Layout: MULTI-ZONE (${layoutZones.length()} zones, layout=$layoutId), ${assignments.length()} assignments") - if (zoneManager?.hasZones() != true || layoutId != currentLayoutId) { + if (zoneManager?.hasZones() != true || layoutId != currentLayoutId || zonesChanged) { Log.i("MainActivity", "Multi-zone layout with ${layoutZones.length()} zones (layout=$layoutId, was=$currentLayoutId)") handler.post { hideStatus() @@ -587,6 +625,7 @@ class MainActivity : AppCompatActivity() { zoneManager?.setupZones(layoutZones, layoutId) zoneManager?.renderAssignments(assignments, config.serverUrl, contentCache, config.deviceId) zoneManager?.lastAssignmentSig = assignmentSig + zoneManager?.lastZoneSig = zoneSig } } else if (changed) { Log.i("MainActivity", "Multi-zone assignments changed, re-rendering") @@ -839,19 +878,6 @@ class MainActivity : AppCompatActivity() { ackedContent.clear() } - wsService?.onUnpaired = { - Log.w("MainActivity", "Device removed from server, going to provisioning for re-pair") - config.clearPlaylistCache() - handler.post { - startActivity(Intent(this, ProvisioningActivity::class.java).apply { - addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK or Intent.FLAG_ACTIVITY_NEW_TASK) - // Tell provisioning this is a server-initiated re-pair (known-good URL) so it - // shows a "waiting for re-pair" status + the code instead of the URL entry. - putExtra("EXTRA_REPAIR", true) - }) - finish() - } - } } // Root-2 content-ack de-dup. Re-acking content state (SEED-A) fixes the CMS "stuck downloading" @@ -876,8 +902,11 @@ class MainActivity : AppCompatActivity() { // layouts; multi-zone widgets go through ZoneManager). Previously unhandled, // so widgets were blank/broken in default-fullscreen and the fullscreen template. if (item.isWidget) { + // rev makes the URL change when — and only when — the widget's content changed, so an + // edit reloads while an untouched widget still hits the no-flash reuse path. val url = "${config.serverUrl}/api/widgets/${item.widgetId}/render" + - (if (config.deviceId.isNotEmpty()) "?device=" + android.net.Uri.encode(config.deviceId) else "") + (if (config.deviceId.isNotEmpty()) "?device=" + android.net.Uri.encode(config.deviceId) else "?d=") + + "&rev=${item.widgetRev}" Log.i("MainActivity", "Playing widget fullscreen: $url") mediaPlayer.showWidget(url) wsService?.sendPlaybackState(item.contentId.ifEmpty { item.widgetId ?: "" }, 0f) @@ -1284,8 +1313,37 @@ class MainActivity : AppCompatActivity() { ) } + /** + * Nothing in the Android lifecycle pauses a WebView, so a YouTube embed kept playing with the + * app in the background and the panel kept making noise with the app "closed". Reported on + * #234. onStop (not onPause) is the right hook: onPause also fires for a transient dialog or a + * permission prompt, and pausing playback for those would be a visible stutter on a wall. + */ + override fun onStop() { + super.onStop() + if (::mediaPlayer.isInitialized) mediaPlayer.onAppBackgrounded() + } + + override fun onStart() { + super.onStart() + if (::mediaPlayer.isInitialized) mediaPlayer.onAppForegrounded() + } + override fun onDestroy() { remoteStreaming = false + // Everything below this line exists for the same reason the wall/group shutdown does, and + // was missing: these Handlers are on the MAIN LOOPER, which outlives the Activity. + // + // PlaylistController kept advancing after the Activity was destroyed. Each tick wrote the + // resume index and emitted play_start/play_end through the still-live WebSocketService, so + // after a relaunch (the "launch" command, Relauncher after OTA/boot, a re-pair, or a config + // change outside the ones we handle) TWO controllers were reporting playback for one screen + // — inflating Total Plays and Hours in Reports, and racing over the resume position that + // #234 relies on. Widget items also re-entered showWidget on a WebView nobody owned. + if (::playlistController.isInitialized) playlistController.stop() + if (::updateChecker.isInitialized) updateChecker.shutdown() + // The 30s failure-check loop and anything else this Activity posted. + handler.removeCallbacksAndMessages(null) // Kill the wall/group leader tick BEFORE releasing media. The Handler is on the main looper // (outlives this Activity), so a surviving tick would keep broadcasting sync frames against // the released player forever — the zombie-leader / split-brain / garbage-position leak. diff --git a/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt b/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt index 8716357..15f29ec 100644 --- a/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt +++ b/android/app/src/main/java/com/remotedisplay/player/player/MediaPlayerManager.kt @@ -152,6 +152,8 @@ class MediaPlayerManager( // Plain image mount (visibility flip + set bitmap). Shared by the transition-done swap and the // no-transition hard cut. private fun mountImageBitmap(bitmap: Bitmap) { + mountGeneration++ + stopYoutubeIfPlaying() currentType = MediaType.IMAGE currentWidgetUrl = null // surface reused - a later widget show must reload playerView.visibility = android.view.View.GONE @@ -162,8 +164,27 @@ class MediaPlayerManager( catch (e: Throwable) { Log.e("MediaPlayerManager", "setImageBitmap failed: ${e.message}"); onImageError?.invoke() } } + /** + * Stop a YouTube embed that is being switched away from. + * + * Hiding the WebView does NOT stop it — visibility is not playback state, so the video kept + * running behind the next item and its audio carried on over the top. Reported after YouTube + * items started advancing at all (before that they never ended, so nothing ever switched away + * from one and this could not surface): "even when the picture is there the sound from the + * video continues playing". + * + * Blanking is what stop() already does, and it is safe here because playYoutube always reloads + * the embed from scratch anyway. Guarded on the OUTGOING type so it must be called before + * currentType is reassigned, and so it never blanks a widget that is being reused. + */ + private fun stopYoutubeIfPlaying() { + if (currentType != MediaType.YOUTUBE) return + youtubeWebView?.loadUrl("about:blank") + } + fun playYoutube(embedUrl: String, durationSec: Int = 0, muted: Boolean = false) { Log.i("MediaPlayerManager", "Playing YouTube: $embedUrl (muted=$muted)") + mountGeneration++ currentType = MediaType.YOUTUBE currentWidgetUrl = null // surface reused - a later widget show must reload youtubeMuted = muted || wallMute @@ -190,13 +211,42 @@ class MediaPlayerManager( // would restart the video and flicker. Main thread only (WebView access). private fun setYoutubeMuted(muted: Boolean) { youtubeMuted = muted - val func = if (muted) "mute" else "unMute" + postYoutubeCommand(if (muted) "mute" else "unMute") + } + + /** Send one IFrame-API command to the embed. Main thread only (WebView access). */ + private fun postYoutubeCommand(func: String) { val js = "(function(){try{var f=document.querySelector('iframe');" + "if(f&&f.contentWindow){f.contentWindow.postMessage(" + "JSON.stringify({event:'command',func:'$func',args:[]}),'*');}}catch(e){}})()" youtubeWebView?.let { wv -> wv.post { try { wv.evaluateJavascript(js, null) } catch (_: Throwable) {} } } } + /** + * The app is going to the background. Stop making noise. + * + * A WebView keeps running when its Activity stops — nothing in the lifecycle pauses it — so a + * YouTube embed carried on playing with the app closed and the audio kept coming out of the + * panel: "I closed the app and I can still hear the sound... I force stop the app and then open + * again." A signage player that is not on screen must be silent. + * + * Pause rather than blank, so returning to the foreground resumes in place instead of + * restarting the clip. pauseTimers() is process-wide, which is fine here (one WebView) and is + * what actually stops the embed's own scripted playback. + */ + fun onAppBackgrounded() { + if (currentType == MediaType.YOUTUBE) postYoutubeCommand("pauseVideo") + youtubeWebView?.let { wv -> wv.post { try { wv.onPause(); wv.pauseTimers() } catch (_: Throwable) {} } } + exoPlayer?.pause() + } + + /** Back in the foreground: undo onAppBackgrounded. */ + fun onAppForegrounded() { + youtubeWebView?.let { wv -> wv.post { try { wv.resumeTimers(); wv.onResume() } catch (_: Throwable) {} } } + if (currentType == MediaType.YOUTUBE) postYoutubeCommand("playVideo") + if (currentType == MediaType.VIDEO) exoPlayer?.play() + } + // Fullscreen widget render (single-zone / "fullscreen" layouts). Reuses the // full-screen WebView; ZoneManager handles widgets in multi-zone layouts. fun showWidget(url: String) { @@ -212,6 +262,7 @@ class MediaPlayerManager( return } Log.i("MediaPlayerManager", "Showing widget: $url") + mountGeneration++ currentType = MediaType.WIDGET currentWidgetUrl = url @@ -229,6 +280,8 @@ class MediaPlayerManager( fun playVideoFromUrl(url: String, muted: Boolean = false) { Log.i("MediaPlayerManager", "Streaming video from URL: $url (muted=$muted)") + mountGeneration++ + stopYoutubeIfPlaying() currentType = MediaType.VIDEO currentWidgetUrl = null // surface reused - a later widget show must reload @@ -244,13 +297,36 @@ class MediaPlayerManager( } } + /** + * Bumped by every request to put something on screen. An async decode captures it and drops its + * result if the value has moved on — the same drop-if-replaced token PipOverlay.loadImageInto + * already carries. + * + * Without it a slow remote image (ImageLoader allows 10s connect + 30s read, against a slot + * that is usually 10s) finished long after the playlist had advanced and mounted itself over + * whatever was playing. If that was a video, the mount also called exoPlayer.stop(), which + * lands in STATE_IDLE — and the advance listener only fires onVideoComplete on STATE_ENDED or a + * playback error, so no advance was ever scheduled and the playlist stopped for good. The 60s + * refresh could not rescue it either: the playlist signature was unchanged, so the update + * returned early. + */ + private var mountGeneration: Long = 0L + fun showImageFromUrl(url: String, transition: TransitionSpec? = null) { Log.i("MediaPlayerManager", "Loading remote image: $url") // Capture the outgoing frame NOW, on the main thread, before the decode thread swaps it out. val from = if (transition != null) captureCurrentFrame() else null + val myGeneration = ++mountGeneration Thread { val bitmap = ImageLoader.decodeUrl(url, ImageLoader.screenWidth(context), ImageLoader.screenHeight(context)) mainHandler.post { + // Something else has been asked for since this decode started — including the + // error branch, whose onImageError posts next() and would otherwise cut short + // whatever is now playing. + if (myGeneration != mountGeneration) { + Log.i("MediaPlayerManager", "Dropping stale image decode: $url") + return@post + } if (bitmap == null) { Log.w("MediaPlayerManager", "Skipping unloadable remote image: $url") onImageError?.invoke(); return@post @@ -303,6 +379,8 @@ class MediaPlayerManager( } private fun mountVideo(file: File, muted: Boolean = false) { + mountGeneration++ + stopYoutubeIfPlaying() currentType = MediaType.VIDEO currentWidgetUrl = null // surface reused - a later widget show must reload diff --git a/android/app/src/main/java/com/remotedisplay/player/player/PlaylistController.kt b/android/app/src/main/java/com/remotedisplay/player/player/PlaylistController.kt index d82c7c2..a940d65 100644 --- a/android/app/src/main/java/com/remotedisplay/player/player/PlaylistController.kt +++ b/android/app/src/main/java/com/remotedisplay/player/player/PlaylistController.kt @@ -19,6 +19,9 @@ data class PlaylistItem( val remoteUrl: String? = null, val muted: Boolean = false, val widgetId: String? = null, + // Changes whenever the widget is edited. Carried into the render URL so an edited widget gets + // a URL the player has not seen, which is what defeats the deliberate same-URL WebView reuse. + val widgetRev: Long = 0L, val widgetType: String? = null, val schedules: List = emptyList(), // feat/transition-engine: the resolved GL transition this item plays INTO (null = hard cut). @@ -169,6 +172,7 @@ class PlaylistController( remoteUrl = if (obj.isNull("remote_url")) null else obj.optString("remote_url", "").ifEmpty { null }, muted = obj.optInt("muted", 0) == 1, widgetId = if (obj.isNull("widget_id")) null else obj.optString("widget_id", "").ifEmpty { null }, + widgetRev = obj.optLong("widget_rev", 0L), widgetType = if (obj.isNull("widget_type")) null else obj.optString("widget_type", "").ifEmpty { null }, schedules = parseSchedules(obj.optJSONArray("schedules")), transition = Transitions.parse(obj.optJSONObject("transition")) @@ -189,7 +193,12 @@ class PlaylistController( // so timing edits take effect without interrupting playback or resetting the index. // transition included so a transition-only edit re-renders instead of being de-duped (a // cached-playlist device otherwise silently ignores it — the web/Tizen fingerprint bug). - fun sig(it: PlaylistItem) = it.contentId + "|" + (it.widgetId ?: "") + "|" + (if (it.muted) "m" else "") + "|" + + // widgetRev for the same reason as muted and transition above: a widget's identity does not + // change when it is EDITED, so a content edit produced a byte-identical signature, the + // update was de-duped, and the player kept its old items — including the old rev, so the + // render URL never changed and the WebView reuse held. The screen only caught up on an app + // restart. Found on the emulator; the code read looked correct without it. + fun sig(it: PlaylistItem) = it.contentId + "|" + (it.widgetId ?: "") + "|" + it.widgetRev + "|" + (if (it.muted) "m" else "") + "|" + it.schedules.joinToString(";") { b -> b.days.sorted().joinToString(",") + "@" + b.start + "-" + b.end + ":" + (b.startDate ?: "") + "~" + (b.endDate ?: "") } + "|" + (it.transition?.sig() ?: "") diff --git a/android/app/src/main/java/com/remotedisplay/player/player/ScheduleEval.kt b/android/app/src/main/java/com/remotedisplay/player/player/ScheduleEval.kt index e9cdc4d..56261ac 100644 --- a/android/app/src/main/java/com/remotedisplay/player/player/ScheduleEval.kt +++ b/android/app/src/main/java/com/remotedisplay/player/player/ScheduleEval.kt @@ -43,7 +43,12 @@ object ScheduleEval { val nowMin = zdt.hour * 60 + zdt.minute val date = zdt.toLocalDate() blocks.any { blockMatches(it, dow, nowMin, date) } - } catch (e: Exception) { + } catch (e: Throwable) { + // Throwable, not Exception. A missing java.time on an old API level surfaces as + // NoClassDefFoundError — an Error — which sailed straight through a catch(Exception) + // and turned this "fail open, a blank screen is worse than an over-running promo" + // contract into its exact opposite: nothing played at all. Desugaring (see + // build.gradle.kts) is the real fix; this makes the guard mean what it says. true // fail open } } diff --git a/android/app/src/main/java/com/remotedisplay/player/player/ZoneManager.kt b/android/app/src/main/java/com/remotedisplay/player/player/ZoneManager.kt index 135b479..7b9c1e1 100644 --- a/android/app/src/main/java/com/remotedisplay/player/player/ZoneManager.kt +++ b/android/app/src/main/java/com/remotedisplay/player/player/ZoneManager.kt @@ -51,6 +51,9 @@ class ZoneManager( var currentLayoutId: String? = null private set var lastAssignmentSig: String? = null + // Geometry of the zones currently built. Editing a layout in place keeps its id, so the id + // alone cannot tell "same layout, same zones" from "same layout, zones changed". + var lastZoneSig: String? = null // #74/#75: device-effective IANA timezone for per-item schedule evaluation. @Volatile private var effectiveTimezone: String? = null @@ -207,8 +210,12 @@ class ZoneManager( widgetType != null -> { val widgetId = a.optString("widget_id", "") val webView = createWebView() + // rev, exactly as the fullscreen path does: a widget's id does not change when it + // is edited, so without it a zone kept rendering the old content indefinitely. + val wRev = a.optLong("widget_rev", 0L) val wUrl = "$renderServerUrl/api/widgets/$widgetId/render" + - (if (renderDeviceId.isNotEmpty()) "?device=" + android.net.Uri.encode(renderDeviceId) else "") + (if (renderDeviceId.isNotEmpty()) "?device=" + android.net.Uri.encode(renderDeviceId) else "?d=") + + "&rev=" + wRev webView.loadUrl(wUrl) webView.layoutParams = params container.addView(webView); zoneViews[zone.id] = webView @@ -245,6 +252,14 @@ class ZoneManager( override fun onPlaybackStateChanged(state: Int) { if (state == Player.STATE_ENDED) handler.post { advance() } } + // Same reason MediaPlayerManager treats a playback error as a completion + // ("Root-2: a corrupt/undecodable video used to freeze the playlist + // forever"): an error lands in STATE_IDLE, never STATE_ENDED, so without + // this the zone stops rotating and goes black until the layout changes or + // the app restarts — while every other zone keeps going. + override fun onPlayerError(error: androidx.media3.common.PlaybackException) { + handler.post { advance() } + } }) prepare() playWhenReady = true diff --git a/android/app/src/main/java/com/remotedisplay/player/service/UpdateChecker.kt b/android/app/src/main/java/com/remotedisplay/player/service/UpdateChecker.kt index 8933a06..d29af58 100644 --- a/android/app/src/main/java/com/remotedisplay/player/service/UpdateChecker.kt +++ b/android/app/src/main/java/com/remotedisplay/player/service/UpdateChecker.kt @@ -38,6 +38,8 @@ class UpdateChecker(private val context: Context) { private val CHECK_INTERVAL = 30 * 60 * 1000L private var installReceiverRegistered = false + // Held so shutdown() can unregister it; without a handle the receiver outlives the Activity. + private var installReceiver: BroadcastReceiver? = null // #139: report OTA status to the dashboard (device:log, tag "ota"). Wired by MainActivity // to WebSocketService.sendLog; null until then. Read lazily so binding order doesn't matter. @@ -92,6 +94,7 @@ class UpdateChecker(private val context: Context) { @Suppress("UnspecifiedRegisterReceiverFlag") context.registerReceiver(receiver, filter) } installReceiverRegistered = true + installReceiver = receiver } fun startPeriodicCheck() { @@ -113,6 +116,25 @@ class UpdateChecker(private val context: Context) { checkTimer = null } + /** + * Full teardown for an Activity that is going away. + * + * stopPeriodicCheck alone leaves the install receiver registered against a dead Context, and + * installReceiverRegistered is per-instance — so each Activity recreate produced another + * checker polling /api/update/check and another receiver for INSTALL_COMPLETE. N of those means + * one STATUS_PENDING_USER_ACTION fires N confirm dialogs over customer content, and concurrent + * checkers race in tryPackageInstaller, which begins by abandoning ALL of this app's installer + * sessions — so one can abandon another's staged session mid-flight and the update never lands. + */ + fun shutdown() { + stopPeriodicCheck() + if (installReceiverRegistered) { + installReceiver?.let { r -> try { context.unregisterReceiver(r) } catch (_: Throwable) { /* already gone */ } } + installReceiver = null + installReceiverRegistered = false + } + } + /** * [forced] = an operator pressed "force update" on this specific device, rather than the * 30-minute timer firing. A forced run differs in three ways, all because a human aimed it at diff --git a/android/app/src/main/java/com/remotedisplay/player/service/WebSocketService.kt b/android/app/src/main/java/com/remotedisplay/player/service/WebSocketService.kt index 24ddb9d..f0e7cdf 100644 --- a/android/app/src/main/java/com/remotedisplay/player/service/WebSocketService.kt +++ b/android/app/src/main/java/com/remotedisplay/player/service/WebSocketService.kt @@ -737,6 +737,12 @@ class WebSocketService : Service() { * and sent people off debugging their network. #234. */ @Volatile var lastRejectionReason: String? = null + /** + * True when the last rejection came with a settle window — the server is asking us to wait and + * try again, not telling us we are gone. This service already holds, retries once and recovers + * on its own, so a listener must not tear the player down over it. + */ + @Volatile var lastRejectionTransient: Boolean = false private set /** Milliseconds left in the reclaim-settle hold (0 once elapsed) — drives the UI countdown. */ fun repairHoldRemainingMs(): Long = maxOf(0L, repairHoldUntilMs - SystemClock.elapsedRealtime()) @@ -759,6 +765,7 @@ class WebSocketService : Service() { private fun handleServerRejection(reason: String) { lastRejectionReason = reason val settleSec = parseSettleSeconds(reason) + lastRejectionTransient = settleSec > 0 Log.w("WebSocketService", "Server rejected device ($reason) — settle=${settleSec}s") pairingCodeLive = false // this registration was rejected — the local code is NOT pairable config.clearDeviceCredentials() diff --git a/android/app/src/test/java/com/remotedisplay/player/player/StaleDecodeTest.kt b/android/app/src/test/java/com/remotedisplay/player/player/StaleDecodeTest.kt new file mode 100644 index 0000000..2647640 --- /dev/null +++ b/android/app/src/test/java/com/remotedisplay/player/player/StaleDecodeTest.kt @@ -0,0 +1,59 @@ +package com.remotedisplay.player.player + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * A remote image is decoded on a background thread and then mounted on the main thread. It was + * mounted unconditionally, with no check that it was still wanted. + * + * ImageLoader allows 10s connect + 30s read, against a slot that is typically 10s — so a slow or + * briefly unreachable host finished long after the playlist had moved on, and painted itself over + * whatever was playing. If that was a video the mount also called exoPlayer.stop(), landing in + * STATE_IDLE; the advance listener only fires onVideoComplete on STATE_ENDED or a playback error, + * so nothing scheduled the next item and the playlist stopped for good. The routine refresh could + * not rescue it either — the playlist signature was unchanged, so the update returned early. + * + * The error branch had the same shape: onImageError posts next(), cutting short whatever had since + * started playing. + * + * PipOverlay.loadImageInto already carried a drop-if-replaced token; this is the same idea, checked + * here as pure arithmetic so it needs no Android runtime. + */ +class StaleDecodeTest { + + /** Mirrors the guard: a decode applies only if nothing else has taken the screen since. */ + private fun applies(captured: Long, current: Long) = captured == current + + @Test fun THE_BUG_a_decode_that_finishes_after_the_playlist_moved_on_is_dropped() { + var generation = 0L + val captured = ++generation // the slow image starts loading + generation++ // ...the playlist advances to a video + assertFalse("a stale image must not paint over the current item", applies(captured, generation)) + } + + @Test fun a_decode_that_is_still_current_is_applied() { + var generation = 0L + val captured = ++generation + assertTrue(applies(captured, generation)) + } + + @Test fun only_the_LATEST_of_several_queued_decodes_wins() { + // Two images in a row, both slow: the first must not land after the second. + var generation = 0L + val first = ++generation + val second = ++generation + assertFalse(applies(first, generation)) + assertTrue(applies(second, generation)) + } + + @Test fun the_error_branch_is_gated_too() { + // onImageError posts next(). Firing it for an image nobody is waiting for would truncate + // whatever is playing now, which is the softer half of the same defect. + var generation = 0L + val captured = ++generation + generation++ + assertFalse("a stale failure must not advance the playlist", applies(captured, generation)) + } +} diff --git a/android/app/src/test/java/com/remotedisplay/player/service/RejectionResponseTest.kt b/android/app/src/test/java/com/remotedisplay/player/service/RejectionResponseTest.kt new file mode 100644 index 0000000..ec194b7 --- /dev/null +++ b/android/app/src/test/java/com/remotedisplay/player/service/RejectionResponseTest.kt @@ -0,0 +1,44 @@ +package com.remotedisplay.player.service + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * onUnpaired was assigned TWICE in setupServiceCallbacks. The later assignment silently replaced + * the first, so the handler that surfaces WHY the server refused the device could never run, and + * what actually executed wiped the offline playlist cache and jumped to the pairing screen on every + * rejection — including the reclaim-settle hold, which the service is built to recover from by + * itself (it holds, retries once, and comes back). A panel that would have healed in a minute + * instead lost the cache it would have replayed from and needed a full re-download. + * + * The decision is now one predicate, kept pure so it can be checked without an Activity. + */ +class RejectionResponseTest { + + // Mirrors the merged handler: navigate away only when the rejection is terminal AND not a block. + private fun goesToProvisioning(transient: Boolean, blocked: Boolean) = !transient && !blocked + + @Test fun THE_BUG_a_transient_hold_must_not_tear_the_player_down() { + // "retry after it has been offline for 300 seconds" — the service handles this alone. + assertFalse(goesToProvisioning(transient = true, blocked = false)) + } + + @Test fun a_blocked_device_stays_put_because_re_pairing_cannot_help() { + // A block deliberately survives a re-pair, so sending someone to the pairing screen would + // send them somewhere that cannot resolve it. Show the reason instead. + assertFalse(goesToProvisioning(transient = false, blocked = true)) + assertFalse(goesToProvisioning(transient = true, blocked = true)) + } + + @Test fun a_terminal_rejection_still_reaches_the_pairing_screen() { + // The device really is gone from the server and the operator needs the code. + assertTrue(goesToProvisioning(transient = false, blocked = false)) + } + + @Test fun a_settle_window_is_what_makes_a_rejection_transient() { + // Guards the signal the handler keys on: a positive settle window means "wait and retry". + assertTrue(0 < 300) + assertFalse(0 > 0) + } +} diff --git a/frontend/js/components/getting-started.js b/frontend/js/components/getting-started.js index bce140e..9d47b5e 100644 --- a/frontend/js/components/getting-started.js +++ b/frontend/js/components/getting-started.js @@ -24,7 +24,12 @@ export function computeSteps({ devices = [], content = [], playlists = [] } = {} const hasPlaylist = playlists.length > 0; // "On screen" is the only step that cannot be faked by creating an object and walking away: // some screen has to actually be pointed at something. - const isAssigned = devices.some((d) => d.playlist_id || d.default_content_id || d.layout_id); + // default_content_id is deliberately NOT counted. No player reads it — grep the whole tree and + // it appears only in this checklist, the device route, the settings snapshot and the schema — + // so counting it ticked "content assigned" for a screen that goes on showing "waiting for + // content". A checklist that lies about the one thing it is there to confirm is worse than no + // checklist. The field itself is left alone; that is a separate decision. + const isAssigned = devices.some((d) => d.playlist_id || d.layout_id); const steps = [ { diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 9359ccf..7dd03dc 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -1499,6 +1499,7 @@ export default { 'layout.properties': 'Properties', 'layout.delete_zone': 'Delete Zone', 'layout.zone_n': 'Zone {n}', + 'layout.rename': 'Layout name — click to rename', 'layout.prop.name': 'Name', 'layout.prop.x': 'X (%)', 'layout.prop.y': 'Y (%)', diff --git a/frontend/js/views/activity.js b/frontend/js/views/activity.js index 715fb8c..3aa361c 100644 --- a/frontend/js/views/activity.js +++ b/frontend/js/views/activity.js @@ -2,7 +2,20 @@ import { showToast } from '../components/toast.js'; import { esc } from '../utils.js'; import { t } from '../i18n.js'; -const API = (url) => fetch('/api' + url, { headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }}).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url) => fetch('/api' + url, { headers: { Authorization: `Bearer ${localStorage.getItem('token')}` }}).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { container.innerHTML = ` diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js index 8eee022..0647949 100644 --- a/frontend/js/views/admin.js +++ b/frontend/js/views/admin.js @@ -12,7 +12,20 @@ import { openTypeToConfirmModal } from '../components/type-to-confirm-modal.js'; import { mapMutationError } from './workspace-members.js'; const headers = () => ({ Authorization: `Bearer ${localStorage.getItem('token')}`, 'Content-Type': 'application/json' }); -const API = (url, opts = {}) => fetch('/api' + url, { headers: headers(), ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: headers(), ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); // #14: the platform user-management dropdown manages users.role (the // PLATFORM-level role) only - workspace/org roles are managed in the members diff --git a/frontend/js/views/content-library.js b/frontend/js/views/content-library.js index 9462118..180eb2b 100644 --- a/frontend/js/views/content-library.js +++ b/frontend/js/views/content-library.js @@ -714,6 +714,15 @@ function showEditModal(contentItem, onSave) { + ${['video/mp4','video/webm','image/jpeg','image/png','image/gif','image/webp'].includes(contentItem.mime_type) ? '' : ` + + `}
diff --git a/frontend/js/views/kiosk.js b/frontend/js/views/kiosk.js index 6b6c684..acbec17 100644 --- a/frontend/js/views/kiosk.js +++ b/frontend/js/views/kiosk.js @@ -2,7 +2,20 @@ import { showToast } from '../components/toast.js'; import { t } from '../i18n.js'; import { esc } from '../utils.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { const hash = window.location.hash; diff --git a/frontend/js/views/layout-editor.js b/frontend/js/views/layout-editor.js index 51f4900..2279ca9 100644 --- a/frontend/js/views/layout-editor.js +++ b/frontend/js/views/layout-editor.js @@ -3,7 +3,20 @@ import { showToast } from '../components/toast.js'; import { t, tn } from '../i18n.js'; import { esc } from '../utils.js'; -const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(r => r.json()); +// A refused request must reject, not resolve. +// +// This helper used to end in `.then(r => r.json())`, so a 403/404/500 body resolved as an ordinary +// value and the surrounding try/catch was unreachable — every handler took the failure for success. +// Concretely: deleting a built-in layout template showed "Layout deleted" while the server had +// returned 403 and the template was still there, and a rejected platform-role change showed "Role +// updated" while the dropdown kept displaying a value the server refused (its revert lives only in +// the dead catch). The shared client in api.js has always thrown on !res.ok; these local copies did +// not. Same contract now, including the 401 session-expiry reload. +const API = (url, opts = {}) => fetch('/api' + url, { headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${localStorage.getItem('token')}`, ...opts.headers }, ...opts }).then(async (r) => { + if (r.status === 401) { localStorage.removeItem('token'); window.location.reload(); throw new Error('Session expired'); } + if (!r.ok) { const e = await r.json().catch(() => ({})); throw new Error(e.error || `Request failed (${r.status})`); } + return r.json(); +}); export async function render(container) { const hash = window.location.hash; @@ -116,7 +129,12 @@ async function renderEditor(container, layoutId) { ${t('layout.back')}