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,59 @@ 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.
+
+        **Index p-1, not p.** `state_of_energy` is `combined_soe`, whose only
+        writer stores `period_data.energy.battery_soe_end` (see
+        `BatterySystemManager._create_updated_schedule`) -- `battery_soe_end`
+        and `battery_soe_start` are distinct fields on `EnergyData`. So
+        `state_of_energy[p]` is the SoE *leaving* period p, and the SoE
+        *entering* it is index p-1. Reading index p directly would answer for
+        the wrong period: at the boundary where the plan first reaches the
+        floor it would report "at floor" one period early, and one period late
+        on the way back up -- exactly the display/write disagreement this
+        method exists to prevent.
+
+        Period 0 has no predecessor in the array. `combined_soe[0]` is that
+        period's own *entering* SoE in the one case where period 0 is a
+        prediction rather than history (period 0 being the optimization
+        period, written as `current_soe`), so index 0 is the correct read
+        there rather than a fallback.
+
+        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.
+        """
+        if self.current_schedule is None:
+            return False
+        soe = self.current_schedule.state_of_energy
+        if period >= len(soe):
+            return False
+        entering_soe = soe[period - 1] if period > 0 else soe[0]
+        return entering_soe <= 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 +754,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,