SetUpCodePairer: don't use DeviceDiscoveryDelegate (#12320)

* SetUpCodePairer: don't use DeviceDiscoveryDelegate

This class replaces the device discovery delegate that gets passed
in in the commissioning parameters which will cause the original
delegate to stop receiving callbacks if this class is used.
Instead, just have the commissioner notify this class directly.
The commissioner owns this part, so it doesn't need to use a derived
delegate because we know what we're talking to.

Also adding a discovery filter. This wasn't as necessary before
because the class would stop receiving notifications by setting the
delegate to null. However, that is still a race because it is
possible to get spurious mdns responses.

* Address review comments.

* Add back define guard on dns-sd function.

src/controller/CHIPDeviceController.cpp

@@ -1608,18 +1608,22 @@ void DeviceCommissioner::OnUserDirectedCommissioningRequest(UDCClientState state
 
     ChipLogDetail(Controller, "------To Accept Enter: discover udc-commission <pincode> <udc-client-index>");
 }
+#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
+
+#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 
 void DeviceCommissioner::OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData)
 {
+#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
     if (mUdcServer != nullptr)
     {
         mUdcServer->OnCommissionableNodeFound(nodeData);
     }
-
+#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
     AbstractDnssdDiscoveryController::OnNodeDiscoveryComplete(nodeData);
+    mSetUpCodePairer.NotifyCommissionableDeviceDiscovered(nodeData);
 }
-
-#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
+#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 
 void BasicSuccess(void * context, uint16_t val)
 {

src/controller/CHIPDeviceController.h

@@ -629,20 +629,22 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
 
     /**
      * @brief
-     *   Overrides method from AbstractDnssdDiscoveryController
-     *
-     * @param nodeData DNS-SD node information
+     *   Return the UDC Server instance
      *
      */
-    void OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
+    UserDirectedCommissioningServer * GetUserDirectedCommissioningServer() { return mUdcServer; }
+#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
 
+#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
     /**
      * @brief
-     *   Return the UDC Server instance
+     *   Overrides method from AbstractDnssdDiscoveryController
+     *
+     * @param nodeData DNS-SD node information
      *
      */
-    UserDirectedCommissioningServer * GetUserDirectedCommissioningServer() { return mUdcServer; }
-#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
+    void OnNodeDiscoveryComplete(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
+#endif
 
     void RegisterPairingDelegate(DevicePairingDelegate * pairingDelegate) { mPairingDelegate = pairingDelegate; }
 

src/controller/SetUpCodePairer.cpp

@@ -108,9 +108,9 @@ CHIP_ERROR SetUpCodePairer::StopConnectOverBle()
 CHIP_ERROR SetUpCodePairer::StartDiscoverOverIP(uint16_t discriminator, bool isShort)
 {
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    mCommissioner->RegisterDeviceDiscoveryDelegate(this);
-    Dnssd::DiscoveryFilter filter(isShort ? Dnssd::DiscoveryFilterType::kShort : Dnssd::DiscoveryFilterType::kLong, discriminator);
-    return mCommissioner->DiscoverCommissionableNodes(filter);
+    currentFilter.type = isShort ? Dnssd::DiscoveryFilterType::kShort : Dnssd::DiscoveryFilterType::kLong;
+    currentFilter.code = discriminator;
+    return mCommissioner->DiscoverCommissionableNodes(currentFilter);
 #else
     return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
@@ -119,7 +119,7 @@ CHIP_ERROR SetUpCodePairer::StartDiscoverOverIP(uint16_t discriminator, bool isS
 CHIP_ERROR SetUpCodePairer::StopConnectOverIP()
 {
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    mCommissioner->RegisterDeviceDiscoveryDelegate(nullptr);
+    currentFilter.type = Dnssd::DiscoveryFilterType::kNone;
 #endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
     return CHIP_NO_ERROR;
 }
@@ -162,8 +162,27 @@ void SetUpCodePairer::OnDiscoveredDeviceOverBleError(void * appState, CHIP_ERROR
 #endif // CONFIG_NETWORK_LAYER_BLE
 
 #if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-void SetUpCodePairer::OnDiscoveredDevice(const Dnssd::DiscoveredNodeData & nodeData)
+
+bool SetUpCodePairer::NodeMatchesCurrentFilter(const Dnssd::DiscoveredNodeData & nodeData)
+{
+    switch (currentFilter.type)
+    {
+    case Dnssd::DiscoveryFilterType::kShort:
+        return ((nodeData.longDiscriminator >> 8) & 0x0F) == currentFilter.code;
+    case Dnssd::DiscoveryFilterType::kLong:
+        return nodeData.longDiscriminator == currentFilter.code;
+    default:
+        return false;
+    }
+    return false;
+}
+
+void SetUpCodePairer::NotifyCommissionableDeviceDiscovered(const Dnssd::DiscoveredNodeData & nodeData)
 {
+    if (!NodeMatchesCurrentFilter(nodeData))
+    {
+        return;
+    }
     LogErrorOnFailure(StopConnectOverBle());
     LogErrorOnFailure(StopConnectOverIP());
     LogErrorOnFailure(StopConnectOverSoftAP());

src/controller/SetUpCodePairer.h

@@ -48,16 +48,18 @@ namespace Controller {
 class DeviceCommissioner;
 
 class DLL_EXPORT SetUpCodePairer
-#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    : public DeviceDiscoveryDelegate
-#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
 {
 public:
     SetUpCodePairer(DeviceCommissioner * commissioner) : mCommissioner(commissioner) {}
     virtual ~SetUpCodePairer() {}
 
     CHIP_ERROR PairDevice(chip::NodeId remoteId, const char * setUpCode);
 
+// Called by the DeviceCommissioner to notify that we have discovered a new device.
+#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+    void NotifyCommissionableDeviceDiscovered(const chip::Dnssd::DiscoveredNodeData & nodeData);
+#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+
 #if CONFIG_NETWORK_LAYER_BLE
     void SetBleLayer(Ble::BleLayer * bleLayer) { mBleLayer = bleLayer; };
 #endif // CONFIG_NETWORK_LAYER_BLE
@@ -73,11 +75,6 @@ class DLL_EXPORT SetUpCodePairer
 
     void OnDeviceDiscovered(RendezvousParameters & params);
 
-#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-    /////////// DeviceDiscoveryDelegate Interface /////////
-    void OnDiscoveredDevice(const chip::Dnssd::DiscoveredNodeData & nodeData) override;
-#endif // CHIP_DEVICE_CONFIG_ENABLE_DNSSD
-
 #if CONFIG_NETWORK_LAYER_BLE
     Ble::BleLayer * mBleLayer = nullptr;
     void OnDiscoveredDeviceOverBle(BLE_CONNECTION_OBJECT connObj);
@@ -86,6 +83,11 @@ class DLL_EXPORT SetUpCodePairer
     static void OnDiscoveredDeviceOverBleError(void * appState, CHIP_ERROR err);
 #endif // CONFIG_NETWORK_LAYER_BLE
 
+#if CHIP_DEVICE_CONFIG_ENABLE_DNSSD
+    bool NodeMatchesCurrentFilter(const Dnssd::DiscoveredNodeData & nodeData);
+    Dnssd::DiscoveryFilter currentFilter;
+#endif
+
     DeviceCommissioner * mCommissioner = nullptr;
     chip::NodeId mRemoteId;
     uint32_t mSetUpPINCode = 0;