-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPP 2.1 messages #845
base: main
Are you sure you want to change the base?
OCPP 2.1 messages #845
Conversation
@@ -74,8 +74,8 @@ struct Callbacks { | |||
/// \return True if evse is reserved for the given id token / group id token, false if it is reserved for another | |||
/// one. | |||
/// | |||
std::function<bool(const int32_t evse_id, const CiString<36> idToken, | |||
const std::optional<CiString<36>> groupIdToken)> | |||
std::function<bool(const int32_t evse_id, const CiString<255> idToken, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really a backwards-compatible change in the 2.1 schemas. Changes like this happened in a few different messages/types and need case-by-case handling. Either by adding additional conversion functions or different message generation for 2.0.1 and 2.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ok to accept larger tokens here. This is only an issue if e.g. OCTT tests for OCPP2.0.1 if we accept larger tokens.
@@ -19,7 +19,7 @@ namespace v201 { | |||
struct AuthorizeRequest : public ocpp::Message { | |||
IdToken idToken; | |||
std::optional<CustomData> customData; | |||
std::optional<CiString<5500>> certificate; | |||
std::optional<CiString<10000>> certificate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could probably be reported via FieldLength
variable for the AuthorizeRequest.certificate
instance which should make this change backwards-compatible with 2.0.1
@@ -20,8 +20,10 @@ namespace v201 { | |||
struct Get15118EVCertificateRequest : public ocpp::Message { | |||
CiString<50> iso15118SchemaVersion; | |||
CertificateActionEnum action; | |||
CiString<5600> exiRequest; | |||
CiString<11000> exiRequest; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a FieldLength
variable for Get15118EVCertificateRequest.exiRequest
should make this backwards-compatible with 2.0.1
@@ -40,7 +40,7 @@ struct GetCertificateStatusResponse : public ocpp::Message { | |||
GetCertificateStatusEnum status; | |||
std::optional<CustomData> customData; | |||
std::optional<StatusInfo> statusInfo; | |||
std::optional<CiString<5500>> ocspResult; | |||
std::optional<CiString<18000>> ocspResult; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a FieldLength
variable for GetCertificateStatusResponse.ocspResult
should make this backwards-compatible with 2.0.1
@@ -18,7 +18,7 @@ namespace v201 { | |||
/// \brief Contains a OCPP InstallCertificate message | |||
struct InstallCertificateRequest : public ocpp::Message { | |||
InstallCertificateUseEnum certificateType; | |||
CiString<5500> certificate; | |||
CiString<10000> certificate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a FieldLength
variable for InstallCertificateRequest.certificate
should make this backwards-compatible with 2.0.1
@@ -17,7 +17,7 @@ namespace v201 { | |||
|
|||
/// \brief Contains a OCPP PublishFirmware message | |||
struct PublishFirmwareRequest : public ocpp::Message { | |||
CiString<512> location; | |||
CiString<2000> location; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a FieldLength
variable for PublishFirmwareRequest.location
should make this backwards-compatible with 2.0.1
@@ -19,8 +19,9 @@ namespace v201 { | |||
struct PublishFirmwareStatusNotificationRequest : public ocpp::Message { | |||
PublishFirmwareStatusEnum status; | |||
std::optional<CustomData> customData; | |||
std::optional<std::vector<CiString<512>>> location; | |||
std::optional<std::vector<CiString<2000>>> location; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a FieldLength
variable for PublishFirmwareStatusNotificationRequest.location
should make this backwards-compatible with 2.0.1
std::optional<std::vector<MeterValue>> meterValue; | ||
std::optional<bool> offline; | ||
std::optional<int32_t> numberOfPhasesUsed; | ||
std::optional<int32_t> cableMaxCurrent; | ||
std::optional<float> cableMaxCurrent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't strictly backwards compatible with 2.0.1 and probably needs additional handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change doesnt seem to make a lot of sense. In transaction_event_req
we could continue to use an int
and make an implicit cast to float
We should make sure that the float is serialized to an int in 2.0.1 when transmitting to the CSMS.
include/ocpp/v201/ocpp_types.hpp
Outdated
struct AdditionalInfo { | ||
CiString<36> additionalIdToken; | ||
CiString<255> additionalIdToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our best chance is to allow 255 here since we can then try to send to the CSMS. If the CSMS can handle it - fine. If it cant, we have at least tried and the CSMS will likely respond with a CALLERROR
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -34,8 +64,8 @@ void from_json(const json& j, AdditionalInfo& k); | |||
std::ostream& operator<<(std::ostream& os, const AdditionalInfo& k); | |||
|
|||
struct IdToken { | |||
CiString<36> idToken; | |||
IdTokenEnum type; | |||
CiString<255> idToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our best chance is to allow 255 here since we can then try to send to the CSMS. If the CSMS can handle it - fine. If it cant, we have at least tried and the CSMS will likely respond with a CALLERROR
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -54,7 +84,7 @@ struct OCSPRequestData { | |||
CiString<128> issuerNameHash; | |||
CiString<128> issuerKeyHash; | |||
CiString<40> serialNumber; | |||
CiString<512> responderURL; | |||
CiString<2000> responderURL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same argument like with the id token. Lets allow the larger value here.
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -69,7 +99,7 @@ std::ostream& operator<<(std::ostream& os, const OCSPRequestData& k); | |||
|
|||
struct MessageContent { | |||
MessageFormatEnum format; | |||
CiString<512> content; | |||
CiString<1024> content; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow since its from CSMS
include/ocpp/v201/ocpp_types.hpp
Outdated
std::optional<CustomData> customData; | ||
std::optional<float> limit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
This was required in 2.0.1 and is optional now in 2.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats ok as long as we make sure that we always set this limit for v201 (which we do because it was required before).
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -285,7 +893,7 @@ void from_json(const json& j, CertificateHashDataChain& k); | |||
std::ostream& operator<<(std::ostream& os, const CertificateHashDataChain& k); | |||
|
|||
struct LogParameters { | |||
CiString<512> remoteLocation; | |||
CiString<2000> remoteLocation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ok
include/ocpp/v201/ocpp_types.hpp
Outdated
std::optional<CiString<50>> signingMethod; | ||
std::optional<CiString<2500>> publicKey; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
These two fields were required in 2.0.1 and are now optional in 2.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we should implement different handling for 2.0.1 and 2.1 in the respective message handling
include/ocpp/v201/ocpp_types.hpp
Outdated
float energyAmount; | ||
float evMinCurrent; | ||
float evMaxCurrent; | ||
float evMaxVoltage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
type change from int to float
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept the change. We dont have any handling in libocpp with this type. We should implement different handling for 2.0.1 and 2.1 and make sure that the float is serialized to an int in 2.0.1 when transmitting to the CSMS.
include/ocpp/v201/ocpp_types.hpp
Outdated
float evMaxCurrent; | ||
float evMaxVoltage; | ||
std::optional<CustomData> customData; | ||
std::optional<int32_t> energyAmount; | ||
std::optional<int32_t> evMaxPower; | ||
std::optional<float> evMaxPower; | ||
std::optional<float> evEnergyCapacity; | ||
std::optional<float> energyAmount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
type change from int to float
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept the change. We dont have any handling in libocpp with this type. We should implement different handling for 2.0.1 and 2.1 and make sure that the float is serialized to an int in 2.0.1 when transmitting to the CSMS.
include/ocpp/v201/ocpp_types.hpp
Outdated
CiString<255> key; | ||
VPNEnum type; | ||
std::optional<CustomData> customData; | ||
std::optional<CiString<20>> group; | ||
std::optional<CiString<50>> group; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept
include/ocpp/v201/ocpp_types.hpp
Outdated
int32_t messageTimeout; | ||
CiString<2000> ocppCsmsUrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -890,7 +1992,7 @@ void from_json(const json& j, SetMonitoringResult& k); | |||
std::ostream& operator<<(std::ostream& os, const SetMonitoringResult& k); | |||
|
|||
struct SetVariableData { | |||
CiString<1000> attributeValue; | |||
CiString<2500> attributeValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -943,7 +2132,7 @@ void from_json(const json& j, Transaction& k); | |||
std::ostream& operator<<(std::ostream& os, const Transaction& k); | |||
|
|||
struct Firmware { | |||
CiString<512> location; | |||
CiString<2000> location; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept
include/ocpp/v201/types.hpp
Outdated
namespace ChargingLimitSourceEnumStringType { | ||
inline const CiString<20> EMS = "EMS"; | ||
inline const CiString<20> OTHER = "Other"; | ||
inline const CiString<20> SO = "SO"; | ||
inline const CiString<20> CSO = "CSO"; | ||
} // namespace ChargingLimitSourceEnumStringType | ||
|
||
namespace IdTokenEnumStringType { | ||
inline const CiString<20> Value = "Value"; | ||
inline const CiString<20> Central = "Central"; | ||
inline const CiString<20> DirectPayment = "DirectPayment"; | ||
inline const CiString<20> eMAID = "eMAID"; | ||
inline const CiString<20> EVCCID = "EVCCID"; | ||
inline const CiString<20> ISO14443 = "ISO14443"; | ||
inline const CiString<20> ISO15693 = "ISO15693"; | ||
inline const CiString<20> KeyCode = "KeyCode"; | ||
inline const CiString<20> Local = "Local"; | ||
inline const CiString<20> MacAddress = "MacAddress"; | ||
inline const CiString<20> NEMA = "NEMA"; | ||
inline const CiString<20> NoAuthorization = "NoAuthorization"; | ||
inline const CiString<20> VIN = "VIN"; | ||
} // namespace IdTokenEnumStringType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: move this to constants.hpp
lib/ocpp/v201/charge_point.cpp
Outdated
@@ -792,7 +792,7 @@ std::optional<std::string> ChargePoint::get_evse_transaction_id(int32_t evse_id) | |||
return evse.get_transaction()->transactionId.get(); | |||
} | |||
|
|||
AuthorizeResponse ChargePoint::validate_token(const IdToken id_token, const std::optional<CiString<5500>>& certificate, | |||
AuthorizeResponse ChargePoint::validate_token(const IdToken id_token, const std::optional<CiString<10000>>& certificate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accept
lib/ocpp/v201/charge_point.cpp
Outdated
@@ -1929,8 +1929,8 @@ bool ChargePoint::is_evse_reserved_for_other(EvseInterface& evse, const IdToken& | |||
const ConnectorStatusEnum status = | |||
evse.get_connector(static_cast<int32_t>(i))->get_effective_connector_status(); | |||
if (status == ConnectorStatusEnum::Reserved) { | |||
const std::optional<CiString<36>> groupIdToken = | |||
group_id_token.has_value() ? group_id_token.value().idToken : std::optional<CiString<36>>{}; | |||
const std::optional<CiString<255>> groupIdToken = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accept
lib/ocpp/v201/charge_point.cpp
Outdated
@@ -2113,7 +2113,7 @@ void ChargePoint::notify_report_req(const int request_id, const std::vector<Repo | |||
} | |||
} | |||
|
|||
AuthorizeResponse ChargePoint::authorize_req(const IdToken id_token, const std::optional<CiString<5500>>& certificate, | |||
AuthorizeResponse ChargePoint::authorize_req(const IdToken id_token, const std::optional<CiString<10000>>& certificate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accept
lib/ocpp/v201/profile.cpp
Outdated
@@ -102,7 +102,7 @@ void period_entry_t::init(const DateTime& in_start, int in_duration, const Charg | |||
const auto start_tp = std::chrono::floor<seconds>(in_start.to_time_point()); | |||
start = std::move(DateTime(start_tp + seconds(in_period.startPeriod))); | |||
end = std::move(DateTime(start_tp + seconds(in_duration))); | |||
limit = in_period.limit; | |||
limit = in_period.limit.value_or(0); // FIXME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
(limit became optional)
lib/ocpp/v201/profile.cpp
Outdated
} else if (candidate_period.limit.has_value()) { | ||
const auto charge_point_max_phases = candidate_period.numberPhases.value_or(default_number_phases); | ||
|
||
const auto period_max_phases = prevailing_period.numberPhases.value_or(default_number_phases); | ||
adjusted_period.numberPhases = std::min(charge_point_max_phases, period_max_phases); | ||
|
||
if (current_charging_rate_unit_enum == ChargingRateUnitEnum::A) { | ||
if (candidate_period.limit < prevailing_period.limit) { | ||
adjusted_period.limit = candidate_period.limit; | ||
if (candidate_period.limit.value() < prevailing_period.limit.value_or(DEFAULT_LIMIT_AMPS)) { | ||
adjusted_period.limit = candidate_period.limit.value(); | ||
} | ||
} else { | ||
const auto charge_point_limit_per_phase = candidate_period.limit / charge_point_max_phases; | ||
const auto period_limit_per_phase = prevailing_period.limit / period_max_phases; | ||
const auto charge_point_limit_per_phase = candidate_period.limit.value() / charge_point_max_phases; | ||
const auto period_limit_per_phase = prevailing_period.limit.value_or(0) / period_max_phases; // FIXME | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: find out implications for backwards compatibility with 2.0.1
(limit became optional)
@@ -74,8 +74,8 @@ struct Callbacks { | |||
/// \return True if evse is reserved for the given id token / group id token, false if it is reserved for another | |||
/// one. | |||
/// | |||
std::function<bool(const int32_t evse_id, const CiString<36> idToken, | |||
const std::optional<CiString<36>> groupIdToken)> | |||
std::function<bool(const int32_t evse_id, const CiString<255> idToken, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ok to accept larger tokens here. This is only an issue if e.g. OCTT tests for OCPP2.0.1 if we accept larger tokens.
std::optional<std::vector<MeterValue>> meterValue; | ||
std::optional<bool> offline; | ||
std::optional<int32_t> numberOfPhasesUsed; | ||
std::optional<int32_t> cableMaxCurrent; | ||
std::optional<float> cableMaxCurrent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change doesnt seem to make a lot of sense. In transaction_event_req
we could continue to use an int
and make an implicit cast to float
We should make sure that the float is serialized to an int in 2.0.1 when transmitting to the CSMS.
include/ocpp/v201/ocpp_types.hpp
Outdated
struct AdditionalInfo { | ||
CiString<36> additionalIdToken; | ||
CiString<255> additionalIdToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our best chance is to allow 255 here since we can then try to send to the CSMS. If the CSMS can handle it - fine. If it cant, we have at least tried and the CSMS will likely respond with a CALLERROR
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -34,8 +64,8 @@ void from_json(const json& j, AdditionalInfo& k); | |||
std::ostream& operator<<(std::ostream& os, const AdditionalInfo& k); | |||
|
|||
struct IdToken { | |||
CiString<36> idToken; | |||
IdTokenEnum type; | |||
CiString<255> idToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our best chance is to allow 255 here since we can then try to send to the CSMS. If the CSMS can handle it - fine. If it cant, we have at least tried and the CSMS will likely respond with a CALLERROR
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -54,7 +84,7 @@ struct OCSPRequestData { | |||
CiString<128> issuerNameHash; | |||
CiString<128> issuerKeyHash; | |||
CiString<40> serialNumber; | |||
CiString<512> responderURL; | |||
CiString<2000> responderURL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same argument like with the id token. Lets allow the larger value here.
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -890,7 +1992,7 @@ void from_json(const json& j, SetMonitoringResult& k); | |||
std::ostream& operator<<(std::ostream& os, const SetMonitoringResult& k); | |||
|
|||
struct SetVariableData { | |||
CiString<1000> attributeValue; | |||
CiString<2500> attributeValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept
include/ocpp/v201/ocpp_types.hpp
Outdated
@@ -943,7 +2132,7 @@ void from_json(const json& j, Transaction& k); | |||
std::ostream& operator<<(std::ostream& os, const Transaction& k); | |||
|
|||
struct Firmware { | |||
CiString<512> location; | |||
CiString<2000> location; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accept
lib/ocpp/v201/charge_point.cpp
Outdated
@@ -792,7 +792,7 @@ std::optional<std::string> ChargePoint::get_evse_transaction_id(int32_t evse_id) | |||
return evse.get_transaction()->transactionId.get(); | |||
} | |||
|
|||
AuthorizeResponse ChargePoint::validate_token(const IdToken id_token, const std::optional<CiString<5500>>& certificate, | |||
AuthorizeResponse ChargePoint::validate_token(const IdToken id_token, const std::optional<CiString<10000>>& certificate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accept
lib/ocpp/v201/charge_point.cpp
Outdated
@@ -1929,8 +1929,8 @@ bool ChargePoint::is_evse_reserved_for_other(EvseInterface& evse, const IdToken& | |||
const ConnectorStatusEnum status = | |||
evse.get_connector(static_cast<int32_t>(i))->get_effective_connector_status(); | |||
if (status == ConnectorStatusEnum::Reserved) { | |||
const std::optional<CiString<36>> groupIdToken = | |||
group_id_token.has_value() ? group_id_token.value().idToken : std::optional<CiString<36>>{}; | |||
const std::optional<CiString<255>> groupIdToken = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accept
lib/ocpp/v201/charge_point.cpp
Outdated
@@ -2113,7 +2113,7 @@ void ChargePoint::notify_report_req(const int request_id, const std::vector<Repo | |||
} | |||
} | |||
|
|||
AuthorizeResponse ChargePoint::authorize_req(const IdToken id_token, const std::optional<CiString<5500>>& certificate, | |||
AuthorizeResponse ChargePoint::authorize_req(const IdToken id_token, const std::optional<CiString<10000>>& certificate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accept
4b42e84
to
46af839
Compare
@@ -349,8 +349,11 @@ evse_security::CaCertificateType from_ocpp(CaCertificateType other) { | |||
return evse_security::CaCertificateType::CSMS; | |||
case CaCertificateType::MF: | |||
return evse_security::CaCertificateType::MF; | |||
case CaCertificateType::OEM: | |||
// FIXME: Add OEM to evse_security::CaCertificateType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a ticket for those two (also for V2G20Certificate, see below)? Otherwise we might forget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would consider support for this a 2.1 requirement and we dont have tickets for this yet.
} | ||
|
||
for (const auto& required_variable : required_variables) { | ||
check_required_variable(required_variable, supported_versions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not really belong here, but the only annoying thing here is that when there are multiple required variables without a value, you only see an error for the first one. You fix it, then comes the error for the second one.
It might just be nice to see a lost of all required variables that are not there.
Is this something we would like to improve? If yes, maybe open a ticket?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we can gather the errors and throw + print when we collected all of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that is better I think. Should have done that at the time I was writing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a ticket for that: #1008
transaction.chargingState = this->chargingState; | ||
transaction.timeSpentCharging = this->timeSpentCharging; | ||
transaction.stoppedReason = this->stoppedReason; | ||
transaction.remoteStartId = this->remoteStartId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we add the other fields here?
operationMode, tariffId, transactionLimit (I think the new 2.1 fields).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those will be added once we start implementing 2.1
@@ -205,7 +212,9 @@ def add_enum_type(name: str, enums: Tuple[str]): | |||
"""Add special class for enum types.""" | |||
for el in parsed_enums: | |||
if el['name'] == name: | |||
raise Exception('Warning: enum ' + name + ' already exists') | |||
#raise Exception('Warning: enum ' + name + ' already exists') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove out commented code.
@@ -560,6 +627,11 @@ def parse_schemas(version: str, schema_dir: Path = Path('schemas/json/'), | |||
})) | |||
first = False | |||
|
|||
if version == 'v2': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 'v2' like it is now or 'v21'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v2 is fine here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some to my opinion valid Codacy warnings in this file. Maybe fix them?
|
||
ChargingProfile | ||
create_charging_profile(int32_t charging_profile_id, ChargingProfilePurposeEnum charging_profile_purpose, | ||
std::vector<ChargingSchedule> charging_schedules, std::optional<std::string> transaction_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codacy: parse 'charging_schedules' by const reference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general: looks good. I have some minor things, some of them I am not even sure of.
OCPP2.1 message generation * Adds changes to code generator and jinja files in order to support OCPP2.1 messages * Adds OCPP2.1 messages into separate v21 directory Introduced SupportedOcppVersions variable * Added internal variable SupportedOcppVersions that allows to specify the supported protocol versions in order of preference * Added member variable ocpp_version to ChargePoint to indicate the selected version * ConnectivityManager now uses the configured value to set up the WebsocketConnectionOptions Refactored smart charging tests * seperated smart_charging_test_utils into hpp and cpp * moved redundant definitions of test cpp files into smart_charging_test_utils * implemented a couple operator== overloads for v21 types Support OCPP 2.1 variables * Extended RequiredComponentVariable with additional property to specify for which ocpp version a variable is required. This is the base to continue implementing a device model integrity check inside libocpps DeviceModel class * Aligned data ctrlr and sampled data ctrlr must always be available and the required variables are really always required. * Check for 'optional required' variables (only required when a component is available) and if they are not used while the whole ctrlr is not available. Adjustments as a result of the changed message types: * allowing more characters in CiStrings if required by OCPP2.1 types * ChargingSchedulePeriod limit property became optional and was required before. It's ensured to be always set so that existing did not have to change * Throwing ConversionsExceptions for enum values that have been added with OCPP2.1 messages but have no handling yet Misc: * Implemented code generator for EVerest yaml types from OCPP JSON Schemas Signed-off-by: Piet Gömpel <[email protected]>
38153a9
to
ec33d8f
Compare
Describe your changes
This PR includes OCPP2.1 message support and adjusts existing code to meet the requirements of the new messages.
The new OCPP2.1 are backwards compatible with the OCPP2.0.1 messages and are also used within existing OCPP2.0.1 handling.
OCPP2.1 message generation
Introduced SupportedOcppVersions variable
Refactored smart charging tests
Support OCPP 2.1 variables
Adjustments as a result of the changed message types
Misc
Issue ticket number and link
Checklist before requesting a review