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))
 - **A tiny solar surplus is no longer planned as an export the inverter will absorb** — below the export the plan can express, the battery charged anyway and ran fuller than planned, spilling the difference later. ([#630](https://github.com/johanzander/bess-manager/issues/630))
 - **The setup wizard no longer locks you out of an inverter platform it failed to auto-detect** — every platform stays selectable, and a re-scan keeps the one you picked. ([#621](https://github.com/johanzander/bess-manager/issues/621))
 - **System no longer gets stuck on "initializing" when many consecutive periods are near-tied** — a long run of volatile prices could make every hourly optimization fail, leaving no schedule at all. ([#624](https://github.com/johanzander/bess-manager/issues/624))

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,51 @@ 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.
+
+        **An unreadable SoC holds, and says so.** `get_battery_soc()` is
+        `float | None`, so a transient unavailable/unknown sensor must be
+        decided here rather than propagating: this runs for every platform on
+        every period write, and two of its callers (the retry closure's
+        apscheduler job and the every-minute discharge-inhibit job) have no
+        exception handling at all, so raising would take down far more than
+        this flag. Holding is chosen over releasing because it is the safe
+        direction and is exactly the pre-#592 behaviour -- releasing is what
+        could let the inverter's own self-use draw the battery down, so it
+        must never happen on a reading we could not verify. This is an
+        explicit, logged branch, not a silent fallback: rules.md forbids
+        degrading quietly, not choosing a safe outcome loudly.
+
+        Validation is `_get_current_battery_soc()`'s, reused rather than
+        restated, so the definition of a valid reading stays in one place.
+        """
+        soc = self._get_current_battery_soc()
+        if soc is None:
+            logger.warning(
+                "Reserve-floor check: SoC unreadable — holding the battery "
+                "(not releasing VPP control) until a valid reading returns"
+            )
+            return False
+        current_soe = self.battery_settings.total_capacity * soc / 100.0
+        return current_soe <= self.battery_settings.min_soe_kwh
+
     _PERIOD_RETRY_DELAYS_MIN: ClassVar[list[int]] = [
         3,
         8,
@@ -2838,6 +2886,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 +3505,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.
@@ -668,16 +675,39 @@ def get_period_settings(self, period: int) -> dict:
             "discharge_rate": discharge_rate,
             "strategic_intent": intent,
             **self._mode_display_fields(
-                intent, grid_charge, discharge_rate, block_passive_charging
+                intent,
+                grid_charge,
+                discharge_rate,
+                block_passive_charging,
+                self._planned_at_reserve_floor(period),
             ),
         }
 
+    def _planned_at_reserve_floor(self, period: int) -> bool:
+        """Whether the *plan* has the battery on its reserve floor entering
+        this period (#592).
+
+        The display counterpart to `BatterySystemManager._at_reserve_floor()`,
+        which reads live SoC. A displayed period is a prediction, so the plan's
+        own SoE trajectory is the correct input -- but it must answer the same
+        question, or the UI shows a hold for periods production releases and
+        `_mode_display_fields` breaks its own no-fabrication contract.
+
+        False when there is no plan to read: with no trajectory there is no
+        prediction to display, and the hold is the unchanged-behaviour answer.
+        """
+        soe = getattr(self.current_schedule, "state_of_energy", None)
+        if not soe or period >= len(soe):
+            return False
+        return soe[period] <= self.battery_settings.min_soe_kwh
+
     def _mode_display_fields(
         self,
         intent: str,
         grid_charge: bool,
         discharge_rate: int,
         block_passive_charging: bool,
+        at_reserve_floor: bool = False,
     ) -> dict:
         """Single source of truth for what mode-related fields a period
         gets, branching on CONTROL_MODEL. Never fabricates a label the
@@ -704,7 +734,11 @@ def _mode_display_fields(
             # SolaxModbusGrowattController) -- no hasattr() duck-typing on a
             # subclass-private method name.
             power_pct, remote_control = self._vpp_display_state(
-                grid_charge, discharge_rate, block_passive_charging, intent
+                grid_charge,
+                discharge_rate,
+                block_passive_charging,
+                intent,
+                at_reserve_floor,
             )
             return {
                 "vpp_power_pct": power_pct,

core/bess/simulation/vpp_simulator.py

@@ -49,9 +49,22 @@ def derive_vpp_commands(
     intents: list[str],
     actions_kw: list[float],
     settings: BatterySettings,
+    soe_trajectory: list[float],
 ) -> list[VppCommand]:
     """The VPP commands production would write for a planned schedule.
 
+    `soe_trajectory` supplies the state of energy entering each period, which
+    `_intent_to_vpp` needs to answer the reserve-floor question (#592). It is
+    required, not defaulted: an optional one is exactly how this harness came
+    to derive every command with `at_reserve_floor=False`, leaving the release
+    branch unreachable through the whole 37-fixture corpus while the baseline
+    looked like it covered it.
+
+    Note this derives commands *eagerly*, so it is only correct where the
+    trajectory is already known. `simulate_vpp` does not use it -- there the
+    SoE is produced by the simulation itself, so each command is derived from
+    the running SoE as the loop reaches it.
+
     Drives the production path end to end:
     `compute_rates_for_period` -> `_intent_to_vpp`, the same two calls
     `BatterySystemManager._apply_period_schedule` makes. Nothing about the
@@ -82,27 +95,55 @@ def derive_vpp_commands(
             f"{len(actions_kw)} actions"
         )
 
+    if len(soe_trajectory) < len(actions_kw):
+        raise ValueError(
+            f"plan is inconsistent: {len(soe_trajectory)} SoE entries for "
+            f"{len(actions_kw)} actions"
+        )
+
+    controller = _vpp_controller(intents, settings)
+    return [
+        _derive_vpp_command(
+            controller,
+            period,
+            action_kw,
+            at_reserve_floor=soe_trajectory[period] <= settings.min_soe_kwh,
+        )
+        for period, action_kw in enumerate(actions_kw)
+    ]
+
+
+def _vpp_controller(
+    intents: list[str], settings: BatterySettings
+) -> SolaxModbusGrowattController:
+    """The production controller, configured for one plan's intents."""
     controller = SolaxModbusGrowattController(settings, control_mode="vpp")
     controller.strategic_intents = list(intents)
+    return controller
 
-    commands = []
-    for period, action_kw in enumerate(actions_kw):
-        grid_charge, discharge_rate, block_passive_charging = (
-            controller.compute_rates_for_period(period, action_kw)
-        )
-        power_pct, remote_control_enabled = controller._intent_to_vpp(
-            grid_charge,
-            discharge_rate,
-            block_passive_charging,
-            intents[period],
-        )
-        commands.append(
-            VppCommand(
-                power_pct=power_pct,
-                remote_control_enabled=remote_control_enabled,
-            )
-        )
-    return commands
+
+def _derive_vpp_command(
+    controller: SolaxModbusGrowattController,
+    period: int,
+    action_kw: float,
+    at_reserve_floor: bool,
+) -> VppCommand:
+    """One period's command, via the same two production calls
+    `BatterySystemManager._apply_period_schedule` makes."""
+    grid_charge, discharge_rate, block_passive_charging = (
+        controller.compute_rates_for_period(period, action_kw)
+    )
+    power_pct, remote_control_enabled = controller._intent_to_vpp(
+        grid_charge,
+        discharge_rate,
+        block_passive_charging,
+        controller.strategic_intents[period],
+        at_reserve_floor,
+    )
+    return VppCommand(
+        power_pct=power_pct,
+        remote_control_enabled=remote_control_enabled,
+    )
 
 
 def vpp_command_to_power(
@@ -258,10 +299,15 @@ def vpp_command_to_power(
 class VppSimulationResult:
     period_data: list = field(default_factory=list)
     realized_cost: float = 0.0
+    # The commands actually issued, derived per period as the run reached it.
+    # Returned rather than taken as input because #592 made the command a
+    # function of the SoE the simulation itself produces.
+    commands: list = field(default_factory=list)
 
 
 def simulate_vpp(
-    commands: list[VppCommand],
+    intents: list[str],
+    actions_kw: list[float],
     solar_production: list[float],
     home_consumption: list[float],
     buy_price: list[float],
@@ -274,10 +320,93 @@ def simulate_vpp(
     """Execute a VPP command sequence, carrying SoE forward, using the
     optimizer's own flow and accounting primitives -- same arrangement as
     `inverter_simulator.simulate`, so realized cost is comparable between the
-    two platforms."""
+    two platforms.
+
+    Takes the *plan* rather than a prebuilt command list: since #592 a
+    command depends on whether the battery is at its reserve floor, which is
+    only known once the run has carried SoE forward to that period. Deriving
+    the list up front is what made the release branch unreachable here, and
+    with it the whole corpus's claim to cover it.
+
+    To execute a command list that no plan would produce -- a hypothetical, to
+    contrast against what production actually writes -- use
+    `simulate_vpp_commands` instead."""
+    if len(intents) != len(actions_kw):
+        raise ValueError(
+            f"plan is inconsistent: {len(intents)} intents vs "
+            f"{len(actions_kw)} actions"
+        )
+
+    controller = _vpp_controller(intents, settings)
+    return _simulate(
+        lambda t, soe: _derive_vpp_command(
+            controller, t, actions_kw[t], at_reserve_floor=soe <= settings.min_soe_kwh
+        ),
+        len(actions_kw),
+        solar_production,
+        home_consumption,
+        buy_price,
+        sell_price,
+        initial_soe,
+        settings,
+        dt,
+        currency,
+    )
+
+
+def simulate_vpp_commands(
+    commands: list[VppCommand],
+    solar_production: list[float],
+    home_consumption: list[float],
+    buy_price: list[float],
+    sell_price: list[float],
+    initial_soe: float,
+    settings: BatterySettings,
+    dt: float,
+    currency: str = "SEK",
+) -> VppSimulationResult:
+    """Execute an explicit command sequence, bypassing derivation.
+
+    For hypotheticals only -- "what would the battery have done had it been
+    commanded this instead". Anything asserting what production *does* must go
+    through `simulate_vpp`, which derives the commands the way production
+    does.
+    """
+    return _simulate(
+        lambda t, _soe: commands[t],
+        len(commands),
+        solar_production,
+        home_consumption,
+        buy_price,
+        sell_price,
+        initial_soe,
+        settings,
+        dt,
+        currency,
+    )
+
+
+def _simulate(
+    command_at,
+    n_periods: int,
+    solar_production: list[float],
+    home_consumption: list[float],
+    buy_price: list[float],
+    sell_price: list[float],
+    initial_soe: float,
+    settings: BatterySettings,
+    dt: float,
+    currency: str,
+) -> VppSimulationResult:
+    """The one execution loop. `command_at(period, soe)` supplies each
+    period's command against the SoE the run has reached, which is what lets
+    a derived command depend on the reserve floor (#592)."""
     soe = initial_soe
     period_data = []
-    for t, cmd in enumerate(commands):
+    commands = []
+    for t in range(n_periods):
+        cmd = command_at(t, soe)
+        commands.append(cmd)
         power = vpp_command_to_power(
             cmd, solar_production[t], home_consumption[t], soe, settings, dt
         )
@@ -324,4 +453,5 @@ def simulate_vpp(
     return VppSimulationResult(
         period_data=period_data,
         realized_cost=sum(pd.economic.hourly_cost for pd in period_data),
+        commands=commands,
     )