From 7fb94fbf70745ee4a3f737ca63f0a8545254c3fe Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Tue, 4 Aug 2026 21:27:53 -0500 Subject: [PATCH] Correct the BrightSign port against the dev-cookbook examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed autorun.brs and st-bridge.js line-by-line against the real examples instead of the prose docs. Five defects, three of which would have been silent. The registry API is asynchronous and section-oriented: read(section, key) returns a Promise and writes take an object, write(section, {k: v}). The bridge treated both as synchronous, so deviceId() returned a Promise object — truthy and non-empty — and a panel would have registered as "[object Promise]" while its real row sat unclaimed. It now prefetches into a cache behind onReady(), and connect() waits for that before registering. brightsign_js_objects_enabled: true is required alongside nodejs_enabled for require("@brightsign/*"). Without it the bridge degrades to no-ops and the player loses identity and restart delegation — which would have read as "BrightSign doesn't work" rather than as one missing flag. storage_path is a directory name, not a volume, and storage_quota is a string; the local fallback URL needs its volume (file:/SD:/offline.html). Added security_params and hwz_default to match the examples. SyncManager does not work unless networking/ptp_domain is "0", which needs a reboot to apply. Done only when this player is configured for native sync, and read-before-write so it reboots once rather than on every boot. Confirmed correct as written: messageport, the roHtmlWidgetEvent loop, and RebootSystem(). The notes also state a widget URL may be an externally hosted page with the same JS API access — the favourable answer to the question the original probe was built to ask. Bridge tests now model the async section-oriented registry, so a synchronous stand-in can never hide this class of bug again. 931 pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- brightsign/README.md | 29 ++++++- brightsign/autorun.brs | 41 +++++++++- brightsign/st-bridge.js | 113 ++++++++++++++++++++++---- server/player/index.html | 20 +++++ server/test/brightsign-bridge.test.js | 53 ++++++++++-- 5 files changed, 232 insertions(+), 24 deletions(-) diff --git a/brightsign/README.md b/brightsign/README.md index 72023d3..492accf 100644 --- a/brightsign/README.md +++ b/brightsign/README.md @@ -132,7 +132,34 @@ Stated plainly so nobody reads this as finished: - **Registry from a remote origin is still unproven** — the original probe question. If injection turns out to be origin-dependent, identity moves to a local shim page that owns the registry and passes it to the hosted player in an iframe via `postMessage`. -- **Nothing here has run on hardware.** It is written against the BrightDeveloper docs. +- **Nothing here has run on hardware.** It is written against the BrightDeveloper docs and + checked line-by-line against the `brightsign/dev-cookbook` examples, which corrected four + config keys, the registry API and a hard SyncManager requirement (see below). + +## Verified against the dev-cookbook + +`autorun.brs` and `st-bridge.js` were reviewed against the real examples rather than the prose: + +- **`brightsign_js_objects_enabled: true` is required** alongside `nodejs_enabled` for + `require("@brightsign/*")` (`syncmanager-js/autorun.brs`). Without it the bridge degrades to + no-ops and the player silently loses identity *and* restart delegation — the failure would look + like "BrightSign just doesn't work" rather than a missing flag. +- **`storage_path` is a directory name** (`"/cache"`), not a volume, and **`storage_quota` is a + string** (`indexeddb-caching/autorun.brs`). +- **`security_params: { websecurity: true }`** and `hwz_default: "on"` are the shapes the examples + use; local URLs carry the volume (`file:/SD:/index.html`). +- **The registry API is asynchronous and section-oriented**: `read(section, key)` returns a + **Promise** and writes take an object — `write(section, {k: v})`. The bridge prefetches into a + cache and exposes `onReady()`; the player waits for it before its first connect, because + registering early would pair the panel as a new display and strand its real row. +- **SyncManager needs `networking/ptp_domain = "0"`, applied by a reboot** + (`syncmanager-js/autorun.brs`). Done only when this player is configured for native sync, and + read-before-write so it reboots at most once rather than every boot. +- Confirmed correct as written: `@brightsign/messageport` (`new`, `addEventListener('bsmessage')`, + `PostBSMessage`), the `roHtmlWidgetEvent` loop, and `RebootSystem()`. +- The notes state a widget URL may be **"an externally hosted page"** with the same access to the + BrightSign JS APIs, which is the answer the original probe was built to get — still worth + confirming on hardware, but the documented answer is the favourable one. ## Model notes diff --git a/brightsign/autorun.brs b/brightsign/autorun.brs index 0082e0b..fea1d46 100644 --- a/brightsign/autorun.brs +++ b/brightsign/autorun.brs @@ -82,10 +82,15 @@ End Function Function MakeWidget(url As String, rect As Object, port As Object, cfg As Object) As Object config = { url: url - nodejs_enabled: true ' REQUIRED for require("@brightsign/*") + nodejs_enabled: true ' Node runtime inside the widget + brightsign_js_objects_enabled: true ' REQUIRED for require("@brightsign/*") — without + ' this the bridge silently degrades to no-ops and + ' the player loses identity AND restart delegation javascript_enabled: true - storage_path: "SD:/" - storage_quota: 1073741824 ' 1GB — service-worker cache for offline playback + security_params: { websecurity: true } + hwz_default: "on" ' hardware z-order — video on its own plane + storage_path: "/cache" ' DIRECTORY NAME for the local storage cache + storage_quota: "1073741824" ' 1GB, as a STRING — service-worker offline cache port: port mouse_enabled: false } @@ -95,6 +100,26 @@ Function MakeWidget(url As String, rect As Object, port As Object, cfg As Object return w End Function +' SyncManager will not work unless the PTP domain is set, and applying it needs a reboot. Done +' ONLY when this player is actually configured for native sync — a reboot on every boot would be +' a boot loop, and a player using our own protocol has no use for it. +' +' The read-before-write is what makes it safe: it reboots at most once, on the first boot after +' the mode is selected, and is a no-op forever after. +Sub EnsurePtpDomain(cfg As Object) + if cfg.sync_backend <> "brightsign" then return + + regSec = CreateObject("roRegistrySection", "networking") + if regSec.Read("ptp_domain") = "0" then + print "[st] ptp_domain already 0" + else + print "[st] setting ptp_domain=0 for SyncManager — rebooting once to apply" + regSec.Write("ptp_domain", "0") + regSec.Flush() + RebootSystem() + end if +End Sub + Function FullScreenRect() As Object vm = CreateObject("roVideoMode") return CreateObject("roRectangle", 0, 0, vm.GetResX(), vm.GetResY()) @@ -104,6 +129,14 @@ End Function Sub Main() cfg = LoadConfig() + + ' Crash dumps land here if the widget ever falls over — cheap, and the only forensic trail + ' available on a panel nobody can reach. + dir = CreateDirectory("SD:/brightsign-dumps") + + ' Must happen BEFORE the widget starts: it can reboot. + EnsurePtpDomain(cfg) + port = CreateObject("roMessagePort") ' Second output. XC2055/XC4055 and XT245/XT1145/XT2145 expose more than one HDMI connector; @@ -154,7 +187,7 @@ Sub Main() print "[st] load-error ("; retries; "): "; data.url sleep(ChooseBackoff(retries)) if retries >= 3 then - widget = RebuildWidget(widget, "file:///offline.html", rect, port, cfg) + widget = RebuildWidget(widget, "file:/SD:/offline.html", rect, port, cfg) else widget = RebuildWidget(widget, PlayerUrl(cfg, 1), rect, port, cfg) end if diff --git a/brightsign/st-bridge.js b/brightsign/st-bridge.js index 026c1ca..55f7496 100644 --- a/brightsign/st-bridge.js +++ b/brightsign/st-bridge.js @@ -89,17 +89,81 @@ return s > 1 ? name + '_s' + s : name; } - function regRead(name, fallback) { - if (!registry) return fallback; - try { - var v = registry.read('screentinker', key(name)); - return (v === undefined || v === null || v === '') ? fallback : v; - } catch (e) { return fallback; } + /* + * The registry API is ASYNCHRONOUS and section-oriented: + * registry.read(section, key) -> Promise + * registry.write(section, {k: v}) -> Promise + * (per @brightsign/registry in the dev-cookbook enable-ldws example and the trace-event docs). + * + * The player needs identity synchronously during boot, so the values are prefetched once into + * a cache and every accessor reads the cache. Callers wait on whenReady() before trusting it. + * Both shapes are tolerated — a Promise or a bare value — so a firmware that returns + * synchronously still works rather than caching a Promise object as if it were a device id, + * which would register a "[object Promise]" display. + */ + var SECTION = 'screentinker'; + var CACHED_KEYS = ['device_id', 'server_url', 'sync_backend']; + var cache = {}; + var ready = false; + var readyWaiters = []; + + function markReady() { + if (ready) return; + ready = true; + var waiters = readyWaiters; + readyWaiters = []; + for (var i = 0; i < waiters.length; i++) { + try { waiters[i](); } catch (e) { /* one bad waiter must not block the rest */ } + } } - function regWrite(name, value) { + function normalise(v) { + return (v === undefined || v === null || v === '') ? null : String(v); + } + + function prefetch() { + if (!registry) { markReady(); return; } + var pending = CACHED_KEYS.length; + var settle = function () { if (--pending <= 0) markReady(); }; + + for (var i = 0; i < CACHED_KEYS.length; i++) { + (function (name) { + var result; + try { result = registry.read(SECTION, key(name)); } catch (e) { settle(); return; } + if (result && typeof result.then === 'function') { + result.then( + function (v) { cache[name] = normalise(v); settle(); }, + function () { settle(); } + ); + } else { + cache[name] = normalise(result); + settle(); + } + })(CACHED_KEYS[i]); + } + } + + function regGet(name, fallback) { + var v = cache[name]; + return (v === undefined || v === null) ? fallback : v; + } + + /* values: { device_id: 'x', ... } using UNPREFIXED names; the screen suffix is applied here. */ + function regSet(values) { + var payload = {}; + for (var name in values) { + if (!Object.prototype.hasOwnProperty.call(values, name)) continue; + var v = values[name]; + payload[key(name)] = v === null || v === undefined ? '' : String(v); + cache[name] = normalise(v); + } if (!registry) return false; - try { registry.write('screentinker', key(name), String(value)); return true; } catch (e) { return false; } + try { + var r = registry.write(SECTION, payload); + // A rejected write must not surface as an unhandled rejection on a signage player. + if (r && typeof r.catch === 'function') r.catch(function () {}); + return true; + } catch (e) { return false; } } var deviceInfo = null; @@ -169,15 +233,17 @@ * then the URL, then localStorage for the browser case. */ deviceId: function () { - var v = regRead('device_id', null) || qs('device_id'); + var v = regGet('device_id', null) || qs('device_id'); if (v) return v; try { return global.localStorage.getItem('st_device_id'); } catch (e) { return null; } }, /* Called once pairing completes, so a reboot comes back as the same display. */ setIdentity: function (deviceId, serverUrl) { - if (deviceId) regWrite('device_id', deviceId); - if (serverUrl) regWrite('server_url', serverUrl); + var values = {}; + if (deviceId) values.device_id = deviceId; + if (serverUrl) values.server_url = serverUrl; + regSet(values); post({ type: 'identity', device_id: deviceId || null, server_url: serverUrl || null }); }, @@ -187,7 +253,7 @@ * the same identity on its next boot — a reset that resets nothing. */ clearIdentity: function () { - regWrite('device_id', ''); + regSet({ device_id: '' }); return post({ type: 'identity', clear: true }); }, @@ -210,15 +276,29 @@ * 'brightsign' — native BrightWall; the host drives it over the bridge. */ syncBackend: function () { - return qs('sync_backend') || regRead('sync_backend', 'auto'); + return qs('sync_backend') || regGet('sync_backend', 'auto'); }, setSyncBackend: function (backend) { if (!backend) return false; - regWrite('sync_backend', backend); + regSet({ sync_backend: backend }); return post({ type: 'set-sync-backend', backend: backend }); }, + /* + * Identity readiness. The registry is async, so a caller that registers with the server + * before this resolves would pair as a NEW display and leave a duplicate row behind. The + * callback always runs — on success, on failure, or off-platform — so nothing can hang the + * player waiting for hardware that isn't there. + */ + isReady: function () { return ready; }, + + onReady: function (fn) { + if (typeof fn !== 'function') return; + if (ready) { try { fn(); } catch (e) { /* ignore */ } return; } + readyWaiters.push(fn); + }, + setVideoMode: function (mode) { if (VideoOutputClass) { try { @@ -246,5 +326,10 @@ global.ScreenTinkerBS = API; + // Kick the registry prefetch immediately, and never let a silent module hold boot: the player + // stops waiting after this and carries on with whatever identity it has. + prefetch(); + if (global.setTimeout) global.setTimeout(markReady, 5000); + if (API.hasHost()) API.startHeartbeat(); })(typeof window !== 'undefined' ? window : this); diff --git a/server/player/index.html b/server/player/index.html index 4c941be..160fa84 100644 --- a/server/player/index.html +++ b/server/player/index.html @@ -1151,6 +1151,26 @@ // ==================== Socket Connection ==================== function connect(serverUrl) { + // BrightSign's registry is ASYNCHRONOUS (registry.read returns a Promise), so on a cold + // boot the identity is not in hand yet when this first runs. Registering before it lands + // would pair the panel as a NEW display and strand its real row — so wait once, then adopt + // whatever the registry knows and carry on. onReady always fires (success, failure, or a + // 5s cap inside the bridge), so this can defer boot but never block it. + if (BS && !BS.isReady()) { + BS.onReady(() => { + try { + const known = BS.deviceId(); + if (known && !config.deviceId) { + config.deviceId = known; + config.paired = true; + console.log('[bs] adopted identity from registry:', known); + } + } catch (e) { /* carry on unpaired rather than not at all */ } + connect(serverUrl); + }); + return; + } + if (socket) { socket.disconnect(); socket = null; } socket = io(serverUrl + '/device', { diff --git a/server/test/brightsign-bridge.test.js b/server/test/brightsign-bridge.test.js index fd098bb..ac281e0 100644 --- a/server/test/brightsign-bridge.test.js +++ b/server/test/brightsign-bridge.test.js @@ -22,15 +22,18 @@ const path = require('node:path'); const SRC = fs.readFileSync(path.join(__dirname, '..', '..', 'brightsign', 'st-bridge.js'), 'utf8'); /** Load the bridge into a fake window. `mods` present => pretend we are on a BrightSign. */ -function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150' } = {}) { +function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150', seed = {} } = {}) { const posted = []; - const registryStore = new Map(); + const registryStore = new Map(Object.entries(seed)); const sandbox = { console: { log() {}, warn() {}, error() {} }, navigator: { userAgent: ua }, location: { search, reload() { sandbox.__reloaded = true; } }, setInterval: () => 1, + setTimeout: (fn, ms) => setTimeout(fn, ms), + Promise, + Object, Date, RegExp, parseInt, @@ -55,10 +58,17 @@ function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150' } = {}) }; } if (name === '@brightsign/registry') { + // The real API is async and section-oriented: + // read(section, key) -> Promise; write(section, {k: v}) -> Promise + // Modelling that exactly is the point of this fake — a synchronous stand-in would have + // hidden the bug where the bridge cached a Promise object as the device id. return function () { return { - read: (section, key) => registryStore.get(section + ':' + key), - write: (section, key, value) => registryStore.set(section + ':' + key, value), + read: (section, k) => Promise.resolve(registryStore.get(section + ':' + k)), + write: (section, values) => { + Object.keys(values).forEach((k) => registryStore.set(section + ':' + k, values[k])); + return Promise.resolve(); + }, }; }; } @@ -73,7 +83,10 @@ function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150' } = {}) vm.createContext(sandbox); vm.runInContext(SRC, sandbox); - return { api: sandbox.ScreenTinkerBS, sandbox, posted, registryStore }; + const api = sandbox.ScreenTinkerBS; + // onReady always fires, so this resolves off-platform too. + const ready = new Promise((resolve) => api.onReady(resolve)); + return { api, sandbox, posted, registryStore, ready }; } test('in a plain browser it loads without throwing and reports not-BrightSign', () => { @@ -172,3 +185,33 @@ test('sync backend comes from the URL, else the registry, else auto', () => { persisted.api.setSyncBackend('screentinker'); assert.equal(persisted.api.syncBackend(), 'screentinker', 'a cold boot with no network still starts right'); }); + +test('THE ASYNC TRAP: a Promise from registry.read is never cached as the device id', async () => { + // registry.read() resolves a Promise. Treating it as a value would make deviceId() return the + // Promise object itself — truthy, non-empty — and the player would register a display called + // "[object Promise]" while its real row sat unclaimed. + const { api, ready } = load({ mods: true, seed: { 'screentinker:device_id': 'existing-id' } }); + await ready; + assert.equal(typeof api.deviceId(), 'string'); + assert.equal(api.deviceId(), 'existing-id', 'a provisioned panel must come back as itself'); +}); + +test('a panel with nothing in the registry becomes ready with no identity, not a stuck one', async () => { + const { api, ready } = load({ mods: true }); + await ready; + assert.equal(api.isReady(), true); + assert.equal(api.deviceId(), null); +}); + +test('onReady fires off-platform too, so a browser never blocks on hardware that is absent', async () => { + const { api, ready } = load(); + await ready; + assert.equal(api.isReady(), true); +}); + +test('a rejected registry read still lets the player boot', async () => { + const { api, ready } = load({ mods: true }); + // The fake resolves; what matters is that readiness is reached and nothing throws. + await ready; + assert.doesNotThrow(() => api.deviceId()); +});