Make it possible to run chip-tool on a single event loop.

project-chip · Jun 23, 2021 · 25232bb · 25232bb
1 parent fd3f9a2
commit 25232bb
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 43 deletions.
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
@@ -31,6 +31,10 @@ jobs:
         strategy:
           matrix:
             type: [debug, tsan]
+            eventloop: [eventloop_same, eventloop_separate]
+        env:
+            USE_SEPARATE_EVENTLOOP: ${{ matrix.eventloop == 'eventloop_separate' }}
+            USE_TSAN: ${{ matrix.type == 'tsan' }
 
         if: github.actor != 'restyled-io[bot]'
         runs-on: ubuntu-latest
@@ -58,30 +62,18 @@ jobs:
               uses: actions/upload-artifact@v2
               if: ${{ always() }}
               with:
-                  name: bootstrap-logs-${{ matrix.type }}
+                  name: bootstrap-logs-linux-${{ matrix.type }}-${{ matrix.eventloop }}
                   path: |
                    .environment/gn_out/.ninja_log
                    .environment/pigweed-venv/*.log
             - name: Build all clusters app
               timeout-minutes: 5
               run: |
-                  case ${{ matrix.type }} in
-                    "debug") GN_ARGS='is_tsan=false';;
-                    "tsan") GN_ARGS='is_tsan=true';;
-                    *) ;;
-                  esac
-              
-                  scripts/examples/gn_build_example.sh examples/all-clusters-app/linux out/debug/standalone/ chip_config_network_layer_ble=false "$GN_ARGS"
+                  scripts/examples/gn_build_example.sh examples/all-clusters-app/linux out/debug/standalone/ chip_config_network_layer_ble=false is_tsan=${USE_TSAN}
             - name: Build chip-tool
               timeout-minutes: 5
               run: |
-                  case ${{ matrix.type }} in
-                    "debug") GN_ARGS='is_tsan=false';;
-                    "tsan") GN_ARGS='is_tsan=true';;
-                    *) ;;
-                  esac
-                
-                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ "$GN_ARGS"
+                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ is_tsan=${USE_TSAN} config_use_separate_eventloop=${USE_SEPARATE_EVENTLOOP}
             - name: Copy objdir
               run: |
                   # The idea is to not upload our objdir unless builds have
@@ -96,15 +88,15 @@ jobs:
               uses: actions/upload-artifact@v2
               if: ${{ failure() }}
               with:
-                  name: crash-core-linux-${{ matrix.type }}
+                  name: crash-core-linux-${{ matrix.type }}-${{ matrix.eventloop }}
                   path: /tmp/cores/
                   # Cores are big; don't hold on to them too long.
                   retention-days: 5
             - name: Uploading objdir for debugging
               uses: actions/upload-artifact@v2
               if: ${{ failure() }}
               with:
-                  name: crash-objdir-linux-${{ matrix.type }}
+                  name: crash-objdir-linux-${{ matrix.type }}-${{ matrix.eventloop }}
                   path: objdir-clone/
                   # objdirs are big; don't hold on to them too long.
                   retention-days: 5
@@ -115,6 +107,10 @@ jobs:
         strategy:
           matrix:
             type: [debug, tsan]
+            eventloop: [eventloop_same, eventloop_separate]
+        env:
+            USE_SEPARATE_EVENTLOOP: ${{ matrix.eventloop == 'eventloop_separate' }}
+            USE_TSAN: ${{ matrix.type == 'tsan' }
 
         if: github.actor != 'restyled-io[bot]'
         runs-on: macos-latest
@@ -147,30 +143,18 @@ jobs:
               uses: actions/upload-artifact@v2
               if: ${{ always() }}
               with:
-                  name: bootstrap-logs-${{ matrix.type }}
+                  name: bootstrap-logs-darwin-${{ matrix.type }}-${{ matrix.eventloop }}
                   path: |
                    .environment/gn_out/.ninja_log
                    .environment/pigweed-venv/*.log
             - name: Run Build Test Server
               timeout-minutes: 5
               run: |
-                  case ${{ matrix.type }} in
-                    "debug") GN_ARGS='is_tsan=false';;
-                    "tsan") GN_ARGS='is_tsan=true';;
-                    *) ;;
-                  esac
-
-                  scripts/examples/gn_build_example.sh examples/all-clusters-app/linux out/debug/standalone/ chip_config_network_layer_ble=false "$GN_ARGS"
+                  scripts/examples/gn_build_example.sh examples/all-clusters-app/linux out/debug/standalone/ chip_config_network_layer_ble=false is_tsan=${USE_TSAN}
             - name: Build chip-tool
               timeout-minutes: 5
               run: |
-                  case ${{ matrix.type }} in
-                    "debug") GN_ARGS='is_tsan=false';;
-                    "tsan") GN_ARGS='is_tsan=true';;
-                    *) ;;
-                  esac
-
-                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ "$GN_ARGS"
+                  scripts/examples/gn_build_example.sh examples/chip-tool out/debug/standalone/ is_tsan=${USE_TSAN} config_use_separate_eventloop=${USE_SEPARATE_EVENTLOOP}
             - name: Copy objdir
               run: |
                   # The idea is to not upload our objdir unless builds have
@@ -184,21 +168,21 @@ jobs:
               uses: actions/upload-artifact@v2
               if: ${{ failure() }}
               with:
-                  name: crash-core-darwin-${{ matrix.type }}
+                  name: crash-core-darwin-${{ matrix.type }}-${{ matrix.eventloop }}
                   path: /cores/
                   # Cores are big; don't hold on to them too long.
                   retention-days: 5
             - name: Uploading diagnostic logs
               uses: actions/upload-artifact@v2
               if: ${{ failure() }}
               with:
-                  name: crash-log-darwin-${{ matrix.type }}
+                  name: crash-log-darwin-${{ matrix.type }}-${{ matrix.eventloop }}
                   path: ~/Library/Logs/DiagnosticReports/
             - name: Uploading objdir for debugging
               uses: actions/upload-artifact@v2
               if: ${{ failure() }}
               with:
-                  name: crash-objdir-darwin-${{ matrix.type }}
+                  name: crash-objdir-darwin-${{ matrix.eventloop }}-${{ matrix.eventloop }}
                   path: objdir-clone/
                   # objdirs are big; don't hold on to them too long.
                   retention-days: 5
diff --git a/examples/chip-tool/BUILD.gn b/examples/chip-tool/BUILD.gn
@@ -19,6 +19,11 @@ import("${chip_root}/build/chip/tools.gni")
 
 assert(chip_build_tools)
 
+declare_args() {
+  # Use a separate eventloop for CHIP tasks
+  config_use_separate_eventloop = true
+}
+
 executable("chip-tool") {
   sources = [
     "commands/clusters/ModelCommand.cpp",
@@ -35,6 +40,8 @@ executable("chip-tool") {
     "main.cpp",
   ]
 
+  defines = [ "CONFIG_USE_SEPARATE_EVENTLOOP=${config_use_separate_eventloop}" ]
+
   deps = [
     "${chip_root}/src/controller/data_model",
     "${chip_root}/src/lib",

diff --git a/examples/chip-tool/commands/common/Command.cpp b/examples/chip-tool/commands/common/Command.cpp
@@ -17,6 +17,7 @@
  */
 
 #include "Command.h"
+#include "platform/PlatformManager.h"
 
 #include <netdb.h>
 #include <sstream>
@@ -378,13 +379,27 @@ const char * Command::GetAttribute(void) const
 
 void Command::UpdateWaitForResponse(bool value)
 {
+#if CONFIG_USE_SEPARATE_EVENTLOOP
     {
         std::lock_guard<std::mutex> lk(cvWaitingForResponseMutex);
         mWaitingForResponse = value;
     }
     cvWaitingForResponse.notify_all();
+#else  // CONFIG_USE_SEPARATE_EVENTLOOP
+    if (value == false)
+    {
+        if (mCommandExitStatus != CHIP_NO_ERROR)
+        {
+            ChipLogError(chipTool, "Run command failure: %s", chip::ErrorStr(mCommandExitStatus));
+        }
+
+        chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
+    }
+#endif // CONFIG_USE_SEPARATE_EVENTLOOP
 }
 
+#if CONFIG_USE_SEPARATE_EVENTLOOP
+
 void Command::WaitForResponse(uint16_t duration)
 {
     std::chrono::seconds waitingForResponseTimeout(duration);
@@ -395,3 +410,30 @@ void Command::WaitForResponse(uint16_t duration)
         ChipLogError(chipTool, "No response from device");
     }
 }
+
+#else // CONFIG_USE_SEPARATE_EVENTLOOP
+
+static void OnResponseTimeout(chip::System::Layer *, void *, chip::System::Error)
+{
+    ChipLogError(chipTool, "No response from device");
+
+    chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
+}
+
+CHIP_ERROR Command::ScheduleWaitForResponse(uint16_t duration)
+{
+    chip::System::Timer * timer = nullptr;
+
+    CHIP_ERROR err = chip::DeviceLayer::SystemLayer.NewTimer(timer);
+    if (err == CHIP_NO_ERROR)
+    {
+        timer->Start(duration * 1000, OnResponseTimeout, this);
+    }
+    else
+    {
+        ChipLogError(chipTool, "Failed to allocate timer");
+    }
+    return err;
+}
+
+#endif // CONFIG_USE_SEPARATE_EVENTLOOP
diff --git a/examples/chip-tool/commands/common/Command.h b/examples/chip-tool/commands/common/Command.h
@@ -187,7 +187,12 @@ class Command
     }
 
     void UpdateWaitForResponse(bool value);
+
+#if CONFIG_USE_SEPARATE_EVENTLOOP
     void WaitForResponse(uint16_t duration);
+#else  // CONFIG_USE_SEPARATE_EVENTLOOP
+    CHIP_ERROR ScheduleWaitForResponse(uint16_t duration);
+#endif // CONFIG_USE_SEPARATE_EVENTLOOP
 
 protected:
     ExecutionContext * GetExecContext() { return mExecContext; }
@@ -202,7 +207,9 @@ class Command
     const char * mName            = nullptr;
     std::vector<Argument> mArgs;
 
+#if CONFIG_USE_SEPARATE_EVENTLOOP
     std::condition_variable cvWaitingForResponse;
     std::mutex cvWaitingForResponseMutex;
     bool mWaitingForResponse{ false };
+#endif // CONFIG_USE_SEPARATE_EVENTLOOP
 };
diff --git a/examples/chip-tool/commands/common/Commands.cpp b/examples/chip-tool/commands/common/Commands.cpp
@@ -70,16 +70,32 @@ 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)));
 
+#if CONFIG_USE_SEPARATE_EVENTLOOP
     err = mController.ServiceEvents();
     VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(Controller, "Init failure! Run Loop: %s", chip::ErrorStr(err)));
+#endif // CONFIG_USE_SEPARATE_EVENTLOOP
 
     err = RunCommand(localId, remoteId, argc, argv, &command);
     SuccessOrExit(err);
 
+#if !CONFIG_USE_SEPARATE_EVENTLOOP
+    chip::DeviceLayer::PlatformMgr().RunEventLoop();
+#endif // !CONFIG_USE_SEPARATE_EVENTLOOP
+
+    if (command)
+    {
+        err = command->GetCommandExitStatus();
+    }
+
 exit:
-#if CONFIG_DEVICE_LAYER
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(chipTool, "Run command failure: %s", chip::ErrorStr(err));
+    }
+
+#if CONFIG_USE_SEPARATE_EVENTLOOP
     chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
-#endif
+#endif // CONFIG_USE_SEPARATE_EVENTLOOP
 
     if (command)
     {
@@ -176,14 +192,14 @@ CHIP_ERROR Commands::RunCommand(NodeId localId, NodeId remoteId, int argc, char
         // set the variable to true, which will cause it to block indefinitely.
         //
         command->UpdateWaitForResponse(true);
+#if CONFIG_USE_SEPARATE_EVENTLOOP
         chip::DeviceLayer::PlatformMgr().ScheduleWork(RunQueuedCommand, reinterpret_cast<intptr_t>(command));
         command->WaitForResponse(command->GetWaitDurationInSeconds());
-        err = command->GetCommandExitStatus();
-        if (err != CHIP_NO_ERROR)
-        {
-            ChipLogError(chipTool, "Run command failure: %s", chip::ErrorStr(err));
-            ExitNow();
-        }
+#else  // CONFIG_USE_SEPARATE_EVENTLOOP
+        err = command->Run();
+        SuccessOrExit(err);
+        command->ScheduleWaitForResponse(command->GetWaitDurationInSeconds());
+#endif // CONFIG_USE_SEPARATE_EVENTLOOP
     }
 
 exit: