diff --git a/docs/player-parity.md b/docs/player-parity.md index 60e52a7..8cfe4f8 100644 --- a/docs/player-parity.md +++ b/docs/player-parity.md @@ -37,16 +37,18 @@ el?.addEventListener('change', () => sendCommand(device.id, cmd, { level: parseI | player | what the handler reads | result | |---|---|---| | Android | `payload.optDouble("level", -1.0)` | ✅ works | -| web | `data.payload?.value ?? data.value` | 💀 `undefined` → `isFinite` fails → silent no-op | -| Tizen | `payload.value ?? payload.volume` | 💀 `undefined` → logs `no usable value in payload` | -| BrightSign | (the web player) | 💀 as web | +| web | `payload.level` (fraction), `value` still read as a percentage | ✅ **fixed in 1.9.31** | +| Tizen | `payload.level` (fraction) | ✅ fixed in 1.9.31 — but see the note below on when the baseline may move | +| BrightSign | (the web player) | ✅ as web | -Three of the four players have a complete, working volume implementation that cannot be driven, -because nobody checked the payload key against the sender. **Fix: one line in -`server/player/index.html` and one in `tizen/js/app.js` — accept `level` (0..1) as well.** Until -then `audio.volume` has been removed from the `web` and `brightsign` baselines, and -`test/player-parity-baselines.test.js` holds that as a **biconditional**: fix the player and the -test fails, telling you to put the baseline entry back. +Three of the four players had a complete, working volume implementation that could not be driven, +because nobody checked the payload key against the sender. Fixed in 1.9.31: the fraction is now +canonical everywhere, and the scale is chosen by WHICH KEY arrived rather than by the magnitude of +the number (`1` is legal under both conventions, so guessing from the value is wrong for somebody). + +`audio.volume` is back in the `web` and `brightsign` baselines as of 1.9.31, and **not** in the +`tizen` one. That asymmetry is the model, not an oversight — see +[When a baseline may move](#when-a-baseline-may-move). ### 2. Every #161 Tier-2 command was refused for the entire fleet — FIXED here @@ -98,7 +100,7 @@ describe content rendering. They are informational, and shown to the operator in | capability | Android | Web | Tizen | BrightSign | |---|---|---|---|---| | `audio.mute` | ✅ `device:mute-changed` → `setVideoMuted`, incl. YouTube via the IFrame bridge | ✅ | ✅ incl. YouTube via `postMessage` | ✅ as web | -| `audio.volume` | ✅ `set_volume` reads `payload.level` | 💀 reads `payload.value`; dashboard sends `level` | 💀 handler exists (`applyVolume`, incl. `tizen.tvaudiocontrol`) and reads `payload.value` | 💀 as web | +| `audio.volume` | ✅ `set_volume` reads `payload.level` | ✅ reads `payload.level` (1.9.31) | ✅ `applyVolume` reads `payload.level` (1.9.31, incl. `tizen.tvaudiocontrol`) | ✅ as web | ## Display @@ -250,8 +252,37 @@ declares anything, every display reading a baseline is running **v1.9.28 or olde | `android` | **removed `display.power`** | v1.9.28 `MainActivity`: `"screen_on" -> Log.w("no privileged wake path on a non-rooted panel — no-op")`. The ON half is dead on 100% of fielded panels, and one capability renders **both** buttons. | | `android` | **removed `system.reboot`** | `STPolicy.reboot()` requires device owner; off-owner v1.9.28 shows the accessibility power *dialog* — which on the accessibility-enabled panels common in this fleet paints that dialog **over the signage**. Owner provisioning is unreleased (#161 / PR #168 still open), so "device owner AND pre-1.9.29" is effectively an empty set. | | `tizen` | **added `display.power`** | v1.9.28 `app.js` implements both halves with no signing and no panel API: `showScreenOff()` / `clearScreenOff()` + `keepAwake()`. Unlike Android, neither half is privilege-gated. Withholding it hid a working control on every Tizen panel. | -| `web` | **removed `audio.volume`** | v1.9.28 `index.html` contains the string `set_volume` **zero** times, and HEAD's handler reads the wrong payload key. | -| `brightsign` | **removed `audio.volume`, `display.power`, `system.reboot`, `system.restart_player`, `offline.cache`** | All five need a host bridge (`hasHost()`) or a service worker that a Supervisor-built widget refuses. `system.restart_player` is the 2026-07-28 panel-blackout path. `offline.cache` is the documented lie this whole model exists to stop. | +| `web` | **removed `audio.volume`, then RESTORED it in 1.9.31** | Removed when v1.9.28 `index.html` contained the string `set_volume` zero times. Restored once the handler landed — this player is served by the server, so there is no fielded build to lag behind. | +| `brightsign` | **removed `display.power`, `system.reboot`, `system.restart_player`, `offline.cache`** (and `audio.volume`, restored in 1.9.31 with `web`) | All five need a host bridge (`hasHost()`) or a service worker that a Supervisor-built widget refuses. `system.restart_player` is the 2026-07-28 panel-blackout path. `offline.cache` is the documented lie this whole model exists to stop. | + +### When a baseline may move + +A baseline describes what an **un-updated** display can do, so the question "has this shipped?" +has two different answers depending on how the player reaches the screen. + +**Served by the server — `web`, `brightsign`.** The player is a document this server hands out. A +display running against this build *is* running this build's player; there is no such thing as a +browser panel stuck on last release's. So the baseline moves the moment the server ships the fix, +and holding it back hides a control that already works. `test/player-parity-baselines.test.js` +judges these two against the working tree, in **both** directions. + +**Shipped as a device artifact — `android`, `tizen`.** The player is an APK or a `.wgt` sitting on +the panel. Cutting a release puts nothing on any screen; a panel updates when somebody updates it, +and this repo cannot know how many are still back on which build. These are judged against the +**previous release**, and only in the over-claim direction: "the baseline claims it, so the shipped +player had better implement it" is always worth failing on, while "HEAD gained the handler, so add +it to the baseline" is a guess about the fleet, not a fact about it. A panel that HAS updated +declares its own capabilities and never reads the baseline at all. + +The cost of the one-directional rule is that a stale entry can sit here after the artifact really +has reached the fleet. That is a judgement call about panels, so a person makes it in +`server/lib/player-capabilities.js` and records why — which is what the Tizen `audio.volume` note +there is doing right now. + +> This distinction was learned the hard way. The test used to read "shipped" as *the newest tag*, +> which is HEAD on a release commit — so tagging 1.9.31 flipped every biconditional at once and +> demanded a baseline change for displays that could not possibly have the fix yet. The build went +> red naming a baseline, with nothing in the diff to explain it. ### Consequence, deliberately accepted @@ -267,7 +298,7 @@ declares `system.reboot` for itself and is unaffected. |---|---|---|---|---| | `playback.*` (all 7) | ✅ | ✅ | ✅ | ✅ | | `audio.mute` | ✅ | ✅ | ✅ | ✅ | -| `audio.volume` | ✅ | ❌ no handler in v1.9.28 | ❌ payload | ❌ no handler in v1.9.28 | +| `audio.volume` | ✅ | ❌ the fielded `.wgt` has no handler | ✅ since 1.9.31 (runs the served player) | ✅ since 1.9.31 | | `display.rotation` | ✅ | ✅ | ⚠️ graphics only | ✅ | | `display.power` | ❌ `screen_on` is a no-op | ✅ | ❌ needs host | ❌ | | `display.brightness` | ✅ Tier 0, since v1.9.10 | ❌ | ❌ | ❌ | diff --git a/server/lib/player-capabilities.js b/server/lib/player-capabilities.js index 4ed90cd..8fba4a2 100644 --- a/server/lib/player-capabilities.js +++ b/server/lib/player-capabilities.js @@ -167,6 +167,14 @@ const BASELINE = { // gated on it, so the entry describes content rendering rather than offering a button. 'display.rotation', 'remote.input', + // RESTORED in 1.9.31 with BASELINE.web, and for the same reason plus one of its own: a + // BrightSign runs the web player we serve, so it gets the fixed handler the moment the server + // does. The unit-specific question is whether the media element is even reachable on a player + // that puts video on a hardware plane — and that question is already settled by `audio.mute` + // above, which this baseline has always claimed: set_volume reaches setMediaVolume() and + // device:mute-changed reaches `currentVideoEl.muted`, the same element by the same path. If hwz + // silently swallowed one it would swallow both, so volume is exactly as honest as mute here. + 'audio.volume', 'sync.clock', // NOT offline.cache. This is the documented case, not a hypothetical: the XT245 on alpha has // navigator.serviceWorker, passes every presence check, and then never fetches sw.js because @@ -182,9 +190,9 @@ const BASELINE = { // NOT system.reboot / display.power / display.resolution / system.self_update: all four are // BrightScript calls through a bridge this unit is not known to have. // - // NOT audio.volume / remote.screenshot / remote.stream: see BASELINE.web — the volume payload - // never lands, and a canvas capture on a hwz player cannot read the video plane, so it returns - // a frame with a hole where the content is. + // NOT remote.screenshot / remote.stream: a canvas capture on a hwz player cannot read the video + // plane, so it returns a frame with a hole where the content is. (audio.volume moved INTO the + // list above in 1.9.31 — the payload it was waiting on now lands.) ], // A browser tab. Deliberately the smallest set: it cannot reboot its host, rotate a panel, or // capture anything outside its own document. @@ -195,16 +203,22 @@ const BASELINE = { 'display.rotation', 'remote.screenshot', 'remote.stream', 'remote.input', 'system.restart_player', + // RESTORED in 1.9.31, having been removed by the audit that found the slider dead. Both of the + // reasons it was removed have expired, and the second one was reasoning from the wrong artifact: + // 1. It read `data.payload?.value ?? data.value` while the dashboard sends `{ level: 0..1 }`, + // so the number was undefined and the handler declined. Fixed in 1.9.31 — index.html now + // takes the fraction as canonical (volumeLevelFromCommand) and set_volume reaches + // setMediaVolume(). + // 2. The removal cited `git show v1.9.28:server/player/index.html` having no handler at all. + // But this player is SERVED BY THE SERVER: a browser panel loads it from whatever build is + // running, not from the release its row was created under. There is no such thing as a + // browser panel stuck on the v1.9.28 player once the server moves — which is the whole + // difference between this baseline and the Android/Tizen ones below, where an un-updated + // panel really is running an old artifact. + // So the moment the server ships the fix, an undeclared web display can be driven, and holding + // the entry back would hide a control that works. Released and live on prod 2026-08-06. + 'audio.volume', 'sync.clock', 'offline.cache', - // NOT audio.volume, removed after audit, and for two independent reasons: - // 1. `git show v1.9.28:server/player/index.html` has no set_volume handler at all — zero - // occurrences of the string. The fielded browser player ignores the command outright. - // 2. Even at HEAD the slider cannot work: index.html reads `data.payload?.value ?? data.value` - // and tizen/js/app.js reads `payload.value ?? payload.volume`, while the dashboard sends - // `{ level: 0..1 }` (frontend/js/views/device-detail.js bindLevel). Only the Android - // handler reads `level`. Fixing that is one line in each player, and - // test/player-parity-baselines.test.js is written as a BICONDITIONAL: the moment a player - // accepts `level`, the test fails and tells you to put the baseline entry back. ], }; diff --git a/server/test/player-capabilities.test.js b/server/test/player-capabilities.test.js index 98f4492..af90e91 100644 --- a/server/test/player-capabilities.test.js +++ b/server/test/player-capabilities.test.js @@ -123,10 +123,14 @@ test('the baseline describes a FIELDED player, not the one we are about to ship' // working controls disappear from every legacy Tizen display. const tizen = { platform: 'Tizen 6.5' }; assert.equal(caps.supports(tizen, 'audio.volume'), false, 'the slider is dead on a fielded panel'); - // Same answer on web and BrightSign, for a second and separate reason: those players read - // payload.value while the dashboard sends payload.level, so even HEAD's handler never fires. - assert.equal(caps.supports({ android_version: 'Web/Chrome' }, 'audio.volume'), false); - assert.equal(caps.supports({ platform: 'brightsign' }, 'audio.volume'), false); + // Web and BrightSign used to answer false here too, on the grounds that those players read + // payload.value while the dashboard sends payload.level. 1.9.31 fixed the payload AND, more to + // the point, those two players are SERVED BY THIS SERVER — an undeclared browser panel runs + // whatever build is answering it, so there is no fielded-vs-shipping gap to describe. Tizen keeps + // the old answer precisely because it does have one: its .wgt sits on the panel until someone + // updates it, and the fix reaching this repo put nothing on any screen. + assert.ok(caps.supports({ android_version: 'Web/Chrome' }, 'audio.volume'), 'the web player this build serves reads payload.level'); + assert.ok(caps.supports({ platform: 'brightsign' }, 'audio.volume'), 'BrightSign runs that same served player'); assert.ok(caps.supports({ client_type: 'apk' }, 'audio.volume'), 'Android reads the payload it is sent'); assert.ok(caps.supports(tizen, 'audio.mute'), 'mute does work'); assert.ok(caps.supports(tizen, 'remote.screenshot'), 'captureAndSend exists in the shipped player'); diff --git a/server/test/player-parity-baselines.test.js b/server/test/player-parity-baselines.test.js index 3997692..a44a9b4 100644 --- a/server/test/player-parity-baselines.test.js +++ b/server/test/player-parity-baselines.test.js @@ -88,17 +88,57 @@ function readShipped(rel) { const needsShippedSource = (t) => shippedFromTag ? false : (t.skip('no release tag available — cannot judge a baseline against shipped source'), true); +/* + * The two families of player update by completely different physics, and a baseline that ignores + * the difference is wrong for half the fleet: + * + * SERVER-SERVED (web, brightsign) — the player is a document this server hands out. A display + * running against this build IS running this build's player; there is no such thing as a browser + * panel stuck on an old one. So the baseline moves the moment the server ships, and the WORKING + * TREE is the honest source. Anything else hides a control that already works. + * + * DEVICE ARTIFACT (android, tizen) — the player is an APK or a .wgt sitting on the panel. + * Releasing puts nothing on anything; a panel updates when someone updates it, and the repo has + * no way to know how many are still back on which build. Judged against the PREVIOUS release, + * and only in the over-claim direction (see below). + * + * Conflating them is what made this file demand BASELINE.web gain audio.volume at the exact moment + * 1.9.31 was tagged — correct conclusion, arrived at by accident, and it would have dragged Tizen + * along with it onto panels that have no such fix. + */ +const SERVER_SERVED = new Set(['web', 'brightsign']); + const SRC = { - web: readShipped('server/player/index.html'), + web: read('server/player/index.html'), android: [ 'android/app/src/main/java/com/remotedisplay/player/MainActivity.kt', 'android/app/src/main/java/com/remotedisplay/player/service/WebSocketService.kt', 'android/app/src/main/java/com/remotedisplay/player/telemetry/PlayerCapabilities.kt', ].map(read).join('\n'), tizen: ['tizen/js/app.js', 'tizen/js/device-control.js', 'tizen/js/capabilities.js'].map(readShipped).join('\n'), - brightsign: ['brightsign/st-bridge.js', 'brightsign/autorun.brs'].map(readShipped).join('\n'), + brightsign: ['brightsign/st-bridge.js', 'brightsign/autorun.brs'].map(read).join('\n'), }; +/* + * Assert a baseline entry against what its player can actually do. + * + * Server-served: BOTH directions. Over-claiming is a dead button; under-claiming hides a working + * one, and both are decidable right here from the tree we are about to serve. + * + * Device artifact: the OVER-CLAIM direction only. "The baseline claims it, so the shipped player + * had better implement it" is always true and always worth failing on. The reverse — "HEAD gained + * the handler, so put it in the baseline" — is not a fact about the fleet, it is a guess about how + * many panels have updated, and the honest answer for an undeclared panel is the floor. A panel + * that HAS updated declares its own capabilities and never reads the baseline at all. The cost is + * that a stale entry can sit here after the artifact really has reached the fleet; that is a + * judgement call, so it is made by a person in player-capabilities.js and noted there. + */ +function assertBaseline(family, capability, playerCan, { claimedMsg, missingMsg }) { + const claimed = caps.BASELINE[family].includes(capability); + if (claimed) { assert.ok(playerCan, claimedMsg); return; } + if (SERVER_SERVED.has(family)) assert.ok(!playerCan, missingMsg); +} + /* * Which command names each player has a branch for. A command a player never names is a dead * button by construction — the socket delivers it and the handler falls off the end. @@ -213,11 +253,10 @@ test('BICONDITIONAL: audio.volume in a baseline iff that player reads the payloa readsLevel.brightsign = readsLevel.web; // BrightSign runs the web player for (const family of ['android', 'web', 'tizen', 'brightsign']) { - const claimed = caps.BASELINE[family].includes('audio.volume'); - assert.equal(claimed, readsLevel[family], - claimed - ? `BASELINE.${family} claims audio.volume but that player never reads payload.level — the slider is dead` - : `the ${family} player now reads payload.level: restore 'audio.volume' to BASELINE.${family}`); + assertBaseline(family, 'audio.volume', readsLevel[family], { + claimedMsg: `BASELINE.${family} claims audio.volume but that player never reads payload.level — the slider is dead`, + missingMsg: `the ${family} player now reads payload.level: restore 'audio.volume' to BASELINE.${family}`, + }); } }); @@ -232,8 +271,9 @@ test('audio.mute is real on all four, unlike its neighbour', () => { } }); -test('BICONDITIONAL: offline.cache in a baseline iff that player has a media cache', (t) => { - if (needsShippedSource(t)) return; +test('BICONDITIONAL: offline.cache in a baseline iff that player has a media cache', () => { + // No shipped-source guard here: this one settles every family by hand below rather than by + // grepping a tag, so a tagless clone loses nothing by running it. // Tizen's media cache is a NEW file — it was not in v1.9.28 — so the baseline must not claim it // even though HEAD's capabilities.js declares it at runtime. BrightSign's widget is not known to // permit a service worker at all.