From b0571746e25dfcae81244717e7384c73133bb304 Mon Sep 17 00:00:00 2001 From: Jakub Rzeszutko Date: Fri, 5 Oct 2018 10:49:08 +0200 Subject: [PATCH] shell: parsing output LF character Some terminals literally interprets shell output data. Hence to print a message in new line shell needs to send `\r\n` each time. To minimize flash usage user can now send `\n` as a line delimiter and shell will automatically add missing CR character. Signed-off-by: Jakub Rzeszutko --- include/shell/shell.h | 17 ++++- include/shell/shell_fprintf.h | 1 - samples/drivers/flash_shell/src/main.c | 71 ++++++++++---------- samples/subsys/shell/shell_module/src/main.c | 15 +++-- subsys/shell/shell.c | 2 + subsys/shell/shell_fprintf.c | 7 ++ tests/bluetooth/shell/src/main.c | 3 +- 7 files changed, 69 insertions(+), 47 deletions(-) diff --git a/include/shell/shell.h b/include/shell/shell.h index 5352977ff17..306ad1d3aff 100644 --- a/include/shell/shell.h +++ b/include/shell/shell.h @@ -166,7 +166,6 @@ enum shell_receive_state { SHELL_RECEIVE_TILDE_EXP }; - /** * @internal @brief Internal shell state. */ @@ -346,6 +345,14 @@ struct shell_ctx { extern const struct log_backend_api log_backend_shell_api; +/** + * @brief Flags for setting shell output newline sequence. + */ +enum shell_flag { + SHELL_FLAG_CRLF_DEFAULT = (1<<0), /* Do not map CR or LF */ + SHELL_FLAG_OLF_CRLF = (1<<1) /* Map LF to CRLF on output */ +}; + /** * @brief Shell instance internals. */ @@ -357,6 +364,8 @@ struct shell { struct shell_history *history; + const enum shell_flag shell_flag; + const struct shell_fprintf *fprintf_ctx; struct shell_stats *stats; @@ -376,9 +385,10 @@ struct shell { * @param[in] _prompt Shell prompt string. * @param[in] transport_iface Pointer to the transport interface. * @param[in] log_queue_size Logger processing queue size. + * @param[in] _shell_flag Shell output newline sequence. */ #define SHELL_DEFINE(_name, _prompt, transport_iface, \ - log_queue_size) \ + log_queue_size, _shell_flag) \ static const struct shell _name; \ static struct shell_ctx UTIL_CAT(_name, _ctx); \ static char _name##prompt[CONFIG_SHELL_PROMPT_LENGTH + 1] = _prompt; \ @@ -386,7 +396,7 @@ struct shell { SHELL_LOG_BACKEND_DEFINE(_name, _name##_out_buffer, \ CONFIG_SHELL_PRINTF_BUFF_SIZE); \ SHELL_HISTORY_DEFINE(_name, 128, 8);/*todo*/ \ - SHELL_FPRINTF_DEFINE(_name## _fprintf, &_name, _name##_out_buffer, \ + SHELL_FPRINTF_DEFINE(_name##_fprintf, &_name, _name##_out_buffer, \ CONFIG_SHELL_PRINTF_BUFF_SIZE, \ true, shell_print_stream); \ LOG_INSTANCE_REGISTER(shell, _name, CONFIG_SHELL_LOG_LEVEL); \ @@ -398,6 +408,7 @@ struct shell { .iface = transport_iface, \ .ctx = &UTIL_CAT(_name, _ctx), \ .history = SHELL_HISTORY_PTR(_name), \ + .shell_flag = _shell_flag, \ .fprintf_ctx = &_name##_fprintf, \ .stats = SHELL_STATS_PTR(_name), \ .log_backend = SHELL_LOG_BACKEND_PTR(_name), \ diff --git a/include/shell/shell_fprintf.h b/include/shell/shell_fprintf.h index 02ee633ba53..7ae81ab5801 100644 --- a/include/shell/shell_fprintf.h +++ b/include/shell/shell_fprintf.h @@ -34,7 +34,6 @@ struct shell_fprintf { struct shell_fprintf_control_block *ctrl_blk; }; - /** * @brief Macro for defining shell_fprintf instance. * diff --git a/samples/drivers/flash_shell/src/main.c b/samples/drivers/flash_shell/src/main.c index 9b128ca959b..ac25d3f1feb 100644 --- a/samples/drivers/flash_shell/src/main.c +++ b/samples/drivers/flash_shell/src/main.c @@ -20,13 +20,14 @@ LOG_MODULE_REGISTER(app); SHELL_UART_DEFINE(shell_transport_uart); -SHELL_DEFINE(uart_shell, "uart:~$ ", &shell_transport_uart, 10); +SHELL_DEFINE(uart_shell, "uart:~$ ", &shell_transport_uart, 10, + SHELL_FLAG_OLF_CRLF); #define PR_SHELL(shell, fmt, ...) \ shell_fprintf(shell, SHELL_NORMAL, fmt, ##__VA_ARGS__) #define PR_ERROR(shell, fmt, ...) \ shell_fprintf(shell, SHELL_ERROR, fmt, ##__VA_ARGS__) -#define PR_INFO(fshell, fmt, ...) \ +#define PR_INFO(shell, fmt, ...) \ shell_fprintf(shell, SHELL_INFO, fmt, ##__VA_ARGS__) #define PR_WARNING(shell, fmt, ...) \ shell_fprintf(shell, SHELL_WARNING, fmt, ##__VA_ARGS__) @@ -91,7 +92,7 @@ static int check_flash_device(const struct shell *shell) { if (flash_device == NULL) { PR_ERROR(shell, "Flash device is unknown." - " Run set_device first.\r\n"); + " Run set_device first.\n"); return -ENODEV; } return 0; @@ -103,7 +104,7 @@ static void dump_buffer(const struct shell *shell, u8_t *buf, size_t size) u8_t *p = buf; while (size >= 8) { - PR_SHELL(shell, "%02x %02x %02x %02x | %02x %02x %02x %02x\r\n", + PR_SHELL(shell, "%02x %02x %02x %02x | %02x %02x %02x %02x\n", p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7]); p += 8; size -= 8; @@ -120,7 +121,7 @@ static void dump_buffer(const struct shell *shell, u8_t *buf, size_t size) newline = true; } if (newline) { - PR_SHELL(shell, "\r\n"); + PR_SHELL(shell, "\n"); } } @@ -173,7 +174,7 @@ static int do_read(const struct shell *shell, off_t offset, size_t len) return 0; err_read: - PR_ERROR(shell, "flash_read error: %d\r\n", ret); + PR_ERROR(shell, "flash_read error: %d\n", ret); return ret; } @@ -185,18 +186,18 @@ static int do_erase(const struct shell *shell, off_t offset, size_t size) ret = flash_write_protection_set(flash_device, false); if (ret) { PR_ERROR(shell, "Failed to disable flash protection (err: %d)." - "\r\n", ret); + "\n", ret); return ret; } ret = flash_erase(flash_device, offset, size); if (ret) { - PR_ERROR(shell, "flash_erase failed (err:%d).\r\n", ret); + PR_ERROR(shell, "flash_erase failed (err:%d).\n", ret); return ret; } ret = flash_write_protection_set(flash_device, true); if (ret) { PR_ERROR(shell, "Failed to enable flash protection (err: %d)." - "\r\n", ret); + "\n", ret); } return ret; } @@ -210,22 +211,22 @@ static int do_write(const struct shell *shell, off_t offset, u8_t *buf, ret = flash_write_protection_set(flash_device, false); if (ret) { PR_ERROR(shell, "Failed to disable flash protection (err: %d)." - "\r\n", ret); + "\n", ret); return ret; } ret = flash_write(flash_device, offset, buf, len); if (ret) { - PR_ERROR(shell, "flash_write failed (err:%d).\r\n", ret); + PR_ERROR(shell, "flash_write failed (err:%d).\n", ret); return ret; } ret = flash_write_protection_set(flash_device, true); if (ret) { PR_ERROR(shell, "Failed to enable flash protection (err: %d)." - "\r\n", ret); + "\n", ret); return ret; } if (read_back) { - PR_SHELL(shell, "Reading back written bytes:\r\n"); + PR_SHELL(shell, "Reading back written bytes:\n"); ret = do_read(shell, offset, len); } return ret; @@ -252,7 +253,7 @@ static int cmd_write_block_size(const struct shell *shell, size_t argc, err = check_flash_device(shell); if (!err) { - PR_SHELL(shell, "%d\r\n", + PR_SHELL(shell, "%d\n", flash_get_write_block_size(flash_device)); } @@ -274,7 +275,7 @@ static int cmd_read(const struct shell *shell, size_t argc, char **argv) } if (parse_ul(argv[1], &offset) || parse_ul(argv[2], &len)) { - PR_ERROR(shell, "Invalid arguments.\r\n"); + PR_ERROR(shell, "Invalid arguments.\n"); err = -EINVAL; goto exit; } @@ -300,7 +301,7 @@ static int cmd_erase(const struct shell *shell, size_t argc, char **argv) } if (parse_ul(argv[1], &offset) || parse_ul(argv[2], &size)) { - PR_ERROR(shell, "Invalid arguments.\r\n"); + PR_ERROR(shell, "Invalid arguments.\n"); err = -EINVAL; goto exit; } @@ -327,15 +328,15 @@ static int cmd_write(const struct shell *shell, size_t argc, char **argv) err = parse_ul(argv[1], &offset); if (err) { - PR_ERROR(shell, "Invalid argument.\r\n"); + PR_ERROR(shell, "Invalid argument.\n"); goto exit; } if ((argc - 2) > ARGC_MAX) { /* Can only happen if Zephyr limit is increased. */ - PR_ERROR(shell, "At most %lu bytes can be written.\r\n" + PR_ERROR(shell, "At most %lu bytes can be written.\n" "In order to write more bytes please increase" - " parameter: CONFIG_SHELL_ARGC_MAX.\r\n", + " parameter: CONFIG_SHELL_ARGC_MAX.\n", (unsigned long)ARGC_MAX); err = -EINVAL; goto exit; @@ -346,9 +347,9 @@ static int cmd_write(const struct shell *shell, size_t argc, char **argv) argv += 2; for (i = 0; i < argc; i++) { if (parse_u8(argv[i], &buf[i])) { - PR_ERROR(shell, "Argument %lu (%s) is not a byte.\r\n" + PR_ERROR(shell, "Argument %lu (%s) is not a byte.\n" "Bytes shall be passed in decimal" - " notation.\r\n", + " notation.\n", i + 1, argv[i]); err = -EINVAL; goto exit; @@ -374,7 +375,7 @@ static int cmd_page_count(const struct shell *shell, size_t argc, char **argv) err = check_flash_device(shell); if (!err) { page_count = flash_get_page_count(flash_device); - PR_SHELL(shell, "Flash device contains %lu pages.\r\n", + PR_SHELL(shell, "Flash device contains %lu pages.\n", (unsigned long int)page_count); } @@ -400,7 +401,7 @@ static bool page_layout_cb(const struct flash_pages_info *info, void *datav) sz = info->size; PR_SHELL(data->shell, - "\tPage %u: start 0x%08x, length 0x%lx (%lu, %lu KB)\r\n", + "\tPage %u: start 0x%08x, length 0x%lx (%lu, %lu KB)\n", info->index, info->start_offset, sz, sz, sz / KB(1)); return true; } @@ -440,7 +441,7 @@ static int cmd_page_layout(const struct shell *shell, size_t argc, char **argv) } break; default: - PR_ERROR(shell, "Invalid argument count.\r\n"); + PR_ERROR(shell, "Invalid argument count.\n"); return -EINVAL; } @@ -451,7 +452,7 @@ static int cmd_page_layout(const struct shell *shell, size_t argc, char **argv) return 0; bail: - PR_ERROR(shell, "Invalid arguments.\r\n"); + PR_ERROR(shell, "Invalid arguments.\n"); return err; } @@ -485,7 +486,7 @@ static int cmd_page_read(const struct shell *shell, size_t argc, char **argv) ret = flash_get_page_info_by_idx(flash_device, page, &info); if (ret) { PR_ERROR(shell, "Function flash_page_info_by_idx returned an" - " error: %d\r\n", ret); + " error: %d\n", ret); return ret; } offset += info.start_offset; @@ -493,7 +494,7 @@ static int cmd_page_read(const struct shell *shell, size_t argc, char **argv) return ret; bail: - PR_ERROR(shell, "Invalid arguments.\r\n"); + PR_ERROR(shell, "Invalid arguments.\n"); return ret; } @@ -526,11 +527,11 @@ static int cmd_page_erase(const struct shell *shell, size_t argc, char **argv) ret = flash_get_page_info_by_idx(flash_device, page + i, &info); if (ret) { PR_ERROR(shell, "flash_get_page_info_by_idx error:" - " %d\r\n", ret); + " %d\n", ret); return ret; } PR_SHELL(shell, "Erasing page %u (start offset 0x%x," - " size 0x%x)\r\n", + " size 0x%x)\n", info.index, info.start_offset, info.size); ret = do_erase(shell, info.start_offset, info.size); if (ret) { @@ -541,7 +542,7 @@ static int cmd_page_erase(const struct shell *shell, size_t argc, char **argv) return ret; bail: - PR_ERROR(shell, "Invalid arguments.\r\n"); + PR_ERROR(shell, "Invalid arguments.\n"); return ret; } @@ -570,7 +571,7 @@ static int cmd_page_write(const struct shell *shell, size_t argc, char **argv) argv += 3; for (i = 0; i < argc; i++) { if (parse_u8(argv[i], &buf[i])) { - PR_ERROR(shell, "Argument %d (%s) is not a byte.\r\n", + PR_ERROR(shell, "Argument %d (%s) is not a byte.\n", i + 2, argv[i]); ret = -EINVAL; goto bail; @@ -579,14 +580,14 @@ static int cmd_page_write(const struct shell *shell, size_t argc, char **argv) ret = flash_get_page_info_by_idx(flash_device, page, &info); if (ret) { - PR_ERROR(shell, "flash_get_page_info_by_idx: %d\r\n", ret); + PR_ERROR(shell, "flash_get_page_info_by_idx: %d\n", ret); return ret; } ret = do_write(shell, info.start_offset + off, buf, i, true); return ret; bail: - PR_ERROR(shell, "Invalid arguments.\r\n"); + PR_ERROR(shell, "Invalid arguments.\n"); return ret; } #endif /* CONFIG_FLASH_PAGE_LAYOUT */ @@ -605,11 +606,11 @@ static int cmd_set_dev(const struct shell *shell, size_t argc, char **argv) /* Run command. */ dev = device_get_binding(name); if (!dev) { - PR_ERROR(shell, "No device named %s.\r\n", name); + PR_ERROR(shell, "No device named %s.\n", name); return -ENOEXEC; } if (flash_device) { - PR_SHELL(shell, "Leaving behind device %s\r\n", + PR_SHELL(shell, "Leaving behind device %s\n", flash_device->config->name); } flash_device = dev; diff --git a/samples/subsys/shell/shell_module/src/main.c b/samples/subsys/shell/shell_module/src/main.c index f0a463a6b9d..e7f46bee589 100644 --- a/samples/subsys/shell/shell_module/src/main.c +++ b/samples/subsys/shell/shell_module/src/main.c @@ -15,7 +15,8 @@ LOG_MODULE_REGISTER(app); SHELL_UART_DEFINE(shell_transport_uart); -SHELL_DEFINE(uart_shell, "uart:~$ ", &shell_transport_uart, 10); +SHELL_DEFINE(uart_shell, "uart:~$ ", &shell_transport_uart, 10, + SHELL_FLAG_OLF_CRLF); extern void foo(void); @@ -37,7 +38,7 @@ static int cmd_log_test_start(const struct shell *shell, size_t argc, } k_timer_start(&log_timer, period, period); - shell_fprintf(shell, SHELL_NORMAL, "Log test started\r\n"); + shell_fprintf(shell, SHELL_NORMAL, "Log test started\n"); return 0; } @@ -66,7 +67,7 @@ static int cmd_log_test_stop(const struct shell *shell, size_t argc, } k_timer_stop(&log_timer); - shell_fprintf(shell, SHELL_NORMAL, "Log test stopped\r\n"); + shell_fprintf(shell, SHELL_NORMAL, "Log test stopped\n"); return 0; } @@ -96,7 +97,7 @@ static int cmd_demo_ping(const struct shell *shell, size_t argc, char **argv) ARG_UNUSED(argc); ARG_UNUSED(argv); - shell_fprintf(shell, SHELL_NORMAL, "pong\r\n"); + shell_fprintf(shell, SHELL_NORMAL, "pong\n"); return 0; } @@ -105,10 +106,10 @@ static int cmd_demo_params(const struct shell *shell, size_t argc, char **argv) { int cnt; - shell_fprintf(shell, SHELL_NORMAL, "argc = %d\r\n", argc); + shell_fprintf(shell, SHELL_NORMAL, "argc = %d\n", argc); for (cnt = 0; cnt < argc; cnt++) { shell_fprintf(shell, SHELL_NORMAL, - " argv[%d] = %s\r\n", cnt, argv[cnt]); + " argv[%d] = %s\n", cnt, argv[cnt]); } return 0; } @@ -119,7 +120,7 @@ static int cmd_version(const struct shell *shell, size_t argc, char **argv) ARG_UNUSED(argv); shell_fprintf(shell, SHELL_NORMAL, - "Zephyr version %s\r\n", KERNEL_VERSION_STRING); + "Zephyr version %s\n", KERNEL_VERSION_STRING); return 0; } diff --git a/subsys/shell/shell.c b/subsys/shell/shell.c index 4aca086577f..172041944c5 100644 --- a/subsys/shell/shell.c +++ b/subsys/shell/shell.c @@ -1154,6 +1154,8 @@ static int shell_instance_init(const struct shell *shell, const void *p_config, { __ASSERT_NO_MSG(shell); __ASSERT_NO_MSG(shell->ctx && shell->iface && shell->prompt); + __ASSERT_NO_MSG((shell->shell_flag == SHELL_FLAG_CRLF_DEFAULT) || + (shell->shell_flag == SHELL_FLAG_OLF_CRLF)); int err; diff --git a/subsys/shell/shell_fprintf.c b/subsys/shell/shell_fprintf.c index e9e08b6dc71..e48282e37ea 100644 --- a/subsys/shell/shell_fprintf.c +++ b/subsys/shell/shell_fprintf.c @@ -5,6 +5,7 @@ */ #include +#include #ifdef CONFIG_NEWLIB_LIBC typedef int (*out_func_t)(int c, void *ctx); @@ -16,8 +17,14 @@ extern int _prf(int (*func)(), void *dest, char *format, va_list vargs); static int out_func(int c, void *ctx) { const struct shell_fprintf *sh_fprintf; + const struct shell *shell; sh_fprintf = (const struct shell_fprintf *)ctx; + shell = (const struct shell *)sh_fprintf->user_ctx; + + if ((shell->shell_flag == SHELL_FLAG_OLF_CRLF) && (c == '\n')) { + (void)out_func('\r', ctx); + } sh_fprintf->buffer[sh_fprintf->ctrl_blk->buffer_cnt] = (u8_t)c; sh_fprintf->ctrl_blk->buffer_cnt++; diff --git a/tests/bluetooth/shell/src/main.c b/tests/bluetooth/shell/src/main.c index fa2e2bb9604..1dd9afdb772 100644 --- a/tests/bluetooth/shell/src/main.c +++ b/tests/bluetooth/shell/src/main.c @@ -34,7 +34,8 @@ #define DEVICE_NAME CONFIG_BT_DEVICE_NAME SHELL_UART_DEFINE(shell_transport_uart); -SHELL_DEFINE(uart_shell, "uart:~$ ", &shell_transport_uart, 10); +SHELL_DEFINE(uart_shell, "uart:~$ ", &shell_transport_uart, 10, + SHELL_FLAG_CRLF_DEFAULT); #if defined(CONFIG_BT_CONN) static bool hrs_simulate;