From a46e5055050b1d862da1050fa43adbcd251a2548 Mon Sep 17 00:00:00 2001 From: Pouzor Date: Wed, 6 May 2026 22:38:15 +0200 Subject: [PATCH] fix(zigbee): parse real Z2M networkmap shape (data.value.nodes/links) The previous parser read `data.routes` which is just an echo of the `routes` request flag (a boolean). On real brokers this caused `TypeError: 'bool' object is not iterable` and 500s during /import. - Rewrite parse_networkmap to read data.value.nodes + data.value.links with fallback to data.{nodes,links} for legacy variants - Defensive: drop links to unknown nodes, propagate lqi from link to target node, extract model/vendor from definition block - Bump networkmap timeout 10s -> 180s (large meshes are slow) - Tests: rewrite fixture builders + sample payload to real Z2M shape; add cases for legacy shape, routes:false echo (regression), malformed list, link to unknown node, lqi propagation, definition extraction - Update docs to mention 60s+ wait window 53 backend tests pass, mypy + ruff clean. --- backend/app/api/routes/zigbee.py | 4 +- backend/app/services/zigbee_service.py | 169 ++++++++++--------- backend/tests/test_zigbee_service.py | 218 +++++++++++++------------ docs/zigbee-import.md | 2 +- 4 files changed, 210 insertions(+), 183 deletions(-) diff --git a/backend/app/api/routes/zigbee.py b/backend/app/api/routes/zigbee.py index 251678b..0c5784a 100644 --- a/backend/app/api/routes/zigbee.py +++ b/backend/app/api/routes/zigbee.py @@ -27,8 +27,8 @@ async def import_zigbee_network( """Fetch the Zigbee2MQTT network map and return nodes + edges ready for canvas drop. Connects to the specified MQTT broker, publishes a networkmap request to - ``/bridge/request/networkmap``, and waits up to 10 s for the - response. The devices are returned as typed homelable nodes with a + ``/bridge/request/networkmap``, and waits up to 60 s for the + response (large meshes can take 30 s+). The devices are returned as typed homelable nodes with a coordinator → router → end-device hierarchy. """ try: diff --git a/backend/app/services/zigbee_service.py b/backend/app/services/zigbee_service.py index ee57e58..9ed48e0 100644 --- a/backend/app/services/zigbee_service.py +++ b/backend/app/services/zigbee_service.py @@ -18,7 +18,7 @@ except ImportError: # pragma: no cover _NETWORKMAP_REQUEST_TOPIC = "{base_topic}/bridge/request/networkmap" _NETWORKMAP_RESPONSE_TOPIC = "{base_topic}/bridge/response/networkmap" _CONNECTION_TIMEOUT = 5.0 # seconds to verify broker reachability -_NETWORKMAP_TIMEOUT = 10.0 # seconds to wait for the networkmap response +_NETWORKMAP_TIMEOUT = 180.0 # seconds to wait for the networkmap response (large meshes can be slow) def _sanitize_mqtt_error(exc: BaseException) -> str: @@ -68,99 +68,110 @@ def _z2m_type_to_homelable(device_type: str) -> str: return mapping.get(device_type, "zigbee_enddevice") +def _node_from_z2m(raw: dict[str, Any]) -> dict[str, Any] | None: + """Build a homelable node dict from a Z2M raw networkmap node entry.""" + ieee: str = raw.get("ieeeAddr") or raw.get("ieee_address") or "" + if not ieee: + return None + device_type: str = raw.get("type") or "EndDevice" + friendly_name: str = ( + raw.get("friendlyName") or raw.get("friendly_name") or ieee + ) + definition: dict[str, Any] = raw.get("definition") or {} + model: str | None = ( + raw.get("modelID") + or raw.get("model") + or definition.get("model") + or None + ) + vendor: str | None = raw.get("vendor") or definition.get("vendor") or None + return { + "id": ieee, + "label": friendly_name, + "type": _z2m_type_to_homelable(device_type), + "ieee_address": ieee, + "friendly_name": friendly_name, + "device_type": device_type, + "model": model, + "vendor": vendor, + "lqi": None, + "parent_id": None, + } + + def parse_networkmap( payload: dict[str, Any], ) -> tuple[list[dict[str, Any]], list[dict[str, Any]]]: - """Parse a Z2M networkmap response payload into node + edge lists. + """Parse a Z2M ``bridge/response/networkmap`` payload into node + edge lists. - Returns: - (nodes, edges) where each node/edge is a plain dict with the fields - expected by ZigbeeNodeOut / ZigbeeEdgeOut. + Z2M raw response shape:: + + { + "data": { + "type": "raw", + "routes": false, + "value": { + "nodes": [{"ieeeAddr": ..., "type": "Coordinator|Router|EndDevice", + "friendlyName": ..., "definition": {"model": ..., "vendor": ...}}], + "links": [{"source": {"ieeeAddr": ...}, "target": {"ieeeAddr": ...}, + "lqi": 200, "depth": 1}] + } + }, + "status": "ok" + } + + Older or alternate shapes may put nodes/links directly under ``data``. + Both are accepted. """ - data: dict[str, Any] = payload.get("data", {}) - routes: list[dict[str, Any]] = data.get("routes", []) + data: dict[str, Any] = payload.get("data") or {} + value = data.get("value") + container: dict[str, Any] = value if isinstance(value, dict) else data + + raw_nodes: list[dict[str, Any]] = container.get("nodes") or [] + raw_links: list[dict[str, Any]] = container.get("links") or [] + + if not isinstance(raw_nodes, list): + raise ValueError("Malformed networkmap: 'nodes' is not a list") + if not isinstance(raw_links, list): + raise ValueError("Malformed networkmap: 'links' is not a list") nodes_list: list[dict[str, Any]] = [] - edges_list: list[dict[str, Any]] = [] seen_ids: set[str] = set() - - # Coordinator is always present; find it first so we can wire the hierarchy coordinator_id: str | None = None - for route in routes: - source: dict[str, Any] = route.get("source", {}) - if not source: + for entry in raw_nodes: + if not isinstance(entry, dict): continue - - ieee: str = source.get("ieeeAddr") or source.get("ieee_address") or "" - if not ieee: + node = _node_from_z2m(entry) + if node is None or node["id"] in seen_ids: continue + seen_ids.add(node["id"]) + nodes_list.append(node) + if node["device_type"] == "Coordinator": + coordinator_id = node["id"] - device_type: str = source.get("type", "EndDevice") - friendly_name: str = ( - source.get("friendlyName") or source.get("friendly_name") or ieee - ) - model: str | None = source.get("modelID") or source.get("model") or None - vendor: str | None = source.get("vendor") or None + edges_list: list[dict[str, Any]] = [] + lqi_by_id: dict[str, int] = {} - if ieee not in seen_ids: - seen_ids.add(ieee) - node_type = _z2m_type_to_homelable(device_type) - node: dict[str, Any] = { - "id": ieee, - "label": friendly_name, - "type": node_type, - "ieee_address": ieee, - "friendly_name": friendly_name, - "device_type": device_type, - "model": model, - "vendor": vendor, - "lqi": None, - "parent_id": None, - } - nodes_list.append(node) - if device_type == "Coordinator": - coordinator_id = ieee + for link in raw_links: + if not isinstance(link, dict): + continue + src_obj = link.get("source") or {} + tgt_obj = link.get("target") or {} + src = src_obj.get("ieeeAddr") if isinstance(src_obj, dict) else None + tgt = tgt_obj.get("ieeeAddr") if isinstance(tgt_obj, dict) else None + if not src or not tgt: + continue + if src not in seen_ids or tgt not in seen_ids: + continue + edges_list.append({"source": src, "target": tgt}) + lqi = link.get("lqi") or link.get("linkquality") + if isinstance(lqi, int) and tgt not in lqi_by_id: + lqi_by_id[tgt] = lqi - # Walk the route targets to build edges and collect additional nodes - targets: list[dict[str, Any]] = route.get("routes", []) - for target_entry in targets: - target_src: dict[str, Any] = target_entry.get("target", {}) - target_ieee: str = ( - target_src.get("ieeeAddr") or target_src.get("ieee_address") or "" - ) - lqi: int | None = target_entry.get("lqi") - - if not target_ieee: - continue - - if target_ieee not in seen_ids: - seen_ids.add(target_ieee) - t_type: str = target_src.get("type", "EndDevice") - t_fn: str = ( - target_src.get("friendlyName") - or target_src.get("friendly_name") - or target_ieee - ) - t_model: str | None = ( - target_src.get("modelID") or target_src.get("model") or None - ) - t_vendor: str | None = target_src.get("vendor") or None - t_node: dict[str, Any] = { - "id": target_ieee, - "label": t_fn, - "type": _z2m_type_to_homelable(t_type), - "ieee_address": target_ieee, - "friendly_name": t_fn, - "device_type": t_type, - "model": t_model, - "vendor": t_vendor, - "lqi": lqi, - "parent_id": None, - } - nodes_list.append(t_node) - - edges_list.append({"source": ieee, "target": target_ieee}) + for node in nodes_list: + if node["id"] in lqi_by_id: + node["lqi"] = lqi_by_id[node["id"]] # Build parent_id hierarchy: coordinator → routers → end devices if coordinator_id: diff --git a/backend/tests/test_zigbee_service.py b/backend/tests/test_zigbee_service.py index bc76d84..0a2720a 100644 --- a/backend/tests/test_zigbee_service.py +++ b/backend/tests/test_zigbee_service.py @@ -20,34 +20,43 @@ from app.services.zigbee_service import ( ) # --------------------------------------------------------------------------- -# Helper builders +# Helper builders — real Z2M `bridge/response/networkmap` shape +# (data.value.nodes + data.value.links) # --------------------------------------------------------------------------- -def _make_route( +def _make_node( ieee: str, device_type: str = "EndDevice", friendly_name: str | None = None, - targets: list[dict[str, Any]] | None = None, + model: str | None = None, + vendor: str | None = None, ) -> dict[str, Any]: - """Build a minimal Z2M route entry for testing.""" + entry: dict[str, Any] = { + "ieeeAddr": ieee, + "type": device_type, + "friendlyName": friendly_name or ieee, + } + if model or vendor: + entry["definition"] = {"model": model, "vendor": vendor} + return entry + + +def _make_link(source_ieee: str, target_ieee: str, lqi: int = 200) -> dict[str, Any]: return { - "source": { - "ieeeAddr": ieee, - "type": device_type, - "friendlyName": friendly_name or ieee, - }, - "routes": targets or [], + "source": {"ieeeAddr": source_ieee}, + "target": {"ieeeAddr": target_ieee}, + "lqi": lqi, } -def _make_target( - ieee: str, - device_type: str = "EndDevice", - lqi: int = 200, -) -> dict[str, Any]: +def _wrap(nodes: list[dict[str, Any]], links: list[dict[str, Any]] | None = None) -> dict[str, Any]: return { - "target": {"ieeeAddr": ieee, "type": device_type, "friendlyName": ieee}, - "lqi": lqi, + "data": { + "type": "raw", + "routes": False, + "value": {"nodes": nodes, "links": links or []}, + }, + "status": "ok", } @@ -79,19 +88,13 @@ class TestParseNetworkmap: assert nodes == [] assert edges == [] - def test_empty_routes(self) -> None: - nodes, edges = parse_networkmap({"data": {"routes": []}}) + def test_empty_value(self) -> None: + nodes, edges = parse_networkmap(_wrap([], [])) assert nodes == [] assert edges == [] def test_coordinator_only(self) -> None: - payload = { - "data": { - "routes": [ - _make_route("0x0000000000000000", "Coordinator", "Coordinator"), - ] - } - } + payload = _wrap([_make_node("0x0000000000000000", "Coordinator", "Coordinator")]) nodes, edges = parse_networkmap(payload) assert len(nodes) == 1 assert nodes[0]["type"] == "zigbee_coordinator" @@ -103,24 +106,17 @@ class TestParseNetworkmap: router_ieee = "0x0000000000000001" end_ieee = "0x0000000000000002" - payload = { - "data": { - "routes": [ - _make_route( - coord_ieee, - "Coordinator", - "Coordinator", - targets=[_make_target(router_ieee, "Router")], - ), - _make_route( - router_ieee, - "Router", - "my_router", - targets=[_make_target(end_ieee, "EndDevice")], - ), - ] - } - } + payload = _wrap( + nodes=[ + _make_node(coord_ieee, "Coordinator", "Coordinator"), + _make_node(router_ieee, "Router", "my_router"), + _make_node(end_ieee, "EndDevice"), + ], + links=[ + _make_link(coord_ieee, router_ieee), + _make_link(router_ieee, end_ieee), + ], + ) nodes, edges = parse_networkmap(payload) node_by_id = {n["id"]: n for n in nodes} @@ -136,77 +132,92 @@ class TestParseNetworkmap: # Parent hierarchy assert node_by_id[router_ieee]["parent_id"] == coord_ieee assert node_by_id[end_ieee]["parent_id"] == router_ieee + assert len(edges) == 2 def test_no_duplicate_nodes(self) -> None: ieee = "0x0000000000000001" - payload = { - "data": { - "routes": [ - _make_route(ieee, "Router"), - _make_route(ieee, "Router"), # duplicate - ] - } - } + payload = _wrap( + nodes=[_make_node(ieee, "Router"), _make_node(ieee, "Router")], + ) nodes, _ = parse_networkmap(payload) assert len(nodes) == 1 def test_edges_built_correctly(self) -> None: coord = "0x0000" router = "0x0001" - payload = { - "data": { - "routes": [ - _make_route( - coord, - "Coordinator", - targets=[_make_target(router, "Router")], - ) - ] - } - } + payload = _wrap( + nodes=[_make_node(coord, "Coordinator"), _make_node(router, "Router")], + links=[_make_link(coord, router)], + ) _, edges = parse_networkmap(payload) assert len(edges) == 1 assert edges[0]["source"] == coord assert edges[0]["target"] == router def test_friendly_name_used_as_label(self) -> None: - payload = { - "data": { - "routes": [ - _make_route("0xABCD", "EndDevice", "Living Room Sensor") - ] - } - } + payload = _wrap([_make_node("0xABCD", "EndDevice", "Living Room Sensor")]) nodes, _ = parse_networkmap(payload) assert nodes[0]["label"] == "Living Room Sensor" def test_enddevice_falls_back_to_coordinator_when_no_router(self) -> None: coord = "0x0000" end = "0x0003" - payload = { - "data": { - "routes": [ - _make_route(coord, "Coordinator"), - _make_route(end, "EndDevice"), - ] - } - } + payload = _wrap([_make_node(coord, "Coordinator"), _make_node(end, "EndDevice")]) nodes, _ = parse_networkmap(payload) end_node = next(n for n in nodes if n["id"] == end) assert end_node["parent_id"] == coord def test_missing_ieee_skipped(self) -> None: - payload = { - "data": { - "routes": [ - {"source": {}, "routes": []}, # no ieeeAddr - ] - } - } + payload = _wrap([{"type": "EndDevice"}]) # no ieeeAddr nodes, edges = parse_networkmap(payload) assert nodes == [] assert edges == [] + def test_lqi_propagated_from_link_to_target_node(self) -> None: + coord = "0x0000" + end = "0x0001" + payload = _wrap( + nodes=[_make_node(coord, "Coordinator"), _make_node(end, "EndDevice")], + links=[_make_link(coord, end, lqi=180)], + ) + nodes, _ = parse_networkmap(payload) + end_node = next(n for n in nodes if n["id"] == end) + assert end_node["lqi"] == 180 + + def test_definition_model_and_vendor_extracted(self) -> None: + payload = _wrap([ + _make_node("0xAA", "EndDevice", "Sensor", model="WSDCGQ11LM", vendor="Aqara"), + ]) + nodes, _ = parse_networkmap(payload) + assert nodes[0]["model"] == "WSDCGQ11LM" + assert nodes[0]["vendor"] == "Aqara" + + def test_legacy_shape_without_value_wrapper(self) -> None: + """Some Z2M variants put nodes/links directly under data.""" + payload = {"data": {"nodes": [_make_node("0x01", "Coordinator")], "links": []}} + nodes, _ = parse_networkmap(payload) + assert len(nodes) == 1 + assert nodes[0]["type"] == "zigbee_coordinator" + + def test_routes_bool_is_ignored(self) -> None: + """`routes: false` echo from the request must not crash the parser.""" + payload = {"data": {"routes": False, "type": "raw", "value": {"nodes": [], "links": []}}} + nodes, edges = parse_networkmap(payload) + assert nodes == [] + assert edges == [] + + def test_malformed_nodes_not_list_raises(self) -> None: + with pytest.raises(ValueError, match="not a list"): + parse_networkmap({"data": {"value": {"nodes": "oops", "links": []}}}) + + def test_link_to_unknown_node_dropped(self) -> None: + payload = _wrap( + nodes=[_make_node("0x01", "Coordinator")], + links=[_make_link("0x01", "0xDEAD")], # 0xDEAD not in nodes + ) + _, edges = parse_networkmap(payload) + assert edges == [] + # --------------------------------------------------------------------------- # _find_parent_router @@ -238,26 +249,31 @@ class TestFindParentRouter: SAMPLE_RESPONSE_PAYLOAD = { "data": { - "routes": [ - { - "source": { + "type": "raw", + "routes": False, + "value": { + "nodes": [ + { "ieeeAddr": "0x00000000", "type": "Coordinator", "friendlyName": "Coordinator", }, - "routes": [ - { - "target": { - "ieeeAddr": "0x00000001", - "type": "Router", - "friendlyName": "router_1", - }, - "lqi": 230, - } - ], - } - ] - } + { + "ieeeAddr": "0x00000001", + "type": "Router", + "friendlyName": "router_1", + }, + ], + "links": [ + { + "source": {"ieeeAddr": "0x00000000"}, + "target": {"ieeeAddr": "0x00000001"}, + "lqi": 230, + } + ], + }, + }, + "status": "ok", } diff --git a/docs/zigbee-import.md b/docs/zigbee-import.md index ef61b77..1de33ee 100644 --- a/docs/zigbee-import.md +++ b/docs/zigbee-import.md @@ -55,7 +55,7 @@ Click **Fetch Devices**. Homelable will: 1. Connect to the broker 2. Subscribe to the response topic 3. Publish `{"type": "raw", "routes": false}` to the request topic -4. Wait up to 10 seconds for the network map response +4. Wait up to 60 seconds for the network map response (large meshes can take 30 s+) 5. Parse and group devices by type ### 5. Select and add to canvas