From 770a6126479efbf5a1530bedcca2551ba04dd3a0 Mon Sep 17 00:00:00 2001
From: Rob Young <rob@robyoung.digital>
Date: Thu, 19 May 2022 16:54:15 +0100
Subject: [PATCH] Merge device info

When getting connected devices merge the device info retrieved by each
method. When the router is running in AP mode the neighbour check does
not return IP addresses for the devices it returns while the wl and arp
checks do seem to. However, because the neighbour check happens last the
Device entries that it produces overwrite those produced for the same
MACs by the previous two checks.

This change makes the merging of those sets also attempt to merge the
fields of the Device. One risk here is that the new device and old
device both have valid but different values for a field. This is
considered an error so I just log and carry on. This has the effect of
making the values in the first device seen for a MAC take precedence
while previously it would have been the last device that would have. I
think this difference is a low risk as either way it's unexpected
behaviour if they don't match up.
---
 aioasuswrt/asuswrt.py | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/aioasuswrt/asuswrt.py b/aioasuswrt/asuswrt.py
index 20e239a..86b4c05 100644
--- a/aioasuswrt/asuswrt.py
+++ b/aioasuswrt/asuswrt.py
@@ -426,14 +426,14 @@ async def async_get_connected_devices(self, use_cache=True):
 
         devices = {}
         dev = await self.async_get_wl()
-        devices.update(dev)
+        merge_devices(devices, dev)
         dev = await self.async_get_arp()
-        devices.update(dev)
+        merge_devices(devices, dev)
         dev = await self.async_get_neigh(devices)
-        devices.update(dev)
+        merge_devices(devices, dev)
         if not self.mode == "ap":
             dev = await self.async_get_leases(devices)
-            devices.update(dev)
+            merge_devices(devices, dev)
 
         filter_devices = await self.async_filter_dev_list(devices)
         ret_devices = {
@@ -587,3 +587,31 @@ async def async_get_temperature(self):
     @property
     def is_connected(self):
         return self.connection.is_connected
+
+
+def merge_devices(devices, new_devices):
+    """Merge a new list of devices into an existing list
+
+    This merge fills in any null values in the base list if the device
+    in the new list has values for them."""
+    for mac, device in new_devices.items():
+        if mac not in devices:
+            devices[mac] = device
+        elif any(val is None for val in devices[mac]):
+            mismatches = [
+                f"{Device._fields[field]}({val1} != {val2})"
+                for field, (val1, val2) in enumerate(zip(devices[mac], device))
+                if val1 and val2 and val1 != val2
+            ]
+            if mismatches:
+                # if filled values do not match between devices from found from different sources
+                # then something is wrong. Log a warning and carry on.
+                _LOGGER.warning(
+                    "Mismatched values for device {}: {}".format(
+                        mac, ", ".join(mismatches)
+                    )
+                )
+            else:
+                devices[mac] = Device(
+                    *(val1 or val2 for val1, val2 in zip(devices[mac], device))
+                )