diff --git a/nickol_knx_mcp/generate_ha.py b/nickol_knx_mcp/generate_ha.py index c5e2f1d..67b8051 100644 --- a/nickol_knx_mcp/generate_ha.py +++ b/nickol_knx_mcp/generate_ha.py @@ -42,6 +42,23 @@ def _ident_tokens(name: str) -> set: _UPDOWN_PHRASES = ("up/down", "auf/ab", "ab/auf", "updown", "up-down", "вверх/вниз") _STOP_WORDS = ("stop", "stopp", "стоп") +# Tokens that mark a bare 1.007/1.010 as belonging to ANOTHER domain, so it must +# never be admitted as a cover's step/stop even if its name says "stop" — a +# "stop" is an OPERATION signal, not a DOMAIN one (council review, issue #11). +_FOREIGN_DOMAIN_TOKENS = frozenset({ + "light", "licht", "lamp", "lampe", "dim", "dimm", "dimmen", "dimming", "led", + "socket", "steckdose", "outlet", "heiz", "heizung", "heating", "hvac", "klima", + "свет", "розетка", "диммер"}) +# NB: ventilation words (vent/Lüftung/fan) are intentionally NOT here — a roof-window +# or ventilation-flap opener is a legitimate cover, so those stay admissible. +# Central/collective tokens: a house-wide "Alle Stopp" 1.007 is not one cover's own +# step/stop and must not be greedily stolen by the first cover (council review). NB: +# "master" is intentionally NOT here — it is a common room qualifier (Master Bedroom) +# and excluding it dropped legitimate covers (gate-1 audit); real macros use all/zentral. +_CENTRAL_TOKENS = frozenset({ + "all", "alle", "zentral", "zentrale", "central", "global", "gesamt", + "universal", "все", "центр", "общий"}) + def _is_updown(name: str) -> bool: """A shutter move (long) command: 'up/down', 'auf/ab', or both directions.""" @@ -72,6 +89,12 @@ def _is_shutter_control(g, slat_addrs) -> bool: if g.address in slat_addrs: return True if g.dpt_main == 1 and g.dpt_sub in (7, 10): + toks = set(base_tokens(g.name or "")) + # A foreign-domain step (lighting dim-stop, HVAC) or a central/collective + # "Alle Stopp" is not a cover's own step/stop — reject before the name + # operation signal (council review: operation != domain; no master-steal). + if toks & _FOREIGN_DOMAIN_TOKENS or toks & _CENTRAL_TOKENS: + return False low = (g.name or "").lower() return (_is_stop(g.name) or _is_updown(g.name) or any(w in low for w in _BLIND_GENERIC)) @@ -285,9 +308,13 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]: def _rank(item): # same ETS Function first (authoritative), then the strongest name - # overlap (type+zone beats zone alone), then the nearest sub index. + # overlap (type+zone beats zone alone), then the nearest sub index, then + # a stable lexicographic address tiebreak so the pick is DETERMINISTIC + # across parses on a full tie (council review; a true semantic tie is a + # known limitation — determinism here is not a correctness claim). sib, same_fn, overlap = item - return (1 if same_fn else 0, overlap, -abs((sib.sub or 0) - (ga.sub or 0))) + return (1 if same_fn else 0, overlap, -abs((sib.sub or 0) - (ga.sub or 0)), + -(sib.main or 0), -(sib.middle or 0), -(sib.sub or 0)) def _take(pred, key): pool = [c for c in cands diff --git a/tests/test_cover_pairing.py b/tests/test_cover_pairing.py index 737c13b..e160ff0 100644 --- a/tests/test_cover_pairing.py +++ b/tests/test_cover_pairing.py @@ -70,6 +70,20 @@ def _build(): gas["2/6/0"] = _ga("2/6/0", "Nordseite Rollläden auf/ab", 1, 8) gas["2/6/1"] = _ga("2/6/1", "Nordseite Licht Schritt", 1, 7) # foreign step gas["2/6/2"] = _ga("2/6/2", "Nordseite Rollläden Stopp", 1, 10) # own stop + # E. a foreign lighting step that DOES carry a stop word ("Licht Stopp") must + # still be rejected — a "stop" is an operation signal, not a domain one + # (council review). The roller takes its own stop. + gas["2/7/0"] = _ga("2/7/0", "Ostseite Rollläden auf/ab", 1, 8) + gas["2/7/1"] = _ga("2/7/1", "Ostseite Licht Stopp", 1, 7) # foreign, has "stop" + gas["2/7/2"] = _ga("2/7/2", "Ostseite Rollläden Stopp", 1, 10) # own stop + # F. a central/collective "Alle Stopp" must not be greedily stolen as one + # cover's step/stop; with no own stop the cover fails closed (council review). + gas["2/8/0"] = _ga("2/8/0", "Nordost Markise auf/ab", 1, 8) + gas["2/8/9"] = _ga("2/8/9", "Nordost Alle Stopp", 1, 7) # central macro + # G. "Master" is a room qualifier, NOT a central macro — a Master-zone cover + # with a bare stop must keep its own step/stop (gate-1 audit regression). + gas["2/9/0"] = _ga("2/9/0", "Master Schlafzimmer auf/ab", 1, 8) + gas["2/9/1"] = _ga("2/9/1", "Master Schlafzimmer Start/Stopp", 1, 7) raw = {"info": {"group_address_style": "ThreeLevel", "schema_version": "21"}, "group_addresses": gas, "communication_objects": {}, "devices": {}, @@ -101,10 +115,16 @@ def main(): "2/5/10": ("2/5/11", None), # C: roller takes its own, NOT 2/5/16 "2/5/15": ("2/5/16", None), # C: awning keeps its own "2/6/0": ("2/6/2", None), # D: own stop, NOT the foreign light step + "2/7/0": ("2/7/2", None), # E: own stop, NOT the "Licht Stopp" + "2/8/0": (None, None), # F: no own stop -> fail closed, not the central + "2/9/0": ("2/9/1", None), # G: "Master" is a zone, not a macro } - # D: the foreign lighting step must never be borrowed as a cover control. - assert "2/6/1" not in [c.get("move_short_address") for c in covers.values()], \ - "a foreign 1.007 (lighting step) was mis-paired as a cover step/stop" + used_shorts = [c.get("move_short_address") for c in covers.values()] + # D/E: a foreign lighting step must never be borrowed as a cover control. + assert "2/6/1" not in used_shorts, "foreign 1.007 'Licht Schritt' mis-paired" + assert "2/7/1" not in used_shorts, "foreign 1.007 'Licht Stopp' mis-paired" + # F: the central 'Alle Stopp' must not be stolen by any cover. + assert "2/8/9" not in used_shorts, "central 'Alle Stopp' was greedily stolen" for mv, (step, spos) in expected.items(): assert mv in covers, f"missing cover for {mv}: {sorted(covers)}" got = covers[mv].get("move_short_address")