diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index ef2dbc140..31198016a 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -301,6 +301,36 @@ def create_missing_arg(self, arg, default): # then looked up as one. self.base.args[arg] = self.base.args[arg] + [None] * (self.id + 1 - len(self.base.args[arg])) + def is_real_entity_configured(self, arg): + """ + True if arg already resolves to a real HA entity id for this inverter (contains a domain, + e.g. 'time.foo'), as opposed to being unset, a bare placeholder value such as '23:59:00' or + '00:00:00' with no domain, or a dummy Predbat created for itself on an earlier + construction. Used to tell "this was configured deliberately" apart from "nothing real is + there yet" before create_missing_arg's own value-vs-list check would conflate the two - a + config item that's present but not a real entity is exactly the case a dummy entity still + needs to be created for. + + "Deliberately" covers a component's automatic_config() as well as apps.yaml: fox.py and + gecloud.py both set_arg() real select.* time entities they discovered, which never appear + in args_from_apps_yaml, so presence in apps.yaml alone is not the question being asked. + + The dummies are recognised from the registry create_entity() records them in rather than by + re-deriving their id format here: self.base.args lives for the whole process and is never + re-read from apps.yaml, and inverters are rebuilt from scratch on the balance path + (execute.py) as well as at startup, so from the second construction onwards args already + holds the dummy - which has a domain and would otherwise read back as genuine + configuration, skipping the recreation that re-registers created_attributes and restores + the state if it has gone away (#4745 review). + """ + values = self.base.args.get(arg) + if not isinstance(values, list) or self.id >= len(values): + return False + value = values[self.id] + if value in getattr(self.base, "predbat_created_entities", ()): + return False + return is_entity_id(value) + def __init__(self, base, id=0, quiet=False): """ Inverter class @@ -625,11 +655,18 @@ def __init__(self, base, id=0, quiet=False): self.base.args["inverter_mode"][id] = self.create_entity("inverter_mode", "Eco") if self.inv_charge_time_format != "HH:MM:SS": + # Some formats (H M) decompose the write into separate hour/minute entities and never + # expect the user to set this directly; others (no time window at all) ship a bare + # placeholder string. Either way Predbat needs somewhere to read/write for its own window + # bookkeeping. But if the user has genuinely pointed this at a real entity - a custom + # inverter definition using a non-HH:MM:SS format together with a real time window, e.g. + # #4738 - that's the one to use, not a self-created dummy that silently discards it. for x in ["charge", "discharge"]: for y in ["start", "end"]: entity_name = f"{x}_{y}_time" self.create_missing_arg(entity_name, "23:59:00") - self.base.args[entity_name][id] = self.create_entity(entity_name, "23:59:00") + if not self.is_real_entity_configured(entity_name): + self.base.args[entity_name][id] = self.create_entity(entity_name, "23:59:00") # Create dummy idle time entities if not self.inv_has_idle_time: @@ -1418,6 +1455,12 @@ def create_entity(self, entity_name, value, uom=None, device_class=None, icon=No attributes["icon"] = icon self.created_attributes[entity_id] = attributes + # Recorded on base, not self: created_attributes is reset per Inverter object, but args + # outlives every rebuild, so this is what lets a later construction tell its own dummy + # apart from real configuration (#4745 review). + if getattr(self.base, "predbat_created_entities", None) is None: + self.base.predbat_created_entities = set() + self.base.predbat_created_entities.add(entity_id) if self.base.get_state_wrapper(entity_id) is None: self.log("Inverter {} **** Creating dummy entity {} with value {} and attributes {}".format(self.id, entity_id, value, attributes)) diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 02b4d6fae..a2509690a 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -41,6 +41,146 @@ def test_foxess_support_discharge_freeze_matches_foxcloud(): return failed +def test_custom_type_respects_configured_time_entity(my_predbat): + """ + Regression test for issue #4738: a custom inverter definition using a non-HH:MM:SS + charge_time_format together with a real time window (has_time_window/has_discharge_enable_time + True) must not have its genuinely user-configured discharge_start_time/discharge_end_time/ + charge_start_time/charge_end_time silently replaced by a self-created dummy sensor. + + Two existing legitimate cases must be unaffected by the fix: + - GS_fb00 style (H M format): discharge_start_time is never user-configured at all - the real + writes go via separate hour/minute entities - so the dummy must still be created. + - SF style (has_time_window False): ships a bare placeholder string, not a real entity - the + dummy must still replace it, since there is nothing real to preserve. + + And a dummy Predbat wrote into args itself must not read back as user configuration on a later + construction, since args is process-lifetime state and inverters are rebuilt from scratch on the + balance path: the dummy has a domain, so a plain "has a dot" test would skip recreating it. + """ + failed = False + print("Test: test_custom_type_respects_configured_time_entity") + + saved_args = copy.deepcopy(my_predbat.args) + saved_def = copy.deepcopy(INVERTER_DEF.get("TEST_CUSTOM_TIME_ENTITY")) + had_created_entities = hasattr(my_predbat, "predbat_created_entities") + saved_created_entities = copy.deepcopy(my_predbat.predbat_created_entities) if had_created_entities else None + + try: + # Case 1: real entity configured - must be kept, not overwritten + my_predbat.args["inverter_type"] = ["TEST_CUSTOM_TIME_ENTITY"] + my_predbat.args["inverter"] = { + "charge_time_format": "S", + "has_time_window": True, + "has_charge_enable_time": True, + "has_discharge_enable_time": True, + } + my_predbat.args["discharge_start_time"] = ["time.real_discharge_start"] + my_predbat.args["discharge_end_time"] = ["time.real_discharge_end"] + my_predbat.args["charge_start_time"] = ["time.real_charge_start"] + my_predbat.args["charge_end_time"] = ["time.real_charge_end"] + my_predbat.args["givtcp_rest"] = None + + Inverter(my_predbat, 0, quiet=True) + + for arg, expected in [ + ("discharge_start_time", "time.real_discharge_start"), + ("discharge_end_time", "time.real_discharge_end"), + ("charge_start_time", "time.real_charge_start"), + ("charge_end_time", "time.real_charge_end"), + ]: + got = my_predbat.args[arg][0] + if got != expected: + print(f"ERROR: test_custom_type_respects_configured_time_entity: {arg} should stay {expected}, got {got} (overwritten by a dummy entity)") + failed = True + + # Case 2: nothing configured for this arg - dummy creation must still kick in + del my_predbat.args["discharge_start_time"] + Inverter(my_predbat, 0, quiet=True) + got = my_predbat.args["discharge_start_time"][0] + if not (isinstance(got, str) and got.startswith("sensor.")): + print(f"ERROR: test_custom_type_respects_configured_time_entity: unconfigured discharge_start_time should get a dummy sensor, got {got}") + failed = True + + # Case 3: SF-style bare placeholder (no domain) - must still be replaced by a dummy, not kept + my_predbat.args["inverter_type"] = ["TEST_CUSTOM_TIME_ENTITY_SF"] + my_predbat.args["inverter"] = { + "charge_time_format": "S", + "has_time_window": False, + "has_charge_enable_time": False, + "has_discharge_enable_time": False, + } + my_predbat.args["discharge_start_time"] = ["00:00:00"] + Inverter(my_predbat, 0, quiet=True) + got = my_predbat.args["discharge_start_time"][0] + if not (isinstance(got, str) and got.startswith("sensor.")): + print(f"ERROR: test_custom_type_respects_configured_time_entity: SF-style bare placeholder should still be replaced by a dummy sensor, got {got}") + failed = True + + # Case 4: rebuild. self.base.args lives for the whole process and is never re-read from + # apps.yaml, and inverters are rebuilt from scratch on the balance path (execute.py), so by + # the second construction args already holds the dummy written by the first. That dummy has + # a domain, so a plain "has a dot" test reads it back as user-configured and skips creation + # - leaving created_attributes unpopulated on the new object and the state unrestored if it + # has gone away. + my_predbat.args["inverter_type"] = ["TEST_CUSTOM_TIME_ENTITY_REBUILD"] + my_predbat.args["inverter"] = { + "charge_time_format": "S", + "has_time_window": True, + "has_charge_enable_time": True, + "has_discharge_enable_time": True, + } + my_predbat.args.pop("discharge_start_time", None) + + Inverter(my_predbat, 0, quiet=True) + dummy_id = my_predbat.args["discharge_start_time"][0] + rebuilt = Inverter(my_predbat, 0, quiet=True) + + if my_predbat.args["discharge_start_time"][0] != dummy_id: + print(f"ERROR: test_custom_type_respects_configured_time_entity: rebuild should keep the same dummy id {dummy_id}, got {my_predbat.args['discharge_start_time'][0]}") + failed = True + if dummy_id not in rebuilt.created_attributes: + print(f"ERROR: test_custom_type_respects_configured_time_entity: rebuild should re-register {dummy_id} in created_attributes, got {sorted(rebuilt.created_attributes)}") + failed = True + + # Case 5: the inverter type changes between constructions (discovery or an apps.yaml edit). + # The dummy id embeds the type, so the one left in args names the old type - it is still + # one Predbat created, so it must be replaced with the current type's dummy rather than + # preserved as though the user had chosen it. + my_predbat.args["inverter_type"] = ["TEST_CUSTOM_TIME_ENTITY_REBUILD2"] + Inverter(my_predbat, 0, quiet=True) + got = my_predbat.args["discharge_start_time"][0] + if got != "sensor.{}_TEST_CUSTOM_TIME_ENTITY_REBUILD2_0_discharge_start_time".format(my_predbat.prefix): + print(f"ERROR: test_custom_type_respects_configured_time_entity: a stale dummy from a previous inverter type should be replaced, got {got}") + failed = True + + # Case 6: a real entity a component's automatic_config() discovered (fox.py/gecloud.py + # set_arg() select.* time entities) is deliberate configuration even though it never + # appears in apps.yaml, so it must be preserved exactly like a hand-written one. + my_predbat.args["discharge_start_time"] = ["select.predbat_fox_abc_battery_schedule_discharge_start_time"] + Inverter(my_predbat, 0, quiet=True) + got = my_predbat.args["discharge_start_time"][0] + if got != "select.predbat_fox_abc_battery_schedule_discharge_start_time": + print(f"ERROR: test_custom_type_respects_configured_time_entity: an auto-discovered component entity should be preserved, got {got}") + failed = True + finally: + my_predbat.args = saved_args + if had_created_entities: + my_predbat.predbat_created_entities = saved_created_entities + else: + if hasattr(my_predbat, "predbat_created_entities"): + del my_predbat.predbat_created_entities + if saved_def is None: + INVERTER_DEF.pop("TEST_CUSTOM_TIME_ENTITY", None) + else: + INVERTER_DEF["TEST_CUSTOM_TIME_ENTITY"] = saved_def + INVERTER_DEF.pop("TEST_CUSTOM_TIME_ENTITY_SF", None) + INVERTER_DEF.pop("TEST_CUSTOM_TIME_ENTITY_REBUILD", None) + INVERTER_DEF.pop("TEST_CUSTOM_TIME_ENTITY_REBUILD2", None) + + return failed + + def test_support_feedin_first_is_opt_in(): """ support_feedin_first says the inverter's Freeze Export really is a "Feed-in First" mode (load, @@ -3283,6 +3423,7 @@ def run_inverter_tests(my_predbat_dummy): print("**** Running Inverter tests ****") failed |= test_foxess_support_discharge_freeze_matches_foxcloud() failed |= test_support_feedin_first_is_opt_in() + failed |= test_custom_type_respects_configured_time_entity(my_predbat) ha = my_predbat.ha_interface time_now = my_predbat.now_utc.strftime("%Y-%m-%dT%H:%M:%S%z")