mgmt: mcumgr: transport: smp_bt: Fix wrongly enabling Bluetooth

Bluetooth does not need to be enabled to register services,
therefore the newly introduced automatic bluetooth SMP transport
registration system can be simplified by returning enabling of
bluetooth back to the application.

Signed-off-by: Jamie McCrae <[email protected]>

Author: nordicjm

include/zephyr/mgmt/mcumgr/transport/smp_bt.h

@@ -33,14 +33,6 @@ int smp_bt_register(void);
  */
 int smp_bt_unregister(void);
 
-/**
- * @brief	Sets up the Bluetooth SMP (MCUmgr) transport. This should be called if the Kconfig
- *		option ``CONFIG_MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT`` is not enabled, this will
- *		register the transport and add it to the Bluetooth registered services so that
- *		clients can access registered MCUmgr commands.
- */
-void smp_bt_start(void);
-
 /**
  * @brief	Transmits an SMP command/response over the specified Bluetooth connection as a
  *		notification.

samples/subsys/mgmt/mcumgr/smp_svr/src/bluetooth.c

@@ -58,9 +58,23 @@ BT_CONN_CB_DEFINE(conn_callbacks) = {
 	.disconnected = disconnected,
 };
 
+static void bt_ready(int err)
+{
+	if (err != 0) {
+		LOG_ERR("Bluetooth failed to initialise: %d", err);
+	} else {
+		k_work_submit(&advertise_work);
+	}
+}
+
 void start_smp_bluetooth_adverts(void)
 {
+	int rc;
+
 	k_work_init(&advertise_work, advertise);
+	rc = bt_enable(bt_ready);
 
-	k_work_submit(&advertise_work);
+	if (rc != 0) {
+		LOG_ERR("Bluetooth enable failed: %d", rc);
+	}
 }

samples/subsys/mgmt/mcumgr/smp_svr/src/main.c

@@ -66,6 +66,7 @@ void main(void)
 		LOG_ERR("Error mounting littlefs [%d]", rc);
 	}
 #endif
+
 #ifdef CONFIG_MCUMGR_TRANSPORT_BT
 	start_smp_bluetooth_adverts();
 #endif

subsys/mgmt/mcumgr/transport/Kconfig.bluetooth

@@ -93,24 +93,4 @@ config MCUMGR_TRANSPORT_BT_CONN_PARAM_CONTROL_RETRY_TIME
 
 endif # MCUMGR_TRASNPORT_BT_CONN_PARAM_CONTROL
 
-config MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT
-	bool "Bluetooth SMP stack enable autostart/setup"
-	default y
-	help
-	  If this option is enabled, then the Bluetooth stack will be enabled
-	  when the Bluetooth SMP (MCUmgr) transport initialises. If another
-	  system or the application does this, then this option should be
-	  disabled. Note that Bluetooth will need to be enabled prior to the
-	  Bluetooth SMP (MCUmgr) transport being initialised (by calling
-	  ``smp_bt_start()``) if this option is disabled.
-
-config MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT_WAIT
-	bool "Bluetooth SMP wait for autostart/setup completion"
-	default y
-	depends on MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT
-	help
-	  If this option is enabled, then the setup handler will wait for the
-	  Bluetooth stack to become ready from the ``bt_enable()`` call,
-	  otherwise will return instantly without waiting.
-
 endif # MCUMGR_TRANSPORT_BT

subsys/mgmt/mcumgr/transport/src/smp_bt.c

@@ -18,15 +18,12 @@
 #include <zephyr/mgmt/mcumgr/smp/smp.h>
 #include <zephyr/mgmt/mcumgr/transport/smp.h>
 #include <zephyr/mgmt/mcumgr/transport/smp_bt.h>
+#include <zephyr/mgmt/mcumgr/mgmt/handlers.h>
 #include <errno.h>
 
 #include <mgmt/mcumgr/transport/smp_internal.h>
 #include <mgmt/mcumgr/transport/smp_reassembly.h>
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT
-#include <zephyr/mgmt/mcumgr/mgmt/handlers.h>
-#endif
-
 #include <zephyr/logging/log.h>
 LOG_MODULE_DECLARE(mcumgr_smp, CONFIG_MCUMGR_TRANSPORT_LOG_LEVEL);
 
@@ -91,10 +88,6 @@ static uint8_t next_id;
 static struct smp_transport smp_bt_transport;
 static struct conn_param_data conn_data[CONFIG_BT_MAX_CONN];
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT
-static K_SEM_DEFINE(bt_ready_sem, 0, 1);
-#endif
-
 /* SMP service.
  * {8D53DC1D-1DB7-4CD3-868B-8A527460AA84}
  */
@@ -107,6 +100,15 @@ static struct bt_uuid_128 smp_bt_svc_uuid = BT_UUID_INIT_128(
 static struct bt_uuid_128 smp_bt_chr_uuid = BT_UUID_INIT_128(
 	BT_UUID_128_ENCODE(0xda2e7828, 0xfbce, 0x4e01, 0xae9e, 0x261174997c48));
 
+static void connected(struct bt_conn *conn, uint8_t err);
+static void disconnected(struct bt_conn *conn, uint8_t reason);
+
+/* Bluetooth connection callback handlers */
+BT_CONN_CB_DEFINE(mcumgr_bt_callbacks) = {
+	.connected = connected,
+	.disconnected = disconnected,
+};
+
 /* Helper function that allocates conn_param_data for a conn. */
 static struct conn_param_data *conn_param_data_alloc(struct bt_conn *conn)
 {
@@ -635,35 +637,13 @@ static bool smp_bt_query_valid_check(struct net_buf *nb, void *arg)
 	return true;
 }
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT_WAIT
-static void bt_ready(int err)
-{
-	if (err) {
-		LOG_ERR("Bluetooth init failed (err %d)", err);
-		return;
-	}
-
-	LOG_INF("Bluetooth initialised");
-
-	k_sem_give(&bt_ready_sem);
-}
-#endif
-
-void smp_bt_start(void)
+static void smp_bt_setup(void)
 {
 	int rc;
 	uint8_t i = 0;
 
 	next_id = 1;
 
-	/* Register BT callbacks */
-	static struct bt_conn_cb conn_callbacks = {
-		.connected = connected,
-		.disconnected = disconnected,
-	};
-
-	bt_conn_cb_register(&conn_callbacks);
-
 	if (IS_ENABLED(CONFIG_MCUMGR_TRANSPORT_BT_CONN_PARAM_CONTROL)) {
 		conn_param_control_init();
 	}
@@ -684,26 +664,4 @@ void smp_bt_start(void)
 	}
 }
 
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT
-static void smp_bt_setup(void)
-{
-	/* Enable Bluetooth */
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT_WAIT
-	int rc = bt_enable(bt_ready);
-#else
-	int rc = bt_enable(NULL);
-#endif
-
-	if (rc != 0) {
-		LOG_ERR("Bluetooth init failed (err %d)", rc);
-	} else {
-#ifdef CONFIG_MCUMGR_TRANSPORT_BT_AUTOMATIC_INIT_WAIT
-		k_sem_take(&bt_ready_sem, K_FOREVER);
-#endif
-	}
-
-	smp_bt_start();
-}
-
 MCUMGR_HANDLER_DEFINE(smp_bt, smp_bt_setup);
-#endif