From 2575a07d6fa3d079e0890688398ab859876bc536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Tue, 3 Aug 2021 18:44:17 +0200 Subject: [PATCH] [nrfconnect] Enable commissionable node advertising (#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 --- .../platform/nrfconnect/util/ThreadUtil.cpp | 8 +- src/app/server/Mdns.cpp | 21 ++- src/include/platform/CHIPDeviceConfig.h | 142 +++++++++--------- src/lib/mdns/Discovery_ImplPlatform.h | 5 +- src/lib/shell/commands/Dns.cpp | 13 +- ...nericThreadStackManagerImpl_OpenThread.cpp | 13 +- .../nrfconnect/CHIPDevicePlatformConfig.h | 2 + 7 files changed, 103 insertions(+), 101 deletions(-) diff --git a/examples/platform/nrfconnect/util/ThreadUtil.cpp b/examples/platform/nrfconnect/util/ThreadUtil.cpp index 4b751f5c069d38..c0315347156caf 100644 --- a/examples/platform/nrfconnect/util/ThreadUtil.cpp +++ b/examples/platform/nrfconnect/util/ThreadUtil.cpp @@ -19,7 +19,8 @@ #include "ThreadUtil.h" #include -#include + +#include #include #include @@ -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(); } diff --git a/src/app/server/Mdns.cpp b/src/app/server/Mdns.cpp index e4f04c76f2ce76..953c374fe3be96 100644 --- a/src/app/server/Mdns.cpp +++ b/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 } diff --git a/src/include/platform/CHIPDeviceConfig.h b/src/include/platform/CHIPDeviceConfig.h index 5b7bfadbeef0df..961f30a4e95464 100644 --- a/src/include/platform/CHIPDeviceConfig.h +++ b/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 diff --git a/src/lib/mdns/Discovery_ImplPlatform.h b/src/lib/mdns/Discovery_ImplPlatform.h index 9115dc4c72f904..36da664ab835b5 100644 --- a/src/lib/mdns/Discovery_ImplPlatform.h +++ b/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; diff --git a/src/lib/shell/commands/Dns.cpp b/src/lib/shell/commands/Dns.cpp index b6cc208d768080..0e52e392157bcb 100644 --- a/src/lib/shell/commands/Dns.cpp +++ b/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); } diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp index 738ac022b5cd4c..2365e2394604b9 100644 --- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp +++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp @@ -1218,15 +1218,18 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_RemoveSrpServic template CHIP_ERROR GenericThreadStackManagerImpl_OpenThread::_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(); diff --git a/src/platform/nrfconnect/CHIPDevicePlatformConfig.h b/src/platform/nrfconnect/CHIPDevicePlatformConfig.h index b2096a58d07f0a..b032eed3e69aa3 100644 --- a/src/platform/nrfconnect/CHIPDevicePlatformConfig.h +++ b/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