Fix multiple operation executions and update FW-/DiagService API

src/MicroOcpp/Model/Diagnostics/DiagnosticsService.cpp

@@ -16,8 +16,8 @@ using Ocpp16::DiagnosticsStatus;
 
 DiagnosticsService::DiagnosticsService(Context& context) : context(context) {
 
-    context.getOperationRegistry().registerOperation("GetDiagnostics", [&context] () {
-        return new Ocpp16::GetDiagnostics(context.getModel());});
+    context.getOperationRegistry().registerOperation("GetDiagnostics", [this] () {
+        return new Ocpp16::GetDiagnostics(*this);});
 
     //Register message handler for TriggerMessage operation
     context.getOperationRegistry().registerOperation("DiagnosticsStatusNotification", [this] () {
@@ -36,7 +36,7 @@ void DiagnosticsService::loop() {
         if (!uploadIssued) {
             if (onUpload != nullptr) {
                 MOCPP_DBG_DEBUG("Call onUpload");
-                onUpload(location, startTime, stopTime);
+                onUpload(location.c_str(), startTime, stopTime);
                 uploadIssued = true;
             } else {
                 MOCPP_DBG_ERR("onUpload must be set! (see setOnUpload). Will abort");
@@ -82,7 +82,7 @@ void DiagnosticsService::loop() {
 }
 
 //timestamps before year 2021 will be treated as "undefined"
-std::string DiagnosticsService::requestDiagnosticsUpload(const std::string &location, int retries, unsigned int retryInterval, Timestamp startTime, Timestamp stopTime) {
+std::string DiagnosticsService::requestDiagnosticsUpload(const char *location, unsigned int retries, unsigned int retryInterval, Timestamp startTime, Timestamp stopTime) {
     if (onUpload == nullptr) //maybe add further plausibility checks
         return std::string{};
 
@@ -100,6 +100,7 @@ std::string DiagnosticsService::requestDiagnosticsUpload(const std::string &loca
         this->stopTime = newStop;
     }
 
+#if MOCPP_DBG_LEVEL >= MOCPP_DL_INFO
     char dbuf [JSONDATE_LENGTH + 1] = {'\0'};
     char dbuf2 [JSONDATE_LENGTH + 1] = {'\0'};
     this->startTime.toJsonString(dbuf, JSONDATE_LENGTH + 1);
@@ -116,6 +117,7 @@ std::string DiagnosticsService::requestDiagnosticsUpload(const std::string &loca
             this->retryInterval,
             dbuf,
             dbuf2);
+#endif
 
     nextTry = context.getModel().getClock().now();
     nextTry += 5; //wait for 5s before upload
@@ -168,7 +170,7 @@ void DiagnosticsService::setRefreshFilename(std::function<std::string()> refresh
     this->refreshFilename = refreshFn;
 }
 
-void DiagnosticsService::setOnUpload(std::function<bool(const std::string &location, Timestamp &startTime, Timestamp &stopTime)> onUpload) {
+void DiagnosticsService::setOnUpload(std::function<bool(const char *location, Timestamp &startTime, Timestamp &stopTime)> onUpload) {
     this->onUpload = onUpload;
 }
 

src/MicroOcpp/Model/Diagnostics/DiagnosticsService.h

@@ -27,15 +27,15 @@ class DiagnosticsService {
     Context& context;
 
     std::string location;
-    int retries = 0;
+    unsigned int retries = 0;
     unsigned int retryInterval = 0;
     Timestamp startTime;
     Timestamp stopTime;
 
     Timestamp nextTry;
 
     std::function<std::string()> refreshFilename;
-    std::function<bool(const std::string &location, Timestamp &startTime, Timestamp &stopTime)> onUpload;
+    std::function<bool(const char *location, Timestamp &startTime, Timestamp &stopTime)> onUpload;
     std::function<UploadStatus()> uploadStatusInput;
     bool uploadIssued = false;
 
@@ -51,13 +51,13 @@ class DiagnosticsService {
     //timestamps before year 2021 will be treated as "undefined"
     //returns empty std::string if onUpload is missing or upload cannot be scheduled for another reason
     //returns fileName of diagnostics file to be uploaded if upload has been scheduled
-    std::string requestDiagnosticsUpload(const std::string &location, int retries = 1, unsigned int retryInterval = 0, Timestamp startTime = Timestamp(), Timestamp stopTime = Timestamp());
+    std::string requestDiagnosticsUpload(const char *location, unsigned int retries = 1, unsigned int retryInterval = 0, Timestamp startTime = Timestamp(), Timestamp stopTime = Timestamp());
 
     Ocpp16::DiagnosticsStatus getDiagnosticsStatus();
 
     void setRefreshFilename(std::function<std::string()> refreshFn); //refresh a new filename which will be used for the subsequent upload tries
 
-    void setOnUpload(std::function<bool(const std::string &location, Timestamp &startTime, Timestamp &stopTime)> onUpload);
+    void setOnUpload(std::function<bool(const char *location, Timestamp &startTime, Timestamp &stopTime)> onUpload);
 
     void setOnUploadStatusInput(std::function<UploadStatus()> uploadStatusInput);
 };

src/MicroOcpp/Model/FirmwareManagement/FirmwareService.cpp

@@ -27,8 +27,8 @@ using MicroOcpp::Ocpp16::FirmwareStatus;
 
 FirmwareService::FirmwareService(Context& context) : context(context) {
 
-    context.getOperationRegistry().registerOperation("UpdateFirmware", [&context] () {
-        return new Ocpp16::UpdateFirmware(context.getModel());});
+    context.getOperationRegistry().registerOperation("UpdateFirmware", [this] () {
+        return new Ocpp16::UpdateFirmware(*this);});
 
     //Register message handler for TriggerMessage operation
     context.getOperationRegistry().registerOperation("FirmwareStatusNotification", [this] () {
@@ -78,7 +78,7 @@ void FirmwareService::loop() {
             MOCPP_DBG_INFO("Start download");
             stage = UpdateStage::Downloading;
             if (onDownload != nullptr) {
-                onDownload(location);
+                onDownload(location.c_str());
                 timestampTransition = mocpp_tick_ms();
                 delayTransition = downloadStatusInput ? 1000 : 30000; //give the download at least 30s
                 return;
@@ -138,7 +138,7 @@ void FirmwareService::loop() {
             stage = UpdateStage::Installing;
 
             if (onInstall) {
-                onInstall(location); //should restart the device on success
+                onInstall(location.c_str()); //should restart the device on success
             } else {
                 MOCPP_DBG_WARN("onInstall must be set! (see setOnInstall). Will abort");
             }
@@ -183,7 +183,7 @@ void FirmwareService::loop() {
     }
 }
 
-void FirmwareService::scheduleFirmwareUpdate(const std::string &location, Timestamp retreiveDate, int retries, unsigned int retryInterval) {
+void FirmwareService::scheduleFirmwareUpdate(const char *location, Timestamp retreiveDate, unsigned int retries, unsigned int retryInterval) {
     this->location = location;
     this->retreiveDate = retreiveDate;
     this->retries = retries;
@@ -197,15 +197,17 @@ void FirmwareService::scheduleFirmwareUpdate(const std::string &location, Timest
     char dbuf [JSONDATE_LENGTH + 1] = {'\0'};
     this->retreiveDate.toJsonString(dbuf, JSONDATE_LENGTH + 1);
 
+#if MOCPP_DBG_LEVEL >= MOCPP_DL_INFO
     MOCPP_DBG_INFO("Scheduled FW update!\n" \
                     "                  location = %s\n" \
                     "                  retrieveDate = %s\n" \
-                    "                  retries = %i" \
+                    "                  retries = %u" \
                     ", retryInterval = %u",
             this->location.c_str(),
             dbuf,
             this->retries,
             this->retryInterval);
+#endif
 
     timestampTransition = mocpp_tick_ms();
     delayTransition = 1000;
@@ -276,15 +278,15 @@ std::unique_ptr<Request> FirmwareService::getFirmwareStatusNotification() {
     return nullptr;
 }
 
-void FirmwareService::setOnDownload(std::function<bool(const std::string &location)> onDownload) {
+void FirmwareService::setOnDownload(std::function<bool(const char *location)> onDownload) {
     this->onDownload = onDownload;
 }
 
 void FirmwareService::setDownloadStatusInput(std::function<DownloadStatus()> downloadStatusInput) {
     this->downloadStatusInput = downloadStatusInput;
 }
 
-void FirmwareService::setOnInstall(std::function<bool(const std::string &location)> onInstall) {
+void FirmwareService::setOnInstall(std::function<bool(const char *location)> onInstall) {
     this->onInstall = onInstall;
 }
 
@@ -310,7 +312,7 @@ FirmwareService *EspWiFi::makeFirmwareService(Context& context) {
      * example of how to integrate a separate download phase (optional)
      */
 #if 0 //separate download phase
-    fwService->setOnDownload([] (const std::string &location) {
+    fwService->setOnDownload([] (const char *location) {
         //download the new binary
         //...
         return true;
@@ -323,7 +325,7 @@ FirmwareService *EspWiFi::makeFirmwareService(Context& context) {
     });
 #endif //separate download phase
 
-    fwService->setOnInstall([fwService] (const std::string &location) {
+    fwService->setOnInstall([fwService] (const char *location) {
 
         fwService->setInstallationStatusInput([](){return InstallationStatus::NotInstalled;});
 
@@ -333,7 +335,7 @@ FirmwareService *EspWiFi::makeFirmwareService(Context& context) {
         client.setTimeout(60); //in seconds
 
         // httpUpdate.setLedPin(LED_BUILTIN, HIGH);
-        t_httpUpdate_return ret = httpUpdate.update(client, location.c_str());
+        t_httpUpdate_return ret = httpUpdate.update(client, location);
 
         switch (ret) {
             case HTTP_UPDATE_FAILED:
@@ -368,7 +370,7 @@ FirmwareService *EspWiFi::makeFirmwareService(Context& context) {
 FirmwareService *EspWiFi::makeFirmwareService(Context& context) {
     FirmwareService *fwService = new FirmwareService(context);
 
-    fwService->setOnInstall([fwService] (const std::string &location) {
+    fwService->setOnInstall([fwService] (const char *location) {
 
         WiFiClient client;
         //WiFiClientSecure client;
@@ -377,7 +379,7 @@ FirmwareService *EspWiFi::makeFirmwareService(Context& context) {
 
         //ESPhttpUpdate.setLedPin(downloadStatusLedPin);
 
-        HTTPUpdateResult ret = ESPhttpUpdate.update(client, location.c_str());
+        HTTPUpdateResult ret = ESPhttpUpdate.update(client, location);
 
         switch (ret) {
             case HTTP_UPDATE_FAILED:

src/MicroOcpp/Model/FirmwareManagement/FirmwareService.h

@@ -45,13 +45,13 @@ class FirmwareService {
     Ocpp16::FirmwareStatus lastReportedStatus = Ocpp16::FirmwareStatus::Idle;
     bool checkedSuccessfulFwUpdate = false;
 
-    std::string location {};
-    Timestamp retreiveDate = Timestamp();
-    int retries = 0;
+    std::string location;
+    Timestamp retreiveDate;
+    unsigned int retries = 0;
     unsigned int retryInterval = 0;
 
-    std::function<bool(const std::string &location)> onDownload;
-    std::function<bool(const std::string &location)> onInstall;
+    std::function<bool(const char *location)> onDownload;
+    std::function<bool(const char *location)> onInstall;
 
     unsigned long delayTransition = 0;
     unsigned long timestampTransition = 0;
@@ -76,15 +76,15 @@ class FirmwareService {
 
     void loop();
 
-    void scheduleFirmwareUpdate(const std::string &location, Timestamp retreiveDate, int retries = 1, unsigned int retryInterval = 0);
+    void scheduleFirmwareUpdate(const char *location, Timestamp retreiveDate, unsigned int retries = 1, unsigned int retryInterval = 0);
 
     Ocpp16::FirmwareStatus getFirmwareStatus();
 
-    void setOnDownload(std::function<bool(const std::string &location)> onDownload);
+    void setOnDownload(std::function<bool(const char *location)> onDownload);
 
     void setDownloadStatusInput(std::function<DownloadStatus()> downloadStatusInput);
 
-    void setOnInstall(std::function<bool(const std::string &location)> onInstall);
+    void setOnInstall(std::function<bool(const char *location)> onInstall);
 
     void setInstallationStatusInput(std::function<InstallationStatus()> installationStatusInput);
 };

src/MicroOcpp/Operations/GetDiagnostics.cpp

@@ -9,57 +9,48 @@
 
 using MicroOcpp::Ocpp16::GetDiagnostics;
 
-GetDiagnostics::GetDiagnostics(Model& model) : model(model) {
+GetDiagnostics::GetDiagnostics(DiagnosticsService& diagService) : diagService(diagService) {
 
 }
 
 void GetDiagnostics::processReq(JsonObject payload) {
-    /*
-     * Process the application data here. Note: you have to implement the FW update procedure in your client code. You have to set
-     * a onSendConfListener in which you initiate a FW update (e.g. calling ESPhttpUpdate.update(...) )
-     */
 
-    const char *loc = payload["location"] | "";
-    location = loc;
+    const char *location = payload["location"] | "";
     //check location URL. Maybe introduce Same-Origin-Policy?
-    if (location.empty()) {
-        formatError = true;
-        MOCPP_DBG_WARN("Could not read location. Abort");
+    if (!*location) {
+        errorCode = "FormationViolation";
         return;
     }
 
-    retries = payload["retries"] | 1;
-    retryInterval = payload["retryInterval"] | 180;
+    int retries = payload["retries"] | 1;
+    int retryInterval = payload["retryInterval"] | 180;
+    if (retries < 0 || retryInterval < 0) {
+        errorCode = "PropertyConstraintViolation";
+        return;
+    }
 
-    //check the integrity of startTime
+    Timestamp startTime;
     if (payload.containsKey("startTime")) {
-        const char *startTimeRaw = payload["startTime"] | "Invalid";
-        if (!startTime.setTime(startTimeRaw)) {
-            formatError = true;
-            MOCPP_DBG_WARN("Could not read startTime. Abort");
+        if (!startTime.setTime(payload["startTime"] | "Invalid")) {
+            errorCode = "PropertyConstraintViolation";
+            MOCPP_DBG_WARN("bad time format");
             return;
         }
     }
 
-    //check the integrity of stopTime
+    Timestamp stopTime;
     if (payload.containsKey("startTime")) {
-        const char *stopTimeRaw = payload["stopTime"] | "Invalid";
-        if (!stopTime.setTime(stopTimeRaw)) {
-            formatError = true;
-            MOCPP_DBG_WARN("Could not read stopTime. Abort");
+        if (!stopTime.setTime(payload["stopTime"] | "Invalid")) {
+            errorCode = "PropertyConstraintViolation";
+            MOCPP_DBG_WARN("bad time format");
             return;
         }
     }
+
+    fileName = diagService.requestDiagnosticsUpload(location, (unsigned int) retries, (unsigned int) retryInterval, startTime, stopTime);
 }
 
 std::unique_ptr<DynamicJsonDocument> GetDiagnostics::createConf(){
-    if (auto diagService = model.getDiagnosticsService()) {
-        fileName = diagService->requestDiagnosticsUpload(location, retries, retryInterval, startTime, stopTime);
-    } else {
-        MOCPP_DBG_WARN("DiagnosticsService has not been initialized before! Please have a look at MicroOcpp.cpp for an example. Abort");
-        return createEmptyDocument();
-    }
-
     if (fileName.empty()) {
         return createEmptyDocument();
     } else {

src/MicroOcpp/Operations/GetDiagnostics.h

@@ -10,32 +10,26 @@
 
 namespace MicroOcpp {
 
-class Model;
+class DiagnosticsService;
 
 namespace Ocpp16 {
 
 class GetDiagnostics : public Operation {
 private:
-    Model& model;
-    std::string location {};
-    int retries = 1;
-    unsigned long retryInterval = 180;
-    Timestamp startTime = Timestamp();
-    Timestamp stopTime = Timestamp();
-
-    std::string fileName {};
-
-    bool formatError = false;
+    DiagnosticsService& diagService;
+    std::string fileName;
+
+    const char *errorCode = nullptr;
 public:
-    GetDiagnostics(Model& model);
+    GetDiagnostics(DiagnosticsService& diagService);
 
     const char* getOperationType() override {return "GetDiagnostics";}
 
     void processReq(JsonObject payload) override;
 
     std::unique_ptr<DynamicJsonDocument> createConf() override;
 
-    const char *getErrorCode() override {return formatError ? "FormationViolation" : nullptr;}
+    const char *getErrorCode() override {return errorCode;}
 };
 
 } //end namespace Ocpp16