diff --git a/apps/predbat/gecloud.py b/apps/predbat/gecloud.py index 15d422ce3..d19294045 100644 --- a/apps/predbat/gecloud.py +++ b/apps/predbat/gecloud.py @@ -606,6 +606,12 @@ async def publish_meter(self, device, meter): async def enable_default_options(self, device, registers): """Enable default options for the device.""" changed = False + # When both a direct power register and its percentage equivalent exist, the power register + # is used as the rate control, so the percentage register must be left at 100% to avoid it + # clamping the power setting. + device_ha_names = {regname_to_ha(registers[key].get("name", "")) for key in registers} + has_charge_power = "battery_charge_power" in device_ha_names + has_discharge_power = "battery_discharge_power" in device_ha_names for key in registers: reg_name = registers[key].get("name", "") value = registers[key].get("value", None) @@ -654,6 +660,29 @@ async def enable_default_options(self, device, registers): changed = True else: self.log("GECloud: Warn: Failed to set {} for {}".format(ha_name, device)) + # Reset the charge/discharge power percentage to 100% when a direct power register is + # present, so that the power register (not the percentage) controls the rate. Match + # discharge first as "discharge_power_rate" also contains the "charge_power_rate" substring. + if ("inverter_discharge_power_percentage" in ha_name) or ("discharge_power_rate" in ha_name): + if has_discharge_power and (not value or value < 100): + self.log("GECloud: Setting {} to 100% for {} as direct power register present, previous value was {}".format(ha_name, device, value)) + result = await self.async_write_inverter_setting(device, key, 100) + if result and ("value" in result): + registers[key]["value"] = result["value"] + await self.publish_registers(device, self.settings[device], select_key=key) + changed = True + else: + self.log("GECloud: Warn: Failed to set {} for {}".format(ha_name, device)) + elif ("inverter_charge_power_percentage" in ha_name) or ("charge_power_rate" in ha_name): + if has_charge_power and (not value or value < 100): + self.log("GECloud: Setting {} to 100% for {} as direct power register present, previous value was {}".format(ha_name, device, value)) + result = await self.async_write_inverter_setting(device, key, 100) + if result and ("value" in result): + registers[key]["value"] = result["value"] + await self.publish_registers(device, self.settings[device], select_key=key) + changed = True + else: + self.log("GECloud: Warn: Failed to set {} for {}".format(ha_name, device)) # Reset AC charge start and end times to 00:00 to disable for charge_id in range(2, 11): if ( @@ -801,6 +830,8 @@ async def async_automatic_config(self, devices): batteries = [devices["gateway"]] # Do we have a charge/discharge power percentage setting? + has_charge_rate = False + has_discharge_rate = False has_charge_power_percent = False has_discharge_power_percent = False has_pause_start_time = False @@ -811,6 +842,10 @@ async def async_automatic_config(self, devices): for key in registers: reg_name = registers[key].get("name", "") ha_name = regname_to_ha(reg_name) + if "battery_charge_power" in ha_name: + has_charge_rate = True + if "battery_discharge_power" in ha_name: + has_discharge_rate = True if "inverter_charge_power_percentage" in ha_name or "charge_power_rate" in ha_name: has_charge_power_percent = True if "inverter_discharge_power_percentage" in ha_name or "discharge_power_rate" in ha_name: @@ -848,7 +883,11 @@ def build_entities(domain, candidates): return entities self.log("GECloud: Auto-configuring Predbat and not using apps.yaml entries for control") - self.log("GECloud: detected features - charge power percent: {}, pause battery: {}, pause start time: {}, discharge target soc: {}".format(has_charge_power_percent, has_pause_battery, has_pause_start_time, has_discharge_target_soc)) + self.log( + "GECloud: detected features - charge rate: {}, discharge rate: {}, charge power percent: {}, discharge power percent: {}, pause battery: {}, pause start time: {}, discharge target soc: {}".format( + has_charge_rate, has_discharge_rate, has_charge_power_percent, has_discharge_power_percent, has_pause_battery, has_pause_start_time, has_discharge_target_soc + ) + ) self.set_arg("inverter_type", ["GEC" for _ in range(num_inverters)]) self.set_arg("num_inverters", num_inverters) @@ -900,11 +939,16 @@ def build_entities(domain, candidates): else: self.set_arg("discharge_target_soc", None) - if has_charge_power_percent or has_discharge_power_percent: + # The percentage rate registers only win as the control when the direct power register is + # absent. When both a power register and its percentage equivalent exist, the power register + # is the primary control and the percentage is left at 100% (reset by enable_default_options). + if has_charge_power_percent and not has_charge_rate: self.set_arg("charge_rate_percent", build_entities("number", ["inverter_charge_power_percentage", "charge_power_rate"])) - self.set_arg("discharge_rate_percent", build_entities("number", ["inverter_discharge_power_percentage", "discharge_power_rate"])) else: self.set_arg("charge_rate_percent", None) + if has_discharge_power_percent and not has_discharge_rate: + self.set_arg("discharge_rate_percent", build_entities("number", ["inverter_discharge_power_percentage", "discharge_power_rate"])) + else: self.set_arg("discharge_rate_percent", None) self.set_arg("givtcp_rest", None) diff --git a/apps/predbat/predbat.py b/apps/predbat/predbat.py index d79223d07..6dffad97a 100644 --- a/apps/predbat/predbat.py +++ b/apps/predbat/predbat.py @@ -35,7 +35,7 @@ import pytz import asyncio -THIS_VERSION = "v8.44.7" +THIS_VERSION = "v8.44.8" from download import predbat_update_move, predbat_update_download, check_install, DEFAULT_PREDBAT_REPOSITORY from const import MINUTE_WATT diff --git a/apps/predbat/tests/test_ge_cloud.py b/apps/predbat/tests/test_ge_cloud.py index 9f5010cb9..277b9b012 100644 --- a/apps/predbat/tests/test_ge_cloud.py +++ b/apps/predbat/tests/test_ge_cloud.py @@ -3015,8 +3015,10 @@ async def test(): assert ge.config_args.get("pause_start_time") == ["select.predbat_gecloud_battery001_pause_battery_start_time"], "pause_start_time should be set" assert ge.config_args.get("pause_end_time") == ["select.predbat_gecloud_battery001_pause_battery_end_time"], "pause_end_time should be set" assert ge.config_args.get("discharge_target_soc") == ["number.predbat_gecloud_battery001_dc_discharge_1_lower_soc_percent_limit"], "discharge_target_soc should be set" - assert ge.config_args.get("charge_rate_percent") == ["number.predbat_gecloud_battery001_inverter_charge_power_percentage"], "charge_rate_percent should be set" - assert ge.config_args.get("discharge_rate_percent") == ["number.predbat_gecloud_battery001_inverter_discharge_power_percentage"], "discharge_rate_percent should be set" + # When both the direct power register and the percentage register exist, the power register + # is the control and the percentage must not be configured (it would clamp the power setting). + assert ge.config_args.get("charge_rate_percent") is None, "charge_rate_percent should be None when battery_charge_power is present" + assert ge.config_args.get("discharge_rate_percent") is None, "discharge_rate_percent should be None when battery_discharge_power is present" # Test 2: Battery without optional features ge.config_args = {} @@ -3684,6 +3686,95 @@ async def mock_write_fail(device, key, value): print("ERROR: Expected value=4 for discharge down to percent, got {}".format(write_calls[0]["value"])) return 1 + # Test 23: Charge power percentage reset to 100 when the direct power register is present + write_calls = [] + registers = { + 300: {"name": "Battery_Charge_Power", "value": 3000, "validation_rules": []}, + 301: {"name": "Inverter_Charge_Power_Percentage", "value": 50, "validation_rules": []}, + } + + result = await ge_cloud.enable_default_options("test123", registers) + + if not result: + print("ERROR: enable_default_options should return True when resetting charge power percentage") + return 1 + if len(write_calls) != 1: + print("ERROR: Expected 1 write call for charge power percentage, got {}".format(len(write_calls))) + return 1 + if write_calls[0]["key"] != 301 or write_calls[0]["value"] != 100: + print("ERROR: Expected charge power percentage (301) reset to 100, got {}".format(write_calls[0])) + return 1 + + # Test 24: Charge power percentage NOT touched when there is no direct power register + write_calls = [] + registers = {301: {"name": "Inverter_Charge_Power_Percentage", "value": 50, "validation_rules": []}} + + result = await ge_cloud.enable_default_options("test123", registers) + + if result: + print("ERROR: enable_default_options should not change charge percentage when power register absent") + return 1 + if len(write_calls) != 0: + print("ERROR: Expected 0 write calls when power register absent, got {}".format(len(write_calls))) + return 1 + + # Test 25: Discharge power percentage reset to 100 when the direct power register is present + write_calls = [] + registers = { + 302: {"name": "Battery_Discharge_Power", "value": 3000, "validation_rules": []}, + 303: {"name": "Inverter_Discharge_Power_Percentage", "value": 80, "validation_rules": []}, + } + + result = await ge_cloud.enable_default_options("test123", registers) + + if not result: + print("ERROR: enable_default_options should return True when resetting discharge power percentage") + return 1 + if len(write_calls) != 1 or write_calls[0]["key"] != 303 or write_calls[0]["value"] != 100: + print("ERROR: Expected discharge power percentage (303) reset to 100, got {}".format(write_calls)) + return 1 + + # Test 26: Alternative three-phase register names (charge_power_rate) are also reset + write_calls = [] + registers = { + 304: {"name": "Battery_Charge_Power", "value": 3000, "validation_rules": []}, + 305: {"name": "Charge_Power_Rate", "value": 40, "validation_rules": []}, + } + + result = await ge_cloud.enable_default_options("test123", registers) + + if len(write_calls) != 1 or write_calls[0]["key"] != 305 or write_calls[0]["value"] != 100: + print("ERROR: Expected charge_power_rate (305) reset to 100, got {}".format(write_calls)) + return 1 + + # Test 27: discharge_power_rate must be treated as discharge only (it contains the + # "charge_power_rate" substring) — with only the discharge power register present it should + # be reset, and no spurious charge reset should occur. + write_calls = [] + registers = { + 306: {"name": "Battery_Discharge_Power", "value": 3000, "validation_rules": []}, + 307: {"name": "Discharge_Power_Rate", "value": 40, "validation_rules": []}, + } + + result = await ge_cloud.enable_default_options("test123", registers) + + if len(write_calls) != 1 or write_calls[0]["key"] != 307 or write_calls[0]["value"] != 100: + print("ERROR: Expected discharge_power_rate (307) reset to 100 only, got {}".format(write_calls)) + return 1 + + # Test 28: Percentage already at 100 — no write needed even when power register present + write_calls = [] + registers = { + 308: {"name": "Battery_Charge_Power", "value": 3000, "validation_rules": []}, + 309: {"name": "Inverter_Charge_Power_Percentage", "value": 100, "validation_rules": []}, + } + + result = await ge_cloud.enable_default_options("test123", registers) + + if len(write_calls) != 0: + print("ERROR: Expected 0 write calls when percentage already 100, got {}".format(len(write_calls))) + return 1 + return 0 return run_async(test())