Skip to content

Commit

Permalink
Set DiscoveredNodeData have a single interfaceId. (#15087)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
andy31415 authored and pull[bot] committed Nov 3, 2023
1 parent 68bf14e commit 875465d
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 10 deletions.
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/pairing/PairingCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion examples/tv-casting-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/controller/SetUpCodePairer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 9 additions & 4 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/Resolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ struct DiscoveredNodeData
Optional<System::Clock::Milliseconds32> 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()
Expand Down
4 changes: 2 additions & 2 deletions src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand Down

0 comments on commit 875465d

Please sign in to comment.