Sigenergy: make Predbat's ownership of the inverter deliberate (option A for #4454)

apps/predbat/sigenergy.py

@@ -132,6 +132,7 @@
 SIGENERGY_TOKEN_EXPIRY_BUFFER = 600   # refresh token 10 min before expiry
 SIGENERGY_MIN_REQUEST_INTERVAL = 6.0  # enforce ≥10 req/min API limit
 SIGENERGY_POLL_INTERVAL = 300         # realtime data poll every 5 minutes
+SIGENERGY_VPP_RECLAIM_INTERVAL = 60   # re-assert VPP ownership every minute (see _manage_vpp_registration)
 SIGENERGY_DEVICE_POLL_INTERVAL = 1800  # device list refresh every 30 minutes
 SIGENERGY_RATE_LIMIT_BACKOFF = [15, 30, 60, 120, 480]  # seconds to wait after code 1201
 SIGENERGY_BATTERY_NOMINAL_VOLTAGE_V = 28.8  # 8S LiFePO4 pack: 8 × 3.6V; used to convert ratedEnergy (Ah) → kWh
@@ -150,6 +151,11 @@
 SIGENERGY_MODE_VPP = 6   # VPP mode
 SIGENERGY_MODE_NBI = 8   # NorthBound (defined for completeness; not switched to by this component)
 
+# Modes that mean a third party is driving the inverter rather than the owner's app.
+# A Sigenergy accepts one controller at a time, so finding the system in one of these
+# means Predbat's VPP registration has been displaced — see _manage_vpp_registration.
+SIGENERGY_THIRD_PARTY_MODES = (SIGENERGY_MODE_NBI,)
+
 # Human-readable names for operationalMode integer values
 SIGENERGY_MODE_NAMES = {
     0: "Maximum Self-Consumption",
@@ -309,6 +315,7 @@ def initialize(self, app_key, app_secret, base_url=None, mqtt_host=None, ca_cert
         self.history_totals = {}  # systemId → {sankey node id: lifetime kWh total}
         self.mqtt_period_raw = {}  # systemId → merged raw 'period' fields (MQTT only sends fields that changed)
         self.current_mode = {}    # systemId → energyStorageOperationMode int
+        self.contended_by = {}    # systemId → mode name of the controller that last displaced Predbat
         self.onboard_status = {}  # systemId → onboarding status string (published for the SaaS UI)
 
         # Age (datetime of last update) of each SIGENERGY_CACHE_KEYS category, used to avoid an
@@ -2176,6 +2183,15 @@ async def _manage_vpp_registration(self, system_id, is_readonly, is_offboard=Fal
           readonly=False + VPP active   → nothing to do (ready for controls)
           readonly=False + VPP inactive → switch to VPP mode to enable controls
 
+        Note on contention: a Sigenergy accepts one controller at a time, and VPP mode
+        (Predbat) and the NorthBound Interface (used by Axle for VPP dispatches) are
+        mutually exclusive. Predbat is treated as the owner of the inverter, so finding
+        the system in NBI means reclaiming it — which overrides whatever the other
+        controller had scheduled. Predbat already ingests Axle sessions as its own export
+        windows (see load_axle_slot), so the event still runs; it runs under Predbat's
+        plan rather than Axle's dispatch. If Axle should instead own the inverter for the
+        duration of its events, that is what the ``axle_control`` option is for.
+
         Args:
             system_id: Sigenergy system unique identifier.
             is_readonly: Current state of the Predbat read-only switch.
@@ -2195,7 +2211,21 @@ async def _manage_vpp_registration(self, system_id, is_readonly, is_offboard=Fal
             return False
 
         if not is_readonly and not in_vpp:
-            self.log("SigenergyAPI: System {} is not in VPP mode — switching to VPP to enable controls".format(system_id))
+            current = self.current_mode.get(system_id, -1)
+            if current in SIGENERGY_THIRD_PARTY_MODES:
+                # Another controller (typically an Axle VPP dispatch, which drives the
+                # system through the NorthBound Interface) has taken the inverter. VPP and
+                # NBI are mutually exclusive on a Sigenergy, so somebody has to lose.
+                # Predbat is the configured owner, so reclaim — and say so plainly, since
+                # this displaces the other controller's schedule.
+                self.contended_by[system_id] = SIGENERGY_MODE_NAMES.get(current, "Unknown")
+                self.log(
+                    "Warn: SigenergyAPI: System {} was taken by another controller ({}) — reclaiming VPP mode, which overrides that controller's schedule".format(
+                        system_id, SIGENERGY_MODE_NAMES.get(current, "Unknown ({})".format(current))
+                    )
+                )
+            else:
+                self.log("SigenergyAPI: System {} is not in VPP mode ({}) — switching to VPP to enable controls".format(system_id, SIGENERGY_MODE_NAMES.get(current, "Unknown")))
             await self.set_operating_mode(system_id, SIGENERGY_MODE_VPP)
             return False  # current_mode will be updated by MQTT/REST on the next cycle
 
@@ -2217,6 +2247,9 @@ def _publish_onboard_status(self):
                     "friendly_name": "Sigenergy {} Onboarding Status".format(sid),
                     "system_id": sid,
                     "in_vpp": self.current_mode.get(sid) == SIGENERGY_MODE_VPP,
+                    # Set when another controller last displaced Predbat's VPP registration,
+                    # so support can tell contention apart from a genuine onboarding problem.
+                    "contended_by": self.contended_by.get(sid),
                 },
                 app="sigenergy",
             )
@@ -2379,10 +2412,19 @@ async def run(self, seconds, first):
             for sid in list(self.systems.keys()):
                 await self.fetch_device_list(sid)
 
-        # VPP registration management — runs at startup and every 5 minutes.
+        # VPP registration management — runs at startup and every minute.
+        #
+        # This used to run on the 5 minute poll interval, which meant that when another
+        # controller (an Axle NBI dispatch, say) grabbed the system, it could hold it for
+        # up to 5 minutes before Predbat noticed and reclaimed VPP. Predbat is the owner
+        # of the inverter, so re-assert that promptly and keep the contended window short
+        # rather than leaving control ambiguous for minutes at a time. set_operating_mode
+        # is an MQTT publish and only fires when the mode is actually wrong, so the faster
+        # cadence costs nothing against the REST rate limit.
+        #
         # Skips any system whose operating mode is not yet known (REST bootstrap
         # may have failed; MQTT will populate current_mode once it arrives).
-        if first or seconds % SIGENERGY_POLL_INTERVAL == 0:
+        if first or seconds % SIGENERGY_VPP_RECLAIM_INTERVAL == 0:
             is_readonly_vpp = self.get_state_wrapper("switch.{}_set_read_only".format(self.prefix), default="off") == "on"
             for sid in list(self.systems.keys()):
                 if sid not in self.current_mode:
@@ -2392,12 +2434,26 @@ async def run(self, seconds, first):
                 is_offboard = self.get_state_wrapper("switch.{}_sigenergy_{}_offboard".format(self.prefix, slug), default="off") == "on"
                 await self._manage_vpp_registration(sid, is_readonly_vpp, is_offboard)
                 # Derive the user-facing onboarding status for the visible system.
+                #
+                # A system sitting in a third-party mode is fully onboarded — another
+                # controller has simply taken it for a moment. Reporting "pending_approval"
+                # there makes the SaaS UI show an amber "waiting for your approval in the
+                # Sigenergy app" banner for the length of every Axle event, telling the user
+                # to go and approve something that needs no approval. Keep it "active" and
+                # expose the contention through the sensor attributes instead.
                 if is_offboard:
                     self.onboard_status[str(sid)] = "offboarded"
                 elif self.current_mode.get(sid) == SIGENERGY_MODE_VPP:
                     self.onboard_status[str(sid)] = "active"
+                    self.contended_by.pop(sid, None)
+                elif self.current_mode.get(sid) in SIGENERGY_THIRD_PARTY_MODES:
+                    self.onboard_status[str(sid)] = "active"
                 else:
                     self.onboard_status[str(sid)] = "pending_approval"
+
+        # Persist the derived status on the slower poll cadence — the reclaim check above
+        # runs every minute and the cache does not need rewriting that often.
+        if first or seconds % SIGENERGY_POLL_INTERVAL == 0:
             await self._save_cache("onboard_status", self.onboard_status)
 
         # Publish onboarding status for the SaaS UI.
@@ -2469,11 +2525,21 @@ async def run(self, seconds, first):
             if first or seconds % 60 == 0:
                 for sid in list(self.systems.keys()):
                     if self.current_mode.get(sid) != SIGENERGY_MODE_VPP:
-                        self.log(
-                            "Warn: SigenergyAPI: System {} is not in VPP mode ({}) — controls skipped until onboard is approved".format(
-                                sid, SIGENERGY_MODE_NAMES.get(self.current_mode.get(sid, -1), "Unknown")
+                        current = self.current_mode.get(sid, -1)
+                        if current in SIGENERGY_THIRD_PARTY_MODES:
+                            # Nothing to approve — another controller holds the system and the
+                            # reclaim check will take it back within a minute.
+                            self.log(
+                                "Warn: SigenergyAPI: System {} is held by another controller ({}) — controls skipped until VPP mode is reclaimed".format(
+                                    sid, SIGENERGY_MODE_NAMES.get(current, "Unknown")
+                                )
+                            )
+                        else:
+                            self.log(
+                                "Warn: SigenergyAPI: System {} is not in VPP mode ({}) — controls skipped until onboard is approved".format(
+                                    sid, SIGENERGY_MODE_NAMES.get(current, "Unknown")
+                                )
                             )
-                        )
                         continue
                     await self.apply_controls(sid)
         else:

apps/predbat/tests/test_sigenergy.py

@@ -22,7 +22,9 @@
     SIGENERGY_CODE_IN_OTHER_VPP,
     SIGENERGY_CODE_SYSTEM_PENDING_REVIEW,
     SIGENERGY_MODE_MSC,
+    SIGENERGY_MODE_NBI,
     SIGENERGY_MODE_VPP,
+    SIGENERGY_VPP_RECLAIM_INTERVAL,
     SIGENERGY_OPTIONS_TIME,
     _safe_float,
     _safe_int,
@@ -2466,6 +2468,118 @@ def test_sigenergy_run_pending_publishes_before_early_exit(my_predbat):
     return failed
 
 
+def _make_contended_api(sid, mode):
+    """Build a MockSigenergyAPI whose system sits in the given operating mode.
+
+    Args:
+        sid: System ID to register.
+        mode: Operating mode integer to report as the system's current mode.
+
+    Returns:
+        A MockSigenergyAPI with run()'s async helpers stubbed out.
+    """
+    api = MockSigenergyAPI()
+    api.systems = {sid: {"deviceList": []}}
+    api.current_mode = {sid: mode}
+    api.system_id_filter = {sid}
+    task = MagicMock()
+    task.done = MagicMock(return_value=False)
+    api._mqtt_task = task
+    api.set_operating_mode = AsyncMock(return_value=True)
+    api.fetch_inverter_realtime = AsyncMock(return_value=True)
+    api.fetch_daily_summary = AsyncMock()
+    api.fetch_history_totals = AsyncMock()
+    api.publish_system_entities = AsyncMock()
+    api.apply_controls = AsyncMock()
+    return api
+
+
+def test_sigenergy_reclaims_vpp_from_third_party_controller(my_predbat):
+    """A system taken into NBI by another controller is reclaimed into VPP, and says so."""
+    failed = False
+    sid = "SIG001"
+
+    api = _make_contended_api(sid, SIGENERGY_MODE_NBI)
+    run_async(api._manage_vpp_registration(sid, is_readonly=False))
+
+    api.set_operating_mode.assert_awaited_once_with(sid, SIGENERGY_MODE_VPP)
+    assert api.contended_by[sid] == "Northbound Integration", "records which controller displaced Predbat"
+
+    # The log must name the displacement rather than implying an onboarding problem.
+    reclaim_logs = [m for m in api.log_messages if "reclaiming VPP mode" in m]
+    assert len(reclaim_logs) == 1, "reclaim is logged once, got {}".format(api.log_messages)
+    assert "Northbound Integration" in reclaim_logs[0], "log names the displacing controller"
+    assert "onboard" not in reclaim_logs[0].lower(), "reclaim log must not blame onboarding"
+
+    return failed
+
+
+def test_sigenergy_contention_does_not_report_pending_approval(my_predbat):
+    """Contention must not surface as pending_approval — that shows a false 'approve in app' banner."""
+    failed = False
+    sid = "SIG001"
+    sensor_key = "sensor.predbat_sigenergy_sig001_onboard_status"
+
+    # Held by another controller → still active (it IS onboarded), contention in attributes.
+    api = _make_contended_api(sid, SIGENERGY_MODE_NBI)
+    api._manage_vpp_registration = AsyncMock(return_value=False)
+    api.contended_by[sid] = "Northbound Integration"
+    run_async(api.run(seconds=300, first=False))
+    assert api.onboard_status[sid] == "active", "contended system stays active, not pending_approval"
+    assert api.dashboard_items[sensor_key]["attributes"]["contended_by"] == "Northbound Integration"
+
+    # A genuinely un-onboarded system (MSC) still reports pending_approval.
+    api_msc = _make_contended_api(sid, SIGENERGY_MODE_MSC)
+    api_msc._manage_vpp_registration = AsyncMock(return_value=False)
+    run_async(api_msc.run(seconds=300, first=False))
+    assert api_msc.onboard_status[sid] == "pending_approval", "MSC still means pending approval"
+
+    # Once VPP is regained the contention marker is cleared.
+    api_back = _make_contended_api(sid, SIGENERGY_MODE_VPP)
+    api_back._manage_vpp_registration = AsyncMock(return_value=True)
+    api_back.contended_by[sid] = "Northbound Integration"
+    run_async(api_back.run(seconds=300, first=False))
+    assert api_back.onboard_status[sid] == "active"
+    assert api_back.dashboard_items[sensor_key]["attributes"]["contended_by"] is None, "contention cleared once back in VPP"
+
+    return failed
+
+
+def test_sigenergy_reclaim_runs_on_the_minute(my_predbat):
+    """The reclaim check runs every minute, so a displaced system is not left contended for a full poll."""
+    failed = False
+    sid = "SIG001"
+
+    assert SIGENERGY_VPP_RECLAIM_INTERVAL == 60, "reclaim cadence is one minute"
+
+    api = _make_contended_api(sid, SIGENERGY_MODE_NBI)
+    api._manage_vpp_registration = AsyncMock(return_value=False)
+
+    # A 60s tick that is NOT a 300s poll tick must still run the reclaim check.
+    run_async(api.run(seconds=60, first=False))
+    api._manage_vpp_registration.assert_awaited_once()
+
+    return failed
+
+
+def test_sigenergy_controls_skipped_message_distinguishes_contention(my_predbat):
+    """Skipping controls because another controller holds the system must not blame onboarding."""
+    failed = False
+    sid = "SIG001"
+
+    api = _make_contended_api(sid, SIGENERGY_MODE_NBI)
+    api._manage_vpp_registration = AsyncMock(return_value=False)
+    run_async(api.run(seconds=60, first=False))
+
+    skip_logs = [m for m in api.log_messages if "controls skipped" in m]
+    assert skip_logs, "controls-skipped message is logged"
+    assert any("held by another controller" in m for m in skip_logs), "message names contention, got {}".format(skip_logs)
+    assert not any("onboard is approved" in m for m in skip_logs), "must not tell the user to approve onboarding"
+    api.apply_controls.assert_not_awaited()
+
+    return failed
+
+
 def run_sigenergy_tests(my_predbat):
     """Run all Sigenergy API unit tests.
 
@@ -2542,6 +2656,10 @@ def run_sigenergy_tests(my_predbat):
         ("publish_onboard_status_sensors", test_sigenergy_publish_onboard_status_sensors),
         ("run_derives_onboard_status", test_sigenergy_run_derives_onboard_status),
         ("run_pending_publishes_before_early_exit", test_sigenergy_run_pending_publishes_before_early_exit),
+        ("reclaims_vpp_from_third_party_controller", test_sigenergy_reclaims_vpp_from_third_party_controller),
+        ("contention_does_not_report_pending_approval", test_sigenergy_contention_does_not_report_pending_approval),
+        ("reclaim_runs_on_the_minute", test_sigenergy_reclaim_runs_on_the_minute),
+        ("controls_skipped_message_distinguishes_contention", test_sigenergy_controls_skipped_message_distinguishes_contention),
     ]
 
     for name, fn in tests: