Repository navigation
fix(solax): hold a lifetime counter that dips rather than read the recovery as energy - #5389
Merged
Merged
Conversation
…covery as energy SolaX can return a lower value for a plant lifetime total for a while and then the right one again. Published as received, the recovery was counted as energy for that day: 181.3 kWh of PV on a 3.2 kWp array. The five plant totals and each inverter's total yield are now held at the last published value while they read lower, seeded from the sensor so a dip straight after a restart is held too. A lower value that lasts 24 hours is taken as a genuine reset. The load is built from the held values. A plant with no inverter that has PV inputs or a yield counter now logs a warning once, as its PV figure is the inverter AC output. Part of #5388. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Reset timing is lost across restarts, and missing or zero-valued counters can be misclassified.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Prevents transient SolaX lifetime-counter dips from inflating energy history and adds diagnostics for plants without a reliable PV source.
Changes:
- Holds dipped plant and inverter counters for 24 hours.
- Calculates load from held values and warns about missing PV sources.
- Adds regression tests and debugging guidance.
| File | Description |
|---|---|
apps/predbat/solax.py |
Implements counter-dip handling and PV-source warnings. |
apps/predbat/tests/test_solax.py |
Tests dip, recovery, reset, restart, and warning behavior. |
tools/debug-journal.md |
Documents the SolaX failure mode and mitigation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… total A plant whose only inverter has no PV inputs and no yield counter (a lone AC-coupled battery inverter) fell back to the plant total yield for PV. That figure is the inverter AC output, so battery discharge was published as PV and counted twice in the load. Such a plant now publishes 0 for the PV yield. The load has no PV term, so on a site with PV on an inverter SolaX does not measure it is low by what that PV supplies. A plant whose inverters have PV inputs but no yield of their own still uses the plant total. Part of #5388. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ield counter as a PV source Review feedback on #5389. The time a dip started was held in memory only, so a restart began the 24 hour hold again and a system restarted every day would never accept a genuine counter reset. The dip start times and the last counter values are now saved through the storage component: at once when a dip starts or ends, otherwise every 15 minutes. The stored last value is used when the sensor is gone, as after a Home Assistant restart. plant_has_pv_source() ignored totalYield, so an inverter whose yield counter reads 0.0 and which has no PV maps, such as a newly commissioned one, was classed as having no PV source. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
springfall2008
added a commit
that referenced
this pull request
Oct 5, 2026
…octopus row (GH#5390) (#5397) * docs(debug-journal): fold in the 2026-10-04 queue slice (9 candidates); mark GH#5376 fixed by PR #5377 (v9.3.5) and GH#5366 by PR #5383; re-verify the SolaX/counter-dip entries after PR #5389; add the CID499 read/write units, marginal-band contamination, GivTCP verify-tolerance, minute-data dip asymmetry, Fox MaxSoc, car-export-blind, re:-first-match and tooltip-clamp entries Co-Authored-By: Claude Code <noreply@anthropic.com> * docs(debug-journal): map the IOG supervision bounce symptom into the octopus row (GH#5390) Co-Authored-By: Claude Code <noreply@anthropic.com> --------- Co-authored-by: CI <ci@example.com> Co-authored-by: Claude Code <noreply@anthropic.com>
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.


This is an automated draft PR generated from issue #5388 — a maintainer should review it before merging.
Part of #5388. It covers the counter dip (part 1 of the issue) and stops a plant with no PV source publishing battery discharge as PV. It does not read meter 2, so the issue should stay open.
Builds on #5357 (merged): the guard also covers the per-inverter yield added there.
Summary
SolaX can return a lower value for a plant lifetime total for a while and then the right one again.
publish_plant_info()published the totals as received, so the recovery was counted as energy for that day: 181.3 kWh of PV on a 3.2 kWp array in the issue.hold_counter_dip(entity_id, value): a lifetime counter that reads lower than the last value published is held at that value, with a warning logged once per dip.total_yield,total_charged,total_discharged,total_imported,total_exported) and to each inverter's..._<sn>_total_yield.total_loadis built from the held values.SOLAX_COUNTER_DIP_HOLD_HOURS(24) is taken as a genuine reset and published.0.0no longer overwrites its known yield. This replaces the separate last-known-yield dict from fix(solax): build PV yield from the inverters, not the plant total #5357: the inverter's own sensor, held on a dip, is now the record.mpptMap/pvMap) or a yield counter now publishes 0 forpv_yieldinstead of the plant total, and logs a warning once. On such a plant (a lone X1-AC) the plant total is the inverter AC output, so battery discharge was published as PV and counted twice intotal_load. A plant whose inverters have PV inputs but report no yield still uses the plant total.Testing
tools/triage_test.sh solax: fails without the fix, passes with it. Withsolax.pystashed the module fails at import (the new constant is missing), which proves little, so the newtest_counter_dip_mainwas also run against the unfixed source with only the constant defined: it fails on the first case (Plant totals should be held at their previous values on a dip, got {'total_yield': 4260.4, ...}). With the fix all eight cases pass: dip held with the load, recovery published, reset accepted after the hold period, dip after a restart held against the sensor value, upward step published, an inverter reading 0.0 keeps its yield in the plant sum and in its own sensor, the no-PV-source warning logged once, and no warning for inverters with PV inputs.solax.pytwo cases fail (PV yield should be 0.0 ... got 19569.5and... got 4437.8); both pass with it, along with the load having no PV term and the plant figure still being used for inverters with PV inputs../run_all --quickand./run_pre_commit: passed on the final commit.Notes
plant_has_pv_source()ignoredtotalYield, so an inverter with a yield counter at 0.0 and no PV maps was classed as having no PV source. Such an inverter keeps the plant total, as before this PR.pv_yieldsensor has no guard of its own. It is the sum of inverter yields, or the plant total, and both are now held, so it cannot dip from a counter glitch. Guarding it separately would freeze it for a day whenever its source changes, for example when a plant moves from the plant total to its inverters.total_earningsis not guarded; nothing in Predbat reads it as energy.total_loadon a site with third-party PV is low by whatever that PV supplies, and the cumulative figure falls while the site exports. Predbat's incrementing-sensor clean-up holds through a fall of under 1 kWh and treats a larger one as a reset, so no energy is invented. PV calibration has no production to compare with, and the plan adds the raw PV forecast to a load that already has the PV taken out, so it will over-estimate the solar available on that site. The warning says so; the real fix for such a site is a PV source from outside SolaX.pv_yieldsteps from the plant total to 0 andtotal_loaddrops by the same amount, once.mpptMapof zeros, so it counts as having PV inputs. A plant made only of that would still use the plant total. Not seen on a live plant.gridPowerM2and theM2energy fields as meter 2, positive for export. On the live plant they are flat: 12 hours of/openapi/v2/device/history_datathrough daylight show yield null andgridPowerM20.0 throughout, and the plant has nodeviceType=3meter device. Reading meter 2 needs a plant where it works.hold_counter_dip()is called frompublish_plant_info(),publish_device_realtime_data()andget_inverter_last_yield(); earlierimpact()runs ratedpublish_plant_infoandpublish_device_realtime_dataLOW with one caller each (run).🤖 Generated with Claude Code