Update DNSSD records for resolve/browse on common vs commission/opera…

…tional data (#18266)

* Update minmdns client to use 0 for query id, otherwise OTBR does not seem to reply

* Split out common discovery data from commissionable/operational discovery specific data

* Restyle

* Add reset for ResolvedNodeData for consistency

* Start splitting common and environment-specific parsing data for dns resolution

* Fix some tests

* Update to make things compile. Remove mExpiryTime

* More compile update - tests and chip-tool

* Updates for platform impl to also compile

* One more change because easy/obvious: make the IP address receiving common between commission and operational discovery

* Remove unused variable

* More unused removal - compiles with clang

* Fix python compile

* Make tv app compile

* Remove unused variable

* Code review comments

* Remove extra check on num ips to be in sync with previous code. Seems buggy but want to test if this makes unit tests pass in repl

* Fix typo in num ip addition

* Add back numips check

examples/chip-tool/commands/pairing/PairingCommand.cpp

@@ -207,19 +207,20 @@ void PairingCommand::OnCommissioningComplete(NodeId nodeId, CHIP_ERROR err)
 void PairingCommand::OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData)
 {
     // Ignore nodes with closed comissioning window
-    VerifyOrReturn(nodeData.commissioningMode != 0);
+    VerifyOrReturn(nodeData.commissionData.commissioningMode != 0);
 
-    const uint16_t port = nodeData.port;
+    const uint16_t port = nodeData.resolutionData.port;
     char buf[chip::Inet::IPAddress::kMaxStringLength];
-    nodeData.ipAddress[0].ToString(buf);
+    nodeData.resolutionData.ipAddress[0].ToString(buf);
     ChipLogProgress(chipTool, "Discovered Device: %s:%u", buf, port);
 
     // Stop Mdns discovery. Is it the right method ?
     CurrentCommissioner().RegisterDeviceDiscoveryDelegate(nullptr);
 
-    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);
+    Inet::InterfaceId interfaceId =
+        nodeData.resolutionData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.resolutionData.interfaceId : Inet::InterfaceId::Null();
+    PeerAddress peerAddress = PeerAddress::UDP(nodeData.resolutionData.ipAddress[0], port, interfaceId);
+    CHIP_ERROR err          = Pair(mNodeId, peerAddress);
     if (CHIP_NO_ERROR != err)
     {
         SetCommandExitStatus(err);

examples/minimal-mdns/client.cpp

@@ -48,7 +48,7 @@ struct Options
     mdns::Minimal::QType type = mdns::Minimal::QType::ANY;
 } gOptions;
 
-constexpr uint32_t kTestMessageId   = 0x1234;
+constexpr uint32_t kTestMessageId   = 0;
 constexpr size_t kMdnsMaxPacketSize = 1'024;
 
 using namespace chip::ArgParser;

examples/platform/linux/ControllerShellCommands.cpp

@@ -125,7 +125,8 @@ static CHIP_ERROR display(bool printHeader)
         else
         {
             streamer_printf(sout, "  Entry %d instanceName=%s host=%s longDiscriminator=%d vendorId=%d productId=%d\r\n", i,
-                            next->instanceName, next->hostName, next->longDiscriminator, next->vendorId, next->productId);
+                            next->commissionData.instanceName, next->resolutionData.hostName,
+                            next->commissionData.longDiscriminator, next->commissionData.vendorId, next->commissionData.productId);
         }
     }
 

src/app/CASESessionManager.cpp

@@ -31,8 +31,6 @@ CHIP_ERROR CASESessionManager::Init(chip::System::Layer * systemLayer, const CAS
 CHIP_ERROR CASESessionManager::FindOrEstablishSession(PeerId peerId, Callback::Callback<OnDeviceConnected> * onConnection,
                                                       Callback::Callback<OnDeviceConnectionFailure> * onFailure)
 {
-    Dnssd::ResolvedNodeData resolutionData;
-
     ChipLogDetail(CASESessionManager, "FindOrEstablishSession: PeerId = " ChipLogFormatX64 ":" ChipLogFormatX64,
                   ChipLogValueX64(peerId.GetCompressedFabricId()), ChipLogValueX64(peerId.GetNodeId()));
 

src/app/OperationalDeviceProxy.h

@@ -153,7 +153,7 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,
     {
         mDeviceAddress = ToPeerAddress(nodeResolutionData);
 
-        mRemoteMRPConfig = nodeResolutionData.GetMRPConfig();
+        mRemoteMRPConfig = nodeResolutionData.resolutionData.GetMRPConfig();
 
         if (mState == State::NeedsAddress)
         {
@@ -195,12 +195,12 @@ class DLL_EXPORT OperationalDeviceProxy : public DeviceProxy,
         // For all other addresses, we should rely on the device's routing table to route messages sent.
         // Forcing messages down an InterfaceId might fail. For example, in bridged networks like Thread,
         // mDNS advertisements are not usually received on the same interface the peer is reachable on.
-        if (nodeData.mAddress[0].IsIPv6LinkLocal())
+        if (nodeData.resolutionData.ipAddress[0].IsIPv6LinkLocal())
         {
-            interfaceId = nodeData.mInterfaceId;
+            interfaceId = nodeData.resolutionData.interfaceId;
         }
 
-        return Transport::PeerAddress::UDP(nodeData.mAddress[0], nodeData.mPort, interfaceId);
+        return Transport::PeerAddress::UDP(nodeData.resolutionData.ipAddress[0], nodeData.resolutionData.port, interfaceId);
     }
 
     /**

src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp

@@ -141,37 +141,38 @@ void DiscoveryCommands::OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData &
     nodeData.LogDetail();
 
     chip::DiscoveryCommandResponse data;
-    data.hostName           = chip::CharSpan(nodeData.hostName, strlen(nodeData.hostName));
-    data.instanceName       = chip::CharSpan(nodeData.instanceName, strlen(nodeData.instanceName));
-    data.longDiscriminator  = nodeData.longDiscriminator;
-    data.shortDiscriminator = ((nodeData.longDiscriminator >> 8) & 0x0F);
-    data.vendorId           = nodeData.vendorId;
-    data.productId          = nodeData.productId;
-    data.commissioningMode  = nodeData.commissioningMode;
-    data.deviceType         = nodeData.deviceType;
-    data.deviceName         = chip::CharSpan(nodeData.deviceName, strlen(nodeData.deviceName));
-    data.rotatingId         = chip::ByteSpan(nodeData.rotatingId, nodeData.rotatingIdLen);
-    data.rotatingIdLen      = nodeData.rotatingIdLen;
-    data.pairingHint        = nodeData.pairingHint;
-    data.pairingInstruction = chip::CharSpan(nodeData.pairingInstruction, strlen(nodeData.pairingInstruction));
-    data.supportsTcp        = nodeData.supportsTcp;
-    data.port               = nodeData.port;
-
-    if (!chip::CanCastTo<uint8_t>(nodeData.numIPs))
+    data.hostName           = chip::CharSpan(nodeData.resolutionData.hostName, strlen(nodeData.resolutionData.hostName));
+    data.instanceName       = chip::CharSpan(nodeData.commissionData.instanceName, strlen(nodeData.commissionData.instanceName));
+    data.longDiscriminator  = nodeData.commissionData.longDiscriminator;
+    data.shortDiscriminator = ((nodeData.commissionData.longDiscriminator >> 8) & 0x0F);
+    data.vendorId           = nodeData.commissionData.vendorId;
+    data.productId          = nodeData.commissionData.productId;
+    data.commissioningMode  = nodeData.commissionData.commissioningMode;
+    data.deviceType         = nodeData.commissionData.deviceType;
+    data.deviceName         = chip::CharSpan(nodeData.commissionData.deviceName, strlen(nodeData.commissionData.deviceName));
+    data.rotatingId         = chip::ByteSpan(nodeData.commissionData.rotatingId, nodeData.commissionData.rotatingIdLen);
+    data.rotatingIdLen      = nodeData.commissionData.rotatingIdLen;
+    data.pairingHint        = nodeData.commissionData.pairingHint;
+    data.pairingInstruction =
+        chip::CharSpan(nodeData.commissionData.pairingInstruction, strlen(nodeData.commissionData.pairingInstruction));
+    data.supportsTcp = nodeData.resolutionData.supportsTcp;
+    data.port        = nodeData.resolutionData.port;
+
+    if (!chip::CanCastTo<uint8_t>(nodeData.resolutionData.numIPs))
     {
         ChipLogError(chipTool, "Too many ips.");
         return;
     }
-    data.numIPs = static_cast<uint8_t>(nodeData.numIPs);
+    data.numIPs = static_cast<uint8_t>(nodeData.resolutionData.numIPs);
 
-    if (nodeData.mrpRetryIntervalIdle.HasValue())
+    if (nodeData.resolutionData.mrpRetryIntervalIdle.HasValue())
     {
-        data.mrpRetryIntervalIdle.SetValue(nodeData.mrpRetryIntervalIdle.Value().count());
+        data.mrpRetryIntervalIdle.SetValue(nodeData.resolutionData.mrpRetryIntervalIdle.Value().count());
     }
 
-    if (nodeData.mrpRetryIntervalActive.HasValue())
+    if (nodeData.resolutionData.mrpRetryIntervalActive.HasValue())
     {
-        data.mrpRetryIntervalActive.SetValue(nodeData.mrpRetryIntervalActive.Value().count());
+        data.mrpRetryIntervalActive.SetValue(nodeData.resolutionData.mrpRetryIntervalActive.Value().count());
     }
 
     chip::app::StatusIB status;

src/controller/AbstractDnssdDiscoveryController.cpp

@@ -30,11 +30,12 @@ void AbstractDnssdDiscoveryController::OnNodeDiscovered(const chip::Dnssd::Disco
     auto discoveredNodes = GetDiscoveredNodes();
     for (auto & discoveredNode : discoveredNodes)
     {
-        if (!discoveredNode.IsValid())
+        if (!discoveredNode.resolutionData.IsValid())
         {
             continue;
         }
-        if (strcmp(discoveredNode.hostName, nodeData.hostName) == 0 && discoveredNode.port == nodeData.port)
+        if (strcmp(discoveredNode.resolutionData.hostName, nodeData.resolutionData.hostName) == 0 &&
+            discoveredNode.resolutionData.port == nodeData.resolutionData.port)
         {
             discoveredNode = nodeData;
             if (mDeviceDiscoveryDelegate != nullptr)
@@ -47,7 +48,7 @@ void AbstractDnssdDiscoveryController::OnNodeDiscovered(const chip::Dnssd::Disco
     // Node not yet in the list
     for (auto & discoveredNode : discoveredNodes)
     {
-        if (!discoveredNode.IsValid())
+        if (!discoveredNode.resolutionData.IsValid())
         {
             discoveredNode = nodeData;
             if (mDeviceDiscoveryDelegate != nullptr)
@@ -57,7 +58,7 @@ void AbstractDnssdDiscoveryController::OnNodeDiscovered(const chip::Dnssd::Disco
             return;
         }
     }
-    ChipLogError(Discovery, "Failed to add discovered node with hostname %s- Insufficient space", nodeData.hostName);
+    ChipLogError(Discovery, "Failed to add discovered node with hostname %s- Insufficient space", nodeData.resolutionData.hostName);
 }
 
 CHIP_ERROR AbstractDnssdDiscoveryController::SetUpNodeDiscovery()
@@ -74,7 +75,7 @@ const Dnssd::DiscoveredNodeData * AbstractDnssdDiscoveryController::GetDiscovere
 {
     // TODO(cecille): Add assertion about main loop.
     auto discoveredNodes = GetDiscoveredNodes();
-    if (0 <= idx && idx < CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES && discoveredNodes.data()[idx].IsValid())
+    if (0 <= idx && idx < CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES && discoveredNodes.data()[idx].resolutionData.IsValid())
     {
         return discoveredNodes.data() + idx;
     }

src/controller/SetUpCodePairer.cpp

@@ -278,17 +278,17 @@ void SetUpCodePairer::OnBLEDiscoveryError(CHIP_ERROR err)
 
 bool SetUpCodePairer::NodeMatchesCurrentFilter(const Dnssd::DiscoveredNodeData & nodeData) const
 {
-    if (nodeData.commissioningMode == 0)
+    if (nodeData.commissionData.commissioningMode == 0)
     {
         return false;
     }
 
     switch (currentFilter.type)
     {
     case Dnssd::DiscoveryFilterType::kShortDiscriminator:
-        return ((nodeData.longDiscriminator >> 8) & 0x0F) == currentFilter.code;
+        return ((nodeData.commissionData.longDiscriminator >> 8) & 0x0F) == currentFilter.code;
     case Dnssd::DiscoveryFilterType::kLongDiscriminator:
-        return nodeData.longDiscriminator == currentFilter.code;
+        return nodeData.commissionData.longDiscriminator == currentFilter.code;
     default:
         return false;
     }
@@ -309,8 +309,10 @@ void SetUpCodePairer::NotifyCommissionableDeviceDiscovered(const Dnssd::Discover
     LogErrorOnFailure(StopConnectOverIP());
     LogErrorOnFailure(StopConnectOverSoftAP());
 
-    Inet::InterfaceId interfaceId      = nodeData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.interfaceId : Inet::InterfaceId::Null();
-    Transport::PeerAddress peerAddress = Transport::PeerAddress::UDP(nodeData.ipAddress[0], nodeData.port, interfaceId);
+    Inet::InterfaceId interfaceId =
+        nodeData.resolutionData.ipAddress[0].IsIPv6LinkLocal() ? nodeData.resolutionData.interfaceId : Inet::InterfaceId::Null();
+    Transport::PeerAddress peerAddress =
+        Transport::PeerAddress::UDP(nodeData.resolutionData.ipAddress[0], nodeData.resolutionData.port, interfaceId);
     mDiscoveredParameters[kIPTransport] = RendezvousParameters().SetPeerAddress(peerAddress);
     ConnectToDiscoveredDevice();
 }

src/controller/python/ChipCommissionableNodeController-ScriptBinding.cpp

@@ -83,42 +83,45 @@ void pychip_CommissionableNodeController_PrintDiscoveredCommissioners(
             continue;
         }
         char rotatingId[chip::Dnssd::kMaxRotatingIdLen * 2 + 1] = "";
-        Encoding::BytesToUppercaseHexString(dnsSdInfo->rotatingId, dnsSdInfo->rotatingIdLen, rotatingId, sizeof(rotatingId));
+        Encoding::BytesToUppercaseHexString(dnsSdInfo->commissionData.rotatingId, dnsSdInfo->commissionData.rotatingIdLen,
+                                            rotatingId, sizeof(rotatingId));
 
         ChipLogProgress(Discovery, "Commissioner %d", i);
-        ChipLogProgress(Discovery, "\tInstance name:\t\t%s", dnsSdInfo->instanceName);
-        ChipLogProgress(Discovery, "\tHost name:\t\t%s", dnsSdInfo->hostName);
-        ChipLogProgress(Discovery, "\tPort:\t\t\t%u", dnsSdInfo->port);
-        ChipLogProgress(Discovery, "\tLong discriminator:\t%u", dnsSdInfo->longDiscriminator);
-        ChipLogProgress(Discovery, "\tVendor ID:\t\t%u", dnsSdInfo->vendorId);
-        ChipLogProgress(Discovery, "\tProduct ID:\t\t%u", dnsSdInfo->productId);
-        ChipLogProgress(Discovery, "\tCommissioning Mode\t%u", dnsSdInfo->commissioningMode);
-        ChipLogProgress(Discovery, "\tDevice Type\t\t%u", dnsSdInfo->deviceType);
-        ChipLogProgress(Discovery, "\tDevice Name\t\t%s", dnsSdInfo->deviceName);
+        ChipLogProgress(Discovery, "\tInstance name:\t\t%s", dnsSdInfo->commissionData.instanceName);
+        ChipLogProgress(Discovery, "\tHost name:\t\t%s", dnsSdInfo->resolutionData.hostName);
+        ChipLogProgress(Discovery, "\tPort:\t\t\t%u", dnsSdInfo->resolutionData.port);
+        ChipLogProgress(Discovery, "\tLong discriminator:\t%u", dnsSdInfo->commissionData.longDiscriminator);
+        ChipLogProgress(Discovery, "\tVendor ID:\t\t%u", dnsSdInfo->commissionData.vendorId);
+        ChipLogProgress(Discovery, "\tProduct ID:\t\t%u", dnsSdInfo->commissionData.productId);
+        ChipLogProgress(Discovery, "\tCommissioning Mode\t%u", dnsSdInfo->commissionData.commissioningMode);
+        ChipLogProgress(Discovery, "\tDevice Type\t\t%u", dnsSdInfo->commissionData.deviceType);
+        ChipLogProgress(Discovery, "\tDevice Name\t\t%s", dnsSdInfo->commissionData.deviceName);
         ChipLogProgress(Discovery, "\tRotating Id\t\t%s", rotatingId);
-        ChipLogProgress(Discovery, "\tPairing Instruction\t%s", dnsSdInfo->pairingInstruction);
-        ChipLogProgress(Discovery, "\tPairing Hint\t\t%u", dnsSdInfo->pairingHint);
-        if (dnsSdInfo->GetMrpRetryIntervalIdle().HasValue())
+        ChipLogProgress(Discovery, "\tPairing Instruction\t%s", dnsSdInfo->commissionData.pairingInstruction);
+        ChipLogProgress(Discovery, "\tPairing Hint\t\t%u", dnsSdInfo->commissionData.pairingHint);
+        if (dnsSdInfo->resolutionData.GetMrpRetryIntervalIdle().HasValue())
         {
-            ChipLogProgress(Discovery, "\tMrp Interval idle\t%u", dnsSdInfo->GetMrpRetryIntervalIdle().Value().count());
+            ChipLogProgress(Discovery, "\tMrp Interval idle\t%u",
+                            dnsSdInfo->resolutionData.GetMrpRetryIntervalIdle().Value().count());
         }
         else
         {
             ChipLogProgress(Discovery, "\tMrp Interval idle\tNot present");
         }
-        if (dnsSdInfo->GetMrpRetryIntervalActive().HasValue())
+        if (dnsSdInfo->resolutionData.GetMrpRetryIntervalActive().HasValue())
         {
-            ChipLogProgress(Discovery, "\tMrp Interval active\t%u", dnsSdInfo->GetMrpRetryIntervalActive().Value().count());
+            ChipLogProgress(Discovery, "\tMrp Interval active\t%u",
+                            dnsSdInfo->resolutionData.GetMrpRetryIntervalActive().Value().count());
         }
         else
         {
             ChipLogProgress(Discovery, "\tMrp Interval active\tNot present");
         }
-        ChipLogProgress(Discovery, "\tSupports TCP\t\t%d", dnsSdInfo->supportsTcp);
-        for (unsigned j = 0; j < dnsSdInfo->numIPs; ++j)
+        ChipLogProgress(Discovery, "\tSupports TCP\t\t%d", dnsSdInfo->resolutionData.supportsTcp);
+        for (unsigned j = 0; j < dnsSdInfo->resolutionData.numIPs; ++j)
         {
             char buf[chip::Inet::IPAddress::kMaxStringLength];
-            dnsSdInfo->ipAddress[j].ToString(buf);
+            dnsSdInfo->resolutionData.ipAddress[j].ToString(buf);
             ChipLogProgress(Discovery, "\tAddress %d:\t\t%s", j, buf);
         }
     }