-
Notifications
You must be signed in to change notification settings - Fork 111
Remove exi dependency #14
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
Conversation
This a quick summary of the changes in this commit: - py4j dependency is removed from .toml file. It is installed at runtime if required. - Make file is updated to point at main.py in evcc and secc projects. start_evcc.py and start_secc.py are removed - encode_signed_info() and decode_signed_info() methods are removed in the attached jar files. There is a more "unified" encode and decode method that would work for signed_info as well - A singleton is added to exi_codec.py class which would hold on to the passed in exi codec. The class is added to help the to_exi() and from_exi() methods that would request the exicodec to use.
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.
Can you also add the logger instantiation to the init.py of the secc and evcc? Check it: https://github.com/SwitchEV/iso15118/blob/rust_exi_codec/iso15118/secc/__init__.py
I noticed that not all log messages were being printed due to the absence of that
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.
Shalin this line needs to be changed
https://github.com/SwitchEV/iso15118/blob/remove_exi_dependency/pyproject.toml#L38
start_secc -> main
I think that is all it is needed to make the Docker work, but please test it with make build
and make dev
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.
LGTM
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.
LGTM
This a quick summary of the changes in this commit:
[Update]
Since py4j may have future dependency conflicts with other packages, is for the best to include it in the toml. For the bundle generation targeting any production device of Josev (where we use the rust exi codec) we can strip out py4j as a dependency)