From 35728c207adf5e1f7ab0c0706b2c82e30be19b30 Mon Sep 17 00:00:00 2001
From: Damian Krolik <damian.krolik@nordicsemi.no>
Date: Thu, 10 Aug 2023 13:21:40 +0200
Subject: [PATCH] [zephyr] Run shell commands in Matter thread

1. Run Matter shell commands in Matter thread instead of
   Zephyr's shell thread. Make the shell thread wait for
   the condition variable signalled when the command ends
   in the Matter thread.

   This is done to avoid data races when accessing Matter's
   data structures, and avoid stack overflow when executing
   Matter functions in the shell thread that uses
   a relatively small stack.
2. Print either:
   - "Done" or
   - "Error: <error_string_or_code>"
   after each shell command. This is to align with other
   platforms and be able to remove unnecessary logging
   from existing shell command implementations.

Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
---
 src/lib/shell/MainLoopZephyr.cpp | 87 +++++++++++++++++++++++++++-----
 1 file changed, 75 insertions(+), 12 deletions(-)

diff --git a/src/lib/shell/MainLoopZephyr.cpp b/src/lib/shell/MainLoopZephyr.cpp
index d106bfa89fa13b..84fca4ef9d29af 100644
--- a/src/lib/shell/MainLoopZephyr.cpp
+++ b/src/lib/shell/MainLoopZephyr.cpp
@@ -14,37 +14,100 @@
  *    See the License for the specific language governing permissions and
  *    limitations under the License.
  */
-#include <zephyr/init.h>
-#include <zephyr/shell/shell.h>
-
 #include <lib/core/CHIPError.h>
 #include <lib/shell/Engine.h>
+#include <lib/shell/streamer.h>
 #include <lib/shell/streamer_zephyr.h>
+#include <platform/CHIPDeviceLayer.h>
+
+#include <zephyr/init.h>
+#include <zephyr/kernel.h>
+#include <zephyr/shell/shell.h>
+
+using namespace chip;
+using namespace chip::DeviceLayer;
+
+namespace {
+
+K_MUTEX_DEFINE(sShellMutex);
+K_CONDVAR_DEFINE(sCommandResultCondVar);
+CHIP_ERROR sCommandResult;
+
+// RAII helper for synchronizing access to resources shared between the Zephyr's shell thread,
+// which reads and parses the user input, and the the Matter thread, which executes a Matter
+// shell command and reports the result back to the shell thread.
+class ShellGuard
+{
+public:
+    ShellGuard() { k_mutex_lock(&sShellMutex, K_FOREVER); }
+    ~ShellGuard() { k_mutex_unlock(&sShellMutex); }
 
-using chip::Shell::Engine;
+    CHIP_ERROR WaitForCommandResult()
+    {
+        k_condvar_wait(&sCommandResultCondVar, &sShellMutex, K_FOREVER);
+        return sCommandResult;
+    }
 
-static int cmd_matter(const struct shell * shell, size_t argc, char ** argv)
+    void PutCommandResult(CHIP_ERROR error)
+    {
+        sCommandResult = error;
+        k_condvar_signal(&sCommandResultCondVar);
+    }
+};
+
+void ExecCommandInMatterThread(intptr_t argvAsInt)
 {
-    chip::Shell::streamer_set_shell(shell);
-    return (Engine::Root().ExecCommand(argc - 1, argv + 1) == CHIP_NO_ERROR) ? 0 : -ENOEXEC;
+    char ** argv = reinterpret_cast<char **>(argvAsInt);
+    int argc     = 0;
+
+    while (argv[argc] != nullptr)
+    {
+        argc++;
+    }
+
+    ShellGuard shellGuard;
+    shellGuard.PutCommandResult(Shell::Engine::Root().ExecCommand(argc, argv));
 }
 
-static int RegisterCommands()
+int ExecCommandInShellThread(const struct shell * shell, size_t argc, char ** argv)
 {
-    Engine::Root().RegisterDefaultCommands();
+    const CHIP_ERROR error = [shell, argv]() -> CHIP_ERROR {
+        ShellGuard shellGuard;
+        Shell::streamer_set_shell(shell);
+        ReturnErrorOnFailure(PlatformMgr().ScheduleWork(ExecCommandInMatterThread, reinterpret_cast<intptr_t>(argv + 1)));
+
+        return shellGuard.WaitForCommandResult();
+    }();
+
+    if (error != CHIP_NO_ERROR)
+    {
+        Shell::streamer_printf(Shell::streamer_get(), "Error: %" CHIP_ERROR_FORMAT "\r\n", error.Format());
+    }
+    else
+    {
+        Shell::streamer_printf(Shell::streamer_get(), "Done\r\n");
+    }
+
+    return error == CHIP_NO_ERROR ? 0 : -ENOEXEC;
+}
+
+int RegisterCommands()
+{
+    Shell::Engine::Root().RegisterDefaultCommands();
     return 0;
 }
 
-SYS_INIT(RegisterCommands, POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);
+} // namespace
 
-SHELL_CMD_ARG_REGISTER(matter, NULL, "Matter commands", cmd_matter, 1, 10);
+SYS_INIT(RegisterCommands, POST_KERNEL, CONFIG_KERNEL_INIT_PRIORITY_DEFAULT);
+SHELL_CMD_ARG_REGISTER(matter, NULL, "Matter commands", ExecCommandInShellThread, 1, CHIP_SHELL_MAX_TOKENS);
 
 namespace chip {
 namespace Shell {
 
 void Engine::RunMainLoop()
 {
-    // Intentionally empty as Zephyr has own thread handling shell
+    // Intentionally empty as Zephyr has its own thread handling shell.
 }
 
 } // namespace Shell