diff --git a/backend/app/services/zigbee_service.py b/backend/app/services/zigbee_service.py index 9ed48e0..a5fbd12 100644 --- a/backend/app/services/zigbee_service.py +++ b/backend/app/services/zigbee_service.py @@ -150,7 +150,13 @@ def parse_networkmap( if node["device_type"] == "Coordinator": coordinator_id = node["id"] - edges_list: list[dict[str, Any]] = [] + # Z2M `links` is bidirectional/mesh: every pair appears twice and routers + # carry sibling-mesh paths. Walk it only to extract LQI per device and to + # resolve which router an end device hangs off; do NOT emit edges directly + # from links. The final edge set is the strict parent→child tree built + # from parent_id below — that avoids duplicate edges and keeps the visual + # flow consistent (parent bottom → child top). + raw_edges: list[dict[str, Any]] = [] lqi_by_id: dict[str, int] = {} for link in raw_links: @@ -164,7 +170,7 @@ def parse_networkmap( continue if src not in seen_ids or tgt not in seen_ids: continue - edges_list.append({"source": src, "target": tgt}) + raw_edges.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 @@ -180,9 +186,16 @@ def parse_networkmap( if node["device_type"] == "Router": node["parent_id"] = coordinator_id elif node["device_type"] == "EndDevice": - parent = _find_parent_router(node["id"], router_ids, edges_list) + parent = _find_parent_router(node["id"], router_ids, raw_edges) node["parent_id"] = parent or coordinator_id + # Final edges = strict parent → child tree (one edge per non-coordinator) + edges_list: list[dict[str, Any]] = [ + {"source": node["parent_id"], "target": node["id"]} + for node in nodes_list + if node.get("parent_id") + ] + return nodes_list, edges_list @@ -227,7 +240,6 @@ async def fetch_networkmap( request_topic = _NETWORKMAP_REQUEST_TOPIC.format(base_topic=base_topic) response_topic = _NETWORKMAP_RESPONSE_TOPIC.format(base_topic=base_topic) - result_event: asyncio.Event = asyncio.Event() response_payload: dict[str, Any] = {} tls_context = _build_tls_context(tls_insecure) if tls else None @@ -242,6 +254,11 @@ async def fetch_networkmap( tls_context=tls_context, ) as client: await client.subscribe(response_topic) + # Give the broker a brief window to register the subscription + # before we publish the request. Without this, brokers that + # race SUBACK with our PUBLISH may deliver the response before + # the subscription is active and we'd hang until timeout. + await asyncio.sleep(0.1) await client.publish( request_topic, json.dumps({"type": "raw", "routes": False}), @@ -249,19 +266,19 @@ async def fetch_networkmap( async def _wait_for_response() -> None: async for message in client.messages: - if str(message.topic) == response_topic: - raw = message.payload - try: - payload_str = ( - raw.decode() if isinstance(raw, bytes | bytearray) else str(raw) - ) - response_payload.update(json.loads(payload_str)) - except (json.JSONDecodeError, TypeError) as exc: - raise ValueError( - f"Malformed networkmap response: {exc}" - ) from exc - result_event.set() - break + if str(message.topic) != response_topic: + continue + raw = message.payload + try: + payload_str = ( + raw.decode() if isinstance(raw, bytes | bytearray) else str(raw) + ) + response_payload.update(json.loads(payload_str)) + except (json.JSONDecodeError, TypeError) as exc: + raise ValueError( + f"Malformed networkmap response: {exc}" + ) from exc + return await asyncio.wait_for(_wait_for_response(), timeout=_NETWORKMAP_TIMEOUT) diff --git a/backend/tests/test_zigbee_service.py b/backend/tests/test_zigbee_service.py index 0a2720a..39ee260 100644 --- a/backend/tests/test_zigbee_service.py +++ b/backend/tests/test_zigbee_service.py @@ -218,6 +218,56 @@ class TestParseNetworkmap: _, edges = parse_networkmap(payload) assert edges == [] + def test_bidirectional_links_yield_single_edge(self) -> None: + """Z2M links are bidirectional — every pair appears twice. The output + must collapse to a single parent→child edge (no back-link, no dup).""" + coord = "0x0000" + router = "0x0001" + payload = _wrap( + nodes=[_make_node(coord, "Coordinator"), _make_node(router, "Router")], + links=[ + _make_link(coord, router), + _make_link(router, coord), # reverse direction + ], + ) + _, edges = parse_networkmap(payload) + assert edges == [{"source": coord, "target": router}] + + def test_router_mesh_siblings_dropped(self) -> None: + """Router↔router mesh paths in `links` must NOT produce sibling edges + in the final tree. Each router gets exactly one edge from coordinator.""" + coord = "0x0000" + r1 = "0x0001" + r2 = "0x0002" + payload = _wrap( + nodes=[ + _make_node(coord, "Coordinator"), + _make_node(r1, "Router"), + _make_node(r2, "Router"), + ], + links=[ + _make_link(coord, r1), + _make_link(coord, r2), + _make_link(r1, r2), # mesh sibling — must be dropped + _make_link(r2, r1), + ], + ) + _, edges = parse_networkmap(payload) + pairs = {(e["source"], e["target"]) for e in edges} + assert pairs == {(coord, r1), (coord, r2)} + + def test_coordinator_has_no_incoming_edge(self) -> None: + coord = "0x0000" + end = "0x0001" + payload = _wrap( + nodes=[_make_node(coord, "Coordinator"), _make_node(end, "EndDevice")], + links=[_make_link(end, coord)], # back-edge from end to coord + ) + _, edges = parse_networkmap(payload) + # No edge should target the coordinator + assert all(e["target"] != coord for e in edges) + assert edges == [{"source": coord, "target": end}] + # --------------------------------------------------------------------------- # _find_parent_router @@ -284,11 +334,15 @@ async def test_fetch_networkmap_success() -> None: class _FakeMessage: topic = "zigbee2mqtt/bridge/response/networkmap" payload = json.dumps(SAMPLE_RESPONSE_PAYLOAD).encode() + _yielded = False def __aiter__(self): return self async def __anext__(self): + if self._yielded: + raise StopAsyncIteration + self._yielded = True return self class _FakeClient: diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index db6253d..e998114 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -345,18 +345,24 @@ export default function App() { } addNode(newNode) }) - // Add IoT edges between Zigbee devices + // Add IoT edges between Zigbee devices: parent bottom -> child top zigbeeEdges.forEach((ze) => { - const sourceId = ze.source - const targetId = ze.target onConnect({ - source: sourceId, - sourceHandle: 'top', - target: targetId, + source: ze.source, + sourceHandle: 'bottom', + target: ze.target, targetHandle: 'top-t', type: 'iot', } as unknown as import('@xyflow/react').Connection) }) + // Auto-select only the freshly imported nodes so the user can drag the + // whole subtree as a group. + const importedIds = new Set(zigbeeNodes.map((zn) => zn.id)) + useCanvasStore.setState((state) => ({ + nodes: state.nodes.map((n) => ({ ...n, selected: importedIds.has(n.id) })), + selectedNodeIds: Array.from(importedIds), + selectedNodeId: importedIds.size === 1 ? Array.from(importedIds)[0] : null, + })) markUnsaved() }, [addNode, onConnect, snapshotHistory, markUnsaved])