Skip to content
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

feat: Support for 15118-20 #33

Merged
merged 8 commits into from
Apr 29, 2022
Merged

feat: Support for 15118-20 #33

merged 8 commits into from
Apr 29, 2022

Conversation

shalinnijel2
Copy link
Contributor

Addressed comments on EVCC side for -20 implementation.

@shalinnijel2 shalinnijel2 requested a review from tropxy April 22, 2022 18:01
Copy link
Contributor

@tropxy tropxy left a comment

Choose a reason for hiding this comment

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

just need to check 2 other files

iso15118/evcc/comm_session_handler.py Outdated Show resolved Hide resolved
iso15118/evcc/evcc_settings.py Outdated Show resolved Hide resolved
iso15118/secc/controller/simulator.py Outdated Show resolved Hide resolved
iso15118/secc/states/iso15118_2_states.py Show resolved Hide resolved
iso15118/shared/comm_session.py Outdated Show resolved Hide resolved
@shalinnijel2 shalinnijel2 requested a review from tropxy April 26, 2022 18:06
Condition that returned None for ScheduledExchangeRes Dynamic_SEResControlMode/Scheduled_SEResControlMode has been removed.
iso15118/evcc/controller/simulator.py Outdated Show resolved Hide resolved
iso15118/evcc/controller/simulator.py Outdated Show resolved Hide resolved
iso15118/evcc/controller/simulator.py Outdated Show resolved Hide resolved
iso15118/evcc/controller/simulator.py Outdated Show resolved Hide resolved
Comment on lines +225 to +228
logger.warning(
"PrivateKeyReadError occurred while trying to create "
"signature for CertificateInstallationReq. Falling back to sending "
f"AuthorizationReq instead.\n{exc}"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this defined in the standard?

Copy link
Contributor Author

@shalinnijel2 shalinnijel2 Apr 29, 2022

Choose a reason for hiding this comment

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

In the current implementation - AuthorisationReq is sent after AuthorisationSetupRes is received if any of the following conditions matches:

  • AuthorisationSetupRes has CertificateInstallationService set to False or EVCC decides it doesn't want certificates installed ( self.comm_session.ev_controller.is_cert_install_needed())
  • CertificateInstallationService in AuthorisationSetupRes is True and self.comm_session.ev_controller.is_cert_install_needed() is True.

self.comm_session.ev_controller.is_cert_install_needed() suggests EVCC has a say in the matter (as in could it continue sending AuthorisationSetupReq even though CertificationInstallationService was set to true by SECC) and can decide against having certificates installed. This possibility is used by EVCC in the current implementation as a fallback to send AuthorizationReq in case it fails to generate the signature used to build CertificateInstallationReq. My understanding is this what is happening above. Is that correct , @MarcMueltin ?

In the spec the reason EVCC could say no certificate installation is required is if it already has the required contract certificates. Otherwise, it is expected to go through CertificateIntstallation-AuthorizationSetupReq loop until RemainingContractCertificateChains is 0.

iso15118/evcc/states/iso15118_20_states.py Outdated Show resolved Hide resolved
iso15118/evcc/states/iso15118_20_states.py Outdated Show resolved Hide resolved
iso15118/evcc/states/iso15118_20_states.py Outdated Show resolved Hide resolved
iso15118/evcc/states/iso15118_20_states.py Outdated Show resolved Hide resolved
iso15118/secc/controller/simulator.py Show resolved Hide resolved
iso15118/secc/controller/simulator.py Outdated Show resolved Hide resolved
iso15118/secc/controller/simulator.py Outdated Show resolved Hide resolved
iso15118/shared/exi_codec.py Show resolved Hide resolved
iso15118/shared/messages/iso15118_20/common_messages.py Outdated Show resolved Hide resolved
iso15118/secc/states/iso15118_20_states.py Show resolved Hide resolved
iso15118/secc/states/iso15118_20_states.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tropxy tropxy left a comment

Choose a reason for hiding this comment

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

ccheck it plz

iso15118/evcc/states/iso15118_20_states.py Outdated Show resolved Hide resolved
iso15118/evcc/controller/simulator.py Outdated Show resolved Hide resolved
iso15118/evcc/controller/simulator.py Show resolved Hide resolved
iso15118/secc/controller/simulator.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tropxy tropxy left a comment

Choose a reason for hiding this comment

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

nice, Shalin ;)

@shalinnijel2 shalinnijel2 merged commit 07ab679 into master Apr 29, 2022
@tropxy tropxy deleted the 15118-20-support branch May 24, 2022 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants