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

OCPP upgrade #506

Merged
merged 15 commits into from
Jan 27, 2023
Merged

OCPP upgrade #506

merged 15 commits into from
Jan 27, 2023

Conversation

matth-x
Copy link
Collaborator

@matth-x matth-x commented Dec 31, 2022

This PR contains the results of my work with OpenEVSE since the last PR #325 in March. I was able to test a lot with OCPP backend operators. The OpenEVSE was also at the OCA Plugfest in November! We got lots of positive feedback there and valuable suggestions for the further development.

In general, the update includes:

  • Support of OCPP Security Profile 2
  • Optimized connectivity of the WebSocket
  • Implementation of requested OCPP functionality (offline-mode optimizations, more metering measurands, ...)
  • Bux fixes

In the OCPP task, I removed much functionality which is implemented in the OCPP lib now and would be redundant now. In detail, the changes / new functions are:

  • Free vend mode (charging without dedicated authorization) is now implemented in the OCPP lib
  • The WebSocket credentials can be managed via ChangeConfiguration command and some more configs get synchronized with OCPP-configs
  • Support of more metering measurands
  • Smart charging schedules work with Amps and Watts (but only one of two; mixed mode outstanding)
  • Usage of the root certificates in root_ca.cpp
  • Bug fix (updating the configs in the GUI sometimes led to crashes)

In the meantime, I published a platform-independent Mongoose Adapter which I developed for other projects. I would really like the OpenEVSE project to use that Mongoose adapter too, since it would save me a lot of overhead of maintaining multiple Mongoose adapters. In this PR, I replaced the Mongoose adapter, removed the Mongoose-OCPP task and integrated the new adapter in the OCPP task.

The changes in the GUI are:

  • New field for the Authentication key (Security Profile 2)
  • No "transaction start point" anymore, but offline and Free Vend option instead
  • Slightly different structure since the old was misleading (at least I hope the new structure is better)

After the PR for the GUI repo OpenEVSE/openevse_wifi_gui#93 has been merged, I will update the GUI reference in this PR.

I tested most of the updates with a real charger, but unfortunately, I didn't have access to my OpenEVSE during the last weeks because of travels. So I tested the latest patch with the Wi-Fi board only. Can you please test with an OCPP backend and check whether the messages on the display are meaningful? (Edit: Tested end result with full OpenEVSE now) This is the compiled firmware, including the changes of the GUI: firmware.zip. Thanks!

@KipK
Copy link
Collaborator

KipK commented Jan 11, 2023

I have tested this PR with Steve, and everything runs ok here

image

@jeremypoulter
Copy link
Collaborator

@matth-x please mark as ready for review if ok to review/merge

@KipK
Copy link
Collaborator

KipK commented Jan 12, 2023

@matth-x, just one thing , the "Skip Authorisation when Offline" doesn't seems to works. Whatever it's checked or not, EVSE will always go to Active mode when OCPP is enabled and Not Connected.

@matth-x matth-x marked this pull request as ready for review January 12, 2023 09:12
@matth-x
Copy link
Collaborator Author

matth-x commented Jan 12, 2023

@jeremypoulter Sorry, I forgot about that, thanks for the hint. The only thing missing is a correct reference to the newest gui submodule.

@KipK Okay I see the problem here. The Offline mode doesn't make sense in combination with the Auto-Authorize mode. In Auto-Authorize mode, the charger immediately starts charging using the given ID-tag, without waiting for any status responses from the OCPP server. So Offline mode doesn't change anything. Btw. Offline mode is only relevant for RFID-card authorization.

Maybe there is a better wording for the Auto-Authorize mode. The most common term is "free vend mode", but that word isn't self-explanatory at all. Or maybe it would help to hide the Offline mode if Auto-Authorize is active. What would be your opinion regarding the best UX?

@KipK
Copy link
Collaborator

KipK commented Jan 12, 2023

Got you but I've also tryied with Auto Authorise disabled, and it was still starting a charge if offline
Edit: but I haven't tried with RFID enabled at this time.

I'll add the display condition also could be conditionned to RFID being enabled

I'm wondering too about the priority service OCPP has. It's higher than Manual Override, so we can't Start / Stop a charge manually from the UI.
It is the same for RFID too.

I get the point if UI is accessible to any users, but if it's dedicated to admin user, shouldn't we have the ability to Stop a charge manually in all case ? ( security purpose )

Do we consider web UI is admin or public ? Taht's an important point, as we don't have a way to stop a charge without disabling OCPP first ( or RFID if not authentified )

@KipK
Copy link
Collaborator

KipK commented Jan 12, 2023

@matth-x , I have tried with RFID enabled, no Auto-auth, and Offline booth active or not, in all case it put the EVSE in Active state when enabling OCPP with no server recheable

@jeremypoulter
Copy link
Collaborator

Regards the priority, don't forget manual is also used for the button on the front of the EVSE so is publicly accessible

@KipK
Copy link
Collaborator

KipK commented Jan 12, 2023

I thought the button was only to stop a charge as menu was disabled.
I have not put button on mine.

@matth-x
Copy link
Collaborator Author

matth-x commented Jan 12, 2023

@KipK Thanks for repeating the tests. There is clearly something wrong. To help me track this down, can you send me the config and status JSON of the charger (an e-mail would also be good)?

It's possible to deactivate the access control function of OCPP completely (also during charge). Have you seen it? How could we promote that function better? To be honest, no one I know understood what the "OCPP forbids charge" and "OCPP allows charge" are about.

@KipK
Copy link
Collaborator

KipK commented Jan 12, 2023

You mean the "OCPP can suspend EVSE" and "OCPP can energize plug" settings?
I got them clear but I'm not representative of an end user I think :)

I 've sent you an email with the endpoint outputs

src/app_config.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@jeremypoulter jeremypoulter left a comment

Choose a reason for hiding this comment

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

All looks good, just that one change then can be erged

@matth-x
Copy link
Collaborator Author

matth-x commented Jan 15, 2023

@KipK Finally I figured out what was going on! Thanks for the error description and the further details you sent me. When a power-cut and reboot occurred while charging, the OCPP claim always remained active until the next successful connection with the OCPP server. You could unplug, plug, and it still remained active. b6ea338 addresses this.

@KipK
Copy link
Collaborator

KipK commented Jan 16, 2023

@matth-x I confirm it works ok now and set state to Disable correctly

edit: but I have some claim loop spamming the log :

Claim from 0x00010009, priority 1050, disabled
Slot 0 valid 0x0
Slot 1 valid 0x0
Slot 2 valid 0x0
Found slot
Claim from 0x00010009, priority 1050, disabled
Slot 0 valid 0x0
Slot 1 valid 0x0
Slot 2 valid 0x0
Found slot
Claim from 0x00010009, priority 1050, disabled
Slot 0 valid 0x0
Slot 1 valid 0x0
Slot 2 valid 0x0
Found slot
Claim from 0x00010009, priority 1050, disabled
Slot 0 valid 0x0
Slot 1 valid 0x0
Slot 2 valid 0x0

@matth-x
Copy link
Collaborator Author

matth-x commented Jan 16, 2023

@KipK Yes, the loop function refreshes the claim every second. This looked like an easy and robust approach to me. Does it disturb other system components like the logs?

@KipK
Copy link
Collaborator

KipK commented Jan 16, 2023

@KipK Yes, the loop function refreshes the claim every second. This looked like an easy and robust approach to me. Does it disturb other system components like the logs?

Nope it's only on the log, was wondering, but it makes the logs unreadable for other stuff :)
wouldn't it be a better way to check the running claim and change it accordingly ?

edit: other thing since your last change, it nows still keep disabled state claim whatever the Skip Authorisation when offline is set or not

@KipK
Copy link
Collaborator

KipK commented Jan 17, 2023

@matth-x , I have to better check, but I see the flash as grown from 75% to 79.5% since your PR.

@matth-x
Copy link
Collaborator Author

matth-x commented Jan 17, 2023

@KipK Okay, I understand. How bad would it be if we put the claims-refresh issue in the backlog? I think that part would require much more testing.

Before the BootNotification succeeds, the charger does nothing. It's because the backend system needs to approve the charger. I need to add a configuration for it in the OCPP lib, but it will be a bigger change. And I know, it means that the offline authorize is not a full offline mode for now.

I can imagine that the new features come with a binary size increase. I added lots of code to handle edge cases and generally make it more robust over offline periods and reboots. Can you check it with the PIO memory inspector? It can show the binary sizes per source folder.

@KipK
Copy link
Collaborator

KipK commented Jan 26, 2023

@matth-x it's only console logs in debug mode, that's not a big issue :) btw that's not a big change, you only have to use an

 if (!_evse->clientHasClaim(EvseClient_OpenEVSE_Ocpp)) {
}

in the loop and that should avoid reclaim for nothing
You can also grab current claim properties easily if needed:
_evse->getState(EvseClient_OpenEVSE_Ocpp)
_evse->getChargeCurrent(EvseClient_OpenEVSE_Ocpp)
etc..

@matth-x
Copy link
Collaborator Author

matth-x commented Jan 26, 2023

@KipK Thanks for providing me with the relevant functions, I added them in 3db42bd. Again, I don't have my OpenEVSE with me to run a test. Can you check the new commit?

@KipK
Copy link
Collaborator

KipK commented Jan 27, 2023

@matth-x I've tested and looks all good to me. Thanks for the change

@jeremypoulter
Copy link
Collaborator

@KipK do you think this is good to merge?

@KipK
Copy link
Collaborator

KipK commented Jan 27, 2023

There's still one review change above before merge @matth-x , can you do it?

@KipK
Copy link
Collaborator

KipK commented Jan 27, 2023

On my side it works as expected and is a good improvement. 👍

@matth-x
Copy link
Collaborator Author

matth-x commented Jan 27, 2023

I thought I marked the change request as resolved but to me it also appears as pending. @jeremypoulter Can you maybe check this?

@jeremypoulter
Copy link
Collaborator

Yeah you did, is good to go then

@jeremypoulter jeremypoulter merged commit bb11105 into OpenEVSE:master Jan 27, 2023
@KipK
Copy link
Collaborator

KipK commented Jan 28, 2023 via email

@matth-x
Copy link
Collaborator Author

matth-x commented Feb 2, 2023

@KipK Sorry for the late reply. Okay, I will add it.

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