Skip to content

fix(solis): derive nominal_voltage from live battery data instead of hard-coded 48V - #4502

Merged
springfall2008 merged 2 commits into
mainfrom
fix/solis-nominal-voltage
Aug 16, 2026
Merged

fix(solis): derive nominal_voltage from live battery data instead of hard-coded 48V#4502
springfall2008 merged 2 commits into
mainfrom
fix/solis-nominal-voltage

Conversation

@chalfontchubby

Copy link
Copy Markdown
Collaborator

Summary

Fixes #4493 - SolisAPI.nominal_voltage was hard-coded to 48.0 and never updated from the actual battery, so every derived power and capacity value was wrong by roughly the ratio of the real pack voltage to 48V (~11x on the reporter's 533V HV battery: 960W computed vs ~10.7kW actual). Reported and confirmed live under load by @jibbej, who also identified a second defect while testing: the capacity calculation needs a genuinely different voltage (nominal pack, not live measured) plus parallel_battery_count, which the max-current calculation already applies but capacity didn't.

  • get_nominal_voltage(inverter_sn): power/current conversions (max charge/discharge power sensors, amp↔watt round trips for writing to the inverter) now use the live measured batteryVoltage, retaining the last known-good reading if it becomes momentarily unavailable rather than reverting to 48V.
  • get_capacity_voltage(inverter_sn) + new solis_nominal_voltage apps.yaml option: the capacity calculation prefers the nominal pack voltage (a fixed physical property, e.g. cell count × nominal cell voltage) when configured.
  • battery_capacity now applies parallel_battery_count, matching the existing max-current calculation (_calculate_max_currents()) which already did.
  • The commented-out soc_max auto-binding to battery_capacity stays deliberately disabled - without solis_nominal_voltage configured the sensor can still wobble with charge state, which soc_max never did before, even with the old wrong constant.

A decision worth reviewer pushback

DEYE already solved the exact same "nominal vs measured voltage" problem for its own capacity calculation (deye.py nominal_pack_voltage()/derive_battery_capacity()) and its answer is to refuse to publish a capacity at all rather than guess, on the basis that a wrong soc_max source is worse than none.

I initially matched that for Solis, but changed it after discussion: without solis_nominal_voltage configured, battery_capacity is still published, falling back to the live measured voltage, flagged unreliable via reliable/voltage_source attributes and a one-time warning log (not repeated every cycle - it runs in publish_entities(), roughly once a minute). The reasoning: every existing Solis Cloud install already has this sensor published today (using the old, worse, hard-coded 48V), and nobody has solis_nominal_voltage set yet since it's brand new - dropping the sensor outright the moment this ships would regress 100% of existing installs with no migration path, for a sensor that isn't wired into soc_max internally anyway (that binding is, and stays, commented out). Cloud integration users skew less technical, which weighed into being cautious here rather than optimising for architectural purity.

This is a genuine tradeoff, not an obviously-correct call - happy to switch to DEYE's stricter behaviour (or something in between) if reviewers feel differently. See get_capacity_voltage()'s docstring in solis.py for the same note in-code.

Test plan

  • New tests in test_solis.py: test_get_nominal_voltage_and_capacity_voltage (live voltage used and remembered; falls back to last known-good, then 48V), test_publish_entities_capacity_voltage_reliability (capacity always published, correctly flagged reliable/unreliable, warning logs exactly once not every cycle, parallel_battery_count applied)
  • Existing test_publish_entities power/capacity assertions updated to reflect live-voltage-based calculations
  • ./run_all --quick passes
  • ./run_pre_commit passes

🤖 Generated with Claude Code

…hard-coded 48V (#4493)

SolisAPI.nominal_voltage was hard-coded to 48.0 and never updated from the
actual battery, making every derived power and capacity value wrong by
roughly the ratio of the real pack voltage to 48V (~11x on a 533V HV
battery). Reported and confirmed live under load by @jibbej, who also
found the capacity path needs a *different* voltage (nominal pack, not
live measured) plus parallel_battery_count, which the max current
calculation already applies but capacity didn't.

- get_nominal_voltage(): power/current conversions (max charge/discharge
  power, amp<->watt round trips) now use the live measured batteryVoltage,
  retaining the last known-good reading if it becomes unavailable rather
  than reverting to 48V.
- get_capacity_voltage() + new solis_nominal_voltage apps.yaml option: the
  capacity calculation uses the nominal pack voltage (a fixed physical
  property) when configured. Without it, capacity still falls back to the
  live voltage - existing installs already had this sensor published and
  dropping it outright had no migration path - but is flagged unreliable
  (reliable/voltage_source attributes, a one-time warning) since that
  value drifts with charge state.
- battery_capacity now applies parallel_battery_count, matching the
  existing max current calculation.
- soc_max auto-binding to battery_capacity stays deliberately disabled:
  without solis_nominal_voltage configured the sensor can still wobble
  with charge state, which soc_max never did before.
@jibbej

jibbej commented Aug 13, 2026

Copy link
Copy Markdown

I have read through the diff. Splitting live measured voltage (for amp/watt
conversion) from a configured nominal pack voltage (for capacity) is exactly the
right distinction - on my system those genuinely differ. The pack sits between
roughly 517 V and 539 V depending on SoC, while nominal is 512 V, so no single
constant could have served both paths correctly.

Applying parallel_battery_count to the capacity calculation also closes the
remaining gap I reported. On my system:

Dyness Stack100, 51.2 kWh, 10 modules
Module:  51.2 V / 100 Ah  (5.12 kWh)
Nominal pack voltage:  512 V
Measured pack voltage today:  517-539 V depending on SoC
CID 172 (battery capacity) reads:  50 Ah

Before:  50 x 533 / 1000              = 26.65 kWh   (what I see today)
After:   50 x 2 x 512 / 1000          = 51.2 kWh    (actual battery size)

I can confirm parallel_battery_count is 2 here rather than inferring it. The
charge_slot1_power entity exposes a max of 21320 W, and since that bound is
derived from max_charge_current * nominal_voltage where max_charge_current is
the parallel-multiplied value (solis.py, max_charge = per_battery_max * battery_count), that works back to 40 A against a raw CID reading of 20 A - so a
count of 2.

One observation that may be useful for the fallback path you added: on my
inverter there is no user-settable capacity anywhere. The Battery1 Setting page
exposes exactly eight parameters (battery model, max charge current, max
discharge current, over-discharge SOC, recovery SOC, forced charge SOC, battery
saving, max charge SOC) and none of them is a capacity or Ah field. So CID 172
appears to come from the BMS over the battery-to-inverter link rather than from
configuration, and users will not be able to correct it locally - which makes the
solis_nominal_voltage option the only lever. Worth keeping the one-time warning
prominent for anyone who has not set it.

Incidentally, that same page confirms the ordering constraint behind the other
issue I reported (#4493): Over-discharge has range 540% and Recovery has
range 21
40%, i.e. recovery must sit at least one point above over-discharge.
That is consistent with the inverter refusing a discharge cutoff SOC equal to the
over-discharge floor, which is what caused my CID 5965 writes to be rejected.

I am currently running with nominal_voltage manually patched to 533.0 as a
workaround. Once this is merged I will set solis_nominal_voltage: 512, drop the
patch, and report back what the capacity and max charge/discharge power entities
read.

@chalfontchubby

Copy link
Copy Markdown
Collaborator Author

Thanks for confirming with real numbers - 50Ah × 2 parallel × 512V / 1000 = 51.2kWh matching your actual battery size is exactly the validation this needed.

Good to know CID 172 comes from the BMS with no local override available on your Battery1 Setting page - that confirms solis_nominal_voltage really is the only lever for anyone in the same position, not just a convenience. The one-time warning stays in place either way (it only fires while the value is unconfigured), so that's covered.

Look forward to the post-merge report once you've dropped the manual patch and set solis_nominal_voltage: 512.

@chalfontchubby

Copy link
Copy Markdown
Collaborator Author

@jibbej
"Incidentally, that same page confirms the ordering constraint behind the other
issue I reported (#4493): Over-discharge has range 540% and Recovery has
range 2140%, i.e. recovery must sit at least one point above over-discharge.
That is consistent with the inverter refusing a discharge cutoff SOC equal to the
over-discharge floor, which is what caused my CID 5965 writes to be rejected."

I'm not sure we captured the implications of that - could you check it is covered in it's own issue if there is outstanding work please.

@jibbej

jibbej commented Aug 13, 2026

Copy link
Copy Markdown

@jibbej "Incidentally, that same page confirms the ordering constraint behind the other issue I reported (#4493): Over-discharge has range 540% and Recovery has range 2140%, i.e. recovery must sit at least one point above over-discharge. That is consistent with the inverter refusing a discharge cutoff SOC equal to the over-discharge floor, which is what caused my CID 5965 writes to be rejected."

I'm not sure we captured the implications of that - could you check it is covered in it's own issue if there is outstanding work please.

Checked - it is not covered anywhere, and my cross-reference was wrong. Apologies: I
wrote "#4493" but that issue is the nominal_voltage one. The over-discharge ordering
has no issue of its own, and there is outstanding work.

It is also not specific to my system. #4184 already contains an independent report of
the same failure on completely different hardware (Solis S6 8kW, Fogstar LV batteries):

"the discharge SOC registry was failing to write as it was trying to write 9, but the
inverter would only accept values between 10-100 ... 'Failed to verify CID 5965
discharge slot 1 SOC ... wrote 9 but read back 25'. I changed
input_number.predbat_set_reserve_min in the HA entries to 10 to fix this."

Same symptom, same manual workaround, different limit. Theirs rejects below 10. Mine
rejected exactly 20, which I confirmed by hand rather than inferring: writing 20 read back
50 (i.e. unchanged), writing 30 read back 30, and it currently sits at 22 and holds. 20 is
also my over-discharge floor, so on this inverter the cut-off has to clear that floor.

I should correct something in my earlier note, though. I wrote that the Battery1 Setting
ranges (Over-discharge 540%, Recovery 2140%) show recovery must sit a point above
over-discharge. That does not actually follow - those ranges only say Recovery's own floor
is 21. The write-test above is the real evidence, not the ranges.

So the general statement is not "recovery must exceed over-discharge". It is that CID
5965 has an accepted range that Predbat neither reads nor clamps to
, and that range
differs per installation - a fixed floor on jakeymd1's, a floor tied to the over-discharge
setting on mine.

The path, on v8.47.7:

  • solis.py:1283-1284 binds both reserve and battery_min_soc to the
    over_discharge_soc entity (CID 158)
  • config.py gives SolisCloud has_reserve_soc: False, so inverter.py:481-484
    takes the not self.inv_has_reserve_soc branch and sets
    reserve_percent = reserve_min unconditionally
  • inverter.py:475-478 raises set_reserve_min up to battery_min_soc if it is lower,
    i.e. up to the over-discharge floor itself
  • inverter.py:2500 then writes target_soc = int(self.reserve_percent) into CID 5965

So the default path lands exactly on the boundary value the inverter refuses, and there
is nothing in solis.py that constrains the written value to what the register accepts.
Both of us fixed it by hand-tuning set_reserve_min, which is a user-side workaround for
a missing clamp.

I checked whether the API could supply the range itself, since
write_time_windows_if_changed already uses sysCommand.max for the current registers
(solis.py:684-688). It cannot: reading the metadata for these registers gives the
placeholder rather than a real range.

CID 5965 SOC1              value=22  dspMin=0 dspMax=65535 hmiMin=0 hmiMax=65535
CID 158  Overdischarge SOC value=20  dspMin=0 dspMax=65535 hmiMin=0 hmiMax=65535
CID 7229 Recovery          value=21  dspMin=0 dspMax=65535 hmiMin=0 hmiMax=65535

So the limits are enforced in inverter firmware and only visible in the local UI. That
rules out a data-driven clamp for the SOC registers and leaves two options:

  1. Derive the floor from registers Predbat already caches - both CID 158 and CID 7229 are
    already in SOLIS_CID_INFREQUENT - and clamp the CID 5965 write to at least
    over_discharge_soc + 1. No extra polling needed. That covers my case exactly. It does
    not cover jakeymd1's absolute 10% floor, so an additional configurable minimum may
    still be wanted.
  2. If a write is still rejected, it currently retries every cycle for the whole export
    window with no state change. Backing off after a confirmed rejection would at least
    stop the log flood, whatever clamp is chosen.

Separately, while reading that area I think startup_reset_registers is dead code:

value = await self.read_cid(device_sn, SOLIS_CID_BATTERY_OVER_DISCHARGE_SOC)
try:
    value = float(value)
except (ValueError, TypeError):
    value = 0
if value > 20:

read_cid returns (value, info), so float() gets a tuple, raises TypeError, and
value is always 0 - the > 20 branch can never run. It is the only call site in
solis.py that does not unpack the tuple (1106, 1110 and 1314 all do).

Worth deciding what it should do before fixing it, rather than just correcting the
unpacking: as written it would stamp over_discharge_soc back to 20 on every startup,
which would override users who deliberately raised it and would fight the very workaround
the range problem above requires. Right now the bug is the only reason that does not
happen.

Happy to open a single issue covering the range/clamp problem with both reports, and a
separate one for the dead reset, if that split suits you.

Copilot AI 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.

Pull request overview

This PR fixes Solis Cloud battery power/capacity calculations by removing the hard-coded 48V assumption and deriving conversion voltages from live inverter data, while introducing an optional configured nominal pack voltage for stable capacity reporting.

Changes:

  • Add get_nominal_voltage() (live measured, with last-known fallback) for all amp↔watt conversions and number write-backs.
  • Add solis_nominal_voltage (nominal pack voltage) for capacity calculation; apply parallel_battery_count to capacity.
  • Update docs/templates and extend Solis tests to cover voltage source selection, reliability flags, and one-time warning behaviour.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
templates/solis_cloud.yaml Documents the new solis_nominal_voltage option and clarifies soc_max guidance.
docs/components.md Adds the Solis nominal_voltage option documentation (mapped to solis_nominal_voltage).
docs/apps-yaml.md Documents solis_nominal_voltage behaviour and the unreliable fallback path.
apps/predbat/tests/test_solis.py Adds new tests for voltage selection/memoization and capacity reliability/warn-once; updates existing assertions for live-voltage conversions.
apps/predbat/solis.py Implements live-voltage conversions, capacity-voltage selection, reliability metadata, and applies parallel_battery_count to capacity.
apps/predbat/config.py Adds solis_nominal_voltage to config schema.
apps/predbat/components.py Wires solis_nominal_voltage into Solis component initialization.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/predbat/solis.py Outdated
Comment thread apps/predbat/solis.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@springfall2008
springfall2008 merged commit 9ad008e into main Aug 16, 2026
2 checks passed
@springfall2008
springfall2008 deleted the fix/solis-nominal-voltage branch August 16, 2026 18:34
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.

[Solis Cloud] Hard-coded nominal_voltage of 48V breaks power/capacity calculations for HV batteries

4 participants