From 1095706f6aa13db344bcce1eb5a4918c4163d9de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Thu, 2 Feb 2023 10:51:40 +0100 Subject: [PATCH] [icd] Defer subscription resumption (#24725) * [zephyr] Add Kconfig to enable persistent subscriptions * [icd] Defer subscription resumption On some platforms, DNS-SD initialization is asynchronous and any attempt to communicate with a peer node before that is completed must fail. Introduce kServerReady event that is emitted when all async initialization tasks are completed (currently, only DNS-SD) and resume persistent subscriptions in response to that event. * Code review * Code review p.2 --- config/nrfconnect/chip-module/CMakeLists.txt | 1 + config/zephyr/Kconfig | 10 ++++ src/app/server/Server.cpp | 47 +++++++++++++++++-- src/app/server/Server.h | 5 ++ .../TestCommissionableNodeController.cpp | 1 + src/include/platform/CHIPDeviceEvent.h | 9 ++++ src/lib/dnssd/Discovery_ImplPlatform.cpp | 5 ++ src/lib/dnssd/Discovery_ImplPlatform.h | 1 + src/lib/dnssd/Resolver.h | 9 +++- src/lib/dnssd/ResolverProxy.h | 2 + src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 6 +++ src/lib/dnssd/Resolver_ImplNone.cpp | 1 + 12 files changed, 92 insertions(+), 5 deletions(-) diff --git a/config/nrfconnect/chip-module/CMakeLists.txt b/config/nrfconnect/chip-module/CMakeLists.txt index 0ca4a816f5f1d2..c48d18c13bee02 100644 --- a/config/nrfconnect/chip-module/CMakeLists.txt +++ b/config/nrfconnect/chip-module/CMakeLists.txt @@ -212,6 +212,7 @@ chip_gn_arg_bool ("chip_config_network_layer_ble" CONFIG_BT) chip_gn_arg_bool ("chip_inet_config_enable_ipv4" CONFIG_NET_IPV4) chip_gn_arg_bool ("chip_enable_nfc" CONFIG_CHIP_NFC_COMMISSIONING) chip_gn_arg_bool ("chip_enable_ota_requestor" CONFIG_CHIP_OTA_REQUESTOR) +chip_gn_arg_bool ("chip_persist_subscriptions" CONFIG_CHIP_PERSISTENT_SUBSCRIPTIONS) chip_gn_arg_bool ("chip_build_tests" CONFIG_CHIP_BUILD_TESTS) chip_gn_arg_bool ("chip_monolithic_tests" CONFIG_CHIP_BUILD_TESTS) chip_gn_arg_bool ("chip_inet_config_enable_tcp_endpoint" CONFIG_CHIP_BUILD_TESTS) diff --git a/config/zephyr/Kconfig b/config/zephyr/Kconfig index 59b87a77b551ef..54717d7e3b80ca 100644 --- a/config/zephyr/Kconfig +++ b/config/zephyr/Kconfig @@ -254,6 +254,16 @@ config CHIP_CRYPTO_PSA based on the PSA crypto API (instead of the default implementation, which is based on the legacy mbedTLS APIs). +config CHIP_PERSISTENT_SUBSCRIPTIONS + bool "Persistent subscriptions" + help + Persists Matter subscriptions on the publisher node. This feature allows + a Matter node to faster get back to the previous operation after it went + offline, for example, due to a power outage. That is, the node can use the + persisted subscription information to quickly re-establish the previous + subscriptions instead of waiting for the subscriber node to realize that + the publisher is alive again. + config CHIP_LIB_SHELL bool "Matter shell commands" default n diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index d7f76abe19a0aa..c41f5c3bb47fb4 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -353,12 +353,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) } } -#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS - ResumeSubscriptions(); -#endif - + PlatformMgr().AddEventHandler(OnPlatformEventWrapper, reinterpret_cast(this)); PlatformMgr().HandleServerStarted(); + mIsDnssdReady = Dnssd::Resolver::Instance().IsInitialized(); + CheckServerReadyEvent(); + exit: if (err != CHIP_NO_ERROR) { @@ -372,6 +372,44 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) return err; } +void Server::OnPlatformEvent(const DeviceLayer::ChipDeviceEvent & event) +{ + switch (event.Type) + { + case DeviceEventType::kDnssdPlatformInitialized: + // Platform DNS-SD implementation uses kPlatformDnssdInitialized event to signal that it's ready. + if (!mIsDnssdReady) + { + mIsDnssdReady = true; + CheckServerReadyEvent(); + } + break; + case DeviceEventType::kServerReady: +#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS + ResumeSubscriptions(); +#endif + break; + default: + break; + } +} + +void Server::CheckServerReadyEvent() +{ + // Check if all asynchronously initialized server components (currently, only DNS-SD) + // are ready, and emit the 'server ready' event if so. + if (mIsDnssdReady) + { + ChipDeviceEvent event = { .Type = DeviceEventType::kServerReady }; + PlatformMgr().PostEventOrDie(&event); + } +} + +void Server::OnPlatformEventWrapper(const DeviceLayer::ChipDeviceEvent * event, intptr_t server) +{ + reinterpret_cast(server)->OnPlatformEvent(*event); +} + void Server::RejoinExistingMulticastGroups() { ChipLogProgress(AppServer, "Joining Multicast groups"); @@ -423,6 +461,7 @@ void Server::ScheduleFactoryReset() void Server::Shutdown() { + PlatformMgr().RemoveEventHandler(OnPlatformEventWrapper, 0); mCASEServer.Shutdown(); mCASESessionManager.Shutdown(); app::DnssdServer::Instance().SetCommissioningModeProvider(nullptr); diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 6fcf79f01bf2af..9318ae71232dc2 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -377,6 +377,10 @@ class Server static Server sServer; void InitFailSafe(); + void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent & event); + void CheckServerReadyEvent(); + + static void OnPlatformEventWrapper(const DeviceLayer::ChipDeviceEvent * event, intptr_t); #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS /** @@ -558,6 +562,7 @@ class Server Credentials::OperationalCertificateStore * mOpCertStore; app::FailSafeContext mFailSafeContext; + bool mIsDnssdReady = false; uint16_t mOperationalServicePort; uint16_t mUserDirectedCommissioningPort; Inet::InterfaceId mInterfaceId; diff --git a/src/controller/tests/TestCommissionableNodeController.cpp b/src/controller/tests/TestCommissionableNodeController.cpp index 6d0c04ea1f831b..d50a267172ec19 100644 --- a/src/controller/tests/TestCommissionableNodeController.cpp +++ b/src/controller/tests/TestCommissionableNodeController.cpp @@ -33,6 +33,7 @@ class MockResolver : public Resolver { public: CHIP_ERROR Init(chip::Inet::EndPointManager * udpEndPointManager) override { return InitStatus; } + bool IsInitialized() override { return true; } void Shutdown() override {} void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {} void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {} diff --git a/src/include/platform/CHIPDeviceEvent.h b/src/include/platform/CHIPDeviceEvent.h index 845751eae020e3..5d9dd5ee312f35 100644 --- a/src/include/platform/CHIPDeviceEvent.h +++ b/src/include/platform/CHIPDeviceEvent.h @@ -230,6 +230,15 @@ enum PublicEventTypes * Signals that the state of the OTA engine changed. */ kOtaStateChanged, + + /** + * Server initialization has completed. + * + * Signals that all server components have been initialized and the node is ready to establish + * connections with other nodes. This event can be used to trigger on-boot actions that require + * sending messages to other nodes. + */ + kServerReady, }; /** diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 8068a9babf47a3..5d8ef3eeaaf7f8 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -603,6 +603,11 @@ CHIP_ERROR DiscoveryImplPlatform::FinalizeServiceUpdate() return ChipDnssdFinalizeServiceUpdate(); } +bool DiscoveryImplPlatform::IsInitialized() +{ + return mState == State::kInitialized; +} + CHIP_ERROR DiscoveryImplPlatform::ResolveNodeId(const PeerId & peerId) { ReturnErrorOnFailure(InitImpl()); diff --git a/src/lib/dnssd/Discovery_ImplPlatform.h b/src/lib/dnssd/Discovery_ImplPlatform.h index 565d4afaf9b33f..43a70eaf334e6f 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.h +++ b/src/lib/dnssd/Discovery_ImplPlatform.h @@ -49,6 +49,7 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver CHIP_ERROR UpdateCommissionableInstanceName() override; // Members that implement Resolver interface. + bool IsInitialized() override; void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mResolverProxy.SetOperationalDelegate(delegate); } void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override { diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 22033b0f5c324a..9f4698a40db563 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -346,7 +346,14 @@ class Resolver * The method must be called before other methods of this class. * If the resolver has already been initialized, the method exits immediately with no error. */ - virtual CHIP_ERROR Init(chip::Inet::EndPointManager * endPointManager) = 0; + virtual CHIP_ERROR Init(Inet::EndPointManager * endPointManager) = 0; + + /** + * Returns whether the resolver has completed the initialization. + * + * Returns true if the resolver is ready to take node resolution and discovery requests. + */ + virtual bool IsInitialized() = 0; /** * Shuts down the resolver if it has been initialized before. diff --git a/src/lib/dnssd/ResolverProxy.h b/src/lib/dnssd/ResolverProxy.h index 58025bdd14ea72..feff0a6bb4da73 100644 --- a/src/lib/dnssd/ResolverProxy.h +++ b/src/lib/dnssd/ResolverProxy.h @@ -108,6 +108,8 @@ class ResolverProxy : public Resolver return mDelegate != nullptr ? CHIP_NO_ERROR : CHIP_ERROR_NO_MEMORY; } + bool IsInitialized() override { return Resolver::Instance().IsInitialized(); } + void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { if (mDelegate != nullptr) diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 5faacb025d9348..c8032cb4e63dba 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -275,6 +275,7 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate ///// Resolver implementation CHIP_ERROR Init(chip::Inet::EndPointManager * udpEndPointManager) override; + bool IsInitialized() override; void Shutdown() override; void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mOperationalDelegate = delegate; } void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override { mCommissioningDelegate = delegate; } @@ -440,6 +441,11 @@ CHIP_ERROR MinMdnsResolver::Init(chip::Inet::EndPointManager *) override { return CHIP_NO_ERROR; } + bool IsInitialized() override { return true; } void Shutdown() override {} void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {} void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {}