fix(gecloud): address #5218 review - empty/comma-in-bracket options, warn once, non-string guard

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Author: CI

apps/predbat/gecloud.py

@@ -402,8 +402,24 @@ def parse_validation_options(validation):
     # Split on the first '(' and drop only the final ')' so option labels that contain brackets survive intact
     post = validation.split("(", 1)[1]
     post = post.rsplit(")", 1)[0]
-    post = post.replace(", ", ",")
-    return post.split(",")
+    if not post.strip():
+        return None
+    # Split only on commas outside brackets so a label such as 'Pause (Battery, Grid)' stays whole
+    options = []
+    label = ""
+    depth = 0
+    for char in post:
+        if char == "(":
+            depth += 1
+        elif char == ")":
+            depth = max(depth - 1, 0)
+        elif char == "," and depth == 0:
+            options.append(label)
+            label = ""
+            continue
+        label += char
+    options.append(label)
+    return [option[1:] if index > 0 and option.startswith(" ") else option for index, option in enumerate(options)]
 
 
 def normalise_register_time(value):
@@ -582,6 +598,8 @@ def initialize(self, ge_cloud_direct, api_key, automatic, automatic_evc=False, e
         # Battery inverters already reported as overriding the EMS, so a standing misconfiguration
         # raises the Predbat status once rather than on every settings refresh
         self.ems_slot_warned = set()
+        # (device, key) registers whose option validation text could not be parsed, so the warning is logged once
+        self.validation_parse_warned = set()
         self.devices_dict = {}
         self.device_list = []
         self.ems_device = None
@@ -776,7 +794,7 @@ async def select_event(self, entity_id, value):
                             if validation_rule.startswith("in:"):
                                 options_values = validation_rule.split(":")[1].split(",")
 
-                    if validation and validation.startswith(VALIDATION_OPTIONS_PREFIX):
+                    if isinstance(validation, str) and validation.startswith(VALIDATION_OPTIONS_PREFIX):
                         options_text = parse_validation_options(validation)
                         if options_text is None:
                             self.log("GECloud: Warn: Unable to parse options for setting {} {} from validation '{}'".format(device, key, validation))
@@ -1352,10 +1370,13 @@ async def publish_registers(self, device, registers, select_key=None):
                         attributes["device_class"] = "power"
                         attributes["unit_of_measurement"] = "W"
 
-            if validation and validation.startswith(VALIDATION_OPTIONS_PREFIX):
+            if isinstance(validation, str) and validation.startswith(VALIDATION_OPTIONS_PREFIX):
                 validation_options = parse_validation_options(validation)
                 if validation_options is None:
-                    self.log("GECloud: Warn: Unable to parse options for setting {} {} from validation '{}'".format(device, key, validation))
+                    # Warn once per register, this runs on every settings refresh
+                    if (device, key) not in self.validation_parse_warned:
+                        self.log("GECloud: Warn: Unable to parse options for setting {} {} from validation '{}'".format(device, key, validation))
+                        self.validation_parse_warned.add((device, key))
                 else:
                     options_text = validation_options
 

apps/predbat/tests/test_ge_cloud.py

@@ -73,6 +73,7 @@ def __init__(self):
         self.ems_device = None
         self.gateway_device = None
         self.ems_slot_warned = set()
+        self.validation_parse_warned = set()
         self._discovery_report = None
         self._now_utc_exact = datetime.now(timezone.utc)
         self.settings_from_cache = False
@@ -4064,6 +4065,28 @@ async def mock_publish(*args, **kwargs):
                 print("ERROR: Expected a warning for unparseable validation, got {}".format(ge_cloud.log_messages))
                 return 1
 
+            # An empty option list must not parse as [""] and reject every value; it falls back to 'in:' too
+            ge_cloud.settings["test123"][90] = {"value": "1", "validation": "Value must be one of: ()", "validation_rules": ["in:1,2"]}
+            await ge_cloud.select_event(entity_id, "1")
+            if write_calls != ["Export (AC3)", "2", "1"]:
+                print("ERROR: Expected empty option list to fall back to 'in:' and accept '1', got {}".format(write_calls))
+                return 1
+
+            # A non-string validation value is ignored rather than raising AttributeError
+            ge_cloud.settings["test123"][90] = {"value": "1", "validation": {"unexpected": True}, "validation_rules": ["in:1,2"]}
+            await ge_cloud.select_event(entity_id, "2")
+            if write_calls != ["Export (AC3)", "2", "1", "2"]:
+                print("ERROR: Expected non-string validation to fall back to 'in:' and accept '2', got {}".format(write_calls))
+                return 1
+
+            # Unparseable validation with no 'in:' rule has nothing to check against, so the write goes through
+            # and the GE Cloud API validates it server-side (the same as any select without option validation text)
+            ge_cloud.settings["test123"][90] = {"value": "1", "validation": "Value must be one of:", "validation_rules": []}
+            await ge_cloud.select_event(entity_id, "5")
+            if write_calls != ["Export (AC3)", "2", "1", "2", "5"]:
+                print("ERROR: Expected write without local validation when no options are known, got {}".format(write_calls))
+                return 1
+
         return 0
 
     return run_async(test())
@@ -4076,6 +4099,13 @@ def _test_parse_validation_options(my_predbat):
     cases = [
         ("Value must be one of: (00:00, 00:30, 01:00)", ["00:00", "00:30", "01:00"]),
         ("Value must be one of: (Eco (Paused), Timed Demand, Export (AC3))", ["Eco (Paused)", "Timed Demand", "Export (AC3)"]),
+        # Commas inside a label's own brackets do not split it
+        ("Value must be one of: (Eco, Pause (Battery, Grid), Export)", ["Eco", "Pause (Battery, Grid)", "Export"]),
+        ("Value must be one of: (0,1,2)", ["0", "1", "2"]),
+        # An empty option list is unparseable, not a single empty-string option
+        ("Value must be one of: ()", None),
+        ("Value must be one of: (", None),
+        ("Value must be one of: ( )", None),
         ("Value must be one of:", None),
         ("Value must be between 0 and 100", None),
         ("", None),
@@ -4334,6 +4364,14 @@ def _test_publish_registers_bracketed_options(my_predbat):
         print("ERROR: Expected later registers to still be published")
         return 1
 
+    # A permanently unparseable register warns once, not on every settings refresh
+    run_async(ge_cloud.publish_registers("test123", registers))
+    run_async(ge_cloud.publish_registers("test123", registers))
+    warnings = [message for message in ge_cloud.log_messages if "Unable to parse options" in message]
+    if len(warnings) != 1:
+        print("ERROR: Expected exactly one unparseable-validation warning across three refreshes, got {}".format(warnings))
+        return 1
+
     return 0