[nrfconnect] Enable commissionable node advertising (project-chip#8454)

* [nrfconnect] Enable commissionable node advertising

1. Clean up DNS-SD default configuration as different
   settings were defined in two different places.
3. Clean up conditions for enabling commissionable node
   advertising.
4. Enable commissionable node advertising for nRF Connect
   platform.

* Fix issue with StartResolver removing existing services

examples/platform/nrfconnect/util/ThreadUtil.cpp

@@ -19,7 +19,8 @@
 #include "ThreadUtil.h"
 
 #include <platform/CHIPDeviceLayer.h>
-#include <platform/internal/DeviceNetworkInfo.h>
+
+#include <app/server/Mdns.h>
 #include <support/ThreadOperationalDataset.h>
 
 #include <zephyr.h>
@@ -33,18 +34,17 @@ void StartDefaultThreadNetwork(void)
     constexpr uint8_t masterkey[] = {
         0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF
     };
-    constexpr uint8_t meshLocalPrefix[] = { 0xfd, 0x11, 0x11, 0x11, 0x11, 0x22, 0x00, 0x00 };
-
     net_bytes_from_str(xpanid, sizeof(xpanid), CONFIG_OPENTHREAD_XPANID);
 
     dataset.SetChannel(CONFIG_OPENTHREAD_CHANNEL);
     dataset.SetExtendedPanId(xpanid);
     dataset.SetMasterKey(masterkey);
-    dataset.SetMeshLocalPrefix(meshLocalPrefix);
     dataset.SetNetworkName(CONFIG_OPENTHREAD_NETWORK_NAME);
     dataset.SetPanId(CONFIG_OPENTHREAD_PANID);
 
     chip::DeviceLayer::ThreadStackMgr().SetThreadEnabled(false);
     chip::DeviceLayer::ThreadStackMgr().SetThreadProvision(dataset.AsByteSpan());
     chip::DeviceLayer::ThreadStackMgr().SetThreadEnabled(true);
+
+    chip::app::Mdns::StartServer();
 }

src/app/server/Mdns.cpp

@@ -48,16 +48,16 @@ NodeId GetCurrentNodeId()
     // mdns advertises a single node id as parameter.
 
     // Search for one admin pairing and use its node id.
-    auto pairing = GetGlobalFabricTable().cbegin();
-    if (pairing != GetGlobalFabricTable().cend())
+    for (const Transport::FabricInfo & fabricInfo : GetGlobalFabricTable())
     {
-        ChipLogProgress(Discovery, "Found admin pairing for admin %" PRIX8 ", node 0x" ChipLogFormatX64, pairing->GetFabricIndex(),
-                        ChipLogValueX64(pairing->GetNodeId()));
-        return pairing->GetNodeId();
+        if (fabricInfo.GetNodeId() != kUndefinedNodeId)
+        {
+            return fabricInfo.GetNodeId();
+        }
     }
 
-    ChipLogError(Discovery, "Failed to find a valid admin pairing. Node ID unknown");
-    return chip::kTestDeviceNodeId;
+    ChipLogProgress(Discovery, "Failed to find a valid admin pairing. Node ID unknown");
+    return kUndefinedNodeId;
 }
 
 // Requires an 8-byte mac to accommodate thread.
@@ -252,7 +252,7 @@ void StartServer()
 
     // TODO: advertise this only when really operational once we support both
     // operational and commisioning advertising is supported.
-    if (DeviceLayer::ConfigurationMgr().IsFullyProvisioned())
+    if (GetCurrentNodeId() != kUndefinedNodeId)
     {
         err = app::Mdns::AdvertiseOperational();
 #if CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
@@ -261,10 +261,7 @@ void StartServer()
     }
     else
     {
-// TODO: Thread devices are not able to advertise using mDNS before being provisioned,
-// so configuraton should be added to enable commissioning advertising based on supported
-// Rendezvous methods.
-#if (!CHIP_DEVICE_CONFIG_ENABLE_THREAD || CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS)
+#if CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS
         err = app::Mdns::AdvertiseCommissionableNode();
 #endif
     }

src/include/platform/CHIPDeviceConfig.h

@@ -608,75 +608,6 @@
 #define CHIP_DEVICE_CONFIG_ENABLE_JUST_IN_TIME_PROVISIONING 0
 #endif
 
-// -------------------- Service Discovery Configuration -----------------------
-
-/**
- * CHIP_DEVICE_CONFIG_ENABLE_MDNS
- *
- * Enable support to use MDNS for service advertising and discovery in CHIP.
- */
-#ifndef CHIP_DEVICE_CONFIG_ENABLE_MDNS
-#define CHIP_DEVICE_CONFIG_ENABLE_MDNS 0
-#endif
-
-/**
- * CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS
- *
- * Enable MDNS commissionable node advertising when not yet provisioned.
- *
- * This should be 1 for WiFi SoftAP devices, ethernet devices, and (probably) bridge devices
- *
- * This should be 0 for Thread/BLE devices and WiFi/BLE devices
- */
-#ifndef CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS
-#define CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS 0
-#endif
-
-/**
- * CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES
- *
- * Maximum number of CHIP Commissioners or Commissionable Nodes that can be discovered
- */
-#ifndef CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES
-#define CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES 10
-#endif
-
-/**
- * CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
- *
- * Enable support to DNS-SD SRP client usage for service advertising and discovery in CHIP.
- */
-#ifndef CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
-#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT 0
-#endif
-
-/**
- * CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES
- *
- * Amount of services available for advertising using SRP.
- */
-#ifndef CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES
-#define CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES 3
-#endif
-
-/**
- * CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY
- *
- * Enable support to Commissionable Discovery for Thread devices.
- */
-#ifndef CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY
-#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY 0
-#endif
-
-/**
- * CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
- *
- * Enable support to DNS client usage for resolving and browsing services in CHIP.
- */
-#ifndef CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
-#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT 0
-#endif
-
 // -------------------- Thread Configuration --------------------
 
 /**
@@ -770,6 +701,42 @@
 #define CHIP_DEVICE_CONFIG_THREAD_ENABLE_CLI 0
 #endif
 
+/**
+ * CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
+ *
+ * Enable support to DNS-SD SRP client usage for service advertising and discovery in CHIP.
+ */
+#ifndef CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT
+#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT 0
+#endif
+
+/**
+ * CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES
+ *
+ * Amount of services available for advertising using SRP.
+ */
+#ifndef CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES
+#define CHIP_DEVICE_CONFIG_THREAD_SRP_MAX_SERVICES 3
+#endif
+
+/**
+ * CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY
+ *
+ * Enable support to Commissionable Discovery for Thread devices.
+ */
+#ifndef CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY
+#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY 0
+#endif
+
+/**
+ * CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
+ *
+ * Enable support to DNS client usage for resolving and browsing services in CHIP.
+ */
+#ifndef CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
+#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT 0
+#endif
+
 // -------------------- Trait Manager Configuration --------------------
 
 /**
@@ -1103,7 +1070,42 @@
 #define CHIP_DEVICE_CONFIG_FIRMWARE_BUILD_TIME __TIME__
 #endif
 
-// -------------------- Device DNS-SD Advertising Configuration --------------------
+// -------------------- Device DNS-SD Configuration --------------------
+
+/**
+ * CHIP_DEVICE_CONFIG_ENABLE_MDNS
+ *
+ * Enable support to use MDNS for service advertising and discovery in CHIP.
+ */
+#ifndef CHIP_DEVICE_CONFIG_ENABLE_MDNS
+#define CHIP_DEVICE_CONFIG_ENABLE_MDNS 0
+#endif
+
+/**
+ * CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS
+ *
+ * Enable MDNS commissionable node advertising when not yet provisioned.
+ *
+ * This should be 1 for WiFi SoftAP devices, ethernet devices, and (probably) bridge devices
+ *
+ * This should be 0 for Thread/BLE devices and WiFi/BLE devices
+ */
+#ifndef CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS
+#if CHIP_DEVICE_CONFIG_ENABLE_THREAD
+#define CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS 0
+#else
+#define CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS 1
+#endif
+#endif
+
+/**
+ * CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES
+ *
+ * Maximum number of CHIP Commissioners or Commissionable Nodes that can be discovered
+ */
+#ifndef CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES
+#define CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES 10
+#endif
 
 /**
  * CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY

src/lib/mdns/Discovery_ImplPlatform.h

@@ -36,8 +36,11 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
 public:
     CHIP_ERROR Init();
 
+    /// Starts the service advertiser if not yet started. Otherwise, removes all existing services.
     CHIP_ERROR Start(Inet::InetLayer * inetLayer, uint16_t port) override;
-    CHIP_ERROR StartResolver(Inet::InetLayer * inetLayer, uint16_t port) override { return Start(inetLayer, port); }
+
+    /// Starts the service resolver if not yet started
+    CHIP_ERROR StartResolver(Inet::InetLayer * inetLayer, uint16_t port) override { return Init(); }
 
     /// Advertises the CHIP node as an operational node
     CHIP_ERROR Advertise(const OperationalAdvertisingParameters & params) override;

src/lib/shell/commands/Dns.cpp

@@ -32,7 +32,6 @@ namespace chip {
 namespace Shell {
 
 static chip::Shell::Engine sShellDnsSubcommands;
-static bool isResolverStarted;
 
 class DnsShellResolverDelegate : public chip::Mdns::ResolverDelegate
 {
@@ -104,19 +103,15 @@ static CHIP_ERROR BrowseHandler(int argc, char ** argv)
 
 static CHIP_ERROR DnsHandler(int argc, char ** argv)
 {
-    if (!isResolverStarted)
-    {
-        chip::Mdns::Resolver::Instance().StartResolver(&chip::DeviceLayer::InetLayer, chip::Mdns::kMdnsPort);
-        chip::Mdns::Resolver::Instance().SetResolverDelegate(&sDnsShellResolverDelegate);
-
-        isResolverStarted = true;
-    }
-
     if (argc == 0)
     {
         DnsHelpHandler(argc, argv);
         return CHIP_NO_ERROR;
     }
+
+    chip::Mdns::Resolver::Instance().StartResolver(&chip::DeviceLayer::InetLayer, chip::Mdns::kMdnsPort);
+    chip::Mdns::Resolver::Instance().SetResolverDelegate(&sDnsShellResolverDelegate);
+
     return sShellDnsSubcommands.ExecCommand(argc, argv);
 }
 

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -1218,15 +1218,18 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_RemoveSrpServic
 template <class ImplClass>
 CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_RemoveAllSrpServices()
 {
-    CHIP_ERROR error;
+    CHIP_ERROR error = CHIP_NO_ERROR;
 
     Impl()->LockThreadStack();
-    const otSrpClientService * services = otSrpClientGetServices(mOTInst);
 
-    // In case of empty list just return with no error
-    VerifyOrExit(services != nullptr, error = CHIP_NO_ERROR);
+    for (typename SrpClient::Service & service : mSrpClient.mServices)
+    {
+        if (!service.IsUsed())
+            continue;
 
-    error = MapOpenThreadError(otSrpClientRemoveHostAndServices(mOTInst, false));
+        error = MapOpenThreadError(otSrpClientRemoveService(mOTInst, &service.mService));
+        SuccessOrExit(error);
+    }
 
 exit:
     Impl()->UnlockThreadStack();

src/platform/nrfconnect/CHIPDevicePlatformConfig.h

@@ -75,6 +75,8 @@
 #ifdef CONFIG_CHIP_ENABLE_DNSSD_SRP
 #define CHIP_DEVICE_CONFIG_ENABLE_MDNS 1
 #define CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT 1
+#define CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY 1
+#define CHIP_DEVICE_CONFIG_ENABLE_UNPROVISIONED_MDNS 1
 #ifdef CONFIG_CHIP_ENABLE_DNS_CLIENT
 #define CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT 1
 #define CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY 1