diff --git a/apps/predbat/execute.py b/apps/predbat/execute.py index 2076b2499..47eff3076 100644 --- a/apps/predbat/execute.py +++ b/apps/predbat/execute.py @@ -855,6 +855,17 @@ def execute_plan(self): "reset_rates": reset_rates, } + # The rates above are only written after this loop (apply_rate_intent), but the immediate + # service calls below send {power} now - so they are handed this cycle's rate rather than + # reading back the last one (#5252). An unclaimed rate goes to maximum when rates are + # being reset, as apply_inverter_rates() will do; otherwise the stored rate stands (None). + # Truncated with int() exactly as apply_inverter_rates() truncates. The service dedup then + # holds as long as the stored rate reads back as written; a rate stored as a percentage, or + # rounded to an entity's step, can read back slightly off (up to 1% of maximum), which costs + # at most one extra start call per rate change. + charge_power = int(charge_rate) if charge_rate is not None else (int(inverter.battery_rate_max_charge * MINUTE_WATT) if reset_rates else None) + discharge_power = int(discharge_rate) if discharge_rate is not None else (int(inverter.battery_rate_max_discharge * MINUTE_WATT) if reset_rates else None) + # Set the SoC just before or within the charge window if self.set_soc_enable: if isExporting: @@ -875,11 +886,11 @@ def execute_plan(self): # Immediate controls if self.set_export_freeze and export_mode_of(self.export_limits_best[0]) == EXPORT_MODE_FREEZE: - inverter.adjust_export_immediate(inverter.soc_percent, freeze=True) + inverter.adjust_export_immediate(inverter.soc_percent, freeze=True, rate=discharge_power) elif not disabled_export: - inverter.adjust_export_immediate(export_target_percent) + inverter.adjust_export_immediate(export_target_percent, rate=discharge_power) else: - inverter.adjust_export_immediate(int(EXPORT_LIMIT_IDLE)) # Dead code right, but kept in case other logic changes + inverter.adjust_export_immediate(int(EXPORT_LIMIT_IDLE), rate=discharge_power) # Dead code right, but kept in case other logic changes elif self.charge_limit_best and (self.minutes_now < inverter.charge_end_time_minutes) and ((inverter.charge_start_time_minutes - self.minutes_now) <= self.set_soc_minutes) and not (disabled_charge_window): if inverter.inv_has_charge_enable_time or isCharging: @@ -889,9 +900,9 @@ def execute_plan(self): inv_target_soc_percent = self.adjust_battery_target_multi(inverter, calc_percent_limit(self.soc_kw, self.soc_max), isCharging, isExporting, isFreezeCharge=True) self.log("Inverter {} within charge freeze setting target SoC to SoC {} global target {}".format(inverter.id, dp0(inv_target_soc_percent), dp0(self.soc_kw))) if inverter.soc_kw >= inverter.reserve: - inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=True) + inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=True, rate=charge_power) else: - inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=False) + inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=False, rate=charge_power) elif not inverter.inv_has_target_soc: self.log("Inverter {} setting charging SoC to 0% as we are not charging and inverter doesn't support target SoC".format(inverter.id)) self.adjust_battery_target_multi(inverter, 0, isCharging, isExporting) @@ -908,7 +919,7 @@ def execute_plan(self): target_soc = calc_percent_limit(max(self.charge_limit_best[0], self.reserve), self.soc_max) self.log("Inverter {} setting charging SoC to {}% as per target".format(inverter.id, target_soc)) inv_target_soc = self.adjust_battery_target_multi(inverter, target_soc, isCharging, isExporting) - inverter.adjust_charge_immediate(inv_target_soc) + inverter.adjust_charge_immediate(inv_target_soc, rate=charge_power) elif not inverter.inv_has_target_soc: self.log("Inverter {} setting charging SoC to 0% as we are not charging and inverter doesn't support target SoC".format(inverter.id)) self.adjust_battery_target_multi(inverter, 0, isCharging, isExporting) @@ -954,10 +965,10 @@ def execute_plan(self): if isCharging: if self.is_freeze_charge(self.charge_limit_best[0]): inv_target_soc_percent = self.adjust_battery_target_multi(inverter, calc_percent_limit(self.soc_kw, self.soc_max), isCharging, isExporting, check=True, isFreezeCharge=True) - inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=True) + inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=True, rate=charge_power) else: inv_target_soc_percent = self.adjust_battery_target_multi(inverter, calc_percent_limit(max(self.charge_limit_best[0], self.reserve), self.soc_max), isCharging, isExporting, check=True) - inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=True) + inverter.adjust_charge_immediate(inv_target_soc_percent, freeze=True, rate=charge_power) # Charging/Discharging off via service # Skipped while exporting: adjust_export_immediate() above already issues its own @@ -967,11 +978,11 @@ def execute_plan(self): # mode discharge_start_service had just set (GH#4165, GH#4641). if not isCharging and not isExporting and self.set_charge_window: if carHolding or boostHolding: - inverter.adjust_charge_immediate(inverter.soc_percent, freeze=True) + inverter.adjust_charge_immediate(inverter.soc_percent, freeze=True, rate=charge_power) else: - inverter.adjust_charge_immediate(0) + inverter.adjust_charge_immediate(0, rate=charge_power) if not isExporting and self.set_export_window: - inverter.adjust_export_immediate(int(EXPORT_LIMIT_IDLE)) + inverter.adjust_export_immediate(int(EXPORT_LIMIT_IDLE), rate=discharge_power) # Reset reserve as discharge is enable but not running right now if self.set_reserve_enable and resetReserve: diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 3386df5cc..5a2c96055 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -2043,6 +2043,16 @@ def get_current_discharge_rate(self): return current_rate + def rate_after_deadband(self, current_rate, new_rate, rate_max): + """ + The rate adjust_charge_rate()/adjust_discharge_rate() leave in place: new_rate, or current_rate + when the change is within their deadband (5% of rate_max, the battery's maximum rate in kWh per + minute, so the deadband in W is rate_max * MINUTE_WATT / 20) + """ + if abs(current_rate - new_rate) > (rate_max * MINUTE_WATT / 20): + return new_rate + return current_rate + def get_current_charge_rate(self): """ Get the current charge rate in watts @@ -2083,7 +2093,7 @@ def adjust_charge_rate(self, new_rate, notify=True): new_rate = int(new_rate + 0.5) current_rate = self.get_current_charge_rate() - if abs(current_rate - new_rate) > (self.battery_rate_max_charge * MINUTE_WATT / 20): + if self.rate_after_deadband(current_rate, new_rate, self.battery_rate_max_charge) != current_rate: self.base.log("Inverter {} current charge rate is {}W and new target is {}W".format(self.id, current_rate, new_rate)) if "charge_rate" in self.base.args: self.write_and_poll_value("charge_rate", self.base.get_arg("charge_rate", indirect=False, index=self.id, required_unit="W"), new_rate, fuzzy=(self.battery_rate_max_charge * MINUTE_WATT / 20), required_unit="W") @@ -2124,7 +2134,7 @@ def adjust_discharge_rate(self, new_rate, notify=True): new_rate = int(new_rate + 0.5) current_rate = self.get_current_discharge_rate() - if abs(current_rate - new_rate) > (self.battery_rate_max_discharge * MINUTE_WATT / 20): + if self.rate_after_deadband(current_rate, new_rate, self.battery_rate_max_discharge) != current_rate: self.base.log("Inverter {} current discharge rate is {}W and new target is {}W".format(self.id, current_rate, new_rate)) if "discharge_rate" in self.base.args: self.write_and_poll_value("discharge_rate", self.base.get_arg("discharge_rate", indirect=False, index=self.id), new_rate, fuzzy=(self.battery_rate_max_discharge * MINUTE_WATT / 20), required_unit="W") @@ -3188,14 +3198,25 @@ def call_service_template(self, service, data, domain="charge", extra_data=None) # transient failure would silently downgrade a freeze into a stop rather than retrying it. return True - def adjust_charge_immediate(self, target_soc, freeze=False): + def adjust_charge_immediate(self, target_soc, freeze=False, rate=None): """ Adjust from charging or not charging based on passed target soc + + rate is the charge rate (W) execute_plan() intends for this cycle. Rates are written after its + per-inverter loop, so reading the stored rate here would send the previous cycle's to the + service as {power} (#5252). None falls back to the stored rate. + + With no charge_rate entity at all (a "power"-controlled inverter without the dummy rate + entity), the stored rate always reads as the maximum, so the deadband is measured against + that rather than the last rate sent: a planned rate is sent as-is unless it is within the + deadband of the maximum, and each change in it sends a new start call. """ service_data_stop = {"device_id": self.base.get_arg("device_id", index=self.id, default="")} extra_data = {"charge_start_time": self.base.get_arg("charge_start_time", index=self.id, default="00:00:00"), "charge_end_time": self.base.get_arg("charge_end_time", index=self.id, default="00:00:00")} if target_soc > 0: current_rate = self.get_current_charge_rate() + if rate is not None: + current_rate = self.rate_after_deadband(current_rate, int(rate + 0.5), self.battery_rate_max_charge) service_data = { "device_id": self.base.get_arg("device_id", index=self.id, default=""), "target_soc": int(target_soc), @@ -3227,21 +3248,28 @@ def adjust_charge_immediate(self, target_soc, freeze=False): else: self.call_service_template("charge_stop_service", service_data_stop, domain="charge") - def adjust_export_immediate(self, target_soc, freeze=False): + def adjust_export_immediate(self, target_soc, freeze=False, rate=None): """ Adjust from exporting or not exporting based on passed target soc + + rate is the discharge rate (W) execute_plan() intends for this cycle - see + adjust_charge_immediate(). None falls back to the stored rate. """ service_data_stop = {"device_id": self.base.get_arg("device_id", index=self.id, default="")} extra_data = {"discharge_start_time": self.base.get_arg("discharge_start_time", index=self.id, default="00:00:00"), "discharge_end_time": self.base.get_arg("discharge_end_time", index=self.id, default="00:00:00")} if target_soc < 100: # Mirrors adjust_charge_immediate()'s charge_start_service payload just above - the - # actual (possibly low-power-scaled) rate already set via adjust_discharge_rate(), not - # always the inverter's maximum, which produced a full-power discharge_start_service - # call even during a planned low-power export (batpred#4619). + # planned (possibly low-power-scaled) rate, not always the inverter's maximum, which + # produced a full-power discharge_start_service call even during a planned low-power + # export (batpred#4619). Passed in as rate, since execute_plan() writes the rate itself + # later in the cycle (#5252). + current_rate = self.get_current_discharge_rate() + if rate is not None: + current_rate = self.rate_after_deadband(current_rate, int(rate + 0.5), self.battery_rate_max_discharge) service_data = { "device_id": self.base.get_arg("device_id", index=self.id, default=""), "target_soc": int(target_soc), - "power": int(self.get_current_discharge_rate()), + "power": int(current_rate), } # Stop charge diff --git a/apps/predbat/tests/test_execute.py b/apps/predbat/tests/test_execute.py index 0f1b7fd46..45f18777a 100644 --- a/apps/predbat/tests/test_execute.py +++ b/apps/predbat/tests/test_execute.py @@ -33,6 +33,8 @@ def __init__(self, id, soc_kw, soc_max, now_utc): self.immediate_discharge_soc_target = -1 self.immediate_charge_soc_freeze = False self.immediate_discharge_soc_freeze = False + self.immediate_charge_power = None + self.immediate_discharge_power = None self.charge_start_time_minutes = -1 self.charge_end_time_minutes = -1 self.charge_rate = 1000 @@ -114,13 +116,18 @@ def adjust_charge_window(self, charge_start_time, charge_end_time, minutes_now): self.charge_time_enable = True # print("Charge start_time {} charge_end_time {}".format(self.charge_start_time_minutes, self.charge_end_time_minutes)) - def adjust_charge_immediate(self, target_soc, freeze=False): + def adjust_charge_immediate(self, target_soc, freeze=False, rate=None): self.immediate_charge_soc_target = target_soc self.immediate_charge_soc_freeze = freeze + # The {power} the real service call would carry: the passed rate, else the stored one (#5252) + if target_soc > 0: + self.immediate_charge_power = rate if rate is not None else self.charge_rate - def adjust_export_immediate(self, target_soc, freeze=False): + def adjust_export_immediate(self, target_soc, freeze=False, rate=None): self.immediate_discharge_soc_target = target_soc self.immediate_discharge_soc_freeze = freeze + if target_soc < 100: + self.immediate_discharge_power = rate if rate is not None else self.discharge_rate def adjust_force_export(self, force_export, new_start_time=None, new_end_time=None): self.force_export = force_export @@ -319,6 +326,8 @@ def run_execute_test( # rather than inheriting whatever the previous scenario in this run happened to leave behind. inverter.immediate_charge_soc_target = -1 inverter.immediate_discharge_soc_target = -1 + inverter.immediate_charge_power = None + inverter.immediate_discharge_power = None if soc_kw_array: inverter.soc_kw = soc_kw_array[inverter.id] else: @@ -495,6 +504,14 @@ def run_execute_test( if assert_status in ["Freeze exporting"] and inverter.immediate_discharge_soc_freeze is not True: print("ERROR: Inverter {} Immediate export SOC freeze should be True got {}".format(inverter.id, inverter.immediate_discharge_soc_freeze)) failed = True + # A service-driven inverter is sent {power} in the same cycle the rate is planned, so it must be + # the rate this cycle writes - not the previous cycle's (#5252) + if inverter.immediate_charge_power is not None and inverter.immediate_charge_power != inverter.charge_rate: + print("ERROR: Inverter {} charge service power {} should match this cycle's charge rate {}".format(inverter.id, inverter.immediate_charge_power, inverter.charge_rate)) + failed = True + if inverter.immediate_discharge_power is not None and inverter.immediate_discharge_power != inverter.discharge_rate: + print("ERROR: Inverter {} export service power {} should match this cycle's discharge rate {}".format(inverter.id, inverter.immediate_discharge_power, inverter.discharge_rate)) + failed = True # Validate isCharging binary sensor state: must be True for any charging status (Freeze charging, Hold charging, Charging variants) charging_statuses = ["Charging", "Freeze charging", "Hold charging"] diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 233c9fa2c..b981b0e0c 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -1457,6 +1457,105 @@ def adjust_charge_immediate(self, target_soc, freeze=False) return failed +def test_immediate_service_power_uses_passed_rate(test_name, my_predbat, ha, inv, dummy_items): + """ + #5252: execute_plan() writes rates after its per-inverter loop, so it passes this cycle's rate to + adjust_charge_immediate()/adjust_export_immediate() for the service's {power}. The passed rate + goes through the same 5% deadband adjust_charge_rate() applies, so a small change keeps the + stored rate - and the service payload - steady instead of re-sending a slightly different + power (and a fresh service call) every cycle. With no rate entity the deadband is measured + against the maximum instead, so a small change is still sent. + """ + print("**** Running Test: {} ****".format(test_name)) + failed = False + + saved_args = copy.deepcopy(my_predbat.args) + saved = (inv.battery_rate_max_charge, inv.battery_rate_max_discharge, inv.soc_percent, ha.service_store_enable, dict(my_predbat.last_service_hash), inv.battery_rate_max_raw) + try: + inv.battery_rate_max_charge = 3000 / MINUTE_WATT # deadband 150W + inv.battery_rate_max_discharge = 3000 / MINUTE_WATT + inv.soc_percent = 50 + my_predbat.args["charge_start_service"] = "charge_start" + my_predbat.args["charge_freeze_service"] = None + my_predbat.args["charge_stop_service"] = "charge_stop" + my_predbat.args["discharge_start_service"] = "discharge_start" + my_predbat.args["discharge_stop_service"] = "discharge_stop" + my_predbat.args["discharge_freeze_service"] = None + my_predbat.args["charge_rate"] = "number.charge_rate" + my_predbat.args["discharge_rate"] = "number.discharge_rate" + my_predbat.args["device_id"] = "DID0" + for key in ["charge_rate_percent", "discharge_rate_percent"]: + my_predbat.args.pop(key, None) + dummy_items["number.charge_rate"] = 1100 + dummy_items["number.discharge_rate"] = 1100 + ha.service_store_enable = True + + def power_sent(call, service, **kwargs): + """Run one immediate call on a fresh dedup state and return the {power} sent to service.""" + my_predbat.last_service_hash.clear() + ha.service_store = [] + call(**kwargs) + powers = [data.get("power") for name, data in ha.get_service_store() if name == service] + return powers[0] if len(powers) == 1 else powers + + cases = ( + ("charge, no rate: stored rate", inv.adjust_charge_immediate, "charge_start", dict(target_soc=100), 1100), + ("charge, new rate", inv.adjust_charge_immediate, "charge_start", dict(target_soc=100, rate=2500), 2500), + ("charge, rate within deadband", inv.adjust_charge_immediate, "charge_start", dict(target_soc=100, rate=1200), 1100), + ("export, no rate: stored rate", inv.adjust_export_immediate, "discharge_start", dict(target_soc=10), 1100), + ("export, new rate", inv.adjust_export_immediate, "discharge_start", dict(target_soc=10, rate=2500), 2500), + ("export, rate within deadband", inv.adjust_export_immediate, "discharge_start", dict(target_soc=10, rate=1200), 1100), + ) + for name, call, service, kwargs, expected in cases: + got = power_sent(call, service, **kwargs) + if got != expected: + print("ERROR: {} {}: {} should be sent power {} got {}".format(test_name, name, service, expected, got)) + failed = True + + # No rate entity at all (a "power"-controlled inverter without the dummy rate entity): the + # stored rate always reads as the maximum, so the deadband is measured against that and a + # planned rate is sent as-is - including a small change from the last one sent. + my_predbat.args.pop("charge_rate", None) + my_predbat.args.pop("discharge_rate", None) + inv.battery_rate_max_raw = 3000 + cases = ( + ("charge, no entity, no rate: maximum", inv.adjust_charge_immediate, "charge_start", dict(target_soc=100), 3000), + ("charge, no entity, new rate", inv.adjust_charge_immediate, "charge_start", dict(target_soc=100, rate=1000), 1000), + ("charge, no entity, within deadband of maximum", inv.adjust_charge_immediate, "charge_start", dict(target_soc=100, rate=2900), 3000), + ("export, no entity, no rate: maximum", inv.adjust_export_immediate, "discharge_start", dict(target_soc=10), 3000), + ("export, no entity, new rate", inv.adjust_export_immediate, "discharge_start", dict(target_soc=10, rate=1000), 1000), + ) + for name, call, service, kwargs, expected in cases: + got = power_sent(call, service, **kwargs) + if got != expected: + print("ERROR: {} {}: {} should be sent power {} got {}".format(test_name, name, service, expected, got)) + failed = True + + # Two consecutive cycles with a small change (inside the deadband): nothing is stored to measure + # against, so the second is not held at the first and goes out as a fresh start call. + for name, call, service, kwargs in ( + ("charge", inv.adjust_charge_immediate, "charge_start", dict(target_soc=100)), + ("export", inv.adjust_export_immediate, "discharge_start", dict(target_soc=10)), + ): + my_predbat.last_service_hash.clear() + ha.service_store = [] + call(rate=1000, **kwargs) + call(rate=1050, **kwargs) + got = [data.get("power") for called, data in ha.get_service_store() if called == service] + if got != [1000, 1050]: + print("ERROR: {} {}, no entity, small change: {} should be sent power [1000, 1050] got {}".format(test_name, name, service, got)) + failed = True + finally: + my_predbat.args.clear() + my_predbat.args.update(saved_args) + inv.battery_rate_max_charge, inv.battery_rate_max_discharge, inv.soc_percent, ha.service_store_enable, hash_saved, inv.battery_rate_max_raw = saved + my_predbat.last_service_hash.clear() + my_predbat.last_service_hash.update(hash_saved) + ha.service_store = [] + + return failed + + def test_call_adjust_export_immediate(test_name, my_predbat, ha, inv, dummy_items, soc, repeat=False, freeze=False, clear=False, charge_stop=False, discharge_start_time="00:00:00", discharge_end_time="23:55:00", no_freeze=False): """ Tests; @@ -5333,6 +5432,7 @@ def run_inverter_tests(my_predbat_dummy): failed |= test_call_adjust_charge_immediate("charge_immediate_freeze_no_service_repeat", my_predbat, ha, inv, dummy_items, 75, freeze=True, no_freeze=True, repeat=True) failed |= test_call_adjust_export_immediate("export_immediate_freeze_no_service", my_predbat, ha, inv, dummy_items, 50, freeze=True, no_freeze=True, clear=True) failed |= test_call_adjust_export_immediate("export_immediate_freeze_no_service_repeat", my_predbat, ha, inv, dummy_items, 30, freeze=True, no_freeze=True, repeat=True) + failed |= test_immediate_service_power_uses_passed_rate("immediate_service_power_passed_rate", my_predbat, ha, inv, dummy_items) if failed: return failed