From 763097b0789ed4184382346ca85890f0aed7fa94 Mon Sep 17 00:00:00 2001 From: matth-x <63792403+matth-x@users.noreply.github.com> Date: Sat, 23 Sep 2023 20:49:55 +0200 Subject: [PATCH 1/5] update reservation with atomic attributes --- .../Model/Reservation/Reservation.cpp | 17 +++++---- src/MicroOcpp/Model/Reservation/Reservation.h | 2 +- .../Model/Reservation/ReservationService.cpp | 10 +++--- .../Model/Reservation/ReservationService.h | 2 +- src/MicroOcpp/Operations/ReserveNow.cpp | 36 ++++++++++++------- 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/src/MicroOcpp/Model/Reservation/Reservation.cpp b/src/MicroOcpp/Model/Reservation/Reservation.cpp index efa5952b..8a18c464 100644 --- a/src/MicroOcpp/Model/Reservation/Reservation.cpp +++ b/src/MicroOcpp/Model/Reservation/Reservation.cpp @@ -109,13 +109,16 @@ const char *Reservation::getParentIdTag() { return parentIdTagString->getString(); } -void Reservation::update(JsonObject payload) { - connectorIdInt->setInt(payload["connectorId"] | -1); - expiryDate.setTime(payload["expiryDate"]); - expiryDateRawString->setString(payload["expiryDate"] | ""); - idTagString->setString(payload["idTag"] | ""); - reservationIdInt->setInt(payload["reservationId"] | -1); - parentIdTagString->setString(payload["parentIdTag"] | ""); +void Reservation::update(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag) { + reservationIdInt->setInt(reservationId); + connectorIdInt->setInt((int) connectorId); + this->expiryDate = expiryDate; + char expiryDate_cstr [JSONDATE_LENGTH + 1]; + if (this->expiryDate.toJsonString(expiryDate_cstr, JSONDATE_LENGTH + 1)) { + expiryDateRawString->setString(expiryDate_cstr); + } + idTagString->setString(idTag); + parentIdTagString->setString(parentIdTag); configuration_save(); } diff --git a/src/MicroOcpp/Model/Reservation/Reservation.h b/src/MicroOcpp/Model/Reservation/Reservation.h index 222c3adf..8bcb3922 100644 --- a/src/MicroOcpp/Model/Reservation/Reservation.h +++ b/src/MicroOcpp/Model/Reservation/Reservation.h @@ -55,7 +55,7 @@ class Reservation { int getReservationId(); const char *getParentIdTag(); - void update(JsonObject payload); + void update(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag = nullptr); void clear(); }; diff --git a/src/MicroOcpp/Model/Reservation/ReservationService.cpp b/src/MicroOcpp/Model/Reservation/ReservationService.cpp index 63b34182..1882a321 100644 --- a/src/MicroOcpp/Model/Reservation/ReservationService.cpp +++ b/src/MicroOcpp/Model/Reservation/ReservationService.cpp @@ -178,15 +178,15 @@ Reservation *ReservationService::getReservationById(int reservationId) { return nullptr; } -bool ReservationService::updateReservation(JsonObject payload) { - if (auto reservation = getReservationById(payload["reservationId"])) { - reservation->update(payload); +bool ReservationService::updateReservation(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag) { + if (auto reservation = getReservationById(reservationId)) { + reservation->update(reservationId, connectorId, expiryDate, idTag, parentIdTag); return true; } // Alternative condition: avoids that one idTag can make two reservations at a time. The specification doesn't // mention that double-reservations should be possible but it seems to mean it. - if (auto reservation = getReservation(payload["connectorId"], nullptr, nullptr)) { + if (auto reservation = getReservation(connectorId, nullptr, nullptr)) { // payload["idTag"], // payload.containsKey("parentIdTag") ? payload["parentIdTag"] : nullptr)) { // if (auto reservation = getReservation(payload["connectorId"].as())) { @@ -198,7 +198,7 @@ bool ReservationService::updateReservation(JsonObject payload) { //update free reservation slot for (auto& reservation : reservations) { if (!reservation->isActive()) { - reservation->update(payload); + reservation->update(reservationId, connectorId, expiryDate, idTag, parentIdTag); return true; } } diff --git a/src/MicroOcpp/Model/Reservation/ReservationService.h b/src/MicroOcpp/Model/Reservation/ReservationService.h index 40876e41..87a6a486 100644 --- a/src/MicroOcpp/Model/Reservation/ReservationService.h +++ b/src/MicroOcpp/Model/Reservation/ReservationService.h @@ -39,7 +39,7 @@ class ReservationService { Reservation *getReservationById(int reservationId); - bool updateReservation(JsonObject payload); + bool updateReservation(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag = nullptr); }; } diff --git a/src/MicroOcpp/Operations/ReserveNow.cpp b/src/MicroOcpp/Operations/ReserveNow.cpp index e3ef457d..9b256b3f 100644 --- a/src/MicroOcpp/Operations/ReserveNow.cpp +++ b/src/MicroOcpp/Operations/ReserveNow.cpp @@ -33,33 +33,45 @@ void ReserveNow::processReq(JsonObject payload) { return; } - Timestamp validateTimestamp; - if (!validateTimestamp.setTime(payload["expiryDate"])) { + int connectorId = payload["connectorId"] | -1; + if (connectorId < 0 || connectorId >= model.getNumConnectors()) { + errorCode = "PropertyConstraintViolation"; + return; + } + + Timestamp expiryDate; + if (!expiryDate.setTime(payload["expiryDate"])) { MOCPP_DBG_WARN("bad time format"); errorCode = "PropertyConstraintViolation"; return; } - unsigned int connectorId = payload["connectorId"]; + const char *idTag = payload["idTag"] | ""; + if (!*idTag) { + errorCode = "PropertyConstraintViolation"; + return; + } + + const char *parentIdTag = nullptr; + if (payload.containsKey("parentIdTag")) { + parentIdTag = payload["parentIdTag"]; + } + + int reservationId = payload["reservationId"] | -1; if (model.getReservationService() && model.getNumConnectors() > 0) { auto rService = model.getReservationService(); auto chargePoint = model.getConnector(0); - if (connectorId >= model.getNumConnectors()) { - errorCode = "PropertyConstraintViolation"; - return; - } - auto reserveConnectorZeroSupportedBool = declareConfiguration("ReserveConnectorZeroSupported", true, CONFIGURATION_VOLATILE); if (connectorId == 0 && (!reserveConnectorZeroSupportedBool || !reserveConnectorZeroSupportedBool->getBool())) { reservationStatus = "Rejected"; return; } - if (auto reservation = rService->getReservationById(payload["reservationId"])) { - reservation->update(payload); + if (auto reservation = rService->getReservationById(reservationId)) { + reservation->update(reservationId, (unsigned int) connectorId, expiryDate, idTag, parentIdTag); reservationStatus = "Accepted"; return; } @@ -67,7 +79,7 @@ void ReserveNow::processReq(JsonObject payload) { Connector *connector = nullptr; if (connectorId > 0) { - connector = model.getConnector(connectorId); + connector = model.getConnector((unsigned int) connectorId); } if (chargePoint->getStatus() == ChargePointStatus::Faulted || @@ -87,7 +99,7 @@ void ReserveNow::processReq(JsonObject payload) { return; } - if (rService->updateReservation(payload)) { + if (rService->updateReservation(reservationId, (unsigned int) connectorId, expiryDate, idTag, parentIdTag)) { reservationStatus = "Accepted"; } else { reservationStatus = "Occupied"; From 04c32583b0fb37dc9dae004cc50c2b684d8ad827 Mon Sep 17 00:00:00 2001 From: matth-x <63792403+matth-x@users.noreply.github.com> Date: Sat, 23 Sep 2023 21:39:58 +0200 Subject: [PATCH 2/5] fix beginTx clears Reservation (addresses #191) --- CMakeLists.txt | 1 + .../Model/Reservation/ReservationService.cpp | 2 +- tests/Reservation.cpp | 71 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 tests/Reservation.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 86150458..ac3ecb15 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -125,6 +125,7 @@ set(MOCPP_SRC_UNIT tests/Api.cpp tests/Metering.cpp tests/Configuration.cpp + tests/Reservation.cpp ) add_executable(mo_unit_tests diff --git a/src/MicroOcpp/Model/Reservation/ReservationService.cpp b/src/MicroOcpp/Model/Reservation/ReservationService.cpp index 1882a321..9d7800af 100644 --- a/src/MicroOcpp/Model/Reservation/ReservationService.cpp +++ b/src/MicroOcpp/Model/Reservation/ReservationService.cpp @@ -48,7 +48,7 @@ void ReservationService::loop() { } //check if other tx started at this connector (e.g. due to RemoteStartTransaction) - if (connector->getTransaction() && connector->getTransaction()->isActive()) { + if (connector->getTransaction() && connector->getTransaction()->isAuthorized()) { reservation->clear(); continue; } diff --git a/tests/Reservation.cpp b/tests/Reservation.cpp new file mode 100644 index 00000000..95d23f0f --- /dev/null +++ b/tests/Reservation.cpp @@ -0,0 +1,71 @@ +#include +#include +#include "./catch2/catch.hpp" +#include "./helpers/testHelper.h" + +#include +#include +#include + +#include +#include +#include +#include + + +#define BASE_TIME "2023-01-01T00:00:00.000Z" + +using namespace MicroOcpp; + +TEST_CASE( "Reservation" ) { + printf("\nRun %s\n", "Reservation"); + + //clean state + auto filesystem = makeDefaultFilesystemAdapter(FilesystemOpt::Use_Mount_FormatOnFail); + FilesystemUtils::remove_if(filesystem, [] (const char*) {return true;}); + + //initialize Context with dummy socket + LoopbackConnection loopback; + + mocpp_set_timer(custom_timer_cb); + + SECTION("Basic reservation") { + mocpp_initialize(loopback, ChargerCredentials("test-runner1234")); + auto& model = getOcppContext()->getModel(); + model.getClock().setTime(BASE_TIME); + + loop(); + + auto connector = model.getConnector(1); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + auto rService = model.getReservationService(); + REQUIRE( rService ); + + //set reservation + int reservationId = 1; + unsigned int connectorId = 1; + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = nullptr; + + rService->updateReservation(reservationId, connectorId, expiryDate, idTag, parentIdTag); + + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + + //transaction blocked by reservation + bool checkTxRejected = false; + setTxNotificationOutput([&checkTxRejected] (Transaction*, TxNotification txNotification) { + if (txNotification == TxNotification::ReservationConflict) { + checkTxRejected = true; + } + }); + + beginTransaction("wrong idTag"); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + REQUIRE( checkTxRejected ); + + mocpp_deinitialize(); + } +} From 3728edce2fa4a8de25ccefa5390a46925aa646d1 Mon Sep 17 00:00:00 2001 From: matth-x <63792403+matth-x@users.noreply.github.com> Date: Sun, 24 Sep 2023 14:51:25 +0200 Subject: [PATCH 3/5] fix reservationId in StartTx (addresses #154) --- .../Model/ConnectorBase/Connector.cpp | 158 +++++++++++------- .../Model/Reservation/ReservationService.cpp | 4 +- tests/Reservation.cpp | 13 +- 3 files changed, 108 insertions(+), 67 deletions(-) diff --git a/src/MicroOcpp/Model/ConnectorBase/Connector.cpp b/src/MicroOcpp/Model/ConnectorBase/Connector.cpp index f9d6d91e..ba2933e0 100644 --- a/src/MicroOcpp/Model/ConnectorBase/Connector.cpp +++ b/src/MicroOcpp/Model/ConnectorBase/Connector.cpp @@ -524,47 +524,62 @@ std::shared_ptr Connector::beginTransaction(const char *idTag) { return nullptr; } - auto rService = model.getReservationService(); + MOCPP_DBG_DEBUG("Begin transaction process (%s), prepare", idTag != nullptr ? idTag : ""); + AuthorizationData *localAuth = nullptr; - bool expiredLocalAuth = false; + bool offlineBlockedAuth = false; //if offline authorization will be blocked by local auth list entry //check local OCPP whitelist - if (auto authService = model.getAuthorizationService()) { + if (model.getAuthorizationService()) { + localAuth = model.getAuthorizationService()->getLocalAuthorization(idTag); - localAuth = authService->getLocalAuthorization(idTag); - - //check if blocked by reservation - Reservation *reservation = nullptr; - if (localAuth && rService) { - reservation = rService->getReservation( - connectorId, - idTag, - localAuth->getParentIdTag() ? localAuth->getParentIdTag() : nullptr); - if (reservation && !reservation->matches( - idTag, - localAuth->getParentIdTag() ? localAuth->getParentIdTag() : nullptr)) { - //reservation found for connector but does not match idTag or parentIdTag - MOCPP_DBG_DEBUG("reservation blocks local auth at connector %u", connectorId); - localAuth = nullptr;; - } - } - + //check authorization status if (localAuth && localAuth->getAuthorizationStatus() != AuthorizationStatus::Accepted) { MOCPP_DBG_DEBUG("local auth denied (%s)", idTag); + offlineBlockedAuth = true; localAuth = nullptr; } //check expiry - if (localAuth && localAuth->getExpiryDate()) { - auto& tnow = model.getClock().now(); - if (tnow >= *localAuth->getExpiryDate()) { - MOCPP_DBG_DEBUG("idTag %s local auth entry expired", idTag); + if (localAuth && localAuth->getExpiryDate() && *localAuth->getExpiryDate() < model.getClock().now()) { + MOCPP_DBG_DEBUG("idTag %s local auth entry expired", idTag); + offlineBlockedAuth = true; + localAuth = nullptr; + } + } + + Reservation *reservation = nullptr; + bool offlineBlockedResv = false; //if offline authorization will be blocked by reservation + + //check if blocked by reservation + if (model.getReservationService()) { + const char *parentIdTag = localAuth ? localAuth->getParentIdTag() : nullptr; + + reservation = model.getReservationService()->getReservation( + connectorId, + idTag, + parentIdTag); + + if (reservation && !reservation->matches( + idTag, + parentIdTag)) { + //reservation blocks connector + offlineBlockedResv = true; //when offline, tx is always blocked + + //if parentIdTag is known, abort this tx immediately, otherwise wait for Authorize.conf to decide + if (parentIdTag) { + //parentIdTag known + MOCPP_DBG_INFO("connector %u reserved - abort transaction", connectorId); + updateTxNotification(TxNotification::ReservationConflict); + return nullptr; + } else { + //parentIdTag unkown but local authorization failed in any case + MOCPP_DBG_INFO("connector %u reserved - no local auth", connectorId); localAuth = nullptr; - expiredLocalAuth = true; } } } - + transaction = allocateTransaction(); if (!transaction) { @@ -581,17 +596,16 @@ std::shared_ptr Connector::beginTransaction(const char *idTag) { transaction->setBeginTimestamp(model.getClock().now()); - MOCPP_DBG_DEBUG("Begin transaction process (%s), prepare", idTag != nullptr ? idTag : ""); - //check for local preauthorization - if (localPreAuthorizeBool && localPreAuthorizeBool->getBool()) { - if (localAuth && localAuth->getAuthorizationStatus() == AuthorizationStatus::Accepted) { - MOCPP_DBG_DEBUG("Begin transaction process (%s), preauthorized locally", idTag != nullptr ? idTag : ""); - - transaction->setAuthorized(); + if (localAuth && localPreAuthorizeBool && localPreAuthorizeBool->getBool()) { + MOCPP_DBG_DEBUG("Begin transaction process (%s), preauthorized locally", idTag != nullptr ? idTag : ""); - updateTxNotification(TxNotification::Authorized); + if (reservation) { + transaction->setReservationId(reservation->getReservationId()); } + transaction->setAuthorized(); + + updateTxNotification(TxNotification::Authorized); } transaction->commit(); @@ -616,15 +630,21 @@ std::shared_ptr Connector::beginTransaction(const char *idTag) { connectorId, tx->getIdTag(), idTagInfo["parentIdTag"] | (const char*) nullptr); - if (reservation && !reservation->matches( - tx->getIdTag(), - idTagInfo["parentIdTag"] | (const char*) nullptr)) { - //reservation found for connector but does not match idTag or parentIdTag - MOCPP_DBG_INFO("connector %u reserved - abort transaction", connectorId); - tx->setInactive(); - tx->commit(); - updateTxNotification(TxNotification::ReservationConflict); - return; + if (reservation) { + //reservation found for connector + if (reservation->matches( + tx->getIdTag(), + idTagInfo["parentIdTag"] | (const char*) nullptr)) { + MOCPP_DBG_INFO("connector %u matches reservationId %i", connectorId, reservation->getReservationId()); + tx->setReservationId(reservation->getReservationId()); + } else { + //reservation found for connector but does not match idTag or parentIdTag + MOCPP_DBG_INFO("connector %u reserved - abort transaction", connectorId); + tx->setInactive(); + tx->commit(); + updateTxNotification(TxNotification::ReservationConflict); + return; + } } } @@ -634,9 +654,19 @@ std::shared_ptr Connector::beginTransaction(const char *idTag) { updateTxNotification(TxNotification::Authorized); }); - authorize->setOnTimeoutListener([this, tx, localAuth, expiredLocalAuth] () { - if (expiredLocalAuth) { + //capture local auth and reservation check in for timeout handler + bool localAuthFound = localAuth; + bool reservationFound = reservation; + int reservationId = reservation ? reservation->getReservationId() : -1; + authorize->setOnTimeoutListener([this, tx, + offlineBlockedAuth, + offlineBlockedResv, + localAuthFound, + reservationFound, + reservationId] () { + + if (offlineBlockedAuth) { //local auth entry exists, but is expired -> avoid offline tx MOCPP_DBG_DEBUG("Abort transaction process (%s), timeout, expired local auth", tx->getIdTag()); tx->setInactive(); @@ -644,9 +674,21 @@ std::shared_ptr Connector::beginTransaction(const char *idTag) { updateTxNotification(TxNotification::AuthorizationTimeout); return; } - - if (localAuth) { + + if (offlineBlockedResv) { + //reservation found for connector but does not match idTag or parentIdTag + MOCPP_DBG_INFO("connector %u reserved (offline) - abort transaction", connectorId); + tx->setInactive(); + tx->commit(); + updateTxNotification(TxNotification::ReservationConflict); + return; + } + + if (localAuthFound) { MOCPP_DBG_DEBUG("Offline transaction process (%s), locally authorized", tx->getIdTag()); + if (reservationFound) { + tx->setReservationId(reservationId); + } tx->setAuthorized(); tx->commit(); @@ -654,29 +696,17 @@ std::shared_ptr Connector::beginTransaction(const char *idTag) { return; } - if (model.getReservationService()) { - auto reservation = model.getReservationService()->getReservation( - connectorId, - tx->getIdTag()); - if (reservation && !reservation->matches( - tx->getIdTag())) { - //reservation found for connector but does not match idTag or parentIdTag - MOCPP_DBG_INFO("connector %u reserved (offline) - abort transaction", connectorId); - tx->setInactive(); - tx->commit(); - updateTxNotification(TxNotification::ReservationConflict); - return; - } - } - if (allowOfflineTxForUnknownIdBool && allowOfflineTxForUnknownIdBool->getBool()) { MOCPP_DBG_DEBUG("Offline transaction process (%s), allow unknown ID", tx->getIdTag()); + if (reservationFound) { + tx->setReservationId(reservationId); + } tx->setAuthorized(); tx->commit(); updateTxNotification(TxNotification::Authorized); return; } - + MOCPP_DBG_DEBUG("Abort transaction process (%s): timeout", tx->getIdTag()); tx->setInactive(); tx->commit(); diff --git a/src/MicroOcpp/Model/Reservation/ReservationService.cpp b/src/MicroOcpp/Model/Reservation/ReservationService.cpp index 9d7800af..fe906e52 100644 --- a/src/MicroOcpp/Model/Reservation/ReservationService.cpp +++ b/src/MicroOcpp/Model/Reservation/ReservationService.cpp @@ -57,11 +57,11 @@ void ReservationService::loop() { //check if tx with same idTag or reservationId has started for (unsigned int cId = 1; cId < context.getModel().getNumConnectors(); cId++) { auto& transaction = context.getModel().getConnector(cId)->getTransaction(); - if (transaction) { + if (transaction && transaction->isAuthorized()) { const char *cIdTag = transaction->getIdTag(); if (transaction->getReservationId() == reservation->getReservationId() || (cIdTag && !strcmp(cIdTag, reservation->getIdTag()))) { - + reservation->clear(); break; } diff --git a/tests/Reservation.cpp b/tests/Reservation.cpp index 95d23f0f..3064e15c 100644 --- a/tests/Reservation.cpp +++ b/tests/Reservation.cpp @@ -43,7 +43,7 @@ TEST_CASE( "Reservation" ) { REQUIRE( rService ); //set reservation - int reservationId = 1; + int reservationId = 123; unsigned int connectorId = 1; Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future const char *idTag = "mIdTag"; @@ -66,6 +66,17 @@ TEST_CASE( "Reservation" ) { REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); REQUIRE( checkTxRejected ); + //idTag matches reservation + beginTransaction("mIdTag"); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Charging ); + REQUIRE( connector->getTransaction()->getReservationId() == reservationId ); + + //reservation is reset after tx + endTransaction(); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + mocpp_deinitialize(); } } From c7ab7e2c502465a9995f41a25e95a9780d213939 Mon Sep 17 00:00:00 2001 From: matth-x <63792403+matth-x@users.noreply.github.com> Date: Sun, 24 Sep 2023 17:58:37 +0200 Subject: [PATCH 4/5] further Reservation tests and fixes --- .../Model/Reservation/Reservation.cpp | 11 +- src/MicroOcpp/Model/Reservation/Reservation.h | 6 +- .../Model/Reservation/ReservationService.cpp | 6 +- tests/Configuration.cpp | 2 - tests/Reservation.cpp | 384 +++++++++++++++++- 5 files changed, 390 insertions(+), 19 deletions(-) diff --git a/src/MicroOcpp/Model/Reservation/Reservation.cpp b/src/MicroOcpp/Model/Reservation/Reservation.cpp index 8a18c464..62af789f 100644 --- a/src/MicroOcpp/Model/Reservation/Reservation.cpp +++ b/src/MicroOcpp/Model/Reservation/Reservation.cpp @@ -6,10 +6,6 @@ #include #include -#ifndef RESERVATION_FN -#define RESERVATION_FN (MOCPP_FILENAME_PREFIX "reservations.jsn") -#endif - using namespace MicroOcpp; Reservation::Reservation(Model& model, unsigned int slot) : model(model), slot(slot) { @@ -19,7 +15,6 @@ Reservation::Reservation(Model& model, unsigned int slot) : model(model), slot(s snprintf(expiryDateRawKey, sizeof(expiryDateRawKey), MOCPP_RESERVATION_EXPDATE_KEY "%u", slot); expiryDateRawString = declareConfiguration(expiryDateRawKey, "", RESERVATION_FN, false, false, false); - expiryDate.setTime(expiryDateRawString->getString()); snprintf(idTagKey, sizeof(idTagKey), MOCPP_RESERVATION_IDTAG_KEY "%u", slot); idTagString = declareConfiguration(idTagKey, "", RESERVATION_FN, false, false, false); @@ -60,8 +55,7 @@ bool Reservation::isActive() { return false; } - auto& t_now = model.getClock().now(); - if (t_now > expiryDate) { + if (model.getClock().now() > getExpiryDate()) { //reservation expired return false; } @@ -94,6 +88,9 @@ int Reservation::getConnectorId() { } Timestamp& Reservation::getExpiryDate() { + if (expiryDate == MIN_TIME && *expiryDateRawString->getString()) { + expiryDate.setTime(expiryDateRawString->getString()); + } return expiryDate; } diff --git a/src/MicroOcpp/Model/Reservation/Reservation.h b/src/MicroOcpp/Model/Reservation/Reservation.h index 8bcb3922..11f9a851 100644 --- a/src/MicroOcpp/Model/Reservation/Reservation.h +++ b/src/MicroOcpp/Model/Reservation/Reservation.h @@ -8,6 +8,10 @@ #include #include +#ifndef RESERVATION_FN +#define RESERVATION_FN (MOCPP_FILENAME_PREFIX "reservations.jsn") +#endif + #define MOCPP_RESERVATION_CID_KEY "cid_" #define MOCPP_RESERVATION_EXPDATE_KEY "expdt_" #define MOCPP_RESERVATION_IDTAG_KEY "idt_" @@ -28,7 +32,7 @@ class Reservation { std::shared_ptr expiryDateRawString; char expiryDateRawKey [sizeof(MOCPP_RESERVATION_EXPDATE_KEY "xxx") + 1]; - Timestamp expiryDate; + Timestamp expiryDate = MIN_TIME; std::shared_ptr idTagString; char idTagKey [sizeof(MOCPP_RESERVATION_IDTAG_KEY "xxx") + 1]; std::shared_ptr reservationIdInt; diff --git a/src/MicroOcpp/Model/Reservation/ReservationService.cpp b/src/MicroOcpp/Model/Reservation/ReservationService.cpp index fe906e52..1c58ac09 100644 --- a/src/MicroOcpp/Model/Reservation/ReservationService.cpp +++ b/src/MicroOcpp/Model/Reservation/ReservationService.cpp @@ -18,7 +18,7 @@ ReservationService::ReservationService(Context& context, unsigned int numConnect if (maxReservations > 0) { reservations.reserve((size_t) maxReservations); for (int i = 0; i < maxReservations; i++) { - reservations.push_back(std::unique_ptr(new Reservation(context.getModel(), i))); + reservations.emplace_back(new Reservation(context.getModel(), i)); } } @@ -180,6 +180,10 @@ Reservation *ReservationService::getReservationById(int reservationId) { bool ReservationService::updateReservation(int reservationId, unsigned int connectorId, Timestamp expiryDate, const char *idTag, const char *parentIdTag) { if (auto reservation = getReservationById(reservationId)) { + if (getReservation(connectorId) && getReservation(connectorId) != reservation && getReservation(connectorId)->isActive()) { + MOCPP_DBG_DEBUG("found blocking reservation at connectorId %u", connectorId); + return false; //cannot transfer reservation to other connector with existing reservation + } reservation->update(reservationId, connectorId, expiryDate, idTag, parentIdTag); return true; } diff --git a/tests/Configuration.cpp b/tests/Configuration.cpp index bf2e2a71..a392b472 100644 --- a/tests/Configuration.cpp +++ b/tests/Configuration.cpp @@ -25,7 +25,6 @@ TEST_CASE( "Configuration" ) { //clean state auto filesystem = makeDefaultFilesystemAdapter(FilesystemOpt::Use_Mount_FormatOnFail); - MOCPP_DBG_DEBUG("remove all"); FilesystemUtils::remove_if(filesystem, [] (const char*) {return true;}); LoopbackConnection loopback; //initialize Context with dummy socket @@ -164,7 +163,6 @@ TEST_CASE( "Configuration" ) { configuration_deinit(); REQUIRE( !strcmp(cString4->getString(), "mValue3") ); - MOCPP_DBG_DEBUG("remove all"); FilesystemUtils::remove_if(filesystem, [] (const char*) {return true;}); //config accessibility / permissions diff --git a/tests/Reservation.cpp b/tests/Reservation.cpp index 3064e15c..fb698639 100644 --- a/tests/Reservation.cpp +++ b/tests/Reservation.cpp @@ -29,17 +29,16 @@ TEST_CASE( "Reservation" ) { mocpp_set_timer(custom_timer_cb); - SECTION("Basic reservation") { - mocpp_initialize(loopback, ChargerCredentials("test-runner1234")); - auto& model = getOcppContext()->getModel(); - model.getClock().setTime(BASE_TIME); + mocpp_initialize(loopback, ChargerCredentials("test-runner1234")); + auto& model = getOcppContext()->getModel(); + auto rService = model.getReservationService(); + auto connector = model.getConnector(1); + model.getClock().setTime(BASE_TIME); - loop(); + loop(); - auto connector = model.getConnector(1); + SECTION("Basic reservation") { REQUIRE( connector->getStatus() == ChargePointStatus::Available ); - - auto rService = model.getReservationService(); REQUIRE( rService ); //set reservation @@ -77,6 +76,375 @@ TEST_CASE( "Reservation" ) { loop(); REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + //RemoteStartTx - idTag doesn't match. The tx will start anyway assuming some start trigger in the backend prevails over reservations in the backend implementation + rService->updateReservation(reservationId, connectorId, expiryDate, idTag, parentIdTag); + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "RemoteStartTransaction", + [] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument(JSON_OBJECT_SIZE(1))); + auto payload = doc->to(); + payload["idTag"] = "wrong idTag"; + return doc;}, + [] (JsonObject) { } //ignore conf + ))); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Charging ); + REQUIRE( connector->getTransaction()->getReservationId() != reservationId ); + + //reservation is reset after tx + endTransaction(); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //RemoteStartTx - idTag does match + rService->updateReservation(reservationId, connectorId, expiryDate, idTag, parentIdTag); + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "RemoteStartTransaction", + [idTag] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument(JSON_OBJECT_SIZE(1))); + auto payload = doc->to(); + payload["idTag"] = idTag; + return doc;}, + [] (JsonObject) { } //ignore conf + ))); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Charging ); + REQUIRE( connector->getTransaction()->getReservationId() == reservationId ); + + //reservation is reset after tx + endTransaction(); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + } + + SECTION("Tx on other connector") { + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //set reservation + int reservationId = 123; + unsigned int connectorIdResvd = 1; //reserve connector 1 + unsigned int connectorIdOther = 2; //start charging on other connector + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = nullptr; + + rService->updateReservation(reservationId, connectorIdResvd, expiryDate, idTag, parentIdTag); + REQUIRE( model.getConnector(connectorIdResvd)->getStatus() == ChargePointStatus::Reserved ); + + beginTransaction(idTag, connectorIdOther); + loop(); + REQUIRE( model.getConnector(connectorIdResvd)->getStatus() == ChargePointStatus::Available ); //reservation on first connector withdrawed + REQUIRE( model.getConnector(connectorIdOther)->getStatus() == ChargePointStatus::Charging ); + REQUIRE( getTransaction(connectorIdOther)->getReservationId() == reservationId ); //reservation transferred to other connector + + endTransaction(nullptr, nullptr, connectorIdOther); + loop(); + REQUIRE( model.getConnector(connectorIdOther)->getStatus() == ChargePointStatus::Available ); + } + + SECTION("parentIdTag") { + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //set reservation + int reservationId = 123; + unsigned int connectorId = 1; + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = "mParentIdTag"; + + rService->updateReservation(reservationId, connectorId, expiryDate, idTag, parentIdTag); + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + + bool checkProcessed = false; + getOcppContext()->getOperationRegistry().registerOperation("Authorize", + [parentIdTag, &checkProcessed] () { + return new Ocpp16::CustomOperation("Authorize", + [] (JsonObject) {}, //ignore req payload + [parentIdTag, &checkProcessed] () { + //create conf + checkProcessed = true; + auto doc = std::unique_ptr(new DynamicJsonDocument( + JSON_OBJECT_SIZE(1) + //payload root + JSON_OBJECT_SIZE(3))); //idTagInfo + auto payload = doc->to(); + payload["idTagInfo"]["parentIdTag"] = parentIdTag; + payload["idTagInfo"]["status"] = "Accepted"; + return doc;}); + }); + beginTransaction("other idTag"); + loop(); + REQUIRE( checkProcessed ); + REQUIRE( connector->getStatus() == ChargePointStatus::Charging ); + REQUIRE( connector->getTransaction()->getReservationId() == reservationId ); + + //reset tx + endTransaction(); + loop(); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + } + + SECTION("ConnectorZero") { + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //set reservation + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = nullptr; + + //if connector 0 is reserved, accept at most one further reservation + REQUIRE( rService->updateReservation(1000, 0, expiryDate, idTag, parentIdTag) ); + REQUIRE( rService->updateReservation(1001, 1, expiryDate, idTag, parentIdTag) ); + REQUIRE( !rService->updateReservation(1002, 2, expiryDate, idTag, parentIdTag) ); + REQUIRE( model.getConnector(2)->getStatus() == ChargePointStatus::Available ); + + //reset reservations + rService->getReservationById(1000)->clear(); + rService->getReservationById(1001)->clear(); + REQUIRE( model.getConnector(1)->getStatus() == ChargePointStatus::Available ); + + //if connector 0 is reserved, ensure that at least one physical connector remains available for the idTag of the reservation + REQUIRE( rService->updateReservation(1000, 0, expiryDate, idTag, parentIdTag) ); + + beginTransaction("other idTag", 1); + loop(); + REQUIRE( model.getConnector(1)->getStatus() == ChargePointStatus::Charging ); + + bool checkTxRejected = false; + setTxNotificationOutput([&checkTxRejected] (Transaction*, TxNotification txNotification) { + if (txNotification == TxNotification::ReservationConflict) { + checkTxRejected = true; + } + }, 2); + + beginTransaction("other idTag 2", 2); + loop(); + REQUIRE( checkTxRejected ); + REQUIRE( model.getConnector(2)->getStatus() == ChargePointStatus::Available ); + + + endTransaction(nullptr, nullptr, 1); + loop(); + } + + SECTION("Expiry date") { + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //set reservation + int reservationId = 123; + unsigned int connectorId = 1; + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = nullptr; + + rService->updateReservation(reservationId, connectorId, expiryDate, idTag, parentIdTag); + + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + + Timestamp expired = expiryDate + 1; + char expired_cstr [JSONDATE_LENGTH + 1]; + expired.toJsonString(expired_cstr, JSONDATE_LENGTH + 1); + model.getClock().setTime(expired_cstr); + + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + } + + SECTION("Reservation persistency") { + unsigned int connectorId = 1; + REQUIRE( getOcppContext()->getModel().getConnector(connectorId)->getStatus() == ChargePointStatus::Available ); + + //set reservation + int reservationId = 123; + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = "mParentIdTag"; + + getOcppContext()->getModel().getReservationService()->updateReservation(reservationId, connectorId, expiryDate, idTag, parentIdTag); + + REQUIRE( getOcppContext()->getModel().getConnector(connectorId)->getStatus() == ChargePointStatus::Reserved ); + + mocpp_deinitialize(); + + mocpp_initialize(loopback, ChargerCredentials("test-runner1234")); + getOcppContext()->getModel().getClock().setTime(BASE_TIME); + loop(); + + REQUIRE( getOcppContext()->getModel().getConnector(connectorId)->getStatus() == ChargePointStatus::Reserved ); + + auto reservation = getOcppContext()->getModel().getReservationService()->getReservationById(reservationId); + REQUIRE( reservation->getReservationId() == reservationId ); + REQUIRE( reservation->getConnectorId() == connectorId ); + REQUIRE( reservation->getExpiryDate() == expiryDate ); + REQUIRE( !strcmp(reservation->getIdTag(), idTag) ); + REQUIRE( !strcmp(reservation->getParentIdTag(), parentIdTag) ); + + reservation->clear(); + mocpp_deinitialize(); + + mocpp_initialize(loopback, ChargerCredentials("test-runner1234")); + getOcppContext()->getModel().getClock().setTime(BASE_TIME); + loop(); + + REQUIRE( getOcppContext()->getModel().getConnector(connectorId)->getStatus() == ChargePointStatus::Available ); + } + + SECTION("ReserveNow") { + + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //set reservation + int reservationId = 123; + unsigned int connectorId = 1; + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = nullptr; + + //simple reservation + bool checkProcessed = false; + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "ReserveNow", + [reservationId, connectorId, expiryDate, idTag, parentIdTag] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument( + JSON_OBJECT_SIZE(5) + + JSONDATE_LENGTH + 1)); + auto payload = doc->to(); + payload["connectorId"] = connectorId; + char expiryDate_cstr [JSONDATE_LENGTH + 1]; + expiryDate.toJsonString(expiryDate_cstr, JSONDATE_LENGTH + 1); + payload["expiryDate"] = expiryDate_cstr; + payload["idTag"] = idTag; + payload["parentIdTag"] = parentIdTag; + payload["reservationId"] = reservationId; + return doc;}, + [&checkProcessed] (JsonObject payload) { + //receive conf + checkProcessed = true; + + REQUIRE( !strcmp(payload["status"] | "_Undefined", "Accepted") ); + } + ))); + loop(); + REQUIRE( checkProcessed ); + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + + model.getReservationService()->getReservationById(reservationId)->clear(); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //reserve while charger is in Faulted state + const char *errorCode = "OtherError"; + addErrorCodeInput([&errorCode] () {return errorCode;}); + REQUIRE( connector->getStatus() == ChargePointStatus::Faulted ); + + checkProcessed = false; + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "ReserveNow", + [reservationId, connectorId, expiryDate, idTag, parentIdTag] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument( + JSON_OBJECT_SIZE(5) + + JSONDATE_LENGTH + 1)); + auto payload = doc->to(); + payload["connectorId"] = connectorId; + char expiryDate_cstr [JSONDATE_LENGTH + 1]; + expiryDate.toJsonString(expiryDate_cstr, JSONDATE_LENGTH + 1); + payload["expiryDate"] = expiryDate_cstr; + payload["idTag"] = idTag; + payload["parentIdTag"] = parentIdTag; + payload["reservationId"] = reservationId; + return doc;}, + [&checkProcessed] (JsonObject payload) { + //receive conf + checkProcessed = true; + + REQUIRE( !strcmp(payload["status"] | "_Undefined", "Faulted") ); + } + ))); + loop(); + REQUIRE( checkProcessed ); + REQUIRE( connector->getStatus() == ChargePointStatus::Faulted ); + + errorCode = nullptr; //reset error + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //reserve while connector is already occupied + setConnectorPluggedInput([] {return true;}); //plug EV + REQUIRE( connector->getStatus() == ChargePointStatus::Preparing ); + + checkProcessed = false; + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "ReserveNow", + [reservationId, connectorId, expiryDate, idTag, parentIdTag] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument( + JSON_OBJECT_SIZE(5) + + JSONDATE_LENGTH + 1)); + auto payload = doc->to(); + payload["connectorId"] = connectorId; + char expiryDate_cstr [JSONDATE_LENGTH + 1]; + expiryDate.toJsonString(expiryDate_cstr, JSONDATE_LENGTH + 1); + payload["expiryDate"] = expiryDate_cstr; + payload["idTag"] = idTag; + payload["parentIdTag"] = parentIdTag; + payload["reservationId"] = reservationId; + return doc;}, + [&checkProcessed] (JsonObject payload) { + //receive conf + checkProcessed = true; + + REQUIRE( !strcmp(payload["status"] | "_Undefined", "Occupied") ); + } + ))); + loop(); + REQUIRE( checkProcessed ); + REQUIRE( connector->getStatus() == ChargePointStatus::Preparing ); + + setConnectorPluggedInput(nullptr); //reset ConnectorPluggedInput + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //Rejected ReserveNow status not possible + + //reserve while connector is inoperative + connector->setAvailabilityVolatile(false); + REQUIRE( connector->getStatus() == ChargePointStatus::Unavailable ); + + checkProcessed = false; + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "ReserveNow", + [reservationId, connectorId, expiryDate, idTag, parentIdTag] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument( + JSON_OBJECT_SIZE(5) + + JSONDATE_LENGTH + 1)); + auto payload = doc->to(); + payload["connectorId"] = connectorId; + char expiryDate_cstr [JSONDATE_LENGTH + 1]; + expiryDate.toJsonString(expiryDate_cstr, JSONDATE_LENGTH + 1); + payload["expiryDate"] = expiryDate_cstr; + payload["idTag"] = idTag; + payload["parentIdTag"] = parentIdTag; + payload["reservationId"] = reservationId; + return doc;}, + [&checkProcessed] (JsonObject payload) { + //receive conf + checkProcessed = true; + + REQUIRE( !strcmp(payload["status"] | "_Undefined", "Unavailable") ); + } + ))); + loop(); + REQUIRE( checkProcessed ); + REQUIRE( connector->getStatus() == ChargePointStatus::Unavailable ); + + connector->setAvailabilityVolatile(true); //revert Unavailable status + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); } + + mocpp_deinitialize(); } From ac566e1ee88e5f6fd1b75247606a9355a5b3efe8 Mon Sep 17 00:00:00 2001 From: matth-x <63792403+matth-x@users.noreply.github.com> Date: Sun, 24 Sep 2023 18:12:29 +0200 Subject: [PATCH 5/5] add CancelReservation test --- tests/Reservation.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/Reservation.cpp b/tests/Reservation.cpp index fb698639..cf113f03 100644 --- a/tests/Reservation.cpp +++ b/tests/Reservation.cpp @@ -446,5 +446,61 @@ TEST_CASE( "Reservation" ) { REQUIRE( connector->getStatus() == ChargePointStatus::Available ); } + SECTION("CancelReservation") { + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //set reservation + int reservationId = 123; + unsigned int connectorId = 1; + Timestamp expiryDate = model.getClock().now() + 3600; //expires one hour in future + const char *idTag = "mIdTag"; + const char *parentIdTag = nullptr; + + rService->updateReservation(reservationId, connectorId, expiryDate, idTag, parentIdTag); + REQUIRE( connector->getStatus() == ChargePointStatus::Reserved ); + + //CancelReservation successfully + bool checkProcessed = false; + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "CancelReservation", + [reservationId, connectorId, expiryDate, idTag, parentIdTag] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument(JSON_OBJECT_SIZE(1))); + auto payload = doc->to(); + payload["reservationId"] = reservationId; + return doc;}, + [&checkProcessed] (JsonObject payload) { + //receive conf + checkProcessed = true; + + REQUIRE( !strcmp(payload["status"] | "_Undefined", "Accepted") ); + } + ))); + loop(); + REQUIRE( checkProcessed ); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + + //CancelReservation while no reservation exists + checkProcessed = false; + getOcppContext()->initiateRequest(makeRequest(new Ocpp16::CustomOperation( + "CancelReservation", + [reservationId, connectorId, expiryDate, idTag, parentIdTag] () { + //create req + auto doc = std::unique_ptr(new DynamicJsonDocument(JSON_OBJECT_SIZE(1))); + auto payload = doc->to(); + payload["reservationId"] = reservationId; + return doc;}, + [&checkProcessed] (JsonObject payload) { + //receive conf + checkProcessed = true; + + REQUIRE( !strcmp(payload["status"] | "_Undefined", "Rejected") ); + } + ))); + loop(); + REQUIRE( checkProcessed ); + REQUIRE( connector->getStatus() == ChargePointStatus::Available ); + } + mocpp_deinitialize(); }