From 8818396f44ed392519a576defab71ed7b7ecd394 Mon Sep 17 00:00:00 2001 From: CI Date: Thu, 24 Sep 2026 18:56:19 +0100 Subject: [PATCH 1/2] fix(givtcp): fill per-inverter arg slots by REST endpoint index, not discovery order (#5209) An endpoint down at discovery shifted every later inverter's entities down a slot while inverter_limit_charge and other hand-set per-inverter args stayed put, so one physical inverter was driven by two Predbat inverters. Adopting the late endpoint on re-probe appended it, turning the shift into a rotation. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/predbat/givtcp.py | 72 ++++++++------ apps/predbat/tests/test_givtcp_component.py | 102 +++++++++++++++----- 2 files changed, 124 insertions(+), 50 deletions(-) diff --git a/apps/predbat/givtcp.py b/apps/predbat/givtcp.py index 2d6577233..3cb7c9996 100644 --- a/apps/predbat/givtcp.py +++ b/apps/predbat/givtcp.py @@ -515,9 +515,8 @@ async def rediscover(self): if not data: continue rest.inverter.rest_data = data - # Appended, never inserted. The order of self.discovered is Predbat's inverter - # numbering, so moving an existing entry would repoint a running inverter at different - # physical hardware mid-flight. + # The order of self.discovered carries no meaning: automatic_config() fills Predbat + # inverter n from REST endpoint n whatever order the endpoints answered in (#5209). self.discovered.append(n) self.identified.add(n) self.log( @@ -774,27 +773,43 @@ def _configured_num_inverters(self): continue return largest - def _keep_configured_tail(self, key, values, fleet_size): + def _per_endpoint_values(self, key, make, fleet_size): """ - values for the inverters discovered here, followed by whatever is already configured for - any inverter beyond them. - - The discovered inverters take the leading slots; the rest of the fleet is configured some - other way, and replacing the whole list with only this component's entities left those - inverters pointing at nothing (#5029). A scalar value applies to every index, so it is - broadcast into the tail rather than dropped; a configured list shorter than the fleet is - left short rather than padded with a made-up entry - Inverter fills its own dummies in for - the keys that need one, and inventing a type or an entity here is how the inverter that is - not ours ends up mis-driven. + One value per Predbat inverter, where Predbat inverter n is always REST endpoint n. + + A discovered endpoint's slot gets make(n). Every other slot keeps whatever is already + configured for it. Filling the slots positionally from self.discovered instead meant an + endpoint that was down at startup shifted every later inverter's entities down one slot, + while per-inverter settings that are not auto-configured (inverter_limit_charge and the + like) stayed where they were - one physical inverter then got driven by two Predbat + inverters and another by none, and adopting the late endpoint on re-probe turned the shift + into a rotation (#5209). + + Past the last discovered endpoint the rest of the fleet is configured some other way, and + replacing the whole list with only this component's entities left those inverters pointing + at nothing (#5029). A scalar value applies to every index, so it is broadcast rather than + dropped; a configured list shorter than the fleet is left short there rather than padded + with a made-up entry - Inverter fills its own dummies in for the keys that need one, and + inventing a type or an entity for an inverter that is not ours is how it ends up mis-driven. + A slot below the last discovered endpoint cannot be left out without renumbering everything + after it, and it is one of our own endpoints that has not answered yet, so where nothing is + configured for it, it gets make(n) - what it will be once it does. """ - if fleet_size <= len(values): - return values configured = self._configured_value(key) - if configured is None: - return values - if not isinstance(configured, list): + if configured is not None and not isinstance(configured, list): configured = [configured] * fleet_size - return values + configured[len(values) : fleet_size] + last_discovered = max(self.discovered) + values = [] + for n in range(fleet_size): + if n in self.discovered: + values.append(make(n)) + elif configured is not None and n < len(configured): + values.append(configured[n]) + elif n < last_discovered: + values.append(make(n)) + else: + break + return values async def automatic_config(self): """Point Predbat's standard entity-based apps.yaml keys at the entities this component publishes.""" @@ -812,15 +827,18 @@ async def automatic_config(self): return n_discovered = len(discovered) - # The fleet only ever grows here. An inverter this component cannot discover - another - # vendor's, or one configured by hand - is still part of the fleet, so writing the + # Predbat inverter n is REST endpoint n, so the fleet reaches at least the highest endpoint + # that answered. It only ever grows here: an inverter this component cannot discover - + # another vendor's, or one configured by hand - is still part of the fleet, so writing the # discovered count over a larger configured num_inverters dropped it from Predbat entirely # (#5029). - n_inverters = max(n_discovered, self._configured_num_inverters()) + n_covered = max(discovered) + 1 + n_inverters = max(n_covered, self._configured_num_inverters()) self.log("GivTCP: configuring Predbat for {} discovered inverter(s)".format(n_discovered)) if n_inverters > n_discovered: - self.log("Info: GivTCP: {} inverter(s) are configured and {} answered here - leaving inverter {} onwards as configured".format(n_inverters, n_discovered, n_discovered)) - self.set_arg_auto("inverter_type", self._keep_configured_tail("inverter_type", ["GE" for _ in range(n_discovered)], n_inverters)) + not_answered = [n for n in range(n_inverters) if n not in discovered] + self.log("Info: GivTCP: {} inverter(s) are configured and {} answered here - leaving inverter(s) {} as configured".format(n_inverters, n_discovered, ", ".join(str(n) for n in not_answered))) + self.set_arg_auto("inverter_type", self._per_endpoint_values("inverter_type", lambda n: "GE", n_inverters)) self.set_arg_auto("num_inverters", n_inverters) keys = list(GIVTCP_AUTO_CONFIG_KEYS) @@ -883,7 +901,7 @@ async def automatic_config(self): domain, _, _ = GIVTCP_CONTROLS.get(key, (None, None, None)) domain = domain or "sensor" if key == "battery_scaling": - self.set_arg_auto(key, self._keep_configured_tail(key, [self._entity_id("sensor", n, "battery_dod_soh") for n in discovered], n_inverters)) + self.set_arg_auto(key, self._per_endpoint_values(key, lambda n: self._entity_id("sensor", n, "battery_dod_soh"), n_inverters)) continue # Indexed by REST endpoint, not by position: _parse_entity feeds self.rest[n] on every # write, so renumbering would route the surviving inverter's writes at a dead client. @@ -891,7 +909,7 @@ async def automatic_config(self): # overwrite=False on the history-bearing energy totals leaves a sensor the user named # themselves in place, so its recorded history survives - set_arg_auto still fills the # key in when they named nothing. - self.set_arg_auto(key, self._keep_configured_tail(key, [self._entity_id(domain, n, key) for n in discovered], n_inverters), overwrite=key not in GIVTCP_AUTO_CONFIG_USER_WINS_KEYS) + self.set_arg_auto(key, self._per_endpoint_values(key, lambda n, domain=domain, key=key: self._entity_id(domain, n, key), n_inverters), overwrite=key not in GIVTCP_AUTO_CONFIG_USER_WINS_KEYS) def _discovery_descriptor(self, n, name, domain, access, attrs, max_battery_rate): """ diff --git a/apps/predbat/tests/test_givtcp_component.py b/apps/predbat/tests/test_givtcp_component.py index e508a3a07..9435d4c26 100644 --- a/apps/predbat/tests/test_givtcp_component.py +++ b/apps/predbat/tests/test_givtcp_component.py @@ -735,25 +735,27 @@ def test_automatic_config_counts_discovered_inverters_not_configured_urls(my_pre return 0 -def test_automatic_config_maps_predbat_inverter_to_the_live_endpoint(my_predbat=None): +def test_automatic_config_keeps_the_live_endpoint_in_its_own_slot(my_predbat=None): """ - When an earlier endpoint is dead, the surviving one still drives Predbat inverter 0. + When an earlier endpoint is dead, the surviving one still drives its own Predbat inverter. - Entity ids stay pinned to the REST endpoint index because _parse_entity feeds self.rest[n] on - every write - renumbering them would route inverter 0's writes at the dead client. + A dead endpoint at startup cannot be told apart from a placeholder URL, and moving the live + endpoint down into slot 0 leaves every per-inverter setting that is not auto-configured + (inverter_limit_charge and the like) applying to the wrong inverter (#5209). Entity ids stay + pinned to the REST endpoint index too, because _parse_entity feeds self.rest[n] on every write. """ base, component = _make_component(rest_urls=["http://dead:6345", "http://givtcp1:6345"]) component.rest[0].read_data = MagicMock(return_value=None) component.rest[1].read_data = MagicMock(return_value=_rest_data_blob()) run_async(component.run(seconds=0, first=True)) - assert base.args["num_inverters"] == 1, f"Expected num_inverters 1, got {base.args.get('num_inverters')}" - assert base.args["charge_rate"] == ["number.predbat_givtcp_1_charge_rate"], f"Expected the live endpoint's own index, got {base.args.get('charge_rate')}" + assert base.args["num_inverters"] == 2, f"Expected num_inverters 2, got {base.args.get('num_inverters')}" + assert base.args["charge_rate"] == ["number.predbat_givtcp_0_charge_rate", "number.predbat_givtcp_1_charge_rate"], f"Expected each slot to carry its own endpoint's index, got {base.args.get('charge_rate')}" - # and that entity must still route a write back to the live client - n, control = component._parse_entity(base.args["charge_rate"][0]) - assert component.rest[n] is component.rest[1], "Predbat inverter 0's entity must address the live REST client" - print("PASS: a dead leading endpoint leaves the live one driving Predbat inverter 0") + # and inverter 1's entity must route a write to the live client + n, control = component._parse_entity(base.args["charge_rate"][1]) + assert component.rest[n] is component.rest[1], "Predbat inverter 1's entity must address the live REST client" + print("PASS: a dead leading endpoint leaves the live one driving its own Predbat inverter") return 0 @@ -836,13 +838,14 @@ def test_rediscovery_picks_up_an_inverter_that_was_down_at_startup(my_predbat=No return 0 -def test_rediscovery_appends_so_running_inverters_keep_their_identity(my_predbat=None): +def test_rediscovery_keeps_inverter_identity_by_endpoint(my_predbat=None): """ - A late arrival is appended, never inserted. + Predbat inverter n is REST endpoint n, however late endpoint n answers. - self.discovered's order *is* Predbat's inverter numbering. Inserting endpoint 0 ahead of the - endpoint already running as inverter 0 would silently repoint inverter 0 at different physical - hardware - its SoC, rates and charge windows would start following the wrong battery. + The order the endpoints answered in used to be Predbat's inverter numbering, so an endpoint + down at startup shifted endpoint 1 into slot 0, and adopting endpoint 0 later appended it as + inverter 1 - a permanent rotation. Every per-inverter setting that is not auto-configured + (inverter_limit_charge and the like) then applied to the wrong physical inverter (#5209). """ base, component = _make_component(rest_urls=["http://dead:6345", "http://givtcp1:6345"]) component.rest[0].read_data = MagicMock(return_value=None) @@ -850,19 +853,71 @@ def test_rediscovery_appends_so_running_inverters_keep_their_identity(my_predbat run_async(component.run(seconds=0, first=True)) assert component.discovered == [1], f"Expected only endpoint 1 discovered, got {component.discovered}" - first_inverter_entity = base.args["charge_rate"][0] + # Endpoint 1 stays in slot 1; slot 0 is endpoint 0's own, waiting for it to answer + assert base.args["charge_rate"] == ["number.predbat_givtcp_0_charge_rate", "number.predbat_givtcp_1_charge_rate"], f"Expected slot n to be endpoint n at startup, got {base.args.get('charge_rate')}" + assert base.args["num_inverters"] == 2 component.rest[0].read_data = MagicMock(return_value=_rest_data_blob()) run_async(component.run(seconds=GIVTCP_REDISCOVER_SECONDS, first=False)) - assert component.discovered == [1, 0], f"Expected the late endpoint appended, got {component.discovered}" - assert base.args["charge_rate"][0] == first_inverter_entity, "Predbat inverter 0 must keep addressing the same physical inverter" - assert base.args["charge_rate"] == ["number.predbat_givtcp_1_charge_rate", "number.predbat_givtcp_0_charge_rate"] + assert sorted(component.discovered) == [0, 1], f"Expected both endpoints discovered, got {component.discovered}" + assert base.args["charge_rate"] == ["number.predbat_givtcp_0_charge_rate", "number.predbat_givtcp_1_charge_rate"], f"Expected slot n to stay endpoint n after re-probe, got {base.args.get('charge_rate')}" assert base.args["num_inverters"] == 2 - print("PASS: a late inverter is appended, leaving running inverter identities untouched") + print("PASS: a late inverter takes its own slot, leaving running inverter identities untouched") return 0 +def test_leading_endpoint_down_does_not_shift_the_fleet(my_predbat=None): + """ + GH#5209: three GivTCP inverters, endpoint 0 down while Predbat starts. + + Positional slot filling put endpoints 1 and 2 into slots 0 and 1 and kept only slot 2 from + apps.yaml, so Predbat inverter 1 wrote its 2600W limit to the AC3 on endpoint 2 while inverter + 2 wrote 3000W back to it every cycle. Slot n must be endpoint n: the down endpoint's slot keeps + what apps.yaml configured for it, and a claimed key the user never configured still comes out + one entry per inverter rather than short (the reporter's "battery_calibration expected 3"). + """ + failed = False + base, component = _make_component(rest_urls=["http://givtcp0:6345", "http://givtcp1:6345", "http://givtcp2:6345"]) + apps_yaml = { + "num_inverters": 3, + "charge_rate": ["number.inv0_charge_rate", "number.inv1_charge_rate", "number.inv2_charge_rate"], + } + base.args_from_apps_yaml = dict(apps_yaml) + base.apps_yaml_override_warned = set() + base.args.update(apps_yaml) + component.rest[0].read_data = MagicMock(return_value=None) + component.rest[1].read_data = MagicMock(return_value=_rest_data_blob()) + component.rest[2].read_data = MagicMock(return_value=_rest_data_blob()) + + run_async(component.run(seconds=0, first=True)) + + if base.args.get("num_inverters") != 3: + print("ERROR: expected num_inverters 3, got {}".format(base.args.get("num_inverters"))) + failed = True + if base.args.get("charge_rate") != ["number.inv0_charge_rate", "number.predbat_givtcp_1_charge_rate", "number.predbat_givtcp_2_charge_rate"]: + print("ERROR: expected slot n to be endpoint n with slot 0 left as configured, got {}".format(base.args.get("charge_rate"))) + failed = True + if base.args.get("battery_calibration") != ["sensor.predbat_givtcp_0_battery_calibration", "sensor.predbat_givtcp_1_battery_calibration", "sensor.predbat_givtcp_2_battery_calibration"]: + print("ERROR: expected an unconfigured claimed key to cover all three slots by endpoint, got {}".format(base.args.get("battery_calibration"))) + failed = True + if base.args.get("inverter_type") != ["GE", "GE", "GE"]: + print("ERROR: expected inverter_type ['GE', 'GE', 'GE'], got {}".format(base.args.get("inverter_type"))) + failed = True + + # Endpoint 0 comes back and is adopted on the hourly re-probe - nothing rotates + component.rest[0].read_data = MagicMock(return_value=_rest_data_blob()) + run_async(component.run(seconds=GIVTCP_REDISCOVER_SECONDS, first=False)) + + if base.args.get("charge_rate") != ["number.predbat_givtcp_0_charge_rate", "number.predbat_givtcp_1_charge_rate", "number.predbat_givtcp_2_charge_rate"]: + print("ERROR: expected slot n to be endpoint n after recovery, got {}".format(base.args.get("charge_rate"))) + failed = True + + if not failed: + print("PASS: a leading endpoint that is down at startup leaves every other inverter in its own slot") + return 1 if failed else 0 + + def test_rediscovery_never_drops_an_inverter_that_stops_answering(my_predbat=None): """ Losing a discovered inverter is a health problem, not a reconfiguration. @@ -2104,7 +2159,7 @@ def test_automatic_config_keeps_a_manually_configured_inverter(my_predbat=None): # A scalar apps.yaml value applies to every inverter, so it fills the tail rather than vanishing # (battery_scaling is only claimed when every inverter reports a design capacity, so the tail # handling is checked directly rather than through a key this fixture may not claim) - scaled = component._keep_configured_tail("battery_scaling", ["sensor.predbat_givtcp_0_battery_dod_soh"], 2) + scaled = component._per_endpoint_values("battery_scaling", lambda n: "sensor.predbat_givtcp_{}_battery_dod_soh".format(n), 2) if scaled != ["sensor.predbat_givtcp_0_battery_dod_soh", 1.0]: print("ERROR: expected the user's scalar battery_scaling to fill the tail, got {}".format(scaled)) failed = True @@ -2714,12 +2769,13 @@ def test_givtcp_component(my_predbat=None): ("soc_kw_binding", test_automatic_config_uses_soc_kw_not_percent, "automatic_config binds soc_kw"), ("write_exception", test_write_event_exception_does_not_propagate, "write exception contained"), ("discovered_count", test_automatic_config_counts_discovered_inverters_not_configured_urls, "num_inverters counts discovered inverters"), - ("discovered_mapping", test_automatic_config_maps_predbat_inverter_to_the_live_endpoint, "live endpoint drives inverter 0"), + ("discovered_mapping", test_automatic_config_keeps_the_live_endpoint_in_its_own_slot, "live endpoint keeps its own slot"), ("discovered_none", test_automatic_config_skipped_when_nothing_was_discovered, "no config when nothing discovered"), ("discovery_drops_dead", test_dead_endpoint_is_not_polled_after_discovery, "dead endpoint dropped after discovery"), ("discovered_pause_gate", test_pause_keys_gated_on_discovered_inverters_only, "pause gate ignores undiscovered endpoints"), ("rediscover_late", test_rediscovery_picks_up_an_inverter_that_was_down_at_startup, "late inverter adopted on re-probe"), - ("rediscover_append", test_rediscovery_appends_so_running_inverters_keep_their_identity, "re-probe appends, preserving identity"), + ("rediscover_append", test_rediscovery_keeps_inverter_identity_by_endpoint, "re-probe keeps slot n on endpoint n"), + ("leading_endpoint_down", test_leading_endpoint_down_does_not_shift_the_fleet, "leading endpoint down does not shift the fleet (#5209)"), ("rediscover_no_shrink", test_rediscovery_never_drops_an_inverter_that_stops_answering, "discovered inverters are never dropped"), ("rediscover_cheap", test_rediscovery_uses_a_cheap_single_probe, "re-probe uses a single cheap GET"), ("rediscover_complete", test_rediscovery_skipped_once_every_endpoint_is_discovered, "no re-probe when fleet is complete"), From 5931298d6ca117d96ea9346dbe2ad15489e82ac2 Mon Sep 17 00:00:00 2001 From: CI Date: Sat, 26 Sep 2026 11:44:00 +0100 Subject: [PATCH 2/2] fix(givtcp): address #5216 review - hand back gated claims, publish on adoption, split gap/tail log - automatic_config() records what each key held before its first claim and hands a capability-gated key back to it when a re-run's gate fails (a late endpoint lacking v3 or a register), instead of leaving the gap-slot claim pointing at an entity that is never published. - run() publishes again straight after rediscover() adopts an endpoint, so the re-run's discovery-key gates see the adopted inverter's sensors rather than failing for all of them. - Gap slots below the highest answered endpoint get their own Warn line naming the URL and the cost (planned without live data until it answers); only tail slots are "left as configured". - Rewrite the automatic_config() header comment to the per-endpoint invariant. - Update the debug-journal GH#5029 row and PR #4645 proxy bullet for _per_endpoint_values(). Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/predbat/givtcp.py | 51 +++++++++++-- apps/predbat/tests/test_givtcp_component.py | 84 +++++++++++++++++++++ tools/debug-journal.md | 4 +- 3 files changed, 131 insertions(+), 8 deletions(-) diff --git a/apps/predbat/givtcp.py b/apps/predbat/givtcp.py index 09258f369..fb39cb6cc 100644 --- a/apps/predbat/givtcp.py +++ b/apps/predbat/givtcp.py @@ -458,6 +458,11 @@ def initialize(self, rest_urls, automatic=True): # published points the arg at a non-existent entity, and get_arg then returns its default # instead of the user's own apps.yaml value. self.published_discovery = {} + # What each key held before automatic_config() first claimed it. A capability-gated claim is + # made on the fleet as it stands, which can include gap slots whose endpoint has not answered + # yet; if that endpoint turns out to lack the capability, the re-run hands the key back to + # this value rather than leaving the stale claim in place until restart. + self.claimed_from = {} # publish_data() runs every poll; the unsupported-model notice is per inverter and only # worth saying once rather than every 60 seconds for the life of the process self.discharge_target_warned = {} @@ -531,7 +536,14 @@ async def run(self, seconds, first): self.refresh_discovery() if rediscover: + fleet_before = len(self.discovered) await self.rediscover() + if len(self.discovered) > fleet_before: + # automatic_config() below gates each discovery key on every managed inverter having + # published it, and the adopted inverter has not been published yet this cycle. Without + # this every such gate fails on the re-run and the key is handed back to apps.yaml for + # the whole fleet, with nothing to re-run it once the next poll does publish. + await self.publish_data() # Re-runs when the fleet has grown. Deliberately not when it shrinks: self.discovered is # append-only, because dropping an inverter that stopped answering would rebuild Predbat's @@ -861,9 +873,14 @@ def _per_endpoint_values(self, key, make, fleet_size): async def automatic_config(self): """Point Predbat's standard entity-based apps.yaml keys at the entities this component publishes.""" # What gets claimed is driven by the endpoints that answered discovery, not by the length of - # the configured givtcp_rest list - claiming an entity for a URL with nothing behind it - # would have Predbat plan and execute against a phantom battery. The fleet size can still - # come out larger than that, but only where the user asked for it: see n_inverters below. + # the configured givtcp_rest list: no slot is claimed past the highest endpoint that + # answered, so the template's trailing placeholder URLs never become inverters. Below that + # endpoint Predbat inverter n has to be REST endpoint n (#5209), so a slot whose endpoint has + # not answered is claimed too - with the user's own configuration where there is one, and + # otherwise with the entities that endpoint will publish once it does. Until then Predbat + # plans that inverter without live data, which is the cost of not renumbering the fleet + # around it; the warning below says so. Past the highest endpoint the fleet can still come + # out larger, but only where the user asked for it: see n_inverters below. if not self.automatic: self.log("Info: GivTCP: givtcp_automatic is off - publishing entities but leaving apps.yaml to you") return @@ -882,9 +899,20 @@ async def automatic_config(self): n_covered = max(discovered) + 1 n_inverters = max(n_covered, self._configured_num_inverters()) self.log("GivTCP: configuring Predbat for {} discovered inverter(s)".format(n_discovered)) - if n_inverters > n_discovered: - not_answered = [n for n in range(n_inverters) if n not in discovered] - self.log("Info: GivTCP: {} inverter(s) are configured and {} answered here - leaving inverter(s) {} as configured".format(n_inverters, n_discovered, ", ".join(str(n) for n in not_answered))) + # The two kinds of slot that did not answer are treated differently by _per_endpoint_values(), + # so they are reported separately: a gap below the highest endpoint that answered is bound + # to its own endpoint's entities wherever nothing is configured for it, a slot past it is + # only ever left as configured. + gap = [n for n in range(n_covered) if n not in discovered] + tail = list(range(n_covered, n_inverters)) + if gap: + self.log( + "Warn: GivTCP: no inverter has answered yet at {} - Predbat inverter(s) {} keep their own slot and are bound to that endpoint's entities wherever nothing is configured for them, so they are planned without live data until it answers. Remove the URL from givtcp_rest if that inverter no longer exists.".format( + ", ".join(self.rest[n].inverter.rest_api for n in gap), ", ".join(str(n) for n in gap) + ) + ) + if tail: + self.log("Info: GivTCP: {} inverter(s) are configured and {} answered here - leaving inverter(s) {} as configured".format(n_inverters, n_discovered, ", ".join(str(n) for n in tail))) self.set_arg_auto("inverter_type", self._per_endpoint_values("inverter_type", lambda n: "GE", n_inverters)) self.set_arg_auto("num_inverters", n_inverters) @@ -944,7 +972,18 @@ async def automatic_config(self): else: self.log("Info: GivTCP: the battery pause time slots are not reported by every inverter - leaving pause_start_time/pause_end_time to your apps.yaml config") + # A key claimed on an earlier run whose gate has since failed - typically a late endpoint + # adopted on re-probe that lacks the capability the earlier fleet had - goes back to what it + # held before. Left alone, its gap-slot entry points at an entity that endpoint will never + # publish, so get_arg returns its default instead of the user's own apps.yaml value. + for key in [key for key in self.claimed_from if key not in keys]: + self.log("Info: GivTCP: {} is no longer supported by every managed inverter - handing it back to your apps.yaml config".format(key)) + self.set_arg(key, self.claimed_from.pop(key)) + for key in keys: + if key not in self.claimed_from: + previous = self._configured_value(key) + self.claimed_from[key] = list(previous) if isinstance(previous, list) else previous domain, _, _ = GIVTCP_CONTROLS.get(key, (None, None, None)) domain = domain or "sensor" if key == "battery_scaling": diff --git a/apps/predbat/tests/test_givtcp_component.py b/apps/predbat/tests/test_givtcp_component.py index d0c71bdfb..487a46f62 100644 --- a/apps/predbat/tests/test_givtcp_component.py +++ b/apps/predbat/tests/test_givtcp_component.py @@ -835,6 +835,9 @@ def test_rediscovery_picks_up_an_inverter_that_was_down_at_startup(my_predbat=No assert component.discovered == [0, 1], f"Expected both endpoints discovered, got {component.discovered}" assert base.args["num_inverters"] == 2, f"Expected automatic_config re-run for 2 inverters, got {base.args.get('num_inverters')}" assert base.args["charge_rate"] == ["number.predbat_givtcp_0_charge_rate", "number.predbat_givtcp_1_charge_rate"] + # Discovery keys are gated on every inverter having published them, so the adopted inverter has + # to be published before the re-run - otherwise the gate fails and the key is handed back + assert base.args.get("battery_calibration") == ["sensor.predbat_givtcp_0_battery_calibration", "sensor.predbat_givtcp_1_battery_calibration"], f"Expected battery_calibration claimed for both, got {base.args.get('battery_calibration')}" print("PASS: an inverter that was down at startup is adopted on re-probe") return 0 @@ -919,6 +922,85 @@ def test_leading_endpoint_down_does_not_shift_the_fleet(my_predbat=None): return 1 if failed else 0 +def test_gated_claim_is_handed_back_when_a_late_endpoint_lacks_the_capability(my_predbat=None): + """ + A capability-gated claim made while a gap endpoint was down is undone if that endpoint lacks it. + + At startup only v3 endpoint 1 answered, so pause_mode and discharge_target_soc pass their + all-v3 gates and gap slot 0 is bound to endpoint 0's own entities. When endpoint 0 is adopted + on re-probe and turns out to be v2, those entities will never be published, so the keys have + to go back to what apps.yaml had for them rather than keep pointing slot 0 at nothing. + """ + failed = False + base, component = _make_component(rest_urls=["http://givtcp0:6345", "http://givtcp1:6345"]) + apps_yaml = {"discharge_target_soc": ["number.inv0_discharge_target", "number.inv1_discharge_target"]} + base.args_from_apps_yaml = dict(apps_yaml) + base.apps_yaml_override_warned = set() + base.args.update(apps_yaml) + component.rest[0].read_data = MagicMock(return_value=None) + component.rest[1].read_data = MagicMock(return_value=_rest_data_blob(version="3.0.4")) + + run_async(component.run(seconds=0, first=True)) + if base.args.get("pause_mode") != ["select.predbat_givtcp_0_pause_mode", "select.predbat_givtcp_1_pause_mode"]: + print("ERROR: expected pause_mode claimed for both slots at startup, got {}".format(base.args.get("pause_mode"))) + failed = True + if base.args.get("discharge_target_soc") != ["number.inv0_discharge_target", "number.predbat_givtcp_1_discharge_target_soc"]: + print("ERROR: expected discharge_target_soc claimed for endpoint 1 only, got {}".format(base.args.get("discharge_target_soc"))) + failed = True + + # Endpoint 0 answers on re-probe, but runs GivTCP v2 + component.rest[0].read_data = MagicMock(return_value=_rest_data_blob(version="2.4.0")) + run_async(component.run(seconds=GIVTCP_REDISCOVER_SECONDS, first=False)) + + if sorted(component.discovered) != [0, 1]: + print("ERROR: expected both endpoints discovered, got {}".format(component.discovered)) + failed = True + if "pause_mode" in base.args: + print("ERROR: expected pause_mode handed back (nothing configured), got {}".format(base.args.get("pause_mode"))) + failed = True + if base.args.get("discharge_target_soc") != apps_yaml["discharge_target_soc"]: + print("ERROR: expected discharge_target_soc handed back to apps.yaml, got {}".format(base.args.get("discharge_target_soc"))) + failed = True + if base.args.get("charge_rate") != ["number.predbat_givtcp_0_charge_rate", "number.predbat_givtcp_1_charge_rate"]: + print("ERROR: expected the always-claimed keys to stay claimed, got {}".format(base.args.get("charge_rate"))) + failed = True + + if not failed: + print("PASS: a gated claim is handed back when a late endpoint lacks the capability") + return 1 if failed else 0 + + +def test_gap_and_tail_slots_are_logged_separately(my_predbat=None): + """ + A gap slot is reported as bound to its own endpoint, a slot past the last endpoint as left alone. + + The two are handled differently by _per_endpoint_values(), and one combined "leaving inverter(s) + ... as configured" line misdescribed the gap slots, which get their endpoint's own entities. + """ + failed = False + base, component = _make_component(rest_urls=["http://dead:6345", "http://givtcp1:6345"]) + base.args["num_inverters"] = 3 + messages = [] + component.log = lambda message, quiet=True: messages.append(str(message)) + component.rest[0].read_data = MagicMock(return_value=None) + component.rest[1].read_data = MagicMock(return_value=_rest_data_blob()) + + run_async(component.run(seconds=0, first=True)) + + gap =[m for m in messages if m.startswith("Warn: GivTCP: no inverter has answered yet")] + tail = [m for m in messages if "leaving inverter(s)" in m] + if len(gap) != 1 or "http://dead:6345" not in gap[0] or "inverter(s) 0 keep their own slot" not in gap[0]: + print("ERROR: expected one warning naming gap slot 0 and its URL, got {}".format(gap)) + failed = True + if len(tail) != 1 or not tail[0].endswith("leaving inverter(s) 2 as configured"): + print("ERROR: expected the tail line to name only inverter 2, got {}".format(tail)) + failed = True + + if not failed: + print("PASS: gap and tail slots are logged separately") + return 1 if failed else 0 + + def test_rediscovery_never_drops_an_inverter_that_stops_answering(my_predbat=None): """ Losing a discovered inverter is a health problem, not a reconfiguration. @@ -2944,6 +3026,8 @@ def test_givtcp_component(my_predbat=None): ("rediscover_late", test_rediscovery_picks_up_an_inverter_that_was_down_at_startup, "late inverter adopted on re-probe"), ("rediscover_append", test_rediscovery_keeps_inverter_identity_by_endpoint, "re-probe keeps slot n on endpoint n"), ("leading_endpoint_down", test_leading_endpoint_down_does_not_shift_the_fleet, "leading endpoint down does not shift the fleet (#5209)"), + ("gated_claim_handed_back", test_gated_claim_is_handed_back_when_a_late_endpoint_lacks_the_capability, "gated claim handed back when a late endpoint lacks the capability"), + ("gap_and_tail_logged", test_gap_and_tail_slots_are_logged_separately, "gap and tail slots logged separately"), ("rediscover_no_shrink", test_rediscovery_never_drops_an_inverter_that_stops_answering, "discovered inverters are never dropped"), ("rediscover_cheap", test_rediscovery_uses_a_cheap_single_probe, "re-probe uses a single cheap GET"), ("rediscover_complete", test_rediscovery_skipped_once_every_endpoint_is_discovered, "no re-probe when fleet is complete"), diff --git a/tools/debug-journal.md b/tools/debug-journal.md index 1255526e3..0c6c622f0 100644 --- a/tools/debug-journal.md +++ b/tools/debug-journal.md @@ -181,7 +181,7 @@ Grep for the named symbol rather than trusting a line number. | Plan seems stuck / doesn't adopt a clearly better replan | `should_replace_plan()` (`plan.py:1103`) compares pre-clip snapshots; a retained incumbent keeps its stale snapshot without the expired-window pruning `charge_window_best` gets, so the improvement gap can read identical for hours while real costs move. Combined with the fragmentation tie-break (`plan.py:1114`), which always favours the plan that adds no slot, a genuinely better plan can stay rejected until something else invalidates the plan outright. Confirmed from a 24h log: 180 consecutive comparisons with an unchanged gap, only 7 of 187 total ever adopting a new plan (GH#4736). Not a bug in the tie-break itself — `test_plan_tiebreak.py` asserts exactly that rule on purpose. | | Operation mode flip-flops between autonomous / self-consumption (or TBC / Self-Powered) within one control cycle and the export never engages — Tesla (and any service-template-controlled) inverter | **Fixed in PR #5114 (merged 2026-09-19)** — keep the mechanism for pre-#5114 logs. The `Charging/Discharging off via service` block (`execute.py`) had no `isExporting` guard: while an export is active `isCharging` is False, so it fired `adjust_charge_immediate(0)` → `charge_stop_service` 1-4s after `adjust_export_immediate()` had already run charge_stop + discharge_start, and on the shipped Tesla template both write the same operation-mode select, so the trailing write reverted the mode every cycle (GH#4165 and GH#4641, verified from both reporters' logs). #5114 skips the block while exporting — `adjust_export_immediate()` already stops charging as part of starting the export — and the `run_execute_test` status assertions now assert the charge-off never fires on `Exporting`/`Freeze exporting` statuses. Pre-fix log signature: `Calling service charge_stop_service` in a second pair *after* the discharge_start pair, within one 5-min cycle. On pre-fix versions `call_service_template`'s dedup would suppress the identical trailing call, but the shipped `templates/tesla_powerwall.yaml` sets `repeat: True` on every select write, which bypasses it — removing `repeat: True` from `charge_stop_service` is the user-side workaround. #4165 is the original; #4641 the dup. | | Charge and hold writes alternate every 5 minutes (GivTCP/GE "Setting Charge Schedule to enable/disable" + pause-mode toggles swapping nightly) while the plan target is rock-stable | The reported SoC is flipping by exactly 1% between cycles across a mid-quantisation-band boundary, not plan churn — in the verified case the target held at 71% for hours (a fractional limit, 6.64/9.39 kWh = 70.7%, printed "71%") while GE's SoC read 71↔70. `execute.py`'s hold branch gates on `soc_percent >= inv_target_soc_percent` with no tolerance; the intended 1% hysteresis (`soc_percent >= inv_target_soc_percent - 1.0`, comment "or within 1% below it") is nested *inside* that gate, so it is always true when reached and its else (re-enable the charge window) is dead — any SoC even 0.1% below target falls to "Charging" and re-enables the window, and next cycle at target it holds again (GH#5083, structure verified on main). Quick confirmation from their log: grep `Charge window will be` and check the target stays constant while current SoC alternates ±1% — read the raw lines, because the strings "current SoC X%," and "target Y%" shift field positions and an awk one-liner can make the target *look* like it flips. The gate/dead-hysteresis structure predates July's #4164 and the reporter's v8.52→v8.53.3 onset window touched neither the hold path nor the plan gate — late-Aug onset was situational (a target landing dead-heat on the quantisation band), not a regression. Fix direction: widen the gate to `target - 1.0`. Related, not duplicates: #4232 (EMS charge-target spam), #3693 (bulk GivTCP writes), and `soc-kw quantised by percent sensor` (same class, different sensor path). | -| An inverter disappears from the plan after upgrading to v9.x (mixed fleet) | Start at the vendor component's `automatic_config()` and its `set_arg_auto(..., overwrite=True)` writes, not `execute.py`'s inverter loop. GH#5029: since the GivTCP REST refactor (#4864, v9.0.0+) GivTCP's auto-config wrote `inverter_type = ["GE" x discovered]` and `num_inverters = len(discovered)` (`givtcp.py`), so in a mixed fleet (GE on GivTCP REST + another brand configured manually) the manual `num_inverters` was shrunk and the non-GE inverter silently dropped out — the component-vs-manual instance of #4824. **Fixed in PR #5037 (v9.0.2)**: GivTCP's auto-config now only ever *raises* `num_inverters` (`_configured_num_inverters()` takes the largest value already configured) and `_keep_configured_tail()` preserves the configured list entries for any inverter beyond the discovered ones. Keep the mechanism for pre-v9.0.2 logs: grep the reporter's full startup log for `auto-discovery wins` to confirm without needing their apps.yaml. `num_inverters`/`inverter_type` are **not** in `GIVTCP_AUTO_CONFIG_USER_WINS_KEYS` (that tuple is only the energy-today history keys, #4959) — don't cite that list as a guard. Fallback workaround at any version: `givtcp_automatic: False` (entities still published, auto-config skipped). Maintainer's preferred direction for the wider class: a per-component inverter offset, same spirit as the myenergi per-device-kind flag preference. | +| An inverter disappears from the plan after upgrading to v9.x (mixed fleet) | Start at the vendor component's `automatic_config()` and its `set_arg_auto(..., overwrite=True)` writes, not `execute.py`'s inverter loop. GH#5029: since the GivTCP REST refactor (#4864, v9.0.0+) GivTCP's auto-config wrote `inverter_type = ["GE" x discovered]` and `num_inverters = len(discovered)` (`givtcp.py`), so in a mixed fleet (GE on GivTCP REST + another brand configured manually) the manual `num_inverters` was shrunk and the non-GE inverter silently dropped out — the component-vs-manual instance of #4824. **Fixed in PR #5037 (v9.0.2)**: GivTCP's auto-config now only ever *raises* `num_inverters` (`_configured_num_inverters()` takes the largest value already configured) and `_keep_configured_tail()` preserved the configured list entries for any inverter beyond the discovered ones. PR #5216 (GH#5209) replaced `_keep_configured_tail()` with `_per_endpoint_values()`, which pins Predbat inverter n to REST endpoint n: the tail past the highest discovered endpoint is still kept as configured, but a gap slot below it (an endpoint that has not answered) keeps its configured entry or, with nothing configured, is bound to that endpoint's own not-yet-published entities, and `num_inverters` covers at least `max(discovered) + 1`. Keep the mechanism for pre-v9.0.2 logs: grep the reporter's full startup log for `auto-discovery wins` to confirm without needing their apps.yaml. `num_inverters`/`inverter_type` are **not** in `GIVTCP_AUTO_CONFIG_USER_WINS_KEYS` (that tuple is only the energy-today history keys, #4959) — don't cite that list as a guard. Fallback workaround at any version: `givtcp_automatic: False` (entities still published, auto-config skipped). Maintainer's preferred direction for the wider class: a per-component inverter offset, same spirit as the myenergi per-device-kind flag preference. | | Inverter service calls silently stopped after an upgrade | Each log grep discriminates one hypothesis: `Calling service` present but the script never runs → HA-layer rejection, look for `was not accepted` / `Service call ... failed` (ha.py); `Skipped service ... as it was previously called` repeatedly → identical-payload dedup, the plan/payload is static, look at why target/power never change; **neither line at all** → the executor branch is never reached: `Exception raised` (run_time_loop logs the traceback) or `Failed to fetch inverter data, not able to execute the plan` — note plan sensors can still look live while execute is dead, so "the plan updates" does not prove `execute_plan()` runs. Also check `predbat.status` *during* a planned window: `Charging` = the executor is reaching the service calls; stuck `Demand` = the charge branch never entered. `last_service_hash` is in-memory only (`predbat.py:598`) — cleared by every restart and by `reset_inverter()` — so any "dedup state survived the migration" theory is dead on arrival. Only two users have hit a variant of this class (discussion #3373, GH#5021) — priority_low until a second report. | ## Traps when investigating @@ -201,7 +201,7 @@ Grep for the named symbol rather than trusting a line number. - **Build test dates from Predbat's clock, not the machine's.** Predbat runs on the timezone set in apps.yaml (Europe/London in the harness), not the host's, so a fixture built with `datetime.now()` is on a different date from the code under test for part of every day. `test_saving_session` and `test_alert_feed` failed exactly that way under `TZ=America/Los_Angeles` at 21:30 local - and the clearest case is instructive: `load_free_slot()` measures a slot against `midnight_utc`, so a window built for the machine's "today" starts before Predbat's day began and is dropped entirely. Anchor fixture dates to `my_predbat.midnight_utc` (`96e1073c`, PR #4877). Hardcoded future dates in test data have the same shelf-life problem and were pushed out to 2099 (`d850365b`). Newest instance: PR #4971's `multi_car_iog` tests force `now_utc` to 12:00 on the *machine's* UTC date while `midnight_utc` stays on the real clock, so between 23:00 UTC and midnight the forced slot falls behind local midnight and is dropped - observed as `multi_car_iog_load_slots_regression` and `multi_car_iog_model_limit_fetch_4967` both failing with "car 0 should have IOG slots, got none" at 00:06 local, on a clean docs-only tree (mechanism reasoned from the code, not reproduced at a passing hour) - **fixed in PR #4998** by pinning `midnight_utc` in those tests, keep the mechanism for the next test that pins only one of the two clocks. Newest instance (2026-09-20 00:12 BST): `test_teslemetry_local_weekday_follows_the_base_clock` asserts its no-base fallback against the *host's* `datetime.now().weekday()` while the fallback itself is `datetime.now(timezone.utc)` — the two clocks disagree in the hour after local midnight (UTC is still the previous day), so the module crashes in that window; observed on a clean docs-only tree, and seen again 2026-09-22 00:09 BST aborting the quick suite mid-run. Counterweight for a different clock: storage-cache expiry is compared against the **real** clock (see the Storage row in the per-integration table), so anything producing or asserting an expiry must use the real one — the discriminator is which clock the code under test compares against. - **`midnight_utc` is local midnight, and `now_utc` is local time.** Both names are wrong in the same direction and it has misled at least one review into hunting for a UTC-vs-local frame mismatch that does not exist. `update_time()` sets `now_utc_real = datetime.now(self.local_tz)` (`predbat.py:654`) - already local, not UTC - and `midnight_utc` is that value with the time zeroed (`predbat.py:655`), i.e. the local midnight of Predbat's configured timezone as a timezone-aware datetime. Anything comparing a slot against `midnight_utc` is therefore working in local wall-clock, which is usually what you want and never what the name says. Before concluding a rate or slot bug is a timezone frame error, check which of the two frames the code is actually in. Corollary for reading logs: `time_abs_str()` renders against `self.midnight`, so a line quoting a past/negative minute key prints in the local frame and will not line up with the UTC timestamps an integration's sensor publishes (GH#5036) — recompute the minutes from the sensor's own timestamps instead of keying evidence off the printed date. - **Test ordering.** Since PR #5102 (merged 2026-09-15) every registry test builds its own `PredBat` (GH#5079), so the shared-fixture pollution class is fixed at the source — a test that fails in a full run but passes alone is now likelier to be one of the remaining hand-rolled restores (test_inverter.py, test_agent_tools.py; see the GH#5079 section) or a genuinely order-dependent case, and running only the targeted test still isolates. `run_debug_cases` builds a fresh instance per case for its own reason: each case replays a full dump. The wall-clock half is closed too: `create_predbat()` now pins `minutes_now`/`now_utc` from `FIXTURE_MINUTES_NOW` (`test_infra.py`), so no module inherits either the wall clock or another module's residue into those two fields — the fixture-level pin the GH#5026 note anticipated has landed and closes that class (keep the mechanism for pre-#5102 logs: a suite run used to sit at `reset_inverter`'s noon residue while a standalone run inherited the wall clock, and a time-of-day-dependent test could pass one way and fail the other — `prediction_batch` failed exactly that way, the ten-trial SoC-cost precondition collapsing onto one value at seed 5's window/rate alignment; equivalence-style subtests are `minutes_now`-insensitive, only precondition checks on seeded-scenario economics are sensitive). The per-module pins that preceded the fixture pin: PR #5028 (merged, `9290055d`) pinned the clock per-module - the third per-module pin of this class, after PR #5013 pinned dates and #4971's tests pinned `midnight_utc` - with `minutes_now` added to `SCENARIO_STATE_ATTRS`. Two caveats outlive the fixture pin: a pin-guard test is **vacuous in suite configuration when the pin value equals the residue value** (suite ambient was already 720), so a guard test must capture the pre-pin entry value rather than compare against the pin constant; and `update_time()` still takes `midnight_utc`/`now_utc_real` from the host clock, so the date-frame half of this trap is unchanged. -- **A textually clean auto-merge can still be a semantic conflict.** Merging `origin/main` into an old PR branch reported no conflicts while main's GivTCP refactor (#4864) had deleted `Inverter.rest_data`, which the PR both tested on and passed to the constructor - the merged tree crashed the first `Inverter()` construction in two test modules, only one of them the PR's own (PR #4645). For PR-cleanup work the full `run_pre_commit` suite is the detector, not the PR's own module. Corollary: `rest_data` is gone since #4864 and `inverter_source_active()` (`inverter.py`) is the replacement question for "is this inverter component/REST-backed" - anything written against `self.rest_data` predates the refactor. And when a test plants a components stub, construction-time calls run against whatever stub the *previous* test module left behind, so a new components-consulting call in `Inverter.__init__` needs a `getattr` guard on the registry surface, not a fake patched into one module. Correction to what this bullet itself used to imply: `inverter_source_active()` answers a **fleet-wide** question, not a per-inverter one — it takes no index and consults no per-inverter state, so one component-backed inverter at index 0 makes it true for a hand-configured, script-driven inverter at index 1 (its docstring says the scope is deliberate). There is no per-index source API; the usable proxy is the shape of the per-inverter args list itself, because auto-config writes slots `0..n_discovered-1` and `_keep_configured_tail()` leaves the rest as the user wrote them — a list that names some inverters but not this one means the source has said what it covers and this index is outside it (PR #4645, still open, gates on that proxy). Related: `create_missing_arg()` pads short per-inverter lists with `None`, not the default, precisely so an unfilled slot is not read back as an entity id — a hand-rolled `while len(...) <= id: append(default)` loop next to it is both dead and wrong about the padding value. +- **A textually clean auto-merge can still be a semantic conflict.** Merging `origin/main` into an old PR branch reported no conflicts while main's GivTCP refactor (#4864) had deleted `Inverter.rest_data`, which the PR both tested on and passed to the constructor - the merged tree crashed the first `Inverter()` construction in two test modules, only one of them the PR's own (PR #4645). For PR-cleanup work the full `run_pre_commit` suite is the detector, not the PR's own module. Corollary: `rest_data` is gone since #4864 and `inverter_source_active()` (`inverter.py`) is the replacement question for "is this inverter component/REST-backed" - anything written against `self.rest_data` predates the refactor. And when a test plants a components stub, construction-time calls run against whatever stub the *previous* test module left behind, so a new components-consulting call in `Inverter.__init__` needs a `getattr` guard on the registry surface, not a fake patched into one module. Correction to what this bullet itself used to imply: `inverter_source_active()` answers a **fleet-wide** question, not a per-inverter one — it takes no index and consults no per-inverter state, so one component-backed inverter at index 0 makes it true for a hand-configured, script-driven inverter at index 1 (its docstring says the scope is deliberate). There is no per-index source API; the usable proxy was the shape of the per-inverter args list itself, because auto-config wrote slots `0..n_discovered-1` and `_keep_configured_tail()` left the rest as the user wrote them — a list that names some inverters but not this one meant the source had said what it covers and this index was outside it (PR #4645, still open, gates on that proxy). **The proxy weakened with PR #5216 (GH#5209):** GivTCP now fills slot n from REST endpoint n via `_per_endpoint_values()`, so a gap slot below the highest discovered endpoint is filled even when nothing is configured for it (with that endpoint's own entities), and a full-length list no longer implies every index is component-covered. Only indices past `max(discovered)` still follow the old rule; a per-index gate has to compare against the component's entity-id prefix (`_givtcp__`) rather than the list length. Related: `create_missing_arg()` pads short per-inverter lists with `None`, not the default, precisely so an unfilled slot is not read back as an entity id — a hand-rolled `while len(...) <= id: append(default)` loop next to it is both dead and wrong about the padding value. - **Check a PR's base staleness before reviewing it.** A long-open PR's stored base commit can be well behind main, and a conflicting PR's headline fix may already be on main: PR #4756's `plan_iboost_smart()` fix duplicated the merged GH#4817 window-average fix, and resolving its `test_iboost.py` conflict in the PR's favour would have silently deleted main's `run_iboost_smart_average_test` — resurrecting the bug. Check `git merge-base --is-ancestor main` and diff any test-file changes against main's current tests, not the PR base, before analysing the "fix". - **GitNexus is indexed from main, so a PR branch is off-index.** `impact()` returns "Target not found" for PR-only symbols, and `detect_changes()` pins line-shifted hunks on unchanged neighbours - read its output as "which files", not "which symbols", and grep for callers instead (observed reviewing PR #5143). Same session: the review-creation API response reports `comments: 0` even when inline comments attach - verify with `gh api .../reviews//comments --jq 'length'` before resubmitting. - **`self.base.args` is process-lifetime, and Predbat writes its own dummy entity ids into it.** `self.args` is assigned once from apps.yaml (`hass.py`, `load_apps_yaml`) and never re-read for the life of the process, while `Inverter.__init__` assigns `create_entity()`'s output — `sensor.{prefix}_{inverter_type}_{id}_{entity_name}` — straight back into `self.base.args[...]`, and inverters are rebuilt from scratch **every 5-minute cycle**: `fetch_inverter_data()` defaults `create=True` and the normal-path caller uses the default, so `execute.py` clears and re-creates the whole list every run and anything `Inverter.__init__` samples from live entity state is re-sampled per cycle, not init-only (GH#5099; the pre-#5149 balance-path construction this sentence used to describe is gone — `balance_inverters()` is a pure function since PR #5149 and builds no `Inverter` objects). So from the second construction onwards, any "is this arg configured?" test reading `self.base.args` is reading what Predbat itself wrote last cycle — and the dummy id contains a `.`, so it passes `is_entity_id()` (PR #4745, issue #4738: a custom `GROWATTSPH` type's real `time.*` entity was overwritten by a Predbat dummy). What is lost if a guard then skips recreation on rebuild: `created_attributes` resets in `__init__` and is repopulated only by `create_entity()`, so a skipped sensor-domain write silently drops `state_class: measurement`; and because the id embeds `inverter_type`, a dummy surviving a type change names the **old** type and strands writes on a stale sensor (reproduced in a test on the PR). The obvious alternative authority, `args_from_apps_yaml`, does not generalise here: components write these same args into live `self.args` and never into the snapshot, so keying off the snapshot treats an auto-discovered inverter's select entities as unconfigured. What distinguishes them is recognising Predbat's own naming or a registry of ids created by `create_entity()` — that guard exists only on the open PR #4745, not on main.