diff --git a/CHANGELOG.md b/CHANGELOG.md index e85bee3..0b8f4bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/nickol_knx_mcp/generate_ha.py b/nickol_knx_mcp/generate_ha.py index 0b817d5..666c814 100644 --- a/nickol_knx_mcp/generate_ha.py +++ b/nickol_knx_mcp/generate_ha.py @@ -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 diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 4f125f8..3f7dd80 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -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 diff --git a/tests/test_real_house_fixes.py b/tests/test_real_house_fixes.py index 598a6de..fea5833 100644 --- a/tests/test_real_house_fixes.py +++ b/tests/test_real_house_fixes.py @@ -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") diff --git a/tools/corpus_baseline.json b/tools/corpus_baseline.json index 122e875..f62418c 100644 --- a/tools/corpus_baseline.json +++ b/tools/corpus_baseline.json @@ -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,