updates based on review feedback

project-chip · Apr 19, 2024 · 1149542 · 1149542
1 parent 139d874
commit 1149542
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 70 deletions.
diff --git a/examples/chip-tool/commands/common/DeviceScanner.cpp b/examples/chip-tool/commands/common/DeviceScanner.cpp
@@ -55,6 +55,7 @@ CHIP_ERROR DeviceScanner::Stop()
 
 void DeviceScanner::OnNodeDiscovered(const DiscoveredNodeData & nodeData)
 {
+    VerifyOrReturn(nodeData.Is<CommissionNodeData>())
     auto & commissionData = nodeData.Get<CommissionNodeData>();
 
     auto discriminator = commissionData.longDiscriminator;
@@ -64,7 +65,7 @@ void DeviceScanner::OnNodeDiscovered(const DiscoveredNodeData & nodeData)
     ChipLogProgress(chipTool, "OnNodeDiscovered (MDNS): discriminator: %u, vendorId: %u, productId: %u", discriminator, vendorId,
                     productId);
 
-    auto & resolutionData = (CommonResolutionData &) commissionData;
+    CommonResolutionData & resolutionData = commissionData;
 
     auto & instanceData  = mDiscoveredResults[commissionData.instanceName];
     auto & interfaceData = instanceData[resolutionData.interfaceId.GetPlatformInterface()];

diff --git a/examples/tv-casting-app/tv-casting-common/core/CastingPlayerDiscovery.cpp b/examples/tv-casting-app/tv-casting-common/core/CastingPlayerDiscovery.cpp
@@ -73,13 +73,12 @@ CHIP_ERROR CastingPlayerDiscovery::StopDiscovery()
     return CHIP_NO_ERROR;
 }
 
-void DeviceDiscoveryDelegateImpl::OnDiscoveredDevice(const chip::Dnssd::CommissionNodeData & discNodeData)
+void DeviceDiscoveryDelegateImpl::OnDiscoveredDevice(const chip::Dnssd::CommissionNodeData & nodeData)
 {
     ChipLogProgress(Discovery, "DeviceDiscoveryDelegateImpl::OnDiscoveredDevice() called");
     VerifyOrReturn(mClientDelegate != nullptr,
                    ChipLogError(Discovery, "DeviceDiscoveryDelegateImpl::OnDiscoveredDevice mClientDelegate is a nullptr"));
 
-    auto & nodeData = discNodeData;
     // convert nodeData to CastingPlayer
     CastingPlayerAttributes attributes;
     snprintf(attributes.id, kIdMaxLength + 1, "%s%u", nodeData.hostName, nodeData.port);

diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp
@@ -337,7 +337,7 @@ bool SetUpCodePairer::NodeMatchesCurrentFilter(const Dnssd::DiscoveredNodeData &
         return false;
     }
 
-    Dnssd::CommissionNodeData nodeData = discNodeData.Get<Dnssd::CommissionNodeData>();
+    const Dnssd::CommissionNodeData & nodeData = discNodeData.Get<Dnssd::CommissionNodeData>();
     if (nodeData.commissioningMode == 0)
     {
         ChipLogProgress(Controller, "Discovered device does not have an open commissioning window.");

diff --git a/src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm b/src/darwin/Framework/CHIP/MTRCommissionableBrowser.mm
@@ -161,6 +161,11 @@ void OnNodeDiscovered(const DiscoveredNodeData & nodeData) override
     {
         assertChipStackLockedByCurrentThread();
 
+        if (!nodeData.Is<CommissionNodeData>()) {
+            // not commissionable/commissioners node 
+            return;
+        }
+
         auto & commissionData = nodeData.Get<CommissionNodeData>();
         auto key = [NSString stringWithUTF8String:commissionData.instanceName];
         if ([mDiscoveredResults objectForKey:key] == nil) {

diff --git a/src/lib/dnssd/ActiveResolveAttempts.h b/src/lib/dnssd/ActiveResolveAttempts.h
@@ -153,24 +153,25 @@ class ActiveResolveAttempts
             {
                 return false;
             }
+            auto & nodeData = data.Get<chip::Dnssd::CommissionNodeData>();
 
             switch (browse.filter.type)
             {
             case chip::Dnssd::DiscoveryFilterType::kNone:
                 return true;
             case chip::Dnssd::DiscoveryFilterType::kShortDiscriminator:
                 return browse.filter.code ==
-                    static_cast<uint64_t>((data.Get<chip::Dnssd::CommissionNodeData>().longDiscriminator >> 8) & 0x0F);
+                    static_cast<uint64_t>((nodeData.longDiscriminator >> 8) & 0x0F);
             case chip::Dnssd::DiscoveryFilterType::kLongDiscriminator:
-                return browse.filter.code == data.Get<chip::Dnssd::CommissionNodeData>().longDiscriminator;
+                return browse.filter.code == nodeData.longDiscriminator;
             case chip::Dnssd::DiscoveryFilterType::kVendorId:
-                return browse.filter.code == data.Get<chip::Dnssd::CommissionNodeData>().vendorId;
+                return browse.filter.code == nodeData.vendorId;
             case chip::Dnssd::DiscoveryFilterType::kDeviceType:
-                return browse.filter.code == data.Get<chip::Dnssd::CommissionNodeData>().deviceType;
+                return browse.filter.code == nodeData.deviceType;
             case chip::Dnssd::DiscoveryFilterType::kCommissioningMode:
-                return browse.filter.code == data.Get<chip::Dnssd::CommissionNodeData>().commissioningMode;
+                return browse.filter.code == nodeData.commissioningMode;
             case chip::Dnssd::DiscoveryFilterType::kInstanceName:
-                return strncmp(browse.filter.instanceName, data.Get<chip::Dnssd::CommissionNodeData>().instanceName,
+                return strncmp(browse.filter.instanceName, nodeData.instanceName,
                                chip::Dnssd::Commission::kInstanceNameMaxLength + 1) == 0;
             case chip::Dnssd::DiscoveryFilterType::kCommissioner:
             case chip::Dnssd::DiscoveryFilterType::kCompressedFabricId:

diff --git a/src/lib/dnssd/IncrementalResolve.cpp b/src/lib/dnssd/IncrementalResolve.cpp
@@ -350,7 +350,8 @@ CHIP_ERROR IncrementalResolver::Take(DiscoveredNodeData & outputData)
     outputData.Set<CommissionNodeData>();
     CommissionNodeData & nodeData     = outputData.Get<CommissionNodeData>();
     nodeData                          = mSpecificResolutionData.Get<CommissionNodeData>();
-    (CommonResolutionData &) nodeData = mCommonResolutionData;
+    CommonResolutionData & resolutionData = nodeData;
+    resolutionData = mCommonResolutionData;
 
     ResetToInactive();
 

diff --git a/src/lib/dnssd/Types.h b/src/lib/dnssd/Types.h
@@ -235,6 +235,7 @@ struct CommissionNodeData : public CommonResolutionData
 
     void LogDetail() const
     {
+        ChipLogDetail(Discovery, "Discovered commissionable/commissioners node:");
         CommonResolutionData::LogDetail();
 
         if (rotatingIdLen > 0)
@@ -301,30 +302,6 @@ struct ResolvedNodeData
 
 using DiscoveredNodeData = Variant<CommissionNodeData>;
 
-#if 0
-struct DiscoveredNodeData
-{
-    CommonResolutionData resolutionData;
-    CommissionNodeData nodeData;
-    DiscoveryType nodeType;
-
-    void Reset()
-    {
-        resolutionData.Reset();
-        nodeData.Reset();
-        nodeType = DiscoveryType::kUnknown;
-    }
-    DiscoveredNodeData() { Reset(); }
-
-    void LogDetail() const
-    {
-        ChipLogDetail(Discovery, "Discovered node:");
-        resolutionData.LogDetail();
-        nodeData.LogDetail();
-    }
-};
-#endif
-
 /// Callbacks for discovering nodes advertising non-operational status:
 ///   - Commissioners
 ///   - Nodes in commissioning modes over IP (e.g. ethernet devices, devices already