diff --git a/include/shell/shell.h b/include/shell/shell.h index 7a75ad074da..e7e19886e23 100644 --- a/include/shell/shell.h +++ b/include/shell/shell.h @@ -30,6 +30,10 @@ extern "C" { #define CONFIG_SHELL_PRINTF_BUFF_SIZE 0 #endif +#ifndef CONFIG_SHELL_HISTORY_BUFFER +#define CONFIG_SHELL_HISTORY_BUFFER 0 +#endif + #define SHELL_CMD_ROOT_LVL (0u) /** @@ -643,7 +647,7 @@ extern void shell_print_stream(const void *user_ctx, const char *data, SHELL_LOG_BACKEND_DEFINE(_name, _name##_out_buffer, \ CONFIG_SHELL_PRINTF_BUFF_SIZE, \ _log_queue_size, _log_timeout); \ - SHELL_HISTORY_DEFINE(_name, CONFIG_SHELL_CMD_BUFF_SIZE, 7); \ + SHELL_HISTORY_DEFINE(_name##_history, CONFIG_SHELL_HISTORY_BUFFER); \ SHELL_FPRINTF_DEFINE(_name##_fprintf, &_name, _name##_out_buffer, \ CONFIG_SHELL_PRINTF_BUFF_SIZE, \ true, shell_print_stream); \ @@ -655,7 +659,8 @@ extern void shell_print_stream(const void *user_ctx, const char *data, .default_prompt = _prompt, \ .iface = _transport_iface, \ .ctx = &UTIL_CAT(_name, _ctx), \ - .history = SHELL_HISTORY_PTR(_name), \ + .history = IS_ENABLED(CONFIG_SHELL_HISTORY) ? \ + &_name##_history : NULL, \ .shell_flag = _shell_flag, \ .fprintf_ctx = &_name##_fprintf, \ .stats = SHELL_STATS_PTR(_name), \ diff --git a/include/shell/shell_history.h b/include/shell/shell_history.h index 2ce6a8d9bc0..02a16328f3e 100644 --- a/include/shell/shell_history.h +++ b/include/shell/shell_history.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #ifdef __cplusplus @@ -18,45 +19,88 @@ extern "C" { struct shell_history { - struct k_mem_slab *mem_slab; + struct ring_buf *ring_buf; sys_dlist_t list; sys_dnode_t *current; }; -struct shell_history_item { - sys_dnode_t dnode; - u16_t len; - char data[]; -}; - -#ifdef CONFIG_SHELL_HISTORY -#define SHELL_HISTORY_DEFINE(_name, block_size, block_count) \ - \ - K_MEM_SLAB_DEFINE(_name##_history_memslab, \ - ROUND_UP(block_size + sizeof(struct shell_history_item), \ - sizeof(void *)), block_count, 4); \ - static struct shell_history _name##_history = { \ - .mem_slab = &_name##_history_memslab \ +/** + * @brief Create shell history instance. + * + * @param _name History instance name. + * @param _size Memory dedicated for shell history. + */ +#define SHELL_HISTORY_DEFINE(_name, _size) \ + static u8_t __noinit __aligned(sizeof(u32_t)) \ + _name##_ring_buf_data[_size]; \ + static struct ring_buf _name##_ring_buf = \ + { \ + .size = _size, \ + .buf = { .buf8 = _name##_ring_buf_data } \ + }; \ + static struct shell_history _name = { \ + .ring_buf = &_name##_ring_buf \ } -#define SHELL_HISTORY_PTR(_name) (&_name##_history) -#else /* CONFIG_SHELL_HISTORY */ -#define SHELL_HISTORY_DEFINE(_name, block_size, block_count) /*empty*/ -#define SHELL_HISTORY_PTR(_name) NULL -#endif +/** + * @brief Initialize shell history module. + * + * @param history Shell history instance. + */ void shell_history_init(struct shell_history *history); +/** + * @brief Purge shell history. + * + * Function clears whole shell command history. + * + * @param history Shell history instance. + * + */ void shell_history_purge(struct shell_history *history); +/** + * @brief Exit history browsing mode. + * + * @param history Shell history instance. + */ void shell_history_mode_exit(struct shell_history *history); -/* returns true if remains in history mode.*/ +/** + * @brief Get next entry in shell command history. + * + * Function returns next (in given direction) stored line. + * + * @param[in] history Shell history instance. + * @param[in] up Direction. + * @param[out] dst Buffer where line is copied. + * @param[in,out] len Buffer size (intput), amount of copied + * data (output). + * @return True if remains in history mode. + */ bool shell_history_get(struct shell_history *history, bool up, u8_t *dst, u16_t *len); +/** + * @brief Put line into shell command history. + * + * If history is full, oldest entry (or entries) is removed. + * + * @param history Shell history instance. + * @param line Data. + * @param len Data length. + * + */ void shell_history_put(struct shell_history *history, u8_t *line, size_t len); +/** + * @brief Get state of shell history. + * + * @param history Shell history instance. + * + * @return True if in browsing mode. + */ static inline bool shell_history_active(struct shell_history *history) { return (history->current) ? true : false; diff --git a/subsys/shell/Kconfig b/subsys/shell/Kconfig index 4fe7070ef65..e5e71fe60c9 100644 --- a/subsys/shell/Kconfig +++ b/subsys/shell/Kconfig @@ -99,6 +99,7 @@ config SHELL_HELP_ON_WRONG_ARGUMENT_COUNT config SHELL_HISTORY bool "Enable history in shell" default y + select RING_BUFFER help Enable commands history. History can be accessed using up and down arrows. @@ -107,7 +108,7 @@ if SHELL_HISTORY config SHELL_HISTORY_BUFFER int "History buffer in bytes" - default 1024 + default 512 help Number of bytes dedicated for storing executed commands. diff --git a/subsys/shell/shell_history.c b/subsys/shell/shell_history.c index 9c72b9444fc..3514aff0d03 100644 --- a/subsys/shell/shell_history.c +++ b/subsys/shell/shell_history.c @@ -7,6 +7,40 @@ #include #include +/* + * History must store strings (commands) and allow traversing them and adding + * new string. When new item is added then first it is compared if it is not + * the same as the last one (then it is not stored). If there is no room in the + * buffer to store the new item, oldest one is removed until there is a room. + * + * Items are allocated and stored in the ring buffer. Items then a linked in + * the list. + * + * Because stored strings must be copied and compared, it is more convenient to + * store them in the ring buffer in a way that they are not split into two + * chunks (when ring buffer wraps). To ensure that item is in a single chunk, + * item includes padding. If continues area for new item cannot be allocated + * then allocated space is increased by the padding. + * + * If item does not fit at the end of the ring buffer padding is added: * + * +-----------+----------------+-----------------------------------+---------+ + * | header | "history item" | | padding | + * | padding | | | | + * +-----------+----------------+-----------------------------------+---------+ + * + * If item fits in the ring buffer available space then there is no padding: + * +-----------------+------------+----------------+--------------------------+ + * | | header | "history item" | | + * | | no padding | | | + * +-----------------+------------+----------------+--------------------------+ + */ +struct shell_history_item { + sys_dnode_t dnode; + u16_t len; + u16_t padding; + char data[0]; +}; + void shell_history_mode_exit(struct shell_history *history) { history->current = NULL; @@ -43,7 +77,7 @@ bool shell_history_get(struct shell_history *history, bool up, history->current = l_item; h_item = CONTAINER_OF(l_item, struct shell_history_item, dnode); - if (h_item) { + if (l_item) { memcpy(dst, h_item->data, h_item->len); *len = h_item->len; dst[*len] = '\0'; @@ -56,30 +90,40 @@ bool shell_history_get(struct shell_history *history, bool up, static void add_to_head(struct shell_history *history, struct shell_history_item *item, - u8_t *src, size_t len) + u8_t *src, size_t len, u16_t padding) { item->len = len; + item->padding = padding; memcpy(item->data, src, len); - item->data[len] = '\0'; sys_dlist_prepend(&history->list, &item->dnode); } -static void remove_from_tail(struct shell_history *history) +/* Returns true if element was removed. */ +static bool remove_from_tail(struct shell_history *history) { sys_dnode_t *l_item; /* list item */ struct shell_history_item *h_item; + u32_t total_len; + + if (sys_dlist_is_empty(&history->list)) { + return false; + } l_item = sys_dlist_peek_tail(&history->list); sys_dlist_remove(l_item); h_item = CONTAINER_OF(l_item, struct shell_history_item, dnode); - k_mem_slab_free(history->mem_slab, (void **)&l_item); + + total_len = offsetof(struct shell_history_item, data) + + h_item->len + h_item->padding; + ring_buf_get_finish(history->ring_buf, total_len); + + return true; } void shell_history_purge(struct shell_history *history) { - while (!sys_dlist_is_empty(&history->list)) { - remove_from_tail(history); + while (remove_from_tail(history)) { } } @@ -87,6 +131,17 @@ void shell_history_put(struct shell_history *history, u8_t *line, size_t len) { sys_dnode_t *l_item; /* list item */ struct shell_history_item *h_item; + u32_t total_len = len + offsetof(struct shell_history_item, data); + u32_t claim_len; + u32_t claim2_len; + u16_t padding = (~total_len + 1) & (sizeof(void *) - 1); + + /* align to word. */ + total_len += padding; + + if (total_len > ring_buf_capacity_get(history->ring_buf)) { + return; + } shell_history_mode_exit(history); @@ -97,20 +152,50 @@ void shell_history_put(struct shell_history *history, u8_t *line, size_t len) l_item = sys_dlist_peek_head(&history->list); h_item = CONTAINER_OF(l_item, struct shell_history_item, dnode); - if (h_item && + if (l_item && (h_item->len == len) && - (strncmp(h_item->data, line, CONFIG_SHELL_CMD_BUFF_SIZE) == 0)) { + (memcmp(h_item->data, line, len) == 0)) { /* Same command as before, do not store */ return; } - while (k_mem_slab_alloc(history->mem_slab, (void **)&h_item, K_NO_WAIT) - != 0) { - /* if no space remove the oldest entry. */ - remove_from_tail(history); - } + do { + claim_len = ring_buf_put_claim(history->ring_buf, + (u8_t **)&h_item, total_len); + /* second allocation may succeed if we were at the end of the + * buffer. + */ + if (claim_len < total_len) { + claim2_len = + ring_buf_put_claim(history->ring_buf, + (u8_t **)&h_item, total_len); + if (claim2_len == total_len) { + ring_buf_put_finish(history->ring_buf, + claim_len); + padding += claim_len; + claim_len = total_len; + } + } - add_to_head(history, h_item, line, len); + if (claim_len == total_len) { + add_to_head(history, h_item, line, len, padding); + ring_buf_put_finish(history->ring_buf, claim_len); + break; + } + + ring_buf_put_finish(history->ring_buf, 0); + if (remove_from_tail(history) == false) { + __ASSERT_NO_MSG(ring_buf_is_empty(history->ring_buf)); + /* if history is empty reset ring buffer. Even when + * ring buffer is empty, it is possible that available + * continues memory in worst case equals half of the + * ring buffer capacity. By reseting ring buffer we + * ensure that it is capable to provide continues memory + * of ring buffer capacity length. + */ + ring_buf_reset(history->ring_buf); + } + } while (1); } void shell_history_init(struct shell_history *history) diff --git a/tests/subsys/shell/shell_history/src/shell_history_test.c b/tests/subsys/shell/shell_history/src/shell_history_test.c index 7b13f677f6b..f71c58d9e21 100644 --- a/tests/subsys/shell/shell_history/src/shell_history_test.c +++ b/tests/subsys/shell/shell_history/src/shell_history_test.c @@ -14,8 +14,8 @@ #include -#define MAX_BUF_SIZE 128 -SHELL_HISTORY_DEFINE(history, MAX_BUF_SIZE, 8); +#define HIST_BUF_SIZE 128 +SHELL_HISTORY_DEFINE(history, HIST_BUF_SIZE); static void init_test_buf(u8_t *buf, size_t len, u8_t offset) { @@ -27,12 +27,12 @@ static void init_test_buf(u8_t *buf, size_t len, u8_t offset) static void test_get(bool ok, bool up, u8_t *exp_buf, u16_t exp_len) { bool res; - u8_t out_buf[MAX_BUF_SIZE]; + u8_t out_buf[HIST_BUF_SIZE]; u16_t out_len; out_len = sizeof(out_buf); - res = shell_history_get(&history_history, up, out_buf, &out_len); + res = shell_history_get(&history, up, out_buf, &out_len); if (ok) { zassert_true(res, "history should contain one entry.\n"); @@ -56,32 +56,34 @@ static void test_get(bool ok, bool up, u8_t *exp_buf, u16_t exp_len) */ void test_history_add_get(void) { - u8_t exp_buf[MAX_BUF_SIZE]; + u8_t exp_buf[HIST_BUF_SIZE]; init_test_buf(exp_buf, sizeof(exp_buf), 0); - shell_history_init(&history_history); + shell_history_init(&history); test_get(false, true, NULL, 0); - shell_history_put(&history_history, exp_buf, 20); + shell_history_put(&history, exp_buf, 20); test_get(true, true, exp_buf, 20); + + shell_history_purge(&history); } /* Test verifies that after purging there is no line in the history. */ void test_history_purge(void) { - u8_t exp_buf[MAX_BUF_SIZE]; + u8_t exp_buf[HIST_BUF_SIZE]; init_test_buf(exp_buf, sizeof(exp_buf), 0); - shell_history_init(&history_history); + shell_history_init(&history); - shell_history_put(&history_history, exp_buf, 20); - shell_history_put(&history_history, exp_buf, 20); + shell_history_put(&history, exp_buf, 20); + shell_history_put(&history, exp_buf, 20); - shell_history_purge(&history_history); + shell_history_purge(&history); test_get(false, true, NULL, 0); } @@ -103,26 +105,26 @@ void test_history_purge(void) */ void test_history_get_up_and_down(void) { - u8_t exp1_buf[MAX_BUF_SIZE]; - u8_t exp2_buf[MAX_BUF_SIZE]; - u8_t exp3_buf[MAX_BUF_SIZE]; + u8_t exp1_buf[HIST_BUF_SIZE]; + u8_t exp2_buf[HIST_BUF_SIZE]; + u8_t exp3_buf[HIST_BUF_SIZE]; init_test_buf(exp1_buf, sizeof(exp1_buf), 0); init_test_buf(exp2_buf, sizeof(exp2_buf), 10); init_test_buf(exp3_buf, sizeof(exp3_buf), 20); - shell_history_init(&history_history); + shell_history_init(&history); - shell_history_put(&history_history, exp1_buf, 20); - shell_history_put(&history_history, exp2_buf, 20); - shell_history_put(&history_history, exp3_buf, 20); + shell_history_put(&history, exp1_buf, 20); + shell_history_put(&history, exp2_buf, 15); + shell_history_put(&history, exp3_buf, 20); test_get(true, true, exp3_buf, 20); /* up - 3*/ - test_get(true, true, exp2_buf, 20); /* up - 2*/ + test_get(true, true, exp2_buf, 15); /* up - 2*/ test_get(true, true, exp1_buf, 20); /* up - 1*/ - test_get(true, false, exp2_buf, 20); /* down - 2 */ + test_get(true, false, exp2_buf, 15); /* down - 2 */ test_get(true, true, exp1_buf, 20); /* up - 1*/ - test_get(true, false, exp2_buf, 20); /* down - 2 */ + test_get(true, false, exp2_buf, 15); /* down - 2 */ test_get(true, false, exp3_buf, 20); /* down - 3 */ test_get(false, false, NULL, 0); /* down - nothing */ }