evcc-io / evcc

Sonne tanken ☀️🚘
https://evcc.io
MIT License
3.38k stars 617 forks source link

Easee: evcc configure fails #11328

Closed GrimmiMeloni closed 9 months ago

GrimmiMeloni commented 9 months ago

As reported in #8059, it seems we have some race between the op_mode update and the charger status when running evcc configure.

Hier die Fehlermeldung nach Configuration mit evcc --log trace configure

Teste die Konfiguration von Easee Home ...
[...]
[easee ] TRACE 2023/12/29 14:48:06 POST https://streams.easee.com/hubs/chargers/negotiate
[easee ] TRACE 2023/12/29 14:48:07 {"negotiateVersion":0,"connectionId":"_NpVXMtLPsUUCWqoNAv37g","availableTransports":[{"transport":"WebSockets","transferFormats":["Text","Binary"]},{"transport":"ServerSentEvents","transferFormats":["Text"]},{"transport":"LongPolling","transferFormats":["Text","Binary"]}]}
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-11-14 20:48:53 +0000 UTC) SELF_TEST_RESULT {"status":"OK","timestamp":1699994933,"backplateId":"8167F38A806A04"}
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-12-28 16:59:10 +0000 UTC) SMART_CHARGING false
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-12-29 10:54:22 +0000 UTC) CABLE_LOCKED false
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-12-29 13:39:27 +0000 UTC) CHARGER_OP_MODE 4
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-12-29 13:37:41 +0000 UTC) TOTAL_POWER 0
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-12-29 13:17:24 +0000 UTC) SESSION_ENERGY 24.86638641357422
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-12-29 13:00:00 +0000 UTC) ENERGY_PER_HOUR 10.437838554382324
[easee ] TRACE 2023/12/29 14:48:07 ProductUpdate EHP49EK3: (2023-12-29 13:00:47 +0000 UTC) WI_FI_RSSI -40
   Fehler: invalid opmode: 0

 Der Test von Easee Home ist fehlgeschlagen. Soll es trotzdem in die Konfiguration aufgenommen werden? (y/N)

Originally posted by @PVJan in https://github.com/evcc-io/evcc/discussions/8059#discussioncomment-7972299

GrimmiMeloni commented 9 months ago

Can reproduce locally.

Triggered here: https://github.com/evcc-io/evcc/blob/master/cmd/configure/devicetest.go#L90

which in turn calls https://github.com/evcc-io/evcc/blob/master/charger/easee.go#L432

Caused by async nature of product updates flowing in. Charger.Status() is called before the charger is actually ready. While the code does wait for the first product update before returning from the charger's factory method, this only confirms that connectivity via SignalR is established. It does however not guarantee a consistent state, yet.

Options: a) wait specifically for a first CHARGER_OP_MODE update, before considering the charger instance "ready". b) do not error out in Status() if c.op_mode is not set, yet. It is "undefined" - or technically it is the default which is easee.ModeOffline.

@andig do we have prior art for such situations? For example, do other charger implementations guarantee a consistent state before returning from their factories?

andig commented 9 months ago

@GrimmiMeloni we typically try to wait for valid state, e.g. in OCPP. Since the wait is guarded by timeout anyway it should be ok to wait for opmode instead of connection. So simply move the once.Close?

andig commented 9 months ago

Btw: not sure why the once.Close needs be put inside a defer while we're already holding the lock? I also don't think we need the c.updated for that purpose at all. One may wish to keep it for checking is SignalR is alive at all, but that's not implemented.

GrimmiMeloni commented 9 months ago

Thanks for the fix.

Regarding c.updated, it's OK to toss I think. My understanding of wrapping Once in that defer was this ensures even if some unexpected exit happens (e.g. future code change?) , we still signal the main thread. But given the timeout on the opposite end of the channel, I have to agree this looks like more code resiliency than really necessary.

GrimmiMeloni commented 9 months ago

Reopening, as we reverted the fix.

GrimmiMeloni commented 9 months ago

will be fixed again by #11435