Repository navigation
fix(config): a battery soc_max of 0 no longer blocks charging - #1964
Merged
Merged
Conversation
Settings saved a blank number field as 0. Since soc_max became the dispatch charge ceiling, a battery whose Max SoC was saved that way never charged, manual Charge included. Core treats soc_max <= 0 as unset, and Settings saves a cleared number field as unset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Validate runs on shallow copies of the live config, so the zero drop clones the batteries map before it writes. A new test loads a zero that an older Core stored in the config database. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A site with a Pixii battery updated to 0.158.0, and the battery stopped charging, manual Charge included. The plan showed charging in the next slot, but the box sent solar to the grid. The owner could not clear Max SoC in Settings; typing Min 0.1 and Max 0.9 made it charge again.
The cause has three parts:
soc_min: 0, soc_max: 0, max_charge_w: 0, max_discharge_w: 0. Core already ignores the two power limits when both are 0 (batteryPowerLimits), for this reason. It had no such guard for SoC.soc_maxdid almost nothing: the planner readsplanner.soc_max, and the Pixii driver does not readcharge_ceil_soc. The stored 0 did no harm.soc_maxthe dispatch charge ceiling, and counts a set 0 as a limit. At 0 the battery is "full" at any SoC, every charge order is set to 0 W, and charging resumes at 0 − 1 %, so never. Discharge still works. The planner keeps its own ceiling, so it plans charging that dispatch then stops.Clearing the field did not help: a field that shows 0 and is then cleared differs from its rendered value, so Settings saved
parseFloat("")→ NaN → 0 again.Change
config.Validatetreats a batterysoc_maxof 0 or less as unset and logs a warning. File load, the config database andPOST /api/configall pass through it, so dispatch andWithBatterySoCBoundsnever see the 0, and the next save drops it. A battery that must not charge setsmax_charge_w: 0.null, not 0. The API decodes into a freshConfig, sonullequals 0 for plain number fields; for optional limits (*float64) it now means unset, as the Batteries tab text says ("Leave blank to use BMS defaults").driverLimitsFromis unchanged: a 0 that reaches it still blocks charging, which is the safe side.Evidence
soc_max0 gives 0 W, paused, resume at 0.00; unset or 0.9 gives 3000 W.TestSavedZeroSoCMaxDoesNotBlockCharge(go/cmd/ftw) loads YAML with the four zeros and charges at 58 % under the planner's 90 %; it fails without theValidatecall.TestDropZeroBatteryCeilingscovers 0, negative, a set value, and thatsoc_min: 0and power limits stay.saves a cleared number as unset, not 0(web/settings-shell.test.mjs) fails on master with{ soc_max: 0, max_charge_w: 0 }.batteries.sungrow-simset to four zeros: the log warns once, Settings → Batteries shows Max SoC blank, clearing the other three and saving leavesbatteries.sungrow-simempty in/api/config, also after a restart, with no warning.make verify(vet, tests, build) andnpm test(901 of 901) pass on this machine.Review
An independent review (Sonnet) checked:
Validatebeforecfg.Batteriesis read: file load, the config database (decodeStored),POST /api/config, handlers that save a copy of the live config, setup drafts and the bootstrap wizard;nullagainst 0. No config type has a customUnmarshalJSON. Plain fields decodenullto 0 as before.fuse.safety_margin_atreats nil and 0 alike. The one driver-config number,write.max_w, reads nil as 0, and the one unit-scaled field skips scaling;soc_max: 0as "never charge", becausemax_charge_w: 0already does that.It found no blocking issue. Changes made from it (10add40):
Validateruns on shallow copies of the live config in several handlers, so the drop clones the batteries map before it writes. The new test fails without the clone.Validatecall.Left as they are:
max_charge_w: 0still blocks charging. That is a real choice, and Settings no longer writes it by accident.🤖 Generated with Claude Code