E3DC: fix status detection and auto-disable phase switching #26378
Conversation
- Fix status detection: Bit 2 varies between wallbox models and is now ignored. Only Bit 5 (charging) and Bit 3 (connected) are used. - Auto-disable Sun Mode and Auto Phase Switching at startup - Set initial phase to 1 for evcc control - Update header comments for clarity
evcc-bot
added
devices
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
ensureAutoPhaseDisabled, you readautoPhasefromwbData[1]whilecheckConfigurationreads the same flag fromwbData[2]; aligning these indices or explaining the difference would help avoid a subtle status-mapping bug. ensureAutoPhaseDisabledreturns silently on RSCP send/container errors, which could hide connectivity or protocol issues; consider at least logging these failures at DEBUG/INFO level for easier diagnosis.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ensureAutoPhaseDisabled`, you read `autoPhase` from `wbData[1]` while `checkConfiguration` reads the same flag from `wbData[2]`; aligning these indices or explaining the difference would help avoid a subtle status-mapping bug.
- `ensureAutoPhaseDisabled` returns silently on RSCP send/container errors, which could hide connectivity or protocol issues; consider at least logging these failures at DEBUG/INFO level for easier diagnosis.
## Individual Comments
### Comment 1
<location> `charger/e3dc.go:229-212` </location>
<code_context>
+// ensureAutoPhaseDisabled checks if auto phase switching is active and disables it.
+// Called before phase switch commands because the user could re-enable it
+// in the E3DC portal at any time without restarting evcc.
+func (wb *E3dc) ensureAutoPhaseDisabled() {
+ res, err := wb.conn.Send(*rscp.NewMessage(rscp.WB_REQ_DATA, []rscp.Message{
+ *rscp.NewMessage(rscp.WB_INDEX, wb.id),
+ *rscp.NewMessage(rscp.WB_REQ_AUTO_PHASE_SWITCH_ENABLED, nil),
+ }))
+ if err != nil {
+ return
+ }
+
+ wbData, err := rscpContainer(*res, 2)
+ if err != nil {
+ return
+ }
+
+ if autoPhase, err := rscpBool(wbData[1]); err == nil && autoPhase {
+ wb.log.WARN.Println("auto phase switch was re-enabled - disabling for evcc control")
+ wb.disableAutoPhaseSwitch()
+ }
+}
+
// getExternDataAlg retrieves the WB_EXTERN_DATA_ALG status byte array.
</code_context>
<issue_to_address>
**issue (bug_risk):** Swallowing errors in ensureAutoPhaseDisabled means phase switching continues even if checking/disabling auto-phase fails.
Previously, Phases1p3p failed fast if the auto-phase flag couldn’t be read or was still enabled. Now ensureAutoPhaseDisabled swallows all errors, so Phases1p3p always sends WB_REQ_SET_NUMBER_PHASES, even when the wallbox may still be in auto mode. This can lead to confusing manual-phase behavior. Please propagate an error (or a success flag) so Phases1p3p can abort and surface a clear error when the auto-phase state can’t be verified or changed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| wb.log.WARN.Println("sun mode was re-enabled - disabling for evcc control") | ||
| wb.disableSunMode() | ||
| } | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): Swallowing errors in ensureAutoPhaseDisabled means phase switching continues even if checking/disabling auto-phase fails.
Previously, Phases1p3p failed fast if the auto-phase flag couldn’t be read or was still enabled. Now ensureAutoPhaseDisabled swallows all errors, so Phases1p3p always sends WB_REQ_SET_NUMBER_PHASES, even when the wallbox may still be in auto mode. This can lead to confusing manual-phase behavior. Please propagate an error (or a success flag) so Phases1p3p can abort and surface a clear error when the auto-phase state can’t be verified or changed.
That doesn‘t sound like a good idea. It will break fast charging during restarts. |
Okay, good points, I hadn't thought of that. |
Removes the code that set 1 phase on startup, which would interrupt fast charging (3p) during evcc restarts. evcc will control phase switching based on charging mode (PV, Min+PV, Fast, etc.). Also adds debug logging to ensure* functions for better diagnostics.
| *rscp.NewMessage(rscp.WB_REQ_AUTO_PHASE_SWITCH_ENABLED, nil), | ||
| })) | ||
| if err != nil { | ||
| wb.log.DEBUG.Printf("failed to check auto phase switch: %v", err) |
There was a problem hiding this comment.
Wenns relevant ist sollte das eine Warnung sein?
|
|
||
| wbData, err := rscpContainer(*res, 2) | ||
| if err != nil { | ||
| wb.log.DEBUG.Printf("failed to parse auto phase switch response: %v", err) |
There was a problem hiding this comment.
Falls nein einfach raus damit. Ist im trace log ja ohnehin erkennbar.
| case b[2]&0b00001000 != 0: // Bit 3: vehicle connected → StatusB | ||
| return api.StatusB, nil | ||
| case b[2]&0b00000100 != 0: // Bit 2: ready, no vehicle → StatusA | ||
| case b[2]&0b00101000 == 0: // Neither Bit 5 nor Bit 3: no vehicle → StatusA |
There was a problem hiding this comment.
Entweder ist das jetzt der Default Case oder der kann weg- Deine Bedingung hat keine 4. Alternative
3 participants
Summary
Background
Status detection failed on some E3DC wallboxes because Bit 2 behavior differs between models. Tested with Multi Connect II, should also work with Multi Connect I (needs hardware testing).