Skip to content

fix(config): a battery soc_max of 0 no longer blocks charging - #1964

Merged
frahlg merged 2 commits into
masterfrom
fix/battery-soc-max-zero
Nov 5, 2026
Merged

frahlg merged 2 commits into
masterfrom
fix/battery-soc-max-zero

Conversation

@frahlg

@frahlg frahlg commented Nov 4, 2026

Copy link
Copy Markdown
Member

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:

  1. Until 76f811b (7 Sep 2026), Settings saved every number field on the open tab, and a blank field became 0. So a Settings save on the Batteries tab could store 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.
  2. Before fix(control): ship SOC ceilings and Charge restart recovery #1870, a battery's soc_max did almost nothing: the planner reads planner.soc_max, and the Pixii driver does not read charge_ceil_soc. The stored 0 did no harm.
  3. fix(control): ship SOC ceilings and Charge restart recovery #1870 (first in v0.157.1-beta.1) made a battery's soc_max the 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.Validate treats a battery soc_max of 0 or less as unset and logs a warning. File load, the config database and POST /api/config all pass through it, so dispatch and WithBatterySoCBounds never see the 0, and the next save drops it. A battery that must not charge sets max_charge_w: 0.
  • Settings saves a cleared number field as null, not 0. The API decodes into a fresh Config, so null equals 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").
  • driverLimitsFrom is unchanged: a 0 that reaches it still blocks charging, which is the safe side.

Evidence

  • A throwaway test on master (not committed), battery at 58 %, manual 3 kW charge: soc_max 0 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 the Validate call. TestDropZeroBatteryCeilings covers 0, negative, a set value, and that soc_min: 0 and 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 }.
  • Browser, local Core with the Ferroamp and Sungrow simulators and batteries.sungrow-sim set to four zeros: the log warns once, Settings → Batteries shows Max SoC blank, clearing the other three and saving leaves batteries.sungrow-sim empty in /api/config, also after a restart, with no warning.
  • make verify (vet, tests, build) and npm test (901 of 901) pass on this machine.

Review

An independent review (Sonnet) checked:

  • that every way a config enters Core passes through Validate before cfg.Batteries is 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;
  • every Settings number field for null against 0. No config type has a custom UnmarshalJSON. Plain fields decode null to 0 as before. fuse.safety_margin_a treats nil and 0 alike. The one driver-config number, write.max_w, reads nil as 0, and the one unit-scaled field skips scaling;
  • that no real config needs soc_max: 0 as "never charge", because max_charge_w: 0 already does that.

It found no blocking issue. Changes made from it (10add40):

  • Validate runs 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.
  • A test loads four zeros stored in the config database, the path real boxes use. It fails without the Validate call.
  • The dispatch test uses the four-zero record real boxes hold, and two comments now say what is true.

Left as they are:

  • A one-sided max_charge_w: 0 still blocks charging. That is a real choice, and Settings no longer writes it by accident.
  • The warning goes only to the log.
  • After you clear a field and switch tabs, Settings shows the field's default rather than 0, as it already does after a reload.

🤖 Generated with Claude Code

frahlg and others added 2 commits November 4, 2026 14:47
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>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T14:03:29.218484Z 10add40 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@frahlg
frahlg merged commit fc4895a into master Nov 5, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant