From 2bc71bf57dfebc2ce041e2cf0a9e274d3ba56167 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Thu, 16 Nov 2023 16:15:23 +0100 Subject: [PATCH 1/2] [nrf fromtree] [dns-sd] Untangle dependencies between Resolver and ResolverProxy The relationship between Resolver and Resolver Proxy is unclear as both classes depend on each other and Resolver Proxy is implemented in a source file of concrete Resolver implementation. This makes it impossible to build Matter library with several Resolver implementations and select one to be used at runtime. The main reason for introducing Resolver Proxy was the need for running multiple parallel DNS-SD queries. Address this by adding Discovery Delegate parameter to the Resolver interface methods for starting and stopping the discovery. Then implement Resolver Proxy as a convenience class on top of the Resolver interface. Additionally, simplify the Commissionable Node Controller class by injecting a test Resolver into Resolver Proxy. --- src/app/CASESessionManager.h | 2 - .../commands/discovery/DiscoveryCommands.cpp | 2 - .../commands/discovery/DiscoveryCommands.h | 6 +- .../AbstractDnssdDiscoveryController.h | 3 +- .../CHIPCommissionableNodeController.cpp | 26 +-- .../CHIPCommissionableNodeController.h | 5 +- .../TestCommissionableNodeController.cpp | 7 +- src/lib/dnssd/BUILD.gn | 2 + src/lib/dnssd/Discovery_ImplPlatform.cpp | 199 ++++++++---------- src/lib/dnssd/Discovery_ImplPlatform.h | 12 +- src/lib/dnssd/Resolver.h | 65 ++++-- src/lib/dnssd/ResolverProxy.cpp | 66 ++++++ src/lib/dnssd/ResolverProxy.h | 116 +++------- src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 81 ++++--- src/lib/dnssd/Resolver_ImplNone.cpp | 36 +--- src/lib/shell/commands/Dns.cpp | 20 +- 16 files changed, 295 insertions(+), 353 deletions(-) create mode 100644 src/lib/dnssd/ResolverProxy.cpp diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index 8b803a0a10..7094c88f42 100644 --- a/src/app/CASESessionManager.h +++ b/src/app/CASESessionManager.h @@ -28,8 +28,6 @@ #include #include -#include - namespace chip { class OperationalSessionSetupPoolDelegate; diff --git a/src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp b/src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp index c3ba7b4ca0..26f5b172b8 100644 --- a/src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp +++ b/src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp @@ -117,7 +117,6 @@ CHIP_ERROR DiscoveryCommands::SetupDiscoveryCommands() ReturnErrorOnFailure(mDNSResolver.Init(chip::DeviceLayer::UDPEndPointManager())); mReady = true; } - mDNSResolver.SetOperationalDelegate(this); mDNSResolver.SetCommissioningDelegate(this); return CHIP_NO_ERROR; } @@ -125,7 +124,6 @@ CHIP_ERROR DiscoveryCommands::SetupDiscoveryCommands() CHIP_ERROR DiscoveryCommands::TearDownDiscoveryCommands() { mDNSResolver.StopDiscovery(); - mDNSResolver.SetOperationalDelegate(nullptr); mDNSResolver.SetCommissioningDelegate(nullptr); return CHIP_NO_ERROR; } diff --git a/src/app/tests/suites/commands/discovery/DiscoveryCommands.h b/src/app/tests/suites/commands/discovery/DiscoveryCommands.h index 37fd9b4f8b..8425ef4ff1 100644 --- a/src/app/tests/suites/commands/discovery/DiscoveryCommands.h +++ b/src/app/tests/suites/commands/discovery/DiscoveryCommands.h @@ -24,7 +24,7 @@ #include -class DiscoveryCommands : public chip::Dnssd::CommissioningResolveDelegate, public chip::Dnssd::OperationalResolveDelegate +class DiscoveryCommands : public chip::Dnssd::CommissioningResolveDelegate { public: DiscoveryCommands(){}; @@ -64,10 +64,6 @@ class DiscoveryCommands : public chip::Dnssd::CommissioningResolveDelegate, publ /////////// CommissioningDelegate Interface ///////// void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override; - /////////// OperationalDelegate Interface ///////// - void OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override{}; - void OnOperationalNodeResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override{}; - private: bool mReady = false; chip::Dnssd::ResolverProxy mDNSResolver; diff --git a/src/controller/AbstractDnssdDiscoveryController.h b/src/controller/AbstractDnssdDiscoveryController.h index 788102282b..6e50d7b40c 100644 --- a/src/controller/AbstractDnssdDiscoveryController.h +++ b/src/controller/AbstractDnssdDiscoveryController.h @@ -39,8 +39,7 @@ namespace Controller { class DLL_EXPORT AbstractDnssdDiscoveryController : public Dnssd::CommissioningResolveDelegate { public: - AbstractDnssdDiscoveryController() {} - ~AbstractDnssdDiscoveryController() override { mDNSResolver.Shutdown(); } + explicit AbstractDnssdDiscoveryController(Dnssd::Resolver * resolver = nullptr) : mDNSResolver(resolver) {} void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override; CHIP_ERROR StopDiscovery() { return mDNSResolver.StopDiscovery(); }; diff --git a/src/controller/CHIPCommissionableNodeController.cpp b/src/controller/CHIPCommissionableNodeController.cpp index fdcf1f7e26..d187cc1289 100644 --- a/src/controller/CHIPCommissionableNodeController.cpp +++ b/src/controller/CHIPCommissionableNodeController.cpp @@ -32,30 +32,12 @@ CHIP_ERROR CommissionableNodeController::DiscoverCommissioners(Dnssd::DiscoveryF { ReturnErrorOnFailure(SetUpNodeDiscovery()); - if (mResolver == nullptr) - { #if CONFIG_DEVICE_LAYER - mDNSResolver.Shutdown(); // reset if already inited - ReturnErrorOnFailure(mDNSResolver.Init(DeviceLayer::UDPEndPointManager())); + mDNSResolver.Shutdown(); // reset if already inited + ReturnErrorOnFailure(mDNSResolver.Init(DeviceLayer::UDPEndPointManager())); #endif - mDNSResolver.SetCommissioningDelegate(this); - return mDNSResolver.DiscoverCommissioners(discoveryFilter); - } - -#if CONFIG_DEVICE_LAYER - ReturnErrorOnFailure(mResolver->Init(DeviceLayer::UDPEndPointManager())); -#endif - return mResolver->DiscoverCommissioners(discoveryFilter); -} - -CHIP_ERROR CommissionableNodeController::StopDiscovery() -{ - if (mResolver == nullptr) - { - return AbstractDnssdDiscoveryController::StopDiscovery(); - } - - return mResolver->StopDiscovery(); + mDNSResolver.SetCommissioningDelegate(this); + return mDNSResolver.DiscoverCommissioners(discoveryFilter); } CommissionableNodeController::~CommissionableNodeController() diff --git a/src/controller/CHIPCommissionableNodeController.h b/src/controller/CHIPCommissionableNodeController.h index e573ba6fc4..5c9f57919a 100644 --- a/src/controller/CHIPCommissionableNodeController.h +++ b/src/controller/CHIPCommissionableNodeController.h @@ -36,14 +36,12 @@ namespace Controller { class DLL_EXPORT CommissionableNodeController : public AbstractDnssdDiscoveryController { public: - CommissionableNodeController(chip::Dnssd::Resolver * resolver = nullptr) : mResolver(resolver) {} + CommissionableNodeController(chip::Dnssd::Resolver * resolver = nullptr) : AbstractDnssdDiscoveryController(resolver) {} ~CommissionableNodeController() override; void RegisterDeviceDiscoveryDelegate(DeviceDiscoveryDelegate * delegate) { mDeviceDiscoveryDelegate = delegate; } CHIP_ERROR DiscoverCommissioners(Dnssd::DiscoveryFilter discoveryFilter = Dnssd::DiscoveryFilter()); - CHIP_ERROR StopDiscovery(); - /** * @return * Pointer to DiscoveredNodeData at index idx in the list of commissioners discovered @@ -57,7 +55,6 @@ class DLL_EXPORT CommissionableNodeController : public AbstractDnssdDiscoveryCon DiscoveredNodeList GetDiscoveredNodes() override { return DiscoveredNodeList(mDiscoveredCommissioners); } private: - Dnssd::Resolver * mResolver = nullptr; Dnssd::DiscoveredNodeData mDiscoveredCommissioners[CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES]; }; diff --git a/src/controller/tests/TestCommissionableNodeController.cpp b/src/controller/tests/TestCommissionableNodeController.cpp index d50a267172..c2aafe500c 100644 --- a/src/controller/tests/TestCommissionableNodeController.cpp +++ b/src/controller/tests/TestCommissionableNodeController.cpp @@ -36,15 +36,14 @@ class MockResolver : public Resolver bool IsInitialized() override { return true; } void Shutdown() override {} void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {} - void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {} CHIP_ERROR ResolveNodeId(const PeerId & peerId) override { return ResolveNodeIdStatus; } void NodeIdResolutionNoLongerNeeded(const PeerId & peerId) override {} - CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return DiscoverCommissionersStatus; } - CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override + CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext &) override { return DiscoverCommissionersStatus; } + CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext &) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; } + CHIP_ERROR StopDiscovery(DiscoveryContext &) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override { return CHIP_ERROR_NOT_IMPLEMENTED; diff --git a/src/lib/dnssd/BUILD.gn b/src/lib/dnssd/BUILD.gn index 313ae72868..68b90f1f8a 100644 --- a/src/lib/dnssd/BUILD.gn +++ b/src/lib/dnssd/BUILD.gn @@ -34,6 +34,8 @@ static_library("dnssd") { "IPAddressSorter.cpp", "IPAddressSorter.h", "Resolver.h", + "ResolverProxy.cpp", + "ResolverProxy.h", "ServiceNaming.cpp", "ServiceNaming.h", "TxtFields.cpp", diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 88cafdebd2..577678f858 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -39,11 +39,11 @@ namespace { static void HandleNodeResolve(void * context, DnssdService * result, const Span & addresses, CHIP_ERROR error) { - ResolverDelegateProxy * proxy = static_cast(context); + DiscoveryContext * discoveryContext = static_cast(context); - if (CHIP_NO_ERROR != error) + if (error != CHIP_NO_ERROR) { - proxy->Release(); + discoveryContext->Release(); return; } @@ -51,31 +51,24 @@ static void HandleNodeResolve(void * context, DnssdService * result, const Span< result->ToDiscoveredNodeData(addresses, nodeData); nodeData.LogDetail(); - proxy->OnNodeDiscovered(nodeData); - proxy->Release(); + discoveryContext->OnNodeDiscovered(nodeData); + discoveryContext->Release(); } static void HandleNodeBrowse(void * context, DnssdService * services, size_t servicesSize, bool finalBrowse, CHIP_ERROR error) { - ResolverDelegateProxy * proxy = static_cast(context); + DiscoveryContext * discoveryContext = static_cast(context); if (error != CHIP_NO_ERROR) { - // We don't have access to the ResolverProxy here to clear out its - // mDiscoveryContext. The underlying implementation of - // ChipDnssdStopBrowse needs to handle a possibly-stale reference - // safely, so this won't lead to crashes, but it can lead to - // mis-behavior if a stale mDiscoveryContext happens to match a newer - // browse operation. - // - // TODO: Have a way to clear that state here. - proxy->Release(); + discoveryContext->ClearBrowseIdentifier(); + discoveryContext->Release(); return; } for (size_t i = 0; i < servicesSize; ++i) { - proxy->Retain(); + discoveryContext->Retain(); // For some platforms browsed services are already resolved, so verify if resolve is really needed or call resolve callback // Check if SRV, TXT and AAAA records were received in DNS responses @@ -92,15 +85,8 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser if (finalBrowse) { - // We don't have access to the ResolverProxy here to clear out its - // mDiscoveryContext. The underlying implementation of - // ChipDnssdStopBrowse needs to handle a possibly-stale reference - // safely, so this won't lead to crashes, but it can lead to - // mis-behavior if a stale mDiscoveryContext happens to match a newer - // browse operation. - // - // TODO: Have a way to clear that state here. - proxy->Release(); + discoveryContext->ClearBrowseIdentifier(); + discoveryContext->Release(); } } @@ -386,7 +372,6 @@ CHIP_ERROR DiscoveryImplPlatform::InitImpl() void DiscoveryImplPlatform::Shutdown() { VerifyOrReturn(mState != State::kUninitialized); - mResolverProxy.Shutdown(); ChipDnssdShutdown(); mState = State::kUninitialized; } @@ -399,20 +384,6 @@ void DiscoveryImplPlatform::HandleDnssdInit(void * context, CHIP_ERROR initError { publisher->mState = State::kInitialized; - // TODO: this is wrong, however we need resolverproxy initialized - // otherwise DiscoveryImplPlatform is not usable. - // - // We rely on the fact that resolverproxy does not use the endpoint - // nor does DiscoveryImplPlatform use it (since init will be called - // twice now) - // - // The problem is that: - // - DiscoveryImplPlatform contains a ResolverProxy - // - ResolverProxy::Init calls Dnssd::Resolver::Instance().Init - // which results in a recursive dependency (proxy initializes the - // class that it is contained in). - publisher->mResolverProxy.Init(nullptr); - // Post an event that will start advertising DeviceLayer::ChipDeviceEvent event; event.Type = DeviceLayer::DeviceEventType::kDnssdInitialized; @@ -657,125 +628,131 @@ void DiscoveryImplPlatform::NodeIdResolutionNoLongerNeeded(const PeerId & peerId ChipDnssdResolveNoLongerNeeded(name); } -CHIP_ERROR DiscoveryImplPlatform::DiscoverCommissionableNodes(DiscoveryFilter filter) -{ - ReturnErrorOnFailure(InitImpl()); - return mResolverProxy.DiscoverCommissionableNodes(filter); -} - -CHIP_ERROR DiscoveryImplPlatform::DiscoverCommissioners(DiscoveryFilter filter) -{ - ReturnErrorOnFailure(InitImpl()); - return mResolverProxy.DiscoverCommissioners(filter); -} - -CHIP_ERROR DiscoveryImplPlatform::StopDiscovery() +CHIP_ERROR DiscoveryImplPlatform::DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext & context) { ReturnErrorOnFailure(InitImpl()); - return mResolverProxy.StopDiscovery(); -} - -CHIP_ERROR DiscoveryImplPlatform::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) -{ - ReturnErrorOnFailure(InitImpl()); - return mResolverProxy.ReconfirmRecord(hostname, address, interfaceId); -} - -DiscoveryImplPlatform & DiscoveryImplPlatform::GetInstance() -{ - return sManager.get(); -} - -ServiceAdvertiser & chip::Dnssd::ServiceAdvertiser::Instance() -{ - return DiscoveryImplPlatform::GetInstance(); -} - -Resolver & chip::Dnssd::Resolver::Instance() -{ - return DiscoveryImplPlatform::GetInstance(); -} - -ResolverProxy::~ResolverProxy() -{ - Shutdown(); -} - -CHIP_ERROR ResolverProxy::DiscoverCommissionableNodes(DiscoveryFilter filter) -{ - StopDiscovery(); - - VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - mDelegate->Retain(); + StopDiscovery(context); if (filter.type == DiscoveryFilterType::kInstanceName) { - // when we have the instance name, no need to browse, only need to resolve + // When we have the instance name, no need to browse, only need to resolve. DnssdService service; ReturnErrorOnFailure(MakeServiceSubtype(service.mName, sizeof(service.mName), filter)); Platform::CopyString(service.mType, kCommissionableServiceName); service.mProtocol = DnssdServiceProtocol::kDnssdProtocolUdp; service.mAddressType = Inet::IPAddressType::kAny; - return ChipDnssdResolve(&service, Inet::InterfaceId::Null(), HandleNodeResolve, mDelegate); + + // Increase the reference count of the context to keep it alive until HandleNodeResolve is called back. + CHIP_ERROR error = ChipDnssdResolve(&service, Inet::InterfaceId::Null(), HandleNodeResolve, context.Retain()); + + if (error != CHIP_NO_ERROR) + { + context.Release(); + } + + return error; } char serviceName[kMaxCommissionableServiceNameSize]; ReturnErrorOnFailure(MakeServiceTypeName(serviceName, sizeof(serviceName), filter, DiscoveryType::kCommissionableNode)); intptr_t browseIdentifier; - ReturnErrorOnFailure(ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny, - Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate, &browseIdentifier)); - mDiscoveryContext.Emplace(browseIdentifier); - return CHIP_NO_ERROR; + // Increase the reference count of the context to keep it alive until HandleNodeBrowse is called back. + CHIP_ERROR error = ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny, + Inet::InterfaceId::Null(), HandleNodeBrowse, context.Retain(), &browseIdentifier); + + if (error == CHIP_NO_ERROR) + { + context.SetBrowseIdentifier(browseIdentifier); + } + else + { + context.Release(); + } + + return error; } -CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter) +CHIP_ERROR DiscoveryImplPlatform::DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext & context) { - StopDiscovery(); - - VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - mDelegate->Retain(); + ReturnErrorOnFailure(InitImpl()); + StopDiscovery(context); if (filter.type == DiscoveryFilterType::kInstanceName) { - // when we have the instance name, no need to browse, only need to resolve + // When we have the instance name, no need to browse, only need to resolve. DnssdService service; ReturnErrorOnFailure(MakeServiceSubtype(service.mName, sizeof(service.mName), filter)); Platform::CopyString(service.mType, kCommissionerServiceName); service.mProtocol = DnssdServiceProtocol::kDnssdProtocolUdp; service.mAddressType = Inet::IPAddressType::kAny; - return ChipDnssdResolve(&service, Inet::InterfaceId::Null(), HandleNodeResolve, mDelegate); + + // Increase the reference count of the context to keep it alive until HandleNodeResolve is called back. + CHIP_ERROR error = ChipDnssdResolve(&service, Inet::InterfaceId::Null(), HandleNodeResolve, context.Retain()); + + if (error != CHIP_NO_ERROR) + { + context.Release(); + } } char serviceName[kMaxCommissionerServiceNameSize]; ReturnErrorOnFailure(MakeServiceTypeName(serviceName, sizeof(serviceName), filter, DiscoveryType::kCommissionerNode)); intptr_t browseIdentifier; - ReturnErrorOnFailure(ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny, - Inet::InterfaceId::Null(), HandleNodeBrowse, mDelegate, &browseIdentifier)); - mDiscoveryContext.Emplace(browseIdentifier); - return CHIP_NO_ERROR; + // Increase the reference count of the context to keep it alive until HandleNodeBrowse is called back. + CHIP_ERROR error = ChipDnssdBrowse(serviceName, DnssdServiceProtocol::kDnssdProtocolUdp, Inet::IPAddressType::kAny, + Inet::InterfaceId::Null(), HandleNodeBrowse, context.Retain(), &browseIdentifier); + + if (error == CHIP_NO_ERROR) + { + context.SetBrowseIdentifier(browseIdentifier); + } + else + { + context.Release(); + } + + return error; } -CHIP_ERROR ResolverProxy::StopDiscovery() +CHIP_ERROR DiscoveryImplPlatform::StopDiscovery(DiscoveryContext & context) { - if (!mDiscoveryContext.HasValue()) + if (!context.GetBrowseIdentifier().HasValue()) { // No discovery going on. return CHIP_NO_ERROR; } - CHIP_ERROR err = ChipDnssdStopBrowse(mDiscoveryContext.Value()); - mDiscoveryContext.ClearValue(); - return err; + const auto browseIdentifier = context.GetBrowseIdentifier().Value(); + context.ClearBrowseIdentifier(); + + return ChipDnssdStopBrowse(browseIdentifier); } -CHIP_ERROR ResolverProxy::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) +CHIP_ERROR DiscoveryImplPlatform::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) { + ReturnErrorOnFailure(InitImpl()); + return ChipDnssdReconfirmRecord(hostname, address, interfaceId); } +DiscoveryImplPlatform & DiscoveryImplPlatform::GetInstance() +{ + return sManager.get(); +} + +ServiceAdvertiser & chip::Dnssd::ServiceAdvertiser::Instance() +{ + return DiscoveryImplPlatform::GetInstance(); +} + +Resolver & chip::Dnssd::Resolver::Instance() +{ + return DiscoveryImplPlatform::GetInstance(); +} + } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Discovery_ImplPlatform.h b/src/lib/dnssd/Discovery_ImplPlatform.h index 994112e9be..a3f79796ca 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.h +++ b/src/lib/dnssd/Discovery_ImplPlatform.h @@ -23,7 +23,6 @@ #include #include #include -#include #include #include @@ -52,15 +51,11 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver // Members that implement Resolver interface. void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mOperationalDelegate = delegate; } - void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override - { - mResolverProxy.SetCommissioningDelegate(delegate); - } CHIP_ERROR ResolveNodeId(const PeerId & peerId) override; void NodeIdResolutionNoLongerNeeded(const PeerId & peerId) override; - CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; - CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; - CHIP_ERROR StopDiscovery() override; + CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext & context) override; + CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext & context) override; + CHIP_ERROR StopDiscovery(DiscoveryContext & context) override; CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override; static DiscoveryImplPlatform & GetInstance(); @@ -97,7 +92,6 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver State mState = State::kUninitialized; uint8_t mCommissionableInstanceName[sizeof(uint64_t)]; - ResolverProxy mResolverProxy; OperationalResolveDelegate * mOperationalDelegate = nullptr; friend class Global; diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 81c8e97548..787cfa195d 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -333,6 +334,43 @@ class CommissioningResolveDelegate virtual void OnNodeDiscovered(const DiscoveredNodeData & nodeData) = 0; }; +/** + * Node discovery context class. + * + * This class enables multiple clients of the global DNS-SD resolver to start simultaneous + * discovery operations. + * + * An object of this class is shared between a resolver client and the concrete resolver + * implementation. The client is responsible for allocating the context and passing it to + * the resolver when initiating a discovery operation. The resolver, in turn, is supposed to retain + * the context until the operation is finished. This allows the client to release the ownership of + * the context at any time without putting the resolver at risk of using a deleted object. + */ +class DiscoveryContext : public ReferenceCounted +{ +public: + void SetBrowseIdentifier(intptr_t identifier) { mBrowseIdentifier.Emplace(identifier); } + void ClearBrowseIdentifier() { mBrowseIdentifier.ClearValue(); } + const Optional & GetBrowseIdentifier() const { return mBrowseIdentifier; } + + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) { mCommissioningDelegate = delegate; } + void OnNodeDiscovered(const DiscoveredNodeData & nodeData) + { + if (mCommissioningDelegate != nullptr) + { + mCommissioningDelegate->OnNodeDiscovered(nodeData); + } + else + { + ChipLogError(Discovery, "Missing commissioning delegate. Data discarded"); + } + } + +private: + CommissioningResolveDelegate * mCommissioningDelegate = nullptr; + Optional mBrowseIdentifier; +}; + /** * Interface for resolving CHIP DNS-SD services */ @@ -366,11 +404,6 @@ class Resolver */ virtual void SetOperationalDelegate(OperationalResolveDelegate * delegate) = 0; - /** - * If nullptr is passed, the previously registered delegate is unregistered. - */ - virtual void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) = 0; - /** * Requests resolution of the given operational node service. * @@ -412,18 +445,26 @@ class Resolver /** * Finds all commissionable nodes matching the given filter. * - * Whenever a new matching node is found and a resolver delegate has been registered, - * the node information is passed to the delegate's `OnNodeDiscoveryComplete` method. + * Whenever a new matching node is found, the node information is passed to + * the `OnNodeDiscovered` method of the commissioning delegate configured + * in the context object. + * + * This method is expected to increase the reference count of the context + * object for as long as it takes to complete the discovery request. */ - virtual CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) = 0; + virtual CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext & context) = 0; /** * Finds all commissioner nodes matching the given filter. * - * Whenever a new matching node is found and a resolver delegate has been registered, - * the node information is passed to the delegate's `OnNodeDiscoveryComplete` method. + * Whenever a new matching node is found, the node information is passed to + * the `OnNodeDiscovered` method of the commissioning delegate configured + * in the context object. + * + * This method is expected to increase the reference count of the context + * object for as long as it takes to complete the discovery request. */ - virtual CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) = 0; + virtual CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext & context) = 0; /** * Stop discovery (of commissionable or commissioner nodes). @@ -431,7 +472,7 @@ class Resolver * Some back ends may not support stopping discovery, so consumers should * not assume they will stop getting callbacks after calling this. */ - virtual CHIP_ERROR StopDiscovery() = 0; + virtual CHIP_ERROR StopDiscovery(DiscoveryContext & context) = 0; /** * Verify the validity of an address that appears to be out of date (for example diff --git a/src/lib/dnssd/ResolverProxy.cpp b/src/lib/dnssd/ResolverProxy.cpp new file mode 100644 index 0000000000..3e7446ebbb --- /dev/null +++ b/src/lib/dnssd/ResolverProxy.cpp @@ -0,0 +1,66 @@ +/* + * + * 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 "ResolverProxy.h" + +#include + +namespace chip { +namespace Dnssd { + +CHIP_ERROR ResolverProxy::Init(Inet::EndPointManager * udpEndPoint) +{ + VerifyOrReturnError(mContext == nullptr, CHIP_ERROR_INCORRECT_STATE); + + ReturnErrorOnFailure(mResolver.Init(udpEndPoint)); + mContext = Platform::New(); + VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_NO_MEMORY); + + return CHIP_NO_ERROR; +} + +void ResolverProxy::Shutdown() +{ + VerifyOrReturn(mContext != nullptr); + mContext->SetCommissioningDelegate(nullptr); + mContext->Release(); + mContext = nullptr; +} + +CHIP_ERROR ResolverProxy::DiscoverCommissionableNodes(DiscoveryFilter filter) +{ + VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_INCORRECT_STATE); + + return mResolver.DiscoverCommissionableNodes(filter, *mContext); +} + +CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter) +{ + VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_INCORRECT_STATE); + + return mResolver.DiscoverCommissioners(filter, *mContext); +} + +CHIP_ERROR ResolverProxy::StopDiscovery() +{ + VerifyOrReturnError(mContext != nullptr, CHIP_ERROR_INCORRECT_STATE); + + return mResolver.StopDiscovery(*mContext); +} + +} // namespace Dnssd +} // namespace chip diff --git a/src/lib/dnssd/ResolverProxy.h b/src/lib/dnssd/ResolverProxy.h index 176316ef8c..09a50e89c1 100644 --- a/src/lib/dnssd/ResolverProxy.h +++ b/src/lib/dnssd/ResolverProxy.h @@ -17,112 +17,48 @@ #pragma once -#include #include namespace chip { namespace Dnssd { -class ResolverDelegateProxy : public ReferenceCounted, public CommissioningResolveDelegate - +/** + * Convenience class for discovering Matter commissioners and commissionable nodes. + * + * The class simplifies the usage of the global DNS-SD resolver to discover Matter nodes by managing + * the lifetime of the discovery context object. + * + * The proxy provides a method to register an external commissioning delegate whose + * `OnNodeDiscovered` method is called whenever a new node is discovered. The delegate is + * automatically unregistered when the proxy is shut down or destroyed. + * + * The proxy can be safely shut down or destroyed even if the last discovery operation has not + * finished yet. + */ +class ResolverProxy { public: - void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) { mCommissioningDelegate = delegate; } - - // CommissioningResolveDelegate - void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override - { - if (mCommissioningDelegate != nullptr) - { - mCommissioningDelegate->OnNodeDiscovered(nodeData); - } - else - { - ChipLogError(Discovery, "Missing commissioning delegate. Data discarded."); - } - } + explicit ResolverProxy(Resolver * resolver = nullptr) : mResolver(resolver != nullptr ? *resolver : Resolver::Instance()) {} + ~ResolverProxy() { Shutdown(); } -private: - CommissioningResolveDelegate * mCommissioningDelegate = nullptr; -}; + CHIP_ERROR Init(Inet::EndPointManager * udpEndPoint = nullptr); + void Shutdown(); -class ResolverProxy : public Resolver -{ -public: - ResolverProxy() {} - ~ResolverProxy() override; - - // Resolver interface. - CHIP_ERROR Init(Inet::EndPointManager * udpEndPoint = nullptr) override + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) { - ReturnErrorOnFailure(chip::Dnssd::Resolver::Instance().Init(udpEndPoint)); - VerifyOrReturnError(mDelegate == nullptr, CHIP_ERROR_INCORRECT_STATE); - mDelegate = chip::Platform::New(); - - if (mDelegate != nullptr) + if (mContext != nullptr) { - if (mPreInitCommissioningDelegate != nullptr) - { - ChipLogProgress(Discovery, "Setting commissioning delegate post init"); - mDelegate->SetCommissioningDelegate(mPreInitCommissioningDelegate); - mPreInitCommissioningDelegate = nullptr; - } + mContext->SetCommissioningDelegate(delegate); } - - return mDelegate != nullptr ? CHIP_NO_ERROR : CHIP_ERROR_NO_MEMORY; } - bool IsInitialized() override { return Resolver::Instance().IsInitialized(); } - - void SetOperationalDelegate(OperationalResolveDelegate * delegate) override - { - /// Unfortunately cannot remove this method since it is in a Resolver interface. - ChipLogError(Discovery, "!!! Operational proxy does NOT support operational discovery"); - ChipLogError(Discovery, "!!! Please use AddressResolver or DNSSD Resolver directly"); - chipDie(); // force detection of invalid usages. - } - - void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override - { - if (mDelegate != nullptr) - { - mDelegate->SetCommissioningDelegate(delegate); - } - else - { - if (delegate != nullptr) - { - ChipLogError(Discovery, "Delaying proxy of commissioning discovery: missing delegate"); - } - mPreInitCommissioningDelegate = delegate; - } - } - - void Shutdown() override - { - VerifyOrReturn(mDelegate != nullptr); - mDelegate->SetCommissioningDelegate(nullptr); - mDelegate->Release(); - mDelegate = nullptr; - } - - CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; - CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; - CHIP_ERROR StopDiscovery() override; - CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override; - - // TODO: ResolverProxy should not be used anymore to implement operational node resolution - // This method still here because Resolver interface requires it - CHIP_ERROR ResolveNodeId(const PeerId & peerId) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - void NodeIdResolutionNoLongerNeeded(const PeerId & peerId) override {} + CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()); + CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()); + CHIP_ERROR StopDiscovery(); private: - ResolverDelegateProxy * mDelegate = nullptr; - CommissioningResolveDelegate * mPreInitCommissioningDelegate = nullptr; - - // While discovery (commissionable or commissioner) is ongoing, - // mDiscoveryContext may have a value to allow StopDiscovery to work. - Optional mDiscoveryContext; + Resolver & mResolver; + DiscoveryContext * mContext = nullptr; }; } // namespace Dnssd diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 416c747d83..6446196536 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -274,6 +273,7 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate { GlobalMinimalMdnsServer::Instance().SetResponseDelegate(this); } + ~MinMdnsResolver() { SetDiscoveryContext(nullptr); } //// MdnsPacketDelegate implementation void OnMdnsPacketData(const BytesRange & data, const chip::Inet::IPPacketInfo * info) override; @@ -283,21 +283,21 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate bool IsInitialized() override; void Shutdown() override; void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mOperationalDelegate = delegate; } - void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override { mCommissioningDelegate = delegate; } CHIP_ERROR ResolveNodeId(const PeerId & peerId) override; void NodeIdResolutionNoLongerNeeded(const PeerId & peerId) override; - CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; - CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; - CHIP_ERROR StopDiscovery() override; + CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext & context) override; + CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext & context) override; + CHIP_ERROR StopDiscovery(DiscoveryContext & context) override; CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override; private: - OperationalResolveDelegate * mOperationalDelegate = nullptr; - CommissioningResolveDelegate * mCommissioningDelegate = nullptr; - System::Layer * mSystemLayer = nullptr; + OperationalResolveDelegate * mOperationalDelegate = nullptr; + DiscoveryContext * mDiscoveryContext = nullptr; + System::Layer * mSystemLayer = nullptr; ActiveResolveAttempts mActiveResolves; PacketParser mPacketParser; + void SetDiscoveryContext(DiscoveryContext * context); void ScheduleIpAddressResolve(SerializedQNameIterator hostName); CHIP_ERROR SendAllPendingQueries(); @@ -332,6 +332,21 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate char qnameStorage[kMaxQnameSize]; }; +void MinMdnsResolver::SetDiscoveryContext(DiscoveryContext * context) +{ + if (mDiscoveryContext != nullptr) + { + mDiscoveryContext->Release(); + } + + if (context != nullptr) + { + context->Retain(); + } + + mDiscoveryContext = context; +} + void MinMdnsResolver::ScheduleIpAddressResolve(SerializedQNameIterator hostName) { HeapQName target(hostName); @@ -409,9 +424,9 @@ void MinMdnsResolver::AdvancePendingResolverStates() if (discoveredNodeIsRelevant) { - if (mCommissioningDelegate != nullptr) + if (mDiscoveryContext != nullptr) { - mCommissioningDelegate->OnNodeDiscovered(nodeData); + mDiscoveryContext->OnNodeDiscovered(nodeData); } else { @@ -670,18 +685,26 @@ void MinMdnsResolver::ExpireIncrementalResolvers() } } -CHIP_ERROR MinMdnsResolver::DiscoverCommissionableNodes(DiscoveryFilter filter) +CHIP_ERROR MinMdnsResolver::DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext & context) { + // minmdns currently supports only one discovery context at a time so override the previous context + SetDiscoveryContext(&context); + return BrowseNodes(DiscoveryType::kCommissionableNode, filter); } -CHIP_ERROR MinMdnsResolver::DiscoverCommissioners(DiscoveryFilter filter) +CHIP_ERROR MinMdnsResolver::DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext & context) { + // minmdns currently supports only one discovery context at a time so override the previous context + SetDiscoveryContext(&context); + return BrowseNodes(DiscoveryType::kCommissionerNode, filter); } -CHIP_ERROR MinMdnsResolver::StopDiscovery() +CHIP_ERROR MinMdnsResolver::StopDiscovery(DiscoveryContext & context) { + SetDiscoveryContext(nullptr); + return mActiveResolves.CompleteAllBrowses(); } @@ -740,37 +763,5 @@ Resolver & chip::Dnssd::Resolver::Instance() return gResolver; } -ResolverProxy::~ResolverProxy() -{ - // TODO: this is a hack: resolver proxies used for commissionable discovery - // and they don't interact well with each other. - gResolver.SetCommissioningDelegate(nullptr); - Shutdown(); -} - -CHIP_ERROR ResolverProxy::DiscoverCommissionableNodes(DiscoveryFilter filter) -{ - VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - chip::Dnssd::Resolver::Instance().SetCommissioningDelegate(mDelegate); - return chip::Dnssd::Resolver::Instance().DiscoverCommissionableNodes(filter); -} - -CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter) -{ - VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - chip::Dnssd::Resolver::Instance().SetCommissioningDelegate(mDelegate); - return chip::Dnssd::Resolver::Instance().DiscoverCommissioners(filter); -} - -CHIP_ERROR ResolverProxy::StopDiscovery() -{ - return CHIP_ERROR_NOT_IMPLEMENTED; -} - -CHIP_ERROR ResolverProxy::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) -{ - return CHIP_ERROR_NOT_IMPLEMENTED; -} - } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Resolver_ImplNone.cpp b/src/lib/dnssd/Resolver_ImplNone.cpp index 3d0d97f57c..9a1f40f300 100644 --- a/src/lib/dnssd/Resolver_ImplNone.cpp +++ b/src/lib/dnssd/Resolver_ImplNone.cpp @@ -17,7 +17,6 @@ #include "Resolver.h" -#include #include namespace chip { @@ -31,7 +30,6 @@ class NoneResolver : public Resolver bool IsInitialized() override { return true; } void Shutdown() override {} void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {} - void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {} CHIP_ERROR ResolveNodeId(const PeerId & peerId) override { @@ -42,12 +40,15 @@ class NoneResolver : public Resolver { ChipLogError(Discovery, "Failed to stop resolving node ID: dnssd resolving not available"); } - CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override + CHIP_ERROR DiscoverCommissionableNodes(DiscoveryFilter filter, DiscoveryContext & context) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - CHIP_ERROR StopDiscovery() override { return CHIP_ERROR_NOT_IMPLEMENTED; } + CHIP_ERROR DiscoverCommissioners(DiscoveryFilter filter, DiscoveryContext & context) override + { + return CHIP_ERROR_NOT_IMPLEMENTED; + } + CHIP_ERROR StopDiscovery(DiscoveryContext & context) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) override { return CHIP_ERROR_NOT_IMPLEMENTED; @@ -63,30 +64,5 @@ Resolver & chip::Dnssd::Resolver::Instance() return gResolver; } -ResolverProxy::~ResolverProxy() -{ - Shutdown(); -} - -CHIP_ERROR ResolverProxy::DiscoverCommissionableNodes(DiscoveryFilter filter) -{ - return CHIP_ERROR_NOT_IMPLEMENTED; -} - -CHIP_ERROR ResolverProxy::DiscoverCommissioners(DiscoveryFilter filter) -{ - return CHIP_ERROR_NOT_IMPLEMENTED; -} - -CHIP_ERROR ResolverProxy::StopDiscovery() -{ - return CHIP_ERROR_NOT_IMPLEMENTED; -} - -CHIP_ERROR ResolverProxy::ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) -{ - return CHIP_ERROR_NOT_IMPLEMENTED; -} - } // namespace Dnssd } // namespace chip diff --git a/src/lib/shell/commands/Dns.cpp b/src/lib/shell/commands/Dns.cpp index 83fd12fc0b..5375c47d0f 100644 --- a/src/lib/shell/commands/Dns.cpp +++ b/src/lib/shell/commands/Dns.cpp @@ -198,12 +198,7 @@ bool ParseSubType(int argc, char ** argv, Dnssd::DiscoveryFilter & filter) CHIP_ERROR BrowseCommissionableHandler(int argc, char ** argv) { Dnssd::DiscoveryFilter filter; - - if (!ParseSubType(argc, argv, filter)) - { - streamer_printf(streamer_get(), "Invalid argument\r\n"); - return CHIP_ERROR_INVALID_ARGUMENT; - } + VerifyOrReturnError(ParseSubType(argc, argv, filter), CHIP_ERROR_INVALID_ARGUMENT); streamer_printf(streamer_get(), "Browsing commissionable nodes...\r\n"); @@ -213,12 +208,7 @@ CHIP_ERROR BrowseCommissionableHandler(int argc, char ** argv) CHIP_ERROR BrowseCommissionerHandler(int argc, char ** argv) { Dnssd::DiscoveryFilter filter; - - if (!ParseSubType(argc, argv, filter)) - { - streamer_printf(streamer_get(), "Invalid argument\r\n"); - return CHIP_ERROR_INVALID_ARGUMENT; - } + VerifyOrReturnError(ParseSubType(argc, argv, filter), CHIP_ERROR_INVALID_ARGUMENT); streamer_printf(streamer_get(), "Browsing commissioners...\r\n"); @@ -233,6 +223,9 @@ CHIP_ERROR BrowseHandler(int argc, char ** argv) return CHIP_NO_ERROR; } + sResolverProxy.Init(DeviceLayer::UDPEndPointManager()); + sResolverProxy.SetCommissioningDelegate(&sDnsShellResolverDelegate); + return sShellDnsBrowseSubcommands.ExecCommand(argc, argv); } @@ -244,9 +237,6 @@ CHIP_ERROR DnsHandler(int argc, char ** argv) return CHIP_NO_ERROR; } - sResolverProxy.Init(DeviceLayer::UDPEndPointManager()); - sResolverProxy.SetCommissioningDelegate(&sDnsShellResolverDelegate); - return sShellDnsSubcommands.ExecCommand(argc, argv); } From 7a2cd5563827765628ad252c3ef18729e96dcf49 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Mon, 20 Nov 2023 10:00:57 +0100 Subject: [PATCH 2/2] [nrf noup] Allow selecting DNS-SD implementation at runtime Add Resolver::SetInstance() and ServiceAdvertiser::SetInstance() methods for dynamically changing the system-wide DNS-SD implementation used by Matter. Also, allow for building "minimal" and "platform" DNS-SD implementations together. --- config/nrfconnect/chip-module/CMakeLists.txt | 8 ++-- src/lib/dnssd/Advertiser.cpp | 41 ++++++++++++++++++++ src/lib/dnssd/Advertiser.h | 21 +++++++++- src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp | 6 ++- src/lib/dnssd/Advertiser_ImplNone.cpp | 6 ++- src/lib/dnssd/BUILD.gn | 25 ++++++++++-- src/lib/dnssd/Discovery_ImplPlatform.cpp | 8 +++- src/lib/dnssd/Resolver.cpp | 41 ++++++++++++++++++++ src/lib/dnssd/Resolver.h | 19 ++++++++- src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 6 ++- src/lib/dnssd/Resolver_ImplNone.cpp | 6 ++- src/platform/device.gni | 6 +++ src/platform/nrfconnect/BUILD.gn | 2 +- 13 files changed, 179 insertions(+), 16 deletions(-) create mode 100644 src/lib/dnssd/Advertiser.cpp create mode 100644 src/lib/dnssd/Resolver.cpp diff --git a/config/nrfconnect/chip-module/CMakeLists.txt b/config/nrfconnect/chip-module/CMakeLists.txt index 5756074545..0b8ce0be6e 100644 --- a/config/nrfconnect/chip-module/CMakeLists.txt +++ b/config/nrfconnect/chip-module/CMakeLists.txt @@ -136,6 +136,8 @@ matter_add_gn_arg_bool ("chip_malloc_sys_heap" CONFIG_CHIP_MA matter_add_gn_arg_bool ("chip_enable_wifi" CONFIG_WIFI_NRF700X) matter_add_gn_arg_bool ("chip_system_config_provide_statistics" CONFIG_CHIP_STATISTICS) matter_add_gn_arg_bool ("chip_enable_icd_server" CONFIG_CHIP_ENABLE_ICD_SUPPORT) +matter_add_gn_arg_bool ("chip_mdns_minimal" CONFIG_WIFI_NRF700X) +matter_add_gn_arg_bool ("chip_mdns_platform" CONFIG_NET_L2_OPENTHREAD) if (CONFIG_CHIP_FACTORY_DATA) matter_add_gn_arg_bool("chip_use_transitional_commissionable_data_provider" FALSE) @@ -149,10 +151,10 @@ if (CONFIG_CHIP_ROTATING_DEVICE_ID) matter_add_gn_arg_bool("chip_enable_additional_data_advertising" TRUE) endif() -if (CONFIG_NET_L2_OPENTHREAD) - matter_add_gn_arg_string("chip_mdns" "platform") -elseif(CONFIG_WIFI_NRF700X) +if(CONFIG_WIFI_NRF700X) matter_add_gn_arg_string("chip_mdns" "minimal") +elseif (CONFIG_NET_L2_OPENTHREAD) + matter_add_gn_arg_string("chip_mdns" "platform") else() matter_add_gn_arg_string("chip_mdns" "none") endif() diff --git a/src/lib/dnssd/Advertiser.cpp b/src/lib/dnssd/Advertiser.cpp new file mode 100644 index 0000000000..1f3d1b3669 --- /dev/null +++ b/src/lib/dnssd/Advertiser.cpp @@ -0,0 +1,41 @@ +/* + * + * 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 "Advertiser.h" + +namespace chip { +namespace Dnssd { + +ServiceAdvertiser * ServiceAdvertiser::sInstance = nullptr; + +ServiceAdvertiser & ServiceAdvertiser::Instance() +{ + if (sInstance == nullptr) + { + sInstance = &GetDefaultAdvertiser(); + } + + return *sInstance; +} + +void ServiceAdvertiser::SetInstance(ServiceAdvertiser & advertiser) +{ + sInstance = &advertiser; +} + +} // namespace Dnssd +} // namespace chip diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h index 322ee3c871..b0f3d8a3bc 100644 --- a/src/lib/dnssd/Advertiser.h +++ b/src/lib/dnssd/Advertiser.h @@ -349,9 +349,28 @@ class ServiceAdvertiser */ virtual CHIP_ERROR UpdateCommissionableInstanceName() = 0; - /// Provides the system-wide implementation of the service advertiser + /** + * Returns the system-wide implementation of the service advertiser. + * + * The method returns a reference to the advertiser object configured by + * a user using the \c ServiceAdvertiser::SetInstance() method, or the + * default advertiser returned by the \c GetDefaultAdvertiser() function. + */ static ServiceAdvertiser & Instance(); + + /** + * Sets the system-wide implementation of the service advertiser. + */ + static void SetInstance(ServiceAdvertiser & advertiser); + +private: + static ServiceAdvertiser * sInstance; }; +/** + * Returns the default implementation of the service advertiser. + */ +extern ServiceAdvertiser & GetDefaultAdvertiser(); + } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp index 26c0a37d0d..2403502aae 100644 --- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp @@ -948,10 +948,14 @@ void AdvertiserMinMdns::AdvertiseRecords(BroadcastAdvertiseType type) AdvertiserMinMdns gAdvertiser; } // namespace -ServiceAdvertiser & ServiceAdvertiser::Instance() +#if CHIP_DNSSD_DEFAULT_MINIMAL + +ServiceAdvertiser & GetDefaultAdvertiser() { return gAdvertiser; } +#endif // CHIP_DNSSD_DEFAULT_MINIMAL + } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Advertiser_ImplNone.cpp b/src/lib/dnssd/Advertiser_ImplNone.cpp index 9493524769..2c1ed7f09b 100644 --- a/src/lib/dnssd/Advertiser_ImplNone.cpp +++ b/src/lib/dnssd/Advertiser_ImplNone.cpp @@ -77,10 +77,14 @@ NoneAdvertiser gAdvertiser; } // namespace -ServiceAdvertiser & ServiceAdvertiser::Instance() +#if CHIP_DNSSD_DEFAULT_NONE + +ServiceAdvertiser & GetDefaultAdvertiser() { return gAdvertiser; } +#endif // CHIP_DNSSD_DEFAULT_NONE + } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/BUILD.gn b/src/lib/dnssd/BUILD.gn index 68b90f1f8a..69df6e45c9 100644 --- a/src/lib/dnssd/BUILD.gn +++ b/src/lib/dnssd/BUILD.gn @@ -30,9 +30,11 @@ static_library("dnssd") { ] sources = [ + "Advertiser.cpp", "Advertiser.h", "IPAddressSorter.cpp", "IPAddressSorter.h", + "Resolver.cpp", "Resolver.h", "ResolverProxy.cpp", "ResolverProxy.h", @@ -42,12 +44,17 @@ static_library("dnssd") { "TxtFields.h", ] + assert(chip_mdns == "none" || chip_mdns == "minimal" || chip_mdns == "platform", + "Please select a valid value for chip_mdns") + if (chip_mdns == "none") { sources += [ "Advertiser_ImplNone.cpp", "Resolver_ImplNone.cpp", ] - } else if (chip_mdns == "minimal") { + } + + if (chip_mdns_minimal) { sources += [ "ActiveResolveAttempts.cpp", "ActiveResolveAttempts.h", @@ -63,15 +70,25 @@ static_library("dnssd") { "${chip_root}/src/tracing", "${chip_root}/src/tracing:macros", ] - } else if (chip_mdns == "platform") { + } + + if (chip_mdns_platform) { sources += [ "Discovery_ImplPlatform.cpp", "Discovery_ImplPlatform.h", ] public_deps += [ "${chip_root}/src/platform" ] - } else { - assert(false, "Unknown Dnssd advertiser implementation.") } + _chip_dnssd_default_none = chip_mdns == "none" + _chip_dnssd_default_minimal = chip_mdns == "minimal" + _chip_dnssd_default_platform = chip_mdns == "platform" + + defines = [ + "CHIP_DNSSD_DEFAULT_NONE=${_chip_dnssd_default_none}", + "CHIP_DNSSD_DEFAULT_MINIMAL=${_chip_dnssd_default_minimal}", + "CHIP_DNSSD_DEFAULT_PLATFORM=${_chip_dnssd_default_platform}", + ] + cflags = [ "-Wconversion" ] } diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 577678f858..1fd7e08618 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -744,15 +744,19 @@ DiscoveryImplPlatform & DiscoveryImplPlatform::GetInstance() return sManager.get(); } -ServiceAdvertiser & chip::Dnssd::ServiceAdvertiser::Instance() +#if CHIP_DNSSD_DEFAULT_PLATFORM + +ServiceAdvertiser & GetDefaultAdvertiser() { return DiscoveryImplPlatform::GetInstance(); } -Resolver & chip::Dnssd::Resolver::Instance() +Resolver & GetDefaultResolver() { return DiscoveryImplPlatform::GetInstance(); } +#endif // CHIP_DNSSD_DEFAULT_PLATFORM + } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Resolver.cpp b/src/lib/dnssd/Resolver.cpp new file mode 100644 index 0000000000..4ed4de3e0a --- /dev/null +++ b/src/lib/dnssd/Resolver.cpp @@ -0,0 +1,41 @@ +/* + * + * 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 "Resolver.h" + +namespace chip { +namespace Dnssd { + +Resolver * Resolver::sInstance = nullptr; + +Resolver & Resolver::Instance() +{ + if (sInstance == nullptr) + { + sInstance = &GetDefaultResolver(); + } + + return *sInstance; +} + +void Resolver::SetInstance(Resolver & resolver) +{ + sInstance = &resolver; +} + +} // namespace Dnssd +} // namespace chip diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 787cfa195d..b4f992ef21 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -481,10 +481,27 @@ class Resolver virtual CHIP_ERROR ReconfirmRecord(const char * hostname, Inet::IPAddress address, Inet::InterfaceId interfaceId) = 0; /** - * Provides the system-wide implementation of the service resolver + * Returns the system-wide implementation of the service resolver. + * + * The method returns a reference to the resolver object configured by + * a user using the \c Resolver::SetInstance() method, or the default + * resolver returned by the \c GetDefaultResolver() function. */ static Resolver & Instance(); + + /** + * Overrides the default implementation of the service resolver + */ + static void SetInstance(Resolver & resolver); + +private: + static Resolver * sInstance; }; +/** + * Returns the default implementation of the service resolver. + */ +extern Resolver & GetDefaultResolver(); + } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 6446196536..b74d0ae87d 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -758,10 +758,14 @@ MinMdnsResolver gResolver; } // namespace -Resolver & chip::Dnssd::Resolver::Instance() +#if CHIP_DNSSD_DEFAULT_MINIMAL + +Resolver & GetDefaultResolver() { return gResolver; } +#endif // CHIP_DNSSD_DEFAULT_MINIMAL + } // namespace Dnssd } // namespace chip diff --git a/src/lib/dnssd/Resolver_ImplNone.cpp b/src/lib/dnssd/Resolver_ImplNone.cpp index 9a1f40f300..36e2bd9aea 100644 --- a/src/lib/dnssd/Resolver_ImplNone.cpp +++ b/src/lib/dnssd/Resolver_ImplNone.cpp @@ -59,10 +59,14 @@ NoneResolver gResolver; } // namespace -Resolver & chip::Dnssd::Resolver::Instance() +#if CHIP_DNSSD_DEFAULT_NONE + +Resolver & GetDefaultResolver() { return gResolver; } +#endif // CHIP_DNSSD_DEFAULT_NONE + } // namespace Dnssd } // namespace chip diff --git a/src/platform/device.gni b/src/platform/device.gni index cf1dc07325..6ca09a5985 100644 --- a/src/platform/device.gni +++ b/src/platform/device.gni @@ -122,6 +122,12 @@ if (chip_device_platform == "nxp" && chip_enable_openthread) { chip_mdns = "minimal" } +declare_args() { + # Select DNS-SD components to build + chip_mdns_minimal = chip_mdns == "minimal" + chip_mdns_platform = chip_mdns == "platform" +} + _chip_device_layer = "none" if (chip_device_platform == "cc13x2_26x2") { _chip_device_layer = "cc13xx_26xx/cc13x2_26x2" diff --git a/src/platform/nrfconnect/BUILD.gn b/src/platform/nrfconnect/BUILD.gn index 5a11b4f3d0..df5e49f3a8 100644 --- a/src/platform/nrfconnect/BUILD.gn +++ b/src/platform/nrfconnect/BUILD.gn @@ -86,7 +86,7 @@ static_library("nrfconnect") { "ThreadStackManagerImpl.h", ] - if (chip_mdns == "platform") { + if (chip_mdns_platform) { sources += [ "../OpenThread/DnssdImpl.cpp", "../OpenThread/OpenThreadDnssdImpl.cpp",