From 4f400707d658965d73164390dd0425312c41d44f Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 11:11:10 -0400 Subject: [PATCH 01/13] Add support for thread encoding to tlv --- .../network-commissioning.cpp | 268 ++++++++++++++---- 1 file changed, 214 insertions(+), 54 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index daa843588f5718..aa531d5e091ba0 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -115,6 +115,214 @@ BitFlags WiFiFeatures(WiFiDriver * driver) return features; } +/// Performs an auto-release of the given item, generally an `Iterator` type +/// like Wifi or Thread scan results. +template +class AutoRelease +{ +public: + AutoRelease(T * iterator) : mValue(iterator) {} + ~AutoRelease() + { + if (mValue != nullptr) + { + mValue->Release(); + } + } + +private: + T * mValue; +}; + +/// convenience macor to auto-create a variable for you to release the given name at +/// the exit of the current scope. +#define DEFER_AUTO_RELEASE(name) AutoRelease autoRelease##__COUNTER__(name) + +/// Handles encoding a WifiScanResponseIterator into a TLV response structure +class WifiScanResponseToTLV : public chip::app::DataModel::EncodableToTLV +{ +public: + WifiScanResponseToTLV(Status status, CharSpan debugText, WiFiScanResponseIterator * networks) : + mStatus(status), mDebugText(debugText), mNetworks(networks) + {} + + CHIP_ERROR EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag) const override; + +private: + Status mStatus; + CharSpan mDebugText; + WiFiScanResponseIterator * mNetworks; +}; + +CHIP_ERROR WifiScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag) const +{ + TLV::TLVType outerType; + ReturnErrorOnFailure(writer.StartContainer(tag, TLV::kTLVType_Structure, outerType)); + + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kNetworkingStatus), mStatus)); + if (mDebugText.size() != 0) + { + ReturnErrorOnFailure( + DataModel::Encode(writer, TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kDebugText), mDebugText)); + } + + { + TLV::TLVType listContainerType; + ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kWiFiScanResults), + TLV::kTLVType_Array, listContainerType)); + + if ((mStatus == Status::kSuccess) && (mNetworks != nullptr)) + { + WiFiScanResponse scanResponse; + size_t networksEncoded = 0; + while (mNetworks->Next(scanResponse)) + { + // NOTE: it is assumed that scanResponse.ssid/bssid lifetime is larger than just within the `next` call + Structs::WiFiInterfaceScanResultStruct::Type result; + result.security = scanResponse.security; + result.ssid = ByteSpan(scanResponse.ssid, scanResponse.ssidLen); + result.bssid = ByteSpan(scanResponse.bssid, sizeof(scanResponse.bssid)); + result.channel = scanResponse.channel; + result.wiFiBand = scanResponse.wiFiBand; + result.rssi = scanResponse.rssi; + ReturnErrorOnFailure(DataModel::Encode(writer, TLV::AnonymousTag(), result)); + + ++networksEncoded; + if (networksEncoded >= kMaxNetworksInScanResponse) + { + break; + } + } + } + + ReturnErrorOnFailure(writer.EndContainer(listContainerType)); + } + + return writer.EndContainer(outerType); +} + +class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV +{ +public: + ThreadScanResponseToTLV(Status status, CharSpan debugText, ThreadScanResponseIterator * networks) : + mStatus(status), mDebugText(debugText), mNetworks(networks) + {} + + CHIP_ERROR EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag) const override; + +private: + Status mStatus; + CharSpan mDebugText; + ThreadScanResponseIterator * mNetworks; + + /// parses mNetworks into `buffer` and performs post-processing (sort and dedup) + /// on them. + /// + /// Returns the valid list of scan responses into `validResponses` + CHIP_ERROR LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, + Span validResponses) const; +}; + +CHIP_ERROR ThreadScanResponseToTLV::LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, + Span validResponses) const +{ + VerifyOrReturnError(scanResponseArray.Alloc(chip::min(mNetworks->Count(), kMaxNetworksInScanResponse)), CHIP_ERROR_NO_MEMORY); + + ThreadScanResponse scanResponse; + size_t scanResponseArrayLength = 0; + for (; mNetworks != nullptr && mNetworks->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; }); + + validResponses = Span(scanResponseArray.Get(), scanResponseArrayLength); + + return CHIP_NO_ERROR; +} + +CHIP_ERROR ThreadScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag) const +{ + Platform::ScopedMemoryBuffer responseArray; + Span responseSpan; + + ReturnErrorOnFailure(LoadResponses(responseArray, responseSpan)); + + TLV::TLVType outerType; + ReturnErrorOnFailure(writer.StartContainer(tag, TLV::kTLVType_Structure, outerType)); + + ReturnErrorOnFailure(writer.Put(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kNetworkingStatus), mStatus)); + if (mDebugText.size() != 0) + { + ReturnErrorOnFailure( + DataModel::Encode(writer, TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kDebugText), mDebugText)); + } + + { + TLV::TLVType listContainerType; + ReturnErrorOnFailure(writer.StartContainer(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kThreadScanResults), + TLV::kTLVType_Array, listContainerType)); + + for (const ThreadScanResponse & response : responseSpan) + { + Structs::ThreadInterfaceScanResultStruct::Type result; + uint8_t extendedAddressBuffer[Thread::kSizeExtendedPanId]; + + Encoding::BigEndian::Put64(extendedAddressBuffer, response.extendedAddress); + result.panId = response.panId; + result.extendedPanId = response.extendedPanId; + result.networkName = CharSpan(response.networkName, response.networkNameLen); + result.channel = response.channel; + result.version = response.version; + result.extendedAddress = ByteSpan(extendedAddressBuffer); + result.rssi = response.rssi; + result.lqi = response.lqi; + + ReturnErrorOnFailure(DataModel::Encode(writer, TLV::AnonymousTag(), result)); + } + + ReturnErrorOnFailure(writer.EndContainer(listContainerType)); + } + + return writer.EndContainer(outerType); +} + } // namespace Instance::Instance(EndpointId aEndpointId, WiFiDriver * apDelegate) : @@ -986,6 +1194,8 @@ void Instance::OnResult(Status commissioningError, CharSpan debugText, int32_t i void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseIterator * networks) { + DEFER_AUTO_RELEASE(networks); + #if CHIP_DEVICE_CONFIG_ENABLE_THREAD CHIP_ERROR err = CHIP_NO_ERROR; auto commandHandleRef = std::move(mAsyncCommandHandle); @@ -1096,14 +1306,14 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI { CommitSavedBreadcrumb(); } - networks->Release(); #endif } void Instance::OnFinished(Status status, CharSpan debugText, WiFiScanResponseIterator * networks) { + DEFER_AUTO_RELEASE(networks); + #if CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION || CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP - CHIP_ERROR err = CHIP_NO_ERROR; auto commandHandleRef = std::move(mAsyncCommandHandle); auto commandHandle = commandHandleRef.Get(); if (commandHandle == nullptr) @@ -1122,63 +1332,13 @@ void Instance::OnFinished(Status status, CharSpan debugText, WiFiScanResponseIte SetLastNetworkingStatusValue(MakeNullable(status)); - TLV::TLVWriter * writer; - TLV::TLVType listContainerType; - WiFiScanResponse scanResponse; - size_t networksEncoded = 0; - - const CommandHandler::InvokeResponseParameters prepareParams(mPath); - SuccessOrExit( - err = commandHandle->PrepareInvokeResponseCommand( - ConcreteCommandPath(mPath.mEndpointId, NetworkCommissioning::Id, Commands::ScanNetworksResponse::Id), prepareParams)); - VerifyOrExit((writer = commandHandle->GetCommandDataIBTLVWriter()) != nullptr, err = CHIP_ERROR_INCORRECT_STATE); - - SuccessOrExit(err = writer->Put(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kNetworkingStatus), status)); - if (debugText.size() != 0) - { - SuccessOrExit( - err = DataModel::Encode(*writer, TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kDebugText), debugText)); - } - SuccessOrExit(err = writer->StartContainer(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kWiFiScanResults), - TLV::TLVType::kTLVType_Array, listContainerType)); - - // Only encode results on success, to avoid stale contents on partial failure. - if ((status == Status::kSuccess) && (networks != nullptr)) - { - while (networks->Next(scanResponse)) - { - Structs::WiFiInterfaceScanResultStruct::Type result; - result.security = scanResponse.security; - result.ssid = ByteSpan(scanResponse.ssid, scanResponse.ssidLen); - result.bssid = ByteSpan(scanResponse.bssid, sizeof(scanResponse.bssid)); - result.channel = scanResponse.channel; - result.wiFiBand = scanResponse.wiFiBand; - result.rssi = scanResponse.rssi; - SuccessOrExit(err = DataModel::Encode(*writer, TLV::AnonymousTag(), result)); - - ++networksEncoded; - if (networksEncoded >= kMaxNetworksInScanResponse) - { - break; - } - } - } + WifiScanResponseToTLV responseBuilder(status, debugText, networks); + commandHandle->AddResponse(mPath, Commands::ScanNetworksResponse::Id, responseBuilder); - SuccessOrExit(err = writer->EndContainer(listContainerType)); - SuccessOrExit(err = commandHandle->FinishCommand()); -exit: - if (err != CHIP_NO_ERROR) - { - ChipLogError(Zcl, "Failed to encode response: %" CHIP_ERROR_FORMAT, err.Format()); - } if (status == Status::kSuccess) { CommitSavedBreadcrumb(); } - if (networks != nullptr) - { - networks->Release(); - } #endif } From 206d6d3ae4fdc0a6cbb3af01359d812ebcdc0b68 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 11:15:47 -0400 Subject: [PATCH 02/13] Preserve awkward sort inside a loop --- .../network-commissioning/network-commissioning.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index aa531d5e091ba0..9379722137bf56 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -215,8 +215,8 @@ class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV CharSpan mDebugText; ThreadScanResponseIterator * mNetworks; - /// parses mNetworks into `buffer` and performs post-processing (sort and dedup) - /// on them. + /// Fills up scanResponseArray with valid and de-duplicated thread responses from mNetworks. + /// Handles sorting and keeping only largers rssi /// /// Returns the valid list of scan responses into `validResponses` CHIP_ERROR LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, @@ -267,11 +267,10 @@ CHIP_ERROR ThreadScanResponseToTLV::LoadResponses(Platform::ScopedMemoryBuffer bool { return a.rssi > b.rssi; }); } - Sorting::InsertionSort(scanResponseArray.Get(), scanResponseArrayLength, - [](const ThreadScanResponse & a, const ThreadScanResponse & b) -> bool { return a.rssi > b.rssi; }); - validResponses = Span(scanResponseArray.Get(), scanResponseArrayLength); return CHIP_NO_ERROR; From 1bfde1501c7edc6e76635abea83407e2f70afbbd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 11:22:55 -0400 Subject: [PATCH 03/13] Add a TODO comment ... insertion sort in a loop is awkward --- .../clusters/network-commissioning/network-commissioning.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 9379722137bf56..2fa4b98d22255d 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -267,6 +267,9 @@ CHIP_ERROR ThreadScanResponseToTLV::LoadResponses(Platform::ScopedMemoryBuffer bool { return a.rssi > b.rssi; }); } From fa2f0518a8b05101fb325592051b3fbb785f4776 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 11:26:47 -0400 Subject: [PATCH 04/13] Make use of the new class --- .../network-commissioning.cpp | 95 +------------------ 1 file changed, 2 insertions(+), 93 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 2fa4b98d22255d..0650e8425e84c3 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -1199,7 +1199,6 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI DEFER_AUTO_RELEASE(networks); #if CHIP_DEVICE_CONFIG_ENABLE_THREAD - CHIP_ERROR err = CHIP_NO_ERROR; auto commandHandleRef = std::move(mAsyncCommandHandle); auto commandHandle = commandHandleRef.Get(); if (commandHandle == nullptr) @@ -1211,99 +1210,9 @@ void Instance::OnFinished(Status status, CharSpan debugText, ThreadScanResponseI SetLastNetworkingStatusValue(MakeNullable(status)); - TLV::TLVWriter * writer; - TLV::TLVType listContainerType; - ThreadScanResponse scanResponse; - Platform::ScopedMemoryBuffer scanResponseArray; - size_t scanResponseArrayLength = 0; - uint8_t extendedAddressBuffer[Thread::kSizeExtendedPanId]; - - const CommandHandler::InvokeResponseParameters prepareParams(mPath); - SuccessOrExit( - err = commandHandle->PrepareInvokeResponseCommand( - ConcreteCommandPath(mPath.mEndpointId, NetworkCommissioning::Id, Commands::ScanNetworksResponse::Id), prepareParams)); - VerifyOrExit((writer = commandHandle->GetCommandDataIBTLVWriter()) != nullptr, err = CHIP_ERROR_INCORRECT_STATE); - - SuccessOrExit(err = writer->Put(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kNetworkingStatus), status)); - if (debugText.size() != 0) - { - SuccessOrExit( - err = DataModel::Encode(*writer, TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kDebugText), debugText)); - } - SuccessOrExit(err = writer->StartContainer(TLV::ContextTag(Commands::ScanNetworksResponse::Fields::kThreadScanResults), - TLV::TLVType::kTLVType_Array, listContainerType)); - - // If no network was found, we encode an empty list, don't call a zero-sized alloc. - if ((status == Status::kSuccess) && (networks->Count() > 0)) - { - 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::ThreadInterfaceScanResultStruct::Type result; - 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 = scanResponseArray[i].rssi; - result.lqi = scanResponseArray[i].lqi; - - SuccessOrExit(err = DataModel::Encode(*writer, TLV::AnonymousTag(), result)); - } - } - - SuccessOrExit(err = writer->EndContainer(listContainerType)); - SuccessOrExit(err = commandHandle->FinishCommand()); + ThreadScanResponseToTLV responseBuilder(status, debugText, networks); + commandHandle->AddResponse(mPath, Commands::ScanNetworksResponse::Id, responseBuilder); -exit: - if (err != CHIP_NO_ERROR) - { - ChipLogError(Zcl, "Failed to encode response: %" CHIP_ERROR_FORMAT, err.Format()); - } if (status == Status::kSuccess) { CommitSavedBreadcrumb(); From 986a179fd475a8bfed537e1c62c85878e2bfacdd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 12:59:18 -0400 Subject: [PATCH 05/13] Add ifdefs --- .../network-commissioning/network-commissioning.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 0650e8425e84c3..c99b4ef4f05916 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -138,6 +138,8 @@ class AutoRelease /// the exit of the current scope. #define DEFER_AUTO_RELEASE(name) AutoRelease autoRelease##__COUNTER__(name) +#if CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION || CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP || CHIP_DEVICE_CONFIG_ENABLE_THREAD + /// Handles encoding a WifiScanResponseIterator into a TLV response structure class WifiScanResponseToTLV : public chip::app::DataModel::EncodableToTLV { @@ -201,6 +203,10 @@ CHIP_ERROR WifiScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag return writer.EndContainer(outerType); } +#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION || CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP || CHIP_DEVICE_CONFIG_ENABLE_THREAD + +#if CHIP_DEVICE_CONFIG_ENABLE_THREAD + class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV { public: @@ -325,6 +331,8 @@ CHIP_ERROR ThreadScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag t return writer.EndContainer(outerType); } +#endif // CHIP_DEVICE_CONFIG_ENABLE_THREAD + } // namespace Instance::Instance(EndpointId aEndpointId, WiFiDriver * apDelegate) : From 7497a5bc3f5a97dbf43d98622ee5069d291d3a77 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 15:32:21 -0400 Subject: [PATCH 06/13] Fix out parameter to be a reference --- .../clusters/network-commissioning/network-commissioning.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index c99b4ef4f05916..e5e8fb6ceb6ab6 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -226,11 +226,11 @@ class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV /// /// Returns the valid list of scan responses into `validResponses` CHIP_ERROR LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, - Span validResponses) const; + Span &validResponses) const; }; CHIP_ERROR ThreadScanResponseToTLV::LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, - Span validResponses) const + Span &validResponses) const { VerifyOrReturnError(scanResponseArray.Alloc(chip::min(mNetworks->Count(), kMaxNetworksInScanResponse)), CHIP_ERROR_NO_MEMORY); From 3a9940218d1dfc1d878822960c89f7024e35e95d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 15:35:00 -0400 Subject: [PATCH 07/13] Restyle --- .../clusters/network-commissioning/network-commissioning.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index e5e8fb6ceb6ab6..2b20b4638d5c2f 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -226,11 +226,11 @@ class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV /// /// Returns the valid list of scan responses into `validResponses` CHIP_ERROR LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, - Span &validResponses) const; + Span & validResponses) const; }; CHIP_ERROR ThreadScanResponseToTLV::LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, - Span &validResponses) const + Span & validResponses) const { VerifyOrReturnError(scanResponseArray.Alloc(chip::min(mNetworks->Count(), kMaxNetworksInScanResponse)), CHIP_ERROR_NO_MEMORY); From 58423495a6e9039dd8c7e85ec6f7f4f817899102 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 15:57:32 -0400 Subject: [PATCH 08/13] Comment typo fix and add one more comment --- .../clusters/network-commissioning/network-commissioning.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 2b20b4638d5c2f..a9c3cfe8867266 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -134,7 +134,7 @@ class AutoRelease T * mValue; }; -/// convenience macor to auto-create a variable for you to release the given name at +/// Convenience macro to auto-create a variable for you to release the given name at /// the exit of the current scope. #define DEFER_AUTO_RELEASE(name) AutoRelease autoRelease##__COUNTER__(name) @@ -207,6 +207,7 @@ CHIP_ERROR WifiScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag #if CHIP_DEVICE_CONFIG_ENABLE_THREAD +/// Handles encoding a ThreadScanResponseIterator into a TLV response structure. class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV { public: From 86b6afbc867a142e0f8318d99f1904512f8e894d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 16:28:14 -0400 Subject: [PATCH 09/13] Remove comment: we encode right away, so no lifetime issue --- src/app/clusters/network-commissioning/network-commissioning.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index a9c3cfe8867266..f1e4a2b2f1bf40 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -179,7 +179,6 @@ CHIP_ERROR WifiScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag size_t networksEncoded = 0; while (mNetworks->Next(scanResponse)) { - // NOTE: it is assumed that scanResponse.ssid/bssid lifetime is larger than just within the `next` call Structs::WiFiInterfaceScanResultStruct::Type result; result.security = scanResponse.security; result.ssid = ByteSpan(scanResponse.ssid, scanResponse.ssidLen); From 24138d720d12c0943645b19695f610c6ad5aa05e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 16:41:41 -0400 Subject: [PATCH 10/13] Update src/app/clusters/network-commissioning/network-commissioning.cpp Co-authored-by: Boris Zbarsky --- .../clusters/network-commissioning/network-commissioning.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index f1e4a2b2f1bf40..4509d2e10814c5 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -222,7 +222,7 @@ class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV ThreadScanResponseIterator * mNetworks; /// Fills up scanResponseArray with valid and de-duplicated thread responses from mNetworks. - /// Handles sorting and keeping only largers rssi + /// Handles sorting and keeping only larger rssi /// /// Returns the valid list of scan responses into `validResponses` CHIP_ERROR LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, From 6427df84ffe9b5c1130b115a1f3a614340d6b175 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 16:41:50 -0400 Subject: [PATCH 11/13] Update src/app/clusters/network-commissioning/network-commissioning.cpp Co-authored-by: Boris Zbarsky --- .../clusters/network-commissioning/network-commissioning.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 4509d2e10814c5..ff1aecfededf26 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -224,7 +224,8 @@ class ThreadScanResponseToTLV : public chip::app::DataModel::EncodableToTLV /// Fills up scanResponseArray with valid and de-duplicated thread responses from mNetworks. /// Handles sorting and keeping only larger rssi /// - /// Returns the valid list of scan responses into `validResponses` + /// Returns the valid list of scan responses into `validResponses`, which is only valid + /// as long as scanResponseArray is valid. CHIP_ERROR LoadResponses(Platform::ScopedMemoryBuffer & scanResponseArray, Span & validResponses) const; }; From 3c5c31ba3a6f6a83d3e9c32803465dbc76b7e3f8 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 16:43:41 -0400 Subject: [PATCH 12/13] Fix ifdef --- .../clusters/network-commissioning/network-commissioning.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index ff1aecfededf26..1cb473d7d9c338 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -138,7 +138,7 @@ class AutoRelease /// the exit of the current scope. #define DEFER_AUTO_RELEASE(name) AutoRelease autoRelease##__COUNTER__(name) -#if CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION || CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP || CHIP_DEVICE_CONFIG_ENABLE_THREAD +#if CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION || CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP /// Handles encoding a WifiScanResponseIterator into a TLV response structure class WifiScanResponseToTLV : public chip::app::DataModel::EncodableToTLV @@ -202,7 +202,7 @@ CHIP_ERROR WifiScanResponseToTLV::EncodeTo(TLV::TLVWriter & writer, TLV::Tag tag return writer.EndContainer(outerType); } -#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION || CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP || CHIP_DEVICE_CONFIG_ENABLE_THREAD +#endif // CHIP_DEVICE_CONFIG_ENABLE_WIFI_STATION || CHIP_DEVICE_CONFIG_ENABLE_WIFI_AP #if CHIP_DEVICE_CONFIG_ENABLE_THREAD From 3b1d668975cbc2a0901a283df4e65fe06608996a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 31 May 2024 16:44:24 -0400 Subject: [PATCH 13/13] Comment update --- .../clusters/network-commissioning/network-commissioning.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp index 1cb473d7d9c338..f2e74033d8266c 100644 --- a/src/app/clusters/network-commissioning/network-commissioning.cpp +++ b/src/app/clusters/network-commissioning/network-commissioning.cpp @@ -275,8 +275,8 @@ CHIP_ERROR ThreadScanResponseToTLV::LoadResponses(Platform::ScopedMemoryBuffer bool { return a.rssi > b.rssi; }); }