Skip to content

Commit e1c65e5

Browse files
fix(core): scope METER_VALUE_ANOMALY to cumulative energy registers per connector (#129)
The rule flattened every sampledValue across measurand, phase, unit, location and connector into one series and asserted monotonicity, but only cumulative Energy.*.Register measurands are monotonic and non-negative per OCPP 1.6 section 7.28. Any charge point reporting more than one measurand per sample, or any multi-connector station, produced a false warning 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. Genuine energy-register anomalies are still detected, so the scenario corpus and conformance contract are unchanged. Adds three regression tests covering shiv3's reproduction. Reported by shiv3 from the ocpp-cp-simulator integration.
1 parent 3cff9b1 commit e1c65e5

4 files changed

Lines changed: 202 additions & 50 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@ocpp-debugkit/toolkit': patch
3+
---
4+
5+
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.

CURRENT_STATE.md

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,35 @@
44
55
## Current Version
66

7-
`0.3.1` — Integrations & OSS Credibility (published)
7+
`0.4.0`, Open OCPP Trace Interop (published 2026-07-17). A `0.4.1` patch is in
8+
progress (METER_VALUE_ANOMALY false-positive fix, #127).
89

910
## Active Milestone
1011

11-
**v0.4.0 - Open OCPP Trace Interop (feature complete)**
12+
**v0.4.1 - detection correctness patch, then v0.5.0 (OCPP 2.0.1)**
1213

13-
v0.3.1 is published to npm (16 detection rules, 15 scenarios, trace diffing,
14-
rich scenario assertions, and the ci/anonymize/diff CLI commands). The interop
15-
milestone reads and writes the shared Open OCPP Trace v1.1 interchange format,
16-
checked against the specification's conformance fixtures: the input adapter
17-
(#121) and the exporter with the `convert` CLI command (#122) have both
18-
landed. Remaining: cut the `v0.4.0` release.
14+
v0.4.0 shipped the Open OCPP Trace interop: the toolkit reads and writes the
15+
shared v1.1 interchange format (input adapter #121, exporter + `convert` CLI
16+
#122), checked against the specification's conformance fixtures. **0.4.1**
17+
fixes a false-positive bug shiv3 found from the simulator integration: the
18+
`METER_VALUE_ANOMALY` rule flattened all measurands and connectors into one
19+
series (#127). Next milestone is **v0.5.0 (OCPP 2.0.1)**, where the same
20+
per-connector/EVSE model this fix introduces is required across detection.
1921

2022
## What's Done
2123

24+
### METER_VALUE_ANOMALY correctness fix (0.4.1, Issue #127)
25+
26+
- ✅ Rule 14 now buckets readings by `(connectorId, measurand, phase, unit,
27+
location)` and applies the monotonic + non-negative checks only to cumulative
28+
`Energy.*.Register` measurands (absent `measurand` defaults to
29+
`Energy.Active.Import.Register`); other measurands are ignored
30+
- ✅ Eliminates false positives on multi-measurand samples and multi-connector
31+
stations; genuine energy-register anomalies still detected (meter-anomaly
32+
scenario and conformance contract unchanged); 3 regression tests added
33+
- Companion issue drafted for the same connector-blindness in
34+
`STATUS_TRANSITION_VIOLATION` (rule 8)
35+
2236
### GitHub Infrastructure
2337

2438
- ✅ GitHub milestones created (M0, M0.5, v0.1.0, v0.2.0, v0.3.0, v1.0.0)

packages/toolkit/src/core/detection.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,6 +1418,100 @@ describe('detectFailures', () => {
14181418
const failures = detectFailures(events, sessions);
14191419
expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false);
14201420
});
1421+
1422+
// Regression tests for the per-(connector, measurand) bucketing fix (#127).
1423+
const meterSession = (meterEvents: Event[]): Event[] => [
1424+
makeEvent('s1', 'ms', 'Call', 'StartTransaction', { connectorId: 1, idTag: 'TAG-127' }, 1000),
1425+
makeEvent(
1426+
's2',
1427+
'ms',
1428+
'CallResult',
1429+
null,
1430+
{ idTagInfo: { status: 'Accepted' }, transactionId: 42 },
1431+
1500,
1432+
'CSMS_TO_CS',
1433+
),
1434+
...meterEvents,
1435+
makeEvent(
1436+
's9',
1437+
'me',
1438+
'Call',
1439+
'StopTransaction',
1440+
{ transactionId: 42, reason: 'Local' },
1441+
9000,
1442+
),
1443+
];
1444+
1445+
const energyPlusPower = (id: string, ts: number, energy: string): Event =>
1446+
makeEvent(
1447+
id,
1448+
id,
1449+
'Call',
1450+
'MeterValues',
1451+
{
1452+
connectorId: 1,
1453+
transactionId: 42,
1454+
meterValue: [
1455+
{
1456+
sampledValue: [
1457+
{ measurand: 'Energy.Active.Import.Register', value: energy, unit: 'Wh' },
1458+
{ measurand: 'Power.Active.Import', value: '3000', unit: 'W' },
1459+
],
1460+
},
1461+
],
1462+
},
1463+
ts,
1464+
);
1465+
1466+
it('does not flag a rising Energy register interleaved with a constant Power sample (#127)', () => {
1467+
const events = meterSession([
1468+
energyPlusPower('a1', 2000, '600'),
1469+
energyPlusPower('a2', 3000, '625'),
1470+
energyPlusPower('a3', 4000, '650'),
1471+
]);
1472+
const failures = detectFailures(events, buildSessionTimeline(events));
1473+
expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false);
1474+
});
1475+
1476+
it('still flags a decreasing Energy register when other measurands are present', () => {
1477+
const events = meterSession([
1478+
energyPlusPower('b1', 2000, '600'),
1479+
energyPlusPower('b2', 3000, '500'),
1480+
]);
1481+
const anomalies = detectFailures(events, buildSessionTimeline(events)).filter(
1482+
(f) => f.code === 'METER_VALUE_ANOMALY',
1483+
);
1484+
expect(anomalies).toHaveLength(1);
1485+
});
1486+
1487+
it('does not flag two connector registers that share a transaction (bucketed by connectorId)', () => {
1488+
const meter = (id: string, ts: number, connectorId: number, energy: string): Event =>
1489+
makeEvent(
1490+
id,
1491+
id,
1492+
'Call',
1493+
'MeterValues',
1494+
{
1495+
connectorId,
1496+
transactionId: 42,
1497+
meterValue: [
1498+
{
1499+
sampledValue: [
1500+
{ measurand: 'Energy.Active.Import.Register', value: energy, unit: 'Wh' },
1501+
],
1502+
},
1503+
],
1504+
},
1505+
ts,
1506+
);
1507+
const events = meterSession([
1508+
meter('c1', 2000, 1, '6000'),
1509+
meter('c2', 3000, 2, '100'),
1510+
meter('c3', 4000, 1, '6100'),
1511+
]);
1512+
const failures = detectFailures(events, buildSessionTimeline(events));
1513+
expect(failures.some((f) => f.code === 'METER_VALUE_ANOMALY')).toBe(false);
1514+
});
14211515
});
14221516

14231517
describe('UNRESPONSIVE_CSMS', () => {

packages/toolkit/src/core/detection.ts

Lines changed: 81 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -883,34 +883,64 @@ function detectHeartbeatIntervalViolation(events: Event[]): Failure[] {
883883
return failures;
884884
}
885885

886+
/**
887+
* Cumulative energy registers, the only measurands with a monotonic,
888+
* non-negative invariant per OCPP 1.6 section 7.28.
889+
*/
890+
const CUMULATIVE_MEASURANDS = new Set([
891+
'Energy.Active.Import.Register',
892+
'Energy.Reactive.Import.Register',
893+
'Energy.Active.Export.Register',
894+
'Energy.Reactive.Export.Register',
895+
]);
896+
897+
/** OCPP 1.6: when `measurand` is absent it defaults to Energy.Active.Import.Register. */
898+
const DEFAULT_MEASURAND = 'Energy.Active.Import.Register';
899+
886900
/**
887901
* Rule 14: METER_VALUE_ANOMALY
888-
* Detects non-monotonic (decreasing) or negative meter readings during
889-
* an active transaction.
902+
* Flags a cumulative energy register that decreases, or is negative, during an
903+
* active transaction.
904+
*
905+
* Only the cumulative `Energy.*.Register` measurands are monotonic and
906+
* non-negative (OCPP 1.6 section 7.28). Other measurands (Power, Current,
907+
* Voltage, Temperature, SoC, ...) legitimately rise and fall, so this rule
908+
* ignores them. Readings are bucketed by (connectorId, measurand, phase, unit,
909+
* location) so independent series never contaminate each other's monotonicity,
910+
* for example a constant Power sample interleaved with a rising Energy register,
911+
* or two connectors' meters on the same transaction.
890912
*/
891913
function detectMeterValueAnomaly(_events: Event[], sessions: Session[]): Failure[] {
892914
const failures: Failure[] = [];
893915

894916
for (const session of sessions) {
895917
if (session.transactionId === null) continue;
896918

897-
// Collect meter values from MeterValues calls in order
898919
const meterEvents = session.events.filter(
899920
(e) => e.messageType === 'Call' && e.action === 'MeterValues',
900921
);
901-
902922
if (meterEvents.length === 0) continue;
903923

904-
// Extract numeric meter values from the payload
905-
// OCPP 1.6 MeterValues: { connectorId, transactionId, meterValue: [{ timestamp, sampledValue: [{ value, ... }] }] }
906-
const readings: { eventId: string; value: number }[] = [];
924+
// Collect cumulative-register readings, in event order, into per-series
925+
// buckets so cross-measurand and cross-connector values never mix.
926+
// OCPP 1.6 MeterValues: { connectorId, transactionId, meterValue: [{ timestamp, sampledValue: [...] }] }
927+
const buckets = new Map<string, { eventId: string; value: number }[]>();
907928

908929
for (const event of meterEvents) {
909930
const payload = event.payload as {
931+
connectorId?: unknown;
910932
meterValue?: {
911-
sampledValue?: { value?: unknown }[];
933+
sampledValue?: {
934+
value?: unknown;
935+
measurand?: unknown;
936+
phase?: unknown;
937+
unit?: unknown;
938+
location?: unknown;
939+
}[];
912940
}[];
913941
};
942+
const connectorId =
943+
typeof payload?.connectorId === 'number' ? payload.connectorId : 'unknown';
914944

915945
const meterValues = payload?.meterValue;
916946
if (!Array.isArray(meterValues)) continue;
@@ -920,48 +950,57 @@ function detectMeterValueAnomaly(_events: Event[], sessions: Session[]): Failure
920950
if (!Array.isArray(sampledValues)) continue;
921951

922952
for (const sv of sampledValues) {
953+
const measurand = typeof sv?.measurand === 'string' ? sv.measurand : DEFAULT_MEASURAND;
954+
if (!CUMULATIVE_MEASURANDS.has(measurand)) continue;
955+
923956
const rawValue = sv?.value;
924-
if (typeof rawValue === 'string') {
925-
const numValue = Number.parseFloat(rawValue);
926-
if (!Number.isNaN(numValue)) {
927-
readings.push({ eventId: event.id, value: numValue });
928-
}
929-
} else if (typeof rawValue === 'number') {
930-
readings.push({ eventId: event.id, value: rawValue });
931-
}
957+
let numValue: number;
958+
if (typeof rawValue === 'string') numValue = Number.parseFloat(rawValue);
959+
else if (typeof rawValue === 'number') numValue = rawValue;
960+
else continue;
961+
if (Number.isNaN(numValue)) continue;
962+
963+
const phase = typeof sv?.phase === 'string' ? sv.phase : '';
964+
const unit = typeof sv?.unit === 'string' ? sv.unit : '';
965+
const location = typeof sv?.location === 'string' ? sv.location : '';
966+
const key = `${connectorId}|${measurand}|${phase}|${unit}|${location}`;
967+
968+
const bucket = buckets.get(key);
969+
if (bucket) bucket.push({ eventId: event.id, value: numValue });
970+
else buckets.set(key, [{ eventId: event.id, value: numValue }]);
932971
}
933972
}
934973
}
935974

936-
if (readings.length === 0) continue;
937-
938-
// Check for negative values
939-
for (const reading of readings) {
940-
if (reading.value < 0) {
941-
failures.push({
942-
code: 'METER_VALUE_ANOMALY',
943-
description: `Negative meter value detected: ${reading.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
944-
severity: SEVERITY.METER_VALUE_ANOMALY,
945-
eventIds: [reading.eventId],
946-
suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
947-
});
975+
for (const readings of buckets.values()) {
976+
// A cumulative register cannot be negative.
977+
for (const reading of readings) {
978+
if (reading.value < 0) {
979+
failures.push({
980+
code: 'METER_VALUE_ANOMALY',
981+
description: `Negative meter value detected: ${reading.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
982+
severity: SEVERITY.METER_VALUE_ANOMALY,
983+
eventIds: [reading.eventId],
984+
suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
985+
});
986+
}
948987
}
949-
}
950988

951-
// Check for non-monotonic (decreasing) values
952-
for (let i = 1; i < readings.length; i++) {
953-
const prev = readings[i - 1];
954-
const curr = readings[i];
955-
if (!prev || !curr) continue;
989+
// A cumulative register must not decrease.
990+
for (let i = 1; i < readings.length; i++) {
991+
const prev = readings[i - 1];
992+
const curr = readings[i];
993+
if (!prev || !curr) continue;
956994

957-
if (curr.value < prev.value) {
958-
failures.push({
959-
code: 'METER_VALUE_ANOMALY',
960-
description: `Non-monotonic meter reading: value decreased from ${prev.value} to ${curr.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
961-
severity: SEVERITY.METER_VALUE_ANOMALY,
962-
eventIds: [prev.eventId, curr.eventId],
963-
suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
964-
});
995+
if (curr.value < prev.value) {
996+
failures.push({
997+
code: 'METER_VALUE_ANOMALY',
998+
description: `Non-monotonic meter reading: value decreased from ${prev.value} to ${curr.value} in session ${session.sessionId} (transaction ${session.transactionId})`,
999+
severity: SEVERITY.METER_VALUE_ANOMALY,
1000+
eventIds: [prev.eventId, curr.eventId],
1001+
suggestedSteps: SUGGESTED_STEPS.METER_VALUE_ANOMALY,
1002+
});
1003+
}
9651004
}
9661005
}
9671006
}

0 commit comments

Comments
 (0)