diff --git a/apps/predbat/config.py b/apps/predbat/config.py index 6028b85a9..c640a4d9b 100644 --- a/apps/predbat/config.py +++ b/apps/predbat/config.py @@ -2550,6 +2550,10 @@ "discharge_start_time": {"type": "sensor_list", "sensor_type": "string", "modify": True, "entries": "num_inverters"}, "discharge_end_time": {"type": "sensor_list", "sensor_type": "string", "modify": True, "entries": "num_inverters"}, "battery_temperature": {"type": "sensor_list", "sensor_type": "float", "entries": "num_inverters"}, + # Optional. When the entity reports the battery is being calibrated, Predbat disables itself for + # that inverter - a calibration cycle deliberately drives the battery outside its normal SoC + # range, so any plan made during one is wrong. Absent (the default) means "never calibrating". + "battery_calibration": {"type": "sensor_list", "sensor_type": "none|string", "entries": "num_inverters"}, "pause_mode": {"type": "sensor_list", "sensor_type": "string", "modify": True, "entries": "num_inverters"}, "pause_start_time": {"type": "sensor_list", "sensor_type": "none|string", "modify": True, "entries": "num_inverters"}, "pause_end_time": {"type": "sensor_list", "sensor_type": "none|string", "modify": True, "entries": "num_inverters"}, diff --git a/apps/predbat/givtcp.py b/apps/predbat/givtcp.py index 061923b43..5f4c2bf1f 100644 --- a/apps/predbat/givtcp.py +++ b/apps/predbat/givtcp.py @@ -22,16 +22,14 @@ (dashboard_item calls, select/number/switch event handlers, automatic_config), matching the pattern already used by fox.py, ohme.py, solax.py etc. -Known gaps in this first pass (deliberately out of scope, not forgotten): - - inverter_mode and pause_mode are not published. Inverter's existing - entity-path code for these auto-detects GE-Cloud-style naming ("Pause - Charge") vs local/REST-style naming ("PauseCharge") from the entity's - current value - getting that right needs closer attention than the other - controls, which are plain W/%/HH:MM:SS values with no such ambiguity. - - soc_max/battery capacity discovery (Inverter.__init__ reading - Battery_Details/Invertor_Details) is untouched - this component only - covers the live status/control surface read in update_status and written - by adjust_*, not one-time capacity discovery at startup. +This now covers the whole surface Inverter used to read or write over REST: +the live status/control entities, inverter_mode and pause_mode, the #4517 +discharge-target model check, and the one-time battery/capacity discovery +(capacity, nominal capacity, temperature, inverter clock, max rates and +calibration state) that Inverter.__init__ used to read off the REST blob. + +GivTCPRest itself is still the REST client underneath - it is this component +that owns it now rather than Inverter. """ import asyncio @@ -45,6 +43,27 @@ # would simply not contain the value the entity is actually holding. GIVTCP_TIME_OPTIONS = ["{:02d}:{:02d}:00".format(m // 60, m % 60) for m in range(0, 24 * 60)] +# GivTCP's own pause-mode vocabulary, as written to /setBatteryPauseMode and read back from +# Control.Battery_pause_mode. Deliberately not the GE Cloud spelling ("Pause Charge", "Not Paused", +# ...) - Inverter.adjust_pause_mode picks between the two by looking at the value it reads back, so +# publishing the native spelling is what keeps it on the GivTCP side of that branch. +GIVTCP_PAUSE_MODES = ["Disabled", "PauseCharge", "PauseDischarge", "PauseBoth"] + +# GivTCP raw.invertor.model values confirmed not to support the Discharge_Target_SOC_1 register +# (#4517). "Ac" (AC Coupled) and "Hybrid_gen1" are confirmed live - the latter on two separate +# reporter inverters, still repeating the write every cycle post-fix until added here. +# "Hybrid_gen2" is inferred from the same GivEnergy firmware-archive generational split +# (github.com/DJBenson/giv-firmware), not independently confirmed on real Gen2 hardware yet. +# +# These inverters accept the write and report success, but it never persists, so the caller sees a +# permanent mismatch and rewrites every cycle. Not publishing the entity at all is what stops that: +# Inverter.adjust_force_export already leaves a target it cannot read alone. +DISCHARGE_TARGET_UNSUPPORTED_MODELS = ("Ac", "Hybrid_gen1", "Hybrid_gen2") + +# Control.Mode values. Predbat only ever writes Eco or Timed Export, but the inverter reports the +# others, and an option list that omitted them could not represent the mode the inverter is in. +GIVTCP_INVERTER_MODES = ["Eco", "Eco (Paused)", "Timed Export", "Timed Charge", "Timed Demand"] + # Poll GivTCP's REST API this often (seconds). Matches the cadence other components use for a # background refresh (e.g. Ohme's device/session poll). GIVTCP_POLL_SECONDS = 60 @@ -62,6 +81,10 @@ "charge_end_time": ("select", "_set_charge_slot", {"icon": "mdi:clock-end", "options": GIVTCP_TIME_OPTIONS}), "discharge_start_time": ("select", "_set_discharge_slot", {"icon": "mdi:clock-start", "options": GIVTCP_TIME_OPTIONS}), "discharge_end_time": ("select", "_set_discharge_slot", {"icon": "mdi:clock-end", "options": GIVTCP_TIME_OPTIONS}), + "inverter_mode": ("select", "set_battery_mode", {"icon": "mdi:home-battery", "options": GIVTCP_INVERTER_MODES}), + "pause_mode": ("select", "set_battery_pause_mode", {"icon": "mdi:pause-octagon", "options": GIVTCP_PAUSE_MODES}), + "pause_start_time": ("select", "_set_pause_slot", {"icon": "mdi:clock-start", "options": GIVTCP_TIME_OPTIONS}), + "pause_end_time": ("select", "_set_pause_slot", {"icon": "mdi:clock-end", "options": GIVTCP_TIME_OPTIONS}), } # sensor name -> HA entity attributes @@ -73,8 +96,24 @@ "grid_power": {"unit_of_measurement": "W", "device_class": "power", "icon": "mdi:transmission-tower"}, "load_power": {"unit_of_measurement": "W", "device_class": "power", "icon": "mdi:home-lightning-bolt"}, "battery_voltage": {"unit_of_measurement": "V", "device_class": "voltage", "icon": "mdi:sine-wave"}, + "soc_max": {"unit_of_measurement": "kWh", "device_class": "energy", "icon": "mdi:battery-high"}, + "battery_temperature": {"unit_of_measurement": "°C", "device_class": "temperature", "icon": "mdi:thermometer"}, + "inverter_time": {"icon": "mdi:clock-outline"}, + "battery_rate_max": {"unit_of_measurement": "W", "device_class": "power", "icon": "mdi:battery-arrow-up"}, + "inverter_limit": {"unit_of_measurement": "W", "device_class": "power", "icon": "mdi:transmission-tower"}, + "battery_calibration": {"icon": "mdi:battery-sync"}, } +# Discovery values Inverter.__init__ used to read straight off the REST blob. Published as sensors +# and claimed below so that path becomes an ordinary entity read like every other inverter type. +GIVTCP_AUTO_CONFIG_DISCOVERY_KEYS = [ + "soc_max", + "battery_temperature", + "inverter_time", + "inverter_limit", + "battery_calibration", +] + # apps.yaml keys automatic_config() points at the published entities - keys not listed here # (soc_max, battery_power_invert, ...) are left for the user/other discovery to configure. # @@ -94,9 +133,20 @@ "charge_end_time", "discharge_start_time", "discharge_end_time", + "inverter_mode", "soc_kw", ] +# Pause control is GivTCP v3 only - v2 has no /setBatteryPauseMode endpoint, and +# Inverter.adjust_pause_mode's REST path was gated on rest_v3 for the same reason. Auto-configured +# only when every configured inverter reports v3, so a mixed fleet leaves these to the user rather +# than pointing half of them at entities that can never be written. +GIVTCP_AUTO_CONFIG_PAUSE_KEYS = [ + "pause_mode", + "pause_start_time", + "pause_end_time", +] + # Power/voltage keys are auto-configured separately: givtcp_rest_power_ignore opts out of them. GIVTCP_AUTO_CONFIG_POWER_KEYS = [ "battery_power", @@ -123,6 +173,9 @@ def initialize(self, rest_urls): state = InverterRestState(id=n, rest_api=url, battery_rate_max_charge=1.0, battery_rate_max_discharge=1.0) self.rest.append(GivTCPRest(self.base, state)) self.automatic_config_done = False + # publish_data() runs every poll; the unsupported-model notice is per inverter and only + # worth saying once rather than every 60 seconds for the life of the process + self.discharge_target_warned = {} async def _run_blocking(self, func, *args): """Run one of GivTCPRest's blocking (requests + time.sleep) calls off the event loop.""" @@ -165,20 +218,51 @@ async def publish_data(self): if not rest.inverter.rest_data: continue - self.dashboard_item(self._entity_id("number", n, "charge_rate"), state=rest.inverter.rest_data.get("Control", {}).get("Battery_Charge_Rate", 0), attributes=GIVTCP_CONTROLS["charge_rate"][2], app="givtcp") - self.dashboard_item(self._entity_id("number", n, "discharge_rate"), state=rest.inverter.rest_data.get("Control", {}).get("Battery_Discharge_Rate", 0), attributes=GIVTCP_CONTROLS["discharge_rate"][2], app="givtcp") + # The rate entities carry the inverter's real maximum rate as their "max" attribute, not + # the generic ceiling in GIVTCP_CONTROLS. Inverter.__init__ derives battery_rate_max_raw + # for a GE inverter from exactly this attribute, so publishing the generic value would + # tell it the battery can take 20kW. + max_battery_rate = rest.max_battery_rate() + charge_rate_attributes = dict(GIVTCP_CONTROLS["charge_rate"][2]) + discharge_rate_attributes = dict(GIVTCP_CONTROLS["discharge_rate"][2]) + if max_battery_rate: + charge_rate_attributes["max"] = max_battery_rate + discharge_rate_attributes["max"] = max_battery_rate + + self.dashboard_item(self._entity_id("number", n, "charge_rate"), state=rest.inverter.rest_data.get("Control", {}).get("Battery_Charge_Rate", 0), attributes=charge_rate_attributes, app="givtcp") + self.dashboard_item(self._entity_id("number", n, "discharge_rate"), state=rest.inverter.rest_data.get("Control", {}).get("Battery_Discharge_Rate", 0), attributes=discharge_rate_attributes, app="givtcp") target_soc = rest.target_soc if target_soc is not None: self.dashboard_item(self._entity_id("number", n, "charge_limit"), state=target_soc, attributes=GIVTCP_CONTROLS["charge_limit"][2], app="givtcp") self.dashboard_item(self._entity_id("number", n, "reserve"), state=rest.inverter.rest_data.get("Control", {}).get("Battery_Power_Reserve", 0), attributes=GIVTCP_CONTROLS["reserve"][2], app="givtcp") - discharge_target = rest.read_discharge_target() - if discharge_target is not None: - self.dashboard_item(self._entity_id("number", n, "discharge_target_soc"), state=discharge_target, attributes=GIVTCP_CONTROLS["discharge_target_soc"][2], app="givtcp") + # An unsupported model gets no entity at all - see DISCHARGE_TARGET_UNSUPPORTED_MODELS. + # Publishing one would restart the every-cycle rewrite loop of #4517, because the write + # reports success and then silently fails to persist. + inverter_model = rest.inverter.rest_data.get("raw", {}).get("invertor", {}).get("model", "") + if inverter_model in DISCHARGE_TARGET_UNSUPPORTED_MODELS: + if not self.discharge_target_warned.get(n): + self.log("Info: GivTCP: inverter {} is {}, which has no working discharge target register - export target will not be written".format(n, inverter_model)) + self.discharge_target_warned[n] = True + else: + discharge_target = rest.read_discharge_target() + if discharge_target is not None: + self.dashboard_item(self._entity_id("number", n, "discharge_target_soc"), state=discharge_target, attributes=GIVTCP_CONTROLS["discharge_target_soc"][2], app="givtcp") self.dashboard_item(self._entity_id("switch", n, "scheduled_charge_enable"), state="on" if rest.charge_enable_time else "off", attributes=GIVTCP_CONTROLS["scheduled_charge_enable"][2], app="givtcp") self.dashboard_item(self._entity_id("switch", n, "scheduled_discharge_enable"), state="on" if rest.discharge_enable_time else "off", attributes=GIVTCP_CONTROLS["scheduled_discharge_enable"][2], app="givtcp") + control = rest.inverter.rest_data.get("Control", {}) + self.dashboard_item(self._entity_id("select", n, "inverter_mode"), state=control.get("Mode", "Eco"), attributes=GIVTCP_CONTROLS["inverter_mode"][2], app="givtcp") + + # v3 only - see GIVTCP_AUTO_CONFIG_PAUSE_KEYS + if rest.inverter.rest_v3: + self.dashboard_item(self._entity_id("select", n, "pause_mode"), state=control.get("Battery_pause_mode", "Disabled"), attributes=GIVTCP_CONTROLS["pause_mode"][2], app="givtcp") + timeslots = rest.inverter.rest_data.get("Timeslots", {}) + if rest.inverter.rest_v3: + self.dashboard_item(self._entity_id("select", n, "pause_start_time"), state=timeslots.get("Battery_pause_start_time_slot", "00:00:00"), attributes=GIVTCP_CONTROLS["pause_start_time"][2], app="givtcp") + self.dashboard_item(self._entity_id("select", n, "pause_end_time"), state=timeslots.get("Battery_pause_end_time_slot", "00:00:00"), attributes=GIVTCP_CONTROLS["pause_end_time"][2], app="givtcp") + self.dashboard_item(self._entity_id("select", n, "charge_start_time"), state=timeslots.get("Charge_start_time_slot_1", "00:00:00"), attributes=GIVTCP_CONTROLS["charge_start_time"][2], app="givtcp") self.dashboard_item(self._entity_id("select", n, "charge_end_time"), state=timeslots.get("Charge_end_time_slot_1", "00:00:00"), attributes=GIVTCP_CONTROLS["charge_end_time"][2], app="givtcp") self.dashboard_item(self._entity_id("select", n, "discharge_start_time"), state=timeslots.get("Discharge_start_time_slot_1", "00:00:00"), attributes=GIVTCP_CONTROLS["discharge_start_time"][2], app="givtcp") @@ -191,6 +275,41 @@ async def publish_data(self): if soc_percent is not None: self.dashboard_item(self._entity_id("sensor", n, "soc_percent"), state=soc_percent, attributes=GIVTCP_SENSORS["soc_percent"], app="givtcp") + # Discovery values - see GIVTCP_AUTO_CONFIG_DISCOVERY_KEYS. Each is only published when + # GivTCP actually reports it, so a missing one falls back to the user's own apps.yaml + # value rather than being published as a zero that would look authoritative. + # + # soc_max carries the nominal (nameplate) capacity instead of the reported capacity when + # battery_capacity_nominal is on: Inverter multiplies whatever it reads here by + # battery_scaling, so choosing the source here reproduces both branches of the old code. + soc_max = rest.battery_capacity_kwh() + nominal_capacity = rest.nominal_capacity() + if self.get_arg("battery_capacity_nominal", default=False) and nominal_capacity: + if soc_max and abs(soc_max - nominal_capacity) > 1.0: + self.log("Warn: GivTCP: inverter {} reports Battery Capacity {}kWh but nominal indicates {}kWh - using nominal".format(n, soc_max, nominal_capacity)) + soc_max = nominal_capacity + if soc_max: + self.dashboard_item(self._entity_id("sensor", n, "soc_max"), state=soc_max, attributes=GIVTCP_SENSORS["soc_max"], app="givtcp") + + battery_temperature = rest.battery_temperature() + if battery_temperature is not None: + self.dashboard_item(self._entity_id("sensor", n, "battery_temperature"), state=battery_temperature, attributes=GIVTCP_SENSORS["battery_temperature"], app="givtcp") + + inverter_time = rest.inverter_time() + if inverter_time: + self.dashboard_item(self._entity_id("sensor", n, "inverter_time"), state=inverter_time, attributes=GIVTCP_SENSORS["inverter_time"], app="givtcp") + + if max_battery_rate: + self.dashboard_item(self._entity_id("sensor", n, "battery_rate_max"), state=max_battery_rate, attributes=GIVTCP_SENSORS["battery_rate_max"], app="givtcp") + + max_inverter_rate = rest.max_inverter_rate() + if max_inverter_rate: + self.dashboard_item(self._entity_id("sensor", n, "inverter_limit"), state=max_inverter_rate, attributes=GIVTCP_SENSORS["inverter_limit"], app="givtcp") + + # Always published, unlike the values above: "not calibrating" is a real answer that + # Predbat needs, and an absent entity would be indistinguishable from one + self.dashboard_item(self._entity_id("sensor", n, "battery_calibration"), state="on" if rest.in_calibration() else "off", attributes=GIVTCP_SENSORS["battery_calibration"], app="givtcp") + power = rest.power_readings() if power: self.dashboard_item(self._entity_id("sensor", n, "battery_power"), state=power["battery_power"], attributes=GIVTCP_SENSORS["battery_power"], app="givtcp") @@ -217,6 +336,13 @@ async def automatic_config(self): else: keys += GIVTCP_AUTO_CONFIG_POWER_KEYS + keys += GIVTCP_AUTO_CONFIG_DISCOVERY_KEYS + + if all(rest.inverter.rest_v3 for rest in self.rest): + keys += GIVTCP_AUTO_CONFIG_PAUSE_KEYS + else: + self.log("Info: GivTCP: pause control needs GivTCP v3 on every inverter - leaving pause_mode/pause_start_time/pause_end_time to your apps.yaml config") + for key in keys: domain, _, _ = GIVTCP_CONTROLS.get(key, (None, None, None)) domain = domain or "sensor" @@ -254,6 +380,14 @@ async def _set_discharge_slot(self, entity_id, value): end = value if control == "discharge_end_time" else timeslots.get("Discharge_end_time_slot_1", "00:00:00") await self._run_blocking(rest.set_discharge_slot1, start, end) + async def _set_pause_slot(self, entity_id, value): + n, control = self._parse_entity(entity_id) + rest = self.rest[n] + timeslots = rest.inverter.rest_data.get("Timeslots", {}) if rest.inverter.rest_data else {} + start = value if control == "pause_start_time" else timeslots.get("Battery_pause_start_time_slot", "00:00:00") + end = value if control == "pause_end_time" else timeslots.get("Battery_pause_end_time_slot", "00:00:00") + await self._run_blocking(rest.set_pause_slot, start, end) + async def _handle_write(self, entity_id, value, is_switch=False, is_number=False): """ Apply one entity write to the inverter, then immediately republish so the entity reflects it. @@ -274,12 +408,16 @@ async def _handle_write(self, entity_id, value, is_switch=False, is_number=False rest = self.rest[n] _, method_name, _ = GIVTCP_CONTROLS[control] - if method_name in ("_set_charge_slot", "_set_discharge_slot"): + if method_name in ("_set_charge_slot", "_set_discharge_slot", "_set_pause_slot"): await getattr(self, method_name)(entity_id, value) elif is_switch: await self._run_blocking(getattr(rest, method_name), value == "on") elif is_number: await self._run_blocking(getattr(rest, method_name), value) + else: + # Plain select (inverter_mode, pause_mode) - the chosen option is the value the + # write method takes, unlike the slot selects which need both ends of the window + await self._run_blocking(getattr(rest, method_name), value) await self.publish_data() except Exception as e: # A failed write must not propagate into the shared HA event dispatch, which would stop diff --git a/apps/predbat/givtcp_rest.py b/apps/predbat/givtcp_rest.py index ab03e050b..81b8dc4c8 100644 --- a/apps/predbat/givtcp_rest.py +++ b/apps/predbat/givtcp_rest.py @@ -32,7 +32,7 @@ import requests from const import MINUTE_WATT, INVERTER_MAX_RETRY_REST, INVERTER_REST_TIMEOUT -from utils import dp3, time_string_to_stamp +from utils import dp2, dp3, time_string_to_stamp class InverterRestState: @@ -134,6 +134,113 @@ def power_readings(self): "battery_voltage": battery_voltage, } + def inverter_details(self): + """ + The inverter detail block, normalised across GivTCP versions. + + v2 puts it under "Invertor_Details"; v3 renames it to the inverter's own serial number, so + an empty "Invertor_Details" on v3 is expected rather than a fault. Returns {} when neither + is present. + """ + rest_data = self.inverter.rest_data + if not rest_data: + return {} + details = rest_data.get("Invertor_Details", {}) + if details: + return details + serial = rest_data.get("raw", {}).get("invertor", {}).get("serial_number", None) + if serial and serial in rest_data: + return rest_data[serial] + return {} + + def battery_capacity_kwh(self): + """Battery capacity in kWh as GivTCP reports it, or None if absent.""" + value = self.inverter_details().get("Battery_Capacity_kWh", None) + return float(value) if value is not None else None + + def nominal_capacity(self): + """ + Nominal (nameplate) battery capacity in kWh, or None if GivTCP does not report it. + + v2 reports this in raw register units and needs scaling; v3 already reports kWh. The 19.53125 + divisor is carried over verbatim from Inverter.__init__, where it was back-calculated rather + than derived - see the XXX note this replaces. + """ + raw_value = self.inverter.rest_data.get("raw", {}).get("invertor", {}).get("battery_nominal_capacity", None) if self.inverter.rest_data else None + if not raw_value: + return None + if self.inverter.rest_v3: + return float(raw_value) + return float(raw_value) / 19.53125 + + def battery_temperature(self): + """ + Mean BMS temperature across the battery packs, or None if no pack reports one. + + Packs report the field under different names depending on model/firmware, and some nest a + further dict per pack, so all three shapes are averaged together the way Inverter.__init__ + did. + """ + rest_data = self.inverter.rest_data + if not rest_data or "Battery_Details" not in rest_data: + return None + total = 0.0 + count = 0 + for battery in rest_data["Battery_Details"]: + details = rest_data["Battery_Details"][battery] + if "BMS_Temperature" in details: + total += float(details["BMS_Temperature"]) + count += 1 + elif "Battery_Temperature" in details: + total += float(details["Battery_Temperature"]) + count += 1 + else: + for item in details.values(): + if isinstance(item, dict) and "Battery_Temperature" in item: + total += float(item["Battery_Temperature"]) + count += 1 + if not count: + return None + return dp2(total / count) + + def inverter_time(self): + """The inverter's own clock as GivTCP reports it, or None if absent.""" + return self.inverter_details().get("Invertor_Time", None) + + def max_battery_rate(self): + """Maximum battery charge/discharge rate in W, or None if GivTCP does not report one.""" + details = self.inverter_details() + for key in ("Invertor_Max_Bat_Rate", "Invertor_Max_Rate"): + if key in details: + return float(details[key]) + return None + + def max_inverter_rate(self): + """Maximum inverter throughput in W, or None if GivTCP does not report one.""" + value = self.inverter_details().get("Invertor_Max_Inv_Rate", None) + return float(value) if value is not None else None + + def in_calibration(self): + """ + Whether the battery is currently being calibrated, during which Predbat cannot function. + + v3 exposes this directly as Control.Battery_Calibration; older GivTCP only has the raw + soc_force_adjust register, where values 1-6 mean a calibration is in progress. + """ + rest_data = self.inverter.rest_data + if not rest_data: + return False + if self.inverter.rest_v3: + return rest_data.get("Control", {}).get("Battery_Calibration", "Off") != "Off" + soc_force_adjust = rest_data.get("raw", {}).get("invertor", {}).get("soc_force_adjust", None) + if not soc_force_adjust: + return False + try: + soc_force_adjust = int(soc_force_adjust) + except (ValueError, TypeError): + return False + return 0 < soc_force_adjust < 7 + def charge_window_times(self): """Current charge window as (start, end) parsed timestamps, or None if no status has been read yet.""" diff --git a/apps/predbat/inverter.py b/apps/predbat/inverter.py index 79f36a320..0aa447c8d 100644 --- a/apps/predbat/inverter.py +++ b/apps/predbat/inverter.py @@ -30,14 +30,6 @@ TIME_FORMAT_HMS = "%H:%M:%S" -# GivTCP raw.invertor.model values confirmed not to support the Discharge_Target_SOC_1 register -# (#4517). "Ac" (AC Coupled) and "Hybrid_gen1" are confirmed live - the latter on two separate -# reporter inverters, still repeating the write every cycle post-fix until added here. -# "Hybrid_gen2" is inferred from the same GivEnergy firmware-archive generational split -# (github.com/DJBenson/giv-firmware), not independently confirmed on real Gen2 hardware yet. -DISCHARGE_TARGET_UNSUPPORTED_MODELS = ("Ac", "Hybrid_gen1", "Hybrid_gen2") - - class Inverter: """Unified inverter control abstraction for multiple brands. @@ -304,117 +296,29 @@ def __init__(self, base, id=0, quiet=False, rest_postCommand=None, rest_getData= self.inv_has_timed_pause = False self.log("Inverter {} does not have timed pause support enabled".format(self.id)) - # Battery size, charge and discharge rates - ivtime = None - if self.rest_data and ("Battery_Details" in self.rest_data): - average_temp = 0 - battery_count = 0 - for battery in self.rest_data["Battery_Details"]: - battery_details = self.rest_data["Battery_Details"][battery] - if "BMS_Temperature" in battery_details: - average_temp += float(battery_details["BMS_Temperature"]) - battery_count += 1 - elif "Battery_Temperature" in battery_details: - average_temp += float(battery_details["Battery_Temperature"]) - battery_count += 1 - else: - for item in battery_details.values(): - if type(item) is dict: - if "Battery_Temperature" in item: - average_temp += float(item["Battery_Temperature"]) - battery_count += 1 - if battery_count > 0: - average_temp /= battery_count - self.battery_temperature = dp2(average_temp) - - if self.rest_data and ("Invertor_Details" in self.rest_data): - idetails = self.rest_data["Invertor_Details"] - if "Battery_Capacity_kWh" in idetails: - self.soc_max = float(idetails["Battery_Capacity_kWh"]) - self.nominal_capacity = self.soc_max - self.soc_max *= self.battery_scaling - self.soc_max = dp3(self.soc_max) - - if self.rest_data and ("raw" in self.rest_data): - raw_data = self.rest_data["raw"] - - # for V3 the inverter details is now named after the serial number - if self.serial_number in self.rest_data: - idetails = self.rest_data[self.serial_number] - if "Battery_Capacity_kWh" in idetails: - self.soc_max = float(idetails["Battery_Capacity_kWh"]) - self.nominal_capacity = self.soc_max - self.soc_max *= self.battery_scaling - self.soc_max = dp3(self.soc_max) - - # Battery capacity nominal - battery_capacity_nominal = raw_data.get("invertor", {}).get("battery_nominal_capacity", None) - if battery_capacity_nominal: - if self.rest_v3: - self.nominal_capacity = float(battery_capacity_nominal) - else: - self.nominal_capacity = float(battery_capacity_nominal) / 19.53125 # XXX: Where does 19.53125 come from? I back calculated but why that number... - - if self.base.battery_capacity_nominal: - if abs(self.soc_max - self.nominal_capacity) > 1.0: - # XXX: Weird workaround for battery reporting wrong capacity issue - self.base.log("Warn: REST data reports Battery Capacity kWh as {} but nominal indicates {} - using nominal".format(self.soc_max, self.nominal_capacity)) - self.soc_max = self.nominal_capacity * self.battery_scaling - - # Rest fails to return battery capacity - if not self.nominal_capacity: - self.log("Warn: REST data does not report Battery Capacity kWh, attempting to use soc_max apps.yaml instead as fallback for nominal capacity") - self.nominal_capacity = self.base.get_arg("soc_max", default=0.0, index=self.id) - self.soc_max = self.nominal_capacity * self.battery_scaling - - if self.rest_v3: - # GivTCP v3 indicates battery is being calibrated via [Control][Battery_Calibration] - if ("Control" in self.rest_data) and ("Battery_Calibration" in self.rest_data["Control"]): - soc_force_adjust = self.rest_data["Control"]["Battery_Calibration"] - if soc_force_adjust != "Off": - self.in_calibration = True - else: - # older GivTCP uses soc_force_adjust to indicate battery calibration - soc_force_adjust = raw_data.get("invertor", {}).get("soc_force_adjust", None) - if soc_force_adjust: - try: - soc_force_adjust = int(soc_force_adjust) - except ValueError: - soc_force_adjust = 0 - if (soc_force_adjust > 0) and (soc_force_adjust < 7): - self.in_calibration = True - - if self.in_calibration: - self.log("Warn: Inverter {} is in calibration mode '{}', Predbat will not function correctly and will be disabled".format(self.id, soc_force_adjust)) - - # Max battery rate - if "Invertor_Max_Bat_Rate" in idetails: - self.battery_rate_max_raw = idetails["Invertor_Max_Bat_Rate"] - elif "Invertor_Max_Rate" in idetails: - self.battery_rate_max_raw = idetails["Invertor_Max_Rate"] - else: - self.battery_rate_max_raw = self.base.get_arg("charge_rate", attribute="max", index=self.id, default=2600.0, required_unit="W") - - # Max invertor rate - if "Invertor_Max_Inv_Rate" in idetails: - self.inverter_limit = idetails["Invertor_Max_Inv_Rate"] / MINUTE_WATT - - # Inverter time - if "Invertor_Time" in idetails: - ivtime = idetails["Invertor_Time"] + # Battery/capacity discovery is an ordinary entity read for every inverter type now. For + # GivTCP these entities are published by GivTCPComponent, which does the REST reading and + # the version normalisation (v2's Invertor_Details vs v3's serial-named block, the nominal + # capacity scaling, the per-pack temperature averaging) - see givtcp.py. + self.battery_temperature = self.base.get_arg("battery_temperature", default=20, index=self.id, required_unit="\u00b0C") + self.nominal_capacity = self.base.get_arg("soc_max", default=0.0, index=self.id) + self.soc_max = self.nominal_capacity * self.battery_scaling + + if self.inverter_type in ["GE", "GEC", "GEE"]: + self.battery_rate_max_raw = self.base.get_arg("charge_rate", attribute="max", index=self.id, default=2600.0, required_unit="W") + elif "battery_rate_max" in self.base.args: + self.battery_rate_max_raw = self.base.get_arg("battery_rate_max", index=self.id, default=2600.0, required_unit="W") else: - self.battery_temperature = self.base.get_arg("battery_temperature", default=20, index=self.id, required_unit="°C") - self.nominal_capacity = self.base.get_arg("soc_max", default=0.0, index=self.id) - self.soc_max = self.nominal_capacity * self.battery_scaling - - if self.inverter_type in ["GE", "GEC", "GEE"]: - self.battery_rate_max_raw = self.base.get_arg("charge_rate", attribute="max", index=self.id, default=2600.0, required_unit="W") - elif "battery_rate_max" in self.base.args: - self.battery_rate_max_raw = self.base.get_arg("battery_rate_max", index=self.id, default=2600.0, required_unit="W") - else: - self.battery_rate_max_raw = 2600.0 + self.battery_rate_max_raw = 2600.0 + + ivtime = self.base.get_arg("inverter_time", index=self.id, default=None) - ivtime = self.base.get_arg("inverter_time", index=self.id, default=None) + # A calibration cycle deliberately drives the battery outside its normal SoC range, so any + # plan made during one is wrong - Predbat disables itself for this inverter until it ends. + # Only inverters that report it configure battery_calibration; absent means never calibrating. + if self.base.get_arg("battery_calibration", default=None, index=self.id) in ("on", "On", "true", "True", True): + self.in_calibration = True + self.log("Warn: Inverter {} is in calibration mode, Predbat will not function correctly and will be disabled".format(self.id)) # Battery rate max charge, discharge (all converted to kW/min) inverter_limit_charge = self.base.get_arg("inverter_limit_charge", self.battery_rate_max_raw, index=self.id, required_unit="W") @@ -2246,45 +2150,40 @@ def adjust_pause_mode(self, pause_charge=False, pause_discharge=False): entity_start = self.base.get_arg("pause_start_time", indirect=False, index=self.id) entity_end = self.base.get_arg("pause_end_time", indirect=False, index=self.id) - if self.rest_data and self.rest_v3: - old_pause_mode = self.rest_data.get("Control", {}).get("Battery_pause_mode", "Disabled") - old_start_time = self.rest_data.get("Timeslots", {}).get("Battery_pause_start_time_slot", "00:00:00") - old_end_time = self.rest_data.get("Timeslots", {}).get("Battery_pause_end_time_slot", "00:00:00") - else: - entity_mode = self.base.get_arg("pause_mode", indirect=False, index=self.id) - old_pause_mode = None - old_start_time = None - old_end_time = None - - # As not all inverters have these options we need to gracefully give up if its missing - if entity_mode: - old_pause_mode = self.base.get_state_wrapper(entity_mode) - if old_pause_mode is None: - entity_mode = None - - if entity_start: - old_start_time = self.base.get_state_wrapper(entity_start) - if old_start_time is None: - entity_start = None - self.log("Note: Inverter {} does not have pause_start_time entity".format(self.id)) - - if entity_end: - old_end_time = self.base.get_state_wrapper(entity_end) - if old_end_time is None: - self.log("Note: Inverter {} does not have pause_end_time entity".format(self.id)) - entity_end = None - - if not entity_mode: - self.log("Warn: Inverter {} does not have pause_mode entity configured correctly".format(self.id)) - return + entity_mode = self.base.get_arg("pause_mode", indirect=False, index=self.id) + old_pause_mode = None + old_start_time = None + old_end_time = None + + # As not all inverters have these options we need to gracefully give up if its missing + if entity_mode: + old_pause_mode = self.base.get_state_wrapper(entity_mode) + if old_pause_mode is None: + entity_mode = None + + if entity_start: + old_start_time = self.base.get_state_wrapper(entity_start) + if old_start_time is None: + entity_start = None + self.log("Note: Inverter {} does not have pause_start_time entity".format(self.id)) + + if entity_end: + old_end_time = self.base.get_state_wrapper(entity_end) + if old_end_time is None: + self.log("Note: Inverter {} does not have pause_end_time entity".format(self.id)) + entity_end = None + + if not entity_mode: + self.log("Warn: Inverter {} does not have pause_mode entity configured correctly".format(self.id)) + return # Some inverters have start/end time registers new_start_time = "00:00:00" new_end_time = "23:59:00" - # GE Cloud has different pause names - if self.rest_data and self.rest_v3: - pause_cloud = False + # GE Cloud has different pause names. GivTCP's own spelling (Disabled/PauseCharge/...) is + # what the GivTCPComponent publishes, so a GivTCP-backed entity lands on the False side here + # exactly as the old REST branch did. if old_pause_mode in ["Not Paused", "Pause Charge", "Pause Discharge", "Pause Charge & Discharge"]: pause_cloud = True else: @@ -2300,27 +2199,19 @@ def adjust_pause_mode(self, pause_charge=False, pause_discharge=False): else: new_pause_mode = "Not Paused" if pause_cloud else "Disabled" - if self.rest_data and self.rest_v3: - if entity_start and ((old_start_time != new_start_time) or (old_end_time != new_end_time)): - self.base.log("Inverter {} set pause slot to {} - {}".format(self.id, new_start_time, new_end_time)) - self.givtcp.set_pause_slot(new_start_time, new_end_time) - else: - if old_start_time and old_start_time != new_start_time: - # Don't poll as inverters with no registers will fail - self.write_and_poll_option("pause_start_time", entity_start, new_start_time, ignore_fail=True) - self.base.log("Inverter {} set pause start time to {}".format(self.id, new_start_time)) + if old_start_time and old_start_time != new_start_time: + # Don't poll as inverters with no registers will fail + self.write_and_poll_option("pause_start_time", entity_start, new_start_time, ignore_fail=True) + self.base.log("Inverter {} set pause start time to {}".format(self.id, new_start_time)) - if old_end_time and old_end_time != new_end_time: - # Don't poll as inverters with no registers will fail - self.write_and_poll_option("pause_end_time", entity_end, new_end_time, ignore_fail=True) - self.base.log("Inverter {} set pause end time to {}".format(self.id, new_end_time)) + if old_end_time and old_end_time != new_end_time: + # Don't poll as inverters with no registers will fail + self.write_and_poll_option("pause_end_time", entity_end, new_end_time, ignore_fail=True) + self.base.log("Inverter {} set pause end time to {}".format(self.id, new_end_time)) # Set the mode if new_pause_mode != old_pause_mode: - if self.rest_data and self.rest_v3: - self.givtcp.set_battery_pause_mode(new_pause_mode) - else: - self.write_and_poll_option("pause_mode", entity_mode, new_pause_mode) + self.write_and_poll_option("pause_mode", entity_mode, new_pause_mode) if self.base.set_inverter_notify: self.base.call_notify("Predbat: Inverter {} pause mode to set {} at time {}".format(self.id, new_pause_mode, self.base.time_now_str())) @@ -2344,17 +2235,19 @@ def adjust_inverter_mode(self, force_export, changed_start_end=False): inverter_mode string """ - inverter_mode_configured = False - if self.rest_data: - old_inverter_mode = self.rest_data["Control"]["Mode"] - else: - inverter_mode_configured = "inverter_mode" in self.base.args - # Inverter mode - if changed_start_end and not self.rest_data: - # XXX: Workaround for GivTCP window state update time to take effort - self.base.log("Sleeping (workaround) as start/end of discharge window was just adjusted") - self.sleep(30) - old_inverter_mode = self.base.get_arg("inverter_mode", index=self.id) + inverter_mode_configured = "inverter_mode" in self.base.args + # Inverter mode + # + # The sleep is a workaround for the lag between writing a window via GivTCP's own HA + # integration entities and GivTCP reflecting it back. It stays gated on rest_api because a + # GivTCP-REST inverter does not have that lag: GivTCPComponent applies the write and + # republishes the entity inline before the write call returns (see its _handle_write), the + # same way the direct REST path used to, so sleeping 30s every window change would be pure + # cost. Entities fed by anything else keep the workaround. + if changed_start_end and not self.rest_api: + self.base.log("Sleeping (workaround) as start/end of discharge window was just adjusted") + self.sleep(30) + old_inverter_mode = self.base.get_arg("inverter_mode", index=self.id) if not self.inv_has_fox_inverter_mode and not self.inv_has_ge_eco_toggle: # For the purpose of this function consider Eco Paused as the same as Eco (it's a difference in reserve setting) @@ -2379,20 +2272,17 @@ def adjust_inverter_mode(self, force_export, changed_start_end=False): # Change inverter mode if old_inverter_mode != new_inverter_mode: self.log("Inverter {} current mode is {} and new target is {} has_ge_eco_toggle {}".format(self.id, old_inverter_mode, new_inverter_mode, self.inv_has_ge_eco_toggle)) - if self.rest_data: - self.givtcp.set_battery_mode(new_inverter_mode) - else: - entity_id = self.base.get_arg("inverter_mode", indirect=False, index=self.id) - if self.inv_has_ge_eco_toggle: - # GE has an eco toggle rather than a mode, so we write the opposite of the force export to the eco toggle - if entity_id: - self.write_and_poll_switch("inverter_mode", entity_id, new_inverter_mode == "on") - else: - if not inverter_mode_configured: - self.log("Warn: Inverter {} adjust_inverter_mode: No entity_id for ECO Toggle, inverter_mode should be set to xxx_enable_eco_mode".format(self.id)) - return + entity_id = self.base.get_arg("inverter_mode", indirect=False, index=self.id) + if self.inv_has_ge_eco_toggle: + # GE has an eco toggle rather than a mode, so we write the opposite of the force export to the eco toggle + if entity_id: + self.write_and_poll_switch("inverter_mode", entity_id, new_inverter_mode == "on") else: - self.write_and_poll_option("inverter_mode", entity_id, new_inverter_mode) + if not inverter_mode_configured: + self.log("Warn: Inverter {} adjust_inverter_mode: No entity_id for ECO Toggle, inverter_mode should be set to xxx_enable_eco_mode".format(self.id)) + return + else: + self.write_and_poll_option("inverter_mode", entity_id, new_inverter_mode) # Notify if self.base.set_inverter_notify: @@ -2632,23 +2522,10 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No # reads back as None, and writing to it every cycle just produces errors. if force_export: target_soc = int(self.reserve_percent) - if self.rest_data and self.rest_v3: - # Some GivTCP inverter models don't have a working Discharge_Target_SOC_1 register - - # GivTCP still reports a write as successful, but it never persists between cycles, so - # the caller sees a permanent mismatch and rewrites indefinitely (#4517). See - # DISCHARGE_TARGET_UNSUPPORTED_MODELS above for what's confirmed vs inferred. - inverter_model = self.rest_data.get("raw", {}).get("invertor", {}).get("model", "") - if inverter_model in DISCHARGE_TARGET_UNSUPPORTED_MODELS: - self.log("Inverter {} is {}, discharge target register not supported, export target not written".format(self.id, inverter_model)) - else: - current = self.givtcp.read_discharge_target() - if current is None: - self.log("Inverter {} No current discharge target to read, export target not written".format(self.id)) - elif current != target_soc: - self.givtcp.set_discharge_target(target_soc) - else: - self.log("Inverter {} Current discharge target is already set to {}".format(self.id, current)) - elif "discharge_target_soc" in self.base.args: + # A model with no working Discharge_Target_SOC_1 register (#4517) reaches here with no + # entity published for it, so the "cannot read it, leave it alone" branch below is what + # stops the every-cycle rewrite - see DISCHARGE_TARGET_UNSUPPORTED_MODELS in givtcp.py. + if "discharge_target_soc" in self.base.args: current = self.base.get_arg("discharge_target_soc", index=self.id, required_unit="%") try: current = float(current) diff --git a/apps/predbat/tests/test_givtcp_component.py b/apps/predbat/tests/test_givtcp_component.py index 2392a4726..ad0ffd430 100644 --- a/apps/predbat/tests/test_givtcp_component.py +++ b/apps/predbat/tests/test_givtcp_component.py @@ -6,14 +6,31 @@ apps.yaml entity keys at them via automatic_config(). """ +import json + from unittest.mock import MagicMock from tests.test_infra import run_async from mock_base import MockBase -from givtcp import GivTCPComponent, GIVTCP_POLL_SECONDS - - -def _rest_data_blob(charge_rate=1000, discharge_rate=2000, target_soc=80, reserve=10, soc_kwh=5.0, soc_percent=50, charge_start="00:30:00", charge_end="04:30:00", discharge_start="16:00:00", discharge_end="19:00:00"): +from givtcp import GivTCPComponent, GIVTCP_POLL_SECONDS, DISCHARGE_TARGET_UNSUPPORTED_MODELS + + +def _rest_data_blob( + charge_rate=1000, + discharge_rate=2000, + target_soc=80, + reserve=10, + soc_kwh=5.0, + soc_percent=50, + charge_start="00:30:00", + charge_end="04:30:00", + discharge_start="16:00:00", + discharge_end="19:00:00", + mode="Eco", + pause_mode="Disabled", + pause_start="00:00:00", + pause_end="00:00:00", +): """A realistic-shaped GivTCP /readData response, sized to what publish_data() reads.""" return { "Control": { @@ -24,6 +41,8 @@ def _rest_data_blob(charge_rate=1000, discharge_rate=2000, target_soc=80, reserv "Enable_Charge_Schedule": "enable", "Enable_Discharge_Schedule": "disable", "Discharge_Target_SOC_1": 20, + "Mode": mode, + "Battery_pause_mode": pause_mode, }, "Power": {"Power": {"SOC_kWh": soc_kwh, "SOC": soc_percent, "Battery_Power": 100.0, "PV_Power": 200.0, "Grid_Power": -50.0, "Load_Power": 250.0, "Battery_Voltage": 51.2}}, "Timeslots": { @@ -31,6 +50,8 @@ def _rest_data_blob(charge_rate=1000, discharge_rate=2000, target_soc=80, reserv "Charge_end_time_slot_1": charge_end, "Discharge_start_time_slot_1": discharge_start, "Discharge_end_time_slot_1": discharge_end, + "Battery_pause_start_time_slot": pause_start, + "Battery_pause_end_time_slot": pause_end, }, } @@ -249,6 +270,236 @@ def test_select_event_discharge_end_time_preserves_start(my_predbat=None): return 0 +def test_publish_data_mode_entities(my_predbat=None): + """inverter_mode publishes on any version; the pause entities are v3 only.""" + base, component = _make_component() + component.rest[0].inverter.rest_data = _rest_data_blob(mode="Timed Export", pause_mode="PauseCharge", pause_start="01:00:00", pause_end="02:00:00") + + # v2: no /setBatteryPauseMode endpoint, so publishing a pause entity would offer a control that + # can never be written - Inverter.adjust_pause_mode's REST path was gated on v3 for the same reason + component.rest[0].inverter.rest_v3 = False + run_async(component.publish_data()) + assert base.entities["select.predbat_givtcp_0_inverter_mode"]["state"] == "Timed Export", f"Expected inverter_mode published, got {base.entities['select.predbat_givtcp_0_inverter_mode']['state']}" + assert "select.predbat_givtcp_0_pause_mode" not in base.entities, "pause_mode must not be published for GivTCP v2" + + component.rest[0].inverter.rest_v3 = True + run_async(component.publish_data()) + assert base.entities["select.predbat_givtcp_0_pause_mode"]["state"] == "PauseCharge", f"Expected pause_mode published, got {base.entities['select.predbat_givtcp_0_pause_mode']['state']}" + assert base.entities["select.predbat_givtcp_0_pause_start_time"]["state"] == "01:00:00" + assert base.entities["select.predbat_givtcp_0_pause_end_time"]["state"] == "02:00:00" + + print("PASS: inverter_mode publishes always, pause entities only on v3") + return 0 + + +def test_select_event_plain_selects_pass_the_value_through(my_predbat=None): + """ + A select that is not a time slot passes its chosen option straight to the write method. + + Regression: _handle_write only had branches for the slot selects, switches and numbers, so a + plain select silently did nothing - the write was dropped and the entity then republished with + the unchanged value, which reads as a successful no-op. + """ + base, component = _make_component() + component.rest[0].inverter.rest_data = _rest_data_blob() + component.rest[0].set_battery_mode = MagicMock(return_value=True) + component.rest[0].set_battery_pause_mode = MagicMock(return_value=True) + + run_async(component.select_event("select.predbat_givtcp_0_inverter_mode", "Timed Export")) + component.rest[0].set_battery_mode.assert_called_once_with("Timed Export") + + run_async(component.select_event("select.predbat_givtcp_0_pause_mode", "PauseBoth")) + component.rest[0].set_battery_pause_mode.assert_called_once_with("PauseBoth") + + print("PASS: plain selects pass their option through to the write method") + return 0 + + +def test_select_event_pause_start_time_preserves_end(my_predbat=None): + """Changing pause_start_time writes the new start alongside the existing end, like the charge slot.""" + base, component = _make_component() + component.rest[0].inverter.rest_data = _rest_data_blob(pause_start="00:00:00", pause_end="23:59:00") + component.rest[0].set_pause_slot = MagicMock(return_value=True) + + run_async(component.select_event("select.predbat_givtcp_0_pause_start_time", "01:00:00")) + + component.rest[0].set_pause_slot.assert_called_once_with("01:00:00", "23:59:00") + print("PASS: select_event on pause_start_time preserves the existing end time") + return 0 + + +def test_automatic_config_pause_keys_need_v3_everywhere(my_predbat=None): + """Pause keys are only claimed when every inverter is v3, mirroring the power_ignore rule.""" + base, component = _make_component(rest_urls=["http://givtcp0:6345", "http://givtcp1:6345"]) + for rest in component.rest: + rest.inverter.rest_data = _rest_data_blob() + + # A mixed fleet must leave the pause keys alone rather than pointing the v2 inverter at an + # entity its GivTCP can never accept a write for + component.rest[0].inverter.rest_v3 = True + component.rest[1].inverter.rest_v3 = False + run_async(component.automatic_config()) + assert "pause_mode" not in base.args, f"Expected pause_mode left unconfigured on a mixed fleet, got {base.args.get('pause_mode')}" + # inverter_mode is not version gated, so it is still claimed + assert "inverter_mode" in base.args, "Expected inverter_mode to be auto-configured regardless of version" + + component.rest[1].inverter.rest_v3 = True + run_async(component.automatic_config()) + assert base.args.get("pause_mode") == ["select.predbat_givtcp_0_pause_mode", "select.predbat_givtcp_1_pause_mode"], f"Expected pause_mode configured for both, got {base.args.get('pause_mode')}" + + print("PASS: pause keys are auto-configured only when every inverter is v3") + return 0 + + +def test_discharge_target_not_published_for_unsupported_models(my_predbat=None): + """ + Regression test for issue #4517, moved here with the model check itself. + + Some GivTCP inverter models have no working Discharge_Target_SOC_1 register - GivTCP reports a + write as successful, but it never persists between cycles, so the caller sees a permanent + mismatch and rewrites indefinitely. "Ac" (AC Coupled) was confirmed first; "Hybrid_gen1" was + added after a reporter confirmed live, post-fix, that two of his Gen1 inverters still repeated + the write every cycle while a third, genuinely AC Coupled, correctly stopped. + + Not publishing the entity is now what stops it: Inverter.adjust_force_export leaves a target it + cannot read alone, so an absent entity means no write is ever attempted. + """ + entity_id = "number.predbat_givtcp_0_discharge_target_soc" + + for model in DISCHARGE_TARGET_UNSUPPORTED_MODELS: + base, component = _make_component() + component.rest[0].inverter.rest_data = _rest_data_blob() + component.rest[0].inverter.rest_data["raw"] = {"invertor": {"model": model, "discharge_target_soc_1": "4"}} + run_async(component.publish_data()) + assert entity_id not in base.entities, f"model={model!r} must not publish a discharge target entity" + + # A model not on the list (including a later Hybrid generation, or none reported at all) must + # still get the entity, so the write goes ahead exactly as before + for model in ["Hybrid", "Hybrid_gen3", ""]: + base, component = _make_component() + component.rest[0].inverter.rest_data = _rest_data_blob() + component.rest[0].inverter.rest_data["raw"] = {"invertor": {"model": model, "discharge_target_soc_1": "4"}} + run_async(component.publish_data()) + assert entity_id in base.entities, f"model={model!r} should still publish a discharge target entity" + + print("PASS: discharge target entity is withheld only for the unsupported models") + return 0 + + +def _rest_from_fixture(filename): + """A component whose single inverter holds a real captured GivTCP /readData response.""" + base, component = _make_component() + with open(filename, "r") as handle: + component.rest[0].inverter.rest_data = json.load(handle) + version = component.rest[0].inverter.rest_data.get("Stats", {}).get("GivTCP_Version", "Unknown") + component.rest[0].inverter.rest_v3 = version.startswith("3") + return base, component + + +def test_discovery_parsing_against_real_captures(my_predbat=None): + """ + Battery/capacity discovery parsed from real captured GivTCP responses, v2 and v3. + + These assertions moved here with the parsing itself: Inverter.__init__ used to read the REST + blob directly and this pinned what it produced, but the version normalisation now lives in + GivTCPRest. v3 is the interesting case - it renames the Invertor_Details block to the inverter's + own serial number, and reports nominal capacity in kWh where v2 reports raw register units. + """ + failed = 0 + + base, component = _rest_from_fixture("cases/rest_v2.json") + rest = component.rest[0] + checks_v2 = { + "battery_capacity_kwh": (rest.battery_capacity_kwh(), 9.523200000000001), + "nominal_capacity": (rest.nominal_capacity(), 9.5232), + "battery_temperature": (rest.battery_temperature(), 15.3), + "max_battery_rate": (rest.max_battery_rate(), 2600), + "max_inverter_rate": (rest.max_inverter_rate(), 3600), + "in_calibration": (rest.in_calibration(), False), + } + for name, (got, expected) in checks_v2.items(): + if got != expected: + print(f"ERROR: v2 {name}: expected {expected}, got {got}") + failed = 1 + if not rest.inverter_time(): + print("ERROR: v2 inverter_time should be reported") + failed = 1 + + base, component = _rest_from_fixture("cases/rest_v3.json") + rest = component.rest[0] + # v3 keeps the detail block under the serial number, so an empty Invertor_Details is expected + checks_v3 = { + "battery_capacity_kwh": (rest.battery_capacity_kwh(), 9.52), + "battery_temperature": (rest.battery_temperature(), 25.0), + "max_battery_rate": (rest.max_battery_rate(), 3600), + "max_inverter_rate": (rest.max_inverter_rate(), 3600), + "in_calibration": (rest.in_calibration(), False), + } + for name, (got, expected) in checks_v3.items(): + if got != expected: + print(f"ERROR: v3 {name}: expected {expected}, got {got}") + failed = 1 + if not rest.inverter_time(): + print("ERROR: v3 inverter_time should be reported") + failed = 1 + + if not failed: + print("PASS: discovery parses correctly from both real GivTCP captures") + return failed + + +def test_calibration_detected_per_version(my_predbat=None): + """ + Calibration is reported differently by version and must be detected either way. + + A calibration cycle deliberately drives the battery outside its normal SoC range, so Predbat + disables itself while one runs - missing it means planning against a battery that is not + behaving normally. v3 exposes Control.Battery_Calibration directly; older GivTCP only has the + raw soc_force_adjust register, where 1-6 means in progress. + """ + failed = 0 + + base, component = _make_component() + rest = component.rest[0] + + rest.inverter.rest_v3 = True + for value, expected in [("Off", False), ("On", True), ("Calibrating", True)]: + rest.inverter.rest_data = {"Control": {"Battery_Calibration": value}} + if rest.in_calibration() != expected: + print(f"ERROR: v3 Battery_Calibration={value!r}: expected {expected}, got {rest.in_calibration()}") + failed = 1 + + rest.inverter.rest_v3 = False + for value, expected in [(0, False), (1, True), (6, True), (7, False), (None, False), ("bad", False)]: + rest.inverter.rest_data = {"raw": {"invertor": {"soc_force_adjust": value}}} + if rest.in_calibration() != expected: + print(f"ERROR: v2 soc_force_adjust={value!r}: expected {expected}, got {rest.in_calibration()}") + failed = 1 + + if not failed: + print("PASS: calibration is detected on both GivTCP versions") + return failed + + +def test_rate_entities_carry_the_real_max(my_predbat=None): + """ + The rate entities must advertise the inverter's own maximum, not the generic ceiling. + + Inverter.__init__ derives battery_rate_max_raw for a GE inverter from the charge_rate entity's + "max" attribute. Publishing GIVTCP_CONTROLS' generic 20000 would tell Predbat the battery can + take 20kW - it only went unnoticed while REST discovery was still overriding it. + """ + base, component = _rest_from_fixture("cases/rest_v2.json") + run_async(component.publish_data()) + + for entity_id in ("number.predbat_givtcp_0_charge_rate", "number.predbat_givtcp_0_discharge_rate"): + got = base.entities[entity_id]["attributes"]["max"] + assert got == 2600, f"{entity_id}: expected max 2600 from Invertor_Max_Bat_Rate, got {got}" + + print("PASS: rate entities advertise the inverter's real maximum rate") + return 0 + + def test_arbitrary_minute_time_is_a_valid_option(my_predbat=None): """ The published select options must cover every minute, not a coarser step. @@ -425,6 +676,14 @@ def test_givtcp_component(my_predbat=None): ("switch_toggle_ignored", test_switch_event_toggle_ignored, "switch_event toggle ignored"), ("select_charge_start", test_select_event_charge_start_time_preserves_end, "select_event charge_start_time"), ("select_discharge_end", test_select_event_discharge_end_time_preserves_start, "select_event discharge_end_time"), + ("publish_modes", test_publish_data_mode_entities, "publish_data mode/pause entities"), + ("select_plain", test_select_event_plain_selects_pass_the_value_through, "plain select writes pass through"), + ("select_pause_start", test_select_event_pause_start_time_preserves_end, "select_event pause_start_time"), + ("auto_config_pause", test_automatic_config_pause_keys_need_v3_everywhere, "pause keys need v3 everywhere"), + ("discharge_target_models", test_discharge_target_not_published_for_unsupported_models, "discharge target withheld for unsupported models (#4517)"), + ("discovery_captures", test_discovery_parsing_against_real_captures, "discovery parsed from real GivTCP captures"), + ("calibration", test_calibration_detected_per_version, "calibration detected on both versions"), + ("rate_max_attr", test_rate_entities_carry_the_real_max, "rate entities carry the real max"), ("time_options", test_arbitrary_minute_time_is_a_valid_option, "every minute is a valid time option"), ("unknown_control", test_unknown_entity_write_logged_not_crashed, "unknown control entity write"), ("unknown_inverter", test_unknown_inverter_index_write_logged_not_crashed, "out-of-range inverter index write"), diff --git a/apps/predbat/tests/test_inverter.py b/apps/predbat/tests/test_inverter.py index 36c37b416..56a4f7c51 100644 --- a/apps/predbat/tests/test_inverter.py +++ b/apps/predbat/tests/test_inverter.py @@ -16,8 +16,7 @@ from utils import calc_percent_limit from tests.test_infra import TestHAInterface from predbat import PredBat -from const import MINUTE_WATT, INVERTER_MAX_RETRY_REST -from inverter import Inverter, DISCHARGE_TARGET_UNSUPPORTED_MODELS +from inverter import Inverter from config import INVERTER_DEF @@ -347,18 +346,20 @@ def test_adjust_force_export(test_name, ha, inv, dummy_rest, prev_start, prev_en dummy1 = copy.deepcopy(inv.rest_data) dummy1["raw"]["invertor"]["discharge_target_soc_1"] = inv.reserve_precent if new_force_export else prev_discharge_target - if new_discharge_target != prev_discharge_target: - dummy_rest.queue_rest_data(dummy1) - # Discharge start/end time is now written via entities (write_and_poll_option), not REST - - # only the target SoC and inverter mode below are still written via the direct REST client. + # Discharge start/end time, the inverter mode and the discharge target are all written via + # entities now, not REST, so this phase issues no REST commands at all. Reset the two entities + # first so it asserts its own writes rather than inheriting what the non-REST phase left. + ha.set_state("select.inverter_mode", prev_mode) + ha.set_state("number.discharge_target_soc", prev_discharge_target) + dummy1["Timeslots"]["Discharge_start_time_slot_1"] = new_start dummy1["Timeslots"]["Discharge_end_time_slot_1"] = new_end dummy1["Control"]["Mode"] = new_mode dummy1["Control"]["Enable_Discharge_Schedule"] = export_schedule_discharge - if prev_mode != new_mode: - dummy_rest.queue_rest_data(dummy1) + # No queue_rest_data for the mode change: it is an entity write now, so nothing consumes a + # queued REST read-back and an entry left here would leak into the next test's runAll dummy_rest.rest_data = copy.deepcopy(dummy1) @@ -371,15 +372,16 @@ def test_adjust_force_export(test_name, ha, inv, dummy_rest, prev_start, prev_en inv.adjust_force_export(new_force_export, new_start_timestamp, new_end_timestamp) rest_command = dummy_rest.get_commands() - expect_data = [] - if new_discharge_target != prev_discharge_target: - expect_data.append(["dummy/setDischargeTarget", {"dischargeToPercent": int(new_discharge_target), "slot": 1}]) - - if prev_mode != new_mode: - expect_data.append(["dummy/setBatteryMode", {"mode": new_mode}]) + if json.dumps([]) != json.dumps(rest_command): + print("ERROR: Rest command should be [] got {}".format(rest_command)) + failed = True - if json.dumps(expect_data) != json.dumps(rest_command): - print("ERROR: Rest command should be {} got {}".format(expect_data, rest_command)) + # The mode and discharge target writes now land on entities even for a REST inverter + if ha.get_state("select.inverter_mode") != new_mode: + print("ERROR: REST inverter mode should be written via the entity as {} got {}".format(new_mode, ha.get_state("select.inverter_mode"))) + failed = True + if ha.get_state("number.discharge_target_soc") != new_discharge_target: + print("ERROR: REST discharge target should be written via the entity as {} got {}".format(new_discharge_target, ha.get_state("number.discharge_target_soc"))) failed = True return failed @@ -484,22 +486,23 @@ def test_adjust_inverter_mode(test_name, ha, inv, dummy_rest, prev_mode, mode, e print("ERROR: Inverter mode should be {} got {}".format(expect_mode, ha.get_state("select.inverter_mode"))) failed = True - # REST Mode + # REST Mode - the mode is written via the entity now (published by GivTCPComponent), so a REST + # inverter issues no setBatteryMode command and lands on the same entity as the path above inv.rest_api = "dummy" inv.rest_data = {} inv.rest_data["Control"] = {} inv.rest_data["Control"]["Mode"] = prev_mode dummy_rest.rest_data = copy.deepcopy(inv.rest_data) dummy_rest.rest_data["Control"]["Mode"] = expect_mode + ha.dummy_items["select.inverter_mode"] = prev_mode inv.adjust_inverter_mode(True if mode == "Timed Export" else False, False) rest_command = dummy_rest.get_commands() - if prev_mode != expect_mode: - expect_data = [["dummy/setBatteryMode", {"mode": expect_mode}]] - else: - expect_data = [] - if json.dumps(expect_data) != json.dumps(rest_command): - print("ERROR: Rest command should be {} got {}".format(expect_data, rest_command)) + if json.dumps([]) != json.dumps(rest_command): + print("ERROR: Rest command should be [] got {}".format(rest_command)) + failed = True + if ha.get_state("select.inverter_mode") != expect_mode: + print("ERROR: REST inverter mode should be written via the entity as {} got {}".format(expect_mode, ha.get_state("select.inverter_mode"))) failed = True return failed @@ -740,98 +743,19 @@ def test_inverter_self_test(test_name, my_predbat): inv.sleep = dummy_sleep inv.self_test(my_predbat.minutes_now) rest = dummy_rest.get_commands() - repeats = INVERTER_MAX_RETRY_REST # configurable number of repeats - expected = [] - # Define the command patterns (each repeated INVERTER_MAX_RETRY_REST times due to the retry loop). - # Battery target/rate/reserve/charge & discharge window/schedule-enable all now write via - # entities regardless of REST config, so they no longer show up as REST commands here - only - # adjust_inverter_mode's mode write remains REST-only (see the "REST exceptions" note in - # inverter.py's Inverter.__init__). old_inverter_mode is read once from the mocked rest_data's - # "Mode" ("Eco") and never mutated by the dummy POSTs, so only the Eco->Timed Export transition - # (mid self_test) ever produces a command; the return to Eco afterwards is a no-op against that - # same stale "Eco" reading. - commands = [ - ["dummy/setBatteryMode", {"mode": "Timed Export"}], - ] - - # Generate expected list with repeats - for command in commands: - for _ in range(repeats): - expected.append(command) + # Battery target/rate/reserve/charge & discharge window/schedule-enable, and now the inverter + # mode too, all write via entities regardless of REST config, so the self test issues no direct + # REST commands at all. The remaining direct-REST users (battery/capacity discovery, the #4517 + # discharge-target model check) are reads or are not exercised here - see the "REST exceptions" + # note in inverter.py's Inverter.__init__. + expected = [] if json.dumps(expected) != json.dumps(rest): print("ERROR: Self test should be {} got {}".format(expected, rest)) failed = True return failed -def test_inverter_rest_template( - test_name, - my_predbat, - filename, - assert_soc_max=9.52, - assert_inverter_limit=3600, - assert_battery_rate_max=2600, - assert_serial_number="Unknown", - assert_nominal_capacity=9.52, - assert_battery_temperature=0, -): - """ - SoC, power, battery voltage and charge/discharge window/enable state are no longer parsed - from raw REST data by Inverter (that now goes via entities, published in production by - GivTCPComponent - see test_givtcp_component.py, and exercised here via test_inverter_update). - This template only still exercises what Inverter.__init__ genuinely discovers from raw REST - data: battery/capacity discovery, calibration, and inverter limits/model info - the REST - exceptions documented in inverter.py's Inverter.__init__. - """ - failed = False - print("**** Running Test: {} ****".format(test_name)) - dummy_rest = DummyRestAPI() - my_predbat.args["givtcp_rest"] = "dummy" - - # Remove inverter_limit and export_limit from config to test REST data parsing - if "inverter_limit" in my_predbat.args: - del my_predbat.args["inverter_limit"] - if "export_limit" in my_predbat.args: - del my_predbat.args["export_limit"] - - dummy_rest.rest_data = {} - with open(filename, "r") as file: - dummy_rest.rest_data = json.load(file) - - my_predbat.restart_active = True - inv = Inverter(my_predbat, 0, rest_postCommand=dummy_rest.dummy_rest_postCommand, rest_getData=dummy_rest.dummy_rest_getData, quiet=False) - inv.sleep = dummy_sleep - - inv.update_status(my_predbat.minutes_now) - my_predbat.restart_active = False - - if assert_soc_max != inv.soc_max: - print("ERROR: SOC Max should be {} got {}".format(assert_soc_max, inv.soc_max)) - failed = True - if assert_inverter_limit != inv.inverter_limit * MINUTE_WATT: - print("ERROR: Inverter limit should be {} got {}".format(assert_inverter_limit, inv.inverter_limit * MINUTE_WATT)) - failed = True - # Verify export_limit defaults correctly from REST data when config unset (should be 99999.0 / MINUTE_WATT = 1.66665) - if inv.export_limit * MINUTE_WATT < 99999.0: - print("ERROR: Export limit should default to 99999 W (1.66665 kW/min) when unset, got {} W ({} kW/min)".format(inv.export_limit * MINUTE_WATT, inv.export_limit)) - failed = True - if assert_battery_rate_max != inv.battery_rate_max_raw: - print("ERROR: Battery rate max should be {} got {}".format(assert_battery_rate_max, inv.battery_rate_max_raw)) - failed = True - if assert_serial_number != inv.serial_number: - print("ERROR: Serial number should be {} got {}".format(assert_serial_number, inv.serial_number)) - failed = True - if assert_nominal_capacity != inv.nominal_capacity: - print("ERROR: Nominal capacity should be {} got {}".format(assert_nominal_capacity, inv.nominal_capacity)) - failed = True - if assert_battery_temperature != inv.battery_temperature: - print("ERROR: Battery temperature should be {} got {}".format(assert_battery_temperature, inv.battery_temperature)) - failed = True - - return failed - - def test_inverter_update( test_name, my_predbat, @@ -1885,10 +1809,12 @@ def setup_entity_case(current_target, reserve_percent): print("ERROR: {}: export target above reserve should be lowered to 20, got {}".format(test_name, ha.get_state("number.discharge_target_soc"))) failed = True - # Case 3: same correction on the REST v3 path + # Case 3: the same correction for an inverter that has REST configured. The target is + # written through the entity now (published by GivTCPComponent) rather than the direct REST + # client, so having rest_api set must no longer divert this to a REST command + setup_entity_case(current_target=4, reserve_percent=20) inv.rest_api = "dummy" inv.rest_v3 = True - inv.reserve_percent = 20 inv.rest_data = { "Control": {"Enable_Discharge_Schedule": "on", "Mode": "Timed Export"}, "Timeslots": {"Discharge_start_time_slot_1": start_time, "Discharge_end_time_slot_1": end_time}, @@ -1896,13 +1822,13 @@ def setup_entity_case(current_target, reserve_percent): } dummy_rest.clear_queue() dummy_rest.rest_data = copy.deepcopy(inv.rest_data) - polled = copy.deepcopy(inv.rest_data) - polled["raw"]["invertor"]["discharge_target_soc_1"] = 20 - dummy_rest.queue_rest_data(polled) inv.adjust_force_export(True, ts, te) - if inv.rest_data["raw"]["invertor"]["discharge_target_soc_1"] != 20: - print("ERROR: {}: REST export target below reserve should be raised to 20, got {}".format(test_name, inv.rest_data["raw"]["invertor"]["discharge_target_soc_1"])) + if float(ha.get_state("number.discharge_target_soc")) != 20: + print("ERROR: {}: REST export target below reserve should be raised to 20 via the entity, got {}".format(test_name, ha.get_state("number.discharge_target_soc"))) + failed = True + if dummy_rest.get_commands(): + print("ERROR: {}: REST inverter should issue no discharge-target REST command, got {}".format(test_name, dummy_rest.get_commands())) failed = True finally: inv.reserve_percent = saved_reserve_percent @@ -2157,70 +2083,6 @@ def test_discharge_target_control_signal(test_name, ha, inv, dummy_rest): return failed -def test_discharge_target_skipped_for_ac_coupled(test_name, ha, inv, dummy_rest): - """ - Regression test for issue #4517: some GivTCP inverter models (see - DISCHARGE_TARGET_UNSUPPORTED_MODELS) don't have a working Discharge_Target_SOC_1 register - - GivTCP reports a write as successful, but it never persists between cycles, so the caller sees a - permanent mismatch and rewrites indefinitely. "Ac" (AC Coupled) was confirmed first; "Hybrid_gen1" - was added after a reporter confirmed live, post-fix, that two of his Gen1 inverters still repeated - the write every cycle while a third, genuinely AC Coupled, correctly stopped. Skip outright rather - than attempting a write already known to be doomed. - """ - failed = False - print("Test: {}".format(test_name)) - - saved_rest_data = inv.rest_data - saved_rest_api = inv.rest_api - saved_rest_v3 = inv.rest_v3 - - try: - inv.rest_api = "dummy" - inv.rest_v3 = True - inv.reserve_percent = 20 - - start_time = "03:33:00" - end_time = "04:44:00" - ts = datetime.strptime(start_time, "%H:%M:%S") - te = datetime.strptime(end_time, "%H:%M:%S") - - inv.rest_data = { - "Control": {"Mode": "Timed Export", "Enable_Discharge_Schedule": "on"}, - "Timeslots": {"Discharge_start_time_slot_1": start_time, "Discharge_end_time_slot_1": end_time}, - "raw": {"invertor": {"discharge_target_soc_1": "4", "model": "Ac"}}, - } - - # Every model confirmed (or inferred - see the constant's own comment) unsupported must - # attempt no discharge-target REST commands at all. - for model in DISCHARGE_TARGET_UNSUPPORTED_MODELS: - inv.rest_data["raw"]["invertor"]["model"] = model - dummy_rest.clear_queue() - dummy_rest.rest_data = copy.deepcopy(inv.rest_data) - inv.adjust_force_export(True, ts, te) - commands = dummy_rest.get_commands() - if commands: - print("ERROR: {}: model={!r} should attempt no discharge-target REST commands, got {}".format(test_name, model, commands)) - failed = True - - # A model not on the unsupported list (including a later Hybrid generation, or no model - # reported at all) must still attempt the write as before. - for model in ["Hybrid", "Hybrid_gen3", ""]: - inv.rest_data["raw"]["invertor"]["model"] = model - dummy_rest.clear_queue() - dummy_rest.rest_data = copy.deepcopy(inv.rest_data) - inv.adjust_force_export(True, ts, te) - commands = dummy_rest.get_commands() - if not any(c[0] == "dummy/setDischargeTarget" for c in commands): - print("ERROR: {}: model={!r} should still attempt setDischargeTarget, got {}".format(test_name, model, commands)) - failed = True - finally: - inv.rest_data = saved_rest_data - inv.rest_api = saved_rest_api - inv.rest_v3 = saved_rest_v3 - - return failed - - def test_discharge_target_read_prefers_control(test_name, ha, inv): """ Regression test for issue #4517: a discharge target write kept firing every cycle even when @@ -2850,28 +2712,6 @@ def run_inverter_tests(my_predbat_dummy): if failed: return failed - failed |= test_inverter_rest_template( - "rest1", - my_predbat, - filename="cases/rest_v2.json", - assert_soc_max=9.523, - assert_nominal_capacity=9.5232, - assert_battery_temperature=15.3, - ) - if failed: - return failed - failed |= test_inverter_rest_template( - "rest2", - my_predbat, - filename="cases/rest_v3.json", - assert_battery_rate_max=3600, - assert_serial_number="EA2303G082", - assert_nominal_capacity=9.52, - assert_battery_temperature=25.0, - ) - if failed: - return failed - failed |= test_battery_scaling_invalid_value_clamped("battery_scaling_invalid_value_clamped", my_predbat) failed |= test_rest_battery_capacity_fallback("rest_capacity_fallback", my_predbat) @@ -3480,7 +3320,6 @@ def run_inverter_tests(my_predbat_dummy): # Regression test for issue #4517 (follow-up): AC Coupled inverters don't have a working # discharge target register, skip the write entirely rather than retrying it forever - failed |= test_discharge_target_skipped_for_ac_coupled("discharge_target_skipped_for_ac_coupled", ha, inv, dummy_rest) if failed: return failed diff --git a/docs/apps-yaml.md b/docs/apps-yaml.md index fa972816d..3a1eb8c21 100644 --- a/docs/apps-yaml.md +++ b/docs/apps-yaml.md @@ -1349,6 +1349,10 @@ This requires at least several days of historical data with charging periods of - **battery_min_soc** - When set limits the target SoC% setting for charge and discharge to a minimum percentage value - **reserve** - sensor name for the reserve SoC % setting. The reserve SoC is the lower limit target % to discharge the battery down to. - **battery_temperature** - Defined the temperature of the battery in degrees C (default is 20 if not set). +- **battery_calibration** - Optional sensor name reporting whether the battery is currently being calibrated (`on`/`off`). +A calibration cycle deliberately drives the battery outside its normal SoC range, so while one is running any plan would be +wrong and Predbat disables itself for that inverter. Leave unset if your inverter does not report this - an absent sensor +means "never calibrating". Set automatically for GivTCP (REST) users. #### Power Data