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

Loadpoint: cleanup phase configuration and drop deprecations #18638

Merged
merged 22 commits into from
Feb 9, 2025
Merged

Conversation

andig
Copy link
Member

@andig andig commented Feb 6, 2025

Fix #18606

@andig andig added the bug Something isn't working label Feb 6, 2025
@andig andig mentioned this pull request Feb 7, 2025
1 task
@naltatis
Copy link
Member

naltatis commented Feb 7, 2025

@andig ich hab das Wording jetzt konsequent auf PhasesConfigured (vorher teilweise ConfiguredPhases) umbenannt. Letzteres wäre sprachlich schöner, aber phasesConfigured ist der Key, den wir bislang auch schon in der DB verwenden. Daher hier aus Kompatiblitäts- und Einfachheitsgründen (kein Mapping!!) konsequent die bestehende Schreibweise.

@naltatis naltatis marked this pull request as ready for review February 7, 2025 14:11
@naltatis naltatis assigned andig and unassigned naltatis Feb 7, 2025
naltatis and others added 5 commits February 7, 2025 18:21
Co-authored-by: andig <andi@evcc.io>
Co-authored-by: andig <andi@evcc.io>
Co-authored-by: andig <andi@evcc.io>
Co-authored-by: andig <andi@evcc.io>
Co-authored-by: andig <andi@evcc.io>
@andig
Copy link
Member Author

andig commented Feb 8, 2025

@naltatis code ist eigentlich fein, aber Test will nicht- könntest Du bitte nochmal schauen?

@andig
Copy link
Member Author

andig commented Feb 8, 2025

@naltatis der letzte Commit sollte eigentlich ein nop sein (außer fehlendem publish). Siehst Du warum der Test failed?

@andig
Copy link
Member Author

andig commented Feb 8, 2025

Ok, ich sehe es im Test. M.E. darf die Logik nicht auf phases gehen sondern auf phasesConfigured! Bug im Test oder im UI?

@naltatis
Copy link
Member

naltatis commented Feb 8, 2025

Das hat wirklich was in der Phasenauswahl via UI kaputt gemacht. Ich schau rein.

Bildschirmfoto 2025-02-08 um 15 18 53

@naltatis
Copy link
Member

naltatis commented Feb 8, 2025

Mit dem letzten Commit hat sich das Publish Verhalten von phasesActive verändert. Bisher war es so, dass beim Umschalten der Configured Phasen via API der phasesActive sofort angepasst wurde. Das scheint jetzt nicht mehr der Fall zu sein.

@andig
Copy link
Member Author

andig commented Feb 8, 2025

Mit dem letzten Commit hat sich das Publish Verhalten von phasesActive verändert

Ähhhh- wo? Das kann nicht an 20845df liegen.

@andig
Copy link
Member Author

andig commented Feb 8, 2025

Ich denke im UI ist das ein Logikfehler- hier muss auf die konfigurierten Phasen geschaut werden, nicht auf die aktiven?

@naltatis
Copy link
Member

naltatis commented Feb 8, 2025

hier muss auf die konfigurierten Phasen geschaut werden, nicht auf die aktiven

Würde ich vom Gefühl auch sagen. Bei 1p3p Charger tun wir das auch. Bei "klassischen" aber nicht. Ich kann das ändern. Mich irritiert aber, dass deine Änderung (ich sehe da auch keinen Fehler drin) diesen Test gebrochen hat. Ich kann mir das gerade nicht erklären. Scheint aber zu einer Verhaltensänderung zu führen.

@andig
Copy link
Member Author

andig commented Feb 8, 2025

Grmpf. Wieso geht das immer noch nicht?

@naltatis
Copy link
Member

naltatis commented Feb 8, 2025

@andig sieht aus als ob wir jetzt ein Locking Problem haben. Der POST /api/config/loadpoints bleibt stecken, wenn ich ihn mit einem Democharger anlegen möchte.

@andig
Copy link
Member Author

andig commented Feb 8, 2025

Ich hab #18669 in den master gezogen und hier gemerged.

@andig
Copy link
Member Author

andig commented Feb 8, 2025

LGTM

@andig andig merged commit 07fb01d into master Feb 9, 2025
6 checks passed
@andig andig deleted the fix/phases-5 branch February 9, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config UI Regression: user phase setting lost on update
2 participants