From 3439792526900a789f96ccb22e12aa20ece8cf57 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 23 Feb 2022 23:57:30 -0500 Subject: [PATCH] Dnssd separate interface between operational and commissionable (#15464) * Change ChipDeviceController to use explicit interfaces instead of generic dnssd resolver delegate interface * Update CASESessionManager to use the updated DNSSD interfaces * Switch python interfaces to the operational node delegate logic * Switch shell to the new interface for operational/commissionable resolve delegates * Switch discovery commands to updated delegate classes * Fix typo in class name * Make unit tests compile * Fix texts for errors in unimplemented methods * Simplify device controller: implement only required interfaces instead of all interfaces + error log virtual methods * Restyle * Make chipcontroller also set the commissionable delegate since it inherits from a commissionabledelegate * Restyle * Remove resolver delegate (now that master usage within the address resolver is gone --- .../chip-tool/commands/discover/Commands.h | 9 ++--- src/app/CASESessionManager.cpp | 4 +- src/app/CASESessionManager.h | 13 +++---- .../commands/discovery/DiscoveryCommands.cpp | 6 +-- .../commands/discovery/DiscoveryCommands.h | 8 ++-- .../AbstractDnssdDiscoveryController.cpp | 2 +- .../AbstractDnssdDiscoveryController.h | 4 +- .../CHIPCommissionableNodeController.cpp | 2 +- .../CHIPCommissionableNodeController.h | 10 ----- src/controller/CHIPDeviceController.cpp | 28 +++++++------- src/controller/CHIPDeviceController.h | 15 ++++---- .../python/chip/discovery/NodeResolution.cpp | 10 ++--- .../TestCommissionableNodeController.cpp | 8 ++-- src/lib/dnssd/Resolver.h | 38 ------------------- src/lib/dnssd/minimal_mdns/Server.cpp | 2 +- src/lib/shell/commands/Dns.cpp | 11 +++--- 16 files changed, 60 insertions(+), 110 deletions(-) diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h index 19c74f84372ea9..5175544202c447 100644 --- a/examples/chip-tool/commands/discover/Commands.h +++ b/examples/chip-tool/commands/discover/Commands.h @@ -24,7 +24,7 @@ #include #include -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; diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index 4303087d33258e..0832464e7e9639 100644 --- a/src/app/CASESessionManager.cpp +++ b/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)); } diff --git a/src/app/CASESessionManager.h b/src/app/CASESessionManager.h index 9455646448b807..3f687dbff03541 100644 --- a/src/app/CASESessionManager.h +++ b/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); diff --git a/src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp b/src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp index 029b3a7af5a934..5afa23e2d9c87c 100644 --- a/src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp +++ b/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 diff --git a/src/app/tests/suites/commands/discovery/DiscoveryCommands.h b/src/app/tests/suites/commands/discovery/DiscoveryCommands.h index 4b308d4bf11b75..d5cfebd9cd4821 100644 --- a/src/app/tests/suites/commands/discovery/DiscoveryCommands.h +++ b/src/app/tests/suites/commands/discovery/DiscoveryCommands.h @@ -43,7 +43,7 @@ struct DiscoveryCommandResult chip::Optional 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 diff --git a/src/controller/AbstractDnssdDiscoveryController.cpp b/src/controller/AbstractDnssdDiscoveryController.cpp index 924ca6183086c0..3a1f1ecc7c3e1d 100644 --- a/src/controller/AbstractDnssdDiscoveryController.cpp +++ b/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) diff --git a/src/controller/AbstractDnssdDiscoveryController.h b/src/controller/AbstractDnssdDiscoveryController.h index df5845711d4fbd..66eab16ff7139d 100644 --- a/src/controller/AbstractDnssdDiscoveryController.h +++ b/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; diff --git a/src/controller/CHIPCommissionableNodeController.cpp b/src/controller/CHIPCommissionableNodeController.cpp index 49ae43d14e30a4..6081cf92d0f500 100644 --- a/src/controller/CHIPCommissionableNodeController.cpp +++ b/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 diff --git a/src/controller/CHIPCommissionableNodeController.h b/src/controller/CHIPCommissionableNodeController.h index af57d1d6abe223..cc381fccb0b583 100644 --- a/src/controller/CHIPCommissionableNodeController.h +++ b/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); } diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index ab3ab76ed006ae..774f899eefa4d6 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/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,7 +1381,7 @@ 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) @@ -1387,7 +1389,7 @@ void DeviceCommissioner::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNo 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 diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 9695647021a274..a7c796e906b2a6 100644 --- a/src/controller/CHIPDeviceController.h +++ b/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; } diff --git a/src/controller/python/chip/discovery/NodeResolution.cpp b/src/controller/python/chip/discovery/NodeResolution.cpp index 7e794b77c8981c..ad3010c73d589c 100644 --- a/src/controller/python/chip/discovery/NodeResolution.cpp +++ b/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); diff --git a/src/controller/tests/TestCommissionableNodeController.cpp b/src/controller/tests/TestCommissionableNodeController.cpp index f3e539c2b8eeb8..a1ccca9bbf8d67 100644 --- a/src/controller/tests/TestCommissionableNodeController.cpp +++ b/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); diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 3fc191796825b1..1b5951f89908cb 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -354,35 +354,6 @@ class CommissioningResolveDelegate virtual void OnNodeDiscovered(const DiscoveredNodeData & nodeData) = 0; }; -/// Groups callbacks for CHIP service resolution requests -/// -/// TEMPORARY defined as a stop-gap to implementing -/// OperationalRsolveDelegate or CommissioningResolveDelegate -class ResolverDelegate : public OperationalResolveDelegate, public CommissioningResolveDelegate -{ -public: - virtual ~ResolverDelegate() = default; - - /// Called when a requested CHIP node ID has been successfully resolved - virtual void OnNodeIdResolved(const ResolvedNodeData & nodeData) = 0; - - /// Called when a CHIP node ID resolution has failed - virtual void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) = 0; - - // 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); } -}; - /** * Interface for resolving CHIP DNS-SD services */ @@ -414,15 +385,6 @@ class Resolver */ 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/minimal_mdns/Server.cpp b/src/lib/dnssd/minimal_mdns/Server.cpp index 816feac1e71e77..f868f34527f910 100644 --- a/src/lib/dnssd/minimal_mdns/Server.cpp +++ b/src/lib/dnssd/minimal_mdns/Server.cpp @@ -356,7 +356,7 @@ CHIP_ERROR ServerBase::BroadcastImpl(chip::System::PacketBufferHandle && data, u return chip::Loop::Continue; } - CHIP_ERROR err; + CHIP_ERROR err = CHIP_NO_ERROR; /// The same packet needs to be sent over potentially multiple interfaces. /// LWIP does not like having a pbuf sent over serparate interfaces, hence we create a copy diff --git a/src/lib/shell/commands/Dns.cpp b/src/lib/shell/commands/Dns.cpp index bf6349265a8db4..e337eb7726ebc6 100644 --- a/src/lib/shell/commands/Dns.cpp +++ b/src/lib/shell/commands/Dns.cpp @@ -38,10 +38,10 @@ namespace { Shell::Engine sShellDnsBrowseSubcommands; Shell::Engine sShellDnsSubcommands; -class DnsShellResolverDelegate : public Dnssd::ResolverDelegate +class DnsShellResolverDelegate : public Dnssd::OperationalResolveDelegate, public Dnssd::CommissioningResolveDelegate { public: - void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) override + void OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeData) override { streamer_printf(streamer_get(), "DNS resolve for " ChipLogFormatX64 "-" ChipLogFormatX64 " succeeded:\r\n", ChipLogValueX64(nodeData.mPeerId.GetCompressedFabricId()), ChipLogValueX64(nodeData.mPeerId.GetNodeId())); @@ -65,9 +65,9 @@ class DnsShellResolverDelegate : public Dnssd::ResolverDelegate streamer_printf(streamer_get(), " Supports TCP: %s\r\n", nodeData.mSupportsTcp ? "yes" : "no"); } - void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override {} + void OnOperationalNodeResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override {} - void OnNodeDiscoveryComplete(const Dnssd::DiscoveredNodeData & nodeData) override + void OnNodeDiscovered(const Dnssd::DiscoveredNodeData & nodeData) override { if (!nodeData.IsValid()) { @@ -224,7 +224,8 @@ CHIP_ERROR DnsHandler(int argc, char ** argv) } sResolverProxy.Init(DeviceLayer::UDPEndPointManager()); - sResolverProxy.SetResolverDelegate(&sDnsShellResolverDelegate); + sResolverProxy.SetOperationalDelegate(&sDnsShellResolverDelegate); + sResolverProxy.SetCommissioningDelegate(&sDnsShellResolverDelegate); return sShellDnsSubcommands.ExecCommand(argc, argv); }