diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index fc59dc841..4b6bf988e 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -2543,6 +2543,11 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No self.log("Warn: Inverter {} unable read discharge window as neither REST, discharge_start_time or discharge_start_hour are set".format(self.id)) return False + # Whether the caller asked us to manage the export times at all this cycle. execute.py calls + # adjust_force_export(False) with no times whenever nothing is being exported, which is a + # different thing from the GE branch below deliberately clearing them. + times_supplied = (new_start_time is not None) or (new_end_time is not None) + # Start time to correct format if new_start_time: new_start_time += timedelta(seconds=self.base.inverter_clock_skew_discharge_start * 60) @@ -2707,8 +2712,14 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No # press zeroes the timed current registers (#4709), and it also triggers the 30s GivTCP sleep in # adjust_inverter_mode. Tracking what we last committed keeps a stable window quiet while still # committing once after a restart, when nothing has been committed yet (#4000). + # When the caller supplied no times at all we are not managing the export window this cycle, so + # neither time can have "changed". Comparing None against the time the inverter still reports is + # never equal, which pressed the update button on every idle cycle for the rest of the day (#2328). + # A genuine transition out of export is still caught by force_export != old_discharge_enable below. + start_changed = times_supplied and new_start != old_start + end_changed = times_supplied and new_end != old_end export_schedule = (new_start, new_end, force_export) - schedule_changed = (new_end != old_end) or (new_start != old_start) or (force_export != old_discharge_enable) + schedule_changed = start_changed or end_changed or (force_export != old_discharge_enable) if is_hm_format and export_schedule != self.last_export_schedule_committed: # Only the H M path rewrites unconditionally, so only it needs the extra commit-once-per-run # safety net; every other format already commits on a real change alone. diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 529592f94..150efacaa 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -2501,6 +2501,72 @@ def test_force_export_stable_window_presses_button_once(test_name, ha, inv): return failed +def test_force_export_off_does_not_press_every_cycle(test_name, ha, my_predbat): + """ + Regression test for issue #2328: with no export scheduled the update button must not be pressed on + every cycle. + + execute.py calls adjust_force_export(False) with no times whenever nothing is being exported. On an + inverter with has_discharge_enable_time set (GS_fb00, and the FoxCloud/TESLA/Enphase/Deye/Sunsynk/ + AlphaESS cloud types) the midnight override is skipped, so new_start/new_end stay None while the + inverter still reports a real time - and None never compares equal, so every cycle looked like a + change and pressed the button. That is most of the day, not just export windows. + + Plain GS takes the midnight-override path and is not affected by this None-comparison route, so it + is checked here too to pin the difference down. (GS was affected by a separate bug - #4711's + unconditional H M register rewrite, which GS also uses - but that is a different code path to the + one this test targets.) + """ + failed = False + print("Test: {}".format(test_name)) + + # my_predbat/ha are shared across the whole test run - save everything this test touches so it + # can be restored exactly, rather than leaking a changed/missing arg or dummy entity value into + # later tests and making results order-dependent. + unset = object() + saved_args = {key: my_predbat.args.get(key, unset) for key in ("inverter_type", "discharge_start_time", "discharge_end_time", "scheduled_discharge_enable")} + saved_items = {key: ha.dummy_items.get(key, unset) for key in ("select.discharge_start_time", "select.discharge_end_time", "switch.scheduled_discharge_enable", "select.inverter_mode")} + + try: + for inverter_type, expected_presses in (("GS_fb00", 1), ("GS", 1)): + my_predbat.args["inverter_type"] = [inverter_type] + inv = Inverter(my_predbat, 0, quiet=True) + inv.rest_data = None + + ha.dummy_items["select.discharge_start_time"] = "00:00:00" + ha.dummy_items["select.discharge_end_time"] = "00:00:00" + ha.dummy_items["switch.scheduled_discharge_enable"] = "off" + ha.dummy_items["select.inverter_mode"] = "Eco" + my_predbat.args["discharge_start_time"] = "select.discharge_start_time" + my_predbat.args["discharge_end_time"] = "select.discharge_end_time" + my_predbat.args["scheduled_discharge_enable"] = "switch.scheduled_discharge_enable" + + presses = [] + # Must report success, as a real press does - a falsy return means "not committed, retry" + inv.press_and_poll_button = lambda side="both", _p=presses: (_p.append(side), True)[1] + + # Several cycles with nothing to export - the first may commit, the rest must be silent + for _ in range(4): + inv.adjust_force_export(False) + + if len(presses) > expected_presses: + print(f"ERROR: {test_name}: {inverter_type} pressed the button {len(presses)} times over 4 idle cycles, expected at most {expected_presses}") + failed = True + finally: + for key, value in saved_args.items(): + if value is unset: + my_predbat.args.pop(key, None) + else: + my_predbat.args[key] = value + for key, value in saved_items.items(): + if value is unset: + ha.dummy_items.pop(key, None) + else: + ha.dummy_items[key] = value + + return failed + + def test_time_entity_hour_write(test_name, ha, inv, dummy_rest, direction, new_start, new_end): """ Test that when *_start_hour / *_end_hour args resolve to time.* entities the full @@ -3635,6 +3701,11 @@ def run_inverter_tests(my_predbat_dummy): if failed: return failed + # Regression test for issue #2328: idle (non-export) cycles must not press the button every time + failed |= test_force_export_off_does_not_press_every_cycle("force_export_off_no_repeat_press", ha, my_predbat) + if failed: + return failed + # Regression test: export target SoC must track the minimum reserve SoC in both directions failed |= test_discharge_target_tracks_reserve("discharge_target_tracks_reserve", ha, inv, dummy_rest) if failed: