From 0a724ac568b80239b28082f0a6bf3056fe2dec17 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 17:02:02 +0000 Subject: [PATCH 1/2] feat(ocpp-cp): honor v201 TriggerMessage(LogStatusNotification) by re-reporting the latest log-upload status (M7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A CSMS asking `TriggerMessage(requestedMessage = LogStatusNotification)` on the OCPP 2.0.1 CP simulator now gets the station's current log-upload status re-reported as one schema-valid `LogStatusNotification` — the direct log-upload twin of #583 (FirmwareStatusNotification) and the 2.0.1 analog of the 1.6J TriggerMessage(DiagnosticsStatusNotification) re-report. Previously the trigger was classified NotImplemented despite the simulator modeling GetLog uploads. - V201LogUploadStore: retain the latest V201LogStatusReport { status, request_id } (default { Idle, None }) alongside the in-flight requestId, recorded at the single send_v201_log_status choke point and deliberately not cleared with the in-flight slot, so a settled Uploaded / terminal failure stays reportable. - v201_command: reclassify LogStatusNotification NotImplemented -> Accepted; add v201_log_status_report(status, Option) re-report builder (the async v201_log_status_notification now delegates to it) that omits requestId for an Idle snapshot. - lib: send_v201_log_status records the status before sending; new trigger_v201_log_status_notification re-reports the snapshot as a pure, side-effect-free CALL on the command-consumer task, wired into send_v201_triggered_message. Ports ocpp.v201.call.LogStatusNotification (request_id: Optional[int]). Pure snapshot: starts no upload, leaves the in-flight store untouched, off the inbound-CALL path. 1.6J unaffected. Closes #584. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0176cR1foeU8uwtbWZQDdsPZ --- crates/ocpp-cp/src/lib.rs | 193 +++++++++++++++++++- crates/ocpp-cp/src/v201_command.rs | 137 +++++++++++--- crates/ocpp-cp/src/v201_log_upload.rs | 160 +++++++++++++++- crates/ocpp-cp/tests/central_system_boot.rs | 15 +- 4 files changed, 458 insertions(+), 47 deletions(-) diff --git a/crates/ocpp-cp/src/lib.rs b/crates/ocpp-cp/src/lib.rs index 2743f15..4dd86e5 100644 --- a/crates/ocpp-cp/src/lib.rs +++ b/crates/ocpp-cp/src/lib.rs @@ -8740,15 +8740,15 @@ impl ChargePoint { MeterValues => self.trigger_v201_meter_values(evse_id).await, TransactionEvent => self.trigger_v201_transaction_event(evse_id).await, 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 - | SignChargingStationCertificate + LogStatusNotification => self.trigger_v201_log_status_notification().await, + // 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. + // (PublishFirmwareStatusNotification → #585 will re-report its latest + // status as FirmwareStatusNotification / LogStatusNotification do; the + // Sign* certificate triggers are a separate, heavier slice.) + other @ (SignChargingStationCertificate | SignV2GCertificate | SignCombinedCertificate | PublishFirmwareStatusNotification) => { @@ -8952,6 +8952,15 @@ impl ChargePoint { /// `request_id` (OCPP 2.0.1 Part 2). A best-effort progress report: a send /// failure is logged, not propagated (the upload state machine continues). async fn send_v201_log_status(&self, status: UploadLogStatusEnumType, request_id: i32) { + // Retain this as the station's latest log-upload status *before* sending, + // so a later `TriggerMessage(LogStatusNotification)` re-reports the current + // status even if this progress CALL failed to transmit — the CP's notion of + // "where the upload is" has advanced regardless. Kept across the in-flight + // slot's clear (see `V201LogUploadStore::record_reported`), so a settled + // `Uploaded` (or terminal failure) stays reportable. + self.v201_log_uploads + .record_reported(status, request_id) + .await; if let Err(e) = self .call(v201_command::v201_log_status_notification( status, request_id, @@ -9285,6 +9294,48 @@ impl ChargePoint { } } + /// Re-report the station's latest log-upload status for a `TriggerMessage` + /// (`requestedMessage = LogStatusNotification`, Issue #584). + /// + /// The log-upload twin of [`trigger_v201_firmware_status_notification`] and + /// the 2.0.1 analog of the 1.6J `TriggerMessage(DiagnosticsStatusNotification)` + /// re-report: a CSMS asks for the *current* log-upload status, and the station + /// answers with a single `LogStatusNotification` carrying the latest status it + /// has reported — without re-running the upload. The snapshot comes from + /// [`V201LogUploadStore::last_reported`](crate::v201_log_upload::V201LogUploadStore::last_reported), + /// which every `send_v201_log_status` step records: + /// + /// - a station that has never run a `GetLog` reports + /// [`Idle`](UploadLogStatusEnumType::Idle) with `requestId` omitted; + /// - an upload in progress reports its most recent interim step (`Uploading`) + /// with the correlating `requestId`; + /// - a settled upload reports its terminal `Uploaded` (or an `UploadFailure` / + /// `AcceptedCanceled`) with that `requestId` — retained past the in-flight + /// slot's clear. + /// + /// This is a pure snapshot re-report: it starts no upload 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_log_status_notification(&self) { + let report = self.v201_log_uploads.last_reported().await; + if let Err(e) = self + .call(v201_command::v201_log_status_report( + report.status, + report.request_id, + )) + .await + { + warn!( + "v201 TriggerMessage(LogStatusNotification): 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`). /// @@ -16436,6 +16487,130 @@ mod tests { ); } + // --- OCPP 2.0.1 TriggerMessage(LogStatusNotification) (M7, Issue #584) --- + // A CSMS asking `TriggerMessage(requestedMessage = LogStatusNotification)` + // gets the station's latest log-upload status re-reported as one + // LogStatusNotification — the log-upload twin of #583. The status is recorded + // at the `send_v201_log_status` choke point across the whole upload lifecycle, + // and retained past the in-flight slot's clear. + + fn log_status_notification_routes() -> std::collections::HashMap { + let mut routes = std::collections::HashMap::new(); + routes.insert( + "BootNotification".to_string(), + boot_response("Accepted", 3600), + ); + // LogStatusNotification.conf is an empty ack. + routes.insert("LogStatusNotification".to_string(), serde_json::json!({})); + routes + } + + /// Drain the capturing channel for the next `LogStatusNotification` 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_log_status( + rx: &mut tokio::sync::mpsc::UnboundedReceiver<(String, serde_json::Value)>, + ) -> ocpp_messages::v201::LogStatusNotificationRequest { + loop { + match tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv()).await { + Ok(Some((action, payload))) if action == "LogStatusNotification" => { + return serde_json::from_value(payload) + .expect("captured LogStatusNotification is typed"); + } + Ok(Some(_)) => continue, // BootNotification / StatusNotification, etc. + Ok(None) | Err(_) => panic!("expected a LogStatusNotification CALL"), + } + } + } + + #[tokio::test] + async fn v201_trigger_log_status_reports_idle_when_no_upload_ran() { + // A station that has never run a GetLog answers the trigger with + // LogStatusNotification(Idle), requestId omitted. + let (addr, mut rx) = spawn_mock_csms_capturing(log_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::LogStatusNotification, None) + .await; + + let report = recv_one_log_status(&mut rx).await; + assert_eq!(report.status, UploadLogStatusEnumType::Idle); + assert_eq!( + report.request_id, None, + "an Idle re-report carries no requestId" + ); + } + + #[tokio::test] + async fn v201_trigger_log_status_reports_the_latest_recorded_status() { + // With a status recorded (as the upload's emit path does), the trigger + // re-reports exactly that status + its correlating requestId — a pure + // snapshot that starts no upload and leaves the in-flight store untouched. + let (addr, mut rx) = spawn_mock_csms_capturing(log_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_log_uploads + .record_reported(UploadLogStatusEnumType::Uploaded, 55) + .await; + + cp.send_v201_triggered_message(MessageTriggerEnumType::LogStatusNotification, None) + .await; + + let report = recv_one_log_status(&mut rx).await; + assert_eq!(report.status, UploadLogStatusEnumType::Uploaded); + assert_eq!(report.request_id, Some(55)); + // The re-report is a snapshot: no upload started, in-flight slot untouched. + assert!( + cp.in_flight_log_upload().await.is_none(), + "re-report must not open an upload" + ); + } + + #[tokio::test] + async fn v201_log_upload_records_its_terminal_status_for_re_report() { + // The recording is wired through the real upload flow (unconnected, so the + // progress CALLs fail-and-warn — record_reported runs before the send). + // The happy path leaves the latest status at the Uploaded terminal. + let cp = ChargePoint::new(ChargePointConfig::for_version(OcppVersion::V201)).unwrap(); + cp.v201_log_uploads.begin(55).await; + cp.run_v201_log_upload(55).await; + assert_eq!( + cp.v201_log_uploads.last_reported().await, + crate::v201_log_upload::V201LogStatusReport { + status: UploadLogStatusEnumType::Uploaded, + request_id: Some(55), + }, + "a completed upload leaves Uploaded as the re-reportable status" + ); + + // A fault-injected upload records its failure terminal instead. + let cp = ChargePoint::new(ChargePointConfig { + log_upload_should_fail: true, + ..ChargePointConfig::for_version(OcppVersion::V201) + }) + .unwrap(); + cp.v201_log_uploads.begin(56).await; + cp.run_v201_log_upload(56).await; + assert_eq!( + cp.v201_log_uploads.last_reported().await, + crate::v201_log_upload::V201LogStatusReport { + status: UploadLogStatusEnumType::UploadFailure, + request_id: Some(56), + }, + "a failed upload 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 ed3c2f0..d94ef17 100644 --- a/crates/ocpp-cp/src/v201_command.rs +++ b/crates/ocpp-cp/src/v201_command.rs @@ -318,13 +318,16 @@ pub fn v201_reset_response( /// [`Heartbeat`](MessageTriggerEnumType::Heartbeat), /// [`StatusNotification`](MessageTriggerEnumType::StatusNotification), /// [`MeterValues`](MessageTriggerEnumType::MeterValues), -/// [`TransactionEvent`](MessageTriggerEnumType::TransactionEvent), and +/// [`TransactionEvent`](MessageTriggerEnumType::TransactionEvent), /// [`FirmwareStatusNotification`](MessageTriggerEnumType::FirmwareStatusNotification) /// (the CP models `UpdateFirmware` and re-reports its latest firmware status on -/// demand, Issue #583) — and to +/// demand, Issue #583), and +/// [`LogStatusNotification`](MessageTriggerEnumType::LogStatusNotification) (the CP +/// models `GetLog` and re-reports its latest log-upload status on demand, Issue +/// #584) — and to /// [`NotImplemented`](TriggerMessageStatusEnumType::NotImplemented) for the -/// diagnostics-log-, publish-firmware-, and certificate-signing triggers the -/// simulator does not yet trigger. +/// 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 @@ -349,22 +352,25 @@ pub fn v201_trigger_message_status( }; match requested { // Messages this CP already builds and sends on the live V201 path. - // `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. + // `FirmwareStatusNotification` (#583) and `LogStatusNotification` (#584) + // re-report the station's latest firmware / log-upload status on demand — + // the CP models `UpdateFirmware` / `GetLog` and retains each status, so the + // triggers are honored, the direct 2.0.1 twins of the 1.6J + // `TriggerMessage(FirmwareStatusNotification / DiagnosticsStatusNotification)` + // re-reports. 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 - | SignChargingStationCertificate + | FirmwareStatusNotification + | LogStatusNotification => TriggerMessageStatusEnumType::Accepted, + // Publish-firmware- and certificate-signing flows the simulator does not + // yet trigger: recognized but not triggerable. + // (PublishFirmwareStatusNotification → #585 will re-report its latest + // status the same way #583/#584 do; the Sign* certificate triggers are a + // separate, heavier slice that must originate a fresh SignCertificate CSR.) + SignChargingStationCertificate | SignV2GCertificate | SignCombinedCertificate | PublishFirmwareStatusNotification => TriggerMessageStatusEnumType::NotImplemented, @@ -2415,17 +2421,39 @@ pub fn v201_log_upload_terminal_status( /// /// The `requestId` is always carried here (it correlates the async progress /// report back to the triggering `GetLogRequest`); it is only absent when a -/// `TriggerMessage` asks for a `LogStatusNotification` with no upload ongoing, -/// which this `GetLog`-driven flow never is. Pure constructor mirroring -/// [`v201_get_log_response`]. Ports `ocpp.v201.call.LogStatusNotification`. +/// `TriggerMessage` asks for a `LogStatusNotification` with no upload ongoing +/// (the [`Idle`](UploadLogStatusEnumType::Idle) re-report built by +/// [`v201_log_status_report`]), which this `GetLog`-driven flow never is. Pure +/// constructor mirroring [`v201_get_log_response`]. Ports +/// `ocpp.v201.call.LogStatusNotification`. #[must_use] pub fn v201_log_status_notification( status: UploadLogStatusEnumType, request_id: i32, +) -> LogStatusNotificationRequest { + v201_log_status_report(status, Some(request_id)) +} + +/// Build a schema-valid `LogStatusNotification.req` +/// ([`LogStatusNotificationRequest`]) with an *optional* `request_id`. +/// +/// The re-report constructor a `TriggerMessage(LogStatusNotification)` uses to +/// answer with the station's latest log-upload status (Issue #584). Unlike the +/// async-progress [`v201_log_status_notification`], `request_id` may be `None`: an +/// [`Idle`](UploadLogStatusEnumType::Idle) status on a station that has never run +/// a `GetLog` 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 upload re-reports its `Some(requestId)`. The +/// log-upload twin of [`v201_firmware_status_report`]. Ports +/// `ocpp.v201.call.LogStatusNotification`. +#[must_use] +pub fn v201_log_status_report( + status: UploadLogStatusEnumType, + request_id: Option, ) -> LogStatusNotificationRequest { LogStatusNotificationRequest { status, - request_id: Some(request_id), + request_id, custom_data: None, } } @@ -3727,6 +3755,9 @@ mod tests { // The CP models UpdateFirmware and retains its latest firmware status, // so a FirmwareStatusNotification trigger is honored by re-report (#583). MessageTriggerEnumType::FirmwareStatusNotification, + // The CP models GetLog and retains its latest log-upload status, so a + // LogStatusNotification trigger is honored by re-report (#584). + MessageTriggerEnumType::LogStatusNotification, ] { assert_eq!( v201_trigger_message_status(requested), @@ -3736,13 +3767,11 @@ mod tests { } } - /// The diagnostics-log-, publish-firmware-, and certificate-signing triggers - /// the simulator does not yet originate resolve to `NotImplemented` - /// (recognized, not triggerable). + /// The 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::SignChargingStationCertificate, MessageTriggerEnumType::SignV2GCertificate, MessageTriggerEnumType::SignCombinedCertificate, @@ -3785,10 +3814,10 @@ mod tests { } } } - assert_eq!(accepted, 6, "expected exactly 6 producible triggers"); + assert_eq!(accepted, 7, "expected exactly 7 producible triggers"); assert_eq!( - not_implemented, 5, - "expected exactly 5 unsupported triggers" + not_implemented, 4, + "expected exactly 4 unsupported triggers" ); } @@ -8356,6 +8385,62 @@ mod tests { } } + #[test] + fn log_status_report_omits_request_id_when_none() { + // The Idle re-report a TriggerMessage(LogStatusNotification) answers with + // on a station that has never run a GetLog (#584): Idle, no requestId. + let req = v201_log_status_report(UploadLogStatusEnumType::Idle, None); + assert_eq!(req.status, UploadLogStatusEnumType::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 log_status_report_carries_request_id_when_some() { + // A status re-reported from a real upload keeps its correlating requestId. + let req = v201_log_status_report(UploadLogStatusEnumType::Uploaded, Some(7)); + assert_eq!(req.status, UploadLogStatusEnumType::Uploaded); + assert_eq!(req.request_id, Some(7)); + // The convenience wrapper produces the same shape as the explicit Some. + assert_eq!( + req, + v201_log_status_notification(UploadLogStatusEnumType::Uploaded, 7) + ); + } + + #[test] + fn built_log_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 an upload. + let validator = SchemaValidator::v201(); + for status in [ + UploadLogStatusEnumType::Idle, + UploadLogStatusEnumType::Uploading, + UploadLogStatusEnumType::Uploaded, + UploadLogStatusEnumType::UploadFailure, + UploadLogStatusEnumType::AcceptedCanceled, + ] { + for request_id in [None, Some(0), Some(-1), Some(i32::MIN), Some(i32::MAX)] { + let req = v201_log_status_report(status, request_id); + let payload = serde_json::to_value(&req).unwrap(); + assert!( + validator + .validate_call("LogStatusNotification", &payload) + .is_ok(), + "built {status:?} LogStatusNotification.req (requestId {request_id:?}) \ + should be schema-valid, got: {payload}" + ); + } + } + } + // --- FirmwareStatusNotification (v201) async update flow (#534) ------------- #[test] diff --git a/crates/ocpp-cp/src/v201_log_upload.rs b/crates/ocpp-cp/src/v201_log_upload.rs index 0e6e10c..b44a747 100644 --- a/crates/ocpp-cp/src/v201_log_upload.rs +++ b/crates/ocpp-cp/src/v201_log_upload.rs @@ -30,19 +30,68 @@ //! can be shared across the charge point's tasks, exactly like the v201 //! [`V201DisplayMessageStore`](crate::v201_display_message::V201DisplayMessageStore). +use ocpp_types::v201::UploadLogStatusEnumType; use tokio::sync::RwLock; +/// The most recent log-upload status the station reported, retained so a +/// `TriggerMessage(LogStatusNotification)` can re-report it on demand (Issue +/// #584) — the OCPP 2.0.1 twin of the firmware +/// [`V201FirmwareStatusReport`](crate::v201_firmware_update::V201FirmwareStatusReport) +/// and of the 1.6J `TriggerMessage(DiagnosticsStatusNotification)` re-report. +/// +/// A station reports upload progress asynchronously +/// (`LogStatusNotification(Uploading → Uploaded)`), but a CSMS may ask for the +/// *current* status at any point via `TriggerMessage`. This snapshot is the +/// answer: [`Idle`](UploadLogStatusEnumType::Idle) with no `request_id` until the +/// first `GetLog` progress step is emitted, then the latest `(status, requestId)` +/// thereafter — retained even after the in-flight slot is cleared, so a settled +/// `Uploaded` (or a terminal `UploadFailure` / `AcceptedCanceled`) stays +/// reportable. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct V201LogStatusReport { + /// The latest reported stage of the log-upload lifecycle. + pub status: UploadLogStatusEnumType, + /// The `requestId` of the `GetLog` that stage belongs to, or `None` for the + /// initial [`Idle`](UploadLogStatusEnumType::Idle) (no upload 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 V201LogStatusReport { + /// A station that has never run an upload: + /// [`Idle`](UploadLogStatusEnumType::Idle), no correlating `requestId`. + fn default() -> Self { + Self { + status: UploadLogStatusEnumType::Idle, + request_id: None, + } + } +} + /// Tracks the single `GetLog` upload a station is currently serving, by its -/// `requestId`. +/// `requestId`, plus the latest log-upload status it has reported. +/// +/// For the in-flight slot: `None` means idle (no upload in flight); +/// `Some(request_id)` names the request whose upload 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 upload in flight); `Some(request_id)` names the request -/// whose upload 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 +/// [`V201LogStatusReport`] the station emitted (via +/// [`record_reported`](Self::record_reported)), so a +/// `TriggerMessage(LogStatusNotification)` can re-report the current status +/// without re-running the upload. It is deliberately *not* cleared when the +/// in-flight slot is ([`complete`](Self::complete) / [`clear`](Self::clear)): a +/// finished upload leaves the station idle but its last status (`Uploaded`, or a +/// terminal failure) remains the truthful thing to report. #[derive(Debug, Default)] pub struct V201LogUploadStore { /// The `requestId` of the upload currently in flight, or `None` when idle. in_flight: RwLock>, + /// The most recent log-upload status the station reported. `Idle`/`None` + /// until the first progress step; see [`V201LogStatusReport`]. + last_reported: RwLock, } impl V201LogUploadStore { @@ -122,6 +171,34 @@ impl V201LogUploadStore { false } } + + /// Record `status` (from `GetLog` request `request_id`) as the latest + /// log-upload status the station has reported. + /// + /// Called at the single emit choke point + /// (`ChargePoint::send_v201_log_status`) for every progress step, so the + /// snapshot tracks the full lifecycle — the interim `Uploading` and the + /// terminal `Uploaded` / `UploadFailure` / `AcceptedCanceled`. It is + /// independent of the in-flight slot: a completed upload clears + /// [`in_flight`](Self::in_flight) but this retains the terminal status so a + /// later `TriggerMessage(LogStatusNotification)` still re-reports it. + /// `request_id` is only stored, never parsed or indexed. + pub async fn record_reported(&self, status: UploadLogStatusEnumType, request_id: i32) { + *self.last_reported.write().await = V201LogStatusReport { + status, + request_id: Some(request_id), + }; + } + + /// The most recent [`V201LogStatusReport`] the station has reported. + /// + /// The snapshot a `TriggerMessage(LogStatusNotification)` re-reports on + /// demand. Defaults to [`Idle`](UploadLogStatusEnumType::Idle) with no + /// `requestId` on a station that has never run an upload. Returns a copy, so + /// the caller decides without holding the store lock. + pub async fn last_reported(&self) -> V201LogStatusReport { + *self.last_reported.read().await + } } #[cfg(test)] @@ -226,4 +303,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 = V201LogUploadStore::new(); + assert_eq!( + store.last_reported().await, + V201LogStatusReport { + status: UploadLogStatusEnumType::Idle, + request_id: None, + }, + "a station that has never run an upload reports Idle, no requestId" + ); + } + + #[tokio::test] + async fn record_reported_tracks_the_latest_status_and_request_id() { + let store = V201LogUploadStore::new(); + store + .record_reported(UploadLogStatusEnumType::Uploading, 42) + .await; + assert_eq!( + store.last_reported().await, + V201LogStatusReport { + status: UploadLogStatusEnumType::Uploading, + request_id: Some(42), + } + ); + // The latest wins — a later step overwrites the earlier one. + store + .record_reported(UploadLogStatusEnumType::Uploaded, 42) + .await; + assert_eq!( + store.last_reported().await, + V201LogStatusReport { + status: UploadLogStatusEnumType::Uploaded, + request_id: Some(42), + } + ); + } + + #[tokio::test] + async fn completing_the_upload_retains_the_last_reported_status() { + // A settled upload returns the in-flight slot to idle, but the terminal + // status must remain reportable for a later TriggerMessage. + let store = V201LogUploadStore::new(); + store.begin(9).await; + store + .record_reported(UploadLogStatusEnumType::Uploaded, 9) + .await; + assert!(store.complete(9).await); + assert!(store.is_idle().await, "in-flight slot cleared"); + assert_eq!( + store.last_reported().await, + V201LogStatusReport { + status: UploadLogStatusEnumType::Uploaded, + request_id: Some(9), + }, + "the terminal status survives the in-flight clear" + ); + } + + #[tokio::test] + async fn record_reported_accepts_extreme_request_ids() { + let store = V201LogUploadStore::new(); + store + .record_reported(UploadLogStatusEnumType::UploadFailure, i32::MIN) + .await; + assert_eq!(store.last_reported().await.request_id, Some(i32::MIN)); + store + .record_reported(UploadLogStatusEnumType::AcceptedCanceled, 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 21b6cd5..419fe6d 100644 --- a/crates/ocpp-cp/tests/central_system_boot.rs +++ b/crates/ocpp-cp/tests/central_system_boot.rs @@ -941,17 +941,18 @@ 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(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.) + // CSMS -> CP: TriggerMessage(PublishFirmwareStatusNotification). The simulator + // does not yet originate a standalone PublishFirmwareStatusNotification on the + // v201 path (the trigger for it is tracked by #585), so the policy classifies + // it NotImplemented and the wiring enqueues nothing. (FirmwareStatusNotification + // and LogStatusNotification are now Accepted — they re-report the latest + // firmware / log-upload status, #583 / #584 — so neither is the NotImplemented + // example here.) let resp = server .call::( "CP201_TRIG_NIMP", V201TriggerMessageRequest { - requested_message: MessageTriggerEnumType::LogStatusNotification, + requested_message: MessageTriggerEnumType::PublishFirmwareStatusNotification, evse: None, custom_data: None, }, From 73c8d98cd7930b36b124d2c5398265448bf2d381 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 28 Sep 2026 17:05:43 +0000 Subject: [PATCH 2/2] docs(ocpp-cp): qualify the intra-doc link to trigger_v201_firmware_status_notification The bare [`trigger_v201_firmware_status_notification`] link in the new trigger_v201_log_status_notification doc comment did not resolve (a method needs a qualified path), failing the `--deny warnings` Documentation CI job. Qualify it with `Self::` so rustdoc resolves it. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0176cR1foeU8uwtbWZQDdsPZ --- crates/ocpp-cp/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/ocpp-cp/src/lib.rs b/crates/ocpp-cp/src/lib.rs index 4dd86e5..af20b0c 100644 --- a/crates/ocpp-cp/src/lib.rs +++ b/crates/ocpp-cp/src/lib.rs @@ -9297,8 +9297,9 @@ impl ChargePoint { /// Re-report the station's latest log-upload status for a `TriggerMessage` /// (`requestedMessage = LogStatusNotification`, Issue #584). /// - /// The log-upload twin of [`trigger_v201_firmware_status_notification`] and - /// the 2.0.1 analog of the 1.6J `TriggerMessage(DiagnosticsStatusNotification)` + /// The log-upload twin of + /// [`trigger_v201_firmware_status_notification`](Self::trigger_v201_firmware_status_notification) + /// and the 2.0.1 analog of the 1.6J `TriggerMessage(DiagnosticsStatusNotification)` /// re-report: a CSMS asks for the *current* log-upload status, and the station /// answers with a single `LogStatusNotification` carrying the latest status it /// has reported — without re-running the upload. The snapshot comes from