From 49b90af61110e73e0683518cae8ad0c8588c7568 Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Thu, 1 Oct 2026 09:55:48 +0100 Subject: [PATCH 1/3] fix(givtcp): accept GivEnergy's quantised charge/discharge rate read-back (#5324) GivEnergy stores the battery charge and discharge rate as a whole percent of nominal battery capacity, rounded down: on a 13.41kWh battery, 1300W reads back 1206W. Since #4864 moved GivTCP REST into its own component, rate writes go through write_and_poll_value, whose tolerance is 5% of the rate ceiling. With an inverter_limit_charge override of 1300W that is 65W, so the 94W miss was rejected and the write was repeated ten times every cycle. - INVERTER_DEF gains rate_step_percent_of_capacity (GE: 1). A custom inverter type built from the GE row does not inherit it. - Inverter.rate_write_tolerance() widens the tolerance for a read-back BELOW the rate written to one step. A read-back above it keeps the plain 5%, because the step rounds down, so a 134W rate is still written down to a 0W hold. rate_needs_write() applies the same rule to the "does this need writing" check. - write_and_poll_value takes an optional fuzzy_below. It defaults to the existing symmetric behaviour, so other callers are unchanged. - The GivTCP discovery record carries step_percent_of_capacity on its rate descriptors, and inverter_definition() maps it across, so the discovery contract still rebuilds the GE row. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/config.py | 1 + apps/predbat/coordinator.py | 14 +++ apps/predbat/givtcp.py | 5 + apps/predbat/inverter.py | 58 ++++++++- apps/predbat/tests/discovery_contract.py | 1 + apps/predbat/tests/test_coordinator.py | 34 +++++ apps/predbat/tests/test_inverter.py | 116 ++++++++++++++++++ docs/inverter-setup.md | 5 + ...overy-inverter-record-vocabulary-design.md | 2 + 9 files changed, 231 insertions(+), 5 deletions(-) diff --git a/apps/predbat/config.py b/apps/predbat/config.py index a78ef531b..663c391f5 100644 --- a/apps/predbat/config.py +++ b/apps/predbat/config.py @@ -1852,6 +1852,7 @@ "can_span_midnight": True, "charge_discharge_with_rate": False, "target_soc_used_for_discharge": False, + "rate_step_percent_of_capacity": 1, }, "GEC": { "name": "GivEnergy Cloud", diff --git a/apps/predbat/coordinator.py b/apps/predbat/coordinator.py index 3f2c0711d..6136a479b 100644 --- a/apps/predbat/coordinator.py +++ b/apps/predbat/coordinator.py @@ -100,6 +100,13 @@ def _clean_number(value): return None +def _clean_percent(value): + """A percentage above 0 and up to 100, or None. Bools are refused, unlike _clean_number.""" + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + return value if 0 < value <= 100 else None + + def _clean_token(value): """A vocabulary token (lower case, no spaces), or None.""" return value if isinstance(value, str) and VOCAB_RE.match(value) else None @@ -159,6 +166,7 @@ def _clean_measure_or_tokens(value): "min": _clean_number, "max": _clean_number, "step": _clean_number, + "step_percent_of_capacity": _clean_percent, "precision": _clean_number, "options": _clean_option_list, "format": _clean_string, @@ -1319,6 +1327,12 @@ def not_applicable_field(field): else: gaps.append("output_charge_control") not_applicable_field("current_dp") + # charge_rate's step stands for both rates - an inverter holding them coarsely holds both alike. Most + # inverters have no such step, so its absence is not a gap: base's value stands, else none + if rate is not None and rate.get("step_percent_of_capacity"): + definition["rate_step_percent_of_capacity"] = rate["step_percent_of_capacity"] + else: + definition.setdefault("rate_step_percent_of_capacity", 0) definition["time_button_press"] = _bound_entity(entities, "schedule_write_button", access="rw") is not None diff --git a/apps/predbat/givtcp.py b/apps/predbat/givtcp.py index fb39cb6cc..76b58cfd5 100644 --- a/apps/predbat/givtcp.py +++ b/apps/predbat/givtcp.py @@ -92,6 +92,10 @@ # restart. Only endpoints not already being managed are probed, and only ever to add them. GIVTCP_REDISCOVER_SECONDS = 3600 +# GivEnergy holds the charge and discharge rates as a whole percent of nominal battery capacity, +# rounded down - 1300W on a 13.41kWh battery reads back 1206W (#5324) +GIVTCP_RATE_STEP_PERCENT_OF_CAPACITY = 1 + # control name -> (domain, GivTCPRest write method name, HA entity attributes) GIVTCP_CONTROLS = { "charge_rate": ("number", "set_charge_rate", {"unit_of_measurement": "W", "device_class": "power", "icon": "mdi:battery-charging", "min": 0, "max": 20000, "step": 100}), @@ -1038,6 +1042,7 @@ def _discovery_descriptor(self, n, name, domain, access, attrs, max_battery_rate elif options: descriptor["options"] = options if name in ("charge_rate", "discharge_rate"): + descriptor["step_percent_of_capacity"] = GIVTCP_RATE_STEP_PERCENT_OF_CAPACITY if max_battery_rate: descriptor["max"] = max_battery_rate else: diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 3a1c79dcf..619e2bcf5 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -469,6 +469,8 @@ def refresh_config(self, quiet=False): if "inverter" in self.base.args: if self.inverter_type not in INVERTER_DEF: INVERTER_DEF[self.inverter_type] = INVERTER_DEF["GE"].copy() + # GivEnergy's rate quantisation is a fact about its hardware, not a default for other inverters + INVERTER_DEF[self.inverter_type].pop("rate_step_percent_of_capacity", None) inverter_def = self.base.args["inverter"] if isinstance(inverter_def, list): @@ -523,6 +525,7 @@ def refresh_config(self, quiet=False): self.inv_charge_discharge_with_rate = INVERTER_DEF[self.inverter_type].get("charge_discharge_with_rate", False) self.inv_target_soc_used_for_discharge = INVERTER_DEF[self.inverter_type].get("target_soc_used_for_discharge", True) self.inv_has_solis_energy_control = INVERTER_DEF[self.inverter_type].get("has_solis_energy_control", False) + self.inv_rate_step_percent_of_capacity = INVERTER_DEF[self.inverter_type].get("rate_step_percent_of_capacity", 0) # If it's not a GE inverter then turn Quiet off if self.inverter_type != "GE": @@ -2078,6 +2081,34 @@ def get_current_charge_rate(self): return current_rate + def rate_write_tolerance(self, rate_max): + """ + Watts a charge/discharge rate read-back may fall short of the rate written and still count as set. + + 5% of rate_max - the rate ceiling, battery_rate_max_charge/discharge per minute - widened to one + hardware step on inverters that store the rate coarsely. GivEnergy holds it as a whole percent of + nominal battery capacity, rounded down: 1300W on a 13.41kWh battery reads back 1206W, a 94W miss + that a 1300W ceiling's 65W tolerance rejected - re-writing it ten times every cycle (#5324). + A read-back can fall short by just under one step, plus 1W for the read-back's own truncation. + """ + tolerance = rate_max * MINUTE_WATT / 20 + if self.inv_rate_step_percent_of_capacity and self.nominal_capacity: + step = self.nominal_capacity * 1000 * self.inv_rate_step_percent_of_capacity / 100 + tolerance = max(tolerance, step + 1) + return tolerance + + def rate_needs_write(self, current_rate, new_rate, rate_max): + """ + Whether the rate the inverter holds is far enough from new_rate that new_rate must be written. + + Short of new_rate by up to rate_write_tolerance() is what writing it would leave anyway. Over it + is never quantisation, since the step rounds down, so only the plain 5% applies there - a rate + held one step up (134W) is still written down to a 0W hold. + """ + if current_rate > new_rate: + return current_rate - new_rate > rate_max * MINUTE_WATT / 20 + return new_rate - current_rate > self.rate_write_tolerance(rate_max) + def adjust_charge_rate(self, new_rate, notify=True): """ Adjust charging rate @@ -2102,10 +2133,17 @@ 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_needs_write(current_rate, new_rate, self.battery_rate_max_charge): 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") + 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", + fuzzy_below=self.rate_write_tolerance(self.battery_rate_max_charge), + ) if "charge_rate_percent" in self.base.args: self.write_and_poll_value("charge_rate_percent", self.base.get_arg("charge_rate_percent", indirect=False, index=self.id, required_unit="%"), min(int(new_rate / self.battery_rate_max_raw * 100), 100), fuzzy=5, required_unit="%") @@ -2143,10 +2181,17 @@ 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_needs_write(current_rate, new_rate, self.battery_rate_max_discharge): 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") + 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", + fuzzy_below=self.rate_write_tolerance(self.battery_rate_max_discharge), + ) if "discharge_rate_percent" in self.base.args: self.write_and_poll_value("discharge_rate_percent", self.base.get_arg("discharge_rate_percent", indirect=False, index=self.id, required_unit="%"), min(int(new_rate / self.battery_rate_max_raw * 100), 100), fuzzy=5, required_unit="%") @@ -2411,9 +2456,10 @@ def switch_matched(state): self._write_backoff_result(name, entity_id, new_value, False) return False - def write_and_poll_value(self, name, entity_id, new_value, fuzzy=0, ignore_fail=False, required_unit=None): + def write_and_poll_value(self, name, entity_id, new_value, fuzzy=0, ignore_fail=False, required_unit=None, fuzzy_below=None): # Modified to cope with sensor entities and writing strings # Re-written to minimise writes + # fuzzy_below, when given, replaces fuzzy for a read short of new_value - for a value the device rounds down if not self.check_write_entity("write_and_poll_value", name, entity_id, new_value): return False domain, entity_name = entity_id.split(".") @@ -2443,6 +2489,8 @@ def value_matched(state): state = value_state(state) if isinstance(new_value, str): return state == new_value + if fuzzy_below is not None and state < new_value: + return new_value - state <= fuzzy_below return abs(state - new_value) <= fuzzy raw_state = self.base.get_state_wrapper(entity_id, required_unit=required_unit) diff --git a/apps/predbat/tests/discovery_contract.py b/apps/predbat/tests/discovery_contract.py index 7cb4c322a..9adcc0026 100644 --- a/apps/predbat/tests/discovery_contract.py +++ b/apps/predbat/tests/discovery_contract.py @@ -25,6 +25,7 @@ "has_ge_eco_toggle": False, "charge_discharge_with_rate": False, "target_soc_used_for_discharge": True, + "rate_step_percent_of_capacity": 0, } # Row fields the record is not expected to rebuild: the display name, and two fields nothing reads diff --git a/apps/predbat/tests/test_coordinator.py b/apps/predbat/tests/test_coordinator.py index 198cee08e..a751a2497 100644 --- a/apps/predbat/tests/test_coordinator.py +++ b/apps/predbat/tests/test_coordinator.py @@ -1877,6 +1877,21 @@ def test_descriptor_invert_must_be_bool(): return 0 +def test_descriptor_step_percent_of_capacity_must_be_a_percentage(): + """step_percent_of_capacity is kept when it is a percentage above 0 and up to 100, dropped otherwise (#5324).""" + record, _ = _one_inverter(entities={ + "charge_rate": {"entity_id": "number.c", "access": "rw", "step_percent_of_capacity": 1}, + "discharge_rate": {"entity_id": "number.d", "access": "rw", "step_percent_of_capacity": True}, + "charge_limit": {"entity_id": "number.l", "access": "rw", "step_percent_of_capacity": 0}, + "reserve": {"entity_id": "number.r", "access": "rw", "step_percent_of_capacity": 150}, + }) + assert record["entities"]["charge_rate"]["step_percent_of_capacity"] == 1, record + for setting in ("discharge_rate", "charge_limit", "reserve"): + assert "step_percent_of_capacity" not in record["entities"][setting], record + print("PASS: step_percent_of_capacity must be a percentage") + return 0 + + def test_new_containers_survive_redaction_unchanged(): """Bool capabilities, a strftime format and a string value stand-in pass the redactor untouched.""" base, coordinator = _coordinator() @@ -1947,6 +1962,23 @@ def test_inverter_definition_presence_needs_a_real_rw_entity(): return 0 +def test_inverter_definition_rate_step_from_charge_rate(): + """rate_step_percent_of_capacity comes from charge_rate's step_percent_of_capacity; absent, base's value stands, else 0 (#5324).""" + record = _full_record() + definition, gaps, _ = inverter_definition(record, 2) + assert definition["rate_step_percent_of_capacity"] == 0 and "rate_step_percent_of_capacity" not in gaps, definition + definition, _, _ = inverter_definition(record, 2, base={"rate_step_percent_of_capacity": 1}) + assert definition["rate_step_percent_of_capacity"] == 1, definition + record["entities"]["charge_rate"]["step_percent_of_capacity"] = 1 + definition, _, _ = inverter_definition(record, 2) + assert definition["rate_step_percent_of_capacity"] == 1, definition + del record["entities"]["charge_rate"] + definition, _, _ = inverter_definition(record, 2) + assert definition["rate_step_percent_of_capacity"] == 0, definition + print("PASS: rate step from charge_rate") + return 0 + + def test_inverter_definition_ge_mode_flags_follow_inverter_mode_domain(): """inverter_mode as a select means a GE inverter-mode select; as a switch, GE Cloud's eco toggle.""" record = _full_record() @@ -2119,10 +2151,12 @@ def test_coordinator_all(my_predbat=None): failures += test_descriptor_needs_exactly_one_of_entity_id_and_value() failures += test_descriptor_value_rejects_free_text() failures += test_descriptor_invert_must_be_bool() + failures += test_descriptor_step_percent_of_capacity_must_be_a_percentage() failures += test_new_containers_survive_redaction_unchanged() failures += test_inverter_definition_builds_every_field_without_a_base() failures += test_inverter_definition_presence_needs_a_real_rw_entity() failures += test_inverter_definition_ge_mode_flags_follow_inverter_mode_domain() + failures += test_inverter_definition_rate_step_from_charge_rate() failures += test_inverter_definition_charge_rate_units() failures += test_inverter_definition_gaps_and_not_applicable() failures += test_inverter_definition_counts_load_entities() diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 08cd9f145..eeaf30112 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -843,6 +843,107 @@ def test_adjust_charge_rate(test_name, ha, inv, dummy_rest, prev_rate, rate, exp return failed +def test_rate_write_quantised(test_name, ha, inv, prev_rate, rate, expect_writes, expect_rate, discharge=False, rate_max=None): + """ + A rate the inverter stores coarsely must verify from its quantised read-back, not be re-written (#5324). + + The inverter is modelled as GivEnergy holds the rate - a whole percent of nominal battery capacity, + rounded down. rate_max is the rate ceiling, defaulting to the request as an inverter_limit_charge override caps it. + """ + failed = False + print("Test: {} prev_rate {} rate {} expect_writes {} expect_rate {}".format(test_name, prev_rate, rate, expect_writes, expect_rate)) + inv.rest_data = None + inv.rest_api = None + entity = "number.discharge_rate" if discharge else "number.charge_rate" + max_attr = "battery_rate_max_discharge" if discharge else "battery_rate_max_charge" + percent_arg = "discharge_rate_percent" if discharge else "charge_rate_percent" + saved = (inv.inv_rate_step_percent_of_capacity, inv.nominal_capacity, getattr(inv, max_attr)) + # GivTCP configures the rate in watts only - with a percent entity too, the current rate is read from that instead + saved_percent = inv.base.args.pop(percent_arg, None) + capacity_wh = 13410 + writes = [] + call_service = ha.call_service + + def quantising_call_service(service, **kwargs): + """Store a rate write as the inverter would hold it.""" + if service == "number/set_value" and kwargs.get("entity_id") == entity: + writes.append(kwargs.get("value")) + kwargs["value"] = int(int(float(kwargs["value"]) * 100 / capacity_wh) * capacity_wh / 100) + return call_service(service, **kwargs) + + try: + inv.inv_rate_step_percent_of_capacity = 1 + inv.nominal_capacity = capacity_wh / 1000 + setattr(inv, max_attr, (rate if rate_max is None else rate_max) / MINUTE_WATT) + ha.call_service = quantising_call_service + ha.dummy_items[entity] = prev_rate + if discharge: + inv.adjust_discharge_rate(rate) + else: + inv.adjust_charge_rate(rate) + finally: + ha.call_service = call_service + if saved_percent is not None: + inv.base.args[percent_arg] = saved_percent + inv.inv_rate_step_percent_of_capacity, inv.nominal_capacity, _ = saved + setattr(inv, max_attr, saved[2]) + + if len(writes) != expect_writes: + print("ERROR: {} expected {} write(s) got {}: {}".format(test_name, expect_writes, len(writes), writes)) + failed = True + if float(ha.get_state(entity)) != expect_rate: + print("ERROR: {} expected rate {} got {}".format(test_name, expect_rate, ha.get_state(entity))) + failed = True + return failed + + +def test_custom_inverter_def_drops_rate_step(test_name, my_predbat): + """ + A custom inverter type is built from a copy of the GE row, but must not inherit GivEnergy's rate step (#5324). + """ + failed = False + print("Test: {}".format(test_name)) + saved = {arg: my_predbat.args.get(arg, None) for arg in ("inverter_type", "inverter")} + try: + my_predbat.args["inverter_type"] = ["CUSTOM5324"] + my_predbat.args["inverter"] = {"name": "Custom", "has_rest_api": False} + inv = Inverter(my_predbat, 0, quiet=True) + if inv.inv_rate_step_percent_of_capacity != 0: + print("ERROR: {} custom inverter inherited rate step {}".format(test_name, inv.inv_rate_step_percent_of_capacity)) + failed = True + if INVERTER_DEF["GE"].get("rate_step_percent_of_capacity") != 1: + print("ERROR: {} GE row lost its rate step".format(test_name)) + failed = True + finally: + INVERTER_DEF.pop("CUSTOM5324", None) + for arg, value in saved.items(): + if value is None: + my_predbat.args.pop(arg, None) + else: + my_predbat.args[arg] = value + return failed + + +def test_rate_write_tolerance(test_name, inv): + """ + The rate verify tolerance is 5% of the allocated rate, widened to one step only where the inverter declares one (#5324). + """ + failed = False + print("Test: {}".format(test_name)) + saved = (inv.inv_rate_step_percent_of_capacity, inv.nominal_capacity) + try: + for step_percent, capacity, rate_max_watts, expect in ((0, 13.41, 1300, 65), (1, 13.41, 1300, 135.1), (1, 13.41, 4000, 200), (1, 0, 1300, 65)): + inv.inv_rate_step_percent_of_capacity = step_percent + inv.nominal_capacity = capacity + got = inv.rate_write_tolerance(rate_max_watts / MINUTE_WATT) + if abs(got - expect) > 0.01: + print("ERROR: {} step {}% capacity {}kWh rate {}W expected tolerance {} got {}".format(test_name, step_percent, capacity, rate_max_watts, expect, got)) + failed = True + finally: + inv.inv_rate_step_percent_of_capacity, inv.nominal_capacity = saved + return failed + + def test_current_reasserted_on_unchanged_rate(test_name, ha, inv, prev_current, rate, discharge=False): """ Test that timed_charge_current / timed_discharge_current is re-asserted on every call to @@ -5157,6 +5258,21 @@ def run_inverter_tests(my_predbat_dummy): if failed: return failed + # #5324: GivEnergy stores the rate as a whole percent of capacity, so 1300W reads back 1206W + failed |= test_rate_write_tolerance("rate_write_tolerance", inv) + failed |= test_custom_inverter_def_drops_rate_step("custom_inverter_def_drops_rate_step", my_predbat) + failed |= test_rate_write_quantised("rate_write_quantised_charge", ha, inv, 2600, 1300, 1, 1206) + failed |= test_rate_write_quantised("rate_write_quantised_charge_settled", ha, inv, 1206, 1300, 0, 1206) + failed |= test_rate_write_quantised("rate_write_quantised_charge_next_step", ha, inv, 1206, 1350, 1, 1341) + failed |= test_rate_write_quantised("rate_write_quantised_discharge", ha, inv, 2600, 1300, 1, 1206, discharge=True) + failed |= test_rate_write_quantised("rate_write_quantised_discharge_settled", ha, inv, 1206, 1300, 0, 1206, discharge=True) + # Rounding is always down, so a rate held above the request is never quantisation - still written + failed |= test_rate_write_quantised("rate_write_quantised_hold_from_one_step", ha, inv, 134, 0, 1, 0, rate_max=2600) + failed |= test_rate_write_quantised("rate_write_quantised_step_down", ha, inv, 1341, 1210, 1, 1206) + failed |= test_rate_write_quantised("rate_write_quantised_discharge_hold_from_one_step", ha, inv, 134, 0, 1, 0, discharge=True, rate_max=2600) + if failed: + return failed + # #4415: timed_charge_current/timed_discharge_current must be re-asserted every call, not # just when the rate itself changes failed |= test_current_reasserted_on_unchanged_rate("current_reassert_charge", ha, inv, 0, 200) diff --git a/docs/inverter-setup.md b/docs/inverter-setup.md index b94d5ae81..672a6e689 100644 --- a/docs/inverter-setup.md +++ b/docs/inverter-setup.md @@ -3837,4 +3837,9 @@ Defines the units of the SoC setting (currently not used), it defaults to "%". Sets the number of seconds between polls of inverter settings. +### rate_step_percent_of_capacity + +Optional, defaults to 0. Set it when the inverter stores its charge and discharge rates in steps of this percentage of nominal battery capacity (the built-in GivEnergy `GE` type uses 1). +Predbat then accepts a rate read-back that is up to one step away from the rate it wrote, rather than re-writing it every cycle. + diff --git a/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md b/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md index c7efd21ba..6092389ca 100644 --- a/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md +++ b/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md @@ -146,6 +146,7 @@ One descriptor per Predbat setting that automatic configuration binds for this d | `entity_id` | one of these two | The HA entity bound to the setting. | | `value` | one of these two | A fixed stand-in where no entity exists (Fox's `pv_power: 0` for a device with no PV; SolaX's constant `battery_min_soc`). Number, bool or short string. | | `domain`, `unit`, `device_class`, `min`, `max`, `step`, `precision`, `options`, `format` | no | As today. | +| `step_percent_of_capacity` | no | On `charge_rate`: the inverter stores both rates in steps of this percent of nominal battery capacity (GivEnergy: 1). Absent means no coarser step than `step`. | | `invert` | no | `true` when the entity's sign is the opposite of Predbat's convention. Replaces the `grid_power_invert`, `battery_power_invert` and `load_power_invert` settings. | A descriptor with both `entity_id` and `value`, or neither, is dropped. @@ -219,6 +220,7 @@ The definition is built from four sources: | `soc_units` | which state-of-charge setting is bound: `soc_percent` -> `%`, `soc_kw` -> `kWh` | | `output_charge_control` | `charge_rate`'s `unit`: W -> `power`, A -> `current`; no `charge_rate` but a bound `charge_rate_percent` (GE Cloud's percentage-rate models) -> `power`; neither -> `none` | | `current_dp` | the decimal places of `charge_rate`'s `step`, when its unit is A | + | `rate_step_percent_of_capacity` | `charge_rate`'s `step_percent_of_capacity`; 0 when absent or `charge_rate` is unbound | | `time_button_press` | whether a `schedule_write_button` entity is present | | `num_load_entities` | 1 plus the number of consecutive `load_power_1`, `load_power_2`, ... entities bound (`inverter.py:1523` adds them into the load reading) | From 3bb8af3837af2e2c3c9d742606989208a52c7faf Mon Sep 17 00:00:00 2001 From: Rik Allen Date: Thu, 1 Oct 2026 23:05:19 +0100 Subject: [PATCH 2/3] refactor(inverter): one rule for a rounded-down rate read-back (#5324 review) Review fixes for the #5324 rate tolerance: - Inverter.rate_tolerances() returns (fuzzy, fuzzy_below) for a rate ceiling. A module-level within_fuzzy() now holds the "short of target / over it" rule, which both the "does this need writing" check and write_and_poll_value's read-back match use, so the 5% rule is no longer written out four times and the two checks cannot drift apart. The docstring records the trade-offs deliberately accepted: a relaxed check rather than predicting GivEnergy's exact read-back, a ~1W boundary where a request exactly one step up is taken as already set, and the step being only as right as nominal_capacity. - The custom-inverter-type test restores the shared fixture's args and HA state whole, since building an Inverter writes both (#5079). - inverter-setup.md says the step's slack applies only below the rate written. The vocabulary spec matches inverter_definition(): an absent step is not a gap, and base's value stands. Co-Authored-By: Claude Opus 5.5 --- apps/predbat/inverter.py | 68 ++++++++++--------- apps/predbat/tests/test_inverter.py | 40 ++++++----- docs/inverter-setup.md | 2 +- ...overy-inverter-record-vocabulary-design.md | 2 +- 4 files changed, 63 insertions(+), 49 deletions(-) diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 619e2bcf5..7424a3927 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -55,6 +55,16 @@ _NOT_COMMITTED = object() +def within_fuzzy(value, target, fuzzy, fuzzy_below=None): + """ + Whether a numeric read counts as target: within fuzzy either side, or within fuzzy_below when it is + short of target and fuzzy_below is given - for a value the device rounds down (#5324). + """ + if fuzzy_below is not None and value < target: + return target - value <= fuzzy_below + return abs(value - target) <= fuzzy + + class Inverter: """Unified inverter control abstraction for multiple brands. @@ -2081,33 +2091,29 @@ def get_current_charge_rate(self): return current_rate - def rate_write_tolerance(self, rate_max): + def rate_tolerances(self, rate_max): """ - Watts a charge/discharge rate read-back may fall short of the rate written and still count as set. + (fuzzy, fuzzy_below) in watts for checking a charge/discharge rate read-back against the rate written. + + fuzzy is 5% of rate_max - the rate ceiling, battery_rate_max_charge/discharge per minute. On an + inverter that stores the rate coarsely, fuzzy_below widens a read-back SHORT of the rate to one + hardware step. GivEnergy holds it as a whole percent of nominal battery capacity, rounded down: + 1300W on a 13.41kWh battery reads back 1206W, a 94W miss that a 1300W ceiling's 65W tolerance + rejected - re-writing it ten times every cycle (#5324). A read-back over the rate is never that + rounding, so it keeps fuzzy: a rate held one step up (134W) is still written down to a 0W hold. - 5% of rate_max - the rate ceiling, battery_rate_max_charge/discharge per minute - widened to one - hardware step on inverters that store the rate coarsely. GivEnergy holds it as a whole percent of - nominal battery capacity, rounded down: 1300W on a 13.41kWh battery reads back 1206W, a 94W miss - that a 1300W ceiling's 65W tolerance rejected - re-writing it ten times every cycle (#5324). - A read-back can fall short by just under one step, plus 1W for the read-back's own truncation. + Deliberately relaxed rather than predicting the exact read-back, which would need Predbat's + capacity to match the inverter's to the watt. The cost is a ~1W boundary: a short read-back can + miss by just under one step plus 1W of read-back truncation, so a request landing exactly one step + above the held rate is taken as already set and the rate stays one step low - the same error the + rounding makes anyway. The step is only as right as nominal_capacity; GivTCP supplies its own. """ - tolerance = rate_max * MINUTE_WATT / 20 + fuzzy = rate_max * MINUTE_WATT / 20 + fuzzy_below = None if self.inv_rate_step_percent_of_capacity and self.nominal_capacity: step = self.nominal_capacity * 1000 * self.inv_rate_step_percent_of_capacity / 100 - tolerance = max(tolerance, step + 1) - return tolerance - - def rate_needs_write(self, current_rate, new_rate, rate_max): - """ - Whether the rate the inverter holds is far enough from new_rate that new_rate must be written. - - Short of new_rate by up to rate_write_tolerance() is what writing it would leave anyway. Over it - is never quantisation, since the step rounds down, so only the plain 5% applies there - a rate - held one step up (134W) is still written down to a 0W hold. - """ - if current_rate > new_rate: - return current_rate - new_rate > rate_max * MINUTE_WATT / 20 - return new_rate - current_rate > self.rate_write_tolerance(rate_max) + fuzzy_below = max(fuzzy, step + 1) + return fuzzy, fuzzy_below def adjust_charge_rate(self, new_rate, notify=True): """ @@ -2133,16 +2139,17 @@ def adjust_charge_rate(self, new_rate, notify=True): new_rate = int(new_rate + 0.5) current_rate = self.get_current_charge_rate() - if self.rate_needs_write(current_rate, new_rate, self.battery_rate_max_charge): + fuzzy, fuzzy_below = self.rate_tolerances(self.battery_rate_max_charge) + if not within_fuzzy(current_rate, new_rate, fuzzy, fuzzy_below): 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), + fuzzy=fuzzy, required_unit="W", - fuzzy_below=self.rate_write_tolerance(self.battery_rate_max_charge), + fuzzy_below=fuzzy_below, ) if "charge_rate_percent" in self.base.args: self.write_and_poll_value("charge_rate_percent", self.base.get_arg("charge_rate_percent", indirect=False, index=self.id, required_unit="%"), min(int(new_rate / self.battery_rate_max_raw * 100), 100), fuzzy=5, required_unit="%") @@ -2181,16 +2188,17 @@ def adjust_discharge_rate(self, new_rate, notify=True): new_rate = int(new_rate + 0.5) current_rate = self.get_current_discharge_rate() - if self.rate_needs_write(current_rate, new_rate, self.battery_rate_max_discharge): + fuzzy, fuzzy_below = self.rate_tolerances(self.battery_rate_max_discharge) + if not within_fuzzy(current_rate, new_rate, fuzzy, fuzzy_below): 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), + fuzzy=fuzzy, required_unit="W", - fuzzy_below=self.rate_write_tolerance(self.battery_rate_max_discharge), + fuzzy_below=fuzzy_below, ) if "discharge_rate_percent" in self.base.args: self.write_and_poll_value("discharge_rate_percent", self.base.get_arg("discharge_rate_percent", indirect=False, index=self.id, required_unit="%"), min(int(new_rate / self.battery_rate_max_raw * 100), 100), fuzzy=5, required_unit="%") @@ -2489,9 +2497,7 @@ def value_matched(state): state = value_state(state) if isinstance(new_value, str): return state == new_value - if fuzzy_below is not None and state < new_value: - return new_value - state <= fuzzy_below - return abs(state - new_value) <= fuzzy + return within_fuzzy(state, new_value, fuzzy, fuzzy_below) raw_state = self.base.get_state_wrapper(entity_id, required_unit=required_unit) current_state = value_state(raw_state, warn=True) diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index eeaf30112..63d397e0b 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -17,7 +17,7 @@ from utils import calc_percent_limit, is_entity_id from tests.test_infra import TestHAInterface from predbat import PredBat -from inverter import Inverter +from inverter import Inverter, within_fuzzy from givtcp_rest import GivTCPRest from config import INVERTER_DEF from const import MINUTE_WATT, TIME_FORMAT_SECONDS @@ -897,13 +897,17 @@ def quantising_call_service(service, **kwargs): return failed -def test_custom_inverter_def_drops_rate_step(test_name, my_predbat): +def test_custom_inverter_def_drops_rate_step(test_name, my_predbat, ha): """ A custom inverter type is built from a copy of the GE row, but must not inherit GivEnergy's rate step (#5324). + + Building an Inverter writes args (soc_max_nominal, battery_scaling_last_known) and publishes entities on + the shared fixture, so both are restored whole afterwards rather than just the args this test sets (#5079). """ failed = False print("Test: {}".format(test_name)) - saved = {arg: my_predbat.args.get(arg, None) for arg in ("inverter_type", "inverter")} + saved_args = copy.deepcopy(my_predbat.args) + saved_items = copy.deepcopy(ha.dummy_items) try: my_predbat.args["inverter_type"] = ["CUSTOM5324"] my_predbat.args["inverter"] = {"name": "Custom", "has_rest_api": False} @@ -916,28 +920,32 @@ def test_custom_inverter_def_drops_rate_step(test_name, my_predbat): failed = True finally: INVERTER_DEF.pop("CUSTOM5324", None) - for arg, value in saved.items(): - if value is None: - my_predbat.args.pop(arg, None) - else: - my_predbat.args[arg] = value + my_predbat.args.clear() + my_predbat.args.update(saved_args) + ha.dummy_items.clear() + ha.dummy_items.update(saved_items) return failed -def test_rate_write_tolerance(test_name, inv): +def test_rate_tolerances(test_name, inv): """ - The rate verify tolerance is 5% of the allocated rate, widened to one step only where the inverter declares one (#5324). + A rate read-back gets 5% of the rate ceiling either side, widened below to one step only where the inverter declares one (#5324). """ failed = False print("Test: {}".format(test_name)) saved = (inv.inv_rate_step_percent_of_capacity, inv.nominal_capacity) try: - for step_percent, capacity, rate_max_watts, expect in ((0, 13.41, 1300, 65), (1, 13.41, 1300, 135.1), (1, 13.41, 4000, 200), (1, 0, 1300, 65)): + for step_percent, capacity, rate_max_watts, expect in ((0, 13.41, 1300, (65, None)), (1, 13.41, 1300, (65, 135.1)), (1, 13.41, 4000, (200, 200)), (1, 0, 1300, (65, None))): inv.inv_rate_step_percent_of_capacity = step_percent inv.nominal_capacity = capacity - got = inv.rate_write_tolerance(rate_max_watts / MINUTE_WATT) - if abs(got - expect) > 0.01: - print("ERROR: {} step {}% capacity {}kWh rate {}W expected tolerance {} got {}".format(test_name, step_percent, capacity, rate_max_watts, expect, got)) + got = inv.rate_tolerances(rate_max_watts / MINUTE_WATT) + if abs(got[0] - expect[0]) > 0.01 or (got[1] is None) != (expect[1] is None) or (got[1] is not None and abs(got[1] - expect[1]) > 0.01): + print("ERROR: {} step {}% capacity {}kWh rate {}W expected tolerances {} got {}".format(test_name, step_percent, capacity, rate_max_watts, expect, got)) + failed = True + # value, target, fuzzy, fuzzy_below, expect - fuzzy_below only ever widens a read SHORT of target + for value, target, fuzzy, fuzzy_below, expect in ((1206, 1300, 65, 135.1, True), (1206, 1300, 65, None, False), (134, 0, 130, 135.1, False), (1365, 1300, 65, 135.1, True), (1366, 1300, 65, 135.1, False)): + if within_fuzzy(value, target, fuzzy, fuzzy_below) != expect: + print("ERROR: {} within_fuzzy({}, {}, {}, {}) expected {}".format(test_name, value, target, fuzzy, fuzzy_below, expect)) failed = True finally: inv.inv_rate_step_percent_of_capacity, inv.nominal_capacity = saved @@ -5259,8 +5267,8 @@ def run_inverter_tests(my_predbat_dummy): return failed # #5324: GivEnergy stores the rate as a whole percent of capacity, so 1300W reads back 1206W - failed |= test_rate_write_tolerance("rate_write_tolerance", inv) - failed |= test_custom_inverter_def_drops_rate_step("custom_inverter_def_drops_rate_step", my_predbat) + failed |= test_rate_tolerances("rate_tolerances", inv) + failed |= test_custom_inverter_def_drops_rate_step("custom_inverter_def_drops_rate_step", my_predbat, ha) failed |= test_rate_write_quantised("rate_write_quantised_charge", ha, inv, 2600, 1300, 1, 1206) failed |= test_rate_write_quantised("rate_write_quantised_charge_settled", ha, inv, 1206, 1300, 0, 1206) failed |= test_rate_write_quantised("rate_write_quantised_charge_next_step", ha, inv, 1206, 1350, 1, 1341) diff --git a/docs/inverter-setup.md b/docs/inverter-setup.md index 672a6e689..8fc8d93df 100644 --- a/docs/inverter-setup.md +++ b/docs/inverter-setup.md @@ -3840,6 +3840,6 @@ Sets the number of seconds between polls of inverter settings. ### rate_step_percent_of_capacity Optional, defaults to 0. Set it when the inverter stores its charge and discharge rates in steps of this percentage of nominal battery capacity (the built-in GivEnergy `GE` type uses 1). -Predbat then accepts a rate read-back that is up to one step away from the rate it wrote, rather than re-writing it every cycle. +Predbat then accepts a rate read-back up to one step *below* the rate it wrote, rather than re-writing it every cycle. A read-back above the rate written still has to be within 5% of the rate ceiling, so this suits an inverter that rounds its rates down. diff --git a/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md b/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md index 6092389ca..f5f251398 100644 --- a/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md +++ b/docs/superpowers/specs/2026-09-24-discovery-inverter-record-vocabulary-design.md @@ -220,7 +220,7 @@ The definition is built from four sources: | `soc_units` | which state-of-charge setting is bound: `soc_percent` -> `%`, `soc_kw` -> `kWh` | | `output_charge_control` | `charge_rate`'s `unit`: W -> `power`, A -> `current`; no `charge_rate` but a bound `charge_rate_percent` (GE Cloud's percentage-rate models) -> `power`; neither -> `none` | | `current_dp` | the decimal places of `charge_rate`'s `step`, when its unit is A | - | `rate_step_percent_of_capacity` | `charge_rate`'s `step_percent_of_capacity`; 0 when absent or `charge_rate` is unbound | + | `rate_step_percent_of_capacity` | `charge_rate`'s `step_percent_of_capacity`. Not a gap when absent or `charge_rate` is unbound: base's value stands, else 0 | | `time_button_press` | whether a `schedule_write_button` entity is present | | `num_load_entities` | 1 plus the number of consecutive `load_power_1`, `load_power_2`, ... entities bound (`inverter.py:1523` adds them into the load reading) | From e7ca9b6a69d64e247e34ee613b92af9698bb4a28 Mon Sep 17 00:00:00 2001 From: CI Date: Fri, 2 Oct 2026 20:35:35 +0100 Subject: [PATCH 3/3] fix(gateway): accept the quantised rate read-back for GivEnergy inverters behind the hub (#5324) The hub reports the charge and discharge rate the inverter holds, so a GivEnergy inverter behind it reads back short of the rate written just as it does through GivTCP. The hub is inverter type GWMQTT whatever brand it drives, so the step cannot go on the INVERTER_DEF row. - gateway.py marks step_percent_of_capacity on the charge_rate and discharge_rate entities of GivEnergy inverters only. - Inverter.refresh_config() takes the step from the charge_rate entity's attribute when it has one, else the INVERTER_DEF row's value as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/predbat/gateway.py | 16 ++++++++++-- apps/predbat/inverter.py | 10 ++++++++ apps/predbat/tests/test_gateway.py | 36 ++++++++++++++++++++++++++ apps/predbat/tests/test_inverter.py | 40 +++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/apps/predbat/gateway.py b/apps/predbat/gateway.py index e0b961bd8..ae845292f 100644 --- a/apps/predbat/gateway.py +++ b/apps/predbat/gateway.py @@ -100,6 +100,11 @@ def _serial_suffix(serial): PLAN_MODE_CHARGE = 1 PLAN_MODE_DISCHARGE = 2 +# GivEnergy holds the charge and discharge rates as a whole percent of nominal battery capacity, +# rounded down - 1300W on a 13.41kWh battery reads back 1206W (#5324) +GIVENERGY_RATE_STEP_PERCENT_OF_CAPACITY = 1 +GIVENERGY_INVERTER_TYPES = (pb.INVERTER_TYPE_GIVENERGY, pb.INVERTER_TYPE_GIVENERGY_EMS, pb.INVERTER_TYPE_GIVENERGY_GATEWAY) + # Entity attribute table — keyed by the semantic suffix used in dashboard_item calls GATEWAY_ATTRIBUTE_TABLE = { # Binary sensors @@ -979,8 +984,15 @@ def _inject_inverter_entities(self, inv, suffix): _raw_export_limit = control.export_limit_w export_limit_publish = 99999 if _raw_export_limit == 0 else (0 if _raw_export_limit == 1 else _raw_export_limit) self.dashboard_item(f"sensor.{pfx}_export_limit_w", export_limit_publish, attributes=GATEWAY_ATTRIBUTE_TABLE.get("export_limit_w", {}), app="gateway") - self.dashboard_item(f"number.{pfx}_charge_rate", control.charge_rate_w, attributes=GATEWAY_ATTRIBUTE_TABLE.get("charge_rate", {}), app="gateway") - self.dashboard_item(f"number.{pfx}_discharge_rate", control.discharge_rate_w, attributes=GATEWAY_ATTRIBUTE_TABLE.get("discharge_rate", {}), app="gateway") + # The hub reports the rate the inverter holds, so a GivEnergy one reads back short of the rate written + # (#5324). Marked per inverter, on a copy of the table entry: GWMQTT is one type for every brand + charge_rate_attributes = dict(GATEWAY_ATTRIBUTE_TABLE.get("charge_rate", {})) + discharge_rate_attributes = dict(GATEWAY_ATTRIBUTE_TABLE.get("discharge_rate", {})) + if inv.type in GIVENERGY_INVERTER_TYPES: + charge_rate_attributes["step_percent_of_capacity"] = GIVENERGY_RATE_STEP_PERCENT_OF_CAPACITY + discharge_rate_attributes["step_percent_of_capacity"] = GIVENERGY_RATE_STEP_PERCENT_OF_CAPACITY + self.dashboard_item(f"number.{pfx}_charge_rate", control.charge_rate_w, attributes=charge_rate_attributes, app="gateway") + self.dashboard_item(f"number.{pfx}_discharge_rate", control.discharge_rate_w, attributes=discharge_rate_attributes, app="gateway") # The reserve ceiling is per-inverter, so it overrides the table's 100: GivEnergy # firmware refuses a reserve of 100 and the gateway reports 98 for it (gateway # issue #346). adjust_reserve() honours this entity's "max" through diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 7424a3927..89f3f827f 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -535,7 +535,17 @@ def refresh_config(self, quiet=False): self.inv_charge_discharge_with_rate = INVERTER_DEF[self.inverter_type].get("charge_discharge_with_rate", False) self.inv_target_soc_used_for_discharge = INVERTER_DEF[self.inverter_type].get("target_soc_used_for_discharge", True) self.inv_has_solis_energy_control = INVERTER_DEF[self.inverter_type].get("has_solis_energy_control", False) + # The charge_rate entity can carry the step itself, for a type that covers more than one brand: the + # hub is GWMQTT whatever it drives, and marks only its GivEnergy inverters. The row's value stands otherwise self.inv_rate_step_percent_of_capacity = INVERTER_DEF[self.inverter_type].get("rate_step_percent_of_capacity", 0) + rate_entity = self.base.get_arg("charge_rate", indirect=False, index=self.id) + if rate_entity: + try: + rate_step = float(self.base.get_state_wrapper(rate_entity, attribute="step_percent_of_capacity")) + except (ValueError, TypeError): + rate_step = 0 + if 0 < rate_step <= 100: + self.inv_rate_step_percent_of_capacity = rate_step # If it's not a GE inverter then turn Quiet off if self.inverter_type != "GE": diff --git a/apps/predbat/tests/test_gateway.py b/apps/predbat/tests/test_gateway.py index 713bb1c28..4362ff8b0 100644 --- a/apps/predbat/tests/test_gateway.py +++ b/apps/predbat/tests/test_gateway.py @@ -666,6 +666,42 @@ def test_reserve_soc_max_does_not_mutate_the_shared_table(self): assert GATEWAY_ATTRIBUTE_TABLE["reserve_soc"]["max"] == 100 + def test_rate_step_marked_on_givenergy_rates(self): + """A GivEnergy inverter's rate entities carry the 1% of capacity step its read-back is rounded down to (#5324).""" + status = self._make_status() + for inverter_type in (pb.INVERTER_TYPE_GIVENERGY, pb.INVERTER_TYPE_GIVENERGY_EMS, pb.INVERTER_TYPE_GIVENERGY_GATEWAY): + status.inverters[0].type = inverter_type + gw = self._make_gateway() + gw._inject_entities(status) + + for rate in ("charge_rate", "discharge_rate"): + state, attrs = gw._dashboard_calls["number.predbat_gateway_456789_" + rate] + assert state == 3000 + assert attrs["step_percent_of_capacity"] == 1, (inverter_type, rate, attrs) + # The rest of the table entry survives + assert attrs["unit_of_measurement"] == "W" + assert attrs["step"] == 10 + + def test_rate_step_not_marked_on_other_brands(self): + """GWMQTT is one inverter type for every brand the hub drives, so the GivEnergy step must not reach the others.""" + from gateway import GATEWAY_ATTRIBUTE_TABLE + + status = self._make_status() + for inverter_type in (pb.INVERTER_TYPE_SOLIS_HYBRID, pb.INVERTER_TYPE_DEYE_SUNSYNK, pb.INVERTER_TYPE_UNKNOWN): + status.inverters[0].type = inverter_type + gw = self._make_gateway() + gw._inject_entities(status) + + for rate in ("charge_rate", "discharge_rate"): + _, attrs = gw._dashboard_calls["number.predbat_gateway_456789_" + rate] + assert "step_percent_of_capacity" not in attrs, (inverter_type, rate, attrs) + + # Marked on a copy: a GivEnergy inverter must not leave the step in the shared table + status.inverters[0].type = pb.INVERTER_TYPE_GIVENERGY + self._make_gateway()._inject_entities(status) + for rate in ("charge_rate", "discharge_rate"): + assert "step_percent_of_capacity" not in GATEWAY_ATTRIBUTE_TABLE[rate] + def test_ems_aggregate_entities(self): """EMS aggregate and sub-inverter entities are published with table attributes.""" from gateway import GATEWAY_ATTRIBUTE_TABLE diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 63d397e0b..c7e8786c6 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -927,6 +927,45 @@ def test_custom_inverter_def_drops_rate_step(test_name, my_predbat, ha): return failed +def test_rate_step_from_charge_rate_entity(test_name, my_predbat, ha): + """ + The charge_rate entity's step_percent_of_capacity attribute sets the rate step per inverter, for a type that + covers more than one brand - the hub is GWMQTT whatever it drives and marks only its GivEnergy inverters (#5324). + + Building an Inverter writes args and publishes entities on the shared fixture, so both are restored whole (#5079). + """ + failed = False + print("Test: {}".format(test_name)) + saved_args = copy.deepcopy(my_predbat.args) + saved_items = copy.deepcopy(ha.dummy_items) + try: + my_predbat.args["charge_rate"] = "number.charge_rate" + # inverter type, charge_rate entity, expected step - an attribute that is absent, blank or not a percentage leaves the row's value + for inverter_type, entity, expect in ( + ("GWMQTT", {"state": 1100, "step_percent_of_capacity": 1}, 1), + ("GWMQTT", {"state": 1100, "max": 6000}, 0), + ("GWMQTT", 1100, 0), + ("GWMQTT", {"state": 1100, "step_percent_of_capacity": 150}, 0), + ("GWMQTT", {"state": 1100, "step_percent_of_capacity": "bad"}, 0), + ("GE", {"state": 1100, "max": 6000}, 1), + ): + my_predbat.args["inverter_type"] = [inverter_type] + ha.dummy_items["number.charge_rate"] = entity + inv = Inverter(my_predbat, 0, quiet=True) + if inv.inv_rate_step_percent_of_capacity != expect: + print("ERROR: {} type {} charge_rate {} expected rate step {} got {}".format(test_name, inverter_type, entity, expect, inv.inv_rate_step_percent_of_capacity)) + failed = True + if "rate_step_percent_of_capacity" in INVERTER_DEF["GWMQTT"]: + print("ERROR: {} the GWMQTT row must not carry a rate step, it covers every brand the hub drives".format(test_name)) + failed = True + finally: + my_predbat.args.clear() + my_predbat.args.update(saved_args) + ha.dummy_items.clear() + ha.dummy_items.update(saved_items) + return failed + + def test_rate_tolerances(test_name, inv): """ A rate read-back gets 5% of the rate ceiling either side, widened below to one step only where the inverter declares one (#5324). @@ -5269,6 +5308,7 @@ def run_inverter_tests(my_predbat_dummy): # #5324: GivEnergy stores the rate as a whole percent of capacity, so 1300W reads back 1206W failed |= test_rate_tolerances("rate_tolerances", inv) failed |= test_custom_inverter_def_drops_rate_step("custom_inverter_def_drops_rate_step", my_predbat, ha) + failed |= test_rate_step_from_charge_rate_entity("rate_step_from_charge_rate_entity", my_predbat, ha) failed |= test_rate_write_quantised("rate_write_quantised_charge", ha, inv, 2600, 1300, 1, 1206) failed |= test_rate_write_quantised("rate_write_quantised_charge_settled", ha, inv, 1206, 1300, 0, 1206) failed |= test_rate_write_quantised("rate_write_quantised_charge_next_step", ha, inv, 1206, 1350, 1, 1341)