Skip to content

Commit 07d54d5

Browse files
mgazzaclaude
andauthored
fix(gateway): write entities for the bound control target, not just primary units (#4360)
On a multi-AIO site automatic_config picks the Gateway/EMS as the control target and binds every PredBat arg to its serial suffix. _inject_entities only ever wrote entities for units flagged primary — and firmware never flags a Gateway or EMS primary — so none of the bound entities were written. They held whatever value they had and silently froze. Observed in production: a site froze for 69 minutes with byte-identical SoC, battery power, grid power, load power and PV power, while PredBat continued to plan and issue control against a stale 76% SoC. The only escaping symptom was the clock-skew warning, because inverter_time is the one bound value whose staleness is self-evident against wall clock — which sends users to fix an inverter clock that is not broken. Inject a unit when it is primary OR when automatic_config bound args to it (tracked in _suffix_to_serial), and write inverter_time under the bound suffix rather than the primary's. Non-primary units that are not the control target stay skipped, preserving the existing guard against double-counted power readings. Adds TestBoundEntitiesAreWritten, which asserts the invariant directly: every entity automatic_config binds must actually be written by _inject_entities. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 61f98d6 commit 07d54d5

2 files changed

Lines changed: 140 additions & 8 deletions

File tree

apps/predbat/gateway.py

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,21 @@
5959
_GATEWAY_OPTIONS_TIME = [(_GATEWAY_BASE_TIME + datetime.timedelta(seconds=m * 60)).strftime("%H:%M:%S") for m in range(0, 24 * 60, 5)]
6060

6161

62+
def _serial_suffix(serial):
63+
"""Return the entity-name suffix for an inverter serial.
64+
65+
Entity IDs use the last six characters of the serial, lower-cased; serials of
66+
six characters or fewer are used whole.
67+
68+
Args:
69+
serial: Inverter serial string.
70+
71+
Returns:
72+
str: The lower-cased suffix used in gateway entity IDs.
73+
"""
74+
return serial[-6:].lower() if len(serial) > 6 else serial.lower()
75+
76+
6277
PLAN_MODE_AUTO = 0
6378
PLAN_MODE_CHARGE = 1
6479
PLAN_MODE_DISCHARGE = 2
@@ -719,6 +734,22 @@ def _process_telemetry(self, data):
719734
if self._needs_reconfigure(status):
720735
self.automatic_config()
721736

737+
def _is_bound_target(self, inv):
738+
"""Whether automatic_config bound PredBat's args to this inverter's suffix.
739+
740+
``_suffix_to_serial`` is populated by ``automatic_config`` with exactly the
741+
control-target suffixes, so it is the authoritative record of which entities
742+
PredBat reads. Empty before the first auto-config, in which case no unit is
743+
bound yet and the primary-only path applies.
744+
745+
Args:
746+
inv: A ``predbat_InverterEntry`` from the gateway status.
747+
748+
Returns:
749+
bool: True if this unit's entities are read by PredBat.
750+
"""
751+
return _serial_suffix(inv.serial) in self._suffix_to_serial
752+
722753
def _inject_entities(self, status):
723754
"""Inject inverter entities into PredBat state cache.
724755
@@ -735,10 +766,14 @@ def _inject_entities(self, status):
735766
app="gateway",
736767
)
737768

738-
# Inverter time from gateway timestamp — use first primary inverter's serial
769+
# Inverter time from gateway timestamp — write it under the suffix PredBat
770+
# actually reads (the control target), not the primary's, or the bound
771+
# inverter_time arg is never updated and silently freezes.
739772
if status.timestamp > 0 and len(status.inverters) > 0:
740-
primary_inv = next((inv for inv in status.inverters if inv.primary), status.inverters[0])
741-
ts_suffix = primary_inv.serial[-6:].lower() if len(primary_inv.serial) > 6 else primary_inv.serial.lower()
773+
ts_inv = next((inv for inv in status.inverters if self._is_bound_target(inv)), None)
774+
if ts_inv is None:
775+
ts_inv = next((inv for inv in status.inverters if inv.primary), status.inverters[0])
776+
ts_suffix = _serial_suffix(ts_inv.serial)
742777
dt = datetime.datetime.fromtimestamp(status.timestamp, tz=self.local_tz)
743778
self.dashboard_item(
744779
f"sensor.{self.prefix}_gateway_{ts_suffix}_inverter_time",
@@ -748,12 +783,15 @@ def _inject_entities(self, status):
748783
)
749784

750785
for inv in status.inverters:
751-
# Skip non-primary inverters — EMS/gateway units report overlapping
752-
# power readings that would cause doubled values on the dashboard.
753-
if not inv.primary:
786+
# Inject primary (battery-bearing) units, plus whichever unit automatic_config
787+
# bound PredBat's args to. On a multi-AIO site the control target is the
788+
# Gateway/EMS, which firmware never flags primary — skipping it left every
789+
# bound arg unwritten and frozen at its last value. Non-primary units that are
790+
# NOT the control target stay skipped: they report overlapping power readings
791+
# that would double up on the dashboard.
792+
if not inv.primary and not self._is_bound_target(inv):
754793
continue
755-
suffix = inv.serial[-6:].lower() if len(inv.serial) > 6 else inv.serial.lower()
756-
self._inject_inverter_entities(inv, suffix)
794+
self._inject_inverter_entities(inv, _serial_suffix(inv.serial))
757795

758796
# EV charger entities (device-level, present only when a charge point is connected)
759797
self._inject_ev_entities(status)

apps/predbat/tests/test_gateway.py

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Tests for GatewayMQTT component.
33
"""
4+
45
import sys
56
import os
67
import math
@@ -291,6 +292,7 @@ def _make_gateway(self):
291292
gw._last_status = None
292293
gw.args = {}
293294
gw.local_tz = pytz.timezone("Europe/London")
295+
gw._suffix_to_serial = {} # set by automatic_config; empty = nothing bound yet
294296
gw._dashboard_calls = {} # entity_id → (state, attributes)
295297

296298
def capture_dashboard(entity_id, state=None, attributes=None, app=None):
@@ -673,6 +675,97 @@ def test_inverter_rate_max_uses_6000_when_both_zero(self):
673675
assert state == 6000
674676

675677

678+
class TestBoundEntitiesAreWritten:
679+
"""Every entity automatic_config() binds must actually be written by _inject_entities().
680+
681+
A bound-but-never-written entity holds whatever value it last had and silently
682+
freezes — surfacing only as PredBat's clock-skew warning once the stale
683+
inverter_time drifts past the threshold.
684+
"""
685+
686+
def _make_gateway(self):
687+
from gateway import GatewayMQTT
688+
from unittest.mock import MagicMock
689+
690+
gw = GatewayMQTT.__new__(GatewayMQTT)
691+
gw.base = MagicMock()
692+
gw.log = MagicMock()
693+
gw.prefix = "predbat"
694+
gw._last_status = None
695+
gw._auto_configured = False
696+
gw._suffix_to_serial = {}
697+
gw.args = {}
698+
gw._args = {}
699+
gw.local_tz = pytz.timezone("Europe/London")
700+
gw.gateway_inverter_serial = []
701+
gw.gateway_evc_automatic = False
702+
gw.gateway_evc_control = False
703+
gw._dashboard_calls = {}
704+
705+
def capture_set_arg(key, value):
706+
gw._args[key] = value
707+
708+
def capture_dashboard(entity_id, state=None, attributes=None, app=None):
709+
gw._dashboard_calls[entity_id] = (state, attributes)
710+
711+
gw.set_arg = capture_set_arg
712+
gw.dashboard_item = capture_dashboard
713+
return gw
714+
715+
def _add_inverter(self, status, serial, primary, inv_type=None):
716+
inv = status.inverters.add()
717+
inv.type = inv_type if inv_type is not None else pb.INVERTER_TYPE_GIVENERGY
718+
inv.serial = serial
719+
inv.primary = primary
720+
inv.connected = True
721+
inv.active = True
722+
inv.battery.soc_percent = 50
723+
inv.battery.capacity_wh = 9500
724+
inv.battery.rate_max_w = 5000
725+
return inv
726+
727+
def _gateway_plus_two_aios(self):
728+
"""Reproduces the field topology: a Gateway coordinating two primary AIOs.
729+
730+
Per GivTCP rules automatic_config picks the Gateway as the control target,
731+
so every arg binds to the Gateway's serial suffix. The Gateway is not
732+
flagged primary — firmware only sets primary on battery inverters.
733+
"""
734+
status = pb.GatewayStatus()
735+
status.device_id = "pbgw_test"
736+
status.firmware = "0.27.0"
737+
status.timestamp = 1741789200
738+
status.schema_version = 1
739+
self._add_inverter(status, "GW2315G357", primary=False, inv_type=pb.INVERTER_TYPE_GIVENERGY_GATEWAY)
740+
self._add_inverter(status, "CH2335G421", primary=True)
741+
self._add_inverter(status, "CH2432G070", primary=True)
742+
return status
743+
744+
def test_bound_inverter_time_entity_is_written(self):
745+
"""The inverter_time entity bound by automatic_config must be written."""
746+
gw = self._make_gateway()
747+
gw._last_status = self._gateway_plus_two_aios()
748+
gw.automatic_config()
749+
750+
bound = gw._args["inverter_time"][0]
751+
752+
gw._inject_entities(gw._last_status)
753+
754+
assert bound in gw._dashboard_calls, "automatic_config bound inverter_time to {} but _inject_entities never wrote it; " "written entities were: {}".format(bound, sorted(gw._dashboard_calls))
755+
756+
def test_bound_soc_entity_is_written(self):
757+
"""The soc_percent entity bound by automatic_config must be written."""
758+
gw = self._make_gateway()
759+
gw._last_status = self._gateway_plus_two_aios()
760+
gw.automatic_config()
761+
762+
bound = gw._args["soc_percent"][0]
763+
764+
gw._inject_entities(gw._last_status)
765+
766+
assert bound in gw._dashboard_calls, "automatic_config bound soc_percent to {} but _inject_entities never wrote it; " "written entities were: {}".format(bound, sorted(gw._dashboard_calls))
767+
768+
676769
class TestDebugLogging:
677770
"""Tests for the gateway_debug verbose telemetry/plan dump helper."""
678771

@@ -4259,6 +4352,7 @@ def run_gateway_tests(my_predbat=None):
42594352
TestInjectEntities,
42604353
TestDebugLogging,
42614354
TestAutomaticConfig,
4355+
TestBoundEntitiesAreWritten,
42624356
TestEvTelemetry,
42634357
TestEvAutoConfig,
42644358
TestEvInitialize,

0 commit comments

Comments
 (0)