From e406ab2ab278ee72b49bb1e87ec5fe70c321b9bc Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Wed, 8 Jul 2026 18:10:40 +0200 Subject: [PATCH] fix(ocpp): drive FirmwareStatusNotification trigger from last-sent notification (L01.FR.26) (#1981) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit TriggerMessage(FirmwareStatusNotification) was gated on `activeFirmwareUpdateRequestId != null && hasFirmwareUpdateInProgress()`, falling back to Idle otherwise. Since `hasFirmwareUpdateInProgress` returns false for every terminal status (`DownloadFailed`, `InvalidSignature`, `InstallationFailed`, `InstallVerificationFailed`, `Installed`), the station returned Idle after every non-Installed terminal, violating OCPP 2.0.1 L01.FR.26 (SHALL, mirrored by L02.FR.17). Persist the last-sent `(requestId, status)` on `OCPP20StationState` and drive the trigger from it. `stationInfo.firmwareStatus` is retained because `hasFirmwareUpdateInProgress` and its 3 consumers (reset rejection, isChargingStationIdle, isEvseIdle) depend on its "in-progress predicate" semantics — deliberate dual-source, not conflation. Closes #1979 --- .../ocpp/2.0/OCPP20IncomingRequestService.ts | 38 +++-- .../ocpp/2.0/__testable__/index.ts | 9 ++ ...omingRequestService-TriggerMessage.test.ts | 140 ++++++++++++++++++ 3 files changed, 175 insertions(+), 12 deletions(-) diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts index 0695d560..acc056cd 100644 --- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts +++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts @@ -262,6 +262,11 @@ const FIRMWARE_STAGE_FAILURE_STATUS = { install: OCPP20FirmwareStatusEnumType.InstallationFailed, } as const satisfies Record, OCPP20FirmwareStatusEnumType> +interface LastFirmwareStatusNotification { + requestId: number + status: OCPP20FirmwareStatusEnumType +} + interface OCPP20StationState { activeFirmwareUpdateAbortController?: AbortController activeFirmwareUpdateRequestId?: number @@ -270,6 +275,7 @@ interface OCPP20StationState { activeLogUploadStatus?: UploadLogStatusEnumType certSigningRetryManager?: OCPP20CertSigningRetryManager isDrainingSecurityEvents: boolean + lastFirmwareStatusNotification?: LastFirmwareStatusNotification preInoperativeConnectorStatuses: Map reportDataCache: Map securityEventQueue: QueuedSecurityEvent[] @@ -574,22 +580,24 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { .catch(errorHandler) break case MessageTriggerEnumType.FirmwareStatusNotification: { - const stationState = this.stationsState.get(chargingStation) - const requestId = stationState?.activeFirmwareUpdateRequestId - const firmwareStatus = - requestId != null && this.hasFirmwareUpdateInProgress(chargingStation) - ? (chargingStation.stationInfo?.firmwareStatus as OCPP20FirmwareStatusEnumType) - : OCPP20FirmwareStatusEnumType.Idle + // L01.FR.25 & L02.FR.16 — last-sent = Installed → { status: Idle }. + // L01.FR.26 & L02.FR.17 — last-sent ≠ Installed → { requestId, status: }. + // L01.FR.20 & L02.FR.14 — requestId mandatory unless status = Idle. + // Fresh station (no notification ever sent): Idle (spec-silent precondition, + // consistent with FirmwareStatusEnumType.Idle §3.33 "trigger-only" restriction). + const lastSent = this.stationsState.get(chargingStation)?.lastFirmwareStatusNotification + const payload: OCPP20FirmwareStatusNotificationRequest = + lastSent == null || lastSent.status === OCPP20FirmwareStatusEnumType.Installed + ? { status: OCPP20FirmwareStatusEnumType.Idle } + : { requestId: lastSent.requestId, status: lastSent.status } chargingStation.ocppRequestService .requestHandler< OCPP20FirmwareStatusNotificationRequest, OCPP20FirmwareStatusNotificationResponse - >( - chargingStation, - OCPP20RequestCommand.FIRMWARE_STATUS_NOTIFICATION, - { requestId, status: firmwareStatus }, - { skipBufferingOnError: true, triggerMessage: true } - ) + >(chargingStation, OCPP20RequestCommand.FIRMWARE_STATUS_NOTIFICATION, payload, { + skipBufferingOnError: true, + triggerMessage: true, + }) .catch(errorHandler) break } @@ -3537,6 +3545,12 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { if (chargingStation.stationInfo != null) { chargingStation.stationInfo.firmwareStatus = status } + // L01.FR.26 & L02.FR.17 last-sent projection: persistent source of truth for the + // trigger case, distinct from `stationInfo.firmwareStatus` (state-machine field reset by #1976). + this.getOrCreateStationState(chargingStation).lastFirmwareStatusNotification = { + requestId, + status, + } return chargingStation.ocppRequestService.requestHandler< OCPP20FirmwareStatusNotificationRequest, OCPP20FirmwareStatusNotificationResponse diff --git a/src/charging-station/ocpp/2.0/__testable__/index.ts b/src/charging-station/ocpp/2.0/__testable__/index.ts index 98fb4e0d..54ad08cf 100644 --- a/src/charging-station/ocpp/2.0/__testable__/index.ts +++ b/src/charging-station/ocpp/2.0/__testable__/index.ts @@ -27,6 +27,8 @@ import type { OCPP20DataTransferResponse, OCPP20DeleteCertificateRequest, OCPP20DeleteCertificateResponse, + OCPP20FirmwareStatusEnumType, + OCPP20FirmwareStatusNotificationResponse, OCPP20GetBaseReportRequest, OCPP20GetBaseReportResponse, OCPP20GetInstalledCertificateIdsRequest, @@ -259,6 +261,12 @@ interface TestableOCPP20IncomingRequestService { presentedIdToken: OCPP20IdTokenType, presentedGroupIdToken?: OCPP20IdTokenType ) => boolean + + sendFirmwareStatusNotification: ( + chargingStation: ChargingStation, + status: OCPP20FirmwareStatusEnumType, + requestId: number + ) => Promise } /** @@ -308,6 +316,7 @@ export function createTestableIncomingRequestService ( handleRequestUnlockConnector: serviceImpl.handleRequestUnlockConnector.bind(service), handleRequestUpdateFirmware: serviceImpl.handleRequestUpdateFirmware.bind(service), isAuthorizedToStopTransaction: serviceImpl.isAuthorizedToStopTransaction.bind(service), + sendFirmwareStatusNotification: serviceImpl.sendFirmwareStatusNotification.bind(service), } } diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-TriggerMessage.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-TriggerMessage.test.ts index 12728a46..d86d45e0 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-TriggerMessage.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-TriggerMessage.test.ts @@ -7,6 +7,7 @@ import assert from 'node:assert/strict' import { afterEach, beforeEach, describe, it, mock } from 'node:test' import type { + OCPP20FirmwareStatusNotificationRequest, OCPP20MeterValuesRequest, OCPP20StatusNotificationRequest, OCPP20TriggerMessageRequest, @@ -19,6 +20,7 @@ import { createTestableIncomingRequestService } from '../../../../src/charging-s import { OCPP20IncomingRequestService } from '../../../../src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.js' import { MessageTriggerEnumType, + OCPP20FirmwareStatusEnumType, OCPP20IncomingRequestCommand, OCPP20MeasurandEnumType, OCPP20ReadingContextEnumType, @@ -651,4 +653,142 @@ await describe('F06 - TriggerMessage', async () => { assert.strictEqual(rejectingMock.mock.callCount(), 1) }) }) + + await describe('F06 - FirmwareStatusNotification trigger last-sent semantics (L01.FR.20/25/26, L02.FR.14/16/17)', async () => { + let incomingRequestServiceForListener: OCPP20IncomingRequestService + let mockStation: MockChargingStation + let requestHandlerMock: ReturnType + let testableService: ReturnType + + beforeEach(() => { + ;({ mockStation, requestHandlerMock } = createTriggerMessageStation()) + incomingRequestServiceForListener = new OCPP20IncomingRequestService() + testableService = createTestableIncomingRequestService(incomingRequestServiceForListener) + }) + + /** + * Emit TRIGGER_MESSAGE(FirmwareStatusNotification) with an Accepted response and + * capture the resulting requestHandler payload. + * @returns The captured request handler payload and the options object it was invoked with + */ + function captureTriggeredFirmwareStatusPayload (): { + options: RequestParams + payload: OCPP20FirmwareStatusNotificationRequest + } { + const request: OCPP20TriggerMessageRequest = { + requestedMessage: MessageTriggerEnumType.FirmwareStatusNotification, + } + const response: OCPP20TriggerMessageResponse = { + status: TriggerMessageStatusEnumType.Accepted, + } + incomingRequestServiceForListener.emit( + OCPP20IncomingRequestCommand.TRIGGER_MESSAGE, + mockStation, + request, + response + ) + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + string, + OCPP20FirmwareStatusNotificationRequest, + RequestParams + ] + const [, command, payload, options] = args + assert.strictEqual(command, OCPP20RequestCommand.FIRMWARE_STATUS_NOTIFICATION) + return { options, payload } + } + + /** + * Persist a last-sent FirmwareStatusNotification via the real code path, then + * reset the mock's call history so subsequent capture assertions see only the + * trigger-fired emission. + * @param status - The FirmwareStatusNotification status to persist + * @param requestId - The requestId to persist + */ + async function seedLastFirmwareStatusNotification ( + status: OCPP20FirmwareStatusEnumType, + requestId: number + ): Promise { + await testableService.sendFirmwareStatusNotification(mockStation, status, requestId) + requestHandlerMock.mock.resetCalls() + } + + await it('should emit { requestId, status: DownloadFailed } after DownloadFailed (L01.FR.26)', async () => { + await seedLastFirmwareStatusNotification(OCPP20FirmwareStatusEnumType.DownloadFailed, 42) + + const { options, payload } = captureTriggeredFirmwareStatusPayload() + + assert.deepStrictEqual(payload, { + requestId: 42, + status: OCPP20FirmwareStatusEnumType.DownloadFailed, + }) + assert.strictEqual(options.skipBufferingOnError, true) + assert.strictEqual(options.triggerMessage, true) + }) + + await it('should emit { requestId, status: InvalidSignature } after InvalidSignature (L01.FR.26)', async () => { + await seedLastFirmwareStatusNotification(OCPP20FirmwareStatusEnumType.InvalidSignature, 7) + + const { payload } = captureTriggeredFirmwareStatusPayload() + + assert.deepStrictEqual(payload, { + requestId: 7, + status: OCPP20FirmwareStatusEnumType.InvalidSignature, + }) + }) + + await it('should emit { requestId, status: InstallationFailed } after InstallationFailed (L01.FR.26)', async () => { + await seedLastFirmwareStatusNotification(OCPP20FirmwareStatusEnumType.InstallationFailed, 99) + + const { payload } = captureTriggeredFirmwareStatusPayload() + + assert.deepStrictEqual(payload, { + requestId: 99, + status: OCPP20FirmwareStatusEnumType.InstallationFailed, + }) + }) + + await it('should emit { status: Idle } after Installed (L01.FR.25 regression)', async () => { + await seedLastFirmwareStatusNotification(OCPP20FirmwareStatusEnumType.Installed, 42) + + const { payload } = captureTriggeredFirmwareStatusPayload() + + assert.deepStrictEqual(payload, { status: OCPP20FirmwareStatusEnumType.Idle }) + assert.strictEqual(payload.requestId, undefined) + }) + + await it('should emit { status: Idle } on a fresh station (no notification ever sent)', () => { + const { payload } = captureTriggeredFirmwareStatusPayload() + + assert.deepStrictEqual(payload, { status: OCPP20FirmwareStatusEnumType.Idle }) + assert.strictEqual(payload.requestId, undefined) + }) + + const nonInstalledStatuses: OCPP20FirmwareStatusEnumType[] = [ + OCPP20FirmwareStatusEnumType.DownloadFailed, + OCPP20FirmwareStatusEnumType.DownloadPaused, + OCPP20FirmwareStatusEnumType.DownloadScheduled, + OCPP20FirmwareStatusEnumType.Downloaded, + OCPP20FirmwareStatusEnumType.Downloading, + OCPP20FirmwareStatusEnumType.InstallRebooting, + OCPP20FirmwareStatusEnumType.InstallScheduled, + OCPP20FirmwareStatusEnumType.InstallVerificationFailed, + OCPP20FirmwareStatusEnumType.InstallationFailed, + OCPP20FirmwareStatusEnumType.Installing, + OCPP20FirmwareStatusEnumType.InvalidSignature, + OCPP20FirmwareStatusEnumType.SignatureVerified, + ] + for (const [index, status] of nonInstalledStatuses.entries()) { + const requestId = 1000 + index + await it(`should echo { requestId: ${requestId.toString()}, status: ${status} } (L01.FR.20 & L01.FR.26)`, async () => { + await seedLastFirmwareStatusNotification(status, requestId) + + const { payload } = captureTriggeredFirmwareStatusPayload() + + assert.strictEqual(payload.status, status) + assert.strictEqual(payload.requestId, requestId) + }) + } + }) }) -- 2.53.0