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

Added more comments to ChargeStatus values #42

Merged
merged 2 commits into from
Apr 19, 2020
Merged

Added more comments to ChargeStatus values #42

merged 2 commits into from
Apr 19, 2020

Conversation

DerAndereAndi
Copy link
Contributor

trying to make the different Status values more clear on what they mean, using the explanations from https://evsim.gonium.net

@andig
Copy link
Member

andig commented Apr 19, 2020

Das stimmt nicht. C ist laden, nicht laden möglich?

@DerAndereAndi
Copy link
Contributor Author

Welcher Status wäre dann, Fahrzeug angeschlossen, Verbindung wird aufgebaut, oder Fahrzeug angeschlossen, laden ist beendet?

@andig
Copy link
Member

andig commented Apr 19, 2020 via email

@andig
Copy link
Member

andig commented Apr 19, 2020

Genauso ist es vei evsim dokumentiert und auch im Code. Bringt die Erweiterung der Kommentare da neue Erkenntnisse?

@DerAndereAndi
Copy link
Contributor Author

Nun, in den Kommentaren steht bei StatusB: Laden möglich: nein, das bedeutet soweit dass "zum Laden Bereit" nicht dazugehört. Und bei StatusC steht bisher nur Laden möglich: ja, aber nicht dass dies nur aktives Laden bedeutet.

Von dem her sind die bisherigen Kommentare für mich nicht eindeutig und eine Erweiterung der Kommentare erscheint mir sinnvoll.

@andig
Copy link
Member

andig commented Apr 19, 2020

Da müssten wir wohl @gonium fragen:

Sobald das Fahrzeug zum Laden bereit ist wird ein weiterer Widerstand (1k2 Ohm) zwischen CP und PE geschaltet. Damit sinkt die CP-PE-Spannung auf 6V, der Zustand C ist erreicht. Die Wallbox schaltet den Ladestrom frei, der Ladevorgang beginnt.

Ich habe "möglich" als "wird geladen" interpretiert. Die Überschriften in seiner Tabelle weichen davon etwas ab, die hatte ich in den Code übernommen. Ich denke wenn man das "möglich" streicht dann ist es eindeutig. Aber obs stimmt?

@DerAndereAndi
Copy link
Contributor Author

DerAndereAndi commented Apr 19, 2020

Die Wallbox schaltet den Ladestrom frei ...

Das ist denke ich der Knackpunkt. Das bedeutet ja nicht dass das Auto auch Strom zieht, sondern es könnte ab jetzt wenn es will das tun. Daher ist "möglich" da für mich deutlich.

D.h. so wie ich das interpretiere aber auch dass wenn die Wallbox so etwas wie Paused, Established, Finished, Active alle StatusC sind. Und davon ist nur einer aktives Laden. Sobald das Auto wieder Strom zieht, geht der von Finished z.B. auf Active. Und das ist für EVCC jedoch kein Unterschied.

Ich würde sogar denken dass wenn die Wallbox z.B. den Status Paused hat, das mit Enabled() = false gleichbedeutend ist. Wenn es elektrisch die Verbindung trennen würde, dann müsste sie ja neu aufgebaut werden. Vermute also in dem Fall bleibt es auf StatusC.

Ich denke deine Implementierung passt trotzdem, nur wenn eine Wallbox solche Status hat, muss halt klar sein wie die zugeordnet werden.

@andig
Copy link
Member

andig commented Apr 19, 2020

Das ist denke ich der Knackpunkt. Das bedeutet ja nicht dass das Auto auch Strom zieht, sondern es könnte ab jetzt wenn es will das tun. Daher ist "möglich" da für mich deutlich.

Ich glaube es ist im Prinzip auch egal. Ich verwende die Statuus so wie beschrieben- C heisst es wird geladen. Zumindest die Wallbe liefert sie auch genau so, tatsächlich als Buchstaben.

@DerAndereAndi
Copy link
Contributor Author

Dann würde ich das wort möglich durch aktiv ersetzen. Dann wäre es deutlicher wie es verwendet wird.

@andig andig merged commit e50cac2 into evcc-io:master Apr 19, 2020
@DerAndereAndi DerAndereAndi deleted the feature/charger-states branch April 19, 2020 16:15
@premultiply
Copy link
Member

Eigentlich ist es unnötig die fahrzeugseitigen Zustände hier abzubilden. Insbesondere sowas wie den Zustand F. Der kann auf der EVSE-Seite nicht vorkommen, da diese dann nicht mehr geht. :-)
Die Steuerungsseite benötigt eigentlich nur die von OpenWB bekannten 3 Zustände: Kein Fahrzeug verbunden, Fahrzeug verbunden, Fahrzeug verbunden mit Netzanforderung. Nicht alle Fahrzeuge laden auch unbedingt wenn sie Netzspannung anfordern. Manche fordern immer sofort Netzspannung an, andere tatsächlich während des Ladevorgangs, wieder andere auch für Timer oder Vorklimatisierung, Batterietemperierung usw.
Fehlerzustände werden ohnehin immer vom Fahrzeug selbst oder vom EVSE-Controller direkt behandelt und sind somit für eine übergeordnete Steuerung nicht von Relevanz.
Stattdessen wären wirkliche Klartextfehler ausserhalb dieser Zustände hilfreich: Spannungsausfall auf einzelnen Phasen, RCD ausgelöst und sowas.

@andig
Copy link
Member

andig commented Apr 27, 2020

Eigentlich ist es unnötig die fahrzeugseitigen Zustände hier abzubilden. Insbesondere sowas wie den Zustand F. Der kann auf der EVSE-Seite nicht vorkommen, da diese dann nicht mehr geht. :-)

Korrekt, sollte hier im Code aber auch nicht stören?

Die Steuerungsseite benötigt eigentlich nur die von OpenWB bekannten 3 Zustände: Kein Fahrzeug verbunden, Fahrzeug verbunden, Fahrzeug verbunden mit Netzanforderung.

Genau so werden sie von EVCC auch genutzt!

@DerAndereAndi
Copy link
Contributor Author

Eigentlich ist es unnötig die fahrzeugseitigen Zustände hier abzubilden. Insbesondere sowas wie den Zustand F. Der kann auf der EVSE-Seite nicht vorkommen, da diese dann nicht mehr geht. :-)

Korrekt, sollte hier im Code aber auch nicht stören?

Er stört insofern, dass man beim Implementieren z.B. nicht genau was worauf dieser Status bei der Wallbox abgebildet werden muss.

Die Steuerungsseite benötigt eigentlich nur die von OpenWB bekannten 3 Zustände: Kein Fahrzeug verbunden, Fahrzeug verbunden, Fahrzeug verbunden mit Netzanforderung.

Genau so werden sie von EVCC auch genutzt!

Wenn die Status nur mit den 3 Fällen genutzt werden, wieso dann die ganzen anderen im Code haben? Und wieso diese Status nicht auch so nennen. Momentan muss ich die Implementierung so genau kennen, um das richtig zu benutzen. Das sollte imho nicht so sein. APIs sollte man ja auch nutzen können, ohne das Backend dahinter zu kennen.

@andig
Copy link
Member

andig commented Apr 27, 2020

APIs sollte man ja auch nutzen können, ohne das Backend dahinter zu kennen.

Konkreter Vorschlag willkommen!

StatusE ChargeStatus = "E" // Fzg. angeschlossen: ja Laden möglich: nein
StatusF ChargeStatus = "F" // Fzg. angeschlossen: ja Laden möglich: nein
StatusA ChargeStatus = "A" // Fzg. angeschlossen: nein Laden aktiv: nein - Kabel nicht angeschlossen
StatusB ChargeStatus = "B" // Fzg. angeschlossen: ja Laden aktiv: nein - Kabel angeschlossen
Copy link
Member

Choose a reason for hiding this comment

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

Dito nur andersrum.

StatusD ChargeStatus = "D" // Fzg. angeschlossen: ja Laden möglich: ja
StatusE ChargeStatus = "E" // Fzg. angeschlossen: ja Laden möglich: nein
StatusF ChargeStatus = "F" // Fzg. angeschlossen: ja Laden möglich: nein
StatusA ChargeStatus = "A" // Fzg. angeschlossen: nein Laden aktiv: nein - Kabel nicht angeschlossen
Copy link
Member

Choose a reason for hiding this comment

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

ich denke das ist redundant. kein Fzg angeschlossen, keine Kabel.

@premultiply
Copy link
Member

Ich würde den aktuellen kombinierten ChargeStatus ganz durch die selbsterklärende Variablen
PlugStatus = True/False
ChargeStatus = True/False
ersetzen.

@andig
Copy link
Member

andig commented Apr 30, 2020

Ich würde den aktuellen kombinierten ChargeStatus ganz durch die selbsterklärende Variablen

Ich habe mittlerweile mindesten 4 unterschiedliche WB-Dokus gesehen die alle die gleiche Notation mit Status A..F, t.w. mit Zwischenstati wie B1 aus verwenden. Vor dem Hintergrund erscheint mir die Nomenklatur zu passen.

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.

None yet

3 participants