From 0cc2ec0aa8b4be1edbdd62c1333828e2c281b02a Mon Sep 17 00:00:00 2001 From: Youngho Yoon <34558998+yhoyoon@users.noreply.github.com> Date: Fri, 18 Nov 2022 06:53:37 +0900 Subject: [PATCH] Fix thread commissioning fail where there are many Thread Border Routers (#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 Signed-off-by: Charles Kim Signed-off-by: Hunsup Signed-off-by: sanghyukko Signed-off-by: Jaehoon You --- .../network-commissioning.cpp | 65 ++++++++++++++++--- src/lib/support/SortUtils.h | 32 +++++++++ 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 13c890a61bd1bb..a63df5cb88321f 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -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 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)); } diff --git a/src/lib/support/SortUtils.h b/src/lib/support/SortUtils.h index 27ab1852d3da1a..9ebe3e368886ac 100644 --- a/src/lib/support/SortUtils.h +++ b/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 +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(i) - 1; + + while (j >= 0 && f(key, items[j])) + { + items[j + 1] = items[j]; + j--; + } + items[j + 1] = key; + } +} + } // namespace Sorting } // namespace chip