From 32b0224999178acf1101ff9075c95072d207206e Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 28 Sep 2021 00:43:14 +0200 Subject: [PATCH] OCPP stack bug fixes: MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit + Readd OCPP message send timeout; + Fix memory leak in the requests cache; + Fix buffered message handling. Signed-off-by: Jérôme Benoit --- src/charging-station/ChargingStation.ts | 41 +++++++++---------- .../ocpp/1.6/OCPP16IncomingRequestService.ts | 6 +-- .../ocpp/OCPPRequestService.ts | 19 ++++----- src/types/ocpp/Requests.ts | 4 +- src/utils/Constants.ts | 2 +- 5 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts index 5b5ac56e..b035e5b3 100644 --- a/src/charging-station/ChargingStation.ts +++ b/src/charging-station/ChargingStation.ts @@ -1,6 +1,6 @@ // Partial Copyright Jerome Benoit. 2021. All Rights Reserved. -import { AvailabilityType, BootNotificationRequest, IncomingRequest, IncomingRequestCommand, Request } from '../types/ocpp/Requests'; +import { AvailabilityType, BootNotificationRequest, CachedRequest, IncomingRequest, IncomingRequestCommand, RequestCommand } from '../types/ocpp/Requests'; import { BootNotificationResponse, RegistrationStatus } from '../types/ocpp/Responses'; import ChargingStationConfiguration, { ConfigurationKey } from '../types/ChargingStationConfiguration'; import ChargingStationTemplate, { CurrentType, PowerUnits, Voltage } from '../types/ChargingStationTemplate'; @@ -43,7 +43,7 @@ export default class ChargingStation { public connectors: Connectors; public configuration!: ChargingStationConfiguration; public wsConnection!: WebSocket; - public requests: Map; + public requests: Map; public performanceStatistics!: PerformanceStatistics; public heartbeatSetInterval!: NodeJS.Timeout; public ocppRequestService!: OCPPRequestService; @@ -70,7 +70,7 @@ export default class ChargingStation { this.wsConnectionRestarted = false; this.autoReconnectRetryCount = 0; - this.requests = new Map(); + this.requests = new Map(); this.messageQueue = new Array(); this.authorizedTags = this.getAuthorizedTags(); @@ -237,13 +237,13 @@ export default class ChargingStation { if (!Constants.SUPPORTED_MEASURANDS.includes(sampledValueTemplates[index]?.measurand ?? MeterValueMeasurand.ENERGY_ACTIVE_IMPORT_REGISTER)) { logger.warn(`${this.logPrefix()} Unsupported MeterValues measurand '${measurand}' ${phase ? `on phase ${phase} ` : ''}in template on connectorId ${connectorId}`); } else if (phase && sampledValueTemplates[index]?.phase === phase && sampledValueTemplates[index]?.measurand === measurand - && this.getConfigurationKey(StandardParametersKey.MeterValuesSampledData).value.includes(measurand)) { + && this.getConfigurationKey(StandardParametersKey.MeterValuesSampledData).value.includes(measurand)) { return sampledValueTemplates[index]; } else if (!phase && !sampledValueTemplates[index].phase && sampledValueTemplates[index]?.measurand === measurand - && this.getConfigurationKey(StandardParametersKey.MeterValuesSampledData).value.includes(measurand)) { + && this.getConfigurationKey(StandardParametersKey.MeterValuesSampledData).value.includes(measurand)) { return sampledValueTemplates[index]; } else if (measurand === MeterValueMeasurand.ENERGY_ACTIVE_IMPORT_REGISTER - && (!sampledValueTemplates[index].measurand || sampledValueTemplates[index].measurand === measurand)) { + && (!sampledValueTemplates[index].measurand || sampledValueTemplates[index].measurand === measurand)) { return sampledValueTemplates[index]; } } @@ -387,7 +387,7 @@ export default class ChargingStation { if (!Utils.isEmptyArray(this.getConnector(connectorId).chargingProfiles)) { this.getConnector(connectorId).chargingProfiles?.forEach((chargingProfile: ChargingProfile, index: number) => { if (chargingProfile.chargingProfileId === cp.chargingProfileId - || (chargingProfile.stackLevel === cp.stackLevel && chargingProfile.chargingProfilePurpose === cp.chargingProfilePurpose)) { + || (chargingProfile.stackLevel === cp.stackLevel && chargingProfile.chargingProfilePurpose === cp.chargingProfilePurpose)) { this.getConnector(connectorId).chargingProfiles[index] = cp; cpReplaced = true; } @@ -595,7 +595,7 @@ export default class ChargingStation { this.addConfigurationKey(StandardParametersKey.AuthorizeRemoteTxRequests, 'true'); } if (!this.getConfigurationKey(StandardParametersKey.LocalAuthListEnabled) - && this.getConfigurationKey(StandardParametersKey.SupportedFeatureProfiles).value.includes(SupportedFeatureProfiles.Local_Auth_List_Management)) { + && this.getConfigurationKey(StandardParametersKey.SupportedFeatureProfiles).value.includes(SupportedFeatureProfiles.Local_Auth_List_Management)) { this.addConfigurationKey(StandardParametersKey.LocalAuthListEnabled, 'false'); } if (!this.getConfigurationKey(StandardParametersKey.ConnectionTimeOut)) { @@ -650,8 +650,9 @@ export default class ChargingStation { let [messageType, messageId, commandName, commandPayload, errorDetails]: IncomingRequest = [0, '', '' as IncomingRequestCommand, {}, {}]; let responseCallback: (payload: Record | string, requestPayload: Record) => void; let rejectCallback: (error: OCPPError) => void; + let requestCommandName: RequestCommand | IncomingRequestCommand; let requestPayload: Record; - let cachedRequest: Request; + let cachedRequest: CachedRequest; let errMsg: string; try { const request = JSON.parse(data.toString()) as IncomingRequest; @@ -676,7 +677,7 @@ export default class ChargingStation { // Respond cachedRequest = this.requests.get(messageId); if (Utils.isIterable(cachedRequest)) { - [responseCallback, , requestPayload] = cachedRequest; + [responseCallback, , , requestPayload] = cachedRequest; } else { throw new OCPPError(ErrorType.PROTOCOL_ERROR, `Response request for message id ${messageId} is not iterable`, commandName); } @@ -684,23 +685,21 @@ export default class ChargingStation { // Error throw new OCPPError(ErrorType.INTERNAL_ERROR, `Response request for unknown message id ${messageId}`, commandName); } - this.requests.delete(messageId); responseCallback(commandName, requestPayload); break; // Error Message case MessageType.CALL_ERROR_MESSAGE: cachedRequest = this.requests.get(messageId); - if (!cachedRequest) { - // Error - throw new OCPPError(ErrorType.INTERNAL_ERROR, `Error request for unknown message id ${messageId}`); - } if (Utils.isIterable(cachedRequest)) { - [, rejectCallback] = cachedRequest; + [, rejectCallback, requestCommandName] = cachedRequest; } else { throw new OCPPError(ErrorType.PROTOCOL_ERROR, `Error request for message id ${messageId} is not iterable`); } - this.requests.delete(messageId); - rejectCallback(new OCPPError(commandName, commandPayload.toString(), null, errorDetails)); + if (!rejectCallback) { + // Error + throw new OCPPError(ErrorType.INTERNAL_ERROR, `Error request for unknown message id ${messageId}`, requestCommandName); + } + rejectCallback(new OCPPError(commandName, commandPayload.toString(), requestCommandName, errorDetails)); break; // Error default: @@ -710,9 +709,9 @@ export default class ChargingStation { } } catch (error) { // Log - logger.error('%s Incoming request message %j processing error %j on content type %j', this.logPrefix(), data, error, this.requests.get(messageId)); + logger.error('%s Incoming request message %j matching cached request %j processing error %j ', this.logPrefix(), data, this.requests.get(messageId), error); // Send error - messageType !== MessageType.CALL_ERROR_MESSAGE && await this.ocppRequestService.sendError(messageId, error, commandName); + messageType === MessageType.CALL_MESSAGE && await this.ocppRequestService.sendError(messageId, error, commandName); } } @@ -1011,7 +1010,7 @@ export default class ChargingStation { this.initialize(); // Restart the ATG if (!this.stationInfo.AutomaticTransactionGenerator.enable && - this.automaticTransactionGenerator) { + this.automaticTransactionGenerator) { this.automaticTransactionGenerator.stop(); } this.startAutomaticTransactionGenerator(); diff --git a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts index 2301a0fe..18c73918 100644 --- a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts +++ b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts @@ -36,15 +36,11 @@ export default class OCPP16IncomingRequestService extends OCPPIncomingRequestSer } catch (error) { // Log logger.error(this.chargingStation.logPrefix() + ' Handle request error: %j', error); - // Send back an error response to inform backend - await this.chargingStation.ocppRequestService.sendError(messageId, error, commandName); throw error; } } else { // Throw exception - const error = new OCPPError(ErrorType.NOT_IMPLEMENTED, `${commandName} is not implemented to handle payload ${JSON.stringify(commandPayload, null, 2)}`, commandName); - await this.chargingStation.ocppRequestService.sendError(messageId, error, commandName); - throw error; + throw new OCPPError(ErrorType.NOT_IMPLEMENTED, `${commandName} is not implemented to handle payload ${JSON.stringify(commandPayload, null, 2)}`, commandName); } // Send the built response await this.chargingStation.ocppRequestService.sendMessage(messageId, response, MessageType.CALL_RESULT_MESSAGE, commandName); diff --git a/src/charging-station/ocpp/OCPPRequestService.ts b/src/charging-station/ocpp/OCPPRequestService.ts index 53eec6ec..fc5e60ad 100644 --- a/src/charging-station/ocpp/OCPPRequestService.ts +++ b/src/charging-station/ocpp/OCPPRequestService.ts @@ -42,16 +42,16 @@ export default abstract class OCPPRequestService { } else if (!skipBufferingOnError) { // Buffer it this.chargingStation.addToMessageQueue(messageToSend); - // Reject it - return rejectCallback(new OCPPError(commandParams?.code ?? ErrorType.GENERIC_ERROR, commandParams?.message ?? `WebSocket closed for message id '${messageId}' with content '${messageToSend}', message buffered`, commandParams?.details ?? {})); + // Reject it but keep the request in the cache + reject(new OCPPError(commandParams?.code ?? ErrorType.GENERIC_ERROR, commandParams?.message ?? `WebSocket closed for message id '${messageId}' with content '${messageToSend}', message buffered`, commandParams?.details ?? {})); } // Response? if (messageType === MessageType.CALL_RESULT_MESSAGE) { // Yes: send Ok resolve(commandName); - } else if (messageType === MessageType.CALL_ERROR_MESSAGE) { + } else { // Send timeout - setTimeout(() => rejectCallback(new OCPPError(commandParams?.code ?? ErrorType.GENERIC_ERROR, commandParams?.message ?? `Timeout for message id '${messageId}' with content '${messageToSend}'`, commandParams?.details ?? {})), Constants.OCPP_ERROR_TIMEOUT); + setTimeout(() => rejectCallback(new OCPPError(commandParams?.code ?? ErrorType.GENERIC_ERROR, commandParams?.message ?? `Timeout for message id '${messageId}' with content '${messageToSend}'`, commandParams?.details ?? {})), Constants.OCPP_SOCKET_TIMEOUT); } /** @@ -66,11 +66,12 @@ export default abstract class OCPPRequestService { } // Send the response await self.ocppResponseService.handleResponse(commandName as RequestCommand, payload, requestPayload); + self.chargingStation.requests.delete(messageId); resolve(payload); } /** - * Function that will receive the request's rejection + * Function that will receive the request's error response * * @param error */ @@ -78,10 +79,8 @@ export default abstract class OCPPRequestService { if (self.chargingStation.getEnableStatistics()) { self.chargingStation.performanceStatistics.addRequestStatistic(commandName, MessageType.CALL_ERROR_MESSAGE); } - logger.debug(`${self.chargingStation.logPrefix()} Error: %j occurred when calling command %s with parameters: %j`, error, commandName, commandParams); - // Build Exception - self.chargingStation.requests.set(messageId, [() => { /* This is intentional */ }, () => { /* This is intentional */ }, {}]); - // Send error + logger.debug(`${self.chargingStation.logPrefix()} Error %j occurred when calling command %s with parameters %j`, error, commandName, commandParams); + self.chargingStation.requests.delete(messageId); reject(error); } }); @@ -100,7 +99,7 @@ export default abstract class OCPPRequestService { // Request case MessageType.CALL_MESSAGE: // Build request - this.chargingStation.requests.set(messageId, [responseCallback, rejectCallback, commandParams as Record]); + this.chargingStation.requests.set(messageId, [responseCallback, rejectCallback, commandName, commandParams as Record]); messageToSend = JSON.stringify([messageType, messageId, commandName, commandParams]); break; // Response diff --git a/src/types/ocpp/Requests.ts b/src/types/ocpp/Requests.ts index 713fa0e6..b2adad37 100644 --- a/src/types/ocpp/Requests.ts +++ b/src/types/ocpp/Requests.ts @@ -30,6 +30,8 @@ export const DiagnosticsStatus = { ...OCPP16DiagnosticsStatus }; -export type Request = [(payload: Record | string, requestPayload: Record) => void, (error: OCPPError) => void, Record]; +export type Request = [MessageType, string, RequestCommand, Record, Record]; export type IncomingRequest = [MessageType, string, IncomingRequestCommand, Record, Record]; + +export type CachedRequest = [(payload: Record | string, requestPayload: Record) => void, (error: OCPPError) => void, RequestCommand | IncomingRequestCommand, Record]; diff --git a/src/utils/Constants.ts b/src/utils/Constants.ts index 2852d8f6..f7705963 100644 --- a/src/utils/Constants.ts +++ b/src/utils/Constants.ts @@ -26,7 +26,7 @@ export default class Constants { static readonly OCPP_TRIGGER_MESSAGE_RESPONSE_NOT_IMPLEMENTED = Object.freeze({ status: TriggerMessageStatus.NOT_IMPLEMENTED }); static readonly OCPP_DEFAULT_BOOT_NOTIFICATION_INTERVAL = 60000; // Ms - static readonly OCPP_ERROR_TIMEOUT = 60000; // Ms + static readonly OCPP_SOCKET_TIMEOUT = 60000; // Ms static readonly OCPP_TRIGGER_MESSAGE_DELAY = 2000; // Ms static readonly CHARGING_STATION_DEFAULT_RESET_TIME = 60000; // Ms -- 2.34.1