From 875465d9ee7935d7e1f23d015d211665350b960e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 14 Feb 2022 11:37:39 -0500 Subject: [PATCH] Set DiscoveredNodeData have a single interfaceId. (#15087) Previous implementation was assuming every IP address has its own interface, however resolved nodes generally are only on one interface: - Platform MDNS will resolve nodes one by one (one IP only) - MinMDNS will process packets one by one, so all addresses belong to a single interface This should have a slight RAM improvement and will make future logic for processing MDNS entries for figuring out the 'best IP address to use' simpler. --- .../chip-tool/commands/pairing/PairingCommand.cpp | 2 +- examples/tv-casting-app/linux/main.cpp | 2 +- src/controller/SetUpCodePairer.cpp | 2 +- src/lib/dnssd/Discovery_ImplPlatform.cpp | 13 +++++++++---- src/lib/dnssd/Resolver.h | 2 +- src/lib/dnssd/Resolver_ImplMinimalMdns.cpp | 4 ++-- 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp index c568bf45f5d08f..c0690578b1a04a 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.cpp +++ b/examples/chip-tool/commands/pairing/PairingCommand.cpp @@ -205,7 +205,7 @@ void PairingCommand::OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & // Stop Mdns discovery. Is it the right method ? CurrentCommissioner().RegisterDeviceDiscoveryDelegate(nullptr); - Inet::InterfaceId interfaceId = nodeData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.interfaceId[0] : Inet::InterfaceId::Null(); + Inet::InterfaceId interfaceId = nodeData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.interfaceId : Inet::InterfaceId::Null(); PeerAddress peerAddress = PeerAddress::UDP(nodeData.ipAddress[0], port, interfaceId); CHIP_ERROR err = Pair(mNodeId, peerAddress); if (CHIP_NO_ERROR != err) diff --git a/examples/tv-casting-app/linux/main.cpp b/examples/tv-casting-app/linux/main.cpp index 6f5cdad6a33302..67dd40f3c15b64 100644 --- a/examples/tv-casting-app/linux/main.cpp +++ b/examples/tv-casting-app/linux/main.cpp @@ -138,7 +138,7 @@ void PrepareForCommissioning(const Dnssd::DiscoveredNodeData * selectedCommissio // Send User Directed commissioning request ReturnOnFailure(Server::GetInstance().SendUserDirectedCommissioningRequest(chip::Transport::PeerAddress::UDP( - selectedCommissioner->ipAddress[0], selectedCommissioner->port, selectedCommissioner->interfaceId[0]))); + selectedCommissioner->ipAddress[0], selectedCommissioner->port, selectedCommissioner->interfaceId))); } #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT } diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp index 75d710308f54bb..6817d2181bf801 100644 --- a/src/controller/SetUpCodePairer.cpp +++ b/src/controller/SetUpCodePairer.cpp @@ -194,7 +194,7 @@ void SetUpCodePairer::NotifyCommissionableDeviceDiscovered(const Dnssd::Discover LogErrorOnFailure(StopConnectOverIP()); LogErrorOnFailure(StopConnectOverSoftAP()); - Inet::InterfaceId interfaceId = nodeData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.interfaceId[0] : Inet::InterfaceId::Null(); + Inet::InterfaceId interfaceId = nodeData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.interfaceId : Inet::InterfaceId::Null(); Transport::PeerAddress peerAddress = Transport::PeerAddress::UDP(nodeData.ipAddress[0], nodeData.port, interfaceId); RendezvousParameters params = RendezvousParameters().SetPeerAddress(peerAddress); OnDeviceDiscovered(params); diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 4a7c5d74fbf3c3..741660bdf1e465 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -53,14 +53,19 @@ static void HandleNodeResolve(void * context, DnssdService * result, CHIP_ERROR } DiscoveredNodeData nodeData; + Platform::CopyString(nodeData.hostName, result->mHostName); Platform::CopyString(nodeData.instanceName, result->mName); - if (result->mAddress.HasValue() && nodeData.numIPs < DiscoveredNodeData::kMaxIPAddresses) + if (result->mAddress.HasValue()) + { + nodeData.ipAddress[0] = result->mAddress.Value(); + nodeData.interfaceId = result->mInterface; + nodeData.numIPs = 1; + } + else { - nodeData.ipAddress[nodeData.numIPs] = result->mAddress.Value(); - nodeData.interfaceId[nodeData.numIPs] = result->mInterface; - nodeData.numIPs++; + nodeData.numIPs = 0; } nodeData.port = result->mPort; diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h index 6a0263b858c53a..3adac58e4a0242 100644 --- a/src/lib/dnssd/Resolver.h +++ b/src/lib/dnssd/Resolver.h @@ -161,7 +161,7 @@ struct DiscoveredNodeData Optional mrpRetryIntervalActive; uint16_t port; unsigned numIPs; - Inet::InterfaceId interfaceId[kMaxIPAddresses]; + Inet::InterfaceId interfaceId; // interface ID for the packet processed Inet::IPAddress ipAddress[kMaxIPAddresses]; void Reset() diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp index b2664da7e03cdd..957aa90819021e 100644 --- a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp +++ b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp @@ -193,8 +193,8 @@ void PacketDataReporter::OnDiscoveredNodeIPAddress(const chip::Inet::IPAddress & { return; } - mDiscoveredNodeData.ipAddress[mDiscoveredNodeData.numIPs] = addr; - mDiscoveredNodeData.interfaceId[mDiscoveredNodeData.numIPs] = mInterfaceId; + mDiscoveredNodeData.ipAddress[mDiscoveredNodeData.numIPs] = addr; + mDiscoveredNodeData.interfaceId = mInterfaceId; mDiscoveredNodeData.numIPs++; }