fix(zigbee): strict tree edges + auto-select imported nodes

Edges - Z2M `links` is bidirectional and includes router mesh paths, which caused duplicate edges and edges entering the coordinator from the bottom. Walk `links` only to derive parent_id + LQI; build final edges strictly from the parent->child hierarchy (one edge per non-coordinator node). Result: parent bottom -> child top, every time. - Tests: bidirectional pair collapses to one edge, router-mesh siblings dropped, coordinator never receives an edge. Auto-select - After import, deselect existing canvas nodes and mark only the freshly-imported ones as selected, so the user can drag the whole subtree as a group.
2026-05-06 23:25:52 +02:00
parent a46e505505
commit 05db9a59f5
3 changed files with 100 additions and 23 deletions
@@ -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)

@@ -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: