Skip to content

Commit

Permalink
[icd] Defer subscription resumption (#24725)
Browse files Browse the repository at this point in the history
* [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
  • Loading branch information
Damian-Nordic authored and pull[bot] committed Nov 17, 2023
1 parent 29529ef commit 1095706
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 5 deletions.
1 change: 1 addition & 0 deletions config/nrfconnect/chip-module/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions config/zephyr/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 43 additions & 4 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -353,12 +353,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
}
}

#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
ResumeSubscriptions();
#endif

PlatformMgr().AddEventHandler(OnPlatformEventWrapper, reinterpret_cast<intptr_t>(this));
PlatformMgr().HandleServerStarted();

mIsDnssdReady = Dnssd::Resolver::Instance().IsInitialized();
CheckServerReadyEvent();

exit:
if (err != CHIP_NO_ERROR)
{
Expand All @@ -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 *>(server)->OnPlatformEvent(*event);
}

void Server::RejoinExistingMulticastGroups()
{
ChipLogProgress(AppServer, "Joining Multicast groups");
Expand Down Expand Up @@ -423,6 +461,7 @@ void Server::ScheduleFactoryReset()

void Server::Shutdown()
{
PlatformMgr().RemoveEventHandler(OnPlatformEventWrapper, 0);
mCASEServer.Shutdown();
mCASESessionManager.Shutdown();
app::DnssdServer::Instance().SetCommissioningModeProvider(nullptr);
Expand Down
5 changes: 5 additions & 0 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
/**
Expand Down Expand Up @@ -558,6 +562,7 @@ class Server
Credentials::OperationalCertificateStore * mOpCertStore;
app::FailSafeContext mFailSafeContext;

bool mIsDnssdReady = false;
uint16_t mOperationalServicePort;
uint16_t mUserDirectedCommissioningPort;
Inet::InterfaceId mInterfaceId;
Expand Down
1 change: 1 addition & 0 deletions src/controller/tests/TestCommissionableNodeController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class MockResolver : public Resolver
{
public:
CHIP_ERROR Init(chip::Inet::EndPointManager<chip::Inet::UDPEndPoint> * udpEndPointManager) override { return InitStatus; }
bool IsInitialized() override { return true; }
void Shutdown() override {}
void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {}
void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {}
Expand Down
9 changes: 9 additions & 0 deletions src/include/platform/CHIPDeviceEvent.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

/**
Expand Down
5 changes: 5 additions & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
1 change: 1 addition & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
9 changes: 8 additions & 1 deletion src/lib/dnssd/Resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Inet::UDPEndPoint> * endPointManager) = 0;
virtual CHIP_ERROR Init(Inet::EndPointManager<Inet::UDPEndPoint> * 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.
Expand Down
2 changes: 2 additions & 0 deletions src/lib/dnssd/ResolverProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate

///// Resolver implementation
CHIP_ERROR Init(chip::Inet::EndPointManager<chip::Inet::UDPEndPoint> * udpEndPointManager) override;
bool IsInitialized() override;
void Shutdown() override;
void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mOperationalDelegate = delegate; }
void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override { mCommissioningDelegate = delegate; }
Expand Down Expand Up @@ -440,6 +441,11 @@ CHIP_ERROR MinMdnsResolver::Init(chip::Inet::EndPointManager<chip::Inet::UDPEndP
return GlobalMinimalMdnsServer::Instance().StartServer(udpEndPointManager, kMdnsPort);
}

bool MinMdnsResolver::IsInitialized()
{
return GlobalMinimalMdnsServer::Server().IsListening();
}

void MinMdnsResolver::Shutdown()
{
GlobalMinimalMdnsServer::Instance().ShutdownServer();
Expand Down
1 change: 1 addition & 0 deletions src/lib/dnssd/Resolver_ImplNone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class NoneResolver : public Resolver
{
public:
CHIP_ERROR Init(chip::Inet::EndPointManager<chip::Inet::UDPEndPoint> *) override { return CHIP_NO_ERROR; }
bool IsInitialized() override { return true; }
void Shutdown() override {}
void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {}
void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {}
Expand Down

0 comments on commit 1095706

Please sign in to comment.