diff --git a/src/controller/tests/TestCommissionableNodeController.cpp b/src/controller/tests/TestCommissionableNodeController.cpp index b852e1af747d3c..f3e539c2b8eeb8 100644 --- a/src/controller/tests/TestCommissionableNodeController.cpp +++ b/src/controller/tests/TestCommissionableNodeController.cpp @@ -33,7 +33,8 @@ class MockResolver : public Resolver public: CHIP_ERROR Init(chip::Inet::EndPointManager * udpEndPointManager) override { return InitStatus; } void Shutdown() override {} - void SetResolverDelegate(ResolverDelegate *) override {} + void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {} + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {} CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override { return ResolveNodeIdStatus; } CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override { return FindCommissionersStatus; } CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override { return CHIP_ERROR_NOT_IMPLEMENTED; } diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 741660bdf1e465..f9c09b92d88b98 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -77,7 +77,7 @@ static void HandleNodeResolve(void * context, DnssdService * result, CHIP_ERROR FillNodeDataFromTxt(key, val, nodeData); } - proxy->OnNodeDiscoveryComplete(nodeData); + proxy->OnNodeDiscovered(nodeData); proxy->Release(); } @@ -86,7 +86,7 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO ResolverDelegateProxy * proxy = static_cast(context); if (CHIP_NO_ERROR != error) { - proxy->OnNodeIdResolutionFailed(PeerId(), error); + proxy->OnOperationalNodeResolutionFailed(PeerId(), error); proxy->Release(); return; } @@ -95,7 +95,7 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO if (result == nullptr) { - proxy->OnNodeIdResolutionFailed(PeerId(), CHIP_ERROR_UNKNOWN_RESOURCE_ID); + proxy->OnOperationalNodeResolutionFailed(PeerId(), CHIP_ERROR_UNKNOWN_RESOURCE_ID); proxy->Release(); return; } @@ -106,7 +106,7 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO error = ExtractIdFromInstanceName(result->mName, &peerId); if (CHIP_NO_ERROR != error) { - proxy->OnNodeIdResolutionFailed(PeerId(), error); + proxy->OnOperationalNodeResolutionFailed(PeerId(), error); proxy->Release(); return; } @@ -137,7 +137,7 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO #if CHIP_CONFIG_MDNS_CACHE_SIZE > 0 LogErrorOnFailure(sDnssdCache.Insert(nodeData)); #endif - proxy->OnNodeIdResolved(nodeData); + proxy->OnOperationalNodeResolved(nodeData); proxy->Release(); } @@ -620,7 +620,7 @@ bool ResolverProxy::ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet:: ResolvedNodeData nodeData; if (sDnssdCache.Lookup(peerId, nodeData) == CHIP_NO_ERROR) { - mDelegate->OnNodeIdResolved(nodeData); + mDelegate->OnOperationalNodeResolved(nodeData); mDelegate->Release(); return true; } diff --git a/src/lib/dnssd/Discovery_ImplPlatform.h b/src/lib/dnssd/Discovery_ImplPlatform.h index 4cb6624ce93b39..4347a762273f67 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.h +++ b/src/lib/dnssd/Discovery_ImplPlatform.h @@ -49,7 +49,11 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver CHIP_ERROR GetCommissionableInstanceName(char * instanceName, size_t maxLength) override; // Members that implement Resolver interface. - void SetResolverDelegate(ResolverDelegate * delegate) override { mResolverProxy.SetResolverDelegate(delegate); } + void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mResolverProxy.SetOperationalDelegate(delegate); } + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override + { + mResolverProxy.SetCommissioningDelegate(delegate); + } CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override; CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 3adac58e4a0242..2b9a6c3a8c4a31 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -299,8 +299,50 @@ enum class DiscoveryType kCommissionableNode, kCommissionerNode }; + +/// Callbacks for resolving operational node resolution +class OperationalResolveDelegate +{ +public: + virtual ~OperationalResolveDelegate() = default; + + /// Called within the CHIP event loop after a successful node resolution. + /// + /// May be called multiple times: implementations may call this once per + /// received packet and MDNS packets may arrive over different interfaces + /// which will make nodeData have different content. + virtual void OnOperationalNodeResolved(const ResolvedNodeData & nodeData) = 0; + + /// Notify a final failure for a node operational resolution. + /// + /// Called within the chip event loop if node resolution could not be performed. + /// This may be due to internal errors or timeouts. + /// + /// This will be called only if 'OnOperationalNodeResolved' is never called. + virtual void OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERROR error) = 0; +}; + +/// Callbacks for discovering nodes advertising non-operational status: +/// - Commissioners +/// - Nodes in commissioning modes over IP (e.g. ethernet devices, devices already +/// connected to thread/wifi or devices with a commissioning window open) +class CommissioningResolveDelegate +{ +public: + virtual ~CommissioningResolveDelegate() = default; + + /// Called within the CHIP event loop once a node is discovered. + /// + /// May be called multiple times as more nodes send their answer to a + /// multicast discovery query + virtual void OnNodeDiscovered(const DiscoveredNodeData & nodeData) = 0; +}; + /// Groups callbacks for CHIP service resolution requests -class ResolverDelegate +/// +/// TEMPORARY defined as a stop-gap to implementing +/// OperationalRsolveDelegate or CommissioningResolveDelegate +class ResolverDelegate : public OperationalResolveDelegate, public CommissioningResolveDelegate { public: virtual ~ResolverDelegate() = default; @@ -313,6 +355,16 @@ class ResolverDelegate // Called when a CHIP Node acting as Commissioner or in commissioning mode is found virtual void OnNodeDiscoveryComplete(const DiscoveredNodeData & nodeData) = 0; + + // OperationalResolveDelegate + void OnOperationalNodeResolved(const ResolvedNodeData & nodeData) override { OnNodeIdResolved(nodeData); } + void OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override + { + OnNodeIdResolutionFailed(peerId, error); + } + + // CommissioningNodeResolveDelegate + void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override { OnNodeDiscoveryComplete(nodeData); } }; /** @@ -337,10 +389,23 @@ class Resolver virtual void Shutdown() = 0; /** - * Registers a resolver delegate. If nullptr is passed, the previously registered delegate - * is unregistered. + * If nullptr is passed, the previously registered delegate is unregistered. + */ + virtual void SetOperationalDelegate(OperationalResolveDelegate * delegate) = 0; + + /** + * If nullptr is passed, the previously registered delegate is unregistered. */ - virtual void SetResolverDelegate(ResolverDelegate * delegate) = 0; + virtual void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) = 0; + + /** + * TEMPORARY setter that sets both operational and commissioning delegates + */ + void SetResolverDelegate(ResolverDelegate * delegate) + { + SetOperationalDelegate(delegate); + SetCommissioningDelegate(delegate); + } /** * Requests resolution of the given operational node service. diff --git a/src/lib/dnssd/ResolverProxy.h b/src/lib/dnssd/ResolverProxy.h index 1767a53d038c3b..adac1935cd0c14 100644 --- a/src/lib/dnssd/ResolverProxy.h +++ b/src/lib/dnssd/ResolverProxy.h @@ -23,38 +23,56 @@ namespace chip { namespace Dnssd { -class ResolverDelegateProxy : public ReferenceCounted, public ResolverDelegate +class ResolverDelegateProxy : public ReferenceCounted, + public OperationalResolveDelegate, + public CommissioningResolveDelegate + { public: - void SetDelegate(ResolverDelegate * delegate) { mDelegate = delegate; } + void SetOperationalDelegate(OperationalResolveDelegate * delegate) { mOperationalDelegate = delegate; } + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) { mCommissioningDelegate = delegate; } - /// ResolverDelegate interface - void OnNodeIdResolved(const ResolvedNodeData & nodeData) override + // OperationalResolveDelegate + void OnOperationalNodeResolved(const ResolvedNodeData & nodeData) override { - if (mDelegate != nullptr) + if (mOperationalDelegate != nullptr) + { + mOperationalDelegate->OnOperationalNodeResolved(nodeData); + } + else { - mDelegate->OnNodeIdResolved(nodeData); + ChipLogError(Discovery, "Missing operational delegate. Data discarded."); } } - void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override + void OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override { - if (mDelegate != nullptr) + if (mOperationalDelegate != nullptr) + { + mOperationalDelegate->OnOperationalNodeResolutionFailed(peerId, error); + } + else { - mDelegate->OnNodeIdResolutionFailed(peerId, error); + ChipLogError(Discovery, "Missing operational delegate. Failure info discarded."); } } - void OnNodeDiscoveryComplete(const DiscoveredNodeData & nodeData) override + // CommissioningResolveDelegate + void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override { - if (mDelegate != nullptr) + if (mCommissioningDelegate != nullptr) + { + mCommissioningDelegate->OnNodeDiscovered(nodeData); + } + else { - mDelegate->OnNodeDiscoveryComplete(nodeData); + ChipLogError(Discovery, "Missing commissioning delegate. Data discarded."); } } private: - ResolverDelegate * mDelegate = nullptr; + OperationalResolveDelegate * mOperationalDelegate = nullptr; + CommissioningResolveDelegate * mCommissioningDelegate = nullptr; }; class ResolverProxy : public Resolver @@ -71,16 +89,35 @@ class ResolverProxy : public Resolver return mDelegate != nullptr ? CHIP_NO_ERROR : CHIP_ERROR_NO_MEMORY; } - void SetResolverDelegate(ResolverDelegate * delegate) override + void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { - VerifyOrReturn(mDelegate != nullptr); - mDelegate->SetDelegate(delegate); + if (mDelegate != nullptr) + { + mDelegate->SetOperationalDelegate(delegate); + } + else + { + ChipLogError(Discovery, "Failed to proxy operational discovery: missing delegate"); + } + } + + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override + { + if (mDelegate != nullptr) + { + mDelegate->SetCommissioningDelegate(delegate); + } + else + { + ChipLogError(Discovery, "Failed to proxy commissioning discovery: missing delegate"); + } } void Shutdown() override { VerifyOrReturn(mDelegate != nullptr); - mDelegate->SetDelegate(nullptr); + mDelegate->SetOperationalDelegate(nullptr); + mDelegate->SetCommissioningDelegate(nullptr); mDelegate->Release(); mDelegate = nullptr; } diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index 957aa90819021e..5932849190adb7 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -70,10 +70,11 @@ using DnssdCacheType = Dnssd::DnssdCache; class PacketDataReporter : public ParserDelegate { public: - PacketDataReporter(ResolverDelegate * delegate, chip::Inet::InterfaceId interfaceId, DiscoveryType discoveryType, - const BytesRange & packet, DnssdCacheType & mdnsCache) : - mDelegate(delegate), - mDiscoveryType(discoveryType), mPacketRange(packet) + PacketDataReporter(OperationalResolveDelegate * opDelegate, CommissioningResolveDelegate * commissionDelegate, + chip::Inet::InterfaceId interfaceId, DiscoveryType discoveryType, const BytesRange & packet, + DnssdCacheType & mdnsCache) : + mOperationalDelegate(opDelegate), + mCommissioningDelegate(commissionDelegate), mDiscoveryType(discoveryType), mPacketRange(packet) { mInterfaceId = interfaceId; } @@ -89,7 +90,8 @@ class PacketDataReporter : public ParserDelegate void OnComplete(ActiveResolveAttempts & activeAttempts); private: - ResolverDelegate * mDelegate = nullptr; + OperationalResolveDelegate * mOperationalDelegate; + CommissioningResolveDelegate * mCommissioningDelegate; DiscoveryType mDiscoveryType; ResolvedNodeData mNodeData; DiscoveredNodeData mDiscoveredNodeData; @@ -324,15 +326,29 @@ void PacketDataReporter::OnComplete(ActiveResolveAttempts & activeAttempts) if ((mDiscoveryType == DiscoveryType::kCommissionableNode || mDiscoveryType == DiscoveryType::kCommissionerNode) && mDiscoveredNodeData.IsValid()) { - mDelegate->OnNodeDiscoveryComplete(mDiscoveredNodeData); + if (mCommissioningDelegate != nullptr) + { + mCommissioningDelegate->OnNodeDiscovered(mDiscoveredNodeData); + } + else + { + ChipLogError(Discovery, "No delegate to report commissioning node discovery"); + } } else if (mDiscoveryType == DiscoveryType::kOperational && mHasIP && mHasNodePort) { activeAttempts.Complete(mNodeData.mPeerId); - mNodeData.LogNodeIdResolved(); mNodeData.PrioritizeAddresses(); - mDelegate->OnNodeIdResolved(mNodeData); + + if (mOperationalDelegate != nullptr) + { + mOperationalDelegate->OnOperationalNodeResolved(mNodeData); + } + else + { + ChipLogError(Discovery, "No delegate to report operational node discovery"); + } } } @@ -350,16 +366,18 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate ///// Resolver implementation CHIP_ERROR Init(chip::Inet::EndPointManager * udpEndPointManager) override; void Shutdown() override; - void SetResolverDelegate(ResolverDelegate * delegate) override { mDelegate = delegate; } + void SetOperationalDelegate(OperationalResolveDelegate * delegate) override { mOperationalDelegate = delegate; } + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override { mCommissioningDelegate = delegate; } CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override; CHIP_ERROR FindCommissionableNodes(DiscoveryFilter filter = DiscoveryFilter()) override; CHIP_ERROR FindCommissioners(DiscoveryFilter filter = DiscoveryFilter()) override; bool ResolveNodeIdFromInternalCache(const PeerId & peerId, Inet::IPAddressType type) override; private: - ResolverDelegate * mDelegate = nullptr; - DiscoveryType mDiscoveryType = DiscoveryType::kUnknown; - System::Layer * mSystemLayer = nullptr; + OperationalResolveDelegate * mOperationalDelegate = nullptr; + CommissioningResolveDelegate * mCommissioningDelegate = nullptr; + DiscoveryType mDiscoveryType = DiscoveryType::kUnknown; + System::Layer * mSystemLayer = nullptr; ActiveResolveAttempts mActiveResolves; CHIP_ERROR SendPendingResolveQueries(); @@ -388,12 +406,12 @@ class MinMdnsResolver : public Resolver, public MdnsPacketDelegate void MinMdnsResolver::OnMdnsPacketData(const BytesRange & data, const chip::Inet::IPPacketInfo * info) { - if (mDelegate == nullptr) + if ((mOperationalDelegate == nullptr) && (mCommissioningDelegate == nullptr)) { return; } - PacketDataReporter reporter(mDelegate, info->Interface, mDiscoveryType, data, sDnssdCache); + PacketDataReporter reporter(mOperationalDelegate, mCommissioningDelegate, info->Interface, mDiscoveryType, data, sDnssdCache); if (!ParsePacket(data, &reporter)) { @@ -607,27 +625,27 @@ Resolver & chip::Dnssd::Resolver::Instance() } // Minimal implementation does not support associating a context to a request (while platforms implementations do). So keep -// updating the delegate that ends up being used by the server by calling 'SetResolverDelegate'. +// updating the delegate that ends up being used by the server by calling 'SetOperationalDelegate'. // This effectively allow minimal to have multiple controllers issuing requests as long the requests are serialized, but // it won't work well if requests are issued in parallel. CHIP_ERROR ResolverProxy::ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) { VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - chip::Dnssd::Resolver::Instance().SetResolverDelegate(mDelegate); + chip::Dnssd::Resolver::Instance().SetOperationalDelegate(mDelegate); return chip::Dnssd::Resolver::Instance().ResolveNodeId(peerId, type); } CHIP_ERROR ResolverProxy::FindCommissionableNodes(DiscoveryFilter filter) { VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - chip::Dnssd::Resolver::Instance().SetResolverDelegate(mDelegate); + chip::Dnssd::Resolver::Instance().SetCommissioningDelegate(mDelegate); return chip::Dnssd::Resolver::Instance().FindCommissionableNodes(filter); } CHIP_ERROR ResolverProxy::FindCommissioners(DiscoveryFilter filter) { VerifyOrReturnError(mDelegate != nullptr, CHIP_ERROR_INCORRECT_STATE); - chip::Dnssd::Resolver::Instance().SetResolverDelegate(mDelegate); + chip::Dnssd::Resolver::Instance().SetCommissioningDelegate(mDelegate); return chip::Dnssd::Resolver::Instance().FindCommissioners(filter); } diff --git a/src/lib/dnssd/Resolver_ImplNone.cpp b/src/lib/dnssd/Resolver_ImplNone.cpp index fb8b190826c5ec..8b0999d2fcc908 100644 --- a/src/lib/dnssd/Resolver_ImplNone.cpp +++ b/src/lib/dnssd/Resolver_ImplNone.cpp @@ -29,7 +29,8 @@ class NoneResolver : public Resolver public: CHIP_ERROR Init(chip::Inet::EndPointManager *) override { return CHIP_NO_ERROR; } void Shutdown() override {} - void SetResolverDelegate(ResolverDelegate *) override {} + void SetOperationalDelegate(OperationalResolveDelegate * delegate) override {} + void SetCommissioningDelegate(CommissioningResolveDelegate * delegate) override {} CHIP_ERROR ResolveNodeId(const PeerId & peerId, Inet::IPAddressType type) override {