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

Easee: wait for pause/resume to be acknowledged #8307

Merged

Conversation

GrimmiMeloni
Copy link
Collaborator

@GrimmiMeloni GrimmiMeloni commented Jun 4, 2023

Fix #8398

I felt the need to address the parallel pause/resume situation described in #7366 (reply in thread).

I have taken the initial change which @naltatis implemented in #7597 to capture the ticks when sending pause/resume commands to the Easee cloud.
I have now added enforcement into Enable() to check if a reply for a previous pause or resume command is still outstanding. If so, Enable() returns an error, so that the operation will be retried on the next interval.
I decided to wait for a max of 30s, otherwise I consider the outstanding reply lost. This is a compromise to ensure we don't get stuck forever waiting, in case Easee decides to ignore/drop requests completely.

Did my best to do proper locking in case replies come in in parallel. I'd appreciate reviewer(s) to double check this, though.

@andig I have NOT tested this yet (car is charged up right now). Can you help me mark this as WIP, or is it sufficient to leave the PR in draft state until tested?

@premultiply premultiply requested a review from andig June 4, 2023 16:48
charger/easee.go Outdated Show resolved Hide resolved
@andig
Copy link
Member

andig commented Jun 5, 2023

Das Problem welches dieser PR löst ist doch nur, dass wir nicht mehrfach enabled/disable schicken würden sondern immer auf den letzten Aufruf warten.
Das Verhalten ist aber jetzt inkonsistent- wir antworten mit Fehler falls nicht verarbeitet. Das müsste man dann konsequenterweise auch beim Initialaufruf passieren. Auch müsste die Logik für fehlende TickIDs analog der im MaxCurrent sein.

Zurück zur Ausgangsfrage: was wollen wir hier konkret erreichen? Eine kompliziertere Logik müsste folgende Fälle berücksichtigen:

  • HTTP 200: ok
  • HTTP 202: falls kein Tick Fehler, falls Tick retry
  • retry: schauen ob es zwischendurch ein ok gab

Lohnt der Aufwand? Alternativ: wir unterstellen sequenzielle Abarbeitung und kümmern uns nur drum die fehlenden Tick auszusteuern

@andig andig added the enhancement New feature or request label Jun 5, 2023
@andig andig changed the title wait for pause/resume to be acknowledged Easee: wait for pause/resume to be acknowledged Jun 5, 2023
@GrimmiMeloni
Copy link
Collaborator Author

Guter Hinweis mit den fehlenden Tick IDs. Bisher habe ich das nur beim /settings Endpoint gesehen, aber das heißt ja nichts. Baue ich noch ein.

@andig kannst du kurz erläutern welchen Initialaufruf du meinst? Geht es um den /settings Aufruf direkt vorher in

resp, err := c.Post(uri, request.JSONContent, request.MarshalJSON(data))
?

@andig
Copy link
Member

andig commented Jun 5, 2023

Ich meinte, dass alle Aufrufe gleich funktionieren sollten: Prüfung auf tick-id, Prüfung auf 200 vs 202. Annahme: auch das Backend ist immer das Gleiche...

Mir ist aber generell nicht klar, wie wir mit der asynchronen Ausführung umgehen wollen: entweder active wait auf die tick-id, oder error return? Vllt. sollten wir erstmal den Ansatz klären?

@GrimmiMeloni
Copy link
Collaborator Author

GrimmiMeloni commented Jun 5, 2023

Zurück zur Ausgangsfrage: was wollen wir hier konkret erreichen? Eine kompliziertere Logik müsste folgende Fälle berücksichtigen:

  • HTTP 200: ok
  • HTTP 202: falls kein Tick Fehler, falls Tick retry
  • retry: schauen ob es zwischendurch ein ok gab

Lohnt der Aufwand? Alternativ: wir unterstellen sequenzielle Abarbeitung und kümmern uns nur drum die fehlenden Tick auszusteuern

Sequentielle Abarbeitung können wir nicht unterstellen. Dafür habe ich hier zuviele Situationen in den letzten Monaten gesehen, wo Easee durcheinander kommt, wenn Kommandos (fast) zeitgleich gesendet werden. Auch die Laufzeiten in der Easee API schwanken, ich habe schon über 1s gesehen.

Ich würde gerne auch nochmal den Begriff "retry" hier etwas genauer definieren wollen. Mein Verständnis der Arbeitsweise von evcc ist, daß pro Intervall ein Sollzustand errechnet wird. Der Loadpoint versucht diesen Zustand auf den Charger zu übertragen in dem er entsprechende APIs aufruft.
Basierend auf diesem (vielleicht falschen?) Verständnis habe ich mich bewusst gegen einen "retry"-Mechanismus bzw. ein active-wait entschieden. Es müßte nach meinem Verständnis ausreichen den Fehler zu werfen, und der Loadpoint wird dann im nächsten Intervall erneut versuchen den Sollzustand herzustellen.

Noch kurz zu diesem Detail:

  • HTTP 202: falls kein Tick Fehler, falls Tick retry

Der Teil "falls Tick retry" kann nicht eintreten. Wenn ein Tick noch gespeichert ist, wird der Request gar nicht gesendet. Die Sequenz ist wie folgt:

  • Initial kein Tick gespeichert
  • pause oder resume Request wird gesendet, Tick kommt zurück und wird gespeichert
  • Ab jetzt werden keine weiteren pause und resume Requests mehr gesendet, weil ein Tick gespeichert ist (oder noch keine 30s seit dem speichern des Ticks vergangen sind)
  • Antwort kommt für den Tick, gespeicherter Tick wird gelöscht, alles wieder von vorn

Das es keine Überschneidungen gibt ist primär nur für pause / resume Aufrufe entscheidend. Dies liegt daran, daß für diese Aufrufe die Reihenfolge der Verarbeitung entscheidend ist. Mal angenommen bei den Settings werden die Amps in falscher Reihenfolge gesetzt, dann ist dies kein Problem, weil evcc die Abweichung von soll und ist Wert bei den Amps erkennt, und dann im nächsten Interval automatisch gerade zieht.

Bzgl. des generellen Ansatz hab ich oben kurz mein Verständnis zur Arbeitsweise erklärt. Wenn dies korrekt ist, würde ich bei dem Error return Ansatz bleiben. Das erscheint mir dann als pragmatisch.

@andig
Copy link
Member

andig commented Jun 5, 2023

Basierend auf diesem (vielleicht falschen?) Verständnis habe ich mich bewusst gegen einen "retry"-Mechanismus bzw. ein active-wait entschieden. Es müßte nach meinem Verständnis ausreichen den Fehler zu werfen, und der Loadpoint wird dann im nächsten Intervall erneut versuchen den Sollzustand herzustellen.

Das ist schon so, trägt aber nur so weit:

  • MaxCurrent kann nur gesetzt werden und merkt sich den Zustand intern (ok, hier geht es weil wir den Strom per Roundtrip bekommen)
  • 1p3p muss- wenn über Circuit geschaltet- erst den Charger disablen, sonst wirkt 1p3p nicht. Hier bräuchte es also ein active wait.

/cc @naltatis wofür brauchten wir den Circuitkram eigentlich? Raus damit???

@GrimmiMeloni
Copy link
Collaborator Author

  • 1p3p muss- wenn über Circuit geschaltet- erst den Charger disablen, sonst wirkt 1p3p nicht.

Vorsicht, das ist genau umgekehrt. Bei einzelner Easee WB (wie hier bei mir zu Hause) wird über den Circuit geschaltet, und dabei ist kein disable/enable erforderlich. Das habe ich mit einem lokalen Build von #7723 bereits geprüft.

Hier bräuchte es also ein active wait.

Ok, ich gebe mal mein Verständnis wieder. Das ist der Sonderfall der sich aus dem "Charger Level Phase Switch" ergibt. Zumindest in Kombination mit #7723, weil dann der Loadpoint nicht mehr schaltet, sondern die WB es übernehmen sollte - was sie aber laut @naltatis nicht tut. Und das Argument für den Active Wait Ansatz wäre dann Uniformität, damit wir nicht beide Ansätze (return Err, Active wait) einbauen und uns für einen Weg entscheiden. Soweit richtig?

@andig
Copy link
Member

andig commented Jun 5, 2023

Es ist ganz einfach: für current und phases hat evcc ein eigenes Bild vom Chargerzustand. Wenn das nicht zur Realität passt wird es mit Glück erkannt, aber eben nicht garantiert. Mit dem Zustand müssen sich die Charger halt arrangieren, wie auch immer wir das lösen.

Beispiel: sofortladen mit 16A. Wir MaxCurrent verschluckt wird halt geladen wie es gerade ist. Schlägt 1p->3p fehl, denkt evcc 3p lädt sber immer weiter mit 1p.

@naltatis
Copy link
Member

naltatis commented Jun 5, 2023

@naltatis wofür brauchten wir den Circuitkram eigentlich? Raus damit???

Was genau meinst du damit? Die Fallunterscheidung? Der von Easee gewünschte Weg für die Phasenumschaltung ist die Regelung über den Circuit (L2=0,L3=0). Bei Circuits mit einem Charger ist das für uns kein Problem. Daher machen wir das auch genau so. Hängen an einem Charger mehrere Boxen könnten wir die nur gesammelt phasenumschalten. Das beißt sich aber mit dem Datenmodell/UI/Regelung/Funktionen von evcc. Daher der Workaround über das Phase-Setting direkt am Charger.

Easee empfielt die Settings (außer dynamicChargerCurrent) nicht unnötig oft zu schreiben. Daher macht es schon Sinn dass die direkte Steuerung am Charger nur unsere 2. Wahl ist wenn die erste nicht geht. Die beiden Codepfade bringen aber natürlich Komplexität mit.

@GrimmiMeloni
Copy link
Collaborator Author

@andig OK, dann lass uns doch mal konkreter über eine active wait Implementierung sprechen. Mein Vorschlag wäre einen Channel im Easee Adapter anzulegen. Nach Kommunikation mit der Easee API und Result Code 202 wird auf eine Antwort über den Channel gewartet. Falls nach 5s immer noch keine Antwort da ist, schlägt ein Timeout zu und wir geben einen Error zurück.
In ProductUpdate and CommandReponse würden die Werte aus der Response in den Channel geschrieben. Auf der Empfängerseite würden dann erwartete und erhaltene TickId abgeglichen. Zusätzlich würde auch der Wert von WasAccepted geprüft um den Fall zu behandeln das Easee das Kommende aktiv abgewiesen hat. Wenn alles passt geht es im Hauptthread wieder weiter, so also wäre die Easee API synchron.

@andig
Copy link
Member

andig commented Jun 6, 2023

Vllt. mittels

err := wb.waitTick(tick)

oder

err := wb.waitResponseProcessed(resp)

@andig
Copy link
Member

andig commented Jun 7, 2023

Nochmal ein etwas grundsätzlicher Lösungsgedanke ohne durchgängiges active wait: #7366 (comment). Die Debugmeldung "current mismatch" ist tatsächlich blöd da sie zu keiner Lösung führt sondern nur das Problem dokumentiert.

@GrimmiMeloni
Copy link
Collaborator Author

@andig please assign to me, I have started working on the active wait implementation.

@andig andig requested a review from naltatis June 11, 2023 10:26
GrimmiMeloni and others added 9 commits June 11, 2023 13:37
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
charger/easee.go Outdated Show resolved Hide resolved
charger/easee.go Outdated Show resolved Hide resolved
charger/easee.go Outdated Show resolved Hide resolved
charger/easee.go Outdated Show resolved Hide resolved
charger/easee.go Outdated Show resolved Hide resolved
GrimmiMeloni and others added 6 commits June 11, 2023 14:43
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: andig <cpuidle@gmx.de>
@andig
Copy link
Member

andig commented Jun 11, 2023

/cc @naltatis könntest Du das auch mal testen?

@andig andig merged commit ce0e427 into evcc-io:master Jun 12, 2023
andig added a commit that referenced this pull request Jun 12, 2023
@GrimmiMeloni
Copy link
Collaborator Author

Can we reopen the PR, or shall we create a new one? Branch is already updated with an (untested) fix.

@andig
Copy link
Member

andig commented Jun 12, 2023

Not possible- but you can start a new PR from the same branch.

naltatis pushed a commit that referenced this pull request Jun 14, 2023
naltatis pushed a commit that referenced this pull request Jun 14, 2023
naltatis added a commit that referenced this pull request Aug 2, 2023
…to UI (#8115)

* Plugins: make javascript return values more permissive

* Persist targetSoc/Energy, minSoc and target time, remove experimental from minSoc UI

* go generate

* Fix tests

* Update minSoc description

* Store settings by index. Show plan outside pv mode. Add help texts.

* remove slug

* go mock

* lint

* chore: simplify templates (#8304)

* Add e2e tests with playwright (#8123)

* Revert "1p3p: let charger handle session stop/restart (#7723)"

This reverts commit dd787ce.

* mazda2mqtt: document vin required (#8319)

* FoxESS: split H1/H3 devices (#7376)

Co-authored-by: premultiply <4681172+premultiply@users.noreply.github.com>

* Translations update from Hosted Weblate (#8124)

* Translated using Weblate (Czech)

Currently translated at 37.3% (99 of 265 strings)

Co-authored-by: Dusan Suja <bc.suja.dusan@googlemail.com>
Translate-URL: https://hosted.weblate.org/projects/evcc/evcc/cs/
Translation: evcc/evcc

* Translated using Weblate (Finnish)

Currently translated at 100.0% (265 of 265 strings)

Co-authored-by: Arna Lepikkö <arna.lepikko@telemail.fi>
Translate-URL: https://hosted.weblate.org/projects/evcc/evcc/fi/
Translation: evcc/evcc

* Translated using Weblate (German)

Currently translated at 100.0% (265 of 265 strings)

Co-authored-by: ThinkEV <claas@rootdir.de>
Translate-URL: https://hosted.weblate.org/projects/evcc/evcc/de/
Translation: evcc/evcc

* Translated using Weblate (Croatian)

Currently translated at 100.0% (265 of 265 strings)

Translated using Weblate (Slovenian)

Currently translated at 100.0% (265 of 265 strings)

Co-authored-by: Žiga Deisinger <ziga@deisinger.si>
Translate-URL: https://hosted.weblate.org/projects/evcc/evcc/hr/
Translate-URL: https://hosted.weblate.org/projects/evcc/evcc/sl/
Translation: evcc/evcc

* Translated using Weblate (Catalan)

Currently translated at 100.0% (265 of 265 strings)

Co-authored-by: Norbert Poch <npoch@yahoo.com>
Translate-URL: https://hosted.weblate.org/projects/evcc/evcc/ca/
Translation: evcc/evcc

* Translated using Weblate (Dutch)

Currently translated at 76.6% (203 of 265 strings)

Translated using Weblate (Dutch)

Currently translated at 74.3% (197 of 265 strings)

Co-authored-by: Ruben Van Boxem <vanboxem.ruben@gmail.com>
Translate-URL: https://hosted.weblate.org/projects/evcc/evcc/nl/
Translation: evcc/evcc

* fix toml

---------

Co-authored-by: Dusan Suja <bc.suja.dusan@googlemail.com>
Co-authored-by: Arna Lepikkö <arna.lepikko@telemail.fi>
Co-authored-by: ThinkEV <claas@rootdir.de>
Co-authored-by: Žiga Deisinger <ziga@deisinger.si>
Co-authored-by: Norbert Poch <npoch@yahoo.com>
Co-authored-by: Ruben Van Boxem <vanboxem.ruben@gmail.com>
Co-authored-by: premultiply <4681172+premultiply@users.noreply.github.com>

* Add OBO Betterman Ion (#8321)

Also aligns heartbeat implementations

* Enphase: add token auth (firmware D7.x.xxx) and grid (#8247)

* Mqtt: fix smartCostLimit topic case (#8328)

* Add ISO15118 vehicle template (#8302)

* Mqtt: simplify setters

* Mqtt: disable message ordering to improve performance

* chore: simplify random state generation

* Porsche: remove deprecated mobile api (#8349)

* Porsche: remove remaining mobile api types

* Cupra: add odometer (#8340)

* chore: refactor go-stylish

* chore: fix stale handler

* mazda2mqtt: longer timeout (#8364)

* chore: pre-process toml

* chore: prevent toml double quotes

* Audi: temporarily switch to etron (#8374)

* Fix nightly build (#8384)

* Update SunSpec templates (#8270)

* Revert "Fix nightly build (#8384)"

This reverts commit 536dbc9.

* Revert "Add e2e tests with playwright (#8123)"

This reverts commit 0df8157.

* Easee: update Observation IDs (#8391)

* Easee: handle smartCharging errors (#8389)

* TWC: add non-Tesla vehicle warning (#8329)

* TWC: allow loadpoint to wakeup vehicle (#8284)

* chore: use gridx consts

* Fix ISO15118 vehicle (#8402)

Such vehicles would be reported as `(Offline)` previously, as no Soc can be fetched and thus needs to behave as the `offline` template.

The SoC is fetched via the charger, so hardcode it to 0 in here

* Modbus: add coils  (#8385)

* OpenEVSE: fix api (#8415)

* chore: refactor offline vehicles (#8404)

* Easee: wait for api confirmation (#8307)

* Revert "Easee: wait for api confirmation (#8307)"

This reverts commit ce0e427.

* automatically switch session log energy unit (#8371)

Show the charged energy in the session log table and details in Wh for
values below 1 kWh and in kWh for larger values

* Fix nightly/release + integration workflow  (#8450)

* Revert "Add e2e tests with playwright (#8123)"

This reverts commit 0df8157.

* Revert "Revert "Add e2e tests with playwright (#8123)""

This reverts commit 2299306.

* fix nightly/release

* fix nightly/release

* fix nightly/release

---------

Co-authored-by: andig <cpuidle@gmx.de>

* chore: no integration on nightly

* chore: no integration on nightly

* chore: no integration on nightly

* Sessions: filter, monthly sums, paging (#8278)

* fmt

* Update i18n/en.toml

Co-authored-by: Simon Riepl <43091717+savus4@users.noreply.github.com>

* Use exp/slices

* Use mux consistently

* add integration tests

---------

Co-authored-by: andig <cpuidle@gmx.de>
Co-authored-by: C64Axel <33828760+C64Axel@users.noreply.github.com>
Co-authored-by: premultiply <4681172+premultiply@users.noreply.github.com>
Co-authored-by: Weblate (bot) <hosted@weblate.org>
Co-authored-by: Dusan Suja <bc.suja.dusan@googlemail.com>
Co-authored-by: Arna Lepikkö <arna.lepikko@telemail.fi>
Co-authored-by: ThinkEV <claas@rootdir.de>
Co-authored-by: Žiga Deisinger <ziga@deisinger.si>
Co-authored-by: Norbert Poch <npoch@yahoo.com>
Co-authored-by: Ruben Van Boxem <vanboxem.ruben@gmail.com>
Co-authored-by: salz3n <79696598+salz3n@users.noreply.github.com>
Co-authored-by: Daniel Paschke <paschdan@gmail.com>
Co-authored-by: Andreas Linde <42185+DerAndereAndi@users.noreply.github.com>
Co-authored-by: lex777777 <84906358+lex777777@users.noreply.github.com>
Co-authored-by: Oscar <OAltr@users.noreply.github.com>
Co-authored-by: Michael Heß <GrimmiMeloni@users.noreply.github.com>
Co-authored-by: RTTTC <94727758+RTTTC@users.noreply.github.com>
Co-authored-by: kscholty <47229207+kscholty@users.noreply.github.com>
Co-authored-by: Jan Alexander <jan.alexander@posteo.de>
Co-authored-by: Simon Riepl <43091717+savus4@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Easee: api commands ignored
3 participants