Update lp.applyAction() handling #1942

rivengh · 2021-11-26T12:18:38Z

allow GetTargetSoC() and GetMinSoC() to be used even when no vehicle is connected
use SetMinSoC() and SetTargetSoC() functions in applyAction()
allow to set minSoC to 0 with applyAction()
apply OnIdentify actions only when OnIdentify node is present in vehicle configuration (optional parameter)

…ow to set minSoC to 0

rivengh · 2021-11-26T13:28:45Z

Vehicle connected and identified:

[lp-1  ] INFO 2021/11/26 12:51:20 car connected
[lp-1  ] INFO 2021/11/26 12:54:31 vehicle updated: unknown -> VW e-up!
[lp-1  ] INFO 2021/11/26 12:54:31 set charge mode: pv
[lp-1  ] INFO 2021/11/26 12:54:31 set min current: 6
[lp-1  ] INFO 2021/11/26 12:54:31 set max current: 16
[lp-1  ] INFO 2021/11/26 12:54:31 set min soc: 20
[lp-1  ] INFO 2021/11/26 12:54:31 set target soc: 60

Reset to loadpoint defaults on vehicle disconnect:

[lp-1  ] INFO 2021/11/26 12:56:40 car disconnected
[lp-1  ] INFO 2021/11/26 12:56:40 vehicle updated: VW e-up! -> unknown
[lp-1  ] INFO 2021/11/26 12:56:40 set charge mode: off
[lp-1  ] INFO 2021/11/26 12:56:40 set min current: 6
[lp-1  ] INFO 2021/11/26 12:56:40 set max current: 16
[lp-1  ] INFO 2021/11/26 12:56:40 set min soc: 0
[lp-1  ] INFO 2021/11/26 12:56:40 set target soc: 80

…cle configuration (optional parameter)

rivengh · 2021-11-29T15:14:35Z

evcc vehicles dump without Identifiers and with OnIdentified options:

e-up
----
SoC:            71%
...
Capacity:       32kWh
OnIdentified:   &{pv 6 16 20 60}

Dump without OnIdentified and with Identifiers options:

dummy
-----
SoC:           100%
Charge status: B
Capacity:      50kWh
Identifiers:   [DUMMY:IDENT]

andig · 2021-11-29T15:26:31Z

apply OnIdentify actions only when OnIdentify node is present in vehicle configuration (optional parameter)

Was willst Du hier erreichen?

rivengh · 2021-11-29T16:22:55Z

apply OnIdentify actions only when OnIdentify node is present in vehicle configuration (optional parameter)

Was willst Du hier erreichen?

Das ist relevant im Zusammenhang mit:

allow to set minSoC to 0 with applyAction()

Dadurch wurde minSoC bei Erkennung immer auf 0 gesetzt, auch wenn OnIdentified am Fahrzeug überhaupt nicht konfiguriert war.

andig · 2021-11-30T06:48:43Z

apply OnIdentify actions only when OnIdentify node is present in vehicle configuration (optional parameter)

Was willst Du hier erreichen?

Das ist relevant im Zusammenhang mit:

allow to set minSoC to 0 with applyAction()

Dadurch wurde minSoC bei Erkennung immer auf 0 gesetzt, auch wenn OnIdentified am Fahrzeug überhaupt nicht konfiguriert war.

Das Problem ist doch aber, dass OnIdentified in dem Fall überhaupt ausgeführt wird, nicht dass irgendwas mit minSoC passiert, oder?

andig

Danke für den PR- bitte schau mal die Kommentare an. Wenn ich alles richtig verstanden habe, scheint das eine gute Präzisierung des Verhaltens zu sein- vielen Dank!

core/loadpoint.go

core/loadpoint_api.go

cmd/dumper.go

rivengh · 2021-11-30T07:55:51Z

Dadurch wurde minSoC bei Erkennung immer auf 0 gesetzt, auch wenn OnIdentified am Fahrzeug überhaupt nicht konfiguriert war.

Das Problem ist doch aber, dass OnIdentified in dem Fall überhaupt ausgeführt wird, nicht dass irgendwas mit minSoC passiert, oder?

Da hast du Recht. Der Effekt war in diesem Fall eben nur am minSoC zu sehen.

andig · 2021-11-30T08:05:19Z

allow to set minSoC to 0 with applyAction()

Ich glaube das funktioniert nicht. 0 ist der Defaultwert. ApplyAction kann nicht wissen ob der gesetzt ist oder nicht. Eine Alternative wäre, alle Einzelwerte zu Pointern zu machen und alle Vergleiche gegen nil zu machen.

rivengh · 2021-11-30T08:13:02Z

allow to set minSoC to 0 with applyAction()

Ich glaube das funktioniert nicht. 0 ist der Defaultwert. ApplyAction kann nicht wissen ob der gesetzt ist oder nicht. Eine Alternative wäre, alle Einzelwerte zu Pointern zu machen und alle Vergleiche gegen nil zu machen.

Das wäre in der Tat noch eine Erweiterung auf die einzelnen Elemente des ActionConfig structs.
Ich wollte es erst mal nicht noch komplexer machen und habe angenommen, wenn man OnIdentified schon konfiguriert, dass man auch alle Werte davon entsprechend setzt.
Kann das aber gerne auch noch erweitern, wenn du möchtest?

Ansonsten funktioniert das Setzen auf 0 schon, da die if-Condition beim minSoC auf >= 0 abfragt.

andig · 2021-11-30T08:38:13Z

Ansonsten funktioniert das Setzen auf 0 schon, da die if-Condition beim minSoC auf >= 0 abfragt.

Genau das funktioniert eben nicht (immer). Wenn ich das Feld gar nicht im YAML hab ists blöderweise auch 0.

Ich würde Vorschlagen, die Einzelwerte (alle) auf Pointer umzubauen und die Settings selbst ohne Pointer zu übergeben.

rivengh · 2021-11-30T08:57:35Z

Ich würde Vorschlagen, die Einzelwerte (alle) auf Pointer umzubauen und die Settings selbst ohne Pointer zu übergeben.

Alles klar, das ist sicher am konsequentesten. Ich baue das dann noch um.

Danke für dein Review.

…mitempty mapstructure annotation

# Conflicts: # go.mod

rivengh · 2021-11-30T16:48:37Z

So, habe es jetzt nochmal umgebaut: Alle Elemente von ActionConfig sind nun Pointer und die Vergleiche in applyAction werden gegen nil gemacht. Die Settings selbst habe ich auch als Pointer drin gelassen. Fand ich jetzt ganz praktisch, wenn ich auch direkt darauf auf nil abfragen kann. Was meinst du?

cmd/dumper.go

core/loadpoint.go

andig · 2021-11-30T18:50:54Z

core/loadpoint.go

-		lp.publish("targetSoC", action.TargetSoC)
-		lp.Unlock()
+func (lp *LoadPoint) applyAction(action *api.ActionConfig) {
+	if action != nil {


Ich find den doppelten Pointer merkwürdig

Doppelter Pointer ist nun entfernt.

andig · 2021-11-30T18:52:49Z

Das ist alles sehr unglücklich. Vmtl müssten wir die Standardkonfig auch als tristate (also mit Pointern) machen. Die würde ich aber ungern überall im Code haben. Mir fehlt grad eine gute Idee. So schein das Problem aus der letzten Überarbeiten jedenfalls immer noch vorhanden, nru diesmal von der Vehicle auf die Standardconfig verlagert.

andig · 2021-11-30T18:55:51Z

…oder wir argumentieren dass das „as designed“ ist. Bin grad unsicher. Dann könnte man nur am Fahrzeug entscheiden, ob Standardwerte überschrieben werden, der Standard selbst wird aber immer wieder hergestellt.

andig · 2021-11-30T22:16:15Z

Die Idee gefällt mir eigentlich gut. Bliebe also nur, das doppelte nil wieder zu beseitigen wenn Du so nett sein willst.

rivengh · 2021-11-30T22:28:38Z

Danke Dir. Ich mache mich morgen nochmal ran.

…onConfig)

…int default values

…ngs (evcc-io#1942) allow GetTargetSoC() and GetMinSoC() to be used even when no vehicle is connected use SetMinSoC() and SetTargetSoC() functions in applyAction() allow to set minSoC to 0 with applyAction() apply OnIdentify actions only when OnIdentify node is present in vehicle configuration (optional parameter)

rivengh added 3 commits November 26, 2021 13:02

allow GetTargetSoC() and GetMinSoC() even when no vehicle is connected

26ac072

use SetMinSoC() and SetTargetSoC() functions in applyAction() and all…

eecae33

…ow to set minSoC to 0

remove Error return value from lp.SetMinSoC and lp.SetTargetSoC()

2fc8c87

andig added the enhancement New feature or request label Nov 26, 2021

rivengh added 5 commits November 28, 2021 12:33

apply OnIdentify actions only when OnIdentify node is present in vehi…

bc4723c

…cle configuration (optional parameter)

dump OnIdentified configuration

faf5704

update mock of OnIdentified API

d929926

Merge branch 'master' into update-apply-action

1b08291

dump OnIdentified and Identifiers config only if defined

115fdef

rivengh marked this pull request as draft November 29, 2021 15:10

rivengh marked this pull request as ready for review November 29, 2021 15:15

andig reviewed Nov 30, 2021

View reviewed changes

core/loadpoint.go Outdated Show resolved Hide resolved

core/loadpoint_api.go Show resolved Hide resolved

core/loadpoint_api.go Show resolved Hide resolved

cmd/dumper.go Outdated Show resolved Hide resolved

dereference and add field names (%+v) when dumping OnIdentified struct

5febff6

rivengh added 5 commits November 30, 2021 11:29

remove usage of deprecated onDisconnect.TargetSoC value

5c8891e

use pointer type for each element of ActionConfig structure and add o…

72c6198

…mitempty mapstructure annotation

minor cleanup: move code for api.Vehicle dump under vehicle section

a9bf04a

Merge branch 'master' into update-apply-action

5b3c38f

# Conflicts: # go.mod

remove indirect from github.com/cjrd/allocate package

3b6f67e

andig reviewed Nov 30, 2021

View reviewed changes

rivengh added 3 commits December 1, 2021 10:54

change type of OnIdentified() vehicle API to ActionConfig (from *Acti…

223a38e

…onConfig)

implement stringer function for ActionConfig structure

577ab5c

align default SoC.Target and SoC.Min initialization with other loadpo…

61f5447

…int default values

andig merged commit a45e240 into evcc-io:master Dec 1, 2021

rivengh deleted the update-apply-action branch December 1, 2021 13:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update lp.applyAction() handling #1942

Update lp.applyAction() handling #1942

rivengh commented Nov 26, 2021 •

edited

rivengh commented Nov 26, 2021

rivengh commented Nov 29, 2021

andig commented Nov 29, 2021

rivengh commented Nov 29, 2021

andig commented Nov 30, 2021

andig left a comment

rivengh commented Nov 30, 2021

andig commented Nov 30, 2021

rivengh commented Nov 30, 2021 •

edited

andig commented Nov 30, 2021 •

edited

rivengh commented Nov 30, 2021

rivengh commented Nov 30, 2021

andig Nov 30, 2021

rivengh Dec 1, 2021

andig commented Nov 30, 2021

andig commented Nov 30, 2021

andig commented Nov 30, 2021

rivengh commented Nov 30, 2021

Update lp.applyAction() handling #1942

Update lp.applyAction() handling #1942

Conversation

rivengh commented Nov 26, 2021 • edited

rivengh commented Nov 26, 2021

rivengh commented Nov 29, 2021

andig commented Nov 29, 2021

rivengh commented Nov 29, 2021

andig commented Nov 30, 2021

andig left a comment

Choose a reason for hiding this comment

rivengh commented Nov 30, 2021

andig commented Nov 30, 2021

rivengh commented Nov 30, 2021 • edited

andig commented Nov 30, 2021 • edited

rivengh commented Nov 30, 2021

rivengh commented Nov 30, 2021

andig Nov 30, 2021

Choose a reason for hiding this comment

rivengh Dec 1, 2021

Choose a reason for hiding this comment

andig commented Nov 30, 2021

andig commented Nov 30, 2021

andig commented Nov 30, 2021

rivengh commented Nov 30, 2021

rivengh commented Nov 26, 2021 •

edited

rivengh commented Nov 30, 2021 •

edited

andig commented Nov 30, 2021 •

edited