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

Feature/add citrine support #53

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

thanaParis
Copy link
Contributor

Adding CitrineOS support
New command option '-c' uses CitrineOS instead of MaEVe

citrineos/add_charger.sh contains the requests necessary to commission a charger, including setting the auth header password.
New db files added with presets for CitrineOS websocket servers defined in the feature/everest-demo CitrineOS branch.

citrineos/add-charger.sh Show resolved Hide resolved
demo-iso15118-2-ac-plus-ocpp.sh Outdated Show resolved Hide resolved
demo-iso15118-2-ac-plus-ocpp.sh Show resolved Hide resolved
demo-iso15118-2-ac-plus-ocpp.sh Outdated Show resolved Hide resolved
Comment on lines +137 to +145
echo "Patching the CSMS to enable EVerest organization"
patch -p1 -i ../everest-demo/maeve/maeve-csms-everest-org.patch

echo "Patching the CSMS to enable local mo root"
patch -p1 -i ../everest-demo/maeve/maeve-csms-local-mo-root.patch

echo "Patching the CSMS to enable local mo root"
patch -p1 -i ../everest-demo/maeve/maeve-csms-ignore-ocsp.patch

Copy link
Collaborator

Choose a reason for hiding this comment

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

@thanaParis I wonder if there is some cert checking (similar to the patches here) in Citrine that is failing and causing SP3 to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Citrine starts up fine with SP3 enabled, so the certs pass validity checks. The issue comes up with EVerest tries to connect--EVerest throws a vague asio error, and Citrine just sees the client hang up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we have seen asio errors before while trying to use the EonTI certs #37 (comment) so it is almost certainly that the certs are misconfigured somewhere. did you try wireshark?

demo-iso15118-2-ac-plus-ocpp.sh Show resolved Hide resolved
demo-iso15118-2-ac-plus-ocpp.sh Outdated Show resolved Hide resolved
demo-iso15118-2-ac-plus-ocpp.sh Outdated Show resolved Hide resolved
demo-iso15118-2-ac-plus-ocpp.sh Outdated Show resolved Hide resolved
@shankari
Copy link
Collaborator

shankari commented Jun 5, 2024

Now that #50 is fixed, the current main works with citrine and SP1; there are currently no errors. Please pull and re-apply your changes.

$ bash demo-iso15118-2-ac-plus-ocpp.sh -c -1

/var/folders/y5/cx3cfzrd2q116myv9ly86sw1rnlmdj/T/tmp.hzWe2rEL/citrine-csms /var/folders/y5/cx3cfzrd2q116myv9ly86sw1rnlmdj/T/tmp.hzWe2rEL
HEAD is now at 63670f3 Merge pull request #99 from citrineos/fix/fix-container-build-pipeline
...

 ✔ Network server_default          Created                                                                0.1s
 ✔ Container server-amqp-broker-1  Created                                                                0.1s
 ✔ Container server-ocpp-db-1      Healthy                                                                0.1s
 ✔ Container server-directus-1     Created                                                                0.1s
 ✔ Container server-citrine-1      Created                                                               10.9s

...
200Password update successful.
API calls to CSMS finished, starting EVerest...
...
[+] Running 4/4
 ✔ Network everest-ac-demo_default          Created                                                       0.0s
 ✔ Container everest-ac-demo-mqtt-server-1  Healthy                                                       0.1s
 ✔ Container everest-ac-demo-manager-1      Healthy                                                       0.1s
 ✔ Container everest-ac-demo-nodered-1      Healthy                                                       0.1s
                                             Successfully copied 6.66kB to everest-ac-demo-manager-1:/ext/source/config/config-sil-ocpp201-pnc.yaml
Copying device DB, configured to SecurityProfile: 1
                                             Successfully copied 84.5kB to everest-ac-demo-manager-1:/ext/source/build/dist/share/everest/modules/OCPP201/device_model_storage.db

2024-06-05 03:46:08.957953 [INFO] ocpp:OCPP201     :: Connecting to plain websocket at uri: ws://host.docker.internal/ws/cp001 with security profile: 1

2024-06-05 03:46:09.053553 [INFO] ocpp:OCPP201     :: OCPP client successfully connected to plain websocket server

2024-06-05 03:47:19.161544 [INFO] ocpp:OCPP201     :: Received BootNotificationResponse: {
    "currentTime": "2024-06-05T03:47:19.157Z",
    "interval": 60,
    "status": "Accepted"
}

@ChrisWeissmann
Copy link
Contributor

@shankari I addressed the comments left and appeased the static code analysis :) Let me know if you would want any further changes.

Copy link
Collaborator

@shankari shankari left a comment

Choose a reason for hiding this comment

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

I think we should standardize on 80/443 for ws and wss and not have separate ports for citrine and MaEVe. That is how production systems will run anyway, and it will allow us to switch between CSMSes, showcasing the iteroperability provided by standardization. But let's do that as part of the ngrok changes.

Comment on lines 96 to 97
echo "Cloning MaEVe CSMS from ${MAEVE_REPO} into ${DEMO_DIR}/maeve-csms and starting it"
git clone "${MAEVE_REPO}" maeve-csms
Copy link
Collaborator

Choose a reason for hiding this comment

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

@thanaParis @ChrisWeissmann I am merging this for now, but I would really like to see a copy-pasted "testing done" in future PRs. I don't believe that this script will work, since MAEVE_REPO is not defined anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was an artifact of the merge to catch up our branch--my apologies. Fixed now.

Signed-off-by: K. Shankari <k.shankari@ee.doe.gov>
Copy link
Collaborator

@shankari shankari left a comment

Choose a reason for hiding this comment

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

Actually, I take that back. I can't merge this because the commits have not been signed off. @thanaParis @ChrisWeissmann all changes to the EVerest project need to be signed off. This is very easy, just use git commit -s. You can also retroactively sign commits.
https://stackoverflow.com/questions/13043357/git-sign-off-previous-commits

@ChrisWeissmann
Copy link
Contributor

@shankari I did my best in regards to the sign off. But since we have merged main into this branch multiple times it became a huge headache to resolve the merge conflicts again. Since the amount of files that were changed weren't that big I ended up creating a new PR off of current main. (#58 ) It also includes the testing done line in the commit message. I will close this one in favor of the other one.

Signed-off-by: Christian Weissmann <christian.weissmann@s44.team>
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.

3 participants