Skip to content

Commit 68ad0c8

Browse files
CIclaude
andcommitted
fix(inverter): read and write the rate per inverter, so a mixed GE Cloud fleet sees the 3-phase unit (#5359)
Review of the Freeze Export change found that in a GE Cloud fleet mixing a unit with a power register and a 3-phase one, the 3-phase inverter was bound to no rate entity at all: auto-config withheld the percentage rate for the whole fleet when any device had a power register, so its grid_charge_only mark was never read and Freeze Export stayed on. GE Cloud auto-config now binds the percentage rate per device, for a device with no power register only. The inverter asks its own entry which control it is bound to rather than whether the setting exists for the fleet, which is the read and write fault in #5359: a power-register inverter no longer reads a silent 100% from an empty percentage entry, and each inverter is written through the control it has. The docs now say the AC-coupled 3-phase model is treated the same way as a precaution, not as a confirmed hardware limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 256a1e8 commit 68ad0c8

6 files changed

Lines changed: 120 additions & 25 deletions

File tree

‎apps/predbat/gecloud.py‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,14 +1696,17 @@ def build_entities(domain, candidates):
16961696
# The percentage rate registers only win as the control when the direct power register is
16971697
# absent. When both a power register and its percentage equivalent exist, the power register
16981698
# is the primary control and the percentage is left at 100% (reset by enable_default_options).
1699-
if has_charge_power_percent and not has_charge_rate:
1700-
self.set_arg("charge_rate_percent", build_entities("number", ["inverter_charge_power_percentage", "charge_power_rate"]))
1701-
else:
1702-
self.set_arg("charge_rate_percent", None)
1703-
if has_discharge_power_percent and not has_discharge_rate:
1704-
self.set_arg("discharge_rate_percent", build_entities("number", ["inverter_discharge_power_percentage", "discharge_power_rate"]))
1705-
else:
1706-
self.set_arg("discharge_rate_percent", None)
1699+
# Decided for each device on its own registers: a fleet can mix a unit with a power register
1700+
# and a 3-phase one with only the percentage, and each then gets the control it has (#5359).
1701+
def build_percent_entities(power_register, candidates):
1702+
"""Per-device percentage rate entity IDs for the devices with no power register, or None if none qualify."""
1703+
entities = [None if power_register in register_names_by_device.get(device, set()) else first_existing_entity("number", device, candidates) for device in batteries]
1704+
if all(entity is None for entity in entities):
1705+
return None
1706+
return entities
1707+
1708+
self.set_arg("charge_rate_percent", build_percent_entities("battery_charge_power", ["inverter_charge_power_percentage", "charge_power_rate"]))
1709+
self.set_arg("discharge_rate_percent", build_percent_entities("battery_discharge_power", ["inverter_discharge_power_percentage", "discharge_power_rate"]))
17071710

17081711
self.set_arg("givtcp_rest", None)
17091712

@@ -1800,9 +1803,10 @@ def _register_features(self, device):
18001803
The register features async_automatic_config() sniffs for, as this one device reports them.
18011804
18021805
The same substrings as its sniffing loop, but scoped to this device's own settings rather
1803-
than ORed across the fleet: automatic_config() gates pause_mode, pause_start_time/end_time,
1804-
discharge_target_soc and the percentage rate controls for every device on what ANY device
1805-
reports, while a discovery record describes only its own device (spec D10).
1806+
than ORed across the fleet: automatic_config() gates pause_mode, pause_start_time/end_time
1807+
and discharge_target_soc for every device on what ANY device reports, while a discovery
1808+
record describes only its own device (spec D10). The percentage rate controls are bound
1809+
per device by both (#5359).
18061810
"""
18071811
features = {"charge_rate": False, "discharge_rate": False, "charge_power_percent": False, "discharge_power_percent": False, "pause_start_time": False, "discharge_target_soc": False, "pause_battery": False}
18081812
for register in self.settings.get(device, {}).values():

‎apps/predbat/inverter.py‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2120,12 +2120,30 @@ def adjust_reserve(self, reserve):
21202120
else:
21212121
self.base.log("Inverter {} Current reserve is {}%, already at target".format(self.id, dp0(current_reserve)))
21222122

2123+
def rate_controls(self, rate_arg):
2124+
"""
2125+
Which of a rate's two controls this inverter uses: (watts, percent) for "charge_rate" or "discharge_rate".
2126+
2127+
Asked of this inverter's own entry rather than of whether the setting exists at all: a fleet can mix
2128+
inverters with a power register and ones with only the percentage (GivEnergy 3-phase), leaving each
2129+
setting bound for some inverters and empty for the rest (#5359). An inverter bound to neither keeps
2130+
the answer the setting's presence used to give, so a missing entity is still reported when written.
2131+
"""
2132+
percent_arg = rate_arg + "_percent"
2133+
has_watts = rate_arg in self.base.args
2134+
has_percent = percent_arg in self.base.args
2135+
bound_watts = bool(has_watts and self.base.get_arg(rate_arg, indirect=False, index=self.id))
2136+
bound_percent = bool(has_percent and self.base.get_arg(percent_arg, indirect=False, index=self.id))
2137+
if bound_watts != bound_percent:
2138+
return bound_watts, bound_percent
2139+
return has_watts, has_percent
2140+
21232141
def get_current_discharge_rate(self):
21242142
"""
21252143
Get the current discharge rate in watts
21262144
"""
21272145

2128-
if "discharge_rate_percent" in self.base.args:
2146+
if self.rate_controls("discharge_rate")[1]:
21292147
current_rate = int(self.base.get_arg("discharge_rate_percent", index=self.id, default=100.0, required_unit="%") * self.battery_rate_max_raw / 100)
21302148
else:
21312149
current_rate = self.base.get_arg("discharge_rate", index=self.id, default=self.battery_rate_max_raw, required_unit="W")
@@ -2142,7 +2160,7 @@ def get_current_charge_rate(self):
21422160
"""
21432161
Get the current charge rate in watts
21442162
"""
2145-
if "charge_rate_percent" in self.base.args:
2163+
if self.rate_controls("charge_rate")[1]:
21462164
current_rate = self.base.get_arg("charge_rate_percent", index=self.id, default=100.0, required_unit="%") * self.battery_rate_max_raw / 100
21472165
else:
21482166
current_rate = self.base.get_arg("charge_rate", index=self.id, default=self.battery_rate_max_raw, required_unit="W")
@@ -2205,7 +2223,8 @@ def adjust_charge_rate(self, new_rate, notify=True):
22052223
fuzzy, fuzzy_below = self.rate_tolerances(self.battery_rate_max_charge)
22062224
if not within_fuzzy(current_rate, new_rate, fuzzy, fuzzy_below):
22072225
self.base.log("Inverter {} current charge rate is {}W and new target is {}W".format(self.id, current_rate, new_rate))
2208-
if "charge_rate" in self.base.args:
2226+
write_watts, write_percent = self.rate_controls("charge_rate")
2227+
if write_watts:
22092228
self.write_and_poll_value(
22102229
"charge_rate",
22112230
self.base.get_arg("charge_rate", indirect=False, index=self.id, required_unit="W"),
@@ -2214,7 +2233,7 @@ def adjust_charge_rate(self, new_rate, notify=True):
22142233
required_unit="W",
22152234
fuzzy_below=fuzzy_below,
22162235
)
2217-
if "charge_rate_percent" in self.base.args:
2236+
if write_percent:
22182237
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="%")
22192238

22202239
if notify and self.base.set_inverter_notify:
@@ -2254,7 +2273,8 @@ def adjust_discharge_rate(self, new_rate, notify=True):
22542273
fuzzy, fuzzy_below = self.rate_tolerances(self.battery_rate_max_discharge)
22552274
if not within_fuzzy(current_rate, new_rate, fuzzy, fuzzy_below):
22562275
self.base.log("Inverter {} current discharge rate is {}W and new target is {}W".format(self.id, current_rate, new_rate))
2257-
if "discharge_rate" in self.base.args:
2276+
write_watts, write_percent = self.rate_controls("discharge_rate")
2277+
if write_watts:
22582278
self.write_and_poll_value(
22592279
"discharge_rate",
22602280
self.base.get_arg("discharge_rate", indirect=False, index=self.id),
@@ -2263,7 +2283,7 @@ def adjust_discharge_rate(self, new_rate, notify=True):
22632283
required_unit="W",
22642284
fuzzy_below=fuzzy_below,
22652285
)
2266-
if "discharge_rate_percent" in self.base.args:
2286+
if write_percent:
22672287
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="%")
22682288

22692289
if notify and self.base.set_inverter_notify:

‎apps/predbat/tests/test_ge_cloud.py‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6095,8 +6095,9 @@ def _test_discovery_record_two_devices(my_predbat):
60956095
"""
60966096
Two inverters give two records whose entity ids differ by device, each describing its own device
60976097
(spec D10). automatic_config()'s "any device has it" gate binds the pause and discharge-target
6098-
entities on the second device although it has no such registers, and withholds its percentage
6099-
rate controls because the first device has a power register; the record does neither.
6098+
entities on the second device although it has no such registers; the record does not. The
6099+
percentage rate controls are bound per device by both: the second device has no power register,
6100+
so it gets its percentage rate while the first keeps its power register (#5359).
61006101
"""
61016102
devices = {"ems": None, "gateway": None, "battery": ["battery001", "battery002"], "pv": [], "battery_meters": {"battery001": [1001], "battery002": [1002]}}
61026103
settings = {
@@ -6115,9 +6116,11 @@ def _test_discovery_record_two_devices(my_predbat):
61156116
assert not (_entity_ids(records[0]) & _entity_ids(records[1])), "the two records must not share an entity"
61166117
captured = capture_automatic_config(ge)
61176118
_agree_at(records[0], captured, 0)
6118-
_agree_at(records[1], captured, 1, missing=("pause_mode", "pause_start_time", "pause_end_time", "discharge_target_soc"), allowed_extra=("charge_rate_percent", "discharge_rate_percent"))
6119+
_agree_at(records[1], captured, 1, missing=("pause_mode", "pause_start_time", "pause_end_time", "discharge_target_soc"))
61196120
assert captured["pause_mode"][1] == "select.predbat_gecloud_battery002_pause_battery", "automatic_config's fleet gate binds pause on the second device"
6120-
assert captured["charge_rate_percent"] is None, "automatic_config's fleet gate withholds the percentage rates"
6121+
assert captured["charge_rate_percent"] == [None, "number.predbat_gecloud_battery002_inverter_charge_power_percentage"], "the percentage charge rate is bound for the device with no power register only"
6122+
assert captured["discharge_rate_percent"] == [None, "number.predbat_gecloud_battery002_inverter_discharge_power_percentage"], "the percentage discharge rate likewise"
6123+
assert captured["charge_rate"][1] is None and captured["charge_rate"][0] is not None, "the power register is bound for the device that has it only"
61216124
second = records[1]["entities"]
61226125
assert second["reserve"]["entity_id"] == "number.predbat_gecloud_battery002_battery_reserve_percent", "the per-device candidate choice"
61236126
assert second["scheduled_charge_enable"]["entity_id"] == "switch.predbat_gecloud_battery002_enable_ac_charge"

‎apps/predbat/tests/test_inverter.py‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,71 @@ def test_custom_inverter_def_drops_rate_step(test_name, my_predbat, ha):
11091109
return failed
11101110

11111111

1112+
def test_rate_control_is_per_inverter(test_name, my_predbat, ha):
1113+
"""
1114+
An inverter reads and writes its rate through the control its own entry is bound to, whichever settings exist
1115+
for the fleet: a GE Cloud fleet mixing a unit with a power register and a 3-phase one with only the percentage
1116+
binds each setting for one inverter and leaves it empty for the other (#5359). It also means the 3-phase unit's
1117+
percentage entity is bound, so its grid_charge_only mark is seen in a mixed fleet (#5082).
1118+
1119+
Building an Inverter writes args and publishes entities on the shared fixture, so both are restored whole (#5079).
1120+
"""
1121+
failed = False
1122+
print("Test: {}".format(test_name))
1123+
saved_args = copy.deepcopy(my_predbat.args)
1124+
saved_items = copy.deepcopy(ha.dummy_items)
1125+
try:
1126+
my_predbat.args["inverter_type"] = ["GEC"]
1127+
# this inverter's entry in each setting, the rate it should read, the entities a new rate is written to
1128+
for watts_entity, percent_entity, expect_read, expect_writes in (
1129+
("number.rate_watts", None, 1500, ["number.rate_watts"]),
1130+
(None, "number.rate_percent", 1300, ["number.rate_percent"]),
1131+
("number.rate_watts", "number.rate_percent", 1300, ["number.rate_watts", "number.rate_percent"]),
1132+
):
1133+
for rate_arg, read_rate, adjust_rate in (("charge_rate", "get_current_charge_rate", "adjust_charge_rate"), ("discharge_rate", "get_current_discharge_rate", "adjust_discharge_rate")):
1134+
for name in ("charge_rate", "charge_rate_percent", "discharge_rate", "discharge_rate_percent"):
1135+
my_predbat.args.pop(name, None)
1136+
# Both settings exist for the fleet; only this inverter's entries differ
1137+
my_predbat.args[rate_arg] = [watts_entity]
1138+
my_predbat.args[rate_arg + "_percent"] = [percent_entity]
1139+
ha.dummy_items["number.rate_watts"] = 1500
1140+
ha.dummy_items["number.rate_percent"] = {"state": 50, "unit_of_measurement": "%", "grid_charge_only": True}
1141+
inv = Inverter(my_predbat, 0, quiet=True)
1142+
inv.battery_rate_max_raw = 2600
1143+
inv.battery_rate_max_charge = inv.battery_rate_max_discharge = 2600 / MINUTE_WATT
1144+
got = getattr(inv, read_rate)()
1145+
if got != expect_read:
1146+
print("ERROR: {} {} watts {} percent {} expected to read {} got {}".format(test_name, rate_arg, watts_entity, percent_entity, expect_read, got))
1147+
failed = True
1148+
written = []
1149+
call_service = ha.call_service
1150+
1151+
def recording_call_service(service, **kwargs):
1152+
"""Record which entity a rate write went to."""
1153+
if service == "number/set_value":
1154+
written.append(kwargs.get("entity_id"))
1155+
return call_service(service, **kwargs)
1156+
1157+
ha.call_service = recording_call_service
1158+
try:
1159+
getattr(inv, adjust_rate)(0)
1160+
finally:
1161+
ha.call_service = call_service
1162+
if sorted(set(written)) != sorted(expect_writes):
1163+
print("ERROR: {} {} watts {} percent {} expected writes to {} got {}".format(test_name, rate_arg, watts_entity, percent_entity, expect_writes, written))
1164+
failed = True
1165+
# The mark on the percentage charge rate is seen whenever this inverter is bound to it
1166+
if rate_arg == "charge_rate" and inv.inv_support_discharge_freeze is not (percent_entity is None):
1167+
print("ERROR: {} watts {} percent {} Freeze Export support is {}".format(test_name, watts_entity, percent_entity, inv.inv_support_discharge_freeze))
1168+
failed = True
1169+
finally:
1170+
my_predbat.args.clear()
1171+
my_predbat.args.update(saved_args)
1172+
ha.dummy_items.clear()
1173+
ha.dummy_items.update(saved_items)
1174+
return failed
1175+
1176+
11121177
def test_grid_charge_only_disables_discharge_freeze(test_name, my_predbat, ha):
11131178
"""
11141179
A charge rate entity marked grid_charge_only turns Freeze Export support off for that inverter: the GivEnergy
@@ -5538,6 +5603,7 @@ def run_inverter_tests(my_predbat_dummy):
55385603
failed |= test_custom_inverter_def_drops_rate_step("custom_inverter_def_drops_rate_step", my_predbat, ha)
55395604
failed |= test_rate_step_from_charge_rate_entity("rate_step_from_charge_rate_entity", my_predbat, ha)
55405605
failed |= test_grid_charge_only_disables_discharge_freeze("grid_charge_only_disables_discharge_freeze", my_predbat, ha)
5606+
failed |= test_rate_control_is_per_inverter("rate_control_is_per_inverter", my_predbat, ha)
55415607
failed |= test_rate_write_quantised("rate_write_quantised_charge", ha, inv, 2600, 1300, 1, 1206)
55425608
failed |= test_rate_write_quantised("rate_write_quantised_charge_settled", ha, inv, 1206, 1300, 0, 1206)
55435609
failed |= test_rate_write_quantised("rate_write_quantised_charge_next_step", ha, inv, 1206, 1350, 1, 1341)

‎docs/inverter-setup.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,11 @@ This is being worked on by the author of GivTCP, e.g. see [GivTCP issue: unable
173173
- Set geserial to your inverter serial number
174174
- If you set **ge_cloud_automatic** to `true` in `apps.yaml` then Predbat will auto-configure itself to use the appropriate GE Cloud controls and will ignore any inverter and battery controls set in `apps.yaml`
175175
- Make sure that the 'discharge down to' registers are set to 4% and charge and discharge slots 2, 3 and 4 are disabled in the portal by setting the start and end times to 00:00 (if you have them).
176-
- Freeze Export is not available on GivEnergy 3-phase inverters, whether controlled through GE Cloud or through the Predbat hub.
177-
These inverters have no battery pause setting and their charge rate only limits charging from the grid, so solar would carry on charging the battery during a Freeze Export slot.
178-
Predbat detects this, logs that Freeze Export is not supported and stops planning it; forced export is not affected.
176+
- Predbat does not use Freeze Export on GivEnergy 3-phase inverters, whether controlled through GE Cloud or through the Predbat hub.
177+
On the 3-phase hybrid there is no battery pause setting and the charge rate only limits charging from the grid, so solar carries on charging the battery during a Freeze Export slot.
178+
The AC-coupled 3-phase model is treated the same way as a precaution; it has not been checked and may in fact be able to hold.
179+
Predbat detects these inverters, logs that Freeze Export is not supported and stops planning it; forced export is not affected.
180+
If any inverter in a multi-inverter system does not support Freeze Export then it is turned off for all of them.
179181
- If you have set **ge_cloud_automatic** to `true` and the GE Cloud does not return accurate **load_today** energy information, you can [override the GE Cloud load data](apps-yaml.md) by creating a custom template sensor and setting **ge_cloud_load_today_ignore** to true in `apps.yaml`.
180182
- The charge/export slot start and end times are stored by GivEnergy in the timezone set on your GivEnergy account. Predbat reads this timezone from the GivEnergy account details and translates the slot times into
181183
the Predbat **timezone** setting, so the times shown on the Predbat select entities may differ from those shown in the GivEnergy portal if the two timezones do not match.

0 commit comments

Comments
 (0)