shell: safe print from different threads

Added display text management to shell_fprintf function.
Now it can be used from diffrent threads with not risk that
displayed lines will overlay.

Signed-off-by: Jakub Rzeszutko <jakub.rzeszutko@nordicsemi.no>
Signed-off-by: Krzysztof Chruściński <krzysztof.chruscinski@nordicsemi.no>

subsys/shell/shell.c

@@ -44,12 +44,26 @@ static void cmd_buffer_clear(const struct shell *shell)
 
 static inline void prompt_print(const struct shell *shell)
 {
-	shell_fprintf(shell, SHELL_INFO, "%s", shell->prompt);
+	/* Below cannot be printed by shell_fprinf because it will cause
+	 * interrupt spin
+	 */
+	if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS) &&
+	    shell->ctx->internal.flags.use_colors &&
+	    (SHELL_INFO != shell->ctx->vt100_ctx.col.col)) {
+		struct shell_vt100_colors col;
+
+		shell_vt100_colors_store(shell, &col);
+		shell_vt100_color_set(shell, SHELL_INFO);
+		shell_raw_fprintf(shell->fprintf_ctx, "%s", shell->prompt);
+		shell_vt100_colors_restore(shell, &col);
+	} else {
+		shell_raw_fprintf(shell->fprintf_ctx, "%s", shell->prompt);
+	}
 }
 
 static inline void cmd_print(const struct shell *shell)
 {
-	shell_fprintf(shell, SHELL_NORMAL, "%s", shell->ctx->cmd_buff);
+	shell_raw_fprintf(shell->fprintf_ctx, "%s", shell->ctx->cmd_buff);
 }
 
 static void cmd_line_print(const struct shell *shell)
@@ -76,7 +90,8 @@ static int cmd_precheck(const struct shell *shell,
 			bool arg_cnt_ok)
 {
 	if (!arg_cnt_ok) {
-		shell_fprintf(shell, SHELL_ERROR, "%s: wrong parameter count\n",
+		shell_fprintf(shell, SHELL_ERROR,
+			      "%s: wrong parameter count\n",
 			      shell->ctx->active_cmd.syntax);
 
 		if (IS_ENABLED(CONFIG_SHELL_HELP_ON_WRONG_ARGUMENT_COUNT)) {
@@ -697,7 +712,7 @@ static int execute(const struct shell *shell)
 						 * calls.
 						 */
 						shell_fprintf(shell,
-							      SHELL_ERROR,
+							SHELL_ERROR,
 							"Error: requested"
 							" multiple function"
 							" executions\n");
@@ -1059,10 +1074,6 @@ static void shell_log_process(const struct shell *shell)
 	int result;
 
 	do {
-		if (shell->ctx->state < SHELL_STATE_PANIC_MODE_ACTIVE) {
-			k_mutex_lock(&shell->ctx->wr_mtx, K_FOREVER);
-		}
-
 		shell_cmd_line_erase(shell);
 
 		processed = shell_log_backend_process(shell->log_backend);
@@ -1081,9 +1092,6 @@ static void shell_log_process(const struct shell *shell)
 
 		k_poll_signal_check(signal, &signaled, &result);
 
-		if (shell->ctx->state < SHELL_STATE_PANIC_MODE_ACTIVE) {
-			k_mutex_unlock(&shell->ctx->wr_mtx);
-		}
 	} while (processed && !signaled);
 }
 
@@ -1204,23 +1212,29 @@ void shell_thread(void *shell_handle, void *arg_log_backend,
 	}
 
 	while (true) {
-		if (shell->iface->api->update) {
-			shell->iface->api->update(shell->iface);
-		}
+		err = k_poll(shell->ctx->events, SHELL_SIGNAL_TXDONE,
+			     K_FOREVER);
+
+		k_mutex_lock(&shell->ctx->wr_mtx, K_FOREVER);
 
-		err = k_poll(shell->ctx->events, SHELL_SIGNALS, K_FOREVER);
 		if (err != 0) {
 			shell_error(shell, "Shell thread error: %d", err);
 			return;
 		}
 
+
+		if (shell->iface->api->update) {
+			shell->iface->api->update(shell->iface);
+		}
+
 		shell_signal_handle(shell, SHELL_SIGNAL_KILL, kill_handler);
 		shell_signal_handle(shell, SHELL_SIGNAL_RXRDY, shell_process);
-		shell_signal_handle(shell, SHELL_SIGNAL_TXDONE, shell_process);
 		if (IS_ENABLED(CONFIG_LOG)) {
 			shell_signal_handle(shell, SHELL_SIGNAL_LOG_MSG,
 					    shell_log_process);
 		}
+
+		k_mutex_unlock(&shell->ctx->wr_mtx);
 	}
 }
 
@@ -1339,24 +1353,25 @@ void shell_process(const struct shell *shell)
 			 internal.value);
 }
 
+
+
 void shell_fprintf(const struct shell *shell, enum shell_vt100_color color,
-		   const char *p_fmt, ...)
+		   const char *fmt, ...)
 {
 	__ASSERT_NO_MSG(shell);
 	__ASSERT(!k_is_in_isr(), "Thread context required.");
 	__ASSERT_NO_MSG(shell->ctx);
 	__ASSERT_NO_MSG(shell->fprintf_ctx);
-	__ASSERT_NO_MSG(p_fmt);
+	__ASSERT_NO_MSG(fmt);
 
 	va_list args = { 0 };
 
 	if (k_current_get() != shell->ctx->tid) {
-		return;
+		k_mutex_lock(&shell->ctx->wr_mtx, K_FOREVER);
+		shell_cmd_line_erase(shell);
 	}
 
-	va_start(args, p_fmt);
-
-	k_mutex_lock(&shell->ctx->wr_mtx, K_FOREVER);
+	va_start(args, fmt);
 
 	if (IS_ENABLED(CONFIG_SHELL_VT100_COLORS) &&
 	    shell->ctx->internal.flags.use_colors &&
@@ -1366,16 +1381,20 @@ void shell_fprintf(const struct shell *shell, enum shell_vt100_color color,
 		shell_vt100_colors_store(shell, &col);
 		shell_vt100_color_set(shell, color);
 
-		shell_fprintf_fmt(shell->fprintf_ctx, p_fmt, args);
+		shell_fprintf_fmt(shell->fprintf_ctx, fmt, args);
 
 		shell_vt100_colors_restore(shell, &col);
 	} else {
-		shell_fprintf_fmt(shell->fprintf_ctx, p_fmt, args);
+		shell_fprintf_fmt(shell->fprintf_ctx, fmt, args);
 	}
 
 	va_end(args);
 
-	k_mutex_unlock(&shell->ctx->wr_mtx);
+	if (k_current_get() != shell->ctx->tid) {
+		cmd_line_print(shell);
+		transport_buffer_flush(shell);
+		k_mutex_unlock(&shell->ctx->wr_mtx);
+	}
 }
 
 int shell_prompt_change(const struct shell *shell, char *prompt)

subsys/shell/shell_cmds.c

@@ -70,7 +70,7 @@ static int cursor_position_get(const struct shell *shell, u16_t *x, u16_t *y)
 	/* fprintf buffer needs to be flushed to start sending prepared
 	 * escape code to the terminal.
 	 */
-	shell_fprintf_buffer_flush(shell->fprintf_ctx);
+	transport_buffer_flush(shell);
 
 	/* timeout for terminal response = ~1s */
 	for (u16_t i = 0; i < 1000; i++) {

subsys/shell/shell_help.c

@@ -8,6 +8,7 @@
 #include "shell_help.h"
 #include "shell_utils.h"
 
+
 /* Function prints a string on terminal screen with requested margin.
  * It takes care to not divide words.
  *   shell		Pointer to shell instance.

subsys/shell/shell_ops.c

@@ -11,8 +11,8 @@ void shell_op_cursor_vert_move(const struct shell *shell, s32_t delta)
 {
 	if (delta != 0) {
 		shell_raw_fprintf(shell->fprintf_ctx, "\033[%d%c",
-				delta > 0 ? delta : -delta,
-				delta > 0 ? 'A' : 'B');
+				  delta > 0 ? delta : -delta,
+				  delta > 0 ? 'A' : 'B');
 	}
 }
 
@@ -20,8 +20,8 @@ void shell_op_cursor_horiz_move(const struct shell *shell, s32_t delta)
 {
 	if (delta != 0) {
 		shell_raw_fprintf(shell->fprintf_ctx, "\033[%d%c",
-				delta > 0 ? delta : -delta,
-				delta > 0 ? 'C' : 'D');
+				  delta > 0 ? delta : -delta,
+				  delta > 0 ? 'C' : 'D');
 	}
 }
 
@@ -224,7 +224,7 @@ static void reprint_from_cursor(const struct shell *shell, u16_t diff,
 	}
 
 	shell_fprintf(shell, SHELL_NORMAL, "%s",
-			&shell->ctx->cmd_buff[shell->ctx->cmd_buff_pos]);
+		      &shell->ctx->cmd_buff[shell->ctx->cmd_buff_pos]);
 	shell->ctx->cmd_buff_pos = shell->ctx->cmd_buff_len;
 
 	if (full_line_cmd(shell)) {
@@ -327,7 +327,8 @@ void shell_cmd_line_erase(const struct shell *shell)
 	shell_multiline_data_calc(&shell->ctx->vt100_ctx.cons,
 				  shell->ctx->cmd_buff_pos,
 				  shell->ctx->cmd_buff_len);
-	shell_op_cursor_horiz_move(shell, -shell->ctx->vt100_ctx.cons.cur_x);
+	shell_op_cursor_horiz_move(shell,
+				   -(shell->ctx->vt100_ctx.cons.cur_x - 1));
 	shell_op_cursor_vert_move(shell, shell->ctx->vt100_ctx.cons.cur_y - 1);
 
 	clear_eos(shell);

subsys/shell/shell_uart.c

@@ -91,12 +91,12 @@ static void uart_rx_handle(const struct shell_uart *sh_uart)
 
 static void uart_tx_handle(const struct shell_uart *sh_uart)
 {
-	u32_t len;
-	u8_t *data;
-	int err;
 	struct device *dev = sh_uart->ctrl_blk->dev;
+	u32_t len;
+	int err;
+	const u8_t *data;
 
-	len = ring_buf_get_claim(sh_uart->tx_ringbuf, &data,
+	len = ring_buf_get_claim(sh_uart->tx_ringbuf, (u8_t **)&data,
 				 sh_uart->tx_ringbuf->size);
 	if (len) {
 		len = uart_fifo_fill(dev, data, len);

subsys/shell/shell_utils.c

@@ -378,3 +378,4 @@ void shell_cmd_trim(const struct shell *shell)
 	buffer_trim(shell->ctx->cmd_buff, &shell->ctx->cmd_buff_len);
 	shell->ctx->cmd_buff_pos = shell->ctx->cmd_buff_len;
 }
+

subsys/shell/shell_wildcard.c

@@ -9,6 +9,7 @@
 #include "shell_wildcard.h"
 #include "shell_utils.h"
 
+
 static void subcmd_get(const struct shell_cmd_entry *cmd,
 		       size_t idx, const struct shell_static_entry **entry,
 		       struct shell_static_entry *d_entry)