From ab1eed7210bfae0a882c106983a005cc0dedb2aa Mon Sep 17 00:00:00 2001 From: sepehr-safari Date: Thu, 30 Jul 2026 12:51:56 +0300 Subject: [PATCH] fix(ocpp): align three detection rules with the OCPP 1.6 spec (#92) The same three mistakes the toolkit had, fixed there in toolkit#154/#155/#156 and published as toolkit 0.4.5. Studio still had all three, so the two engines disagreed everywhere outside the pinned corpus. FIRMWARE_UPDATE_FAILURE matched DownloadPaused, InstallFailed and InstallRebootingFailed, none of which are values in the 1.6 FirmwareStatus enumeration (edition 2, section 7.25), and did not match InstallationFailed, which is one of the two failure values 1.6 defines. A conformant station reporting a failed install produced nothing, which is the direction that costs the user. The set is now exactly DownloadFailed and InstallationFailed. isValidTransition disagreed with the section 4.9 transition table in both directions: it flagged 22 transitions the table permits and permitted 2 it omits. The false positives sat mostly in the recovery rows, where I1-I8 lets a connector return from Faulted to any pre-fault state and the H row lets one come back from Unavailable into any operative state, so a station that faulted mid-session and resumed charging, or that took a scheduled availability change during a session, was told its firmware was wrong when it was right. Preparing to Unavailable and Finishing to Reserved have no cell in the table and are now reported. The matrix was a single 700-column expression, which is how it drifted unnoticed; it is now a row-per-status table carrying the spec's own cell labels, so a row can be checked against section 4.9 rather than against intuition. FAILED_AUTHORIZATION fired on Invalid alone, while the 1.6 AuthorizationStatus enumeration (section 7.2) has five values of which only Accepted permits charging. Blocked, Expired and ConcurrentTx are refusals with the same shape on the wire and the same outcome for the driver. All four now report, with the status named in the description. contract-v1 is untouched: no fixture or golden edited, and the harness is 15/15 under native test and studio ci. Parity was checked against the published toolkit 0.4.5 directly rather than only through the corpus: both engines return identical failure-code sets on 97 probe traces covering 10 firmware statuses, 6 authorization statuses, and all 81 ordered ChargePointStatus pairs. Adds three tests, one of which transcribes the section 4.9 table independently and walks every ordered pair, which is the guard that was missing. Two adjacent questions are left alone so this stays a straight port: a repeated identical status is still a violation, since the table has no diagonal, though a TriggerMessage-driven StatusNotification legitimately repeats the current status; and connectorId 0 is still checked against the full table rather than the narrower set section 4.9 allows it. --- CURRENT_STATE.md | 33 +++++++ src/ocpp/detection.zig | 201 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 222 insertions(+), 12 deletions(-) diff --git a/CURRENT_STATE.md b/CURRENT_STATE.md index c6ea678..681ab68 100644 --- a/CURRENT_STATE.md +++ b/CURRENT_STATE.md @@ -39,6 +39,39 @@ workflow, which builds and attaches the macOS + Linux packages. Next themes ## What's done +### OCPP 1.6 spec-conformance parity (Issue #92) + +Three rules read the 1.6 spec wrongly in the same way the toolkit's did, fixed +there in toolkit#154/#155/#156 and published as toolkit 0.4.5. Ported here so the +two engines stay equivalent: + +- **`FIRMWARE_UPDATE_FAILURE`** matches exactly the two failure values of the + `FirmwareStatus` enumeration (edition 2, section 7.25), `DownloadFailed` and + `InstallationFailed`. It previously matched `DownloadPaused`, `InstallFailed` + and `InstallRebootingFailed`, none of them 1.6 values, and missed + `InstallationFailed`, so a conformant failed install went undetected. +- **`STATUS_TRANSITION_VIOLATION`** uses the section 4.9 transition table + transcribed cell by cell, 53 permitted transitions, with the spec's cell labels + alongside each row. The old matrix flagged 22 transitions the table permits, + most of them in the `Faulted` and `Unavailable` recovery rows, and permitted 2 + it omits (`Preparing -> Unavailable`, `Finishing -> Reserved`). +- **`FAILED_AUTHORIZATION`** reports every refusing `AuthorizationStatus` + (section 7.2): `Blocked`, `Expired`, `ConcurrentTx` alongside `Invalid`, with + the status named in the description. It fired on `Invalid` alone. + +`contract-v1` is untouched: no fixture or golden edited, and the harness is +15/15 under `native test` and `studio ci`. Verified against the published +toolkit 0.4.5 directly, not just through the corpus: both engines return +identical failure-code sets on 97 probe traces covering 10 firmware statuses, +6 authorization statuses, and all 81 ordered `ChargePointStatus` pairs. Three +tests added, one of them transcribing the section 4.9 table independently so the +matrix cannot drift again without a named failure. + +Deliberately unchanged, each its own issue if wanted: a repeated identical +status is still a violation (the table has no diagonal, though a +`TriggerMessage`-driven `StatusNotification` legitimately repeats), and +connectorId 0 is not checked against its narrower applicable set. + ### S0 — Foundation ✅ - **Repository genesis** — Apache-2.0 `LICENSE`, `.gitignore`. diff --git a/src/ocpp/detection.zig b/src/ocpp/detection.zig index 012b18c..796190f 100644 --- a/src/ocpp/detection.zig +++ b/src/ocpp/detection.zig @@ -275,7 +275,16 @@ pub fn detectFailures(arena: std.mem.Allocator, events: []const Event, sessions: // Rule 1: FAILED_AUTHORIZATION // --------------------------------------------------------------------------- -/// An Authorize response whose idTagInfo.status is "Invalid". +/// The refusing values of the OCPP 1.6 AuthorizationStatus enumeration (edition +/// 2, section 7.2). The enumeration has five values and only Accepted permits +/// charging: Blocked and Expired refuse a known identifier, Invalid means the +/// identifier is unknown, and ConcurrentTx means it is already in another +/// transaction. Section 7.2 marks ConcurrentTx as only relevant to +/// StartTransaction.req, so seeing it on an Authorize response is itself +/// irregular, but it is still a refusal. +const authorization_refusal_statuses = [_][]const u8{ "Blocked", "Expired", "Invalid", "ConcurrentTx" }; + +/// An Authorize response whose idTagInfo.status refuses charging. fn detectFailedAuthorization(arena: std.mem.Allocator, events: []const Event, list: *FailureList) !void { for (events) |event| { if (event.message_type != .call_result) continue; @@ -291,11 +300,11 @@ fn detectFailedAuthorization(arena: std.mem.Allocator, events: []const Event, li const call = matching orelse continue; const status = authorizeStatus(event) orelse continue; - if (std.mem.eql(u8, status, "Invalid")) { + if (inList(&authorization_refusal_statuses, status)) { const desc = try std.fmt.allocPrint( arena, - "Authorization rejected: idTag returned \"Invalid\" status (messageId: {s})", - .{event.message_id}, + "Authorization rejected: idTag returned \"{s}\" status (messageId: {s})", + .{ status, event.message_id }, ); try add(arena, list, .failed_authorization, desc, try ids2(arena, call.id, event.id)); } @@ -421,17 +430,62 @@ const valid_connector_statuses = [_][]const u8{ "Finishing", "Reserved", "Unavailable", "Faulted", }; -/// FirmwareStatusNotification statuses that indicate a failed update. -const firmware_failure_statuses = [_][]const u8{ - "DownloadFailed", "DownloadPaused", "InstallFailed", "InstallRebootingFailed", +/// The two failure values of the OCPP 1.6 FirmwareStatus enumeration (edition 2, +/// section 7.25). The full enumeration is Downloaded, DownloadFailed, +/// Downloading, Idle, InstallationFailed, Installing, Installed; the rest are +/// progress or success states. Values from later OCPP generations (for example +/// DownloadPaused, which 2.0.1 defines as an intermediate state rather than a +/// failure) are deliberately not matched. +const firmware_failure_statuses = [_][]const u8{ "DownloadFailed", "InstallationFailed" }; + +/// The status transition table of OCPP 1.6 edition 2, section 4.9, transcribed +/// cell by cell: 53 permitted transitions across the nine ChargePointStatus +/// values. Each row lists its targets in the table's column order (Available, +/// Preparing, Charging, SuspendedEV, SuspendedEVSE, Finishing, Reserved, +/// Unavailable, Faulted) and carries the table's own cell labels, so a row can be +/// checked against the spec rather than against intuition. Keep it that way: this +/// matrix drifted from the table once already. +/// +/// Two properties of the table worth knowing before reading a violation: +/// +/// - The Faulted row (I1-I8) permits recovery to any pre-fault state, and the +/// Unavailable row permits resuming directly into an operative state, so a +/// station that faults mid-session and resumes is conformant. +/// - The table has no diagonal, so a repeated identical status counts as a +/// violation. A charge point answering a TriggerMessage for +/// StatusNotification legitimately repeats its current status. +/// +/// The table applies to connectorId > 0. Section 4.9 limits connectorId 0 to +/// Available, Unavailable and Faulted; every transition among those three is in +/// the table, so one matrix serves both. +const transition_table = [_]struct { from: []const u8, to: []const []const u8 }{ + // A: A2, A3, A4, A5, A7, A8, A9 + .{ .from = "Available", .to = &.{ "Preparing", "Charging", "SuspendedEV", "SuspendedEVSE", "Reserved", "Unavailable", "Faulted" } }, + // B: B1, B3, B4, B5, B6, B9 + .{ .from = "Preparing", .to = &.{ "Available", "Charging", "SuspendedEV", "SuspendedEVSE", "Finishing", "Faulted" } }, + // C: C1, C4, C5, C6, C8, C9 + .{ .from = "Charging", .to = &.{ "Available", "SuspendedEV", "SuspendedEVSE", "Finishing", "Unavailable", "Faulted" } }, + // D: D1, D3, D5, D6, D8, D9 + .{ .from = "SuspendedEV", .to = &.{ "Available", "Charging", "SuspendedEVSE", "Finishing", "Unavailable", "Faulted" } }, + // E: E1, E3, E4, E6, E8, E9 + .{ .from = "SuspendedEVSE", .to = &.{ "Available", "Charging", "SuspendedEV", "Finishing", "Unavailable", "Faulted" } }, + // F: F1, F2, F8, F9 + .{ .from = "Finishing", .to = &.{ "Available", "Preparing", "Unavailable", "Faulted" } }, + // G: G1, G2, G8, G9 + .{ .from = "Reserved", .to = &.{ "Available", "Preparing", "Unavailable", "Faulted" } }, + // H: H1, H2, H3, H4, H5, H9 + .{ .from = "Unavailable", .to = &.{ "Available", "Preparing", "Charging", "SuspendedEV", "SuspendedEVSE", "Faulted" } }, + // I: I1 through I8 + .{ .from = "Faulted", .to = &.{ "Available", "Preparing", "Charging", "SuspendedEV", "SuspendedEVSE", "Finishing", "Reserved", "Unavailable" } }, }; -/// The OCPP 1.6 connector state model: allowed successor statuses per status. +/// Whether section 4.9 lists a transition from `from` to `to`. /// An unknown predecessor imposes no constraint (matches the contract). fn isValidTransition(from: []const u8, to: []const u8) bool { - const allowed: []const []const u8 = - if (std.mem.eql(u8, from, "Available")) &.{ "Preparing", "Charging", "Reserved", "Unavailable", "Faulted" } else if (std.mem.eql(u8, from, "Preparing")) &.{ "Charging", "Available", "SuspendedEVSE", "Faulted", "Unavailable" } else if (std.mem.eql(u8, from, "Charging")) &.{ "SuspendedEVSE", "SuspendedEV", "Finishing", "Available", "Faulted" } else if (std.mem.eql(u8, from, "SuspendedEVSE")) &.{ "Charging", "Finishing", "Available", "Faulted" } else if (std.mem.eql(u8, from, "SuspendedEV")) &.{ "Charging", "Finishing", "Available", "Faulted" } else if (std.mem.eql(u8, from, "Finishing")) &.{ "Available", "Reserved", "Faulted" } else if (std.mem.eql(u8, from, "Reserved")) &.{ "Available", "Unavailable", "Faulted" } else if (std.mem.eql(u8, from, "Unavailable")) &.{ "Available", "Faulted" } else if (std.mem.eql(u8, from, "Faulted")) &.{ "Unavailable", "Available" } else return true; - return inList(allowed, to); + for (transition_table) |row| { + if (std.mem.eql(u8, from, row.from)) return inList(row.to, to); + } + return true; } /// The heartbeat interval (ms) from a BootNotification's response, else default. @@ -1007,6 +1061,39 @@ test "FAILED_AUTHORIZATION fires on an Invalid Authorize response" { try testing.expectEqual(@as(usize, 0), accepted.len); } +test "FAILED_AUTHORIZATION fires on every refusing AuthorizationStatus" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + // OCPP 1.6 edition 2, section 7.2: five values, only Accepted permits + // charging. The rule used to fire on Invalid alone. + for ([_][]const u8{ "Invalid", "Blocked", "Expired", "ConcurrentTx" }) |status| { + const json = try std.fmt.allocPrint( + a, + "{{\"events\":[" ++ + "{{\"message\":[2,\"m1\",\"Authorize\",{{\"idTag\":\"TAG-BAD\"}}]}}," ++ + "{{\"message\":[3,\"m1\",{{\"idTagInfo\":{{\"status\":\"{s}\"}}}}]}}]}}", + .{status}, + ); + const res = try detect(a, json); + try testing.expectEqual(@as(usize, 1), res.len); + try testing.expectEqual(FailureCode.failed_authorization, res[0].code); + try testing.expectEqual(FailureSeverity.warning, res[0].severity); + // The description names which status refused. + const quoted = try std.fmt.allocPrint(a, "\"{s}\"", .{status}); + try testing.expect(std.mem.indexOf(u8, res[0].description, quoted) != null); + } + + // A status outside the enumeration is not a refusal. + const unknown = try detect(a, + \\{"events":[ + \\{"message":[2,"m1","Authorize",{"idTag":"TAG-BAD"}]}, + \\{"message":[3,"m1",{"idTagInfo":{"status":"NotAnAuthorizationStatus"}}]}]} + ); + try testing.expect(!has(unknown, .failed_authorization)); +} + test "CONNECTOR_FAULT fires on a Faulted status during a transaction" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); @@ -1214,6 +1301,61 @@ test "STATUS_TRANSITION_VIOLATION is tracked per connector, not globally" { try testing.expect(has(cp, .status_transition_violation)); } +test "STATUS_TRANSITION_VIOLATION matches the section 4.9 table in both directions" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + // The table of OCPP 1.6 edition 2, section 4.9, transcribed independently of + // `transition_table` so the two have to agree. Row by row, as the spec reads, + // with the cell labels in the comments. + const spec = [_]struct { from: []const u8, to: []const []const u8 }{ + // A2, A3, A4, A5, A7, A8, A9 + .{ .from = "Available", .to = &.{ "Preparing", "Charging", "SuspendedEV", "SuspendedEVSE", "Reserved", "Unavailable", "Faulted" } }, + // B1, B3, B4, B5, B6, B9 + .{ .from = "Preparing", .to = &.{ "Available", "Charging", "SuspendedEV", "SuspendedEVSE", "Finishing", "Faulted" } }, + // C1, C4, C5, C6, C8, C9 + .{ .from = "Charging", .to = &.{ "Available", "SuspendedEV", "SuspendedEVSE", "Finishing", "Unavailable", "Faulted" } }, + // D1, D3, D5, D6, D8, D9 + .{ .from = "SuspendedEV", .to = &.{ "Available", "Charging", "SuspendedEVSE", "Finishing", "Unavailable", "Faulted" } }, + // E1, E3, E4, E6, E8, E9 + .{ .from = "SuspendedEVSE", .to = &.{ "Available", "Charging", "SuspendedEV", "Finishing", "Unavailable", "Faulted" } }, + // F1, F2, F8, F9 + .{ .from = "Finishing", .to = &.{ "Available", "Preparing", "Unavailable", "Faulted" } }, + // G1, G2, G8, G9 + .{ .from = "Reserved", .to = &.{ "Available", "Preparing", "Unavailable", "Faulted" } }, + // H1, H2, H3, H4, H5, H9 + .{ .from = "Unavailable", .to = &.{ "Available", "Preparing", "Charging", "SuspendedEV", "SuspendedEVSE", "Faulted" } }, + // I1 through I8 + .{ .from = "Faulted", .to = &.{ "Available", "Preparing", "Charging", "SuspendedEV", "SuspendedEVSE", "Finishing", "Reserved", "Unavailable" } }, + }; + + const flagged = struct { + fn f(alloc: std.mem.Allocator, from: []const u8, to: []const u8) !bool { + const json = try std.fmt.allocPrint( + alloc, + "{{\"events\":[" ++ + "{{\"message\":[2,\"n1\",\"StatusNotification\",{{\"connectorId\":1,\"status\":\"{s}\"}}]}}," ++ + "{{\"message\":[2,\"n2\",\"StatusNotification\",{{\"connectorId\":1,\"status\":\"{s}\"}}]}}]}}", + .{ from, to }, + ); + return has(try detect(alloc, json), .status_transition_violation); + } + }.f; + + // Every ordered pair, both directions of the comparison. The diagonal is + // covered separately, since the table has no self-transitions. + for (spec) |row| { + for (spec) |target| { + if (std.mem.eql(u8, row.from, target.from)) continue; + const permitted = inList(row.to, target.from); + try testing.expectEqual(!permitted, try flagged(a, row.from, target.from)); + } + // A repeated identical status is a violation: no diagonal in the table. + try testing.expect(try flagged(a, row.from, row.from)); + } +} + test "DIAGNOSTICS_FAILURE and FIRMWARE_UPDATE_FAILURE fire on failure statuses" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit(); @@ -1229,15 +1371,50 @@ test "DIAGNOSTICS_FAILURE and FIRMWARE_UPDATE_FAILURE fire on failure statuses" try testing.expect(!has(diag_ok, .diagnostics_failure)); const fw = try detect(a, - \\{"events":[{"message":[2,"f1","FirmwareStatusNotification",{"status":"InstallFailed"}]}]} + \\{"events":[{"message":[2,"f1","FirmwareStatusNotification",{"status":"InstallationFailed"}]}]} ); try testing.expect(has(fw, .firmware_update_failure)); + const fw_dl = try detect(a, + \\{"events":[{"message":[2,"f1","FirmwareStatusNotification",{"status":"DownloadFailed"}]}]} + ); + try testing.expect(has(fw_dl, .firmware_update_failure)); const fw_ok = try detect(a, \\{"events":[{"message":[2,"f1","FirmwareStatusNotification",{"status":"Installed"}]}]} ); try testing.expect(!has(fw_ok, .firmware_update_failure)); } +test "FIRMWARE_UPDATE_FAILURE ignores statuses outside the OCPP 1.6 enum" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + // The rule used to match these three. DownloadPaused is an OCPP 2.0.1 value, + // and an intermediate state there rather than a failure; the other two are in + // neither enumeration (2.0.1 spells them InstallationFailed and + // InstallRebooting). + for ([_][]const u8{ "DownloadPaused", "InstallFailed", "InstallRebootingFailed" }) |status| { + const json = try std.fmt.allocPrint( + a, + "{{\"events\":[{{\"message\":[2,\"f1\",\"FirmwareStatusNotification\",{{\"status\":\"{s}\"}}]}}]}}", + .{status}, + ); + const res = try detect(a, json); + try testing.expect(!has(res, .firmware_update_failure)); + } + + // The progress and success statuses are not failures either. + for ([_][]const u8{ "Downloading", "Downloaded", "Idle", "Installing", "Installed" }) |status| { + const json = try std.fmt.allocPrint( + a, + "{{\"events\":[{{\"message\":[2,\"f1\",\"FirmwareStatusNotification\",{{\"status\":\"{s}\"}}]}}]}}", + .{status}, + ); + const res = try detect(a, json); + try testing.expect(!has(res, .firmware_update_failure)); + } +} + test "SUSPICIOUS_SESSION_DURATION fires on a sub-minute transaction" { var arena = std.heap.ArenaAllocator.init(testing.allocator); defer arena.deinit();