From 5a02c45a859cdfedabf582328d62e964102e77d8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Thu, 2 Apr 2026 12:14:47 +0200 Subject: [PATCH] refactor(ocpp): consolidate variable reading through shared helpers Promote readVariableAs* helpers to public on OCPP20ServiceUtils. Replace 10 inline getVariables + manual parsing patterns with helper calls. Remove duplicated getVariableValue from CertSigningRetryManager and parseBooleanVariable from OCPP20AuthAdapter. Replace last parseInt with convertToIntOrNaN. --- .../ocpp/2.0/OCPP20CertSigningRetryManager.ts | 41 +++-- .../ocpp/2.0/OCPP20IncomingRequestService.ts | 125 ++++++------- .../ocpp/2.0/OCPP20ServiceUtils.ts | 166 +++++++++--------- .../ocpp/auth/adapters/OCPP20AuthAdapter.ts | 75 ++------ 4 files changed, 165 insertions(+), 242 deletions(-) diff --git a/src/charging-station/ocpp/2.0/OCPP20CertSigningRetryManager.ts b/src/charging-station/ocpp/2.0/OCPP20CertSigningRetryManager.ts index c883d859..487f5775 100644 --- a/src/charging-station/ocpp/2.0/OCPP20CertSigningRetryManager.ts +++ b/src/charging-station/ocpp/2.0/OCPP20CertSigningRetryManager.ts @@ -10,7 +10,7 @@ import { OCPP20RequestCommand, } from '../../../types/index.js' import { computeExponentialBackOffDelay, logger } from '../../../utils/index.js' -import { OCPP20VariableManager } from './OCPP20VariableManager.js' +import { OCPP20ServiceUtils } from './OCPP20ServiceUtils.js' const moduleName = 'OCPP20CertSigningRetryManager' @@ -51,8 +51,13 @@ export class OCPP20CertSigningRetryManager { public startRetryTimer (certificateType?: string): void { this.cancelRetryTimer() this.retryAborted = false - const waitMinimum = this.getVariableValue(OCPP20OptionalVariableName.CertSigningWaitMinimum) - if (waitMinimum == null || waitMinimum <= 0) { + const waitMinimum = OCPP20ServiceUtils.readVariableAsInteger( + this.chargingStation, + OCPP20ComponentName.SecurityCtrlr, + OCPP20OptionalVariableName.CertSigningWaitMinimum, + 0 + ) + if (waitMinimum <= 0) { logger.warn( `${this.chargingStation.logPrefix()} ${moduleName}.startRetryTimer: ${OCPP20OptionalVariableName.CertSigningWaitMinimum} not configured or invalid, retry disabled` ) @@ -64,23 +69,13 @@ export class OCPP20CertSigningRetryManager { this.scheduleNextRetry(certificateType, waitMinimum) } - private getVariableValue (variableName: OCPP20OptionalVariableName): number | undefined { - const variableManager = OCPP20VariableManager.getInstance() - const results = variableManager.getVariables(this.chargingStation, [ - { - component: { name: OCPP20ComponentName.SecurityCtrlr }, - variable: { name: variableName }, - }, - ]) - if (results.length > 0 && results[0]?.attributeValue != null) { - const parsed = parseInt(results[0].attributeValue, 10) - return Number.isNaN(parsed) ? undefined : parsed - } - return undefined - } - private scheduleNextRetry (certificateType?: string, waitMinimumSeconds?: number): void { - const maxRetries = this.getVariableValue(OCPP20OptionalVariableName.CertSigningRepeatTimes) ?? 0 + const maxRetries = OCPP20ServiceUtils.readVariableAsInteger( + this.chargingStation, + OCPP20ComponentName.SecurityCtrlr, + OCPP20OptionalVariableName.CertSigningRepeatTimes, + 0 + ) if (this.retryCount >= maxRetries) { logger.warn( `${this.chargingStation.logPrefix()} ${moduleName}.scheduleNextRetry: Max retry count ${maxRetries.toString()} reached, giving up` @@ -91,8 +86,12 @@ export class OCPP20CertSigningRetryManager { const baseDelayMs = secondsToMilliseconds( waitMinimumSeconds ?? - this.getVariableValue(OCPP20OptionalVariableName.CertSigningWaitMinimum) ?? - 60 + OCPP20ServiceUtils.readVariableAsInteger( + this.chargingStation, + OCPP20ComponentName.SecurityCtrlr, + OCPP20OptionalVariableName.CertSigningWaitMinimum, + 60 + ) ) const delayMs = computeExponentialBackOffDelay({ baseDelayMs, diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts index 14cc212b..6951bc61 100644 --- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts +++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts @@ -126,8 +126,8 @@ import { UploadLogStatusEnumType, } from '../../../types/index.js' import { - convertToBoolean, convertToDate, + convertToIntOrNaN, generateUUID, logger, promiseWithTimeout, @@ -1335,7 +1335,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ) ) if (maxChainSizeKey?.value != null) { - const maxChainSize = parseInt(maxChainSizeKey.value, 10) + const maxChainSize = convertToIntOrNaN(maxChainSizeKey.value) if (!Number.isNaN(maxChainSize) && maxChainSize > 0) { const chainByteSize = Buffer.byteLength(certificateChain, 'utf8') if (chainByteSize > maxChainSize) { @@ -1963,17 +1963,13 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { const { evseId, type } = commandPayload - const variableManager = OCPP20VariableManager.getInstance() - const allowResetResults = variableManager.getVariables(chargingStation, [ - { - component: { name: OCPP20ComponentName.EVSE }, - variable: { name: 'AllowReset' }, - }, - ]) if ( - allowResetResults.length > 0 && - allowResetResults[0].attributeValue != null && - !convertToBoolean(allowResetResults[0].attributeValue) + !OCPP20ServiceUtils.readVariableAsBoolean( + chargingStation, + OCPP20ComponentName.EVSE, + 'AllowReset', + true + ) ) { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.handleRequestReset: AllowReset is false, rejecting reset request` @@ -2215,26 +2211,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } } - const variableManager = OCPP20VariableManager.getInstance() - const currentSecurityProfileResults = variableManager.getVariables(chargingStation, [ - { - attributeType: AttributeEnumType.Actual, - component: { name: OCPP20ComponentName.SecurityCtrlr }, - variable: { name: OCPP20RequiredVariableName.SecurityProfile }, - }, - ]) - const currentSecurityProfile = Number(currentSecurityProfileResults[0]?.attributeValue ?? '0') + const currentSecurityProfile = OCPP20ServiceUtils.readVariableAsInteger( + chargingStation, + OCPP20ComponentName.SecurityCtrlr, + OCPP20RequiredVariableName.SecurityProfile, + 0 + ) const newSecurityProfile = commandPayload.connectionData.securityProfile if (newSecurityProfile < currentSecurityProfile) { // B09.FR.04 (errata 2025-09): Check AllowSecurityProfileDowngrade before rejecting - const allowDowngradeResults = variableManager.getVariables(chargingStation, [ - { - attributeType: AttributeEnumType.Actual, - component: { name: OCPP20ComponentName.SecurityCtrlr }, - variable: { name: 'AllowSecurityProfileDowngrade' }, - }, - ]) - const allowDowngrade = convertToBoolean(allowDowngradeResults[0]?.attributeValue) + const allowDowngrade = OCPP20ServiceUtils.readVariableAsBoolean( + chargingStation, + OCPP20ComponentName.SecurityCtrlr, + 'AllowSecurityProfileDowngrade', + false + ) // B09.FR.31 (errata 2025-09 §2.12): Allow downgrade except to profile 1 when enabled if (!allowDowngrade || newSecurityProfile <= 1) { @@ -2251,14 +2242,12 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } } - const priorityResults = variableManager.getVariables(chargingStation, [ - { - attributeType: AttributeEnumType.Actual, - component: { name: OCPP20ComponentName.OCPPCommCtrlr }, - variable: { name: OCPP20RequiredVariableName.NetworkConfigurationPriority }, - }, - ]) - const priorityValue = priorityResults[0]?.attributeValue ?? '' + const priorityValue = + OCPP20ServiceUtils.readVariableValue( + chargingStation, + OCPP20ComponentName.OCPPCommCtrlr, + OCPP20RequiredVariableName.NetworkConfigurationPriority + ) ?? '' if (priorityValue.length > 0) { const priorities = priorityValue.split(',').map(Number) if (!priorities.includes(commandPayload.configurationSlot)) { @@ -2366,29 +2355,21 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } } - const variableManager = OCPP20VariableManager.getInstance() - const authorizeRemoteStartResults = variableManager.getVariables(chargingStation, [ - { - attributeType: AttributeEnumType.Actual, - component: { name: OCPP20ComponentName.AuthCtrlr }, - variable: { name: OCPP20RequiredVariableName.AuthorizeRemoteStart }, - }, - ]) - const shouldAuthorizeRemoteStart = - authorizeRemoteStartResults[0]?.attributeValue == null || - convertToBoolean(authorizeRemoteStartResults[0].attributeValue) + const shouldAuthorizeRemoteStart = OCPP20ServiceUtils.readVariableAsBoolean( + chargingStation, + OCPP20ComponentName.AuthCtrlr, + OCPP20RequiredVariableName.AuthorizeRemoteStart, + true + ) let isAuthorized = true if (shouldAuthorizeRemoteStart) { // C12.FR.09: Check MasterPassGroupId before authorization - const masterPassGroupIdResults = variableManager.getVariables(chargingStation, [ - { - attributeType: AttributeEnumType.Actual, - component: { name: OCPP20ComponentName.AuthCtrlr }, - variable: { name: 'MasterPassGroupId' }, - }, - ]) - const masterPassGroupId = masterPassGroupIdResults[0]?.attributeValue + const masterPassGroupId = OCPP20ServiceUtils.readVariableValue( + chargingStation, + OCPP20ComponentName.AuthCtrlr, + 'MasterPassGroupId' + ) if ( masterPassGroupId != null && masterPassGroupId.length > 0 && @@ -3537,17 +3518,12 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { if (checkAborted()) return // L01.FR.04: Simulate signature verification - const variableManager = OCPP20VariableManager.getInstance() - const verificationResults = variableManager.getVariables(chargingStation, [ - { - attributeType: AttributeEnumType.Actual, - component: { name: OCPP20ComponentName.FirmwareCtrlr as string }, - variable: { - name: OCPP20VendorVariableName.SimulateSignatureVerificationFailure as string, - }, - }, - ]) - const simulateFailure = convertToBoolean(verificationResults[0]?.attributeValue) + const simulateFailure = OCPP20ServiceUtils.readVariableAsBoolean( + chargingStation, + OCPP20ComponentName.FirmwareCtrlr as string, + OCPP20VendorVariableName.SimulateSignatureVerificationFailure as string, + false + ) if (simulateFailure) { // L01.FR.03: InvalidSignature + SecurityEventNotification @@ -3596,15 +3572,12 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { .iterateEvses(true) .some(({ evseStatus }) => this.hasEvseActiveTransactions(evseStatus)) if (hasActiveTransactionsBeforeInstall) { - const variableManager = OCPP20VariableManager.getInstance() - const allowNewSessionsResults = variableManager.getVariables(chargingStation, [ - { - attributeType: AttributeEnumType.Actual, - component: { name: OCPP20ComponentName.ChargingStation }, - variable: { name: 'AllowNewSessionsPendingFirmwareUpdate' }, - }, - ]) - const allowNewSessions = convertToBoolean(allowNewSessionsResults[0]?.attributeValue) + const allowNewSessions = OCPP20ServiceUtils.readVariableAsBoolean( + chargingStation, + OCPP20ComponentName.ChargingStation, + 'AllowNewSessionsPendingFirmwareUpdate', + false + ) while ( !checkAborted() && chargingStation diff --git a/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts b/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts index f8245d0a..9ce5bcbb 100644 --- a/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts +++ b/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts @@ -206,24 +206,24 @@ export class OCPP20ServiceUtils { chargingStation: ChargingStation, retryCount: number ): number { - const variableManager = OCPP20VariableManager.getInstance() - const results = variableManager.getVariables(chargingStation, [ - { - component: { name: OCPP20ComponentName.OCPPCommCtrlr }, - variable: { name: OCPP20OptionalVariableName.RetryBackOffWaitMinimum }, - }, - { - component: { name: OCPP20ComponentName.OCPPCommCtrlr }, - variable: { name: OCPP20OptionalVariableName.RetryBackOffRandomRange }, - }, - { - component: { name: OCPP20ComponentName.OCPPCommCtrlr }, - variable: { name: OCPP20OptionalVariableName.RetryBackOffRepeatTimes }, - }, - ]) - const waitMinimum = convertToInt(results[0]?.attributeValue) || 30 - const randomRange = convertToInt(results[1]?.attributeValue) || 10 - const repeatTimes = convertToInt(results[2]?.attributeValue) || 5 + const waitMinimum = OCPP20ServiceUtils.readVariableAsInteger( + chargingStation, + OCPP20ComponentName.OCPPCommCtrlr, + OCPP20OptionalVariableName.RetryBackOffWaitMinimum, + 30 + ) + const randomRange = OCPP20ServiceUtils.readVariableAsInteger( + chargingStation, + OCPP20ComponentName.OCPPCommCtrlr, + OCPP20OptionalVariableName.RetryBackOffRandomRange, + 10 + ) + const repeatTimes = OCPP20ServiceUtils.readVariableAsInteger( + chargingStation, + OCPP20ComponentName.OCPPCommCtrlr, + OCPP20OptionalVariableName.RetryBackOffRepeatTimes, + 5 + ) return computeExponentialBackOffDelay({ baseDelayMs: secondsToMilliseconds(waitMinimum), jitterMs: secondsToMilliseconds(randomRange), @@ -476,6 +476,71 @@ export class OCPP20ServiceUtils { return { bytesLimit, itemsLimit } } + public static readVariableAsBoolean ( + chargingStation: ChargingStation, + componentName: string, + variableName: string, + defaultValue: boolean + ): boolean { + const value = OCPP20ServiceUtils.readVariableValue(chargingStation, componentName, variableName) + return value != null ? convertToBoolean(value) : defaultValue + } + + public static readVariableAsInteger ( + chargingStation: ChargingStation, + componentName: string, + variableName: string, + defaultValue: number + ): number { + const value = OCPP20ServiceUtils.readVariableValue(chargingStation, componentName, variableName) + if (value != null) { + try { + return convertToInt(value) + } catch { + logger.warn( + `${moduleName}.readVariableAsInteger: Cannot convert '${value}' to integer for ${componentName}.${variableName}, using default ${defaultValue.toString()}` + ) + return defaultValue + } + } + return defaultValue + } + + public static readVariableAsIntervalMs ( + chargingStation: ChargingStation, + componentName: string, + variableName: string, + defaultSeconds: number + ): number { + const intervalSeconds = OCPP20ServiceUtils.readVariableAsInteger( + chargingStation, + componentName, + variableName, + defaultSeconds + ) + return intervalSeconds > 0 + ? secondsToMilliseconds(intervalSeconds) + : secondsToMilliseconds(defaultSeconds) + } + + public static readVariableValue ( + chargingStation: ChargingStation, + componentName: string, + variableName: string + ): string | undefined { + const variableManager = OCPP20VariableManager.getInstance() + const results = variableManager.getVariables(chargingStation, [ + { + component: { name: componentName }, + variable: { name: variableName }, + }, + ]) + if (results.length > 0 && results[0].attributeValue != null) { + return results[0].attributeValue + } + return undefined + } + /** * Deauthorize an active transaction per OCPP 2.0.1 E05 requirements. * @param chargingStation - Target charging station @@ -1094,71 +1159,6 @@ export class OCPP20ServiceUtils { return endedMeterValues.length > 0 ? endedMeterValues : [] } - private static readVariableAsBoolean ( - chargingStation: ChargingStation, - componentName: string, - variableName: string, - defaultValue: boolean - ): boolean { - const value = OCPP20ServiceUtils.readVariableValue(chargingStation, componentName, variableName) - return value != null ? convertToBoolean(value) : defaultValue - } - - private static readVariableAsInteger ( - chargingStation: ChargingStation, - componentName: string, - variableName: string, - defaultValue: number - ): number { - const value = OCPP20ServiceUtils.readVariableValue(chargingStation, componentName, variableName) - if (value != null) { - try { - return convertToInt(value) - } catch { - logger.warn( - `${moduleName}.readVariableAsInteger: Cannot convert '${value}' to integer for ${componentName}.${variableName}, using default ${defaultValue.toString()}` - ) - return defaultValue - } - } - return defaultValue - } - - private static readVariableAsIntervalMs ( - chargingStation: ChargingStation, - componentName: string, - variableName: string, - defaultSeconds: number - ): number { - const intervalSeconds = OCPP20ServiceUtils.readVariableAsInteger( - chargingStation, - componentName, - variableName, - defaultSeconds - ) - return intervalSeconds > 0 - ? secondsToMilliseconds(intervalSeconds) - : secondsToMilliseconds(defaultSeconds) - } - - private static readVariableValue ( - chargingStation: ChargingStation, - componentName: string, - variableName: string - ): string | undefined { - const variableManager = OCPP20VariableManager.getInstance() - const results = variableManager.getVariables(chargingStation, [ - { - component: { name: componentName }, - variable: { name: variableName }, - }, - ]) - if (results.length > 0 && results[0].attributeValue != null) { - return results[0].attributeValue - } - return undefined - } - private static resolveActiveTransaction ( chargingStation: ChargingStation, connectorId: number diff --git a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts index f4b46c22..0938fcb9 100644 --- a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts +++ b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts @@ -14,9 +14,8 @@ import type { Identifier, } from '../types/AuthTypes.js' -import { OCPP20VariableManager } from '../../2.0/OCPP20VariableManager.js' +import { OCPP20ServiceUtils } from '../../2.0/OCPP20ServiceUtils.js' import { - GetVariableStatusEnumType, OCPP20ComponentName, OCPP20IdTokenEnumType, type OCPP20IdTokenType, @@ -24,7 +23,7 @@ import { OCPP20RequiredVariableName, OCPPVersion, } from '../../../../types/index.js' -import { logger, truncateId } from '../../../../utils/index.js' +import { convertToBoolean, logger, truncateId } from '../../../../utils/index.js' import { AuthContext, AuthenticationMethod, @@ -373,12 +372,13 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { // Check if station is online and can communicate const isOnline = this.chargingStation.inAcceptedState() - // Check AuthorizeRemoteStart variable (with type validation) + // Check AuthorizeRemoteStart variable const remoteStartValue = this.getVariableValue( OCPP20ComponentName.AuthCtrlr, OCPP20RequiredVariableName.AuthorizeRemoteStart ) - const remoteStartEnabled = this.parseBooleanVariable(remoteStartValue, true) + const remoteStartEnabled = + remoteStartValue != null ? convertToBoolean(remoteStartValue) : true return isOnline && remoteStartEnabled } catch (error) { @@ -513,7 +513,7 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { OCPP20ComponentName.AuthCtrlr, OCPP20RequiredVariableName.LocalAuthorizationOffline ) - return this.parseBooleanVariable(value, true) + return value != null ? convertToBoolean(value) : true } catch (error) { logger.warn( `${this.chargingStation.logPrefix()} ${moduleName}.getOfflineAuthorizationConfig: Error getting offline authorization config`, @@ -536,37 +536,15 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { useDefaultFallback = true ): string | undefined { try { - const variableManager = OCPP20VariableManager.getInstance() - - const results = variableManager.getVariables(this.chargingStation, [ - { - component: { name: component }, - variable: { name: variable }, - }, - ]) - - // Check if variable was successfully retrieved - if (results.length === 0) { - logger.debug( - `${this.chargingStation.logPrefix()} ${moduleName}.getVariableValue: Variable ${component}.${variable} not found in registry` - ) - return this.getDefaultVariableValue(component, variable, useDefaultFallback) - } - - const result = results[0] - - // Check for errors or rejection - if ( - result.attributeStatus !== GetVariableStatusEnumType.Accepted || - result.attributeValue == null - ) { - logger.debug( - `${this.chargingStation.logPrefix()} ${moduleName}.getVariableValue: Variable ${component}.${variable} not available: ${result.attributeStatus}` - ) - return this.getDefaultVariableValue(component, variable, useDefaultFallback) + const value = OCPP20ServiceUtils.readVariableValue(this.chargingStation, component, variable) + if (value != null) { + return value } - return result.attributeValue + logger.debug( + `${this.chargingStation.logPrefix()} ${moduleName}.getVariableValue: Variable ${component}.${variable} not available` + ) + return this.getDefaultVariableValue(component, variable, useDefaultFallback) } catch (error) { logger.warn( `${this.chargingStation.logPrefix()} ${moduleName}.getVariableValue: Error getting variable ${component}.${variable}`, @@ -575,31 +553,4 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { return this.getDefaultVariableValue(component, variable, useDefaultFallback) } } - - /** - * Parse and validate a boolean variable value - * @param value - String value to parse ('true', 'false', '1', '0') - * @param defaultValue - Fallback value when parsing fails or value is undefined - * @returns Parsed boolean value, or defaultValue if parsing fails - */ - private parseBooleanVariable (value: string | undefined, defaultValue: boolean): boolean { - if (value == null) { - return defaultValue - } - - const normalized = value.toLowerCase().trim() - - if (normalized === 'true' || normalized === '1') { - return true - } - - if (normalized === 'false' || normalized === '0') { - return false - } - - logger.warn( - `${this.chargingStation.logPrefix()} ${moduleName}.parseBooleanVariable: Invalid boolean value '${value}', using default: ${defaultValue.toString()}` - ) - return defaultValue - } } -- 2.53.0