Fix thread commissioning fail where there are many Thread Border Rout…

…ers (#23492)

Matter Commissioner sometimes fails to commission Matter Thread Device
where there are many Thread Border Routers. Specifically, in order to
connect the Matter Thread Device to the Thread Border Router during the
commissioning process, the Matter Commissioner sends Thread ScanNetworks
command to the Matter Thread Device. However, no matter how many times
Matter Commissioner requests ScanNetworks, there is no Thread Border
Router it wants to connect to in the response, so the commissioning
has failed.

According to the message size requirements of the Matter specification
document, the maximum length of a Service Data Unit is 1024 bytes. Since
Thread ScanNetworks' responses have to be stored in it, network
commissioing cluster only stores 15 of them. Therefore, Thread Device
sends only 15 of the scan results in response to Thread ScanNetworks
command, and the scan result list contains duplicate items and is used
without sorting. There were about 50 items in scan result list on the
problem situation, and they were sorted in ascending order by channel.
And the channel of the Thread Border Router it wants to connect to is
the largest number, so it is not always included in the response of
Thread ScanNetworks command. Accordingly, the Matter Commissioner
determined that the Matter Device did not discover the Thread Border
Router it wants to connect to, and the commissioning failed.
This commit changes the Thread Scan result list in two ways. First, it
prevents the addition of duplicate items in the list. Next, sort the
list in ascending order based on RSSI. The reason for using RSSI is that
where there are many Thread Border Routers, it is reasonable to expect
that users will start commissioning close to the Thread Border Routers
they want Thread Devices to connect to.

Signed-off-by: Youngho Yoon <[email protected]>
Signed-off-by: Charles Kim <[email protected]>
Signed-off-by: Hunsup <[email protected]>
Signed-off-by: sanghyukko <[email protected]>
Signed-off-by: Jaehoon You <[email protected]>

src/app/clusters/network-commissioning/network-commissioning.cpp

@@ -26,6 +26,7 @@
 #include <app/server/Server.h>
 #include <app/util/attribute-storage.h>
 #include <lib/support/SafeInt.h>
+#include <lib/support/SortUtils.h>
 #include <lib/support/ThreadOperationalDataset.h>
 #include <platform/DeviceControlServer.h>
 #include <platform/PlatformManager.h>
@@ -528,7 +529,8 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI
     TLV::TLVWriter * writer;
     TLV::TLVType listContainerType;
     ThreadScanResponse scanResponse;
-    size_t networksEncoded = 0;
+    chip::Platform::ScopedMemoryBuffer<ThreadScanResponse> scanResponseArray;
+    size_t scanResponseArrayLength = 0;
     uint8_t extendedAddressBuffer[Thread::kSizeExtendedPanId];
 
     SuccessOrExit(err = commandHandle->PrepareCommand(
@@ -546,18 +548,61 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI
         err = writer->StartContainer(TLV::ContextTag(to_underlying(Commands::ScanNetworksResponse::Fields::kThreadScanResults)),
                                      TLV::TLVType::kTLVType_Array, listContainerType));
 
-    for (; networks != nullptr && networks->Next(scanResponse) && networksEncoded < kMaxNetworksInScanResponse; networksEncoded++)
+    VerifyOrExit(scanResponseArray.Alloc(chip::min(networks->Count(), kMaxNetworksInScanResponse)), err = CHIP_ERROR_NO_MEMORY);
+    for (; networks != nullptr && networks->Next(scanResponse);)
+    {
+        if ((scanResponseArrayLength == kMaxNetworksInScanResponse) &&
+            (scanResponseArray[scanResponseArrayLength - 1].rssi > scanResponse.rssi))
+        {
+            continue;
+        }
+
+        bool isDuplicated = false;
+
+        for (size_t i = 0; i < scanResponseArrayLength; i++)
+        {
+            if ((scanResponseArray[i].panId == scanResponse.panId) &&
+                (scanResponseArray[i].extendedPanId == scanResponse.extendedPanId))
+            {
+                if (scanResponseArray[i].rssi < scanResponse.rssi)
+                {
+                    scanResponseArray[i] = scanResponseArray[--scanResponseArrayLength];
+                }
+                else
+                {
+                    isDuplicated = true;
+                }
+                break;
+            }
+        }
+
+        if (isDuplicated)
+        {
+            continue;
+        }
+
+        if (scanResponseArrayLength < kMaxNetworksInScanResponse)
+        {
+            scanResponseArrayLength++;
+        }
+        scanResponseArray[scanResponseArrayLength - 1] = scanResponse;
+        Sorting::InsertionSort(scanResponseArray.Get(), scanResponseArrayLength,
+                               [](const ThreadScanResponse & a, const ThreadScanResponse & b) -> bool { return a.rssi > b.rssi; });
+    }
+
+    for (size_t i = 0; i < scanResponseArrayLength; i++)
     {
         Structs::ThreadInterfaceScanResult::Type result;
-        Encoding::BigEndian::Put64(extendedAddressBuffer, scanResponse.extendedAddress);
-        result.panId           = scanResponse.panId;
-        result.extendedPanId   = scanResponse.extendedPanId;
-        result.networkName     = CharSpan(scanResponse.networkName, scanResponse.networkNameLen);
-        result.channel         = scanResponse.channel;
-        result.version         = scanResponse.version;
+        Encoding::BigEndian::Put64(extendedAddressBuffer, scanResponseArray[i].extendedAddress);
+        result.panId           = scanResponseArray[i].panId;
+        result.extendedPanId   = scanResponseArray[i].extendedPanId;
+        result.networkName     = CharSpan(scanResponseArray[i].networkName, scanResponseArray[i].networkNameLen);
+        result.channel         = scanResponseArray[i].channel;
+        result.version         = scanResponseArray[i].version;
         result.extendedAddress = ByteSpan(extendedAddressBuffer);
-        result.rssi            = scanResponse.rssi;
-        result.lqi             = scanResponse.lqi;
+        result.rssi            = scanResponseArray[i].rssi;
+        result.lqi             = scanResponseArray[i].lqi;
+
         SuccessOrExit(err = DataModel::Encode(*writer, TLV::AnonymousTag(), result));
     }
 

src/lib/support/SortUtils.h

@@ -66,6 +66,38 @@ void BubbleSort(T * items, size_t n, CompareFunc f)
     }
 }
 
+/**
+ *
+ * Impements the insertion sort algorithm to sort an array
+ * of items of size 'n'.
+ *
+ * The provided comparison function SHALL have the following signature:
+ *
+ *      bool Compare(const T& a, const T& b);
+ *
+ * If a is deemed less than (i.e is ordered before) b, return true.
+ * Else, return false.
+ *
+ * This is a stable sort.
+ *
+ */
+template <typename T, typename CompareFunc>
+void InsertionSort(T * items, size_t n, CompareFunc f)
+{
+    for (size_t i = 1; i < n; i++)
+    {
+        const T key = items[i];
+        int j       = static_cast<int>(i) - 1;
+
+        while (j >= 0 && f(key, items[j]))
+        {
+            items[j + 1] = items[j];
+            j--;
+        }
+        items[j + 1] = key;
+    }
+}
+
 } // namespace Sorting
 
 } // namespace chip