From 6031b35803b12bac860445c3ee382af49c94f893 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Thu, 11 Jan 2024 10:53:29 +0800 Subject: [PATCH] fix(console): Made setting command context less error-prone and clearer --- components/console/commands.c | 29 ++--- components/console/esp_console.h | 26 ++--- .../test_apps/console/main/test_console.c | 103 +++++++++--------- 3 files changed, 70 insertions(+), 88 deletions(-) diff --git a/components/console/commands.c b/components/console/commands.c index 800b57fac6..a91e4fc337 100644 --- a/components/console/commands.c +++ b/components/console/commands.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2016-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2016-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -135,8 +135,15 @@ esp_err_t esp_console_cmd_register(const esp_console_cmd_t *cmd) item->hint = buf; } item->argtable = cmd->argtable; - item->func = cmd->func; - item->func_w_context = cmd->func_w_context; + + if (cmd->func) { + item->func = cmd->func; + } else { + // cmd->func_w_context is valid here according to check above + item->func_w_context = cmd->func_w_context; + item->context = cmd->context; + } + cmd_item_t *last = NULL; cmd_item_t *it; SLIST_FOREACH(it, &s_cmd_list, next) { @@ -153,22 +160,6 @@ esp_err_t esp_console_cmd_register(const esp_console_cmd_t *cmd) return ESP_OK; } -esp_err_t esp_console_cmd_set_context(const char *cmd, void *context) -{ - if (cmd == NULL ) { - return ESP_ERR_INVALID_ARG; - } - - cmd_item_t *it; - SLIST_FOREACH(it, &s_cmd_list, next) { - if (strcmp(cmd, it->command) == 0) { - it->context = context; - return ESP_OK; - } - } - return ESP_ERR_NOT_FOUND; -} - void esp_console_get_completion(const char *buf, linenoiseCompletions *lc) { size_t len = strlen(buf); diff --git a/components/console/esp_console.h b/components/console/esp_console.h index dff737ba00..236d6949dc 100644 --- a/components/console/esp_console.h +++ b/components/console/esp_console.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2016-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2016-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -206,15 +206,19 @@ typedef struct { * @note: Setting both \c func and \c func_w_context is not allowed. */ esp_console_cmd_func_with_context_t func_w_context; + /** + * Context pointer to user-defined per-command context data. + * This is used if context aware function \c func_w_context is set. + */ + void *context; } esp_console_cmd_t; /** * @brief Register console command * @param cmd pointer to the command description; can point to a temporary value * - * @note If the member func_w_context of cmd is set instead of func, then there - * MUST be a subsequent call to \c esp_console_cmd_set_context to initialize the - * function context before it is used! + * @note If the member \c func_w_context of cmd is set instead of func, then the member \c context + * is passed to the function pointed to by \c func_w_context. * * @return * - ESP_OK on success @@ -225,20 +229,6 @@ typedef struct { */ esp_err_t esp_console_cmd_register(const esp_console_cmd_t *cmd); -/** - * @brief Register context for a command registered with \c func_w_context before - * - * \c context is only used if \c func_w_context has been set in the structure - * passed to esp_console_cmd_register() - * @param cmd pointer to the command name - * @param context pointer to user-defined per-command context data - * @return - * - ESP_OK on success - * - ESP_ERR_NOT_FOUND if command was not found - * - ESP_ERR_INVALID_ARG if invalid arguments - */ -esp_err_t esp_console_cmd_set_context(const char *cmd, void *context); - /** * @brief Run command line * @param cmdline command line (command name followed by a number of arguments) diff --git a/components/console/test_apps/console/main/test_console.c b/components/console/test_apps/console/main/test_console.c index ef63ca683f..78cda6c453 100644 --- a/components/console/test_apps/console/main/test_console.c +++ b/components/console/test_apps/console/main/test_console.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -94,6 +94,24 @@ TEST_CASE("esp console init/deinit test", "[console]") TEST_ESP_OK(esp_console_deinit()); } +TEST_CASE("esp console init/deinit with context test", "[console]") +{ + int dummy = 47; + esp_console_config_t console_config = ESP_CONSOLE_CONFIG_DEFAULT(); + TEST_ESP_OK(esp_console_init(&console_config)); + const esp_console_cmd_t cmd = { + .command = "hello", + .help = "Print Hello World", + .hint = NULL, + .func_w_context = do_not_call, + .context = &dummy, + }; + TEST_ESP_OK(esp_console_cmd_register(&cmd)); + // re-register the same command, just for test + TEST_ESP_OK(esp_console_cmd_register(&cmd)); + TEST_ESP_OK(esp_console_deinit()); +} + static esp_console_repl_t *s_repl = NULL; /* handle 'quit' command */ @@ -196,22 +214,6 @@ TEST_CASE("esp console init/deinit test, minimal config", "[console]") TEST_ESP_OK(esp_console_deinit()); } -TEST_CASE("esp console test set_context", "[console]") -{ - /* Test with minimal init config */ - esp_console_config_t console_config = { - .max_cmdline_args = 2, - .max_cmdline_length = 100, - }; - - TEST_ESP_OK(esp_console_init(&console_config)); - - TEST_ASSERT_EQUAL(esp_console_cmd_set_context(NULL, NULL), ESP_ERR_INVALID_ARG); - TEST_ASSERT_EQUAL(esp_console_cmd_set_context("invalid", NULL), ESP_ERR_NOT_FOUND); - - TEST_ESP_OK(esp_console_deinit()); -} - TEST_CASE("esp console test with context", "[console]") { /* Test with minimal init config */ @@ -222,44 +224,43 @@ TEST_CASE("esp console test with context", "[console]") TEST_ESP_OK(esp_console_init(&console_config)); - const esp_console_cmd_t cmds[] = { - { - .command = "hello-c1", - .help = "Print Hello World in context c1", - .hint = NULL, - .func_w_context = do_hello_cmd_with_context, - }, - { - .command = "hello-c2", - .help = "Print Hello World in context c2", - .hint = NULL, - .func_w_context = do_hello_cmd_with_context, - }, - }; - cmd_context_t contexts[] = { - { - .in = "c1", - .out = NULL, - }, - { - .in = "c2", - .out = NULL, - }, + cmd_context_t context0 = { + .in = "c1", + .out = NULL, }; - static_assert((sizeof(contexts) / sizeof(contexts[0])) == (sizeof(cmds) / sizeof(cmds[0]))); + cmd_context_t context1 = { + .in = "c2", + .out = NULL, + }; - for (int i=0; i < sizeof(cmds) / sizeof(cmds[0]); i++) { - TEST_ESP_OK(esp_console_cmd_register(&cmds[i])); - TEST_ESP_OK(esp_console_cmd_set_context(cmds[i].command, &contexts[i])); - } + const esp_console_cmd_t cmd0 = { + .command = "hello-c1", + .help = "Print Hello World in context c1", + .hint = NULL, + .func_w_context = do_hello_cmd_with_context, + .context = &context0, + }; - for (int i=0; i < sizeof(cmds) / sizeof(cmds[0]); i++) { - int ret; - TEST_ESP_OK(esp_console_run(cmds[i].command, &ret)); - TEST_ASSERT_EQUAL(ret, 0); - TEST_ASSERT_EQUAL(contexts[i].in, contexts[i].out); - } + const esp_console_cmd_t cmd1 = { + .command = "hello-c2", + .help = "Print Hello World in context c2", + .hint = NULL, + .func_w_context = do_hello_cmd_with_context, + .context = &context1, + }; + + TEST_ESP_OK(esp_console_cmd_register(&cmd0)); + TEST_ESP_OK(esp_console_cmd_register(&cmd1)); + + int ret; + TEST_ESP_OK(esp_console_run(cmd0.command, &ret)); + TEST_ASSERT_EQUAL(ret, 0); + TEST_ASSERT_EQUAL(context0.in, context0.out); + + TEST_ESP_OK(esp_console_run(cmd1.command, &ret)); + TEST_ASSERT_EQUAL(ret, 0); + TEST_ASSERT_EQUAL(context1.in, context1.out); TEST_ESP_OK(esp_console_deinit()); }