Fix MeterValues tx-related message behavior (#356)

* revise latest changes in actual integration * fix MeterValues tx-related message order * adopt tx retry behavior * fix TriggerMessage for MeterValues
matth-x · Aug 18, 2024 · 7e017cf · 7e017cf
1 parent ed3034f
commit 7e017cf
Show file tree

Hide file tree

Showing 16 changed files with 635 additions and 139 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,7 +41,7 @@
 - Don't change into Unavailable upon Reset ([#344](https://github.com/matth-x/MicroOcpp/pull/344))
 - Reject DataTransfer by default ([#344](https://github.com/matth-x/MicroOcpp/pull/344))
 - UnlockConnector NotSupported if connectorId invalid ([#344](https://github.com/matth-x/MicroOcpp/pull/344))
-- Fix regression bug of [#345](https://github.com/matth-x/MicroOcpp/pull/345) ([#353](https://github.com/matth-x/MicroOcpp/pull/353))
+- Fix regression bug of [#345](https://github.com/matth-x/MicroOcpp/pull/345) ([#353](https://github.com/matth-x/MicroOcpp/pull/353), [#356](https://github.com/matth-x/MicroOcpp/pull/356))
 - Correct MeterValue PreBoot timestamp ([#354](https://github.com/matth-x/MicroOcpp/pull/354))
 
 ## [1.1.0] - 2024-05-21

diff --git a/src/MicroOcpp/Core/Request.cpp b/src/MicroOcpp/Core/Request.cpp
@@ -29,7 +29,7 @@ namespace MicroOcpp {
 
 using namespace MicroOcpp;
 
-Request::Request(std::unique_ptr<Operation> msg, const char *memory_tag) : MemoryManaged(memory_tag), messageID(makeString(getMemoryTag())), operation(std::move(msg)) {
+Request::Request(std::unique_ptr<Operation> msg) : MemoryManaged("Request.", msg->getOperationType()), messageID(makeString(getMemoryTag())), operation(std::move(msg)) {
     timeout_start = mocpp_tick_ms();
     debugRequest_start = mocpp_tick_ms();
 }
@@ -289,19 +289,15 @@ bool Request::isRequestSent() {
 
 namespace MicroOcpp {
 
-std::unique_ptr<Request> makeRequest(std::unique_ptr<Operation> operation, const char *memoryTag){
+std::unique_ptr<Request> makeRequest(std::unique_ptr<Operation> operation){
     if (operation == nullptr) {
         return nullptr;
     }
-    return std::unique_ptr<Request>(new Request(std::move(operation), memoryTag));
+    return std::unique_ptr<Request>(new Request(std::move(operation)));
 }
 
 std::unique_ptr<Request> makeRequest(Operation *operation) {
     return makeRequest(std::unique_ptr<Operation>(operation));
 }
 
-std::unique_ptr<Request> makeRequest(const char *memoryTag, Operation *operation) {
-    return makeRequest(std::unique_ptr<Operation>(operation), memoryTag);
-}
-
 } //end namespace MicroOcpp
diff --git a/src/MicroOcpp/Core/Request.h b/src/MicroOcpp/Core/Request.h
@@ -41,7 +41,7 @@ class Request : public MemoryManaged {
     bool requestSent = false;
 public:
 
-    Request(std::unique_ptr<Operation> msg, const char *memory_tag = "Request");
+    Request(std::unique_ptr<Operation> msg);
 
     ~Request();
 
@@ -121,9 +121,8 @@ class Request : public MemoryManaged {
 /*
  * Simple factory functions
  */
-std::unique_ptr<Request> makeRequest(std::unique_ptr<Operation> op, const char *memoryTag = "Request");
+std::unique_ptr<Request> makeRequest(std::unique_ptr<Operation> op);
 std::unique_ptr<Request> makeRequest(Operation *op); //takes ownership of op
-std::unique_ptr<Request> makeRequest(const char *memoryTag, Operation *op); //takes ownership of op
 
 } //end namespace MicroOcpp
 

diff --git a/src/MicroOcpp/Core/RequestQueue.h b/src/MicroOcpp/Core/RequestQueue.h
@@ -18,7 +18,7 @@
 #endif
 
 #ifndef MO_NUM_REQUEST_QUEUES
-#define MO_NUM_REQUEST_QUEUES 5
+#define MO_NUM_REQUEST_QUEUES 10
 #endif
 
 namespace MicroOcpp {

diff --git a/src/MicroOcpp/Model/ConnectorBase/Connector.cpp b/src/MicroOcpp/Model/ConnectorBase/Connector.cpp
@@ -92,14 +92,14 @@ Connector::Connector(Context& context, std::shared_ptr<FilesystemAdapter> filesy
             if (txNrPivot == std::numeric_limits<unsigned int>::max()) {
                 txNrPivot = parsedTxNr;
                 txNrBegin = parsedTxNr;
-                txNrBack = (parsedTxNr + 1) % MAX_TX_CNT;
+                txNrEnd = (parsedTxNr + 1) % MAX_TX_CNT;
                 return 0;
             }
 
             if ((parsedTxNr + MAX_TX_CNT - txNrPivot) % MAX_TX_CNT < MAX_TX_CNT / 2) {
                 //parsedTxNr is after pivot point
-                if ((parsedTxNr + 1 + MAX_TX_CNT - txNrPivot) % MAX_TX_CNT > (txNrBack + MAX_TX_CNT - txNrPivot) % MAX_TX_CNT) {
-                    txNrBack = (parsedTxNr + 1) % MAX_TX_CNT;
+                if ((parsedTxNr + 1 + MAX_TX_CNT - txNrPivot) % MAX_TX_CNT > (txNrEnd + MAX_TX_CNT - txNrPivot) % MAX_TX_CNT) {
+                    txNrEnd = (parsedTxNr + 1) % MAX_TX_CNT;
                 }
             } else if ((txNrPivot + MAX_TX_CNT - parsedTxNr) % MAX_TX_CNT < MAX_TX_CNT / 2) {
                 //parsedTxNr is before pivot point
@@ -108,16 +108,16 @@ Connector::Connector(Context& context, std::shared_ptr<FilesystemAdapter> filesy
                 }
             }
 
-            MO_DBG_DEBUG("found %s%u.jsn - Internal range from %u to %u (exclusive)", txFnamePrefix, parsedTxNr, txNrBegin, txNrBack);
+            MO_DBG_DEBUG("found %s%u.jsn - Internal range from %u to %u (exclusive)", txFnamePrefix, parsedTxNr, txNrBegin, txNrEnd);
         }
         return 0;
     });
 
-    MO_DBG_DEBUG("found %u transactions for connector %u. Internal range from %u to %u (exclusive)", (txNrBack + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT, connectorId, txNrBegin, txNrBack);
+    MO_DBG_DEBUG("found %u transactions for connector %u. Internal range from %u to %u (exclusive)", (txNrEnd + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT, connectorId, txNrBegin, txNrEnd);
     txNrFront = txNrBegin;
 
     if (model.getTransactionStore()) {
-        unsigned int txNrLatest = (txNrBack + MAX_TX_CNT - 1) % MAX_TX_CNT; //txNr of the most recent tx on flash
+        unsigned int txNrLatest = (txNrEnd + MAX_TX_CNT - 1) % MAX_TX_CNT; //txNr of the most recent tx on flash
         transaction = model.getTransactionStore()->getTransaction(connectorId, txNrLatest); //returns nullptr if txNrLatest does not exist on flash
     } else {
         MO_DBG_ERR("must initialize TxStore before Connector");
@@ -263,26 +263,26 @@ void Connector::loop() {
         }
 
         if (removed) {
-            if (txNrFront == txNrBack) {
+            if (txNrFront == txNrEnd) {
                 txNrFront = transaction->getTxNr();
             }
-            txNrBack = transaction->getTxNr(); //roll back creation of last tx entry
+            txNrEnd = transaction->getTxNr(); //roll back creation of last tx entry
         }
 
         MO_DBG_DEBUG("collect aborted or silent transaction %u-%u %s", connectorId, transaction->getTxNr(), removed ? "" : "failure");
-        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrBack=%u", txNrBegin, txNrFront, txNrBack);
+        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrEnd=%u", txNrBegin, txNrFront, txNrEnd);
         transaction = nullptr;
     }
 
     if (transaction && transaction->isAborted()) {
         MO_DBG_DEBUG("collect aborted transaction %u-%u", connectorId, transaction->getTxNr());
-        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrBack=%u", txNrBegin, txNrFront, txNrBack);
+        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrEnd=%u", txNrBegin, txNrFront, txNrEnd);
         transaction = nullptr;
     }
 
     if (transaction && transaction->getStopSync().isRequested()) {
         MO_DBG_DEBUG("collect obsolete transaction %u-%u", connectorId, transaction->getTxNr());
-        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrBack=%u", txNrBegin, txNrFront, txNrBack);
+        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrEnd=%u", txNrBegin, txNrFront, txNrEnd);
         transaction = nullptr;
     }
 
@@ -550,8 +550,8 @@ std::shared_ptr<Transaction> Connector::allocateTransaction() {
     std::shared_ptr<Transaction> tx;
 
     //clean possible aborted tx
-    unsigned int txr = txNrBack;
-    unsigned int txSize = (txNrBack + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT;
+    unsigned int txr = txNrEnd;
+    unsigned int txSize = (txNrEnd + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT;
     for (unsigned int i = 0; i < txSize; i++) {
         txr = (txr + MAX_TX_CNT - 1) % MAX_TX_CNT; //decrement by 1
 
@@ -567,10 +567,10 @@ std::shared_ptr<Transaction> Connector::allocateTransaction() {
                 removed &= model.getTransactionStore()->remove(connectorId, txr);
             }
             if (removed) {
-                if (txNrFront == txNrBack) {
+                if (txNrFront == txNrEnd) {
                     txNrFront = txr;
                 }
-                txNrBack = txr;
+                txNrEnd = txr;
                 MO_DBG_WARN("deleted dangling silent or aborted tx for new transaction");
             } else {
                 MO_DBG_ERR("memory corruption");
@@ -582,18 +582,18 @@ std::shared_ptr<Transaction> Connector::allocateTransaction() {
         }
     }
 
-    txSize = (txNrBack + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT; //refresh after cleaning txs
+    txSize = (txNrEnd + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT; //refresh after cleaning txs
 
     //try to create new transaction
     if (txSize < MO_TXRECORD_SIZE) {
-        tx = model.getTransactionStore()->createTransaction(connectorId, txNrBack);
+        tx = model.getTransactionStore()->createTransaction(connectorId, txNrEnd);
     }
 
     if (!tx) {
         //could not create transaction - now, try to replace tx history entry
 
         unsigned int txl = txNrBegin;
-        txSize = (txNrBack + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT;
+        txSize = (txNrEnd + MAX_TX_CNT - txNrBegin) % MAX_TX_CNT;
 
         for (unsigned int i = 0; i < txSize; i++) {
 
@@ -621,9 +621,9 @@ std::shared_ptr<Transaction> Connector::allocateTransaction() {
                         txNrFront = txNrBegin;
                     }
                     MO_DBG_DEBUG("deleted tx history entry for new transaction");
-                    MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrBack=%u", txNrBegin, txNrFront, txNrBack);
+                    MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrEnd=%u", txNrBegin, txNrFront, txNrEnd);
 
-                    tx = model.getTransactionStore()->createTransaction(connectorId, txNrBack);
+                    tx = model.getTransactionStore()->createTransaction(connectorId, txNrEnd);
                 } else {
                     MO_DBG_ERR("memory corruption");
                     break;
@@ -643,7 +643,7 @@ std::shared_ptr<Transaction> Connector::allocateTransaction() {
         //couldn't create normal transaction -> check if to start charging without real transaction
         if (silentOfflineTransactionsBool && silentOfflineTransactionsBool->getBool()) {
             //try to handle charging session without sending StartTx or StopTx to the server
-            tx = model.getTransactionStore()->createTransaction(connectorId, txNrBack, true);
+            tx = model.getTransactionStore()->createTransaction(connectorId, txNrEnd, true);
 
             if (tx) {
                 MO_DBG_DEBUG("created silent transaction");
@@ -661,9 +661,9 @@ std::shared_ptr<Transaction> Connector::allocateTransaction() {
     }
 
     if (tx) {
-        txNrBack = (txNrBack + 1) % MAX_TX_CNT;
-        MO_DBG_DEBUG("advance txNrBack %u-%u", connectorId, txNrBack);
-        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrBack=%u", txNrBegin, txNrFront, txNrBack);
+        txNrEnd = (txNrEnd + 1) % MAX_TX_CNT;
+        MO_DBG_DEBUG("advance txNrEnd %u-%u", connectorId, txNrEnd);
+        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrEnd=%u", txNrBegin, txNrFront, txNrEnd);
     }
 
     return tx;
@@ -1083,12 +1083,12 @@ unsigned int Connector::getFrontRequestOpNr() {
      * Advance front transaction?
      */
 
-    unsigned int txSize = (txNrBack + MAX_TX_CNT - txNrFront) % MAX_TX_CNT;
+    unsigned int txSize = (txNrEnd + MAX_TX_CNT - txNrFront) % MAX_TX_CNT;
 
     if (transactionFront && txSize == 0) {
         //catch edge case where txBack has been rolled back and txFront was equal to txBack
         MO_DBG_DEBUG("collect front transaction %u-%u after tx rollback", connectorId, transactionFront->getTxNr());
-        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrBack=%u", txNrBegin, txNrFront, txNrBack);
+        MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrEnd=%u", txNrBegin, txNrFront, txNrEnd);
         transactionFront = nullptr;
     }
 
@@ -1110,7 +1110,7 @@ unsigned int Connector::getFrontRequestOpNr() {
             MO_DBG_DEBUG("collect front transaction %u-%u", connectorId, transactionFront->getTxNr());
             transactionFront = nullptr;
             txNrFront = (txNrFront + 1) % MAX_TX_CNT;
-            MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrBack=%u", txNrBegin, txNrFront, txNrBack);
+            MO_DBG_VERBOSE("txNrBegin=%u, txNrFront=%u, txNrEnd=%u", txNrBegin, txNrFront, txNrEnd);
         } else {
             //front is accurate. Done here
             break;
@@ -1166,7 +1166,7 @@ std::unique_ptr<Request> Connector::fetchFrontRequest() {
             }
 
             Timestamp nextAttempt = transactionFront->getStartSync().getAttemptTime() +
-                                    transactionFront->getStartSync().getAttemptNr() * transactionMessageRetryIntervalInt->getInt();
+                                    transactionFront->getStartSync().getAttemptNr() * std::max(0, transactionMessageRetryIntervalInt->getInt());
 
             if (nextAttempt > model.getClock().now()) {
                 return nullptr;
@@ -1223,7 +1223,7 @@ std::unique_ptr<Request> Connector::fetchFrontRequest() {
             }
 
             Timestamp nextAttempt = transactionFront->getStopSync().getAttemptTime() +
-                                    transactionFront->getStopSync().getAttemptNr() * transactionMessageRetryIntervalInt->getInt();
+                                    transactionFront->getStopSync().getAttemptNr() * std::max(0, transactionMessageRetryIntervalInt->getInt());
 
             if (nextAttempt > model.getClock().now()) {
                 return nullptr;
@@ -1270,3 +1270,15 @@ std::unique_ptr<Request> Connector::fetchFrontRequest() {
 
     return nullptr;
 }
+
+unsigned int Connector::getTxNrBeginHistory() {
+    return txNrBegin;
+}
+
+unsigned int Connector::getTxNrFront() {
+    return txNrFront;
+}
+
+unsigned int Connector::getTxNrEnd() {
+    return txNrEnd;
+}
diff --git a/src/MicroOcpp/Model/ConnectorBase/Connector.h b/src/MicroOcpp/Model/ConnectorBase/Connector.h
@@ -95,7 +95,7 @@ class Connector : public RequestEmitter, public MemoryManaged {
 
     unsigned int txNrBegin = 0; //oldest (historical) transaction on flash. Has no function, but is useful for error diagnosis
     unsigned int txNrFront = 0; //oldest transaction which is still queued to be sent to the server
-    unsigned int txNrBack = 0; //one position behind newest transaction
+    unsigned int txNrEnd = 0; //one position behind newest transaction
 
     std::shared_ptr<Transaction> transactionFront;
 public:
@@ -157,6 +157,10 @@ class Connector : public RequestEmitter, public MemoryManaged {
 
     unsigned int getFrontRequestOpNr() override;
     std::unique_ptr<Request> fetchFrontRequest() override;
+
+    unsigned int getTxNrBeginHistory(); //if getTxNrBeginHistory() != getTxNrFront(), then return value is the txNr of the oldest tx history entry. If equal to getTxNrFront(), then the history is empty
+    unsigned int getTxNrFront(); //if getTxNrEnd() != getTxNrFront(), then return value is the txNr of the oldest transaction queued to be sent to the server. If equal to getTxNrEnd(), then there is no tx to be sent to the server
+    unsigned int getTxNrEnd(); //upper limit for the range of valid txNrs
 };
 
 } //end namespace MicroOcpp

diff --git a/src/MicroOcpp/Model/Diagnostics/DiagnosticsService.cpp b/src/MicroOcpp/Model/Diagnostics/DiagnosticsService.cpp
@@ -19,8 +19,9 @@
 #include <MicroOcpp/Version.h> //for MO_ENABLE_V201
 #include <MicroOcpp/Model/ConnectorBase/UnlockConnectorResult.h> //for MO_ENABLE_CONNECTOR_LOCK
 
-using namespace MicroOcpp;
-using Ocpp16::DiagnosticsStatus;
+using MicroOcpp::DiagnosticsService;
+using MicroOcpp::Ocpp16::DiagnosticsStatus;
+using MicroOcpp::Request;
 
 DiagnosticsService::DiagnosticsService(Context& context) : MemoryManaged("v16.Diagnostics.DiagnosticsService"), context(context), location(makeString(getMemoryTag())), diagFileList(makeVector<String>(getMemoryTag())) {
 

diff --git a/src/MicroOcpp/Model/Metering/MeterValue.cpp b/src/MicroOcpp/Model/Metering/MeterValue.cpp
@@ -2,6 +2,8 @@
 // Copyright Matthias Akstaller 2019 - 2024
 // MIT License
 
+#include <limits>
+
 #include <MicroOcpp/Model/Metering/MeterValue.h>
 #include <MicroOcpp/Core/Configuration.h>
 #include <MicroOcpp/Debug.h>
@@ -67,6 +69,42 @@ ReadingContext MeterValue::getReadingContext() {
     return ReadingContext::NOT_SET;
 }
 
+void MeterValue::setTxNr(unsigned int txNr) {
+    if (txNr > (unsigned int)std::numeric_limits<int>::max()) {
+        MO_DBG_ERR("invalid arg");
+        return;
+    }
+    this->txNr = (int)txNr;
+}
+
+int MeterValue::getTxNr() {
+    return txNr;
+}
+
+void MeterValue::setOpNr(unsigned int opNr) {
+    this->opNr = opNr;
+}
+
+unsigned int MeterValue::getOpNr() {
+    return opNr;
+}
+
+void MeterValue::advanceAttemptNr() {
+    attemptNr++;
+}
+
+unsigned int MeterValue::getAttemptNr() {
+    return attemptNr;
+}
+
+unsigned long MeterValue::getAttemptTime() {
+    return attemptTime;
+}
+
+void MeterValue::setAttemptTime(unsigned long timestamp) {
+    this->attemptTime = timestamp;
+}
+
 MeterValueBuilder::MeterValueBuilder(const Vector<std::unique_ptr<SampledValueSampler>> &samplers,
             std::shared_ptr<Configuration> samplersSelectStr) :
             MemoryManaged("v16.Metering.MeterValueBuilder"),

diff --git a/src/MicroOcpp/Model/Metering/MeterValue.h b/src/MicroOcpp/Model/Metering/MeterValue.h
@@ -18,6 +18,11 @@ class MeterValue : public MemoryManaged {
 private:
     Timestamp timestamp;
     Vector<std::unique_ptr<SampledValue>> sampledValue;
+
+    int txNr = -1;
+    unsigned int opNr = 1;
+    unsigned int attemptNr = 0;
+    unsigned long attemptTime = 0;
 public:
     MeterValue(const Timestamp& timestamp);
     MeterValue(const MeterValue& other) = delete;
@@ -30,6 +35,18 @@ class MeterValue : public MemoryManaged {
     void setTimestamp(Timestamp timestamp);
 
     ReadingContext getReadingContext();
+
+    void setTxNr(unsigned int txNr);
+    int getTxNr();
+
+    void setOpNr(unsigned int opNr);
+    unsigned int getOpNr();
+
+    void advanceAttemptNr();
+    unsigned int getAttemptNr();
+
+    unsigned long getAttemptTime();
+    void setAttemptTime(unsigned long timestamp);
 };
 
 class MeterValueBuilder : public MemoryManaged {