Skip to content

Loadpoint: correct phase sync when measured phases fall below commanded#31497

Draft
andig wants to merge 1 commit into
masterfrom
fix/phase-sync-measured-below-commanded
Draft

Loadpoint: correct phase sync when measured phases fall below commanded#31497
andig wants to merge 1 commit into
masterfrom
fix/phase-sync-measured-below-commanded

Conversation

@andig

@andig andig commented Jul 5, 2026

Copy link
Copy Markdown
Member

fixes #31491

The measured-phase fallback in syncCharger only corrected the mismatch in one direction: when the charger reports more active phases than expected. It never handled the opposite case, so when a 1p→3p switch is commanded but never physically takes effect (relay/vehicle doesn't actually engage the third phase), measuredPhases sticks at the lower value. Since maxActivePhases() is capped by measuredPhases, this permanently blocks any further scale-up attempt for the rest of the session, matching the reported symptom of 1p/3p switching "getting stuck" until a restart.

  • syncCharger now corrects lp.phases whenever the measured phase count differs from the commanded one (not just when it's higher), restoring the self-healing loop that lets pvScalePhases retry 3p later
  • Updated TestSyncChargerPhasesByMeasurement to assert the corrected behavior

🤖 Generated with Claude Code

The measured-phases fallback in syncCharger only corrected the case where
the charger reports more active phases than expected. When a 1p to 3p
switch silently fails to take physical effect, measuredPhases sticks at
the lower value, which caps maxActivePhases forever and blocks any future
scale-up attempt until restart.
@andig andig added the bug Something isn't working label Jul 5, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The warning message "charger logic error: phases mismatch" is now triggered for both over- and under-reported phases; consider rewording it or adding more context so it accurately reflects both directions of mismatch.
  • The new condition chargerPhases > 0 && chargerPhases != phases intentionally ignores a zero-measured-phases case; if a 3p→0p failure is possible in practice, it might be worth clarifying or revisiting why 0 is excluded here.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The warning message "charger logic error: phases mismatch" is now triggered for both over- and under-reported phases; consider rewording it or adding more context so it accurately reflects both directions of mismatch.
- The new condition `chargerPhases > 0 && chargerPhases != phases` intentionally ignores a zero-measured-phases case; if a 3p→0p failure is possible in practice, it might be worth clarifying or revisiting why 0 is excluded here.

## Individual Comments

### Comment 1
<location path="core/loadpoint_sync_test.go" line_range="188-189" />
<code_context>
 		{1, 1, 1},
 		{1, 3, 3},
 		{3, 0, 3},
-		{3, 1, 3}, // ignore
+		{3, 1, 1}, // force
 		{3, 3, 3},
 	}
</code_context>
<issue_to_address>
**suggestion (testing):** Clarify and extend tests around zero measured phases vs non-zero mismatches

With `chargerPhases > 0 && chargerPhases != phases`, `{3, 0, 3}` now shows that a 0 measurement is ignored, while `{3, 1, 1}` covers non-zero mismatches that trigger correction. To make this clearer and ensure both directions are tested, consider (a) updating the `{3, 0, 3}` comment to `// ignore 0 measurement`, and (b) adding `{1, 3, 3}` with `// force up`, or a dedicated test, to document the symmetric low→high behavior if it’s handled by the same logic.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread core/loadpoint_sync_test.go
@andig andig added the waiting for feedback Suspended progress label Jul 5, 2026
@andig andig marked this pull request as draft July 8, 2026 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working waiting for feedback Suspended progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

automatic 1p/3p switch stops working after a while (openWB)

1 participant