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

Add loadpoint config api (BC) #12958

Closed
wants to merge 0 commits into from
Closed

Add loadpoint config api (BC) #12958

wants to merge 0 commits into from

Conversation

andig
Copy link
Member

@andig andig commented Mar 15, 2024

Fix #12903

This PR deprecates the following yaml loadpoint settings:

  • mode
  • title
  • phases
  • min/max current
  • priority

TODO

  • UI for loadpoint properties @naltatis
  • UI for adding/changing charger @naltatis
  • UI for adding/changing lp meter @naltatis
  • e2e tests @naltatis
  • add translations @naltatis
  • UI default vehicle select @naltatis
  • UI circuit assignment @naltatis
  • API for assignable circuits @andig
  • API set meter, charger, defaultVehicle @andig
  • UI for adding/removing loadpoints @naltatis
  • API for adding/remove loadpoints @andig
  • implement config as settings
  • unable to save priority=0 @andig
  • remove orphan meters/chargers (no site/loadpoint ref) on startup
  • fix unit tests @andig
  • fix initial lp saving issue (logger, settings) @andig
  • store loadpoint settings in config (based on chore: persist device details as json #15224) @andig
  • handle pointer values like smartcostlimit
  • decide/adapt migration strategy (use evcc migrate?) @andig @naltatis
    • evcc migrate --reset removes all ui configured entities (vehicles, meters, chargers, loadpoints)
    • migrating these entities from yaml to ui will be done in a separate issue later
  • demo mode solution @naltatis
  • loadpoint settings adapter (config & runtime values) @andig

@andig andig added the enhancement New feature or request label Mar 15, 2024
@andig andig requested a review from naltatis March 15, 2024 09:17
@andig andig changed the title Add loadpoint config api Add loadpoint config api (BC) Mar 15, 2024
@naltatis
Copy link
Member

@andig die APIs sehen gut aus. Funktioniert alles wie es soll. Wir hatten ja gesagt, wir wollten den Scope klein halten und Vehicle/Charger erstmal ausklammern. Ich glaub es ist doch ne gute Idee, wenn wir die Referenz-Felder hier gleich mit aufnehmen. Sonst haben wir einen komischen Zwischenstand den man schwer releasen kann. Magst du die beiden Felder ergänzen?

Bildschirmfoto 2024-03-25 um 09 04 49

@andig
Copy link
Member Author

andig commented Mar 25, 2024

Mache ich. Am Fahrzeug könnten wir dann noch maxCurrent1p und minCurrent3p ergänzen 🙃.

@naltatis
Copy link
Member

↔ wider layout (general)
🏷️ loadpoint status

Bildschirmfoto 2024-03-25 um 20 20 24

@andig
Copy link
Member Author

andig commented Mar 25, 2024

Ich glaub es ist doch ne gute Idee, wenn wir die Referenz-Felder hier gleich mit aufnehmen.

So, jetzt nochmal langsam. Was meinst Du damit konkret? Phases z.B. ist doch drin?

@naltatis
Copy link
Member

Mit Referenz Felder meine ich:

  • lp.vehicle
  • lp.charger
  • lp.meter

Da stehen dann die 'name's der am loadpoint verknüpften devices drin und sind darüber auch änderbar.

@andig
Copy link
Member Author

andig commented Mar 26, 2024

Ah, ok. Aber erstmal ohne Updatefähigkeit?

@naltatis
Copy link
Member

Wie es passt. Update wäre cool (bspw default Fahrzeug). Lesen wäre aber auch ein Fortschritt.

@naltatis
Copy link
Member

@andig wollen wir, wo wir gerade dabei sind, nicht auch gleich soc und enable/disable mit reinnehmen? Dann sind wir hier durch und wir haben keinen Zwischenstand, den wir wieder erklären müssen. Könnten wir einfach flach abbilden. So viele Werte sind das ja glücklicherweise nicht.

  • socPollMode
  • socPollInterval
  • socEstimate
  • enableThreshold
  • enableDelay
  • disableThreshold
  • disableDelay

@andig
Copy link
Member Author

andig commented Mar 26, 2024

Die Refs sind drin (read-only).
socPollMode/socPollInterval gehören m.E. eher ans Fahrzeug wobei sich die Frage stellt, was wir dann mit integrated Devices machen- einfach immer abfragen wie wir lustig sind (These: ja).
Thresholds und Delay gehören an den LP können m.E: im ersten Schritt aber weg- weniger ist mehr, die Defaults sind nicht verkehrt.

@andig andig added the prio Priority label Mar 26, 2024
@naltatis
Copy link
Member

Mein eigentlicher Gedanke war, dass wir, wenn wir soc/enable/disable mit dazunehmen eine einfach verständliche Story für den Anwender haben: "Alle Loadpoint Einstellungen sind jetzt im UI." Deutlich einfacher zu verstehen als ein Misch-Setup wo sich von Release zu Release die Verhältnisse verändern.

Thresholds und Delay gehören an den LP können m.E: im ersten Schritt aber weg- weniger ist mehr, die Defaults sind nicht verkehrt.

Wäre für mich bspw. ein Deal Breaker. Ich nutze hier für das Zusammenspiel von EV und Heizstab bewusst nicht die Defaults. Soc-Settings ans Fahrzeug migrieren find ich auch gut, aber würde ich in einem separaten Schritt machen. Hier wäre mein Fokus erstmal "nur" yaml-Einstellungen zu UI-Einstellungen konvertieren.

@naltatis
Copy link
Member

@andig Wenn priority am lp 0 ist, bekomme ich keinen Wert und hab damit auch keine Auswahl im UI. Das könnte ich mit nem Default lösen. Schöner wäre aber der echte Wert.

Das eigentliche Problem ist, dass Speichern von Priority != 0 funktioniert. Schaust du da noch mal rein?

@naltatis
Copy link
Member

improved layout, more fields
Bildschirmfoto 2024-03-27 um 16 31 18

real values, badge colors
Bildschirmfoto 2024-03-27 um 16 32 16

@andig
Copy link
Member Author

andig commented Mar 28, 2024

Wäre für mich bspw. ein Deal Breaker

Verstehe ich nicht. Dir LPs brauchts weiter in der yaml, also sind auch die Settings noch da?

@naltatis
Copy link
Member

Verstehe ich nicht. Dir LPs brauchts weiter in der yaml, also sind auch die Settings noch da?

Ja, mir ging es ja um diesen Punkt:

Deutlich einfacher zu verstehen als ein Misch-Setup wo sich von Release zu Release die Verhältnisse verändern.

@andig
Copy link
Member Author

andig commented Mar 28, 2024

Yoah, aber breaking ist da nix. Schöner wärs.

@andig
Copy link
Member Author

andig commented Mar 28, 2024

unable to save priority=0

@naltatis ist behoben- auch 0 wird jetzt ausgegeben. Spannend ist allerdings auch SmartCostLimit. Es gibt keine Möglichkeit ein Limit zu löschen und per Default würde <0ct geladen.

@naltatis
Copy link
Member

Das Problem hat smartCostLimit ja in der aktuellen api auch schon. 0 bedeutet "aus". Negative Werte sind erlaubt und auch gewünscht.
SmartCostLimit würd ich in config ui auch nicht anzeigen. Der ist in den Einstellungen im Main ui gut aufgehoben (Visualisierung und so) und ich würde, wo immer möglich und sinnvoll, nicht mehrere Wege für die gleiche Einstellung anbieten.

@andig
Copy link
Member Author

andig commented Mar 29, 2024

@naltatis soc ist auch drin. Du müsstest rebasen ;)

@github-actions github-actions bot added the stale Outdated and ready to close label Apr 26, 2024
@andig andig removed the stale Outdated and ready to close label Apr 27, 2024
@github-actions github-actions bot added the stale Outdated and ready to close label May 18, 2024
@naltatis
Copy link
Member

@andig Ich hab jetzt die UI für das Hinzufügen und Entfernen von Ladepunkten hinzugefügt. Die entsprechenden Endpunkte (POST, DELETE) habe ich beispielhaft angelegt.

@naltatis
Copy link
Member

Anlegen schlägt fehl:

curl 'http://localhost:7070/api/config/loadpoints' \
-X 'POST' \
--data-binary '{"title":"a","phases":0,"minCurrent":6,"maxCurrent":16,"priority":1,"mode":"","thresholds":{"enable":{"delay":2,"threshold":4},"disable":{"delay":3,"threshold":5}},"soc":{"poll":{"mode":"pollconnected","interval":7},"estimate":true},"defaultVehicle":"vehicle_5","charger":"db:38","meter":""}'

{
    "error": "decoding failed due to the following error(s):\n\n'' has invalid keys: defaultVehicle"
}

@andig
Copy link
Member Author

andig commented Jun 30, 2024

@naltatis das defaultVehicle muss hier vehicle heissen, analog:

	CircuitRef string `mapstructure:"circuit"` // Circuit reference
	ChargerRef string `mapstructure:"charger"` // Charger reference
	VehicleRef string `mapstructure:"vehicle"` // Vehicle reference
	MeterRef   string `mapstructure:"meter"`   // Charge meter reference

@naltatis
Copy link
Member

@andig magst du dir hier mal die Lint/Test Fehler anschauen, dass wir den Branch wie der Grün bekommen. Circuit Umbau

@naltatis
Copy link
Member

Wir haben hier noch ein Datenproblem. Im UI kann der Nutzer zwischen auto/1p/3p auswählen. Die Auto-Option (0) funktioniert aber nur, wenn der initialisierte Charger am Ladepunkt auch 1p3p Capability hat. Sonst schlägt das Speichern fehl. Aktuell hat die UI keinen Weg das rauszufinden. Hier brauchen wir eine Lösung.

Als schnelle Lösung würde ich vorschlagen, dass wir SetPhases so anpassen, dass die Werte 0,1,3 immer akzeptiert werden.

Langfristig müssen wir hier a) die Info ans UI transportieren damit auch die Auto-Option nur angeboten wird, wenns möglich ist und b) auch bei Szenarien wie "Charger ändern am LP ändern" sicher stellen, dass wir dieses Setting dann entsprechend mit korrigieren.

@naltatis
Copy link
Member

naltatis commented Jul 10, 2024

🍕 One of the last puzzle pieces.
🎉 Add/remove loadpoint (+charger, +meter) via UI

Add.Loadpoint.webm

@naltatis
Copy link
Member

@andig Der generelle Round-Trip mit Anlegen, Neustarten, Bearbeiten, Löschen funktioniert. Beim Anlegen gibts aber noch ein generelles Problem. Hier scheinen die Settings-Daten nicht persistiert zu werden. Nach einem Neustart fehlt bspw. der Title des Ladepunktes. In der Datenbank (settings) sind die Werte auch nicht drin. Späteres bearbeiten funktioniert und behält auch die Daten.

@VolkerK62
Copy link
Contributor

im Video ganz am Schluss ist bei dem neuen Loadpoint KEIN Lademodus markiert.

@andig
Copy link
Member Author

andig commented Jul 10, 2024

@naltatis wir haben noch ein anderes Problem mit Loadpointnamen die auch der Logger braucht. Diese können nicht mehr aus der Reihenfolge in der YAML abgeleitet werden sondern müssten jetzt explizit konfiguriert werden- wiederum aber nur für interne Zwecke. Da bräuchten wie eine schlaue Idee. Vmtl. klappt deshalb auch das Speichern nicht da der Name Teil des Settings-Keys ist.

@naltatis
Copy link
Member

im Video ganz am Schluss ist bei dem neuen Loadpoint KEIN Lademodus markiert.

Ja, wird der gleiche Grund sein, wieder fehlende Namen beim ersten Anlegen.

@naltatis
Copy link
Member

@andig Anlegen von Ladepunkten führt aktuell zu einem Memory Error

Request

curl 'http://localhost:7071/api/config/loadpoints' \
-X 'POST' \
-H 'Accept: application/json' \
-H 'Content-Type: application/json' \
--data-binary '{"title":"Hello","phases":3,"minCurrent":6,"maxCurrent":16,"priority":0,"mode":"","thresholds":{"enable":{"delay":60000000000,"threshold":0},"disable":{"delay":180000000000,"threshold":0}},"soc":{"poll":{"mode":"pollcharging","interval":3600000000000},"estimate":false},"vehicle":"","charger":"db:82","meter":""}'

Log

[server] ERROR 2024/08/11 11:20:31 http: panic serving [::1]:55715: runtime error: invalid memory address or nil pointer dereference
goroutine 691 [running]:
net/http.(*conn).serve.func1()
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:1898 +0xb0
panic({0x105124de0?, 0x107a818a0?})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/runtime/panic.go:770 +0x124
github.com/evcc-io/evcc/core.(*Loadpoint).migrateSettings(0x14001e4e408)
	/Users/michael/lab/evcc/core/loadpoint.go:315 +0x3f4
github.com/evcc-io/evcc/core.NewLoadpointFromConfig(0x14001010d38?, {0x0?, 0x0?}, 0x14001f187e0)
	/Users/michael/lab/evcc/core/loadpoint.go:226 +0x430
github.com/evcc-io/evcc/server.(*HTTPd).RegisterSystemHandler.newLoadpointHandler.func30({0x1056a6680, 0x14001882180}, 0x40?)
	/Users/michael/lab/evcc/server/http_config_loadpoint_handler.go:193 +0x11c
net/http.HandlerFunc.ServeHTTP(0x14000c56900?, {0x1056a6680?, 0x14001882180?}, 0x1040e4160?)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:2166 +0x38
github.com/evcc-io/evcc/server.(*HTTPd).RegisterSystemHandler.ensureAuthHandler.func17.1({0x1056a6680, 0x14001882180}, 0x14000c56900)
	/Users/michael/lab/evcc/server/http_auth.go:141 +0x94
net/http.HandlerFunc.ServeHTTP(0x10532c920?, {0x1056a6680?, 0x14001882180?}, 0x1b?)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:2166 +0x38
github.com/gorilla/handlers.(*cors).ServeHTTP(0x14000791560, {0x1056a6680, 0x14001882180}, 0x14000c56900)
	/Users/michael/go/pkg/mod/github.com/gorilla/[email protected]/cors.go:136 +0x5bc
github.com/gorilla/handlers.CompressHandler.CompressHandlerLevel.func1({0x1056a0ba0, 0x14000191260}, 0x14000c56900)
	/Users/michael/go/pkg/mod/github.com/gorilla/[email protected]/compress.go:141 +0x4e8
net/http.HandlerFunc.ServeHTTP(0x10532c920?, {0x1056a0ba0?, 0x14000191260?}, 0xc?)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:2166 +0x38
github.com/evcc-io/evcc/server.jsonHandler.func1({0x1056a0ba0, 0x14000191260}, 0x14000c56900)
	/Users/michael/lab/evcc/server/http_site_handler.go:73 +0xf0
net/http.HandlerFunc.ServeHTTP(0x14000c567e0?, {0x1056a0ba0?, 0x14000191260?}, 0x2?)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:2166 +0x38
github.com/gorilla/mux.(*Router).ServeHTTP(0x14000b34840, {0x1056a0ba0, 0x14000191260}, 0x14000c566c0)
	/Users/michael/go/pkg/mod/github.com/gorilla/[email protected]/mux.go:212 +0x194
net/http.serverHandler.ServeHTTP({0x105698060?}, {0x1056a0ba0?, 0x14000191260?}, 0x6?)
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:3137 +0xbc
net/http.(*conn).serve(0x140007913b0, {0x1056a9810, 0x14001e81980})
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:2039 +0x508
created by net/http.(*Server).Serve in goroutine 1
	/opt/homebrew/Cellar/go/1.22.1/libexec/src/net/http/server.go:3285 +0x3f0
[site  ] DEBUG 2024/08/11 11:20:33 ----

@naltatis
Copy link
Member

Added circuit assigment (when availabe) to loadpoint ui.
Introduced tab structure (solar, vehicle, electrical)

circuit, tabs

@andig andig force-pushed the feat/loadpoint-config branch 2 times, most recently from f2755f5 to 419c9f0 Compare October 4, 2024 11:31
@naltatis naltatis force-pushed the feat/loadpoint-config branch from 56fd7d9 to a086289 Compare October 24, 2024 10:06
core/settings/config.go Outdated Show resolved Hide resolved
cmd/setup.go Outdated Show resolved Hide resolved
@naltatis
Copy link
Member

@andig Ich hab einen Failing-Test hinzugefügt, der das Problem beim Aktualiseren eines angelegten Ladepunkts verdeutlicht. Aktualisiert man bspw. die Priorität, lässt aber den Titel gleich, verlieren wir den Titel.

@naltatis
Copy link
Member

naltatis commented Nov 5, 2024

@andig Die Initialisierung der Loadpoints schlägt fehl, wenn man einen LP konfiguriert hat und das Sponsortoken löscht (vmtl. auch wenns abläuft)

[main  ] FATAL 2024/11/05 20:20:05 cannot create charger 'db:236': cannot create charger type 'template': cannot create charger type 'go-e': sponsorship required, see

Aktuell werden in diesem Fall gar keine LPs mehr angezeigt. Heißt man kann den betroffenen LP auch nicht entfernen um einen validen Zustand herzustellen. Magst du da mal reinschauen?

@andig andig force-pushed the feat/loadpoint-config branch from fd7e1e3 to 32550fe Compare December 6, 2024 17:27
@naltatis
Copy link
Member

Setup von evcc mit einer leeren evcc.yaml 🎉

setup.webm

@naltatis
Copy link
Member

@andig ich hab gerade noch mal ausprobiert wie sich die Einstellungen von UI-konfigurierten Fahrzeugen an UI-konfigurierten Ladepunkten verhalten. Das sieht alles korrekt, bzw. wie erwartet aus. Die fahrzeugspezifischen Werte (bspw. vehicle.db:329.planSoc) landen weiterhin gepräfixt in der settings Tabelle. Heißt das Fahrzeug kann zwischen den Ladepunkte weiterhin hin und verwechseln ohne dass wir Daten verlieren.

Dementsprechend steht hier nur noch eine gute Lösung für den Demo-Modus aus. Hier tendiere ich gerade dazu das aktuelle Verhalten erst einmal zu belassen:

  • keine evcc.yaml > Demo Modus
  • existierende evcc.yaml > Normaler Modus (Welcome Dialog, wenn keine Ladepunkte)

Ein explizites Anwählen des Demo Modus (bspw. durch CLI Param) würd ich erst dann machen, wenn wir Config UI nicht mehr hinter experimental Flag haben.

@naltatis naltatis mentioned this pull request Dec 31, 2024
cmd/migrate.go Outdated Show resolved Hide resolved
cmd/setup.go Outdated Show resolved Hide resolved
core/keys/loadpoint.go Outdated Show resolved Hide resolved
EnableThreshold = "enableThreshold"
DisableThreshold = "disableThreshold"
EnableDelay = "enableDelay"
DisableDelay = "disableDelay"
BatteryBoost = "batteryBoost"

PhasesConfigured = "phasesConfigured" // configured phases (1/3, 0 for auto on 1p3p chargers, nil for plain chargers)
Phases = "phases" // configured phases (1/3, 0 for auto on 1p3p chargers, nil for plain chargers)
PhasesConfigured = "phasesConfigured" // TODO mirrors "phases" for UI purposes
Copy link
Member Author

Choose a reason for hiding this comment

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

Zur Diskussion

Copy link
Member Author

Choose a reason for hiding this comment

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

Depends on #18062

core/loadpoint/config.go Outdated Show resolved Hide resolved
i18n/en.toml Outdated Show resolved Hide resolved
i18n/en.toml Outdated Show resolved Hide resolved
i18n/en.toml Outdated Show resolved Hide resolved
server/http_config_device_handler.go Outdated Show resolved Hide resolved
server/http_config_device_handler.go Outdated Show resolved Hide resolved
@andig andig closed this Jan 7, 2025
@andig andig force-pushed the feat/loadpoint-config branch from a835bfc to 54b6b91 Compare January 7, 2025 21:22
@naltatis naltatis mentioned this pull request Jan 8, 2025
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Things to do later prio Priority
Projects
Development

Successfully merging this pull request may close these issues.

UI: Make loadpoints configurable
4 participants