From 4984cc943f39eda4ddefefe95b5d3fe474f77584 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Fri, 25 Sep 2026 08:46:07 +0100 Subject: [PATCH 1/2] feat(fetch): warn when export_more_solar is enabled without set_export_freeze export_more_solar works by enabling Freeze Export on otherwise-idle slots that have predicted solar, so optimise_solar() returns immediately when set_export_freeze is off. The switch is then on but does nothing at all, with no indication anywhere - a system in #4865 had run that combination for months without knowing. Warn once per incident rather than every cycle, since fetch_config_options() runs every ~5 minutes and the combination may be deliberate. The flag resets when the combination becomes valid again so a later recurrence is reported. Mirrors the existing car_charging_energy_warned handling in the same function. The docs already stated the dependency; they now also mention the warning. Co-Authored-By: Claude Opus 5 --- apps/predbat/fetch.py | 11 ++ apps/predbat/predbat.py | 1 + .../tests/test_export_more_solar_warning.py | 164 ++++++++++++++++++ apps/predbat/unit_test.py | 2 + docs/customisation.md | 2 +- 5 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 apps/predbat/tests/test_export_more_solar_warning.py diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index b2990ab31..d01d03f0c 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -3227,6 +3227,17 @@ def fetch_config_options(self): self.set_freeze_export_during_demand = self.get_arg("set_freeze_export_during_demand") self.export_more_solar = self.get_arg("export_more_solar") self.export_more_solar_threshold = self.get_arg("export_more_solar_threshold") + # export_more_solar works by enabling Freeze Export on idle solar slots, so optimise_solar() + # returns immediately when set_export_freeze is off - the switch is then on but does nothing + # at all, with no indication anywhere. Warn once per incident rather than every cycle (same + # reasoning as car_charging_energy_warned below), and reset when the combination is fixed so + # a later recurrence is reported again. + if self.export_more_solar and not self.set_export_freeze: + if not self.export_more_solar_warned: + self.log("Warn: export_more_solar is enabled but set_export_freeze is off - export_more_solar has no effect, as it works by enabling Freeze Export on idle solar slots. Enable set_export_freeze, or turn export_more_solar off.") + self.export_more_solar_warned = True + else: + self.export_more_solar_warned = False # Mode self.predbat_mode = self.get_arg("mode") if self.predbat_mode == PREDBAT_MODE_OPTIONS[PREDBAT_MODE_CONTROL_SOC]: diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index 4ba550789..e87ce40af 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -398,6 +398,7 @@ def reset(self): self.metric_min_improvement_plan = 2.0 self.export_more_solar = False self.export_more_solar_threshold = 1.0 + self.export_more_solar_warned = False self.metric_battery_cycle = 0.0 self.metric_battery_value_scaling = 1.0 self.metric_battery_value_export_scaling = 0.8 diff --git a/apps/predbat/tests/test_export_more_solar_warning.py b/apps/predbat/tests/test_export_more_solar_warning.py new file mode 100644 index 000000000..2fb0f6f32 --- /dev/null +++ b/apps/predbat/tests/test_export_more_solar_warning.py @@ -0,0 +1,164 @@ +# ----------------------------------------------------------------------------- +# Predbat Home Battery System +# Copyright Trefor Southwell 2026 - All Rights Reserved +# This application maybe used for personal use only and not for commercial use +# ----------------------------------------------------------------------------- +# fmt: off +# pylint: disable=consider-using-f-string +# pylint: disable=line-too-long +# pylint: disable=attribute-defined-outside-init +"""Tests for the export_more_solar / set_export_freeze dependency warning. + +export_more_solar works by enabling Freeze Export on otherwise-idle slots that have predicted solar, +so optimise_solar() returns immediately when set_export_freeze is off (plan.py). The switch is then +on but does nothing whatsoever, and nothing anywhere said so - a real system in issue #4865 had run +that combination for months. + +The docs already state the dependency ("The feature relies on switch.predbat_set_export_freeze being +enabled"), so this is purely about telling a user who has not read them. +""" + + +def _override_switches(my_predbat, export_more_solar, set_export_freeze): + """Force the two switches under test for the next fetch_config_options() call. + + fetch_config_options() re-reads every setting through get_arg(), so setting the attributes + directly would just be overwritten. Intercept those two keys and let everything else through, + which keeps the rest of the real config path in play. + """ + real_get_arg = my_predbat.get_arg + overrides = {"export_more_solar": export_more_solar, "set_export_freeze": set_export_freeze} + + def get_arg(arg, *args, **kwargs): + if arg in overrides: + return overrides[arg] + return real_get_arg(arg, *args, **kwargs) + + my_predbat.get_arg = get_arg + return real_get_arg + + +def test_optimise_solar_is_a_no_op_without_export_freeze(my_predbat): + """The premise of the warning: optimise_solar() really does nothing when freeze export is off. + + Asserted directly so the warning cannot outlive the behaviour it describes - if optimise_solar() + ever learns to work without Freeze Export, this fails and the warning must be removed. + """ + print("**** test_optimise_solar_is_a_no_op_without_export_freeze ****") + failed = False + + saved = (my_predbat.calculate_best_export, my_predbat.set_export_freeze, my_predbat.export_window_best) + try: + my_predbat.calculate_best_export = True + my_predbat.set_export_freeze = False + my_predbat.export_window_best = [{"start": 0, "end": 30, "average": 15.0}] + + sentinel = (1.0, 2.0, 3.0, 4.0, 5.0, 6.0) + result = my_predbat.optimise_solar(*sentinel, 1) + + if result != sentinel: + print("ERROR: optimise_solar() should return its inputs untouched when set_export_freeze is off, got {}".format(result)) + failed = True + finally: + my_predbat.calculate_best_export, my_predbat.set_export_freeze, my_predbat.export_window_best = saved + + if not failed: + print("PASS") + return failed + + +def test_warns_once_when_export_more_solar_has_no_effect(my_predbat): + """Warn when export_more_solar is on and set_export_freeze is off - but only once per incident. + + fetch_config_options() runs every cycle (~5 minutes), so an un-throttled warning would spam the + log for a setup the user may have chosen deliberately. Mirrors car_charging_energy_warned. + """ + print("**** test_warns_once_when_export_more_solar_has_no_effect ****") + failed = False + + logged = [] + real_log = my_predbat.log + my_predbat.log = lambda message: logged.append(message) + saved = (my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned) + + def warnings(): + return [message for message in logged if "export_more_solar" in message and "no effect" in message] + + try: + my_predbat.export_more_solar_warned = False + + # First cycle with the broken combination warns + real_get_arg = _override_switches(my_predbat, export_more_solar=True, set_export_freeze=False) + my_predbat.fetch_config_options() + if len(warnings()) != 1: + print("ERROR: expected exactly one warning on the first cycle, got {}".format(len(warnings()))) + failed = True + + # Subsequent cycles stay quiet + logged.clear() + my_predbat.fetch_config_options() + my_predbat.fetch_config_options() + if warnings(): + print("ERROR: the warning repeated on later cycles - it should be logged once per incident, got {}".format(len(warnings()))) + failed = True + + # Fixing the combination re-arms it, so a recurrence is reported again + logged.clear() + _override_switches(my_predbat, export_more_solar=True, set_export_freeze=True) + my_predbat.fetch_config_options() + if my_predbat.export_more_solar_warned: + print("ERROR: the warned flag should reset once the combination is valid again") + failed = True + _override_switches(my_predbat, export_more_solar=True, set_export_freeze=False) + my_predbat.fetch_config_options() + if len(warnings()) != 1: + print("ERROR: expected the warning again after the combination was fixed and re-broken, got {}".format(len(warnings()))) + failed = True + finally: + my_predbat.log = real_log + my_predbat.get_arg = real_get_arg + my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned = saved + + if not failed: + print("PASS") + return failed + + +def test_no_warning_for_valid_combinations(my_predbat): + """The other three combinations are all legitimate and must stay silent.""" + print("**** test_no_warning_for_valid_combinations ****") + failed = False + + logged = [] + real_log = my_predbat.log + real_get_arg = my_predbat.get_arg + my_predbat.log = lambda message: logged.append(message) + saved = (my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned) + + try: + for export_more_solar, set_export_freeze in ((False, False), (False, True), (True, True)): + logged.clear() + my_predbat.export_more_solar_warned = False + _override_switches(my_predbat, export_more_solar, set_export_freeze) + my_predbat.fetch_config_options() + warnings = [message for message in logged if "export_more_solar" in message and "no effect" in message] + if warnings: + print("ERROR: export_more_solar={} set_export_freeze={} is a valid combination but warned: {}".format(export_more_solar, set_export_freeze, warnings)) + failed = True + finally: + my_predbat.log = real_log + my_predbat.get_arg = real_get_arg + my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned = saved + + if not failed: + print("PASS") + return failed + + +def run_export_more_solar_warning_tests(my_predbat): + """Run the export_more_solar dependency warning tests.""" + failed = False + failed |= test_optimise_solar_is_a_no_op_without_export_freeze(my_predbat) + failed |= test_warns_once_when_export_more_solar_has_no_effect(my_predbat) + failed |= test_no_warning_for_valid_combinations(my_predbat) + return failed diff --git a/apps/predbat/unit_test.py b/apps/predbat/unit_test.py index d57918509..606bc4ef7 100644 --- a/apps/predbat/unit_test.py +++ b/apps/predbat/unit_test.py @@ -150,6 +150,7 @@ from tests.test_hit_charge_cache import run_hit_charge_cache_tests from tests.test_window_selection import run_window_selection_tests from tests.test_export_encoding import run_export_encoding_tests +from tests.test_export_more_solar_warning import run_export_more_solar_warning_tests from tests.test_find_charge_rate import test_find_charge_rate, test_find_charge_rate_pv_overlap, test_find_charge_rate_string_temperature, test_find_charge_rate_string_charge_curve from tests.test_manual_api import run_test_manual_api from tests.test_manual_soc import run_test_manual_soc @@ -431,6 +432,7 @@ def main(): TEST_REGISTRY = [ ("secrets", run_secrets_tests, "Secrets loading tests", False), ("export_encoding", run_export_encoding_tests, "Packed export limit encoding accessor tests", False), + ("export_more_solar_warning", run_export_more_solar_warning_tests, "export_more_solar / set_export_freeze dependency warning tests", False), ("perf", run_perf_test, "Performance tests", False), ("model", run_model_tests, "Model tests", False), ("plot", run_plot_tests, "Failure plot display is opt-in (--plot) tests", False), diff --git a/docs/customisation.md b/docs/customisation.md index 941396405..695b17380 100644 --- a/docs/customisation.md +++ b/docs/customisation.md @@ -348,7 +348,7 @@ By default with this option On the latest export slots of the same value will be **switch.predbat_export_more_solar** When turned On, late in the planning stage Predbat will try enabling Freeze Export on every otherwise Idle slot that has predicted solar generation. With Freeze Export the battery is not charged from the surplus solar, so that solar is exported to the grid instead. -This alternative plan is only kept if it does not increase the overall plan metric by more than **input_number.predbat_export_more_solar_threshold** (_expert mode_, default 1p), otherwise the original plan is restored. This lets you favour exporting solar over storing it when doing so is roughly cost-neutral. The feature relies on **switch.predbat_set_export_freeze** being enabled (and your inverter supporting export/discharge freeze); it is an optimiser-only setting and uses the existing Freeze Export execution behaviour. +This alternative plan is only kept if it does not increase the overall plan metric by more than **input_number.predbat_export_more_solar_threshold** (_expert mode_, default 1p), otherwise the original plan is restored. This lets you favour exporting solar over storing it when doing so is roughly cost-neutral. The feature relies on **switch.predbat_set_export_freeze** being enabled (and your inverter supporting export/discharge freeze); it is an optimiser-only setting and uses the existing Freeze Export execution behaviour. If you turn this on while **switch.predbat_set_export_freeze** is off, Predbat will log a warning to say the setting is having no effect. ## Battery margins and metrics options From 609b9c4b5cce8b03453f00bac0e1122e3b954895 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Sat, 26 Sep 2026 09:09:40 +0100 Subject: [PATCH 2/2] fix(fetch): warn when export_more_solar is ineffective for any reason, not just the switch The check read set_export_freeze in fetch_config_options(), before fetch_inverter_data() forces it off for an inverter without freeze support, so that case never warned. It also missed calculate_best_export being off (mode not Control charge & discharge), which short-circuits optimise_solar() the same way. Move the check to check_export_more_solar_effective(), run from update_pred() right after the first successful fetch_inverter_data(), and name the actual cause: mode, an unsupported inverter (asked of the inverter, so the user is never told to enable a switch that would change nothing), or the switch. Track the reason rather than a bool so fixing one cause while another remains warns again instead of going quiet. The premise test now swaps prediction for an object that raises on use, with a positive control, so it tests the guard rather than leftover fixture state. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/fetch.py | 41 ++- apps/predbat/predbat.py | 3 +- .../tests/test_export_more_solar_warning.py | 262 ++++++++++++------ docs/customisation.md | 2 +- 4 files changed, 203 insertions(+), 105 deletions(-) diff --git a/apps/predbat/fetch.py b/apps/predbat/fetch.py index d01d03f0c..bda6f4b5a 100644 --- a/apps/predbat/fetch.py +++ b/apps/predbat/fetch.py @@ -3021,6 +3021,36 @@ def validate_curve(self, curve, curve_name): return validated_curve + def check_export_more_solar_effective(self): + """ + Warn when export_more_solar is on but cannot have any effect. + + export_more_solar works by enabling Freeze Export on idle solar slots, and optimise_solar() returns + immediately unless export windows are being calculated and set_export_freeze is on. Otherwise the + switch is on but does nothing, with no indication anywhere (#4865). Must run after + fetch_inverter_data(), which forces set_export_freeze off for an inverter without freeze support, + so the value read in fetch_config_options() is not yet the one the plan will use. + + Warns once per incident rather than every cycle, as the combination may be deliberate, but again + whenever the reason changes - otherwise fixing one cause would silently leave the next one in place. + Re-arms once export_more_solar is effective so a later recurrence is reported. + """ + reason = None + if self.export_more_solar: + if not self.calculate_best_export: + reason = "Predbat mode is not Control charge & discharge, so export slots are not planned. Change the mode, or turn export_more_solar off." + elif not self.set_export_freeze: + # Ask the inverter rather than the switch: an unsupported inverter forces set_export_freeze off + # whatever the switch says, so advising the user to turn the switch on would change nothing. + if self.inverters and not self.inverters[0].inv_support_discharge_freeze: + reason = "the inverter does not support Freeze Export. Turn export_more_solar off." + else: + reason = "set_export_freeze is off. Enable set_export_freeze, or turn export_more_solar off." + + if reason and reason != self.export_more_solar_warned_reason: + self.log("Warn: export_more_solar is enabled but has no effect, as it works by enabling Freeze Export on idle solar slots and " + reason) + self.export_more_solar_warned_reason = reason + def fetch_config_options(self): """ Fetch all the configuration options @@ -3227,17 +3257,6 @@ def fetch_config_options(self): self.set_freeze_export_during_demand = self.get_arg("set_freeze_export_during_demand") self.export_more_solar = self.get_arg("export_more_solar") self.export_more_solar_threshold = self.get_arg("export_more_solar_threshold") - # export_more_solar works by enabling Freeze Export on idle solar slots, so optimise_solar() - # returns immediately when set_export_freeze is off - the switch is then on but does nothing - # at all, with no indication anywhere. Warn once per incident rather than every cycle (same - # reasoning as car_charging_energy_warned below), and reset when the combination is fixed so - # a later recurrence is reported again. - if self.export_more_solar and not self.set_export_freeze: - if not self.export_more_solar_warned: - self.log("Warn: export_more_solar is enabled but set_export_freeze is off - export_more_solar has no effect, as it works by enabling Freeze Export on idle solar slots. Enable set_export_freeze, or turn export_more_solar off.") - self.export_more_solar_warned = True - else: - self.export_more_solar_warned = False # Mode self.predbat_mode = self.get_arg("mode") if self.predbat_mode == PREDBAT_MODE_OPTIONS[PREDBAT_MODE_CONTROL_SOC]: diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index e87ce40af..a4e7340c2 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -398,7 +398,7 @@ def reset(self): self.metric_min_improvement_plan = 2.0 self.export_more_solar = False self.export_more_solar_threshold = 1.0 - self.export_more_solar_warned = False + self.export_more_solar_warned_reason = None self.metric_battery_cycle = 0.0 self.metric_battery_value_scaling = 1.0 self.metric_battery_value_export_scaling = 0.8 @@ -1087,6 +1087,7 @@ def update_pred(self, scheduled=True): self.log("Error: Failed to fetch inverter data, not able to compute a plan") self.record_status("Error: Failed to fetch inverter data, not able to compute a plan", had_errors=True) return + self.check_export_more_solar_effective() # Check if we have valid import rates if self.rate_min == self.rate_max == 0: diff --git a/apps/predbat/tests/test_export_more_solar_warning.py b/apps/predbat/tests/test_export_more_solar_warning.py index 2fb0f6f32..e85d3b013 100644 --- a/apps/predbat/tests/test_export_more_solar_warning.py +++ b/apps/predbat/tests/test_export_more_solar_warning.py @@ -10,145 +10,221 @@ """Tests for the export_more_solar / set_export_freeze dependency warning. export_more_solar works by enabling Freeze Export on otherwise-idle slots that have predicted solar, -so optimise_solar() returns immediately when set_export_freeze is off (plan.py). The switch is then -on but does nothing whatsoever, and nothing anywhere said so - a real system in issue #4865 had run -that combination for months. +so optimise_solar() returns immediately unless export slots are being planned (calculate_best_export) +and set_export_freeze is on (plan.py). The switch is then on but does nothing whatsoever, and nothing +anywhere said so - a real system in issue #4865 had run that combination for months. + +set_export_freeze can be off either because the user turned it off or because fetch_inverter_data() +forced it off for an inverter without freeze support, so the check runs after that, reads the +effective value, and asks the inverter which of the two it was. The docs already state the dependency ("The feature relies on switch.predbat_set_export_freeze being enabled"), so this is purely about telling a user who has not read them. """ -def _override_switches(my_predbat, export_more_solar, set_export_freeze): - """Force the two switches under test for the next fetch_config_options() call. +SAVED_ATTRS = ("export_more_solar", "calculate_best_export", "set_export_freeze", "export_more_solar_warned_reason", "inverters") + + +class _StubInverter: + """Just the capability flag the check reads.""" + + def __init__(self, inv_support_discharge_freeze): + """Record whether this inverter supports export/discharge freeze.""" + self.inv_support_discharge_freeze = inv_support_discharge_freeze + + +class _Harness: + """Capture log output and control the inverter's freeze capability, restoring both on exit.""" + + def __init__(self, my_predbat): + """Remember the fixture under test.""" + self.my_predbat = my_predbat + self.logged = [] + + def __enter__(self): + """Install the log capture and start from a clean warned state.""" + my_predbat = self.my_predbat + self.real_log = my_predbat.log + self.saved = tuple(getattr(my_predbat, attr) for attr in SAVED_ATTRS) + my_predbat.log = lambda message: self.logged.append(message) + my_predbat.export_more_solar_warned_reason = None + return self + + def __exit__(self, *exc): + """Restore everything touched.""" + my_predbat = self.my_predbat + my_predbat.log = self.real_log + for attr, value in zip(SAVED_ATTRS, self.saved): + setattr(my_predbat, attr, value) + return False + + def run(self, export_more_solar=True, calculate_best_export=True, set_export_freeze=True, inverter_supports_freeze=True): + """Set the effective state, run one check, and return the warnings it logged.""" + my_predbat = self.my_predbat + my_predbat.export_more_solar = export_more_solar + my_predbat.calculate_best_export = calculate_best_export + my_predbat.set_export_freeze = set_export_freeze + my_predbat.inverters = [_StubInverter(inverter_supports_freeze)] + self.logged.clear() + my_predbat.check_export_more_solar_effective() + return [message for message in self.logged if "export_more_solar" in message and "no effect" in message] - fetch_config_options() re-reads every setting through get_arg(), so setting the attributes - directly would just be overwritten. Intercept those two keys and let everything else through, - which keeps the rest of the real config path in play. - """ - real_get_arg = my_predbat.get_arg - overrides = {"export_more_solar": export_more_solar, "set_export_freeze": set_export_freeze} - def get_arg(arg, *args, **kwargs): - if arg in overrides: - return overrides[arg] - return real_get_arg(arg, *args, **kwargs) +class _Unreachable: + """Stands in for self.prediction: any use means optimise_solar() got past its guard.""" - my_predbat.get_arg = get_arg - return real_get_arg + def __getattr__(self, name): + """Fail loudly on any attribute access.""" + raise AssertionError("optimise_solar() read prediction.{} - it got past its guard".format(name)) -def test_optimise_solar_is_a_no_op_without_export_freeze(my_predbat): - """The premise of the warning: optimise_solar() really does nothing when freeze export is off. +def test_optimise_solar_is_a_no_op_when_warned(my_predbat): + """The premise of the warning: optimise_solar() really does nothing in each case that warns. Asserted directly so the warning cannot outlive the behaviour it describes - if optimise_solar() - ever learns to work without Freeze Export, this fails and the warning must be removed. + ever learns to work in one of these states, this fails and the warning must be changed. prediction + is swapped for an object that raises on use, so the guard is what is tested rather than whatever an + earlier test happened to leave in the fixture; the effective case is the positive control. """ - print("**** test_optimise_solar_is_a_no_op_without_export_freeze ****") + print("**** test_optimise_solar_is_a_no_op_when_warned ****") failed = False - saved = (my_predbat.calculate_best_export, my_predbat.set_export_freeze, my_predbat.export_window_best) + # prediction only exists once a plan has been calculated, so it may be absent depending on test order + had_prediction = hasattr(my_predbat, "prediction") + saved = (my_predbat.calculate_best_export, my_predbat.set_export_freeze, my_predbat.export_window_best, getattr(my_predbat, "prediction", None)) try: - my_predbat.calculate_best_export = True - my_predbat.set_export_freeze = False my_predbat.export_window_best = [{"start": 0, "end": 30, "average": 15.0}] - + my_predbat.prediction = _Unreachable() sentinel = (1.0, 2.0, 3.0, 4.0, 5.0, 6.0) - result = my_predbat.optimise_solar(*sentinel, 1) + for calculate_best_export, set_export_freeze in ((True, False), (False, True)): + my_predbat.calculate_best_export = calculate_best_export + my_predbat.set_export_freeze = set_export_freeze + try: + result = my_predbat.optimise_solar(*sentinel, 1) + except AssertionError as error: + print("ERROR: calculate_best_export={} set_export_freeze={}: {}".format(calculate_best_export, set_export_freeze, error)) + failed = True + continue + if result != sentinel: + print("ERROR: optimise_solar() should return its inputs untouched with calculate_best_export={} set_export_freeze={}, got {}".format(calculate_best_export, set_export_freeze, result)) + failed = True - if result != sentinel: - print("ERROR: optimise_solar() should return its inputs untouched when set_export_freeze is off, got {}".format(result)) + my_predbat.calculate_best_export = True + my_predbat.set_export_freeze = True + try: + my_predbat.optimise_solar(*sentinel, 1) + print("ERROR: positive control - optimise_solar() should get past its guard when effective, so this test is not exercising the guard") failed = True + except AssertionError: + pass finally: - my_predbat.calculate_best_export, my_predbat.set_export_freeze, my_predbat.export_window_best = saved + my_predbat.calculate_best_export, my_predbat.set_export_freeze, my_predbat.export_window_best, my_predbat.prediction = saved + if not had_prediction: + del my_predbat.prediction if not failed: print("PASS") return failed -def test_warns_once_when_export_more_solar_has_no_effect(my_predbat): - """Warn when export_more_solar is on and set_export_freeze is off - but only once per incident. +def test_warns_with_the_actual_reason(my_predbat): + """Each ineffective state warns, and the message names the cause so the advice is actionable. - fetch_config_options() runs every cycle (~5 minutes), so an un-throttled warning would spam the - log for a setup the user may have chosen deliberately. Mirrors car_charging_energy_warned. + An inverter without freeze support must not be told to enable set_export_freeze - including when + the switch is also off, as turning it on would change nothing. """ - print("**** test_warns_once_when_export_more_solar_has_no_effect ****") + print("**** test_warns_with_the_actual_reason ****") failed = False - logged = [] - real_log = my_predbat.log - my_predbat.log = lambda message: logged.append(message) - saved = (my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned) + cases = ( + ("mode", dict(calculate_best_export=False), "mode"), + ("user switch off", dict(set_export_freeze=False), "Enable set_export_freeze"), + ("inverter unsupported", dict(set_export_freeze=False, inverter_supports_freeze=False), "does not support Freeze Export"), + ) + with _Harness(my_predbat) as harness: + for name, state, expected in cases: + my_predbat.export_more_solar_warned_reason = None + warnings = harness.run(**state) + if len(warnings) != 1 or expected not in warnings[0]: + print("ERROR: {}: expected one warning mentioning '{}', got {}".format(name, expected, warnings)) + failed = True + elif name == "inverter unsupported" and "Enable set_export_freeze" in warnings[0]: + print("ERROR: {}: must not advise enabling set_export_freeze, got {}".format(name, warnings[0])) + failed = True + + if not failed: + print("PASS") + return failed + - def warnings(): - return [message for message in logged if "export_more_solar" in message and "no effect" in message] +def test_warns_once_per_incident(my_predbat): + """Warn once, stay quiet on later cycles, and re-arm once the setting becomes effective again. - try: - my_predbat.export_more_solar_warned = False + The check runs every cycle (~5 minutes), so an un-throttled warning would spam the log for a setup + the user may have chosen deliberately. + """ + print("**** test_warns_once_per_incident ****") + failed = False - # First cycle with the broken combination warns - real_get_arg = _override_switches(my_predbat, export_more_solar=True, set_export_freeze=False) - my_predbat.fetch_config_options() - if len(warnings()) != 1: - print("ERROR: expected exactly one warning on the first cycle, got {}".format(len(warnings()))) + with _Harness(my_predbat) as harness: + if len(harness.run(set_export_freeze=False)) != 1: + print("ERROR: expected exactly one warning on the first cycle") failed = True - - # Subsequent cycles stay quiet - logged.clear() - my_predbat.fetch_config_options() - my_predbat.fetch_config_options() - if warnings(): - print("ERROR: the warning repeated on later cycles - it should be logged once per incident, got {}".format(len(warnings()))) + if harness.run(set_export_freeze=False) or harness.run(set_export_freeze=False): + print("ERROR: the warning repeated on later cycles - it should be logged once per incident") failed = True - - # Fixing the combination re-arms it, so a recurrence is reported again - logged.clear() - _override_switches(my_predbat, export_more_solar=True, set_export_freeze=True) - my_predbat.fetch_config_options() - if my_predbat.export_more_solar_warned: - print("ERROR: the warned flag should reset once the combination is valid again") + harness.run() + if my_predbat.export_more_solar_warned_reason is not None: + print("ERROR: the warned state should reset once export_more_solar is effective again") failed = True - _override_switches(my_predbat, export_more_solar=True, set_export_freeze=False) - my_predbat.fetch_config_options() - if len(warnings()) != 1: - print("ERROR: expected the warning again after the combination was fixed and re-broken, got {}".format(len(warnings()))) + if len(harness.run(calculate_best_export=False)) != 1: + print("ERROR: expected the warning again after the setting was effective and then broken again") failed = True - finally: - my_predbat.log = real_log - my_predbat.get_arg = real_get_arg - my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned = saved if not failed: print("PASS") return failed -def test_no_warning_for_valid_combinations(my_predbat): - """The other three combinations are all legitimate and must stay silent.""" - print("**** test_no_warning_for_valid_combinations ****") +def test_warns_again_when_the_reason_changes(my_predbat): + """Fixing one cause while another remains must report the remaining one. + + Otherwise a user who follows the first warning's advice sees the log go quiet and reasonably + concludes export_more_solar now works, when it still does nothing. + """ + print("**** test_warns_again_when_the_reason_changes ****") failed = False - logged = [] - real_log = my_predbat.log - real_get_arg = my_predbat.get_arg - my_predbat.log = lambda message: logged.append(message) - saved = (my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned) + with _Harness(my_predbat) as harness: + first = harness.run(calculate_best_export=False, set_export_freeze=False) + second = harness.run(set_export_freeze=False) + if len(first) != 1 or "mode" not in first[0]: + print("ERROR: expected a mode warning first, got {}".format(first)) + failed = True + if len(second) != 1 or "Enable set_export_freeze" not in second[0]: + print("ERROR: after fixing the mode, expected a set_export_freeze warning, got {}".format(second)) + failed = True - try: - for export_more_solar, set_export_freeze in ((False, False), (False, True), (True, True)): - logged.clear() - my_predbat.export_more_solar_warned = False - _override_switches(my_predbat, export_more_solar, set_export_freeze) - my_predbat.fetch_config_options() - warnings = [message for message in logged if "export_more_solar" in message and "no effect" in message] + if not failed: + print("PASS") + return failed + + +def test_no_warning_when_effective_or_off(my_predbat): + """export_more_solar off never warns, whatever else is set; fully enabled never warns either.""" + print("**** test_no_warning_when_effective_or_off ****") + failed = False + + with _Harness(my_predbat) as harness: + states = [dict(export_more_solar=False, calculate_best_export=c, set_export_freeze=f, inverter_supports_freeze=i) for c in (False, True) for f in (False, True) for i in (False, True)] + states.append(dict()) + for state in states: + my_predbat.export_more_solar_warned_reason = None + warnings = harness.run(**state) if warnings: - print("ERROR: export_more_solar={} set_export_freeze={} is a valid combination but warned: {}".format(export_more_solar, set_export_freeze, warnings)) + print("ERROR: {} should not warn, got {}".format(state, warnings)) failed = True - finally: - my_predbat.log = real_log - my_predbat.get_arg = real_get_arg - my_predbat.export_more_solar, my_predbat.set_export_freeze, my_predbat.export_more_solar_warned = saved if not failed: print("PASS") @@ -158,7 +234,9 @@ def test_no_warning_for_valid_combinations(my_predbat): def run_export_more_solar_warning_tests(my_predbat): """Run the export_more_solar dependency warning tests.""" failed = False - failed |= test_optimise_solar_is_a_no_op_without_export_freeze(my_predbat) - failed |= test_warns_once_when_export_more_solar_has_no_effect(my_predbat) - failed |= test_no_warning_for_valid_combinations(my_predbat) + failed |= test_optimise_solar_is_a_no_op_when_warned(my_predbat) + failed |= test_warns_with_the_actual_reason(my_predbat) + failed |= test_warns_once_per_incident(my_predbat) + failed |= test_warns_again_when_the_reason_changes(my_predbat) + failed |= test_no_warning_when_effective_or_off(my_predbat) return failed diff --git a/docs/customisation.md b/docs/customisation.md index 695b17380..cc689ad56 100644 --- a/docs/customisation.md +++ b/docs/customisation.md @@ -348,7 +348,7 @@ By default with this option On the latest export slots of the same value will be **switch.predbat_export_more_solar** When turned On, late in the planning stage Predbat will try enabling Freeze Export on every otherwise Idle slot that has predicted solar generation. With Freeze Export the battery is not charged from the surplus solar, so that solar is exported to the grid instead. -This alternative plan is only kept if it does not increase the overall plan metric by more than **input_number.predbat_export_more_solar_threshold** (_expert mode_, default 1p), otherwise the original plan is restored. This lets you favour exporting solar over storing it when doing so is roughly cost-neutral. The feature relies on **switch.predbat_set_export_freeze** being enabled (and your inverter supporting export/discharge freeze); it is an optimiser-only setting and uses the existing Freeze Export execution behaviour. If you turn this on while **switch.predbat_set_export_freeze** is off, Predbat will log a warning to say the setting is having no effect. +This alternative plan is only kept if it does not increase the overall plan metric by more than **input_number.predbat_export_more_solar_threshold** (_expert mode_, default 1p), otherwise the original plan is restored. This lets you favour exporting solar over storing it when doing so is roughly cost-neutral. The feature relies on **switch.predbat_set_export_freeze** being enabled (and your inverter supporting export/discharge freeze); it is an optimiser-only setting and uses the existing Freeze Export execution behaviour. If you turn this on when it cannot have any effect - **switch.predbat_set_export_freeze** is off, your inverter does not support freeze export, or **select.predbat_mode** is not _Control charge & discharge_ - Predbat will log a warning saying so. ## Battery margins and metrics options