Skip to content

Compatibility check and publish response code #164

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.

Sign up for GitHub

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

Jump to bottom
Open
martinlukas84 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Compatibility check and publish response code #164

martinlukas84 wants to merge 5 commits into from

Conversation

@martinlukas84
Copy link
Collaborator

@martinlukas84 martinlukas84 commented Nov 28, 2025

Describe your changes

This PR adds the compatibility check according to the standard IEC61851-23 CC.5.6.
Furthermore, adding the response code to the feedback to get this information available for other modules.
This is helpful for process view in PLC, HMI etc.

Issue ticket number and link

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

@martinlukas84
The standard IEC61851-23 CC.5.6 requires a compatibility check at sta…
42c9b35 
…te ChargeParameterDiscovery.

This will be added here.

Signed-off-by: Martin Lukas <martin.lukas@chargebyte.com>
@martinlukas84
To get the response code available for other modules in EVerest it's …
5e4892d 
…send via feedback to the Iso module.

Signed-off-by: Martin Lukas <martin.lukas@chargebyte.com>
SebaLukas
SebaLukas requested changes Nov 28, 2025
View reviewed changes
src/iso15118/d20/state/dc_charge_parameter_discovery.cpp Outdated
// the EVerest environment (power supply, grid), here we assume that both are the same

bool compatiblity_flag = true;
const auto ev_max_limits_cpd = ev_max_limits;
Copy link
Member

@SebaLukas SebaLukas Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add another copy here? Why not using the argument ev_max_limits as const ref

Copy link
Collaborator Author

@martinlukas84 martinlukas84 Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up to now i was not sure how to handle the CPD and RATED index. I thought to work with new named vars to keep a kind sort of readability. But when we now get rid of the CPD and RATED index it can be removed or implemented in other way.

Copy link
Member

@SebaLukas SebaLukas Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you like to use variables for this, it's better to use const ref instead of copying each variable.

@martinlukas84 martinlukas84 force-pushed the feature/compatibility_check branch from cb38711 to 8a92f25 Compare December 1, 2025 05:18
@martinlukas84
Rework after review
35f8ade 
Signed-off-by: Martin Lukas <martin.lukas@chargebyte.com>
@martinlukas84 martinlukas84 force-pushed the feature/compatibility_check branch 2 times, most recently from c8b084b to 4d31def Compare December 1, 2025 05:53
@martinlukas84
A good case test failed where the EV max voltage is below 500V. This …
863ac54 
…concerns part CC.5.6 2a in standard

IEC61851-23. The test case defines a powersupply voltage of 900V. But this must be meet
the requirement from mentioned part min(V_ev +50, V_evse, 500).
So here the V_Evse must be <= 450V.

Signed-off-by: Martin Lukas <martin.lukas@chargebyte.com>
@martinlukas84 martinlukas84 force-pushed the feature/compatibility_check branch from 4d31def to 863ac54 Compare December 1, 2025 06:01
@martinlukas84
Add unit test for MCS and MCS_BPT for testing compatibility check.
59c71bf 
Fix wrong parameter usage in compatibility check.

Signed-off-by: Martin Lukas <martin.lukas@chargebyte.com>
@martinlukas84 martinlukas84 force-pushed the feature/compatibility_check branch from 902e5a8 to 59c71bf Compare December 1, 2025 10:15
@SebaLukas
Copy link
Member

SebaLukas commented Dec 5, 2025 •
edited
Loading

I took a closer look at CC.5.6 in IEC61851-23:2023 and don't seem to interpret it the same way you do.
I understand section CC.5.6. 2) a) - c) to mean rather that the charger should adjust the parameters for ChargeParameterDiscoveryRes and not check whether the values are within the value range.
For me, it would be fatal if a charger ended the session with an error every time just because the max EVSE voltage is 1000V and the EV only specifies a maximum voltage of 500V.
Therefore, a) - c) should adjust the parameters of the ChargeParameterDiscoveryRes accordingly.

For me the g) section also describes a check in which the charger changes the ResponseCode, Processing, and StatusCode accordingly.

In addition, ChargeParameterDiscoveryRes or any other res message should not have any values in the event of an error, if possible. I saw that in the newly added unit tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@a-w50 a-w50 Awaiting requested review from a-w50 a-w50 is a code owner

@james-ctc james-ctc Awaiting requested review from james-ctc james-ctc is a code owner

@mlitre mlitre Awaiting requested review from mlitre mlitre is a code owner

@SebaLukas SebaLukas Awaiting requested review from SebaLukas SebaLukas is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@SebaLukas SebaLukas

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@martinlukas84 @SebaLukas