mirror of
https://github.com/NickoScope/nickol-knx-mcp.git
synced 2026-09-29 19:31:12 +02:00
load_ga_export, entity naming, climate setpoint shift; fix suggest_repairs crash; CI runs every test
Three things found by auditing a TapPlan export, plus one regression found on the way. load_ga_export(path): new tool. Reads an ETS ga-export/01 XML (ETS "Export Group Addresses", or the import file a planning tool writes) into a project without devices, through safe_fromstring, capped at 50 MB and 8 levels of range nesting. Invalid or duplicate addresses and unknown DPT tokens go to import_warnings, never silently. Round trip with our own generate_ets_xml is covered by a test. Entity naming: lights, covers and climates are named after the common word prefix of their member names, cutting only function words. The first version of the rule turned "01. <room> - All Blinds - Move" into "01" on a real project, so anything not in the function vocabulary now stays. On six real projects 214 of 1189 entities got a shorter name, each rename reviewed, no address mapping changed. Regenerated packages show different names; noted in the changelog. Setpoint shift: 9.002 / 6.010 with a shift word maps to setpoint_shift_address, setpoint_shift_state_address and setpoint_shift_mode (keys checked against the HA KNX climate docs). It used to become a plain sensor. Fixed: suggest_repairs raised UnboundLocalError on any project with a missing status GA. My local rename in the 12.09 typing cleanup left two references to the old name. test_council_fixes covers it but was not in CI, so it shipped. CI now runs all 21 test files instead of a hand-picked 10; the 11 added ones pass from a clean clone. Verified: all 21 tests, ruff, mypy with the package installed, corpus guard no drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
29bfba7684
commit
ec63bcf106
@@ -0,0 +1,170 @@
|
||||
"""Read an ETS group-address export (ga-export/01 XML) as a project without devices.
|
||||
|
||||
A full ``.knxproj`` is not always what people have. ETS can export just the group
|
||||
addresses, and planning tools (TapPlan and others) produce the same format for
|
||||
import into ETS. That file carries names, addresses, DPTs, descriptions, the
|
||||
security flag and the range tree, which is everything the GA-level checks and the
|
||||
Home Assistant / ETS generators work on.
|
||||
|
||||
The export is turned into the same raw shape xknxproject produces for the group
|
||||
address part, then goes through the normal ``build_loaded_from_raw``. Devices,
|
||||
communication objects, ETS Functions and topology are simply empty, so the
|
||||
device-level tools have nothing to report rather than guessing.
|
||||
|
||||
The file is untrusted input: size-capped and parsed through ``safe_fromstring``
|
||||
(no DTD, no entities, no external fetches).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
from pathlib import Path
|
||||
from typing import Any, Optional, cast
|
||||
|
||||
from .project import LoadedProject, build_loaded_from_raw
|
||||
from .safexml import SafeXmlError, safe_fromstring
|
||||
|
||||
MAX_EXPORT_BYTES = 50 * 1024 * 1024 # a 15 000-GA export is a few MB; this only bounds abuse
|
||||
MAX_RANGE_DEPTH = 8 # ETS nests main/middle (2); planning tools rarely go deeper
|
||||
|
||||
_DPT_RE = re.compile(r"^DPS?T-(\d+)(?:-(\d+))?$", re.IGNORECASE)
|
||||
|
||||
|
||||
class GaExportError(ValueError):
|
||||
"""The file is not a readable ETS group-address export."""
|
||||
|
||||
|
||||
def _local(tag: str) -> str:
|
||||
return tag.rsplit("}", 1)[-1]
|
||||
|
||||
|
||||
def parse_dpt(value: Optional[str]) -> Optional[dict[str, Optional[int]]]:
|
||||
"""'DPST-1-1' -> {main 1, sub 1}; 'DPT-9' -> {main 9, sub None}; several -> the first."""
|
||||
if not value:
|
||||
return None
|
||||
first = re.split(r"[\s,;]+", value.strip())[0]
|
||||
m = _DPT_RE.match(first)
|
||||
if not m:
|
||||
return None
|
||||
return {"main": int(m.group(1)), "sub": int(m.group(2)) if m.group(2) else None}
|
||||
|
||||
|
||||
def _raw_address(address: str) -> Optional[int]:
|
||||
try:
|
||||
parts = [int(x) for x in address.split("/")]
|
||||
except ValueError:
|
||||
return None
|
||||
if len(parts) == 3 and parts[0] <= 31 and parts[1] <= 7 and parts[2] <= 255:
|
||||
return (parts[0] << 11) | (parts[1] << 8) | parts[2]
|
||||
if len(parts) == 2 and parts[0] <= 31 and parts[1] <= 2047:
|
||||
return (parts[0] << 11) | parts[1]
|
||||
if len(parts) == 1 and 0 <= parts[0] <= 65535:
|
||||
return parts[0]
|
||||
return None
|
||||
|
||||
|
||||
def _style(addresses: list[str]) -> str:
|
||||
depths = {a.count("/") for a in addresses}
|
||||
if depths == {2}:
|
||||
return "ThreeLevel"
|
||||
if depths == {1}:
|
||||
return "TwoLevel"
|
||||
if depths == {0}:
|
||||
return "Free"
|
||||
return "Mixed" if depths else ""
|
||||
|
||||
|
||||
def read_ga_export_bytes(data: bytes, name: str = "ga-export") -> dict[str, Any]:
|
||||
"""Parse export bytes into the raw project dict (no devices). Raises GaExportError."""
|
||||
try:
|
||||
root = safe_fromstring(data)
|
||||
except SafeXmlError as e:
|
||||
raise GaExportError(str(e)) from e
|
||||
if _local(root.tag) != "GroupAddress-Export":
|
||||
raise GaExportError(f"not an ETS group-address export (root element is <{_local(root.tag)}>)")
|
||||
|
||||
gas: dict[str, dict[str, Any]] = {}
|
||||
warnings: list[str] = []
|
||||
|
||||
def walk_range(el, depth: int = 1) -> dict[str, Any]:
|
||||
if depth > MAX_RANGE_DEPTH:
|
||||
raise GaExportError(f"group ranges nested deeper than {MAX_RANGE_DEPTH} levels — refused")
|
||||
start = el.get("RangeStart")
|
||||
end = el.get("RangeEnd")
|
||||
rng: dict[str, Any] = {
|
||||
"name": el.get("Name", ""),
|
||||
"address_start": int(start) if start and start.isdigit() else None,
|
||||
"address_end": int(end) if end and end.isdigit() else None,
|
||||
"comment": el.get("Description", ""),
|
||||
"group_addresses": [],
|
||||
"group_ranges": {},
|
||||
}
|
||||
for child in el:
|
||||
tag = _local(child.tag)
|
||||
if tag == "GroupRange":
|
||||
sub = walk_range(child, depth + 1)
|
||||
rng["group_ranges"][f"{sub['name']}@{sub['address_start']}"] = sub
|
||||
elif tag == "GroupAddress":
|
||||
addr = add_ga(child)
|
||||
if addr:
|
||||
rng["group_addresses"].append(addr)
|
||||
return rng
|
||||
|
||||
def add_ga(el) -> Optional[str]:
|
||||
address = (el.get("Address") or "").strip()
|
||||
raw = _raw_address(address)
|
||||
if raw is None:
|
||||
warnings.append(f"skipped group address with invalid Address {address!r}")
|
||||
return None
|
||||
if address in gas:
|
||||
warnings.append(f"duplicate address {address} — kept the first entry")
|
||||
return None
|
||||
dpts = el.get("DPTs")
|
||||
dpt = parse_dpt(dpts)
|
||||
if dpts and dpt is None:
|
||||
warnings.append(f"{address}: DPT {dpts!r} not understood, left unset")
|
||||
gas[address] = {
|
||||
"name": el.get("Name", ""),
|
||||
"identifier": f"GA-{raw}",
|
||||
"raw_address": raw,
|
||||
"address": address,
|
||||
"project_uid": None,
|
||||
"dpt": dpt,
|
||||
"data_secure": (el.get("Security") or "").strip().lower() == "on",
|
||||
"communication_object_ids": [],
|
||||
"description": el.get("Description", "") or "",
|
||||
"comment": "",
|
||||
}
|
||||
return address
|
||||
|
||||
ranges: dict[str, Any] = {}
|
||||
for child in root:
|
||||
tag = _local(child.tag)
|
||||
if tag == "GroupRange":
|
||||
rng = walk_range(child)
|
||||
ranges[f"{rng['name']}@{rng['address_start']}"] = rng
|
||||
elif tag == "GroupAddress":
|
||||
add_ga(child)
|
||||
|
||||
info = {
|
||||
"name": name,
|
||||
"source": "ga-export",
|
||||
"group_address_style": _style(list(gas)),
|
||||
"tool_version": None,
|
||||
"import_warnings": warnings,
|
||||
}
|
||||
return {"info": info, "group_addresses": gas, "group_ranges": ranges, "devices": {},
|
||||
"communication_objects": {}, "functions": {}, "locations": {}, "topology": {}}
|
||||
|
||||
|
||||
def load_ga_export(path: str) -> LoadedProject:
|
||||
"""Load an ETS ga-export/01 XML file as a read-only project without devices."""
|
||||
if not os.path.isfile(path):
|
||||
raise GaExportError(f"file not found: {path}")
|
||||
size = os.path.getsize(path)
|
||||
if size > MAX_EXPORT_BYTES:
|
||||
raise GaExportError(f"file is {size} bytes, over the {MAX_EXPORT_BYTES}-byte limit")
|
||||
with open(path, "rb") as fh:
|
||||
data = fh.read(MAX_EXPORT_BYTES + 1)
|
||||
raw = read_ga_export_bytes(data, name=Path(path).stem)
|
||||
return build_loaded_from_raw(cast(Any, raw), path)
|
||||
@@ -164,6 +164,72 @@ def _has(name: str, words) -> bool:
|
||||
return any(w in low for w in words)
|
||||
|
||||
|
||||
# Setpoint shift (HA climate `setpoint_shift_address`, DPT 6.010 or 9.002). The DPT
|
||||
# alone is not enough — 9.002 is any temperature difference — so a shift needs the
|
||||
# word as well. German "Sollwertverschiebung", Russian "смещение/сдвиг уставки".
|
||||
_SHIFT_WORDS = ("shift", "verschieb", "смещ", "сдвиг")
|
||||
_SHIFT_MODE = {(9, 2): "DPT9002", (6, 10): "DPT6010"}
|
||||
|
||||
|
||||
def _is_shift(g: GARecord) -> bool:
|
||||
return (g.dpt_main, g.dpt_sub) in _SHIFT_MODE and _has(g.name, _SHIFT_WORDS)
|
||||
|
||||
|
||||
_NAME_TRIM = " \t-–—:;,./|_"
|
||||
_NAME_SPLIT = "/-_.,()[]:;|–—"
|
||||
|
||||
# Words that name what an address DOES, never which device it is. Only these may be
|
||||
# cut off the end of an entity name. Anything else — a room, "дверь", "А/С", "ТП",
|
||||
# a channel letter — is identity and stays.
|
||||
_NAME_FUNC_WORDS = frozenset(_FUNC_WORDS | _DIR_WORDS | {
|
||||
"operation", "mode", "hvac", "absolute", "relative", "open", "close", "switch",
|
||||
"switching", "b.value", "bewegen", "fahren", "wert", "helligkeit",
|
||||
"режим", "абсолютное", "относительное", "димм", "открыть", "закрыть",
|
||||
"управление", "команда",
|
||||
})
|
||||
|
||||
|
||||
def _name_words(text: str) -> list[str]:
|
||||
low = text.lower().replace("b.value", " value ")
|
||||
for ch in _NAME_SPLIT:
|
||||
low = low.replace(ch, " ")
|
||||
return [w for w in low.split() if w]
|
||||
|
||||
|
||||
def _entity_name(anchor: str, member_names: list[str]) -> str:
|
||||
"""Name a multi-GA entity after what its addresses share, not after one of them.
|
||||
|
||||
A light built from "Kitchen Spots On-Off", "Kitchen Spots B.Value" and their
|
||||
feedbacks should be "Kitchen Spots", not whichever address anchored it. Takes the
|
||||
longest common word prefix of the member names, and only accepts it when every
|
||||
word cut from the anchor is a function word (value, brightness, up/down, mode,
|
||||
Движение, Яркость…). If the names diverge earlier — a different room or device
|
||||
word, a channel number — the anchor name is kept, which is exactly the behaviour
|
||||
before this change. The candidate must still carry an identity token.
|
||||
"""
|
||||
names = [n for n in member_names if n and n.strip()]
|
||||
if len(names) < 2:
|
||||
return anchor
|
||||
norm = [[w.strip(_NAME_TRIM).lower() for w in n.split()] for n in names]
|
||||
common = 0
|
||||
for i in range(min(len(ws) for ws in norm)):
|
||||
if len({ws[i] for ws in norm}) != 1:
|
||||
break
|
||||
common = i + 1
|
||||
anchor_words = anchor.split()
|
||||
if common == 0 or common >= len(anchor_words):
|
||||
return anchor
|
||||
if [w.strip(_NAME_TRIM).lower() for w in anchor_words[:common]] != norm[0][:common]:
|
||||
return anchor
|
||||
dropped = _name_words(" ".join(anchor_words[common:]))
|
||||
if any(w not in _NAME_FUNC_WORDS for w in dropped):
|
||||
return anchor
|
||||
candidate = " ".join(anchor_words[:common]).strip(_NAME_TRIM)
|
||||
if not candidate or not _pair_ident(candidate):
|
||||
return anchor
|
||||
return candidate
|
||||
|
||||
|
||||
def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
|
||||
"""Return {'yaml': str, 'review': [...], 'counts': {...}}."""
|
||||
status_gas = [g for g in project.gas.values() if _is_status_ga(g)]
|
||||
@@ -454,6 +520,8 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
|
||||
ctrl_cmd = _pick(lambda g: g.dpt_main == 20 and g.dpt_sub == 105 and g.kind == "command")
|
||||
ctrl_state = _pick(lambda g: g.dpt_main == 20 and g.dpt_sub == 105 and _is_status_ga(g))
|
||||
valve = _pick(lambda g: g.dpt_main == 5 and _is_status_ga(g))
|
||||
shift_cmd = _pick(lambda g: _is_shift(g) and not _is_status_ga(g))
|
||||
shift_state = _pick(lambda g: _is_shift(g) and _is_status_ga(g))
|
||||
|
||||
if not (cur and tgt_state):
|
||||
review.append({"reason": "manual_climate", "address": ga.address,
|
||||
@@ -478,7 +546,15 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
|
||||
ent["controller_mode_state_address"] = ctrl_state.address
|
||||
if valve:
|
||||
ent["command_value_state_address"] = valve.address
|
||||
for m in (cur, tgt_state, tgt_cmd, op_cmd, op_state, ctrl_cmd, ctrl_state, valve):
|
||||
if shift_cmd:
|
||||
ent["setpoint_shift_address"] = shift_cmd.address
|
||||
if shift_state:
|
||||
ent["setpoint_shift_state_address"] = shift_state.address
|
||||
shift_ref = shift_cmd or shift_state
|
||||
if shift_ref:
|
||||
ent["setpoint_shift_mode"] = _SHIFT_MODE[(shift_ref.dpt_main, shift_ref.dpt_sub)]
|
||||
for m in (cur, tgt_state, tgt_cmd, op_cmd, op_state, ctrl_cmd, ctrl_state, valve,
|
||||
shift_cmd, shift_state):
|
||||
if m:
|
||||
consumed.add(m.address)
|
||||
climates.append(ent)
|
||||
@@ -491,11 +567,20 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
|
||||
issues.append("operation_mode has a command but no state address")
|
||||
if ctrl_cmd and not ctrl_state:
|
||||
issues.append("controller_mode has a command but no state address")
|
||||
if not tgt_cmd:
|
||||
issues.append("setpoint is read-only (no target_temperature command)")
|
||||
note = ("set `controller_modes`/`operation_modes` explicitly — HA auto-detection is "
|
||||
"often wrong; if this zone uses setpoint-shift, provide BOTH the command and "
|
||||
"state addresses and set `setpoint_shift_mode`")
|
||||
if not tgt_cmd and not shift_cmd:
|
||||
issues.append("setpoint is read-only (no target_temperature or setpoint shift command)")
|
||||
if shift_cmd and not shift_state:
|
||||
issues.append("setpoint shift has a command but no state address")
|
||||
if shift_state and not shift_cmd:
|
||||
issues.append("setpoint shift has a state but no command address")
|
||||
if shift_ref:
|
||||
note = ("set `controller_modes`/`operation_modes` explicitly — HA auto-detection is "
|
||||
f"often wrong; setpoint shift mapped as {ent['setpoint_shift_mode']} from the "
|
||||
"DPT, check it matches the thermostat's parameter")
|
||||
else:
|
||||
note = ("set `controller_modes`/`operation_modes` explicitly — HA auto-detection is "
|
||||
"often wrong; if this zone uses setpoint-shift, provide BOTH the command and "
|
||||
"state addresses and set `setpoint_shift_mode`")
|
||||
if issues:
|
||||
note += " — " + "; ".join(issues)
|
||||
review.append({"reason": "verify_climate", "address": ga.address,
|
||||
@@ -576,6 +661,27 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]:
|
||||
"entity to an Area in the HA UI); and entity `name`s drive voice/Assist "
|
||||
"matching — keep them descriptive and unique."})
|
||||
|
||||
# Multi-GA entities are named after what their addresses share (see _entity_name).
|
||||
# Two entities that would end up with the same name keep their anchor names, so
|
||||
# the rename never merges two things into one Home Assistant name.
|
||||
multi = [e for group in (lights, covers, climates) for e in group]
|
||||
proposed: dict[int, str] = {}
|
||||
for e in multi:
|
||||
members = [project.gas[v].name for k, v in e.items()
|
||||
if k.endswith("address") and isinstance(v, str) and v in project.gas]
|
||||
proposed[id(e)] = _entity_name(e["name"], members)
|
||||
taken: dict[str, int] = {}
|
||||
for e in multi:
|
||||
key = proposed[id(e)].lower()
|
||||
taken[key] = taken.get(key, 0) + 1
|
||||
for e in switches + sensors + binary_sensors:
|
||||
key = (e.get("name") or "").lower()
|
||||
taken[key] = taken.get(key, 0) + 1
|
||||
for e in multi:
|
||||
new = proposed[id(e)]
|
||||
if new != e["name"] and taken[new.lower()] == 1:
|
||||
e["name"] = new
|
||||
|
||||
knx: dict[str, Any] = {}
|
||||
if switches:
|
||||
knx["switch"] = switches
|
||||
|
||||
@@ -131,7 +131,7 @@ def suggest_repairs(project: LoadedProject) -> dict[str, Any]:
|
||||
new = _next_free(used, sga.main if sga.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}{_suffix(ga.name, ' (статус)', ' (status)')}", "dpt": sdpt,
|
||||
"name": f"{sga.name}{_suffix(sga.name, ' (статус)', ' (status)')}", "dpt": sdpt,
|
||||
"rationale": "status/feedback GA so Home Assistant reads real state",
|
||||
})
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@ from typing import Any, Optional
|
||||
from mcp.server.fastmcp import FastMCP
|
||||
|
||||
from .project import load_project as load_project_file, LoadedProject
|
||||
from .ga_export import load_ga_export as load_ga_export_file
|
||||
from .analyze import (validate_naming, detect_missing_status, detect_dpt_issues,
|
||||
detect_topology_issues, secure_posture)
|
||||
from .generate_ha import generate_ha_yaml
|
||||
@@ -126,6 +127,41 @@ def load_project(path: str, password: Optional[str] = None,
|
||||
}
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
def load_ga_export(path: str) -> dict[str, Any]:
|
||||
"""Load an ETS group-address export (ga-export/01 XML) instead of a full .knxproj.
|
||||
|
||||
For when you only have the GA list: an ETS "Export Group Addresses" file, or the
|
||||
ETS import file a planning tool produces (TapPlan and similar). Names, addresses,
|
||||
DPTs, descriptions, the security flag and the range tree are read; the result
|
||||
replaces the loaded project for every other tool.
|
||||
|
||||
Works: check_naming, check_missing_status, check_dpt, check_policy, check_secure,
|
||||
analyze_all, suggest_repairs, project_report, generate_ha_package,
|
||||
generate_ets_group_addresses. Nothing to read (the export has no devices, ETS
|
||||
Functions or topology): get_devices, get_topology, check_topology,
|
||||
decompose_device, check_device_parameters, parse_devices_from_project. Pairing
|
||||
relies on names only, since there are no ETS Function roles.
|
||||
|
||||
Args:
|
||||
path: Path to the exported .xml file.
|
||||
"""
|
||||
proj = load_ga_export_file(path)
|
||||
_STATE["project"] = proj
|
||||
with_dpt = sum(1 for g in proj.gas.values() if g.dpt_main is not None)
|
||||
return {
|
||||
"loaded": True,
|
||||
"source": "ga-export",
|
||||
"name": proj.info.get("name"),
|
||||
"ga_style": proj.style,
|
||||
"group_addresses": len(proj.gas),
|
||||
"with_dpt": with_dpt,
|
||||
"devices": 0,
|
||||
"import_warnings": proj.info.get("import_warnings", []),
|
||||
"note": ("GA-only project: device, function and topology tools have nothing to read; "
|
||||
"command/status pairing uses names only."),
|
||||
}
|
||||
|
||||
@mcp.tool()
|
||||
def list_group_addresses(category: Optional[str] = None,
|
||||
kind: Optional[str] = None,
|
||||
|
||||
Reference in New Issue
Block a user