From 9f8cf84cd9ef2deebd6675386743425b19769e99 Mon Sep 17 00:00:00 2001 From: Kai Hermann Date: Mon, 4 Dec 2023 18:43:31 +0100 Subject: [PATCH 01/18] OCPP2.0.1 add data transfer in line with OCPP1.6 implementation (#279) * OCPP2.0.1 add data transfer in line with OCPP1.6 implementation * Remove obsolete OCPP 2.0.1 data transfer callbacks These were not set in the code at all and there's a new mechanismn to do it * Fix typos * Return UnknownVendorId if OCPP201 data transfer callback not registered --------- Signed-off-by: Kai-Uwe Hermann --- include/ocpp/v201/charge_point.hpp | 13 ++++++------- lib/ocpp/v201/charge_point.cpp | 26 ++++++++++---------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/include/ocpp/v201/charge_point.hpp b/include/ocpp/v201/charge_point.hpp index 5c0bb0208..c0464e6a8 100644 --- a/include/ocpp/v201/charge_point.hpp +++ b/include/ocpp/v201/charge_point.hpp @@ -142,6 +142,10 @@ struct Callbacks { /// \brief Callback function that can be called when all connectors are unavailable std::optional> all_connectors_unavailable_callback; + + /// \brief Callback function that can be used to handle arbitrary data transfers for all vendorId and + /// messageId + std::optional> data_transfer_callback; }; /// \brief Combines ChangeAvailabilityRequest with persist flag for scheduled Availability changes @@ -164,11 +168,6 @@ class ChargePoint : ocpp::ChargingStationBase { std::map scheduled_change_availability_requests; - std::map& msg)>>> - data_transfer_callbacks; - std::mutex data_transfer_callbacks_mutex; - std::map> remote_start_id_per_evse; // timers @@ -642,8 +641,8 @@ class ChargePoint : ocpp::ChargingStationBase { /// \param messageId /// \param data /// \return DataTransferResponse contaning the result from CSMS - DataTransferResponse data_transfer_req(const CiString<255>& vendorId, const CiString<50>& messageId, - const std::string& data); + DataTransferResponse data_transfer_req(const CiString<255>& vendorId, const std::optional>& messageId, + const std::optional& data); }; } // namespace v201 diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index ea6b3bcae..3dd6da9af 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -2783,31 +2783,25 @@ void ChargePoint::handle_customer_information_req(Call call) { const auto msg = call.msg; DataTransferResponse response; - const auto vendor_id = msg.vendorId.get(); - const auto message_id = msg.messageId.value_or(CiString<50>()).get(); - { - std::lock_guard lock(data_transfer_callbacks_mutex); - if (this->data_transfer_callbacks.count(vendor_id) == 0) { - response.status = ocpp::v201::DataTransferStatusEnum::UnknownVendorId; - } else if (this->data_transfer_callbacks.count(vendor_id) and - this->data_transfer_callbacks[vendor_id].count(message_id) == 0) { - response.status = ocpp::v201::DataTransferStatusEnum::UnknownMessageId; - } else { - // there is a callback registered for this vendorId and messageId - response = this->data_transfer_callbacks[vendor_id][message_id](msg.data); - } + + if (this->callbacks.data_transfer_callback.has_value()) { + response = this->callbacks.data_transfer_callback.value()(call.msg); + } else { + response.status = DataTransferStatusEnum::UnknownVendorId; + EVLOG_warning << "Received a DataTransferRequest but no data transfer callback was registered"; } ocpp::CallResult call_result(response, call.uniqueId); this->send(call_result); } -DataTransferResponse ChargePoint::data_transfer_req(const CiString<255>& vendorId, const CiString<50>& messageId, - const std::string& data) { +DataTransferResponse ChargePoint::data_transfer_req(const CiString<255>& vendorId, + const std::optional>& messageId, + const std::optional& data) { DataTransferRequest req; req.vendorId = vendorId; req.messageId = messageId; - req.data.emplace(data); + req.data = data; DataTransferResponse response; ocpp::Call call(req, this->message_queue->createMessageId()); From 497b08c58405e0bcc0b4fb23843b063be1f83f99 Mon Sep 17 00:00:00 2001 From: Kai Hermann Date: Tue, 5 Dec 2023 11:02:29 +0100 Subject: [PATCH 02/18] Fix union annotation for python<3.10 (#289) Signed-off-by: Kai-Uwe Hermann --- config/v201/init_device_model_db.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/v201/init_device_model_db.py b/config/v201/init_device_model_db.py index 0afb75e1a..43852baa8 100644 --- a/config/v201/init_device_model_db.py +++ b/config/v201/init_device_model_db.py @@ -19,6 +19,8 @@ components and variables. """ +from __future__ import annotations + import argparse import json import sqlite3 From d0b429b96ba5303bf98792e12885184d60fd177c Mon Sep 17 00:00:00 2001 From: Fabian Klemm Date: Wed, 6 Dec 2023 17:30:44 +0100 Subject: [PATCH 03/18] EV369: Message queue buffering (#284) * MessageQueue: add config struct; add parameters for queueing; implement dropping non-transactional; tests - Extract MessageQueue config into dedicated struct - add parameters for limiting queue size and Queueing all message types - add check for message sizes and removal logic - restructure unit tests (common executuable) * add v201 config var for queue size threshold * add queue config parameters * remove temporary tests * add missing test resources * clang format * add missing constants; add missing headers Signed-off-by: Fabian Klemm --- config/v16/profile_schemas/Internal.json | 11 + .../standardized/InternalCtrlr.json | 17 + include/ocpp/common/database_handler_base.hpp | 6 +- include/ocpp/common/message_queue.hpp | 82 +++- .../ocpp/v16/charge_point_configuration.hpp | 3 + include/ocpp/v16/charge_point_impl.hpp | 1 + .../ocpp/v201/ctrlr_component_variables.hpp | 1 + lib/ocpp/common/types.cpp | 10 +- lib/ocpp/v16/charge_point_configuration.cpp | 16 + lib/ocpp/v16/charge_point_impl.cpp | 22 +- lib/ocpp/v201/charge_point.cpp | 10 +- lib/ocpp/v201/ctrlr_component_variables.cpp | 7 + tests/CMakeLists.txt | 19 +- tests/lib/ocpp/common/CMakeLists.txt | 2 + tests/lib/ocpp/common/test_message_queue.cpp | 365 ++++++++++++++++++ tests/lib/ocpp/v201/CMakeLists.txt | 28 +- .../v201/test_device_model_storage_sqlite.cpp | 6 +- .../ocpp/v201/test_ocsp_updater.cpp} | 0 .../unittest_device_model.db | Bin .../unittest_device_model_missing_required.db | Bin 20 files changed, 533 insertions(+), 73 deletions(-) create mode 100644 tests/lib/ocpp/common/CMakeLists.txt create mode 100644 tests/lib/ocpp/common/test_message_queue.cpp rename tests/{ocsp_updater_tests.cpp => lib/ocpp/v201/test_ocsp_updater.cpp} (100%) rename tests/{lib/ocpp/v201 => resources}/unittest_device_model.db (100%) rename tests/{lib/ocpp/v201 => resources}/unittest_device_model_missing_required.db (100%) diff --git a/config/v16/profile_schemas/Internal.json b/config/v16/profile_schemas/Internal.json index 352670bf1..892ea8ddd 100644 --- a/config/v16/profile_schemas/Internal.json +++ b/config/v16/profile_schemas/Internal.json @@ -222,6 +222,17 @@ "readOnly": false, "minimum": 0, "default": 60 + }, + "QueueAllMessages": { + "$comment": "If set to true, also non-transactional messages are queued in memory in case they cannot be sent immediately.", + "type": "boolean", + "readOnly": true + }, + "MessageQueueSizeThreshold": { + "$comment": "Threshold for the size of in-memory message queues used to buffer messages (and store e.g. while offline). If threshold is exceeded, messages will be dropped according to OCPP specification to avoid memory issues.", + "type": "integer", + "readOnly": true, + "minimum": 1 } }, "additionalProperties": false diff --git a/config/v201/component_schemas/standardized/InternalCtrlr.json b/config/v201/component_schemas/standardized/InternalCtrlr.json index c31d495b1..ae7981879 100644 --- a/config/v201/component_schemas/standardized/InternalCtrlr.json +++ b/config/v201/component_schemas/standardized/InternalCtrlr.json @@ -470,6 +470,23 @@ "description": "Seconds between two checks for client certificate expiration and potential renewal", "default": "43200", "type": "integer" + }, + "MessageQueueSizeThreshold": { + "variable_name": "MessageQueueSizeThreshold", + "characteristics": { + "minLimit": 1, + "supportsMonitoring": true, + "dataType": "integer" + }, + "attributes": [ + { + "type": "Actual", + "mutability": "ReadOnly" + } + ], + "description": "Threshold for the size of in-memory message queues used to buffer messages (and store e.g. while offline). If threshold is exceeded, messages will be dropped according to OCPP specification to avoid memory issues.", + "minimum": 1, + "type": "integer" } }, "required": [ diff --git a/include/ocpp/common/database_handler_base.hpp b/include/ocpp/common/database_handler_base.hpp index dd0da8640..8f69b34e0 100644 --- a/include/ocpp/common/database_handler_base.hpp +++ b/include/ocpp/common/database_handler_base.hpp @@ -37,17 +37,17 @@ class DatabaseHandlerBase { /// \brief Get transaction messages from transaction messages queue table. /// \return The transaction messages. - std::vector get_transaction_messages(); + virtual std::vector get_transaction_messages(); /// \brief Insert a new transaction message that needs to be sent to the CSMS. /// \param transaction_message The message to be stored. /// \return True on success. - bool insert_transaction_message(const DBTransactionMessage& transaction_message); + virtual bool insert_transaction_message(const DBTransactionMessage& transaction_message); /// \brief Remove a transaction message from the database. /// \param unique_id The unique id of the transaction message. /// \return True on success. - void remove_transaction_message(const std::string& unique_id); + virtual void remove_transaction_message(const std::string& unique_id); }; } // namespace ocpp::common diff --git a/include/ocpp/common/message_queue.hpp b/include/ocpp/common/message_queue.hpp index fe6f97547..eed83426c 100644 --- a/include/ocpp/common/message_queue.hpp +++ b/include/ocpp/common/message_queue.hpp @@ -29,6 +29,17 @@ namespace ocpp { const auto STANDARD_MESSAGE_TIMEOUT = std::chrono::seconds(30); +struct MessageQueueConfig { + int transaction_message_attempts; + int transaction_message_retry_interval; // seconds + + // threshold for the accumulated sizes of the queues; if the queues exceed this limit, + // messages are potentially dropped in accordance with OCPP 2.0.1. Specification (cf. QueueAllMessages parameter) + int queues_total_size_threshold; + + bool queue_all_messages; // cf. OCPP 2.0.1. "QueueAllMessages" in OCPPCommCtrlr +}; + /// \brief Contains a OCPP message in json form with additional information template struct EnhancedMessage { json message; ///< The OCPP message as json @@ -68,9 +79,9 @@ template struct ControlMessage { /// \brief contains a message queue that makes sure that OCPPs synchronicity requirements are met template class MessageQueue { private: + MessageQueueConfig config; std::shared_ptr database_handler; - int transaction_message_attempts; - int transaction_message_retry_interval; // seconds + std::thread worker_thread; /// message deque for transaction related messages std::deque>> transaction_message_queue; @@ -135,6 +146,7 @@ template class MessageQueue { std::lock_guard lk(this->message_mutex); this->normal_message_queue.push(message); this->new_message = true; + this->check_queue_sizes(); } this->cv.notify_all(); EVLOG_debug << "Notified message queue worker"; @@ -149,19 +161,46 @@ template class MessageQueue { message->uniqueId()}; this->database_handler->insert_transaction_message(db_message); this->new_message = true; + this->check_queue_sizes(); } this->cv.notify_all(); EVLOG_debug << "Notified message queue worker"; } + void check_queue_sizes() { + if (this->transaction_message_queue.size() + this->normal_message_queue.size() <= + this->config.queues_total_size_threshold) { + return; + } + EVLOG_warning << "Queue sizes exceed threshold (" << this->config.queues_total_size_threshold << ") with " + << this->transaction_message_queue.size() << " transaction and " + << this->normal_message_queue.size() << " normal messages in queue"; + + while (this->transaction_message_queue.size() + this->normal_message_queue.size() > + this->config.queues_total_size_threshold && + !this->normal_message_queue.empty()) { + this->drop_messages_from_normal_message_queue(); + } + } + + void drop_messages_from_normal_message_queue() { + // try to drop approx 10% of the allowed size (at least 1) + int number_of_dropped_messages = std::min((int)this->normal_message_queue.size(), + std::max(this->config.queues_total_size_threshold / 10, 1)); + + EVLOG_warning << "Dropping " << number_of_dropped_messages << " messages from normal message queue."; + + for (int i = 0; i < number_of_dropped_messages; i++) { + this->normal_message_queue.pop(); + } + } + public: /// \brief Creates a new MessageQueue object with the provided \p configuration and \p send_callback - MessageQueue(const std::function& send_callback, const int transaction_message_attempts, - const int transaction_message_retry_interval, const std::vector& external_notify, - std::shared_ptr database_handler) : - database_handler(database_handler), - transaction_message_attempts(transaction_message_attempts), - transaction_message_retry_interval(transaction_message_retry_interval), + MessageQueue(const std::function& send_callback, const MessageQueueConfig& config, + const std::vector& external_notify, std::shared_ptr database_handler) : + database_handler(std::move(database_handler)), + config(config), external_notify(external_notify), paused(true), running(true), @@ -272,8 +311,11 @@ template class MessageQueue { EVLOG_info << "The message in flight is transaction related and will be sent again once the " "connection can be established again."; if (this->in_flight->message.at(CALL_ACTION) == "TransactionEvent") { - this->in_flight->message.at(3)["offline"] = true; + this->in_flight->message.at(CALL_PAYLOAD)["offline"] = true; } + } else if (this->config.queue_all_messages) { + EVLOG_info << "The message in flight will be sent again once the connection can be " + "established again since QueueAllMessages is set to 'true'."; } else { EVLOG_info << "The message in flight is not transaction related and will be dropped"; if (queue_type == QueueType::Normal) { @@ -310,11 +352,9 @@ template class MessageQueue { }); } - MessageQueue(const std::function& send_callback, const int transaction_message_attempts, - const int transaction_message_retry_interval, + MessageQueue(const std::function& send_callback, const MessageQueueConfig& config, std::shared_ptr databaseHandler) : - MessageQueue(send_callback, transaction_message_attempts, transaction_message_retry_interval, {}, - databaseHandler) { + MessageQueue(send_callback, config, {}, databaseHandler) { } void get_transaction_messages_from_db() { @@ -352,7 +392,7 @@ template class MessageQueue { } else { // all other messages are allowed to "jump the queue" to improve user experience // TODO: decide if we only want to allow this for a subset of messages - if (!this->paused || message->messageType == M::BootNotification) { + if (!this->paused || this->config.queue_all_messages || message->messageType == M::BootNotification) { this->add_to_normal_message_queue(message); } } @@ -512,16 +552,16 @@ template class MessageQueue { EVLOG_warning << "Message timeout or CALLERROR for: " << this->in_flight->messageType << " (" << this->in_flight->uniqueId() << ")"; if (this->isTransactionMessage(this->in_flight)) { - if (this->in_flight->message_attempts < this->transaction_message_attempts) { + if (this->in_flight->message_attempts < this->config.transaction_message_attempts) { EVLOG_warning << "Message is transaction related and will therefore be sent again"; this->in_flight->message[MESSAGE_ID] = this->createMessageId(); - if (this->transaction_message_retry_interval > 0) { + if (this->config.transaction_message_retry_interval > 0) { // exponential backoff this->in_flight->timestamp = DateTime(this->in_flight->timestamp.to_time_point() + - std::chrono::seconds(this->transaction_message_retry_interval) * + std::chrono::seconds(this->config.transaction_message_retry_interval) * this->in_flight->message_attempts); - EVLOG_debug << "Retry interval > 0: " << this->transaction_message_retry_interval + EVLOG_debug << "Retry interval > 0: " << this->config.transaction_message_retry_interval << " attempting to retry message at: " << this->in_flight->timestamp; } else { // immediate retry @@ -530,7 +570,7 @@ template class MessageQueue { } EVLOG_warning << "Attempt: " << this->in_flight->message_attempts + 1 << "/" - << this->transaction_message_attempts << " will be sent at " + << this->config.transaction_message_attempts << " will be sent at " << this->in_flight->timestamp; this->transaction_message_queue.push_front(this->in_flight); @@ -626,12 +666,12 @@ template class MessageQueue { /// \brief Set transaction_message_attempts to given \p transaction_message_attempts void update_transaction_message_attempts(const int transaction_message_attempts) { - this->transaction_message_attempts = transaction_message_attempts; + this->config.transaction_message_attempts = transaction_message_attempts; } /// \brief Set transaction_message_retry_interval to given \p transaction_message_retry_interval in seconds void update_transaction_message_retry_interval(const int transaction_message_retry_interval) { - this->transaction_message_retry_interval = transaction_message_retry_interval; + this->config.transaction_message_retry_interval = transaction_message_retry_interval; } /// \brief Creates a unique message ID diff --git a/include/ocpp/v16/charge_point_configuration.hpp b/include/ocpp/v16/charge_point_configuration.hpp index 7f02976be..7ffc22a34 100644 --- a/include/ocpp/v16/charge_point_configuration.hpp +++ b/include/ocpp/v16/charge_point_configuration.hpp @@ -104,6 +104,9 @@ class ChargePointConfiguration { std::optional getHostName(); std::optional getHostNameKeyValue(); + std::optional getQueueAllMessages(); + std::optional getMessageQueueSizeThreshold(); + // Core Profile - optional std::optional getAllowOfflineTxForUnknownId(); void setAllowOfflineTxForUnknownId(bool enabled); diff --git a/include/ocpp/v16/charge_point_impl.hpp b/include/ocpp/v16/charge_point_impl.hpp index ea45d3597..d2499c82d 100644 --- a/include/ocpp/v16/charge_point_impl.hpp +++ b/include/ocpp/v16/charge_point_impl.hpp @@ -183,6 +183,7 @@ class ChargePointImpl : ocpp::ChargingStationBase { void init_websocket(); void init_state_machine(const std::map& connector_status_map); WebsocketConnectionOptions get_ws_connection_options(); + std::unique_ptr> create_message_queue(); void message_callback(const std::string& message); void handle_message(const EnhancedMessage& message); bool allowed_to_send_message(json::array_t message_type); diff --git a/include/ocpp/v201/ctrlr_component_variables.hpp b/include/ocpp/v201/ctrlr_component_variables.hpp index dc460e784..e30f33e6d 100644 --- a/include/ocpp/v201/ctrlr_component_variables.hpp +++ b/include/ocpp/v201/ctrlr_component_variables.hpp @@ -64,6 +64,7 @@ extern const ComponentVariable& V2GCertificateExpireCheckInitialDelaySeconds; extern const ComponentVariable& V2GCertificateExpireCheckIntervalSeconds; extern const ComponentVariable& ClientCertificateExpireCheckInitialDelaySeconds; extern const ComponentVariable& ClientCertificateExpireCheckIntervalSeconds; +extern const ComponentVariable& MessageQueueSizeThreshold; extern const ComponentVariable& AlignedDataCtrlrEnabled; extern const ComponentVariable& AlignedDataCtrlrAvailable; extern const RequiredComponentVariable& AlignedDataInterval; diff --git a/lib/ocpp/common/types.cpp b/lib/ocpp/common/types.cpp index 553c69c05..82d0b79c9 100644 --- a/lib/ocpp/common/types.cpp +++ b/lib/ocpp/common/types.cpp @@ -75,23 +75,23 @@ std::ostream& operator<<(std::ostream& os, const DateTimeImpl& dt) { } bool operator>(const DateTimeImpl& lhs, const DateTimeImpl& rhs) { - return lhs.to_rfc3339() > rhs.to_rfc3339(); + return lhs.timepoint > rhs.timepoint; } bool operator>=(const DateTimeImpl& lhs, const DateTimeImpl& rhs) { - return lhs.to_rfc3339() >= rhs.to_rfc3339(); + return lhs.timepoint >= rhs.timepoint; } bool operator<(const DateTimeImpl& lhs, const DateTimeImpl& rhs) { - return lhs.to_rfc3339() < rhs.to_rfc3339(); + return lhs.timepoint < rhs.timepoint; } bool operator<=(const DateTimeImpl& lhs, const DateTimeImpl& rhs) { - return lhs.to_rfc3339() <= rhs.to_rfc3339(); + return lhs.timepoint <= rhs.timepoint; } bool operator==(const DateTimeImpl& lhs, const DateTimeImpl& rhs) { - return lhs.to_rfc3339() == rhs.to_rfc3339(); + return lhs.timepoint == rhs.timepoint; } CallError::CallError() { diff --git a/lib/ocpp/v16/charge_point_configuration.cpp b/lib/ocpp/v16/charge_point_configuration.cpp index 8ddc9a83d..293a5891c 100644 --- a/lib/ocpp/v16/charge_point_configuration.cpp +++ b/lib/ocpp/v16/charge_point_configuration.cpp @@ -677,6 +677,22 @@ std::optional ChargePointConfiguration::getHostName() { return hostName_key; } +std::optional ChargePointConfiguration::getQueueAllMessages() { + std::optional queue_all_messages = std::nullopt; + if (this->config["Internal"].contains("QueueAllMessages")) { + queue_all_messages.emplace(this->config["Internal"]["QueueAllMessages"]); + } + return queue_all_messages; +} + +std::optional ChargePointConfiguration::getMessageQueueSizeThreshold() { + std::optional message_queue_size_threshold = std::nullopt; + if (this->config["Internal"].contains("MessageQueueSizeThreshold")) { + message_queue_size_threshold.emplace(this->config["Internal"]["MessageQueueSizeThreshold"]); + } + return message_queue_size_threshold; +} + // Core Profile - optional std::optional ChargePointConfiguration::getAllowOfflineTxForUnknownId() { std::optional unknown_offline_auth = std::nullopt; diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index 96bd62d36..104c96739 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -18,6 +18,7 @@ const auto V2G_CERTIFICATE_TIMER_INTERVAL = std::chrono::hours(12); const auto OCSP_REQUEST_TIMER_INTERVAL = std::chrono::hours(12); const auto INITIAL_CERTIFICATE_REQUESTS_DELAY = std::chrono::seconds(60); const auto WEBSOCKET_INIT_DELAY = std::chrono::seconds(2); +const auto DEFAULT_MESSAGE_QUEUE_SIZE_THRESHOLD = 2E5; ChargePointImpl::ChargePointImpl(const std::string& config, const fs::path& share_path, const fs::path& user_config_path, const fs::path& database_path, @@ -42,10 +43,7 @@ ChargePointImpl::ChargePointImpl(const std::string& config, const fs::path& shar this->database_handler->open_db_connection(this->configuration->getNumberOfConnectors()); this->transaction_handler = std::make_unique(this->configuration->getNumberOfConnectors()); this->external_notify = {v16::MessageType::StartTransactionResponse}; - this->message_queue = std::make_unique>( - [this](json message) -> bool { return this->websocket->send(message.dump()); }, - this->configuration->getTransactionMessageAttempts(), this->configuration->getTransactionMessageRetryInterval(), - this->external_notify, this->database_handler); + this->message_queue = this->create_message_queue(); auto log_formats = this->configuration->getLogMessagesFormat(); bool log_to_console = std::find(log_formats.begin(), log_formats.end(), "console") != log_formats.end(); bool detailed_log_to_console = @@ -144,6 +142,17 @@ ChargePointImpl::ChargePointImpl(const std::string& config, const fs::path& shar } } +std::unique_ptr> ChargePointImpl::create_message_queue() { + return std::make_unique>( + [this](json message) -> bool { return this->websocket->send(message.dump()); }, + MessageQueueConfig{ + this->configuration->getTransactionMessageAttempts(), + this->configuration->getTransactionMessageRetryInterval(), + this->configuration->getMessageQueueSizeThreshold().value_or(DEFAULT_MESSAGE_QUEUE_SIZE_THRESHOLD), + this->configuration->getQueueAllMessages().value_or(false)}, + this->external_notify, this->database_handler); +} + void ChargePointImpl::init_websocket() { auto connection_options = this->get_ws_connection_options(); @@ -792,10 +801,7 @@ bool ChargePointImpl::restart(const std::map& connector_ EVLOG_info << "Restarting OCPP Chargepoint"; this->database_handler->open_db_connection(this->configuration->getNumberOfConnectors()); // instantiating new message queue on restart - this->message_queue = std::make_unique>( - [this](json message) -> bool { return this->websocket->send(message.dump()); }, - this->configuration->getTransactionMessageAttempts(), - this->configuration->getTransactionMessageRetryInterval(), this->external_notify, this->database_handler); + this->message_queue = this->create_message_queue(); this->initialized = true; return this->start(connector_status_map); diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index 3dd6da9af..a3a28ce4c 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -20,6 +20,7 @@ namespace v201 { const auto DEFAULT_BOOT_NOTIFICATION_RETRY_INTERVAL = std::chrono::seconds(30); const auto WEBSOCKET_INIT_DELAY = std::chrono::seconds(2); +const auto DEFAULT_MESSAGE_QUEUE_SIZE_THRESHOLD = 2E5; bool Callbacks::all_callbacks_valid() const { return this->is_reset_allowed_callback != nullptr and this->reset_callback != nullptr and @@ -143,8 +144,13 @@ ChargePoint::ChargePoint(const std::map& evse_connector_struct this->message_queue = std::make_unique>( [this](json message) -> bool { return this->websocket->send(message.dump()); }, - this->device_model->get_value(ControllerComponentVariables::MessageAttempts), - this->device_model->get_value(ControllerComponentVariables::MessageAttemptInterval), + MessageQueueConfig{ + this->device_model->get_value(ControllerComponentVariables::MessageAttempts), + this->device_model->get_value(ControllerComponentVariables::MessageAttemptInterval), + this->device_model->get_optional_value(ControllerComponentVariables::MessageQueueSizeThreshold) + .value_or(DEFAULT_MESSAGE_QUEUE_SIZE_THRESHOLD), + this->device_model->get_optional_value(ControllerComponentVariables::QueueAllMessages) + .value_or(false)}, this->database_handler); } diff --git a/lib/ocpp/v201/ctrlr_component_variables.cpp b/lib/ocpp/v201/ctrlr_component_variables.cpp index 4ef6c8c24..a73db0e68 100644 --- a/lib/ocpp/v201/ctrlr_component_variables.cpp +++ b/lib/ocpp/v201/ctrlr_component_variables.cpp @@ -235,6 +235,13 @@ const ComponentVariable& ClientCertificateExpireCheckIntervalSeconds = { "ClientCertificateExpireCheckIntervalSeconds", }), }; +const ComponentVariable& MessageQueueSizeThreshold = { + ControllerComponents::InternalCtrlr, + std::nullopt, + std::optional({ + "MessageQueueSizeThreshold", + }), +}; const ComponentVariable& AlignedDataCtrlrEnabled = { ControllerComponents::AlignedDataCtrlr, std::nullopt, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3fcce5e29..74ef3e953 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -14,25 +14,30 @@ target_link_libraries(database_tests PRIVATE pthread ) -add_subdirectory(lib/ocpp/v201) add_test(database_tests database_tests) +add_executable(libocpp_unit_tests) -add_executable(ocsp_updater_tests ocsp_updater_tests.cpp comparators.cpp) +add_custom_command(TARGET libocpp_unit_tests POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/resources/unittest_device_model.db ${CMAKE_CURRENT_BINARY_DIR}/resources/unittest_device_model.db + COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/resources/unittest_device_model_missing_required.db ${CMAKE_CURRENT_BINARY_DIR}/resources/unittest_device_model_missing_required.db +) -target_include_directories(ocsp_updater_tests PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ${GTEST_INCLUDE_DIRS}) +add_test(libocpp_unit_tests libocpp_unit_tests) -find_package(GTest REQUIRED) - -target_link_libraries(ocsp_updater_tests PRIVATE +target_include_directories(libocpp_unit_tests PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ${GTEST_INCLUDE_DIRS}) +target_link_libraries(libocpp_unit_tests PRIVATE ocpp GTest::gmock_main ${GTEST_LIBRARIES} ${GTEST_MAIN_LIBRARIES} ) +target_sources(libocpp_unit_tests PRIVATE + comparators.cpp) -add_test(ocsp_updater_tests ocsp_updater_tests) +add_subdirectory(lib/ocpp/v201) +add_subdirectory(lib/ocpp/common) add_executable(utils_tests utils_tests.cpp) diff --git a/tests/lib/ocpp/common/CMakeLists.txt b/tests/lib/ocpp/common/CMakeLists.txt new file mode 100644 index 000000000..837540742 --- /dev/null +++ b/tests/lib/ocpp/common/CMakeLists.txt @@ -0,0 +1,2 @@ + +target_sources(libocpp_unit_tests PRIVATE test_message_queue.cpp) diff --git a/tests/lib/ocpp/common/test_message_queue.cpp b/tests/lib/ocpp/common/test_message_queue.cpp new file mode 100644 index 000000000..8327ffb3b --- /dev/null +++ b/tests/lib/ocpp/common/test_message_queue.cpp @@ -0,0 +1,365 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2020 - 2023 Pionix GmbH and Contributors to EVerest + +#include +#include +#include +#include + +namespace ocpp { + +using json = nlohmann::json; + +enum class TestMessageType { + TRANSACTIONAL, + TRANSACTIONAL_RESPONSE, + NON_TRANSACTIONAL, + NON_TRANSACTIONAL_RESPONSE, + InternalError, + BootNotification +}; + +static std::string to_string(TestMessageType m) { + switch (m) { + case TestMessageType::TRANSACTIONAL: + return "transactional"; + case TestMessageType::TRANSACTIONAL_RESPONSE: + return "transactionalResponse"; + case TestMessageType::NON_TRANSACTIONAL: + return "non_transactional"; + case TestMessageType::NON_TRANSACTIONAL_RESPONSE: + return "non_transactionalResponse"; + case TestMessageType::InternalError: + return "internal_error"; + case TestMessageType::BootNotification: + return "boot_notification"; + } + throw std::out_of_range("unknown TestMessageType"); +}; + +static TestMessageType to_test_message_type(const std::string& s) { + if (s == "transactional") { + return TestMessageType::TRANSACTIONAL; + } + if (s == "transactionalResponse") { + return TestMessageType::TRANSACTIONAL_RESPONSE; + } + if (s == "non_transactional") { + return TestMessageType::NON_TRANSACTIONAL; + } + if (s == "non_transactionalResponse") { + return TestMessageType::NON_TRANSACTIONAL_RESPONSE; + } + if (s == "internal_error") { + return TestMessageType::InternalError; + } + if (s == "boot_notification") { + return TestMessageType::BootNotification; + } + throw std::out_of_range("unknown string for TestMessageType"); +}; + +struct TestRequest : Message { + TestMessageType type = TestMessageType::NON_TRANSACTIONAL; + std::optional data; + std::string get_type() const { + return to_string(type); + }; +}; + +void to_json(json& j, const TestRequest& k) { + j = json{}; + if (k.data) { + j["data"] = k.data.value(); + } +} + +void from_json(const json& j, TestRequest& k) { + if (j.contains("data")) { + k.data.emplace(j.at("data")); + } +} + +template <> std::string MessageQueue::messagetype_to_string(TestMessageType m) { + return to_string(m); +} + +template <> TestMessageType MessageQueue::string_to_messagetype(const std::string& s) { + return to_test_message_type(s); +} + +template <> ControlMessage::ControlMessage(json message) { + this->message = message.get(); + EVLOG_info << this->message; + this->messageType = to_test_message_type(this->message[2]); + this->message_attempts = 0; +} + +std::ostream& operator<<(std::ostream& os, const TestMessageType& message_type) { + os << to_string(message_type); + return os; +}; + +template <> +bool MessageQueue::isTransactionMessage( + const std::shared_ptr> message) const { + if (message == nullptr) { + return false; + } + return message->messageType == TestMessageType::TRANSACTIONAL; +} + +class DatabaseHandlerBaseMock : public common::DatabaseHandlerBase { +public: + MOCK_METHOD(std::vector, get_transaction_messages, (), (override)); + MOCK_METHOD(bool, insert_transaction_message, (const common::DBTransactionMessage&), (override)); + MOCK_METHOD(void, remove_transaction_message, (const std::string&), (override)); +}; + +class MessageQueueTest : public ::testing::Test { + int internal_message_count{0}; + int call_count{0}; + +protected: + MessageQueueConfig config; + std::shared_ptr db; + std::mutex call_marker_mutex; + std::condition_variable call_marker_cond_var; + testing::MockFunction send_callback_mock; + Everest::SteadyTimer reception_timer; + std::unique_ptr> message_queue; + + int get_call_count() { + std::lock_guard lock(call_marker_mutex); + return call_count; + } + + template auto MarkAndReturn(R value, bool respond = false) { + return testing::Invoke([this, value, respond](json::array_t s) -> R { + if (respond) { + reception_timer.timeout( + [this, s]() { + this->message_queue->receive(json{3, s[1], ""}.dump()); + }, + std::chrono::milliseconds(0)); + } + std::lock_guard lock(call_marker_mutex); + this->call_count++; + this->call_marker_cond_var.notify_one(); + return value; + }); + }; + + void wait_for_calls(int expected_calls = 1) { + std::unique_lock lock(call_marker_mutex); + EXPECT_TRUE(call_marker_cond_var.wait_for( + lock, std::chrono::seconds(3), [this, expected_calls] { return this->call_count >= expected_calls; })); + } + + std::string push_message_call(const TestMessageType& message_type) { + std::stringstream stream; + stream << "test_call_" << internal_message_count; + std::string unique_identifier = stream.str(); + internal_message_count++; + return push_message_call(message_type, unique_identifier); + } + + std::string push_message_call(const TestMessageType& message_type, const std::string& identifier) { + Call call; + call.msg.type = message_type; + call.msg.data = identifier; + call.uniqueId = identifier; + message_queue->push(call); + return identifier; + } + + void init_message_queue() { + if (message_queue) { + message_queue->stop(); + } + message_queue = std::make_unique>(send_callback_mock.AsStdFunction(), config, db); + message_queue->resume(); + } + + void SetUp() override { + call_count = 0; + config = MessageQueueConfig{1, 1, 2, false}; + db = std::make_shared(); + init_message_queue(); + } + + void TearDown() override { + message_queue->stop(); + }; +}; + +// \brief Test sending a transactional message +TEST_F(MessageQueueTest, test_transactional_message_is_sent) { + + EXPECT_CALL(send_callback_mock, Call(json{2, "0", "transactional", json{{"data", "test_data"}}})) + .WillOnce(MarkAndReturn(true)); + EXPECT_CALL(*db, insert_transaction_message(testing::_)).WillOnce(testing::Return(true)); + + Call call; + call.msg.type = TestMessageType::TRANSACTIONAL; + call.msg.data = "test_data"; + call.uniqueId = "0"; + message_queue->push(call); + + wait_for_calls(); +} + +// \brief Test sending a non-transactional message +TEST_F(MessageQueueTest, test_non_transactional_message_is_sent) { + + EXPECT_CALL(send_callback_mock, Call(json{2, "0", "non_transactional", json{{"data", "test_data"}}})) + .WillOnce(MarkAndReturn(true)); + + Call call; + call.msg.type = TestMessageType::NON_TRANSACTIONAL; + call.msg.data = "test_data"; + call.uniqueId = "0"; + message_queue->push(call); + + wait_for_calls(); +} + +// \brief Test transactional messages that are sent while being offline are sent afterwards +TEST_F(MessageQueueTest, test_queuing_up_of_transactional_messages) { + + int message_count = config.queues_total_size_threshold + 3; + testing::Sequence s; + + // Setup: reject the first call ("offline"); after that, accept any call + EXPECT_CALL(send_callback_mock, Call(testing::_)).InSequence(s).WillOnce(MarkAndReturn(false)); + EXPECT_CALL(send_callback_mock, Call(testing::_)) + .Times(message_count) + .InSequence(s) + .WillRepeatedly(MarkAndReturn(true, true)); + EXPECT_CALL(*db, insert_transaction_message(testing::_)).WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*db, remove_transaction_message(testing::_)).Times(message_count).WillRepeatedly(testing::Return()); + + // Act: + // push first call and wait for callback; then push all other calls and resume queue + push_message_call(TestMessageType::TRANSACTIONAL); + wait_for_calls(1); + + for (int i = 1; i < message_count; i++) { + push_message_call(TestMessageType::TRANSACTIONAL); + } + + message_queue->resume(); + + // expect one repeated and all other calls been made + wait_for_calls(message_count + 1); +} + +// \brief Test that - with default setting - non-transactional messages that are not sent afterwards +TEST_F(MessageQueueTest, test_non_queuing_up_of_non_transactional_messages) { + + int message_count = config.queues_total_size_threshold + 3; + testing::Sequence s; + + // Setup: reject the first call ("offline"); after that, accept any call + EXPECT_CALL(send_callback_mock, Call(testing::_)).InSequence(s).WillOnce(MarkAndReturn(false)); + EXPECT_CALL(send_callback_mock, Call(testing::_)).InSequence(s).WillRepeatedly(MarkAndReturn(true, true)); + + // Act: + // push first call and wait for callback; then push all other calls and resume queue + push_message_call(TestMessageType::NON_TRANSACTIONAL); + wait_for_calls(1); + + for (int i = 1; i < message_count; i++) { + push_message_call(TestMessageType::NON_TRANSACTIONAL); + } + + message_queue->resume(); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + // expect calls not repeated + EXPECT_EQ(1, get_call_count()); +} + +// \brief Test that if queue_all_messages is set to true, non-transactional messages that are sent when online again +TEST_F(MessageQueueTest, test_queuing_up_of_non_transactional_messages) { + config.queue_all_messages = true; + init_message_queue(); + + int message_count = config.queues_total_size_threshold; + testing::Sequence s; + + // Setup: reject the first call ("offline"); after that, accept any call + EXPECT_CALL(send_callback_mock, Call(testing::_)).InSequence(s).WillOnce(MarkAndReturn(false)); + EXPECT_CALL(send_callback_mock, Call(testing::_)).InSequence(s).WillRepeatedly(MarkAndReturn(true, true)); + + // Act: + // push first call and wait for callback; then push all other calls and resume queue + push_message_call(TestMessageType::NON_TRANSACTIONAL); + wait_for_calls(1); + + for (int i = 1; i < message_count; i++) { + push_message_call(TestMessageType::NON_TRANSACTIONAL); + } + + message_queue->resume(); + + // expect calls _are_ repeated + wait_for_calls(message_count + 1); +} + +// \brief Test that if the max size threshold is exceeded, the non-transactional messages are dropped +// Sends both non-transactions and transactional messages while on pause, expects a certain amount of non-transactional +// to be dropped. +TEST_F(MessageQueueTest, test_clean_up_non_transactional_queue) { + + const int sent_transactional_messages = 10; + const int sent_non_transactional_messages = 15; + config.queues_total_size_threshold = + 20; // expect two messages to be dropped each round (3x), end up with 15-6=9 non-transactional remaining + config.queue_all_messages = true; + const int expected_skipped_transactional_messages = 6; + init_message_queue(); + + EXPECT_CALL(*db, insert_transaction_message(testing::_)) + .Times(sent_transactional_messages) + .WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*db, remove_transaction_message(testing::_)) + .Times(sent_transactional_messages) + .WillRepeatedly(testing::Return()); + + // go offline + message_queue->pause(); + + testing::Sequence s; + for (int i = 0; i < sent_non_transactional_messages; i++) { + auto msg_id = push_message_call(TestMessageType::NON_TRANSACTIONAL); + + if (i >= expected_skipped_transactional_messages) { + EXPECT_CALL(send_callback_mock, + Call(json{2, msg_id, to_string(TestMessageType::NON_TRANSACTIONAL), json{{"data", msg_id}}})) + .InSequence(s) + .WillOnce(MarkAndReturn(true, true)); + } + } + for (int i = 0; i < sent_transactional_messages; i++) { + auto msg_id = push_message_call(TestMessageType::TRANSACTIONAL); + EXPECT_CALL(send_callback_mock, + Call(json{2, msg_id, to_string(TestMessageType::TRANSACTIONAL), json{{"data", msg_id}}})) + .InSequence(s) + .WillOnce(MarkAndReturn(true, true)); + } + + // go online again + message_queue->resume(); + + // expect calls _are_ repeated + wait_for_calls(sent_transactional_messages + sent_non_transactional_messages - + expected_skipped_transactional_messages); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + // assert no further calls + EXPECT_EQ(sent_transactional_messages + sent_non_transactional_messages - expected_skipped_transactional_messages, + get_call_count()); +} + +} // namespace ocpp diff --git a/tests/lib/ocpp/v201/CMakeLists.txt b/tests/lib/ocpp/v201/CMakeLists.txt index da1d286e3..9531f018f 100644 --- a/tests/lib/ocpp/v201/CMakeLists.txt +++ b/tests/lib/ocpp/v201/CMakeLists.txt @@ -1,26 +1,4 @@ -add_executable(device_model_storage_sqlite_tests test_device_model_storage_sqlite.cpp - "${CMAKE_CURRENT_SOURCE_DIR}/../../../../lib/ocpp/v201/device_model_storage_sqlite.cpp") -target_include_directories(device_model_storage_sqlite_tests PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} "${CMAKE_CURRENT_SOURCE_DIR}/../../../../include" ${GTEST_INCLUDE_DIRS}) - -find_package(GTest REQUIRED) - -target_link_libraries(device_model_storage_sqlite_tests PRIVATE - GTest::gmock_main - everest::timer - everest::evse_security # since it contains the everest-logging dependency; to be improved - SQLite::SQLite3 - nlohmann_json::nlohmann_json - ${GTEST_LIBRARIES} - ${GTEST_MAIN_LIBRARIES} -) - -add_custom_command(TARGET device_model_storage_sqlite_tests POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/unittest_device_model.db ${CMAKE_CURRENT_BINARY_DIR}/unittest_device_model.db - COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/unittest_device_model_missing_required.db ${CMAKE_CURRENT_BINARY_DIR}/unittest_device_model_missing_required.db -) - -add_test(device_model_storage_sqlite_tests device_model_storage_sqlite_tests) -set_tests_properties(device_model_storage_sqlite_tests PROPERTIES ENVIRONMENT "DATADIR=blabla") -set_tests_properties(device_model_storage_sqlite_tests PROPERTIES - WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests) \ No newline at end of file +target_sources(libocpp_unit_tests PRIVATE + test_device_model_storage_sqlite.cpp + test_ocsp_updater.cpp) diff --git a/tests/lib/ocpp/v201/test_device_model_storage_sqlite.cpp b/tests/lib/ocpp/v201/test_device_model_storage_sqlite.cpp index 84b0d765b..2197fc600 100644 --- a/tests/lib/ocpp/v201/test_device_model_storage_sqlite.cpp +++ b/tests/lib/ocpp/v201/test_device_model_storage_sqlite.cpp @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2020 - 2023 Pionix GmbH and Contributors to EVerest #include #include @@ -8,8 +10,8 @@ namespace v201 { class DeviceModelStorageSQLiteTest : public ::testing::Test { protected: - const std::string DEVICE_MODEL_DATABASE = "./unittest_device_model.db"; - const std::string INVALID_DEVICE_MODEL_DATABASE = "./unittest_device_model_missing_required.db"; + const std::string DEVICE_MODEL_DATABASE = "./resources/unittest_device_model.db"; + const std::string INVALID_DEVICE_MODEL_DATABASE = "./resources/unittest_device_model_missing_required.db"; }; /// \brief Tests check_integrity does not raise error for valid database diff --git a/tests/ocsp_updater_tests.cpp b/tests/lib/ocpp/v201/test_ocsp_updater.cpp similarity index 100% rename from tests/ocsp_updater_tests.cpp rename to tests/lib/ocpp/v201/test_ocsp_updater.cpp diff --git a/tests/lib/ocpp/v201/unittest_device_model.db b/tests/resources/unittest_device_model.db similarity index 100% rename from tests/lib/ocpp/v201/unittest_device_model.db rename to tests/resources/unittest_device_model.db diff --git a/tests/lib/ocpp/v201/unittest_device_model_missing_required.db b/tests/resources/unittest_device_model_missing_required.db similarity index 100% rename from tests/lib/ocpp/v201/unittest_device_model_missing_required.db rename to tests/resources/unittest_device_model_missing_required.db From bb11e09e1d71da5526e81f2906c5f3f8eaa24c22 Mon Sep 17 00:00:00 2001 From: Fabian Klemm Date: Wed, 6 Dec 2023 17:46:16 +0100 Subject: [PATCH 04/18] EV369b: drop update transactional messages if necessary (#285) * MessageQueue: add config struct; add parameters for queueing; implement dropping non-transactional; tests - Extract MessageQueue config into dedicated struct - add parameters for limiting queue size and Queueing all message types - add check for message sizes and removal logic - restructure unit tests (common executuable) Signed-off-by: Fabian Klemm * add v201 config var for queue size threshold Signed-off-by: Fabian Klemm * add queue config parameters Signed-off-by: Fabian Klemm * remove temporary tests Signed-off-by: Fabian Klemm * add missing test resources Signed-off-by: Fabian Klemm * clang format Signed-off-by: Fabian Klemm * EV269 Part II: drop transaction update events if necessary Signed-off-by: Fabian Klemm * Update tests/lib/ocpp/common/test_message_queue.cpp Co-authored-by: Kai Hermann Signed-off-by: corneliusclaussen <62659547+corneliusclaussen@users.noreply.github.com> --------- Signed-off-by: Fabian Klemm Signed-off-by: corneliusclaussen <62659547+corneliusclaussen@users.noreply.github.com> Co-authored-by: corneliusclaussen <62659547+corneliusclaussen@users.noreply.github.com> Co-authored-by: Kai Hermann --- include/ocpp/common/message_queue.hpp | 62 +++++- lib/ocpp/common/message_queue.cpp | 39 ++-- tests/lib/ocpp/common/test_message_queue.cpp | 204 ++++++++++++++++++- 3 files changed, 268 insertions(+), 37 deletions(-) diff --git a/include/ocpp/common/message_queue.hpp b/include/ocpp/common/message_queue.hpp index eed83426c..be1faf65c 100644 --- a/include/ocpp/common/message_queue.hpp +++ b/include/ocpp/common/message_queue.hpp @@ -71,9 +71,15 @@ template struct ControlMessage { /// \brief Provides the unique message ID stored in the message /// \returns the unique ID of the contained message - MessageId uniqueId() const { + [[nodiscard]] MessageId uniqueId() const { return this->message[MESSAGE_ID]; } + + /// \brief Determine whether message is considered as transaction-related. + bool isTransactionMessage() const; + + /// \brief True for transactional messages containing updates (measurements) for a transaction + bool isTransactionUpdateMessage() const; }; /// \brief contains a message queue that makes sure that OCPPs synchronicity requirements are met @@ -138,8 +144,6 @@ template class MessageQueue { return false; } - bool isTransactionMessage(const std::shared_ptr> message) const; - void add_to_normal_message_queue(std::shared_ptr> message) { EVLOG_debug << "Adding message to normal message queue"; { @@ -181,6 +185,11 @@ template class MessageQueue { !this->normal_message_queue.empty()) { this->drop_messages_from_normal_message_queue(); } + + while (this->transaction_message_queue.size() + this->normal_message_queue.size() > + this->config.queues_total_size_threshold && + this->drop_update_messages_from_transactional_message_queue()) { + } } void drop_messages_from_normal_message_queue() { @@ -195,6 +204,43 @@ template class MessageQueue { } } + /** + * Heuristically drops every second update messag. + * Drops every first, third, ... update message in between two non-update message; disregards transaction + * ids etc! + * Cf. OCPP 2.0.1. specification 2.1.9 "QueueAllMessages" + */ + bool drop_update_messages_from_transactional_message_queue() { + int drop_count = 0; + std::deque>> temporary_swap_queue; + bool remove_next_update_message = true; + while (!transaction_message_queue.empty()) { + auto element = transaction_message_queue.front(); + transaction_message_queue.pop_front(); + // drop every second update message (except last one) + if (remove_next_update_message && element->isTransactionUpdateMessage() && + transaction_message_queue.size() > 1) { + EVLOG_debug << "Drop transactional message " << element->uniqueId(); + database_handler->remove_transaction_message(element->uniqueId()); + drop_count++; + remove_next_update_message = false; + } else { + remove_next_update_message = true; + temporary_swap_queue.push_back(element); + } + } + + std::swap(transaction_message_queue, temporary_swap_queue); + + if (drop_count > 0) { + EVLOG_warning << "Dropped " << drop_count << " transactional update messages to reduce queue size."; + return true; + } else { + EVLOG_warning << "There are no further transaction update messages to drop!"; + return false; + } + } + public: /// \brief Creates a new MessageQueue object with the provided \p configuration and \p send_callback MessageQueue(const std::function& send_callback, const MessageQueueConfig& config, @@ -307,7 +353,7 @@ template class MessageQueue { if (!this->send_callback(this->in_flight->message)) { this->paused = true; EVLOG_error << "Could not send message, this is most likely because the charge point is offline."; - if (this->isTransactionMessage(this->in_flight)) { + if (this->in_flight && this->in_flight->isTransactionMessage()) { EVLOG_info << "The message in flight is transaction related and will be sent again once the " "connection can be established again."; if (this->in_flight->message.at(CALL_ACTION) == "TransactionEvent") { @@ -382,7 +428,7 @@ template class MessageQueue { } auto message = std::make_shared>(call); - if (this->isTransactionMessage(message)) { + if (message->isTransactionMessage()) { // according to the spec the "transaction related messages" StartTransaction, StopTransaction and // MeterValues have to be delivered in chronological order @@ -440,7 +486,7 @@ template class MessageQueue { auto enhanced_message = EnhancedMessage(); enhanced_message.offline = true; message->promise.set_value(enhanced_message); - } else if (this->isTransactionMessage(message)) { + } else if (message->isTransactionMessage()) { // according to the spec the "transaction related messages" StartTransaction, StopTransaction and // MeterValues have to be delivered in chronological order this->add_to_transaction_message_queue(message); @@ -528,7 +574,7 @@ template class MessageQueue { this->in_flight->message.at(CALL_ACTION).template get() + std::string("Response")); this->in_flight->promise.set_value(enhanced_message); - if (isTransactionMessage(this->in_flight)) { + if (this->in_flight->isTransactionMessage()) { // We only remove the message as soon as a response is received. Otherwise we might miss a message if // the charging station just boots after sending, but before receiving the result. this->database_handler->remove_transaction_message(this->in_flight->uniqueId()); @@ -551,7 +597,7 @@ template class MessageQueue { std::lock_guard lk(this->message_mutex); EVLOG_warning << "Message timeout or CALLERROR for: " << this->in_flight->messageType << " (" << this->in_flight->uniqueId() << ")"; - if (this->isTransactionMessage(this->in_flight)) { + if (this->in_flight->isTransactionMessage()) { if (this->in_flight->message_attempts < this->config.transaction_message_attempts) { EVLOG_warning << "Message is transaction related and will therefore be sent again"; this->in_flight->message[MESSAGE_ID] = this->createMessageId(); diff --git a/lib/ocpp/common/message_queue.cpp b/lib/ocpp/common/message_queue.cpp index 1a76b916d..9844a8a66 100644 --- a/lib/ocpp/common/message_queue.cpp +++ b/lib/ocpp/common/message_queue.cpp @@ -13,36 +13,37 @@ template <> ControlMessage::ControlMessage(json message) { this->message_attempts = 0; } +template <> bool ControlMessage::isTransactionMessage() const { + if (this->messageType == v16::MessageType::StartTransaction || + this->messageType == v16::MessageType::StopTransaction || this->messageType == v16::MessageType::MeterValues || + this->messageType == v16::MessageType::SecurityEventNotification) { + return true; + } + return false; +} + +template <> bool ControlMessage::isTransactionUpdateMessage() const { + return (this->messageType == v16::MessageType::MeterValues); +} + template <> ControlMessage::ControlMessage(json message) { this->message = message.get(); this->messageType = v201::conversions::string_to_messagetype(message.at(CALL_ACTION)); this->message_attempts = 0; } -template <> -bool MessageQueue::isTransactionMessage( - const std::shared_ptr> message) const { - if (message == nullptr) { - return false; - } - if (message->messageType == v16::MessageType::StartTransaction || - message->messageType == v16::MessageType::StopTransaction || - message->messageType == v16::MessageType::MeterValues || - message->messageType == v16::MessageType::SecurityEventNotification) { +template <> bool ControlMessage::isTransactionMessage() const { + if (this->messageType == v201::MessageType::TransactionEvent || + this->messageType == v201::MessageType::SecurityEventNotification) { // A04.FR.02 return true; } return false; } -template <> -bool MessageQueue::isTransactionMessage( - const std::shared_ptr> message) const { - if (message == nullptr) { - return false; - } - if (message->messageType == v201::MessageType::TransactionEvent || - message->messageType == v201::MessageType::SecurityEventNotification) { // A04.FR.02 - return true; +template <> bool ControlMessage::isTransactionUpdateMessage() const { + if (this->messageType == v201::MessageType::TransactionEvent) { + return v201::TransactionEventRequest{this->message.at(CALL_PAYLOAD)}.eventType == + v201::TransactionEventEnum::Updated; } return false; } diff --git a/tests/lib/ocpp/common/test_message_queue.cpp b/tests/lib/ocpp/common/test_message_queue.cpp index 8327ffb3b..0a3f8d7cf 100644 --- a/tests/lib/ocpp/common/test_message_queue.cpp +++ b/tests/lib/ocpp/common/test_message_queue.cpp @@ -1,18 +1,28 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2020 - 2023 Pionix GmbH and Contributors to EVerest - #include #include #include #include +#include +#include +#include +#include +#include namespace ocpp { using json = nlohmann::json; +/************************************************************************************************ + * Test Message Types + */ + enum class TestMessageType { TRANSACTIONAL, TRANSACTIONAL_RESPONSE, + TRANSACTIONAL_UPDATE, + TRANSACTIONAL_UPDATE_RESPONSE, NON_TRANSACTIONAL, NON_TRANSACTIONAL_RESPONSE, InternalError, @@ -25,6 +35,10 @@ static std::string to_string(TestMessageType m) { return "transactional"; case TestMessageType::TRANSACTIONAL_RESPONSE: return "transactionalResponse"; + case TestMessageType::TRANSACTIONAL_UPDATE: + return "transactional_update"; + case TestMessageType::TRANSACTIONAL_UPDATE_RESPONSE: + return "transactional_updateResponse"; case TestMessageType::NON_TRANSACTIONAL: return "non_transactional"; case TestMessageType::NON_TRANSACTIONAL_RESPONSE: @@ -44,6 +58,12 @@ static TestMessageType to_test_message_type(const std::string& s) { if (s == "transactionalResponse") { return TestMessageType::TRANSACTIONAL_RESPONSE; } + if (s == "transactional_update") { + return TestMessageType::TRANSACTIONAL_UPDATE; + } + if (s == "transactional_updateResponse") { + return TestMessageType::TRANSACTIONAL_UPDATE_RESPONSE; + } if (s == "non_transactional") { return TestMessageType::NON_TRANSACTIONAL; } @@ -100,15 +120,102 @@ std::ostream& operator<<(std::ostream& os, const TestMessageType& message_type) return os; }; -template <> -bool MessageQueue::isTransactionMessage( - const std::shared_ptr> message) const { - if (message == nullptr) { - return false; - } - return message->messageType == TestMessageType::TRANSACTIONAL; +template <> bool ControlMessage::isTransactionMessage() const { + return this->messageType == TestMessageType::TRANSACTIONAL || + this->messageType == TestMessageType::TRANSACTIONAL_UPDATE; +} + +template <> bool ControlMessage::isTransactionUpdateMessage() const { + return this->messageType == TestMessageType::TRANSACTIONAL_UPDATE; +} + +/************************************************************************************************ + * ControlMessage + * + * Test implementations of ControlMessage template + */ +class ControlMessageV16Test : public ::testing::Test { + +protected: +}; + +TEST_F(ControlMessageV16Test, test_is_transactional) { + + EXPECT_TRUE( + (ControlMessage{Call{v16::StartTransactionRequest{}, "0"}}) + .isTransactionMessage()); + EXPECT_TRUE( + (ControlMessage{Call{v16::StopTransactionRequest{}, "0"}}) + .isTransactionMessage()); + EXPECT_TRUE((ControlMessage{ + Call{v16::SecurityEventNotificationRequest{}, "0"}}) + .isTransactionMessage()); + EXPECT_TRUE((ControlMessage{Call{v16::MeterValuesRequest{}, "0"}}) + .isTransactionMessage()); + + EXPECT_TRUE(!(ControlMessage{Call{v16::AuthorizeRequest{}, "0"}}) + .isTransactionMessage()); +} + +TEST_F(ControlMessageV16Test, test_is_transactional_update) { + + EXPECT_TRUE( + !(ControlMessage{Call{v16::StartTransactionRequest{}, "0"}}) + .isTransactionUpdateMessage()); + EXPECT_TRUE( + !(ControlMessage{Call{v16::StopTransactionRequest{}, "0"}}) + .isTransactionUpdateMessage()); + EXPECT_TRUE(!(ControlMessage{ + Call{v16::SecurityEventNotificationRequest{}, "0"}}) + .isTransactionUpdateMessage()); + EXPECT_TRUE((ControlMessage{Call{v16::MeterValuesRequest{}, "0"}}) + .isTransactionUpdateMessage()); + + EXPECT_TRUE(!(ControlMessage{Call{v16::AuthorizeRequest{}, "0"}}) + .isTransactionUpdateMessage()); +} + +class ControlMessageV201Test : public ::testing::Test { + +protected: +}; + +TEST_F(ControlMessageV201Test, test_is_transactional) { + + EXPECT_TRUE( + (ControlMessage{Call{v201::TransactionEventRequest{}, "0"}}) + .isTransactionMessage()); + + EXPECT_TRUE(!(ControlMessage{Call{v201::AuthorizeRequest{}, "0"}}) + .isTransactionMessage()); +} + +TEST_F(ControlMessageV201Test, test_is_transactional_update) { + + v201::TransactionEventRequest transaction_event_request{}; + transaction_event_request.eventType = v201::TransactionEventEnum::Updated; + + EXPECT_TRUE((ControlMessage{Call{transaction_event_request, "0"}}) + .isTransactionUpdateMessage()); + + transaction_event_request.eventType = v201::TransactionEventEnum::Started; + EXPECT_TRUE( + !(ControlMessage{Call{transaction_event_request, "0"}}) + .isTransactionUpdateMessage()); + + transaction_event_request.eventType = v201::TransactionEventEnum::Ended; + EXPECT_TRUE( + !(ControlMessage{Call{transaction_event_request, "0"}}) + .isTransactionUpdateMessage()); + + EXPECT_TRUE(!(ControlMessage{Call{v201::AuthorizeRequest{}, "0"}}) + .isTransactionUpdateMessage()); } +/************************************************************************************************ + * MessageQueueTest + */ + class DatabaseHandlerBaseMock : public common::DatabaseHandlerBase { public: MOCK_METHOD(std::vector, get_transaction_messages, (), (override)); @@ -121,7 +228,7 @@ class MessageQueueTest : public ::testing::Test { int call_count{0}; protected: - MessageQueueConfig config; + MessageQueueConfig config{}; std::shared_ptr db; std::mutex call_marker_mutex; std::condition_variable call_marker_cond_var; @@ -135,7 +242,7 @@ class MessageQueueTest : public ::testing::Test { } template auto MarkAndReturn(R value, bool respond = false) { - return testing::Invoke([this, value, respond](json::array_t s) -> R { + return testing::Invoke([this, value, respond](const json::array_t& s) -> R { if (respond) { reception_timer.timeout( [this, s]() { @@ -362,4 +469,81 @@ TEST_F(MessageQueueTest, test_clean_up_non_transactional_queue) { get_call_count()); } +// \brief Test that if the max size threshold is exceeded, intermediate transactional (update) messages are dropped +// Sends both non-transactions and transactional messages while on pause, expects all non-transactional, and any except +// every forth transactional to be dropped +TEST_F(MessageQueueTest, test_clean_up_transactional_queue) { + + const int sent_non_transactional_messages = 10; + const std::vector transaction_update_messages{0, 4, 6, + 2}; // meaning there are 4 transactions, each with a "start" and + // "stop" message and the provided number of updates; + // in total 4*2 + 4+ 6 +2 = 20 messages + config.queues_total_size_threshold = 13; + /** + * Message IDs: + * non-transactional: 0 - 9 + * Transaction I: 10 - 11 + * Transaction II: 12 - 17 + * Transaction III: 18 - 25 + * Transaction IV: 26 - 29 + * + * Expected dropping behavior + * - adding msg 13-22 -> each drop 1 non-transactional (floored 10% of queue thresholds) + * - adding msg 23 (update of third transaction) -> drop 4 messages with ids 13,15,19,21 + * - adding msg 27 (update of fourth transaction) -> drop 3 message with ids 14,20,23 + */ + const std::set expected_dropped_transaction_messages = { + "test_call_13", "test_call_15", "test_call_19", "test_call_21", "test_call_14", "test_call_20", "test_call_23", + }; + const int expected_sent_messages = 13; + config.queue_all_messages = true; + init_message_queue(); + + EXPECT_CALL(*db, insert_transaction_message(testing::_)).Times(20).WillRepeatedly(testing::Return(true)); + EXPECT_CALL(*db, remove_transaction_message(testing::_)).Times(20).WillRepeatedly(testing::Return()); + + // go offline + message_queue->pause(); + + // Send messages / set up expected calls + testing::Sequence s; + for (int i = 0; i < sent_non_transactional_messages; i++) { + push_message_call(TestMessageType::NON_TRANSACTIONAL); + } + + for (int update_messages : transaction_update_messages) { + // transaction "start" + auto start_msg_id = push_message_call(TestMessageType::TRANSACTIONAL); + EXPECT_CALL(send_callback_mock, Call(json{2, start_msg_id, to_string(TestMessageType::TRANSACTIONAL), + json{{"data", start_msg_id}}})) + .InSequence(s) + .WillOnce(MarkAndReturn(true, true)); + + for (int i = 0; i < update_messages; i++) { + auto update_msg_id = push_message_call(TestMessageType::TRANSACTIONAL_UPDATE); + + if (!expected_dropped_transaction_messages.count(update_msg_id)) { + EXPECT_CALL(send_callback_mock, + Call(json{2, update_msg_id, to_string(TestMessageType::TRANSACTIONAL_UPDATE), + json{{"data", update_msg_id}}})) + .InSequence(s) + .WillOnce(MarkAndReturn(true, true)); + } + } + + auto stop_msg_id = push_message_call(TestMessageType::TRANSACTIONAL); + // transaction "end" + EXPECT_CALL(send_callback_mock, + Call(json{2, stop_msg_id, to_string(TestMessageType::TRANSACTIONAL), json{{"data", stop_msg_id}}})) + .InSequence(s) + .WillOnce(MarkAndReturn(true, true)); + } + + // Resume & verify + message_queue->resume(); + + wait_for_calls(expected_sent_messages); +} + } // namespace ocpp From 775f02b084cee6fc1ba9b0909d5f9fbdf1163883 Mon Sep 17 00:00:00 2001 From: Kai Hermann Date: Thu, 7 Dec 2023 13:44:56 +0100 Subject: [PATCH 05/18] OCPP2.0.1: Ensure individual firmware status notifications are sent once (#277) Make firmware_status_id an optional and check its value in on_firmware_status_notification Send InstallScheduled if firmware is to be installed during active transactions Don't send same firmware update status notification with request_id = -1 multiple times as well Signed-off-by: Kai-Uwe Hermann Co-authored-by: valentin-dimov <143783161+valentin-dimov@users.noreply.github.com> --- include/ocpp/v201/charge_point.hpp | 4 +++- lib/ocpp/v201/charge_point.cpp | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/include/ocpp/v201/charge_point.hpp b/include/ocpp/v201/charge_point.hpp index c0464e6a8..27d78e623 100644 --- a/include/ocpp/v201/charge_point.hpp +++ b/include/ocpp/v201/charge_point.hpp @@ -186,7 +186,9 @@ class ChargePoint : ocpp::ChargingStationBase { RegistrationStatusEnum registration_status; OperationalStatusEnum operational_state; FirmwareStatusEnum firmware_status; - int32_t firmware_status_id; + // The request ID in the last firmware update status received + std::optional firmware_status_id; + // The last firmware status which will be posted before the firmware is installed. FirmwareStatusEnum firmware_status_before_installing = FirmwareStatusEnum::SignatureVerified; UploadLogStatusEnum upload_log_status; int32_t upload_log_status_id; diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index a3a28ce4c..6776570ed 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -200,6 +200,13 @@ void ChargePoint::disconnect_websocket(websocketpp::close::status::value code) { void ChargePoint::on_firmware_update_status_notification(int32_t request_id, const FirmwareStatusEnum& firmware_update_status) { + if (this->firmware_status == firmware_update_status) { + if (request_id == -1 or + this->firmware_status_id.has_value() and this->firmware_status_id.value() == request_id) { + // already sent, do not send again + return; + } + } FirmwareStatusNotificationRequest req; req.status = firmware_update_status; // Firmware status and id are stored for future trigger message request. @@ -228,6 +235,17 @@ void ChargePoint::on_firmware_update_status_notification(int32_t request_id, } if (this->firmware_status_before_installing == req.status) { + // FIXME(Kai): This is a temporary workaround, because the EVerest System module does not keep track of + // transactions and can't inquire about their status from the OCPP modules. If the firmware status is expected + // to become "Installing", but we still have a transaction running, the update will wait for the transaction to + // finish, and so we send an "InstallScheduled" status. This is necessary for OCTT TC_L_15_CS to pass. + const auto transaction_active = this->any_transaction_active(std::nullopt); + if (transaction_active) { + this->firmware_status = FirmwareStatusEnum::InstallScheduled; + req.status = firmware_status; + ocpp::Call call(req, this->message_queue->createMessageId()); + this->send_async(call); + } this->change_all_connectors_to_unavailable_for_firmware_update(); } } From 01c3a6bddd685340150a183cecbd45d6a1bc0958 Mon Sep 17 00:00:00 2001 From: Rinze van der Wal Date: Thu, 30 Nov 2023 13:55:57 +0000 Subject: [PATCH 06/18] Use sqlite_close_v2 Signed-off-by: Rinze van der Wal --- lib/ocpp/v201/database_handler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ocpp/v201/database_handler.cpp b/lib/ocpp/v201/database_handler.cpp index 729f89bae..03816116b 100644 --- a/lib/ocpp/v201/database_handler.cpp +++ b/lib/ocpp/v201/database_handler.cpp @@ -129,7 +129,7 @@ void DatabaseHandler::open_connection() { } void DatabaseHandler::close_connection() { - if (sqlite3_close(this->db) == SQLITE_OK) { + if (sqlite3_close_v2(this->db) == SQLITE_OK) { EVLOG_debug << "Successfully closed database: " << this->database_file_path; } else { EVLOG_error << "Error closing database file: " << sqlite3_errmsg(this->db); From daa4950103cca09213d25356df1e083c96ac650c Mon Sep 17 00:00:00 2001 From: valentin-dimov <143783161+valentin-dimov@users.noreply.github.com> Date: Fri, 8 Dec 2023 10:08:45 +0100 Subject: [PATCH 07/18] Feat: Optionally delay the resumption of the message queue after reconnect (#283) Signed-off-by: Valentin Dimov --- include/ocpp/common/message_queue.hpp | 62 ++++++++++++++++++-------- include/ocpp/v16/charge_point.hpp | 4 ++ include/ocpp/v16/charge_point_impl.hpp | 9 ++++ include/ocpp/v201/charge_point.hpp | 8 ++++ lib/ocpp/v16/charge_point.cpp | 4 ++ lib/ocpp/v16/charge_point_impl.cpp | 6 +-- lib/ocpp/v201/charge_point.cpp | 2 +- 7 files changed, 73 insertions(+), 22 deletions(-) diff --git a/include/ocpp/common/message_queue.hpp b/include/ocpp/common/message_queue.hpp index be1faf65c..bafcdf4ac 100644 --- a/include/ocpp/common/message_queue.hpp +++ b/include/ocpp/common/message_queue.hpp @@ -94,8 +94,8 @@ template class MessageQueue { /// message queue for non-transaction related messages std::queue>> normal_message_queue; std::shared_ptr> in_flight; - std::mutex message_mutex; - std::condition_variable cv; + std::recursive_mutex message_mutex; + std::condition_variable_any cv; std::function send_callback; std::vector external_notify; bool paused; @@ -107,6 +107,12 @@ template class MessageQueue { Everest::SteadyTimer in_flight_timeout_timer; Everest::SteadyTimer notify_queue_timer; + // This timer schedules the resumption of the message queue + Everest::SteadyTimer resume_timer; + // Counts the number of pause()/resume() calls. + // Used by the resume timer callback to abort itself in case the timer triggered before it could be cancelled. + u_int64_t pause_resume_ctr = 0; + // key is the message id of the stop transaction and the value is the transaction id // this map is used for StopTransaction.req that have been put on the message queue without having received a // transactionId from the backend (e.g. when offline) it is used to replace the transactionId in the @@ -147,7 +153,7 @@ template class MessageQueue { void add_to_normal_message_queue(std::shared_ptr> message) { EVLOG_debug << "Adding message to normal message queue"; { - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); this->normal_message_queue.push(message); this->new_message = true; this->check_queue_sizes(); @@ -158,7 +164,7 @@ template class MessageQueue { void add_to_transaction_message_queue(std::shared_ptr> message) { EVLOG_debug << "Adding message to transaction message queue"; { - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); this->transaction_message_queue.push_back(message); ocpp::common::DBTransactionMessage db_message{message->message, messagetype_to_string(message->messageType), message->message_attempts, message->timestamp, @@ -241,6 +247,16 @@ template class MessageQueue { } } + // The public resume() delegates the actual resumption to this method + void resume_now(u_int64_t expected_pause_resume_ctr) { + std::lock_guard lk(this->message_mutex); + if (this->pause_resume_ctr == expected_pause_resume_ctr) { + this->paused = false; + this->cv.notify_one(); + EVLOG_debug << "resume() notified message queue"; + } + } + public: /// \brief Creates a new MessageQueue object with the provided \p configuration and \p send_callback MessageQueue(const std::function& send_callback, const MessageQueueConfig& config, @@ -260,8 +276,9 @@ template class MessageQueue { while (this->running) { EVLOG_debug << "Waiting for a message from the message queue"; - std::unique_lock lk(this->message_mutex); + std::unique_lock lk(this->message_mutex); using namespace std::chrono_literals; + // It's safe to wait on the cv here because we're guaranteed to only lock this->message_mutex once this->cv.wait(lk, [this]() { return !this->running || (!this->paused && this->new_message && this->in_flight == nullptr); }); @@ -532,7 +549,7 @@ template class MessageQueue { // we need to remove Call messages from in_flight if we receive a CallResult OR a CallError // TODO(kai): we need to do some error handling in the CallError case - std::unique_lock lk(this->message_mutex); + std::unique_lock lk(this->message_mutex); if (this->in_flight == nullptr) { EVLOG_error << "Received a CALLRESULT OR CALLERROR without a message in flight, this should not happen"; @@ -594,7 +611,7 @@ template class MessageQueue { /// \brief Handles a message timeout or a CALLERROR. \p enhanced_message_opt is set only in case of CALLERROR void handle_timeout_or_callerror(const std::optional>& enhanced_message_opt) { - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); EVLOG_warning << "Message timeout or CALLERROR for: " << this->in_flight->messageType << " (" << this->in_flight->uniqueId() << ")"; if (this->in_flight->isTransactionMessage()) { @@ -664,28 +681,37 @@ template class MessageQueue { /// \brief Pauses the message queue void pause() { EVLOG_debug << "pause()"; - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); + this->pause_resume_ctr++; + this->resume_timer.stop(); this->paused = true; this->cv.notify_one(); EVLOG_debug << "pause() notified message queue"; } /// \brief Resumes the message queue - void resume() { - EVLOG_debug << "resume()"; - std::lock_guard lk(this->message_mutex); - this->paused = false; - this->cv.notify_one(); - EVLOG_debug << "resume() notified message queue"; + void resume(std::chrono::seconds delay_on_reconnect) { + EVLOG_debug << "resume() called"; + std::lock_guard lk(this->message_mutex); + this->pause_resume_ctr++; + // Do not delay if this is the first call to resume(), i.e. this is the initial connection + if (this->pause_resume_ctr > 1 && delay_on_reconnect > std::chrono::seconds(0)) { + EVLOG_debug << "Delaying message queue resume by " << delay_on_reconnect.count() << " seconds"; + u_int64_t expected_pause_resume_ctr = this->pause_resume_ctr; + this->resume_timer.timeout( + [this, expected_pause_resume_ctr] { this->resume_now(expected_pause_resume_ctr); }, delay_on_reconnect); + } else { + this->resume_now(this->pause_resume_ctr); + } } bool is_transaction_message_queue_empty() { - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); return this->transaction_message_queue.empty(); } bool contains_transaction_messages(const CiString<36> transaction_id) { - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); for (const auto control_message : this->transaction_message_queue) { if (control_message->messageType == v201::MessageType::TransactionEvent) { v201::TransactionEventRequest req = control_message->message.at(CALL_PAYLOAD); @@ -698,7 +724,7 @@ template class MessageQueue { } bool contains_stop_transaction_message(const int32_t transaction_id) { - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); for (const auto control_message : this->transaction_message_queue) { if (control_message->messageType == v16::MessageType::StopTransaction) { v16::StopTransactionRequest req = control_message->message.at(CALL_PAYLOAD); @@ -753,7 +779,7 @@ template class MessageQueue { // replace transaction id in meter values if start_transaction_message_id is present in map // this is necessary when the chargepoint queued MeterValue.req for a transaction with unknown transaction_id - std::lock_guard lk(this->message_mutex); + std::lock_guard lk(this->message_mutex); if (this->start_transaction_mid_meter_values_mid_map.count(start_transaction_message_id)) { for (auto it = this->transaction_message_queue.begin(); it != transaction_message_queue.end(); ++it) { for (const auto& meter_value_message_id : diff --git a/include/ocpp/v16/charge_point.hpp b/include/ocpp/v16/charge_point.hpp index 0088c39ca..bbc17cc23 100644 --- a/include/ocpp/v16/charge_point.hpp +++ b/include/ocpp/v16/charge_point.hpp @@ -458,6 +458,10 @@ class ChargePoint { /// \param callback void register_is_token_reserved_for_connector_callback( const std::function& callback); + + /// \brief Delay draining the message queue after reconnecting, so the CSMS can perform post-reconnect checks first + /// \param delay The delay period (seconds) + void set_message_queue_resume_delay(std::chrono::seconds delay); }; } // namespace v16 diff --git a/include/ocpp/v16/charge_point_impl.hpp b/include/ocpp/v16/charge_point_impl.hpp index d2499c82d..e1d7ac960 100644 --- a/include/ocpp/v16/charge_point_impl.hpp +++ b/include/ocpp/v16/charge_point_impl.hpp @@ -138,6 +138,9 @@ class ChargePointImpl : ocpp::ChargingStationBase { FirmwareStatusEnumType signed_firmware_status; int signed_firmware_status_request_id; + /// \brief optional delay to resumption of message queue after reconnecting to the CSMS + std::chrono::seconds message_queue_resume_delay = std::chrono::seconds(0); + // callbacks std::function enable_evse_callback; std::function disable_evse_callback; @@ -729,6 +732,12 @@ class ChargePointImpl : ocpp::ChargingStationBase { /// \param value /// \return Indicates the result of the operation ConfigurationStatus set_custom_configuration_key(CiString<50> key, CiString<500> value); + + /// \brief Delay draining the message queue after reconnecting, so the CSMS can perform post-reconnect checks first + /// \param delay The delay period (seconds) + void set_message_queue_resume_delay(std::chrono::seconds delay) { + this->message_queue_resume_delay = delay; + } }; } // namespace v16 diff --git a/include/ocpp/v201/charge_point.hpp b/include/ocpp/v201/charge_point.hpp index 27d78e623..af9108dd8 100644 --- a/include/ocpp/v201/charge_point.hpp +++ b/include/ocpp/v201/charge_point.hpp @@ -228,6 +228,8 @@ class ChargePoint : ocpp::ChargingStationBase { /// \brief Handler for automatic or explicit OCSP cache updates OcspUpdater ocsp_updater; + /// \brief optional delay to resumption of message queue after reconnecting to the CSMS + std::chrono::seconds message_queue_resume_delay = std::chrono::seconds(0); bool send(CallError call_error); @@ -645,6 +647,12 @@ class ChargePoint : ocpp::ChargingStationBase { /// \return DataTransferResponse contaning the result from CSMS DataTransferResponse data_transfer_req(const CiString<255>& vendorId, const std::optional>& messageId, const std::optional& data); + + /// \brief Delay draining the message queue after reconnecting, so the CSMS can perform post-reconnect checks first + /// \param delay The delay period (seconds) + void set_message_queue_resume_delay(std::chrono::seconds delay) { + this->message_queue_resume_delay = delay; + } }; } // namespace v201 diff --git a/lib/ocpp/v16/charge_point.cpp b/lib/ocpp/v16/charge_point.cpp index d3c247ad1..a5f646912 100644 --- a/lib/ocpp/v16/charge_point.cpp +++ b/lib/ocpp/v16/charge_point.cpp @@ -298,5 +298,9 @@ void ChargePoint::register_is_token_reserved_for_connector_callback( this->charge_point->register_is_token_reserved_for_connector_callback(callback); } +void ChargePoint::set_message_queue_resume_delay(std::chrono::seconds delay) { + this->charge_point->set_message_queue_resume_delay(delay); +} + } // namespace v16 } // namespace ocpp diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index 104c96739..914423e11 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -162,14 +162,14 @@ void ChargePointImpl::init_websocket() { if (this->connection_state_changed_callback != nullptr) { this->connection_state_changed_callback(true); } - this->message_queue->resume(); // - this->connected_callback(); // + this->message_queue->resume(this->message_queue_resume_delay); + this->connected_callback(); }); this->websocket->register_disconnected_callback([this]() { if (this->connection_state_changed_callback != nullptr) { this->connection_state_changed_callback(false); } - this->message_queue->pause(); // + this->message_queue->pause(); if (this->ocsp_request_timer != nullptr) { this->ocsp_request_timer->stop(); } diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index 6776570ed..a368733c4 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -672,7 +672,7 @@ void ChargePoint::init_websocket() { this->websocket = std::make_unique(connection_options, this->evse_security, this->logging); this->websocket->register_connected_callback([this](const int security_profile) { - this->message_queue->resume(); + this->message_queue->resume(this->message_queue_resume_delay); const auto& security_profile_cv = ControllerComponentVariables::SecurityProfile; if (security_profile_cv.variable.has_value()) { From 32bc5c4a185a4d22c830976df141c5f091564b6e Mon Sep 17 00:00:00 2001 From: Fabian Klemm Date: Mon, 11 Dec 2023 13:44:07 +0100 Subject: [PATCH 08/18] EV377: split notify report requests (#295) * add notify report splitter Signed-off-by: Fabian Klemm * add MaxMessageSize parameter to schema Signed-off-by: Fabian Klemm * clean up Signed-off-by: Fabian Klemm * correct schema type + add missing value lists Signed-off-by: Fabian Klemm * add missing type conversion Signed-off-by: Fabian Klemm * format Signed-off-by: Fabian Klemm * surpress unused warnings Signed-off-by: Fabian Klemm * another attempt to supress unused warning Signed-off-by: Fabian Klemm * attempt II Signed-off-by: Fabian Klemm * another attempt Signed-off-by: Fabian Klemm * minor review fixes Signed-off-by: Fabian Klemm * Fix message queue unit tests Signed-off-by: Valentin Dimov --------- Signed-off-by: Fabian Klemm Signed-off-by: Valentin Dimov Co-authored-by: Valentin Dimov --- .../custom/Connector_1_1.json | 3 +- .../custom/Connector_2_1.json | 3 +- .../v201/component_schemas/custom/EVSE_1.json | 3 +- .../v201/component_schemas/custom/EVSE_2.json | 3 +- .../standardized/ChargingStation.json | 3 +- .../standardized/InternalCtrlr.json | 20 ++- include/ocpp/common/message_queue.hpp | 21 ++- include/ocpp/v201/charge_point.hpp | 2 +- .../ocpp/v201/ctrlr_component_variables.hpp | 1 + include/ocpp/v201/device_model.hpp | 5 + .../v201/notify_report_requests_splitter.hpp | 56 ++++++ lib/CMakeLists.txt | 15 +- lib/ocpp/common/message_queue.cpp | 4 +- lib/ocpp/v201/charge_point.cpp | 27 ++- lib/ocpp/v201/ctrlr_component_variables.cpp | 7 + .../v201/notify_report_requests_splitter.cpp | 113 ++++++++++++ tests/lib/ocpp/common/test_message_queue.cpp | 14 +- tests/lib/ocpp/v201/CMakeLists.txt | 1 + .../test_notify_report_requests_splitter.cpp | 161 ++++++++++++++++++ 19 files changed, 426 insertions(+), 36 deletions(-) create mode 100644 include/ocpp/v201/notify_report_requests_splitter.hpp create mode 100644 lib/ocpp/v201/notify_report_requests_splitter.cpp create mode 100644 tests/lib/ocpp/v201/test_notify_report_requests_splitter.cpp diff --git a/config/v201/component_schemas/custom/Connector_1_1.json b/config/v201/component_schemas/custom/Connector_1_1.json index b3d4d11df..a74d09f1d 100644 --- a/config/v201/component_schemas/custom/Connector_1_1.json +++ b/config/v201/component_schemas/custom/Connector_1_1.json @@ -10,7 +10,8 @@ "variable_name": "AvailabilityState", "characteristics": { "supportsMonitoring": true, - "dataType": "OptionList" + "dataType": "OptionList", + "valuesList": "Available,Occupied,Reserved,Unavailable,Faulted" }, "attributes": [ { diff --git a/config/v201/component_schemas/custom/Connector_2_1.json b/config/v201/component_schemas/custom/Connector_2_1.json index 3acf5752e..504930300 100644 --- a/config/v201/component_schemas/custom/Connector_2_1.json +++ b/config/v201/component_schemas/custom/Connector_2_1.json @@ -10,7 +10,8 @@ "variable_name": "AvailabilityState", "characteristics": { "supportsMonitoring": true, - "dataType": "OptionList" + "dataType": "OptionList", + "valuesList": "Available,Occupied,Reserved,Unavailable,Faulted" }, "attributes": [ { diff --git a/config/v201/component_schemas/custom/EVSE_1.json b/config/v201/component_schemas/custom/EVSE_1.json index 83c4dcc46..caa5c4dc3 100644 --- a/config/v201/component_schemas/custom/EVSE_1.json +++ b/config/v201/component_schemas/custom/EVSE_1.json @@ -24,7 +24,8 @@ "variable_name": "AvailabilityState", "characteristics": { "supportsMonitoring": true, - "dataType": "OptionList" + "dataType": "OptionList", + "valuesList": "Available,Occupied,Reserved,Unavailable,Faulted" }, "attributes": [ { diff --git a/config/v201/component_schemas/custom/EVSE_2.json b/config/v201/component_schemas/custom/EVSE_2.json index 07ec4df42..c6ea0e921 100644 --- a/config/v201/component_schemas/custom/EVSE_2.json +++ b/config/v201/component_schemas/custom/EVSE_2.json @@ -24,7 +24,8 @@ "variable_name": "AvailabilityState", "characteristics": { "supportsMonitoring": true, - "dataType": "OptionList" + "dataType": "OptionList", + "valuesList": "Available,Occupied,Reserved,Unavailable,Faulted" }, "attributes": [ { diff --git a/config/v201/component_schemas/standardized/ChargingStation.json b/config/v201/component_schemas/standardized/ChargingStation.json index f98c4689b..2b1ace9a1 100644 --- a/config/v201/component_schemas/standardized/ChargingStation.json +++ b/config/v201/component_schemas/standardized/ChargingStation.json @@ -24,7 +24,8 @@ "variable_name": "AvailabilityState", "characteristics": { "supportsMonitoring": true, - "dataType": "OptionList" + "dataType": "OptionList", + "valuesList": "Available,Occupied,Reserved,Unavailable,Faulted" }, "attributes": [ { diff --git a/config/v201/component_schemas/standardized/InternalCtrlr.json b/config/v201/component_schemas/standardized/InternalCtrlr.json index ae7981879..a3d08d609 100644 --- a/config/v201/component_schemas/standardized/InternalCtrlr.json +++ b/config/v201/component_schemas/standardized/InternalCtrlr.json @@ -38,7 +38,7 @@ "variable_name": "NetworkConnectionProfiles", "characteristics": { "supportsMonitoring": true, - "dataType": "MemberList" + "dataType": "string" }, "attributes": [ { @@ -487,6 +487,24 @@ "description": "Threshold for the size of in-memory message queues used to buffer messages (and store e.g. while offline). If threshold is exceeded, messages will be dropped according to OCPP specification to avoid memory issues.", "minimum": 1, "type": "integer" + }, + "MaxMessageSize": { + "variable_name": "MaxMessageSize", + "characteristics": { + "minLimit": 1, + "supportsMonitoring": true, + "dataType": "integer" + }, + "attributes": [ + { + "type": "Actual", + "mutability": "ReadOnly" + } + ], + "description": "Maximum size in bytes for messages sent to the CSMS via websocket. If a message exceeds this size and is eligible to be split into multiple messages, it will be split. Otherwise, this value is ignored.", + "minimum": 1, + "default": "65000", + "type": "integer" } }, "required": [ diff --git a/include/ocpp/common/message_queue.hpp b/include/ocpp/common/message_queue.hpp index bafcdf4ac..a2daeec93 100644 --- a/include/ocpp/common/message_queue.hpp +++ b/include/ocpp/common/message_queue.hpp @@ -67,7 +67,7 @@ template struct ControlMessage { DateTime timestamp; ///< A timestamp that shows when this message can be sent /// \brief Creates a new ControlMessage object from the provided \p message - explicit ControlMessage(json message); + explicit ControlMessage(const json& message); /// \brief Provides the unique message ID stored in the message /// \returns the unique ID of the contained message @@ -443,20 +443,29 @@ template class MessageQueue { if (!running) { return; } + json call_json = call; + push(call_json); + } - auto message = std::make_shared>(call); - if (message->isTransactionMessage()) { + void push(const json& message) { + if (!running) { + return; + } + + auto control_message = std::make_shared>(message); + if (control_message->isTransactionMessage()) { // according to the spec the "transaction related messages" StartTransaction, StopTransaction and // MeterValues have to be delivered in chronological order // intentionally break this message for testing... // message->message[CALL_PAYLOAD]["broken"] = this->createMessageId(); - this->add_to_transaction_message_queue(message); + this->add_to_transaction_message_queue(control_message); } else { // all other messages are allowed to "jump the queue" to improve user experience // TODO: decide if we only want to allow this for a subset of messages - if (!this->paused || this->config.queue_all_messages || message->messageType == M::BootNotification) { - this->add_to_normal_message_queue(message); + if (!this->paused || this->config.queue_all_messages || + control_message->messageType == M::BootNotification) { + this->add_to_normal_message_queue(control_message); } } this->cv.notify_all(); diff --git a/include/ocpp/v201/charge_point.hpp b/include/ocpp/v201/charge_point.hpp index af9108dd8..813d69b2b 100644 --- a/include/ocpp/v201/charge_point.hpp +++ b/include/ocpp/v201/charge_point.hpp @@ -369,7 +369,7 @@ class ChargePoint : ocpp::ChargingStationBase { // Functional Block B: Provisioning void boot_notification_req(const BootReasonEnum& reason); - void notify_report_req(const int request_id, const int seq_no, const std::vector& report_data); + void notify_report_req(const int request_id, const std::vector& report_data); // Functional Block C: Authorization AuthorizeResponse authorize_req(const IdToken id_token, const std::optional>& certificate, diff --git a/include/ocpp/v201/ctrlr_component_variables.hpp b/include/ocpp/v201/ctrlr_component_variables.hpp index e30f33e6d..4c1997fbe 100644 --- a/include/ocpp/v201/ctrlr_component_variables.hpp +++ b/include/ocpp/v201/ctrlr_component_variables.hpp @@ -65,6 +65,7 @@ extern const ComponentVariable& V2GCertificateExpireCheckIntervalSeconds; extern const ComponentVariable& ClientCertificateExpireCheckInitialDelaySeconds; extern const ComponentVariable& ClientCertificateExpireCheckIntervalSeconds; extern const ComponentVariable& MessageQueueSizeThreshold; +extern const ComponentVariable& MaxMessageSize; extern const ComponentVariable& AlignedDataCtrlrEnabled; extern const ComponentVariable& AlignedDataCtrlrAvailable; extern const RequiredComponentVariable& AlignedDataInterval; diff --git a/include/ocpp/v201/device_model.hpp b/include/ocpp/v201/device_model.hpp index f11d64257..4b4bb5889 100644 --- a/include/ocpp/v201/device_model.hpp +++ b/include/ocpp/v201/device_model.hpp @@ -31,6 +31,11 @@ template T to_specific_type(const std::string& value) { return std::stoi(value); } else if constexpr (std::is_same::value) { return std::stod(value); + } else if constexpr (std::is_same::value) { + std::stringstream s(value); + size_t res; + s >> res; + return res; } else if constexpr (std::is_same::value) { return DateTime(value); } else if constexpr (std::is_same::value) { diff --git a/include/ocpp/v201/notify_report_requests_splitter.hpp b/include/ocpp/v201/notify_report_requests_splitter.hpp new file mode 100644 index 000000000..115f1aa08 --- /dev/null +++ b/include/ocpp/v201/notify_report_requests_splitter.hpp @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2020 - 2023 Pionix GmbH and Contributors to EVerest + +#ifndef OCPP_NOTIFY_REPORT_REQUESTS_SPLITTER_HPP +#define OCPP_NOTIFY_REPORT_REQUESTS_SPLITTER_HPP + +#include "ocpp/common/call_types.hpp" +#include "ocpp/v201/messages/NotifyReport.hpp" +#include "ocpp/v201/types.hpp" + +namespace ocpp { +namespace v201 { + +/// \brief Utility class that is used to split NotifyReportRequest into several ones in case ReportData is too big. +class NotifyReportRequestsSplitter { + +private: + // cppcheck-suppress unusedStructMember + static const std::string MESSAGE_TYPE; // NotifyReport + const NotifyReportRequest& original_request; + // cppcheck-suppress unusedStructMember + size_t max_size; + const std::function& message_id_generator_callback; + json request_json_template; // json that is used as template for request json + // cppcheck-suppress unusedStructMember + const size_t json_skeleton_size; // size of the json skeleton for a call json object which includes everything + // except the requests' reportData and the messageId + +public: + NotifyReportRequestsSplitter(const NotifyReportRequest& originalRequest, size_t max_size, + const std::function& message_id_generator_callback); + NotifyReportRequestsSplitter() = delete; + + /// \brief Splits the provided NotifyReportRequest into (potentially) several Call payloads + /// \returns the json messages that serialize the resulting Call objects + std::vector create_call_payloads(); + +private: + size_t create_request_template_json_and_return_skeleton_size(); + + // Create next call payload (with as many reportData items as possible) + json create_next_payload(const int& seq_no, + std::vector::const_iterator& report_data_iterator, + const std::vector::const_iterator& report_data_end, + const std::string& message_id); + + // Create next request payload (with as many reportData items as possible) to be contained in next call payload + static json create_next_report_data_json(std::vector::const_iterator& report_data_iterator, + const std::vector::const_iterator& report_data_end, + const size_t& remaining_size); +}; + +} // namespace v201 +} // namespace ocpp + +#endif // OCPP_NOTIFY_REPORT_REQUESTS_SPLITTER_HPP diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 4df9e472f..e5c4a5176 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -30,20 +30,21 @@ target_sources(ocpp ocpp/v16/enums.cpp ocpp/v16/ocpp_types.cpp ocpp/v16/types.cpp + ocpp/v201/average_meter_values.cpp ocpp/v201/charge_point.cpp + ocpp/v201/connector.cpp + ocpp/v201/ctrlr_component_variables.cpp ocpp/v201/database_handler.cpp - ocpp/v201/device_model_storage_sqlite.cpp ocpp/v201/device_model.cpp - ocpp/v201/transaction.cpp + ocpp/v201/device_model_storage_sqlite.cpp ocpp/v201/enums.cpp + ocpp/v201/evse.cpp + ocpp/v201/notify_report_requests_splitter.cpp ocpp/v201/ocpp_types.cpp + ocpp/v201/ocsp_updater.cpp + ocpp/v201/transaction.cpp ocpp/v201/types.cpp - ocpp/v201/connector.cpp - ocpp/v201/evse.cpp ocpp/v201/utils.cpp - ocpp/v201/ctrlr_component_variables.cpp - ocpp/v201/ocsp_updater.cpp - ocpp/v201/average_meter_values.cpp ) add_subdirectory(ocpp/common/websocket) diff --git a/lib/ocpp/common/message_queue.cpp b/lib/ocpp/common/message_queue.cpp index 9844a8a66..4ff5c2d4e 100644 --- a/lib/ocpp/common/message_queue.cpp +++ b/lib/ocpp/common/message_queue.cpp @@ -7,7 +7,7 @@ namespace ocpp { -template <> ControlMessage::ControlMessage(json message) { +template <> ControlMessage::ControlMessage(const json& message) { this->message = message.get(); this->messageType = v16::conversions::string_to_messagetype(message.at(CALL_ACTION)); this->message_attempts = 0; @@ -26,7 +26,7 @@ template <> bool ControlMessage::isTransactionUpdateMessage() return (this->messageType == v16::MessageType::MeterValues); } -template <> ControlMessage::ControlMessage(json message) { +template <> ControlMessage::ControlMessage(const json& message) { this->message = message.get(); this->messageType = v201::conversions::string_to_messagetype(message.at(CALL_ACTION)); this->message_attempts = 0; diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index a368733c4..f106cbfcf 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -21,6 +22,7 @@ namespace v201 { const auto DEFAULT_BOOT_NOTIFICATION_RETRY_INTERVAL = std::chrono::seconds(30); const auto WEBSOCKET_INIT_DELAY = std::chrono::seconds(2); const auto DEFAULT_MESSAGE_QUEUE_SIZE_THRESHOLD = 2E5; +const auto DEFAULT_MAX_MESSAGE_SIZE = 65000; bool Callbacks::all_callbacks_valid() const { return this->is_reset_allowed_callback != nullptr and this->reset_callback != nullptr and @@ -1580,17 +1582,28 @@ void ChargePoint::boot_notification_req(const BootReasonEnum& reason) { this->send(call); } -void ChargePoint::notify_report_req(const int request_id, const int seq_no, - const std::vector& report_data) { +void ChargePoint::notify_report_req(const int request_id, const std::vector& report_data) { NotifyReportRequest req; req.requestId = request_id; - req.seqNo = seq_no; + req.seqNo = 0; req.generatedAt = ocpp::DateTime(); req.reportData.emplace(report_data); + req.tbc = false; - ocpp::Call call(req, this->message_queue->createMessageId()); - this->send(call); + if (report_data.size() <= 1) { + ocpp::Call call(req, this->message_queue->createMessageId()); + this->send(call); + } else { + NotifyReportRequestsSplitter splitter{ + req, + this->device_model->get_optional_value(ControllerComponentVariables::MaxMessageSize) + .value_or(DEFAULT_MAX_MESSAGE_SIZE), + [this]() { return this->message_queue->createMessageId(); }}; + for (const auto& msg : splitter.create_call_payloads()) { + this->message_queue->push(msg); + } + } } AuthorizeResponse ChargePoint::authorize_req(const IdToken id_token, const std::optional>& certificate, @@ -2014,7 +2027,7 @@ void ChargePoint::handle_get_base_report_req(Call call) { // TODO(piet): Propably split this up into several NotifyReport.req depending on ItemsPerMessage / // BytesPerMessage const auto report_data = this->device_model->get_report_data(msg.reportBase); - this->notify_report_req(msg.requestId, 0, report_data); + this->notify_report_req(msg.requestId, report_data); } } @@ -2037,7 +2050,7 @@ void ChargePoint::handle_get_report_req(Call call) { this->send(call_result); if (response.status == GenericDeviceModelStatusEnum::Accepted) { - this->notify_report_req(msg.requestId, 0, report_data); + this->notify_report_req(msg.requestId, report_data); } } diff --git a/lib/ocpp/v201/ctrlr_component_variables.cpp b/lib/ocpp/v201/ctrlr_component_variables.cpp index a73db0e68..ae23c492b 100644 --- a/lib/ocpp/v201/ctrlr_component_variables.cpp +++ b/lib/ocpp/v201/ctrlr_component_variables.cpp @@ -242,6 +242,13 @@ const ComponentVariable& MessageQueueSizeThreshold = { "MessageQueueSizeThreshold", }), }; +const ComponentVariable& MaxMessageSize = { + ControllerComponents::InternalCtrlr, + std::nullopt, + std::optional({ + "MaxMessageSize", + }), +}; const ComponentVariable& AlignedDataCtrlrEnabled = { ControllerComponents::AlignedDataCtrlr, std::nullopt, diff --git a/lib/ocpp/v201/notify_report_requests_splitter.cpp b/lib/ocpp/v201/notify_report_requests_splitter.cpp new file mode 100644 index 000000000..91a2c93ab --- /dev/null +++ b/lib/ocpp/v201/notify_report_requests_splitter.cpp @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2020 - 2023 Pionix GmbH and Contributors to EVerest + +#include +#include + +namespace ocpp { +namespace v201 { + +const std::string NotifyReportRequestsSplitter::MESSAGE_TYPE = + conversions::messagetype_to_string(MessageType::NotifyReport); + +std::vector NotifyReportRequestsSplitter::create_call_payloads() { + + // In case there is no report data, fallback to no-splitting call creation + if (!original_request.reportData.has_value()) { + return std::vector{ + {MessageTypeId::CALL, message_id_generator_callback().get(), MESSAGE_TYPE, json(original_request)}}; + } + + // Loop along reportData and create payloads + std::vector payloads{}; + int seq_no = 0; + + auto report_data_iterator = original_request.reportData->begin(); + while (seq_no == 0 || report_data_iterator != original_request.reportData->end()) { + payloads.emplace_back(create_next_payload(seq_no, report_data_iterator, original_request.reportData->end(), + message_id_generator_callback().get())); + seq_no++; + } + + if (seq_no > 1) { + EVLOG_info << "Split NotifyReportRequest '" << original_request.requestId << "' into " << seq_no + << " messages."; + } + + return payloads; +} + +json NotifyReportRequestsSplitter::create_next_report_data_json( + std::vector::const_iterator& report_data_iterator, + const std::vector::const_iterator& report_data_end, const size_t& remaining_size) { + + if (report_data_iterator == report_data_end) { + return json::array(); + } + + json report_data_json{*report_data_iterator}; + report_data_iterator++; + if (report_data_iterator == report_data_end) { + return report_data_json; + } + + auto size = report_data_json.dump().size(); + + for (; report_data_iterator != report_data_end; report_data_iterator++) { + json current_json = *report_data_iterator; + // new report data object will increase payload size by its dump + 1 (caused by the separating comma) + auto additional_json_size = current_json.dump().size() + 1; + if (size + additional_json_size <= remaining_size) { + size += additional_json_size; + report_data_json.emplace_back(std::move(current_json)); + } else { + break; + } + } + + return report_data_json; +} + +json NotifyReportRequestsSplitter::create_next_payload( + const int& seq_no, std::vector::const_iterator& report_data_iterator, + const std::vector::const_iterator& report_data_end, const std::string& message_id) { + + json call_base{MessageTypeId::CALL, message_id, MESSAGE_TYPE}; + + size_t base_json_string_length = this->json_skeleton_size + message_id.size(); + size_t remaining_size = this->max_size >= base_json_string_length ? this->max_size - base_json_string_length : 0; + + auto request_json = request_json_template; + request_json["reportData"] = create_next_report_data_json(report_data_iterator, report_data_end, remaining_size); + request_json["tbc"] = report_data_iterator != report_data_end; + request_json["seqNo"] = seq_no; + + call_base.emplace_back(request_json); + + return call_base; +} +NotifyReportRequestsSplitter::NotifyReportRequestsSplitter( + const NotifyReportRequest& originalRequest, size_t max_size, + const std::function& message_id_generator_callback) : + original_request(originalRequest), + max_size(max_size), + message_id_generator_callback(message_id_generator_callback), + json_skeleton_size(create_request_template_json_and_return_skeleton_size()) { +} + +size_t NotifyReportRequestsSplitter::create_request_template_json_and_return_skeleton_size() { + + NotifyReportRequest req{}; + req.requestId = original_request.requestId; + req.generatedAt = original_request.generatedAt; + req.tbc = false; + this->request_json_template = req; + + // Skeleton json sizeof( [MessageTypeId::CALL, "", "NotifyReport", {,"reportData":}] ) + return json{MessageTypeId::CALL, "", MESSAGE_TYPE, request_json_template}.dump().size() + + std::string{R"(,"reportData":)"}.size(); +} + +} // namespace v201 +} // namespace ocpp diff --git a/tests/lib/ocpp/common/test_message_queue.cpp b/tests/lib/ocpp/common/test_message_queue.cpp index 0a3f8d7cf..7ed0d3cb1 100644 --- a/tests/lib/ocpp/common/test_message_queue.cpp +++ b/tests/lib/ocpp/common/test_message_queue.cpp @@ -108,7 +108,7 @@ template <> TestMessageType MessageQueue::string_to_messagetype return to_test_message_type(s); } -template <> ControlMessage::ControlMessage(json message) { +template <> ControlMessage::ControlMessage(const json& message) { this->message = message.get(); EVLOG_info << this->message; this->messageType = to_test_message_type(this->message[2]); @@ -285,7 +285,7 @@ class MessageQueueTest : public ::testing::Test { message_queue->stop(); } message_queue = std::make_unique>(send_callback_mock.AsStdFunction(), config, db); - message_queue->resume(); + message_queue->resume(std::chrono::seconds(0)); } void SetUp() override { @@ -355,7 +355,7 @@ TEST_F(MessageQueueTest, test_queuing_up_of_transactional_messages) { push_message_call(TestMessageType::TRANSACTIONAL); } - message_queue->resume(); + message_queue->resume(std::chrono::seconds(0)); // expect one repeated and all other calls been made wait_for_calls(message_count + 1); @@ -380,7 +380,7 @@ TEST_F(MessageQueueTest, test_non_queuing_up_of_non_transactional_messages) { push_message_call(TestMessageType::NON_TRANSACTIONAL); } - message_queue->resume(); + message_queue->resume(std::chrono::seconds(0)); std::this_thread::sleep_for(std::chrono::milliseconds(50)); // expect calls not repeated @@ -408,7 +408,7 @@ TEST_F(MessageQueueTest, test_queuing_up_of_non_transactional_messages) { push_message_call(TestMessageType::NON_TRANSACTIONAL); } - message_queue->resume(); + message_queue->resume(std::chrono::seconds(0)); // expect calls _are_ repeated wait_for_calls(message_count + 1); @@ -457,7 +457,7 @@ TEST_F(MessageQueueTest, test_clean_up_non_transactional_queue) { } // go online again - message_queue->resume(); + message_queue->resume(std::chrono::seconds(0)); // expect calls _are_ repeated wait_for_calls(sent_transactional_messages + sent_non_transactional_messages - @@ -541,7 +541,7 @@ TEST_F(MessageQueueTest, test_clean_up_transactional_queue) { } // Resume & verify - message_queue->resume(); + message_queue->resume(std::chrono::seconds(0)); wait_for_calls(expected_sent_messages); } diff --git a/tests/lib/ocpp/v201/CMakeLists.txt b/tests/lib/ocpp/v201/CMakeLists.txt index 9531f018f..8d9dc8935 100644 --- a/tests/lib/ocpp/v201/CMakeLists.txt +++ b/tests/lib/ocpp/v201/CMakeLists.txt @@ -1,4 +1,5 @@ target_sources(libocpp_unit_tests PRIVATE test_device_model_storage_sqlite.cpp + test_notify_report_requests_splitter.cpp test_ocsp_updater.cpp) diff --git a/tests/lib/ocpp/v201/test_notify_report_requests_splitter.cpp b/tests/lib/ocpp/v201/test_notify_report_requests_splitter.cpp new file mode 100644 index 000000000..c1cecc54e --- /dev/null +++ b/tests/lib/ocpp/v201/test_notify_report_requests_splitter.cpp @@ -0,0 +1,161 @@ + +#include +#include +#include + +namespace ocpp { +namespace v201 { + +class NotifyReportRequestsSplitterTest : public ::testing::Test { + int message_count = 0; + +protected: + std::vector message_ids{}; + MessageId generate_message_id() { + std::stringstream s; + s << "test_message_" << message_count; + message_count++; + message_ids.emplace_back(s.str()); + return message_ids.back(); + } + + // verify returned payloads are actual serializations of Call instances + static void check_valid_call_payload(json payload) { + ASSERT_EQ(payload.size(), 4); + ASSERT_THAT(payload[MESSAGE_ID].dump(), testing::MatchesRegex("^\"test_message_[0-9]+\"$")); + ASSERT_EQ(payload[CALL_ACTION].dump(), R"("NotifyReport")"); + ASSERT_EQ(payload[MESSAGE_TYPE_ID], MessageTypeId::CALL); + Call call{}; + from_json(payload, call); + ASSERT_EQ(call.msg.get_type(), "NotifyReport"); + } +}; + +/// \brief Test a request with no report data results into a single message +TEST_F(NotifyReportRequestsSplitterTest, test_create_single_request_no_report_data) { + + // Setup + NotifyReportRequest req{}; + req.reportData = std::nullopt; + json req_json = req; + NotifyReportRequestsSplitter splitter{req, 1000, [this]() { return this->generate_message_id(); }}; + + // Act: create payloads + auto res = splitter.create_call_payloads(); + + // Verify: Expect single payload; check fields + ASSERT_EQ(res.size(), 1); + auto request = res[0]; + check_valid_call_payload(res[0]); + ASSERT_EQ("test_message_0", request[1]); + ASSERT_EQ(json(req_json).dump(), request[3].dump()); +} + +/// \brief Test a request that fits exactly the provided bound is not split +TEST_F(NotifyReportRequestsSplitterTest, test_create_single_request) { + + // Setup + NotifyReportRequest req{}; + req.reportData = {ReportData{{"component_name"}, {"variable_name"}, {}, {}, {}}, + ReportData{{"component_name2"}, {"variable_name2"}, {}, {}, {}}}; + req.requestId = 42; + req.tbc = false; + req.seqNo = 0; + json req_json = req; + size_t full_size = json{2, "test_message_0", "NotifyReport", req}.dump().size(); + + // Create splitter with size exactly fitting the expected payload - thus no split should be done + NotifyReportRequestsSplitter splitter{req, full_size, [this]() { return this->generate_message_id(); }}; + auto res = splitter.create_call_payloads(); + + // Assert no split + ASSERT_EQ(res.size(), 1); + auto request = res[0]; + check_valid_call_payload(request); + ASSERT_EQ(request[1], "test_message_0"); + + std::stringstream expected_report_data_json; + expected_report_data_json << "[" << json(req.reportData.value()[0]) << "," << json(req.reportData.value()[1]) + << "]"; + ASSERT_EQ(expected_report_data_json.str(), request[3]["reportData"].dump()); + ASSERT_EQ("false", request[3]["tbc"].dump()); + ASSERT_EQ("0", request[3]["seqNo"].dump()); + ASSERT_EQ(req_json["generatedAt"], request[3]["generatedAt"]); +} + +// \brief Test a request that is one byte too long is split +TEST_F(NotifyReportRequestsSplitterTest, test_create_split_request) { + // Setup + NotifyReportRequest req{}; + req.requestId = 42; + req.reportData = {ReportData{{"component_name"}, {"variable_name"}, {}, {}, {}}, + ReportData{{"component_name2"}, {"variable_name2"}, {}, {}, {}}}; + req.tbc = false; + json req_json = req; + + size_t full_size = json{2, "test_message_0", "NotifyReport", req}.dump().size(); + + // Create splitter with size one less than the expected payload - thus a split should be done + NotifyReportRequestsSplitter splitter{req, full_size - 1, [this]() { return this->generate_message_id(); }}; + auto res = splitter.create_call_payloads(); + + // Verify split is done + ASSERT_EQ(res.size(), 2); + + for (int i = 0; i < 2; i++) { + auto request = res[i]; + check_valid_call_payload(request); + + std::stringstream expected_report_data_json; + expected_report_data_json << "[" << json(req.reportData.value()[i]).dump() << "]"; + ASSERT_EQ(expected_report_data_json.str(), request[3]["reportData"].dump()); + + ASSERT_EQ(req_json["generatedAt"], request[3]["generatedAt"]); + if (i == 0) { + ASSERT_EQ(request[3]["tbc"].dump(), "true"); + ASSERT_EQ(request[3]["seqNo"].dump(), "0"); + } else { + ASSERT_EQ(request[3]["tbc"].dump(), "false"); + ASSERT_EQ(request[3]["seqNo"].dump(), "1"); + } + } +} + +// \brief Test that each split contains at least one report data object, even if it exceeds the size bound. +TEST_F(NotifyReportRequestsSplitterTest, test_splits_contains_at_least_one_report) { + // Setup + NotifyReportRequest req{}; + req.requestId = 42; + req.reportData = {ReportData{{"component_name"}, {"variable_name"}, {}, {}, {}}, + ReportData{{"component_name2"}, {"variable_name2"}, {}, {}, {}}}; + req.tbc = false; + json req_json = req; + + // Act: create splitter with minimal max size + NotifyReportRequestsSplitter splitter{req, 1, [this]() { return this->generate_message_id(); }}; + auto res = splitter.create_call_payloads(); + + // Verify message are split into two messages + ASSERT_EQ(res.size(), 2); + for (int i = 0; i < 2; i++) { + auto request = res[i]; + check_valid_call_payload(request); + ASSERT_EQ(message_ids[i].get(), request[1]); + + std::stringstream expected_report_data_json; + expected_report_data_json << "[" << json(req.reportData.value()[i]).dump() << "]"; + ASSERT_EQ(expected_report_data_json.str(), request[3]["reportData"].dump()); + + ASSERT_EQ(req_json["generatedAt"], request[3]["generatedAt"]); + if (i == 0) { + ASSERT_EQ(request[3]["tbc"].dump(), "true"); + ASSERT_EQ(request[3]["seqNo"].dump(), "0"); + } else { + ASSERT_EQ(request[3]["tbc"].dump(), "false"); + ASSERT_EQ(request[3]["seqNo"].dump(), "1"); + } + } +} + +} // namespace v201 +} // namespace ocpp From aba6f3b99312c4f16e54e0127a93127d71baf6ee Mon Sep 17 00:00:00 2001 From: Kai Hermann Date: Mon, 11 Dec 2023 17:49:21 +0100 Subject: [PATCH 09/18] Fix potential crashes and fix TxProfile during transaction (#299) * Smart charging: set ignore_no_transaction in validate_profile call This was a bit too strict since it does reject some valid charging profiles (like TxProfiles that omit the transaction id) * Log an error if attempting to stop a transaction that is unknown to libocpp * Check if websocket is nullptr in authorize_id_token Signed-off-by: Kai-Uwe Hermann --- lib/ocpp/v16/charge_point_impl.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index 914423e11..138e8c534 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -1865,7 +1865,7 @@ void ChargePointImpl::handleSetChargingProfileRequest(ocpp::Callsmart_charging_handler->validate_profile( - profile, connector_id, false, this->configuration->getChargeProfileMaxStackLevel(), + profile, connector_id, true, this->configuration->getChargeProfileMaxStackLevel(), this->configuration->getMaxChargingProfilesInstalled(), this->configuration->getChargingScheduleMaxPeriods(), this->configuration->getChargingScheduleAllowedChargingRateUnitVector())) { @@ -2550,8 +2550,10 @@ IdTagInfo ChargePointImpl::authorize_id_token(CiString<20> idTag) { // - LocalPreAuthorize is true and CP is online // OR // - LocalAuthorizeOffline is true and CP is offline - if ((this->configuration->getLocalPreAuthorize() && this->websocket->is_connected()) || - (this->configuration->getLocalAuthorizeOffline() && !this->websocket->is_connected())) { + if ((this->configuration->getLocalPreAuthorize() && + (this->websocket != nullptr && this->websocket->is_connected())) || + (this->configuration->getLocalAuthorizeOffline() && + (this->websocket == nullptr || !this->websocket->is_connected()))) { if (this->configuration->getLocalAuthListEnabled()) { const auto auth_list_entry_opt = this->database_handler->get_local_authorization_list_entry(idTag); if (auth_list_entry_opt.has_value()) { @@ -3203,13 +3205,19 @@ void ChargePointImpl::on_transaction_stopped(const int32_t connector, const std: const Reason& reason, ocpp::DateTime timestamp, float energy_wh_import, std::optional> id_tag_end, std::optional signed_meter_value) { + auto transaction = this->transaction_handler->get_transaction(connector); + if (transaction == nullptr) { + EVLOG_error << "Trying to stop a transaction that is unknown on connector: " << connector + << ", with session_id: " << session_id; + return; + } if (signed_meter_value) { const auto meter_value = this->get_signed_meter_value(signed_meter_value.value(), ReadingContext::Transaction_End, timestamp); - this->transaction_handler->get_transaction(connector)->add_meter_value(meter_value); + transaction->add_meter_value(meter_value); } const auto stop_energy_wh = std::make_shared(timestamp, energy_wh_import); - this->transaction_handler->get_transaction(connector)->add_stop_energy_wh(stop_energy_wh); + transaction->add_stop_energy_wh(stop_energy_wh); this->status->submit_event(connector, FSMEvent::TransactionStoppedAndUserActionRequired, ocpp::DateTime()); this->stop_transaction(connector, reason, id_tag_end); From 89158a10f670af0a210f2b296e7d07c499577064 Mon Sep 17 00:00:00 2001 From: valentin-dimov <143783161+valentin-dimov@users.noreply.github.com> Date: Mon, 11 Dec 2023 17:57:54 +0100 Subject: [PATCH 10/18] OCPP 2.0.1: Make message interval, retries, and timeout configurable at run-time (#298) * feat: make message interval and message retries configurable at runtime for OCPP 2.0.1 * fix: intervals between resending messages are configurable and computed correctly now * fix: regenerate message IDs when retrying after CallError * Set default message timeout to 30 seconds. * fix: do not reuse message IDs, even in case of a timeout --------- Signed-off-by: Valentin Dimov --- include/ocpp/common/message_queue.hpp | 30 ++++++++++++++++++++++----- lib/ocpp/v201/charge_point.cpp | 24 ++++++++++++++++++++- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/include/ocpp/common/message_queue.hpp b/include/ocpp/common/message_queue.hpp index a2daeec93..e7dabfc10 100644 --- a/include/ocpp/common/message_queue.hpp +++ b/include/ocpp/common/message_queue.hpp @@ -27,8 +27,6 @@ namespace ocpp { -const auto STANDARD_MESSAGE_TIMEOUT = std::chrono::seconds(30); - struct MessageQueueConfig { int transaction_message_attempts; int transaction_message_retry_interval; // seconds @@ -38,6 +36,8 @@ struct MessageQueueConfig { int queues_total_size_threshold; bool queue_all_messages; // cf. OCPP 2.0.1. "QueueAllMessages" in OCPPCommCtrlr + + int message_timeout_seconds = 30; }; /// \brief Contains a OCPP message in json form with additional information @@ -257,6 +257,12 @@ template class MessageQueue { } } + // Computes the current message timeout = interval * attempt + message timeout + std::chrono::seconds current_message_timeout(unsigned int attempt) { + return std::chrono::seconds(this->config.message_timeout_seconds + + (this->config.transaction_message_retry_interval * attempt)); + } + public: /// \brief Creates a new MessageQueue object with the provided \p configuration and \p send_callback MessageQueue(const std::function& send_callback, const MessageQueueConfig& config, @@ -392,7 +398,7 @@ template class MessageQueue { } else { EVLOG_debug << "Successfully sent message. UID: " << this->in_flight->uniqueId(); this->in_flight_timeout_timer.timeout([this]() { this->handle_timeout_or_callerror(std::nullopt); }, - STANDARD_MESSAGE_TIMEOUT); + this->current_message_timeout(message->message_attempts)); switch (queue_type) { case QueueType::Normal: this->normal_message_queue.pop(); @@ -621,11 +627,20 @@ template class MessageQueue { /// \brief Handles a message timeout or a CALLERROR. \p enhanced_message_opt is set only in case of CALLERROR void handle_timeout_or_callerror(const std::optional>& enhanced_message_opt) { std::lock_guard lk(this->message_mutex); - EVLOG_warning << "Message timeout or CALLERROR for: " << this->in_flight->messageType << " (" - << this->in_flight->uniqueId() << ")"; + // We got a timeout iff enhanced_message_opt is empty. Otherwise, enhanced_message_opt contains the CallError. + bool timeout = !enhanced_message_opt.has_value(); + if (timeout) { + EVLOG_warning << "Message timeout for: " << this->in_flight->messageType << " (" + << this->in_flight->uniqueId() << ")"; + } else { + EVLOG_warning << "CALLERROR for: " << this->in_flight->messageType << " (" << this->in_flight->uniqueId() + << ")"; + } + if (this->in_flight->isTransactionMessage()) { if (this->in_flight->message_attempts < this->config.transaction_message_attempts) { EVLOG_warning << "Message is transaction related and will therefore be sent again"; + // Generate a new message ID for the retry this->in_flight->message[MESSAGE_ID] = this->createMessageId(); if (this->config.transaction_message_retry_interval > 0) { // exponential backoff @@ -755,6 +770,11 @@ template class MessageQueue { this->config.transaction_message_retry_interval = transaction_message_retry_interval; } + /// \brief Set message_timeout to given \p timeout (in seconds) + void update_message_timeout(const int timeout) { + this->config.message_timeout_seconds = timeout; + } + /// \brief Creates a unique message ID /// \returns the unique message ID MessageId createMessageId() { diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index f106cbfcf..6133ff80f 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -152,7 +152,8 @@ ChargePoint::ChargePoint(const std::map& evse_connector_struct this->device_model->get_optional_value(ControllerComponentVariables::MessageQueueSizeThreshold) .value_or(DEFAULT_MESSAGE_QUEUE_SIZE_THRESHOLD), this->device_model->get_optional_value(ControllerComponentVariables::QueueAllMessages) - .value_or(false)}, + .value_or(false), + this->device_model->get_value(ControllerComponentVariables::MessageTimeout)}, this->database_handler); } @@ -1371,6 +1372,27 @@ void ChargePoint::handle_variable_changed(const SetVariableData& set_variable_da this->websocket->set_connection_options(connection_options); } + if (component_variable == ControllerComponentVariables::MessageAttemptInterval) { + if (component_variable.variable.has_value()) { + this->message_queue->update_transaction_message_retry_interval( + this->device_model->get_value(ControllerComponentVariables::MessageAttemptInterval)); + } + } + + if (component_variable == ControllerComponentVariables::MessageAttempts) { + if (component_variable.variable.has_value()) { + this->message_queue->update_transaction_message_attempts( + this->device_model->get_value(ControllerComponentVariables::MessageAttempts)); + } + } + + if (component_variable == ControllerComponentVariables::MessageTimeout) { + if (component_variable.variable.has_value()) { + this->message_queue->update_message_timeout( + this->device_model->get_value(ControllerComponentVariables::MessageTimeout)); + } + } + // TODO(piet): other special handling of changed variables can be added here... } From fef9c41780be2ca9341deb9f67b7d64c83018abd Mon Sep 17 00:00:00 2001 From: Kai-Uwe Hermann Date: Mon, 11 Dec 2023 19:13:16 +0100 Subject: [PATCH 11/18] Make sure the session_id_logging map is properly locked when modifying it Signed-off-by: Kai-Uwe Hermann --- include/ocpp/common/ocpp_logging.hpp | 1 + lib/ocpp/common/ocpp_logging.cpp | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/include/ocpp/common/ocpp_logging.hpp b/include/ocpp/common/ocpp_logging.hpp index 852c1cc6a..c757146d8 100644 --- a/include/ocpp/common/ocpp_logging.hpp +++ b/include/ocpp/common/ocpp_logging.hpp @@ -36,6 +36,7 @@ class MessageLogging { std::mutex output_file_mutex; std::function message_callback; std::map lookup_map; + std::recursive_mutex session_id_logging_mutex; std::map> session_id_logging; void log_output(unsigned int typ, const std::string& message_type, const std::string& json_str); diff --git a/lib/ocpp/common/ocpp_logging.cpp b/lib/ocpp/common/ocpp_logging.cpp index 0da0d9d4d..3b8ef0ca1 100644 --- a/lib/ocpp/common/ocpp_logging.cpp +++ b/lib/ocpp/common/ocpp_logging.cpp @@ -93,6 +93,7 @@ void MessageLogging::charge_point(const std::string& message_type, const std::st auto formatted = format_message(message_type, json_str); log_output(0, formatted.message_type, formatted.message); if (this->session_logging) { + std::scoped_lock lock(this->session_id_logging_mutex); for (auto const& [session_id, logging] : this->session_id_logging) { logging->charge_point(message_type, json_str); } @@ -106,6 +107,7 @@ void MessageLogging::central_system(const std::string& message_type, const std:: auto formatted = format_message(message_type, json_str); log_output(1, formatted.message_type, formatted.message); if (this->session_logging) { + std::scoped_lock lock(this->session_id_logging_mutex); for (auto const& [session_id, logging] : this->session_id_logging) { logging->central_system(message_type, json_str); } @@ -115,6 +117,7 @@ void MessageLogging::central_system(const std::string& message_type, const std:: void MessageLogging::sys(const std::string& msg) { log_output(2, msg, ""); if (this->session_logging) { + std::scoped_lock lock(this->session_id_logging_mutex); for (auto const& [session_id, logging] : this->session_id_logging) { log_output(2, msg, ""); } @@ -201,11 +204,13 @@ FormattedMessageWithType MessageLogging::format_message(const std::string& messa } void MessageLogging::start_session_logging(const std::string& session_id, const std::string& log_path) { + std::scoped_lock lock(this->session_id_logging_mutex); this->session_id_logging[session_id] = std::make_shared( true, log_path, "incomplete-ocpp", false, false, false, true, false, nullptr); } void MessageLogging::stop_session_logging(const std::string& session_id) { + std::scoped_lock lock(this->session_id_logging_mutex); if (this->session_id_logging.count(session_id)) { auto old_file_path = this->session_id_logging.at(session_id)->get_message_log_path() + "/" + "incomplete-ocpp.html"; From 191e6d16e1acb93743fe97586b7fd89e7f16b0d9 Mon Sep 17 00:00:00 2001 From: valentin-dimov <143783161+valentin-dimov@users.noreply.github.com> Date: Tue, 12 Dec 2023 12:33:08 +0100 Subject: [PATCH 12/18] fix: Remove optional StatusNotifications on reset (#302) Signed-off-by: Valentin Dimov --- lib/ocpp/v201/charge_point.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index 6133ff80f..fe582e39c 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -2221,15 +2221,6 @@ void ChargePoint::handle_reset_req(Call call) { } if (response.status == ResetStatusEnum::Accepted) { - if (call.msg.evseId.has_value()) { - // B11.FR.08 - this->evses.at(call.msg.evseId.value())->submit_event(1, ConnectorEvent::Unavailable); - } else { - // B11.FR.03 - for (auto const& [evse_id, evse] : this->evses) { - evse->submit_event(1, ConnectorEvent::Unavailable); - } - } this->callbacks.reset_callback(call.msg.evseId, ResetEnum::Immediate); } } From 049b19c102306080a6fbc874691836fd81dc68ab Mon Sep 17 00:00:00 2001 From: valentin-dimov <143783161+valentin-dimov@users.noreply.github.com> Date: Tue, 12 Dec 2023 12:39:16 +0100 Subject: [PATCH 13/18] Fix: Validate CN = FQDN for TLS websockets connections (#287) * added check to verify csms certificate CN against FQDN for TLS connections * chore: resolve Codacy warning --------- Signed-off-by: pietfried Signed-off-by: Valentin Dimov Co-authored-by: pietfried --- lib/ocpp/common/websocket/websocket_tls.cpp | 37 +++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/lib/ocpp/common/websocket/websocket_tls.cpp b/lib/ocpp/common/websocket/websocket_tls.cpp index 121d522bf..7841309c4 100644 --- a/lib/ocpp/common/websocket/websocket_tls.cpp +++ b/lib/ocpp/common/websocket/websocket_tls.cpp @@ -14,6 +14,41 @@ namespace ocpp { +// verify that the csms certificate's commonName matches the CSMS FQDN +bool verify_csms_cn(const std::string& hostname, bool preverified, boost::asio::ssl::verify_context& ctx) { + + if (!preverified) { + EVLOG_error << "Could not verify CSMS server certificate"; + return false; + } + + int depth = X509_STORE_CTX_get_error_depth(ctx.native_handle()); + + // only check for CSMS server certificate + if (depth == 0) { + // Get server certificate + X509* server_cert = X509_STORE_CTX_get_current_cert(ctx.native_handle()); + + // Extract CN from csms server's certificate + X509_NAME* subject_name = X509_get_subject_name(server_cert); + char common_name[256]; + if (X509_NAME_get_text_by_NID(subject_name, NID_commonName, common_name, sizeof(common_name)) <= 0) { + EVLOG_error << "Could not extract CN from CSMS server certificate"; + return false; + } + + // Compare the extracted CN with the expected FQDN + if (hostname != common_name) { + EVLOG_error << "Server certificate CN does not match CSMS FQDN"; + return false; + } + + EVLOG_info << "FQDN matches CN of server certificate"; + } + + return true; +} + WebsocketTLS::WebsocketTLS(const WebsocketConnectionOptions& connection_options, std::shared_ptr evse_security) : WebsocketBase(), evse_security(evse_security) { @@ -200,6 +235,8 @@ tls_context WebsocketTLS::on_tls_init(std::string hostname, websocketpp::connect } context->set_verify_mode(boost::asio::ssl::verify_peer); + context->set_verify_callback(websocketpp::lib::bind( + &verify_csms_cn, hostname, websocketpp::lib::placeholders::_1, websocketpp::lib::placeholders::_2)); if (this->evse_security->is_ca_certificate_installed(ocpp::CaCertificateType::CSMS)) { EVLOG_info << "Loading ca csms bundle to verify server certificate: " << this->evse_security->get_verify_file(ocpp::CaCertificateType::CSMS); From c294586d15af4563c175c53d60886cd82f756c35 Mon Sep 17 00:00:00 2001 From: Menno de Graaf Date: Tue, 12 Dec 2023 13:34:16 +0100 Subject: [PATCH 14/18] Generate signing request not only for CSMS (#290) Signed-off-by: Menno de Graaf Co-authored-by: Menno de Graaf --- lib/ocpp/common/evse_security_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ocpp/common/evse_security_impl.cpp b/lib/ocpp/common/evse_security_impl.cpp index c970a2a87..640901d76 100644 --- a/lib/ocpp/common/evse_security_impl.cpp +++ b/lib/ocpp/common/evse_security_impl.cpp @@ -83,7 +83,7 @@ std::string EvseSecurityImpl::generate_certificate_signing_request(const Certifi const std::string& country, const std::string& organization, const std::string& common) { - return this->evse_security->generate_certificate_signing_request(evse_security::LeafCertificateType::CSMS, country, + return this->evse_security->generate_certificate_signing_request(conversions::from_ocpp(certificate_type), country, organization, common); } From 3f05b14200e95fcfb2ad3d0447acd233850a0c93 Mon Sep 17 00:00:00 2001 From: Menno de Graaf Date: Tue, 12 Dec 2023 13:34:31 +0100 Subject: [PATCH 15/18] Add component SeccId and add warning for missing components/variables (#300) * Add component SeccId and add warning for missing components/variables --------- Signed-off-by: Menno de Graaf Co-authored-by: Menno de Graaf --- include/ocpp/v201/ctrlr_component_variables.hpp | 1 + lib/ocpp/v201/charge_point.cpp | 6 ++---- lib/ocpp/v201/ctrlr_component_variables.cpp | 7 +++++++ lib/ocpp/v201/device_model.cpp | 2 ++ 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/include/ocpp/v201/ctrlr_component_variables.hpp b/include/ocpp/v201/ctrlr_component_variables.hpp index 4c1997fbe..b1c3fa762 100644 --- a/include/ocpp/v201/ctrlr_component_variables.hpp +++ b/include/ocpp/v201/ctrlr_component_variables.hpp @@ -122,6 +122,7 @@ extern const RequiredComponentVariable& EVSESupplyPhases; extern const ComponentVariable& CentralContractValidationAllowed; extern const RequiredComponentVariable& ContractValidationOffline; extern const ComponentVariable& RequestMeteringReceipt; +extern const ComponentVariable& ISO15118CtrlrSeccId; extern const ComponentVariable& ISO15118CtrlrCountryName; extern const ComponentVariable& ISO15118CtrlrOrganizationName; extern const ComponentVariable& PnCEnabled; diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index fe582e39c..bce274d21 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -1558,11 +1558,9 @@ void ChargePoint::sign_certificate_req(const ocpp::CertificateSigningUseEnum& ce organization = this->device_model->get_optional_value(ControllerComponentVariables::OrganizationName); country = - this->device_model->get_optional_value(ControllerComponentVariables::ISO15118CtrlrCountryName) - .value_or("DE"); + this->device_model->get_optional_value(ControllerComponentVariables::ISO15118CtrlrCountryName); } else { - common = - this->device_model->get_optional_value(ControllerComponentVariables::ChargeBoxSerialNumber); + common = this->device_model->get_optional_value(ControllerComponentVariables::ISO15118CtrlrSeccId); organization = this->device_model->get_optional_value( ControllerComponentVariables::ISO15118CtrlrOrganizationName); country = diff --git a/lib/ocpp/v201/ctrlr_component_variables.cpp b/lib/ocpp/v201/ctrlr_component_variables.cpp index ae23c492b..b8f71c413 100644 --- a/lib/ocpp/v201/ctrlr_component_variables.cpp +++ b/lib/ocpp/v201/ctrlr_component_variables.cpp @@ -625,6 +625,13 @@ const ComponentVariable& RequestMeteringReceipt = { "RequestMeteringReceipt", }), }; +const ComponentVariable& ISO15118CtrlrSeccId = { + ControllerComponents::ISO15118Ctrlr, + std::nullopt, + std::optional({ + "SeccId", + }), +}; const ComponentVariable& ISO15118CtrlrCountryName = { ControllerComponents::ISO15118Ctrlr, std::nullopt, diff --git a/lib/ocpp/v201/device_model.cpp b/lib/ocpp/v201/device_model.cpp index 7b93d0bed..25d501175 100644 --- a/lib/ocpp/v201/device_model.cpp +++ b/lib/ocpp/v201/device_model.cpp @@ -109,6 +109,7 @@ GetVariableStatusEnum DeviceModel::request_value_internal(const Component& compo bool allow_write_only) { const auto component_it = this->device_model.find(component_id); if (component_it == this->device_model.end()) { + EVLOG_warning << "unknown component in " << component_id.name << "." << variable_id.name; return GetVariableStatusEnum::UnknownComponent; } @@ -116,6 +117,7 @@ GetVariableStatusEnum DeviceModel::request_value_internal(const Component& compo const auto& variable_it = component.find(variable_id); if (variable_it == component.end()) { + EVLOG_warning << "unknown variable in " << component_id.name << "." << variable_id.name; return GetVariableStatusEnum::UnknownVariable; } From bc6a9188a6b6eb76ad8756e86c384655cfa7c015 Mon Sep 17 00:00:00 2001 From: corneliusclaussen <62659547+corneliusclaussen@users.noreply.github.com> Date: Wed, 13 Dec 2023 21:15:30 +0100 Subject: [PATCH 16/18] Bugfix/hotfix tls intermediates (#305) * Use complete chain instead of only leaf certificate * Fix domain name validation * Comment out preverified * Make verification of CSMS common name with the FQDN configurable Signed-off-by: Kai-Uwe Hermann Signed-off-by: Cornelius Claussen --- config/v16/profile_schemas/Internal.json | 6 ++ .../ocpp/common/websocket/websocket_base.hpp | 1 + .../ocpp/v16/charge_point_configuration.hpp | 2 + lib/ocpp/common/websocket/websocket_tls.cpp | 76 +++++++++++++++---- lib/ocpp/v16/charge_point_configuration.cpp | 15 ++++ lib/ocpp/v16/charge_point_impl.cpp | 3 +- lib/ocpp/v201/charge_point.cpp | 5 +- 7 files changed, 93 insertions(+), 15 deletions(-) diff --git a/config/v16/profile_schemas/Internal.json b/config/v16/profile_schemas/Internal.json index 892ea8ddd..68e0879fc 100644 --- a/config/v16/profile_schemas/Internal.json +++ b/config/v16/profile_schemas/Internal.json @@ -184,6 +184,12 @@ "readOnly": true, "default": true }, + "VerifyCsmsCommonName": { + "$comment": "Verify that the CSMS certificates commonName matches the CSMS FQDN", + "type": "boolean", + "readOnly": true, + "default": true + }, "OcspRequestInterval": { "$comment": "Interval in seconds used to request OCSP revocation status information on the CSO Sub-CA certificates", "type": "integer", diff --git a/include/ocpp/common/websocket/websocket_base.hpp b/include/ocpp/common/websocket/websocket_base.hpp index 82b1b3276..df3c61ebf 100644 --- a/include/ocpp/common/websocket/websocket_base.hpp +++ b/include/ocpp/common/websocket/websocket_base.hpp @@ -34,6 +34,7 @@ struct WebsocketConnectionOptions { bool use_ssl_default_verify_paths; std::optional additional_root_certificate_check; std::optional hostName; + bool verify_csms_common_name; }; /// diff --git a/include/ocpp/v16/charge_point_configuration.hpp b/include/ocpp/v16/charge_point_configuration.hpp index 7ffc22a34..ed3464bb1 100644 --- a/include/ocpp/v16/charge_point_configuration.hpp +++ b/include/ocpp/v16/charge_point_configuration.hpp @@ -79,6 +79,8 @@ class ChargePointConfiguration { KeyValue getSupportedCiphers13KeyValue(); bool getUseSslDefaultVerifyPaths(); KeyValue getUseSslDefaultVerifyPathsKeyValue(); + bool getVerifyCsmsCommonName(); + KeyValue getVerifyCsmsCommonNameKeyValue(); int32_t getRetryBackoffRandomRange(); void setRetryBackoffRandomRange(int32_t retry_backoff_random_range); diff --git a/lib/ocpp/common/websocket/websocket_tls.cpp b/lib/ocpp/common/websocket/websocket_tls.cpp index 7841309c4..71a069671 100644 --- a/lib/ocpp/common/websocket/websocket_tls.cpp +++ b/lib/ocpp/common/websocket/websocket_tls.cpp @@ -14,16 +14,48 @@ namespace ocpp { +static std::vector get_subject_alt_names(const X509* x509) { + std::vector list; + GENERAL_NAMES* subject_alt_names = + static_cast(X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL)); + if (subject_alt_names == nullptr) { + return list; + } + for (int i = 0; i < sk_GENERAL_NAME_num(subject_alt_names); i++) { + GENERAL_NAME* gen = sk_GENERAL_NAME_value(subject_alt_names, i); + if (gen == nullptr) { + continue; + } + if (gen->type == GEN_URI || gen->type == GEN_DNS || gen->type == GEN_EMAIL) { + ASN1_IA5STRING* asn1_str = gen->d.uniformResourceIdentifier; + std::string san = std::string(reinterpret_cast(ASN1_STRING_get0_data(asn1_str)), + ASN1_STRING_length(asn1_str)); + list.push_back(san); + } else if (gen->type == GEN_IPADD) { + unsigned char* ip = gen->d.ip->data; + if (gen->d.ip->length == 4) { // only support IPv4 for now + std::stringstream ip_stream; + ip_stream << static_cast(ip[0]) << '.' << static_cast(ip[1]) << '.' << static_cast(ip[2]) + << '.' << static_cast(ip[3]); + list.push_back(ip_stream.str()); + } + } + } + GENERAL_NAMES_free(subject_alt_names); + return list; +} + // verify that the csms certificate's commonName matches the CSMS FQDN bool verify_csms_cn(const std::string& hostname, bool preverified, boost::asio::ssl::verify_context& ctx) { + /* + FIXME(cc): This does not work, always returns false here if (!preverified) { - EVLOG_error << "Could not verify CSMS server certificate"; - return false; - } + EVLOG_error << "Could not verify CSMS server certificate"; + return false; + }*/ int depth = X509_STORE_CTX_get_error_depth(ctx.native_handle()); - // only check for CSMS server certificate if (depth == 0) { // Get server certificate @@ -37,13 +69,24 @@ bool verify_csms_cn(const std::string& hostname, bool preverified, boost::asio:: return false; } + auto alt_names = get_subject_alt_names(server_cert); + // Compare the extracted CN with the expected FQDN - if (hostname != common_name) { - EVLOG_error << "Server certificate CN does not match CSMS FQDN"; - return false; + if (hostname == common_name) { + EVLOG_info << "FQDN matches CN of server certificate: " << hostname; + return true; + } + + // If the CN does not match, go through all alternative names + for (auto name : alt_names) { + if (hostname == name) { + EVLOG_info << "FQDN matches alternative name of server certificate: " << hostname; + return true; + } } - EVLOG_info << "FQDN matches CN of server certificate"; + EVLOG_info << "FQDN does not match CN or alternative names."; + return false; } return true; @@ -223,11 +266,12 @@ tls_context WebsocketTLS::on_tls_init(std::string hostname, websocketpp::connect EVLOG_AND_THROW(std::runtime_error( "Connecting with security profile 3 but no client side certificate is present or valid")); } - if (SSL_CTX_use_certificate_file(context->native_handle(), - certificate_key_pair.value().certificate_path.c_str(), - SSL_FILETYPE_PEM) != 1) { + EVLOG_info << "Using certificate: " << certificate_key_pair.value().certificate_path; + if (SSL_CTX_use_certificate_chain_file(context->native_handle(), + certificate_key_pair.value().certificate_path.c_str()) != 1) { EVLOG_AND_THROW(std::runtime_error("Could not use client certificate file within SSL context")); } + EVLOG_info << "Using key file: " << certificate_key_pair.value().key_path; if (SSL_CTX_use_PrivateKey_file(context->native_handle(), certificate_key_pair.value().key_path.c_str(), SSL_FILETYPE_PEM) != 1) { EVLOG_AND_THROW(std::runtime_error("Could not set private key file within SSL context")); @@ -235,8 +279,14 @@ tls_context WebsocketTLS::on_tls_init(std::string hostname, websocketpp::connect } context->set_verify_mode(boost::asio::ssl::verify_peer); - context->set_verify_callback(websocketpp::lib::bind( - &verify_csms_cn, hostname, websocketpp::lib::placeholders::_1, websocketpp::lib::placeholders::_2)); + if (this->connection_options.verify_csms_common_name) { + context->set_verify_callback(websocketpp::lib::bind( + &verify_csms_cn, hostname, websocketpp::lib::placeholders::_1, websocketpp::lib::placeholders::_2)); + + } else { + EVLOG_warning << "Not verifying the CSMS certificates commonName with the Fully Qualified Domain Name " + "(FQDN) of the server because it has been explicitly turned off via the configuration!"; + } if (this->evse_security->is_ca_certificate_installed(ocpp::CaCertificateType::CSMS)) { EVLOG_info << "Loading ca csms bundle to verify server certificate: " << this->evse_security->get_verify_file(ocpp::CaCertificateType::CSMS); diff --git a/lib/ocpp/v16/charge_point_configuration.cpp b/lib/ocpp/v16/charge_point_configuration.cpp index 293a5891c..554e2218b 100644 --- a/lib/ocpp/v16/charge_point_configuration.cpp +++ b/lib/ocpp/v16/charge_point_configuration.cpp @@ -305,6 +305,10 @@ bool ChargePointConfiguration::getUseSslDefaultVerifyPaths() { return this->config["Internal"]["UseSslDefaultVerifyPaths"]; } +bool ChargePointConfiguration::getVerifyCsmsCommonName() { + return this->config["Internal"]["VerifyCsmsCommonName"]; +} + KeyValue ChargePointConfiguration::getChargePointIdKeyValue() { KeyValue kv; kv.key = "ChargePointId"; @@ -486,6 +490,14 @@ KeyValue ChargePointConfiguration::getUseSslDefaultVerifyPathsKeyValue() { return kv; } +KeyValue ChargePointConfiguration::getVerifyCsmsCommonNameKeyValue() { + KeyValue kv; + kv.key = "VerifyCsmsCommonName"; + kv.readonly = true; + kv.value.emplace(ocpp::conversions::bool_to_string(this->getVerifyCsmsCommonName())); + return kv; +} + KeyValue ChargePointConfiguration::getWebsocketPingPayloadKeyValue() { KeyValue kv; kv.key = "WebsocketPingPayload"; @@ -2163,6 +2175,9 @@ std::optional ChargePointConfiguration::get(CiString<50> key) { if (key == "UseSslDefaultVerifyPaths") { return this->getUseSslDefaultVerifyPathsKeyValue(); } + if (key == "VerifyCsmsCommonName") { + return this->getVerifyCsmsCommonNameKeyValue(); + } if (key == "OcspRequestInterval") { return this->getOcspRequestIntervalKeyValue(); } diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index 138e8c534..dbe00a698 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -236,7 +236,8 @@ WebsocketConnectionOptions ChargePointImpl::get_ws_connection_options() { this->configuration->getWebsocketPongTimeout(), this->configuration->getUseSslDefaultVerifyPaths(), this->configuration->getAdditionalRootCertificateCheck(), - this->configuration->getHostName()}; + this->configuration->getHostName(), + this->configuration->getVerifyCsmsCommonName()}; return connection_options; } diff --git a/lib/ocpp/v201/charge_point.cpp b/lib/ocpp/v201/charge_point.cpp index bce274d21..990857247 100644 --- a/lib/ocpp/v201/charge_point.cpp +++ b/lib/ocpp/v201/charge_point.cpp @@ -817,7 +817,10 @@ WebsocketConnectionOptions ChargePoint::get_ws_connection_options(const int32_t this->device_model->get_optional_value(ControllerComponentVariables::UseSslDefaultVerifyPaths) .value_or(true), this->device_model->get_optional_value(ControllerComponentVariables::AdditionalRootCertificateCheck) - .value_or(false)}; + .value_or(false), + std::nullopt, // hostName + true // verify_csms_common_name + }; return connection_options; } From 4f87fd893c2e0831461e03c13b38371312b448ee Mon Sep 17 00:00:00 2001 From: Coury Richards <146002925+couryrr-afs@users.noreply.github.com> Date: Mon, 4 Dec 2023 12:44:06 -0500 Subject: [PATCH 17/18] Add build and test to repo Signed-off-by: Coury Richards <146002925+couryrr-afs@users.noreply.github.com> --- .ci/build-kit/install_and_test.sh | 17 ++++++++++ .github/workflows/build_and_test.yaml | 46 +++++++++++++++++++++++++++ .github/workflows/lint.yaml | 21 ------------ .gitignore | 5 ++- tests/database_tests.cpp | 5 +-- 5 files changed, 70 insertions(+), 24 deletions(-) create mode 100755 .ci/build-kit/install_and_test.sh create mode 100644 .github/workflows/build_and_test.yaml delete mode 100644 .github/workflows/lint.yaml diff --git a/.ci/build-kit/install_and_test.sh b/.ci/build-kit/install_and_test.sh new file mode 100755 index 000000000..2d0c47b48 --- /dev/null +++ b/.ci/build-kit/install_and_test.sh @@ -0,0 +1,17 @@ +#!/bin/sh + +set -e + +cmake \ + -B build \ + -S "$EXT_MOUNT/source" \ + -DBUILD_TESTING_LIBOCPP=ON \ + -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_INSTALL_PREFIX="$WORKSPACE_PATH/dist" + +make -j$(nproc) -C build install + +cd ./build/tests/ + +./database_tests +./utils_tests diff --git a/.github/workflows/build_and_test.yaml b/.github/workflows/build_and_test.yaml new file mode 100644 index 000000000..75350c896 --- /dev/null +++ b/.github/workflows/build_and_test.yaml @@ -0,0 +1,46 @@ +name: Build and test libocpp +on: + pull_request: {} + workflow_dispatch: + inputs: + runner: + description: Which runner to use + type: choice + default: 'ubuntu-22.04' + required: true + options: + - 'ubuntu-22.04' + - 'large-ubuntu-22.04-xxl' +jobs: + lint: + name: Install and test + strategy: + matrix: + os: [ubuntu-22.04] + runs-on: ${{ matrix.os }} + steps: + - name: Checkout libocpp + uses: actions/checkout@v3 + with: + path: source + - name: Run clang-format + uses: everest/everest-ci/github-actions/run-clang-format@v1.0.0 + with: + source-dir: source + extensions: hpp,cpp + exclude: cache + - name: Setup run scripts + run: | + mkdir scripts + rsync -a source/.ci/build-kit/ scripts + - name: Pull docker container + run: | + docker pull --platform=linux/x86_64 --quiet ghcr.io/everest/build-kit-alpine:latest + docker image tag ghcr.io/everest/build-kit-alpine:latest build-kit + - name: Run install with tests + run: | + docker run \ + --volume "$(pwd):/ext" \ + --name test-container \ + build-kit run-script install_and_test + diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml deleted file mode 100644 index 862b5a65e..000000000 --- a/.github/workflows/lint.yaml +++ /dev/null @@ -1,21 +0,0 @@ -name: Lint -on: [workflow_dispatch, pull_request] - -jobs: - lint: - name: Lint - strategy: - matrix: - os: [ubuntu-22.04] - runs-on: ${{ matrix.os }} - steps: - - name: Checkout libocpp - uses: actions/checkout@v3 - with: - path: source - - name: Run clang-format - uses: everest/everest-ci/github-actions/run-clang-format@v1.0.0 - with: - source-dir: source - extensions: hpp,cpp - exclude: cache diff --git a/.gitignore b/.gitignore index fe793897e..d2a1bc5ee 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,9 @@ *build* +!.ci/** +!.github/** +!.gitignore *vscode -.cache/ +cache/ workspace.yaml CMakeLists.txt.user !doc/build-with-fetchcontent diff --git a/tests/database_tests.cpp b/tests/database_tests.cpp index 4ad038139..2c487d89d 100644 --- a/tests/database_tests.cpp +++ b/tests/database_tests.cpp @@ -52,8 +52,9 @@ ChargingProfile get_sample_charging_profile() { class DatabaseTest : public ::testing::Test { protected: void SetUp() override { - this->db_handler = std::make_unique(CP_ID, std::filesystem::path("/tmp"), - std::filesystem::path("../../config/v16/init.sql")); + this->db_handler = + std::make_unique(CP_ID, std::filesystem::path("/tmp"), + std::filesystem::path("../../dist/share/everest/modules/OCPP/init.sql")); this->db_handler->open_db_connection(2); } From 844e35154d7ad08eb23936acbbf2a49040d6623f Mon Sep 17 00:00:00 2001 From: Coury Richards <146002925+couryrr-afs@users.noreply.github.com> Date: Mon, 18 Dec 2023 12:09:29 -0500 Subject: [PATCH 18/18] Addressed PR Comments Signed-off-by: Coury Richards <146002925+couryrr-afs@users.noreply.github.com> --- .ci/build-kit/install_and_test.sh | 12 +++++------- .github/workflows/build_and_test.yaml | 24 +++++++++++++++++++++++- .gitignore | 7 +++---- CMakeLists.txt | 4 +++- README.md | 14 +------------- tests/CMakeLists.txt | 1 + tests/database_tests.cpp | 7 ++++--- 7 files changed, 40 insertions(+), 29 deletions(-) diff --git a/.ci/build-kit/install_and_test.sh b/.ci/build-kit/install_and_test.sh index 2d0c47b48..a3b8c3e89 100755 --- a/.ci/build-kit/install_and_test.sh +++ b/.ci/build-kit/install_and_test.sh @@ -1,17 +1,15 @@ -#!/bin/sh +!/bin/sh set -e cmake \ -B build \ -S "$EXT_MOUNT/source" \ - -DBUILD_TESTING_LIBOCPP=ON \ + -G Ninja \ + -DBUILD_TESTING=ON \ -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_INSTALL_PREFIX="$WORKSPACE_PATH/dist" -make -j$(nproc) -C build install +ninja -j$(nproc) -C build install -cd ./build/tests/ - -./database_tests -./utils_tests +ninja -j$(nproc) -C build tests/test \ No newline at end of file diff --git a/.github/workflows/build_and_test.yaml b/.github/workflows/build_and_test.yaml index 75350c896..2a57d2d0a 100644 --- a/.github/workflows/build_and_test.yaml +++ b/.github/workflows/build_and_test.yaml @@ -1,5 +1,10 @@ +# Please reference work here https://github.com/EVerest/everest-core/tree/main/.github/workflows +# TODO: modify to reuse the above workflow to DRY up CI. + name: Build and test libocpp on: + push: + branch: "**" pull_request: {} workflow_dispatch: inputs: @@ -13,7 +18,7 @@ on: - 'large-ubuntu-22.04-xxl' jobs: lint: - name: Install and test + name: Lint strategy: matrix: os: [ubuntu-22.04] @@ -29,6 +34,17 @@ jobs: source-dir: source extensions: hpp,cpp exclude: cache + install_and_test: + name: Install and test + strategy: + matrix: + os: [ubuntu-22.04] + runs-on: ${{ matrix.os }} + steps: + - name: Checkout libocpp + uses: actions/checkout@v3 + with: + path: source - name: Setup run scripts run: | mkdir scripts @@ -43,4 +59,10 @@ jobs: --volume "$(pwd):/ext" \ --name test-container \ build-kit run-script install_and_test + - name: Archive test results + if: always() + uses: actions/upload-artifact@v3 + with: + name: ctest-report + path: /workspace/build/tests/Testing/Temporary/LastTest.log diff --git a/.gitignore b/.gitignore index d2a1bc5ee..8e78b9cb2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,8 @@ -*build* -!.ci/** -!.github/** +/build !.gitignore *vscode -cache/ +.cache/ workspace.yaml CMakeLists.txt.user !doc/build-with-fetchcontent +/dist diff --git a/CMakeLists.txt b/CMakeLists.txt index 182250711..2eed8f2b2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -103,7 +103,9 @@ if(CMAKE_RUN_CLANG_TIDY) -export-fixes=clang-tidy-fixes.yaml) endif() -if(BUILD_TESTING_LIBOCPP) +if(BUILD_TESTING) + enable_testing() + include(CTest) add_subdirectory(tests) endif() diff --git a/README.md b/README.md index dcfe27f8f..ef7b0bbb7 100644 --- a/README.md +++ b/README.md @@ -498,19 +498,7 @@ The main reference for the integration of libocpp for OCPP1.6 is the ocpp::v16:: ## Unit testing -If you want to run the unit tests in the tests subdirectory: install the needed dependencies. -For Debian GNU/Linux 11 you can install it like this: - -```bash -sudo apt install libgtest-dev lcov -python3 -m pip install gcovr -``` - -Run the unit tests - -```bash - cmake .. -DBUILD_TESTING=ON -``` +GTest is required for building the test cases target. To build the target and run the tests you can reference the script `.ci/build-kit/install_and_test.sh`. The script allows the GitHub Actions runner to execute. Local testing is still in progress. ## Building with FetchContent instead of EDM In [doc/build-with-fetchcontent](doc/build-with-fetchcontent) you can find an example how to build libocpp with FetchContent instead of EDM. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 74ef3e953..f142b847b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,3 +1,4 @@ +add_definitions(-D_SQL_INIT_FILE="${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATADIR}/everest/modules/OCPP/init.sql") add_executable(database_tests database_tests.cpp) target_include_directories(database_tests PUBLIC ${GTEST_INCLUDE_DIRS}) diff --git a/tests/database_tests.cpp b/tests/database_tests.cpp index 2c487d89d..b6400f88a 100644 --- a/tests/database_tests.cpp +++ b/tests/database_tests.cpp @@ -10,6 +10,7 @@ namespace ocpp { namespace v16 { +#define SQL_INIT_FILE _SQL_INIT_FILE ChargingProfile get_sample_charging_profile() { ChargingSchedulePeriod period1; @@ -52,9 +53,9 @@ ChargingProfile get_sample_charging_profile() { class DatabaseTest : public ::testing::Test { protected: void SetUp() override { - this->db_handler = - std::make_unique(CP_ID, std::filesystem::path("/tmp"), - std::filesystem::path("../../dist/share/everest/modules/OCPP/init.sql")); + std::cout << SQL_INIT_FILE <db_handler = std::make_unique(CP_ID,std::filesystem::path("/tmp"), + std::filesystem::path(SQL_INIT_FILE)); this->db_handler->open_db_connection(2); }