OCPP stack bug fixes:

+ Readd OCPP message send timeout;
+ Fix memory leak in the requests cache;
+ Fix buffered message handling.

Signed-off-by: Jérôme Benoit <jerome.benoit@piment-noir.org>

diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts
index 5b5ac56e4d79e8ced4d3e15758f7b554f331550e..b035e5b3dc297ca239e0df7fc80bd87848962a5e 100644 (file)
--- 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<string, Request>;
+  public requests: Map<string, CachedRequest>;
   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<string, Request>();
+    this.requests = new Map<string, CachedRequest>();
     this.messageQueue = new Array<string>();
 
     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, unknown> | string, requestPayload: Record<string, unknown>) => void;
     let rejectCallback: (error: OCPPError) => void;
+    let requestCommandName: RequestCommand | IncomingRequestCommand;
     let requestPayload: Record<string, unknown>;
-    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 2301a0fec0d54a744db8f8a8adfc7a993ded3c7d..18c73918fb9111851dd1fa43174750fe5265a475 100644 (file)
--- 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 53eec6ec659c46d329fb3c8c750d1bb46690460c..fc5e60ad855bc5f7ff21efaaebf3cc8aa8681b52 100644 (file)
--- 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<string, unknown>]);
+        this.chargingStation.requests.set(messageId, [responseCallback, rejectCallback, commandName, commandParams as Record<string, unknown>]);
         messageToSend = JSON.stringify([messageType, messageId, commandName, commandParams]);
         break;
       // Response

diff --git a/src/types/ocpp/Requests.ts b/src/types/ocpp/Requests.ts
index 713fa0e6da50d64f42f89d4eb3f4a57f64783d20..b2adad378ab71b8fe4105790cf253b54486ecec9 100644 (file)
--- 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, unknown> | string, requestPayload: Record<string, unknown>) => void, (error: OCPPError) => void, Record<string, unknown>];
+export type Request = [MessageType, string, RequestCommand, Record<string, unknown>, Record<string, unknown>];
 
 export type IncomingRequest = [MessageType, string, IncomingRequestCommand, Record<string, unknown>, Record<string, unknown>];
+
+export type CachedRequest = [(payload: Record<string, unknown> | string, requestPayload: Record<string, unknown>) => void, (error: OCPPError) => void, RequestCommand | IncomingRequestCommand, Record<string, unknown>];

diff --git a/src/utils/Constants.ts b/src/utils/Constants.ts
index 2852d8f634a481198e4340c96f3dd08e859445cc..f77059632d72f6baba33d791e7b9abc1498e61e4 100644 (file)
--- 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