Repository navigation
Fix sticky reserve floor on inverters without managed reserve SOC (e.g. SolisCloud) - #3709
Conversation
…dd warning log Agent-Logs-Url: https://github.com/springfall2008/batpred/sessions/e08373e6-d20f-4e46-8a22-9372b56b5bdb Co-authored-by: springfall2008 <48591903+springfall2008@users.noreply.github.com>
| @@ -476,6 +476,14 @@ def __init__(self, base, id=0, quiet=False, rest_postCommand=None, rest_getData= | |||
There was a problem hiding this comment.
I'm wondering if this should be:
if (self.base.set_reserve_enable and self.inv_has_reserve_soc) or not self.inv_has_reserve_soc
Then the other change isn't required?
There was a problem hiding this comment.
@copilot Is there a simpler fix?
inverter.py line 475 could be:
if (self.base.set_reserve_enable and self.inv_has_reserve_soc) or not self.inv_has_reserve_soc
And then the other change might not be required?
There was a problem hiding this comment.
Done in 5caa936. Changed the condition to (self.base.set_reserve_enable and self.inv_has_reserve_soc) or not self.inv_has_reserve_soc, which means non-managed inverters (like SolisCloud) always use reserve_min as the planning floor. The adjust_reserve() change and the warning log were both reverted since they're no longer needed.
…n-managed inverters Agent-Logs-Url: https://github.com/springfall2008/batpred/sessions/afdc9cdf-fc4f-4025-b656-02c2eba79ced Co-authored-by: springfall2008 <48591903+springfall2008@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
Fixes a reserve “sticky floor” issue for inverter types that don’t support managed reserve SoC (e.g. SolisCloud), where planning/clamping could remain pinned to an elevated inverter-reported reserve value even after set_reserve_min is lowered.
Changes:
- Update
Inverter.__init__()to usereserve_minas the planning/clamp floor wheninv_has_reserve_socisFalse. - Extend the inverter reserve adjustment test helper to allow simulating
reserve_percentdiffering fromreserve_minand setinv.reserve_minexplicitly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/predbat/inverter.py | Adjusts how reserve_percent (planning/clamp floor) is derived for inverters without managed reserve SoC. |
| apps/predbat/tests/test_inverter.py | Updates test_adjust_reserve helper to support scenarios where reserve_percent and reserve_min differ. |
| self.base.log("Reserve min: {}%, battery_min: {}%".format(self.reserve_min, dp0(battery_min_soc))) | ||
| if self.base.set_reserve_enable and self.inv_has_reserve_soc: | ||
| if (self.base.set_reserve_enable and self.inv_has_reserve_soc) or not self.inv_has_reserve_soc: | ||
| self.reserve_percent = self.reserve_min | ||
| else: | ||
| self.reserve_percent = self.reserve_percent_current |
There was a problem hiding this comment.
The behavior change to set reserve_percent = reserve_min when inv_has_reserve_soc is False isn’t currently covered by a unit test. In apps/predbat/tests/test_inverter.py, all existing inverter instantiations use inverter_type=["GE"] (managed reserve), so this regression-prone path (e.g. inverter_type="SolisCloud" with a high/stale reserve sensor) isn’t asserted. Please add a test that constructs an inverter with inv_has_reserve_soc=False and verifies reserve_percent (and thus adjust_reserve’s clamp floor) follows set_reserve_min rather than the current sensor value.
On inverters where
inv_has_reserve_soc = False(e.g. SolisCloud), the battery plan would permanently show a floor at whatever reserve value Predbat had last written — even afterset_reserve_minwas lowered — becausereserve_percentwas set from the inverter-reportedreserve_percent_current(the elevated/sticky value) rather thanreserve_min.Mechanism: charge-freeze logic calls
adjust_reserve(soc_percent + 1)→ writes e.g. 80% to the reserve entity → freeze ends →reserve_percentis still read back as 80% from the inverter → plan is forever floored at 80% even thoughset_reserve_minis 4%.Changes
inverter.py—__init__(): Extended the condition that setsreserve_percent = reserve_minto also cover inverters whereinv_has_reserve_soc = False(i.e. Predbat does not manage the reserve entity). Previously only inverters withinv_has_reserve_soc = Trueandset_reserve_enable = Trueusedreserve_minas the planning floor.tests/test_inverter.py: Updatedtest_adjust_reserve()to also setinv.reserve_min(now relevant to the planning floor logic). Addedreserve_percentparameter to allow testing scenarios wherereserve_percentandreserve_mindiffer.