Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ESP Linenoise] new esp linenoise with multiconsole support (IDFGH-9126) #10526

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions components/console/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ set(argtable_srcs argtable3/arg_cmd.c

idf_component_register(SRCS "commands.c"
"esp_console_repl.c"
"esp_linenoise.c"
"split_argv.c"
"linenoise/linenoise.c"
${argtable_srcs}
Expand Down
32 changes: 11 additions & 21 deletions components/console/commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,37 +42,22 @@ static SLIST_HEAD(cmd_list_, cmd_item_) s_cmd_list;
/** run-time configuration options */
static esp_console_config_t s_config;

/** temporary buffer used for command line parsing */
static char *s_tmp_line_buf;

static const cmd_item_t *find_command_by_name(const char *name);

esp_err_t esp_console_init(const esp_console_config_t *config)
{
if (!config) {
return ESP_ERR_INVALID_ARG;
}
if (s_tmp_line_buf) {
return ESP_ERR_INVALID_STATE;
}
memcpy(&s_config, config, sizeof(s_config));
if (s_config.hint_color == 0) {
s_config.hint_color = ANSI_COLOR_DEFAULT;
}
s_tmp_line_buf = calloc(config->max_cmdline_length, 1);
if (s_tmp_line_buf == NULL) {
return ESP_ERR_NO_MEM;
}
return ESP_OK;
}

esp_err_t esp_console_deinit(void)
{
if (!s_tmp_line_buf) {
return ESP_ERR_INVALID_STATE;
}
free(s_tmp_line_buf);
s_tmp_line_buf = NULL;
cmd_item_t *it, *tmp;
SLIST_FOREACH_SAFE(it, &s_cmd_list, next, tmp) {
SLIST_REMOVE(&s_cmd_list, it, cmd_item_, next);
Expand Down Expand Up @@ -137,7 +122,7 @@ esp_err_t esp_console_cmd_register(const esp_console_cmd_t *cmd)
return ESP_OK;
}

void esp_console_get_completion(const char *buf, linenoiseCompletions *lc)
void esp_console_get_completion(const char *buf, esp_linenoise_completions_t *lc)
{
size_t len = strlen(buf);
if (len == 0) {
Expand Down Expand Up @@ -184,27 +169,32 @@ static const cmd_item_t *find_command_by_name(const char *name)

esp_err_t esp_console_run(const char *cmdline, int *cmd_ret)
{
if (s_tmp_line_buf == NULL) {
return ESP_ERR_INVALID_STATE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably still need to return ESP_ERR_INVALID_STATE if esp_console_init wasn't called?

char* tmp_line_buf = calloc(s_config.max_cmdline_length, 1);
if (tmp_line_buf == NULL) {
return ESP_ERR_NO_MEM;
}
char **argv = (char **) calloc(s_config.max_cmdline_args, sizeof(char *));
if (argv == NULL) {
free(tmp_line_buf);
return ESP_ERR_NO_MEM;
}
strlcpy(s_tmp_line_buf, cmdline, s_config.max_cmdline_length);
strlcpy(tmp_line_buf, cmdline, s_config.max_cmdline_length);

size_t argc = esp_console_split_argv(s_tmp_line_buf, argv,
size_t argc = esp_console_split_argv(tmp_line_buf, argv,
s_config.max_cmdline_args);
if (argc == 0) {
free(tmp_line_buf);
free(argv);
return ESP_ERR_INVALID_ARG;
}
const cmd_item_t *cmd = find_command_by_name(argv[0]);
if (cmd == NULL) {
free(tmp_line_buf);
free(argv);
return ESP_ERR_NOT_FOUND;
}
*cmd_ret = (*cmd->func)(argc, argv);
free(tmp_line_buf);
free(argv);
return ESP_OK;
}
Expand Down Expand Up @@ -246,4 +236,4 @@ esp_err_t esp_console_register_help_command(void)
.func = &help_command
};
return esp_console_cmd_register(&command);
}
}
8 changes: 3 additions & 5 deletions components/console/esp_console.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ extern "C" {
#include <stddef.h>
#include "sdkconfig.h"
#include "esp_err.h"

// Forward declaration. Definition in linenoise/linenoise.h.
typedef struct linenoiseCompletions linenoiseCompletions;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we keep this as a forward declaration instead of pulling in the dependency on esp_linenoise.h header?

#include "esp_linenoise.h"

/**
* @brief Parameters for console initialization
Expand Down Expand Up @@ -244,9 +242,9 @@ size_t esp_console_split_argv(char *line, char **argv, size_t argv_size);
* linenoiseSetCompletionCallback(&esp_console_get_completion);
*
* @param buf the string typed by the user
* @param lc linenoiseCompletions to be filled in
* @param lc esp_linenoise_completions_t to be filled in
*/
void esp_console_get_completion(const char *buf, linenoiseCompletions *lc);
void esp_console_get_completion(const char *buf, esp_linenoise_completions_t *lc);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: with this change, does the examples/system/console/advanced example from IDF v5.0 compile okay with this brach?

(I assume the answer is "yes" since you didn't have to modify the example. Is that due to that `#include "esp_linenoise.h" above which you have added?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done my testing with the "basic" example. But I think you are correct.


/**
* @brief Callback which provides command hints for linenoise library
Expand Down
Loading