Session log: close unfinished sessions on startup

[GrimmiMeloni]

fix #8788

WIP. Marking as draft.

[GrimmiMeloni]

@andig I am having some trouble with placement of the cleanup routine, and could use some help.

Right now it resides in the core/session/DB. It feels somewhat correct there, as the cleanup is session specific. I would therefore not put in server/db.

However, core/session/DB gets instantiated per loadpoint, so effectively this will run the cleanup once per loadpoint, even though only the first will actually do anything. All other executions of the cleanup will simply select the records, not see any remaining problems, and move on.

Ideal would be a database reference somewhere on "site" level, maybe in NewSiteFromConfig. But that would then "bleed" knowledge about sessions out of the loadpoint into site. Do you maybe have a good idea where we could put this? Or am I simply too concerned about referencing sessions in other parts of the system but the loadpoint?

[andig]

Ich versteh die Frage nicht- Du hast die Routine ja schon? Ich würde da auch nicht soviel Magie rein packen sondern einfach die letzte Session je Loadpoint- falls offen- mit dem Zählerstand überschreiben. Die Vergangenheit muss halt von Hand repariert werden. In dem Fall passt dann auch der Loop über die Loadpoints.

[GrimmiMeloni]

"je Loadpoint" war entscheidend. Das war generell nicht sauber.
Ich habe das jetzt entsprechend korrigiert und erweitert.
Der Fix für die Historie war schon da, den habe ich jetzt drin gelassen. Zusätzlich wird jetzt auch die letzte Session (falls diese offen ist) mit dem aktuellen ChargeMeter Wert gefüllt.

Unittest ist hoffentlich verständlich. Mir war wichtig im Wechsel in die DB zu schreiben, damit das Handling bei nicht aufeinander folgenden Primärschlüsseln auch geprüft wird.

IMHO ist das so fertig.

[andig]

Konsistenz: wir sollten noch checken, ob wir die Session auch beim Abbruch (nicht nur beim Neustart) schließen.

[GrimmiMeloni]

Konsistenz: wir sollten noch checken, ob wir die Session auch beim Abbruch (nicht nur beim Neustart) schließen.

Das erledigt nach meinem Verständnis der Shutdown hook der in site.Go#L151 registriert wird.

[andig]

Ich bin eigentlich kein Fan davon, diese Komplexität einzubauen. Es sei denn, wir hätten heute noch typische Fälle, wo das vorkommen könnte? Falls nein wäre die Korrektur auch lokal in SQL durchzuführen.

[GrimmiMeloni]

Ich kann leider auch nicht einschätzen wie häufig das passiert.
Es scheint eher die Ausnahme zu sein (im Issue wird initial Stromausfall erwähnt). So fern User evcc nicht mit SIGKILL "restarten" oder die Elektroinstallation fragwürdig ist, sollte es eher gar nicht passieren.

In Summe find ich der Change ist aber übersichtlich. Wir haben eine Cleanup Routine die einmalig in site.go beim Startup aufgerufen wird. Die Routine selbst ist sauber getestet - ich würde es reinnehmen. Dann ist zumindest den betroffenen Usern geholfen.

[andig]

Dann rein damit- danke!

[naltatis]

@GrimmiMeloni @andig

Der aktuelle Master möchte nicht mehr mit meiner Test-Datenbank reden.

Hier die Datenbank zum Reproduzieren evcc.db.zip

[db    ] INFO 2023/10/14 18:46:53 using sqlite database: /Users/michael/.evcc/evcc.db
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x105bd9aac]

goroutine 1 [running]:
github.com/evcc-io/evcc/core/session.(*DB).ClosePendingSessionsInHistory(0x140004de6e0, 0x0)
	/Users/michael/lab/evcc/core/session/db.go:77 +0x1ac
github.com/evcc-io/evcc/core.NewSiteFromConfig(0x14000f73050, 0x106dcdd28?, {0x140019c5630, 0x2, 0x2}, {{0x5e}, {0x106dcdd28, 0x140019b9680}, {0x106dcdd28, 0x140019b96b0}, ...})
	/Users/michael/lab/evcc/core/site.go:146 +0x65c
github.com/evcc-io/evcc/cmd.configureSite(0x14000c73538?, {0x140019c5630?, 0x14000c73550?, 0x5?}, {{0x5e}, {0x106dcdd28, 0x140019b9680}, {0x106dcdd28, 0x140019b96b0}, {0x106dcdd00, ...}, ...})
	/Users/michael/lab/evcc/cmd/setup.go:640 +0x88
github.com/evcc-io/evcc/cmd.configureSiteAndLoadpoints({{0x0, 0x0}, {{0x105e16dc3, 0x4}, {0x105e2d0f6, 0xa}, 0x1b9e}, {0x14000c726db, 0x5}, {0x0, ...}, ...})
	/Users/michael/lab/evcc/cmd/setup.go:636 +0x1bc
github.com/evcc-io/evcc/cmd.runRoot(0x14000c98700?, {0x105e23b65?, 0x7?, 0x105e16dd3?})
	/Users/michael/lab/evcc/cmd/root.go:191 +0x870
github.com/spf13/cobra.(*Command).execute(0x108b53f00, {0x1400004e730, 0x2, 0x2})
	/Users/michael/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:944 +0x640
github.com/spf13/cobra.(*Command).ExecuteC(0x108b53f00)
	/Users/michael/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x320
github.com/spf13/cobra.(*Command).Execute(...)
	/Users/michael/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992
github.com/evcc-io/evcc/cmd.Execute()
	/Users/michael/lab/evcc/cmd/root.go:110 +0x24
main.main()
	/Users/michael/lab/evcc/main.go:45 +0x38
exit status 2

[GrimmiMeloni]

@naltatis diese Datenbank wurde aber nicht von evcc (konkret gorm) geschrieben, oder? Die fehlenden Werte für den Charge Meter dürften meiner Ansicht nach nie als NULL in der DB landen, da wir immer einen float64 für das Feld meterStart haben.

[naltatis]

Die Datenbank ist schon sehr alt. Das mit den null Werten haben wir irgendwann mal geändert.

[naltatis]

Ich hab da gerade mal reingeschaut. Das null betrifft ja nicht nur alte Einträge. Auch die neuen haben das teilweise ja.
Diese DB verwende ich überwiegend mit der demo.yaml aus dem Projekt oder Abwandlungen davon.

[ThinkEV]

Da hier nach der Häufigkeit gefragt wurde: Bei mir gibt es externe Netzschwankungen (60x in den letzten 18 Monaten), kommen von der 380kV- und 110kV- Netzebene. Dann schaltet die Powerwall das Netz weg und ca. 7 Sekunden danach auf Backup und dann ist der Raspi mit evcc erstmal hart abgewürgt. Also kein rein theoretisches Szenario.