Skip to content

Commit

Permalink
[zephyr] Run shell commands in Matter thread (project-chip#28623)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Damian-Nordic authored Aug 16, 2023
1 parent 8ff2331 commit 1df99a9
Showing 1 changed file with 75 additions and 12 deletions.
87 changes: 75 additions & 12 deletions src/lib/shell/MainLoopZephyr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1df99a9

Please sign in to comment.