From cd8138f9eb598c7c8938d5b9dca5822f5e9941f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Thu, 18 Jan 2024 20:59:36 +0100 Subject: [PATCH 01/13] [1.1] Cherry pick Thread DNS client and memory leak fixes (#31457) * [app] Fix DeferredAttributePersister memory leak (#31075) * [app] Fix DeferredAttributePerister memory leak ScopedMemoryBuffer's Release() method was used instead of Free(). Add CHECK_RETURN_VALUE annotation to the Release() method to prevent from making such a mistake in the future. Signed-off-by: Damian Krolik * Code review --------- Signed-off-by: Damian Krolik (cherry picked from commit 3e8aeeb7d6cb58811b7ec5be57f576aef7855e13) * [OpenThread] Harden DNS record parsing (#31227) OpenThread applications would crash upon receiving an empty DNS TXT record. The reason was that the code for copying OT DNS service info object into Matter DnssdService object would not initialize the TXT entry count in the latter object in such a case. In the reported case, the Matter stack was presented an empty TXT record because OpenThread's DNS client received a TXT record with TTL 0 and it discarded its contents. Nevertheless, the issue could be reproduced by publishing Matter service without TXT entries and kicking off DNS query. 1. Initialize the TXT entry and subtype count properly in all scenarios. 2. Do not even process the service info object if an error was returned by OpenThread before. 3. Extract some boilerplate to a separate function to improve readability. Signed-off-by: Damian Krolik (cherry picked from commit 76b6bb5bcc13c4de6aa67885f6c2328fc8b5998b) --- .../DeferredAttributePersistenceProvider.cpp | 2 +- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 4 +- src/lib/support/ScopedBuffer.h | 30 +++++--- ...nericThreadStackManagerImpl_OpenThread.hpp | 72 +++++++++---------- 4 files changed, 59 insertions(+), 49 deletions(-) diff --git a/src/app/DeferredAttributePersistenceProvider.cpp b/src/app/DeferredAttributePersistenceProvider.cpp index 62d5c446fe2091..82652792806d5e 100644 --- a/src/app/DeferredAttributePersistenceProvider.cpp +++ b/src/app/DeferredAttributePersistenceProvider.cpp @@ -39,7 +39,7 @@ void DeferredAttribute::Flush(AttributePersistenceProvider & persister) { VerifyOrReturn(IsArmed()); persister.WriteValue(mPath, ByteSpan(mValue.Get(), mValue.AllocatedSize())); - mValue.Release(); + mValue.Free(); } CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index d0ef32ec1b0bf9..e38d5e3dacfe2b 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -1141,8 +1141,8 @@ - (void)readAttributePaths:(NSArray * _Nullable)attri // callback->AdoptReadClient(std::move(readClient)); callback.release(); - attributePathParamsList.Release(); - eventPathParamsList.Release(); + IgnoreUnusedVariable(attributePathParamsList.Release()); + IgnoreUnusedVariable(eventPathParamsList.Release()); return err; }); std::move(*bridge).DispatchAction(self); diff --git a/src/lib/support/ScopedBuffer.h b/src/lib/support/ScopedBuffer.h index 258b7f262935fb..7a3c0aaca5902c 100644 --- a/src/lib/support/ScopedBuffer.h +++ b/src/lib/support/ScopedBuffer.h @@ -25,6 +25,7 @@ #pragma once #include +#include #include #include @@ -84,10 +85,11 @@ class ScopedMemoryBufferBase const void * Ptr() const { return mBuffer; } /** - * Releases the undelying buffer. Buffer stops being managed and will not be - * auto-freed. + * Releases the underlying buffer. + * + * The buffer stops being managed and will not be auto-freed. */ - void * Release() + CHECK_RETURN_VALUE void * Release() { void * buffer = mBuffer; mBuffer = nullptr; @@ -139,13 +141,18 @@ class ScopedMemoryBuffer : public Impl::ScopedMemoryBufferBase static_assert(std::is_trivially_destructible::value, "Destructors won't get run"); - inline T * Get() { return static_cast(Base::Ptr()); } - inline T & operator[](size_t index) { return Get()[index]; } + T * Get() { return static_cast(Base::Ptr()); } + T & operator[](size_t index) { return Get()[index]; } - inline const T * Get() const { return static_cast(Base::Ptr()); } - inline const T & operator[](size_t index) const { return Get()[index]; } + const T * Get() const { return static_cast(Base::Ptr()); } + const T & operator[](size_t index) const { return Get()[index]; } - inline T * Release() { return static_cast(Base::Release()); } + /** + * Releases the underlying buffer. + * + * The buffer stops being managed and will not be auto-freed. + */ + CHECK_RETURN_VALUE T * Release() { return static_cast(Base::Release()); } ScopedMemoryBuffer & Calloc(size_t elementCount) { @@ -222,7 +229,12 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer ScopedMemoryBuffer::Free(); } - T * Release() + /** + * Releases the underlying buffer. + * + * The buffer stops being managed and will not be auto-freed. + */ + CHECK_RETURN_VALUE T * Release() { T * buffer = ScopedMemoryBuffer::Release(); mCount = 0; diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp index a75f791966ac41..21bb87668cefb2 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp @@ -94,6 +94,29 @@ void initNetworkCommissioningThreadDriver(void) #endif } +#if CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT +CHIP_ERROR ReadDomainNameComponent(const char *& in, char * out, size_t outSize) +{ + const char * dotPos = strchr(in, '.'); + VerifyOrReturnError(dotPos != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + + const size_t componentSize = static_cast(dotPos - in); + VerifyOrReturnError(componentSize < outSize, CHIP_ERROR_INVALID_ARGUMENT); + + memcpy(out, in, componentSize); + out[componentSize] = '\0'; + in += componentSize + 1; + + return CHIP_NO_ERROR; +} + +template +CHIP_ERROR ReadDomainNameComponent(const char *& in, char (&out)[N]) +{ + return ReadDomainNameComponent(in, out, N); +} +#endif + NetworkCommissioning::otScanResponseIterator mScanResponseIter; } // namespace @@ -2317,29 +2340,8 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::FromOtDnsRespons { char protocol[chip::Dnssd::kDnssdProtocolTextMaxSize + 1]; - if (strchr(serviceType, '.') == nullptr) - return CHIP_ERROR_INVALID_ARGUMENT; - - // Extract from the ... the part. - size_t substringSize = strchr(serviceType, '.') - serviceType; - if (substringSize >= ArraySize(mdnsService.mType)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - Platform::CopyString(mdnsService.mType, substringSize + 1, serviceType); - - // Extract from the ... the part. - const char * protocolSubstringStart = serviceType + substringSize + 1; - - if (strchr(protocolSubstringStart, '.') == nullptr) - return CHIP_ERROR_INVALID_ARGUMENT; - - substringSize = strchr(protocolSubstringStart, '.') - protocolSubstringStart; - if (substringSize >= ArraySize(protocol)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - Platform::CopyString(protocol, substringSize + 1, protocolSubstringStart); + ReturnErrorOnFailure(ReadDomainNameComponent(serviceType, mdnsService.mType)); + ReturnErrorOnFailure(ReadDomainNameComponent(serviceType, protocol)); if (strncmp(protocol, "_udp", chip::Dnssd::kDnssdProtocolTextMaxSize) == 0) { @@ -2354,24 +2356,20 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::FromOtDnsRespons mdnsService.mProtocol = chip::Dnssd::DnssdServiceProtocol::kDnssdProtocolUnknown; } + mdnsService.mInterface = Inet::InterfaceId::Null(); + mdnsService.mSubTypeSize = 0; + mdnsService.mTextEntrySize = 0; + // Check if SRV record was included in DNS response. - if (error != OT_ERROR_NOT_FOUND) + // If not, return partial information about the service and exit early. + if (error != OT_ERROR_NONE) { - if (strchr(serviceInfo.mHostNameBuffer, '.') == nullptr) - return CHIP_ERROR_INVALID_ARGUMENT; - - // Extract from the .. the part. - substringSize = strchr(serviceInfo.mHostNameBuffer, '.') - serviceInfo.mHostNameBuffer; - if (substringSize >= ArraySize(mdnsService.mHostName)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - Platform::CopyString(mdnsService.mHostName, substringSize + 1, serviceInfo.mHostNameBuffer); - - mdnsService.mPort = serviceInfo.mPort; + return CHIP_NO_ERROR; } - mdnsService.mInterface = Inet::InterfaceId::Null(); + const char * host = serviceInfo.mHostNameBuffer; + ReturnErrorOnFailure(ReadDomainNameComponent(host, mdnsService.mHostName)); + mdnsService.mPort = serviceInfo.mPort; // Check if AAAA record was included in DNS response. From 28383526eedf5826f6a7f0ec96b396212b1ddcd5 Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Fri, 2 Feb 2024 22:27:33 +0800 Subject: [PATCH 02/13] ESP32: Add EndpointQueueFilter for ESP32 platform (#31440) * Add EndpointQueueFilter for ESP32 platform * Restyled by clang-format * Restyled by gn * fix compile error when disabling inet ipv4 * review changes * Restyled by clang-format * review changes * review changes --------- Co-authored-by: Restyled.io --- config/esp32/components/chip/Kconfig | 7 + src/platform/ESP32/BUILD.gn | 3 + .../ESP32/ConnectivityManagerImpl_WiFi.cpp | 30 ++++ src/platform/ESP32/ESP32EndpointQueueFilter.h | 129 ++++++++++++++++++ 4 files changed, 169 insertions(+) create mode 100644 src/platform/ESP32/ESP32EndpointQueueFilter.h diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index 00bfed5df9b124..7f9e56652b020b 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -186,6 +186,13 @@ menu "CHIP Core" help Enable this option to use LwIP default IPv6 route hook for Route Information Option(RIO) feature. + config ENABLE_ENDPOINT_QUEUE_FILTER + bool "Enable UDP Endpoint queue filter for mDNS Broadcast packets" + depends on USE_MINIMAL_MDNS + default y + help + Enable this option to start a UDP Endpoint queue filter for mDNS Broadcast packets + config ENABLE_LWIP_THREAD_SAFETY bool "Enable LwIP Thread safety options" default y diff --git a/src/platform/ESP32/BUILD.gn b/src/platform/ESP32/BUILD.gn index 0a9c9b7dbaef20..496b4c6f922ec3 100644 --- a/src/platform/ESP32/BUILD.gn +++ b/src/platform/ESP32/BUILD.gn @@ -128,6 +128,9 @@ static_library("ESP32") { "WiFiDnssdImpl.h", ] } + if (chip_mdns == "minimal") { + sources += [ "ESP32EndpointQueueFilter.h" ] + } if (chip_enable_route_hook) { sources += [ "route_hook/ESP32RouteHook.c", diff --git a/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp b/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp index e52319b55bb365..5fefcb5490f059 100644 --- a/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp +++ b/src/platform/ESP32/ConnectivityManagerImpl_WiFi.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -1107,6 +1108,35 @@ void ConnectivityManagerImpl::OnStationIPv6AddressAvailable(const ip_event_got_i event.Type = DeviceEventType::kInterfaceIpAddressChanged; event.InterfaceIpAddressChanged.Type = InterfaceIpChangeType::kIpV6_Assigned; PlatformMgr().PostEventOrDie(&event); + +#if CONFIG_ENABLE_ENDPOINT_QUEUE_FILTER + uint8_t station_mac[6]; + if (esp_wifi_get_mac(WIFI_IF_STA, station_mac) == ESP_OK) + { + static chip::Inet::ESP32EndpointQueueFilter sEndpointQueueFilter; + char station_mac_str[12]; + for (size_t i = 0; i < 6; ++i) + { + uint8_t dig1 = (station_mac[i] & 0xF0) >> 4; + uint8_t dig2 = station_mac[i] & 0x0F; + station_mac_str[2 * i] = static_cast(dig1 > 9 ? ('A' + dig1 - 0xA) : ('0' + dig1)); + station_mac_str[2 * i + 1] = static_cast(dig2 > 9 ? ('A' + dig2 - 0xA) : ('0' + dig2)); + } + if (sEndpointQueueFilter.SetMdnsHostName(chip::CharSpan(station_mac_str)) == CHIP_NO_ERROR) + { + chip::Inet::UDPEndPointImpl::SetQueueFilter(&sEndpointQueueFilter); + } + else + { + ChipLogError(DeviceLayer, "Failed to set mDNS hostname for endpoint queue filter"); + } + } + else + { + ChipLogError(DeviceLayer, "Failed to get the MAC address of station netif"); + } +#endif // CONFIG_ENABLE_ENDPOINT_QUEUE_FILTER + #if CONFIG_ENABLE_ROUTE_HOOK esp_route_hook_init(esp_netif_get_handle_from_ifkey(ESP32Utils::kDefaultWiFiStationNetifKey)); #endif diff --git a/src/platform/ESP32/ESP32EndpointQueueFilter.h b/src/platform/ESP32/ESP32EndpointQueueFilter.h new file mode 100644 index 00000000000000..031ea29b64d798 --- /dev/null +++ b/src/platform/ESP32/ESP32EndpointQueueFilter.h @@ -0,0 +1,129 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +namespace chip { +namespace Inet { + +class ESP32EndpointQueueFilter : public EndpointQueueFilter +{ +public: + CHIP_ERROR SetMdnsHostName(const chip::CharSpan & hostName) + { + ReturnErrorCodeIf(hostName.size() != sizeof(mHostNameBuffer), CHIP_ERROR_INVALID_ARGUMENT); + ReturnErrorCodeIf(!IsValidMdnsHostName(hostName), CHIP_ERROR_INVALID_ARGUMENT); + memcpy(mHostNameBuffer, hostName.data(), hostName.size()); + return CHIP_NO_ERROR; + } + + FilterOutcome FilterBeforeEnqueue(const void * endpoint, const IPPacketInfo & pktInfo, + const chip::System::PacketBufferHandle & pktPayload) override + { + if (!IsMdnsBroadcastPacket(pktInfo)) + { + return FilterOutcome::kAllowPacket; + } + // Drop the mDNS packets which don't contain 'matter' or ''. + const uint8_t matterBytes[] = { 'm', 'a', 't', 't', 'e', 'r' }; + if (PayloadContains(pktPayload, ByteSpan(matterBytes)) || PayloadContainsHostNameCaseInsensitive(pktPayload)) + { + return FilterOutcome::kAllowPacket; + } + return FilterOutcome::kDropPacket; + } + + FilterOutcome FilterAfterDequeue(const void * endpoint, const IPPacketInfo & pktInfo, + const chip::System::PacketBufferHandle & pktPayload) override + { + return FilterOutcome::kAllowPacket; + } + +private: + // TODO: Add unit tests for these static functions + static bool IsMdnsBroadcastPacket(const IPPacketInfo & pktInfo) + { + if (pktInfo.DestPort == 5353) + { +#if INET_CONFIG_ENABLE_IPV4 + ip_addr_t mdnsIPv4BroadcastAddr = IPADDR4_INIT_BYTES(224, 0, 0, 251); + if (pktInfo.DestAddress == chip::Inet::IPAddress(mdnsIPv4BroadcastAddr)) + { + return true; + } +#endif + ip_addr_t mdnsIPv6BroadcastAddr = IPADDR6_INIT_HOST(0xFF020000, 0, 0, 0xFB); + if (pktInfo.DestAddress == chip::Inet::IPAddress(mdnsIPv6BroadcastAddr)) + { + return true; + } + } + return false; + } + + static bool PayloadContains(const chip::System::PacketBufferHandle & payload, const chip::ByteSpan & byteSpan) + { + if (payload->HasChainedBuffer() || payload->TotalLength() < byteSpan.size()) + { + return false; + } + for (size_t i = 0; i <= payload->TotalLength() - byteSpan.size(); ++i) + { + if (memcmp(payload->Start() + i, byteSpan.data(), byteSpan.size()) == 0) + { + return true; + } + } + return false; + } + + bool PayloadContainsHostNameCaseInsensitive(const chip::System::PacketBufferHandle & payload) + { + uint8_t hostNameLowerCase[12]; + memcpy(hostNameLowerCase, mHostNameBuffer, sizeof(mHostNameBuffer)); + for (size_t i = 0; i < sizeof(hostNameLowerCase); ++i) + { + if (hostNameLowerCase[i] <= 'F' && hostNameLowerCase[i] >= 'A') + { + hostNameLowerCase[i] = static_cast('a' + hostNameLowerCase[i] - 'A'); + } + } + return PayloadContains(payload, ByteSpan(mHostNameBuffer)) || PayloadContains(payload, ByteSpan(hostNameLowerCase)); + } + + static bool IsValidMdnsHostName(const chip::CharSpan & hostName) + { + for (size_t i = 0; i < hostName.size(); ++i) + { + char ch_data = *(hostName.data() + i); + if (!((ch_data >= '0' && ch_data <= '9') || (ch_data >= 'A' && ch_data <= 'F'))) + { + return false; + } + } + return true; + } + + uint8_t mHostNameBuffer[12] = { 0 }; +}; + +} // namespace Inet +} // namespace chip From 73f04c54e21495b2f8799a2c0833b43d5e6725e2 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Wed, 6 Dec 2023 12:17:08 +0530 Subject: [PATCH 03/13] [ESP32] Limit number of returned WiFi scan results to configured limit (#30780) Scan results are allocated on the heap and on a resource critical device where heap is less, this may fail if there are a lot of APs in the vicinity. --- src/platform/ESP32/NetworkCommissioningDriver.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/platform/ESP32/NetworkCommissioningDriver.cpp b/src/platform/ESP32/NetworkCommissioningDriver.cpp index e2ef8bb660ed0b..80e54d74803e7e 100644 --- a/src/platform/ESP32/NetworkCommissioningDriver.cpp +++ b/src/platform/ESP32/NetworkCommissioningDriver.cpp @@ -345,6 +345,10 @@ void ESPWiFiDriver::OnScanWiFiNetworkDone() mpScanCallback = nullptr; return; } + + // Since this is the dynamic memory allocation, restrict it to a configured limit + ap_number = std::min(static_cast(CHIP_DEVICE_CONFIG_MAX_SCAN_NETWORKS_RESULTS), ap_number); + std::unique_ptr ap_buffer_ptr(new wifi_ap_record_t[ap_number]); if (ap_buffer_ptr == NULL) { From 3596eef78562221eb1c783b9e3585313ffaca62a Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Tue, 7 Nov 2023 02:07:45 +0530 Subject: [PATCH 04/13] [ESP32] Fix the threading issue in nimble (#29180) * [ESP32] Fix the threading issue in nimble Send ble connection error than executing in nimble thread context * comment explaining why we are posting connection error event * Adding a comment for kCHIPoBLEConnectionError --- src/include/platform/CHIPDeviceEvent.h | 6 ++++++ src/platform/ESP32/nimble/BLEManagerImpl.cpp | 9 ++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/include/platform/CHIPDeviceEvent.h b/src/include/platform/CHIPDeviceEvent.h index bf4c55ca438377..561cd5f539367b 100644 --- a/src/include/platform/CHIPDeviceEvent.h +++ b/src/include/platform/CHIPDeviceEvent.h @@ -258,6 +258,12 @@ enum InternalEventTypes kCHIPoBLEUnsubscribe, kCHIPoBLEWriteReceived, kCHIPoBLEIndicateConfirm, + + /** + * Post this event in case of a BLE connection error. This event should be posted + * if the BLE central disconnects without unsubscribing from the BLE characteristic. + * This event should populate CHIPoBLEConnectionError structure. + */ kCHIPoBLEConnectionError, kCHIPoBLENotifyConfirm }; diff --git a/src/platform/ESP32/nimble/BLEManagerImpl.cpp b/src/platform/ESP32/nimble/BLEManagerImpl.cpp index 66a4d29df768d4..877f49e5fb006c 100644 --- a/src/platform/ESP32/nimble/BLEManagerImpl.cpp +++ b/src/platform/ESP32/nimble/BLEManagerImpl.cpp @@ -1273,6 +1273,8 @@ CHIP_ERROR BLEManagerImpl::HandleGAPDisconnect(struct ble_gap_event * gapEvent) peer_delete(gapEvent->disconnect.conn.conn_handle); #endif + // There can be a case where the BLE central disconnects without unsubscribing from the BLE characteristic. + // In such situations, it is necessary to clear the subscription and post a connection error event. if (UnsetSubscribed(gapEvent->disconnect.conn.conn_handle)) { CHIP_ERROR disconReason; @@ -1288,7 +1290,12 @@ CHIP_ERROR BLEManagerImpl::HandleGAPDisconnect(struct ble_gap_event * gapEvent) disconReason = BLE_ERROR_CHIPOBLE_PROTOCOL_ABORT; break; } - HandleConnectionError(gapEvent->disconnect.conn.conn_handle, disconReason); + + ChipDeviceEvent connectionErrorEvent; + connectionErrorEvent.Type = DeviceEventType::kCHIPoBLEConnectionError; + connectionErrorEvent.CHIPoBLEConnectionError.ConId = gapEvent->disconnect.conn.conn_handle; + connectionErrorEvent.CHIPoBLEConnectionError.Reason = disconReason; + ReturnErrorOnFailure(PlatformMgr().PostEvent(&connectionErrorEvent)); } ChipDeviceEvent disconnectEvent; From 917c69376868e9c1b8b46f8d7f96b5d1566c9d7c Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Fri, 5 Jan 2024 04:46:39 +0800 Subject: [PATCH 05/13] IM: Create ReadHandler after Session Establishment for Subscription Resumption (#30491) * IM: Create ReadHandler after Session Establishment for Subscription Resumption * Restyled by clang-format * Make SubscriptionResumptionHelper inherits from SubscriptionInfo * review changes * Rename Helper to SessionEstablisher * Restyled by clang-format * RAII changes * Restyled by clang-format --------- Co-authored-by: Restyled.io --- src/app/BUILD.gn | 7 + src/app/InteractionModelEngine.cpp | 35 ++--- src/app/InteractionModelEngine.h | 2 + src/app/ReadHandler.cpp | 93 +++++-------- src/app/ReadHandler.h | 34 ++--- ...bscriptionResumptionSessionEstablisher.cpp | 124 ++++++++++++++++++ ...SubscriptionResumptionSessionEstablisher.h | 55 ++++++++ 7 files changed, 252 insertions(+), 98 deletions(-) create mode 100644 src/app/SubscriptionResumptionSessionEstablisher.cpp create mode 100644 src/app/SubscriptionResumptionSessionEstablisher.h diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index a7a9baddf55cd8..8d58a0414ecd9e 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -226,6 +226,13 @@ static_library("app") { ] } + if (chip_persist_subscriptions) { + sources += [ + "SubscriptionResumptionSessionEstablisher.cpp", + "SubscriptionResumptionSessionEstablisher.h", + ] + } + if (chip_enable_read_client) { sources += [ "BufferedReadCallback.cpp", diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 541b0902ce90c9..e96f8ae32be943 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -41,6 +41,18 @@ namespace chip { namespace app { +class AutoReleaseSubscriptionInfoIterator +{ +public: + AutoReleaseSubscriptionInfoIterator(SubscriptionResumptionStorage::SubscriptionInfoIterator * iterator) : mIterator(iterator){}; + ~AutoReleaseSubscriptionInfoIterator() { mIterator->Release(); } + + SubscriptionResumptionStorage::SubscriptionInfoIterator * operator->() const { return mIterator; } + +private: + SubscriptionResumptionStorage::SubscriptionInfoIterator * mIterator; +}; + using Protocols::InteractionModel::Status; Global sInteractionModelEngine; @@ -1835,7 +1847,7 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap bool resumedSubscriptions = false; #endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION SubscriptionResumptionStorage::SubscriptionInfo subscriptionInfo; - auto * iterator = imEngine->mpSubscriptionResumptionStorage->IterateSubscriptions(); + AutoReleaseSubscriptionInfoIterator iterator(imEngine->mpSubscriptionResumptionStorage->IterateSubscriptions()); while (iterator->Next(subscriptionInfo)) { // If subscription happens between reboot and this timer callback, it's already live and should skip resumption @@ -1853,31 +1865,24 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap continue; } - auto requestedAttributePathCount = subscriptionInfo.mAttributePaths.AllocatedSize(); - auto requestedEventPathCount = subscriptionInfo.mEventPaths.AllocatedSize(); - if (!imEngine->EnsureResourceForSubscription(subscriptionInfo.mFabricIndex, requestedAttributePathCount, - requestedEventPathCount)) + auto subscriptionResumptionSessionEstablisher = Platform::MakeUnique(); + if (subscriptionResumptionSessionEstablisher == nullptr) { - ChipLogProgress(InteractionModel, "no resource for Subscription resumption"); - iterator->Release(); + ChipLogProgress(InteractionModel, "Failed to create SubscriptionResumptionSessionEstablisher"); return; } - ReadHandler * handler = imEngine->mReadHandlers.CreateObject(*imEngine, imEngine->GetReportScheduler()); - if (handler == nullptr) + if (subscriptionResumptionSessionEstablisher->ResumeSubscription(*imEngine->mpCASESessionMgr, subscriptionInfo) != + CHIP_NO_ERROR) { - ChipLogProgress(InteractionModel, "no resource for ReadHandler creation"); - iterator->Release(); + ChipLogProgress(InteractionModel, "Failed to ResumeSubscription 0x%" PRIx32, subscriptionInfo.mSubscriptionId); return; } - - ChipLogProgress(InteractionModel, "Resuming subscriptionId %" PRIu32, subscriptionInfo.mSubscriptionId); - handler->ResumeSubscription(*imEngine->mpCASESessionMgr, subscriptionInfo); + subscriptionResumptionSessionEstablisher.release(); #if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION resumedSubscriptions = true; #endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION } - iterator->Release(); #if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION // If no persisted subscriptions needed resumption then all resumption retries are done diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 0ecf3f6c6b1ec0..827b571a4a2401 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -377,6 +378,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, friend class reporting::Engine; friend class TestCommandInteraction; friend class TestInteractionModelEngine; + friend class SubscriptionResumptionSessionEstablisher; using Status = Protocols::InteractionModel::Status; void OnDone(CommandHandler & apCommandObj) override; diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 0f79c1a1d0d5f4..5ff5576a930300 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -55,10 +55,6 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon InteractionType aInteractionType, Observer * observer) : mExchangeCtx(*this), mManagementCallback(apCallback) -#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS - , - mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this) -#endif { VerifyOrDie(apExchangeContext != nullptr); @@ -83,8 +79,7 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) : - mExchangeCtx(*this), mManagementCallback(apCallback), mOnConnectedCallback(HandleDeviceConnected, this), - mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this) + mExchangeCtx(*this), mManagementCallback(apCallback) { mInteractionType = InteractionType::Subscribe; mFlags.ClearAll(); @@ -93,31 +88,30 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) : mObserver = observer; } -void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager, - SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo) +void ReadHandler::OnSubscriptionResumed(const SessionHandle & sessionHandle, + SubscriptionResumptionSessionEstablisher & resumptionSessionEstablisher) { - mSubscriptionId = subscriptionInfo.mSubscriptionId; - mMinIntervalFloorSeconds = subscriptionInfo.mMinInterval; - mMaxInterval = subscriptionInfo.mMaxInterval; - SetStateFlag(ReadHandlerFlags::FabricFiltered, subscriptionInfo.mFabricFiltered); + mSubscriptionId = resumptionSessionEstablisher.mSubscriptionInfo.mSubscriptionId; + mMinIntervalFloorSeconds = resumptionSessionEstablisher.mSubscriptionInfo.mMinInterval; + mMaxInterval = resumptionSessionEstablisher.mSubscriptionInfo.mMaxInterval; + SetStateFlag(ReadHandlerFlags::FabricFiltered, resumptionSessionEstablisher.mSubscriptionInfo.mFabricFiltered); // Move dynamically allocated attributes and events from the SubscriptionInfo struct into // the object pool managed by the IM engine - for (size_t i = 0; i < subscriptionInfo.mAttributePaths.AllocatedSize(); i++) + for (size_t i = 0; i < resumptionSessionEstablisher.mSubscriptionInfo.mAttributePaths.AllocatedSize(); i++) { - AttributePathParams attributePathParams = subscriptionInfo.mAttributePaths[i].GetParams(); - CHIP_ERROR err = - InteractionModelEngine::GetInstance()->PushFrontAttributePathList(mpAttributePathList, attributePathParams); + AttributePathParams params = resumptionSessionEstablisher.mSubscriptionInfo.mAttributePaths[i].GetParams(); + CHIP_ERROR err = InteractionModelEngine::GetInstance()->PushFrontAttributePathList(mpAttributePathList, params); if (err != CHIP_NO_ERROR) { Close(); return; } } - for (size_t i = 0; i < subscriptionInfo.mEventPaths.AllocatedSize(); i++) + for (size_t i = 0; i < resumptionSessionEstablisher.mSubscriptionInfo.mEventPaths.AllocatedSize(); i++) { - EventPathParams eventPathParams = subscriptionInfo.mEventPaths[i].GetParams(); - CHIP_ERROR err = InteractionModelEngine::GetInstance()->PushFrontEventPathParamsList(mpEventPathList, eventPathParams); + EventPathParams params = resumptionSessionEstablisher.mSubscriptionInfo.mEventPaths[i].GetParams(); + CHIP_ERROR err = InteractionModelEngine::GetInstance()->PushFrontEventPathParamsList(mpEventPathList, params); if (err != CHIP_NO_ERROR) { Close(); @@ -125,9 +119,26 @@ void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager, } } - // Ask IM engine to start CASE session with subscriber - ScopedNodeId peerNode = ScopedNodeId(subscriptionInfo.mNodeId, subscriptionInfo.mFabricIndex); - caseSessionManager.FindOrEstablishSession(peerNode, &mOnConnectedCallback, &mOnConnectionFailureCallback); + mSessionHandle.Grab(sessionHandle); + + SetStateFlag(ReadHandlerFlags::ActiveSubscription); + + auto * appCallback = mManagementCallback.GetAppCallback(); + if (appCallback) + { + appCallback->OnSubscriptionEstablished(*this); + } + // Notify the observer that a subscription has been resumed + mObserver->OnSubscriptionEstablished(this); + + MoveToState(HandlerState::CanStartReporting); + + ObjectList * attributePath = mpAttributePathList; + while (attributePath) + { + InteractionModelEngine::GetInstance()->GetReportingEngine().SetDirty(attributePath->mValue); + attributePath = attributePath->mpNext; + } } #endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS @@ -892,43 +903,5 @@ void ReadHandler::ClearStateFlag(ReadHandlerFlags aFlag) SetStateFlag(aFlag, false); } -void ReadHandler::HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, - const SessionHandle & sessionHandle) -{ - ReadHandler * const _this = static_cast(context); - - _this->mSessionHandle.Grab(sessionHandle); - - _this->SetStateFlag(ReadHandlerFlags::ActiveSubscription); - - auto * appCallback = _this->mManagementCallback.GetAppCallback(); - if (appCallback) - { - appCallback->OnSubscriptionEstablished(*_this); - } - // Notify the observer that a subscription has been resumed - _this->mObserver->OnSubscriptionEstablished(_this); - - _this->MoveToState(HandlerState::CanStartReporting); - - ObjectList * attributePath = _this->mpAttributePathList; - while (attributePath) - { - InteractionModelEngine::GetInstance()->GetReportingEngine().SetDirty(attributePath->mValue); - attributePath = attributePath->mpNext; - } -} - -void ReadHandler::HandleDeviceConnectionFailure(void * context, const ScopedNodeId & peerId, CHIP_ERROR err) -{ - ReadHandler * const _this = static_cast(context); - VerifyOrDie(_this != nullptr); - - // TODO: Have a retry mechanism tied to wake interval for IC devices - ChipLogError(DataManagement, "Failed to establish CASE for subscription-resumption with error '%" CHIP_ERROR_FORMAT "'", - err.Format()); - _this->Close(); -} - } // namespace app } // namespace chip diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 4c9ad469c782eb..28097868c2f79a 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -253,6 +254,16 @@ class ReadHandler : public Messaging::ExchangeDelegate return CHIP_NO_ERROR; } +#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS + /** + * + * @brief Initialize a ReadHandler for a resumed subsciption + * + * Used after the SubscriptionResumptionSessionEstablisher establishs the CASE session + */ + void OnSubscriptionResumed(const SessionHandle & sessionHandle, SubscriptionResumptionSessionEstablisher & sessionEstablisher); +#endif + private: PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } EventNumber & GetEventMin() { return mEventMin; } @@ -302,18 +313,6 @@ class ReadHandler : public Messaging::ExchangeDelegate */ void OnInitialRequest(System::PacketBufferHandle && aPayload); -#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS - /** - * - * @brief Resume a persisted subscription - * - * Used after ReadHandler(ManagementCallback & apCallback). This will start a CASE session - * with the subscriber if one doesn't already exist, and send full priming report when connected. - */ - void ResumeSubscription(CASESessionManager & caseSessionManager, - SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo); -#endif - /** * Send ReportData to initiator * @@ -485,11 +484,6 @@ class ReadHandler : public Messaging::ExchangeDelegate /// @param aFlag Flag to clear void ClearStateFlag(ReadHandlerFlags aFlag); - // Helpers for continuing the subscription resumption - static void HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, - const SessionHandle & sessionHandle); - static void HandleDeviceConnectionFailure(void * context, const ScopedNodeId & peerId, CHIP_ERROR error); - AttributePathExpandIterator mAttributePathExpandIterator = AttributePathExpandIterator(nullptr); // The current generation of the reporting engine dirty set the last time we were notified that a path we're interested in was @@ -571,12 +565,6 @@ class ReadHandler : public Messaging::ExchangeDelegate // TODO (#27675): Merge all observers into one and that one will dispatch the callbacks to the right place. Observer * mObserver = nullptr; - -#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS - // Callbacks to handle server-initiated session success/failure - chip::Callback::Callback mOnConnectedCallback; - chip::Callback::Callback mOnConnectionFailureCallback; -#endif }; } // namespace app } // namespace chip diff --git a/src/app/SubscriptionResumptionSessionEstablisher.cpp b/src/app/SubscriptionResumptionSessionEstablisher.cpp new file mode 100644 index 00000000000000..f0d53566ad6b28 --- /dev/null +++ b/src/app/SubscriptionResumptionSessionEstablisher.cpp @@ -0,0 +1,124 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +namespace chip { +namespace app { + +class AutoDeleteEstablisher +{ +public: + AutoDeleteEstablisher(SubscriptionResumptionSessionEstablisher * sessionEstablisher) : mSessionEstablisher(sessionEstablisher) + {} + ~AutoDeleteEstablisher() { chip::Platform::Delete(mSessionEstablisher); } + + SubscriptionResumptionSessionEstablisher * operator->() const { return mSessionEstablisher; } + + SubscriptionResumptionSessionEstablisher & operator*() const { return *mSessionEstablisher; } + +private: + SubscriptionResumptionSessionEstablisher * mSessionEstablisher; +}; + +SubscriptionResumptionSessionEstablisher::SubscriptionResumptionSessionEstablisher() : + mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this) +{} + +CHIP_ERROR +SubscriptionResumptionSessionEstablisher::ResumeSubscription( + CASESessionManager & caseSessionManager, const SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo) +{ + mSubscriptionInfo.mNodeId = subscriptionInfo.mNodeId; + mSubscriptionInfo.mFabricIndex = subscriptionInfo.mFabricIndex; + mSubscriptionInfo.mSubscriptionId = subscriptionInfo.mSubscriptionId; + mSubscriptionInfo.mMinInterval = subscriptionInfo.mMinInterval; + mSubscriptionInfo.mMaxInterval = subscriptionInfo.mMaxInterval; + mSubscriptionInfo.mFabricFiltered = subscriptionInfo.mFabricFiltered; + // Copy the Attribute Paths and Event Paths + if (subscriptionInfo.mAttributePaths.AllocatedSize() > 0) + { + mSubscriptionInfo.mAttributePaths.Alloc(subscriptionInfo.mAttributePaths.AllocatedSize()); + if (!mSubscriptionInfo.mAttributePaths.Get()) + { + return CHIP_ERROR_NO_MEMORY; + } + for (size_t i = 0; i < mSubscriptionInfo.mAttributePaths.AllocatedSize(); ++i) + { + mSubscriptionInfo.mAttributePaths[i] = subscriptionInfo.mAttributePaths[i]; + } + } + if (subscriptionInfo.mEventPaths.AllocatedSize() > 0) + { + mSubscriptionInfo.mEventPaths.Alloc(subscriptionInfo.mEventPaths.AllocatedSize()); + if (!mSubscriptionInfo.mEventPaths.Get()) + { + return CHIP_ERROR_NO_MEMORY; + } + for (size_t i = 0; i < mSubscriptionInfo.mEventPaths.AllocatedSize(); ++i) + { + mSubscriptionInfo.mEventPaths[i] = subscriptionInfo.mEventPaths[i]; + } + } + + ScopedNodeId peerNode = ScopedNodeId(mSubscriptionInfo.mNodeId, mSubscriptionInfo.mFabricIndex); + caseSessionManager.FindOrEstablishSession(peerNode, &mOnConnectedCallback, &mOnConnectionFailureCallback); + return CHIP_NO_ERROR; +} + +void SubscriptionResumptionSessionEstablisher::HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, + const SessionHandle & sessionHandle) +{ + AutoDeleteEstablisher establisher(static_cast(context)); + SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo = establisher->mSubscriptionInfo; + InteractionModelEngine * imEngine = InteractionModelEngine::GetInstance(); + if (!imEngine->EnsureResourceForSubscription(subscriptionInfo.mFabricIndex, subscriptionInfo.mAttributePaths.AllocatedSize(), + subscriptionInfo.mEventPaths.AllocatedSize())) + { + ChipLogProgress(InteractionModel, "no resource for subscription resumption"); + return; + } + ReadHandler * readHandler = imEngine->mReadHandlers.CreateObject(*imEngine, imEngine->GetReportScheduler()); + if (readHandler == nullptr) + { + ChipLogProgress(InteractionModel, "no resource for ReadHandler creation"); + return; + } + readHandler->OnSubscriptionResumed(sessionHandle, *establisher); +} + +void SubscriptionResumptionSessionEstablisher::HandleDeviceConnectionFailure(void * context, const ScopedNodeId & peerId, + CHIP_ERROR error) +{ + AutoDeleteEstablisher establisher(static_cast(context)); + SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo = establisher->mSubscriptionInfo; + ChipLogError(DataManagement, "Failed to establish CASE for subscription-resumption with error '%" CHIP_ERROR_FORMAT "'", + error.Format()); + // If the device fails to establish the session, the subscriber might be offline and its subscription read client will + // be deleted when the device reconnect to the subscriber. This subscription will be never used again. So clean up + // the persistent subscription information storage. + auto * subscriptionResumptionStorage = InteractionModelEngine::GetInstance()->GetSubscriptionResumptionStorage(); + if (subscriptionResumptionStorage) + { + subscriptionResumptionStorage->Delete(subscriptionInfo.mNodeId, subscriptionInfo.mFabricIndex, + subscriptionInfo.mSubscriptionId); + } +} + +} // namespace app +} // namespace chip diff --git a/src/app/SubscriptionResumptionSessionEstablisher.h b/src/app/SubscriptionResumptionSessionEstablisher.h new file mode 100644 index 00000000000000..a66c403c1c11ab --- /dev/null +++ b/src/app/SubscriptionResumptionSessionEstablisher.h @@ -0,0 +1,55 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +namespace chip { +namespace app { + +/** + * Session Establisher to resume persistent subscription. A CASE session will be established upon invoking + * ResumeSubscription(), followed by the creation and intialization of a ReadHandler. This class helps prevent + * a scenario where all ReadHandlers in the pool grab the invalid session handle. In such scenario, if the device + * receives a new subscription request, it will crash as there is no evictable ReadHandler. + */ + +class SubscriptionResumptionSessionEstablisher +{ +public: + SubscriptionResumptionSessionEstablisher(); + + CHIP_ERROR ResumeSubscription(CASESessionManager & caseSessionManager, + const SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo); + + SubscriptionResumptionStorage::SubscriptionInfo mSubscriptionInfo; + +private: + // Callback funstions for continuing the subscription resumption + static void HandleDeviceConnected(void * context, Messaging::ExchangeManager & exchangeMgr, + const SessionHandle & sessionHandle); + static void HandleDeviceConnectionFailure(void * context, const ScopedNodeId & peerId, CHIP_ERROR error); + + // Callbacks to handle server-initiated session success/failure + chip::Callback::Callback mOnConnectedCallback; + chip::Callback::Callback mOnConnectionFailureCallback; +}; +} // namespace app +} // namespace chip From 5a0a40aa77e02ac878464bc9bb6c1ddeefae7a34 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Sat, 2 Dec 2023 00:07:01 +0530 Subject: [PATCH 06/13] [ESP32] Fix adding NDEBUG flag to CPPFLAGS (#30763) In esp-idf, NDEBUG flag is added to CPPFLAGS only if assertions are disabled. Making this inline to that. --- config/esp32/components/chip/CMakeLists.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt index b2058fe75df41c..e32588db436e2c 100644 --- a/config/esp32/components/chip/CMakeLists.txt +++ b/config/esp32/components/chip/CMakeLists.txt @@ -38,12 +38,9 @@ if(NOT "${IDF_TARGET}" STREQUAL "esp32h2") endif() if (NOT CMAKE_BUILD_EARLY_EXPANSION) - if (CONFIG_COMPILER_OPTIMIZATION_DEFAULT OR CONFIG_COMPILER_OPTIMIZATION_NONE) + if (CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE) set(is_debug TRUE) else() - if (NOT CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE) - message(FATAL_ERROR "CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE shall be set") - endif() set(is_debug FALSE) endif() endif() From c43821750b289aded9d201c3f707bdbc9adbad83 Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Tue, 5 Mar 2024 12:40:19 +0800 Subject: [PATCH 07/13] Add records of session establishment for subscription resumption (#31755) * Add records of session establishment for subscription resumption * Restyled by clang-format * review changes * Schedule subscription resumption when failing to establish the session in SubscriptionResumptionSessionEstablisher * Add option to set subscription timeout resumption retry interval seconds for Linux app Add cirque test for subscription resumption timeout * Restyled by clang-format * Restyled by autopep8 * Restyled by isort * fix CI building * Add test to the test list * add subscription resumption restries number to SubscriptionInfo struct * review changes * make resumption retries persistent * Restyled by clang-format * ci build fixes * try to fix cirque test --------- Co-authored-by: Restyled.io --- examples/platform/linux/AppMain.cpp | 12 ++ examples/platform/linux/Options.cpp | 40 ++++++ examples/platform/linux/Options.h | 8 +- scripts/tests/cirque_tests.sh | 3 + src/app/InteractionModelEngine.cpp | 14 +- src/app/InteractionModelEngine.h | 20 ++- .../SimpleSubscriptionResumptionStorage.cpp | 16 +++ src/app/SimpleSubscriptionResumptionStorage.h | 1 + ...bscriptionResumptionSessionEstablisher.cpp | 33 ++++- src/app/SubscriptionResumptionStorage.h | 3 + .../subscription_resumption_timeout_test.py | 125 ++++++++++++++++ .../SubscriptionResumptionTimeoutTest.py | 136 ++++++++++++++++++ 12 files changed, 404 insertions(+), 7 deletions(-) create mode 100755 src/controller/python/test/test_scripts/subscription_resumption_timeout_test.py create mode 100755 src/test_driver/linux-cirque/SubscriptionResumptionTimeoutTest.py diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp index 56e6edd44687f2..0de5ab5bea6188 100644 --- a/examples/platform/linux/AppMain.cpp +++ b/examples/platform/linux/AppMain.cpp @@ -561,6 +561,18 @@ void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl) // Init ZCL Data Model and CHIP App Server Server::GetInstance().Init(initParams); +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + // Set ReadHandler Capacity for Subscriptions + chip::app::InteractionModelEngine::GetInstance()->SetHandlerCapacityForSubscriptions( + LinuxDeviceOptions::GetInstance().subscriptionCapacity); + chip::app::InteractionModelEngine::GetInstance()->SetForceHandlerQuota(true); +#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + // Set subscription time resumption retry interval seconds + chip::app::InteractionModelEngine::GetInstance()->SetSubscriptionTimeoutResumptionRetryIntervalSeconds( + LinuxDeviceOptions::GetInstance().subscriptionResumptionRetryIntervalSec); +#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION +#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST + // Now that the server has started and we are done with our startup logging, // log our discovery/onboarding information again so it's not lost in the // noise. diff --git a/examples/platform/linux/Options.cpp b/examples/platform/linux/Options.cpp index d97566213a579e..a0066f2a249506 100644 --- a/examples/platform/linux/Options.cpp +++ b/examples/platform/linux/Options.cpp @@ -84,6 +84,16 @@ enum kCommissionerOption_FabricID = 0x1020, kTraceTo = 0x1021, kOptionSimulateNoInternalTime = 0x1022, +#if defined(PW_RPC_ENABLED) + kOptionRpcServerPort = 0x1023, +#endif +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + kDeviceOption_SubscriptionCapacity = 0x1024, +#endif + kDeviceOption_WiFiSupports5g = 0x1025, +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + kDeviceOption_SubscriptionResumptionRetryIntervalSec = 0x1026, +#endif }; constexpr unsigned kAppUsageLength = 64; @@ -138,6 +148,13 @@ OptionDef sDeviceOptionDefs[] = { { "trace-to", kArgumentRequired, kTraceTo }, #endif { "simulate-no-internal-time", kNoArgument, kOptionSimulateNoInternalTime }, +#if defined(PW_RPC_ENABLED) + { "rpc-server-port", kArgumentRequired, kOptionRpcServerPort }, +#endif +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + { "subscription-capacity", kArgumentRequired, kDeviceOption_SubscriptionCapacity }, + { "subscription-resumption-retry-interval", kArgumentRequired, kDeviceOption_SubscriptionResumptionRetryIntervalSec }, +#endif {} }; @@ -254,6 +271,16 @@ const char * sDeviceOptionHelp = #endif " --simulate-no-internal-time\n" " Time cluster does not use internal platform time\n" +#if defined(PW_RPC_ENABLED) + " --rpc-server-port\n" + " Start RPC server on specified port\n" +#endif +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + " --subscription-capacity\n" + " Max number of subscriptions the device will allow\n" + " --subscription-resumption-retry-interval\n" + " subscription timeout resumption retry interval in seconds\n" +#endif "\n"; bool Base64ArgToVector(const char * arg, size_t maxSize, std::vector & outVector) @@ -507,6 +534,19 @@ bool HandleOption(const char * aProgram, OptionSet * aOptions, int aIdentifier, case kOptionSimulateNoInternalTime: LinuxDeviceOptions::GetInstance().mSimulateNoInternalTime = true; break; +#if defined(PW_RPC_ENABLED) + case kOptionRpcServerPort: + LinuxDeviceOptions::GetInstance().rpcServerPort = static_cast(atoi(aValue)); + break; +#endif +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + case kDeviceOption_SubscriptionCapacity: + LinuxDeviceOptions::GetInstance().subscriptionCapacity = static_cast(atoi(aValue)); + break; + case kDeviceOption_SubscriptionResumptionRetryIntervalSec: + LinuxDeviceOptions::GetInstance().subscriptionResumptionRetryIntervalSec = static_cast(atoi(aValue)); + break; +#endif default: PrintArgError("%s: INTERNAL ERROR: Unhandled option: %s\n", aProgram, aName); retval = false; diff --git a/examples/platform/linux/Options.h b/examples/platform/linux/Options.h index 8a080680cefbcf..e6fc2852c455cf 100644 --- a/examples/platform/linux/Options.h +++ b/examples/platform/linux/Options.h @@ -68,7 +68,13 @@ struct LinuxDeviceOptions chip::FabricId commissionerFabricId = chip::kUndefinedFabricId; std::vector traceTo; bool mSimulateNoInternalTime = false; - +#if defined(PW_RPC_ENABLED) + uint16_t rpcServerPort = 33000; +#endif +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + int32_t subscriptionCapacity = CHIP_IM_MAX_NUM_SUBSCRIPTIONS; + int32_t subscriptionResumptionRetryIntervalSec = -1; +#endif static LinuxDeviceOptions & GetInstance(); }; diff --git a/scripts/tests/cirque_tests.sh b/scripts/tests/cirque_tests.sh index d3d988fd1e515d..fc313456ac0c11 100755 --- a/scripts/tests/cirque_tests.sh +++ b/scripts/tests/cirque_tests.sh @@ -49,6 +49,9 @@ CIRQUE_TESTS=( "CommissioningFailureOnReportTest" "PythonCommissioningTest" "CommissioningWindowTest" + "SubscriptionResumptionTest" + "SubscriptionResumptionCapacityTest" + "SubscriptionResumptionTimeoutTest" ) BOLD_GREEN_TEXT="\033[1;32m" diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index e96f8ae32be943..41122c0d82ae7b 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -345,7 +345,11 @@ void InteractionModelEngine::OnDone(ReadHandler & apReadObj) mReportingEngine.ResetReadHandlerTracker(&apReadObj); mReadHandlers.ReleaseObject(&apReadObj); + TryToResumeSubscriptions(); +} +void InteractionModelEngine::TryToResumeSubscriptions() +{ #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION if (!mSubscriptionResumptionScheduled && HasSubscriptionsToResume()) { @@ -354,8 +358,10 @@ void InteractionModelEngine::OnDone(ReadHandler & apReadObj) mpExchangeMgr->GetSessionManager()->SystemLayer()->StartTimer( System::Clock::Seconds32(timeTillNextSubscriptionResumptionSecs), ResumeSubscriptionsTimerCallback, this); mNumSubscriptionResumptionRetries++; + ChipLogProgress(InteractionModel, "Schedule subscription resumption when failing to establish session, Retries: %" PRIu32, + mNumSubscriptionResumptionRetries); } -#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS +#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION } Status InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * apExchangeContext, @@ -1898,6 +1904,12 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION uint32_t InteractionModelEngine::ComputeTimeSecondsTillNextSubscriptionResumption() { +#if CONFIG_BUILD_FOR_HOST_UNIT_TEST + if (mSubscriptionResumptionRetrySecondsOverride > 0) + { + return static_cast(mSubscriptionResumptionRetrySecondsOverride); + } +#endif if (mNumSubscriptionResumptionRetries > CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX) { return CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_RETRY_INTERVAL_SECS; diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 827b571a4a2401..2b63e74496e544 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -346,6 +346,19 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, // void SetForceHandlerQuota(bool forceHandlerQuota) { mForceHandlerQuota = forceHandlerQuota; } +#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + // + // Override the subscription timeout resumption retry interval seconds. The default retry interval will be + // 300s + GetFibonacciForIndex(retry_times) * 300s, which is too long for unit-tests. + // + // If -1 is passed in, no override is instituted and default behavior resumes. + // + void SetSubscriptionTimeoutResumptionRetryIntervalSeconds(int32_t seconds) + { + mSubscriptionResumptionRetrySecondsOverride = seconds; + } +#endif + // // When testing subscriptions using the high-level APIs in src/controller/ReadInteraction.h, // they don't provide for the ability to shut down those subscriptions after they've been established. @@ -384,6 +397,8 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, void OnDone(CommandHandler & apCommandObj) override; void OnDone(ReadHandler & apReadObj) override; + void TryToResumeSubscriptions(); + ReadHandler::ApplicationCallback * GetAppCallback() override { return mpReadHandlerApplicationCallback; } CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override; @@ -627,7 +642,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, // enforce such check based on the configured size. This flag is used for unit tests only, there is another compare time flag // CHIP_CONFIG_IM_FORCE_FABRIC_QUOTA_CHECK for stress tests. bool mForceHandlerQuota = false; -#endif +#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + int mSubscriptionResumptionRetrySecondsOverride = -1; +#endif // CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION +#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION bool HasSubscriptionsToResume(); diff --git a/src/app/SimpleSubscriptionResumptionStorage.cpp b/src/app/SimpleSubscriptionResumptionStorage.cpp index f6f96978d5ba9d..e233c18af9218a 100644 --- a/src/app/SimpleSubscriptionResumptionStorage.cpp +++ b/src/app/SimpleSubscriptionResumptionStorage.cpp @@ -46,6 +46,7 @@ constexpr TLV::Tag SimpleSubscriptionResumptionStorage::kClusterIdTag; constexpr TLV::Tag SimpleSubscriptionResumptionStorage::kAttributeIdTag; constexpr TLV::Tag SimpleSubscriptionResumptionStorage::kEventIdTag; constexpr TLV::Tag SimpleSubscriptionResumptionStorage::kEventPathTypeTag; +constexpr TLV::Tag SimpleSubscriptionResumptionStorage::kResumptionRetriesTag; SimpleSubscriptionResumptionStorage::SimpleSubscriptionInfoIterator::SimpleSubscriptionInfoIterator( SimpleSubscriptionResumptionStorage & storage) : @@ -252,6 +253,18 @@ CHIP_ERROR SimpleSubscriptionResumptionStorage::Load(uint16_t subscriptionIndex, } ReturnErrorOnFailure(reader.ExitContainer(eventsListType)); +#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + // If the reader cannot get resumption retries, set it to 0 for subscriptionInfo + if (reader.Next(kResumptionRetriesTag) == CHIP_NO_ERROR) + { + ReturnErrorOnFailure(reader.Get(subscriptionInfo.mResumptionRetries)); + } + else + { + subscriptionInfo.mResumptionRetries = 0; + } +#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + ReturnErrorOnFailure(reader.ExitContainer(subscriptionContainerType)); return CHIP_NO_ERROR; @@ -307,6 +320,9 @@ CHIP_ERROR SimpleSubscriptionResumptionStorage::Save(TLV::TLVWriter & writer, Su ReturnErrorOnFailure(writer.EndContainer(eventContainerType)); } ReturnErrorOnFailure(writer.EndContainer(eventsListType)); +#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + ReturnErrorOnFailure(writer.Put(kResumptionRetriesTag, subscriptionInfo.mResumptionRetries)); +#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION ReturnErrorOnFailure(writer.EndContainer(subscriptionContainerType)); diff --git a/src/app/SimpleSubscriptionResumptionStorage.h b/src/app/SimpleSubscriptionResumptionStorage.h index 7cd6a0a98b49d8..729f6f96f23b54 100644 --- a/src/app/SimpleSubscriptionResumptionStorage.h +++ b/src/app/SimpleSubscriptionResumptionStorage.h @@ -132,6 +132,7 @@ class SimpleSubscriptionResumptionStorage : public SubscriptionResumptionStorage static constexpr TLV::Tag kAttributeIdTag = TLV::ContextTag(13); static constexpr TLV::Tag kEventIdTag = TLV::ContextTag(14); static constexpr TLV::Tag kEventPathTypeTag = TLV::ContextTag(16); + static constexpr TLV::Tag kResumptionRetriesTag = TLV::ContextTag(17); PersistentStorageDelegate * mStorage; ObjectPool mSubscriptionInfoIterators; diff --git a/src/app/SubscriptionResumptionSessionEstablisher.cpp b/src/app/SubscriptionResumptionSessionEstablisher.cpp index f0d53566ad6b28..3c6b3969512c89 100644 --- a/src/app/SubscriptionResumptionSessionEstablisher.cpp +++ b/src/app/SubscriptionResumptionSessionEstablisher.cpp @@ -50,6 +50,9 @@ SubscriptionResumptionSessionEstablisher::ResumeSubscription( mSubscriptionInfo.mMinInterval = subscriptionInfo.mMinInterval; mSubscriptionInfo.mMaxInterval = subscriptionInfo.mMaxInterval; mSubscriptionInfo.mFabricFiltered = subscriptionInfo.mFabricFiltered; +#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + mSubscriptionInfo.mResumptionRetries = subscriptionInfo.mResumptionRetries; +#endif // Copy the Attribute Paths and Event Paths if (subscriptionInfo.mAttributePaths.AllocatedSize() > 0) { @@ -100,6 +103,15 @@ void SubscriptionResumptionSessionEstablisher::HandleDeviceConnected(void * cont return; } readHandler->OnSubscriptionResumed(sessionHandle, *establisher); +#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + // Reset the resumption retries to 0 if subscription is resumed + subscriptionInfo.mResumptionRetries = 0; + auto * subscriptionResumptionStorage = InteractionModelEngine::GetInstance()->GetSubscriptionResumptionStorage(); + if (subscriptionResumptionStorage) + { + subscriptionResumptionStorage->Save(subscriptionInfo); + } +#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION } void SubscriptionResumptionSessionEstablisher::HandleDeviceConnectionFailure(void * context, const ScopedNodeId & peerId, @@ -109,12 +121,25 @@ void SubscriptionResumptionSessionEstablisher::HandleDeviceConnectionFailure(voi SubscriptionResumptionStorage::SubscriptionInfo & subscriptionInfo = establisher->mSubscriptionInfo; ChipLogError(DataManagement, "Failed to establish CASE for subscription-resumption with error '%" CHIP_ERROR_FORMAT "'", error.Format()); - // If the device fails to establish the session, the subscriber might be offline and its subscription read client will - // be deleted when the device reconnect to the subscriber. This subscription will be never used again. So clean up - // the persistent subscription information storage. auto * subscriptionResumptionStorage = InteractionModelEngine::GetInstance()->GetSubscriptionResumptionStorage(); - if (subscriptionResumptionStorage) + if (!subscriptionResumptionStorage) + { + ChipLogError(DataManagement, "Failed to get subscription resumption storage"); + return; + } +#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + if (subscriptionInfo.mResumptionRetries <= CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION_MAX_FIBONACCI_STEP_INDEX) + { + InteractionModelEngine::GetInstance()->TryToResumeSubscriptions(); + subscriptionInfo.mResumptionRetries++; + subscriptionResumptionStorage->Save(subscriptionInfo); + } + else +#endif // CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION { + // If the device fails to establish the session several times, the subscriber might be offline and its subscription + // read client will be deleted when the device reconnects to the subscriber. This subscription will be never used again. + // Clean up the persistent subscription information storage. subscriptionResumptionStorage->Delete(subscriptionInfo.mNodeId, subscriptionInfo.mFabricIndex, subscriptionInfo.mSubscriptionId); } diff --git a/src/app/SubscriptionResumptionStorage.h b/src/app/SubscriptionResumptionStorage.h index 316adc3bd3c2e3..e23285aac0c329 100644 --- a/src/app/SubscriptionResumptionStorage.h +++ b/src/app/SubscriptionResumptionStorage.h @@ -73,6 +73,9 @@ class SubscriptionResumptionStorage NodeId mNodeId; FabricIndex mFabricIndex; SubscriptionId mSubscriptionId; +#if CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION + uint32_t mResumptionRetries; +#endif uint16_t mMinInterval; uint16_t mMaxInterval; bool mFabricFiltered; diff --git a/src/controller/python/test/test_scripts/subscription_resumption_timeout_test.py b/src/controller/python/test/test_scripts/subscription_resumption_timeout_test.py new file mode 100755 index 00000000000000..1f6411f63699c3 --- /dev/null +++ b/src/controller/python/test/test_scripts/subscription_resumption_timeout_test.py @@ -0,0 +1,125 @@ +#!/usr/bin/env python3 + +# +# Copyright (c) 2024 Project CHIP Authors +# All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +# Commissioning test. + +import os +import sys +from optparse import OptionParser + +from base import BaseTestHelper, FailIfNot, TestFail, TestTimeout, logger + +TEST_DISCRIMINATOR = 3840 +TEST_SETUPPIN = 20202021 + +TEST_ENDPOINT_ID = 0 + + +def main(): + optParser = OptionParser() + optParser.add_option( + "-t", + "--timeout", + action="store", + dest="testTimeout", + default=90, + type='int', + help="The program will return with timeout after specified seconds.", + metavar="", + ) + optParser.add_option( + "-a", + "--address", + action="store", + dest="deviceAddress", + default='', + type='str', + help="Address of the device", + metavar="", + ) + optParser.add_option( + "--nodeid", + action="store", + dest="nodeid", + default=1, + type=int, + help="The Node ID issued to the device", + metavar="" + ) + optParser.add_option( + "--discriminator", + action="store", + dest="discriminator", + default=TEST_DISCRIMINATOR, + type=int, + help="Discriminator of the device", + metavar="" + ) + optParser.add_option( + "--setuppin", + action="store", + dest="setuppin", + default=TEST_SETUPPIN, + type=int, + help="Setup PIN of the device", + metavar="" + ) + optParser.add_option( + "-p", + "--paa-trust-store-path", + action="store", + dest="paaTrustStorePath", + default='', + type='str', + help="Path that contains valid and trusted PAA Root Certificates.", + metavar="" + ) + + (options, remainingArgs) = optParser.parse_args(sys.argv[1:]) + + timeoutTicker = TestTimeout(options.testTimeout) + timeoutTicker.start() + + test = BaseTestHelper( + nodeid=112233, paaTrustStorePath=options.paaTrustStorePath, testCommissioner=True) + + FailIfNot( + test.TestOnNetworkCommissioning(options.discriminator, options.setuppin, options.nodeid, options.deviceAddress), + "Failed on on-network commissioing") + try: + test.devCtrl.ZCLSubscribeAttribute("BasicInformation", "NodeLabel", options.nodeid, TEST_ENDPOINT_ID, 1, 2, + keepSubscriptions=True, autoResubscribe=False) + except Exception as ex: + TestFail(f"Failed to subscribe attribute: {ex}") + + timeoutTicker.stop() + + logger.info("Test finished") + + # TODO: Python device controller cannot be shutdown clean sometimes and will block on AsyncDNSResolverSockets shutdown. + # Call os._exit(0) to force close it. + os._exit(0) + + +if __name__ == "__main__": + try: + main() + except Exception as ex: + logger.exception(ex) + TestFail("Exception occurred when running tests.") diff --git a/src/test_driver/linux-cirque/SubscriptionResumptionTimeoutTest.py b/src/test_driver/linux-cirque/SubscriptionResumptionTimeoutTest.py new file mode 100755 index 00000000000000..d916504cfb1e9a --- /dev/null +++ b/src/test_driver/linux-cirque/SubscriptionResumptionTimeoutTest.py @@ -0,0 +1,136 @@ +#!/usr/bin/env python3 +""" +Copyright (c) 2024 Project CHIP Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +""" + +import logging +import os +import sys +import time + +from helper.CHIPTestBase import CHIPVirtualHome + +""" +Subscription Resumption Timeout Test to validate that the device will keep resuming subscription +when it receives no status report from the controller. +Steps for this test: + 1. Subcription an attribute on the controller + 2. Shutdown the controller + 3. Verify that the server app with keep resuming the subscription +""" + +logger = logging.getLogger('SubscriptionResumptionTest') +logger.setLevel(logging.INFO) + +sh = logging.StreamHandler() +sh.setFormatter( + logging.Formatter( + '%(asctime)s [%(name)s] %(levelname)s %(message)s')) +logger.addHandler(sh) + +CHIP_PORT = 5540 + +CIRQUE_URL = "http://localhost:5000" +CHIP_REPO = os.path.join(os.path.abspath( + os.path.dirname(__file__)), "..", "..", "..") +TEST_EXTPANID = "fedcba9876543210" +TEST_DISCRIMINATOR = 3840 +MATTER_DEVELOPMENT_PAA_ROOT_CERTS = "credentials/development/paa-root-certs" +TEST_END_DEVICE_APP = "chip-all-clusters-app" + +DEVICE_CONFIG = { + 'device0': { + 'type': 'MobileDevice', + 'base_image': '@default', + 'capability': ['TrafficControl', 'Mount'], + 'rcp_mode': True, + 'docker_network': 'Ipv6', + 'traffic_control': {'latencyMs': 25}, + "mount_pairs": [[CHIP_REPO, CHIP_REPO]], + }, + 'device1': { + 'type': 'CHIPEndDevice', + 'base_image': '@default', + 'capability': ['Thread', 'TrafficControl', 'Mount'], + 'rcp_mode': True, + 'docker_network': 'Ipv6', + 'traffic_control': {'latencyMs': 25}, + "mount_pairs": [[CHIP_REPO, CHIP_REPO]], + } +} + + +class TestSubscriptionResumptionTimeout(CHIPVirtualHome): + def __init__(self, device_config): + super().__init__(CIRQUE_URL, device_config) + self.logger = logger + + def setup(self): + self.initialize_home() + + def test_routine(self): + self.run_subscription_resumption_timeout_test() + + def run_subscription_resumption_timeout_test(self): + ethernet_ip = [device['description']['ipv6_addr'] for device in self.non_ap_devices + if device['type'] == 'CHIPEndDevice'][0] + server_ids = [device['id'] for device in self.non_ap_devices + if device['type'] == 'CHIPEndDevice'] + req_ids = [device['id'] for device in self.non_ap_devices + if device['type'] == 'MobileDevice'] + + server_device_id = server_ids[0] + self.execute_device_cmd( + server_device_id, + ("CHIPCirqueDaemon.py -- run gdb -batch -return-child-result -q -ex \"set pagination off\" " + "-ex run -ex \"thread apply all bt\" --args {} --thread --discriminator {} " + "--subscription-resumption-retry-interval 5").format( + os.path.join(CHIP_REPO, "out/debug/standalone", TEST_END_DEVICE_APP), TEST_DISCRIMINATOR)) + + self.reset_thread_devices(server_ids) + + req_device_id = req_ids[0] + + self.execute_device_cmd(req_device_id, "pip3 install {}".format(os.path.join( + CHIP_REPO, "out/debug/linux_x64_gcc/controller/python/chip_clusters-0.0-py3-none-any.whl"))) + self.execute_device_cmd(req_device_id, "pip3 install {}".format(os.path.join( + CHIP_REPO, "out/debug/linux_x64_gcc/controller/python/chip_core-0.0-cp37-abi3-linux_x86_64.whl"))) + self.execute_device_cmd(req_device_id, "pip3 install {}".format(os.path.join( + CHIP_REPO, "out/debug/linux_x64_gcc/controller/python/chip_repl-0.0-py3-none-any.whl"))) + + command = ("gdb -batch -return-child-result -q -ex run -ex \"thread apply all bt\" " + "--args python3 {} -t 300 -a {} --paa-trust-store-path {}").format( + os.path.join( + CHIP_REPO, "src/controller/python/test/test_scripts/subscription_resumption_timeout_test.py"), ethernet_ip, + os.path.join(CHIP_REPO, MATTER_DEVELOPMENT_PAA_ROOT_CERTS)) + ret = self.execute_device_cmd(req_device_id, command) + + self.assertEqual(ret['return_code'], '0', + "Test failed: non-zero return code") + + # Wait for some time so that the sever will try to resume the subscription for several times + time.sleep(120) + + # Check the device can resume subscriptions + self.logger.info("checking device log for {}".format( + self.get_device_pretty_id(server_device_id))) + self.assertTrue(self.sequenceMatch(self.get_device_log(server_device_id).decode('utf-8'), [ + "Schedule subscription resumption when failing to establish session, Retries: 1", + "Schedule subscription resumption when failing to establish session, Retries: 2"]), + "SubscriptionResumption test failed: cannot find matching string from device {}".format(server_device_id)) + + +if __name__ == "__main__": + sys.exit(TestSubscriptionResumptionTimeout(DEVICE_CONFIG).run_test()) From 162a77961bea2b71ab91c2cdb4f1786352df8915 Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Tue, 16 Jan 2024 22:38:30 +0800 Subject: [PATCH 08/13] ESP32: check ap info in IsStationConnected (#31438) --- src/platform/ESP32/ESP32Utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/ESP32/ESP32Utils.cpp b/src/platform/ESP32/ESP32Utils.cpp index 9d2fcfc4845a03..ffa45ee72241dd 100644 --- a/src/platform/ESP32/ESP32Utils.cpp +++ b/src/platform/ESP32/ESP32Utils.cpp @@ -82,7 +82,7 @@ bool ESP32Utils::IsStationProvisioned(void) CHIP_ERROR ESP32Utils::IsStationConnected(bool & connected) { wifi_ap_record_t apInfo; - connected = (esp_wifi_sta_get_ap_info(&apInfo) == ESP_OK); + connected = (esp_wifi_sta_get_ap_info(&apInfo) == ESP_OK && apInfo.ssid[0] != 0); return CHIP_NO_ERROR; } From 28ea1c4a8e4dcaf5f142da581088eaff737c0144 Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Fri, 8 Mar 2024 22:57:46 +0800 Subject: [PATCH 09/13] Add checks for mOTInst in GenericThreadStackManagerImpl_OpenThread (#32482) * Add checks for mOTInst in GenericThreadStackManagerImpl_OpenThread * review changes --- ...nericThreadStackManagerImpl_OpenThread.hpp | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp index 21bb87668cefb2..4be9f90fab38b1 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp @@ -158,6 +158,7 @@ void GenericThreadStackManagerImpl_OpenThread::_ProcessThreadActivity template bool GenericThreadStackManagerImpl_OpenThread::_HaveRouteToAddress(const Inet::IPAddress & destAddr) { + VerifyOrReturnValue(mOTInst, false); bool res = false; // Lock OpenThread @@ -251,6 +252,7 @@ void GenericThreadStackManagerImpl_OpenThread::_OnPlatformEvent(const template bool GenericThreadStackManagerImpl_OpenThread::_IsThreadEnabled(void) { + VerifyOrReturnValue(mOTInst, false); otDeviceRole curRole; Impl()->LockThreadStack(); @@ -263,6 +265,7 @@ bool GenericThreadStackManagerImpl_OpenThread::_IsThreadEnabled(void) template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetThreadEnabled(bool val) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); otError otErr = OT_ERROR_NONE; Impl()->LockThreadStack(); @@ -297,6 +300,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetThreadEnable template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetThreadProvision(ByteSpan netInfo) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); otError otErr = OT_ERROR_FAILED; otOperationalDatasetTlvs tlvs; @@ -323,6 +327,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetThreadProvis template bool GenericThreadStackManagerImpl_OpenThread::_IsThreadProvisioned(void) { + VerifyOrReturnValue(mOTInst, false); bool provisioned; Impl()->LockThreadStack(); @@ -335,6 +340,7 @@ bool GenericThreadStackManagerImpl_OpenThread::_IsThreadProvisioned(v template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetThreadProvision(Thread::OperationalDataset & dataset) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(Impl()->IsThreadProvisioned(), CHIP_ERROR_INCORRECT_STATE); otOperationalDatasetTlvs datasetTlv; @@ -354,6 +360,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetThreadProvis template bool GenericThreadStackManagerImpl_OpenThread::_IsThreadAttached(void) { + VerifyOrReturnValue(mOTInst, false); otDeviceRole curRole; Impl()->LockThreadStack(); @@ -398,6 +405,7 @@ template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_StartThreadScan(NetworkCommissioning::ThreadDriver::ScanCallback * callback) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR error = CHIP_NO_ERROR; #if CHIP_CONFIG_ENABLE_ICD_SERVER otLinkModeConfig linkMode; @@ -506,6 +514,7 @@ void GenericThreadStackManagerImpl_OpenThread::_OnNetworkScanFinished template ConnectivityManager::ThreadDeviceType GenericThreadStackManagerImpl_OpenThread::_GetThreadDeviceType(void) { + VerifyOrReturnValue(mOTInst, ConnectivityManager::kThreadDeviceType_NotSupported); ConnectivityManager::ThreadDeviceType deviceType; Impl()->LockThreadStack(); @@ -538,6 +547,7 @@ template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetThreadDeviceType(ConnectivityManager::ThreadDeviceType deviceType) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR err = CHIP_NO_ERROR; otLinkModeConfig linkMode; @@ -626,6 +636,7 @@ GenericThreadStackManagerImpl_OpenThread::_SetThreadDeviceType(Connec template bool GenericThreadStackManagerImpl_OpenThread::_HaveMeshConnectivity(void) { + VerifyOrReturnValue(mOTInst, false); bool res; otDeviceRole curRole; @@ -674,6 +685,7 @@ bool GenericThreadStackManagerImpl_OpenThread::_HaveMeshConnectivity( template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetAndLogThreadStatsCounters(void) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR err = CHIP_NO_ERROR; otError otErr; otOperationalDataset activeDataset; @@ -768,6 +780,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetAndLogThread template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetAndLogThreadTopologyMinimal(void) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR err = CHIP_NO_ERROR; #if CHIP_PROGRESS_LOGGING @@ -836,6 +849,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetAndLogThread template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetAndLogThreadTopologyFull() { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR err = CHIP_NO_ERROR; #if CHIP_PROGRESS_LOGGING @@ -1005,6 +1019,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetAndLogThread template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetPrimary802154MACAddress(uint8_t * buf) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); const otExtAddress * extendedAddr = otLinkGetExtendedAddress(mOTInst); memcpy(buf, extendedAddr, sizeof(otExtAddress)); return CHIP_NO_ERROR; @@ -1013,6 +1028,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetPrimary80215 template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetExternalIPv6Address(chip::Inet::IPAddress & addr) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); const otNetifAddress * otAddresses = otIp6GetUnicastAddresses(mOTInst); // Look only for the global unicast addresses, not internally assigned by Thread. @@ -1694,6 +1710,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_WriteThreadNetw template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_GetPollPeriod(uint32_t & buf) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); Impl()->LockThreadStack(); buf = otLinkGetPollPeriod(mOTInst); Impl()->UnlockThreadStack(); @@ -1781,6 +1798,7 @@ bool GenericThreadStackManagerImpl_OpenThread::IsThreadInterfaceUpNoL template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetPollingInterval(System::Clock::Milliseconds32 pollingInterval) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR err = CHIP_NO_ERROR; Impl()->LockThreadStack(); @@ -1822,6 +1840,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetPollingInter template void GenericThreadStackManagerImpl_OpenThread::_ErasePersistentInfo(void) { + VerifyOrReturn(mOTInst); ChipLogProgress(DeviceLayer, "Erasing Thread persistent info..."); Impl()->LockThreadStack(); otThreadSetEnabled(mOTInst, false); @@ -1854,6 +1873,7 @@ void GenericThreadStackManagerImpl_OpenThread::OnJoinerComplete(otErr template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_JoinerStart(void) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR error = CHIP_NO_ERROR; Impl()->LockThreadStack(); @@ -1903,6 +1923,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_JoinerStart(voi template void GenericThreadStackManagerImpl_OpenThread::_UpdateNetworkStatus() { + VerifyOrReturn(mOTInst); // Thread is not enabled, then we are not trying to connect to the network. VerifyOrReturn(ThreadStackMgrImpl().IsThreadEnabled() && mpStatusChangeCallback != nullptr); @@ -2298,6 +2319,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_SetupSrpHost(co template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_ClearSrpHost(const char * aHostName) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR error = CHIP_NO_ERROR; Impl()->LockThreadStack(); @@ -2412,6 +2434,7 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::FromOtDnsRespons template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::ResolveAddress(intptr_t context, otDnsAddressCallback callback) { + VerifyOrReturnError(ThreadStackMgrImpl().OTInstance(), CHIP_ERROR_INCORRECT_STATE); DnsResult * dnsResult = reinterpret_cast(context); ThreadStackMgrImpl().LockThreadStack(); @@ -2566,6 +2589,7 @@ template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_DnsBrowse(const char * aServiceName, DnsBrowseCallback aCallback, void * aContext) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR error = CHIP_NO_ERROR; Impl()->LockThreadStack(); @@ -2676,6 +2700,7 @@ template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_DnsResolve(const char * aServiceName, const char * aInstanceName, DnsResolveCallback aCallback, void * aContext) { + VerifyOrReturnError(mOTInst, CHIP_ERROR_INCORRECT_STATE); CHIP_ERROR error = CHIP_NO_ERROR; Impl()->LockThreadStack(); From cb9f02f28ba5c0d168204f0f67676009aaf6d835 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Mon, 22 Apr 2024 19:06:36 +0530 Subject: [PATCH 10/13] [ESP32] Fix few attributes with fixed quality in DeviceInfoProvider (#32893) * [ESP32] Fix few attributes with fixed quality in DeviceInfoProvider Fixed labels, supported locales, supported calendar types were being read from the nvs(flash) and during OTA its a hassle if one wants to upgrade these values. Added few APIs to set the data for these attributes in ESP32DeviceInfoProvider. * Restyled by clang-format * Restyled by prettier-markdown * fix the lint errors * Add back the original Device info provider which reads from the nvs Add StaticESP32DeviceInfoProvider along with APIs to set data Remove changes from example and add a guide along with usage --------- Co-authored-by: Restyled.io --- docs/guides/esp32/README.md | 1 + docs/guides/esp32/factory_data.md | 10 +- docs/guides/esp32/providers.md | 76 ++++++++++ .../tools/generate_esp32_chip_factory_bin.py | 123 +-------------- src/platform/ESP32/BUILD.gn | 2 + .../ESP32/StaticESP32DeviceInfoProvider.cpp | 122 +++++++++++++++ .../ESP32/StaticESP32DeviceInfoProvider.h | 141 ++++++++++++++++++ 7 files changed, 346 insertions(+), 129 deletions(-) create mode 100644 docs/guides/esp32/providers.md create mode 100644 src/platform/ESP32/StaticESP32DeviceInfoProvider.cpp create mode 100644 src/platform/ESP32/StaticESP32DeviceInfoProvider.h diff --git a/docs/guides/esp32/README.md b/docs/guides/esp32/README.md index e487a58f318591..443058a17b07aa 100644 --- a/docs/guides/esp32/README.md +++ b/docs/guides/esp32/README.md @@ -18,3 +18,4 @@ example on ESP32 series of SoCs - [Matter OTA](ota.md) - [Generating and Using ESP Secure Cert Partition](secure_cert_partition.md) - [BLE Settings](ble_settings.md) +- [Providers](providers.md) diff --git a/docs/guides/esp32/factory_data.md b/docs/guides/esp32/factory_data.md index 179fb2e315a9c0..2e7d8387bb5ab8 100644 --- a/docs/guides/esp32/factory_data.md +++ b/docs/guides/esp32/factory_data.md @@ -30,13 +30,9 @@ Following data can be added to the manufacturing partition using - Serial Number - Unique identifier -- Device information - - Fixed Labels - - Supported locales - - Supported calendar types - - Supported modes - - Note: As per spec at max size of label should be 64 and `\0` will be - added at the end. +- Supported modes + - Note: As per spec at max size of label should be 64 and `\0` will be + added at the end. ### Configuration Options diff --git a/docs/guides/esp32/providers.md b/docs/guides/esp32/providers.md new file mode 100644 index 00000000000000..59b91b624a49fc --- /dev/null +++ b/docs/guides/esp32/providers.md @@ -0,0 +1,76 @@ +## Providers Implemented for ESP32 Platform + +The ESP32 platform has implemented several providers that can be used with data +stored in the factory or by setting fixed data. + +Below are the providers that have been implemented: + +- [Commissionable Data Provider](https://github.com/project-chip/connectedhomeip/blob/master/src/platform/ESP32/ESP32FactoryDataProvider.h#L47) + This provider reads the discriminator and setup pincode related parameters + from the factory partition. +- [Device Attestation Credentials Provider](https://github.com/project-chip/connectedhomeip/blob/master/src/platform/ESP32/ESP32FactoryDataProvider.h#L56) + This provider manages the attestation data. +- [Device Instance Info Provider](https://github.com/project-chip/connectedhomeip/blob/master/src/platform/ESP32/ESP32FactoryDataProvider.h#L86) + This provider reads basic device information from the factory partition. +- [Device Info Provider](https://github.com/project-chip/connectedhomeip/blob/master/src/platform/ESP32/ESP32DeviceInfoProvider.h#L31) + This provider provides fixed labels, supported calendar types, and supported + locales from the factory partition. +- [Supported Modes](https://github.com/project-chip/connectedhomeip/blob/master/examples/platform/esp32/mode-support/static-supported-modes-manager.h#L28) + This provider offers the supported modes for the mode-select cluster. + +More information can be found in the [factory data guide](factory_data.md). + +### Device Info Provider + +Currently, there are two implementations for this provider: + +1. [Reads data stored in the factory partition](https://github.com/project-chip/connectedhomeip/blob/master/src/platform/ESP32/ESP32FactoryDataProvider.h#L56) + _(This will be deprecated in the future)_ +2. [Provides APIs to set fixed data that gets read later](https://github.com/project-chip/connectedhomeip/blob/master/src/platform/ESP32/StaticESP32DeviceInfoProvider.h) + +- New products should use the `StaticESP32DeviceInfoProvider`. Utilize the + `Set...()` APIs to set the fixed data. +- Existing products using the first implementation can continue to use it if + they do not wish to change the data. +- For products using the first implementation and wanting to change the fixed + data via OTA, they should switch to the second implementation in the OTA + image and use the `Set...()` APIs to set the fixed data. + +#### Example: + +```cpp +#include + +DeviceLayer::StaticESP32DeviceInfoProvider deviceInfoProvider; + +// Define array for Supported Calendar Types +using namespace chip::app::Clusters::TimeFormatLocalization::CalendarTypeEnum; +CalendarTypeEnum supportedCalendarTypes[] = { + CalendarTypeEnum::kGregorian, CalendarTypeEnum::kCoptic, + CalendarTypeEnum::kEthiopian, CalendarTypeEnum::kChinese, +}; + +// Define array for Supported Locales +const char* supportedLocales[] = { + "en-US", + "en-EU", +}; + +// Define array for Fixed labels { EndpointId, Label, Value } +struct StaticESP32DeviceInfoProvider::FixedLabelEntry fixedLabels[] = { + { 0, "Room", "Bedroom 2" }, + { 0, "Orientation", "North" }, + { 0, "Direction", "Up" }, +}; + +Span sSupportedCalendarTypes(supportedCalendarTypes); +Span sSupportedLocales(supportedLocales); +Span sFixedLabels(fixedLabels); + +{ + deviceInfoProvider.SetSupportedLocales(sSupportedLocales); + deviceInfoProvider.SetSupportedCalendarTypes(sSupportedCalendarTypes); + deviceInfoProvider.SetFixedLabels(sFixedLabels); + DeviceLayer::SetDeviceInfoProvider(&deviceInfoProvider); +} +``` diff --git a/scripts/tools/generate_esp32_chip_factory_bin.py b/scripts/tools/generate_esp32_chip_factory_bin.py index 6ee881d50689fe..596eb9417382a5 100755 --- a/scripts/tools/generate_esp32_chip_factory_bin.py +++ b/scripts/tools/generate_esp32_chip_factory_bin.py @@ -18,7 +18,6 @@ import argparse import base64 -import enum import logging import os import sys @@ -27,6 +26,7 @@ import cryptography.x509 from bitarray import bitarray from bitarray.util import ba2int +from esp_secure_cert.tlv_format import generate_partition_ds, generate_partition_no_ds, tlv_priv_key_t, tlv_priv_key_type_t CHIP_TOPDIR = os.path.dirname(os.path.realpath(__file__))[:-len(os.path.join('scripts', 'tools'))] sys.path.insert(0, os.path.join(CHIP_TOPDIR, 'scripts', 'tools', 'spake2p')) @@ -148,52 +148,9 @@ 'encoding': 'hex2bin', 'value': None, }, - # DeviceInfoProvider - 'cal-types': { - 'type': 'data', - 'encoding': 'u32', - 'value': None, - }, - 'locale-sz': { - 'type': 'data', - 'encoding': 'u32', - 'value': None, - }, - - # Other device info provider keys are dynamically generated - # in the respective functions. } -class CalendarTypes(enum.Enum): - Buddhist = 0 - Chinese = 1 - Coptic = 2 - Ethiopian = 3 - Gregorian = 4 - Hebrew = 5 - Indian = 6 - Islamic = 7 - Japanese = 8 - Korean = 9 - Persian = 10 - Taiwanese = 11 - - -# Supported Calendar types is stored as a bit array in one uint32_t. -def calendar_types_to_uint32(calendar_types): - result = bitarray(32, endian='little') - result.setall(0) - for calendar_type in calendar_types: - try: - result[CalendarTypes[calendar_type].value] = 1 - except KeyError: - logging.error('Unknown calendar type: %s', calendar_type) - logging.error('Supported calendar types: %s', ', '.join(CalendarTypes.__members__)) - sys.exit(1) - return ba2int(result) - - def ishex(s): try: _ = int(s, 16) @@ -201,31 +158,6 @@ def ishex(s): except ValueError: return False -# get_fixed_label_dict() converts the list of strings to per endpoint dictionaries. -# example input : ['0/orientation/up', '1/orientation/down', '2/orientation/down'] -# example output : {'0': [{'orientation': 'up'}], '1': [{'orientation': 'down'}], '2': [{'orientation': 'down'}]} - - -def get_fixed_label_dict(fixed_labels): - fl_dict = {} - for fl in fixed_labels: - _l = fl.split('/') - - if len(_l) != 3: - logging.error('Invalid fixed label: %s', fl) - sys.exit(1) - - if not (ishex(_l[0]) and (len(_l[1]) > 0 and len(_l[1]) < 16) and (len(_l[2]) > 0 and len(_l[2]) < 16)): - logging.error('Invalid fixed label: %s', fl) - sys.exit(1) - - if _l[0] not in fl_dict.keys(): - fl_dict[_l[0]] = list() - - fl_dict[_l[0]].append({_l[1]: _l[2]}) - - return fl_dict - # get_supported_modes_dict() converts the list of strings to per endpoint dictionaries. # example with semantic tags # input : ['0/label1/1/"1\0x8000, 2\0x8000" 1/label2/1/"1\0x8000, 2\0x8000"'] @@ -345,52 +277,6 @@ def populate_factory_data(args, spake2p_params): if args.hw_ver_str: FACTORY_DATA['hw-ver-str']['value'] = args.hw_ver_str - if args.calendar_types: - FACTORY_DATA['cal-types']['value'] = calendar_types_to_uint32(args.calendar_types) - - # Supported locale is stored as multiple entries, key format: "locale/, example key: "locale/0" - if args.locales: - FACTORY_DATA['locale-sz']['value'] = len(args.locales) - - for i in range(len(args.locales)): - _locale = { - 'type': 'data', - 'encoding': 'string', - 'value': args.locales[i] - } - FACTORY_DATA.update({'locale/{:x}'.format(i): _locale}) - - # Each endpoint can contains the fixed lables - # - fl-sz/ : number of fixed labels for the endpoint - # - fl-k// : fixed label key for the endpoint and index - # - fl-v// : fixed label value for the endpoint and index - if args.fixed_labels: - dict = get_fixed_label_dict(args.fixed_labels) - for key in dict.keys(): - _sz = { - 'type': 'data', - 'encoding': 'u32', - 'value': len(dict[key]) - } - FACTORY_DATA.update({'fl-sz/{:x}'.format(int(key)): _sz}) - - for i in range(len(dict[key])): - entry = dict[key][i] - - _label_key = { - 'type': 'data', - 'encoding': 'string', - 'value': list(entry.keys())[0] - } - _label_value = { - 'type': 'data', - 'encoding': 'string', - 'value': list(entry.values())[0] - } - - FACTORY_DATA.update({'fl-k/{:x}/{:x}'.format(int(key), i): _label_key}) - FACTORY_DATA.update({'fl-v/{:x}/{:x}'.format(int(key), i): _label_value}) - # SupportedModes are stored as multiple entries # - sm-sz/ : number of supported modes for the endpoint # - sm-label// : supported modes label key for the endpoint and index @@ -547,13 +433,6 @@ def any_base_int(s): return int(s, 0) help=('128-bit unique identifier for generating rotating device identifier, ' 'provide 32-byte hex string, e.g. "1234567890abcdef1234567890abcdef"')) - # These will be used by DeviceInfoProvider - parser.add_argument('--calendar-types', nargs='+', - help=('List of supported calendar types.\nSupported Calendar Types: Buddhist, Chinese, Coptic, Ethiopian, ' - 'Gregorian, Hebrew, Indian, Islamic, Japanese, Korean, Persian, Taiwanese')) - parser.add_argument('--locales', nargs='+', help='List of supported locales, Language Tag as defined by BCP47, eg. en-US en-GB') - parser.add_argument('--fixed-labels', nargs='+', - help='List of fixed labels, eg: "0/orientation/up" "1/orientation/down" "2/orientation/down"') parser.add_argument('--supported-modes', type=str, nargs='+', required=False, help='List of supported modes, eg: mode1/label1/ep/"tagValue1\\mfgCode, tagValue2\\mfgCode" mode2/label2/ep/"tagValue1\\mfgCode, tagValue2\\mfgCode" mode3/label3/ep/"tagValue1\\mfgCode, tagValue2\\mfgCode"') diff --git a/src/platform/ESP32/BUILD.gn b/src/platform/ESP32/BUILD.gn index 496b4c6f922ec3..9d1e0bc01c270b 100644 --- a/src/platform/ESP32/BUILD.gn +++ b/src/platform/ESP32/BUILD.gn @@ -182,6 +182,8 @@ static_library("ESP32") { sources += [ "ESP32DeviceInfoProvider.cpp", "ESP32DeviceInfoProvider.h", + "StaticESP32DeviceInfoProvider.cpp", + "StaticESP32DeviceInfoProvider.h", ] } diff --git a/src/platform/ESP32/StaticESP32DeviceInfoProvider.cpp b/src/platform/ESP32/StaticESP32DeviceInfoProvider.cpp new file mode 100644 index 00000000000000..d65bdf8fc280fe --- /dev/null +++ b/src/platform/ESP32/StaticESP32DeviceInfoProvider.cpp @@ -0,0 +1,122 @@ +/* + + * Copyright (c) 2024 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include +#include + +namespace chip { +namespace DeviceLayer { + +StaticESP32DeviceInfoProvider & StaticESP32DeviceInfoProvider::GetDefaultInstance(void) +{ + static StaticESP32DeviceInfoProvider sInstance; + return sInstance; +} + +DeviceInfoProvider::FixedLabelIterator * StaticESP32DeviceInfoProvider::IterateFixedLabel(EndpointId endpoint) +{ + return chip::Platform::New(endpoint, mFixedLabels); +} + +StaticESP32DeviceInfoProvider::StaticFixedLabelIteratorImpl::StaticFixedLabelIteratorImpl(EndpointId endpoint, + const Span & labels) +{ + mEndpoint = endpoint; + mLabels = labels; + mIndex = 0; +} + +size_t StaticESP32DeviceInfoProvider::StaticFixedLabelIteratorImpl::Count() +{ + size_t count = 0; + for (size_t i = 0; i < mLabels.size(); i++) + { + const FixedLabelEntry & entry = mLabels.data()[i]; + + if (entry.endpointId == mEndpoint) + { + count++; + } + } + return count; +} + +bool StaticESP32DeviceInfoProvider::StaticFixedLabelIteratorImpl::Next(FixedLabelType & output) +{ + ChipLogDetail(DeviceLayer, "Get the fixed label with index:%u at endpoint:%d", static_cast(mIndex), mEndpoint); + + while (mIndex < mLabels.size()) + { + const FixedLabelEntry & entry = mLabels.data()[mIndex++]; + if (entry.endpointId == mEndpoint) + { + output.label = entry.label; + output.value = entry.value; + return true; + } + } + + return false; +} + +DeviceInfoProvider::SupportedLocalesIterator * StaticESP32DeviceInfoProvider::IterateSupportedLocales() +{ + return chip::Platform::New(mSupportedLocales); +} + +StaticESP32DeviceInfoProvider::StaticSupportedLocalesIteratorImpl::StaticSupportedLocalesIteratorImpl( + const Span & locales) +{ + mLocales = locales; +} + +size_t StaticESP32DeviceInfoProvider::StaticSupportedLocalesIteratorImpl::Count() +{ + return mLocales.empty() ? 0 : mLocales.size(); +} + +bool StaticESP32DeviceInfoProvider::StaticSupportedLocalesIteratorImpl::Next(CharSpan & output) +{ + VerifyOrReturnValue(mIndex < mLocales.size(), false); + output = mLocales.data()[mIndex++]; + return true; +} + +DeviceInfoProvider::SupportedCalendarTypesIterator * StaticESP32DeviceInfoProvider::IterateSupportedCalendarTypes() +{ + return chip::Platform::New(mSupportedCalendarTypes); +} + +StaticESP32DeviceInfoProvider::StaticSupportedCalendarTypesIteratorImpl::StaticSupportedCalendarTypesIteratorImpl( + const Span & calendarTypes) +{ + mCalendarTypes = calendarTypes; +} + +size_t StaticESP32DeviceInfoProvider::StaticSupportedCalendarTypesIteratorImpl::Count() +{ + return mCalendarTypes.empty() ? 0 : mCalendarTypes.size(); +} + +bool StaticESP32DeviceInfoProvider::StaticSupportedCalendarTypesIteratorImpl::Next(CalendarType & output) +{ + VerifyOrReturnValue(mIndex < mCalendarTypes.size(), false); + output = mCalendarTypes.data()[mIndex++]; + return true; +} + +} // namespace DeviceLayer +} // namespace chip diff --git a/src/platform/ESP32/StaticESP32DeviceInfoProvider.h b/src/platform/ESP32/StaticESP32DeviceInfoProvider.h new file mode 100644 index 00000000000000..860110cc0ac9a2 --- /dev/null +++ b/src/platform/ESP32/StaticESP32DeviceInfoProvider.h @@ -0,0 +1,141 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include + +namespace chip { +namespace DeviceLayer { + +class StaticESP32DeviceInfoProvider : public ESP32DeviceInfoProvider +{ +public: + StaticESP32DeviceInfoProvider() = default; + ~StaticESP32DeviceInfoProvider() override {} + + // Iterators + FixedLabelIterator * IterateFixedLabel(EndpointId endpoint); + SupportedLocalesIterator * IterateSupportedLocales(); + SupportedCalendarTypesIterator * IterateSupportedCalendarTypes(); + + static StaticESP32DeviceInfoProvider & GetDefaultInstance(); + + struct FixedLabelEntry + { + EndpointId endpointId; + CharSpan label; + CharSpan value; + }; + + /** + * @brief API to set the supported calendar types + * + * @param[in] supportedCalendarTypes Span of type chip::app::Clusters::TimeFormatLocalization::CalendarTypeEnum + * containing the supported calendar types. The underlying data must remain allocated throughout + * the lifetime of the device, as the API does not make a copy. + * + * @return CHIP_ERROR indicating the success or failure of the operation. + */ + CHIP_ERROR SetSupportedCalendarTypes(const Span & supportedCalendarTypes) + { + VerifyOrReturnError(!supportedCalendarTypes.empty(), CHIP_ERROR_INVALID_ARGUMENT); + mSupportedCalendarTypes = supportedCalendarTypes; + return CHIP_NO_ERROR; + } + + /** + * @brief API to set the supported Locales + * + * @param[in] supportedLocales Span of type chip::CharSpan containing the supported locales. + * The underlying data must remain allocated throughout the lifetime of the device, + * as the API does not make a copy. + * + * @return CHIP_ERROR indicating the success or failure of the operation. + */ + CHIP_ERROR SetSupportedLocales(const Span & supportedLocales) + { + VerifyOrReturnError(!supportedLocales.empty(), CHIP_ERROR_INVALID_ARGUMENT); + mSupportedLocales = supportedLocales; + return CHIP_NO_ERROR; + } + + /** + * @brief API to set the fixed labels + * + * @param[in] fixedLabels Span of type chip::DeviceLayer::StaticESP32DeviceInfoProvider::FixedLabelEntry + * containing the fixed labels for supported endpoints. + * The underlying data must remain allocated throughout the lifetime of the device, + * as the API does not make a copy. + * + * @return CHIP_ERROR indicating the success or failure of the operation. + */ + CHIP_ERROR SetFixedLabels(const Span & supportedFixedLabels) + { + VerifyOrReturnError(!supportedFixedLabels.empty(), CHIP_ERROR_INVALID_ARGUMENT); + mFixedLabels = supportedFixedLabels; + return CHIP_NO_ERROR; + } + +protected: + class StaticFixedLabelIteratorImpl : public FixedLabelIterator + { + public: + StaticFixedLabelIteratorImpl(EndpointId endpoint, const Span & labels); + size_t Count(); + bool Next(FixedLabelType & output); + void Release() { chip::Platform::Delete(this); } + + private: + EndpointId mEndpoint = 0; + size_t mIndex = 0; + Span mLabels; + }; + + class StaticSupportedLocalesIteratorImpl : public SupportedLocalesIterator + { + public: + StaticSupportedLocalesIteratorImpl(const Span & locales); + size_t Count(); + bool Next(CharSpan & output); + void Release() { chip::Platform::Delete(this); } + + private: + size_t mIndex = 0; + Span mLocales; + }; + + class StaticSupportedCalendarTypesIteratorImpl : public SupportedCalendarTypesIterator + { + public: + StaticSupportedCalendarTypesIteratorImpl(const Span & calendarTypes); + size_t Count(); + bool Next(CalendarType & output); + void Release() { chip::Platform::Delete(this); } + + private: + size_t mIndex = 0; + Span mCalendarTypes; + }; + +private: + Span mSupportedCalendarTypes; + Span mSupportedLocales; + Span mFixedLabels; +}; + +} // namespace DeviceLayer +} // namespace chip From b80b8677f81feaab1a40e7383c13f53dcdbde742 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Thu, 25 Apr 2024 14:34:39 +0530 Subject: [PATCH 11/13] Implement BLE Manager Shutdown for nimble host (#33109) * [ESP32] Implement BLE Manager Shutdown for nimble host - Replace ble deinit imple in Esp32AppServer with BLEMgr().Shutdown() - Replace few ESP_LOG with ChipLog in Esp32AppServer - Move ble deinit kCommissioningComplete switch case - Make USE_BLE_ONLY_FOR_COMMISSIONING depends on BT_ENABLED * Restyled by clang-format * address reviews * Add checks for timer handler --------- Co-authored-by: Restyled.io --- config/esp32/components/chip/Kconfig | 13 ++-- .../esp32/common/CommonDeviceCallbacks.cpp | 2 +- .../platform/esp32/common/Esp32AppServer.cpp | 44 ++---------- src/platform/ESP32/BLEManagerImpl.h | 6 +- src/platform/ESP32/nimble/BLEManagerImpl.cpp | 68 ++++++++++++++++++- 5 files changed, 85 insertions(+), 48 deletions(-) diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index 7f9e56652b020b..db6edc2d9a334c 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -632,12 +632,13 @@ menu "CHIP Device Layer" When set, WoBLE advertisements will stop while a WoBLE connection is active. config USE_BLE_ONLY_FOR_COMMISSIONING - bool "Use BLE only for commissioning" - default y - help - Disable this flag if BLE is used for any other purpose than commissioning. - When enabled, it deinitialized the BLE on successful commissioning, and on - bootup do not initialize the BLE if device is already provisioned with Wi-Fi/Thread credentials. + depends on BT_ENABLED + bool "Use BLE only for commissioning" + default y + help + Disable this flag if BLE is used for any other purpose than commissioning. + When enabled, it deinitialized the BLE on successful commissioning, and on + bootup do not initialize the BLE if device is already provisioned with Wi-Fi/Thread credentials. endmenu diff --git a/examples/platform/esp32/common/CommonDeviceCallbacks.cpp b/examples/platform/esp32/common/CommonDeviceCallbacks.cpp index 0830c49a478cc3..6fc549bbf82cee 100644 --- a/examples/platform/esp32/common/CommonDeviceCallbacks.cpp +++ b/examples/platform/esp32/common/CommonDeviceCallbacks.cpp @@ -51,7 +51,6 @@ void CommonDeviceCallbacks::DeviceEventCallback(const ChipDeviceEvent * event, i case DeviceEventType::kCHIPoBLEConnectionClosed: ESP_LOGI(TAG, "CHIPoBLE disconnected"); - Esp32AppServer::DeInitBLEIfCommissioned(); break; case DeviceEventType::kDnssdInitialized: @@ -67,6 +66,7 @@ void CommonDeviceCallbacks::DeviceEventCallback(const ChipDeviceEvent * event, i case DeviceEventType::kCommissioningComplete: { ESP_LOGI(TAG, "Commissioning complete"); + Esp32AppServer::DeInitBLEIfCommissioned(); } break; diff --git a/examples/platform/esp32/common/Esp32AppServer.cpp b/examples/platform/esp32/common/Esp32AppServer.cpp index 8baafcb458a2d7..26ded8b0e5d3af 100644 --- a/examples/platform/esp32/common/Esp32AppServer.cpp +++ b/examples/platform/esp32/common/Esp32AppServer.cpp @@ -115,46 +115,12 @@ static size_t hex_string_to_binary(const char * hex_string, uint8_t * buf, size_ void Esp32AppServer::DeInitBLEIfCommissioned(void) { -#if CONFIG_BT_ENABLED && CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING +#ifdef CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING if (chip::Server::GetInstance().GetFabricTable().FabricCount() > 0) { - esp_err_t err = ESP_OK; - -#if CONFIG_BT_NIMBLE_ENABLED - if (!ble_hs_is_enabled()) - { - ESP_LOGI(TAG, "BLE already deinited"); - return; - } - if (nimble_port_stop() != 0) - { - ESP_LOGE(TAG, "nimble_port_stop() failed"); - return; - } - vTaskDelay(100); - nimble_port_deinit(); - -#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) - err = esp_nimble_hci_and_controller_deinit(); -#endif -#endif /* CONFIG_BT_NIMBLE_ENABLED */ - -#if CONFIG_IDF_TARGET_ESP32 - err |= esp_bt_mem_release(ESP_BT_MODE_BTDM); -#elif CONFIG_IDF_TARGET_ESP32C2 || CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32S3 || CONFIG_IDF_TARGET_ESP32H2 - err |= esp_bt_mem_release(ESP_BT_MODE_BLE); -#endif - - if (err != ESP_OK) - { - ESP_LOGE(TAG, "BLE deinit failed"); - } - else - { - ESP_LOGI(TAG, "BLE deinit successful and memory reclaimed"); - } + chip::DeviceLayer::Internal::BLEMgr().Shutdown(); } -#endif /* CONFIG_BT_ENABLED && CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING */ +#endif /* CONFIG_USE_BLE_ONLY_FOR_COMMISSIONING */ } void Esp32AppServer::Init(AppDelegate * sAppDelegate) @@ -165,7 +131,7 @@ void Esp32AppServer::Init(AppDelegate * sAppDelegate) if (hex_string_to_binary(CONFIG_TEST_EVENT_TRIGGER_ENABLE_KEY, sTestEventTriggerEnableKey, sizeof(sTestEventTriggerEnableKey)) == 0) { - ESP_LOGE(TAG, "Failed to convert the EnableKey string to octstr type value"); + ChipLogError(DeviceLayer, "Failed to convert the EnableKey string to octstr type value"); memset(sTestEventTriggerEnableKey, 0, sizeof(sTestEventTriggerEnableKey)); } static OTATestEventTriggerDelegate testEventTriggerDelegate{ ByteSpan(sTestEventTriggerEnableKey) }; @@ -189,7 +155,7 @@ void Esp32AppServer::Init(AppDelegate * sAppDelegate) if (chip::DeviceLayer::ConnectivityMgr().IsThreadProvisioned() && (chip::Server::GetInstance().GetFabricTable().FabricCount() != 0)) { - ESP_LOGI(TAG, "Thread has been provisioned, publish the dns service now"); + ChipLogProgress(DeviceLayer, "Thread has been provisioned, publish the dns service now"); chip::app::DnssdServer::Instance().StartServer(); } #endif diff --git a/src/platform/ESP32/BLEManagerImpl.h b/src/platform/ESP32/BLEManagerImpl.h index 5cb1421750786a..6d9ef3223de109 100644 --- a/src/platform/ESP32/BLEManagerImpl.h +++ b/src/platform/ESP32/BLEManagerImpl.h @@ -156,7 +156,7 @@ class BLEManagerImpl final : public BLEManager, // ===== Members that implement the BLEManager internal interface. CHIP_ERROR _Init(void); - void _Shutdown() {} + void _Shutdown(); bool _IsAdvertisingEnabled(void); CHIP_ERROR _SetAdvertisingEnabled(bool val); bool _IsAdvertising(void); @@ -296,6 +296,7 @@ class BLEManagerImpl final : public BLEManager, void DriveBLEState(void); CHIP_ERROR InitESPBleLayer(void); + void DeinitESPBleLayer(void); CHIP_ERROR ConfigureAdvertisingData(void); CHIP_ERROR StartAdvertising(void); @@ -330,6 +331,9 @@ class BLEManagerImpl final : public BLEManager, static void HandleGAPEvent(esp_gap_ble_cb_event_t event, esp_ble_gap_cb_param_t * param); #elif CONFIG_BT_NIMBLE_ENABLED + CHIP_ERROR DeinitBLE(); + static void ClaimBLEMemory(System::Layer *, void *); + void HandleRXCharRead(struct ble_gatt_char_context * param); void HandleRXCharWrite(struct ble_gatt_char_context * param); void HandleTXCharWrite(struct ble_gatt_char_context * param); diff --git a/src/platform/ESP32/nimble/BLEManagerImpl.cpp b/src/platform/ESP32/nimble/BLEManagerImpl.cpp index 877f49e5fb006c..a880e881780bc8 100644 --- a/src/platform/ESP32/nimble/BLEManagerImpl.cpp +++ b/src/platform/ESP32/nimble/BLEManagerImpl.cpp @@ -246,6 +246,21 @@ CHIP_ERROR BLEManagerImpl::_Init() return err; } +void BLEManagerImpl::_Shutdown() +{ + BleLayer::Shutdown(); + + // selectively setting kGATTServiceStarted flag, in order to notify the state machine to stop the CHIPoBLE GATT service + mFlags.ClearAll().Set(Flags::kGATTServiceStarted); + mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled; + +#if CONFIG_ENABLE_ESP32_BLE_CONTROLLER + OnChipBleConnectReceived = nullptr; +#endif // CONFIG_ENABLE_ESP32_BLE_CONTROLLER + + PlatformMgr().ScheduleWork(DriveBLEState, 0); +} + CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -806,7 +821,8 @@ void BLEManagerImpl::DriveBLEState(void) // Stop the CHIPoBLE GATT service if needed. if (mServiceMode != ConnectivityManager::kCHIPoBLEServiceMode_Enabled && mFlags.Has(Flags::kGATTServiceStarted)) { - // TODO: Not supported + DeinitESPBleLayer(); + mFlags.ClearAll(); } exit: @@ -932,6 +948,56 @@ CHIP_ERROR BLEManagerImpl::InitESPBleLayer(void) return err; } +void BLEManagerImpl::DeinitESPBleLayer() +{ + VerifyOrReturn(DeinitBLE() == CHIP_NO_ERROR); + BLEManagerImpl::ClaimBLEMemory(nullptr, nullptr); +} + +void BLEManagerImpl::ClaimBLEMemory(System::Layer *, void *) +{ + TaskHandle_t handle = xTaskGetHandle("nimble_host"); + if (handle) + { + ChipLogDetail(DeviceLayer, "Schedule ble memory reclaiming since nimble host is still running"); + + // Rescheduling it for later, 2 seconds is an arbitrary value, keeping it a bit more so that + // we dont have to reschedule it again + SystemLayer().StartTimer(System::Clock::Seconds32(2), ClaimBLEMemory, nullptr); + } + else + { + // Free up all the space occupied by ble and add it to heap + esp_err_t err = ESP_OK; + +#if CONFIG_IDF_TARGET_ESP32 + err = esp_bt_mem_release(ESP_BT_MODE_BTDM); +#elif CONFIG_IDF_TARGET_ESP32C2 || CONFIG_IDF_TARGET_ESP32C3 || CONFIG_IDF_TARGET_ESP32S3 || CONFIG_IDF_TARGET_ESP32H2 || \ + CONFIG_IDF_TARGET_ESP32C6 + err = esp_bt_mem_release(ESP_BT_MODE_BLE); +#endif + + VerifyOrReturn(err == ESP_OK, ChipLogError(DeviceLayer, "BLE deinit failed")); + ChipLogProgress(DeviceLayer, "BLE deinit successful and memory reclaimed"); + // TODO: post an event when ble is deinitialized and memory is added to heap + } +} + +CHIP_ERROR BLEManagerImpl::DeinitBLE() +{ + VerifyOrReturnError(ble_hs_is_enabled(), CHIP_ERROR_INCORRECT_STATE, ChipLogProgress(DeviceLayer, "BLE already deinited")); + VerifyOrReturnError(0 == nimble_port_stop(), MapBLEError(ESP_FAIL), ChipLogError(DeviceLayer, "nimble_port_stop() failed")); + + esp_err_t err = nimble_port_deinit(); + VerifyOrReturnError(err == ESP_OK, MapBLEError(err)); + +#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) + err = esp_nimble_hci_and_controller_deinit(); +#endif + + return MapBLEError(err); +} + CHIP_ERROR BLEManagerImpl::ConfigureAdvertisingData(void) { CHIP_ERROR err; From dc2bc7a4af4c7bc3c5ab8fdc8aa339c4f2c10601 Mon Sep 17 00:00:00 2001 From: shripad621git <79364691+shripad621git@users.noreply.github.com> Date: Thu, 9 Nov 2023 16:00:20 +0530 Subject: [PATCH 12/13] [ESP32] Made a provision to generate esp_secure_cert partition in factory partition script. (#29840) * Made a provision to generate esp_secure_cert partition in factory partition script. - Added the provision to generate esp_secure_cert_partition based on option --dac-in-secure-cert. - Refactored some code of the existing script - made it more modular. * made outdir user governed --- .../tools/generate_esp32_chip_factory_bin.py | 84 +++++++++++++++---- 1 file changed, 68 insertions(+), 16 deletions(-) diff --git a/scripts/tools/generate_esp32_chip_factory_bin.py b/scripts/tools/generate_esp32_chip_factory_bin.py index 596eb9417382a5..3feaeac6c05859 100755 --- a/scripts/tools/generate_esp32_chip_factory_bin.py +++ b/scripts/tools/generate_esp32_chip_factory_bin.py @@ -51,6 +51,7 @@ FACTORY_PARTITION_CSV = 'nvs_partition.csv' FACTORY_PARTITION_BIN = 'factory_partition.bin' NVS_KEY_PARTITION_BIN = 'nvs_key_partition.bin' +ESP_SECURE_CERT_PARTITION_BIN = 'esp_secure_cert_partititon.bin' FACTORY_DATA = { # CommissionableDataProvider @@ -247,17 +248,41 @@ def populate_factory_data(args, spake2p_params): FACTORY_DATA['iteration-count']['value'] = spake2p_params['Iteration Count'] FACTORY_DATA['salt']['value'] = spake2p_params['Salt'] FACTORY_DATA['verifier']['value'] = spake2p_params['Verifier'] + if not args.dac_in_secure_cert: + if args.dac_cert: + FACTORY_DATA['dac-cert']['value'] = os.path.abspath(args.dac_cert) + if args.pai_cert: + FACTORY_DATA['pai-cert']['value'] = os.path.abspath(args.pai_cert) + if args.dac_key: + FACTORY_DATA['dac-key']['value'] = os.path.abspath('dac_raw_privkey.bin') + FACTORY_DATA['dac-pub-key']['value'] = os.path.abspath('dac_raw_pubkey.bin') + else: + # esp secure cert partition + secure_cert_partition_file_path = os.path.join(args.output_dir, ESP_SECURE_CERT_PARTITION_BIN) + if args.ds_peripheral: + if args.target != "esp32h2": + logging.error("DS peripheral is only supported for esp32h2 target") + exit(1) + if args.efuse_key_id == -1: + logging.error("--efuse-key-id is required when -ds or --ds-peripheral option is used") + exit(1) + priv_key = tlv_priv_key_t(key_type=tlv_priv_key_type_t.ESP_SECURE_CERT_ECDSA_PERIPHERAL_KEY, + key_path=args.dac_key, key_pass=None) + # priv_key_len is in bits + priv_key.priv_key_len = 256 + priv_key.efuse_key_id = args.efuse_key_id + generate_partition_ds(priv_key=priv_key, device_cert=args.dac_cert, + ca_cert=args.pai_cert, idf_target=args.target, + op_file=secure_cert_partition_file_path) + else: + priv_key = tlv_priv_key_t(key_type=tlv_priv_key_type_t.ESP_SECURE_CERT_DEFAULT_FORMAT_KEY, + key_path=args.dac_key, key_pass=None) + generate_partition_no_ds(priv_key=priv_key, device_cert=args.dac_cert, + ca_cert=args.pai_cert, idf_target=args.target, + op_file=secure_cert_partition_file_path) - if args.dac_cert: - FACTORY_DATA['dac-cert']['value'] = os.path.abspath(args.dac_cert) - if args.pai_cert: - FACTORY_DATA['pai-cert']['value'] = os.path.abspath(args.pai_cert) if args.cd: FACTORY_DATA['cert-dclrn']['value'] = os.path.abspath(args.cd) - if args.dac_key: - FACTORY_DATA['dac-key']['value'] = os.path.abspath('dac_raw_privkey.bin') - FACTORY_DATA['dac-pub-key']['value'] = os.path.abspath('dac_raw_pubkey.bin') - if args.serial_num: FACTORY_DATA['serial-num']['value'] = args.serial_num if args.rd_id_uid: @@ -372,9 +397,9 @@ def generate_nvs_csv(out_csv_filename): logging.info('Generated the factory partition csv file : {}'.format(os.path.abspath(out_csv_filename))) -def generate_nvs_bin(encrypt, size, csv_filename, bin_filename): +def generate_nvs_bin(encrypt, size, csv_filename, bin_filename, output_dir): nvs_args = SimpleNamespace(version=2, - outdir=os.getcwd(), + outdir=output_dir, input=csv_filename, output=bin_filename, size=hex(size)) @@ -403,7 +428,8 @@ def clean_up(): os.remove(FACTORY_DATA['dac-key']['value']) -def main(): +def get_args(): + def any_base_int(s): return int(s, 0) parser = argparse.ArgumentParser(description='Chip Factory NVS binary generator tool') @@ -420,6 +446,14 @@ def any_base_int(s): return int(s, 0) parser.add_argument('--pai-cert', help='The path to the PAI certificate in der format') parser.add_argument('--cd', help='The path to the certificate declaration der format') + # Options for esp_secure_cert_partition + parser.add_argument('--dac-in-secure-cert', action="store_true", + help='Store DAC in secure cert partition. By default, DAC is stored in nvs factory partition.') + parser.add_argument('-ds', '--ds-peripheral', action="store_true", + help='Use DS Peripheral in generating secure cert partition.') + parser.add_argument('--efuse-key-id', type=int, choices=range(0, 6), default=-1, + help='Provide the efuse key_id which contains/will contain HMAC_KEY, default is 1') + # These will be used by DeviceInstanceInfoProvider parser.add_argument('--vendor-id', type=any_base_int, help='Vendor id') parser.add_argument('--vendor-name', help='Vendor name') @@ -438,13 +472,20 @@ def any_base_int(s): return int(s, 0) parser.add_argument('-s', '--size', type=any_base_int, default=0x6000, help='The size of the partition.bin, default: 0x6000') + parser.add_argument('--target', default='esp32', + help='The platform type of device. eg: one of esp32, esp32c3, etc.') parser.add_argument('-e', '--encrypt', action='store_true', help='Encrypt the factory parititon NVS binary') parser.add_argument('--no-bin', action='store_false', dest='generate_bin', help='Do not generate the factory partition binary') + parser.add_argument('--output_dir', type=str, default='bin', help='Created image output file path') + parser.set_defaults(generate_bin=True) - args = parser.parse_args() + return parser.parse_args() + + +def set_up_factory_data(args): validate_args(args) if args.passcode is not None: @@ -454,18 +495,29 @@ def any_base_int(s): return int(s, 0) populate_factory_data(args, spake2p_params) - if args.dac_key: + if args.dac_key and not args.dac_in_secure_cert: gen_raw_ec_keypair_from_der(args.dac_key, FACTORY_DATA['dac-pub-key']['value'], FACTORY_DATA['dac-key']['value']) - generate_nvs_csv(FACTORY_PARTITION_CSV) +def generate_factory_partiton_binary(args): + generate_nvs_csv(FACTORY_PARTITION_CSV) if args.generate_bin: - generate_nvs_bin(args.encrypt, args.size, FACTORY_PARTITION_CSV, FACTORY_PARTITION_BIN) + generate_nvs_bin(args.encrypt, args.size, FACTORY_PARTITION_CSV, FACTORY_PARTITION_BIN, args.output_dir) print_flashing_help(args.encrypt, FACTORY_PARTITION_BIN) - clean_up() +def set_up_out_dirs(args): + os.makedirs(args.output_dir, exist_ok=True) + + +def main(): + args = get_args() + set_up_out_dirs(args) + set_up_factory_data(args) + generate_factory_partiton_binary(args) + + if __name__ == "__main__": logging.basicConfig(format='[%(asctime)s] [%(levelname)7s] - %(message)s', level=logging.INFO) main() From 9b559cb39756a7589e367224726225803631b0e0 Mon Sep 17 00:00:00 2001 From: shripad621git <79364691+shripad621git@users.noreply.github.com> Date: Wed, 10 Jan 2024 09:59:53 +0530 Subject: [PATCH 13/13] Added the support for onboarding paylaod in factory script (#31274) --- .../tools/generate_esp32_chip_factory_bin.py | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/scripts/tools/generate_esp32_chip_factory_bin.py b/scripts/tools/generate_esp32_chip_factory_bin.py index 3feaeac6c05859..ff00daedb53682 100755 --- a/scripts/tools/generate_esp32_chip_factory_bin.py +++ b/scripts/tools/generate_esp32_chip_factory_bin.py @@ -24,13 +24,13 @@ from types import SimpleNamespace import cryptography.x509 -from bitarray import bitarray -from bitarray.util import ba2int from esp_secure_cert.tlv_format import generate_partition_ds, generate_partition_no_ds, tlv_priv_key_t, tlv_priv_key_type_t CHIP_TOPDIR = os.path.dirname(os.path.realpath(__file__))[:-len(os.path.join('scripts', 'tools'))] sys.path.insert(0, os.path.join(CHIP_TOPDIR, 'scripts', 'tools', 'spake2p')) from spake2p import generate_verifier # noqa: E402 isort:skip +sys.path.insert(0, os.path.join(CHIP_TOPDIR, 'src', 'setup_payload', 'python')) +from generate_setup_payload import CommissioningFlow, SetupPayload # noqa: E402 isort:skip if os.getenv('IDF_PATH'): sys.path.insert(0, os.path.join(os.getenv('IDF_PATH'), @@ -47,11 +47,11 @@ TOOLS = {} - FACTORY_PARTITION_CSV = 'nvs_partition.csv' FACTORY_PARTITION_BIN = 'factory_partition.bin' NVS_KEY_PARTITION_BIN = 'nvs_key_partition.bin' ESP_SECURE_CERT_PARTITION_BIN = 'esp_secure_cert_partititon.bin' +ONBOARDING_DATA_FILE = 'onboarding_codes.csv' FACTORY_DATA = { # CommissionableDataProvider @@ -480,6 +480,12 @@ def any_base_int(s): return int(s, 0) help='Do not generate the factory partition binary') parser.add_argument('--output_dir', type=str, default='bin', help='Created image output file path') + parser.add_argument('-cf', '--commissioning-flow', type=any_base_int, default=0, + help='Device commissioning flow, 0:Standard, 1:User-Intent, 2:Custom. \ + Default is 0.', choices=[0, 1, 2]) + parser.add_argument('-dm', '--discovery-mode', type=any_base_int, default=1, + help='Commissionable device discovery networking technology. \ + 0:WiFi-SoftAP, 1:BLE, 2:On-network. Default is BLE.', choices=[0, 1, 2]) parser.set_defaults(generate_bin=True) return parser.parse_args() @@ -511,11 +517,33 @@ def set_up_out_dirs(args): os.makedirs(args.output_dir, exist_ok=True) +def generate_onboarding_data(args): + if (args.vendor_id and args.product_id): + payloads = SetupPayload(args.discriminator, args.passcode, args.discovery_mode, CommissioningFlow(args.commissioning_flow), + args.vendor_id, args.product_id) + else: + payloads = SetupPayload(args.discriminator, args.passcode, args.discovery_mode, CommissioningFlow(args.commissioning_flow)) + + chip_qrcode = payloads.generate_qrcode() + chip_manualcode = payloads.generate_manualcode() + + logging.info('Generated QR code: ' + chip_qrcode) + logging.info('Generated manual code: ' + chip_manualcode) + + csv_data = 'qrcode,manualcode\n' + csv_data += chip_qrcode + ',' + chip_manualcode + '\n' + + with open(os.path.join(args.output_dir, ONBOARDING_DATA_FILE), 'w') as f: + f.write(csv_data) + + def main(): args = get_args() set_up_out_dirs(args) set_up_factory_data(args) generate_factory_partiton_binary(args) + if (args.discriminator and args.passcode): + generate_onboarding_data(args) if __name__ == "__main__":