Skip to content

Commit

Permalink
Separate out DNSSD ResolverDelegate into Operational and Commissionin…
Browse files Browse the repository at this point in the history
…g Delegates (#15271)

* Split out the resolver interfaces into commissioner and operational node discovery. The lookup methods and result types are completely different in both cases

* Fix for compiling logic

* Fix mock type definitions

* Remove strange newline added by a failed autoformat

* Fix callbacks to proxy on platform dns impl

* Fix typos
  • Loading branch information
andy31415 authored and pull[bot] committed Jul 11, 2023
1 parent 86d3489 commit 1049440
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 48 deletions.
3 changes: 2 additions & 1 deletion src/controller/tests/TestCommissionableNodeController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ class MockResolver : public Resolver
public:
CHIP_ERROR Init(chip::Inet::EndPointManager<chip::Inet::UDPEndPoint> * 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; }
Expand Down
12 changes: 6 additions & 6 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -86,7 +86,7 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO
ResolverDelegateProxy * proxy = static_cast<ResolverDelegateProxy *>(context);
if (CHIP_NO_ERROR != error)
{
proxy->OnNodeIdResolutionFailed(PeerId(), error);
proxy->OnOperationalNodeResolutionFailed(PeerId(), error);
proxy->Release();
return;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
73 changes: 69 additions & 4 deletions src/lib/dnssd/Resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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); }
};

/**
Expand All @@ -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.
Expand Down
71 changes: 54 additions & 17 deletions src/lib/dnssd/ResolverProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,38 +23,56 @@
namespace chip {
namespace Dnssd {

class ResolverDelegateProxy : public ReferenceCounted<ResolverDelegateProxy>, public ResolverDelegate
class ResolverDelegateProxy : public ReferenceCounted<ResolverDelegateProxy>,
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
Expand All @@ -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;
}
Expand Down
Loading

0 comments on commit 1049440

Please sign in to comment.