From b89d83eb08ed414aaecfe88eebdec459f786d8d2 Mon Sep 17 00:00:00 2001 From: Youngho Yoon <34558998+yhoyoon@users.noreply.github.com> Date: Fri, 4 Nov 2022 16:05:43 +0900 Subject: [PATCH] Fix thread commissioing fail where many thread BRs 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 | 54 +++++++++++++++---- 1 file changed, 44 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..02ee76c05fae25 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -31,6 +31,7 @@ #include #include #include +#include using namespace chip; using namespace chip::app; @@ -528,7 +529,7 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI TLV::TLVWriter * writer; TLV::TLVType listContainerType; ThreadScanResponse scanResponse; - size_t networksEncoded = 0; + std::vector scanResponses; uint8_t extendedAddressBuffer[Thread::kSizeExtendedPanId]; SuccessOrExit(err = commandHandle->PrepareCommand( @@ -546,18 +547,51 @@ 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++) + for (; networks != nullptr && networks->Next(scanResponse);) + { + bool isDuplicated = false; + + for (size_t i = 0; i < scanResponses.size(); i++) + { + if ((scanResponses[i].panId == scanResponse.panId) && (scanResponses[i].extendedPanId == scanResponse.extendedPanId)) + { + isDuplicated = true; + if (scanResponses[i].rssi < scanResponse.rssi) + { + scanResponses[i].rssi = scanResponse.rssi; + } + break; + } + } + + if (isDuplicated) + { + continue; + } + + scanResponses.push_back(scanResponse); + + if (scanResponses.size() > kMaxNetworksInScanResponse) + { + std::sort(scanResponses.begin(), scanResponses.end(), + [](const ThreadScanResponse & a, const ThreadScanResponse & b) -> bool { return a.rssi > b.rssi; }); + scanResponses.pop_back(); + } + } + + for (size_t i = 0; i < scanResponses.size(); 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, scanResponses[i].extendedAddress); + result.panId = scanResponses[i].panId; + result.extendedPanId = scanResponses[i].extendedPanId; + result.networkName = CharSpan(scanResponses[i].networkName, scanResponses[i].networkNameLen); + result.channel = scanResponses[i].channel; + result.version = scanResponses[i].version; result.extendedAddress = ByteSpan(extendedAddressBuffer); - result.rssi = scanResponse.rssi; - result.lqi = scanResponse.lqi; + result.rssi = scanResponses[i].rssi; + result.lqi = scanResponses[i].lqi; + SuccessOrExit(err = DataModel::Encode(*writer, TLV::AnonymousTag(), result)); }