diff --git a/.changeset/fix-meter-value-anomaly-bucketing.md b/.changeset/fix-meter-value-anomaly-bucketing.md new file mode 100644 index 0000000..5408ac4 --- /dev/null +++ b/.changeset/fix-meter-value-anomaly-bucketing.md @@ -0,0 +1,5 @@ +--- +'@ocpp-debugkit/toolkit': patch +--- + +Fix `METER_VALUE_ANOMALY` false positives (#127). The rule flattened every `sampledValue` across measurand, phase, unit, location and connector into a single series and asserted monotonicity, but only cumulative `Energy.*.Register` measurands are monotonic and non-negative per OCPP 1.6 section 7.28, so any charge point reporting more than one measurand per sample, or any multi-connector station, produced warnings on nearly every `MeterValues` message. Readings are now bucketed by `(connectorId, measurand, phase, unit, location)` and the monotonic and non-negative checks apply only to cumulative energy registers (an absent `measurand` defaults to `Energy.Active.Import.Register`); other measurands are ignored by this rule. Reported by shiv3 from the ocpp-cp-simulator integration. diff --git a/CURRENT_STATE.md b/CURRENT_STATE.md index d9f2662..33e6993 100644 --- a/CURRENT_STATE.md +++ b/CURRENT_STATE.md @@ -4,21 +4,35 @@ ## Current Version -`0.3.1` — Integrations & OSS Credibility (published) +`0.4.0`, Open OCPP Trace Interop (published 2026-07-17). A `0.4.1` patch is in +progress (METER_VALUE_ANOMALY false-positive fix, #127). ## Active Milestone -**v0.4.0 - Open OCPP Trace Interop (feature complete)** +**v0.4.1 - detection correctness patch, then v0.5.0 (OCPP 2.0.1)** -v0.3.1 is published to npm (16 detection rules, 15 scenarios, trace diffing, -rich scenario assertions, and the ci/anonymize/diff CLI commands). The interop -milestone reads and writes the shared Open OCPP Trace v1.1 interchange format, -checked against the specification's conformance fixtures: the input adapter -(#121) and the exporter with the `convert` CLI command (#122) have both -landed. Remaining: cut the `v0.4.0` release. +v0.4.0 shipped the Open OCPP Trace interop: the toolkit reads and writes the +shared v1.1 interchange format (input adapter #121, exporter + `convert` CLI +#122), checked against the specification's conformance fixtures. **0.4.1** +fixes a false-positive bug shiv3 found from the simulator integration: the +`METER_VALUE_ANOMALY` rule flattened all measurands and connectors into one +series (#127). Next milestone is **v0.5.0 (OCPP 2.0.1)**, where the same +per-connector/EVSE model this fix introduces is required across detection. ## What's Done +### METER_VALUE_ANOMALY correctness fix (0.4.1, Issue #127) + +- ✅ Rule 14 now buckets readings by `(connectorId, measurand, phase, unit, + location)` and applies the monotonic + non-negative checks only to cumulative + `Energy.*.Register` measurands (absent `measurand` defaults to + `Energy.Active.Import.Register`); other measurands are ignored +- ✅ Eliminates false positives on multi-measurand samples and multi-connector + stations; genuine energy-register anomalies still detected (meter-anomaly + scenario and conformance contract unchanged); 3 regression tests added +- Companion issue drafted for the same connector-blindness in + `STATUS_TRANSITION_VIOLATION` (rule 8) + ### GitHub Infrastructure - ✅ GitHub milestones created (M0, M0.5, v0.1.0, v0.2.0, v0.3.0, v1.0.0) diff --git a/packages/toolkit/src/core/detection.test.ts b/packages/toolkit/src/core/detection.test.ts index 9149c00..a47c9a8 100644 --- a/packages/toolkit/src/core/detection.test.ts +++ b/packages/toolkit/src/core/detection.test.ts @@ -1418,6 +1418,100 @@ describe('detectFailures', () => { const failures = detectFailures(events, sessions); expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false); }); + + // Regression tests for the per-(connector, measurand) bucketing fix (#127). + const meterSession = (meterEvents: Event[]): Event[] => [ + makeEvent('s1', 'ms', 'Call', 'StartTransaction', { connectorId: 1, idTag: 'TAG-127' }, 1000), + makeEvent( + 's2', + 'ms', + 'CallResult', + null, + { idTagInfo: { status: 'Accepted' }, transactionId: 42 }, + 1500, + 'CSMS_TO_CS', + ), + ...meterEvents, + makeEvent( + 's9', + 'me', + 'Call', + 'StopTransaction', + { transactionId: 42, reason: 'Local' }, + 9000, + ), + ]; + + const energyPlusPower = (id: string, ts: number, energy: string): Event => + makeEvent( + id, + id, + 'Call', + 'MeterValues', + { + connectorId: 1, + transactionId: 42, + meterValue: [ + { + sampledValue: [ + { measurand: 'Energy.Active.Import.Register', value: energy, unit: 'Wh' }, + { measurand: 'Power.Active.Import', value: '3000', unit: 'W' }, + ], + }, + ], + }, + ts, + ); + + it('does not flag a rising Energy register interleaved with a constant Power sample (#127)', () => { + const events = meterSession([ + energyPlusPower('a1', 2000, '600'), + energyPlusPower('a2', 3000, '625'), + energyPlusPower('a3', 4000, '650'), + ]); + const failures = detectFailures(events, buildSessionTimeline(events)); + expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false); + }); + + it('still flags a decreasing Energy register when other measurands are present', () => { + const events = meterSession([ + energyPlusPower('b1', 2000, '600'), + energyPlusPower('b2', 3000, '500'), + ]); + const anomalies = detectFailures(events, buildSessionTimeline(events)).filter( + (f) => f.code === 'METER_VALUE_ANOMALY', + ); + expect(anomalies).toHaveLength(1); + }); + + it('does not flag two connector registers that share a transaction (bucketed by connectorId)', () => { + const meter = (id: string, ts: number, connectorId: number, energy: string): Event => + makeEvent( + id, + id, + 'Call', + 'MeterValues', + { + connectorId, + transactionId: 42, + meterValue: [ + { + sampledValue: [ + { measurand: 'Energy.Active.Import.Register', value: energy, unit: 'Wh' }, + ], + }, + ], + }, + ts, + ); + const events = meterSession([ + meter('c1', 2000, 1, '6000'), + meter('c2', 3000, 2, '100'), + meter('c3', 4000, 1, '6100'), + ]); + const failures = detectFailures(events, buildSessionTimeline(events)); + expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false); + }); }); describe('UNRESPONSIVE_CSMS', () => { diff --git a/packages/toolkit/src/core/detection.ts b/packages/toolkit/src/core/detection.ts index 62b9f7a..c418480 100644 --- a/packages/toolkit/src/core/detection.ts +++ b/packages/toolkit/src/core/detection.ts @@ -883,10 +883,32 @@ function detectHeartbeatIntervalViolation(events: Event[]): Failure[] { return failures; } +/** + * Cumulative energy registers, the only measurands with a monotonic, + * non-negative invariant per OCPP 1.6 section 7.28. + */ +const CUMULATIVE_MEASURANDS = new Set([ + 'Energy.Active.Import.Register', + 'Energy.Reactive.Import.Register', + 'Energy.Active.Export.Register', + 'Energy.Reactive.Export.Register', +]); + +/** OCPP 1.6: when `measurand` is absent it defaults to Energy.Active.Import.Register. */ +const DEFAULT_MEASURAND = 'Energy.Active.Import.Register'; + /** * Rule 14: METER_VALUE_ANOMALY - * Detects non-monotonic (decreasing) or negative meter readings during - * an active transaction. + * Flags a cumulative energy register that decreases, or is negative, during an + * active transaction. + * + * Only the cumulative `Energy.*.Register` measurands are monotonic and + * non-negative (OCPP 1.6 section 7.28). Other measurands (Power, Current, + * Voltage, Temperature, SoC, ...) legitimately rise and fall, so this rule + * ignores them. Readings are bucketed by (connectorId, measurand, phase, unit, + * location) so independent series never contaminate each other's monotonicity, + * for example a constant Power sample interleaved with a rising Energy register, + * or two connectors' meters on the same transaction. */ function detectMeterValueAnomaly(_events: Event[], sessions: Session[]): Failure[] { const failures: Failure[] = []; @@ -894,23 +916,31 @@ function detectMeterValueAnomaly(_events: Event[], sessions: Session[]): Failure for (const session of sessions) { if (session.transactionId === null) continue; - // Collect meter values from MeterValues calls in order const meterEvents = session.events.filter( (e) => e.messageType === 'Call' && e.action === 'MeterValues', ); - if (meterEvents.length === 0) continue; - // Extract numeric meter values from the payload - // OCPP 1.6 MeterValues: { connectorId, transactionId, meterValue: [{ timestamp, sampledValue: [{ value, ... }] }] } - const readings: { eventId: string; value: number }[] = []; + // Collect cumulative-register readings, in event order, into per-series + // buckets so cross-measurand and cross-connector values never mix. + // OCPP 1.6 MeterValues: { connectorId, transactionId, meterValue: [{ timestamp, sampledValue: [...] }] } + const buckets = new Map(); for (const event of meterEvents) { const payload = event.payload as { + connectorId?: unknown; meterValue?: { - sampledValue?: { value?: unknown }[]; + sampledValue?: { + value?: unknown; + measurand?: unknown; + phase?: unknown; + unit?: unknown; + location?: unknown; + }[]; }[]; }; + const connectorId = + typeof payload?.connectorId === 'number' ? payload.connectorId : 'unknown'; const meterValues = payload?.meterValue; if (!Array.isArray(meterValues)) continue; @@ -920,48 +950,57 @@ function detectMeterValueAnomaly(_events: Event[], sessions: Session[]): Failure if (!Array.isArray(sampledValues)) continue; for (const sv of sampledValues) { + const measurand = typeof sv?.measurand === 'string' ? sv.measurand : DEFAULT_MEASURAND; + if (!CUMULATIVE_MEASURANDS.has(measurand)) continue; + const rawValue = sv?.value; - if (typeof rawValue === 'string') { - const numValue = Number.parseFloat(rawValue); - if (!Number.isNaN(numValue)) { - readings.push({ eventId: event.id, value: numValue }); - } - } else if (typeof rawValue === 'number') { - readings.push({ eventId: event.id, value: rawValue }); - } + let numValue: number; + if (typeof rawValue === 'string') numValue = Number.parseFloat(rawValue); + else if (typeof rawValue === 'number') numValue = rawValue; + else continue; + if (Number.isNaN(numValue)) continue; + + const phase = typeof sv?.phase === 'string' ? sv.phase : ''; + const unit = typeof sv?.unit === 'string' ? sv.unit : ''; + const location = typeof sv?.location === 'string' ? sv.location : ''; + const key = `${connectorId}|${measurand}|${phase}|${unit}|${location}`; + + const bucket = buckets.get(key); + if (bucket) bucket.push({ eventId: event.id, value: numValue }); + else buckets.set(key, [{ eventId: event.id, value: numValue }]); } } } - if (readings.length === 0) continue; - - // Check for negative values - for (const reading of readings) { - if (reading.value < 0) { - failures.push({ - code: 'METER_VALUE_ANOMALY', - description: `Negative meter value detected: ${reading.value} in session ${session.sessionId} (transaction ${session.transactionId})`, - severity: SEVERITY.METER_VALUE_ANOMALY, - eventIds: [reading.eventId], - suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY, - }); + for (const readings of buckets.values()) { + // A cumulative register cannot be negative. + for (const reading of readings) { + if (reading.value < 0) { + failures.push({ + code: 'METER_VALUE_ANOMALY', + description: `Negative meter value detected: ${reading.value} in session ${session.sessionId} (transaction ${session.transactionId})`, + severity: SEVERITY.METER_VALUE_ANOMALY, + eventIds: [reading.eventId], + suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY, + }); + } } - } - // Check for non-monotonic (decreasing) values - for (let i = 1; i < readings.length; i++) { - const prev = readings[i - 1]; - const curr = readings[i]; - if (!prev || !curr) continue; + // A cumulative register must not decrease. + for (let i = 1; i < readings.length; i++) { + const prev = readings[i - 1]; + const curr = readings[i]; + if (!prev || !curr) continue; - if (curr.value < prev.value) { - failures.push({ - code: 'METER_VALUE_ANOMALY', - description: `Non-monotonic meter reading: value decreased from ${prev.value} to ${curr.value} in session ${session.sessionId} (transaction ${session.transactionId})`, - severity: SEVERITY.METER_VALUE_ANOMALY, - eventIds: [prev.eventId, curr.eventId], - suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY, - }); + if (curr.value < prev.value) { + failures.push({ + code: 'METER_VALUE_ANOMALY', + description: `Non-monotonic meter reading: value decreased from ${prev.value} to ${curr.value} in session ${session.sessionId} (transaction ${session.transactionId})`, + severity: SEVERITY.METER_VALUE_ANOMALY, + eventIds: [prev.eventId, curr.eventId], + suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY, + }); + } } } }