Skip to content

Commit e70314f

Browse files
chalfontchubbyclaude
authored andcommitted
fix(inverter): also skip the discharge target write on Hybrid Gen1/Gen2 (#4517)
Extends the AC Coupled exclusion from #4543 - a reporter running the merged fix confirmed live that his genuinely AC Coupled inverter correctly stopped attempting the write, but two separate Hybrid Gen1 inverters on the same fleet kept repeating it every cycle, exactly the original symptom. Per the same GivEnergy firmware archive already used for the AC Coupled case, Gen1 and Gen2 Hybrid are a real, distinct generational split from Gen3 (unlike "AC Coupled", which has no sub-generations at all) - only Gen3 supports Discharge_Target_SOC_1. "Hybrid_gen1" is confirmed live on two separate inverters. "Hybrid_gen2" is inferred from the same firmware-archive split, not yet independently confirmed on real Gen2 hardware - noted as such in the constant's own comment so it's easy to revisit if that turns out wrong. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 4139784 commit e70314f

2 files changed

Lines changed: 35 additions & 27 deletions

File tree

‎apps/predbat/inverter.py‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@
2929

3030
TIME_FORMAT_HMS = "%H:%M:%S"
3131

32+
# GivTCP raw.invertor.model values confirmed not to support the Discharge_Target_SOC_1 register
33+
# (#4517). "Ac" (AC Coupled) and "Hybrid_gen1" are confirmed live - the latter on two separate
34+
# reporter inverters, still repeating the write every cycle post-fix until added here.
35+
# "Hybrid_gen2" is inferred from the same GivEnergy firmware-archive generational split
36+
# (github.com/DJBenson/giv-firmware), not independently confirmed on real Gen2 hardware yet.
37+
DISCHARGE_TARGET_UNSUPPORTED_MODELS = ("Ac", "Hybrid_gen1", "Hybrid_gen2")
38+
3239

3340
class Inverter:
3441
"""Unified inverter control abstraction for multiple brands.
@@ -2527,16 +2534,13 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No
25272534
if force_export:
25282535
target_soc = int(self.reserve_percent)
25292536
if self.rest_data and self.rest_v3:
2530-
# AC Coupled inverters don't have a working Discharge_Target_SOC_1 register - GivTCP
2531-
# still reports a write as successful, but it never persists between cycles, so the
2532-
# caller sees a permanent mismatch and rewrites indefinitely (#4517). Confirmed
2533-
# against GivEnergy's own firmware archive (github.com/DJBenson/giv-firmware): "AC
2534-
# Coupled" is a single product line with exactly two firmware releases ever published
2535-
# (D212-A212, A214-D214) - not an early/late generational split the way Hybrid has
2536-
# Gen1/2/3 - so there's no newer AC-coupled variant this would need to keep working for.
2537+
# Some GivTCP inverter models don't have a working Discharge_Target_SOC_1 register -
2538+
# GivTCP still reports a write as successful, but it never persists between cycles, so
2539+
# the caller sees a permanent mismatch and rewrites indefinitely (#4517). See
2540+
# DISCHARGE_TARGET_UNSUPPORTED_MODELS above for what's confirmed vs inferred.
25372541
inverter_model = self.rest_data.get("raw", {}).get("invertor", {}).get("model", "")
2538-
if inverter_model == "Ac":
2539-
self.log("Inverter {} is AC Coupled, discharge target register not supported, export target not written".format(self.id))
2542+
if inverter_model in DISCHARGE_TARGET_UNSUPPORTED_MODELS:
2543+
self.log("Inverter {} is {}, discharge target register not supported, export target not written".format(self.id, inverter_model))
25402544
else:
25412545
current = self.rest_readDischargeTarget()
25422546
if current is None:

‎apps/predbat/tests/test_inverter.py‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from tests.test_infra import TestHAInterface
1818
from predbat import PredBat
1919
from const import MINUTE_WATT, INVERTER_MAX_RETRY_REST
20-
from inverter import Inverter
20+
from inverter import Inverter, DISCHARGE_TARGET_UNSUPPORTED_MODELS
2121

2222

2323
def dummy_sleep(seconds):
@@ -1929,13 +1929,13 @@ def test_discharge_target_control_signal(test_name, ha, inv, dummy_rest):
19291929

19301930
def test_discharge_target_skipped_for_ac_coupled(test_name, ha, inv, dummy_rest):
19311931
"""
1932-
Regression test for issue #4517: AC Coupled inverters (raw.invertor.model == "Ac") don't have a
1933-
working Discharge_Target_SOC_1 register - GivTCP reports a write as successful, but it never
1934-
persists between cycles, so the caller sees a permanent mismatch and rewrites indefinitely.
1935-
Confirmed against GivEnergy's own firmware archive: "AC Coupled" is a single product line with
1936-
only two firmware releases ever published, not an early/late generational split - so there's no
1937-
newer AC-coupled variant that would need this write to still happen. Skip it outright rather than
1938-
attempting a write already known to be doomed.
1932+
Regression test for issue #4517: some GivTCP inverter models (see
1933+
DISCHARGE_TARGET_UNSUPPORTED_MODELS) don't have a working Discharge_Target_SOC_1 register -
1934+
GivTCP reports a write as successful, but it never persists between cycles, so the caller sees a
1935+
permanent mismatch and rewrites indefinitely. "Ac" (AC Coupled) was confirmed first; "Hybrid_gen1"
1936+
was added after a reporter confirmed live, post-fix, that two of his Gen1 inverters still repeated
1937+
the write every cycle while a third, genuinely AC Coupled, correctly stopped. Skip outright rather
1938+
than attempting a write already known to be doomed.
19391939
"""
19401940
failed = False
19411941
print("Test: {}".format(test_name))
@@ -1959,18 +1959,22 @@ def test_discharge_target_skipped_for_ac_coupled(test_name, ha, inv, dummy_rest)
19591959
"Timeslots": {"Discharge_start_time_slot_1": start_time, "Discharge_end_time_slot_1": end_time},
19601960
"raw": {"invertor": {"discharge_target_soc_1": "4", "model": "Ac"}},
19611961
}
1962-
dummy_rest.clear_queue()
1963-
dummy_rest.rest_data = copy.deepcopy(inv.rest_data)
1964-
1965-
inv.adjust_force_export(True, ts, te)
19661962

1967-
commands = dummy_rest.get_commands()
1968-
if commands:
1969-
print("ERROR: {}: AC Coupled should attempt no discharge-target REST commands at all, got {}".format(test_name, commands))
1970-
failed = True
1963+
# Every model confirmed (or inferred - see the constant's own comment) unsupported must
1964+
# attempt no discharge-target REST commands at all.
1965+
for model in DISCHARGE_TARGET_UNSUPPORTED_MODELS:
1966+
inv.rest_data["raw"]["invertor"]["model"] = model
1967+
dummy_rest.clear_queue()
1968+
dummy_rest.rest_data = copy.deepcopy(inv.rest_data)
1969+
inv.adjust_force_export(True, ts, te)
1970+
commands = dummy_rest.get_commands()
1971+
if commands:
1972+
print("ERROR: {}: model={!r} should attempt no discharge-target REST commands, got {}".format(test_name, model, commands))
1973+
failed = True
19711974

1972-
# A non-AC-Coupled model (or no model reported at all) must still attempt the write as before.
1973-
for model in ["Hybrid", ""]:
1975+
# A model not on the unsupported list (including a later Hybrid generation, or no model
1976+
# reported at all) must still attempt the write as before.
1977+
for model in ["Hybrid", "Hybrid_gen3", ""]:
19741978
inv.rest_data["raw"]["invertor"]["model"] = model
19751979
dummy_rest.clear_queue()
19761980
dummy_rest.rest_data = copy.deepcopy(inv.rest_data)

0 commit comments

Comments
 (0)