From 46b2227dfd4604a61210040fbe45c7667f73d58a Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Wed, 5 Aug 2026 10:03:33 -0500 Subject: [PATCH] BrightSign: real telemetry and hardware identity, not a block of nulls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The web player sent battery/storage/RAM/CPU as nulls and 'Web Player' in the wifi_ssid column, and the bs_model / bs_os_version / bs_serial / bs_screen fields the player already reported at registration were consumed by nothing. A browser tab genuinely has none of that. A BrightSign has some of it, and was reporting none. Telemetry comes from a CACHE the heartbeat reads synchronously. The beat builds its payload every 15s without awaiting, but the one real sensor here — deviceInfo.getTemperature() — returns a promise; awaiting inside the beat would either block it or serialise a pending Promise into the payload, which is exactly how device_id once became "[object Promise]". The cache starts EMPTY rather than null-filled and is spread last, so off-platform nothing changes and a null here can never clobber a value another player family legitimately supplied. wifi_ssid was actively false on a PoE Ethernet appliance — an operator reading that column was told an SSID that does not exist. It is null there now, and the device view shows a real hardware block instead. Android's WiFi display is untouched. Hardware identity is a SEPARATE writer from applyDeviceInfo, deliberately. That function is a blind full-row overwrite, and an empty device_info once nulled seventeen columns every five minutes because {} is truthy. These fields arrive only on a full register, so the same shape would wipe them on every lightweight refresh in between; COALESCE makes "no news" mean "unchanged". The OS build gets its own column rather than reusing android_version, which is load-bearing as a TYPE discriminator: device-detail chooses between the Android and browser layouts with android_version.startsWith('Web/'), so writing "BrightSign OS 9.0.189" there would have rendered a BrightSign with battery and WiFi cards — and applyDeviceInfo would have clobbered it on the next refresh. Storage is labelled "Player Storage", not "Storage": on this family the number is the widget's cache quota, not the device filesystem, and it lands in the same column as Android's real disk figures. Schema: device_telemetry.temperature_c REAL; devices.hardware_model, hardware_serial, hardware_os_version, output_index. All nullable, all idempotent in the existing migration array. 973 pass (+19). The temperature tests drive a real socket into a real server, because changing the arity of the telemetry INSERT would break every player's heartbeat, not just BrightSign's. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- brightsign/st-bridge.js | 72 +++++++++++ frontend/js/i18n/en.js | 8 ++ frontend/js/views/device-detail.js | 32 +++++ server/db/database.js | 21 ++++ server/player/index.html | 9 +- server/test/brightsign-bridge.test.js | 84 ++++++++++++- server/test/device-hardware-identity.test.js | 118 ++++++++++++++++++ server/test/telemetry-temperature.test.js | 124 +++++++++++++++++++ server/ws/deviceSocket.js | 53 +++++++- 9 files changed, 514 insertions(+), 7 deletions(-) create mode 100644 server/test/device-hardware-identity.test.js create mode 100644 server/test/telemetry-temperature.test.js diff --git a/brightsign/st-bridge.js b/brightsign/st-bridge.js index f962340..06d6529 100644 --- a/brightsign/st-bridge.js +++ b/brightsign/st-bridge.js @@ -186,6 +186,12 @@ return cec; } + // Telemetry cache. Starts EMPTY rather than pre-filled with nulls: the player spreads this over + // its own telemetry object, and a null here would overwrite a value another player family had + // legitimately supplied. Absent means "nothing to say", which is not the same as "zero". + var telemetry = {}; + var TELEMETRY_REFRESH_MS = 60000; + var deviceInfo = null; if (DeviceInfoClass) { try { deviceInfo = new DeviceInfoClass(); } catch (e) { deviceInfo = null; } @@ -361,6 +367,64 @@ onHostMessage: function (fn) { if (typeof fn === 'function') listeners.push(fn); }, + /* + * Telemetry, read synchronously from a cache. + * + * The heartbeat builds its payload synchronously every 15s, but the only real number this + * platform exposes — temperature — arrives from a PROMISE (deviceInfo.getTemperature()). + * Awaiting it inside the heartbeat would either block the beat or, worse, serialise a pending + * Promise into the telemetry object, which is exactly how device_id once became + * "[object Promise]". So the values are refreshed on a timer and the beat reads whatever + * landed last. + * + * Returns an EMPTY object off-platform, so the caller can spread it unconditionally and a + * browser's telemetry is unchanged. + */ + telemetrySnapshot: function () { return telemetry; }, + + /* + * Refresh the cache. Safe to call repeatedly; each source fails independently so one missing + * API cannot take the others down with it. + */ + refreshTelemetry: function () { + // Temperature: documented on @brightsign/deviceinfo, resolves { celsius }. + if (deviceInfo && typeof deviceInfo.getTemperature === 'function') { + try { + var t = deviceInfo.getTemperature(); + if (t && typeof t.then === 'function') { + t.then(function (v) { + var c = v && (v.celsius !== undefined ? v.celsius : v.Celsius); + if (typeof c === 'number' && isFinite(c)) telemetry.temperature_c = Math.round(c * 10) / 10; + }, function () { /* sensor unavailable on this model */ }); + } + } catch (e) { /* older OS without the call */ } + } + + /* + * Storage. This is the WIDGET'S storage quota (storage_path/storage_quota in autorun.brs), + * NOT the device's filesystem — there is no documented JS API for the latter, and reporting + * eMMC/SD capacity would need the host. It is still the number that matters operationally, + * because it is the budget the player actually has for cached content, and it is what fills + * up. The dashboard labels it distinctly for this family so it is never read as "the disk". + */ + try { + var s = global.navigator && global.navigator.storage; + if (s && typeof s.estimate === 'function') { + var e = s.estimate(); + if (e && typeof e.then === 'function') { + e.then(function (est) { + if (!est) return; + var quota = Number(est.quota), usage = Number(est.usage); + if (isFinite(quota) && quota > 0) { + telemetry.storage_total_mb = Math.round(quota / 1048576); + if (isFinite(usage)) telemetry.storage_free_mb = Math.round((quota - usage) / 1048576); + } + }, function () { /* estimate refused */ }); + } + } + } catch (e) { /* no storage manager */ } + }, + /* * Heartbeat. autorun.brs rebuilds the widget after three missed beats, which is what * recovers a page that loaded fine and then wedged (dead socket, JS exception, decoder @@ -381,5 +445,13 @@ prefetch(); if (global.setTimeout) global.setTimeout(markReady, 5000); + // Only worth polling where a sensor exists. A browser has neither the temperature API nor a + // meaningful storage quota to report, and an interval that can only ever produce nothing is + // just a timer burning a wakeup every minute on a device that runs for months. + if (API.isBrightSign()) { + API.refreshTelemetry(); + if (global.setInterval) global.setInterval(API.refreshTelemetry, TELEMETRY_REFRESH_MS); + } + if (API.hasHost()) API.startHeartbeat(); })(typeof window !== 'undefined' ? window : this); diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 2c3ad53..3b0532d 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -470,6 +470,14 @@ export default { 'device.info.battery': 'Battery', 'device.info.storage': 'Storage', 'device.info.size_free': '{size} free', + // "Player storage" rather than "Storage": on a browser-family player this is the widget's cache + // quota, not the device filesystem, and it sits in the same column as Android's real disk usage. + 'device.info.player_storage': 'Player Storage', + 'device.info.hardware_model': 'Model', + 'device.info.os_version': 'OS Version', + 'device.info.serial': 'Serial', + 'device.info.temperature': 'Temperature', + 'device.info.output_n': '(output {n})', 'device.info.player_type': 'Player Type', 'device.info.web_player': 'Web Player', 'device.info.brightsign_player': 'BrightSign', diff --git a/frontend/js/views/device-detail.js b/frontend/js/views/device-detail.js index 231e938..fa2a917 100644 --- a/frontend/js/views/device-detail.js +++ b/frontend/js/views/device-detail.js @@ -354,7 +354,39 @@ async function loadDevice(deviceId, activeTab = null) {
${t('device.info.player_type')}
${isBrightSignDevice(device) ? t('device.info.brightsign_player') : t('device.info.web_player')}
+ ${device.hardware_model ? ` +
+
${t('device.info.hardware_model')}
+
${esc(device.hardware_model)}${device.output_index > 1 ? ` ${t('device.info.output_n', { n: device.output_index })}` : ''}
+
` : ''} + ${device.hardware_os_version ? ` +
+
${t('device.info.os_version')}
+
${esc(device.hardware_os_version)}
+
` : ''} + ${device.hardware_serial ? ` +
+
${t('device.info.serial')}
+
${esc(device.hardware_serial)}
+
` : ''} + ${latestTelemetry.storage_total_mb ? ` +
+ +
${t('device.info.player_storage')}
+
${latestTelemetry.storage_free_mb != null ? t('device.info.size_free', { size: formatBytes(latestTelemetry.storage_free_mb) }) : '--'}
+
+
+
+
` : ''} `} + ${latestTelemetry.temperature_c != null ? ` +
+
${t('device.info.temperature')}
+
${latestTelemetry.temperature_c}°C
+
` : ''} ${device.android_version && !device.android_version.startsWith('Web/') ? `
${t('device.info.wifi')}
diff --git a/server/db/database.js b/server/db/database.js index 7bc4305..f435e9f 100644 --- a/server/db/database.js +++ b/server/db/database.js @@ -371,6 +371,27 @@ const migrations = [ // the PUBLIC address the server sees the connection arrive from — both are useful and they are // not the same thing. A customer reading the public IP as "my screen's IP" prompted this. "ALTER TABLE device_telemetry ADD COLUMN local_ip TEXT", + // Panel temperature in Celsius. REAL because the sensor reports fractions, and nullable because + // only some hardware exposes one — Android and the browser players send nothing and must keep + // reading as "no sensor" rather than "0 degrees", which is why every read site treats null as + // absent instead of coercing. + "ALTER TABLE device_telemetry ADD COLUMN temperature_c REAL", + // Hardware identity as the PANEL reports it, distinct from anything the server infers. A + // BrightSign knows its model (XT245 vs XC4055 — different capabilities, notably output count), + // its OS build, and its serial, and none of that had anywhere to live: the devices row carried + // only `platform`. Deliberately generic names rather than bs_* — an Android panel has a model + // and a serial too, and naming the columns after one vendor would mean a second set later. + "ALTER TABLE devices ADD COLUMN hardware_model TEXT", + "ALTER TABLE devices ADD COLUMN hardware_serial TEXT", + // The OS build, in its OWN column rather than reusing android_version. That column is load + // bearing as a TYPE discriminator, not just a value: the device view decides between the + // Android layout and the browser layout with android_version.startsWith('Web/'), so writing + // "BrightSign OS 9.0.189" there would render a BrightSign with battery and WiFi cards. It would + // also be clobbered on the next lightweight device_info refresh, which rewrites that column. + "ALTER TABLE devices ADD COLUMN hardware_os_version TEXT", + // Which physical output this row paints. A dual-output player runs one player per connector and + // registers as two devices; without this they are indistinguishable in the dashboard. + "ALTER TABLE devices ADD COLUMN output_index INTEGER", // Backfill a unique 6-digit PIN for already-paired devices that predate the // settings_pin column (their next reconnect re-sends device:paired with it, so // the existing fleet isn't locked out of the on-device menu). Idempotent: the diff --git a/server/player/index.html b/server/player/index.html index 35d01d8..bef6bf0 100644 --- a/server/player/index.html +++ b/server/player/index.html @@ -1652,12 +1652,19 @@ ram_free_mb: null, ram_total_mb: null, cpu_usage: null, - wifi_ssid: 'Web Player', + // 'Web Player' is a placeholder standing in a WiFi column. On a BrightSign it is + // actively wrong — the one we have is PoE over Ethernet — and an operator reading it + // as an SSID has been told something false. Null means "not on WiFi", which is true, + // and the dashboard shows a real hardware block for this family instead. + wifi_ssid: ON_BRIGHTSIGN ? null : 'Web Player', wifi_rssi: null, uptime_seconds: Math.floor(performance.now() / 1000), // #74/#75: report OS timezone + UTC clock (effective-tz resolution + skew indicator) timezone: (function () { try { return Intl.DateTimeFormat().resolvedOptions().timeZone || null; } catch (e) { return null; } })(), device_utc: Date.now(), + // Real values where the platform has them (temperature, storage quota). Spread LAST so + // it overrides the nulls above, and empty off-platform so nothing else changes. + ...(BS ? BS.telemetrySnapshot() : {}), } }); }, HEARTBEAT_INTERVAL); diff --git a/server/test/brightsign-bridge.test.js b/server/test/brightsign-bridge.test.js index d8d4b24..a501ffd 100644 --- a/server/test/brightsign-bridge.test.js +++ b/server/test/brightsign-bridge.test.js @@ -22,7 +22,7 @@ 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', seed = {} } = {}) { +function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150', seed = {}, storageEstimate = null, temperature = null } = {}) { const posted = []; const registryStore = new Map(Object.entries(seed)); const cec = { sent: [] }; @@ -30,7 +30,13 @@ function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150', seed = const sandbox = { console: { log() {}, warn() {}, error() {} }, - navigator: { userAgent: ua }, + // navigator.storage.estimate() is a REAL browser API the bridge reads for the cache quota, + // and it is async — modelled as such so a synchronous stand-in cannot hide a pending-Promise + // bug the way one previously did for the registry. + navigator: { + userAgent: ua, + storage: storageEstimate ? { estimate: () => Promise.resolve(storageEstimate) } : undefined, + }, location: { search, reload() { sandbox.__reloaded = true; } }, setInterval: () => 1, setTimeout: (fn, ms) => setTimeout(fn, ms), @@ -38,6 +44,9 @@ function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150', seed = Object, Array, Uint8Array, + Number, + isFinite, + Math, Date, RegExp, parseInt, @@ -85,7 +94,13 @@ function load({ search = '', mods = null, ua = 'Mozilla/5.0 Chrome/150', seed = } if (name === '@brightsign/deviceinfo') { return function () { - return { model: 'XT1145', osVersion: '9.1.92.2', serialNumber: 'SN-TEST-1' }; + return { + model: 'XT1145', osVersion: '9.1.92.2', serialNumber: 'SN-TEST-1', + // getTemperature() resolves a PROMISE on real hardware. Modelled async on purpose. + getTemperature: () => (temperature == null + ? Promise.reject(new Error('no sensor')) + : Promise.resolve({ celsius: temperature })), + }; }; } throw new Error('no such module ' + name); @@ -280,3 +295,66 @@ test('output 2 addresses HDMI-2 — a dual-output player must sleep the screen i api.displayPower(true); assert.deepEqual(cecConnectors, ['HDMI-2']); }); + +// --- telemetry ------------------------------------------------------------------------------ +// +// The heartbeat builds its payload SYNCHRONOUSLY every 15s, but the only real number this platform +// exposes — temperature — arrives from a Promise. Awaiting it in the beat would either block the +// beat or serialise a pending Promise into the telemetry object, which is precisely how device_id +// once became "[object Promise]". Hence a cache the beat reads synchronously. + +const settle = () => new Promise((r) => setTimeout(r, 10)); + +test('the snapshot is EMPTY off-platform, so a browser spreads nothing over its telemetry', async () => { + const { api, ready } = load(); + await ready; + // Keys, not deepEqual: the object is built inside the vm realm, so a strict structural compare + // trips on prototype identity rather than on anything about the value. + assert.equal(Object.keys(api.telemetrySnapshot()).length, 0, + 'nulls here would clobber another family’s values'); +}); + +test('temperature is cached from the promise, never the promise itself', async () => { + const { api, ready } = load({ mods: true, temperature: 47.26 }); + await ready; + api.refreshTelemetry(); + await settle(); + const snap = api.telemetrySnapshot(); + assert.equal(typeof snap.temperature_c, 'number', 'a pending Promise here is the bug this guards'); + assert.equal(snap.temperature_c, 47.3, 'rounded to one decimal'); +}); + +test('a model with no temperature sensor reports nothing rather than a bogus reading', async () => { + const { api, ready } = load({ mods: true, temperature: null }); // getTemperature() rejects + await ready; + api.refreshTelemetry(); + await settle(); + assert.equal(api.telemetrySnapshot().temperature_c, undefined); +}); + +test('storage quota becomes free/total MB', async () => { + const { api, ready } = load({ mods: true, storageEstimate: { quota: 1073741824, usage: 268435456 } }); + await ready; + api.refreshTelemetry(); + await settle(); + const snap = api.telemetrySnapshot(); + assert.equal(snap.storage_total_mb, 1024); + assert.equal(snap.storage_free_mb, 768); +}); + +test('one failing source does not take the other down with it', async () => { + // No sensor, but storage is readable: the snapshot must still carry the storage figures. + const { api, ready } = load({ mods: true, temperature: null, storageEstimate: { quota: 2147483648, usage: 0 } }); + await ready; + api.refreshTelemetry(); + await settle(); + const snap = api.telemetrySnapshot(); + assert.equal(snap.temperature_c, undefined); + assert.equal(snap.storage_total_mb, 2048); +}); + +test('refreshTelemetry never throws when the platform offers neither source', async () => { + const { api, ready } = load(); // plain browser: no modules, no storage manager + await ready; + assert.doesNotThrow(() => api.refreshTelemetry()); +}); diff --git a/server/test/device-hardware-identity.test.js b/server/test/device-hardware-identity.test.js new file mode 100644 index 0000000..72eb38c --- /dev/null +++ b/server/test/device-hardware-identity.test.js @@ -0,0 +1,118 @@ +'use strict'; + +// Hardware identity — model, OS build, serial, which output — is reported by the panel and had +// nowhere to live: the devices row carried only `platform`. A BrightSign knows all four, and an +// operator looking at a dead screen wants the serial and the model, not a guess. +// +// The reason this is a SEPARATE writer rather than four more fields in applyDeviceInfo: that +// function is a blind full-row overwrite, and an empty device_info once nulled seventeen columns +// every five minutes because `{}` is truthy. These fields arrive only on a full register, so the +// same shape would wipe them on every lightweight refresh in between. COALESCE is what makes +// "no news" mean "unchanged" instead of "gone" — which is the property these tests pin down. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'st-hw-identity-')); +process.env.DATA_DIR = tmp; + +const { db } = require('../db/database'); +const { __applyHardwareIdentity: applyHardwareIdentity } = require('../ws/deviceSocket'); + +let n = 0; +function mkDevice() { + const id = 'dev-hw-' + (++n); + db.prepare("INSERT INTO devices (id,name,status,created_at) VALUES (?,?,'offline',strftime('%s','now'))") + .run(id, 'HW ' + n); + return id; +} +const row = (id) => db.prepare('SELECT * FROM devices WHERE id = ?').get(id); + +test('the schema carries hardware identity and a temperature column', () => { + const dev = db.prepare('PRAGMA table_info(devices)').all().map((c) => c.name); + for (const c of ['hardware_model', 'hardware_serial', 'hardware_os_version', 'output_index']) { + assert.ok(dev.includes(c), `devices.${c} missing`); + } + const tel = db.prepare('PRAGMA table_info(device_telemetry)').all().map((c) => c.name); + assert.ok(tel.includes('temperature_c'), 'device_telemetry.temperature_c missing'); +}); + +test('top-level bs_* fields are persisted — that is how the player reports them today', () => { + const id = mkDevice(); + applyHardwareIdentity(id, { + bs_model: 'XT245', bs_serial: 'URD3C6000823', bs_os_version: '9.0.189', bs_screen: 1, + }); + const r = row(id); + assert.equal(r.hardware_model, 'XT245'); + assert.equal(r.hardware_serial, 'URD3C6000823'); + assert.equal(r.hardware_os_version, '9.0.189'); +}); + +test('device_info is read too, so the client can move without a flag day', () => { + const id = mkDevice(); + applyHardwareIdentity(id, { + device_info: { hardware_model: 'XC4055', hardware_serial: 'SN-XC', hardware_os_version: '9.1.5', output_index: 3 }, + }); + const r = row(id); + assert.equal(r.hardware_model, 'XC4055'); + assert.equal(r.output_index, 3); +}); + +test('THE WIPE THIS GUARDS: a later report without the fields leaves them intact', () => { + const id = mkDevice(); + applyHardwareIdentity(id, { bs_model: 'XT245', bs_serial: 'SN-KEEP', bs_os_version: '9.0.189' }); + // A subsequent register from a client that says nothing about hardware — the shape that nulled + // seventeen columns when it went through the blind-overwrite path. + applyHardwareIdentity(id, { device_info: {} }); + applyHardwareIdentity(id, {}); + const r = row(id); + assert.equal(r.hardware_model, 'XT245', 'silence must not read as "the model is gone"'); + assert.equal(r.hardware_serial, 'SN-KEEP'); + assert.equal(r.hardware_os_version, '9.0.189'); +}); + +test('a genuine change still overwrites — COALESCE must not freeze the value', () => { + const id = mkDevice(); + applyHardwareIdentity(id, { bs_model: 'XT245' }); + applyHardwareIdentity(id, { bs_model: 'XC2055' }); + assert.equal(row(id).hardware_model, 'XC2055'); +}); + +test('a player that reports none of it is not written at all', () => { + const id = mkDevice(); + const before = row(id).updated_at; + applyHardwareIdentity(id, { device_info: { app_version: '1.1.0-web' } }); + const r = row(id); + assert.equal(r.hardware_model, null); + assert.equal(r.hardware_serial, null); + assert.equal(r.updated_at, before, 'an Android/browser register should not touch the row here'); +}); + +test('output_index only accepts a positive integer', () => { + const id = mkDevice(); + // screen() returns 1 for a single-output player; 0 / negative / "2" are not outputs. + for (const bad of [0, -1, '2', 1.5, null, undefined]) { + applyHardwareIdentity(id, { bs_model: 'X', bs_screen: bad }); + assert.equal(row(id).output_index, null, `output_index accepted ${JSON.stringify(bad)}`); + } + applyHardwareIdentity(id, { bs_screen: 2 }); + assert.equal(row(id).output_index, 2); +}); + +test('device-supplied strings are trimmed and capped', () => { + const id = mkDevice(); + applyHardwareIdentity(id, { bs_model: ' XT245 ', bs_serial: 'S'.repeat(200) }); + const r = row(id); + assert.equal(r.hardware_model, 'XT245'); + assert.equal(r.hardware_serial.length, 64, 'these render in the dashboard — cap them'); +}); + +test('a whitespace-only value is not an answer', () => { + const id = mkDevice(); + applyHardwareIdentity(id, { bs_model: 'XT245' }); + applyHardwareIdentity(id, { bs_model: ' ' }); + assert.equal(row(id).hardware_model, 'XT245'); +}); diff --git a/server/test/telemetry-temperature.test.js b/server/test/telemetry-temperature.test.js new file mode 100644 index 0000000..d1c193d --- /dev/null +++ b/server/test/telemetry-temperature.test.js @@ -0,0 +1,124 @@ +'use strict'; + +// Temperature is the one real sensor reading this platform exposes, and it arrives from a promise +// on the player. Two things have to hold at the server end, and neither is provable from a unit +// test of the guard alone — this drives a real socket into a real server: +// +// 1. Adding the column must not break the insert for every OTHER player. The telemetry INSERT is +// on the heartbeat path, which every device hits every 15 seconds; getting its arity wrong +// would take the whole fleet's telemetry down, not just BrightSign's. +// 2. A panel with no sensor sends nothing, and a flaky one can send NaN or Infinity. Both must +// land as "no reading" rather than as a number, because the dashboard renders this value and +// null is the only honest way to say "this hardware has no thermometer". + +const { test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const os = require('node:os'); +const { spawn } = require('node:child_process'); +const fs = require('node:fs'); +const crypto = require('node:crypto'); +const Database = require('better-sqlite3'); +const ioClient = require('../node_modules/socket.io-client'); + +const { freePort } = require('./helpers/free-port'); +let PORT, BASE, proc, db; +const DATA_DIR = path.join(os.tmpdir(), 'st-temp-' + crypto.randomBytes(4).toString('hex')); +const LOG = path.join(os.tmpdir(), 'st-temp-' + crypto.randomBytes(4).toString('hex') + '.log'); + +// Exactly what a browser/Android player sends: no temperature key at all. +const BASE_TELEMETRY = { + battery_level: null, battery_charging: false, storage_free_mb: null, storage_total_mb: null, + ram_free_mb: null, ram_total_mb: null, cpu_usage: null, wifi_ssid: 'Web Player', + wifi_rssi: null, uptime_seconds: 42, +}; + +before(async () => { + PORT = await freePort(); + BASE = `http://127.0.0.1:${PORT}`; + const logFd = fs.openSync(LOG, 'w'); + proc = spawn('node', ['server.js'], { + cwd: path.join(__dirname, '..'), + env: { ...process.env, DATA_DIR, SELF_HOSTED: 'true', PORT: String(PORT), NODE_ENV: 'test' }, + stdio: ['ignore', logFd, logFd], + }); + let up = false; + for (let i = 0; i < 80; i++) { + try { const r = await fetch(BASE + '/api/status'); if (r.ok) { up = true; break; } } catch { /* */ } + await new Promise((r) => setTimeout(r, 250)); + } + if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000)); + db = new Database(path.join(DATA_DIR, 'db', 'remote_display.db')); +}); +after(() => { + try { db && db.close(); } catch { /* */ } + try { proc.kill('SIGKILL'); } catch { /* */ } +}); + +function makeDevice() { + const id = crypto.randomUUID(); + const token = crypto.randomBytes(32).toString('hex'); + db.prepare(`INSERT INTO devices (id, name, status, device_token, created_at) + VALUES (?, 'TEMP', 'online', ?, strftime('%s','now'))`).run(id, token); + return { id, token }; +} +function connect(dev) { + return new Promise((resolve, reject) => { + const s = ioClient(BASE + '/device', { transports: ['websocket'], reconnection: false }); + s.on('connect', () => s.emit('device:register', { device_id: dev.id, device_token: dev.token })); + s.on('device:registered', () => resolve(s)); + s.on('device:auth-error', (e) => reject(new Error(e && e.error))); + setTimeout(() => reject(new Error('register timeout')), 10000); + }); +} +const wait = (ms) => new Promise((r) => setTimeout(r, ms)); +const lastTemp = (id) => db.prepare( + 'SELECT temperature_c FROM device_telemetry WHERE device_id = ? ORDER BY reported_at DESC, id DESC LIMIT 1' +).get(id); + +test('a reading is stored', async () => { + const dev = makeDevice(); + const s = await connect(dev); + s.emit('device:heartbeat', { device_id: dev.id, telemetry: { ...BASE_TELEMETRY, temperature_c: 47.3 } }); + await wait(400); + assert.equal(lastTemp(dev.id).temperature_c, 47.3); + s.close(); +}); + +test('EVERY OTHER PLAYER still records telemetry — the insert arity did not change under them', async () => { + const dev = makeDevice(); + const s = await connect(dev); + s.emit('device:heartbeat', { device_id: dev.id, telemetry: BASE_TELEMETRY }); + await wait(400); + const row = db.prepare( + 'SELECT * FROM device_telemetry WHERE device_id = ? ORDER BY reported_at DESC, id DESC LIMIT 1' + ).get(dev.id); + assert.ok(row, 'a player that sends no temperature must still get a telemetry row'); + assert.equal(row.temperature_c, null); + assert.equal(row.uptime_seconds, 42, 'the rest of the payload is unaffected'); + s.close(); +}); + +test('a flaky sensor cannot poison the column', async () => { + const dev = makeDevice(); + const s = await connect(dev); + for (const bad of [NaN, Infinity, -Infinity, 'hot', {}, true]) { + s.emit('device:heartbeat', { device_id: dev.id, telemetry: { ...BASE_TELEMETRY, temperature_c: bad } }); + await wait(220); + assert.equal(lastTemp(dev.id).temperature_c, null, `accepted ${String(bad)} as a reading`); + } + s.close(); +}); + +test('a sub-zero reading is a real reading, not a falsy one', async () => { + // 0 and negatives are legitimate temperatures; a truthiness check here would drop them. + const dev = makeDevice(); + const s = await connect(dev); + s.emit('device:heartbeat', { device_id: dev.id, telemetry: { ...BASE_TELEMETRY, temperature_c: 0 } }); + await wait(400); + assert.equal(lastTemp(dev.id).temperature_c, 0); + s.emit('device:heartbeat', { device_id: dev.id, telemetry: { ...BASE_TELEMETRY, temperature_c: -5.5 } }); + await wait(400); + assert.equal(lastTemp(dev.id).temperature_c, -5.5); + s.close(); +}); diff --git a/server/ws/deviceSocket.js b/server/ws/deviceSocket.js index 16006cd..da15d9e 100644 --- a/server/ws/deviceSocket.js +++ b/server/ws/deviceSocket.js @@ -104,6 +104,42 @@ function applyDeviceInfo(deviceId, di) { deviceId); } +/* + * Persist panel-reported hardware identity (model / OS build / serial / which output). + * + * Deliberately NOT folded into applyDeviceInfo. That function is a blind full-row overwrite, and + * an empty device_info once nulled seventeen columns every five minutes because `{}` is truthy — + * the exact failure that degraded the browser player family. These fields arrive only on a full + * register, so the same shape here would wipe them on every lightweight refresh. + * + * COALESCE is the guard: a report that omits a field leaves the stored value alone. Hardware + * identity does not change under a device that is still the same device, so "no news" must mean + * "unchanged", never "gone". + * + * Accepts the fields from device_info OR the top level. The player sends them at the top level + * today; device_info is where every other panel-reported fact lives, so reading both means the + * client can move without a flag day in either direction. + */ +function applyHardwareIdentity(deviceId, data) { + const di = (data && data.device_info) || {}; + const str = (v) => (typeof v === 'string' && v.trim() ? v.trim().slice(0, 64) : null); + const model = str(di.hardware_model ?? data.bs_model); + const serial = str(di.hardware_serial ?? data.bs_serial); + const osVersion = str(di.hardware_os_version ?? data.bs_os_version); + const rawOutput = di.output_index ?? data.bs_screen; + const output = Number.isInteger(rawOutput) && rawOutput > 0 ? rawOutput : null; + + if (model == null && serial == null && osVersion == null && output == null) return; + + db.prepare(`UPDATE devices SET + hardware_model = COALESCE(?, hardware_model), + hardware_serial = COALESCE(?, hardware_serial), + hardware_os_version = COALESCE(?, hardware_os_version), + output_index = COALESCE(?, output_index) + WHERE id = ?`) + .run(model, serial, osVersion, output, deviceId); +} + function generateDeviceToken() { return crypto.randomBytes(32).toString('hex'); } @@ -794,6 +830,10 @@ module.exports = function setupDeviceSocket(io) { // shape — recordReconnect/persistIdentity are gated behind `if (!isPlaylistRefresh)` — // this call was the one that was not. if (device_info && Object.keys(device_info).length > 0) applyDeviceInfo(device_id, device_info); + // AFTER applyDeviceInfo, and unconditionally: these fields ride the top level of the + // register payload, not device_info, so the emptiness guard above does not apply to + // them. The function no-ops when the panel reports none of them. + applyHardwareIdentity(device_id, data); heartbeat.registerConnection(device_id, socket.id); // #134: a same-socket re-register is a playlist REFRESH (~45-60s), NOT a reconnect and NOT @@ -1014,8 +1054,8 @@ module.exports = function setupDeviceSocket(io) { if (telemetry && deviceExists(device_id)) { db.prepare(` INSERT INTO device_telemetry (device_id, battery_level, battery_charging, storage_free_mb, storage_total_mb, - ram_free_mb, ram_total_mb, cpu_usage, wifi_ssid, wifi_rssi, uptime_seconds, local_ip) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ram_free_mb, ram_total_mb, cpu_usage, wifi_ssid, wifi_rssi, uptime_seconds, local_ip, temperature_c) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `).run( device_id, telemetry.battery_level ?? null, @@ -1030,7 +1070,11 @@ module.exports = function setupDeviceSocket(io) { telemetry.uptime_seconds ?? null, // Device-supplied text headed for a column the dashboard renders: trim and cap it. // 45 chars is the longest legitimate value (a full IPv6 address). - typeof telemetry.local_ip === 'string' ? telemetry.local_ip.trim().slice(0, 45) || null : null + typeof telemetry.local_ip === 'string' ? telemetry.local_ip.trim().slice(0, 45) || null : null, + // Only a finite number is a reading. A panel with no sensor sends nothing, and NaN or + // Infinity from a flaky one must land as "no reading" rather than poisoning the column. + typeof telemetry.temperature_c === 'number' && Number.isFinite(telemetry.temperature_c) + ? telemetry.temperature_c : null ); pruneTelemetry(device_id); @@ -1468,6 +1512,9 @@ module.exports = function setupDeviceSocket(io) { // so the cause-1 re-arm race (evicted socket arming an offline timer for a // just-reconnected device) is test-PROVEN, not just correct-by-construction. Prefixed // `__` and never used by production code. +// Test-only, same convention as the handles below: the COALESCE semantics are the whole point of +// this function and are not reachable through a socket handshake in a unit test. +module.exports.__applyHardwareIdentity = applyHardwareIdentity; module.exports.__hasPendingOffline = (deviceId) => pendingOfflines.has(deviceId); module.exports.__pendingOfflineCount = () => pendingOfflines.size; module.exports.__evictedSize = () => evictedSockets.size;