From 0ed90b1dc9bba68d12fa1dd95e652d2abda001aa Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Thu, 9 Jul 2026 19:08:03 +0200 Subject: [PATCH] fix(ocpp): cross-check OCPP 1.6 lifecycle flags in trigger, re-entry, and simulation entry (#1987) The OCPP 1.6 TriggerMessage handler cases for DiagnosticsStatusNotification and FirmwareStatusNotification read stationInfo.diagnosticsStatus and stationInfo.firmwareStatus directly. When an exception, abort, or lifecycle drift left stationInfo.*Status at a stale non-terminal value, the trigger faithfully reported that stale value to the CSMS instead of Idle. The identical anti-pattern was present in handleRequestUpdateFirmware's re-entry guard (which suppressed legitimate new UpdateFirmware.req when stationInfo.firmwareStatus was stuck at Downloading / Downloaded / Installing) and in the base-class event dispatcher's unconditional fan-out of UPDATE_FIRMWARE emits (which spawned duplicate concurrent updateFirmwareSimulation invocations emitting duplicate progress notifications to the CSMS). Track lifecycle progress with two per-station boolean flags on OCPP16StationState (diagnosticsUploadInProgress, firmwareUpdateInProgress), set on lifecycle entry and cleared in a finally block on every exit path (happy, return, throw). The flags are consumed at four sites: the two TriggerMessage cases, the handleRequestUpdateFirmware re-entry guard, and the updateFirmwareSimulation entry guard. OCPP 1.6 GetDiagnostics.req and UpdateFirmware.req core-profile payloads do not carry a requestId (unlike OCPP 2.0.1 GetLog and UpdateFirmware), so lifecycle progress is tracked with boolean flags rather than requestId-presence sentinels. Satisfies OCPP 1.6 SHALL clauses in section 4.4 (Diagnostics) and section 4.5 (Firmware), and the Idle enumeration definitions in section 7.24 and section 7.25. Closes #1973 --- .../ocpp/1.6/OCPP16IncomingRequestService.ts | 350 ++++++++++-------- ...P16IncomingRequestService-Firmware.test.ts | 122 +++++- ...omingRequestService-TriggerMessage.test.ts | 336 ++++++++++++++++- ...ncomingRequestService-StationState.test.ts | 4 +- 4 files changed, 657 insertions(+), 155 deletions(-) diff --git a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts index b1dfd0c7..609bac06 100644 --- a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts +++ b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts @@ -144,25 +144,59 @@ const moduleName = 'OCPP16IncomingRequestService' /** * Per-station lifecycle state carried on {@link OCPP16IncomingRequestService}. * - * Populated incrementally as companion PRs land. Fields currently - * present: - * - `deferredFirmwareUpdateTimer` (#1972). - * - * Companion PRs still pending will add their own fields: - * - #1971 (GetDiagnostics supersession): `activeDiagnosticsAbortController`, - * `activeDiagnosticsRequestId`; - * - #1973 (trigger cross-check): `activeDiagnosticsRequestId` (shared with - * #1971), `activeFirmwareUpdateRequestId`. + * OCPP 1.6 core-profile `GetDiagnostics.req` and `UpdateFirmware.req` + * payloads do not carry a `requestId` (unlike OCPP 2.0.1 `GetLog` / + * `UpdateFirmware`, and unlike the OCPP 1.6 Security-Extension + * `SignedUpdateFirmware` variant which the simulator does not + * implement), so lifecycle progress is tracked with boolean flags + * rather than requestId-presence sentinels. */ interface OCPP16StationState { /** - * `setTimeout` handle for the deferred `updateFirmwareSimulation` + * `setTimeout` handle for a deferred `updateFirmwareSimulation` * scheduled by the `UPDATE_FIRMWARE` event listener when the incoming * request carries a future `retrieveDate`. Released by * {@link OCPP16IncomingRequestService.cancelDeferredFirmwareUpdate} - * on stop, supersession, or normal fire (#1972). + * on stop, supersession, or normal fire. */ deferredFirmwareUpdateTimer?: NodeJS.Timeout + /** + * `true` while a diagnostics upload lifecycle is active (from the + * FTP upload branch of `handleRequestGetDiagnostics` entry to its + * terminal status emission or exception unwind). Read at two sites: + * 1. The `TriggerMessage(DiagnosticsStatusNotification)` case: emits + * `Idle` unless the flag is set, so a stale + * `stationInfo.diagnosticsStatus` left at `Uploading` after an + * exception unwind does not leak to the CSMS, per OCPP 1.6 §4.4 / + * §7.24 (Idle when not busy uploading diagnostics). + * 2. `handleRequestGetDiagnostics`'s FTP-branch entry guard: + * early-returns the empty `GetDiagnostics.conf` payload when a + * prior lifecycle is still in flight, preventing concurrent FTP + * uploads from racing on the same + * `${chargingStationId}_logs.tar.gz` archive filename and + * emitting duplicate `DiagnosticsStatusNotification` progress + * messages to the CSMS. + */ + diagnosticsUploadInProgress?: boolean + /** + * `true` while a firmware update lifecycle is active (from + * `updateFirmwareSimulation` entry to its terminal status emission + * or exception unwind). Read at three sites: + * 1. The `TriggerMessage(FirmwareStatusNotification)` case: emits + * `Idle` unless the flag is set and `stationInfo.firmwareStatus` + * is in the §4.5 / §7.25 pass-through set, so a stale non-terminal + * status left by an exception unwind does not leak to the CSMS. + * 2. `handleRequestUpdateFirmware`'s re-entry guard: warn-logs and + * returns the empty `UpdateFirmware.conf` payload when a + * concurrent request arrives (OCPP 1.6 `UpdateFirmware.conf` has + * no fields to signal rejection to the CSMS). + * 3. `updateFirmwareSimulation`'s entry guard: early-returns before + * the try/finally when a prior lifecycle is still in flight, + * preventing duplicate `FirmwareStatusNotification` progress + * messages if the base-class event dispatcher fans out a second + * invocation before the first completes. + */ + firmwareUpdateInProgress?: boolean } /** @@ -423,7 +457,19 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService( chargingStation, OCPP16RequestCommand.DIAGNOSTICS_STATUS_NOTIFICATION, - { - // §4.4 + §7.24: Idle when not busy uploading diagnostics - status: - chargingStation.stationInfo?.diagnosticsStatus === - OCPP16DiagnosticsStatus.Uploading - ? OCPP16DiagnosticsStatus.Uploading - : OCPP16DiagnosticsStatus.Idle, - }, - { - triggerMessage: true, - } + { status: diagnosticsTriggerStatus }, + { triggerMessage: true } ) .catch(errorHandler) break - case OCPP16MessageTrigger.FirmwareStatusNotification: + } + case OCPP16MessageTrigger.FirmwareStatusNotification: { + // §4.5 + §7.25: Idle when not busy downloading/installing + // firmware. Cross-check the per-station lifecycle flag so + // a stale stationInfo.firmwareStatus (e.g. left at + // 'Downloading' after an exception unwind of + // updateFirmwareSimulation) does not leak to the CSMS. + const firmwareInProgress = + this.stationsState.get(chargingStation)?.firmwareUpdateInProgress === true + const firmwareStatus = chargingStation.stationInfo?.firmwareStatus + const firmwareTriggerStatus = + firmwareInProgress && + (firmwareStatus === OCPP16FirmwareStatus.Downloading || + firmwareStatus === OCPP16FirmwareStatus.Downloaded || + firmwareStatus === OCPP16FirmwareStatus.Installing) + ? firmwareStatus + : OCPP16FirmwareStatus.Idle chargingStation.ocppRequestService .requestHandler< OCPP16FirmwareStatusNotificationRequest, @@ -453,23 +506,12 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService( chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, - { - // §4.5 + §7.25: Idle when not busy downloading/installing firmware - status: - chargingStation.stationInfo?.firmwareStatus === - OCPP16FirmwareStatus.Downloading || - chargingStation.stationInfo?.firmwareStatus === - OCPP16FirmwareStatus.Downloaded || - chargingStation.stationInfo?.firmwareStatus === OCPP16FirmwareStatus.Installing - ? chargingStation.stationInfo.firmwareStatus - : OCPP16FirmwareStatus.Idle, - }, - { - triggerMessage: true, - } + { status: firmwareTriggerStatus }, + { triggerMessage: true } ) .catch(errorHandler) break + } case OCPP16MessageTrigger.Heartbeat: chargingStation.ocppRequestService .requestHandler( @@ -620,10 +662,10 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService( ConfigurationSection.log @@ -1308,6 +1354,8 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { - status: OCPP16FirmwareStatus.Downloading, - }) - if (chargingStation.stationInfo != null) { - chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloading + const stationState = this.getOrCreateStationState(chargingStation) + if (stationState.firmwareUpdateInProgress === true) { + logger.warn( + `${chargingStation.logPrefix()} ${moduleName}.updateFirmwareSimulation: skipped, a firmware update lifecycle is already in flight` + ) + return } - if ( - chargingStation.stationInfo?.firmwareUpgrade?.failureStatus === - OCPP16FirmwareStatus.DownloadFailed - ) { + stationState.firmwareUpdateInProgress = true + try { + for (const { connectorId, connectorStatus } of chargingStation.iterateConnectors(true)) { + if (connectorStatus.transactionStarted === false) { + await sendAndSetConnectorStatus(chargingStation, { + connectorId, + status: OCPP16ChargePointStatus.Unavailable, + } as OCPP16StatusNotificationRequest) + } + } + await chargingStation.ocppRequestService.requestHandler< + OCPP16FirmwareStatusNotificationRequest, + OCPP16FirmwareStatusNotificationResponse + >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { + status: OCPP16FirmwareStatus.Downloading, + }) + if (chargingStation.stationInfo != null) { + chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloading + } + if ( + chargingStation.stationInfo?.firmwareUpgrade?.failureStatus === + OCPP16FirmwareStatus.DownloadFailed + ) { + await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) + await chargingStation.ocppRequestService.requestHandler< + OCPP16FirmwareStatusNotificationRequest, + OCPP16FirmwareStatusNotificationResponse + >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { + status: chargingStation.stationInfo.firmwareUpgrade.failureStatus, + }) + chargingStation.stationInfo.firmwareStatus = + chargingStation.stationInfo.firmwareUpgrade.failureStatus + return + } await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) await chargingStation.ocppRequestService.requestHandler< OCPP16FirmwareStatusNotificationRequest, OCPP16FirmwareStatusNotificationResponse >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { - status: chargingStation.stationInfo.firmwareUpgrade.failureStatus, + status: OCPP16FirmwareStatus.Downloaded, }) - chargingStation.stationInfo.firmwareStatus = - chargingStation.stationInfo.firmwareUpgrade.failureStatus - return - } - await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) - await chargingStation.ocppRequestService.requestHandler< - OCPP16FirmwareStatusNotificationRequest, - OCPP16FirmwareStatusNotificationResponse - >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { - status: OCPP16FirmwareStatus.Downloaded, - }) - if (chargingStation.stationInfo != null) { - chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloaded - } - let wasTransactionsStarted = false - let transactionsStarted: boolean - do { - const runningTransactions = chargingStation.getNumberOfRunningTransactions() - if (runningTransactions > 0) { - const waitTime = secondsToMilliseconds(15) - logger.debug( - `${chargingStation.logPrefix()} ${moduleName}.updateFirmwareSimulation: ${runningTransactions.toString()} transaction(s) in progress, waiting ${formatDurationMilliSeconds( - waitTime - )} before continuing firmware update simulation` - ) - await sleep(waitTime) - transactionsStarted = true - wasTransactionsStarted = true - } else { - for (const { connectorId, connectorStatus } of chargingStation.iterateConnectors(true)) { - if (connectorStatus.status !== OCPP16ChargePointStatus.Unavailable) { - await sendAndSetConnectorStatus(chargingStation, { - connectorId, - status: OCPP16ChargePointStatus.Unavailable, - } as OCPP16StatusNotificationRequest) + if (chargingStation.stationInfo != null) { + chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloaded + } + let wasTransactionsStarted = false + let transactionsStarted: boolean + do { + const runningTransactions = chargingStation.getNumberOfRunningTransactions() + if (runningTransactions > 0) { + const waitTime = secondsToMilliseconds(15) + logger.debug( + `${chargingStation.logPrefix()} ${moduleName}.updateFirmwareSimulation: ${runningTransactions.toString()} transaction(s) in progress, waiting ${formatDurationMilliSeconds( + waitTime + )} before continuing firmware update simulation` + ) + await sleep(waitTime) + transactionsStarted = true + wasTransactionsStarted = true + } else { + for (const { connectorId, connectorStatus } of chargingStation.iterateConnectors(true)) { + if (connectorStatus.status !== OCPP16ChargePointStatus.Unavailable) { + await sendAndSetConnectorStatus(chargingStation, { + connectorId, + status: OCPP16ChargePointStatus.Unavailable, + } as OCPP16StatusNotificationRequest) + } } + transactionsStarted = false } - transactionsStarted = false + } while (transactionsStarted) + !wasTransactionsStarted && + (await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1)))) + if (!checkChargingStationState(chargingStation, chargingStation.logPrefix())) { + return } - } while (transactionsStarted) - !wasTransactionsStarted && - (await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1)))) - if (!checkChargingStationState(chargingStation, chargingStation.logPrefix())) { - return - } - await chargingStation.ocppRequestService.requestHandler< - OCPP16FirmwareStatusNotificationRequest, - OCPP16FirmwareStatusNotificationResponse - >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { - status: OCPP16FirmwareStatus.Installing, - }) - if (chargingStation.stationInfo != null) { - chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Installing - } - if ( - chargingStation.stationInfo?.firmwareUpgrade?.failureStatus === - OCPP16FirmwareStatus.InstallationFailed - ) { - await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) await chargingStation.ocppRequestService.requestHandler< OCPP16FirmwareStatusNotificationRequest, OCPP16FirmwareStatusNotificationResponse >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { - status: chargingStation.stationInfo.firmwareUpgrade.failureStatus, + status: OCPP16FirmwareStatus.Installing, }) - chargingStation.stationInfo.firmwareStatus = - chargingStation.stationInfo.firmwareUpgrade.failureStatus - return - } - await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) - await chargingStation.ocppRequestService.requestHandler< - OCPP16FirmwareStatusNotificationRequest, - OCPP16FirmwareStatusNotificationResponse - >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { - status: OCPP16FirmwareStatus.Installed, - }) - if (chargingStation.stationInfo != null) { - chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Installed - } - if (chargingStation.stationInfo?.firmwareUpgrade?.reset === true) { + if (chargingStation.stationInfo != null) { + chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Installing + } + if ( + chargingStation.stationInfo?.firmwareUpgrade?.failureStatus === + OCPP16FirmwareStatus.InstallationFailed + ) { + await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) + await chargingStation.ocppRequestService.requestHandler< + OCPP16FirmwareStatusNotificationRequest, + OCPP16FirmwareStatusNotificationResponse + >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { + status: chargingStation.stationInfo.firmwareUpgrade.failureStatus, + }) + chargingStation.stationInfo.firmwareStatus = + chargingStation.stationInfo.firmwareUpgrade.failureStatus + return + } await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) - await chargingStation.reset(OCPP16StopTransactionReason.REBOOT) + await chargingStation.ocppRequestService.requestHandler< + OCPP16FirmwareStatusNotificationRequest, + OCPP16FirmwareStatusNotificationResponse + >(chargingStation, OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION, { + status: OCPP16FirmwareStatus.Installed, + }) + if (chargingStation.stationInfo != null) { + chargingStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Installed + } + if (chargingStation.stationInfo?.firmwareUpgrade?.reset === true) { + await sleep(secondsToMilliseconds(randomInt(minDelay, maxDelay + 1))) + await chargingStation.reset(OCPP16StopTransactionReason.REBOOT) + } + } finally { + delete stationState.firmwareUpdateInProgress } } } diff --git a/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts b/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts index e4a4b645..3cab7823 100644 --- a/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts +++ b/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-Firmware.test.ts @@ -12,13 +12,14 @@ import type { GetDiagnosticsRequest } from '../../../../src/types/index.js' import { OCPP16IncomingRequestService } from '../../../../src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.js' import { + ConfigurationSection, OCPP16FirmwareStatus, OCPP16IncomingRequestCommand, OCPP16StandardParametersKey, type OCPP16UpdateFirmwareRequest, type OCPP16UpdateFirmwareResponse, } from '../../../../src/types/index.js' -import { Constants, logger } from '../../../../src/utils/index.js' +import { Configuration, Constants, logger } from '../../../../src/utils/index.js' import { flushMicrotasks, standardCleanup, @@ -101,6 +102,79 @@ await describe('OCPP16IncomingRequestService — Firmware', async () => { assert.notStrictEqual(response, undefined) assert.strictEqual(Object.keys(response).length, 0) }) + + await it('should clear diagnosticsUploadInProgress in the finally block when the FTP branch returns early on missing log file', async t => { + // Arrange: enable FirmwareManagement so the FTP branch is entered, + // then force the early return path inside the try block by returning + // a log configuration with no file. This exercises the finally clause + // through a non-throwing exit path. + const { incomingRequestService, station, testableService } = context + upsertConfigurationKey( + station, + OCPP16StandardParametersKey.SupportedFeatureProfiles, + 'Core,FirmwareManagement' + ) + t.mock.method(Configuration, 'getConfigurationSection', (section: ConfigurationSection) => { + if (section === ConfigurationSection.log) { + return { file: undefined } + } + return {} + }) + + const request: GetDiagnosticsRequest = { + location: 'ftp://localhost/diagnostics', + } + + // Act + const response = await testableService.handleRequestGetDiagnostics(station, request) + + // Assert + assert.strictEqual(Object.keys(response).length, 0) + const plumbing = incomingRequestService as unknown as { + stationsState: WeakMap + } + assert.strictEqual( + plumbing.stationsState.get(station)?.diagnosticsUploadInProgress, + undefined + ) + }) + + await it('should skip the diagnostics FTP upload when diagnosticsUploadInProgress is already set (concurrent invocation safety)', async t => { + // Regression guard: two concurrent GetDiagnostics.req arriving via + // the base-class dispatcher's fire-and-forget WebSocket message + // handling must not spawn concurrent FTP uploads racing on the same + // ${chargingStationId}_logs.tar.gz archive filename or emit + // duplicate DiagnosticsStatusNotification progress messages to the + // CSMS. + + // Arrange + const { incomingRequestService, station, testableService } = context + upsertConfigurationKey( + station, + OCPP16StandardParametersKey.SupportedFeatureProfiles, + 'Core,FirmwareManagement' + ) + const plumbing = incomingRequestService as unknown as { + getOrCreateStationState: (cs: ChargingStation) => { diagnosticsUploadInProgress?: boolean } + stationsState: WeakMap + } + plumbing.getOrCreateStationState(station).diagnosticsUploadInProgress = true + const warnSpy = t.mock.method(logger, 'warn') + + // Act + const response = await testableService.handleRequestGetDiagnostics(station, { + location: 'ftp://localhost/diagnostics', + }) + + // Assert + assert.strictEqual(Object.keys(response).length, 0) + assert.strictEqual(plumbing.stationsState.get(station)?.diagnosticsUploadInProgress, true) + const warnMessages = warnSpy.mock.calls.map(call => call.arguments[0] as unknown as string) + assert.ok( + warnMessages.some(m => m.includes('a diagnostics upload lifecycle is already in flight')), + `expected an "already in flight" warning, got: ${JSON.stringify(warnMessages)}` + ) + }) }) // §6.4: UpdateFirmware @@ -142,7 +216,42 @@ await describe('OCPP16IncomingRequestService — Firmware', async () => { assert.strictEqual(Object.keys(response).length, 0) }) - await it('should return empty response when firmware update is already in progress', () => { + await it('should return empty response and log the "already in progress" warning when firmwareUpdateInProgress is set', t => { + // Arrange + const { incomingRequestService, station, testableService } = context + upsertConfigurationKey( + station, + OCPP16StandardParametersKey.SupportedFeatureProfiles, + 'Core,FirmwareManagement' + ) + const plumbing = incomingRequestService as unknown as { + getOrCreateStationState: (cs: ChargingStation) => { firmwareUpdateInProgress?: boolean } + } + plumbing.getOrCreateStationState(station).firmwareUpdateInProgress = true + const warnSpy = t.mock.method(logger, 'warn') + + // Act + const response = testableService.handleRequestUpdateFirmware(station, { + location: 'ftp://localhost/firmware.bin', + retrieveDate: new Date(), + }) + + // Assert + assert.strictEqual(Object.keys(response).length, 0) + const warnMessages = warnSpy.mock.calls.map(call => call.arguments[0] as unknown as string) + assert.ok( + warnMessages.some(m => m.includes('firmware update is already in progress')), + `expected an "already in progress" warning, got: ${JSON.stringify(warnMessages)}` + ) + }) + + await it('should NOT log the "already in progress" warning when stationInfo.firmwareStatus is stale but firmwareUpdateInProgress is unset', t => { + // Regression guard: a stale stationInfo.firmwareStatus left at + // Downloading / Downloaded / Installing by a prior exception unwind + // must not suppress a legitimate new UpdateFirmware.req. The + // authoritative predicate is firmwareUpdateInProgress on the + // per-station state, not stationInfo. + // Arrange const { station, testableService } = context upsertConfigurationKey( @@ -153,6 +262,7 @@ await describe('OCPP16IncomingRequestService — Firmware', async () => { if (station.stationInfo != null) { station.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloading } + const warnSpy = t.mock.method(logger, 'warn') // Act const response = testableService.handleRequestUpdateFirmware(station, { @@ -161,8 +271,12 @@ await describe('OCPP16IncomingRequestService — Firmware', async () => { }) // Assert - assert.notStrictEqual(response, undefined) assert.strictEqual(Object.keys(response).length, 0) + const warnMessages = warnSpy.mock.calls.map(call => call.arguments[0] as unknown as string) + assert.ok( + warnMessages.every(m => !m.includes('firmware update is already in progress')), + `expected no "already in progress" warning, got: ${JSON.stringify(warnMessages)}` + ) }) }) @@ -271,7 +385,7 @@ await describe('OCPP16IncomingRequestService — Firmware', async () => { }) }) - // #1972: deferred UpdateFirmware timer lifecycle on OCPP16StationState + // Deferred UpdateFirmware timer lifecycle on OCPP16StationState await describe('UPDATE_FIRMWARE deferred timer lifecycle', async () => { interface OCPP16StationStateShape { deferredFirmwareUpdateTimer?: NodeJS.Timeout diff --git a/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-TriggerMessage.test.ts b/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-TriggerMessage.test.ts index a4f20409..d05d4572 100644 --- a/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-TriggerMessage.test.ts +++ b/tests/charging-station/ocpp/1.6/OCPP16IncomingRequestService-TriggerMessage.test.ts @@ -1,19 +1,28 @@ /** * @file Tests for OCPP16IncomingRequestService TriggerMessage handler * @description Tests for TriggerMessage (§10.1) incoming request handler covering - * accepted triggers, unimplemented triggers, and feature profile validation + * accepted triggers, unimplemented triggers, feature profile validation, and + * the per-station lifecycle-flag cross-check that satisfies OCPP 1.6 §4.4 / + * §7.24 (DiagnosticsStatusNotification Idle when not busy uploading) and + * §4.5 / §7.25 (FirmwareStatusNotification Idle when not busy downloading / + * installing). */ import assert from 'node:assert/strict' import { afterEach, beforeEach, describe, it, mock } from 'node:test' +import type { ChargingStation } from '../../../../src/charging-station/index.js' import type { + OCPP16DiagnosticsStatusNotificationRequest, + OCPP16FirmwareStatusNotificationRequest, OCPP16TriggerMessageRequest, OCPP16TriggerMessageResponse, } from '../../../../src/types/index.js' import { OCPP16IncomingRequestService } from '../../../../src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.js' import { + OCPP16DiagnosticsStatus, + OCPP16FirmwareStatus, OCPP16IncomingRequestCommand, OCPP16MessageTrigger, OCPP16RequestCommand, @@ -21,7 +30,7 @@ import { OCPP16TriggerMessageStatus, OCPPVersion, } from '../../../../src/types/index.js' -import { Constants } from '../../../../src/utils/index.js' +import { Constants, logger } from '../../../../src/utils/index.js' import { flushMicrotasks, standardCleanup } from '../../../helpers/TestLifecycleHelpers.js' import { TEST_CHARGING_STATION_BASE_NAME } from '../../ChargingStationTestConstants.js' import { createMockChargingStation } from '../../helpers/StationHelpers.js' @@ -305,4 +314,327 @@ await describe('OCPP16IncomingRequestService — TriggerMessage', async () => { assert.strictEqual(rejectingMock.mock.callCount(), 1) }) }) + + // OCPP 1.6 §4.4 / §7.24 (Diagnostics) and §4.5 / §7.25 (Firmware): + // the Charge Point SHALL only send status Idle after receipt of a + // TriggerMessage when it is not busy uploading diagnostics / + // downloading or installing firmware. These tests exercise the + // lifecycle-flag cross-check that collapses stale stationInfo + // values to Idle when no lifecycle is actually active. + await describe('lifecycle-flag cross-check', async () => { + interface OCPP16StationStateShape { + diagnosticsUploadInProgress?: boolean + firmwareUpdateInProgress?: boolean + } + + interface PlumbingAccess { + getOrCreateStationState: (chargingStation: ChargingStation) => OCPP16StationStateShape + stationsState: WeakMap + } + + const asPlumbing = (service: OCPP16IncomingRequestService): PlumbingAccess => + service as unknown as PlumbingAccess + + const emitTrigger = ( + service: OCPP16IncomingRequestService, + station: ChargingStation, + trigger: OCPP16MessageTrigger + ): void => { + const request: OCPP16TriggerMessageRequest = { requestedMessage: trigger } + const response: OCPP16TriggerMessageResponse = { + status: OCPP16TriggerMessageStatus.ACCEPTED, + } + service.emit(OCPP16IncomingRequestCommand.TRIGGER_MESSAGE, station, request, response) + } + + let service: OCPP16IncomingRequestService + let plumbing: PlumbingAccess + let station: ChargingStation + let requestHandlerMock: ReturnType + + beforeEach(() => { + service = new OCPP16IncomingRequestService() + plumbing = asPlumbing(service) + ;({ requestHandlerMock, station } = createOCPP16ListenerStation('listener-lifecycle-flag')) + }) + + afterEach(() => { + standardCleanup() + }) + + await it('should emit Idle for DiagnosticsStatusNotification trigger when no upload is in progress', async () => { + emitTrigger(service, station, OCPP16MessageTrigger.DiagnosticsStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16DiagnosticsStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[1], OCPP16RequestCommand.DIAGNOSTICS_STATUS_NOTIFICATION) + assert.strictEqual(args[2].status, OCPP16DiagnosticsStatus.Idle) + }) + + await it('should emit Uploading for DiagnosticsStatusNotification trigger when upload is in progress', async () => { + plumbing.getOrCreateStationState(station).diagnosticsUploadInProgress = true + if (station.stationInfo != null) { + station.stationInfo.diagnosticsStatus = OCPP16DiagnosticsStatus.Uploading + } + + emitTrigger(service, station, OCPP16MessageTrigger.DiagnosticsStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16DiagnosticsStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[1], OCPP16RequestCommand.DIAGNOSTICS_STATUS_NOTIFICATION) + assert.strictEqual(args[2].status, OCPP16DiagnosticsStatus.Uploading) + }) + + await it('should emit Idle for DiagnosticsStatusNotification when stationInfo is stale but lifecycle is not active', async () => { + // Regression guard: stationInfo.diagnosticsStatus is left at + // `Uploading` by a prior exception unwind that did not reset it, + // but the lifecycle flag correctly reports no active upload. + if (station.stationInfo != null) { + station.stationInfo.diagnosticsStatus = OCPP16DiagnosticsStatus.Uploading + } + assert.strictEqual(plumbing.stationsState.has(station), false) + + emitTrigger(service, station, OCPP16MessageTrigger.DiagnosticsStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16DiagnosticsStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[2].status, OCPP16DiagnosticsStatus.Idle) + }) + + await it('should emit Idle for FirmwareStatusNotification trigger when no firmware update is in progress', async () => { + emitTrigger(service, station, OCPP16MessageTrigger.FirmwareStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16FirmwareStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[1], OCPP16RequestCommand.FIRMWARE_STATUS_NOTIFICATION) + assert.strictEqual(args[2].status, OCPP16FirmwareStatus.Idle) + }) + + const activeFirmwareStatuses: OCPP16FirmwareStatus[] = [ + OCPP16FirmwareStatus.Downloading, + OCPP16FirmwareStatus.Downloaded, + OCPP16FirmwareStatus.Installing, + ] + + for (const activeStatus of activeFirmwareStatuses) { + await it(`should emit ${activeStatus} for FirmwareStatusNotification trigger when lifecycle is active with ${activeStatus}`, async () => { + plumbing.getOrCreateStationState(station).firmwareUpdateInProgress = true + if (station.stationInfo != null) { + station.stationInfo.firmwareStatus = activeStatus + } + + emitTrigger(service, station, OCPP16MessageTrigger.FirmwareStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16FirmwareStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[2].status, activeStatus) + }) + } + + await it('should emit Idle for FirmwareStatusNotification when stationInfo is stale but lifecycle is not active', async () => { + // Regression guard for the firmware-side stale stationInfo scenario. + if (station.stationInfo != null) { + station.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloading + } + assert.strictEqual(plumbing.stationsState.has(station), false) + + emitTrigger(service, station, OCPP16MessageTrigger.FirmwareStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16FirmwareStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[2].status, OCPP16FirmwareStatus.Idle) + }) + + await it('should emit Idle for FirmwareStatusNotification when lifecycle is active but stationInfo status is terminal (DownloadFailed)', async () => { + // §4.5 / §7.25: only Downloading / Downloaded / Installing carry over; + // any other value (including terminal failure statuses) collapses to Idle. + plumbing.getOrCreateStationState(station).firmwareUpdateInProgress = true + if (station.stationInfo != null) { + station.stationInfo.firmwareStatus = OCPP16FirmwareStatus.DownloadFailed + } + + emitTrigger(service, station, OCPP16MessageTrigger.FirmwareStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const args = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16FirmwareStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[2].status, OCPP16FirmwareStatus.Idle) + }) + + await it('should clear firmwareUpdateInProgress in the finally block of updateFirmwareSimulation and then emit Idle on trigger', async () => { + // The `try/finally` around updateFirmwareSimulation's body guarantees + // firmwareUpdateInProgress is cleared on every exit path (happy, + // return, throw). We exercise the throw path by rejecting the first + // request emitted from inside the try block. + const rejectingHandler = mock.fn(async () => + Promise.reject(new Error('simulated network error')) + ) + const { station: rejectStation } = createMockChargingStation({ + baseName: TEST_CHARGING_STATION_BASE_NAME, + connectorsCount: 2, + ocppRequestService: { requestHandler: rejectingHandler }, + started: true, + stationInfo: { ocppVersion: OCPPVersion.VERSION_16 }, + websocketPingInterval: Constants.DEFAULT_WS_PING_INTERVAL_SECONDS, + }) + + const invokeSimulation = ( + service as unknown as { + updateFirmwareSimulation: ( + chargingStation: ChargingStation, + maxDelay?: number, + minDelay?: number + ) => Promise + } + ).updateFirmwareSimulation.bind(service) + + await assert.rejects(invokeSimulation(rejectStation)) + + assert.strictEqual( + plumbing.stationsState.get(rejectStation)?.firmwareUpdateInProgress, + undefined + ) + + // Now that the flag is cleared, TriggerMessage(FirmwareStatusNotification) + // should collapse any lingering stationInfo status to Idle. + if (rejectStation.stationInfo != null) { + rejectStation.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloading + } + const listenerHandlerMock = mock.fn((..._args: unknown[]) => Promise.resolve({})) + rejectStation.ocppRequestService.requestHandler = + listenerHandlerMock as typeof rejectStation.ocppRequestService.requestHandler + emitTrigger(service, rejectStation, OCPP16MessageTrigger.FirmwareStatusNotification) + await flushMicrotasks() + + assert.strictEqual(listenerHandlerMock.mock.callCount(), 1) + const args = listenerHandlerMock.mock.calls[0].arguments as unknown as [ + unknown, + OCPP16RequestCommand, + OCPP16FirmwareStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(args[2].status, OCPP16FirmwareStatus.Idle) + }) + + await it('should skip updateFirmwareSimulation when firmwareUpdateInProgress is already set (concurrent invocation safety)', async t => { + // Regression guard: if the base-class event dispatcher fans out a + // second UPDATE_FIRMWARE emit while a prior lifecycle is still in + // flight, the second updateFirmwareSimulation invocation must + // early-return before entering the try/finally — otherwise duplicate + // FirmwareStatusNotification progress messages would reach the CSMS. + + // Arrange + const requestHandlerSpy = mock.fn((..._args: unknown[]) => Promise.resolve({})) + const { station: concurrentStation } = createMockChargingStation({ + baseName: TEST_CHARGING_STATION_BASE_NAME, + connectorsCount: 2, + ocppRequestService: { requestHandler: requestHandlerSpy }, + started: true, + stationInfo: { ocppVersion: OCPPVersion.VERSION_16 }, + websocketPingInterval: Constants.DEFAULT_WS_PING_INTERVAL_SECONDS, + }) + plumbing.getOrCreateStationState(concurrentStation).firmwareUpdateInProgress = true + const warnSpy = t.mock.method(logger, 'warn') + const invokeSimulation = ( + service as unknown as { + updateFirmwareSimulation: ( + chargingStation: ChargingStation, + maxDelay?: number, + minDelay?: number + ) => Promise + } + ).updateFirmwareSimulation.bind(service) + + // Act + await invokeSimulation(concurrentStation) + + // Assert + assert.strictEqual(requestHandlerSpy.mock.callCount(), 0) + assert.strictEqual( + plumbing.stationsState.get(concurrentStation)?.firmwareUpdateInProgress, + true + ) + const warnMessages = warnSpy.mock.calls.map(call => call.arguments[0] as unknown as string) + assert.ok( + warnMessages.some(m => m.includes('a firmware update lifecycle is already in flight')), + `expected an "already in flight" warning, got: ${JSON.stringify(warnMessages)}` + ) + }) + + await it('should isolate lifecycle flags across stations so a trigger on B is unaffected by A', async () => { + const { requestHandlerMock: handlerMockB, station: stationB } = createOCPP16ListenerStation( + 'listener-lifecycle-flag-b' + ) + plumbing.getOrCreateStationState(station).firmwareUpdateInProgress = true + if (station.stationInfo != null) { + station.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloading + } + if (stationB.stationInfo != null) { + stationB.stationInfo.firmwareStatus = OCPP16FirmwareStatus.Downloading + } + + emitTrigger(service, station, OCPP16MessageTrigger.FirmwareStatusNotification) + emitTrigger(service, stationB, OCPP16MessageTrigger.FirmwareStatusNotification) + await flushMicrotasks() + + assert.strictEqual(requestHandlerMock.mock.callCount(), 1) + const argsA = requestHandlerMock.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16FirmwareStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(argsA[2].status, OCPP16FirmwareStatus.Downloading) + + assert.strictEqual(handlerMockB.mock.callCount(), 1) + const argsB = handlerMockB.mock.calls[0].arguments as [ + unknown, + OCPP16RequestCommand, + OCPP16FirmwareStatusNotificationRequest, + ...unknown[] + ] + assert.strictEqual(argsB[2].status, OCPP16FirmwareStatus.Idle) + }) + }) }) diff --git a/tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts b/tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts index 8a416593..54a745be 100644 --- a/tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts +++ b/tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts @@ -1,8 +1,8 @@ /** * @file Tests for OCPPIncomingRequestService per-station state plumbing (#1963) * @description Unit tests for the shared WeakMap + lazy-init + stop() template - * in `OCPPIncomingRequestService`, plus the OCPP 1.6 no-op `resetStationState` - * contract that companion PRs (#1971, #1972, #1973) will replace. + * in `OCPPIncomingRequestService` and for the OCPP 1.6 `resetStationState` + * contract. * * The OCPP 2.0.1 5-statement ordering invariant (abort firmware → abort log → * cancel retry → resetActiveFirmwareUpdateState → resetActiveLogUploadState) -- 2.53.0