CLI for Controller Board via UART Rx #278
Conversation
projects/uart_cli/src/cli.c
Outdated
| "MSXV Controller Board CLI. Usage: \n\r" | ||
| "<peripheral> <action> <parameters> \n\r\n" | ||
| "Enter \"help\" after any argument for detailed reference. \n\r\n" | ||
| "List of Peripherals: gpio \n\r"; |
There was a problem hiding this comment.
This should iterate over the cmd_lookup table to print out available peripherals, so we don't have to update this each time. this could be done in a print_help function
projects/uart_cli/inc/gpio_cli.h
Outdated
| #include "cli_base.h" | ||
|
|
||
| void gpio_cmd(char *input); | ||
| void prv_set(char *args); |
There was a problem hiding this comment.
prv functions should not be declared in the header, they should be declared static in the c file. That's what makes them private, is that they can't be used outside of the module
| @@ -78,6 +78,7 @@ StatusCode uart_init(UartPort uart, UartSettings *settings) { | |||
| // Init with disabled TX interrupts otherwise we will get TX | |||
| // buffer empty continuously when there is no data to send | |||
| USART_ITConfig(s_port[uart].base, USART_IT_TXE, DISABLE); | |||
| USART_ITConfig(s_port[uart].base, USART_IT_RXNE, ENABLE); | |||
There was a problem hiding this comment.
This was breaking logs before. I would check that log_debug still works for other projects
There was a problem hiding this comment.
Which specific project breaks from this line? I tried scons sim on leds and the log_debug statements display blink correctly.
projects/uart_cli/inc/cli.h
Outdated
| @@ -0,0 +1,11 @@ | |||
| #ifndef PROJECTS_UART_CLI_INC_CLI_H_ | |||
There was a problem hiding this comment.
Use #pragma once instead of ifndef
projects/uart_cli/inc/cli_base.h
Outdated
| @@ -0,0 +1,13 @@ | |||
| #ifndef PROJECTS_UART_CLI_INC_CLI_BASE_H_ | |||
projects/uart_cli/src/cli.c
Outdated
| static const CmdStruct cmd_lookup[] = { { .cmd_name = "gpio", .cmd_func = &gpio_cmd }, | ||
| { .cmd_name = "GPIO", .cmd_func = &gpio_cmd } }; | ||
|
|
||
| TASK(cli_task, TASK_STACK_512) { |
There was a problem hiding this comment.
add this and main function to a separate main.c file
|
|
||
| void cli_init() { | ||
| gpio_init(); | ||
| setbuf(stdout, NULL); |
There was a problem hiding this comment.
What does this do on arm?
projects/uart_cli/src/cli.c
Outdated
| cmd_buffer[idx % MAX_CMD_LEN] = data; | ||
| ++idx; | ||
| printf("%c", data); | ||
| if (idx == MAX_CMD_LEN) { |
There was a problem hiding this comment.
If this occurs, you should probably just stop accepting new characters instead of printing a warning. You can add this to the top, where you check against idx, and if they enter more characters, just don't echo anything or put them in the buffer, unless you get a newline.
projects/uart_cli/src/cli.c
Outdated
|
|
||
| void cmd_parse(char *cmd) { | ||
| char peripheral[MAX_CMD_LEN + 1] = { 0 }; | ||
| strip_ws(cmd); |
There was a problem hiding this comment.
I would just add the strip_ws call to inside tok_cmd, since you'll need it for all instances of calling tok_cmd
projects/uart_cli/src/cli.c
Outdated
| strip_ws(cmd); | ||
| tok_cmd(cmd, peripheral); | ||
|
|
||
| if (strcmp(peripheral, "help") == 0 || strcmp(peripheral, "h") == 0 || |
There was a problem hiding this comment.
Should just make the cmd automatically lower case here. This will cover any mixed-case or upper case scenarios, and will mean that you only need to add one check for each peripheral
| printf("\r%s\n", cli_help); | ||
| } | ||
|
|
||
| void print_help() { |
There was a problem hiding this comment.
is this function used anywhere?
|
|
||
| int valid_pin_mode(char *pin_mode) { | ||
| const char *pin_modes[NUM_GPIO_MODES] = { | ||
| "analog", "input_floating", "input_pull_down", "input_pull_up", "output_open_drain", |
There was a problem hiding this comment.
NIT: fix spacing between these strings
There was a problem hiding this comment.
scons format gives this spacing - the build fails if I change it
| strip_ws(cmd_in); | ||
| strncpy(tmp, cmd_in, sizeof(tmp) - 1); | ||
| char *tmp_tok; | ||
| tmp_tok = strtok(tmp, delim); // NOLINT |
There was a problem hiding this comment.
Why the comment for no lint?
No description provided.