Descalon/connections fix

digi/xbee/devices.py

@@ -11900,7 +11900,8 @@ def __process_discovered_neighbor_data(self, requester, routes, neighbor, nodes_
                 discover their neighbors are stored.
         """
         self._log.debug("     o Discovered neighbor of %s: %s (%s)",
-                        requester, neighbor.node, neighbor.relationship.name)
+                        requester, neighbor.node,
+                        neighbor.relationship.name if neighbor.relationship else None)
 
         # Requester node is clearly reachable
         self._set_node_reachable(requester, True)
@@ -11940,18 +11941,25 @@ def __process_discovered_neighbor_data(self, requester, routes, neighbor, nodes_
                                     lq_b2a=LinkQuality.UNKNOWN, status_a2b=route.status,
                                     status_b2a=RouteStatus.UNKNOWN)
             self._log.debug("       - Using route for the connection: %d", route.status.id)
-        elif (neighbor.node.get_role() != Role.UNKNOWN
-              and neighbor.relationship != NeighborRelationship.PREVIOUS_CHILD
-              and neighbor.relationship != NeighborRelationship.SIBLING):
+        elif neighbor.relationship not in (NeighborRelationship.PREVIOUS_CHILD, None):
+            # Build a connection from the neighbor entry even when no route exists.
+            # The XCTU Java implementation applies no role check here: connections
+            # are created for PARENT/CHILD/SIBLING/UNDETERMINED regardless of the
+            # neighbor node's cached role. This is important because ZDO neighbor
+            # entries sometimes report device_type=3 (UNKNOWN), which would leave
+            # the cached role as UNKNOWN even for a fully functioning router, and
+            # silently dropping the measured LQI causes the reverse direction of
+            # an existing connection to stay UNKNOWN (LQI -9999, status -1).
             self._log.debug(
                 "       - No route for this node, using relationship for the connection: %s",
-                neighbor.relationship.name)
+                neighbor.relationship.name if neighbor.relationship else None)
             if neighbor.relationship == NeighborRelationship.PARENT:
                 connection = Connection(node, requester, lq_a2b=neighbor.lq,
                                         lq_b2a=LinkQuality.UNKNOWN, status_a2b=RouteStatus.ACTIVE,
                                         status_b2a=RouteStatus.UNKNOWN)
             elif neighbor.relationship in (NeighborRelationship.CHILD,
-                                           NeighborRelationship.UNDETERMINED):
+                                           NeighborRelationship.UNDETERMINED,
+                                           NeighborRelationship.SIBLING):
                 connection = Connection(requester, node, lq_a2b=neighbor.lq,
                                         lq_b2a=LinkQuality.UNKNOWN, status_a2b=RouteStatus.ACTIVE,
                                         status_b2a=RouteStatus.UNKNOWN)
@@ -12036,6 +12044,33 @@ class DigiMeshNetwork(XBeeNetwork):
     local one and stores them.
     """
 
+    _REMOTE_NEIGHBOR_TIMEOUT_FACTOR = 2
+    """
+    Multiplier applied to the local node timeout ('N?') when waiting for a
+    *remote* node's 'FN' (find neighbors) answer.
+
+    'N?' is the time the local node needs for its own neighbor discovery. A
+    remote 'FN' takes longer: the request must travel to the remote node, the
+    remote runs its own neighbor scan (up to its own discovery time) before it
+    can answer, and the answer must travel back. Waiting only 'N?' would time
+    out an online but slow-to-answer remote and wrongly mark it non-reachable,
+    so remote requesters are given this many times the local timeout.
+
+    The value starts at 2, chosen as a safe upper limit. If we assume every node
+    is set up the same way ('NT'/'NN'/'NH'), as '_calculate_timeout' already
+    does, a remote 'FN' takes at most two things: the remote doing the same
+    neighbor scan the local node does (about 'N?'), plus the time for the request
+    to reach it and the answer to come back (also no more than about 'N?'). That
+    adds up to about 2 x 'N?', so 2 is a safe ceiling, not a tight figure.
+
+    We multiply 'N?' instead of adding a fixed amount on purpose: the radio works
+    out 'N?' from the maximum number of hops ('NH'), so multiplying it makes the
+    extra time grow on its own as the network gets deeper. The downside is that a
+    bigger factor makes discovery slower when a node is really gone (most of all
+    in CASCADE mode, where each node's wait time adds up). The exact number is a
+    best guess and can be tuned later.
+    """
+
     def __init__(self, device):
         """
         Class constructor. Instantiates a new `DigiMeshNetwork`.
@@ -12118,7 +12153,9 @@ def _prepare_network_discovery(self):
             if utils.is_bit_enabled(self.__saved_no[0], 2):
                 self.__saved_no = None
             else:
-                self.set_discovery_options({DiscoveryOptions.APPEND_RSSI})
+                no_value = bytearray(self.__saved_no)
+                no_value[0] = no_value[0] | 0x04
+                self._local_xbee.set_parameter(ATStringCommand.NO, no_value, apply=True)
 
             self._log.debug("[*] Preconfiguring %s", ATStringCommand.SO.command)
             self.__saved_so = self._local_xbee.get_parameter(
@@ -12185,8 +12222,18 @@ def modem_st_cb(modem_status):
             while not awake.wait(timeout=node_timeout):
                 pass
 
+        # 'self.__real_node_timeout' is based on the local node's 'N?' and only
+        # covers a local neighbor discovery. A remote 'FN' also needs time to
+        # reach the remote node, for the remote to run its own neighbor scan,
+        # and for the answer to travel back, so give remote requesters extra
+        # time to avoid timing out (and wrongly marking as non-reachable) a node
+        # that is online but slow to answer.
+        finder_timeout = self.__real_node_timeout
+        if requester.is_remote():
+            finder_timeout *= self._REMOTE_NEIGHBOR_TIMEOUT_FACTOR
+
         from digi.xbee.models.zdo import NeighborFinder
-        finder = NeighborFinder(requester, timeout=self.__real_node_timeout)
+        finder = NeighborFinder(requester, timeout=finder_timeout)
         finder.get_neighbors(neighbor_cb=__new_neighbor_cb,
                              finished_cb=__neighbor_discover_finished_cb)
 
@@ -12275,7 +12322,8 @@ def __process_discovered_neighbor_data(self, requester, neighbor, nodes_queue):
                 discover their neighbors are stored.
         """
         self._log.debug("     o Discovered neighbor of %s: %s (%s)",
-                        requester, neighbor.node, neighbor.relationship.name)
+                        requester, neighbor.node,
+                        neighbor.relationship.name if neighbor.relationship else None)
 
         # Requester node is clearly reachable
         self._set_node_reachable(requester, True)

digi/xbee/models/zdo.py

@@ -1239,7 +1239,8 @@ def _parse_data(self, data):
             # The coordinator is already received with its real 64-bit address
             if neighbor and neighbor.node.get_64bit_addr() != XBee64BitAddress.COORDINATOR_ADDRESS:
                 self._logger.debug("Neighbor of '%s': %s (relation: %s, depth: %s, lqi: %s)",
-                                   self._xbee, neighbor.node, neighbor.relationship.name,
+                                   self._xbee, neighbor.node,
+                                   neighbor.relationship.name if neighbor.relationship else None,
                                    neighbor.depth, neighbor.lq)
                 self.__neighbors.append(neighbor)
                 if self.__cb:
@@ -1770,4 +1771,9 @@ def __fn_packet_cb(self, frame):
                 self.stop()
                 return
 
+            # An empty response signals the end of the neighbor list.
+            if not frame.command_value:
+                self.stop()
+                return
+
             self.__parse_data(frame.command_value)