diff --git a/CHANGELOG.md b/CHANGELOG.md index f6c99bc..8cfe1c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Noise & unsafe repairs surfaced by an external expert review on the demo house.** (1) Scene-control + GAs (DPT 17/18) are no longer flagged as `missing_status` — they recall a preset and have no single + state to read back (now `scene_no_status`, INFO), and `suggest_repairs` no longer synthesises a bogus + boolean status GA for them. (2) Generic group commands (`All blinds down`, `All shutters up`) are now + recognised as central macros (INFO) like `All lights off`, not missing-status warnings. (3) Synthesised + repair names match the GA's language — an English project no longer gets a Russian `(статус)`/`Значение + яркости` suffix. On the demo house this cut `missing_status_address` warnings from ~7 (mostly scenes) to + the one genuine case. `tests/test_council_fixes.py`. + - **`skills/ha-git-backup`**: `install.sh` now pins `core.sshCommand` + a repo-local `known_hosts` so the nightly sync pushes from HA's Core container (was failing `Host key verification failed`). diff --git a/nickol_knx_mcp/analyze.py b/nickol_knx_mcp/analyze.py index 76697ef..5bfae44 100644 --- a/nickol_knx_mcp/analyze.py +++ b/nickol_knx_mcp/analyze.py @@ -171,6 +171,9 @@ def _is_status_ga(ga: GARecord) -> bool: # rather than a defect — surfaced as INFO, not a 🟡 warning. _CENTRAL_MACRO_TOKENS = ( "общее", "групповое", "все ", "всё", "central", "all groups", "all lights", + # generic group/broadcast commands (multi-word so they don't false-match "wall") + "all blinds", "all shutters", "all covers", "all windows", "all sockets", + "all off", "all on", "master off", "всех ", ) @@ -179,6 +182,12 @@ def _is_central_macro(name: str) -> bool: return any(t in low for t in _CENTRAL_MACRO_TOKENS) +def _is_scene(ga: GARecord) -> bool: + """Scene-control / scene-number GAs (17.x / 18.x) have no single real state to + read back — a missing status is expected, not a defect.""" + return ga.dpt_main in (17, 18) or ga.category == "scene" + + # --------------------------------------------------------------------------- # # Sub-DPT sanity (A1): a name implies a specific DPT sub-type. Flag when the # main matches but the sub is wrong (9.001 temp vs 9.004 lux), or — for strong @@ -257,7 +266,15 @@ def detect_missing_status(project: LoadedProject) -> list[dict[str, Any]]: if self_reporting(ga, project): n_self_report += 1 continue - if _is_central_macro(ga.name): + if _is_scene(ga): + findings.append(_finding( + SEVERITY_INFO, "scene_no_status", addr, + f"Scene control '{ga.name}' (DPT {ga.dpt or '?'}) has no status GA — " + "expected: a scene recalls/stores a preset, it has no single state to " + "read back. No status is applicable.", + name=ga.name, dpt=ga.dpt, category=ga.category, + )) + elif _is_central_macro(ga.name): findings.append(_finding( SEVERITY_INFO, "central_macro_no_status", addr, f"Central/group macro '{ga.name}' has no status GA — expected " diff --git a/nickol_knx_mcp/repair.py b/nickol_knx_mcp/repair.py index f44fecf..9c45e69 100644 --- a/nickol_knx_mcp/repair.py +++ b/nickol_knx_mcp/repair.py @@ -22,6 +22,16 @@ _STATUS_DPT = {1: "1.011", 3: "5.001", 5: "5.001", 9: "9.001", 13: "13.013", 14: "14.056", 20: "20.102"} +def _has_cyrillic(s: str) -> bool: + return any("Ѐ" <= c <= "ӿ" for c in (s or "")) + + +def _suffix(name: str, ru: str, en: str) -> str: + """Match the suffix language to the GA name so an English project doesn't get a + Russian '(статус)' and vice-versa (council feedback: mixed-language names).""" + return ru if _has_cyrillic(name) else en + + def _infer_dpt(ga: Any) -> str: """Best-effort DPT for a GA that has none, from its name/category/kind.""" exp = _expected_subdpt(ga.name) @@ -88,7 +98,8 @@ def suggest_repairs(project: LoadedProject) -> dict[str, Any]: new = _next_free(used, ga.main if ga.main is not None else 1) proposals.append({ "code": code, "action": "add_ga", "address": new, "for": addr, - "name": f"{ga.name} - Значение яркости", "dpt": "5.001", + "name": f"{ga.name}{_suffix(ga.name, ' - Значение яркости', ' - Brightness value')}", + "dpt": "5.001", "rationale": "absolute-brightness GA so Home Assistant can set a level", }) @@ -103,7 +114,7 @@ def suggest_repairs(project: LoadedProject) -> dict[str, Any]: new = _next_free(used, ga.main if ga.main is not None else 1, prefer_middle=4) proposals.append({ "code": "missing_status", "action": "add_ga", "address": new, "for": addr, - "name": f"{ga.name} (статус)", "dpt": sdpt, + "name": f"{ga.name}{_suffix(ga.name, ' (статус)', ' (status)')}", "dpt": sdpt, "rationale": "status/feedback GA so Home Assistant reads real state", }) diff --git a/tests/test_council_fixes.py b/tests/test_council_fixes.py new file mode 100644 index 0000000..bd01eb3 --- /dev/null +++ b/tests/test_council_fixes.py @@ -0,0 +1,61 @@ +"""Fixes from the external-council review (run on the demo house): + * scene-control GAs (17/18) must NOT be flagged as missing_status (they have no + single real state) -> a scene_no_status INFO instead, and no synthesised status; + * generic group commands like "All blinds down" are central macros (INFO), like + "All lights off" -> not a missing_status warning; + * synthesised repair names match the GA's language (no Russian "(статус)" on an + English project). +""" +from nickol_knx_mcp.project import build_loaded_from_raw +from nickol_knx_mcp.analyze import detect_missing_status +from nickol_knx_mcp.repair import suggest_repairs + + +def _ga(addr, name, dmain, dsub): + return {"name": name, "identifier": f"GA-{addr}", "raw_address": 0, "address": addr, + "project_uid": None, "dpt": {"main": dmain, "sub": dsub}, "data_secure": False, + "communication_object_ids": [], "description": "", "comment": ""} + + +def _proj(gas): + raw = {"info": {"group_address_style": "ThreeLevel", "schema_version": "21"}, + "group_addresses": gas, "communication_objects": {}, "devices": {}, + "functions": {}, "topology": {}, "group_ranges": {}} + return build_loaded_from_raw(raw, "t.knxproj") + + +def main(): + gas = { + "0/0/1": _ga("0/0/1", "Scene Movie", 18, 1), # scene -> not missing_status + "0/1/2": _ga("0/1/2", "All blinds down", 1, 8), # central macro -> INFO + "1/0/1": _ga("1/0/1", "Kitchen light on", 1, 1), # EN switch, no status + "3/0/1": _ga("3/0/1", "Кухня свет вкл", 1, 1), # RU switch, no status + } + p = _proj(gas) + ms = {(f["code"], f["address"]) for f in detect_missing_status(p)} + + # scene -> scene_no_status INFO, never missing_status_address + assert ("scene_no_status", "0/0/1") in ms, ms + assert ("missing_status_address", "0/0/1") not in ms + + # "All blinds down" -> central macro INFO (was wrongly a warning before) + assert ("central_macro_no_status", "0/1/2") in ms, ms + assert ("missing_status_address", "0/1/2") not in ms + + # the plain switches DO still get flagged + assert ("missing_status_address", "1/0/1") in ms + assert ("missing_status_address", "3/0/1") in ms + + # repair language matches the GA name; and scenes get no synthesised status + props = suggest_repairs(p)["proposals"] + names = {p_["for"]: p_["name"] for p_ in props if p_.get("action") == "add_ga"} + assert names.get("1/0/1", "").endswith("(status)"), names # EN + assert names.get("3/0/1", "").endswith("(статус)"), names # RU + assert "0/0/1" not in names, "a scene must not get a synthesised status GA" + + print("test_council_fixes: OK — scenes exempt (scene_no_status, no synth status), " + "'All blinds down' recognised as central macro, repair suffix matches language.") + + +if __name__ == "__main__": + main()