From 785e4415731a808ae30fe970b49eb40df2a1aedf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 15:14:18 +0000 Subject: [PATCH] feat(ocpp-cp): honor v201 TriggerMessage(FirmwareStatusNotification) by re-reporting the latest firmware status (M7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OCPP 2.0.1 CP simulator classified `TriggerMessage(FirmwareStatusNotification)` as `NotImplemented`, even though it models firmware updates (`UpdateFirmware` drives an async `FirmwareStatusNotification(Downloading → … → Installed)` stream). A real station answers this trigger with its *current* firmware status — exactly as the 1.6J side already does via its `firmware_status` field. Wire the v201 twin (Issue #583). The v201 side couldn't before because nothing persisted the latest status: `V201FirmwareUpdateStore` tracked only the in-flight `requestId`. - `V201FirmwareUpdateStore` now retains the latest reported `V201FirmwareStatusReport { status, requestId }` (default `{ Idle, None }`), recorded at the single emit choke point (`send_v201_firmware_status`) so the full lifecycle — interim and terminal, happy path and injected failures — is captured. It is deliberately *not* cleared with the in-flight slot, so a settled `Installed` (or a terminal failure) stays reportable. - `v201_trigger_message_status` reclassifies `FirmwareStatusNotification` as `Accepted`; the existing enqueue path then drains it to a new dispatch arm. - `ChargePoint::trigger_v201_firmware_status_notification` re-reports the latest status as one `FirmwareStatusNotification` — a pure snapshot that starts no update and leaves the in-flight store untouched. Runs on the command-consumer task (off the inbound-CALL path), so the `TriggerMessage` CALLRESULT flushes first and the receive loop never re-enters. - A never-ran station reports `Idle` with `requestId` omitted (the 2.0.1 schema omits `requestId` for a status not tied to a request); a new `v201_firmware_status_report` builder takes an `Option` for that, with the async-progress builder delegating to it. The sibling triggers stay `NotImplemented` and are tracked as follow-ups: LogStatusNotification (#584), PublishFirmwareStatusNotification (#585); the certificate-signing triggers are a separate, heavier slice. Ports the Charging Station behavior pinned by the already-ported types (`ocpp/v201/enums.py` FirmwareStatusEnumType / MessageTriggerEnumType, `ocpp/v201/call.py` FirmwareStatusNotification / TriggerMessage). Tests: store (latest-status tracking, retention past the in-flight clear, extreme requestIds); pure builder (`Idle` omits `requestId`, `Some` carries it, schema validity, wrapper equivalence); policy (`FirmwareStatusNotification` is `Accepted`, totals updated); and over-the-wire via a capturing mock CSMS (idle re-report, latest-status re-report, and the real rollout recording its terminal). The existing NotImplemented-trigger integration test now uses `LogStatusNotification`. `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test --workspace` all green. Closes #583 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01EPMgZ2Vem4gFw75rQv9i1f --- crates/ocpp-cp/src/lib.rs | 192 +++++++++++++++++++- crates/ocpp-cp/src/v201_command.rs | 128 +++++++++++-- crates/ocpp-cp/src/v201_firmware_update.rs | 157 +++++++++++++++- crates/ocpp-cp/tests/central_system_boot.rs | 11 +- 4 files changed, 455 insertions(+), 33 deletions(-) diff --git a/crates/ocpp-cp/src/lib.rs b/crates/ocpp-cp/src/lib.rs index 0cd2413..f150d7d 100644 --- a/crates/ocpp-cp/src/lib.rs +++ b/crates/ocpp-cp/src/lib.rs @@ -8648,12 +8648,15 @@ impl ChargePoint { StatusNotification => self.trigger_v201_status_notification(evse_id).await, MeterValues => self.trigger_v201_meter_values(evse_id).await, TransactionEvent => self.trigger_v201_transaction_event(evse_id).await, - // Firmware-, diagnostics-log-, and certificate-signing triggers the - // simulator does not implement. `v201_trigger_message_status` reports - // these `NotImplemented`, so the handler never enqueues them; this arm - // keeps the match exhaustive and aligned with that policy. + FirmwareStatusNotification => self.trigger_v201_firmware_status_notification().await, + // Diagnostics-log-, publish-firmware-, and certificate-signing triggers + // the simulator does not yet originate. `v201_trigger_message_status` + // reports these `NotImplemented`, so the handler never enqueues them; + // this arm keeps the match exhaustive and aligned with that policy. + // (LogStatusNotification → #584, PublishFirmwareStatusNotification → + // #585 will re-report their latest status as FirmwareStatusNotification + // does; the Sign* certificate triggers are a separate, heavier slice.) other @ (LogStatusNotification - | FirmwareStatusNotification | SignChargingStationCertificate | SignV2GCertificate | SignCombinedCertificate @@ -8960,6 +8963,15 @@ impl ChargePoint { /// routed through the [`v201_command`] constructor so the v201 wire type stays /// out of this module's imports. async fn send_v201_firmware_status(&self, status: FirmwareStatusEnumType, request_id: i32) { + // Retain this as the station's latest firmware status *before* sending, so + // a later `TriggerMessage(FirmwareStatusNotification)` re-reports the + // current status even if this progress CALL failed to transmit — the CP's + // notion of "where the rollout is" has advanced regardless. Kept across the + // in-flight slot's clear (see `V201FirmwareUpdateStore::record_reported`), + // so a settled `Installed` (or terminal failure) stays reportable. + self.v201_firmware_updates + .record_reported(status, request_id) + .await; if let Err(e) = self .call(v201_command::v201_firmware_status_notification( status, request_id, @@ -9140,6 +9152,48 @@ impl ChargePoint { } } + /// Re-report the station's latest firmware status for a `TriggerMessage` + /// (`requestedMessage = FirmwareStatusNotification`, Issue #583). + /// + /// The 2.0.1 twin of the 1.6J `TriggerMessage(FirmwareStatusNotification)` + /// arm: a CSMS asks for the *current* firmware status, and the station answers + /// with a single `FirmwareStatusNotification` carrying the latest status it has + /// reported — without re-running the update. The snapshot comes from + /// [`V201FirmwareUpdateStore::last_reported`](crate::v201_firmware_update::V201FirmwareUpdateStore::last_reported), + /// which every `send_v201_firmware_status` step records: + /// + /// - a station that has never run an `UpdateFirmware` reports + /// [`Idle`](FirmwareStatusEnumType::Idle) with `requestId` omitted; + /// - a rollout in progress reports its most recent interim step + /// (`Downloading` / `Downloaded` / `Installing`) with the correlating + /// `requestId`; + /// - a settled rollout reports its terminal `Installed` (or a `DownloadFailed` + /// / `InstallationFailed`) with that `requestId` — retained past the + /// in-flight slot's clear. + /// + /// This is a pure snapshot re-report: it starts no update and leaves the + /// in-flight store untouched. `TriggerMessage` carries no EVSE scope for this + /// station-wide message, so `evse_id` is not a parameter (mirroring the + /// `BootNotification` / `Heartbeat` arms). Runs on the command-consumer task + /// (off the inbound-CALL path), so the `TriggerMessage` CALLRESULT is flushed + /// before this CALL and the receive loop never re-enters itself. + async fn trigger_v201_firmware_status_notification(&self) { + let report = self.v201_firmware_updates.last_reported().await; + if let Err(e) = self + .call(v201_command::v201_firmware_status_report( + report.status, + report.request_id, + )) + .await + { + warn!( + "v201 TriggerMessage(FirmwareStatusNotification): re-report of \ + {:?} (request {:?}) failed: {e}", + report.status, report.request_id + ); + } + } + /// Emit a 2.0.1 `StatusNotification` for the EVSE(s) a `TriggerMessage` /// targets (`requestedMessage = StatusNotification`). /// @@ -16062,6 +16116,134 @@ mod tests { } } + // --- OCPP 2.0.1 TriggerMessage(FirmwareStatusNotification) (M7, Issue #583) --- + // A CSMS asking `TriggerMessage(requestedMessage = FirmwareStatusNotification)` + // gets the station's latest firmware status re-reported as one + // FirmwareStatusNotification — the v201 twin of the 1.6J behavior. The status + // is recorded at the `send_v201_firmware_status` choke point across the whole + // rollout lifecycle, and retained past the in-flight slot's clear. + + fn firmware_status_notification_routes() -> std::collections::HashMap + { + let mut routes = std::collections::HashMap::new(); + routes.insert( + "BootNotification".to_string(), + boot_response("Accepted", 3600), + ); + // FirmwareStatusNotification.conf is an empty ack. + routes.insert( + "FirmwareStatusNotification".to_string(), + serde_json::json!({}), + ); + routes + } + + /// Drain the capturing channel for the next `FirmwareStatusNotification` CALL, + /// deserialized into the typed 2.0.1 request. Skips the boot/status chatter a + /// freshly-connected CP emits; bounded by a timeout so a missing CALL fails fast. + async fn recv_one_firmware_status( + rx: &mut tokio::sync::mpsc::UnboundedReceiver<(String, serde_json::Value)>, + ) -> ocpp_messages::v201::FirmwareStatusNotificationRequest { + loop { + match tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv()).await { + Ok(Some((action, payload))) if action == "FirmwareStatusNotification" => { + return serde_json::from_value(payload) + .expect("captured FirmwareStatusNotification is typed"); + } + Ok(Some(_)) => continue, // BootNotification / StatusNotification, etc. + Ok(None) | Err(_) => panic!("expected a FirmwareStatusNotification CALL"), + } + } + } + + #[tokio::test] + async fn v201_trigger_firmware_status_reports_idle_when_no_update_ran() { + // A station that has never run an UpdateFirmware answers the trigger with + // FirmwareStatusNotification(Idle), requestId omitted. + let (addr, mut rx) = spawn_mock_csms_capturing(firmware_status_notification_routes()).await; + let cp = ChargePoint::new(ChargePointConfig { + central_system_url: format!("ws://{addr}"), + ..ChargePointConfig::for_version(OcppVersion::V201) + }) + .unwrap(); + cp.connect().await.unwrap(); + + cp.send_v201_triggered_message(MessageTriggerEnumType::FirmwareStatusNotification, None) + .await; + + let report = recv_one_firmware_status(&mut rx).await; + assert_eq!(report.status, FirmwareStatusEnumType::Idle); + assert_eq!( + report.request_id, None, + "an Idle re-report carries no requestId" + ); + } + + #[tokio::test] + async fn v201_trigger_firmware_status_reports_the_latest_recorded_status() { + // With a status recorded (as the rollout's emit path does), the trigger + // re-reports exactly that status + its correlating requestId — a pure + // snapshot that starts no update and leaves the in-flight store untouched. + let (addr, mut rx) = spawn_mock_csms_capturing(firmware_status_notification_routes()).await; + let cp = ChargePoint::new(ChargePointConfig { + central_system_url: format!("ws://{addr}"), + ..ChargePointConfig::for_version(OcppVersion::V201) + }) + .unwrap(); + cp.connect().await.unwrap(); + + cp.v201_firmware_updates + .record_reported(FirmwareStatusEnumType::Installed, 55) + .await; + + cp.send_v201_triggered_message(MessageTriggerEnumType::FirmwareStatusNotification, None) + .await; + + let report = recv_one_firmware_status(&mut rx).await; + assert_eq!(report.status, FirmwareStatusEnumType::Installed); + assert_eq!(report.request_id, Some(55)); + // The re-report is a snapshot: no update started, in-flight slot untouched. + assert!( + cp.in_flight_firmware_update().await.is_none(), + "re-report must not open a rollout" + ); + } + + #[tokio::test] + async fn v201_firmware_rollout_records_its_terminal_status_for_re_report() { + // The recording is wired through the real state machine (unconnected, so + // the progress CALLs fail-and-warn — record_reported runs before the send). + // The happy path leaves the latest status at the Installed terminal. + let cp = ChargePoint::new(ChargePointConfig::for_version(OcppVersion::V201)).unwrap(); + cp.v201_firmware_updates.begin(55).await; + cp.run_v201_firmware_update(55).await; + assert_eq!( + cp.v201_firmware_updates.last_reported().await, + crate::v201_firmware_update::V201FirmwareStatusReport { + status: FirmwareStatusEnumType::Installed, + request_id: Some(55), + }, + "a completed rollout leaves Installed as the re-reportable status" + ); + + // A fault-injected rollout records its failure terminal instead. + let cp = ChargePoint::new(ChargePointConfig { + firmware_update_outcome: FirmwareUpdateOutcome::DownloadFailed, + ..ChargePointConfig::for_version(OcppVersion::V201) + }) + .unwrap(); + cp.v201_firmware_updates.begin(56).await; + cp.run_v201_firmware_update(56).await; + assert_eq!( + cp.v201_firmware_updates.last_reported().await, + crate::v201_firmware_update::V201FirmwareStatusReport { + status: FirmwareStatusEnumType::DownloadFailed, + request_id: Some(56), + }, + "a failed rollout leaves its failure terminal as the re-reportable status" + ); + } + // --- OCPP 2.0.1 GetDisplayMessages (M7, issue #508) -------------------- // A `for_version(V201)` CP answers the query synchronously (Accepted / // Unknown) off a snapshot of its `V201DisplayMessageStore`, and queues a diff --git a/crates/ocpp-cp/src/v201_command.rs b/crates/ocpp-cp/src/v201_command.rs index 9bd819e..ed3c2f0 100644 --- a/crates/ocpp-cp/src/v201_command.rs +++ b/crates/ocpp-cp/src/v201_command.rs @@ -317,11 +317,14 @@ pub fn v201_reset_response( /// [`BootNotification`](MessageTriggerEnumType::BootNotification), /// [`Heartbeat`](MessageTriggerEnumType::Heartbeat), /// [`StatusNotification`](MessageTriggerEnumType::StatusNotification), -/// [`MeterValues`](MessageTriggerEnumType::MeterValues), and -/// [`TransactionEvent`](MessageTriggerEnumType::TransactionEvent) — and to +/// [`MeterValues`](MessageTriggerEnumType::MeterValues), +/// [`TransactionEvent`](MessageTriggerEnumType::TransactionEvent), and +/// [`FirmwareStatusNotification`](MessageTriggerEnumType::FirmwareStatusNotification) +/// (the CP models `UpdateFirmware` and re-reports its latest firmware status on +/// demand, Issue #583) — and to /// [`NotImplemented`](TriggerMessageStatusEnumType::NotImplemented) for the -/// firmware-, log-, and certificate-signing triggers the simulator has no -/// support for. +/// diagnostics-log-, publish-firmware-, and certificate-signing triggers the +/// simulator does not yet trigger. /// /// `MeterValues` is `Accepted` at the policy level because the CP produces meter /// readings; in 2.0.1 those ride inside `TransactionEvent`, so the slice-5b @@ -346,13 +349,21 @@ pub fn v201_trigger_message_status( }; match requested { // Messages this CP already builds and sends on the live V201 path. - BootNotification | Heartbeat | StatusNotification | MeterValues | TransactionEvent => { - TriggerMessageStatusEnumType::Accepted - } - // Firmware-, diagnostics-log-, and certificate-signing flows the - // simulator does not implement: recognized but not triggerable. + // `FirmwareStatusNotification` re-reports the station's latest firmware + // status on demand — the CP models `UpdateFirmware` and retains that + // status, so the trigger is honored (Issue #583), the direct twin of the + // 1.6J `TriggerMessage(FirmwareStatusNotification)` re-report. + BootNotification + | Heartbeat + | StatusNotification + | MeterValues + | TransactionEvent + | FirmwareStatusNotification => TriggerMessageStatusEnumType::Accepted, + // Diagnostics-log-, publish-firmware-, and certificate-signing flows the + // simulator does not yet trigger: recognized but not triggerable. + // (LogStatusNotification → #584, PublishFirmwareStatusNotification → #585 + // will re-report their latest status the same way #583 does.) LogStatusNotification - | FirmwareStatusNotification | SignChargingStationCertificate | SignV2GCertificate | SignCombinedCertificate @@ -2427,17 +2438,37 @@ pub fn v201_log_status_notification( /// The `requestId` is always carried here (it correlates the async progress /// report back to the triggering `UpdateFirmwareRequest`); it is only absent /// when a `TriggerMessage` asks for a `FirmwareStatusNotification` with no update -/// ongoing, which this `UpdateFirmware`-driven flow never is. The firmware twin -/// of [`v201_log_status_notification`]. Ports +/// ongoing (the [`Idle`](FirmwareStatusEnumType::Idle) re-report built by +/// [`v201_firmware_status_report`]), which this `UpdateFirmware`-driven flow never +/// is. The firmware twin of [`v201_log_status_notification`]. Ports /// `ocpp.v201.call.FirmwareStatusNotification`. #[must_use] pub fn v201_firmware_status_notification( status: FirmwareStatusEnumType, request_id: i32, +) -> FirmwareStatusNotificationRequest { + v201_firmware_status_report(status, Some(request_id)) +} + +/// Build a schema-valid `FirmwareStatusNotification.req` +/// ([`FirmwareStatusNotificationRequest`]) with an *optional* `request_id`. +/// +/// The re-report constructor a `TriggerMessage(FirmwareStatusNotification)` uses +/// to answer with the station's latest firmware status (Issue #583). Unlike the +/// async-progress [`v201_firmware_status_notification`], `request_id` may be +/// `None`: an [`Idle`](FirmwareStatusEnumType::Idle) status on a station that has +/// never run an `UpdateFirmware` is not tied to a specific request, and the 2.0.1 +/// schema omits `requestId` (`skip_serializing_if = "Option::is_none"`) in that +/// case; a status carried over from a real rollout re-reports its `Some(requestId)`. +/// Ports `ocpp.v201.call.FirmwareStatusNotification`. +#[must_use] +pub fn v201_firmware_status_report( + status: FirmwareStatusEnumType, + request_id: Option, ) -> FirmwareStatusNotificationRequest { FirmwareStatusNotificationRequest { status, - request_id: Some(request_id), + request_id, custom_data: None, } } @@ -3693,6 +3724,9 @@ mod tests { MessageTriggerEnumType::StatusNotification, MessageTriggerEnumType::MeterValues, MessageTriggerEnumType::TransactionEvent, + // The CP models UpdateFirmware and retains its latest firmware status, + // so a FirmwareStatusNotification trigger is honored by re-report (#583). + MessageTriggerEnumType::FirmwareStatusNotification, ] { assert_eq!( v201_trigger_message_status(requested), @@ -3702,13 +3736,13 @@ mod tests { } } - /// The firmware-, log-, and certificate-signing triggers the simulator has - /// no way to emit resolve to `NotImplemented` (recognized, not triggerable). + /// The diagnostics-log-, publish-firmware-, and certificate-signing triggers + /// the simulator does not yet originate resolve to `NotImplemented` + /// (recognized, not triggerable). #[test] fn unsupported_triggers_are_not_implemented() { for requested in [ MessageTriggerEnumType::LogStatusNotification, - MessageTriggerEnumType::FirmwareStatusNotification, MessageTriggerEnumType::SignChargingStationCertificate, MessageTriggerEnumType::SignV2GCertificate, MessageTriggerEnumType::SignCombinedCertificate, @@ -3751,10 +3785,10 @@ mod tests { } } } - assert_eq!(accepted, 5, "expected exactly 5 producible triggers"); + assert_eq!(accepted, 6, "expected exactly 6 producible triggers"); assert_eq!( - not_implemented, 6, - "expected exactly 6 unsupported triggers" + not_implemented, 5, + "expected exactly 5 unsupported triggers" ); } @@ -8374,6 +8408,62 @@ mod tests { } } + #[test] + fn firmware_status_report_omits_request_id_when_none() { + // The Idle re-report a TriggerMessage(FirmwareStatusNotification) answers + // with on a station that has never run an update (#583): Idle, no requestId. + let req = v201_firmware_status_report(FirmwareStatusEnumType::Idle, None); + assert_eq!(req.status, FirmwareStatusEnumType::Idle); + assert_eq!(req.request_id, None); + assert!(req.custom_data.is_none()); + // `requestId` must be *absent* from the wire, not `null` — the schema has + // no `null` for it (skip_serializing_if = "Option::is_none"). + let payload = serde_json::to_value(&req).unwrap(); + assert!( + payload.get("requestId").is_none(), + "an Idle re-report omits requestId entirely, got: {payload}" + ); + } + + #[test] + fn firmware_status_report_carries_request_id_when_some() { + // A status re-reported from a real rollout keeps its correlating requestId. + let req = v201_firmware_status_report(FirmwareStatusEnumType::Installed, Some(7)); + assert_eq!(req.status, FirmwareStatusEnumType::Installed); + assert_eq!(req.request_id, Some(7)); + // The convenience wrapper produces the same shape as the explicit Some. + assert_eq!( + req, + v201_firmware_status_notification(FirmwareStatusEnumType::Installed, 7) + ); + } + + #[test] + fn built_firmware_status_reports_are_schema_valid() { + // The trigger re-report path (optional requestId) is schema-valid both as + // an Idle/None snapshot and carrying a status + requestId from a rollout. + let validator = SchemaValidator::v201(); + for status in [ + FirmwareStatusEnumType::Idle, + FirmwareStatusEnumType::Downloading, + FirmwareStatusEnumType::Installed, + FirmwareStatusEnumType::DownloadFailed, + FirmwareStatusEnumType::InstallationFailed, + ] { + for request_id in [None, Some(0), Some(-1), Some(i32::MIN), Some(i32::MAX)] { + let req = v201_firmware_status_report(status, request_id); + let payload = serde_json::to_value(&req).unwrap(); + assert!( + validator + .validate_call("FirmwareStatusNotification", &payload) + .is_ok(), + "built {status:?} FirmwareStatusNotification.req (requestId {request_id:?}) \ + should be schema-valid, got: {payload}" + ); + } + } + } + // --- GetInstalledCertificateIds (v201) decision + response builder (#521) --- /// Two distinct installed anchors, as a `snapshot()` would present them. diff --git a/crates/ocpp-cp/src/v201_firmware_update.rs b/crates/ocpp-cp/src/v201_firmware_update.rs index d85be66..d881ea2 100644 --- a/crates/ocpp-cp/src/v201_firmware_update.rs +++ b/crates/ocpp-cp/src/v201_firmware_update.rs @@ -39,20 +39,66 @@ //! can be shared across the charge point's tasks, exactly like the //! [`V201LogUploadStore`](crate::v201_log_upload::V201LogUploadStore). +use ocpp_types::v201::FirmwareStatusEnumType; use tokio::sync::RwLock; +/// The most recent firmware status the station reported, retained so a +/// `TriggerMessage(FirmwareStatusNotification)` can re-report it on demand +/// (Issue #583) — the OCPP 2.0.1 twin of the 1.6J `firmware_status` field. +/// +/// A station reports firmware progress asynchronously +/// (`FirmwareStatusNotification(Downloading → … → Installed)`), but a CSMS may +/// ask for the *current* status at any point via `TriggerMessage`. This snapshot +/// is the answer: [`Idle`](FirmwareStatusEnumType::Idle) with no `request_id` +/// until the first `UpdateFirmware` progress step is emitted, then the latest +/// `(status, requestId)` thereafter — retained even after the in-flight slot is +/// cleared, so a settled `Installed` (or a terminal failure) is still reportable. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct V201FirmwareStatusReport { + /// The latest reported stage of the firmware download/install lifecycle. + pub status: FirmwareStatusEnumType, + /// The `requestId` of the `UpdateFirmware` that stage belongs to, or `None` + /// for the initial [`Idle`](FirmwareStatusEnumType::Idle) (no update has run, + /// so the status is not tied to a specific request — the 2.0.1 schema omits + /// `requestId` in that case). + pub request_id: Option, +} + +impl Default for V201FirmwareStatusReport { + /// A station that has never run an update: [`Idle`](FirmwareStatusEnumType::Idle), + /// no correlating `requestId`. + fn default() -> Self { + Self { + status: FirmwareStatusEnumType::Idle, + request_id: None, + } + } +} + /// Tracks the single `UpdateFirmware` rollout a station is currently serving, by -/// its `requestId`. +/// its `requestId`, plus the latest firmware status it has reported. +/// +/// For the in-flight slot: `None` means idle (no update in flight); +/// `Some(request_id)` names the request whose update is underway. The `requestId` +/// is CSMS-supplied and stored as an opaque `i32` — never parsed or indexed — so +/// no wire value (including `i32::MIN`/`MAX`) can panic here. /// -/// `None` means idle (no update in flight); `Some(request_id)` names the request -/// whose update is underway. The `requestId` is CSMS-supplied and stored as an -/// opaque `i32` — never parsed or indexed — so no wire value (including -/// `i32::MIN`/`MAX`) can panic here. +/// Separately, [`last_reported`](Self::last_reported) retains the most recent +/// [`V201FirmwareStatusReport`] the station emitted (via +/// [`record_reported`](Self::record_reported)), so a +/// `TriggerMessage(FirmwareStatusNotification)` can re-report the current status +/// without re-running the update. It is deliberately *not* cleared when the +/// in-flight slot is ([`complete`](Self::complete) / [`clear`](Self::clear)): a +/// finished rollout leaves the station idle but its last status (`Installed`, or a +/// terminal failure) remains the truthful thing to report. #[derive(Debug, Default)] pub struct V201FirmwareUpdateStore { /// The `requestId` of the firmware update currently in flight, or `None` when /// idle. in_flight: RwLock>, + /// The most recent firmware status the station reported. `Idle`/`None` until + /// the first progress step; see [`V201FirmwareStatusReport`]. + last_reported: RwLock, } impl V201FirmwareUpdateStore { @@ -134,6 +180,34 @@ impl V201FirmwareUpdateStore { false } } + + /// Record `status` (from `UpdateFirmware` request `request_id`) as the latest + /// firmware status the station has reported. + /// + /// Called at the single emit choke point + /// (`ChargePoint::send_v201_firmware_status`) for every progress step, so the + /// snapshot tracks the full lifecycle — interim (`Downloading` … `Installing`) + /// and terminal (`Installed` / `DownloadFailed` / `InstallationFailed`). It is + /// independent of the in-flight slot: a completed rollout clears + /// [`in_flight`](Self::in_flight) but this retains the terminal status so a + /// later `TriggerMessage(FirmwareStatusNotification)` still re-reports it. + /// `request_id` is only stored, never parsed or indexed. + pub async fn record_reported(&self, status: FirmwareStatusEnumType, request_id: i32) { + *self.last_reported.write().await = V201FirmwareStatusReport { + status, + request_id: Some(request_id), + }; + } + + /// The most recent [`V201FirmwareStatusReport`] the station has reported. + /// + /// The snapshot a `TriggerMessage(FirmwareStatusNotification)` re-reports on + /// demand. Defaults to [`Idle`](FirmwareStatusEnumType::Idle) with no + /// `requestId` on a station that has never run an update. Returns a copy, so + /// the caller decides without holding the store lock. + pub async fn last_reported(&self) -> V201FirmwareStatusReport { + *self.last_reported.read().await + } } #[cfg(test)] @@ -238,4 +312,77 @@ mod tests { assert!(!store.complete(i32::MIN).await); assert!(!store.complete(i32::MAX).await); } + + #[tokio::test] + async fn a_new_store_reports_idle_with_no_request_id() { + let store = V201FirmwareUpdateStore::new(); + assert_eq!( + store.last_reported().await, + V201FirmwareStatusReport { + status: FirmwareStatusEnumType::Idle, + request_id: None, + }, + "a station that has never run an update reports Idle, no requestId" + ); + } + + #[tokio::test] + async fn record_reported_tracks_the_latest_status_and_request_id() { + let store = V201FirmwareUpdateStore::new(); + store + .record_reported(FirmwareStatusEnumType::Downloading, 42) + .await; + assert_eq!( + store.last_reported().await, + V201FirmwareStatusReport { + status: FirmwareStatusEnumType::Downloading, + request_id: Some(42), + } + ); + // The latest wins — a later step overwrites the earlier one. + store + .record_reported(FirmwareStatusEnumType::Installed, 42) + .await; + assert_eq!( + store.last_reported().await, + V201FirmwareStatusReport { + status: FirmwareStatusEnumType::Installed, + request_id: Some(42), + } + ); + } + + #[tokio::test] + async fn completing_the_rollout_retains_the_last_reported_status() { + // A settled rollout returns the in-flight slot to idle, but the terminal + // status must remain reportable for a later TriggerMessage. + let store = V201FirmwareUpdateStore::new(); + store.begin(9).await; + store + .record_reported(FirmwareStatusEnumType::Installed, 9) + .await; + assert!(store.complete(9).await); + assert!(store.is_idle().await, "in-flight slot cleared"); + assert_eq!( + store.last_reported().await, + V201FirmwareStatusReport { + status: FirmwareStatusEnumType::Installed, + request_id: Some(9), + }, + "the terminal status survives the in-flight clear" + ); + } + + #[tokio::test] + async fn record_reported_accepts_extreme_request_ids() { + let store = V201FirmwareUpdateStore::new(); + store + .record_reported(FirmwareStatusEnumType::DownloadFailed, i32::MIN) + .await; + assert_eq!(store.last_reported().await.request_id, Some(i32::MIN)); + store + .record_reported(FirmwareStatusEnumType::InstallationFailed, i32::MAX) + .await; + assert_eq!(store.last_reported().await.request_id, Some(i32::MAX)); + } } diff --git a/crates/ocpp-cp/tests/central_system_boot.rs b/crates/ocpp-cp/tests/central_system_boot.rs index 3c0cd2e..21b6cd5 100644 --- a/crates/ocpp-cp/tests/central_system_boot.rs +++ b/crates/ocpp-cp/tests/central_system_boot.rs @@ -941,14 +941,17 @@ async fn v201_trigger_unsupported_message_is_not_implemented_and_emits_nothing() cp.connect().await.expect("v201 connect + boot sequence"); assert_eq!(boots.load(std::sync::atomic::Ordering::SeqCst), 1); - // CSMS -> CP: TriggerMessage(FirmwareStatusNotification). The simulator has no - // firmware state machine on the v201 path, so slice 5a classifies it - // NotImplemented and the wiring enqueues nothing. + // CSMS -> CP: TriggerMessage(LogStatusNotification). The simulator does not yet + // originate a standalone LogStatusNotification on the v201 path (the trigger for + // it is tracked by #584), so the policy classifies it NotImplemented and the + // wiring enqueues nothing. (FirmwareStatusNotification is now Accepted — it + // re-reports the latest firmware status, #583 — so it is no longer the + // NotImplemented example here.) let resp = server .call::( "CP201_TRIG_NIMP", V201TriggerMessageRequest { - requested_message: MessageTriggerEnumType::FirmwareStatusNotification, + requested_message: MessageTriggerEnumType::LogStatusNotification, evse: None, custom_data: None, },