mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 22:03:13 -06:00
BrightSign: real telemetry and hardware identity, not a block of nulls
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
This commit is contained in:
parent
141deb97a5
commit
46b2227dfd
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -354,7 +354,39 @@ async function loadDevice(deviceId, activeTab = null) {
|
|||
<div class="info-card-label">${t('device.info.player_type')}</div>
|
||||
<div class="info-card-value small">${isBrightSignDevice(device) ? t('device.info.brightsign_player') : t('device.info.web_player')}</div>
|
||||
</div>
|
||||
${device.hardware_model ? `
|
||||
<div class="info-card">
|
||||
<div class="info-card-label">${t('device.info.hardware_model')}</div>
|
||||
<div class="info-card-value small">${esc(device.hardware_model)}${device.output_index > 1 ? ` <span style="color:var(--text-muted)">${t('device.info.output_n', { n: device.output_index })}</span>` : ''}</div>
|
||||
</div>` : ''}
|
||||
${device.hardware_os_version ? `
|
||||
<div class="info-card">
|
||||
<div class="info-card-label">${t('device.info.os_version')}</div>
|
||||
<div class="info-card-value small">${esc(device.hardware_os_version)}</div>
|
||||
</div>` : ''}
|
||||
${device.hardware_serial ? `
|
||||
<div class="info-card">
|
||||
<div class="info-card-label">${t('device.info.serial')}</div>
|
||||
<div class="info-card-value small">${esc(device.hardware_serial)}</div>
|
||||
</div>` : ''}
|
||||
${latestTelemetry.storage_total_mb ? `
|
||||
<div class="info-card">
|
||||
<!-- Labelled "player storage", not "storage": on this family the number is the
|
||||
widget's cache quota, not the device filesystem. Same column as Android's real
|
||||
disk figures, so the label is what stops it being read as "the disk is 1 GB". -->
|
||||
<div class="info-card-label">${t('device.info.player_storage')}</div>
|
||||
<div class="info-card-value small" id="telStorage">${latestTelemetry.storage_free_mb != null ? t('device.info.size_free', { size: formatBytes(latestTelemetry.storage_free_mb) }) : '--'}</div>
|
||||
<div class="progress-bar">
|
||||
<div class="progress-bar-fill ${((latestTelemetry.storage_total_mb - latestTelemetry.storage_free_mb) / latestTelemetry.storage_total_mb) < 0.8 ? 'success' : 'warning'}"
|
||||
style="width:${((latestTelemetry.storage_total_mb - latestTelemetry.storage_free_mb) / latestTelemetry.storage_total_mb * 100)}%"></div>
|
||||
</div>
|
||||
</div>` : ''}
|
||||
`}
|
||||
${latestTelemetry.temperature_c != null ? `
|
||||
<div class="info-card">
|
||||
<div class="info-card-label">${t('device.info.temperature')}</div>
|
||||
<div class="info-card-value small" id="telTemp">${latestTelemetry.temperature_c}°C</div>
|
||||
</div>` : ''}
|
||||
${device.android_version && !device.android_version.startsWith('Web/') ? `
|
||||
<div class="info-card">
|
||||
<div class="info-card-label">${t('device.info.wifi')}</div>
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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());
|
||||
});
|
||||
|
|
|
|||
118
server/test/device-hardware-identity.test.js
Normal file
118
server/test/device-hardware-identity.test.js
Normal file
|
|
@ -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');
|
||||
});
|
||||
124
server/test/telemetry-temperature.test.js
Normal file
124
server/test/telemetry-temperature.test.js
Normal file
|
|
@ -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();
|
||||
});
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in a new issue