Don't wait for PV timer when battery boost ends without surplus

[blampe]

If we start battery boosting with PV surplus, and the boost finishes when no more surplus remains, then we end up waiting for the full pvTimer duration before we stop charging. This ends up sipping a little extra from the battery, and it can give the impression that the feature isn't working correctly because the battery continues discharging despite hitting the cutoff.

Instead, if boosting ends without a PV surplus we can stop charging immediately by pre-expiring pvTimer. This only triggers an immediate stop when boost ends in a no-surplus situation.

If boosting ends with PV surplus still available, targetCurrent >= minCurrent so the disable sequence is never entered and the pre-expired pvTimer has no effect.

A failing unit test is included to confirm the behavior.

NB: I can understand if this isn't accepted, since it does introduce an (unlikely) edge case where boosting could expire when the surplus is borderline, which could lead to some flapping. I'm also very excited for the optimizer work since this battery boosting is very valuable but also very suboptimal due to all the manual thresholds!

[sourcery-ai]

Hey - I've left some high level feedback:

• Mutating lp.pvTimer inside the targetCurrent < minCurrent branch of pvMaxCurrent introduces a side effect in what is otherwise mostly a calculation/helper; consider tightening the condition (e.g. only when batteryBuffered is true, not batteryStart) or moving the timer manipulation to a place where side effects are expected to avoid surprising future maintainers.
• The new behavior relies on the sentinel elapsed value and the mock clock interaction in a fairly implicit way; a brief explanation near the lp.pvTimer = elapsed assignment (or a small helper) clarifying that this is a deliberate pre-expiration mechanism would make the intent and coupling to the timer logic clearer.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Mutating `lp.pvTimer` inside the `targetCurrent < minCurrent` branch of `pvMaxCurrent` introduces a side effect in what is otherwise mostly a calculation/helper; consider tightening the condition (e.g. only when `batteryBuffered` is true, not `batteryStart`) or moving the timer manipulation to a place where side effects are expected to avoid surprising future maintainers.
- The new behavior relies on the sentinel `elapsed` value and the mock clock interaction in a fairly implicit way; a brief explanation near the `lp.pvTimer = elapsed` assignment (or a small helper) clarifying that this is a deliberate pre-expiration mechanism would make the intent and coupling to the timer logic clearer.

---

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.}