Skip to content

Commit 0405c68

Browse files
CIclaude
andcommitted
fix(gateway): retry a failed discovery save and publish newly bound inverters at once
Review feedback on #5475: - Storage.save() reports a failed write by returning False, which was ignored after the dirty flag had been cleared, so one failed write lost the record. The save now stays pending and is retried after five minutes. - An inverter that is only published once bound (one the hub does not flag primary, or a PV-only one) had no entities on the status that bound it, because entities were injected before auto-config ran. They are now injected again after a successful auto-config, before the API is declared started. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 122336d commit 0405c68

2 files changed

Lines changed: 90 additions & 4 deletions

File tree

‎apps/predbat/gateway.py‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ def _serial_suffix(serial):
108108
GATEWAY_COORDINATOR_TYPES = (pb.INVERTER_TYPE_GIVENERGY_EMS, pb.INVERTER_TYPE_GIVENERGY_GATEWAY)
109109
# Where the discovery record of each inverter in gateway_inverter_serial is kept (see _update_discovery)
110110
GATEWAY_STORAGE_MODULE = "gateway"
111+
# How long a failed save of the discovery records waits before it is tried again
112+
GATEWAY_DISCOVERY_SAVE_RETRY_SECONDS = 5 * 60
111113

112114
# Entity attribute table — keyed by the semantic suffix used in dashboard_item calls
113115
GATEWAY_ATTRIBUTE_TABLE = {
@@ -233,6 +235,7 @@ class GatewayMQTT(ComponentBase):
233235
_unavailable_inverters = ((), ())
234236
_discovery = {} # replaced, never changed in place, so this default is not shared state
235237
_discovery_dirty = False
238+
_discovery_save_retry_at = 0
236239
_pv_only_serials = ()
237240
_inverter_slot_serials = ()
238241
gateway_device_id = None
@@ -342,6 +345,7 @@ def initialize(self, gateway_device_id=None, mqtt_host=None, mqtt_port=8883, mqt
342345
self._pv_only_serials = () # listed inverters with no battery: counted for PV, never written to
343346
self._discovery = {} # upper-case serial -> what the hub last reported it to be, for gateway_inverter_serial
344347
self._discovery_dirty = False
348+
self._discovery_save_retry_at = 0
345349
self._unavailable_inverters = ((), ())
346350
self._configured_ev_chargers = frozenset() # EV charge point ids registered at the last auto-config
347351
self._last_published_plan = None
@@ -860,6 +864,11 @@ def _process_telemetry(self, data):
860864

861865
if self._needs_reconfigure(status):
862866
self.automatic_config()
867+
if self._auto_configured:
868+
# The injection above ran before this status's inverters were bound, so one that
869+
# is only published once bound has no entities yet. Publish again before the API
870+
# is declared started below.
871+
self._inject_entities(status)
863872
self._check_configured_inverters(status)
864873

865874
# Declare the API started only once auto-config has wired up the inverter args.
@@ -1473,15 +1482,30 @@ async def _load_discovery(self):
14731482
self.log(f"Info: GatewayMQTT: loaded stored discovery for inverter(s) {sorted(loaded)}")
14741483

14751484
async def _save_discovery(self):
1476-
"""Save the discovery records when they have changed."""
1485+
"""Save the discovery records when they have changed.
1486+
1487+
A save that fails stays pending and is tried again after
1488+
GATEWAY_DISCOVERY_SAVE_RETRY_SECONDS: without the stored record a configured
1489+
inverter the hub cannot read would be left out after the next restart.
1490+
"""
14771491
storage = self.storage
1478-
if not self._discovery_dirty or storage is None:
1492+
if not self._discovery_dirty or storage is None or _monotonic() < self._discovery_save_retry_at:
14791493
return
1494+
# Cleared before the save so a record that changes while it is being written is saved again
14801495
self._discovery_dirty = False
14811496
try:
1482-
await storage.save(GATEWAY_STORAGE_MODULE, self._discovery_filename(), {"inverters": self._discovery}, format="json")
1497+
saved = await storage.save(GATEWAY_STORAGE_MODULE, self._discovery_filename(), {"inverters": self._discovery}, format="json")
14831498
except Exception as e:
14841499
self.log(f"Warn: GatewayMQTT: failed to save inverter discovery: {e}")
1500+
saved = False
1501+
else:
1502+
if not saved:
1503+
self.log("Warn: GatewayMQTT: failed to save inverter discovery, will retry")
1504+
if saved:
1505+
self._discovery_save_retry_at = 0
1506+
else:
1507+
self._discovery_dirty = True
1508+
self._discovery_save_retry_at = _monotonic() + GATEWAY_DISCOVERY_SAVE_RETRY_SECONDS
14851509

14861510
@staticmethod
14871511
def _stored_unit(record):
@@ -1866,7 +1890,6 @@ def automatic_config(self):
18661890
# Register the gateway's OCPP EV charger as a PredBat car (opt-in). Done after the
18671891
# inverter config so a failure here never blocks battery control.
18681892
self._register_ev_car(status)
1869-
self._inject_unreported_inverters(status)
18701893

18711894
self._auto_configured = True
18721895
self._configured_inverter_serials = self._usable_serials(status) | frozenset(self._inverter_slot_serials)

‎apps/predbat/tests/test_gateway.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3720,6 +3720,69 @@ def test_discovery_is_saved_and_not_overwritten_while_an_inverter_is_offline(sel
37203720
restarted = self._listed_gateway(["CE2223G800", "CE2225G400"], stored=gw._saved[("gateway", "discovery_pbgw_two_ac")]["inverters"])
37213721
assert sorted(restarted._discovery) == ["CE2223G800", "CE2225G400"]
37223722

3723+
def test_failed_discovery_save_is_retried_after_a_wait(self):
3724+
"""A save the storage refuses is kept pending and tried again after a wait, not on every status message."""
3725+
import gateway
3726+
3727+
gw = self._listed_gateway(["CE2223G800", "CE2225G400"])
3728+
storage = gw.base.components.get_component.return_value
3729+
real_save = storage.save
3730+
attempts = []
3731+
3732+
async def failing_save(module, filename, data, format="yaml", expiry=None, indent=None):
3733+
attempts.append(1)
3734+
return False
3735+
3736+
now = [1000.0]
3737+
real_monotonic = gateway._monotonic
3738+
gateway._monotonic = lambda: now[0]
3739+
try:
3740+
storage.save = failing_save
3741+
gw._process_telemetry(self._two_ac_status("ok").SerializeToString())
3742+
asyncio.run(gw._save_discovery())
3743+
assert attempts == [1]
3744+
assert gw._discovery_dirty is True
3745+
assert gw._saved == {}
3746+
assert self._logged(gw, "Warn", "failed to save inverter discovery")
3747+
3748+
# Still failing, but inside the wait: not tried again
3749+
asyncio.run(gw._save_discovery())
3750+
assert attempts == [1]
3751+
3752+
storage.save = real_save
3753+
now[0] += gateway.GATEWAY_DISCOVERY_SAVE_RETRY_SECONDS
3754+
asyncio.run(gw._save_discovery())
3755+
assert gw._discovery_dirty is False
3756+
assert sorted(gw._saved[("gateway", "discovery_pbgw_two_ac")]["inverters"]) == ["CE2223G800", "CE2225G400"]
3757+
finally:
3758+
gateway._monotonic = real_monotonic
3759+
3760+
def test_newly_bound_inverter_has_its_entities_on_the_same_status(self):
3761+
"""An inverter auto-config has just bound is published before the API is declared started.
3762+
3763+
One the hub does not flag primary is skipped by the injection that runs ahead of
3764+
auto-config, so without a second pass its soc and size entities would not exist
3765+
until the next status message.
3766+
"""
3767+
gw = self._listed_gateway(["CE2223G800", "CE2225G400"])
3768+
3769+
gw._process_telemetry(self._two_ac_status("not_primary").SerializeToString())
3770+
3771+
assert gw.api_started is True
3772+
assert gw._args["num_inverters"] == 2
3773+
assert gw._states["sensor.predbat_gateway_25g400_soc"] == 50
3774+
assert gw._states["sensor.predbat_gateway_25g400_battery_capacity"] == 9.5
3775+
3776+
# The same for a PV-only inverter the hub does not flag primary
3777+
gw = self._listed_gateway(["FD0000P001", "CH0000B002"])
3778+
status = self._pv_and_battery_status(pv_power=900)
3779+
status.inverters[0].primary = False
3780+
3781+
gw._process_telemetry(status.SerializeToString())
3782+
3783+
assert gw._pv_only_serials == ("FD0000P001",)
3784+
assert gw._states["sensor.predbat_gateway_00p001_pv_power"] == 900
3785+
37233786
def test_no_serial_list_stores_nothing(self):
37243787
"""With no serial list nothing is recorded or saved, and auto-config uses what it finds."""
37253788
gw = self._listed_gateway([])

0 commit comments

Comments
 (0)