From 104190c354af7dc38e48022d76fc734cc0560f85 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 10 Nov 2025 15:12:06 +0000 Subject: [PATCH 1/5] fix: setting a configuration key now detects if the key exists feat: set custom key removed and replaced with set key This allows all OCPP 1.6 keys to be set via EVerest internal and stable APIs Signed-off-by: James Chapman --- include/ocpp/v16/charge_point.hpp | 4 +- include/ocpp/v16/charge_point_impl.hpp | 4 +- lib/ocpp/v16/charge_point.cpp | 4 +- lib/ocpp/v16/charge_point_configuration.cpp | 191 ++++++------------- lib/ocpp/v16/charge_point_impl.cpp | 24 +-- tests/lib/ocpp/common/message_queue_test.cpp | 49 +++++ tests/lib/ocpp/v16/CMakeLists.txt | 1 + tests/lib/ocpp/v16/test_configuration.cpp | 48 +++++ 8 files changed, 175 insertions(+), 150 deletions(-) create mode 100644 tests/lib/ocpp/common/message_queue_test.cpp create mode 100644 tests/lib/ocpp/v16/test_configuration.cpp diff --git a/include/ocpp/v16/charge_point.hpp b/include/ocpp/v16/charge_point.hpp index 869055ee9..1a072a737 100644 --- a/include/ocpp/v16/charge_point.hpp +++ b/include/ocpp/v16/charge_point.hpp @@ -632,11 +632,11 @@ class ChargePoint { /// \return a response containing the requested key(s) including the values and unkown keys if present GetConfigurationResponse get_configuration_key(const GetConfigurationRequest& request); - /// \brief Sets a custom configuration key + /// \brief Sets a configuration key /// \param key /// \param value /// \return Indicates the result of the operation - ConfigurationStatus set_custom_configuration_key(CiString<50> key, CiString<500> value); + ConfigurationStatus set_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) diff --git a/include/ocpp/v16/charge_point_impl.hpp b/include/ocpp/v16/charge_point_impl.hpp index deb497dc8..653d427f4 100644 --- a/include/ocpp/v16/charge_point_impl.hpp +++ b/include/ocpp/v16/charge_point_impl.hpp @@ -921,11 +921,11 @@ class ChargePointImpl : ocpp::ChargingStationBase { /// \return a response containing the requested key(s) including the values and unkown keys if present GetConfigurationResponse get_configuration_key(const GetConfigurationRequest& request); - /// \brief Sets a custom configuration key + /// \brief Sets a configuration key /// \param key /// \param value /// \return Indicates the result of the operation - ConfigurationStatus set_custom_configuration_key(CiString<50> key, CiString<500> value); + ConfigurationStatus set_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) diff --git a/lib/ocpp/v16/charge_point.cpp b/lib/ocpp/v16/charge_point.cpp index afe514b17..8b02a89c6 100644 --- a/lib/ocpp/v16/charge_point.cpp +++ b/lib/ocpp/v16/charge_point.cpp @@ -375,8 +375,8 @@ GetConfigurationResponse ChargePoint::get_configuration_key(const GetConfigurati return this->charge_point->get_configuration_key(request); } -ConfigurationStatus ChargePoint::set_custom_configuration_key(CiString<50> key, CiString<500> value) { - return this->charge_point->set_custom_configuration_key(key, value); +ConfigurationStatus ChargePoint::set_configuration_key(CiString<50> key, CiString<500> value) { + return this->charge_point->set_configuration_key(key, value); } void ChargePoint::set_message_queue_resume_delay(std::chrono::seconds delay) { diff --git a/lib/ocpp/v16/charge_point_configuration.cpp b/lib/ocpp/v16/charge_point_configuration.cpp index 94c9d26d7..e472d6dc4 100644 --- a/lib/ocpp/v16/charge_point_configuration.cpp +++ b/lib/ocpp/v16/charge_point_configuration.cpp @@ -3677,8 +3677,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 if (this->setIgnoredProfilePurposesOffline(value) == false) { return ConfigurationStatus::Rejected; } - } - if (key == "AllowOfflineTxForUnknownId") { + } else if (key == "AllowOfflineTxForUnknownId") { if (this->getAllowOfflineTxForUnknownId() == std::nullopt) { return ConfigurationStatus::NotSupported; } @@ -3687,8 +3686,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } else { return ConfigurationStatus::Rejected; } - } - if (key == "AuthorizationCacheEnabled") { + } else if (key == "AuthorizationCacheEnabled") { if (this->getAuthorizationCacheEnabled() == std::nullopt) { return ConfigurationStatus::NotSupported; } @@ -3697,8 +3695,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } else { return ConfigurationStatus::Rejected; } - } - if (key == "AuthorizationKey") { + } else if (key == "AuthorizationKey") { std::string authorization_key = value.get(); if (authorization_key.length() >= AUTHORIZATION_KEY_MIN_LENGTH) { this->setAuthorizationKey(value.get()); @@ -3707,11 +3704,9 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 EVLOG_warning << "Attempt to change AuthorizationKey to value with < 8 characters"; return ConfigurationStatus::Rejected; } - } - if (key == "AuthorizeRemoteTxRequests") { + } else if (key == "AuthorizeRemoteTxRequests") { this->setAuthorizeRemoteTxRequests(ocpp::conversions::string_to_bool(value.get())); - } - if (key == "BlinkRepeat") { + } else if (key == "BlinkRepeat") { if (this->getBlinkRepeat() == std::nullopt) { return ConfigurationStatus::NotSupported; } @@ -3726,8 +3721,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "ClockAlignedDataInterval") { + } else if (key == "ClockAlignedDataInterval") { try { auto [valid, interval] = is_positive_integer(value.get()); if (!valid) { @@ -3739,8 +3733,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "ConnectionTimeOut") { + } else if (key == "ConnectionTimeOut") { try { auto [valid, timeout] = is_positive_integer(value.get()); if (!valid) { @@ -3752,22 +3745,19 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "ConnectorPhaseRotation") { + } else if (key == "ConnectorPhaseRotation") { if (this->isConnectorPhaseRotationValid(value.get())) { this->setConnectorPhaseRotation(value.get()); } else { return ConfigurationStatus::Rejected; } - } - if (key == "CentralContractValidationAllowed") { + } else if (key == "CentralContractValidationAllowed") { if (this->getCentralContractValidationAllowed() == std::nullopt) { return ConfigurationStatus::NotSupported; } else { this->setCentralContractValidationAllowed(ocpp::conversions::string_to_bool(value.get())); } - } - if (key == "CertSigningWaitMinimum") { + } else if (key == "CertSigningWaitMinimum") { if (this->getCertSigningWaitMinimum() == std::nullopt) { return ConfigurationStatus::NotSupported; } else { @@ -3783,8 +3773,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 return ConfigurationStatus::Rejected; } } - } - if (key == "CertSigningRepeatTimes") { + } else if (key == "CertSigningRepeatTimes") { if (this->getCertSigningRepeatTimes() == std::nullopt) { return ConfigurationStatus::NotSupported; } else { @@ -3800,17 +3789,13 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 return ConfigurationStatus::Rejected; } } - } - if (key == "ContractValidationOffline") { + } else if (key == "ContractValidationOffline") { this->setContractValidationOffline(ocpp::conversions::string_to_bool(value.get())); - } - if (key == "CpoName") { + } else if (key == "CpoName") { this->setCpoName(value.get()); - } - if (key == "DisableSecurityEventNotifications") { + } else if (key == "DisableSecurityEventNotifications") { this->setDisableSecurityEventNotifications(ocpp::conversions::string_to_bool(value.get())); - } - if (key == "HeartbeatInterval") { + } else if (key == "HeartbeatInterval") { try { auto [valid, interval] = is_positive_integer(value.get()); if (!valid) { @@ -3822,14 +3807,11 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "ISO15118CertificateManagementEnabled") { + } else if (key == "ISO15118CertificateManagementEnabled") { this->setISO15118CertificateManagementEnabled(ocpp::conversions::string_to_bool(value.get())); - } - if (key == "ISO15118PnCEnabled") { + } else if (key == "ISO15118PnCEnabled") { this->setISO15118PnCEnabled(ocpp::conversions::string_to_bool(value.get())); - } - if (key == "LightIntensity") { + } else if (key == "LightIntensity") { if (this->getLightIntensity() == std::nullopt) { return ConfigurationStatus::NotSupported; } @@ -3844,22 +3826,19 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "LocalAuthorizeOffline") { + } else if (key == "LocalAuthorizeOffline") { if (isBool(value.get())) { this->setLocalAuthorizeOffline(ocpp::conversions::string_to_bool(value.get())); } else { return ConfigurationStatus::Rejected; } - } - if (key == "LocalPreAuthorize") { + } else if (key == "LocalPreAuthorize") { if (isBool(value.get())) { this->setLocalPreAuthorize(ocpp::conversions::string_to_bool(value.get())); } else { return ConfigurationStatus::Rejected; } - } - if (key == "MaxEnergyOnInvalidId") { + } else if (key == "MaxEnergyOnInvalidId") { if (this->getMaxEnergyOnInvalidId() == std::nullopt) { return ConfigurationStatus::NotSupported; } @@ -3874,18 +3853,15 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "MeterValuesAlignedData") { + } else if (key == "MeterValuesAlignedData") { if (!this->setMeterValuesAlignedData(value.get())) { return ConfigurationStatus::Rejected; } - } - if (key == "MeterValuesSampledData") { + } else if (key == "MeterValuesSampledData") { if (!this->setMeterValuesSampledData(value.get())) { return ConfigurationStatus::Rejected; } - } - if (key == "MeterValueSampleInterval") { + } else if (key == "MeterValueSampleInterval") { try { auto [valid, meter_value_sample_interval] = is_positive_integer(value.get()); if (!valid) { @@ -3897,8 +3873,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "MinimumStatusDuration") { + } else if (key == "MinimumStatusDuration") { if (this->getMinimumStatusDuration() == std::nullopt) { return ConfigurationStatus::NotSupported; } @@ -3913,8 +3888,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "OcspRequestInterval") { + } else if (key == "OcspRequestInterval") { try { auto [valid, ocsp_request_interval] = is_positive_integer(value.get()); if (!valid or ocsp_request_interval < 86400) { @@ -3926,8 +3900,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "WaitForStopTransactionsOnResetTimeout") { + } else if (key == "WaitForStopTransactionsOnResetTimeout") { try { auto [valid, wait_for_stop_transactions_on_reset_timeout] = is_positive_integer(value.get()); if (!valid) { @@ -3939,8 +3912,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "ResetRetries") { + } else if (key == "ResetRetries") { try { auto [valid, reset_retries] = is_positive_integer(value.get()); if (!valid) { @@ -3952,25 +3924,21 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "StopTransactionOnInvalidId") { + } else if (key == "StopTransactionOnInvalidId") { if (isBool(value.get())) { this->setStopTransactionOnInvalidId(ocpp::conversions::string_to_bool(value.get())); } else { return ConfigurationStatus::Rejected; } - } - if (key == "StopTxnAlignedData") { + } else if (key == "StopTxnAlignedData") { if (!this->setStopTxnAlignedData(value.get())) { return ConfigurationStatus::Rejected; } - } - if (key == "StopTxnSampledData") { + } else if (key == "StopTxnSampledData") { if (!this->setStopTxnSampledData(value.get())) { return ConfigurationStatus::Rejected; } - } - if (key == "TransactionMessageAttempts") { + } else if (key == "TransactionMessageAttempts") { try { auto [valid, message_attempts] = is_positive_integer(value.get()); if (!valid) { @@ -3982,8 +3950,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "TransactionMessageRetryInterval") { + } else if (key == "TransactionMessageRetryInterval") { try { auto [valid, retry_inverval] = is_positive_integer(value.get()); if (!valid) { @@ -3995,15 +3962,13 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "UnlockConnectorOnEVSideDisconnect") { + } else if (key == "UnlockConnectorOnEVSideDisconnect") { if (isBool(value.get()) and !this->getUnlockConnectorOnEVSideDisconnectKeyValue().readonly) { this->setUnlockConnectorOnEVSideDisconnect(ocpp::conversions::string_to_bool(value.get())); } else { return ConfigurationStatus::Rejected; } - } - if (key == "WebSocketPingInterval") { + } else if (key == "WebSocketPingInterval") { if (this->getWebsocketPingInterval() == std::nullopt) { return ConfigurationStatus::NotSupported; } @@ -4018,17 +3983,14 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "StopTransactionIfUnlockNotSupported") { + } else if (key == "StopTransactionIfUnlockNotSupported") { if (isBool(value.get())) { this->setStopTransactionIfUnlockNotSupported(ocpp::conversions::string_to_bool(value.get())); } else { return ConfigurationStatus::Rejected; } - } - - // Local Auth List Management - if (key == "LocalAuthListEnabled") { + } else if (key == "LocalAuthListEnabled") { + // Local Auth List Management if (this->supported_feature_profiles.count(SupportedFeatureProfiles::LocalAuthListManagement)) { if (isBool(value.get())) { this->setLocalAuthListEnabled(ocpp::conversions::string_to_bool(value.get())); @@ -4038,9 +4000,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } else { return ConfigurationStatus::NotSupported; } - } - - if (key == "CompositeScheduleDefaultLimitAmps") { + } else if (key == "CompositeScheduleDefaultLimitAmps") { if (not this->getCompositeScheduleDefaultLimitAmps().has_value()) { return ConfigurationStatus::NotSupported; } @@ -4055,8 +4015,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "CompositeScheduleDefaultLimitWatts") { + } else if (key == "CompositeScheduleDefaultLimitWatts") { if (not this->getCompositeScheduleDefaultLimitWatts().has_value()) { return ConfigurationStatus::NotSupported; } @@ -4071,8 +4030,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "CompositeScheduleDefaultNumberPhases") { + } else if (key == "CompositeScheduleDefaultNumberPhases") { if (not this->getCompositeScheduleDefaultNumberPhases().has_value()) { return ConfigurationStatus::NotSupported; } @@ -4087,8 +4045,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - if (key == "SupplyVoltage") { + } else if (key == "SupplyVoltage") { if (not this->getSupplyVoltage().has_value()) { return ConfigurationStatus::NotSupported; } @@ -4103,18 +4060,14 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - - if (key == "VerifyCsmsAllowWildcards") { + } else if (key == "VerifyCsmsAllowWildcards") { if (isBool(value.get())) { this->setVerifyCsmsAllowWildcards(ocpp::conversions::string_to_bool(value.get())); } else { return ConfigurationStatus::Rejected; } - } - - // Hubject PnC Extension keys - if (key == "SeccLeafSubjectCommonName") { + } else if (key == "SeccLeafSubjectCommonName") { + // Hubject PnC Extension keys if (this->getSeccLeafSubjectCommonName().has_value()) { if (value.get().length() < SECC_LEAF_SUBJECT_COMMON_NAME_MIN_LENGTH or value.get().length() > SECC_LEAF_SUBJECT_COMMON_NAME_MAX_LENGTH) { @@ -4125,8 +4078,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } else { return ConfigurationStatus::NotSupported; } - } - if (key == "SeccLeafSubjectCountry") { + } else if (key == "SeccLeafSubjectCountry") { if (this->getSeccLeafSubjectCountry().has_value()) { if (value.get().length() != SECC_LEAF_SUBJECT_COUNTRY_LENGTH) { EVLOG_warning << "Attempt to set SeccLeafSubjectCountry with invalid number of characters"; @@ -4136,8 +4088,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } else { return ConfigurationStatus::NotSupported; } - } - if (key == "SeccLeafSubjectOrganization") { + } else if (key == "SeccLeafSubjectOrganization") { if (this->getSeccLeafSubjectOrganization().has_value()) { if (value.get().length() > SECC_LEAF_SUBJECT_ORGANIZATION_MAX_LENGTH) { EVLOG_warning << "Attempt to set SeccLeafSubjectOrganization with invalid number of characters"; @@ -4147,8 +4098,7 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } else { return ConfigurationStatus::NotSupported; } - } - if (key == "ConnectorEvseIds") { + } else if (key == "ConnectorEvseIds") { if (this->getConnectorEvseIds().has_value()) { if (validate_connector_evse_ids(value.get())) { this->setConnectorEvseIds(value.get()); @@ -4158,59 +4108,42 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } else { return ConfigurationStatus::NotSupported; } - } - if (key == "AllowChargingProfileWithoutStartSchedule") { + } else if (key == "AllowChargingProfileWithoutStartSchedule") { if (this->getAllowChargingProfileWithoutStartSchedule().has_value()) { this->setAllowChargingProfileWithoutStartSchedule(ocpp::conversions::string_to_bool(value.get())); } else { return ConfigurationStatus::NotSupported; } - } - - if (key.get().find("DefaultPriceText") == 0) { + } else if (key.get().find("DefaultPriceText") == 0) { const ConfigurationStatus result = this->setDefaultPriceText(key, value); if (result != ConfigurationStatus::Accepted) { return result; } - } - - if (key == "DefaultPrice") { + } else if (key == "DefaultPrice") { const ConfigurationStatus result = this->setDefaultPrice(value); if (result != ConfigurationStatus::Accepted) { return result; } - } - - if (key == "TimeOffset") { + } else if (key == "TimeOffset") { const ConfigurationStatus result = this->setDisplayTimeOffset(value); if (result != ConfigurationStatus::Accepted) { return result; } - } - - if (key == "NextTimeOffsetTransitionDateTime") { + } else if (key == "NextTimeOffsetTransitionDateTime") { const ConfigurationStatus result = this->setNextTimeOffsetTransitionDateTime(value); if (result != ConfigurationStatus::Accepted) { return result; } - } - - if (key == "TimeOffsetNextTransition") { + } else if (key == "TimeOffsetNextTransition") { const ConfigurationStatus result = this->setTimeOffsetNextTransition(value); if (result != ConfigurationStatus::Accepted) { return result; } - } - - if (key == "CustomIdleFeeAfterStop") { + } else if (key == "CustomIdleFeeAfterStop") { this->setCustomIdleFeeAfterStop(ocpp::conversions::string_to_bool(value)); - } - - if (key == "Language") { + } else if (key == "Language") { this->setLanguage(value); - } - - if (key == "WaitForSetUserPriceTimeout") { + } else if (key == "WaitForSetUserPriceTimeout") { try { auto [valid, wait_for_set_user_price_timeout] = is_positive_integer(value.get()); if (!valid) { @@ -4225,15 +4158,13 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 } catch (const std::out_of_range& e) { return ConfigurationStatus::Rejected; } - } - - if (key == "CentralSystemURI") { + } else if (key == "CentralSystemURI") { this->setCentralSystemURI(value.get()); return ConfigurationStatus::RebootRequired; - } - - if (this->config.contains("Custom") and this->config["Custom"].contains(key.get())) { + } else if (this->config.contains("Custom") and this->config["Custom"].contains(key.get())) { return this->setCustomKey(key, value, false); + } else { + return ConfigurationStatus::NotSupported; } return ConfigurationStatus::Accepted; diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index c4e9a768c..413041db9 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -2782,8 +2782,7 @@ void ChargePointImpl::sign_certificate(const ocpp::CertificateSigningUseEnum& ce this->configuration->getCpoName().value(), this->configuration->getChargeBoxSerialNumber(), use_tpm); if (response.status != GetCertificateSignRequestStatus::Accepted || !response.csr.has_value()) { - EVLOG_error << "Create CSR (TPM=" << use_tpm << ")" - << " failed for:" + EVLOG_error << "Create CSR (TPM=" << use_tpm << ")" << " failed for:" << ocpp::conversions::certificate_signing_use_enum_to_string(certificate_signing_use); std::string gen_error = @@ -4872,19 +4871,16 @@ GetConfigurationResponse ChargePointImpl::get_configuration_key(const GetConfigu return response; } -ConfigurationStatus ChargePointImpl::set_custom_configuration_key(CiString<50> key, CiString<500> value) { - // attempt to set the custom key - const auto result = this->configuration->setCustomKey(key, value, true); - if (result != ConfigurationStatus::Accepted) { - // return immediately if not "Accepted" - return result; - } +ConfigurationStatus ChargePointImpl::set_configuration_key(CiString<50> key, CiString<500> value) { + const auto result = this->configuration->set(key, value); - // notify callback if registered and change was accepted - if (this->configuration_key_changed_callbacks.count(key) and - this->configuration_key_changed_callbacks[key] != nullptr and result == ConfigurationStatus::Accepted) { - KeyValue kv = {key, false, value}; - this->configuration_key_changed_callbacks[key](kv); + if (result == ConfigurationStatus::Accepted) { + // notify callback if registered and change was accepted + if (this->configuration_key_changed_callbacks.count(key) and + this->configuration_key_changed_callbacks[key] != nullptr and result == ConfigurationStatus::Accepted) { + KeyValue kv = {key, false, value}; + this->configuration_key_changed_callbacks[key](kv); + } } return result; diff --git a/tests/lib/ocpp/common/message_queue_test.cpp b/tests/lib/ocpp/common/message_queue_test.cpp new file mode 100644 index 000000000..96f5e0f39 --- /dev/null +++ b/tests/lib/ocpp/common/message_queue_test.cpp @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2020 - 2023 Pionix GmbH and Contributors to EVerest + +#include "ocpp/v16/types.hpp" +#include + +#include +#include + +namespace { +using namespace ocpp; + +#if 0 + MessageQueue( + const std::function& send_callback, const MessageQueueConfig& config, + const std::vector& external_notify, std::shared_ptr database_handler, + const std::function + start_transaction_message_retry_callback = + [](const std::string& new_message_id, const std::string& old_message_id) {}) +#endif + +bool send_callback(json message) { + return true; +} + +void start_transaction_message_retry_callback(const std::string& new_message_id, const std::string& old_message_id) { +} + +struct DatabaseHandlerCommonTest : public common::DatabaseHandlerCommon { + DatabaseHandlerCommonTest() : + common::DatabaseHandlerCommon(std::unique_ptr{}, "", 1) { + } + void init_sql() override { + } +}; + +TEST(MessageQueue, init) { + MessageQueueConfig config; + std::vector external_notify; + std::shared_ptr database_handler = std::make_shared(); + + MessageQueue queue(&send_callback, config, external_notify, database_handler, + &start_transaction_message_retry_callback); + + queue.start(); + queue.stop(); +} + +} // namespace \ No newline at end of file diff --git a/tests/lib/ocpp/v16/CMakeLists.txt b/tests/lib/ocpp/v16/CMakeLists.txt index 8ac06fd0c..a8f9e2705 100644 --- a/tests/lib/ocpp/v16/CMakeLists.txt +++ b/tests/lib/ocpp/v16/CMakeLists.txt @@ -14,6 +14,7 @@ target_sources(libocpp_unit_tests PRIVATE test_charge_point_state_machine.cpp test_composite_schedule.cpp test_config_validation.cpp + test_configuration.cpp ) # Copy the json files used for testing to the destination directory diff --git a/tests/lib/ocpp/v16/test_configuration.cpp b/tests/lib/ocpp/v16/test_configuration.cpp new file mode 100644 index 000000000..1536df48e --- /dev/null +++ b/tests/lib/ocpp/v16/test_configuration.cpp @@ -0,0 +1,48 @@ + +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2020 - 2025 Pionix GmbH and Contributors to EVerest + +#include +#include + +#include + +#include + +namespace { +using namespace ocpp::v16; + +struct ConfigurationTester : public testing::Test { + std::unique_ptr config; + + void SetUp() override { + std::ifstream ifs(CONFIG_FILE_LOCATION_V16); + const std::string config_file((std::istreambuf_iterator(ifs)), (std::istreambuf_iterator())); + config = std::make_unique(config_file, CONFIG_DIR_V16, USER_CONFIG_FILE_LOCATION_V16); + } +}; + +TEST_F(ConfigurationTester, SetUnknown) { + auto get_result = config->get("HeartBeatInterval"); + EXPECT_TRUE(get_result.has_value()); + get_result = config->get("DoesNotExist"); + EXPECT_FALSE(get_result.has_value()); + + auto set_result = config->set("HeartBeatInterval", "352"); + EXPECT_EQ(set_result, ConfigurationStatus::Accepted); + set_result = config->set("DoesNotExist", "never-set"); + EXPECT_EQ(set_result, ConfigurationStatus::NotSupported); +} + +TEST_F(ConfigurationTester, BrokenChain) { + // set() has a chain of if .. else if .. + // test that there isn't a missing else + // IgnoredProfilePurposesOffline is the fist key + + // actually returns rejected rather than accepted + // this is fine since the error case would be NotSupported - see SetUnknown + auto set_result = config->set("IgnoredProfilePurposesOffline", "TxProfile"); + EXPECT_NE(set_result, ConfigurationStatus::NotSupported); +} + +} // namespace From 279365985af5f0c030e6024515bbeddac742512b Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 17 Nov 2025 10:01:42 +0000 Subject: [PATCH 2/5] fix: setting keys via methods (rather than OCPP) was not calling actions When some keys are updated then an action is performed. This commit ensures the same actions are preformed regardless of where the update comes from (method call or OCPP) Signed-off-by: James Chapman --- include/ocpp/v16/charge_point_impl.hpp | 2 + lib/ocpp/v16/charge_point_impl.cpp | 282 +++++++++++++------------ 2 files changed, 145 insertions(+), 139 deletions(-) diff --git a/include/ocpp/v16/charge_point_impl.hpp b/include/ocpp/v16/charge_point_impl.hpp index 653d427f4..69ef06844 100644 --- a/include/ocpp/v16/charge_point_impl.hpp +++ b/include/ocpp/v16/charge_point_impl.hpp @@ -926,6 +926,8 @@ class ChargePointImpl : ocpp::ChargingStationBase { /// \param value /// \return Indicates the result of the operation ConfigurationStatus set_configuration_key(CiString<50> key, CiString<500> value); + std::pair> + set_configuration_key_internal(CiString<50> key, CiString<500> value, std::optional uniqueId); /// \brief Delay draining the message queue after reconnecting, so the CSMS can perform post-reconnect checks first /// \param delay The delay period (seconds) diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index 413041db9..c93f56c8e 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -1838,140 +1838,11 @@ void ChargePointImpl::handleChangeAvailabilityRequest(ocpp::Call call) { EVLOG_debug << "Received ChangeConfigurationRequest: " << call.msg << "\nwith messageId: " << call.uniqueId; - ChangeConfigurationResponse response; - // when reconnect or switching security profile the response has to be sent before that - bool responded = false; + auto [result, response] = set_configuration_key_internal(call.msg.key, call.msg.value, call.uniqueId); - auto kv = this->configuration->get(call.msg.key); - if (kv || call.msg.key == "AuthorizationKey") { - if (call.msg.key != "AuthorizationKey" && kv.value().readonly) { - // supported but could not be changed - response.status = ConfigurationStatus::Rejected; - } else { - // TODO(kai): how to signal RebootRequired? or what does need reboot required? - response.status = this->configuration->set(call.msg.key, call.msg.value); - if (response.status == ConfigurationStatus::Accepted) { - if (call.msg.key == "HeartbeatInterval") { - this->update_heartbeat_interval(); - } else if (call.msg.key == "MeterValueSampleInterval") { - this->update_meter_values_sample_interval(); - } else if (call.msg.key == "ClockAlignedDataInterval") { - this->update_clock_aligned_meter_values_interval(); - } else if (call.msg.key == "AuthorizationKey") { - EVLOG_info << "AuthorizationKey was changed by central system"; - this->websocket->set_authorization_key(this->configuration->getAuthorizationKey().value()); - if (this->configuration->getSecurityProfile() == 0) { - EVLOG_info << "AuthorizationKey was changed while on security profile 0."; - } else if (this->configuration->getSecurityProfile() == 1 || - this->configuration->getSecurityProfile() == 2) { - EVLOG_info - << "AuthorizationKey was changed while on security profile 1 or 2. Reconnect Websocket."; - ocpp::CallResult call_result(response, call.uniqueId); - this->message_dispatcher->dispatch_call_result(call_result); - responded = true; - this->websocket->reconnect(1000); - } else { - EVLOG_info << "AuthorizationKey was changed while on security profile 3. Nothing to do."; - } - } else if (call.msg.key == "SecurityProfile") { - try { - const auto security_profile = std::stoi(call.msg.value.get()); - const auto current_security_profile = this->configuration->getSecurityProfile(); - if (security_profile <= current_security_profile) { - EVLOG_warning << "New security profile is <= current security profile. Rejecting request."; - response.status = ConfigurationStatus::Rejected; - } else if ((security_profile == 1 || security_profile == 2) && - this->configuration->getAuthorizationKey() == std::nullopt) { - EVLOG_warning << "New security level set to 1 or 2 but no authorization key is set. " - "Rejecting request."; - response.status = ConfigurationStatus::Rejected; - } else if ((security_profile == 2 || security_profile == 3) && - !this->evse_security->is_ca_certificate_installed(ocpp::CaCertificateType::CSMS)) { - EVLOG_warning - << "New security level set to 2 or 3 but no CentralSystemRootCertificateInstalled"; - response.status = ConfigurationStatus::Rejected; - } else if (security_profile == 3 && - this->evse_security - ->get_leaf_certificate_info( - ocpp::CertificateSigningUseEnum::ChargingStationCertificate) - .status != ocpp::GetCertificateInfoStatus::Accepted) { - EVLOG_warning << "New security level set to 3 but no Client Certificate is installed"; - response.status = ConfigurationStatus::Rejected; - } else if (security_profile > 3) { - response.status = ConfigurationStatus::Rejected; - } else { - // valid set of security profile - ocpp::CallResult call_result(response, call.uniqueId); - this->message_dispatcher->dispatch_call_result(call_result); - int32_t security_profile = std::stoi(call.msg.value); - responded = true; - this->switch_security_profile_callback = [this, security_profile]() { - this->switchSecurityProfile(security_profile, 1); - }; - // disconnected_callback will trigger security_profile_callback when it is set - this->websocket->disconnect(WebsocketCloseReason::Normal); - } - } catch (const std::invalid_argument& e) { - response.status = ConfigurationStatus::Rejected; - } - } else if (call.msg.key == "ConnectionTimeout") { - this->call_set_connection_timeout(); - } else if (call.msg.key == "TransactionMessageAttempts") { - this->message_queue->update_transaction_message_attempts( - this->configuration->getTransactionMessageAttempts()); - } else if (call.msg.key == "TransactionMessageRetryInterval") { - this->message_queue->update_transaction_message_retry_interval( - this->configuration->getTransactionMessageRetryInterval()); - } else if (call.msg.key == "WebSocketPingInterval") { - auto websocket_ping_interval_option = this->configuration->getWebsocketPingInterval(); - - if (websocket_ping_interval_option.has_value()) { - auto websocket_ping_interval = websocket_ping_interval_option.value(); - auto websocket_pong_timeout = this->configuration->getWebsocketPongTimeout(); - - this->websocket->set_websocket_ping_interval(websocket_ping_interval, websocket_pong_timeout); - } - } else if (call.msg.key == "ISO15118CertificateManagementEnabled") { - if (ocpp::conversions::string_to_bool(call.msg.value.get())) { - this->ocsp_request_timer->stop(); - this->ocsp_request_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); - this->v2g_certificate_timer->stop(); - this->v2g_certificate_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); - } else { - this->ocsp_request_timer->stop(); - this->v2g_certificate_timer->stop(); - } - } else if (call.msg.key == "OcspRequestInterval") { - if (this->is_iso15118_certificate_management_enabled()) { - this->ocsp_request_timer->stop(); - this->ocsp_request_timer->interval( - std::chrono::seconds(this->configuration->getOcspRequestInterval())); - } - } else if (call.msg.key == "NextTimeOffsetTransitionDateTime") { - if (this->configuration->getNextTimeOffsetTransitionDateTime().has_value()) { - set_time_offset_timer(this->configuration->getNextTimeOffsetTransitionDateTime().value()); - } - } - } - } - } else { - response.status = ConfigurationStatus::NotSupported; - } - - if (!responded) { - ocpp::CallResult call_result(response, call.uniqueId); - this->message_dispatcher->dispatch_call_result(call_result); - } - - if (this->configuration_key_changed_callbacks.count(call.msg.key) and - this->configuration_key_changed_callbacks[call.msg.key] != nullptr and - response.status == ConfigurationStatus::Accepted) { - kv.value().value = call.msg.value; - this->configuration_key_changed_callbacks[call.msg.key](kv.value()); - } else if (this->generic_configuration_key_changed_callback != nullptr and - response.status == ConfigurationStatus::Accepted) { - kv.value().value = call.msg.value; - this->generic_configuration_key_changed_callback(kv.value()); + if (response) { + ocpp::CallResult call_result(response.value(), call.uniqueId); + message_dispatcher->dispatch_call_result(call_result); } } @@ -4872,18 +4743,151 @@ GetConfigurationResponse ChargePointImpl::get_configuration_key(const GetConfigu } ConfigurationStatus ChargePointImpl::set_configuration_key(CiString<50> key, CiString<500> value) { - const auto result = this->configuration->set(key, value); + // const auto result = this->configuration->set(key, value); + const auto [result, _] = set_configuration_key_internal(key, value, {}); + return result; +} + +std::pair> +ChargePointImpl::set_configuration_key_internal(CiString<50> key, CiString<500> value, + std::optional uniqueId) { + ConfigurationStatus result{ConfigurationStatus::NotSupported}; + std::optional response = ChangeConfigurationResponse(); + const auto kv = configuration->get(key); + + if (kv || key == "AuthorizationKey") { + if (key != "AuthorizationKey" && kv.value().readonly) { + // supported but could not be changed + result = ConfigurationStatus::Rejected; + } else { + // TODO(kai): how to signal RebootRequired? or what does need reboot required? + result = configuration->set(key, value); + if (result == ConfigurationStatus::Accepted) { + if (key == "HeartbeatInterval") { + update_heartbeat_interval(); + } else if (key == "MeterValueSampleInterval") { + update_meter_values_sample_interval(); + } else if (key == "ClockAlignedDataInterval") { + update_clock_aligned_meter_values_interval(); + } else if (key == "AuthorizationKey") { + EVLOG_info << "AuthorizationKey was changed by central system"; + websocket->set_authorization_key(configuration->getAuthorizationKey().value()); + if (configuration->getSecurityProfile() == 0) { + EVLOG_info << "AuthorizationKey was changed while on security profile 0."; + } else if (configuration->getSecurityProfile() == 1 || configuration->getSecurityProfile() == 2) { + EVLOG_info + << "AuthorizationKey was changed while on security profile 1 or 2. Reconnect Websocket."; + if (uniqueId) { + response.value().status = result; + ocpp::CallResult call_result(response.value(), + uniqueId.value()); + message_dispatcher->dispatch_call_result(call_result); + } + response.reset(); // response has been sent + websocket->reconnect(1000); + } else { + EVLOG_info << "AuthorizationKey was changed while on security profile 3. Nothing to do."; + } + } else if (key == "SecurityProfile") { + try { + const auto security_profile = std::stoi(value); + const auto current_security_profile = configuration->getSecurityProfile(); + if (security_profile <= current_security_profile) { + EVLOG_warning << "New security profile is <= current security profile. Rejecting request."; + result = ConfigurationStatus::Rejected; + } else if ((security_profile == 1 || security_profile == 2) && + configuration->getAuthorizationKey() == std::nullopt) { + EVLOG_warning << "New security level set to 1 or 2 but no authorization key is set. " + "Rejecting request."; + result = ConfigurationStatus::Rejected; + } else if ((security_profile == 2 || security_profile == 3) && + !evse_security->is_ca_certificate_installed(ocpp::CaCertificateType::CSMS)) { + EVLOG_warning + << "New security level set to 2 or 3 but no CentralSystemRootCertificateInstalled"; + result = ConfigurationStatus::Rejected; + } else if (security_profile == 3 && + evse_security + ->get_leaf_certificate_info( + ocpp::CertificateSigningUseEnum::ChargingStationCertificate) + .status != ocpp::GetCertificateInfoStatus::Accepted) { + EVLOG_warning << "New security level set to 3 but no Client Certificate is installed"; + result = ConfigurationStatus::Rejected; + } else if (security_profile > 3) { + result = ConfigurationStatus::Rejected; + } else { + // valid set of security profile + if (uniqueId) { + response.value().status = result; + ocpp::CallResult call_result(response.value(), + uniqueId.value()); + message_dispatcher->dispatch_call_result(call_result); + } + response.reset(); // response has been sent + int32_t security_profile = std::stoi(value); + switch_security_profile_callback = [this, security_profile]() { + switchSecurityProfile(security_profile, 1); + }; + // disconnected_callback will trigger security_profile_callback when it is set + websocket->disconnect(WebsocketCloseReason::Normal); + } + } catch (const std::invalid_argument& e) { + result = ConfigurationStatus::Rejected; + } + } else if (key == "ConnectionTimeout") { + call_set_connection_timeout(); + } else if (key == "TransactionMessageAttempts") { + message_queue->update_transaction_message_attempts(configuration->getTransactionMessageAttempts()); + } else if (key == "TransactionMessageRetryInterval") { + message_queue->update_transaction_message_retry_interval( + configuration->getTransactionMessageRetryInterval()); + } else if (key == "WebSocketPingInterval") { + auto websocket_ping_interval_option = configuration->getWebsocketPingInterval(); + + if (websocket_ping_interval_option.has_value()) { + auto websocket_ping_interval = websocket_ping_interval_option.value(); + auto websocket_pong_timeout = configuration->getWebsocketPongTimeout(); + + websocket->set_websocket_ping_interval(websocket_ping_interval, websocket_pong_timeout); + } + } else if (key == "ISO15118CertificateManagementEnabled") { + if (ocpp::conversions::string_to_bool(value)) { + ocsp_request_timer->stop(); + ocsp_request_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); + v2g_certificate_timer->stop(); + v2g_certificate_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); + } else { + ocsp_request_timer->stop(); + v2g_certificate_timer->stop(); + } + } else if (key == "OcspRequestInterval") { + if (is_iso15118_certificate_management_enabled()) { + ocsp_request_timer->stop(); + ocsp_request_timer->interval(std::chrono::seconds(configuration->getOcspRequestInterval())); + } + } else if (key == "NextTimeOffsetTransitionDateTime") { + if (configuration->getNextTimeOffsetTransitionDateTime().has_value()) { + set_time_offset_timer(configuration->getNextTimeOffsetTransitionDateTime().value()); + } + } + } + } + } + + if (response) { + response.value().status = result; + } if (result == ConfigurationStatus::Accepted) { // notify callback if registered and change was accepted - if (this->configuration_key_changed_callbacks.count(key) and - this->configuration_key_changed_callbacks[key] != nullptr and result == ConfigurationStatus::Accepted) { - KeyValue kv = {key, false, value}; - this->configuration_key_changed_callbacks[key](kv); + KeyValue key_value = {key, false, value}; + if (configuration_key_changed_callbacks.count(key) and configuration_key_changed_callbacks[key] != nullptr) { + configuration_key_changed_callbacks[key](key_value); + } else if (generic_configuration_key_changed_callback != nullptr) { + generic_configuration_key_changed_callback(key_value); } } - return result; + return {result, response}; } } // namespace v16 From da45aff12372e6f422fa883c0e4a206e1e99adff Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 17 Nov 2025 14:45:18 +0000 Subject: [PATCH 3/5] fix: clang format update Signed-off-by: James Chapman --- lib/ocpp/v16/charge_point_impl.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index c93f56c8e..a86d0ff9e 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -2653,7 +2653,8 @@ void ChargePointImpl::sign_certificate(const ocpp::CertificateSigningUseEnum& ce this->configuration->getCpoName().value(), this->configuration->getChargeBoxSerialNumber(), use_tpm); if (response.status != GetCertificateSignRequestStatus::Accepted || !response.csr.has_value()) { - EVLOG_error << "Create CSR (TPM=" << use_tpm << ")" << " failed for:" + EVLOG_error << "Create CSR (TPM=" << use_tpm << ")" + << " failed for:" << ocpp::conversions::certificate_signing_use_enum_to_string(certificate_signing_use); std::string gen_error = From 4957555d456bfec9a65f46f689110fbaf9163871 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 21 Nov 2025 13:40:09 +0000 Subject: [PATCH 4/5] fix: integration test failures fix: set configurtaion now returns optional This enables the set function to result nullopt when the key to set doesn't match any expected values. Previously accepted was returned for unknown keys. Signed-off-by: James Chapman --- include/ocpp/v16/charge_point_configuration.hpp | 2 +- lib/ocpp/v16/charge_point_configuration.cpp | 7 +++++-- lib/ocpp/v16/charge_point_impl.cpp | 11 +++++++++-- tests/lib/ocpp/v16/test_configuration.cpp | 6 +++--- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/include/ocpp/v16/charge_point_configuration.hpp b/include/ocpp/v16/charge_point_configuration.hpp index 7f06b3472..932800210 100644 --- a/include/ocpp/v16/charge_point_configuration.hpp +++ b/include/ocpp/v16/charge_point_configuration.hpp @@ -528,7 +528,7 @@ class ChargePointConfiguration { std::vector get_all_key_value(); - ConfigurationStatus set(CiString<50> key, CiString<500> value); + std::optional set(CiString<50> key, CiString<500> value); }; } // namespace v16 diff --git a/lib/ocpp/v16/charge_point_configuration.cpp b/lib/ocpp/v16/charge_point_configuration.cpp index e472d6dc4..7861489c5 100644 --- a/lib/ocpp/v16/charge_point_configuration.cpp +++ b/lib/ocpp/v16/charge_point_configuration.cpp @@ -3671,7 +3671,7 @@ std::vector ChargePointConfiguration::get_all_key_value() { return all; } -ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500> value) { +std::optional ChargePointConfiguration::set(CiString<50> key, CiString<500> value) { std::lock_guard lock(this->configuration_mutex); if (key == "IgnoredProfilePurposesOffline") { if (this->setIgnoredProfilePurposesOffline(value) == false) { @@ -4163,8 +4163,11 @@ ConfigurationStatus ChargePointConfiguration::set(CiString<50> key, CiString<500 return ConfigurationStatus::RebootRequired; } else if (this->config.contains("Custom") and this->config["Custom"].contains(key.get())) { return this->setCustomKey(key, value, false); + } else if (key == "SecurityProfile") { + // do nothing here (key is valid!) } else { - return ConfigurationStatus::NotSupported; + // the key is not one that is recognised for setting + return std::nullopt; } return ConfigurationStatus::Accepted; diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index a86d0ff9e..69b2f0b64 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -1838,7 +1838,7 @@ void ChargePointImpl::handleChangeAvailabilityRequest(ocpp::Call call) { EVLOG_debug << "Received ChangeConfigurationRequest: " << call.msg << "\nwith messageId: " << call.uniqueId; - auto [result, response] = set_configuration_key_internal(call.msg.key, call.msg.value, call.uniqueId); + auto [_, response] = set_configuration_key_internal(call.msg.key, call.msg.value, call.uniqueId); if (response) { ocpp::CallResult call_result(response.value(), call.uniqueId); @@ -4762,7 +4762,14 @@ ChargePointImpl::set_configuration_key_internal(CiString<50> key, CiString<500> result = ConfigurationStatus::Rejected; } else { // TODO(kai): how to signal RebootRequired? or what does need reboot required? - result = configuration->set(key, value); + + // to get here get() has returned a valid key/value result + // set can fail for many reasons and std::nullopt is returned when + // there is no match for the key. As a result the key name is valid + // but not for set, hence using Rejected. + const auto set_result = configuration->set(key, value); + result = set_result.value_or(ConfigurationStatus::Rejected); + if (result == ConfigurationStatus::Accepted) { if (key == "HeartbeatInterval") { update_heartbeat_interval(); diff --git a/tests/lib/ocpp/v16/test_configuration.cpp b/tests/lib/ocpp/v16/test_configuration.cpp index 1536df48e..3243f0095 100644 --- a/tests/lib/ocpp/v16/test_configuration.cpp +++ b/tests/lib/ocpp/v16/test_configuration.cpp @@ -31,7 +31,7 @@ TEST_F(ConfigurationTester, SetUnknown) { auto set_result = config->set("HeartBeatInterval", "352"); EXPECT_EQ(set_result, ConfigurationStatus::Accepted); set_result = config->set("DoesNotExist", "never-set"); - EXPECT_EQ(set_result, ConfigurationStatus::NotSupported); + EXPECT_FALSE(set_result.has_value()); // std::nullopt indicates key not known } TEST_F(ConfigurationTester, BrokenChain) { @@ -40,9 +40,9 @@ TEST_F(ConfigurationTester, BrokenChain) { // IgnoredProfilePurposesOffline is the fist key // actually returns rejected rather than accepted - // this is fine since the error case would be NotSupported - see SetUnknown + // this is fine since the error case would be std::nullopt auto set_result = config->set("IgnoredProfilePurposesOffline", "TxProfile"); - EXPECT_NE(set_result, ConfigurationStatus::NotSupported); + EXPECT_TRUE(set_result.has_value()); } } // namespace From 797bf8eb130d4b91ccbf9524c3510588efa295f1 Mon Sep 17 00:00:00 2001 From: James Chapman Date: Mon, 24 Nov 2025 15:26:27 +0000 Subject: [PATCH 5/5] fix: set_configuration_key_internal now a private method Signed-off-by: James Chapman --- include/ocpp/v16/charge_point_impl.hpp | 12 +- lib/ocpp/v16/charge_point_impl.cpp | 298 ++++++++++++------------- 2 files changed, 159 insertions(+), 151 deletions(-) diff --git a/include/ocpp/v16/charge_point_impl.hpp b/include/ocpp/v16/charge_point_impl.hpp index 69ef06844..a8bd98ee7 100644 --- a/include/ocpp/v16/charge_point_impl.hpp +++ b/include/ocpp/v16/charge_point_impl.hpp @@ -392,6 +392,16 @@ class ChargePointImpl : ocpp::ChargingStationBase { /// \param connector for which availability change shall be checked and executed void execute_queued_availability_change(const int32_t connector); + /// \brief Sets a configuration key (internal implementation) + /// \param key + /// \param value + /// \param uniqueId used when an OCPP response is sent + /// \return Indicates the result of the operation with an optional response message + /// \note the optional response message will be nullopt when a response has + /// been sent by this method + std::pair> + set_configuration_key_internal(CiString<50> key, CiString<500> value, std::optional uniqueId); + public: /// \brief The main entrypoint for libOCPP for OCPP 1.6 /// \param config a nlohmann json config object that contains the libocpp 1.6 config. There are example configs that @@ -926,8 +936,6 @@ class ChargePointImpl : ocpp::ChargingStationBase { /// \param value /// \return Indicates the result of the operation ConfigurationStatus set_configuration_key(CiString<50> key, CiString<500> value); - std::pair> - set_configuration_key_internal(CiString<50> key, CiString<500> value, std::optional uniqueId); /// \brief Delay draining the message queue after reconnecting, so the CSMS can perform post-reconnect checks first /// \param delay The delay period (seconds) diff --git a/lib/ocpp/v16/charge_point_impl.cpp b/lib/ocpp/v16/charge_point_impl.cpp index 69b2f0b64..1f5142087 100644 --- a/lib/ocpp/v16/charge_point_impl.cpp +++ b/lib/ocpp/v16/charge_point_impl.cpp @@ -1815,6 +1815,155 @@ void ChargePointImpl::execute_queued_availability_change(const int32_t connector } } +std::pair> +ChargePointImpl::set_configuration_key_internal(CiString<50> key, CiString<500> value, + std::optional uniqueId) { + ConfigurationStatus result{ConfigurationStatus::NotSupported}; + std::optional response = ChangeConfigurationResponse(); + const auto kv = configuration->get(key); + + if (kv || key == "AuthorizationKey") { + if (key != "AuthorizationKey" && kv.value().readonly) { + // supported but could not be changed + result = ConfigurationStatus::Rejected; + } else { + // TODO(kai): how to signal RebootRequired? or what does need reboot required? + + // to get here get() has returned a valid key/value result + // set can fail for many reasons and std::nullopt is returned when + // there is no match for the key. As a result the key name is valid + // but not for set, hence using Rejected. + const auto set_result = configuration->set(key, value); + result = set_result.value_or(ConfigurationStatus::Rejected); + + if (result == ConfigurationStatus::Accepted) { + if (key == "HeartbeatInterval") { + update_heartbeat_interval(); + } else if (key == "MeterValueSampleInterval") { + update_meter_values_sample_interval(); + } else if (key == "ClockAlignedDataInterval") { + update_clock_aligned_meter_values_interval(); + } else if (key == "AuthorizationKey") { + EVLOG_info << "AuthorizationKey was changed by central system"; + websocket->set_authorization_key(configuration->getAuthorizationKey().value()); + if (configuration->getSecurityProfile() == 0) { + EVLOG_info << "AuthorizationKey was changed while on security profile 0."; + } else if (configuration->getSecurityProfile() == 1 || configuration->getSecurityProfile() == 2) { + EVLOG_info + << "AuthorizationKey was changed while on security profile 1 or 2. Reconnect Websocket."; + if (uniqueId) { + response.value().status = result; + ocpp::CallResult call_result(response.value(), + uniqueId.value()); + message_dispatcher->dispatch_call_result(call_result); + } + response.reset(); // response has been sent + websocket->reconnect(1000); + } else { + EVLOG_info << "AuthorizationKey was changed while on security profile 3. Nothing to do."; + } + } else if (key == "SecurityProfile") { + try { + const auto security_profile = std::stoi(value); + const auto current_security_profile = configuration->getSecurityProfile(); + if (security_profile <= current_security_profile) { + EVLOG_warning << "New security profile is <= current security profile. Rejecting request."; + result = ConfigurationStatus::Rejected; + } else if ((security_profile == 1 || security_profile == 2) && + configuration->getAuthorizationKey() == std::nullopt) { + EVLOG_warning << "New security level set to 1 or 2 but no authorization key is set. " + "Rejecting request."; + result = ConfigurationStatus::Rejected; + } else if ((security_profile == 2 || security_profile == 3) && + !evse_security->is_ca_certificate_installed(ocpp::CaCertificateType::CSMS)) { + EVLOG_warning + << "New security level set to 2 or 3 but no CentralSystemRootCertificateInstalled"; + result = ConfigurationStatus::Rejected; + } else if (security_profile == 3 && + evse_security + ->get_leaf_certificate_info( + ocpp::CertificateSigningUseEnum::ChargingStationCertificate) + .status != ocpp::GetCertificateInfoStatus::Accepted) { + EVLOG_warning << "New security level set to 3 but no Client Certificate is installed"; + result = ConfigurationStatus::Rejected; + } else if (security_profile > 3) { + result = ConfigurationStatus::Rejected; + } else { + // valid set of security profile + if (uniqueId) { + response.value().status = result; + ocpp::CallResult call_result(response.value(), + uniqueId.value()); + message_dispatcher->dispatch_call_result(call_result); + } + response.reset(); // response has been sent + int32_t security_profile = std::stoi(value); + switch_security_profile_callback = [this, security_profile]() { + switchSecurityProfile(security_profile, 1); + }; + // disconnected_callback will trigger security_profile_callback when it is set + websocket->disconnect(WebsocketCloseReason::Normal); + } + } catch (const std::invalid_argument& e) { + result = ConfigurationStatus::Rejected; + } + } else if (key == "ConnectionTimeout") { + call_set_connection_timeout(); + } else if (key == "TransactionMessageAttempts") { + message_queue->update_transaction_message_attempts(configuration->getTransactionMessageAttempts()); + } else if (key == "TransactionMessageRetryInterval") { + message_queue->update_transaction_message_retry_interval( + configuration->getTransactionMessageRetryInterval()); + } else if (key == "WebSocketPingInterval") { + auto websocket_ping_interval_option = configuration->getWebsocketPingInterval(); + + if (websocket_ping_interval_option.has_value()) { + auto websocket_ping_interval = websocket_ping_interval_option.value(); + auto websocket_pong_timeout = configuration->getWebsocketPongTimeout(); + + websocket->set_websocket_ping_interval(websocket_ping_interval, websocket_pong_timeout); + } + } else if (key == "ISO15118CertificateManagementEnabled") { + if (ocpp::conversions::string_to_bool(value)) { + ocsp_request_timer->stop(); + ocsp_request_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); + v2g_certificate_timer->stop(); + v2g_certificate_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); + } else { + ocsp_request_timer->stop(); + v2g_certificate_timer->stop(); + } + } else if (key == "OcspRequestInterval") { + if (is_iso15118_certificate_management_enabled()) { + ocsp_request_timer->stop(); + ocsp_request_timer->interval(std::chrono::seconds(configuration->getOcspRequestInterval())); + } + } else if (key == "NextTimeOffsetTransitionDateTime") { + if (configuration->getNextTimeOffsetTransitionDateTime().has_value()) { + set_time_offset_timer(configuration->getNextTimeOffsetTransitionDateTime().value()); + } + } + } + } + } + + if (response) { + response.value().status = result; + } + + if (result == ConfigurationStatus::Accepted) { + // notify callback if registered and change was accepted + KeyValue key_value = {key, false, value}; + if (configuration_key_changed_callbacks.count(key) and configuration_key_changed_callbacks[key] != nullptr) { + configuration_key_changed_callbacks[key](key_value); + } else if (generic_configuration_key_changed_callback != nullptr) { + generic_configuration_key_changed_callback(key_value); + } + } + + return {result, response}; +} + void ChargePointImpl::handleChangeAvailabilityRequest(ocpp::Call call) { EVLOG_debug << "Received ChangeAvailabilityRequest: " << call.msg << "\nwith messageId: " << call.uniqueId; @@ -4749,154 +4898,5 @@ ConfigurationStatus ChargePointImpl::set_configuration_key(CiString<50> key, CiS return result; } -std::pair> -ChargePointImpl::set_configuration_key_internal(CiString<50> key, CiString<500> value, - std::optional uniqueId) { - ConfigurationStatus result{ConfigurationStatus::NotSupported}; - std::optional response = ChangeConfigurationResponse(); - const auto kv = configuration->get(key); - - if (kv || key == "AuthorizationKey") { - if (key != "AuthorizationKey" && kv.value().readonly) { - // supported but could not be changed - result = ConfigurationStatus::Rejected; - } else { - // TODO(kai): how to signal RebootRequired? or what does need reboot required? - - // to get here get() has returned a valid key/value result - // set can fail for many reasons and std::nullopt is returned when - // there is no match for the key. As a result the key name is valid - // but not for set, hence using Rejected. - const auto set_result = configuration->set(key, value); - result = set_result.value_or(ConfigurationStatus::Rejected); - - if (result == ConfigurationStatus::Accepted) { - if (key == "HeartbeatInterval") { - update_heartbeat_interval(); - } else if (key == "MeterValueSampleInterval") { - update_meter_values_sample_interval(); - } else if (key == "ClockAlignedDataInterval") { - update_clock_aligned_meter_values_interval(); - } else if (key == "AuthorizationKey") { - EVLOG_info << "AuthorizationKey was changed by central system"; - websocket->set_authorization_key(configuration->getAuthorizationKey().value()); - if (configuration->getSecurityProfile() == 0) { - EVLOG_info << "AuthorizationKey was changed while on security profile 0."; - } else if (configuration->getSecurityProfile() == 1 || configuration->getSecurityProfile() == 2) { - EVLOG_info - << "AuthorizationKey was changed while on security profile 1 or 2. Reconnect Websocket."; - if (uniqueId) { - response.value().status = result; - ocpp::CallResult call_result(response.value(), - uniqueId.value()); - message_dispatcher->dispatch_call_result(call_result); - } - response.reset(); // response has been sent - websocket->reconnect(1000); - } else { - EVLOG_info << "AuthorizationKey was changed while on security profile 3. Nothing to do."; - } - } else if (key == "SecurityProfile") { - try { - const auto security_profile = std::stoi(value); - const auto current_security_profile = configuration->getSecurityProfile(); - if (security_profile <= current_security_profile) { - EVLOG_warning << "New security profile is <= current security profile. Rejecting request."; - result = ConfigurationStatus::Rejected; - } else if ((security_profile == 1 || security_profile == 2) && - configuration->getAuthorizationKey() == std::nullopt) { - EVLOG_warning << "New security level set to 1 or 2 but no authorization key is set. " - "Rejecting request."; - result = ConfigurationStatus::Rejected; - } else if ((security_profile == 2 || security_profile == 3) && - !evse_security->is_ca_certificate_installed(ocpp::CaCertificateType::CSMS)) { - EVLOG_warning - << "New security level set to 2 or 3 but no CentralSystemRootCertificateInstalled"; - result = ConfigurationStatus::Rejected; - } else if (security_profile == 3 && - evse_security - ->get_leaf_certificate_info( - ocpp::CertificateSigningUseEnum::ChargingStationCertificate) - .status != ocpp::GetCertificateInfoStatus::Accepted) { - EVLOG_warning << "New security level set to 3 but no Client Certificate is installed"; - result = ConfigurationStatus::Rejected; - } else if (security_profile > 3) { - result = ConfigurationStatus::Rejected; - } else { - // valid set of security profile - if (uniqueId) { - response.value().status = result; - ocpp::CallResult call_result(response.value(), - uniqueId.value()); - message_dispatcher->dispatch_call_result(call_result); - } - response.reset(); // response has been sent - int32_t security_profile = std::stoi(value); - switch_security_profile_callback = [this, security_profile]() { - switchSecurityProfile(security_profile, 1); - }; - // disconnected_callback will trigger security_profile_callback when it is set - websocket->disconnect(WebsocketCloseReason::Normal); - } - } catch (const std::invalid_argument& e) { - result = ConfigurationStatus::Rejected; - } - } else if (key == "ConnectionTimeout") { - call_set_connection_timeout(); - } else if (key == "TransactionMessageAttempts") { - message_queue->update_transaction_message_attempts(configuration->getTransactionMessageAttempts()); - } else if (key == "TransactionMessageRetryInterval") { - message_queue->update_transaction_message_retry_interval( - configuration->getTransactionMessageRetryInterval()); - } else if (key == "WebSocketPingInterval") { - auto websocket_ping_interval_option = configuration->getWebsocketPingInterval(); - - if (websocket_ping_interval_option.has_value()) { - auto websocket_ping_interval = websocket_ping_interval_option.value(); - auto websocket_pong_timeout = configuration->getWebsocketPongTimeout(); - - websocket->set_websocket_ping_interval(websocket_ping_interval, websocket_pong_timeout); - } - } else if (key == "ISO15118CertificateManagementEnabled") { - if (ocpp::conversions::string_to_bool(value)) { - ocsp_request_timer->stop(); - ocsp_request_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); - v2g_certificate_timer->stop(); - v2g_certificate_timer->timeout(INITIAL_CERTIFICATE_REQUESTS_DELAY); - } else { - ocsp_request_timer->stop(); - v2g_certificate_timer->stop(); - } - } else if (key == "OcspRequestInterval") { - if (is_iso15118_certificate_management_enabled()) { - ocsp_request_timer->stop(); - ocsp_request_timer->interval(std::chrono::seconds(configuration->getOcspRequestInterval())); - } - } else if (key == "NextTimeOffsetTransitionDateTime") { - if (configuration->getNextTimeOffsetTransitionDateTime().has_value()) { - set_time_offset_timer(configuration->getNextTimeOffsetTransitionDateTime().value()); - } - } - } - } - } - - if (response) { - response.value().status = result; - } - - if (result == ConfigurationStatus::Accepted) { - // notify callback if registered and change was accepted - KeyValue key_value = {key, false, value}; - if (configuration_key_changed_callbacks.count(key) and configuration_key_changed_callbacks[key] != nullptr) { - configuration_key_changed_callbacks[key](key_value); - } else if (generic_configuration_key_changed_callback != nullptr) { - generic_configuration_key_changed_callback(key_value); - } - } - - return {result, response}; -} - } // namespace v16 } // namespace ocpp