Don't ratchet the reserve register while the battery is charging - #4754
Don't ratchet the reserve register while the battery is charging#4754chalfontchubby wants to merge 1 commit into
Conversation
The car-charging hold pins reserve just above the current SoC to stop the battery feeding the car. While the battery is actually charging that costs a register write for every 1% of the climb - a whole overnight charge's worth of writes, on every inverter, for a hold that cannot be doing anything: the battery is filling from the grid, so it is not discharging into the car. Skip the reserve write while charging is running. Reserve resets for the duration via the existing reset path, and is latched at the SoC reached on the first cycle after charging stops - which is the point the inverter returns to demand and the hold starts to mean something. adjust_discharge_rate(0) is left in place throughout as before, so discharge stays blocked meanwhile. The iBoost hold immediately below already sits out a charge for the same reason; this makes the car hold consistent with it. Reported in #3899, where a two-inverter GivEnergy system with an EMS (so register writes land in flash) saw 95 reserve writes across a single overnight charge, all from this path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| # and latched at the SoC reached once charging stops - which is the point the | ||
| # inverter returns to demand and the hold starts to mean something. The sibling | ||
| # iBoost hold below already sits out a charge for the same reason. | ||
| if self.set_reserve_enable and status != "Charging": |
There was a problem hiding this comment.
Should this be 'and not isCharging'?
There was a problem hiding this comment.
Checked both against the surrounding logic - status != "Charging" is deliberately narrower than not isCharging, and I think that's correct here.
status == "Charging" is only set in the sub-branch where SoC is actively below target and climbing from grid current (execute.py:365) - the exact mechanism behind the #3899 write storm. isCharging is broader: it's also True during "Freeze charging" and "Hold charging" (execute.py:381), where SoC is held flat rather than climbing, so the register-write-storm this PR targets doesn't occur there regardless of which check is used.
Using not isCharging would additionally skip the reserve write during those hold/freeze states. That's not a safety gap - the adjust_discharge_rate(0) write two lines up (execute.py:587-589) fires unconditionally on resetDischarge, independent of this gate, so discharge stays blocked either way in every sub-case I traced. But it would go beyond what the PR describes ("the battery is being filled from the grid, so it cannot be feeding the car" is only true for literal "Charging"), and it would diverge from the iBoost sibling block this PR cites as the pattern being matched (status not in ["Exporting", "Charging"] at execute.py:614, also status-based).
No existing test (including the new car_charge_no_reserve_ratchet, or the pre-existing car_charge2 Hold-charging + car-hold case) discriminates between the two, since car_charge2 uses a timed-pause inverter where this reserve branch doesn't execute at all. So there's no regression risk either way - it's a scope call, and I'd rather keep the gate matched to the narrow mechanism described in the PR body. Leaving as status != "Charging" unless you want the broader scope intentionally.
The problem
The car-charging hold (
car_charging_from_batteryoff) stops the battery feeding the car by pinning reserve 1% above the current SoC:adjust_reservewrites whenever the value differs, so while SoC is moving this writes on every 1% of change, every 5-minute cycle, for as long as it keeps moving.The catch is that SoC only climbs like that when the battery is charging - and while it is charging from the grid it cannot be discharging into the car, so the hold has nothing to do. The writes are generated precisely in the situation that cannot require them.
Reported in #3899: a two-inverter GivEnergy system, EMS installed so there is no RTC and register writes land in flash. Their log covers ~1.8 days and 115 register writes, of which 95 are reserve, essentially all from one overnight charge window that overlapped an Octopus Intelligent car slot:
discharge_ratewas written to 0 once at 00:01 and held there all night, so discharge was already blocked for the entire window by that alone.The fix
Skip the reserve write while charging is actually running.
resetReserveis left set, so the existing reset path puts reserve back to its baseline for the duration, and the hold is latched at the SoC reached on the first cycle after charging stops - the point the inverter returns to demand and the hold starts to mean something. Worst-case exposure is a single 5-minute cycle, withadjust_discharge_rate(0)still blocking discharge throughout.Deliberately narrow: only the reserve write is gated.
adjust_discharge_rate(0)still runs as belt-and-braces - it is written once and is idempotent, so it costs nothing, and it is likely the mechanism that actually does the job on inverters where reserve is not enough (the reserve line was added here in da02736, a Fox-focused commit).The
+1lead is untouched, so this changes only when the write happens, not what value goes in.Consistency
This makes the car hold match its immediate sibling - the iBoost hold 15 lines below already guards
status not in ["Exporting", "Charging"]and sits out a charge for the same reason. The car branch was the outlier.Tests
New
car_charge_no_reserve_ratchetscenario: actively charging, car slot active, no timed pause. Verified it fails without the fix (ERROR: Inverter 0 Reserve should be 0 got 51) and passes with it. The existingcar2scenario still covers reserve being latched at SoC+1 when not charging../run_all --quickand pre-commit pass.Note on the original report
#3899 is closed - the reporter worked around it by raising
calculate_plan_everyto 30 minutes and adjusting thresholds. Worth flagging thatcalculate_plan_everyonly throttles replanning;RUN_EVERYis a hard-coded 5 andexecute_plan()still runs every cycle, so their improvement likely came from the threshold changes producing fewer windows rather than from the interval itself. This patch addresses the underlying write pattern, which predates their reported 8.35.1 -> 8.37.10 regression window and so may not be the whole story of that particular before/after.🤖 Generated with Claude Code