fix: release VPP control when IDLE at the reserve floor (#592)

CHANGELOG.md

@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Fixed
 
+- **Growatt VPP now lets the inverter and BMS sleep through a long idle at minimum SoC** — an empty battery was still held under continuous remote control, which nothing was protecting. ([#592](https://github.com/johanzander/bess-manager/issues/592))
 - **Grid charging now reaches the planned amount instead of stopping just short** — the charge rate is written as a whole percent, and rounding it down meant the battery charged slightly less than the plan counted on.
 - **Growatt VPP no longer briefly executes the previous period's power command when switching modes** — enabling remote control commits immediately, so the power target is now written before it, and cleared on release. ([#593](https://github.com/johanzander/bess-manager/issues/593))
 - **The battery now covers house load exactly instead of exporting a few Wh and committing the inverter** — a period whose load fell between two discharge steps was planned as a small export, which forces `grid_first` at a fixed rate and imports any load spike. ([#352](https://github.com/johanzander/bess-manager/issues/352))

core/bess/battery_system_manager.py

@@ -2715,6 +2715,8 @@ def _apply_period_schedule(self, period: int) -> None:
                         error=e,
                     )
 
+        at_reserve_floor = self._at_reserve_floor()
+
         # Store the schedule's desired discharge rate before inhibit check so that
         # apply_discharge_inhibit() can restore it when the inhibit sensor clears.
         self._desired_discharge_rate = discharge_rate
@@ -2764,6 +2766,7 @@ def _apply_period_schedule(self, period: int) -> None:
             discharge_rate,
             block_passive_charging,
             strategic_intent,
+            at_reserve_floor,
         )
 
         if not success:
@@ -2790,6 +2793,32 @@ def _apply_period_schedule(self, period: int) -> None:
         # Apply charging power rate (BSM-level concern: uses power monitor)
         self.adjust_charging_power()
 
+    def _at_reserve_floor(self) -> bool:
+        """Whether the battery is sitting on its reserve floor right now (#592).
+
+        Read live rather than taken from the plan: an IDLE hold exists to
+        protect stored energy from self-consumption, so what decides whether
+        the hold is worth anything is whether energy is actually there now. A
+        plan that expected a reserve does not mean one survived.
+
+        Called fresh at each write, including retries minutes later, for the
+        same reason -- a captured flag would command the inverter on a SoC
+        that has since moved.
+
+        The SoE conversion deliberately mirrors `min_soe_kwh`'s own
+        (`total_capacity * pct / 100`, settings.py) rather than the equivalent
+        `pct / 100 * total_capacity`. The two can differ in the last bit, and
+        the case that decides this branch is exact equality -- a battery
+        parked on its floor overnight, which is precisely the reported
+        scenario.
+        """
+        current_soe = (
+            self.battery_settings.total_capacity
+            * self.controller.get_battery_soc()
+            / 100.0
+        )
+        return current_soe <= self.battery_settings.min_soe_kwh
+
     _PERIOD_RETRY_DELAYS_MIN: ClassVar[list[int]] = [
         3,
         8,
@@ -2838,6 +2867,7 @@ def retry_period_write():
                 discharge_rate,
                 block_passive_charging,
                 strategic_intent,
+                self._at_reserve_floor(),
             )
             self._runtime_failure_tracker.dismiss_by_category("period_apply")
             if not success:
@@ -3456,6 +3486,10 @@ def apply_discharge_inhibit(self) -> None:
             target_rate,
             self._desired_block_passive_charging,
             self._desired_strategic_intent,
+            # Fresh, not the value from the scheduled write: this runs
+            # mid-period, and omitting it would default to False and
+            # re-assert the battery_first hold #592 released.
+            self._at_reserve_floor(),
         )
         self._last_applied_discharge_rate = target_rate
 

core/bess/inverter_controller.py

@@ -578,6 +578,7 @@ def apply_period(
         discharge_rate: int,
         block_passive_charging: bool = False,
         strategic_intent: str = "",
+        at_reserve_floor: bool = False,
     ) -> tuple[bool, str]:
         """Write period control settings to hardware.
 
@@ -599,6 +600,12 @@ def apply_period(
                 BATTERY_EXPORT to the same values, so platforms that need to
                 treat them differently (VPP-style -- see #413) require the
                 intent itself. Register-based platforms ignore this.
+            at_reserve_floor: Whether the battery is at (or below) its
+                configured minimum SoE right now. Register-based platforms
+                ignore this -- their min_soc register already stops discharge
+                at the floor. Forced-power platforms use it to stop holding a
+                battery that has nothing left to hold, releasing the inverter
+                so its BMS can sleep -- see #592.
 
         Returns:
             Tuple of (success, error_message). error_message is empty on success.

core/bess/solax_modbus_growatt_controller.py

@@ -45,6 +45,13 @@
   self-consumption on grid/solar instead. grid_first (power<=0) does not
   achieve this -- per #118, self-consumption is still drawn from the
   battery in grid_first, only battery-first releases it to grid/solar)
+- IDLE (rate=0) **at the reserve floor** -> power=0, remote_control
+  DISABLED (#592 -- the hold above protects stored energy, and at the floor
+  there is none left to protect; keeping remote control enabled rewrites the
+  command every period to refresh the fallback timer, so the inverter is
+  never handed back and its BMS never sleeps). Releasing rather than
+  commanding power=0 with remote control enabled is what keeps this
+  flow-neutral -- see _intent_to_vpp
 - SOLAR_EXPORT (rate=0, block_passive_charging=True) -> power=0,
   remote_control ENABLED (grid_first hold, per the Growatt VPP protocol
   V2.01 section 3.5 -- battery held flat, solar bypasses to grid)
@@ -230,6 +237,7 @@ def apply_period(
         discharge_rate: int,
         block_passive_charging: bool = False,
         strategic_intent: str = "",
+        at_reserve_floor: bool = False,
     ) -> tuple[bool, str]:
         """Write period control settings for the current control mode.
 
@@ -253,6 +261,12 @@ def apply_period(
                 grid_charge/discharge_rate/block_passive_charging alone
                 can't tell them apart, but VPP mode must, since each needs a
                 different remote_control/power command.
+            at_reserve_floor: Whether the battery is currently at (or below)
+                its configured minimum SoE. TOU mode ignores this. VPP mode
+                uses it to release the IDLE hold (#592) -- see
+                _intent_to_vpp. Derived from a live SoC read, not from the
+                plan: the hold exists to protect stored energy, so what
+                matters is whether any is actually there now.
 
         Returns:
             Tuple of (success, error_message). error_message is empty on success.
@@ -264,6 +278,7 @@ def apply_period(
                 discharge_rate,
                 block_passive_charging,
                 strategic_intent,
+                at_reserve_floor,
             )
         return self._apply_period_tou(controller, grid_charge, discharge_rate)
 
@@ -331,9 +346,11 @@ def _intent_to_vpp(
         discharge_rate: int,
         block_passive_charging: bool = False,
         strategic_intent: str = "",
+        at_reserve_floor: bool = False,
     ) -> tuple[int, bool]:
         """Map (grid_charge, discharge_rate, block_passive_charging,
-        strategic_intent) to (power_pct, remote_control_enabled).
+        strategic_intent, at_reserve_floor) to
+        (power_pct, remote_control_enabled).
 
         - grid_charge=True                       -> +100% (charge at max rate)
         - grid_charge=False, intent=LOAD_SUPPORT  -> 0%, remote control
@@ -367,6 +384,36 @@ def _intent_to_vpp(
           branch below so IDLE doesn't fall into SOLAR_STORAGE's self-use
           disable. Not yet real-hardware-validated; ships experimental
           pending confirmation.
+        - grid_charge=False, rate=0, intent=IDLE, at_reserve_floor=True
+          -> 0%, remote control DISABLED (#592). The battery_first hold above
+          protects stored energy from self-consumption; at the floor there is
+          none left to protect, so it buys nothing and costs the inverter its
+          sleep -- remote control being enabled makes _apply_period_vpp
+          rewrite the command every period to refresh the fallback timer
+          (#404), so the inverter is never handed back and the BMS never
+          idles down.
+
+          Releasing, rather than writing power=0 with remote control still
+          enabled (which is grid_first), is what keeps this flow-neutral:
+          load_first still absorbs passive solar surplus exactly as the
+          battery_first hold does, where grid_first would bypass it to the
+          grid -- a real change, since IDLE's DP cost model does credit that
+          absorption. Flow-neutrality is proved in
+          test_vpp_simulator_branches.py::TestIdleAtReserveFloor.
+
+          **How far the battery can actually fall is the inverter's own
+          discharge_stop_soc, not BESS's min_soc, and in VPP mode BESS does
+          not write that register** -- initialize_hardware returns before
+          sync_soc_limits for control_mode="vpp" (#309). So if the inverter's
+          own floor sits below the configured min_soc, released self-use can
+          draw the gap between them to cover house load. That is not new
+          here: LOAD_SUPPORT (#413) and SOLAR_STORAGE already release control
+          the same way at any SoC, so this extends an existing exposure to
+          IDLE rather than creating one -- and it is bounded by the gap
+          between the two floors, which is zero on a correctly configured
+          inverter. `vpp_simulator` models the release as a hold at
+          min_soe_kwh (available == 0), i.e. it assumes the two floors agree.
+          Flagged for real-hardware confirmation with #592's reporter.
         - grid_charge=False, rate=0, block=False  -> 0%, remote control DISABLED
           (load_first self-use -- SOLAR_STORAGE, battery may absorb solar)
         - grid_charge=False, rate>0 (otherwise, i.e. BATTERY_EXPORT)
@@ -378,7 +425,7 @@ def _intent_to_vpp(
             return 0, False
         if discharge_rate == 0:
             if strategic_intent == "IDLE":
-                return 1, True
+                return (0, False) if at_reserve_floor else (1, True)
             return 0, block_passive_charging
         return -discharge_rate, True
 
@@ -449,6 +496,7 @@ def _apply_period_vpp(
         discharge_rate: int,
         block_passive_charging: bool = False,
         strategic_intent: str = "",
+        at_reserve_floor: bool = False,
     ) -> tuple[bool, str]:
         """Write one period's VPP power command.
 
@@ -459,7 +507,11 @@ def _apply_period_vpp(
         timer to protect.
         """
         power_pct, remote_control_enabled = self._intent_to_vpp(
-            grid_charge, discharge_rate, block_passive_charging, strategic_intent
+            grid_charge,
+            discharge_rate,
+            block_passive_charging,
+            strategic_intent,
+            at_reserve_floor,
         )
 
         needs_write = remote_control_enabled or (

core/bess/tests/unit/test_period_retry.py

@@ -11,6 +11,11 @@ def _make_bsm_with_mocks():
     bsm = BatterySystemManager.__new__(BatterySystemManager)
     bsm._inverter_controller = MagicMock()
     bsm._controller = MagicMock()
+    # A real SoC, not a bare MagicMock: the write path reads it to decide
+    # whether the battery is on its reserve floor (#592). Well above the
+    # default min_soc, so these retry tests keep exercising the ordinary
+    # (not-at-floor) case they were written for.
+    bsm._controller.get_battery_soc.return_value = 50.0
     bsm._runtime_failure_tracker = MagicMock()
     bsm._scheduler = MagicMock()
     bsm._last_applied_discharge_rate = 0

core/bess/tests/unit/test_solax_modbus_growatt_vpp.py

@@ -129,6 +129,40 @@ def test_idle_enables_battery_first_hold(self, controller):
         assert power_pct == 1
         assert enabled is True
 
+    def test_idle_at_reserve_floor_releases_remote_control(self, controller):
+        """#592: at the reserve floor the battery-first hold has nothing left
+        to protect, but keeping remote control enabled re-asserts a command
+        every period so the inverter (and its BMS) never sleeps.
+
+        Releasing to the inverter's own load_first self-use is flow-neutral
+        here -- there is no headroom to discharge (the hardware's own
+        discharge_stop_soc register is the floor) and passive solar
+        absorption is preserved, which grid_first (power=0, enabled) would
+        lose. See test_idle_at_floor_is_flow_neutral in
+        test_vpp_simulator_branches.py for the outcome-level proof."""
+        power_pct, enabled = controller._intent_to_vpp(
+            grid_charge=False,
+            discharge_rate=0,
+            block_passive_charging=False,
+            strategic_intent="IDLE",
+            at_reserve_floor=True,
+        )
+        assert power_pct == 0
+        assert enabled is False
+
+    def test_idle_above_reserve_floor_still_holds_battery_first(self, controller):
+        """#592 must not weaken #466: above the floor there IS energy being
+        held back for a later peak, so the battery-first hold stays."""
+        power_pct, enabled = controller._intent_to_vpp(
+            grid_charge=False,
+            discharge_rate=0,
+            block_passive_charging=False,
+            strategic_intent="IDLE",
+            at_reserve_floor=False,
+        )
+        assert power_pct == 1
+        assert enabled is True
+
     def test_solar_export_keeps_remote_control_enabled(self, controller):
         """SOLAR_EXPORT (block_passive_charging=True) -> grid-first hold,
         not self-use -- see #355. Remote control stays enabled with

core/bess/tests/unit/test_vpp_idle_at_reserve_floor.py

@@ -0,0 +1,110 @@
+"""IDLE at the reserve floor releases VPP control so the BMS can sleep (#592).
+
+Reported behaviour: during a long overnight IDLE with the battery already at
+its minimum SoC, the inverter was held in `battery_first` (`vpp_power=+1`,
+remote control enabled) and that command was re-asserted every period, so the
+inverter was never handed back and its BMS never slept.
+
+`battery_first` is right whenever IDLE is *holding energy back* for a later
+peak (#466) -- it keeps self-consumption on grid/solar instead of draining the
+battery, which IDLE's own DP cost model (`_idle_battery_flows`) never credits.
+At the reserve floor there is nothing left to hold, so the hold buys nothing
+and costs the BMS its sleep.
+
+**These tests drive the real production write path**
+(`BatterySystemManager._apply_period_schedule`), not `_intent_to_vpp` with
+hand-built arguments. That is deliberate: the mapping alone could be correct
+while the floor flag never reaches it -- the branch would be dead in
+production and a unit test on the mapping would still pass. What is asserted
+here is the command that actually lands on the inverter.
+
+Flow-neutrality of the swap (the reason this needs no VPP baseline re-pin) is
+proved separately in
+`test_vpp_simulator_branches.py::TestIdleAtReserveFloor`.
+"""
+
+from types import SimpleNamespace
+
+from core.bess.battery_system_manager import BatterySystemManager
+from core.bess.price_manager import MockSource
+from core.bess.tests.conftest import MockHomeAssistantController
+
+PERIOD = 12  # 03:00 -- the overnight idle stretch from the report
+
+
+def _make_vpp_bsm(
+    soc: float,
+) -> tuple[BatterySystemManager, MockHomeAssistantController]:
+    controller = MockHomeAssistantController()
+    controller.settings["battery_soc"] = soc
+    bsm = BatterySystemManager(
+        controller=controller,
+        price_source=MockSource([1.0] * 96),
+        addon_options={
+            "inverter": {
+                "platform": "solax_modbus_growatt_min",
+                "control_mode": "vpp",
+            }
+        },
+    )
+    intents = ["IDLE"] * 96
+    bsm._inverter_controller.strategic_intents = intents
+    bsm._inverter_controller.current_schedule = SimpleNamespace(actions=[0.0] * 96)
+    return bsm, controller
+
+
+def _last_vpp_command(controller: MockHomeAssistantController) -> dict:
+    return controller.calls["growatt_vpp_periods"][-1]
+
+
+class TestIdleAtReserveFloorReleasesControl:
+    def test_idle_at_the_floor_releases_the_inverter(self):
+        """At min SoC the written command must release remote control, so the
+        inverter reverts to its own self-use and stops being commanded."""
+        bsm, controller = _make_vpp_bsm(soc=10.0)
+        assert (
+            bsm.battery_settings.min_soc == 10.0
+        ), "fixture assumes the default 10% floor; the SoC above must equal it"
+
+        bsm._apply_period_schedule(PERIOD)
+
+        command = _last_vpp_command(controller)
+        assert command["power_pct"] == 0
+        assert command["remote_control_enabled"] is False
+
+    def test_idle_above_the_floor_still_holds_battery_first(self):
+        """#466 must survive #592: with energy still banked for the morning
+        peak, IDLE holds battery_first exactly as before."""
+        bsm, controller = _make_vpp_bsm(soc=50.0)
+
+        bsm._apply_period_schedule(PERIOD)
+
+        command = _last_vpp_command(controller)
+        assert command["power_pct"] == 1
+        assert command["remote_control_enabled"] is True
+
+    def test_released_control_stops_re_asserting_every_period(self):
+        """The actual mechanism behind "the BMS never sleeps": with remote
+        control enabled `_apply_period_vpp` rewrites every period to refresh
+        the inverter's fallback timer (#404). Once released there is nothing
+        to refresh, so the writes must stop rather than continue silently."""
+        bsm, controller = _make_vpp_bsm(soc=10.0)
+
+        for period in range(PERIOD, PERIOD + 4):
+            bsm._apply_period_schedule(period)
+
+        assert len(controller.calls["growatt_vpp_periods"]) == 1, (
+            "a released inverter must be written once, not re-commanded every "
+            "period -- re-asserting is what kept the BMS awake"
+        )
+
+    def test_hold_still_re_asserts_every_period_above_the_floor(self):
+        """Guard rail on the test above: the every-period refresh is correct
+        and must be preserved wherever remote control is genuinely active,
+        otherwise the fallback timer would lapse mid-hold (#404)."""
+        bsm, controller = _make_vpp_bsm(soc=50.0)
+
+        for period in range(PERIOD, PERIOD + 4):
+            bsm._apply_period_schedule(period)
+
+        assert len(controller.calls["growatt_vpp_periods"]) == 4