From 04de43b1a119665532b6b2a0c6bbcbefd47eb7d6 Mon Sep 17 00:00:00 2001 From: Jakub Rzeszutko Date: Tue, 9 Jan 2024 18:35:06 +0100 Subject: [PATCH] shell: Fix shell init procedure when configured as inactive on startup The previous behavior of the CONFIG_SHELL_AUTOSTART option, where setting it to 'n' disabled shell interaction at startup but kept the logger active as a shell backend, was inconsistent. Now, when CONFIG_SHELL_AUTOSTART is set to 'n', both the shell and the logger are inactive at startup. Calling the shell_start function will activate them and print any pending logs. This change ensures a more consistent and logical behavior regarding shell and logger activity at startup. Additionally, now when the shell_stop function is called, both the shell and the logger are halted. This enhancement ensures that stopping the shell also effectively stops the associated logger. Signed-off-by: Jakub Rzeszutko --- include/zephyr/shell/shell.h | 4 ++++ subsys/shell/shell.c | 18 ++++++++++-------- subsys/shell/shell_ops.h | 13 +++++++++++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/include/zephyr/shell/shell.h b/include/zephyr/shell/shell.h index 8382dc8290..0bcb831c90 100644 --- a/include/zephyr/shell/shell.h +++ b/include/zephyr/shell/shell.h @@ -743,6 +743,7 @@ struct shell_backend_ctx_flags { uint32_t cmd_ctx :1; /*!< Shell is executing command */ uint32_t print_noinit :1; /*!< Print request from not initialized shell */ uint32_t sync_mode :1; /*!< Shell in synchronous mode */ + uint32_t handle_log :1; /*!< Shell is handling logger backend */ }; BUILD_ASSERT((sizeof(struct shell_backend_ctx_flags) == sizeof(uint32_t)), @@ -798,6 +799,9 @@ struct shell_ctx { /** When bypass is set, all incoming data is passed to the callback. */ shell_bypass_cb_t bypass; + /*!< Logging level for a backend. */ + uint32_t log_level; + #if defined CONFIG_SHELL_GETOPT /*!< getopt context for a shell backend. */ struct getopt_state getopt; diff --git a/subsys/shell/shell.c b/subsys/shell/shell.c index efff35e263..944e30ae70 100644 --- a/subsys/shell/shell.c +++ b/subsys/shell/shell.c @@ -1315,21 +1315,16 @@ void shell_thread(void *shell_handle, void *arg_log_backend, void *arg_log_level) { struct shell *sh = shell_handle; - bool log_backend = (bool)arg_log_backend; - uint32_t log_level = POINTER_TO_UINT(arg_log_level); int err; + z_flag_handle_log_set(sh, (bool)arg_log_backend); + sh->ctx->log_level = POINTER_TO_UINT(arg_log_level); + err = sh->iface->api->enable(sh->iface, false); if (err != 0) { return; } - if (IS_ENABLED(CONFIG_SHELL_LOG_BACKEND) && log_backend - && !IS_ENABLED(CONFIG_SHELL_START_OBSCURED)) { - z_shell_log_backend_enable(sh->log_backend, (void *)sh, - log_level); - } - if (IS_ENABLED(CONFIG_SHELL_AUTOSTART)) { /* Enable shell and print prompt. */ err = shell_start(sh); @@ -1430,6 +1425,11 @@ int shell_start(const struct shell *sh) return -ENOTSUP; } + if (IS_ENABLED(CONFIG_SHELL_LOG_BACKEND) && z_flag_handle_log_get(sh) + && !z_flag_obscure_get(sh)) { + z_shell_log_backend_enable(sh->log_backend, (void *)sh, sh->ctx->log_level); + } + k_mutex_lock(&sh->ctx->wr_mtx, K_FOREVER); if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS)) { @@ -1460,6 +1460,8 @@ int shell_stop(const struct shell *sh) state_set(sh, SHELL_STATE_INITIALIZED); + z_shell_log_backend_disable(sh->log_backend); + return 0; } diff --git a/subsys/shell/shell_ops.h b/subsys/shell/shell_ops.h index 59c4d8f738..293396cc28 100644 --- a/subsys/shell/shell_ops.h +++ b/subsys/shell/shell_ops.h @@ -221,6 +221,19 @@ static inline bool z_flag_sync_mode_set(const struct shell *sh, bool val) return ret; } +static inline bool z_flag_handle_log_get(const struct shell *sh) +{ + return sh->ctx->ctx.flags.handle_log == 1; +} + +static inline bool z_flag_handle_log_set(const struct shell *sh, bool val) +{ + bool ret; + + Z_SHELL_SET_FLAG_ATOMIC(sh, ctx, handle_log, val, ret); + return ret; +} + /* Function sends VT100 command to clear the screen from cursor position to * end of the screen. */