diff --git a/src/nodered/commandRegistry.js b/src/nodered/commandRegistry.js index d4764fb..b4a77f1 100644 --- a/src/nodered/commandRegistry.js +++ b/src/nodered/commandRegistry.js @@ -26,14 +26,60 @@ function _describeUnit(unit) { try { return convert().describe(unit); } catch (_) { return null; } } +// A numeric scalar is a finite number, or a non-empty string that parses to a +// finite number ("60", "1.5"). Node-RED inject/`change` nodes and upstream MQTT +// payloads routinely arrive as strings; treating them as non-numeric here is the +// gap that let values reach a handler unconverted. +function _asNumber(x) { + if (typeof x === 'number') return Number.isFinite(x) ? x : null; + if (typeof x === 'string' && x.trim() !== '') { + const n = Number(x); + return Number.isFinite(n) ? n : null; + } + return null; +} + function _extractValueAndUnit(msg) { if (!msg || typeof msg !== 'object') return null; const p = msg.payload; - if (typeof p === 'number') return { value: p, unit: msg.unit }; - if (p && typeof p === 'object' && typeof p.value === 'number') { - return { value: p.value, unit: p.unit ?? msg.unit }; + if (p && typeof p === 'object') { + const value = _asNumber(p.value); + if (value === null) return null; + return { value, unit: p.unit ?? msg.unit }; } - return null; + const value = _asNumber(p); + if (value === null) return null; + return { value, unit: msg.unit }; +} + +// Derive the dimensional measure (e.g. 'volumeFlowRate') from a unit string. +// Returns null when convert doesn't recognise the unit. +function _measureOf(unit) { + const desc = _describeUnit(unit); + return desc ? desc.measure : null; +} + +// Command origin = which control authority issued this message (the rotatingMachine +// `allowedSources` vocabulary: 'parent' = automation/parent controller, 'GUI' = +// SCADA/HMI operator, 'fysical' = physical buttons). Default 'parent'. Named +// `origin` on the message because `source` is already the domain instance handed +// to handlers. +const DEFAULT_ORIGIN = 'parent'; + +function _resolveOrigin(msg, descriptor) { + const o = msg && typeof msg.origin === 'string' && msg.origin.trim() !== '' + ? msg.origin.trim() + : (descriptor.defaultOrigin || DEFAULT_ORIGIN); + return o; +} + +// allowedSources values may be a Set (post config processing, as rotatingMachine +// stores them) or a plain array (raw config / other nodes). Accept both. +function _setHas(coll, value) { + if (!coll) return false; + if (typeof coll.has === 'function') return coll.has(value); + if (Array.isArray(coll)) return coll.includes(value); + return false; } class CommandRegistry { @@ -76,6 +122,8 @@ class CommandRegistry { payloadSchema: cmd.payloadSchema || null, description: typeof cmd.description === 'string' ? cmd.description : null, units, + gated: cmd.gated === true, + defaultOrigin: typeof cmd.defaultOrigin === 'string' ? cmd.defaultOrigin : null, handler: cmd.handler, }; this._byKey.set(cmd.topic, descriptor); @@ -87,12 +135,25 @@ class CommandRegistry { } _validateUnits(cmd) { - if (cmd.units === undefined || cmd.units === null) return null; - const { measure, default: def } = cmd.units; - if (typeof measure !== 'string' || measure.length === 0 || - typeof def !== 'string' || def.length === 0) { + // Two ways to declare the unit, normalised to the same internal shape: + // unit: 'm3/h' (preferred — measure derived) + // units: { default: 'm3/h' } (measure derived) + // units: { measure, default: 'm3/h' } (legacy — measure ignored, derived) + // The measure is always derived from the unit so it can never drift from it. + let def; + if (typeof cmd.unit === 'string') def = cmd.unit; + else if (cmd.units === undefined || cmd.units === null) return null; + else if (typeof cmd.units === 'string') def = cmd.units; + else def = cmd.units.default; + + if (typeof def !== 'string' || def.length === 0) { throw new TypeError( - `command '${cmd.topic}' units requires { measure: string, default: string }`); + `command '${cmd.topic}' requires a unit string (unit: 'm3/h' or units: { default: 'm3/h' })`); + } + const measure = _measureOf(def); + if (!measure) { + throw new TypeError( + `command '${cmd.topic}' declares unit '${def}' which convert does not recognise`); } return { measure, default: def }; } @@ -137,11 +198,31 @@ class CommandRegistry { return; } if (topic !== descriptor.topic) this._noteAlias(topic, descriptor.topic, log); + // Always stamp the command origin so handlers + gating can rely on it. + msg.origin = _resolveOrigin(msg, descriptor); + if (!this._originAllowed(descriptor, source, msg.origin, log)) return; if (descriptor.units) this._normaliseUnits(descriptor, msg, log); if (!this._validatePayload(descriptor, msg, log)) return; return descriptor.handler(source, msg, ctx); } + // Mode-gated control-authority arbitration. Opt-in per command via + // `gated: true`. The asset's mode (e.g. rotatingMachine's auto / + // virtualControl / fysicalControl) decides which origins it accepts via + // `source.config.mode.allowedSources[mode]`. Release = changing the mode. + // Nodes without a mode model are advisory (allow-all) so this is inert + // until a node opts in — never a silent behaviour change. + _originAllowed(descriptor, source, origin, log) { + if (!descriptor.gated) return true; + const allowedSources = source && source.config && source.config.mode + ? source.config.mode.allowedSources : null; + const mode = source ? source.currentMode : undefined; + if (!allowedSources || !mode) return true; // no mode model → advisory + if (_setHas(allowedSources[mode], origin)) return true; + log.warn?.(`${descriptor.topic}: origin '${origin}' not allowed in mode '${mode}'`); + return false; + } + _noteAlias(alias, canonical, log) { const prev = this._deprecationCounts.get(alias) || 0; this._deprecationCounts.set(alias, prev + 1); diff --git a/test/basic/commandRegistry.basic.test.js b/test/basic/commandRegistry.basic.test.js index 9697680..9e6cbea 100644 --- a/test/basic/commandRegistry.basic.test.js +++ b/test/basic/commandRegistry.basic.test.js @@ -394,43 +394,163 @@ test('units: object payload {value} without unit falls back to default unit sile assert.equal(logger._calls.warn.length, 0); }); -test('units: non-numeric payload (no normalisation applied) passes through to handler', async () => { +test('units: non-NUMERIC string payload (not normalisable) passes through to handler', async () => { const logger = makeLogger(); const seen = []; const reg = createRegistry([{ topic: 'set.demand', - units: { measure: 'volumeFlowRate', default: 'm3/h' }, + unit: 'm3/h', handler: (_s, msg) => { seen.push(msg.payload); }, }], { logger }); - // string payload — not normalisable. Should not crash; handler still fires. + // non-numeric string — not normalisable. Should not crash; handler still fires. await reg.dispatch({ topic: 'set.demand', payload: 'magic' }, {}, {}); assert.equal(seen.length, 1); assert.equal(seen[0], 'magic'); }); -test('units: missing default field throws at construction', () => { +test('units: NUMERIC string payload is always converted (closes the string gap)', async () => { + const logger = makeLogger(); + const seen = []; + const reg = createRegistry([{ + topic: 'set.demand', + unit: 'm3/h', + handler: (_s, msg) => { seen.push({ payload: msg.payload, unit: msg.unit }); }, + }], { logger }); + + // "1" m3/s must convert to 3600 m3/h — same as the numeric-payload case. + await reg.dispatch({ topic: 'set.demand', payload: '1', unit: 'm3/s' }, {}, {}); + assert.equal(seen.length, 1); + assert.ok(Math.abs(seen[0].payload - 3600) < 1e-6, `expected 3600, got ${seen[0].payload}`); + assert.equal(seen[0].unit, 'm3/h'); + assert.equal(logger._calls.warn.length, 0); +}); + +test('units: { value: numeric-string } object payload is converted too', async () => { + const seen = []; + const reg = createRegistry([{ + topic: 'set.pressure', + unit: 'Pa', + handler: (_s, msg) => { seen.push({ payload: msg.payload, unit: msg.unit }); }, + }]); + await reg.dispatch({ topic: 'set.pressure', payload: { value: '5', unit: 'mbar' } }, {}, {}); + assert.ok(Math.abs(seen[0].payload - 500) < 1e-6, `expected 500, got ${seen[0].payload}`); + assert.equal(seen[0].unit, 'Pa'); +}); + +test('unit: shorthand declares the unit and DERIVES the measure', async () => { + const seen = []; + const reg = createRegistry([{ + topic: 'set.demand', + unit: 'm3/h', + handler: (_s, msg) => { seen.push({ payload: msg.payload, unit: msg.unit }); }, + }]); + await reg.dispatch({ topic: 'set.demand', payload: 1, unit: 'm3/s' }, {}, {}); + assert.ok(Math.abs(seen[0].payload - 3600) < 1e-6); + assert.equal(seen[0].unit, 'm3/h'); +}); + +test('units: { default } without measure derives the measure (measure no longer required)', () => { + const reg = createRegistry([{ + topic: 'set.demand', + units: { default: 'm3/h' }, + handler: () => {}, + }]); + assert.deepEqual(reg.list()[0].units, { measure: 'volumeFlowRate', default: 'm3/h' }); +}); + +test('units: legacy { measure, default } still works; measure is re-derived from the unit', () => { + const reg = createRegistry([{ + topic: 'set.demand', + units: { measure: 'totallyWrong', default: 'm3/h' }, + handler: () => {}, + }]); + // declared measure ignored — derived from the unit so it can never drift. + assert.deepEqual(reg.list()[0].units, { measure: 'volumeFlowRate', default: 'm3/h' }); +}); + +test('units: missing unit/default throws at construction', () => { assert.throws(() => createRegistry([{ topic: 'set.demand', units: { measure: 'volumeFlowRate' }, handler: () => {}, - }]), /units requires/); + }]), /requires a unit string/); }); -test('units: missing measure field throws at construction', () => { +test('units: unrecognised declared unit throws at construction (descriptor bug caught early)', () => { assert.throws(() => createRegistry([{ topic: 'set.demand', - units: { default: 'm3/h' }, + unit: 'flarbargs', handler: () => {}, - }]), /units requires/); + }]), /convert does not recognise/); }); test('units: descriptor.units surfaces in list() output', () => { const reg = createRegistry([ - { topic: 'set.demand', units: { measure: 'volumeFlowRate', default: 'm3/h' }, handler: () => {} }, - { topic: 'set.mode', handler: () => {} }, + { topic: 'set.demand', unit: 'm3/h', handler: () => {} }, + { topic: 'set.mode', handler: () => {} }, ]); const list = reg.list(); assert.deepEqual(list[0].units, { measure: 'volumeFlowRate', default: 'm3/h' }); assert.equal(list[1].units, null); }); + +// --------------------------------------------------------------------------- +// origin — command provenance + mode-gated control-authority arbitration +// --------------------------------------------------------------------------- + +test('origin: defaults to parent and is stamped on msg before the handler runs', async () => { + const seen = []; + const reg = createRegistry([{ topic: 'set.mode', handler: (_s, msg) => { seen.push(msg.origin); } }]); + await reg.dispatch({ topic: 'set.mode', payload: 'auto' }, {}, {}); + assert.equal(seen[0], 'parent'); +}); + +test('origin: explicit msg.origin is preserved and trimmed', async () => { + const seen = []; + const reg = createRegistry([{ topic: 'set.mode', handler: (_s, msg) => { seen.push(msg.origin); } }]); + await reg.dispatch({ topic: 'set.mode', payload: 'auto', origin: ' GUI ' }, {}, {}); + assert.equal(seen[0], 'GUI'); +}); + +test('origin: defaultOrigin descriptor overrides the global parent default', async () => { + const seen = []; + const reg = createRegistry([{ topic: 'set.x', defaultOrigin: 'fysical', handler: (_s, msg) => { seen.push(msg.origin); } }]); + await reg.dispatch({ topic: 'set.x' }, {}, {}); + assert.equal(seen[0], 'fysical'); +}); + +test('origin gating: non-gated command is never blocked, even with a mode model', async () => { + let invoked = false; + const reg = createRegistry([{ topic: 'set.x', handler: () => { invoked = true; } }]); + const source = { currentMode: 'fysicalControl', config: { mode: { allowedSources: { fysicalControl: ['fysical'] } } } }; + await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, source, {}); + assert.equal(invoked, true); +}); + +test('origin gating: gated command rejects an origin disallowed by the current mode', async () => { + const logger = makeLogger(); + let invoked = false; + const reg = createRegistry([{ topic: 'set.x', gated: true, handler: () => { invoked = true; } }], { logger }); + const source = { currentMode: 'fysicalControl', config: { mode: { allowedSources: { fysicalControl: ['fysical'] } } } }; + await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, source, {}); + assert.equal(invoked, false); + assert.ok(logger._calls.warn.some((m) => m.includes("origin 'GUI' not allowed in mode 'fysicalControl'"))); +}); + +test('origin gating: gated command accepts an allowed origin (Set or array allowedSources)', async () => { + let count = 0; + const reg = createRegistry([{ topic: 'set.x', gated: true, handler: () => { count += 1; } }]); + const arraySrc = { currentMode: 'auto', config: { mode: { allowedSources: { auto: ['parent', 'GUI', 'fysical'] } } } }; + const setSrc = { currentMode: 'auto', config: { mode: { allowedSources: { auto: new Set(['parent', 'GUI', 'fysical']) } } } }; + await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, arraySrc, {}); + await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, setSrc, {}); + assert.equal(count, 2); +}); + +test('origin gating: gated command on a node WITHOUT a mode model is advisory (allow-all)', async () => { + let invoked = false; + const reg = createRegistry([{ topic: 'set.x', gated: true, handler: () => { invoked = true; } }]); + await reg.dispatch({ topic: 'set.x', origin: 'GUI' }, { id: 'no-mode' }, {}); + assert.equal(invoked, true); +});