Switch to CBV2G from OpenV2G for DIN, ISO-2, PnC and SAP #563

SiebrenW · 2024-02-25T14:01:58Z

This is the initial work to switch from OpenV2G to CBV2G in the EvseV2g module.

Currently The following is done:

SebaLukas

For now, the PR looks really good. I have just successfully started and finished a DIN session in the SIL. Now the code has to be tested a lot in the SIL and on real cars. I'll be doing that over the next few weeks

modules/EvseV2G/CMakeLists.txt

.clangd

modules/EvseV2G/iso_server.cpp

modules/EvseV2G/din_server.cpp

SebaLukas

I found a few little things

modules/EvseV2G/v2g.hpp

modules/EvseV2G/din_server.cpp

modules/EvseV2G/v2g_server.cpp

SebaLukas · 2024-03-07T07:34:16Z

I was able to successfully test DIN and ISO-2 AC & DC EIM with the SIL. Only with PnC there seems to be a problem with the authorization. I'll have a look at that soon. Once this has been resolved, I would also test this PR on a few cars.

Please update/rebase your branch to eversest-core main. Then there should be no more conflict with the dependencies.yaml.

SiebrenW · 2024-03-07T12:48:43Z

I had a few errors building with main, which is why this was rebased to the release initially (probaly had to checkout different dependencies, but am not aware of an easy way to do that. So I didn't 😐 ). I didn't think it was going to cause merge conflicts in the actual module code.
I will rebase to main hopefully later today (and cleanup those comments).

~~Then the iso2_ParameterSetType_5_ARRAY_SIZE being insufficient, I would prefer to keep using the constants from cbv2g, so I will wait for that update.~~ (or not #563 (comment) )
I marked master as the dependency tag, so we will see a compile error once this number (and thus the name) has changed and fix it then and there.

SebaLukas · 2024-03-07T13:00:10Z

You can also just clone everest-core and during cmake edm will take care of pulling the right dependencies with the right versions. Then building should not really be a problem after rebasing.

I totally agree with iso2_ParameterSetType_5_ARRAY_SIZE. This should be fixed in libcbv2g and not here. That was actually just a reminder for me not to forget this.

SiebrenW · 2024-03-07T13:04:27Z

I will try again then when I get home. Last time I tried to build master as is it didn't like my effort.
Or do you mean without everest-workspace? The readme does not cite this as an option, so I never tried.

SebaLukas · 2024-03-07T13:48:46Z

You have two options:
Like the readme says, create a everest-workspace with edm init and get all necessary repos/libs in one folder.

Or you can clone only the repos you work on. For me it is usually:

.
├── everest-core
├── Josev
└── libiso15118

First, edm checks whether all necessary repos are stored locally in the same folder as everest-core. If not, then edm download the missing repos in the $CPM_SOURCE_CACHE folder or build folder.

The second option should be somewhere in our documentation 😅

SiebrenW · 2024-03-07T21:18:13Z

Question: does it make sense to enforce the requirement of unused variables in the SignedInfo type?
For instance to set ANY_isUsed = 0; in SignatureMethod?
I made use of the fact that the object needs no conversion, it's the same one, but the conversion did set some of these fields to 0 explicitly.
I will add this in another commit for now. We can always drop it.
Edit: I made it, just forgot to push it. Will do that when I get home 😐

SebaLukas · 2024-03-08T07:02:26Z

Yes, add that 👍

SebaLukas · 2024-03-08T07:39:46Z

I activated the pr actions (A maintainer have to approve this for forks 🤷)
The clang-format check failed. Can you run clang-format on your changes (in your everest-core folder) with:

docker run \
--volume "$(pwd):/source
ghcr.io/everest/everest-clang-format:latest \
-r /source

This Docker image is also used here in the actions. This ensures that the same clang-format version is used.

SiebrenW · 2024-03-11T17:05:57Z

I activated the pr actions (A maintainer have to approve this for forks 🤷) The clang-format check failed. Can you run clang-format on your changes (in your everest-core folder) with:
docker run \
--volume "$(pwd):/source
ghcr.io/everest/everest-clang-format:latest \
-r /source
This Docker image is also used here in the actions. This ensures that the same clang-format version is used.

The command has some issues. The quote isn't even closed and when I do add it docker can't find the image.

I did the clang check manually with git-clang-format HEAD~4. I fixed it in one of the commits.

SebaLukas · 2024-03-13T10:10:44Z

modules/EvseV2G/iso_server.cpp

-    bitstream_t stream = {MAX_EXI_SIZE, buf, &buffer_pos, 0, 8 /* Set to 8 for send and 0 for recv */};
+    const struct iso2_ReferenceType* req_ref = &sig->SignedInfo.Reference.array[0];
+    exi_bitstream_t stream;
+    exi_bitstream_init(&stream, buf, MAX_EXI_SIZE, 8, NULL);


Please change the 8 to a zero. That is the reason why PnC don't work. The Exi fragment is encoded with an 8 byte offset, so of course the digest value is no longer correct.

Well. That's embarrassing.
I didn't even copy paste that. I actually typed 8 for some reason.

SebaLukas · 2024-03-13T10:12:15Z

Now PnC works also in the SIL. All that's left now are the tests with real cars.

SebaLukas · 2024-03-13T10:53:42Z

The command has some issues. The quote isn't even closed and when I do add it docker can't find the image.

I did the clang check manually with git-clang-format HEAD~4. I fixed it in one of the commits.

Sorry about the incorrect clang-format cmd. I always use podman, so my cmd is slightly different.

That should work here:

docker run --user "1000:100" --volume "$(pwd):/source" http://ghcr.io/everest/everest-clang-format:latest -r /source --extensions "hpp,cpp" -e "/source/cache" -e "/source/build" -e "/source/build-cross" -e "/source/build-clang" -i

Docker should find the image, I was able to pull it normally with podman earlier.

SiebrenW · 2024-03-13T11:02:17Z

The 8 has be turned into a 0 🤦
If the SIL was better documented I would have tested this myself. I am already happy to be able to test DC BPT with nodered.

Docker is still complaining with docker: invalid reference format. ~~I think my git-clang-tidy was effective anyways.~~

The eventual command was:

docker run --user "1000:100" --volume "$(pwd):/source" ghcr.io/everest/everest-clang-format:latest -r /source --extensions "hpp,cpp" -e "/source/cache" -e "/source/build" -e "/source/build-cross" -e "/source/build-clang" -i

barsnick · 2024-03-13T11:06:41Z

~~Then the iso2_ParameterSetType_5_ARRAY_SIZE being insufficient, I would prefer to keep using the constants from cbv2g, so I will wait for that update.~~ (or not #563 (comment) )
I marked master as the dependency tag, so we will see a compile error once this number (and thus the name) has changed and fix it then and there.

EVerest/cbexigen#74

corneliusclaussen · 2024-03-22T10:52:25Z

Will tag as post-release, I think we need a lot more testing and it should be merged after the march stable release

Signed-off-by: Siebren <siebren.w@gmail.com>

Signed-off-by: Siebren <siebren.w@gmail.com> remove unnecessary '-1' Signed-off-by: Siebren Weertman <siebren.weertman@heliox-energy.com> fix PnC offset

Signed-off-by: Siebren Weertman <siebren.w@gmail.com>

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch 2 times, most recently from f2c8c5e to 6ccfc8b Compare February 25, 2024 17:09

SiebrenW mentioned this pull request Feb 27, 2024

EvseV2G: Integration of libcbv2g as OpenV2G replacement #544

Open

8 tasks

SebaLukas reviewed Feb 28, 2024

View reviewed changes

modules/EvseV2G/CMakeLists.txt Outdated Show resolved Hide resolved

.clangd Outdated Show resolved Hide resolved

SebaLukas reviewed Feb 29, 2024

View reviewed changes

modules/EvseV2G/iso_server.cpp Outdated Show resolved Hide resolved

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch from ba9982d to 63a6b24 Compare February 29, 2024 15:03

SiebrenW changed the title ~~Switch to CBV2G from OpenV2G for DIN and SAP~~ Switch to CBV2G from OpenV2G for DIN, ISO-2, PnC and SAP Mar 2, 2024

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch from 63a6b24 to 3378b16 Compare March 6, 2024 11:16

SiebrenW commented Mar 6, 2024

View reviewed changes

modules/EvseV2G/din_server.cpp Outdated Show resolved Hide resolved

SebaLukas reviewed Mar 7, 2024

View reviewed changes

modules/EvseV2G/v2g.hpp Outdated Show resolved Hide resolved

modules/EvseV2G/din_server.cpp Outdated Show resolved Hide resolved

modules/EvseV2G/v2g_server.cpp Outdated Show resolved Hide resolved

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch from 3378b16 to 25e7afb Compare March 7, 2024 17:31

SiebrenW requested review from corneliusclaussen, Pietfried and hikinggrass as code owners March 11, 2024 17:09

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch 3 times, most recently from a82aeb4 to f434789 Compare March 11, 2024 17:29

SebaLukas reviewed Mar 13, 2024

View reviewed changes

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch from f434789 to ea7591f Compare March 13, 2024 10:56

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch from ea7591f to b3a57db Compare March 13, 2024 11:05

barsnick requested review from barsnick and FaHaGit March 13, 2024 13:25

SiebrenW force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch from b3a57db to f338f36 Compare March 13, 2024 18:36

corneliusclaussen added the post-release Tag that this PR should not go into the current merge window for the upcoming release label Mar 22, 2024

Pietfried assigned SebaLukas Apr 2, 2024

hikinggrass removed the post-release Tag that this PR should not go into the current merge window for the upcoming release label Apr 9, 2024

SiebrenW added 3 commits April 12, 2024 18:44

Switch to CBV2G from OpenV2G for DIN and SAP

b078589

Signed-off-by: Siebren <siebren.w@gmail.com>

switch iso1 to cbv2g iso2 and remove openv2g

649e3e8

Signed-off-by: Siebren <siebren.w@gmail.com> remove unnecessary '-1' Signed-off-by: Siebren Weertman <siebren.weertman@heliox-energy.com> fix PnC offset

make fields in SignedInfo unused to comply with V2G2-771

2755cfe

Signed-off-by: Siebren Weertman <siebren.w@gmail.com>

corneliusclaussen force-pushed the feature/544-switch_from_openv2g_to_cbv2g_for_din_sap branch from f338f36 to 2755cfe Compare April 12, 2024 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to CBV2G from OpenV2G for DIN, ISO-2, PnC and SAP #563

Switch to CBV2G from OpenV2G for DIN, ISO-2, PnC and SAP #563

SiebrenW commented Feb 25, 2024 •

edited

SebaLukas left a comment

SebaLukas left a comment

SebaLukas commented Mar 7, 2024

SiebrenW commented Mar 7, 2024 •

edited

SebaLukas commented Mar 7, 2024 •

edited

SiebrenW commented Mar 7, 2024

SebaLukas commented Mar 7, 2024 •

edited

SiebrenW commented Mar 7, 2024 •

edited

SebaLukas commented Mar 8, 2024

SebaLukas commented Mar 8, 2024

SiebrenW commented Mar 11, 2024 •

edited

SebaLukas Mar 13, 2024 •

edited

SiebrenW Mar 13, 2024 •

edited

SebaLukas commented Mar 13, 2024

SebaLukas commented Mar 13, 2024

SiebrenW commented Mar 13, 2024 •

edited

barsnick commented Mar 13, 2024

corneliusclaussen commented Mar 22, 2024

Switch to CBV2G from OpenV2G for DIN, ISO-2, PnC and SAP #563

Are you sure you want to change the base?

Switch to CBV2G from OpenV2G for DIN, ISO-2, PnC and SAP #563

Conversation

SiebrenW commented Feb 25, 2024 • edited

SebaLukas left a comment

Choose a reason for hiding this comment

SebaLukas left a comment

Choose a reason for hiding this comment

SebaLukas commented Mar 7, 2024

SiebrenW commented Mar 7, 2024 • edited

SebaLukas commented Mar 7, 2024 • edited

SiebrenW commented Mar 7, 2024

SebaLukas commented Mar 7, 2024 • edited

SiebrenW commented Mar 7, 2024 • edited

SebaLukas commented Mar 8, 2024

SebaLukas commented Mar 8, 2024

SiebrenW commented Mar 11, 2024 • edited

SebaLukas Mar 13, 2024 • edited

Choose a reason for hiding this comment

SiebrenW Mar 13, 2024 • edited

Choose a reason for hiding this comment

SebaLukas commented Mar 13, 2024

SebaLukas commented Mar 13, 2024

SiebrenW commented Mar 13, 2024 • edited

barsnick commented Mar 13, 2024

corneliusclaussen commented Mar 22, 2024

SiebrenW commented Feb 25, 2024 •

edited

SiebrenW commented Mar 7, 2024 •

edited

SebaLukas commented Mar 7, 2024 •

edited

SebaLukas commented Mar 7, 2024 •

edited

SiebrenW commented Mar 7, 2024 •

edited

SiebrenW commented Mar 11, 2024 •

edited

SebaLukas Mar 13, 2024 •

edited

SiebrenW Mar 13, 2024 •

edited

SiebrenW commented Mar 13, 2024 •

edited