Skip to content

Commit fad7af0

Browse files
CIclaude
andcommitted
fix(gateway): track PV by serial and handle a status that names no inverter
The integrated PV tracks were a positional list built when integration began. When auto-config re-ran and changed the slots (a PV-only inverter gaining a battery, or an inverter becoming usable later) the tracks were paired with the wrong counters or dropped. They are now keyed by serial, for slot and PV-only inverters alike. With gateway_inverter_serial set and a discovery record held, a status with an empty inverter list is no longer ignored: the stored inverters are bound at start-up, and once running their powers are zeroed and the unread error is raised. It does not count towards the hub being alive. Without a list or a record it is ignored as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 75af61d commit fad7af0

2 files changed

Lines changed: 146 additions & 18 deletions

File tree

‎apps/predbat/gateway.py‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -858,11 +858,15 @@ def _process_telemetry(self, data):
858858
self._debug_dump("RX telemetry", status, raw=data)
859859

860860
if len(status.inverters) == 0:
861-
return
862-
861+
if not (self.gateway_inverter_serial and self._discovery):
862+
return
863+
# With a serial list the discovered inverters are the site whatever the hub reports,
864+
# so a status naming none of them is every one of them unread, not nothing to do
865+
# (#5471). It is not fresh data though, so it does not count towards being alive.
866+
else:
867+
self._last_telemetry_time = time.time()
868+
self.update_success_timestamp()
863869
self._last_status = status
864-
self._last_telemetry_time = time.time()
865-
self.update_success_timestamp()
866870

867871
self._update_discovery(status)
868872
self._update_integrated_energy(status)
@@ -1127,10 +1131,12 @@ def _update_integrated_energy(self, status):
11271131
"charge": sum(max(inv.battery.power_w, 0) for inv in inverters),
11281132
"discharge": sum(max(-inv.battery.power_w, 0) for inv in inverters),
11291133
}
1130-
pv_wh = [inv.energy.pv_today_wh for inv in inverters]
1134+
# By serial and not by slot: auto-config can re-run and change the slots, and a counter
1135+
# must go on being compared with its own last value
1136+
pv_wh = {inv.serial: inv.energy.pv_today_wh for inv in inverters}
11311137
# A PV-only inverter's generation is part of the load too. It has no slot, so it is not
11321138
# needed for the site to be complete: one that is away simply adds nothing meanwhile.
1133-
pv_only_wh = {serial: by_serial[serial].energy.pv_today_wh for serial in self._pv_only_serials if serial in by_serial and by_serial[serial].energy.ByteSize() > 0}
1139+
pv_wh.update({serial: by_serial[serial].energy.pv_today_wh for serial in self._pv_only_serials if serial in by_serial and by_serial[serial].energy.ByteSize() > 0})
11341140

11351141
state = self._integrated_energy
11361142
if state is None:
@@ -1151,20 +1157,19 @@ def _update_integrated_energy(self, status):
11511157
today_share = min(max((now - midnight) / (now - previous), 0.0), 1.0)
11521158

11531159
if previous is None:
1154-
state["pv"] = [{"base": wh, "high": None, "high_time": 0} for wh in pv_wh]
1155-
state["pv_only"] = {serial: {"base": wh, "high": None, "high_time": 0} for serial, wh in pv_only_wh.items()}
1160+
state["pv"] = {serial: {"base": wh, "high": None, "high_time": 0} for serial, wh in pv_wh.items()}
11561161
else:
11571162
elapsed = now - previous
11581163
hours = elapsed / 3600.0
11591164
# Per inverter, so one counter dropping a sample does not hide the others' generation
11601165
pv_rise_limit_wh = GATEWAY_INTEGRATE_MAX_PV_W * hours + GATEWAY_INTEGRATE_PV_STEP_WH
1161-
pv_rise_wh = sum(self._pv_counter_rise(track, wh, now, pv_rise_limit_wh) for track, wh in zip(state["pv"], pv_wh))
1162-
pv_only_tracks = state.setdefault("pv_only", {})
1163-
for serial, wh in pv_only_wh.items():
1164-
if serial in pv_only_tracks:
1165-
pv_rise_wh += self._pv_counter_rise(pv_only_tracks[serial], wh, now, pv_rise_limit_wh)
1166+
pv_rise_wh = 0.0
1167+
for serial, wh in pv_wh.items():
1168+
if serial in state["pv"]:
1169+
pv_rise_wh += self._pv_counter_rise(state["pv"][serial], wh, now, pv_rise_limit_wh)
11661170
else:
1167-
pv_only_tracks[serial] = {"base": wh, "high": None, "high_time": 0}
1171+
# First seen since the integration began: counted from here on
1172+
state["pv"][serial] = {"base": wh, "high": None, "high_time": 0}
11681173
if elapsed <= GATEWAY_INTEGRATE_MAX_GAP_SECONDS:
11691174
energy = {name: (state["power"][name] + power[name]) / 2.0 * hours * today_share for name in power}
11701175
for name in energy:
@@ -1796,10 +1801,6 @@ def automatic_config(self):
17961801
status = self._last_status
17971802
all_inverters = list(status.inverters)
17981803

1799-
if not all_inverters:
1800-
self.log("Error: GatewayMQTT: no inverters in gateway status")
1801-
return
1802-
18031804
# With gateway_inverter_serial set, a listed inverter the hub has reported before is
18041805
# part of the site even when this status leaves it out (#5471). The records are brought
18051806
# up to date here as well as in _process_telemetry(), for a caller that comes straight here.
@@ -1809,6 +1810,10 @@ def automatic_config(self):
18091810
if serial_set:
18101811
all_inverters += [self._stored_unit(record) for key, record in sorted(self._discovery.items()) if key not in reported]
18111812

1813+
if not all_inverters:
1814+
self.log("Error: GatewayMQTT: no inverters in gateway status")
1815+
return
1816+
18121817
# Classify discovered units and route control per GivTCP / GE-Cloud. Neither a
18131818
# GivEnergy Gateway (INVERTER_TYPE_GIVENERGY_GATEWAY) nor a Plant EMS
18141819
# (INVERTER_TYPE_GIVENERGY_EMS) is itself a battery inverter — they are

‎apps/predbat/tests/test_gateway.py‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3684,6 +3684,72 @@ def test_stored_inverter_unread_at_startup_is_still_bound(self):
36843684
assert self._logged(gw, "Error", "CE2225G400", "not being read"), second
36853685
assert gw.non_fatal_error_occurred.call_count == 1, second
36863686

3687+
def _empty_status(self):
3688+
"""A status in which the hub names no inverter at all."""
3689+
status = pb.GatewayStatus()
3690+
status.device_id = "pbgw_two_ac"
3691+
status.firmware = "1.0.34"
3692+
status.schema_version = 1
3693+
return status
3694+
3695+
def test_stored_inverter_is_bound_when_the_hub_reports_no_inverters_at_startup(self):
3696+
"""A single listed inverter with a stored record keeps its slot when the first status names no inverter."""
3697+
gw = self._listed_gateway(["CE2225G400"], stored=self._STORED_400)
3698+
3699+
gw._process_telemetry(self._empty_status().SerializeToString())
3700+
3701+
assert gw._auto_configured is True
3702+
assert gw._args["num_inverters"] == 1
3703+
assert gw._inverter_slot_serials == ["CE2225G400"]
3704+
assert gw._states["sensor.predbat_gateway_25g400_soc"] == 0
3705+
assert gw._states["sensor.predbat_gateway_25g400_battery_capacity"] == 9.5
3706+
assert self._logged(gw, "Error", "CE2225G400", "not being read")
3707+
assert gw.non_fatal_error_occurred.call_count == 1
3708+
# Nothing was read, so the status does not count towards the hub being alive
3709+
gw.update_success_timestamp.assert_not_called()
3710+
3711+
def test_status_with_no_inverters_leaves_configured_inverters_unread(self):
3712+
"""Once configured, a status naming no inverter zeroes the powers, keeps the rest and is an error."""
3713+
gw = self._listed_gateway(["CE2223G800", "CE2225G400"])
3714+
healthy = self._two_ac_status("ok")
3715+
for inv in healthy.inverters:
3716+
inv.battery.power_w = -3000
3717+
gw._process_telemetry(healthy.SerializeToString())
3718+
assert gw._states["sensor.predbat_gateway_23g800_battery_power"] == 3000
3719+
assert self._errors(gw) == []
3720+
3721+
gw._process_telemetry(self._empty_status().SerializeToString())
3722+
3723+
assert gw._args["num_inverters"] == 2
3724+
for suffix in ("23g800", "25g400"):
3725+
assert gw._states[f"sensor.predbat_gateway_{suffix}_battery_power"] == 0, suffix
3726+
assert gw._states[f"sensor.predbat_gateway_{suffix}_soc"] == 50, suffix
3727+
assert self._logged(gw, "Error", "CE2223G800", "CE2225G400", "not being read")
3728+
assert gw.non_fatal_error_occurred.call_count == 1
3729+
3730+
gw._process_telemetry(healthy.SerializeToString())
3731+
assert gw._states["sensor.predbat_gateway_23g800_battery_power"] == 3000
3732+
assert self._logged(gw, "Info", "every configured inverter is being read by the hub again")
3733+
3734+
def test_status_with_no_inverters_is_ignored_without_a_list_or_a_record(self):
3735+
"""With no serial list, or no inverter discovered yet, a status naming no inverter is ignored as before."""
3736+
for listed, stored in (([], None), (["CE2225G400"], None)):
3737+
gw = self._listed_gateway(listed, stored=stored)
3738+
3739+
gw._process_telemetry(self._empty_status().SerializeToString())
3740+
3741+
assert gw._last_status is None, listed
3742+
assert gw._auto_configured is False, listed
3743+
assert gw._args == {}, listed
3744+
3745+
# No list: inverters come and go, so one that goes is not held on to
3746+
gw = self._listed_gateway([])
3747+
gw._process_telemetry(self._two_ac_status("ok").SerializeToString())
3748+
healthy = gw._last_status
3749+
gw._process_telemetry(self._empty_status().SerializeToString())
3750+
assert gw._last_status is healthy
3751+
assert self._errors(gw) == []
3752+
36873753
def test_changed_serial_list_wins_over_stored_discovery(self):
36883754
"""A stored record for a serial no longer in the list is not used: the list is the setting."""
36893755
gw = self._listed_gateway(["CE2223G800"], stored=self._STORED_400)
@@ -6927,6 +6993,63 @@ def status(timestamp, pv_only_wh, with_pv_only=True):
69276993
self._feed(gw, status(self.BASE_TIME + 1800, 3500))
69286994
assert approx_equal(gw._integrated_energy["load_wh"], 500.0)
69296995

6996+
def test_pv_only_inverter_promoted_to_a_slot_keeps_its_generation(self):
6997+
"""Auto-config re-running and changing the slots does not lose or mix up any inverter's generation."""
6998+
gw = self._make_gateway()
6999+
# The PV-only inverter sorts first, so once it has a slot every slot index moves
7000+
gw.gateway_inverter_serial = ["AA0000P001", "CE2223G800"]
7001+
7002+
def status(timestamp, pv_only_wh, has_battery=False):
7003+
built = self._status(timestamp, grid_w=0, battery_w=(0,), pv_today_wh=1000, serials=("CE2223G800",))
7004+
inv = built.inverters.add()
7005+
inv.type = pb.INVERTER_TYPE_GIVENERGY
7006+
inv.serial = "AA0000P001"
7007+
inv.primary = True
7008+
inv.connected = True
7009+
inv.pv.power_w = 1000
7010+
inv.energy.pv_today_wh = pv_only_wh
7011+
if has_battery:
7012+
inv.battery.soc_percent = 40
7013+
inv.battery.capacity_wh = 5000
7014+
return built
7015+
7016+
self._feed(gw, status(self.BASE_TIME, 3000))
7017+
self._feed(gw, status(self.BASE_TIME + 600, 3200))
7018+
assert gw._pv_only_serials == ("AA0000P001",)
7019+
assert approx_equal(gw._integrated_energy["load_wh"], 200.0)
7020+
7021+
# Its battery readings arrive: _process_telemetry integrates, then re-runs auto-config
7022+
self._feed(gw, status(self.BASE_TIME + 1200, 3300, has_battery=True))
7023+
gw.automatic_config()
7024+
assert gw._inverter_slot_serials == ["AA0000P001", "CE2223G800"]
7025+
assert gw._pv_only_serials == ()
7026+
assert approx_equal(gw._integrated_energy["load_wh"], 300.0)
7027+
7028+
self._feed(gw, status(self.BASE_TIME + 1800, 3500, has_battery=True))
7029+
assert approx_equal(gw._integrated_energy["load_wh"], 500.0)
7030+
7031+
def test_inverter_given_a_slot_later_has_its_generation_counted_from_then(self):
7032+
"""An inverter that joins the slots after the integration began is counted from its first status, not left out."""
7033+
gw = self._make_gateway()
7034+
self._feed(gw, self._status(self.BASE_TIME, grid_w=0, battery_w=(0,), pv_today_wh=1000, serials=("CE2225G400",)))
7035+
self._feed(gw, self._status(self.BASE_TIME + 600, grid_w=0, battery_w=(0,), pv_today_wh=1100, serials=("CE2225G400",)))
7036+
assert approx_equal(gw._integrated_energy["load_wh"], 100.0)
7037+
7038+
def both(timestamp, first_wh, second_wh):
7039+
built = self._status(timestamp, grid_w=0, battery_w=(0, 0), pv_today_wh=first_wh)
7040+
built.inverters[1].energy.pv_today_wh = second_wh
7041+
return built
7042+
7043+
# CE2223G800 appears and takes slot 0; its counter already stands at 7000
7044+
gw._last_status = both(self.BASE_TIME + 1200, 7000, 1200)
7045+
gw.automatic_config()
7046+
assert gw._inverter_slot_serials == ["CE2223G800", "CE2225G400"]
7047+
7048+
self._feed(gw, both(self.BASE_TIME + 1200, 7000, 1200))
7049+
assert approx_equal(gw._integrated_energy["load_wh"], 200.0)
7050+
self._feed(gw, both(self.BASE_TIME + 1800, 7300, 1250))
7051+
assert approx_equal(gw._integrated_energy["load_wh"], 550.0)
7052+
69307053
def test_import_and_load_are_integrated_from_power(self):
69317054
"""6 kW of import for 6 minutes is 0.6 kWh; 3 kW of that went into a battery, so load is 0.3 kWh."""
69327055
gw = self._make_gateway()

0 commit comments

Comments
 (0)