Restyle WIP: Make chip-tool run on a single event loop

examples/chip-tool/commands/clusters/ModelCommand.cpp

@@ -48,19 +48,13 @@ CHIP_ERROR ModelCommand::Run(NodeId localId, NodeId remoteId)
     //
     UpdateWaitForResponse(true);
 
-    {
-        chip::DeviceLayer::StackLock lock;
+    err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice);
+    VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId));
 
-        err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice);
-        VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId));
+    err = SendCommand(mDevice, mEndPointId);
+    VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Failed to send message: %s", ErrorStr(err)));
 
-        err = SendCommand(mDevice, mEndPointId);
-        VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Failed to send message: %s", ErrorStr(err)));
-    }
-
-    WaitForResponse(kWaitDurationInSeconds);
-
-    VerifyOrExit(GetCommandExitStatus(), err = CHIP_ERROR_INTERNAL);
+    ScheduleWaitForResponse(kWaitDurationInSeconds, [] {});
 
 exit:
     return err;

examples/chip-tool/commands/common/Command.cpp

@@ -17,6 +17,7 @@
  */
 
 #include "Command.h"
+#include "platform/PlatformManager.h"
 
 #include <netdb.h>
 #include <sstream>
@@ -378,11 +379,15 @@ const char * Command::GetAttribute(void) const
 
 void Command::UpdateWaitForResponse(bool value)
 {
+    if (value == false)
     {
-        std::lock_guard<std::mutex> lk(cvWaitingForResponseMutex);
-        mWaitingForResponse = value;
+        if (mCommandExitStatus == false)
+        {
+            ChipLogError(chipTool, "Run command failure");
+        }
+
+        chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
     }
-    cvWaitingForResponse.notify_all();
 }
 
 void Command::WaitForResponse(uint16_t duration)
@@ -395,3 +400,34 @@ void Command::WaitForResponse(uint16_t duration)
         ChipLogError(chipTool, "No response from device");
     }
 }
+
+void Command::OnResponseTimer(chip::System::Layer * aLayer, void * aAppState, chip::System::Error aError)
+{
+    Command * _this = static_cast<Command *>(aAppState);
+
+    ChipLogError(chipTool, "No response from device");
+
+    if (_this->mCleanupFunc)
+    {
+        _this->mCleanupFunc();
+    }
+
+    chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
+}
+
+void Command::ScheduleWaitForResponse(uint16_t duration, std::function<void()> cleanupFunc)
+{
+    chip::System::Timer * timer = nullptr;
+
+    mCleanupFunc = cleanupFunc;
+
+    if (chip::DeviceLayer::SystemLayer.NewTimer(timer) == CHIP_NO_ERROR)
+    {
+        timer->Start(duration * 1000, OnResponseTimer, this);
+    }
+    else
+    {
+        ChipLogError(chipTool, "Failed to allocate timer");
+        chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
+    }
+}

examples/chip-tool/commands/common/Command.h

@@ -174,6 +174,9 @@ class Command
     void UpdateWaitForResponse(bool value);
     void WaitForResponse(uint16_t duration);
 
+    static void OnResponseTimer(chip::System::Layer * aLayer, void * aAppState, chip::System::Error aError);
+    void ScheduleWaitForResponse(uint16_t duration, std::function<void()> f);
+
 protected:
     ExecutionContext * GetExecContext() { return mExecContext; }
     ExecutionContext * mExecContext;
@@ -187,6 +190,8 @@ class Command
     const char * mName      = nullptr;
     std::vector<Argument> mArgs;
 
+    std::function<void()> mCleanupFunc = nullptr;
+
     std::condition_variable cvWaitingForResponse;
     std::mutex cvWaitingForResponseMutex;
     bool mWaitingForResponse{ false };

examples/chip-tool/commands/common/Commands.cpp

@@ -69,17 +69,12 @@ int Commands::Run(NodeId localId, NodeId remoteId, int argc, char ** argv)
     err = mController.Init(localId, initParams);
     VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Controller, "Init failure! Commissioner: %s", chip::ErrorStr(err)));
 
-    err = mController.ServiceEvents();
-    VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Controller, "Init failure! Run Loop: %s", chip::ErrorStr(err)));
-
     err = RunCommand(localId, remoteId, argc, argv);
     SuccessOrExit(err);
 
-exit:
-#if CONFIG_DEVICE_LAYER
-    chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
-#endif
+    chip::DeviceLayer::PlatformMgr().RunEventLoop();
 
+exit:
     //
     // We can call DeviceController::Shutdown() safely without grabbing the stack lock
     // since the CHIP thread and event queue have been stopped, preventing any thread

examples/chip-tool/commands/discover/DiscoverCommand.cpp

@@ -31,22 +31,13 @@ CHIP_ERROR DiscoverCommand::Run(NodeId localId, NodeId remoteId)
     //
     UpdateWaitForResponse(true);
 
-    {
-        chip::DeviceLayer::StackLock lock;
+    GetExecContext()->commissioner->RegisterDeviceAddressUpdateDelegate(this);
 
-        GetExecContext()->commissioner->RegisterDeviceAddressUpdateDelegate(this);
-        err = RunCommand(mNodeId, mFabricId);
-        SuccessOrExit(err);
-    }
+    err = RunCommand(mNodeId, mFabricId);
+    SuccessOrExit(err);
 
-    WaitForResponse(kWaitDurationInSeconds);
+    ScheduleWaitForResponse(kWaitDurationInSeconds, [] {});
 
 exit:
-    if (err != CHIP_NO_ERROR)
-    {
-        return err;
-    }
-
-    VerifyOrReturnError(GetCommandExitStatus(), CHIP_ERROR_INTERNAL);
-    return CHIP_NO_ERROR;
+    return err;
 }

examples/chip-tool/commands/pairing/PairingCommand.cpp

@@ -31,23 +31,13 @@ CHIP_ERROR PairingCommand::Run(NodeId localId, NodeId remoteId)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
 
-    {
-        chip::DeviceLayer::StackLock lock;
-
-        GetExecContext()->commissioner->RegisterDeviceAddressUpdateDelegate(this);
-        GetExecContext()->commissioner->RegisterPairingDelegate(this);
-    }
+    GetExecContext()->commissioner->RegisterDeviceAddressUpdateDelegate(this);
+    GetExecContext()->commissioner->RegisterPairingDelegate(this);
 
     err = RunInternal(remoteId);
     VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init Failure! PairDevice: %s", ErrorStr(err)));
 
 exit:
-
-    if (err == CHIP_NO_ERROR)
-    {
-        VerifyOrReturnError(GetCommandExitStatus(), CHIP_ERROR_INTERNAL);
-    }
-
     return err;
 }
 
@@ -71,32 +61,27 @@ CHIP_ERROR PairingCommand::RunInternal(NodeId remoteId)
     // appropriately. None of the following calls below before the unlock are, nor should be,
     // blocking.
     //
+    switch (mPairingMode)
     {
-        chip::DeviceLayer::StackLock lock;
-
-        switch (mPairingMode)
-        {
-        case PairingMode::None:
-            err = Unpair(remoteId);
-            break;
-        case PairingMode::Bypass:
-            err = PairWithoutSecurity(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
-            break;
-        case PairingMode::Ble:
-            err = Pair(remoteId, PeerAddress::BLE());
-            break;
-        case PairingMode::OnNetwork:
-        case PairingMode::SoftAP:
-            err = Pair(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
-            break;
-        case PairingMode::Ethernet:
-            err = Pair(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
-            break;
-        }
+    case PairingMode::None:
+        err = Unpair(remoteId);
+        break;
+    case PairingMode::Bypass:
+        err = PairWithoutSecurity(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
+        break;
+    case PairingMode::Ble:
+        err = Pair(remoteId, PeerAddress::BLE());
+        break;
+    case PairingMode::OnNetwork:
+    case PairingMode::SoftAP:
+        err = Pair(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
+        break;
+    case PairingMode::Ethernet:
+        err = Pair(remoteId, PeerAddress::UDP(mRemoteAddr.address, mRemotePort));
+        break;
     }
 
-    WaitForResponse(kWaitDurationInSeconds);
-    ReleaseCallbacks();
+    ScheduleWaitForResponse(kWaitDurationInSeconds, [this] { ReleaseCallbacks(); });
 
     return err;
 }

examples/chip-tool/commands/reporting/ReportingCommand.cpp

@@ -40,20 +40,16 @@ CHIP_ERROR ReportingCommand::Run(NodeId localId, NodeId remoteId)
     //
     UpdateWaitForResponse(true);
 
-    {
-        chip::DeviceLayer::StackLock lock;
+    err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice);
+    VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId));
 
-        err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice);
-        VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(chipTool, "Init failure! No pairing for device: %" PRIu64, localId));
+    AddReportCallbacks(mEndPointId);
+    cluster.Associate(mDevice, mEndPointId);
 
-        AddReportCallbacks(mEndPointId);
-        cluster.Associate(mDevice, mEndPointId);
+    err = cluster.MfgSpecificPing(nullptr, nullptr);
+    VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Controller, "Init failure! Ping failure: %s", ErrorStr(err)));
 
-        err = cluster.MfgSpecificPing(nullptr, nullptr);
-        VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Controller, "Init failure! Ping failure: %s", ErrorStr(err)));
-    }
-
-    WaitForResponse(kWaitDurationInSeconds);
+    ScheduleWaitForResponse(kWaitDurationInSeconds, [] {});
 
 exit:
     return err;

examples/chip-tool/commands/tests/TestCommand.cpp

@@ -31,18 +31,12 @@ CHIP_ERROR TestCommand::Run(NodeId localId, NodeId remoteId)
     //
     UpdateWaitForResponse(true);
 
-    {
-        chip::DeviceLayer::StackLock lock;
+    err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice);
+    ReturnErrorOnFailure(err);
 
-        err = GetExecContext()->commissioner->GetDevice(remoteId, &mDevice);
-        ReturnErrorOnFailure(err);
+    err = NextTest();
+    ReturnErrorOnFailure(err);
 
-        err = NextTest();
-        ReturnErrorOnFailure(err);
-    }
-
-    WaitForResponse(kWaitDurationInSeconds);
-
-    VerifyOrReturnError(GetCommandExitStatus(), CHIP_ERROR_INTERNAL);
+    ScheduleWaitForResponse(kWaitDurationInSeconds, [] {});
     return CHIP_NO_ERROR;
 }

src/platform/Darwin/PlatformManagerImpl.cpp

@@ -44,6 +44,8 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
     err = Internal::PosixConfig::Init();
     SuccessOrExit(err);
 
+    mRunLoopSem = dispatch_semaphore_create(0);
+
     // Call _InitChipStack() on the generic implementation base class
     // to finish the initialization process.
     err = Internal::GenericPlatformManagerImpl<PlatformManagerImpl>::_InitChipStack();
@@ -68,16 +70,23 @@ CHIP_ERROR PlatformManagerImpl::_StartEventLoopTask()
 
 CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask()
 {
-
-    if (mIsWorkQueueRunning == true)
+    if (dispatch_get_current_queue() != mWorkQueue)
     {
-        mIsWorkQueueRunning = false;
-
-        // dispatch_sync is used in order to guarantee serialization of the caller with
-        // respect to any tasks that might already be on the queue, or running.
-        dispatch_sync(mWorkQueue, ^{
-            dispatch_suspend(mWorkQueue);
-        });
+        if (mIsWorkQueueRunning == true)
+        {
+            mIsWorkQueueRunning = false;
+
+            // dispatch_sync is used in order to guarantee serialization of the caller with
+            // respect to any tasks that might already be on the queue, or running.
+            dispatch_sync(mWorkQueue, ^{
+                dispatch_suspend(mWorkQueue);
+            });
+        }
+    }
+    else
+    {
+        dispatch_suspend(mWorkQueue);
+        dispatch_semaphore_signal(mRunLoopSem);
     }
 
     return CHIP_NO_ERROR;
@@ -86,8 +95,12 @@ CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask()
 void PlatformManagerImpl::_RunEventLoop()
 {
     _StartEventLoopTask();
-    CFRunLoopRun();
-};
+
+    //
+    // Block on the sem till we're signalled to stop by _StopEventLoopTask()
+    //
+    dispatch_semaphore_wait(mRunLoopSem, DISPATCH_TIME_FOREVER);
+}
 
 CHIP_ERROR PlatformManagerImpl::_Shutdown()
 {

src/platform/Darwin/PlatformManagerImpl.h

@@ -80,7 +80,9 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
     static PlatformManagerImpl sInstance;
 
     dispatch_queue_t mWorkQueue = nullptr;
-    bool mIsWorkQueueRunning    = false;
+    dispatch_semaphore_t mRunLoopSem;
+
+    bool mIsWorkQueueRunning = false;
 
     inline ImplClass * Impl() { return static_cast<PlatformManagerImpl *>(this); }
 };