fix(inverter): also prefer Control.Discharge_Target_SOC_1 when deciding whether to write

adjust_force_export() still decided whether a discharge target write was needed by
reading raw.invertor.discharge_target_soc_1 alone - the same slow, self_run-poll-refreshed
field #4492 moved away from as the primary signal inside rest_setDischargeTarget() itself.
On hardware where that field never catches up, the write looked permanently "needed" and
fired every cycle even when nothing had changed (#4517).

rest_readDischargeTarget() centralises the same Control-first/raw-fallback read used for
write verification, so the caller now correctly skips the write once it's actually landed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: chalfontchubby

apps/predbat/inverter.py

@@ -2511,19 +2511,13 @@ def adjust_force_export(self, force_export, new_start_time=None, new_end_time=No
         if force_export:
             target_soc = int(self.reserve_percent)
             if self.rest_data and self.rest_v3:
-                if "raw" in self.rest_data and "invertor" in self.rest_data["raw"] and "discharge_target_soc_1" in self.rest_data["raw"]["invertor"]:
-                    current = self.rest_data["raw"]["invertor"]["discharge_target_soc_1"]
-                    try:
-                        current = float(current)
-                    except (ValueError, TypeError) as e:
-                        current = None
-
-                    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.rest_setDischargeTarget(target_soc)
-                    else:
-                        self.log("Inverter {} Current discharge target is already set to {}".format(self.id, current))
+                current = self.rest_readDischargeTarget()
+                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.rest_setDischargeTarget(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:
                 current = self.base.get_arg("discharge_target_soc", index=self.id, required_unit="%")
                 try:
@@ -3399,6 +3393,30 @@ def rest_setChargeSlot1(self, start, finish):
         self.base.record_status("Warn: Inverter {} REST failed to setChargeSlot1".format(self.id), had_errors=True)
         return False
 
+    def rest_readDischargeTarget(self):
+        """
+        Read GivTCP's currently applied discharge target percent, or None if it can't be read.
+
+        Mirrors rest_setDischargeTarget()'s own preference order: Control.Discharge_Target_SOC_1 is
+        GivTCP's synchronous write-time signal, updated the moment a write is accepted, so it's
+        checked first. raw.invertor.discharge_target_soc_1 is a fallback for GivTCP setups where
+        Control doesn't expose the key - but on its own it's unreliable as a "did this actually
+        change" signal, since it only refreshes on GivTCP's separate background self_run poll cycle,
+        not synchronously with any write. A caller that reads raw.invertor alone to decide whether a
+        write is even needed can end up re-writing every cycle on hardware where that field never
+        catches up (#4421, #4517).
+        """
+        try:
+            result = int(float(self.rest_data.get("Control", {}).get("Discharge_Target_SOC_1", None)))
+        except (ValueError, TypeError):
+            result = None
+        if result is None:
+            try:
+                result = int(float(self.rest_data.get("raw", {}).get("invertor", {}).get("discharge_target_soc_1", None)))
+            except (ValueError, TypeError):
+                result = None
+        return result
+
     def rest_setDischargeTarget(self, target):
         """
         Configure discharge to percent via REST

apps/predbat/tests/test_inverter.py

@@ -1882,6 +1882,51 @@ def test_discharge_target_control_signal(test_name, ha, inv, dummy_rest):
     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
+    unchanged, because the caller that decides whether to write at all (adjust_force_export) read
+    only raw.invertor.discharge_target_soc_1 - the same slow, self_run-poll-refreshed field #4492
+    moved away from as the primary signal inside rest_setDischargeTarget() itself. On hardware where
+    that field never catches up, the caller saw a permanent mismatch and re-wrote on every cycle.
+
+    rest_readDischargeTarget() is the fix: a shared helper both the caller and (potentially)
+    rest_setDischargeTarget() can use, checking Control.Discharge_Target_SOC_1 (GivTCP's synchronous
+    write-time signal) first and falling back to raw.invertor only if Control doesn't have it.
+    """
+    failed = False
+    print("Test: {}".format(test_name))
+
+    saved_rest_data = inv.rest_data
+
+    try:
+        # Control has the real, current value - stale raw must not override it (the core of #4517:
+        # the old caller ignored Control entirely and would have seen "0", not "20", here).
+        inv.rest_data = {"Control": {"Discharge_Target_SOC_1": "20"}, "raw": {"invertor": {"discharge_target_soc_1": "0"}}}
+        result = inv.rest_readDischargeTarget()
+        if result != 20:
+            print("ERROR: {}: expected Control's value 20, got {}".format(test_name, result))
+            failed = True
+
+        # Control missing the key entirely - falls back to raw.
+        inv.rest_data = {"Control": {}, "raw": {"invertor": {"discharge_target_soc_1": "15"}}}
+        result = inv.rest_readDischargeTarget()
+        if result != 15:
+            print("ERROR: {}: expected raw fallback value 15, got {}".format(test_name, result))
+            failed = True
+
+        # Neither present - no crash, just None (matches "No current discharge target to read" path).
+        inv.rest_data = {"Control": {}, "raw": {"invertor": {}}}
+        result = inv.rest_readDischargeTarget()
+        if result is not None:
+            print("ERROR: {}: expected None when neither field is present, got {}".format(test_name, result))
+            failed = True
+    finally:
+        inv.rest_data = saved_rest_data
+
+    return failed
+
+
 def test_force_export_unchanged_times_HM_format(test_name, ha, inv):
     """
     Regression test for GS_fb00 (Solis) 'count register writes 0' bug.
@@ -3074,5 +3119,11 @@ def run_inverter_tests(my_predbat_dummy):
     if failed:
         return failed
 
+    # Regression test for issue #4517: the caller deciding whether to write at all must also prefer
+    # Control.Discharge_Target_SOC_1 over the slow raw.invertor fallback, or it re-writes every cycle
+    failed |= test_discharge_target_read_prefers_control("discharge_target_read_prefers_control", ha, inv)
+    if failed:
+        return failed
+
     failed |= test_inverter_self_test("self_test1", my_predbat)
     return failed