From 4af6824abc5e8530cc2edd01a1c30f99ecf9c6a9 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Mon, 24 Aug 2026 18:23:46 +0100 Subject: [PATCH 1/2] fix(inverter): don't press the update button on idle non-export cycles execute.py calls adjust_force_export(False) with no times at all whenever nothing is being exported. On an inverter with has_discharge_enable_time set 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 idle cycle looked like a schedule change and pressed the update button. That is most of the day, not just export windows (#2328). Only compare times the caller actually asked us to set. A genuine transition out of export is still caught by force_export != old_discharge_enable, and the GE branch that deliberately clears the times keeps its existing behaviour - it is distinguished by whether the caller supplied any times, not by the times being None once we are past that branch. Affects GS_fb00 and the FoxCloud/TESLA/Enphase/Deye/Sunsynk/AlphaESS cloud types; plain GS takes the midnight-override path and was never affected. Not yet confirmed against a live install - awaiting logs on #4709. Co-Authored-By: Claude Opus 5 --- apps/predbat/inverter.py | 13 ++++++- apps/predbat/tests/test_inverter.py | 56 +++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 3024d8700..1ecf46311 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,12 +2712,18 @@ 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) # Separately, whether the start/end times themselves actually moved - used below to gate the # GivTCP settle sleep, which exists for the window write specifically ("start/end of discharge # window was just adjusted"). schedule_changed alone is too broad for that: it also goes True on # a bare scheduled_discharge_enable flip with the window untouched, which doesn't need settling. - times_changed = (new_end != old_end) or (new_start != old_start) + times_changed = start_changed or end_changed schedule_changed = times_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 diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 186dbd227..97c1f6cb1 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -2577,6 +2577,59 @@ def test_force_export_enable_only_flip_skips_settle_sleep(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 was never affected, so it is checked here too to pin + the difference down. + """ + failed = False + print("Test: {}".format(test_name)) + + saved_type = my_predbat.args.get("inverter_type") + + 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: + if saved_type is None: + my_predbat.args.pop("inverter_type", None) + else: + my_predbat.args["inverter_type"] = saved_type + + 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 @@ -3713,6 +3766,9 @@ def run_inverter_tests(my_predbat_dummy): # Regression test: an enable-only flip (times unchanged) must not trigger the GivTCP settle sleep failed |= test_force_export_enable_only_flip_skips_settle_sleep("force_export_enable_only_flip_skips_settle_sleep", ha, inv) + + # 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 From 7009184055d0bc6d5be6ad237679afb1914b5e75 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 28 Aug 2026 09:26:28 +0100 Subject: [PATCH 2/2] Address Copilot review findings on test_force_export_off_does_not_press_every_cycle - save/restore the discharge_* args and dummy_items this test overwrites, not just inverter_type, so nothing leaks into later tests - reword the docstring: plain GS wasn't affected by this test's specific None-comparison bug, but it does use the same "H M" time format that #4711's separate unconditional-write bug affected --- apps/predbat/tests/test_inverter.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 97c1f6cb1..e9c7788cf 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -2588,13 +2588,20 @@ def test_force_export_off_does_not_press_every_cycle(test_name, ha, my_predbat): 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 was never affected, so it is checked here too to pin - the difference down. + 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)) - saved_type = my_predbat.args.get("inverter_type") + # 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)): @@ -2622,10 +2629,16 @@ def test_force_export_off_does_not_press_every_cycle(test_name, ha, my_predbat): 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: - if saved_type is None: - my_predbat.args.pop("inverter_type", None) - else: - my_predbat.args["inverter_type"] = saved_type + 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