Skip to content

Commit

Permalink
Fix issues with Shell app example (#11346)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@jmartinez-silabs @pull
jmartinez-silabs authored and pull[bot] committed Jan 3, 2023
1 parent 77d05a7 commit 1157180
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 44 deletions.
1 change: 1 addition & 0 deletions examples/platform/efr32/project_include/OpenThreadConfig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion examples/shell/efr32/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
10 changes: 8 additions & 2 deletions examples/shell/efr32/src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion examples/shell/shell_common/cmd_send.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
56 changes: 28 additions & 28 deletions src/lib/shell/commands/Dns.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand All @@ -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));
}
}

Expand All @@ -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));
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
5 changes: 4 additions & 1 deletion src/platform/EFR32/CHIPDevicePlatformConfig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
15 changes: 4 additions & 11 deletions src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -2081,7 +2075,6 @@ void GenericThreadStackManagerImpl_OpenThread<ImplClass>::OnDnsResolveResult(otE

exit:

ThreadStackMgrImpl().UnlockThreadStack();
ThreadStackMgrImpl().mDnsResolveCallback(aContext, &(resolveResult.mMdnsService), error);
}

Expand Down

0 comments on commit 1157180

Please sign in to comment.