Skip to content

Commit c27d579

Browse files
fix(inverter): enable Enable_Charge_Target before writing a GivTCP discharge target (#4517)
adjust_force_export() decided a discharge-target write was needed based on rest_readDischargeTarget() alone, but on some inverters GivTCP reports each write as successful while it never actually sticks - Control.Enable_Charge_Target stays off, so the value is written but the inverter ignores it, and Predbat sees a permanent mismatch and rewrites every cycle indefinitely. adjust_battery_target() already calls rest_enableChargeTarget(True) before writing a charge target for exactly this reason (documented: without it the inverter ignores the target). Mirror that call for the discharge target too. No-ops wherever it's already enabled, which is the common case. Whether every rest_v3 inverter actually has this register at all is unconfirmed - if enabling itself fails, skip the discharge-target write rather than also attempting one already known to be doomed, so a genuinely unsupported inverter ends up retrying one REST endpoint per cycle instead of two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 7ae4318 commit c27d579

2 files changed

Lines changed: 161 additions & 1 deletion

File tree

‎apps/predbat/inverter.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2527,7 +2527,19 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No
25272527
if current is None:
25282528
self.log("Inverter {} No current discharge target to read, export target not written".format(self.id))
25292529
elif current != target_soc:
2530-
self.rest_setDischargeTarget(target_soc)
2530+
# Without Enable_Charge_Target on, the inverter ignores a written discharge
2531+
# target - GivTCP still reports the write as successful, but it never sticks, so
2532+
# this repeats every cycle indefinitely (#4517). adjust_battery_target() already
2533+
# enables this before writing a charge target; mirror that here. No-ops if
2534+
# already enabled.
2535+
# Unconfirmed whether every rest_v3 inverter actually has this register - if
2536+
# enabling itself fails, don't also attempt a discharge-target write we already
2537+
# know is doomed. That keeps a genuinely unsupported inverter retrying one REST
2538+
# endpoint each cycle instead of two.
2539+
if self.rest_enableChargeTarget(True):
2540+
self.rest_setDischargeTarget(target_soc)
2541+
else:
2542+
self.log("Inverter {} Unable to enable charge target, discharge target not written".format(self.id))
25312543
else:
25322544
self.log("Inverter {} Current discharge target is already set to {}".format(self.id, current))
25332545
elif "discharge_target_soc" in self.base.args:

‎apps/predbat/tests/test_inverter.py‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,10 @@ def test_adjust_force_export(test_name, ha, inv, dummy_rest, prev_start, prev_en
344344
inv.rest_data["Control"] = {}
345345
inv.rest_data["Control"]["Enable_Discharge_Schedule"] = prev_force_export
346346
inv.rest_data["Control"]["Mode"] = prev_mode
347+
# Already enabled, matching a normal working inverter - keeps this helper's assertions about the
348+
# discharge target/slot/mode writes focused on those, rather than also exercising the
349+
# enableChargeTarget guard (#4517 follow-up), which has its own dedicated test.
350+
inv.rest_data["Control"]["Enable_Charge_Target"] = "enable"
347351
inv.rest_data["Timeslots"] = {}
348352
inv.rest_data["Timeslots"]["Discharge_start_time_slot_1"] = prev_start
349353
inv.rest_data["Timeslots"]["Discharge_end_time_slot_1"] = prev_end
@@ -1927,6 +1931,138 @@ def test_discharge_target_control_signal(test_name, ha, inv, dummy_rest):
19271931
return failed
19281932

19291933

1934+
def test_discharge_target_enables_before_write(test_name, ha, inv, dummy_rest):
1935+
"""
1936+
Regression test for issue #4517 (follow-up): on some inverters (confirmed against a real
1937+
reporter's REST_Response.txt - AC-coupled, GivTCP reports Control.Enable_Charge_Target as
1938+
"disable" and Control.Discharge_Target_SOC_1 is entirely absent), a written discharge target
1939+
never sticks while Enable_Charge_Target stays off - GivTCP's own log still reports each write as
1940+
a success, but it doesn't take, so adjust_force_export sees a permanent mismatch and rewrites
1941+
every cycle indefinitely.
1942+
1943+
adjust_battery_target() already calls rest_enableChargeTarget(True) before writing a charge
1944+
target for exactly this reason (the register is documented to be ignored otherwise) - this
1945+
mirrors that same call for the discharge target write. Not confirmed to be the true root cause
1946+
on real hardware yet (correlational, from one working and one broken sample), but low-risk: it's
1947+
a no-op wherever Enable_Charge_Target is already on, which is the common case exercised by every
1948+
other discharge-target test in this file.
1949+
"""
1950+
failed = False
1951+
print("Test: {}".format(test_name))
1952+
1953+
saved_rest_data = inv.rest_data
1954+
saved_rest_api = inv.rest_api
1955+
saved_rest_v3 = inv.rest_v3
1956+
1957+
try:
1958+
inv.rest_api = "dummy"
1959+
inv.rest_v3 = True
1960+
inv.reserve_percent = 20
1961+
1962+
start_time = "03:33:00"
1963+
end_time = "04:44:00"
1964+
ts = datetime.strptime(start_time, "%H:%M:%S")
1965+
te = datetime.strptime(end_time, "%H:%M:%S")
1966+
1967+
# Mirrors a reporter's real REST_Response.txt: Enable_Charge_Target off, no
1968+
# Discharge_Target_SOC_1 key in Control at all, raw.invertor reporting a plausible but wrong
1969+
# value (so rest_readDischargeTarget's fallback doesn't return None and skip the write).
1970+
# Timeslots/Enable_Discharge_Schedule/Mode already match the new values so only the discharge
1971+
# target write itself is exercised - the slot/mode writes have their own dedicated tests.
1972+
inv.rest_data = {
1973+
"Control": {"Mode": "Timed Export", "Enable_Discharge_Schedule": "on", "Enable_Charge_Target": "disable"},
1974+
"Timeslots": {"Discharge_start_time_slot_1": start_time, "Discharge_end_time_slot_1": end_time},
1975+
"raw": {"invertor": {"discharge_target_soc_1": "4"}},
1976+
}
1977+
dummy_rest.clear_queue()
1978+
dummy_rest.rest_data = copy.deepcopy(inv.rest_data)
1979+
1980+
# First queued response: consumed by rest_enableChargeTarget's own readback, showing GivTCP
1981+
# accepted the enable.
1982+
enabled = copy.deepcopy(inv.rest_data)
1983+
enabled["Control"]["Enable_Charge_Target"] = "enable"
1984+
dummy_rest.queue_rest_data(enabled)
1985+
1986+
# Second queued response: consumed by rest_setDischargeTarget's readback, now that the
1987+
# register is enabled the write finally sticks.
1988+
settled = copy.deepcopy(enabled)
1989+
settled["Control"]["Discharge_Target_SOC_1"] = "20"
1990+
dummy_rest.queue_rest_data(settled)
1991+
1992+
inv.adjust_force_export(True, ts, te)
1993+
1994+
commands = dummy_rest.get_commands()
1995+
expect_commands = [
1996+
["dummy/enableChargeTarget", {"state": "enable"}],
1997+
["dummy/setDischargeTarget", {"dischargeToPercent": 20, "slot": 1}],
1998+
]
1999+
if json.dumps(commands[:2]) != json.dumps(expect_commands):
2000+
print("ERROR: {}: expected enableChargeTarget before setDischargeTarget, got {}".format(test_name, commands))
2001+
failed = True
2002+
finally:
2003+
inv.rest_data = saved_rest_data
2004+
inv.rest_api = saved_rest_api
2005+
inv.rest_v3 = saved_rest_v3
2006+
2007+
return failed
2008+
2009+
2010+
def test_discharge_target_skips_write_when_enable_fails(test_name, ha, inv, dummy_rest):
2011+
"""
2012+
Regression test for issue #4517 (follow-up): if Enable_Charge_Target can't be turned on at all -
2013+
e.g. a genuinely unsupported register on this inverter, not just left off - don't also attempt a
2014+
discharge-target write we already know is doomed. Without this, a genuinely unsupported inverter
2015+
would retry *two* REST endpoints every cycle (enableChargeTarget then setDischargeTarget) instead
2016+
of the one it retried before this fix, which would make the write-storm this fix exists to
2017+
reduce worse, not better, for exactly the inverters it can't help.
2018+
"""
2019+
failed = False
2020+
print("Test: {}".format(test_name))
2021+
2022+
saved_rest_data = inv.rest_data
2023+
saved_rest_api = inv.rest_api
2024+
saved_rest_v3 = inv.rest_v3
2025+
2026+
try:
2027+
inv.rest_api = "dummy"
2028+
inv.rest_v3 = True
2029+
inv.reserve_percent = 20
2030+
2031+
start_time = "03:33:00"
2032+
end_time = "04:44:00"
2033+
ts = datetime.strptime(start_time, "%H:%M:%S")
2034+
te = datetime.strptime(end_time, "%H:%M:%S")
2035+
2036+
inv.rest_data = {
2037+
"Control": {"Mode": "Timed Export", "Enable_Discharge_Schedule": "on", "Enable_Charge_Target": "disable"},
2038+
"Timeslots": {"Discharge_start_time_slot_1": start_time, "Discharge_end_time_slot_1": end_time},
2039+
"raw": {"invertor": {"discharge_target_soc_1": "4"}},
2040+
}
2041+
dummy_rest.clear_queue()
2042+
dummy_rest.rest_data = copy.deepcopy(inv.rest_data)
2043+
# Enable_Charge_Target never flips to "enable" on any readback - simulates a register the
2044+
# inverter genuinely doesn't support, so rest_enableChargeTarget exhausts its retries and
2045+
# returns False.
2046+
2047+
inv.adjust_force_export(True, ts, te)
2048+
2049+
commands = dummy_rest.get_commands()
2050+
set_discharge_target_commands = [c for c in commands if c[0] == "dummy/setDischargeTarget"]
2051+
if set_discharge_target_commands:
2052+
print("ERROR: {}: setDischargeTarget should not be attempted when enabling fails, got {}".format(test_name, commands))
2053+
failed = True
2054+
enable_charge_target_commands = [c for c in commands if c[0] == "dummy/enableChargeTarget"]
2055+
if not enable_charge_target_commands:
2056+
print("ERROR: {}: expected at least one enableChargeTarget attempt, got {}".format(test_name, commands))
2057+
failed = True
2058+
finally:
2059+
inv.rest_data = saved_rest_data
2060+
inv.rest_api = saved_rest_api
2061+
inv.rest_v3 = saved_rest_v3
2062+
2063+
return failed
2064+
2065+
19302066
def test_discharge_target_read_prefers_control(test_name, ha, inv):
19312067
"""
19322068
Regression test for issue #4517: a discharge target write kept firing every cycle even when
@@ -3173,6 +3309,18 @@ def run_inverter_tests(my_predbat_dummy):
31733309
if failed:
31743310
return failed
31753311

3312+
# Regression test for issue #4517 (follow-up): enable Enable_Charge_Target before writing a
3313+
# discharge target, mirroring adjust_battery_target()'s existing precedent for the charge side
3314+
failed |= test_discharge_target_enables_before_write("discharge_target_enables_before_write", ha, inv, dummy_rest)
3315+
if failed:
3316+
return failed
3317+
3318+
# Regression test for issue #4517 (follow-up): if enabling itself fails, skip the discharge
3319+
# target write rather than retrying two doomed REST endpoints instead of one
3320+
failed |= test_discharge_target_skips_write_when_enable_fails("discharge_target_skips_write_when_enable_fails", ha, inv, dummy_rest)
3321+
if failed:
3322+
return failed
3323+
31763324
# Regression test for issue #4517: the caller deciding whether to write at all must also prefer
31773325
# Control.Discharge_Target_SOC_1 over the slow raw.invertor fallback, or it re-writes every cycle
31783326
failed |= test_discharge_target_read_prefers_control("discharge_target_read_prefers_control", ha, inv)

0 commit comments

Comments
 (0)