fix(inverter): hard-raise when no charge window source is configured at all

Splits what update_status() previously collapsed into one "safe defaults,
retry next update" path, per maintainer review on #4288:

- REST configured (givtcp_rest) but rest_data hasn't returned anything yet
  this cycle - genuinely transient (fetch hiccup, or before the first poll
  on a fresh start), so still falls through to the graceful retry path.
- Neither a REST API nor charge_start_time configured at all - a permanent
  setup gap, not something that resolves on its own. Retrying forever would
  misrepresent a plan Predbat can't actually deliver, so this now raises a
  ValueError with a proper message (unlike the original bare `raise
  ValueError`) and records the error status before doing so.

Split into two tests: the existing no-source-configured test now expects
the raise, and a new test covers the REST-pending case staying graceful.

Author: chalfontchubby

apps/predbat/inverter.py

@@ -1467,14 +1467,23 @@ def update_status(self, minutes_now, quiet=False):
             elif "charge_start_time" in self.base.args:
                 charge_start_time = time_string_to_stamp(self.base.get_arg("charge_start_time", index=self.id))
                 charge_end_time = time_string_to_stamp(self.base.get_arg("charge_end_time", index=self.id))
-            else:
-                # Neither a data source nor a fallback config value is available - log the specific
-                # reason here, but let the shared handling below (which also covers a source that
-                # returned an unusable value) set the status entity and safe defaults, so this
-                # doesn't hard-crash the whole plan for what may just be a temporarily empty feed.
-                self.log("Error: Inverter {} unable to read charge window time as neither REST, charge_start_time or charge_start_hour are set".format(self.id))
+            elif self.rest_api:
+                # A REST data source (givtcp_rest) is configured but hasn't returned anything this
+                # cycle - genuinely transient (a fetch hiccup, or before the first poll on a fresh
+                # start, e.g. GivEnergy cloud "no devices"), so fall through to the same safe-defaults/
+                # retry-next-update handling below as a configured-but-currently-unusable value,
+                # rather than crashing the whole plan for something that should resolve itself.
                 charge_start_time = None
                 charge_end_time = None
+            else:
+                # Neither a REST API nor a charge_start_time config value is configured at all - a
+                # permanent setup gap, not something that will resolve on its own. Retrying every
+                # cycle forever would be misleading, so don't pretend to make a plan Predbat can't
+                # actually deliver (maintainer call on #4288/#4179 - see PR review discussion).
+                message = "Error: Inverter {} unable to read charge window time as neither REST, charge_start_time or charge_start_hour are set".format(self.id)
+                self.log(message)
+                self.base.record_status(message, had_errors=True)
+                raise ValueError(message)
 
             if charge_start_time is None or charge_end_time is None:
                 self.log("Warn: Inverter {} unable to read charge window time as charge_start_time or charge_end_time is None, will retry next update".format(self.id))

apps/predbat/tests/test_inverter.py

@@ -1395,10 +1395,11 @@ def test_charge_window_none_value(test_name, my_predbat, dummy_items):
 
 def test_charge_window_no_source_configured(test_name, my_predbat, dummy_items):
     """
-    Test charge window handling when neither REST data nor a charge_start_time/charge_end_time
-    config source is available at all (issue #4179) - previously a bare `raise ValueError` that
-    crashed the whole plan with an unhelpful "Error: Exception raised" status. Should now fall
-    through to the same safe-defaults/retry-next-update handling as an unusable configured value.
+    Test charge window handling when neither a REST API (givtcp_rest) nor a charge_start_time/
+    charge_end_time config source is configured at all (issue #4179 / PR #4288 review). This is a
+    permanent setup gap, not a transient one, so update_status() should raise a clear, informative
+    ValueError (unlike the original bare `raise ValueError` with no message) rather than silently
+    retrying forever on a config gap that will never close itself - maintainer call on #4288.
     """
     failed = False
     print(f"**** Running Test: {test_name} ****")
@@ -1415,12 +1416,54 @@ def test_charge_window_no_source_configured(test_name, my_predbat, dummy_items):
     original_charge_end_time = my_predbat.args.pop("charge_end_time", None)
     dummy_items["switch.scheduled_charge_enable"] = "on"
 
+    try:
+        inv.update_status(my_predbat.minutes_now)
+        print(f"ERROR: {test_name} - update_status should raise ValueError when no charge window source is configured at all")
+        failed = True
+    except ValueError as e:
+        if "neither REST, charge_start_time or charge_start_hour are set" not in str(e):
+            print(f"ERROR: {test_name} - ValueError message should explain the cause, got: {e}")
+            failed = True
+        if "Error: Inverter" not in my_predbat.current_status:
+            print(f"ERROR: {test_name} - current_status should reflect the error, got: {my_predbat.current_status}")
+            failed = True
+    finally:
+        # Restore config so this doesn't affect later tests
+        if original_charge_start_time is not None:
+            my_predbat.args["charge_start_time"] = original_charge_start_time
+        if original_charge_end_time is not None:
+            my_predbat.args["charge_end_time"] = original_charge_end_time
+
+    return failed
+
+
+def test_charge_window_rest_configured_but_no_data_yet(test_name, my_predbat, dummy_items):
+    """
+    Test charge window handling when a REST API (givtcp_rest) *is* configured but rest_data hasn't
+    successfully returned anything yet this cycle (issue #4179's actual production trigger -
+    GivEnergy cloud "no devices" or a fresh start before the first poll). Unlike the "nothing
+    configured at all" case above, this is genuinely transient - the data source is legitimate, it
+    just hasn't produced a value yet - so it should fall through to the same safe-defaults/
+    retry-next-update handling as a configured-but-currently-unusable value, not raise.
+    """
+    failed = False
+    print(f"**** Running Test: {test_name} ****")
+
+    inv = Inverter(my_predbat, 0)
+    inv.sleep = dummy_sleep
+    inv.inv_has_charge_enable_time = True
+    inv.rest_api = "http://givtcp:6345"
+    inv.rest_data = None
+
+    original_charge_start_time = my_predbat.args.pop("charge_start_time", None)
+    original_charge_end_time = my_predbat.args.pop("charge_end_time", None)
+    dummy_items["switch.scheduled_charge_enable"] = "on"
+
     try:
         inv.update_status(my_predbat.minutes_now)
     except ValueError as e:
-        print(f"ERROR: {test_name} - update_status should not raise when no charge window source is configured, got ValueError({e})")
+        print(f"ERROR: {test_name} - update_status should not raise while a configured REST source just hasn't returned data yet, got ValueError({e})")
         failed = True
-        # Restore config before returning so this doesn't affect later tests
         if original_charge_start_time is not None:
             my_predbat.args["charge_start_time"] = original_charge_start_time
         if original_charge_end_time is not None:
@@ -2697,6 +2740,10 @@ def run_inverter_tests(my_predbat_dummy):
     if failed:
         return failed
 
+    failed |= test_charge_window_rest_configured_but_no_data_yet("charge_window_rest_configured_but_no_data_yet", my_predbat, dummy_items)
+    if failed:
+        return failed
+
     # Test discharge window None handling
     failed |= test_discharge_window_none_illegal_time("discharge_window_illegal_time", my_predbat, dummy_items)
     if failed: