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

improved secc and evcc configuration handling and updated readme #6

Merged
merged 6 commits into from
Dec 28, 2021

Conversation

tropxy
Copy link
Contributor

@tropxy tropxy commented Dec 21, 2021

This PR introduces quite important changes that allow a better user experience on starting the system

@tropxy tropxy added documentation Improvements or additions to documentation enhancement New feature or request labels Dec 21, 2021
@tropxy tropxy self-assigned this Dec 21, 2021
@tropxy tropxy requested a review from MarcMueltin December 21, 2021 18:08
@tropxy tropxy marked this pull request as ready for review December 22, 2021 16:47
@tropxy tropxy changed the title improved secc configuration handling and updated readme improved secc and evcc configuration handling and updated readme Dec 22, 2021
@tropxy tropxy force-pushed the config_handling_improvements branch from 2dc1e90 to 282ccfd Compare December 23, 2021 16:09
Copy link
Contributor

@MarcMueltin MarcMueltin left a comment

Choose a reason for hiding this comment

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

Looks good, just a few questions from my side

iso15118/secc/secc_settings.py Show resolved Hide resolved
iso15118/secc/secc_settings.py Show resolved Hide resolved
iso15118/secc/secc_settings.py Outdated Show resolved Hide resolved
iso15118/evcc/evcc_settings.py Show resolved Hide resolved
iso15118/evcc/evcc_settings.py Show resolved Hide resolved
iso15118/evcc/evcc_settings.py Show resolved Hide resolved
README.md Outdated
| EVCC_CONTROLLER_SIM | `False` | Whether or not to simulate the EVCC Controller Interface |
| EVCC_USE_TLS | `True` | Whether or not the EVCC signals the preference to communicate with a TLS connection |
| EVCC_ENFORCE_TLS | `False` | Whether or not the EVCC will only accept TLS connections |
| PKI_PATH | `<CWD>/iso15118/shared/pki/` | Path for the location of the PKI where the certificates are located |
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a CWD in the settings.py file. Only WORK_DIR, SHARED_CWD, and PKI_PATH.
Should this entry maybe be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CWD is an acronym for Current Working Directory, so <CWD> is a placeholder for the current working directory path, which is mutable.

Copy link
Contributor

@MarcMueltin MarcMueltin Dec 28, 2021

Choose a reason for hiding this comment

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

I know what CWD stands for, I'm just wondering if <CWD> is kind of a command that Python reads and automatically replaces with the current working directory, or is that something the user needs to explicitly replace? I didn't fully understand that yet.

It is mutable means ...? Who changes it, the system or the user?

Copy link
Contributor Author

@tropxy tropxy Dec 28, 2021

Choose a reason for hiding this comment

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

Kinda both. Is the user's choice of where to start the main script, so the CWD can change if the user changes the directory where to start the application. Nevertheless, the directory is acquired automatically by python, as showed here: https://github.com/SwitchEV/iso15118/pull/6/files#diff-e79f57e0bf880362c0b200553691b24374be13279139649baf6e76c061f55153R40

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a note for that ENV, to be clear about what happens if the user does not define the PKI_PATH

@tropxy tropxy requested a review from MarcMueltin December 26, 2021 13:17
MarcMueltin
MarcMueltin previously approved these changes Dec 28, 2021
Copy link
Contributor

@MarcMueltin MarcMueltin left a comment

Choose a reason for hiding this comment

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

I have just one more clarification question for CWD (see my comment), but it's good to merge from my point of view

@tropxy tropxy merged commit 3b9db49 into master Dec 28, 2021
MarcMueltin pushed a commit that referenced this pull request Jan 4, 2022
…s to implement the -20 states until ScheduleExchange.

Update README.md (#1)

* Update README.md

Corrected a few typos and added a sentence explaining why JRE is needed

* Update README.md

* Update README.md

* Update README.md

Removed mqtt api as dependency (#3)

Fix compatibility with linux (#2)

* Added ipv6 support in docker world; fixed compatibility issue with sed between BSD and Linux systems

* improved readme

* renamed the .env files to be more intuitive in regards to their purpose; Updated Readme, adding a more detailed explanation of how to handle IPv6 communication within docker; fixed a bug on handling the configuration of the NIC interface

* fixed bug in the host mode compose file

* Added a few more clarification in the build Makefile recipe

* Fixed language typos in the README and removed references to the MQTT api

* Refactored network tools methods and general handling of it, to be more consise and close to the use case we have. For example, as the NETWORK_INTERFACE is always present, there is no need to scan for all the available interfaces to find one that contains a link-local address.

* Removed references ot the PYPI credentials in the Dockerfile

* Fixed bug on printing the interface used

* addressed editorial comments and renamed exception to be consistent

* Fixed docstring typo

* Due to the changes regarding the handling of the exceptions of the code, I did some reworking on the UDP client and server services, as in some occasions (e.g. when the interface configured was invalid) the application would throw the error 'evcc_1   | sys:1: RuntimeWarning: coroutine 'CommunicationSessionHandler.get_from_rcv_queue' was never awaited
evcc_1   | RuntimeWarning: Enable tracemalloc to get the object allocation traceback
evcc_1   | sys:1: RuntimeWarning: coroutine 'CommunicationSessionHandler.restart_sdp' was never awaited
evcc_1   | RuntimeWarning: Enable tracemalloc to get the object allocation traceback'

* improved docs

* fixed grammar and context

* removed more PYPI references

* fixed bug on EVCC side when requesting to stop charging

Simplification of the Authorization process_message method (#5)

* Removed explicit call to process CharParameterDiscoveryReq message; instead we can just decide which is the next expected state

* Updated README and Authoruzation docstring

* converted the wanring to a TODO

improved secc and evcc configuration handling and updated readme (#6)

* improved secc configuration handling and updated readme

* Added configuration handling for the evcc

* Refactored the collection of shared settings and log configuration. Also provided a way to select the location of the PKI certificate structure directory

* Added new line at the end

* improved README

* minor editorial change

Fix 1090 physical types validation error (#7)

* Fixed max value validation for the PhysicalValue types

* Changed multiplier to test the changes

* On favour of DRY, I moved the root_validator of the PhysicalTypes to their parent class

* added extra comment related to the private method validation with pydantic

* changed physical value restrictions after comments by Marc

* Revert "changed physical value restrictions after comments by Marc"

This reverts commit cf88330.

* used the enums for a few fields

Github actions workflow and reformat of the code (#8)

* Added github workflows for testing and publish; Reformatted the code using black and updated the CHANGELOG

* Added initial tests

* reformatted tests

* typo

* install with pip

* attempt to run tests without poetry

* added pytest to the workflow

* suspended temporarily the tests run

* fixed typo

* re-added poetry to call black

* added iso15118 for black

* attempt to run tests (using docker)

* copied tests dir to the container

* retry poetry tests

* added init to iso15118 dir

* docker as testing stage

* try to run tests with poetry again

* run tests with docker

* typo on the github action tag

* Editorial change

Added the skeleton for the remaining states (message handling classes) on the SECC side

Added the skeleton for the remaining states (message handling classes) on the EVCC side

Minor editorial edits and deactivating PnC in the settings to test EIM implementation first

Fixed the list issues regarding the EXI codec as raised by Shalin for the AuthorizationReq message and CertificateInstallationReq message

Implemented AuthorizationReq for EVCC and SECC

Modified exi_codec to complete the list of ISO 15118-20 messages that shall be used to parse an object from an incoming decoded EXI stream.

Implemented ServiceDiscoveryReq and -Res

Implemented ServiceDetailReq and -Res

Implemented ServiceSelectionReq and -Res

Changed settings.py in shared directory as ROOT_DIR did not point to the actual root directory but to settings.py in the shared folder, which messed up the path to the pki folder. As a result, the ssl_context could not be created.

Minor editorial edits and bugfix to get the application to run again

Made use of the new enums INT_16_MIN, INT_16_MAX, INT_8_MIN, INT_8_MAX in common_messages.py and common_types.py

Made use of the new enums INT_16_MAX in evcc_settings.py. Plus minor editorial edits to respect column length of 88

Added the specific payload type for each V2GTP message the EVCC and SECC sends in ISO 15118-20. In -20, we have energy mode-specific payload types we need to distinguish so that the EXI decoder gets pointed to the right XSD to decode the message.

Added the failed response for DCChargeParameterDiscoveryReq and modified the failed_responses.py to return also the payload type, which is important for the EXI codec (as in -20, we have energy mode-specific payload types).

Improved implementation of the class ServiceSelection on EVCC and SECC side

Implemented class DCChargeParameterDiscovery on SECC side

Fixed a small bug after changing the signature of the EVCC interface's get_charge_params_v20() function

Improved implementation of ServiceDiscovery and ServiceDetails in order to better handle the offered and selected service and their associated parameter sets. Needed some changes in the comm_session.py and comm_session_handler.py (both EVCC and SECC) as well as the state classes.

Working on finishing the implementation of the class DC_ChargeParameterDiscovery on the EVCC side. Implementing the ScheduleExchangeReq message.

Changed the implementation of the selected energy service and selected value-added services as session variables in the comm_session object. This way, we have easier access to the various service parameters, especially useful for the selected energy service.

Applied ge and le restrictions to departure_time and added the missing fields ev_max_v2x_energy_request and ev_min_v2x_energy_request to the class DynamicScheduleExchangeReqParams

Finished implementing state DCChargeParameterDiscovery on the EVCC side

Improved/fixed the implementation of class ServiceSelection on SECC side to check if the EVCC selected the correct pairs of service ID and parameter set ID per selected service

Introduced NumericID type in common_types.py and replaced simple int types for schedules IDs etc. with that type. Also changed the name of the class ListOfRootCertificateIDs to RootCertificateIDList to be consistent with the latest changes coming with PR#4

Started implememting the class ScheduleExchange on the SECC side, which includes work on the get_scheduled_se_params() and get_dynamic_se_params() functions on the EVCC and SECC side.

Finished implememting the class ScheduleExchange on the SECC side

Ran black on the files in this branch

Ran flake8 on the files in this branch

Forgot to add evcc_state.py to my last commit
shalinnijel2 pushed a commit that referenced this pull request Apr 19, 2022
…s to implement the -20 states until ScheduleExchange.

Update README.md (#1)

* Update README.md

Corrected a few typos and added a sentence explaining why JRE is needed

* Update README.md

* Update README.md

* Update README.md

Removed mqtt api as dependency (#3)

Fix compatibility with linux (#2)

* Added ipv6 support in docker world; fixed compatibility issue with sed between BSD and Linux systems

* improved readme

* renamed the .env files to be more intuitive in regards to their purpose; Updated Readme, adding a more detailed explanation of how to handle IPv6 communication within docker; fixed a bug on handling the configuration of the NIC interface

* fixed bug in the host mode compose file

* Added a few more clarification in the build Makefile recipe

* Fixed language typos in the README and removed references to the MQTT api

* Refactored network tools methods and general handling of it, to be more consise and close to the use case we have. For example, as the NETWORK_INTERFACE is always present, there is no need to scan for all the available interfaces to find one that contains a link-local address.

* Removed references ot the PYPI credentials in the Dockerfile

* Fixed bug on printing the interface used

* addressed editorial comments and renamed exception to be consistent

* Fixed docstring typo

* Due to the changes regarding the handling of the exceptions of the code, I did some reworking on the UDP client and server services, as in some occasions (e.g. when the interface configured was invalid) the application would throw the error 'evcc_1   | sys:1: RuntimeWarning: coroutine 'CommunicationSessionHandler.get_from_rcv_queue' was never awaited
evcc_1   | RuntimeWarning: Enable tracemalloc to get the object allocation traceback
evcc_1   | sys:1: RuntimeWarning: coroutine 'CommunicationSessionHandler.restart_sdp' was never awaited
evcc_1   | RuntimeWarning: Enable tracemalloc to get the object allocation traceback'

* improved docs

* fixed grammar and context

* removed more PYPI references

* fixed bug on EVCC side when requesting to stop charging

Simplification of the Authorization process_message method (#5)

* Removed explicit call to process CharParameterDiscoveryReq message; instead we can just decide which is the next expected state

* Updated README and Authoruzation docstring

* converted the wanring to a TODO

improved secc and evcc configuration handling and updated readme (#6)

* improved secc configuration handling and updated readme

* Added configuration handling for the evcc

* Refactored the collection of shared settings and log configuration. Also provided a way to select the location of the PKI certificate structure directory

* Added new line at the end

* improved README

* minor editorial change

Fix 1090 physical types validation error (#7)

* Fixed max value validation for the PhysicalValue types

* Changed multiplier to test the changes

* On favour of DRY, I moved the root_validator of the PhysicalTypes to their parent class

* added extra comment related to the private method validation with pydantic

* changed physical value restrictions after comments by Marc

* Revert "changed physical value restrictions after comments by Marc"

This reverts commit cf88330.

* used the enums for a few fields

Github actions workflow and reformat of the code (#8)

* Added github workflows for testing and publish; Reformatted the code using black and updated the CHANGELOG

* Added initial tests

* reformatted tests

* typo

* install with pip

* attempt to run tests without poetry

* added pytest to the workflow

* suspended temporarily the tests run

* fixed typo

* re-added poetry to call black

* added iso15118 for black

* attempt to run tests (using docker)

* copied tests dir to the container

* retry poetry tests

* added init to iso15118 dir

* docker as testing stage

* try to run tests with poetry again

* run tests with docker

* typo on the github action tag

* Editorial change

Added the skeleton for the remaining states (message handling classes) on the SECC side

Added the skeleton for the remaining states (message handling classes) on the EVCC side

Minor editorial edits and deactivating PnC in the settings to test EIM implementation first

Fixed the list issues regarding the EXI codec as raised by Shalin for the AuthorizationReq message and CertificateInstallationReq message

Implemented AuthorizationReq for EVCC and SECC

Modified exi_codec to complete the list of ISO 15118-20 messages that shall be used to parse an object from an incoming decoded EXI stream.

Implemented ServiceDiscoveryReq and -Res

Implemented ServiceDetailReq and -Res

Implemented ServiceSelectionReq and -Res

Changed settings.py in shared directory as ROOT_DIR did not point to the actual root directory but to settings.py in the shared folder, which messed up the path to the pki folder. As a result, the ssl_context could not be created.

Minor editorial edits and bugfix to get the application to run again

Made use of the new enums INT_16_MIN, INT_16_MAX, INT_8_MIN, INT_8_MAX in common_messages.py and common_types.py

Made use of the new enums INT_16_MAX in evcc_settings.py. Plus minor editorial edits to respect column length of 88

Added the specific payload type for each V2GTP message the EVCC and SECC sends in ISO 15118-20. In -20, we have energy mode-specific payload types we need to distinguish so that the EXI decoder gets pointed to the right XSD to decode the message.

Added the failed response for DCChargeParameterDiscoveryReq and modified the failed_responses.py to return also the payload type, which is important for the EXI codec (as in -20, we have energy mode-specific payload types).

Improved implementation of the class ServiceSelection on EVCC and SECC side

Implemented class DCChargeParameterDiscovery on SECC side

Fixed a small bug after changing the signature of the EVCC interface's get_charge_params_v20() function

Improved implementation of ServiceDiscovery and ServiceDetails in order to better handle the offered and selected service and their associated parameter sets. Needed some changes in the comm_session.py and comm_session_handler.py (both EVCC and SECC) as well as the state classes.

Working on finishing the implementation of the class DC_ChargeParameterDiscovery on the EVCC side. Implementing the ScheduleExchangeReq message.

Changed the implementation of the selected energy service and selected value-added services as session variables in the comm_session object. This way, we have easier access to the various service parameters, especially useful for the selected energy service.

Applied ge and le restrictions to departure_time and added the missing fields ev_max_v2x_energy_request and ev_min_v2x_energy_request to the class DynamicScheduleExchangeReqParams

Finished implementing state DCChargeParameterDiscovery on the EVCC side

Improved/fixed the implementation of class ServiceSelection on SECC side to check if the EVCC selected the correct pairs of service ID and parameter set ID per selected service

Introduced NumericID type in common_types.py and replaced simple int types for schedules IDs etc. with that type. Also changed the name of the class ListOfRootCertificateIDs to RootCertificateIDList to be consistent with the latest changes coming with PR#4

Started implememting the class ScheduleExchange on the SECC side, which includes work on the get_scheduled_se_params() and get_dynamic_se_params() functions on the EVCC and SECC side.

Finished implememting the class ScheduleExchange on the SECC side

Ran black on the files in this branch

Ran flake8 on the files in this branch

Forgot to add evcc_state.py to my last commit
@tropxy tropxy deleted the config_handling_improvements branch May 24, 2022 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants