Keba: use enable / current register for failsafe heartbeat

[mfuchs1984]

Fix #21093

According to #21093 (reply in thread) Keba stated that just reading the Enabled register in the heartbeat function is not sufficient to reset the failsafe timeout. Instead, renewing current or enabled register will do the job.

Dear Mr. Lang,

Simply reading the values is not sufficient in this case.

You should, for example:

Regularly renew its current specification: Register 5004, or
Confirm the 'enabled' status: Register 5014, or
Renew its failsafe settings.
Kind regards,
Your KEBA Energy Automation Team
Emobility Support

So I used the exisiting Enable function and implemented the same pattern as in https://github.com/evcc-io/evcc/blob/master/charger/alfen.go (where instead of enabled, current is used).

Since I don't have a Keba Wallbox, it is currently untested. @Myky09, a test run would be appreciated. If you are able to compile and run evcc, you can do it any time with a binary created from the source branch. Otherwise, you can test a nightly version in case the evcc decides to merge this change.

[sourcery-ai]

Hey @mfuchs1984 - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Local enabled flag not initialized from hardware on startup

Because enabled defaults to false, the first heartbeat can disable an already-on charger. Initialize it in setup (e.g., via wb.Enabled()) to match the hardware state.

[mfuchs1984]

Guter Gedanke, aber ist das wirklich ein Problem beim Start von evcc? Wenn ja, dann vermutlich auch ein Problem bei der alfen, wo es entsprechend current betrifft.

[sourcery-ai]

Stimmt, ohne Initialisierung kann beim ersten Heartbeat aus Versehen ein „Disable“ auf einen bereits aktiven Wallbox-Ausgang geschickt werden. Das gleiche gilt übrigens für die Alfen-Implementierung: dort ist der lokale current-Wert 0 und der erste Heartbeat setzt dann ebenfalls 0 A.

Mein Vorschlag wäre daher, direkt in Setup() den Ist-Zustand vom Ladestrom bzw. von Enabled() auszulesen und das lokale Flag entsprechend zu setzen. Beispiel (vereinfacht):

func (wb *Keba) Setup() error {
    // … bestehende Initialisierung …
    state, err := wb.Enabled()
    if err != nil {
        return fmt.Errorf("keba: enabled initial read failed: %w", err)
    }
    wb.mu.Lock()
    wb.enabled = state
    wb.mu.Unlock()
    return nil
}

Und analog bei Alfen:

func (wb *Alfen) Setup() error {
    cur, err := wb.Current()
    if err != nil {
        return fmt.Errorf("alfen: current initial read failed: %w", err)
    }
    wb.mu.Lock()
    wb.current = cur
    wb.mu.Unlock()
    return nil
}

Damit verhindert man, dass der allererste Heartbeat einen falschen Schreibvorgang auslöst. Wenn das für dich ok ist, kann ich das in den PR aufnehmen.

[andig]

@mfuchs1984 das gleiche Problem ist mir auch beim refactoring des Heartbeats aufgefallen weshalb der PR noch offen ist. Da haben wir bei vielen Boxen ein kleines Logikproblem.

[mfuchs1984]

Bin echt überrascht über die Qualität des AI Reviews.

[mfuchs1984]

Hast du schon einen Plan für die Lösung? Vorher auslesen wie hier vorgeschlagen klingt erstmal plausibel.

[sourcery-ai]

Genau, ich würde das wie im Alfen-Beispiel direkt in Setup() beim Hochfahren auslesen und ins lokale Flag schreiben. Dann schieben spätere Heartbeats nichts Ungewolltes mehr. Grob sähe das für Keba so aus:

func (wb *Keba) Setup(log *util.Logger) error {
    // 1) bestehende Initialisierung (Connection, embed, etc.)
    if err := wb.embed.Setup(log); err != nil {
        return err
    }

    // 2) Ist-Zustand vom Enabled-Register auslesen
    state, err := wb.Enabled()
    if err != nil {
        return fmt.Errorf("keba: enabled initial read failed: %w", err)
    }

    // 3) ins lokale Flag schreiben
    wb.mu.Lock()
    wb.enabled = state
    wb.mu.Unlock()

    wb.log.Infof("keba: initial enabled state = %v", state)
    return nil
}

Analog können wir beim Alfen-Adapter wb.Current() in Setup() auslesen und wb.current setzen.

Damit ist sichergestellt, dass der allererste Heartbeat nicht versehentlich „0 A“ bzw. „Disable“ schreibt, nur weil unser lokaler Default anders ist. Wenn das so für dich passt, kann ich das direkt in den PR übernehmen.

[mfuchs1984]

Der sehr gute Test von @Myky09 hat noch eine kleine Lücke aufgedeckt, update kommt bald.

[mfuchs1984]

@andig das aufgedeckte Problem ist m.E. doch etwas größer. Und zwar wird

evcc/core/loadpoint.go

Line 1861 in 3721eff

// always disable charger if not connected

zumindest nicht in allen Fällen gemacht, wenn das Auto abgesteckt wird. Der Code in

evcc/core/loadpoint.go

Line 898 in 3721eff

if err := lp.charger.Enable(enabled); err != nil {

wird nicht ausgeführt, da die Bedingung enabled != lp.enable nach dem Aufruf von

evcc/core/loadpoint.go

Line 1844 in 3721eff

if err := lp.syncCharger(); err != nil {

nicht in allen Fällen mehr zutrifft (auf den ersten Blick hängt es davon ab, ob in den letzen 60 Sekunden der Ladestrom geändert wurde oder Phasen umgeschaltet wurden, wenn nicht, ist die Bedingung nicht mehr zutreffend).

Das bedeutet, dass // always disable charger if not connected nicht zutrifft und die disable routinen der charger nicht in allen Fällen aufgerufen werden. Bei allen chargern, die sich enabled merken (https://github.com/search?q=repo%3Aevcc-io%2Fevcc+%22wb.enabled+%3D+%22&type=code), wenn Enable aufgerufen wird, ist der gespeicherte Status nach dem Abstecken also falsch. Je nach wallbox bedeutet das auch, dass nach dem Anstecken losgeladen wird (was man hier auch immer wieder in den discussions sieht).