generate_ha: on/off lighting becomes a light; brightness-only "lights" go to review instead of invalid YAML

Found on a real 1312-GA house, measured on all seven corpus projects. B. Lighting on/off (1.001, lighting category) is emitted as a Home Assistant light with address + state_address, not a switch. The HA KNX light platform documents exactly that ("Simple light"). suggest.py already proposed light for these; the two engines disagreed. Without status it is reported as light_without_status. D. A 5.001 lighting GA with no on/off GA in its zone produced a light with only brightness_address. Home Assistant requires address on a KNX light, so that YAML was invalid. 51 of them across six projects, mostly motion-detector parameters on 5.001. They now go to review as light_without_switch, and the brightness status is no longer consumed for an entity that does not exist, so it surfaces as not_mapped. Corpus, before -> after: lights without address 51 -> 0 everywhere; no address lost from YAML or review on any project; switch -> light moves of 220 (house), 205, 62, 43, 17, 10, 13. test_pipeline B1 fixture gains its on/off GAs (it encoded an invalid light) plus a test for the review route. CHANGELOG also records fix A from 481bab0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-02 13:02:51 +02:00 · 2026-09-16 18:32:42 +02:00
parent 481bab0e0c
commit cb226a008f
5 changed files with 100 additions and 28 deletions
@@ -6,6 +6,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ## [Unreleased]

+### Fixed
+
+- **Home Assistant lights without `address` are no longer generated.** A 5.001 lighting GA with no
+  on/off GA in its zone used to become a light with only `brightness_address`, which Home Assistant
+  rejects (`address` is required on a KNX light). On six real projects 51 such lights were generated,
+  mostly motion-detector parameters on 5.001 ("sensitivity", "daytime command"). They now go to the
+  review list as `light_without_switch`; no address is dropped silently.
+- **`subdpt_suspect` false positives.** A 1-bit, date/time or text GA named after a quantity
+  ("on by motion detector by illuminance", "CO2 threshold", "meter value recorded, date") is a flag or
+  timestamp, not the value, and is no longer checked (12 of 12 such hits in the corpus were false).
+  Legitimate DPTs of the same quantity are accepted: power 9.024 and 14.080, energy 13.010 to 13.016
+  and 14.031, and power factor has its own rule (14.057). Checked against xknx 3.20. On a real
+  1312-GA house: 17 findings -> 2.
+
+### Changed
+
+- **On/off lighting is generated as a Home Assistant `light`, not a `switch`.** The KNX light platform
+  takes a plain on/off light with `address` + `state_address`, and light entities are what Assist and
+  "all lights" act on. Lighting without a status GA is reported as `light_without_status`. Regenerated
+  packages move these entities from `switch:` to `light:` (220 on a real house); review before replacing
+  a deployed package.
+
 ## [0.8.2] - 2026-09-14

 ### Added
@@ -413,11 +413,6 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
        if ga.address in consumed:
            continue
        if ga.category == "lighting" and ga.dpt_main == 5 and ga.kind == "command":
-            entity = {"name": ga.name, "brightness_address": ga.address}
-            st5 = status_for_dpt(ga, 5)   # brightness status (5.x)
-            if st5:
-                entity["brightness_state_address"] = st5.address
-                consumed.add(st5.address)
            # two-pass sibling pick: EXACT identity first, subset only as a
            # fallback — else "RGBW подсветка яркость" ({living, подсветка})
            # grabs "камин подсветка" ({living, камин, подсветка}) by subset
@@ -428,6 +423,23 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
            my_ident = _pair_ident(ga.name)
            sib = next((s for s in sibs if _pair_ident(s.name) == my_ident), None) \
                or next((s for s in sibs if _identity_match(ga.name, s.name)), None)
+            if sib is None:
+                # Home Assistant requires `address` on a KNX light, so a brightness GA
+                # with no on/off GA in its zone would be invalid YAML. On real projects
+                # these are mostly device parameters on 5.001 (motion-detector
+                # sensitivity, "daytime command"), not lights. Fail closed.
+                review.append({"reason": "light_without_switch", "address": ga.address,
+                               "name": ga.name, "dpt": ga.dpt,
+                               "hint": "5.001 lighting GA with no on/off GA in its zone. A Home "
+                                       "Assistant light needs `address`; attach the on/off GA "
+                                       "manually, or it is a device parameter, not a light."})
+                consumed.add(ga.address)
+                continue
+            entity = {"name": ga.name, "brightness_address": ga.address}
+            st5 = status_for_dpt(ga, 5)   # brightness status (5.x)
+            if st5:
+                entity["brightness_state_address"] = st5.address
+                consumed.add(st5.address)
            if sib is not None:
                entity["address"] = sib.address
                s1 = status_for_dpt(sib, 1)   # on/off status (1.x)
@@ -466,7 +478,7 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
        attach_colour(entity, ga)
        lights.append(entity)

-    # ---- 3. SWITCHES (1.001 command) ----
+    # ---- 3. ON/OFF LIGHTS and SWITCHES (1.001 command) ----
    for ga in project.gas.values():
        if ga.address in consumed:
            continue
@@ -477,14 +489,19 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
                consumed.add(ga.address)
                continue
            entity = {"name": ga.name, "address": ga.address}
+            # Lighting on/off is a light in Home Assistant, not a switch: the KNX light
+            # platform takes a plain on/off light with `address` + `state_address`
+            # ("Simple light" in the HA KNX docs), and light entities are what Assist
+            # and "all lights" targeting act on.
+            is_light = ga.category == "lighting"
            st = status_for_dpt(ga, 1)
            if st:
                entity["state_address"] = st.address
                consumed.add(st.address)
            else:
-                review.append({"reason": "switch_without_status",
+                review.append({"reason": "light_without_status" if is_light else "switch_without_status",
                               "address": ga.address, "name": ga.name})
-            switches.append(entity)
+            (lights if is_light else switches).append(entity)
            consumed.add(ga.address)

    # ---- 3b. CLIMATE — anchored on an HVAC mode (DPT 20.102/20.105). Zone
@@ -398,21 +398,36 @@ assert _e["color_temperature_mode"] == "absolute", _e
 print("OK: RGBW + colour-temp assembled into one light entity")

 # -- B1: a light with no own brightness status must NOT borrow a sibling's -----
+# (both lights carry their on/off GA: Home Assistant requires `address` on a light,
+#  so a brightness-only light is invalid and goes to review — checked just below)
 _rawB = {"group_addresses": {
+    "1/0/11": ga("1/0/11", "Kitchen worktop LED on/off", 1, 1),
    "1/2/11": ga("1/2/11", "Kitchen worktop LED brightness", 5, 1),   # no own status
+    "1/0/12": ga("1/0/12", "Kitchen island pendants on/off", 1, 1),
    "1/2/12": ga("1/2/12", "Kitchen island pendants brightness", 5, 1),
    "1/5/12": ga("1/5/12", "Kitchen island pendants brightness status", 5, 1),
 }}
 _pB = build_loaded_from_raw(_rawB, "mem")
 _lB = _yaml.safe_load(generate_ha_yaml(_pB)["yaml"].split("\n\n", 1)[1])["knx"]["light"]
 _byname = {l["name"]: l for l in _lB}
-_worktop = _byname["Kitchen worktop LED brightness"]
-_island = _byname["Kitchen island pendants brightness"]
+_worktop = _byname["Kitchen worktop LED"]
+_island = _byname["Kitchen island pendants"]
 assert "brightness_state_address" not in _worktop, \
    f"B1 regression: worktop borrowed a status: {_worktop}"
 assert _island.get("brightness_state_address") == "1/5/12", _island
+assert all("address" in l for l in _lB), _lB
 print("OK: B1 — worktop took no status; island kept its own (no cross-borrow)")

+# -- a brightness GA with no on/off GA is not a valid HA light -> review ------
+_rawD = {"group_addresses": {
+    "1/2/20": ga("1/2/20", "Kitchen worktop LED brightness", 5, 1),
+}}
+_haD = generate_ha_yaml(build_loaded_from_raw(_rawD, "mem"))
+_pkgD = _yaml.safe_load(_haD["yaml"].split("\n\n", 1)[1]) or {}
+assert not (_pkgD.get("knx") or {}).get("light"), _pkgD
+assert any(r["reason"] == "light_without_switch" and r["address"] == "1/2/20" for r in _haD["review"]), _haD["review"]
+print("OK: brightness-only GA -> review light_without_switch, no invalid light in YAML")
+
 # -- A-climate: thermostat zone assembles; a mode-only zone goes to review -----
 _rawT = {"group_addresses": {
    # full zone -> valid climate
@@ -9,6 +9,9 @@ A. subdpt_suspect false positives
     "electricity" name. (Checked against xknx 3.20.)
   - real findings survive: a bare DPT 9 named "lux threshold", a 5.x named
     "temperature control".
+B. on/off lighting (1.001, lighting category) is emitted as a Home Assistant light
+   (address + state_address), not a switch. A brightness-only light, which HA would
+   reject for lacking `address`, goes to review (tested in test_pipeline.py).
 """
 import os
 import sys
@@ -58,4 +61,19 @@ missed = [a for a, *_ in real_findings if a not in flagged]
 assert not missed, f"real findings no longer flagged: {missed}"
 print(f"OK: A — {len(false_positives)} false positives silent, {len(real_findings)} real findings still flagged")

+# ---------------------------------------------------------------- B. on/off lighting -> light
+import yaml
+from nickol_knx_mcp.generate_ha import generate_ha_yaml
+
+pB = _project([
+    ("1/0/1", "1.05 Bedroom - Ceiling - on/off", 1, 1),
+    ("1/1/1", "1.05 Bedroom - Ceiling - on/off status", 1, 11),
+    ("5/0/1", "Garden pump - on/off", 1, 1),
+])
+knxB = yaml.safe_load(generate_ha_yaml(pB)["yaml"].split("\n\n", 1)[1])["knx"]
+lightsB = {l["address"]: l for l in knxB.get("light", [])}
+assert "1/0/1" in lightsB and lightsB["1/0/1"].get("state_address") == "1/1/1", knxB
+assert not any(s["address"] == "1/0/1" for s in knxB.get("switch", [])), knxB
+print("OK: B — on/off lighting is a light with address + state_address, not a switch")
+
 print("\nALL REAL-HOUSE REGRESSION TESTS PASSED")
@@ -33,9 +33,9 @@
        "binary_sensor": 13,
        "climate": 6,
        "cover": 6,
-        "light": 12,
+        "light": 25,
        "sensor": 41,
-        "switch": 19
+        "switch": 6
      },
      "ha_review_items": 57,
      "suggestion_hints": {
@@ -87,11 +87,11 @@
        "climate": 3,
        "cover": 1,
        "expose": 1,
-        "light": 142,
+        "light": 180,
        "sensor": 55,
-        "switch": 85
+        "switch": 42
      },
-      "ha_review_items": 408,
+      "ha_review_items": 413,
      "suggestion_hints": {
        "channels": 229,
        "diagnostics_skipped": 2,
@@ -137,9 +137,9 @@
      "group_addresses": 167,
      "ha_entities": {
        "expose": 1,
-        "light": 12,
+        "light": 29,
        "sensor": 19,
-        "switch": 20
+        "switch": 3
      },
      "ha_review_items": 69,
      "suggestion_hints": {
@@ -194,11 +194,11 @@
        "binary_sensor": 5,
        "cover": 8,
        "expose": 1,
-        "light": 1,
+        "light": 10,
        "sensor": 87,
-        "switch": 58
+        "switch": 48
      },
-      "ha_review_items": 233,
+      "ha_review_items": 234,
      "suggestion_hints": {
        "channels": 135,
        "diagnostics_skipped": 8,
@@ -259,11 +259,11 @@
        "climate": 7,
        "cover": 7,
        "expose": 1,
-        "light": 15,
+        "light": 75,
        "sensor": 111,
-        "switch": 141
+        "switch": 79
      },
-      "ha_review_items": 324,
+      "ha_review_items": 326,
      "suggestion_hints": {
        "channels": 179,
        "diagnostics_skipped": 21,
@@ -324,9 +324,9 @@
        "climate": 16,
        "cover": 11,
        "expose": 1,
-        "light": 12,
+        "light": 232,
        "sensor": 159,
-        "switch": 315
+        "switch": 95
      },
      "ha_review_items": 598,
      "suggestion_hints": {
@@ -390,11 +390,11 @@
        "climate": 23,
        "cover": 40,
        "expose": 1,
-        "light": 250,
+        "light": 412,
        "sensor": 343,
-        "switch": 524
+        "switch": 319
      },
-      "ha_review_items": 1545,
+      "ha_review_items": 1589,
      "suggestion_hints": {
        "channels": 983,
        "diagnostics_skipped": 53,