diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c69019..39c2a74 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,9 @@ jobs: - name: GA-export intake, entity naming, climate setpoint shift run: python tests/test_ga_export.py + - name: Regressions from a real 1312-GA house (subDPT false positives, HA lights, self-reporting status) + run: python tests/test_real_house_fixes.py + - name: Repair proposals and council-review fixes run: python tests/test_council_fixes.py diff --git a/nickol_knx_mcp/analyze.py b/nickol_knx_mcp/analyze.py index 9879c55..576ee5f 100644 --- a/nickol_knx_mcp/analyze.py +++ b/nickol_knx_mcp/analyze.py @@ -211,6 +211,9 @@ def _is_safety_input(ga: GARecord) -> bool: # (tokens, main, sub, strong) — strong => also flag a wrong main. # --------------------------------------------------------------------------- # _SUBDPT_RULES: tuple = ( + # power factor before power: "Фактор мощности" contains "мощност" but is 14.057 + (("фактор мощност", "коэффициент мощност", "power factor", "leistungsfaktor", "cos φ", "cos phi"), + 14, 57, True), (("влажност", "humidity", "feucht"), 9, 7, True), (("co2", "со2", "углекисл", "kohlendioxid"), 9, 8, True), (("освещённост", "освещенност", "luminosity", "lux", "helligkeit ("), 9, 4, True), @@ -222,6 +225,24 @@ _SUBDPT_RULES: tuple = ( ) +# Other DPTs that legitimately carry the same quantity, so they are not "wrong". +# Checked against xknx 3.20 (DPTPower2Byte 9.024 kW, DPTApparentPower 14.080, +# DPTActiveEnergy 13.010 … DPTActiveEnergyMWh 13.016, DPTEnergy 14.031 J). +# Energy names in Russian ("электроэнергия - текущее потребление, W") are routinely +# used for power readings, so power DPTs are accepted under energy names too. +_SUBDPT_ALTERNATIVES: dict[tuple[int, int], frozenset] = { + (14, 56): frozenset({(9, 24), (14, 80)}), + (13, 13): frozenset({(13, 10), (13, 11), (13, 12), (13, 14), (13, 15), (13, 16), + (14, 31), (14, 56), (9, 24)}), +} + +# A quantity word in the name of a 1-bit, date/time or text GA names what the flag, +# threshold trigger or timestamp is ABOUT ("вкл по освещённости", "порог CO2", +# "запись значения, дата"), not the value itself. On seven real projects every such +# hit was a false positive (12 of 12 on 1-bit), so these mains are not checked. +_SUBDPT_EXEMPT_MAINS = frozenset({1, 10, 11, 16, 19}) + + def _expected_subdpt(name: str) -> Optional[tuple[int, int, bool]]: low = (name or "").lower() for tokens, m, s, strong in _SUBDPT_RULES: @@ -469,10 +490,14 @@ def detect_dpt_issues(project: LoadedProject) -> list[dict[str, Any]]: for addr, ga in project.gas.items(): if ga.intent != INTENT_FUNCTIONAL or ga.dpt_main is None: continue + if ga.dpt_main in _SUBDPT_EXEMPT_MAINS: + continue exp = _expected_subdpt(ga.name) if exp is None: continue em, es, strong = exp + if (ga.dpt_main, ga.dpt_sub) in _SUBDPT_ALTERNATIVES.get((em, es), frozenset()): + continue if ga.dpt_main == em and ga.dpt_sub != es: findings.append(_finding( SEVERITY_WARN, "subdpt_suspect", addr, diff --git a/tests/test_real_house_fixes.py b/tests/test_real_house_fixes.py new file mode 100644 index 0000000..598a6de --- /dev/null +++ b/tests/test_real_house_fixes.py @@ -0,0 +1,61 @@ +"""Regressions found on a real 1312-GA Zennio house (ETS 5.7), anonymised. + +A. subdpt_suspect false positives + - a 1-bit flag named after a quantity ("on by motion detector by illuminance", + "CO2 threshold 1") is a flag, not the value -> never checked; + - date/time and text GAs are never checked ("meter value recorded, date"); + - legitimate DPTs for the same quantity are not "wrong": power 9.024 kW, + power factor 14.057, reactive energy 13.015, power 14.056 under an + "electricity" name. (Checked against xknx 3.20.) + - real findings survive: a bare DPT 9 named "lux threshold", a 5.x named + "temperature control". +""" +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from nickol_knx_mcp.project import build_loaded_from_raw +from nickol_knx_mcp.analyze import detect_dpt_issues + + +def _ga(addr, name, main, sub): + return {"name": name, "identifier": f"GA-{addr}", "raw_address": 0, "address": addr, + "project_uid": None, "dpt": {"main": main, "sub": sub} if main else None, + "data_secure": False, "communication_object_ids": [], "description": "", "comment": ""} + + +def _project(rows): + return build_loaded_from_raw({"group_addresses": {a: _ga(a, n, m, s) for a, n, m, s in rows}, + "info": {"group_address_style": "ThreeLevel"}}, "mem") + + +# ---------------------------------------------------------------- A. subdpt_suspect +false_positives = [ + ("1/7/63", "1.04 - Winter garden - on by motion by освещенности", 1, 6), + ("1/7/70", "1.01 - Vestibule вкл по ДД по освещенности", 1, 2), + ("6/0/3", "Опорный сигнал освещенности для дневного откл ДД", 1, 1), + ("2/5/1", "02. Living - порог CO2 1", 1, 1), + ("5/2/1", "Электроэнергия - запись значения, дата", 11, 1), + ("5/2/8", "3 Фазы - Активная мощность, кВт", 9, 24), + ("5/2/9", "Boiler produced мощность", 9, 24), + ("5/2/10", "3 Фазы - Фактор мощности", 14, 57), + ("5/2/13", "3 Фазы - Индуктивная реактивная энергия", 13, 15), + ("5/2/20", "Электроэнергия - Текущее потребление, W", 14, 56), + ("5/2/21", "Meter energy MWh", 13, 16), +] +real_findings = [ + ("1/5/1", "01. Corridor PDK1.1 - порог освещенности, люкс", 9, None), + ("3/1/1", "ПВУ 1 - Контроль температуры", 5, None), + ("5/2/30", "Phase power L1", 5, 1), + ("5/2/31", "Room temperature", 9, 2), +] +p = _project(false_positives + real_findings) +flagged = {f["address"] for f in detect_dpt_issues(p) if f["code"] == "subdpt_suspect"} +wrong = [a for a, *_ in false_positives if a in flagged] +assert not wrong, f"false positives still flagged: {wrong}" +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") + +print("\nALL REAL-HOUSE REGRESSION TESTS PASSED") diff --git a/tools/corpus_baseline.json b/tools/corpus_baseline.json index 07d9436..122e875 100644 --- a/tools/corpus_baseline.json +++ b/tools/corpus_baseline.json @@ -246,12 +246,12 @@ "reserve_without_dpt": 23, "safety_input_no_status": 22, "status_pairing_summary": 1, - "subdpt_suspect": 7 + "subdpt_suspect": 2 }, "findings_by_severity": { "error": 6, "info": 49, - "warning": 45 + "warning": 40 }, "group_addresses": 685, "ha_entities": { @@ -311,12 +311,12 @@ "reserve_without_dpt": 25, "safety_input_no_status": 32, "status_pairing_summary": 1, - "subdpt_suspect": 17 + "subdpt_suspect": 2 }, "findings_by_severity": { "error": 9, "info": 60, - "warning": 85 + "warning": 70 }, "group_addresses": 1312, "ha_entities": { @@ -377,13 +377,12 @@ "safety_input_no_status": 47, "scene_no_status": 2, "status_pairing_summary": 1, - "subdpt_suspect": 1, "topology_segment_limit": 1 }, "findings_by_severity": { "error": 32, "info": 250, - "warning": 124 + "warning": 123 }, "group_addresses": 3646, "ha_entities": {