From 3419852b908b69ad4a567ca7823b8bef867c9dcc Mon Sep 17 00:00:00 2001 From: Mark Gascoyne Date: Fri, 7 Aug 2026 07:24:28 +0100 Subject: [PATCH] fix(sigenergy): make Predbat's ownership of the inverter deliberate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Sigenergy accepts one controller at a time, and VPP mode (Predbat) and the NorthBound Interface (Axle's VPP dispatch channel) are mutually exclusive. Today Predbat wins that contest by accident: _manage_vpp_registration ran on the 5 minute poll tick and silently switched the system back to VPP, so an Axle dispatch could hold the inverter for up to 5 minutes before being displaced with no indication of what had happened. Observed on a live system on 2026-08-06: mode went to Northbound Integration at 19:35:18 and back to VPP at 19:40:12 — exactly one poll interval — while the log only said "controls skipped until onboard is approved". This keeps Predbat as the owner but makes that a decision rather than a race: - reclaim VPP every minute instead of every 5, so control is never ambiguous for long (set_operating_mode is an MQTT publish and only fires when the mode is wrong, so this costs nothing against the REST rate limit) - log the reclaim explicitly, naming the controller being displaced - stop reporting a contended system as pending_approval. The SaaS UI renders that as an amber "approve this in the Sigenergy app" banner, so every Axle event told the user to go and approve something that needed no approval - expose contended_by on the onboard-status sensor so support can tell contention apart from a real onboarding failure Predbat already ingests Axle sessions as its own export windows, so events still run — under Predbat's plan rather than Axle's dispatch. Co-Authored-By: Claude Opus 5 --- apps/predbat/sigenergy.py | 80 ++++++++++++++++-- apps/predbat/tests/test_sigenergy.py | 118 +++++++++++++++++++++++++++ 2 files changed, 191 insertions(+), 7 deletions(-) diff --git a/apps/predbat/sigenergy.py b/apps/predbat/sigenergy.py index 9522e0204..24d0d5e44 100644 --- a/apps/predbat/sigenergy.py +++ b/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: diff --git a/apps/predbat/tests/test_sigenergy.py b/apps/predbat/tests/test_sigenergy.py index 351012f51..727fabbec 100644 --- a/apps/predbat/tests/test_sigenergy.py +++ b/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: