smp: shell: Add support for SMP in new shell.

Added smp support for new shell.

Signed-off-by: Mieszko Mierunski <mieszko.mierunski@nordicsemi.no>

include/mgmt/smp_shell.h

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2019 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/** @file
+ * @brief Shell transport for the mcumgr SMP protocol.
+ */
+
+#ifndef ZEPHYR_INCLUDE_MGMT_SMP_SHELL_H_
+#define ZEPHYR_INCLUDE_MGMT_SMP_SHELL_H_
+
+#include <zephyr/types.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** @brief Data used by SMP shell */
+struct smp_shell_data {
+	char mcumgr_buff[128];
+	bool cmd_rdy;
+	atomic_t esc_state;
+	u32_t cur;
+	u32_t end;
+};
+
+/**
+ * @brief Attempts to process a received byte as part of an SMP frame.
+ *
+ * This function should be called with every received byte.
+ *
+ * @param data SMP shell transfer data.
+ * @param byte The byte just received.
+ *
+ * @return true if the command being received is an mcumgr frame; false if it
+ *		is a plain shell command.
+ */
+bool smp_shell_rx_byte(struct smp_shell_data *data, uint8_t byte);
+
+/**
+ * @brief Processes SMP data and executes command if full frame was received.
+ *
+ * This function should be called from thread context.
+ *
+ * @param data SMP shell transfer data.
+ */
+void smp_shell_process(struct smp_shell_data *data);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif

include/shell/shell.h

@@ -330,6 +330,15 @@ struct shell_transport_api {
 	int (*read)(const struct shell_transport *transport,
 		    void *data, size_t length, size_t *cnt);
 
+	/**
+	 * @brief Function called in shell thread loop.
+	 *
+	 * Can be used for backend operations that require longer execution time
+	 *
+	 * @param[in] transport Pointer to the transfer instance.
+	 */
+	void (*update)(const struct shell_transport *transport);
+
 };
 
 struct shell_transport {

include/shell/shell_uart.h

@@ -10,6 +10,9 @@
 #include <shell/shell.h>
 #include <ring_buffer.h>
 #include <atomic.h>
+#ifdef CONFIG_MCUMGR_SMP_SHELL
+#include "mgmt/smp_shell.h"
+#endif
 
 #ifdef __cplusplus
 extern "C" {
@@ -24,6 +27,9 @@ struct shell_uart_ctrl_blk {
 	void *context;
 	atomic_t tx_busy;
 	bool blocking;
+#ifdef CONFIG_MCUMGR_SMP_SHELL
+	struct smp_shell_data smp;
+#endif /* CONFIG_MCUMGR_SMP_SHELL */
 };
 
 #ifdef CONFIG_SHELL_BACKEND_SERIAL_INTERRUPT_DRIVEN

subsys/mgmt/Kconfig

@@ -19,8 +19,8 @@ config MCUMGR_SMP_BT
 config MCUMGR_SMP_SHELL
 	bool "Shell mcumgr SMP transport"
 	select MCUMGR
-	select UART_CONSOLE_MCUMGR
-	select CONSOLE_SHELL
+	select SHELL
+	select SHELL_BACKEND_SERIAL
 	select BASE64
 	help
 	  Enables handling of SMP commands received over shell.  This allows

subsys/mgmt/smp_shell.c

@@ -17,30 +17,120 @@
 #include "mgmt/serial.h"
 #include "mgmt/buf.h"
 #include "mgmt/smp.h"
-
-struct device;
+#include "mgmt/smp_shell.h"
 
 static struct zephyr_smp_transport smp_shell_transport;
 
 static struct mcumgr_serial_rx_ctxt smp_shell_rx_ctxt;
 
-/**
- * Processes a single line (i.e., a single SMP frame)
- */
-static int smp_shell_rx_line(const char *line, void *arg)
+/** SMP mcumgr frame fragments. */
+enum smp_shell_esc_mcumgr {
+	ESC_MCUMGR_PKT_1,
+	ESC_MCUMGR_PKT_2,
+	ESC_MCUMGR_FRAG_1,
+	ESC_MCUMGR_FRAG_2,
+};
+
+/** These states indicate whether an mcumgr frame is being received. */
+enum smp_shell_mcumgr_state {
+	SMP_SHELL_MCUMGR_STATE_NONE,
+	SMP_SHELL_MCUMGR_STATE_HEADER,
+	SMP_SHELL_MCUMGR_STATE_PAYLOAD
+};
+
+static int read_mcumgr_byte(struct smp_shell_data *data, u8_t byte)
 {
-	struct net_buf *nb;
-	int line_len;
+	bool frag_1;
+	bool frag_2;
+	bool pkt_1;
+	bool pkt_2;
 
-	/* Strip the trailing newline. */
-	line_len = strlen(line) - 1;
+	pkt_1 = atomic_test_bit(&data->esc_state, ESC_MCUMGR_PKT_1);
+	pkt_2 = atomic_test_bit(&data->esc_state, ESC_MCUMGR_PKT_2);
+	frag_1 = atomic_test_bit(&data->esc_state, ESC_MCUMGR_FRAG_1);
+	frag_2 = atomic_test_bit(&data->esc_state, ESC_MCUMGR_FRAG_2);
 
-	nb = mcumgr_serial_process_frag(&smp_shell_rx_ctxt, line, line_len);
-	if (nb != NULL) {
-		zephyr_smp_rx_req(&smp_shell_transport, nb);
+	if (pkt_2 || frag_2) {
+		/* Already fully framed. */
+		return SMP_SHELL_MCUMGR_STATE_PAYLOAD;
 	}
 
-	return 0;
+	if (pkt_1) {
+		if (byte == MCUMGR_SERIAL_HDR_PKT_2) {
+			/* Final framing byte received. */
+			atomic_set_bit(&data->esc_state, ESC_MCUMGR_PKT_2);
+			return SMP_SHELL_MCUMGR_STATE_PAYLOAD;
+		}
+	} else if (frag_1) {
+		if (byte == MCUMGR_SERIAL_HDR_FRAG_2) {
+			/* Final framing byte received. */
+			atomic_set_bit(&data->esc_state, ESC_MCUMGR_FRAG_2);
+			return SMP_SHELL_MCUMGR_STATE_PAYLOAD;
+		}
+	} else {
+		if (byte == MCUMGR_SERIAL_HDR_PKT_1) {
+			/* First framing byte received. */
+			atomic_set_bit(&data->esc_state, ESC_MCUMGR_PKT_1);
+			return SMP_SHELL_MCUMGR_STATE_HEADER;
+		} else if (byte == MCUMGR_SERIAL_HDR_FRAG_1) {
+			/* First framing byte received. */
+			atomic_set_bit(&data->esc_state, ESC_MCUMGR_FRAG_1);
+			return SMP_SHELL_MCUMGR_STATE_HEADER;
+		}
+	}
+
+	/* Non-mcumgr byte received. */
+	return SMP_SHELL_MCUMGR_STATE_NONE;
+}
+
+bool smp_shell_rx_byte(struct smp_shell_data *data, uint8_t byte)
+{
+	int mcumgr_state;
+
+	mcumgr_state = read_mcumgr_byte(data, byte);
+	if (mcumgr_state == SMP_SHELL_MCUMGR_STATE_NONE) {
+		/* Not an mcumgr command; let the shell process the byte. */
+		return false;
+	}
+
+	/*
+	 * The received byte is part of an mcumgr command.  Process the byte
+	 * and return true to indicate that shell should ignore it.
+	 */
+	if (data->cur + data->end < sizeof(data->mcumgr_buff) - 1) {
+		data->mcumgr_buff[data->cur++] = byte;
+	}
+	if (mcumgr_state == SMP_SHELL_MCUMGR_STATE_PAYLOAD && byte == '\n') {
+		data->mcumgr_buff[data->cur + data->end] = '\0';
+		data->cmd_rdy = true;
+		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_1);
+		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_PKT_2);
+		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_1);
+		atomic_clear_bit(&data->esc_state, ESC_MCUMGR_FRAG_2);
+		data->cur = 0U;
+		data->end = 0U;
+	}
+
+	return true;
+}
+
+void smp_shell_process(struct smp_shell_data *data)
+{
+	if (data->cmd_rdy) {
+		data->cmd_rdy = false;
+		struct net_buf *nb;
+		int line_len;
+
+		/* Strip the trailing newline. */
+		line_len = strlen(data->mcumgr_buff) - 1;
+
+		nb = mcumgr_serial_process_frag(&smp_shell_rx_ctxt,
+						data->mcumgr_buff,
+						line_len);
+		if (nb != NULL) {
+			zephyr_smp_rx_req(&smp_shell_transport, nb);
+		}
+	}
 }
 
 static u16_t smp_shell_get_mtu(const struct net_buf *nb)
@@ -72,7 +162,6 @@ static int smp_shell_init(struct device *dev)
 
 	zephyr_smp_transport_init(&smp_shell_transport, smp_shell_tx_pkt,
 				  smp_shell_get_mtu, NULL, NULL);
-	shell_register_mcumgr_handler(smp_shell_rx_line, NULL);
 
 	return 0;
 }

subsys/shell/shell.c

@@ -1216,6 +1216,9 @@ void shell_thread(void *shell_handle, void *arg_log_backend,
 	}
 
 	while (true) {
+		if (shell->iface->api->update) {
+			shell->iface->api->update(shell->iface);
+		}
 		int num_events = (shell->ctx->state != SHELL_STATE_COMMAND) ?
 				SHELL_SIGNALS : SHELL_SIGNAL_TXDONE;
 

subsys/shell/shell_uart.c

@@ -41,11 +41,30 @@ static void uart_rx_handle(const struct shell_uart *sh_uart)
 		if (len) {
 			rd_len = uart_fifo_read(sh_uart->ctrl_blk->dev,
 						data, len);
+#ifdef CONFIG_MCUMGR_SMP_SHELL
+			/* Divert bytes from shell handling if it is
+			 * part of an mcumgr frame.
+			 */
+			size_t i;
 
+			for (i = 0; i < rd_len; i++) {
+				if (!smp_shell_rx_byte(&sh_uart->ctrl_blk->smp,
+						       data[i])) {
+					break;
+				}
+			}
+			rd_len -= i;
+			if (rd_len) {
+				new_data = true;
+				for (u32_t j = 0; j < rd_len; j++) {
+					data[j] = data[i + j];
+				}
+			}
+#else
 			if (rd_len) {
 				new_data = true;
 			}
-
+#endif /* CONFIG_MCUMGR_SMP_SHELL */
 			ring_buf_put_finish(sh_uart->rx_ringbuf, rd_len);
 		} else {
 			u8_t dummy;
@@ -55,6 +74,12 @@ static void uart_rx_handle(const struct shell_uart *sh_uart)
 
 			rd_len = uart_fifo_read(sh_uart->ctrl_blk->dev,
 						&dummy, 1);
+#ifdef CONFIG_MCUMGR_SMP_SHELL
+			/* Divert this byte from shell handling if it
+			 * is part of an mcumgr frame.
+			 */
+			smp_shell_rx_byte(&sh_uart->ctrl_blk->smp, dummy);
+#endif /* CONFIG_MCUMGR_SMP_SHELL */
 		}
 	} while (rd_len && (rd_len == len));
 
@@ -219,12 +244,24 @@ static int read(const struct shell_transport *transport,
 	return 0;
 }
 
+#ifdef CONFIG_MCUMGR_SMP_SHELL
+static void update(const struct shell_transport *transport)
+{
+	struct shell_uart *sh_uart = (struct shell_uart *)transport->ctx;
+
+	smp_shell_process(&sh_uart->ctrl_blk->smp);
+}
+#endif /* CONFIG_MCUMGR_SMP_SHELL */
+
 const struct shell_transport_api shell_uart_transport_api = {
 	.init = init,
 	.uninit = uninit,
 	.enable = enable,
 	.write = write,
-	.read = read
+	.read = read,
+#ifdef CONFIG_MCUMGR_SMP_SHELL
+	.update = update,
+#endif /* CONFIG_MCUMGR_SMP_SHELL */
 };
 
 static int enable_shell_uart(struct device *arg)