mirror of
https://github.com/NickoScope/nickol-knx-mcp.git
synced 2026-09-29 19:31:12 +02:00
fix(#12): recognize a '… RM' (Rückmeldung) shutter feedback as the cover's position status, not a light
Field data by Kris1166 (issue #12). A DPT-5.001 shutter position feedback named only '… RM' was classified lighting/light and emitted as a bogus HA light entity. Two layers: - classification (project.py): an ETS Function carrying a shutter role promotes a member to shutter — but only on POSITIVE evidence (member is a step/stop 1.007/1.010 OR a 5.001 position STATUS, the Function has an up/down move sibling, and the name has no explicit non-shutter domain), so a scene-recall/dimmer-feedback/lock sharing a mixed Function is left alone (LLM-council review; a Function is a free grouping, not proof of domain). - status recognition (project.py name_is_status): a standalone 'RM'/'FB' token is now a status — the 'rm '/'rm_' substrings and the stopword tokenizer both missed a name ending in bare 'RM'. - analyze._is_status_ga delegates to name_is_status; generate_ha then wires it as the cover's position_state_address. Passed both gates (self-audit + LLM-council). tests/test_rm_status.py pins it (red/green + negatives: a light's RM stays light, a stray light / a scene-recall in a shutter Function are not promoted). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Nikolay Miroshnichenko
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
fb53018b59
commit
f20b2e4126
@@ -43,6 +43,9 @@ jobs:
|
||||
- name: Cover step/stop pairing stays within the ETS Function (issue #11)
|
||||
run: python tests/test_cover_pairing.py
|
||||
|
||||
- name: RM/Rückmeldung shutter feedback classifies as status, not light (issue #12)
|
||||
run: python tests/test_rm_status.py
|
||||
|
||||
- name: Console script is installed
|
||||
run: |
|
||||
python -c "import importlib.metadata as m; print('entry points:', [e.name for e in m.entry_points(group='console_scripts') if e.name == 'nickol-knx-mcp'])"
|
||||
|
||||
@@ -14,7 +14,7 @@ import re
|
||||
from collections import defaultdict
|
||||
from typing import Any, Optional
|
||||
|
||||
from .project import LoadedProject, GARecord, STATUS_KEYWORDS
|
||||
from .project import LoadedProject, GARecord, name_is_status
|
||||
from .pairing import (find_status, function_status_pairs, base_tokens,
|
||||
positional_status, self_reporting)
|
||||
from .intent import INTENT_FUNCTIONAL, INTENT_RESERVE, INTENT_SCRATCH
|
||||
@@ -161,8 +161,7 @@ def _function_role_status(project: LoadedProject) -> list[dict[str, Any]]:
|
||||
def _is_status_ga(ga: GARecord) -> bool:
|
||||
if ga.kind == "status":
|
||||
return True
|
||||
low = ga.name.lower()
|
||||
return any(k in low for k in STATUS_KEYWORDS)
|
||||
return name_is_status(ga.name)
|
||||
|
||||
|
||||
# Central / group-macro command names ("Общее освещение - Все группы", "Все
|
||||
|
||||
+70
-23
@@ -31,6 +31,27 @@ COMMAND_KEYWORDS = [
|
||||
"soll", "вкл", "выкл", "упр", "команд", "задан",
|
||||
]
|
||||
|
||||
# Standalone status abbreviations that sit as a whole trailing token ("… RM").
|
||||
# The substring keywords above carry "rm "/"rm_" (a following delimiter) and so
|
||||
# MISS a name that *ends* in the bare token, and base_tokens() drops "rm"/"fb"
|
||||
# as stopwords, so token-overlap pairing can't see them either — a DPT-5.001
|
||||
# shutter feedback named "… RM" then falls to the lighting default (issue #12,
|
||||
# field data by Kris1166). Region conventions vary; RM/FB are the German-market
|
||||
# standard for Rückmeldung / Feedback.
|
||||
_STATUS_ABBREV = frozenset({"rm", "fb"})
|
||||
|
||||
|
||||
def name_is_status(name: str) -> bool:
|
||||
"""True if a GA name marks it a status/feedback object — via the substring
|
||||
keywords OR a standalone status abbreviation token (RM/FB) that both the
|
||||
substring form and the tokenizer miss."""
|
||||
low = (name or "").lower()
|
||||
if any(k in low for k in STATUS_KEYWORDS):
|
||||
return True
|
||||
for ch in "/-_.,()[]:":
|
||||
low = low.replace(ch, " ")
|
||||
return any(t in _STATUS_ABBREV for t in low.split())
|
||||
|
||||
# Domain terms by name (used to decide a GA's functional domain as ONE signal,
|
||||
# combined with the DPT and the group-range context — see _classify_category).
|
||||
# Checked in this priority order (most specific first; lighting is the generic
|
||||
@@ -256,7 +277,7 @@ def _split_three_level(address: str) -> tuple[Optional[int], Optional[int], Opti
|
||||
def _override_kind_by_name(name: str, kind: str) -> str:
|
||||
"""Refine command/status using name keywords (helps when DPT is generic)."""
|
||||
low = name.lower()
|
||||
if any(k in low for k in STATUS_KEYWORDS):
|
||||
if name_is_status(name):
|
||||
return "status"
|
||||
if any(k in low for k in COMMAND_KEYWORDS):
|
||||
# only upgrade unknown -> command; never overwrite explicit sensor
|
||||
@@ -356,29 +377,55 @@ def build_loaded_from_raw(raw: KNXProject, path: str) -> LoadedProject:
|
||||
)
|
||||
gas[rec.address] = rec
|
||||
|
||||
# Authoritative shutter classification from ETS Function roles. A GA whose
|
||||
# Function role is a shutter role (MoveUpDown / StopStepUpDown / slat) IS a
|
||||
# shutter object — even when its bare DPT is domain-agnostic (1.007 step,
|
||||
# 1.010 start/stop) and its name carries no shutter keyword. The Function
|
||||
# role outranks DPT and name (explain_ga's own hierarchy), so it rescues the
|
||||
# step/stop GAs a name-only classifier leaves 'unknown' (issue #11, field
|
||||
# data by Kris1166). Never demotes: only promotes a non-shutter GA.
|
||||
# Authoritative shutter classification from ETS Function MEMBERSHIP.
|
||||
# A Function that carries a shutter role (MoveUpDown / StopStepUpDown / slat)
|
||||
# on ANY of its members is a shutter Function — so every GA inside it is a
|
||||
# shutter object, even one with a blank role, a domain-agnostic bare DPT
|
||||
# (1.007 step, 1.010 start/stop) and no shutter keyword in its name:
|
||||
# * the step/stop a name-only classifier leaves 'unknown' (issue #11), and
|
||||
# * a DPT-5.001 position feedback named only "… RM" (Rückmeldung) that
|
||||
# otherwise falls to the lighting brightness default (issue #12).
|
||||
# The Function outranks DPT and name (explain_ga's hierarchy). Never demotes;
|
||||
# only promotes a non-shutter GA whose DPT is plausibly a shutter object
|
||||
# (1.x control / 5.x position) — never a 9.x temperature / 13.x energy GA.
|
||||
# An ETS Function is a free grouping, not proof of a single domain (an
|
||||
# integrator can drop lighting, shutters and a scene recall into one "Floor 1"
|
||||
# Function). So promote a member to shutter only on POSITIVE shutter evidence,
|
||||
# not blanket membership (LLM-council review of the issue #12 fix):
|
||||
# * the member is itself a step/stop control (DPT 1.007 / 1.010), or a
|
||||
# DPT-5.001 POSITION STATUS (a feedback — never a 5.001 command, which could
|
||||
# be a scene recall / dimming value), AND
|
||||
# * the Function actually carries an up/down MOVE (DPT 1.008) — the sibling
|
||||
# that makes it a real shutter, not just a shutter-role substring, AND
|
||||
# * the member's name carries no EXPLICIT non-shutter domain (defense in depth).
|
||||
# This rescues the issue #11 step/stop and the issue #12 "… RM" position feedback
|
||||
# while leaving a scene recall / dimmer feedback / boolean lock in a mixed
|
||||
# Function untouched.
|
||||
def _promote_to_shutter(rec: Optional[GARecord], has_move: bool) -> None:
|
||||
if rec is None or rec.category == "shutter" or not has_move:
|
||||
return
|
||||
is_step = rec.dpt_main == 1 and rec.dpt_sub in (7, 10)
|
||||
is_pos_status = rec.dpt_main == 5 and rec.dpt_sub == 1 and rec.kind == "status"
|
||||
if not (is_step or is_pos_status):
|
||||
return
|
||||
dom = _domain_from_text(rec.name)
|
||||
if dom and dom != "shutter":
|
||||
return
|
||||
rec.category = "shutter"
|
||||
if rec.ha_platform in ("light", "switch", "unknown"):
|
||||
rec.ha_platform = "cover"
|
||||
|
||||
for fn in (raw.get("functions", {}) or {}).values():
|
||||
for a_key, ref in (fn.get("group_addresses", {}) or {}).items():
|
||||
role = (ref.get("role") or "").lower()
|
||||
if not any(t in role for t in _SHUTTER_ROLE_TOKENS):
|
||||
continue
|
||||
rec = gas.get(ref.get("address") or a_key)
|
||||
if rec is None or rec.category == "shutter":
|
||||
continue
|
||||
# Guard: a shutter role only promotes a plausibly-shutter DPT — a 1-bit
|
||||
# control (move/step/stop) or a 5.x position — never retype a 9.x
|
||||
# temperature / 13.x energy GA on a role substring like "updown".
|
||||
if rec.dpt_main not in (1, 5):
|
||||
continue
|
||||
rec.category = "shutter"
|
||||
if rec.ha_platform in ("light", "switch", "unknown"):
|
||||
rec.ha_platform = "cover"
|
||||
members = fn.get("group_addresses", {}) or {}
|
||||
recs = [gas.get(ref.get("address") or a_key) for a_key, ref in members.items()]
|
||||
has_shutter_role = any(
|
||||
any(t in (ref.get("role") or "").lower() for t in _SHUTTER_ROLE_TOKENS)
|
||||
for ref in members.values())
|
||||
has_move = any(r is not None and r.dpt_main == 1 and r.dpt_sub == 8 for r in recs)
|
||||
if not has_shutter_role:
|
||||
continue
|
||||
for rec in recs:
|
||||
_promote_to_shutter(rec, has_move)
|
||||
|
||||
return LoadedProject(
|
||||
path=path,
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
"""Regression: a DPT-5.001 shutter position feedback named "… RM" (Rückmeldung,
|
||||
the German-market status abbreviation) must be recognised as the cover's position
|
||||
STATUS — not misclassified as a standalone lighting/light entity (issue #12, field
|
||||
data by Kris1166).
|
||||
|
||||
Two layers were broken and are both covered here:
|
||||
* classification — the RM GA sits in a shutter ETS Function (blank role) but a
|
||||
bare DPT 5.001 with no shutter keyword fell to the lighting default; Function
|
||||
membership now promotes it to `shutter`;
|
||||
* status recognition — "RM"/"FB" is a whole trailing token the substring
|
||||
keywords ("rm "/"rm_") and the stopword tokenizer both dropped, so it read as
|
||||
a command; `name_is_status` now recognises it, so it wires as the cover's
|
||||
`position_state_address` (a status), never a `position_address` (a command).
|
||||
|
||||
Negative guard: a genuine LIGHT brightness feedback named "… RM" that is NOT in a
|
||||
shutter Function must stay `light` — the promotion is Function-scoped, not a
|
||||
blanket "RM ⇒ shutter".
|
||||
"""
|
||||
from nickol_knx_mcp.project import build_loaded_from_raw
|
||||
from nickol_knx_mcp.generate_ha import generate_ha_yaml
|
||||
from nickol_knx_mcp.analyze import detect_missing_status
|
||||
import yaml
|
||||
|
||||
|
||||
def _ga(addr, name, dm, ds):
|
||||
return {"name": name, "identifier": f"G{addr}", "raw_address": 0, "address": addr,
|
||||
"project_uid": None, "dpt": {"main": dm, "sub": ds}, "data_secure": False,
|
||||
"communication_object_ids": [], "description": "", "comment": ""}
|
||||
|
||||
|
||||
def _fn(name, *addr_roles):
|
||||
return {"name": name, "group_addresses": {a: {"address": a, "role": r} for a, r in addr_roles}}
|
||||
|
||||
|
||||
def _build():
|
||||
gas, fns = {}, {}
|
||||
# Window shutter: RM status IS in the per-channel function (blank role); move
|
||||
# carries MoveUpDown. This is the 26/29 case that was misclassified as light.
|
||||
gas["2/1/26"] = _ga("2/1/26", "Zone18 Fenster Bewegen", 1, 8)
|
||||
gas["2/2/26"] = _ga("2/2/26", "Zone18 Fenster Schritt/Stop", 1, 7)
|
||||
gas["2/3/26"] = _ga("2/3/26", "Zone18 Fenster RM", 5, 1)
|
||||
fns["FN-Fenster-26"] = _fn("Fenster", ("2/1/26", "MoveUpDown"),
|
||||
("2/2/26", "StopStepUpDown"), ("2/3/26", ""))
|
||||
# Awning: today it is "accidentally" rescued by the "Markise" keyword — it must
|
||||
# still resolve to the cover's position status, now for the RIGHT reason.
|
||||
gas["2/1/50"] = _ga("2/1/50", "MarkiseZone1 Bewegen", 1, 8)
|
||||
gas["2/2/50"] = _ga("2/2/50", "MarkiseZone1 Schritt/Stop", 1, 17)
|
||||
gas["2/3/50"] = _ga("2/3/50", "MarkiseZone1 RM", 5, 1)
|
||||
fns["FN-Markise-1"] = _fn("Markise Z1", ("2/1/50", ""), ("2/2/50", ""), ("2/3/50", ""))
|
||||
# NEGATIVE: a real light with a brightness feedback named "RM", NOT in a
|
||||
# shutter function — must stay a light, RM is its brightness state.
|
||||
gas["1/0/0"] = _ga("1/0/0", "Küche Licht schalten", 1, 1)
|
||||
gas["1/1/0"] = _ga("1/1/0", "Küche Licht Helligkeit", 5, 1)
|
||||
gas["1/3/0"] = _ga("1/3/0", "Küche Licht RM", 5, 1)
|
||||
# NEGATIVE 2 (gate-1 audit): a blank-role light GA that shares a heterogeneous
|
||||
# shutter Function must NOT be promoted — its explicit light domain wins.
|
||||
gas["2/1/60"] = _ga("2/1/60", "Zone20 Rollladen Bewegen", 1, 8)
|
||||
gas["2/2/60"] = _ga("2/2/60", "Living Room Light On", 1, 1) # stray light in a shutter fn
|
||||
fns["FN-Mixed-60"] = _fn("Mixed", ("2/1/60", "MoveUpDown"), ("2/2/60", ""))
|
||||
# NEGATIVE 3 (LLM-council): a DPT-5.001 scene-recall COMMAND (blank role, neutral
|
||||
# name) sharing a shutter Function must NOT become a cover — it is not a position
|
||||
# STATUS, so promotion must skip it even though the Function is a shutter one.
|
||||
gas["2/1/70"] = _ga("2/1/70", "Zone30 Rollladen Bewegen", 1, 8)
|
||||
gas["2/2/70"] = _ga("2/2/70", "Zone30 Rollladen Schritt/Stop", 1, 7)
|
||||
gas["2/4/70"] = _ga("2/4/70", "Zone30 Szene Abruf", 5, 1) # 5.001 COMMAND, not status
|
||||
fns["FN-Scene-70"] = _fn("Rollladen70", ("2/1/70", "MoveUpDown"),
|
||||
("2/2/70", "StopStepUpDown"), ("2/4/70", ""))
|
||||
|
||||
raw = {"info": {"group_address_style": "ThreeLevel", "schema_version": "21"},
|
||||
"group_addresses": gas, "communication_objects": {}, "devices": {},
|
||||
"functions": fns, "topology": {},
|
||||
"group_ranges": {"2": {"address_start": 4096, "name": "Beschattung",
|
||||
"group_ranges": {}}}}
|
||||
return build_loaded_from_raw(raw, "rm-status.knxproj")
|
||||
|
||||
|
||||
def main():
|
||||
p = _build()
|
||||
|
||||
# 1. Classification: each shutter RM GA is category=shutter, kind=status.
|
||||
for a in ("2/3/26", "2/3/50"):
|
||||
g = p.gas[a]
|
||||
assert g.category == "shutter", f"{a}: category {g.category!r}, expected shutter"
|
||||
assert g.kind == "status", f"{a}: kind {g.kind!r}, expected status"
|
||||
# negative: the LIGHT's RM stays lighting (not promoted — not a shutter function)
|
||||
assert p.gas["1/3/0"].category == "lighting", p.gas["1/3/0"].category
|
||||
assert p.gas["1/3/0"].kind == "status", "a light's RM is still a status"
|
||||
# negative 2: a stray light inside a heterogeneous shutter function stays lighting
|
||||
assert p.gas["2/2/60"].category == "lighting", \
|
||||
f"stray light in a shutter fn was over-promoted: {p.gas['2/2/60'].category}"
|
||||
# negative 3: a 5.001 scene-recall COMMAND in a shutter function is NOT promoted
|
||||
# (only a 5.001 position STATUS is), while the step/stop 2/2/70 still is.
|
||||
assert p.gas["2/4/70"].category != "shutter", \
|
||||
f"a 5.001 scene-recall command was over-promoted to shutter: {p.gas['2/4/70'].category}"
|
||||
assert p.gas["2/2/70"].category == "shutter", "the step/stop in the same fn must promote"
|
||||
|
||||
doc = yaml.safe_load(generate_ha_yaml(p)["yaml"])
|
||||
covers = {c["move_long_address"]: c for c in doc.get("knx", {}).get("cover", []) or []}
|
||||
lights = {l.get("address"): l for l in doc.get("knx", {}).get("light", []) or []}
|
||||
|
||||
# 2. Each cover gets its RM GA as position_state — and the RM GA is NOT a light.
|
||||
assert covers["2/1/26"].get("position_state_address") == "2/3/26", covers["2/1/26"]
|
||||
assert covers["2/1/50"].get("position_state_address") == "2/3/50", covers["2/1/50"]
|
||||
light_addrs = {a for l in lights.values() for a in
|
||||
(l.get("address"), l.get("state_address"), l.get("brightness_address"),
|
||||
l.get("brightness_state_address"))}
|
||||
assert "2/3/26" not in light_addrs, "the shutter RM was wired as a light"
|
||||
assert "2/3/50" not in light_addrs, "the awning RM was wired as a light"
|
||||
|
||||
# 3. negative: the real light still exists and uses its RM as brightness state.
|
||||
assert "1/0/0" in lights, "the genuine light disappeared"
|
||||
|
||||
# 4. no false 'missing status' for the covers whose RM status now pairs.
|
||||
miss = {f["address"] for f in detect_missing_status(p)
|
||||
if f.get("code") == "missing_status_address"}
|
||||
assert "2/1/26" not in miss and "2/1/50" not in miss, f"false missing-status: {miss}"
|
||||
|
||||
print("test_rm_status: OK — '… RM' shutter feedback classifies as shutter/status and "
|
||||
"wires as the cover's position_state (issue #12); a light's RM stays a light.")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
Reference in New Issue
Block a user