Fix issues with Shell app example (project-chip#11346)

* fix Shell build

* Enable thread dns client, Fix a deadlock in thread implementation when doing resolve with shell

restyle

* add Carriage return to shell dns printfs

* dns browse had the same deadlock. Not that it works, browse commissionable result was causing an overflow of the thread task in EFR32, Bump the task stack to 4k

examples/platform/efr32/project_include/OpenThreadConfig.h

@@ -47,6 +47,7 @@
 #define OPENTHREAD_CONFIG_NCP_HDLC_ENABLE 1
 #define OPENTHREAD_CONFIG_IP6_SLAAC_ENABLE 1
 
+#define OPENTHREAD_CONFIG_DNS_CLIENT_ENABLE 1
 #define OPENTHREAD_CONFIG_SRP_CLIENT_ENABLE 1
 #define OPENTHREAD_CONFIG_ECDSA_ENABLE 1
 

examples/shell/efr32/BUILD.gn

@@ -43,7 +43,6 @@ efr32_sdk("sdk") {
     "${chip_root}/src/platform/EFR32",
     "${efr32_project_dir}/include",
     "${examples_plat_dir}",
-    "${examples_plat_dir}/${efr32_family}/${efr32_board}",
   ]
 
   defines = [

examples/shell/efr32/src/main.cpp

@@ -82,6 +82,11 @@ void appError(int err)
         ;
 }
 
+void appError(CHIP_ERROR error)
+{
+    appError(static_cast<int>(error.AsInteger()));
+}
+
 extern "C" unsigned int sleep(unsigned int seconds)
 {
     const TickType_t xDelay = 1000 * seconds / portTICK_PERIOD_MS;
@@ -169,9 +174,10 @@ int main(void)
     }
 #endif // CHIP_ENABLE_OPENTHREAD
 
-    ret = chip::Shell::streamer_init(chip::Shell::streamer_get());
-    assert(ret == 0);
+    int status = chip::Shell::streamer_init(chip::Shell::streamer_get());
+    assert(status == 0);
 
+    cmd_misc_init();
     cmd_otcli_init();
     cmd_ping_init();
     cmd_send_init();

examples/shell/shell_common/cmd_send.cpp

@@ -232,7 +232,7 @@ void ProcessCommand(streamer_t * stream, char * destination)
     else
 #endif
     {
-        peerAddress = Transport::PeerAddress::UDP(gDestAddr, gSendArguments.GetPort(), Inet::InterfaceId::Null());
+        peerAddress = Transport::PeerAddress::UDP(gDestAddr, gSendArguments.GetPort(), chip::Inet::InterfaceId::Null());
 
         err = gSessionManager.Init(&DeviceLayer::SystemLayer(), &gUDPManager, &gMessageCounterManager);
         SuccessOrExit(err);

src/lib/shell/commands/Dns.cpp

@@ -42,23 +42,23 @@ class DnsShellResolverDelegate : public Dnssd::ResolverDelegate
 public:
     void OnNodeIdResolved(const Dnssd::ResolvedNodeData & nodeData) override
     {
-        streamer_printf(streamer_get(), "DNS resolve for " ChipLogFormatX64 "-" ChipLogFormatX64 " succeeded:\n",
+        streamer_printf(streamer_get(), "DNS resolve for " ChipLogFormatX64 "-" ChipLogFormatX64 " succeeded:\r\n",
                         ChipLogValueX64(nodeData.mPeerId.GetCompressedFabricId()), ChipLogValueX64(nodeData.mPeerId.GetNodeId()));
-        streamer_printf(streamer_get(), "   Hostname: %s\n", nodeData.mHostName);
-        streamer_printf(streamer_get(), "   IP address: %s\n", nodeData.mAddress.ToString(ipAddressBuf));
-        streamer_printf(streamer_get(), "   Port: %" PRIu16 "\n", nodeData.mPort);
+        streamer_printf(streamer_get(), "   Hostname: %s\r\n", nodeData.mHostName);
+        streamer_printf(streamer_get(), "   IP address: %s\r\n", nodeData.mAddress.ToString(ipAddressBuf));
+        streamer_printf(streamer_get(), "   Port: %" PRIu16 "\r\n", nodeData.mPort);
 
         auto retryInterval = nodeData.GetMrpRetryIntervalIdle();
 
         if (retryInterval.HasValue())
-            streamer_printf(streamer_get(), "   MRP retry interval (idle): %" PRIu32 "ms\n", retryInterval.Value());
+            streamer_printf(streamer_get(), "   MRP retry interval (idle): %" PRIu32 "ms\r\n", retryInterval.Value());
 
         retryInterval = nodeData.GetMrpRetryIntervalActive();
 
         if (retryInterval.HasValue())
-            streamer_printf(streamer_get(), "   MRP retry interval (active): %" PRIu32 "ms\n", retryInterval.Value());
+            streamer_printf(streamer_get(), "   MRP retry interval (active): %" PRIu32 "ms\r\n", retryInterval.Value());
 
-        streamer_printf(streamer_get(), "   Supports TCP: %s\n", nodeData.mSupportsTcp ? "yes" : "no");
+        streamer_printf(streamer_get(), "   Supports TCP: %s\r\n", nodeData.mSupportsTcp ? "yes" : "no");
     }
 
     void OnNodeIdResolutionFailed(const PeerId & peerId, CHIP_ERROR error) override {}
@@ -67,41 +67,41 @@ class DnsShellResolverDelegate : public Dnssd::ResolverDelegate
     {
         if (!nodeData.IsValid())
         {
-            streamer_printf(streamer_get(), "DNS browse failed - not found valid services \n");
+            streamer_printf(streamer_get(), "DNS browse failed - not found valid services \r\n");
             return;
         }
 
         char rotatingId[Dnssd::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: %" 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(), "DNS browse succeeded: \r\n");
+        streamer_printf(streamer_get(), "   Hostname: %s\r\n", nodeData.hostName);
+        streamer_printf(streamer_get(), "   Vendor ID: %" PRIu16 "\r\n", nodeData.vendorId);
+        streamer_printf(streamer_get(), "   Product ID: %" PRIu16 "\r\n", nodeData.productId);
+        streamer_printf(streamer_get(), "   Long discriminator: %" PRIu16 "\r\n", nodeData.longDiscriminator);
+        streamer_printf(streamer_get(), "   Device type: %" PRIu16 "\r\n", nodeData.deviceType);
         streamer_printf(streamer_get(), "   Device name: %s\n", nodeData.deviceName);
-        streamer_printf(streamer_get(), "   Commissioning mode: %d\n", static_cast<int>(nodeData.commissioningMode));
-        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);
+        streamer_printf(streamer_get(), "   Commissioning mode: %d\r\n", static_cast<int>(nodeData.commissioningMode));
+        streamer_printf(streamer_get(), "   Pairing hint: %" PRIu16 "\r\n", nodeData.pairingHint);
+        streamer_printf(streamer_get(), "   Pairing instruction: %s\r\n", nodeData.pairingInstruction);
+        streamer_printf(streamer_get(), "   Rotating ID %s\r\n", rotatingId);
 
         auto retryInterval = nodeData.GetMrpRetryIntervalIdle();
 
         if (retryInterval.HasValue())
-            streamer_printf(streamer_get(), "   MRP retry interval (idle): %" PRIu32 "ms\n", retryInterval.Value());
+            streamer_printf(streamer_get(), "   MRP retry interval (idle): %" PRIu32 "ms\r\n", retryInterval.Value());
 
         retryInterval = nodeData.GetMrpRetryIntervalActive();
 
         if (retryInterval.HasValue())
-            streamer_printf(streamer_get(), "   MRP retry interval (active): %" PRIu32 "ms\n", retryInterval.Value());
+            streamer_printf(streamer_get(), "   MRP retry interval (active): %" PRIu32 "ms\r\n", retryInterval.Value());
 
-        streamer_printf(streamer_get(), "   Supports TCP: %s\n", nodeData.supportsTcp ? "yes" : "no");
-        streamer_printf(streamer_get(), "   IP addresses:\n");
+        streamer_printf(streamer_get(), "   Supports TCP: %s\r\n", nodeData.supportsTcp ? "yes" : "no");
+        streamer_printf(streamer_get(), "   IP addresses:\r\n");
         for (uint8_t i = 0; i < nodeData.kMaxIPAddresses; i++)
         {
             if (nodeData.ipAddress[i] != Inet::IPAddress::Any)
-                streamer_printf(streamer_get(), "      %s\n", nodeData.ipAddress[i].ToString(ipAddressBuf));
+                streamer_printf(streamer_get(), "      %s\r\n", nodeData.ipAddress[i].ToString(ipAddressBuf));
         }
     }
 
@@ -115,7 +115,7 @@ CHIP_ERROR ResolveHandler(int argc, char ** argv)
 {
     VerifyOrReturnError(argc == 2, CHIP_ERROR_INVALID_ARGUMENT);
 
-    streamer_printf(streamer_get(), "Resolving ...\n");
+    streamer_printf(streamer_get(), "Resolving ...\r\n");
 
     PeerId peerId;
     peerId.SetCompressedFabricId(strtoull(argv[0], NULL, 10));
@@ -175,11 +175,11 @@ CHIP_ERROR BrowseCommissionableHandler(int argc, char ** argv)
 
     if (!ParseSubType(argc, argv, filter))
     {
-        streamer_printf(streamer_get(), "Invalid argument\n");
+        streamer_printf(streamer_get(), "Invalid argument\r\n");
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    streamer_printf(streamer_get(), "Browsing commissionable nodes...\n");
+    streamer_printf(streamer_get(), "Browsing commissionable nodes...\r\n");
 
     return Dnssd::Resolver::Instance().FindCommissionableNodes(filter);
 }
@@ -190,11 +190,11 @@ CHIP_ERROR BrowseCommissionerHandler(int argc, char ** argv)
 
     if (!ParseSubType(argc, argv, filter))
     {
-        streamer_printf(streamer_get(), "Invalid argument\n");
+        streamer_printf(streamer_get(), "Invalid argument\r\n");
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    streamer_printf(streamer_get(), "Browsing commissioners...\n");
+    streamer_printf(streamer_get(), "Browsing commissioners...\r\n");
 
     return Dnssd::Resolver::Instance().FindCommissioners(filter);
 }

src/platform/EFR32/CHIPDevicePlatformConfig.h

@@ -38,6 +38,9 @@
 #define CHIP_DEVICE_CONFIG_ENABLE_THREAD_SRP_CLIENT 1
 #endif
 
+#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT 1
+#define CHIP_DEVICE_CONFIG_ENABLE_THREAD_COMMISSIONABLE_DISCOVERY 1
+
 #define CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY 1
 #define CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONABLE_DISCOVERY 1
 
@@ -126,7 +129,7 @@
 #if defined(EFR32MG21)
 #define CHIP_DEVICE_CONFIG_THREAD_TASK_STACK_SIZE (2 * 1024)
 #else
-#define CHIP_DEVICE_CONFIG_THREAD_TASK_STACK_SIZE (3 * 1024)
+#define CHIP_DEVICE_CONFIG_THREAD_TASK_STACK_SIZE (4 * 1024)
 #endif
 #endif // CHIP_DEVICE_CONFIG_THREAD_TASK_STACK_SIZE
 

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp

@@ -549,9 +549,8 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetAndLogThread
                     "IP Rx Fail:                   %" PRIu32 "\n",
                     ipCounters->mTxSuccess, ipCounters->mRxSuccess, ipCounters->mTxFailure, ipCounters->mRxFailure);
 
-    Impl()->UnlockThreadStack();
-
 exit:
+    Impl()->UnlockThreadStack();
     return err;
 }
 
@@ -604,9 +603,9 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetAndLogThread
                     extAddress->m8[1], extAddress->m8[2], extAddress->m8[3], extAddress->m8[4], extAddress->m8[5],
                     extAddress->m8[6], extAddress->m8[7], instantRssi);
 
+exit:
     Impl()->UnlockThreadStack();
 
-exit:
     if (err != CHIP_NO_ERROR)
     {
         ChipLogError(DeviceLayer, "GetAndLogThreadTopologyMinimul failed: %" CHIP_ERROR_FORMAT, err.Format());
@@ -758,9 +757,10 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::_GetAndLogThread
                         neighbor->mFullNetworkData ? 'Y' : 'n', neighbor->mIsChild ? 'Y' : 'n', printBuf);
     }
 
+exit:
+
     Impl()->UnlockThreadStack();
 
-exit:
     if (err != CHIP_NO_ERROR)
     {
         ChipLogError(DeviceLayer, "GetAndLogThreadTopologyFull failed: %s", ErrorStr(err));
@@ -1971,8 +1971,6 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsBrowseResult(otEr
         return;
     }
 
-    ThreadStackMgrImpl().LockThreadStack();
-
     VerifyOrExit(aError == OT_ERROR_NONE, error = MapOpenThreadError(aError));
 
     error = MapOpenThreadError(otDnsBrowseResponseGetServiceName(aResponse, type, sizeof(type)));
@@ -2005,8 +2003,6 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsBrowseResult(otEr
 
 exit:
 
-    ThreadStackMgrImpl().UnlockThreadStack();
-
     // In case no service was found invoke callback to notify about failure. In other case it was already called before.
     if (!wasAnythingBrowsed)
         ThreadStackMgrImpl().mDnsBrowseCallback(aContext, nullptr, 0, error);
@@ -2059,8 +2055,6 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsResolveResult(otE
         return;
     }
 
-    ThreadStackMgrImpl().LockThreadStack();
-
     VerifyOrExit(aError == OT_ERROR_NONE, error = MapOpenThreadError(aError));
 
     error = MapOpenThreadError(otDnsServiceResponseGetServiceName(aResponse, resolveResult.mMdnsService.mName,
@@ -2081,7 +2075,6 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsResolveResult(otE
 
 exit:
 
-    ThreadStackMgrImpl().UnlockThreadStack();
     ThreadStackMgrImpl().mDnsResolveCallback(aContext, &(resolveResult.mMdnsService), error);
 }