Report mdns results from all interfaces instead of the highest priori… (#35597)
* [chip-tool] Add discover find-commissionable-by-instance-name command
* Report mdns results from all interfaces instead of the highest priority interface
* Update DnssdContexts.cpp
diff --git a/examples/chip-tool/commands/discover/Commands.h b/examples/chip-tool/commands/discover/Commands.h
index d308d4a..d70191f 100644
--- a/examples/chip-tool/commands/discover/Commands.h
+++ b/examples/chip-tool/commands/discover/Commands.h
@@ -84,6 +84,7 @@
make_unique<DiscoverCommissionableByCommissioningModeCommand>(credsIssuerConfig),
make_unique<DiscoverCommissionableByVendorIdCommand>(credsIssuerConfig),
make_unique<DiscoverCommissionableByDeviceTypeCommand>(credsIssuerConfig),
+ make_unique<DiscoverCommissionableByInstanceNameCommand>(credsIssuerConfig),
make_unique<DiscoverCommissionersCommand>(credsIssuerConfig),
};
diff --git a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp b/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp
index ad5e7fe..35d833e 100644
--- a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp
+++ b/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.cpp
@@ -122,3 +122,11 @@
chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kDeviceType, mDeviceType);
return mCommissioner->DiscoverCommissionableNodes(filter);
}
+
+CHIP_ERROR DiscoverCommissionableByInstanceNameCommand::RunCommand()
+{
+ mCommissioner = &CurrentCommissioner();
+ mCommissioner->RegisterDeviceDiscoveryDelegate(this);
+ chip::Dnssd::DiscoveryFilter filter(chip::Dnssd::DiscoveryFilterType::kInstanceName, mInstanceName);
+ return mCommissioner->DiscoverCommissionableNodes(filter);
+}
diff --git a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h b/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h
index d1fec30..2ba81b6 100644
--- a/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h
+++ b/examples/chip-tool/commands/discover/DiscoverCommissionablesCommand.h
@@ -159,3 +159,19 @@
// TODO: possibly 32-bit - see spec issue #3226
uint16_t mDeviceType;
};
+
+class DiscoverCommissionableByInstanceNameCommand : public DiscoverCommissionablesCommandBase
+{
+public:
+ DiscoverCommissionableByInstanceNameCommand(CredentialIssuerCommands * credsIssuerConfig) :
+ DiscoverCommissionablesCommandBase("find-commissionable-by-instance-name", credsIssuerConfig)
+ {
+ AddArgument("value", &mInstanceName);
+ }
+
+ /////////// CHIPCommand Interface /////////
+ CHIP_ERROR RunCommand() override;
+
+private:
+ char * mInstanceName;
+};
diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp
index 54947cd..6f98ac7 100644
--- a/src/lib/dnssd/Discovery_ImplPlatform.cpp
+++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp
@@ -43,7 +43,7 @@
{
DiscoveryContext * discoveryContext = static_cast<DiscoveryContext *>(context);
- if (error != CHIP_NO_ERROR)
+ if (error != CHIP_NO_ERROR && error != CHIP_ERROR_IN_PROGRESS)
{
discoveryContext->Release();
return;
@@ -55,7 +55,13 @@
nodeData.Get<CommissionNodeData>().LogDetail();
discoveryContext->OnNodeDiscovered(nodeData);
- discoveryContext->Release();
+
+ // CHIP_ERROR_IN_PROGRESS indicates that more results are coming, so don't release
+ // the context yet.
+ if (error == CHIP_NO_ERROR)
+ {
+ discoveryContext->Release();
+ }
}
static void HandleNodeOperationalBrowse(void * context, DnssdService * result, CHIP_ERROR error)
diff --git a/src/platform/Darwin/DnssdContexts.cpp b/src/platform/Darwin/DnssdContexts.cpp
index 9d293c0..e6b590d 100644
--- a/src/platform/Darwin/DnssdContexts.cpp
+++ b/src/platform/Darwin/DnssdContexts.cpp
@@ -564,88 +564,78 @@
};
#endif // TARGET_OS_TV
- for (auto interfaceIndex : priorityInterfaceIndices)
+ std::vector<InterfaceKey> interfacesOrder;
+ for (auto priorityInterfaceIndex : priorityInterfaceIndices)
{
- if (interfaceIndex == 0)
+ if (priorityInterfaceIndex == 0)
{
// Not actually an interface we have, since if_nametoindex
// returned 0.
continue;
}
- if (TryReportingResultsForInterfaceIndex(static_cast<uint32_t>(interfaceIndex)))
+ for (auto & interface : interfaces)
{
- return;
- }
- }
-
- for (auto & interface : interfaces)
- {
- if (TryReportingResultsForInterfaceIndex(interface.first.interfaceId, interface.first.hostname,
- interface.first.isSRPResult))
- {
- return;
- }
- }
-
- ChipLogError(Discovery, "Successfully finalizing resolve for %s without finding any actual IP addresses.",
- instanceName.c_str());
-}
-
-bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, const std::string & hostname, bool isSRPResult)
-{
- InterfaceKey interfaceKey = { interfaceIndex, hostname, isSRPResult };
- auto & interface = interfaces[interfaceKey];
- auto & ips = interface.addresses;
-
- // Some interface may not have any ips, just ignore them.
- if (ips.size() == 0)
- {
- return false;
- }
-
- ChipLogProgress(Discovery, "Mdns: Resolve success on interface %" PRIu32, interfaceIndex);
-
- auto & service = interface.service;
- auto addresses = Span<Inet::IPAddress>(ips.data(), ips.size());
- if (nullptr == callback)
- {
- auto delegate = static_cast<DiscoverNodeDelegate *>(context);
- DiscoveredNodeData nodeData;
-
- // Check whether mType (service name) exactly matches with operational service name
- if (strcmp(service.mType, kOperationalServiceName) == 0)
- {
- service.ToDiscoveredOperationalNodeBrowseData(nodeData);
- }
- else
- {
- service.ToDiscoveredCommissionNodeData(addresses, nodeData);
- }
- delegate->OnNodeDiscovered(nodeData);
- }
- else
- {
- callback(context, &service, addresses, CHIP_NO_ERROR);
- }
-
- return true;
-}
-
-bool ResolveContext::TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex)
-{
- for (auto & interface : interfaces)
- {
- if (interface.first.interfaceId == interfaceIndex)
- {
- if (TryReportingResultsForInterfaceIndex(interface.first.interfaceId, interface.first.hostname,
- interface.first.isSRPResult))
+ if (interface.second.HasAddresses() && priorityInterfaceIndex == interface.first.interfaceId)
{
- return true;
+ interfacesOrder.push_back(interface.first);
}
}
}
- return false;
+
+ for (auto & interface : interfaces)
+ {
+ // Skip interfaces that have already been prioritized to avoid duplicate results
+ auto interfaceKey = std::find(std::begin(interfacesOrder), std::end(interfacesOrder), interface.first);
+ if (interfaceKey != std::end(interfacesOrder))
+ {
+ continue;
+ }
+
+ // Some interface may not have any ips, just ignore them.
+ if (!interface.second.HasAddresses())
+ {
+ continue;
+ }
+
+ interfacesOrder.push_back(interface.first);
+ }
+
+ for (auto & interfaceKey : interfacesOrder)
+ {
+ auto & interfaceInfo = interfaces[interfaceKey];
+ auto & service = interfaceInfo.service;
+ auto & ips = interfaceInfo.addresses;
+ auto addresses = Span<Inet::IPAddress>(ips.data(), ips.size());
+
+ ChipLogProgress(Discovery, "Mdns: Resolve success on interface %" PRIu32, interfaceKey.interfaceId);
+
+ if (nullptr == callback)
+ {
+ auto delegate = static_cast<DiscoverNodeDelegate *>(context);
+ DiscoveredNodeData nodeData;
+
+ // Check whether mType (service name) exactly matches with operational service name
+ if (strcmp(service.mType, kOperationalServiceName) == 0)
+ {
+ service.ToDiscoveredOperationalNodeBrowseData(nodeData);
+ }
+ else
+ {
+ service.ToDiscoveredCommissionNodeData(addresses, nodeData);
+ }
+ delegate->OnNodeDiscovered(nodeData);
+ }
+ else
+ {
+ CHIP_ERROR error = &interfaceKey == &interfacesOrder.back() ? CHIP_NO_ERROR : CHIP_ERROR_IN_PROGRESS;
+ callback(context, &service, addresses, error);
+ }
+ }
+
+ VerifyOrDo(interfacesOrder.size(),
+ ChipLogError(Discovery, "Successfully finalizing resolve for %s without finding any actual IP addresses.",
+ instanceName.c_str()));
}
void ResolveContext::SRPTimerExpiredCallback(chip::System::Layer * systemLayer, void * callbackContext)
diff --git a/src/platform/Darwin/DnssdImpl.h b/src/platform/Darwin/DnssdImpl.h
index d4cd05f..9d10b4b 100644
--- a/src/platform/Darwin/DnssdImpl.h
+++ b/src/platform/Darwin/DnssdImpl.h
@@ -233,6 +233,7 @@
std::vector<Inet::IPAddress> addresses;
std::string fullyQualifiedDomainName;
bool isDNSLookUpRequested = false;
+ bool HasAddresses() const { return addresses.size() != 0; };
};
struct InterfaceKey
@@ -247,6 +248,11 @@
(this->isSRPResult < other.isSRPResult));
}
+ inline bool operator==(const InterfaceKey & other) const
+ {
+ return this->interfaceId == other.interfaceId && this->hostname == other.hostname && this->isSRPResult == other.isSRPResult;
+ }
+
uint32_t interfaceId;
std::string hostname;
bool isSRPResult = false;
@@ -310,16 +316,6 @@
*
*/
void CancelSRPTimerIfRunning();
-
-private:
- /**
- * Try reporting the results we got on the provided interface index.
- * Returns true if information was reported, false if not (e.g. if there
- * were no IP addresses, etc).
- */
- bool TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex, const std::string & hostname, bool isSRPResult);
-
- bool TryReportingResultsForInterfaceIndex(uint32_t interfaceIndex);
};
} // namespace Dnssd
