[shell] Add "dns browse commissioner" command (#8836)

* [shell] Add "dns browse commissioner" command

The existing "dns browse" command searches for Matter
commissionable nodes only. Split it into "dns browse
commissionable" and "dns browse commissioner" to provide
means to test commissioner discovery on all devices.

Also, allow to specify a subtype for both commands and fix
buffer sizes in the OpenThread-based DNS implementation to
accomodate filtering by subtypes.

* Restyled by clang-format

Co-authored-by: Restyled.io <commits@restyled.io>

src/lib/mdns/platform/Mdns.h

@@ -43,8 +43,9 @@ static constexpr uint8_t kMdnsHostNameMaxSize     = 16; // 64-bits in hex.
 static constexpr size_t kMdnsProtocolTextMaxSize  = std::max(sizeof(kOperationalProtocol), sizeof(kCommissionProtocol)) - 1;
 static constexpr size_t kMdnsTypeMaxSize =
     std::max({ sizeof(kCommissionableServiceName), sizeof(kOperationalServiceName), sizeof(kCommissionerServiceName) }) - 1;
-static constexpr uint8_t kMdnsTypeAndProtocolMaxSize = kMdnsTypeMaxSize + kMdnsProtocolTextMaxSize + 1; // <type>.<protocol>
-static constexpr uint16_t kMdnsTextMaxSize           = 64;
+static constexpr uint8_t kMdnsTypeAndProtocolMaxSize     = kMdnsTypeMaxSize + kMdnsProtocolTextMaxSize + 1; // <type>.<protocol>
+static constexpr uint16_t kMdnsTextMaxSize               = 64;
+static constexpr uint8_t kMdnsFullTypeAndProtocolMaxSize = kMaxSubtypeDescSize + /* '.' */ 1 + kMdnsTypeAndProtocolMaxSize;
 
 enum class MdnsServiceProtocol : uint8_t
 {

src/lib/shell/commands/Dns.cpp

@@ -24,19 +24,23 @@
 #include <lib/shell/Commands.h>
 #include <lib/shell/Engine.h>
 #include <lib/shell/commands/Help.h>
+#include <lib/support/BytesToHex.h>
+#include <lib/support/CHIPArgParser.hpp>
+#include <lib/support/CodeUtils.h>
 #include <platform/CHIPDeviceLayer.h>
-#include <support/CHIPArgParser.hpp>
-#include <support/CodeUtils.h>
 
 namespace chip {
 namespace Shell {
 
-static chip::Shell::Engine sShellDnsSubcommands;
+namespace {
 
-class DnsShellResolverDelegate : public chip::Mdns::ResolverDelegate
+Shell::Engine sShellDnsBrowseSubcommands;
+Shell::Engine sShellDnsSubcommands;
+
+class DnsShellResolverDelegate : public Mdns::ResolverDelegate
 {
 public:
-    void OnNodeIdResolved(const chip::Mdns::ResolvedNodeData & nodeData) override
+    void OnNodeIdResolved(const Mdns::ResolvedNodeData & nodeData) override
     {
         streamer_printf(streamer_get(), "DNS resolve for " ChipLogFormatX64 "-" ChipLogFormatX64 " succeeded:\n",
                         ChipLogValueX64(nodeData.mPeerId.GetFabricId()), ChipLogValueX64(nodeData.mPeerId.GetNodeId()));
@@ -59,21 +63,29 @@ class DnsShellResolverDelegate : public chip::Mdns::ResolverDelegate
 
     void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override {}
 
-    void OnNodeDiscoveryComplete(const chip::Mdns::DiscoveredNodeData & nodeData) override
+    void OnNodeDiscoveryComplete(const Mdns::DiscoveredNodeData & nodeData) override
     {
         if (!nodeData.IsValid())
         {
             streamer_printf(streamer_get(), "DNS browse failed - not found valid services \n");
             return;
         }
+
+        char rotatingId[Mdns::kMaxRotatingIdLen * 2 + 1];
+        Encoding::BytesToUppercaseHexString(nodeData.rotatingId, nodeData.rotatingIdLen, rotatingId, sizeof(rotatingId));
+
         streamer_printf(streamer_get(), "DNS browse succeeded: \n");
         streamer_printf(streamer_get(), "   Hostname: %s\n", nodeData.hostName);
-        streamer_printf(streamer_get(), "   Vendor ID: %d\n", nodeData.vendorId);
-        streamer_printf(streamer_get(), "   Product ID: %d\n", nodeData.productId);
-        streamer_printf(streamer_get(), "   Long discriminator: %d\n", nodeData.longDiscriminator);
-        streamer_printf(streamer_get(), "   Device type: %d\n", nodeData.deviceType);
+        streamer_printf(streamer_get(), "   Vendor ID: %" PRIu16 "\n", nodeData.vendorId);
+        streamer_printf(streamer_get(), "   Product ID: %" PRIu16 "\n", nodeData.productId);
+        streamer_printf(streamer_get(), "   Long discriminator: %" PRIu16 "\n", nodeData.longDiscriminator);
+        streamer_printf(streamer_get(), "   Device type: %" PRIu16 "\n", nodeData.deviceType);
         streamer_printf(streamer_get(), "   Device name: %s\n", nodeData.deviceName);
-        streamer_printf(streamer_get(), "   Commissioning mode: %d\n", nodeData.commissioningMode);
+        streamer_printf(streamer_get(), "   Commissioning mode: %d\n", static_cast<int>(nodeData.commissioningMode));
+        streamer_printf(streamer_get(), "   Additional pairing: %d\n", static_cast<int>(nodeData.additionalPairing));
+        streamer_printf(streamer_get(), "   Pairing hint: %" PRIu16 "\n", nodeData.pairingHint);
+        streamer_printf(streamer_get(), "   Pairing instruction: %s\n", nodeData.pairingInstruction);
+        streamer_printf(streamer_get(), "   Rotating ID %s\n", rotatingId);
 
         auto retryInterval = nodeData.GetMrpRetryIntervalIdle();
 
@@ -89,67 +101,156 @@ class DnsShellResolverDelegate : public chip::Mdns::ResolverDelegate
         streamer_printf(streamer_get(), "   IP addresses:\n");
         for (uint8_t i = 0; i < nodeData.kMaxIPAddresses; i++)
         {
-            if (nodeData.ipAddress[i] != chip::Inet::IPAddress::Any)
+            if (nodeData.ipAddress[i] != Inet::IPAddress::Any)
                 streamer_printf(streamer_get(), "      %s\n", nodeData.ipAddress[i].ToString(ipAddressBuf));
         }
     }
 
 private:
-    char ipAddressBuf[chip::Inet::kMaxIPAddressStringLength];
+    char ipAddressBuf[Inet::kMaxIPAddressStringLength];
 };
 
-static DnsShellResolverDelegate sDnsShellResolverDelegate;
-
-CHIP_ERROR DnsHelpHandler(int argc, char ** argv)
-{
-    sShellDnsSubcommands.ForEachCommand(PrintCommandHelp, nullptr);
-    return CHIP_NO_ERROR;
-}
+DnsShellResolverDelegate sDnsShellResolverDelegate;
 
-static CHIP_ERROR ResolveHandler(int argc, char ** argv)
+CHIP_ERROR ResolveHandler(int argc, char ** argv)
 {
-    streamer_printf(streamer_get(), "Resolving ...\r\n");
-
     VerifyOrReturnError(argc == 2, CHIP_ERROR_INVALID_ARGUMENT);
 
+    streamer_printf(streamer_get(), "Resolving ...\n");
+
     PeerId peerId;
     peerId.SetFabricId(strtoull(argv[0], NULL, 10));
     peerId.SetNodeId(strtoull(argv[1], NULL, 10));
 
-    return chip::Mdns::Resolver::Instance().ResolveNodeId(peerId, Inet::kIPAddressType_Any);
+    return Mdns::Resolver::Instance().ResolveNodeId(peerId, Inet::kIPAddressType_Any);
+}
+
+bool ParseSubType(int argc, char ** argv, Mdns::DiscoveryFilter & filter)
+{
+    if (argc == 0)
+    {
+        // No filtering by subtype.
+        return true;
+    }
+
+    VerifyOrReturnError(argc == 1, false);
+
+    const char * subtype = argv[0];
+    // All supported subtypes are:_<S><dd...>, where the number of digits is greater than 0.
+    VerifyOrReturnError(strlen(subtype) >= 3, false);
+    VerifyOrReturnError(subtype[0] == '_', false);
+
+    auto filterType = Mdns::DiscoveryFilterType::kNone;
+
+    switch (subtype[1])
+    {
+    case 'S':
+        filterType = Mdns::DiscoveryFilterType::kShort;
+        break;
+    case 'L':
+        filterType = Mdns::DiscoveryFilterType::kLong;
+        break;
+    case 'V':
+        filterType = Mdns::DiscoveryFilterType::kVendor;
+        break;
+    case 'T':
+        filterType = Mdns::DiscoveryFilterType::kDeviceType;
+        break;
+    case 'C':
+        filterType = Mdns::DiscoveryFilterType::kCommissioningMode;
+        break;
+    case 'A':
+        filterType = Mdns::DiscoveryFilterType::kCommissioningModeFromCommand;
+        break;
+    default:
+        return false;
+    }
+
+    uint16_t code;
+    VerifyOrReturnError(ArgParser::ParseInt(subtype + 2, code), false);
+
+    filter = Mdns::DiscoveryFilter(filterType, code);
+    return true;
+}
+
+CHIP_ERROR BrowseCommissionableHandler(int argc, char ** argv)
+{
+    Mdns::DiscoveryFilter filter;
+
+    if (!ParseSubType(argc, argv, filter))
+    {
+        streamer_printf(streamer_get(), "Invalid argument\n");
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
+
+    streamer_printf(streamer_get(), "Browsing commissionable nodes...\n");
+
+    return Mdns::Resolver::Instance().FindCommissionableNodes(filter);
 }
 
-static CHIP_ERROR BrowseHandler(int argc, char ** argv)
+CHIP_ERROR BrowseCommissionerHandler(int argc, char ** argv)
 {
-    streamer_printf(streamer_get(), "Browsing ...\r\n");
+    Mdns::DiscoveryFilter filter;
+
+    if (!ParseSubType(argc, argv, filter))
+    {
+        streamer_printf(streamer_get(), "Invalid argument\n");
+        return CHIP_ERROR_INVALID_ARGUMENT;
+    }
+
+    streamer_printf(streamer_get(), "Browsing commissioners...\n");
 
-    return chip::Mdns::Resolver::Instance().FindCommissionableNodes();
+    return Mdns::Resolver::Instance().FindCommissioners(filter);
 }
 
-static CHIP_ERROR DnsHandler(int argc, char ** argv)
+CHIP_ERROR BrowseHandler(int argc, char ** argv)
 {
     if (argc == 0)
     {
-        DnsHelpHandler(argc, argv);
+        sShellDnsBrowseSubcommands.ForEachCommand(PrintCommandHelp, nullptr);
         return CHIP_NO_ERROR;
     }
 
-    chip::Mdns::Resolver::Instance().StartResolver(&chip::DeviceLayer::InetLayer, chip::Mdns::kMdnsPort);
-    chip::Mdns::Resolver::Instance().SetResolverDelegate(&sDnsShellResolverDelegate);
+    return sShellDnsBrowseSubcommands.ExecCommand(argc, argv);
+}
+
+CHIP_ERROR DnsHandler(int argc, char ** argv)
+{
+    if (argc == 0)
+    {
+        sShellDnsSubcommands.ForEachCommand(PrintCommandHelp, nullptr);
+        return CHIP_NO_ERROR;
+    }
+
+    Mdns::Resolver::Instance().StartResolver(&DeviceLayer::InetLayer, Mdns::kMdnsPort);
+    Mdns::Resolver::Instance().SetResolverDelegate(&sDnsShellResolverDelegate);
 
     return sShellDnsSubcommands.ExecCommand(argc, argv);
 }
 
+} // namespace
+
 void RegisterDnsCommands()
 {
+    static const shell_command_t sDnsBrowseSubCommands[] = {
+        { &BrowseCommissionableHandler, "commissionable",
+          "Browse Matter commissionable nodes. Usage: dns browse commissionable [subtype]" },
+        { &BrowseCommissionerHandler, "commissioner",
+          "Browse Matter commissioner nodes. Usage: dns browse commissioner [subtype]" },
+    };
+
     static const shell_command_t sDnsSubCommands[] = {
         { &ResolveHandler, "resolve",
           "Resolve the DNS service. Usage: dns resolve <fabric-id> <node-id> (e.g. dns resolve 5544332211 1)" },
-        { &BrowseHandler, "browse", "Browse the services published by mdns. Usage: dns browse" },
+        { &BrowseHandler, "browse",
+          "Browse DNS services published by Matter nodes. Usage: dns browse <commissionable|commissioner>" },
     };
 
     static const shell_command_t sDnsCommand = { &DnsHandler, "dns", "Dns client commands" };
 
+    // Register `dns browse` subcommands
+    sShellDnsBrowseSubcommands.RegisterCommands(sDnsBrowseSubCommands, ArraySize(sDnsBrowseSubCommands));
+
     // Register `dns` subcommands with the local shell dispatcher.
     sShellDnsSubcommands.RegisterCommands(sDnsSubCommands, ArraySize(sDnsSubCommands));
 

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -1364,8 +1364,8 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsBrowseResult(otEr
     // each entry consists of txt_entry_size (1B) + txt_entry_key + "=" + txt_entry_data
     uint8_t txtBuffer[kMaxDnsServiceTxtEntriesNumber + kTotalDnsServiceTxtBufferSize];
     otDnsServiceInfo serviceInfo;
-    uint16_t index = 0;
-    bool wasAnythingBrowsed;
+    uint16_t index          = 0;
+    bool wasAnythingBrowsed = false;
 
     if (ThreadStackMgrImpl().mDnsBrowseCallback == nullptr)
     {
@@ -1421,18 +1421,17 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_DnsBrowse(const
     CHIP_ERROR error = CHIP_NO_ERROR;
 
     Impl()->LockThreadStack();
-    const otDnsQueryConfig * defaultConfig = otDnsClientGetDefaultConfig(mOTInst);
 
     VerifyOrExit(aServiceName, error = CHIP_ERROR_INVALID_ARGUMENT);
 
     mDnsBrowseCallback = aCallback;
 
     // Append default SRP domain name to the service name.
-    // fullServiceName buffer size is kMdnsTypeAndProtocolMaxSize + . separator + kDefaultDomainNameSize + termination character.
-    char fullServiceName[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1 + SrpClient::kDefaultDomainNameSize + 1];
+    // fullServiceName buffer size is kMdnsFullTypeAndProtocolMaxSize + . + kDefaultDomainNameSize + null-terminator.
+    char fullServiceName[Mdns::kMdnsFullTypeAndProtocolMaxSize + 1 + SrpClient::kDefaultDomainNameSize + 1];
     snprintf(fullServiceName, sizeof(fullServiceName), "%s.%s", aServiceName, SrpClient::kDefaultDomainName);
 
-    error = MapOpenThreadError(otDnsClientBrowse(mOTInst, fullServiceName, OnDnsBrowseResult, aContext, defaultConfig));
+    error = MapOpenThreadError(otDnsClientBrowse(mOTInst, fullServiceName, OnDnsBrowseResult, aContext, /* config */ nullptr));
 
 exit:
 

src/platform/OpenThread/MdnsImpl.cpp

@@ -88,7 +88,7 @@ CHIP_ERROR ChipMdnsBrowse(const char * type, MdnsServiceProtocol protocol, Inet:
     if (type == nullptr || callback == nullptr)
         return CHIP_ERROR_INVALID_ARGUMENT;
 
-    char serviceType[chip::Mdns::kMdnsTypeAndProtocolMaxSize + 1];
+    char serviceType[Mdns::kMdnsFullTypeAndProtocolMaxSize + 1]; // +1 for null-terminator
     snprintf(serviceType, sizeof(serviceType), "%s.%s", type, GetProtocolString(protocol));
 
     return ThreadStackMgr().DnsBrowse(serviceType, callback, context);