-
Notifications
You must be signed in to change notification settings - Fork 15
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 support #82
OCPP support #82
Conversation
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.
Looks good Matthias, like your coding style; mg_mgr_poll in loop causes crashes so thats why I put it in a separate task. If you see another solution, of course I'm open for that.
"I wish I could pay my developers for every line of code they make. I would pay them double for every line of code they delete."
void Mongoose_Poll(void * parameter) { | ||
while (true) { | ||
if (WiFi.isConnected()) | ||
mg_mgr_poll(&mgr, 1000); |
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.
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.
Each call into the mongoose module needs to happen on the same RTOS thread, as the documentation says: https://mongoose.ws/documentation/tutorials/core/multi-threaded/#overview
I believe that the root cause of the crash you mention is somewhere else and unfortunately, such callback-driven frameworks are notoriously hard to debug. If I find possible causes, I will look for alternative solutions.
In general, preemptive multitasking like FreeRTOS threading is dangerous if the software components are not explicitly protected against it (and most high-level libraries on microcontrollers aren't). I would suggest moving all networking-related stuff on the main task as well as non-time-critical code, i.e. mongoose (including MQTT and the web server), OCPP and Wi-Fi management. Otherwise you always need to bear in mind which task is executing what which makes the code more complex to follow + adds the need for extra synchronization mechanisms.
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.
See my email...
@@ -4811,6 +4822,67 @@ void handleWIFImode() { | |||
} | |||
} | |||
|
|||
void ocppInit() { |
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.
I would love it if you would put your code in #if OCPP ...... #endif blocks, so we can decide compile time: -DOCPP=1 or -DOCPP=0 . I prefer this over #ifndef....
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.
Sure, I can also imagine introducing a new PlatformIO environment for that adding MicroOcpp in the lib_ignore section and not compiling it into the firmware at all.
if (!LocalTimeSet && WIFImode == 1) { | ||
_LOG_A("Time not synced with NTP yet.\n"); | ||
|
||
static unsigned long lastNtpCheck = 0; |
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.
Why is this an improvement to the vTaskDelay 1000 statement? Its one more static var ...
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.
Relates to the suggested change above of putting the networking related functionality on the main task. Then we need the loop function to execute as frequent as possible and can't put delays on it anymore.
O boy, this might be a major one, I never noticed that part of the docs.
Thanks for pointing this out, I'm going to look into it tomorrow; I'd like to take these kinds of non-OCPP related stuff separate and commit them as atomic as possible, if thats ok with you.
Thanks!
Matthias Akstaller ***@***.***> schreef op 1 juni 2024 11:07:53 CEST:
…
@matth-x commented on this pull request.
> @@ -2999,15 +3021,6 @@ void Timer1S(void * parameter) {
} // while(1)
}
-void Mongoose_Poll(void * parameter) {
- while (true) {
- if (WiFi.isConnected())
- mg_mgr_poll(&mgr, 1000);
Each call into the mongoose module needs to happen on the same RTOS thread, as the documentation says: https://mongoose.ws/documentation/tutorials/core/multi-threaded/#overview
I believe that the root cause of the crash you mention is somewhere else and unfortunately, such callback-driven frameworks are notoriously hard to debug. If I find possible causes, I will look for alternative solutions.
In general, preemptive multitasking like FreeRTOS threading is dangerous if the software components are not explicitly protected against it (and most high-level libraries on microcontrollers aren't). I would suggest moving all networking-related stuff on the main task as well as non-time-critical code, i.e. mongoose (including MQTT and the web server), OCPP and Wi-Fi management. Otherwise you always need to bear in mind which task is executing what which makes the code more complex to follow + adds the need for extra synchronization mechanisms.
--
Reply to this email directly or view it on GitHub:
#82 (comment)
You are receiving this because you commented.
Message ID: ***@***.***>
|
Hi @dingo35, No problem to move the tasks change to another PR. In the end, I could imagine to take all OCPP-related code changes of this PR and apply them in a clean way on a fresh new branch from master. Then it makes no difference if we take out the non-OCPP changes from here. Anyway, the most recent updates contain:
For the next steps I have two questions:
|
SmartEVSE-3/src/evse.cpp
Outdated
} | ||
|
||
if (shouldReboot) { | ||
delay(1000); | ||
ESP.restart(); | ||
} | ||
|
||
mg_mgr_poll(&mgr, 1000); |
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.
How about I move the mgr_init, mgr_poll and mgr_free to a separate commit in my master, and insulate it from OCPP ?
mgr_init should be in the wifi event "connected" since we need it to stop/start when the captive portal is started....
Hi Matthias,
Great work! I added some comments, stopped adding the "missing the ENABLE_OCPP here" at some time, you probably get the message :-) .
About the mgr_init, mgr_poll and mgr_free:
For us, mgr_init should be in the wifi event "connected" since we need it to stop/start when the captive portal is started.... Same for mgr_free in wifi event "disconnect" . I don't think this is any problem for you?
I will move mgr_poll to loop(), how about I do that in a separate commit in my master, and insulate it from OCPP ?
As to MQTT announce, every topic that MQTT publishes needs to be announce so MQTT clients (like HomeAssistant) can auto-discover it.
In your case, the topics do not have a unit of measurement, so place them both after:
announce("RFIDLastRead", "sensor");
... and your topics will appear automagically in HomeAssistant...
As for the web dashboard, we have primitively used a normal text editor but I would love to have a great WYSIWYG editor to maintain it. Of course it should be open source so the community can use it freely. Perhaps we can get rid of that awfull .css too , I don't dare to touch it....
Best regards, Hans.
"Matthias Akstaller" ***@***.***> schreef op 4 juni 2024 om 22:34:
…
Hi @dingo35 https://github.com/dingo35 ,
No problem to move the tasks change to another PR. In the end, I could imagine to take all OCPP-related code changes of this PR and apply them in a clean way on a fresh new branch from master. Then it makes no difference if we take out the non-OCPP changes from here.
Anyway, the most recent updates contain:
* RFID authorization: as a first step, I think OCPP should monopolize the RFID reader. If OCPP is enabled and RFID is in EnableOne or EnableAll mode, then the UUID is forwarded to the OCPP lib
* The HTTP API allows to set the server URL, enable / disable OCPP and view the connection status
* Two MQTT topics
* LittleFS enabled
For the next steps I have two questions:
* What is "announce" in the MQTT area? Do you think it's necessary for OCPP?
* How to develop the web dashboard? Do you use a WYSIWYG editor or do you develop the code in a text editor?
—
Reply to this email directly, view it on GitHub #82 (comment) , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOSIX7QJNR3RBGTFLOW22DZFYQF3AVCNFSM6AAAAABITFPLJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBYGM3TCMZRG4 .
You are receiving this because you were mentioned.
|
Hi Hans, Thanks for the code review! I changed the menu position as suggested and added the missing ENABLE_OCPP statements (it compiles now when ENABLE_OCPP=0). The Mongoose lifecycle is actually a critical topic, because my WebSocket wrapper depends on it and so does MicroOcpp, transitively. In the Mongoose docs and examples I never found that initializing the manager in the very beginning of the boot procedure would be a somehow discouraged practice. In fact, I haven't seen any problems if Mongoose is initialized before the Wi-Fi connection succeeds and remains alive throughout network reconfigurations. OCPP is also active without network connection. If the charger boots into an offline state, most operators want that it still authorizes RFID cards according to local whitelists and captures charging sessions for a later transmission to the server. So it would be important to be able to have OCPP always on. How hard would it be extend the lifecycle of Mongoose with the current captive portal implementation? Okay, I will implement the MQTT announce routine by duplicating the RFID code then. For the web dashboard, it's no problem to work with a text editor to add the few OCPP settings. |
In the current master branch mgr_init is called at WifiSetup, and mgr_free is never caĺled in the normal lifecycle. Only when the portal is called, mgr_free is called, and at the end, mgr_init again. Would that be a problem for OCPP? I can look for a way to only shut the listeners down? Also: do you need certificates on the device? Are they specific to the device or can they be generic? Keep up the great work! Hans. |
From the user perspective it's no problem to disable OCPP while the Wi-Fi portal is up and running. The only thing necessary is to shut down OCPP before MG is deinitialized and then to start it up again. I looked for possible locations on the recent master branch, but doing so I still found that the MG lifecycle model seems quite complex. The reason to shut down MG is to free the ports 80 and 443 again for WiFiManager? I'm wondering why not unbind the listeners on these ports and keep MG initialized. The other background tasks of MG (i.e. MQTT and OCPP trying to reconnect) shouldn't be a problem. This would also avoid MG functions being called from another thread here: SmartEVSE-3.5/SmartEVSE-3/src/evse.cpp Line 4769 in f605232
SmartEVSE-3.5/SmartEVSE-3/src/evse.cpp Line 4777 in f605232
What do you think? I can also make a quick write up in another draft PR so we can discuss about it there.
The device needs only one root CA cert for the OCPP server. But between OCPP servers, the root certs can differ of course. How do you plan to do certificate management for MQTT? I think the easiest is to just use the same approach for both protocols. In OCPP, there is also the possibility to have the certificates managed by the server. I will add that as an extra feature in future, but I'm not sure how widely it is used in practice at the moment.
Thanks, I also appreciate how well this project is made! |
The initial integration is ready now! To comment on the latest changes: in 3ee0fb1 I implemented the Mongoose HTTP server lifecycle as described in #82 (comment). In the next commit 71e8412 I recreated the MQTT timer lifecycle that it stops while the WiFi connection is lost. I think that it wouldn't be harmful if that timer continued running (maybe at a lower frequency), but it's no hassle to stop it. For Smart Charging, the connector lock (and RFID authorization) I implemented "OCPP-only" modes, i.e. OCPP takes over full control over the connector lock and RFID (if in mode EnableOne or EnableAll) and the load balancer if the load balancer is disabled. For firmware updates and diagnostics uploads I finally made the built-in solution of MicroOcpp ready (which will also be tested in other installations). We could, however, customize the OTA process and add more hardware diagnostics, if we have use cases which require that. |
Nice, tried both the fork of @matth-x and the merged one here. With the fork of Matthias I can connect to E-Flux With the merged one here I doesn't connect. Pretty weird? First test with remotestart and remotestop seems to work. Only to be shut off after 1-2s since the station and tag is not authorized. But I hear the contactor clicking. Tagging @dingo35 @mstegen so they know I'm already fiddling around. They only thing that I cannot get to work is the RFID reader. Looks like the local rfid.txt list is disabled/ignored (what I've also read here in the conv.) but the RFIDLastRead is not getting updated when holding tags in front of the reader. So looks like it's really shut off? |
Hi @fluppie, thanks for taking a look at the OCPP integration! The existing RFID whitelisting is fully bypassed in this first integration. OCPP catches the RFID events and doesn't forward them to SmartEVSE again. So RFIDLastRead remains blank. But you should see Authorize messages arriving at the server. To use RFID-based authorization with OCPP, the RFID settings must be "EnableOne" or "EnableAll". To use OCPP Smart Charging, the SmartEVSE load balancer must be turned off so that it's in mode "0". Not too user friendly, but should do the job in the beginning. |
I can't get the RFID stuff to work. Tried it on a real one connected to a vehicle and then on a EVSE next to my computer with only an RFID reader connected. is there a way to see if the EVSE is actually reading a tag? Maybe we should flash the red and green LED's on the reader at the same time when a tag is read? (that's more towards @dingo35 and @mstegen ) So we get visual feedback that something is happening. For example on the Alfen Eve charging points there's also a buzzer that beeps when you hold a tag in front of the reader. Some "succes" beep and a "rejected" beepbeep. BTW did you also try to build the merged version here? when I compile from Dingo35's repo it will not connect to the backoffice. Compiling from your repo works. |
Just noticed the RFID readers we use send 'only' the 6 LSB of the UID, while Mifare cards can have a 4, 7 or 10 byte UID. Yes good idea. The LED should flash if the card is accepted or not. |
@mstegen I installed Steven on a VM and have it running, but I don't see any RFID information being communicated to the platform. At least I don't see anything, but could be due to insufficient knowledge of Steve. EDIT: I kind of have it working. Only the reaction times for the starting and stopping of a session are bad. Or maybe it sometimes misses/skips a RFID read and send of the tag id. But in general it's working :). |
@fluppie, I am following the same path and I can't see any RFID info either..... |
@fluppie, how many bytes is the RFID tag you had to define in OCCP? Does it correspond to the 6 bytes in the SmartEVSE logs or did you have to find out the 7th byte? |
Testing with the Steve implementation Matthias provided. If i try to authenticate with a RFID card, the ID is visible on the serial output of the SmartEVSE:
When i go to the SteVe webserver, Data Management -> OCPP tags, then click on 'Unknown Tags (i)' The ID is visible here. Now the card should be accepted by the SmartEVSE.
|
Yes it only started working when I made a user and assigned a tag to a user.
Is rfid.txt already ready to support the 7th byte? Or maybe we shouldn't be using the 7th at all, since an Alfen doesn't read it either. Maybe we should think of some debounce for sending the tag id? 1000ms? 2000ms? |
Can you document that Alphen only uses 6 bytes for OCPP? |
I just pushed a 1,5s debounce 3e963e9 |
Nope, with a Robocharge tag it's longer. Can we flash the led green or red when the tag is valid or invalid? Based on that idTagInfo message?
|
@fluppie Yes that's already on the road map for the next revision. |
@matth-x would you be so kind to apply this patch to your mongoose adapter, so we can upgrade to mongoose 7.14: |
@dingo35 Thank you for the contribution! The new commit hash is dae388fe04beff6174b7fc30bc67dd4ad8343df3 |
Moving the integration + discussion to a new PR (this branch and dingo35/master have diverged quite a bit): #97 |
Integrate OCPP 1.6 into the SmartEVSE firmware.
Current TODO list: