From a88d5d7e4fa3076e1e295cfde23daa6531dbaafe Mon Sep 17 00:00:00 2001 From: Jakub Rzeszutko Date: Thu, 18 Oct 2018 14:33:24 +0200 Subject: [PATCH] shell: commands help unification 1. Changed return value of function: shell_cmd_precheck from bool to int. Now it returns: 0 when argument count is correct and help print is not requested 1 when help was requested and printed -EINVAL on wrong arguments count This change simply shell_cmd_precheck usege in command handlers. 2. Unified all commands in shell_cmd.c file. 3. Fixed a bug where help was not printed on wrong argument count. Signed-off-by: Jakub Rzeszutko --- include/shell/shell.h | 12 +- subsys/shell/shell.c | 18 +-- subsys/shell/shell_cmds.c | 288 ++++++++++++++++++++++---------------- 3 files changed, 187 insertions(+), 131 deletions(-) diff --git a/include/shell/shell.h b/include/shell/shell.h index 306ad1d3aff..4a1271146b0 100644 --- a/include/shell/shell.h +++ b/include/shell/shell.h @@ -572,12 +572,14 @@ int shell_prompt_change(const struct shell *shell, char *prompt); * @param[in] opt Pointer to the optional option array. * @param[in] opt_len Option array size. * - * @return True if check passed, false otherwise or help was requested. + * @return 0 if check passed + * @return 1 if help was requested + * @return -EINVAL if wrong argument count */ -bool shell_cmd_precheck(const struct shell *shell, - bool arg_cnt_ok, - const struct shell_getopt_option *opt, - size_t opt_len); +int shell_cmd_precheck(const struct shell *shell, + bool arg_cnt_ok, + const struct shell_getopt_option *opt, + size_t opt_len); /** * @internal @brief This function shall not be used directly, it is required by diff --git a/subsys/shell/shell.c b/subsys/shell/shell.c index 172041944c5..c413d923585 100644 --- a/subsys/shell/shell.c +++ b/subsys/shell/shell.c @@ -1700,29 +1700,29 @@ int shell_prompt_change(const struct shell *shell, char *prompt) return -1; } -bool shell_cmd_precheck(const struct shell *shell, - bool arg_cnt_ok, - const struct shell_getopt_option *opt, - size_t opt_len) +int shell_cmd_precheck(const struct shell *shell, + bool arg_cnt_ok, + const struct shell_getopt_option *opt, + size_t opt_len) { if (shell_help_requested(shell)) { shell_help_print(shell, opt, opt_len); - return false; + return 1; /* help printed */ } if (!arg_cnt_ok) { shell_fprintf(shell, SHELL_ERROR, - "%s: wrong parameter count\r\n", + "%s: wrong parameter count\n", shell->ctx->active_cmd.syntax); - if (IS_ENABLED(SHELL_HELP_ON_WRONG_ARGUMENT_COUNT)) { + if (IS_ENABLED(CONFIG_SHELL_HELP_ON_WRONG_ARGUMENT_COUNT)) { shell_help_print(shell, opt, opt_len); } - return false; + return -EINVAL; } - return true; + return 0; } int shell_execute_cmd(const struct shell *shell, const char *cmd) diff --git a/subsys/shell/shell_cmds.c b/subsys/shell/shell_cmds.c index 80c69166242..ea3befd3bd4 100644 --- a/subsys/shell/shell_cmds.c +++ b/subsys/shell/shell_cmds.c @@ -9,7 +9,7 @@ #include "shell_vt100.h" #define SHELL_HELP_CLEAR "Clear screen." -#define SHELL_HELP_BACKSPACE_MODE "Toggle backspace key mode.\r\n" \ +#define SHELL_HELP_BACKSPACE_MODE "Toggle backspace key mode.\n" \ "Some terminals are not sending separate escape code for" \ "backspace and delete button. Hence backspace is not working as" \ "expected. This command can force shell to interpret delete" \ @@ -182,156 +182,195 @@ static int terminal_size_get(const struct shell *shell) static int cmd_clear(const struct shell *shell, size_t argc, char **argv) { - (void)argv; + ARG_UNUSED(argv); - if ((argc == 2) && (shell_help_requested(shell))) { - shell_help_print(shell, NULL, 0); - return 0; + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret) { + return ret; } + SHELL_VT100_CMD(shell, SHELL_VT100_CURSORHOME); SHELL_VT100_CMD(shell, SHELL_VT100_CLEARSCREEN); + return 0; } static int cmd_shell(const struct shell *shell, size_t argc, char **argv) { - (void)argv; + int ret = shell_cmd_precheck(shell, (argc == 2), NULL, 0); - if ((argc == 1) || ((argc == 2) && shell_help_requested(shell))) { - shell_help_print(shell, NULL, 0); - return 0; + if (ret) { + return ret; } - shell_fprintf(shell, SHELL_ERROR, SHELL_MSG_SPECIFY_SUBCOMMAND); - return 0; -} + shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\n", argv[0], + SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); -static int cmd_bacskpace_mode(const struct shell *shell, size_t argc, - char **argv) -{ - (void)shell_cmd_precheck(shell, (argc == 2), NULL, 0); - return 0; + return -EINVAL; } static int cmd_bacskpace_mode_backspace(const struct shell *shell, size_t argc, - char **argv) + char **argv) { - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ARG_UNUSED(argv); + + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell->ctx->internal.flags.mode_delete = 0; } - shell->ctx->internal.flags.mode_delete = 0; - return 0; + return ret; } static int cmd_bacskpace_mode_delete(const struct shell *shell, size_t argc, char **argv) { - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ARG_UNUSED(argv); + + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell->ctx->internal.flags.mode_delete = 1; } - shell->ctx->internal.flags.mode_delete = 1; - return 0; + return ret; +} + +static int cmd_bacskpace_mode(const struct shell *shell, size_t argc, + char **argv) +{ + int ret = shell_cmd_precheck(shell, (argc == 2), NULL, 0); + + if (ret) { + return ret; + } + + shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\n", argv[0], + SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); + + return -EINVAL; } static int cmd_colors_off(const struct shell *shell, size_t argc, char **argv) { - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ARG_UNUSED(argv); + + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell->ctx->internal.flags.use_colors = 0; } - shell->ctx->internal.flags.use_colors = 0; - return 0; + return ret; } static int cmd_colors_on(const struct shell *shell, size_t argc, char **argv) { - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ARG_UNUSED(argv); + + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell->ctx->internal.flags.use_colors = 1; } - shell->ctx->internal.flags.use_colors = 1; - return 0; + + return ret; } static int cmd_colors(const struct shell *shell, size_t argc, char **argv) { - if (argc == 1) { - shell_help_print(shell, NULL, 0); - return 0; + int ret = shell_cmd_precheck(shell, (argc == 2), NULL, 0); + + if (ret) { + return ret; } - if (!shell_cmd_precheck(shell, (argc == 2), NULL, 0)) { - return 0; - } - - shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\r\n", argv[0], + shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\n", argv[0], SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); - return -ENOEXEC; -} -static int cmd_echo(const struct shell *shell, size_t argc, char **argv) -{ - if (!shell_cmd_precheck(shell, (argc <= 2), NULL, 0)) { - return 0; - } - - if (argc == 2) { - shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\r\n", argv[0], - SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); - return -ENOEXEC; - } - - shell_fprintf(shell, SHELL_NORMAL, "Echo status: %s\r\n", - flag_echo_is_set(shell) ? "on" : "off"); - return 0; + return -EINVAL; } static int cmd_echo_off(const struct shell *shell, size_t argc, char **argv) { - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ARG_UNUSED(argv); + + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell->ctx->internal.flags.echo = 0; } - shell->ctx->internal.flags.echo = 0; - return 0; + return ret; } static int cmd_echo_on(const struct shell *shell, size_t argc, char **argv) { - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ARG_UNUSED(argv); + + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell->ctx->internal.flags.echo = 1; } - shell->ctx->internal.flags.echo = 1; + return ret; +} + +static int cmd_echo(const struct shell *shell, size_t argc, char **argv) +{ + int ret = shell_cmd_precheck(shell, (argc <= 2), NULL, 0); + + if (ret) { + return ret; + } + + if (argc == 2) { + shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\n", argv[0], + SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); + return -EINVAL; + } + + shell_fprintf(shell, SHELL_NORMAL, "Echo status: %s\n", + flag_echo_is_set(shell) ? "on" : "off"); + return 0; } static int cmd_help(const struct shell *shell, size_t argc, char **argv) { - shell_fprintf(shell, SHELL_NORMAL, "Please press the button to " - "see all available commands.\r\n" - "You can also use the button " - "to prompt or auto-complete all " - "commands or its subcommands.\r\n" - "You can try to call commands " - "with <-h> or <--help> parameter to " - "get know what they are doing.\r\n"); + ARG_UNUSED(argc); + ARG_UNUSED(argv); + + shell_fprintf(shell, SHELL_NORMAL, + "Please press the button to see all available commands.\n" + "You can also use the button to prompt or auto-complete" + "all commands or its subcommands.\n" + "You can try to call commands with <-h> or <--help> parameter" + " for more information.\n"); + return 0; } static int cmd_history(const struct shell *shell, size_t argc, char **argv) { + ARG_UNUSED(argv); + size_t i = 0; size_t len; + int ret; if (!IS_ENABLED(CONFIG_SHELL_HISTORY)) { - shell_fprintf(shell, SHELL_ERROR, "Command not supported.\r\n"); + shell_fprintf(shell, SHELL_ERROR, "Command not supported.\n"); return -ENOEXEC; } - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret) { + return ret; } while (1) { @@ -339,7 +378,7 @@ static int cmd_history(const struct shell *shell, size_t argc, char **argv) shell->ctx->temp_buff, &len); if (len) { - shell_fprintf(shell, SHELL_NORMAL, "[%3d] %s\r\n", + shell_fprintf(shell, SHELL_NORMAL, "[%3d] %s\n", i++, shell->ctx->temp_buff); } else { @@ -348,70 +387,82 @@ static int cmd_history(const struct shell *shell, size_t argc, char **argv) } shell->ctx->temp_buff[0] = '\0'; - return 0; -} -static int cmd_shell_stats(const struct shell *shell, size_t argc, char **argv) -{ - if (argc == 1) { - shell_help_print(shell, NULL, 0); - return 0; - } - - if (argc == 2) { - shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\r\n", argv[0], - SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); - return -ENOEXEC; - } - - (void)shell_cmd_precheck(shell, (argc <= 2), NULL, 0); return 0; } static int cmd_shell_stats_show(const struct shell *shell, size_t argc, char **argv) { + ARG_UNUSED(argv); + if (!IS_ENABLED(CONFIG_SHELL_STATS)) { - shell_fprintf(shell, SHELL_ERROR, "Command not supported.\r\n"); + shell_fprintf(shell, SHELL_ERROR, "Command not supported.\n"); return -ENOEXEC; } - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell_fprintf(shell, SHELL_NORMAL, "Lost logs: %u\n", + shell->stats->log_lost_cnt); } - shell_fprintf(shell, SHELL_NORMAL, "Lost logs: %u\r\n", - shell->stats->log_lost_cnt); - return 0; + return ret; } static int cmd_shell_stats_reset(const struct shell *shell, size_t argc, char **argv) { + ARG_UNUSED(argv); + if (!IS_ENABLED(CONFIG_SHELL_STATS)) { - shell_fprintf(shell, SHELL_ERROR, "Command not supported.\r\n"); + shell_fprintf(shell, SHELL_ERROR, "Command not supported.\n"); return -ENOEXEC; } - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + shell->stats->log_lost_cnt = 0; } - shell->stats->log_lost_cnt = 0; - return 0; + return ret; +} + +static int cmd_shell_stats(const struct shell *shell, size_t argc, char **argv) +{ + ARG_UNUSED(argc); + + if (shell_help_requested(shell)) { + shell_help_print(shell, NULL, 0); + return 1; + } else if (argc == 1) { + shell_help_print(shell, NULL, 0); + } else { + shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\n", argv[0], + SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); + } + + return -EINVAL; } static int cmd_resize_default(const struct shell *shell, size_t argc, char **argv) { - if (!shell_cmd_precheck(shell, (argc == 1), NULL, 0)) { - return 0; + ARG_UNUSED(argv); + + int ret = shell_cmd_precheck(shell, (argc == 1), NULL, 0); + + if (ret == 0) { + SHELL_VT100_CMD(shell, SHELL_VT100_SETCOL_80); + shell->ctx->vt100_ctx.cons.terminal_wid = + SHELL_DEFAULT_TERMINAL_WIDTH; + shell->ctx->vt100_ctx.cons.terminal_hei = + SHELL_DEFAULT_TERMINAL_HEIGHT; } - SHELL_VT100_CMD(shell, SHELL_VT100_SETCOL_80); - shell->ctx->vt100_ctx.cons.terminal_wid = SHELL_DEFAULT_TERMINAL_WIDTH; - shell->ctx->vt100_ctx.cons.terminal_hei = SHELL_DEFAULT_TERMINAL_HEIGHT; - return 0; + return ret; } static int cmd_resize(const struct shell *shell, size_t argc, char **argv) @@ -419,18 +470,19 @@ static int cmd_resize(const struct shell *shell, size_t argc, char **argv) int err; if (!IS_ENABLED(CONFIG_SHELL_CMDS_RESIZE)) { - shell_fprintf(shell, SHELL_ERROR, "Command not supported.\r\n"); + shell_fprintf(shell, SHELL_ERROR, "Command not supported.\n"); return -ENOEXEC; } - if (!shell_cmd_precheck(shell, (argc <= 2), NULL, 0)) { - return 0; + err = shell_cmd_precheck(shell, (argc <= 2), NULL, 0); + if (err) { + return err; } if (argc != 1) { - shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\r\n", argv[0], + shell_fprintf(shell, SHELL_ERROR, "%s:%s%s\n", argv[0], SHELL_MSG_UNKNOWN_PARAMETER, argv[1]); - return -ENOEXEC; + return -EINVAL; } err = terminal_size_get(shell); @@ -441,8 +493,10 @@ static int cmd_resize(const struct shell *shell, size_t argc, char **argv) SHELL_DEFAULT_TERMINAL_HEIGHT; shell_fprintf(shell, SHELL_WARNING, "No response from the terminal, assumed 80x24 " - "screen size\r\n"); + "screen size\n"); + return -ENOEXEC; } + return 0; }