Dnssd separate interface between operational and commissionable

examples/chip-tool/commands/discover/Commands.h

@@ -24,7 +24,7 @@
 #include <controller/DeviceAddressUpdateDelegate.h>
 #include <lib/dnssd/Resolver.h>
 
-class Resolve : public DiscoverCommand, public chip::Dnssd::ResolverDelegate
+class Resolve : public DiscoverCommand, public chip::Dnssd::OperationalResolveDelegate
 {
 public:
     Resolve(CredentialIssuerCommands * credsIssuerConfig) : DiscoverCommand("resolve", credsIssuerConfig) {}
@@ -33,13 +33,13 @@ class Resolve : public DiscoverCommand, public chip::Dnssd::ResolverDelegate
     CHIP_ERROR RunCommand(NodeId remoteId, uint64_t fabricId) override
     {
         ReturnErrorOnFailure(mDNSResolver.Init(chip::DeviceLayer::UDPEndPointManager()));
-        mDNSResolver.SetResolverDelegate(this);
+        mDNSResolver.SetOperationalDelegate(this);
         ChipLogProgress(chipTool, "Dnssd: Searching for NodeId: %" PRIx64 " FabricId: %" PRIx64 " ...", remoteId, fabricId);
         return mDNSResolver.ResolveNodeId(chip::PeerId().SetNodeId(remoteId).SetCompressedFabricId(fabricId),
                                           chip::Inet::IPAddressType::kAny);
     }
 
-    void OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override
+    void OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override
     {
         char addrBuffer[chip::Transport::PeerAddress::kMaxToStringSize];
 
@@ -65,12 +65,11 @@ class Resolve : public DiscoverCommand, public chip::Dnssd::ResolverDelegate
         SetCommandExitStatus(CHIP_NO_ERROR);
     }
 
-    void OnNodeIdResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override
+    void OnOperationalNodeResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override
     {
         ChipLogProgress(chipTool, "NodeId Resolution: failed!");
         SetCommandExitStatus(CHIP_ERROR_INTERNAL);
     }
-    void OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData) override {}
 
 private:
     chip::Dnssd::ResolverProxy mDNSResolver;

src/app/CASESessionManager.cpp

@@ -78,7 +78,7 @@ CHIP_ERROR CASESessionManager::ResolveDeviceAddress(FabricInfo * fabric, NodeId
     return mConfig.dnsResolver->ResolveNodeId(fabric->GetPeerIdForNode(nodeId), Inet::IPAddressType::kAny);
 }
 
-void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData)
+void CASESessionManager::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeData)
 {
     ChipLogProgress(Controller, "Address resolved for node: 0x" ChipLogFormatX64, ChipLogValueX64(nodeData.mPeerId.GetNodeId()));
 
@@ -94,7 +94,7 @@ void CASESessionManager::OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeDa
     LogErrorOnFailure(session->UpdateDeviceData(OperationalDeviceProxy::ToPeerAddress(nodeData), nodeData.GetMRPConfig()));
 }
 
-void CASESessionManager::OnNodeIdResolutionFailed(const PeerId & peer, CHIP_ERROR error)
+void CASESessionManager::OnOperationalNodeResolutionFailed(const PeerId & peer, CHIP_ERROR error)
 {
     ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error));
 }

src/app/CASESessionManager.h

@@ -48,7 +48,7 @@ struct CASESessionManagerConfig
  * 4. During session establishment, trigger node ID resolution (if needed), and update the DNS-SD cache (if resolution is
  * successful)
  */
-class CASESessionManager : public Dnssd::ResolverDelegate
+class CASESessionManager : public Dnssd::OperationalResolveDelegate
 {
 public:
     CASESessionManager() = delete;
@@ -65,7 +65,7 @@ class CASESessionManager : public Dnssd::ResolverDelegate
         if (mConfig.dnsResolver == nullptr)
         {
             ReturnErrorOnFailure(mDNSResolver.Init(DeviceLayer::UDPEndPointManager()));
-            mDNSResolver.SetResolverDelegate(this);
+            mDNSResolver.SetOperationalDelegate(this);
             mConfig.dnsResolver = &mDNSResolver;
         }
         return CHIP_NO_ERROR;
@@ -92,7 +92,7 @@ class CASESessionManager : public Dnssd::ResolverDelegate
      * This API triggers the DNS-SD resolution for the given node ID. The node ID will be looked up
      * on the fabric that was configured for the CASESessionManager object.
      *
-     * The results of the DNS-SD resolution request is provided to the class via `ResolverDelegate`
+     * The results of the DNS-SD resolution request is provided to the class via `OperationalResolveDelegate`
      * implementation of CASESessionManager.
      */
     CHIP_ERROR ResolveDeviceAddress(FabricInfo * fabric, NodeId nodeId);
@@ -107,10 +107,9 @@ class CASESessionManager : public Dnssd::ResolverDelegate
      */
     CHIP_ERROR GetPeerAddress(PeerId peerId, Transport::PeerAddress & addr);
 
-    //////////// ResolverDelegate Implementation ///////////////
-    void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) override;
-    void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override;
-    void OnNodeDiscoveryComplete(const Dnssd::DiscoveredNodeData & nodeData) override {}
+    //////////// OperationalResolveDelegate Implementation ///////////////
+    void OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeData) override;
+    void OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override;
 
 private:
     OperationalDeviceProxy * FindSession(const SessionHandle & session);

src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp

@@ -104,13 +104,13 @@ CHIP_ERROR DiscoveryCommands::SetupDiscoveryCommands()
         ReturnErrorOnFailure(mDNSResolver.Init(chip::DeviceLayer::UDPEndPointManager()));
         mReady = true;
     }
-    mDNSResolver.SetResolverDelegate(this);
+    mDNSResolver.SetCommissioningDelegate(this);
     return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR DiscoveryCommands::TearDownDiscoveryCommands()
 {
-    mDNSResolver.SetResolverDelegate(nullptr);
+    mDNSResolver.SetCommissioningDelegate(nullptr);
     return CHIP_NO_ERROR;
 }
 
@@ -126,7 +126,7 @@ uint16_t DiscoveryCommands::GetUniqueDiscriminator()
     return mDiscriminatorUseForFiltering;
 }
 
-void DiscoveryCommands::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData)
+void DiscoveryCommands::OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData)
 {
     // TODO: If multiple results are found for the same filter, then the test result depends
     //       on which result comes first. At the moment, the code assume that there is only

src/app/tests/suites/commands/discovery/DiscoveryCommands.h

@@ -43,7 +43,7 @@ struct DiscoveryCommandResult
     chip::Optional<uint32_t> mrpRetryIntervalActive;
 };
 
-class DiscoveryCommands : public chip::Dnssd::ResolverDelegate
+class DiscoveryCommands : public chip::Dnssd::CommissioningResolveDelegate
 {
 public:
     DiscoveryCommands(){};
@@ -66,10 +66,8 @@ class DiscoveryCommands : public chip::Dnssd::ResolverDelegate
     CHIP_ERROR TearDownDiscoveryCommands();
     virtual void OnDiscoveryCommandsResults(const DiscoveryCommandResult & nodeData){};
 
-    /////////// ResolverDelegate Interface /////////
-    void OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override{};
-    void OnNodeIdResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override{};
-    void OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
+    /////////// CommissioningDelegate Interface /////////
+    void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
 
 protected:
     // This function initialize a random discriminator once and returns it all the time afterwards

src/controller/AbstractDnssdDiscoveryController.cpp

@@ -25,7 +25,7 @@
 namespace chip {
 namespace Controller {
 
-void AbstractDnssdDiscoveryController::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData)
+void AbstractDnssdDiscoveryController::OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData)
 {
     auto discoveredNodes = GetDiscoveredNodes();
     for (auto & discoveredNode : discoveredNodes)

src/controller/AbstractDnssdDiscoveryController.h

@@ -36,13 +36,13 @@ namespace Controller {
  *   to maintain a list of DiscoveredNodes and providing the implementation
  *   of the template GetDiscoveredNodes() function.
  */
-class DLL_EXPORT AbstractDnssdDiscoveryController : public Dnssd::ResolverDelegate
+class DLL_EXPORT AbstractDnssdDiscoveryController : public Dnssd::CommissioningResolveDelegate
 {
 public:
     AbstractDnssdDiscoveryController() {}
     virtual ~AbstractDnssdDiscoveryController() {}
 
-    void OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
+    void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
 
 protected:
     using DiscoveredNodeList = FixedSpan<Dnssd::DiscoveredNodeData, CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES>;

src/controller/CHIPCommissionableNodeController.cpp

@@ -37,7 +37,7 @@ CHIP_ERROR CommissionableNodeController::DiscoverCommissioners(Dnssd::DiscoveryF
 #if CONFIG_DEVICE_LAYER
         ReturnErrorOnFailure(mDNSResolver.Init(DeviceLayer::UDPEndPointManager()));
 #endif
-        mDNSResolver.SetResolverDelegate(this);
+        mDNSResolver.SetCommissioningDelegate(this);
         return mDNSResolver.FindCommissioners(discoveryFilter);
     }
     else

src/controller/CHIPCommissionableNodeController.h

@@ -50,16 +50,6 @@ class DLL_EXPORT CommissionableNodeController : public AbstractDnssdDiscoveryCon
      */
     const Dnssd::DiscoveredNodeData * GetDiscoveredCommissioner(int idx);
 
-    void OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override
-    {
-        ChipLogError(Controller, "Unsupported operation CommissionableNodeController::OnNodeIdResolved");
-    }
-
-    void OnNodeIdResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override
-    {
-        ChipLogError(Controller, "Unsupported operation CommissionableNodeController::OnNodeIdResolutionFailed");
-    }
-
 protected:
     DiscoveredNodeList GetDiscoveredNodes() override { return DiscoveredNodeList(mDiscoveredCommissioners); }
 

src/controller/CHIPDeviceController.cpp

@@ -126,7 +126,8 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
     ReturnErrorOnFailure(mDNSResolver.Init(params.systemState->UDPEndPointManager()));
-    mDNSResolver.SetResolverDelegate(this);
+    mDNSResolver.SetOperationalDelegate(this);
+    mDNSResolver.SetCommissioningDelegate(this);
     RegisterDeviceAddressUpdateDelegate(params.deviceAddressUpdateDelegate);
     RegisterDeviceDiscoveryDelegate(params.deviceDiscoveryDelegate);
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
@@ -544,22 +545,23 @@ CHIP_ERROR DeviceController::OpenCommissioningWindowInternal()
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 
-void DeviceController::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData)
+void DeviceController::OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData)
 {
-    VerifyOrReturn(mState == State::Initialized, ChipLogError(Controller, "OnNodeIdResolved was called in incorrect state"));
-    mCASESessionManager->OnNodeIdResolved(nodeData);
+    VerifyOrReturn(mState == State::Initialized,
+                   ChipLogError(Controller, "OnOperationalNodeResolved was called in incorrect state"));
+    mCASESessionManager->OnOperationalNodeResolved(nodeData);
     if (mDeviceAddressUpdateDelegate != nullptr)
     {
         mDeviceAddressUpdateDelegate->OnAddressUpdateComplete(nodeData.mPeerId.GetNodeId(), CHIP_NO_ERROR);
     }
 };
 
-void DeviceController::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error)
+void DeviceController::OnOperationalNodeResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error)
 {
     ChipLogError(Controller, "Error resolving node id: %s", ErrorStr(error));
     VerifyOrReturn(mState == State::Initialized,
-                   ChipLogError(Controller, "OnNodeIdResolutionFailed was called in incorrect state"));
-    mCASESessionManager->OnNodeIdResolutionFailed(peer, error);
+                   ChipLogError(Controller, "OnOperationalNodeResolutionFailed was called in incorrect state"));
+    mCASESessionManager->OnOperationalNodeResolutionFailed(peer, error);
 
     if (mDeviceAddressUpdateDelegate != nullptr)
     {
@@ -1379,15 +1381,15 @@ void DeviceCommissioner::FindCommissionableNode(char * instanceName)
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 
-void DeviceCommissioner::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData)
+void DeviceCommissioner::OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData)
 {
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
     if (mUdcServer != nullptr)
     {
         mUdcServer->OnCommissionableNodeFound(nodeData);
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
-    AbstractDnssdDiscoveryController::OnNodeDiscoveryComplete(nodeData);
+    AbstractDnssdDiscoveryController::OnNodeDiscovered(nodeData);
     mSetUpCodePairer.NotifyCommissionableDeviceDiscovered(nodeData);
 }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
@@ -1419,7 +1421,7 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin
 }
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-void DeviceCommissioner::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData)
+void DeviceCommissioner::OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData)
 {
     ChipLogProgress(Controller, "OperationalDiscoveryComplete for device ID 0x" ChipLogFormatX64,
                     ChipLogValueX64(nodeData.mPeerId.GetNodeId()));
@@ -1428,10 +1430,10 @@ void DeviceCommissioner::OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData &
     mDNSCache.Insert(nodeData);
 
     mCASESessionManager->FindOrEstablishSession(nodeData.mPeerId, &mOnDeviceConnectedCallback, &mOnDeviceConnectionFailureCallback);
-    DeviceController::OnNodeIdResolved(nodeData);
+    DeviceController::OnOperationalNodeResolved(nodeData);
 }
 
-void DeviceCommissioner::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error)
+void DeviceCommissioner::OnOperationalNodeResolutionFailed(const chip::PeerId & peer, CHIP_ERROR error)
 {
     if (mDeviceBeingCommissioned != nullptr)
     {
@@ -1441,7 +1443,7 @@ void DeviceCommissioner::OnNodeIdResolutionFailed(const chip::PeerId & peer, CHI
             CommissioningStageComplete(error);
         }
     }
-    DeviceController::OnNodeIdResolutionFailed(peer, error);
+    DeviceController::OnOperationalNodeResolutionFailed(peer, error);
 }
 
 #endif

src/controller/CHIPDeviceController.h

@@ -171,7 +171,8 @@ typedef void (*OnOpenCommissioningWindow)(void * context, NodeId deviceId, CHIP_
 class DLL_EXPORT DeviceController : public SessionRecoveryDelegate
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
     ,
-                                    public AbstractDnssdDiscoveryController
+                                    public AbstractDnssdDiscoveryController,
+                                    public Dnssd::OperationalResolveDelegate
 #endif
 {
 public:
@@ -375,9 +376,9 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate
     void OnFirstMessageDeliveryFailed(const SessionHandle & session) override;
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    //////////// ResolverDelegate Implementation ///////////////
-    void OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override;
-    void OnNodeIdResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override;
+    //////////// OperationalResolveDelegate Implementation ///////////////
+    void OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override;
+    void OnOperationalNodeResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override;
     DiscoveredNodeList GetDiscoveredNodes() override { return DiscoveredNodeList(mCommissionableNodes); }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 
@@ -621,8 +622,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
      */
     int GetMaxCommissionableNodesSupported() { return kMaxCommissionableNodes; }
 
-    void OnNodeIdResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override;
-    void OnNodeIdResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override;
+    void OnOperationalNodeResolved(const chip::Dnssd::ResolvedNodeData & nodeData) override;
+    void OnOperationalNodeResolutionFailed(const chip::PeerId & peerId, CHIP_ERROR error) override;
 #endif
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY // make this commissioner discoverable
     /**
@@ -652,7 +653,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
      * @param nodeData DNS-SD node information
      *
      */
-    void OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
+    void OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
 #endif
 
     void RegisterPairingDelegate(DevicePairingDelegate * pairingDelegate) { mPairingDelegate = pairingDelegate; }

src/controller/python/chip/discovery/NodeResolution.cpp

@@ -33,10 +33,10 @@ namespace {
 using DiscoverSuccessCallback = void (*)(uint64_t fabricId, uint64_t nodeId, uint32_t interfaceId, const char * ip, uint16_t port);
 using DiscoverFailureCallback = void (*)(uint64_t fabricId, uint64_t nodeId, ChipError::StorageType error_code);
 
-class PythonResolverDelegate : public ResolverDelegate
+class PythonResolverDelegate : public OperationalResolveDelegate
 {
 public:
-    void OnNodeIdResolved(const ResolvedNodeData & nodeData) override
+    void OnOperationalNodeResolved(const ResolvedNodeData & nodeData) override
     {
         if (mSuccessCallback != nullptr)
         {
@@ -58,7 +58,7 @@ class PythonResolverDelegate : public ResolverDelegate
         }
     }
 
-    void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override
+    void OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override
     {
         if (mFailureCallback != nullptr)
         {
@@ -70,8 +70,6 @@ class PythonResolverDelegate : public ResolverDelegate
         }
     }
 
-    void OnNodeDiscoveryComplete(const DiscoveredNodeData & nodeData) override {}
-
     void SetSuccessCallback(DiscoverSuccessCallback cb) { mSuccessCallback = cb; }
     void SetFailureCallback(DiscoverFailureCallback cb) { mFailureCallback = cb; }
 
@@ -97,7 +95,7 @@ extern "C" ChipError::StorageType pychip_discovery_resolve(uint64_t fabricId, ui
     chip::python::ChipMainThreadScheduleAndWait([&] {
         result = Resolver::Instance().Init(chip::DeviceLayer::UDPEndPointManager());
         ReturnOnFailure(result);
-        Resolver::Instance().SetResolverDelegate(&gPythonResolverDelegate);
+        Resolver::Instance().SetOperationalDelegate(&gPythonResolverDelegate);
 
         result = Resolver::Instance().ResolveNodeId(chip::PeerId().SetCompressedFabricId(fabricId).SetNodeId(nodeId),
                                                     chip::Inet::IPAddressType::kAny);

src/controller/tests/TestCommissionableNodeController.cpp

@@ -62,7 +62,7 @@ void TestGetDiscoveredCommissioner_HappyCase(nlTestSuite * inSuite, void * inCon
     inNodeData.numIPs++;
     inNodeData.port = 5540;
 
-    controller.OnNodeDiscoveryComplete(inNodeData);
+    controller.OnNodeDiscovered(inNodeData);
 
     NL_TEST_ASSERT(inSuite, controller.GetDiscoveredCommissioner(0) != nullptr);
     NL_TEST_ASSERT(inSuite, strcmp(inNodeData.hostName, controller.GetDiscoveredCommissioner(0)->hostName) == 0);
@@ -80,7 +80,7 @@ void TestGetDiscoveredCommissioner_InvalidNodeDiscovered_ReturnsNullptr(nlTestSu
     inNodeData.numIPs++;
     inNodeData.port = 5540;
 
-    controller.OnNodeDiscoveryComplete(inNodeData);
+    controller.OnNodeDiscovered(inNodeData);
 
     for (int i = 0; i < CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES; i++)
     {
@@ -103,8 +103,8 @@ void TestGetDiscoveredCommissioner_HappyCase_OneValidOneInvalidNode(nlTestSuite
     validNodeData.numIPs++;
     validNodeData.port = 5540;
 
-    controller.OnNodeDiscoveryComplete(validNodeData);
-    controller.OnNodeDiscoveryComplete(invalidNodeData);
+    controller.OnNodeDiscovered(validNodeData);
+    controller.OnNodeDiscovered(invalidNodeData);
 
     NL_TEST_ASSERT(inSuite, controller.GetDiscoveredCommissioner(0) != nullptr);
     NL_TEST_ASSERT(inSuite, strcmp(validNodeData.hostName, controller.GetDiscoveredCommissioner(0)->hostName) == 0);