From d653a5182c41934a4bfd94c28e03e8a81c431231 Mon Sep 17 00:00:00 2001 From: Krzysztof Chruscinski Date: Wed, 5 Dec 2018 13:25:43 +0100 Subject: [PATCH] shell: Allow calling shell_fprintf from various contexts Extended shell to allow command to indicate that shell should halt not accepting any input until termination sequence is received (CTRL+C) or shell_command_exit() is called. While shell is in that state it is allowed to print to shell from any thread context. Signed-off-by: Krzysztof Chruscinski --- doc/subsystems/shell/shell.rst | 42 +++++++-- include/shell/shell.h | 65 ++++++++++--- subsys/shell/shell.c | 168 ++++++++++++++++++++++++--------- subsys/shell/shell_ops.c | 3 +- 4 files changed, 212 insertions(+), 66 deletions(-) diff --git a/doc/subsystems/shell/shell.rst b/doc/subsystems/shell/shell.rst index d1e21565f6a..7c3623033a3 100644 --- a/doc/subsystems/shell/shell.rst +++ b/doc/subsystems/shell/shell.rst @@ -235,13 +235,41 @@ Simple command handler implementation: return 0; } -.. warning:: - Do not use function :cpp:func:`shell_fprintf` or shell print macros: - :c:macro:`shell_print`, :c:macro:`shell_info`, :c:macro:`shell_warn`, - :c:macro:`shell_error` outside of the command handler because this - might lead to incorrect text display on the screen. If any text should - be displayed outside of the command context, then use the - :ref:`logger` or `printk` function. +Function :cpp:func:`shell_fprintf` or the shell print macros: +:c:macro:`shell_print`, :c:macro:`shell_info`, :c:macro:`shell_warn` and +:c:macro:`shell_error` can only be used from the command handler, or if the +command context is forced to stay in the foreground by calling +:cpp:func:`shell_command_enter` from within the command handler. In this latter +case, the shell stops reading input and writing to the output (except for the +logs), allowing a user to print from any thread context. Function +:cpp:func:`shell_command_exit` or entering a :kbd:`CTRL+C` terminates a +'foreground' command. + +Here is an example foreground command implementation: + +.. code-block:: c + + static int cmd_handler(const struct shell *shell, size_t argc, + char **argv) + { + ARG_UNUSED(argc); + ARG_UNUSED(argv); + + shell_command_enter(shell); + + foo_shell = shell; + foo_signal_thread(); /* Swtich context */ + + return 0; + } + + static void foo_thread_context(void) + { + shell_print(foo_shell, "Lorem ipsum"); + + /* Terminate foreground command. */ + shell_command_exit(foo_shell); + } Command help ------------ diff --git a/include/shell/shell.h b/include/shell/shell.h index 77769910545..d864618190a 100644 --- a/include/shell/shell.h +++ b/include/shell/shell.h @@ -89,6 +89,15 @@ struct shell_static_args { /** * @brief Shell command handler prototype. + * + * @param shell Shell instance. + * @param argc Arguments count. + * @param argv Arguments. + * + * @retval 0 Successful command execution. + * @retval 1 Help printed and command not executed. + * @retval -EINVAL Argument validation failed. + * @retval -ENOEXEC Command not executed. */ typedef int (*shell_cmd_handler)(const struct shell *shell, size_t argc, char **argv); @@ -238,6 +247,7 @@ enum shell_state { SHELL_STATE_UNINITIALIZED, SHELL_STATE_INITIALIZED, SHELL_STATE_ACTIVE, + SHELL_STATE_COMMAND, SHELL_STATE_PANIC_MODE_ACTIVE, /*!< Panic activated.*/ SHELL_STATE_PANIC_MODE_INACTIVE /*!< Panic requested, not supported.*/ }; @@ -297,10 +307,10 @@ struct shell_transport_api { /** * @brief Function for writing data to the transport interface. * - * @param[in] transport Pointer to the transfer instance. - * @param[in] data Pointer to the source buffer. - * @param[in] length Source buffer length. - * @param[in] cnt Pointer to the sent bytes counter. + * @param[in] transport Pointer to the transfer instance. + * @param[in] data Pointer to the source buffer. + * @param[in] length Source buffer length. + * @param[out] cnt Pointer to the sent bytes counter. * * @return Standard error code. */ @@ -310,10 +320,10 @@ struct shell_transport_api { /** * @brief Function for reading data from the transport interface. * - * @param[in] p_transport Pointer to the transfer instance. - * @param[in] p_data Pointer to the destination buffer. - * @param[in] length Destination buffer length. - * @param[in] cnt Pointer to the received bytes counter. + * @param[in] p_transport Pointer to the transfer instance. + * @param[in] p_data Pointer to the destination buffer. + * @param[in] length Destination buffer length. + * @param[out] cnt Pointer to the received bytes counter. * * @return Standard error code. */ @@ -370,9 +380,10 @@ union shell_internal { enum shell_signal { SHELL_SIGNAL_RXRDY, - SHELL_SIGNAL_TXDONE, SHELL_SIGNAL_LOG_MSG, SHELL_SIGNAL_KILL, + SHELL_SIGNAL_COMMAND_EXIT, + SHELL_SIGNAL_TXDONE, SHELL_SIGNALS }; @@ -407,6 +418,9 @@ struct shell_ctx { struct k_poll_signal signals[SHELL_SIGNALS]; struct k_poll_event events[SHELL_SIGNALS]; + + struct k_mutex wr_mtx; + k_tid_t tid; }; extern const struct log_backend_api log_backend_shell_api; @@ -561,7 +575,11 @@ int shell_stop(const struct shell *shell); /** * @brief printf-like function which sends formatted data stream to the shell. - * This function shall not be used outside of the shell command context. + * + * This function shall not be used outside of the shell command context unless + * command requested to stay in the foreground (see @ref shell_command_enter). + * In that case, function can be called from any thread context until command is + * terminated with CTRL+C or @ref shell_command_exit call. * * @param[in] shell Pointer to the shell instance. * @param[in] color Printed text color. @@ -574,7 +592,7 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color, /** * @brief Print info message to the shell. * - * This function shall not be used outside of the shell command context. + * See @ref shell_fprintf. * * @param[in] _sh Pointer to the shell instance. * @param[in] _ft Format string. @@ -586,7 +604,7 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color, /** * @brief Print normal message to the shell. * - * This function shall not be used outside of the shell command context. + * See @ref shell_fprintf. * * @param[in] _sh Pointer to the shell instance. * @param[in] _ft Format string. @@ -598,7 +616,7 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color, /** * @brief Print warning message to the shell. * - * This function shall not be used outside of the shell command context. + * See @ref shell_fprintf. * * @param[in] _sh Pointer to the shell instance. * @param[in] _ft Format string. @@ -610,7 +628,8 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color, /** * @brief Print error message to the shell. * - * This function shall not be used outside of the shell command context. + * See @ref shell_fprintf. + * * @param[in] _sh Pointer to the shell instance. * @param[in] _ft Format string. * @param[in] ... List of parameters to print. @@ -626,6 +645,24 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color, */ void shell_process(const struct shell *shell); +/** + * @brief Indicate to shell that command stay in foreground, blocking the shell. + * + * Command in foreground is terminated by @ref shell_command_exit or CTRL+C. + * + * @param[in] shell Pointer to the shell instance. + */ +void shell_command_enter(const struct shell *shell); + +/** + * @brief Exit command in foreground state. + * + * See @ref shell_command_enter. + * + * @param[in] shell Pointer to the shell instance. + */ +void shell_command_exit(const struct shell *shell); + /** * @brief Change displayed shell prompt. * diff --git a/subsys/shell/shell.c b/subsys/shell/shell.c index 4f80fb468cb..3719ff73854 100644 --- a/subsys/shell/shell.c +++ b/subsys/shell/shell.c @@ -859,17 +859,22 @@ static void state_collect(const struct shell *shell) switch (shell->ctx->receive_state) { case SHELL_RECEIVE_DEFAULT: if (process_nl(shell, data)) { + int err = 0; + if (!shell->ctx->cmd_buff_len) { history_mode_exit(shell); cursor_next_line_move(shell); } else { /* Command execution */ - (void)execute(shell); + err = execute(shell); } /* Function responsible for printing prompt * on received NL. */ - state_set(shell, SHELL_STATE_ACTIVE); + if (err || + shell->ctx->state != SHELL_STATE_COMMAND) { + state_set(shell, SHELL_STATE_ACTIVE); + } return; } @@ -1008,6 +1013,8 @@ static void state_collect(const struct shell *shell) break; } } + + transport_buffer_flush(shell); } static void transport_evt_handler(enum shell_transport_evt evt_type, void *ctx) @@ -1024,24 +1031,37 @@ static void transport_evt_handler(enum shell_transport_evt evt_type, void *ctx) static void shell_log_process(const struct shell *shell) { bool processed; - int signaled; + int signaled = 0; int result; do { - shell_cmd_line_erase(shell); - processed = shell_log_backend_process(shell->log_backend); - cmd_line_print(shell); - - /* Arbitrary delay added to ensure that prompt is readable and - * can be used to enter further commands. - */ - if (shell->ctx->cmd_buff_len) { - k_sleep(K_MSEC(15)); + if (shell->ctx->state < SHELL_STATE_PANIC_MODE_ACTIVE) { + k_mutex_lock(&shell->ctx->wr_mtx, K_FOREVER); } - k_poll_signal_check(&shell->ctx->signals[SHELL_SIGNAL_RXRDY], - &signaled, &result); + shell_cmd_line_erase(shell); + processed = shell_log_backend_process(shell->log_backend); + + if (shell->ctx->state != SHELL_STATE_COMMAND) { + struct k_poll_signal *signal = + &shell->ctx->signals[SHELL_SIGNAL_RXRDY]; + + cmd_line_print(shell); + + /* Arbitrary delay added to ensure that prompt is readable and + * can be used to enter further commands. + */ + if (shell->ctx->cmd_buff_len) { + k_sleep(K_MSEC(15)); + } + + k_poll_signal_check( signal, &signaled, &result); + } + + if (shell->ctx->state < SHELL_STATE_PANIC_MODE_ACTIVE) { + k_mutex_unlock(&shell->ctx->wr_mtx); + } } while (processed && !signaled); } @@ -1056,14 +1076,15 @@ static int instance_init(const struct shell *shell, const void *p_config, int err = shell->iface->api->init(shell->iface, p_config, transport_evt_handler, (void *) shell); - if (err != 0) { return err; } + memset(shell->ctx, 0, sizeof(*shell->ctx)); + history_init(shell); - memset(shell->ctx, 0, sizeof(*shell->ctx)); + k_mutex_init(&shell->ctx->wr_mtx); if (IS_ENABLED(CONFIG_SHELL_STATS)) { shell->stats->log_lost_cnt = 0; @@ -1110,6 +1131,35 @@ static int instance_uninit(const struct shell *shell) return 0; } +typedef void (*shell_signal_handler_t)(const struct shell *shell); + +static void shell_signal_handle(const struct shell *shell, + enum shell_signal sig_idx, + shell_signal_handler_t handler) +{ + struct k_poll_signal *signal = &shell->ctx->signals[sig_idx]; + int set; + int res; + + k_poll_signal_check(signal, &set, &res); + + if (set) { + k_poll_signal_reset(signal); + handler(shell); + } +} + +static void kill_handler(const struct shell *shell) +{ + (void)instance_uninit(shell); + k_thread_abort(k_current_get()); +} + +static void command_exit_handler(const struct shell *shell) +{ + state_set(shell, SHELL_STATE_ACTIVE); +} + void shell_thread(void *shell_handle, void *arg_log_backend, void *arg_log_level) { @@ -1137,40 +1187,29 @@ void shell_thread(void *shell_handle, void *arg_log_backend, } while (true) { - int signaled; - int result; + int num_events = (shell->ctx->state != SHELL_STATE_COMMAND) ? + SHELL_SIGNALS : SHELL_SIGNAL_TXDONE; - err = k_poll(shell->ctx->events, SHELL_SIGNALS, K_FOREVER); + err = k_poll(shell->ctx->events, num_events, K_FOREVER); if (err != 0) { return; } - k_poll_signal_check(&shell->ctx->signals[SHELL_SIGNAL_KILL], - &signaled, &result); + /* Check for KILL request */ + shell_signal_handle(shell, SHELL_SIGNAL_KILL, kill_handler); + shell_signal_handle(shell, SHELL_SIGNAL_RXRDY, shell_process); - if (signaled) { - k_poll_signal_reset( - &shell->ctx->signals[SHELL_SIGNAL_KILL]); - (void)instance_uninit(shell); - - k_thread_abort(k_current_get()); + if (shell->ctx->state == SHELL_STATE_COMMAND) { + shell_signal_handle(shell, SHELL_SIGNAL_COMMAND_EXIT, + command_exit_handler); + } else { + shell_signal_handle(shell, SHELL_SIGNAL_TXDONE, + shell_process); } - k_poll_signal_check(&shell->ctx->signals[SHELL_SIGNAL_LOG_MSG], - &signaled, &result); - - if (!signaled) { - /* Other signals handled together.*/ - k_poll_signal_reset( - &shell->ctx->signals[SHELL_SIGNAL_RXRDY]); - k_poll_signal_reset( - &shell->ctx->signals[SHELL_SIGNAL_TXDONE]); - shell_process(shell); - } else if (IS_ENABLED(CONFIG_LOG)) { - k_poll_signal_reset( - &shell->ctx->signals[SHELL_SIGNAL_LOG_MSG]); - /* process log msg */ - shell_log_process(shell); + if (IS_ENABLED(CONFIG_LOG)) { + shell_signal_handle(shell, SHELL_SIGNAL_LOG_MSG, + shell_log_process); } } } @@ -1193,6 +1232,7 @@ int shell_init(const struct shell *shell, const void *transport_config, (void *)init_log_level, K_LOWEST_APPLICATION_THREAD_PRIO, 0, K_NO_WAIT); + shell->ctx->tid = tid; k_thread_name_set(tid, shell->thread_name); return 0; @@ -1254,6 +1294,35 @@ int shell_stop(const struct shell *shell) return 0; } +void shell_command_enter(const struct shell *shell) +{ + state_set(shell, SHELL_STATE_COMMAND); +} + +void shell_command_exit(const struct shell *shell) +{ + struct k_poll_signal *signal = + &shell->ctx->signals[SHELL_SIGNAL_COMMAND_EXIT]; + + (void)k_poll_signal_raise(signal, 0); +} + +static void shell_state_command(const struct shell *shell) +{ + size_t count; + char data; + + (void)shell->iface->api->read(shell->iface, &data, + sizeof(data), &count); + if (count == 0) { + return; + } + + if (data == SHELL_VT100_ASCII_CTRL_C) { + state_set(shell, SHELL_STATE_ACTIVE); + } +} + void shell_process(const struct shell *shell) { __ASSERT_NO_MSG(shell); @@ -1276,13 +1345,14 @@ void shell_process(const struct shell *shell) case SHELL_STATE_ACTIVE: state_collect(shell); break; + case SHELL_STATE_COMMAND: + shell_state_command(shell); + break; default: break; } - transport_buffer_flush(shell); - internal.value = 0xFFFFFFFF; internal.flags.processing = 0; (void)atomic_and((atomic_t *)&shell->ctx->internal.value, @@ -1293,14 +1363,22 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color, const char *p_fmt, ...) { __ASSERT_NO_MSG(shell); + __ASSERT(!k_is_in_isr(), "Thread context required."); __ASSERT_NO_MSG(shell->ctx); __ASSERT_NO_MSG(shell->fprintf_ctx); __ASSERT_NO_MSG(p_fmt); va_list args = { 0 }; + if (k_current_get() != shell->ctx->tid && + shell->ctx->state != SHELL_STATE_COMMAND) { + return; + } + va_start(args, p_fmt); + k_mutex_lock(&shell->ctx->wr_mtx, K_FOREVER); + if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS) && shell->ctx->internal.flags.use_colors && (color != shell->ctx->vt100_ctx.col.col)) { @@ -1317,6 +1395,8 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color, } va_end(args); + + k_mutex_unlock(&shell->ctx->wr_mtx); } int shell_prompt_change(const struct shell *shell, char *prompt) diff --git a/subsys/shell/shell_ops.c b/subsys/shell/shell_ops.c index 763347982c1..de180c477f8 100644 --- a/subsys/shell/shell_ops.c +++ b/subsys/shell/shell_ops.c @@ -276,7 +276,8 @@ void shell_cmd_line_erase(const struct shell *shell) static void shell_pend_on_txdone(const struct shell *shell) { - if (IS_ENABLED(CONFIG_MULTITHREADING)) { + if (IS_ENABLED(CONFIG_MULTITHREADING) && + (shell->ctx->state < SHELL_STATE_PANIC_MODE_ACTIVE)) { k_poll(&shell->ctx->events[SHELL_SIGNAL_TXDONE], 1, K_FOREVER); k_poll_signal_reset(&shell->ctx->signals[SHELL_SIGNAL_TXDONE]); } else {