ble: apply changes suggested in review

code style fixes

Signed-off-by: Robert Gałat <[email protected]>

subsys/sal/common/CMakeLists.txt

@@ -9,5 +9,3 @@ zephyr_include_directories(sid_pal_ifc)
 zephyr_include_directories(sid_time_ops)
 
 add_subdirectory_ifdef(CONFIG_SIDEWALK_ON_DEV_CERT sid_on_dev_cert)
-
-zephyr_library_sources(sid_ifc/bt_app_callbacks.c)

subsys/sal/common/sid_ifc/bt_app_callbacks.h → subsys/sal/sid_pal/include/bt_app_callbacks.h

@@ -10,7 +10,7 @@
 #include <stdbool.h>
 #include <zephyr/bluetooth/bluetooth.h>
 
-#if defined (CONFIG_BT_APP_IFC)
+#if defined(CONFIG_BT_APP_IFC)
 
 /**
  * @brief Wrapper for @bt_enable, with reference tracking.
@@ -19,32 +19,31 @@
  * @param cb callback passed to @bt_enable
  * @return int result from @bt_enable call or 0 if called multiple times
  */
-int app_bt_enable(bt_ready_cb_t cb);
-
+int sid_ble_bt_enable(bt_ready_cb_t cb);
 
 /**
  * @brief Wrapper for @bt_disable.
  * This function removes internal reference.
  * If the internal reference counter shows 0, real @bt_disable is called
  * 
- * @return int result from @bt_disable or 0 if app_bt_enable has been called more than app_bt_disable
+ * @return int result from @bt_disable or 0 if sid_ble_bt_enable has been called more than sid_ble_bt_disable
  */
-int app_bt_disable();
-
+int sid_ble_bt_disable();
 
 /**
  * @brief BT ids used for extended advertising. 
  * This allows to identify connections from different extended adverticements.
  */
-enum BT_id_values{
-        _BT_ID_DEFAULT = BT_ID_DEFAULT,
-        BT_ID_SIDEWALK,
-        #if defined (CONFIG_SIDEWALK_DFU)
-        BT_ID_SMP_DFU,
-        #endif
-        _BT_ID_MAX
+enum sid_ble_id_values {
+	_BT_ID_DEFAULT = BT_ID_DEFAULT,
+	BT_ID_SIDEWALK,
+#if defined(CONFIG_SIDEWALK_DFU)
+	BT_ID_SMP_DFU,
+#endif
+	_BT_ID_MAX
 };
 
-BUILD_ASSERT(_BT_ID_MAX <= CONFIG_BT_ID_MAX, "Too many BT Ids! increase CONFIG_BT_ID_MAX, to match _BT_ID_MAX");
+BUILD_ASSERT(_BT_ID_MAX <= CONFIG_BT_ID_MAX,
+	     "Too many BT Ids! increase CONFIG_BT_ID_MAX, to match _BT_ID_MAX");
 #endif
 #endif

subsys/sal/sid_pal/src/CMakeLists.txt

@@ -72,3 +72,5 @@ endif() # CONFIG_SOC_NRF52840
 zephyr_library_sources_ifdef(CONFIG_SIDEWALK sid_common.c)
 
 zephyr_library_sources_ifdef(DEPRECATED_SIDEWALK_PAL_INIT pal_init.c)
+
+zephyr_library_sources(bt_app_callbacks.c)

subsys/sal/common/sid_ifc/bt_app_callbacks.c → subsys/sal/sid_pal/src/bt_app_callbacks.c

@@ -10,7 +10,7 @@
 
 static uint32_t bt_enable_count = 0;
 
-int app_bt_enable(bt_ready_cb_t cb)
+int sid_ble_bt_enable(bt_ready_cb_t cb)
 {
 	if (bt_enable_count == 0) {
 		int ret = bt_enable(cb);
@@ -27,7 +27,7 @@ int app_bt_enable(bt_ready_cb_t cb)
 	return 0;
 }
 
-int app_bt_disable()
+int sid_ble_bt_disable()
 {
 	if (bt_enable_count <= 0) {
 		bt_enable_count = 0;

subsys/sal/sid_pal/src/sid_ble_adapter.c

@@ -242,7 +242,7 @@ static sid_error_t ble_adapter_init(const sid_ble_config_t *cfg)
 
 	LOG_INF("Enable BT");
 	int err_code;
-	err_code = app_bt_enable(NULL);
+	err_code = sid_ble_bt_enable(NULL);
 	switch (err_code) {
 	case -EALREADY:
 	case 0:
@@ -441,7 +441,7 @@ static sid_error_t ble_adapter_deinit(void)
 	sid_ble_advert_deinit();
 	bt_id_delete(BT_ID_SIDEWALK);
 	bt_id_reset(BT_ID_SIDEWALK, NULL, NULL);
-	int err = app_bt_disable();
+	int err = sid_ble_bt_disable();
 
 	if (err) {
 		LOG_ERR("BT disable failed (error %d)", err);

subsys/sal/sid_pal/src/sid_ble_advert.c

@@ -22,9 +22,7 @@ LOG_MODULE_REGISTER(sid_ble_advert, CONFIG_SIDEWALK_BLE_ADAPTER_LOG_LEVEL);
 
 #define MS_TO_INTERVAL_VAL(ms) (uint16_t)((ms) / 0.625f)
 
-#define AMA_ADV_OPTIONS                                                                            \
-	(BT_LE_ADV_OPT_CONNECTABLE | BT_LE_ADV_OPT_USE_NAME | BT_LE_ADV_OPT_FORCE_NAME_IN_AD |     \
-	 BT_LE_ADV_OPT_ONE_TIME)
+#define AMA_ADV_OPTIONS (BT_LE_ADV_OPT_CONNECTABLE | BT_LE_ADV_OPT_ONE_TIME)
 
 #if 10240 < (CONFIG_SIDEWALK_BLE_ADV_INT_FAST + CONFIG_SIDEWALK_BLE_ADV_INT_PRECISION)
 #error "Invalid value for CONFIG_SIDEWALK_BLE_ADV_INT_FAST or CONFIG_SIDEWALK_BLE_ADV_INT_PRECISION, sum of those values have to be smaller than 10240"
@@ -70,7 +68,7 @@ static struct bt_le_ext_adv *adv_set_slow = NULL;
 	(BT_GAP_ADV_MAX_ADV_DATA_LEN - AD_TLV_TYPE_AND_LENGTH - AD_TLV_LEN(AD_FLAGS_LEN) -         \
 	 AD_TLV_LEN(AD_SERVICES_LEN) - AD_TLV_LEN(AD_NAME_SHORT_LEN))
 
-enum adv_data_items { ADV_DATA_FLAGS, ADV_DATA_SERVICES, ADV_DATA_MANUF_DATA };
+enum adv_data_items { ADV_DATA_FLAGS, ADV_DATA_SERVICES, ADV_DATA_MANUF_DATA, ADV_DATA_NAME };
 
 typedef enum { BLE_ADV_DISABLE, BLE_ADV_FAST, BLE_ADV_SLOW } sid_ble_adv_state_t;
 
@@ -86,7 +84,14 @@ static struct bt_data adv_data[] = {
 	[ADV_DATA_SERVICES] =
 		BT_DATA_BYTES(BT_DATA_UUID16_ALL, BT_UUID_16_ENCODE(AMA_SERVICE_UUID_VAL)),
 	[ADV_DATA_MANUF_DATA] =
-		BT_DATA(BT_DATA_MANUFACTURER_DATA, &bt_adv_manuf_data, AD_MANUF_DATA_LEN_MAX)
+		BT_DATA(BT_DATA_MANUFACTURER_DATA, &bt_adv_manuf_data, AD_MANUF_DATA_LEN_MAX),
+	[ADV_DATA_NAME] = BT_DATA(BT_DATA_NAME_COMPLETE, CONFIG_SIDEWALK_BLE_NAME,
+				  sizeof(CONFIG_SIDEWALK_BLE_NAME) - 1),
+};
+
+static const struct bt_data sd[] = {
+	BT_DATA(BT_DATA_NAME_COMPLETE, CONFIG_SIDEWALK_BLE_NAME,
+		sizeof(CONFIG_SIDEWALK_BLE_NAME) - 1),
 };
 
 /**
@@ -123,7 +128,8 @@ static void change_advertisement_interval(struct k_work *work)
 			LOG_ERR("Failed to stop fast adv errno %d (%s)", err, strerror(err));
 			return;
 		}
-		err = bt_le_ext_adv_set_data(adv_set_slow, adv_data, ARRAY_SIZE(adv_data), NULL, 0);
+		err = bt_le_ext_adv_set_data(adv_set_slow, adv_data, ARRAY_SIZE(adv_data), sd,
+					     ARRAY_SIZE(sd));
 		if (err) {
 			atomic_set(&adv_state, BLE_ADV_DISABLE);
 			LOG_ERR("Failed to set adv data to slow adv errno %d (%s)", err,
@@ -191,15 +197,16 @@ int sid_ble_advert_start(void)
 	struct bt_le_ext_adv_start_param ext_adv_start_param = { 0 };
 	int err = 0;
 
-	err = bt_le_ext_adv_set_data(adv_set_fast, adv_data, ARRAY_SIZE(adv_data), NULL, 0);
+	err = bt_le_ext_adv_set_data(adv_set_fast, adv_data, ARRAY_SIZE(adv_data), sd,
+				     ARRAY_SIZE(sd));
 	if (err) {
-		LOG_ERR("Failed to set adv data errno: %d (%s)", err, strerror(err));
+		LOG_ERR("Failed to set fast adv data errno: %d (%s)", err, strerror(err));
 		return err;
 	}
 
 	err = bt_le_ext_adv_start(adv_set_fast, &ext_adv_start_param);
 	if (err) {
-		LOG_ERR("Failed to start adv errno: %d (%s)", err, strerror(err));
+		LOG_ERR("Failed to start fast adv errno: %d (%s)", err, strerror(err));
 		return err;
 	}
 
@@ -236,7 +243,7 @@ int sid_ble_advert_update(uint8_t *data, uint8_t data_len)
 	if (BLE_ADV_DISABLE != state) {
 		/* Update currently advertised set, the other one will be set on start/transition */
 		err = bt_le_ext_adv_set_data((state == BLE_ADV_FAST ? adv_set_fast : adv_set_slow),
-					     adv_data, ARRAY_SIZE(adv_data), NULL, 0);
+					     adv_data, ARRAY_SIZE(adv_data), sd, ARRAY_SIZE(sd));
 	}
 
 	return err;

subsys/sal/sid_pal/src/sid_ble_connection.c

@@ -34,7 +34,14 @@ static struct bt_conn_cb conn_callbacks = {
 
 static struct bt_gatt_cb gatt_callbacks = { .att_mtu_updated = ble_mtu_cb };
 
-static bool should_handle_event(struct bt_conn *conn)
+/**
+ * @brief Check if the connection came from right adv, and if it is valid.
+ * 
+ * @param conn connection to check
+ * @return true if the connection should be handled by Sidewalk
+ * @return false connection is not for Sidewlak
+ */
+static bool is_connection_valid(struct bt_conn *conn)
 {
 	struct bt_conn_info conn_info = {};
 
@@ -53,9 +60,9 @@ static bool should_handle_event(struct bt_conn *conn)
  */
 static void ble_connect_cb(struct bt_conn *conn, uint8_t err)
 {
-	const bt_addr_le_t *bt_addr_le;
+	const bt_addr_le_t *bt_addr_le = NULL;
 
-	if (!should_handle_event(conn)) {
+	if (!is_connection_valid(conn)) {
 		return;
 	}
 
@@ -90,7 +97,7 @@ static void ble_connect_cb(struct bt_conn *conn, uint8_t err)
  */
 static void ble_disconnect_cb(struct bt_conn *conn, uint8_t reason)
 {
-	if (!should_handle_event(conn) || conn_params.conn != conn) {
+	if (!is_connection_valid(conn) || conn_params.conn != conn) {
 		return;
 	}
 

tests/unit_tests/pal_ble_adapter/Kconfig

@@ -37,4 +37,8 @@ config BT_APP_IFC
 config BT_ID_MAX
 	default 2
 
+config SIDEWALK_BLE_NAME
+	string "BLE name adverticed for Sidewalk"
+	default "SID_APP"
+
 source "Kconfig.zephyr"

tests/unit_tests/pal_ble_adapter/src/main.c

@@ -179,11 +179,11 @@ void test_sid_pal_ble_adapter_init(void)
 	__cmock_sid_ble_conn_init_Expect();
 	__cmock_sid_ble_advert_init_IgnoreAndReturn(0);
 	TEST_ASSERT_EQUAL(SID_ERROR_NONE, p_test_ble_ifc->init(&test_ble_cfg));
-	app_bt_disable();
+	sid_ble_bt_disable();
 
 	__cmock_sid_ble_conn_init_Expect();
 	TEST_ASSERT_EQUAL(SID_ERROR_NONE, p_test_ble_ifc->init(NULL));
-	app_bt_disable();
+	sid_ble_bt_disable();
 
 	bt_enable_fake.return_val = -ENOENT;
 	TEST_ASSERT_EQUAL(SID_ERROR_GENERIC, p_test_ble_ifc->init(&test_ble_cfg));

tests/unit_tests/sid_ble_advert/Kconfig

@@ -35,4 +35,9 @@ config SIDEWALK_BLE_ADV_INT_TRANSITION
 	int "test value for Sidewalk configuration macro"
 	default 30
 
+
+config SIDEWALK_BLE_NAME
+	string "BLE name adverticed for Sidewalk"
+	default "SID_APP"
+
 source "Kconfig.zephyr"

utils/sidewalk_dfu/nordic_dfu.c

@@ -105,12 +105,10 @@ static void exit_dfu_mode(struct k_timer *timer_id)
 
 static void connected(struct bt_conn *conn, uint8_t err)
 {
-	struct bt_conn_info cinfo;
-	int ec;
-
-	ec = bt_conn_get_info(conn, &cinfo);
-	if (ec) {
-		printk("Unable to get connection info (err %d)\n", ec);
+	struct bt_conn_info cinfo = {};
+	int ret = bt_conn_get_info(conn, &cinfo);
+	if (ret) {
+		printk("Unable to get connection info (err %d)\n", ret);
 		return;
 	}
 
@@ -124,7 +122,7 @@ static void connected(struct bt_conn *conn, uint8_t err)
 static void disconnected(struct bt_conn *conn, uint8_t reason)
 {
 	int err;
-	struct bt_conn_info cinfo;
+	struct bt_conn_info cinfo = {};
 
 	err = bt_conn_get_info(conn, &cinfo);
 	if (err) {
@@ -209,7 +207,7 @@ int nordic_dfu_ble_start(void)
 	application_state_dfu(&global_state_notifier, true);
 	dk_leds_init();
 
-	int err = app_bt_enable(NULL);
+	int err = sid_ble_bt_enable(NULL);
 	if (err && err != -EALREADY) {
 		LOG_ERR("Bluetooth enable failed (err %d %s)", err, strerror(err));
 		return err;
@@ -271,7 +269,7 @@ int nordic_dfu_ble_stop(void)
 	}
 	adv = NULL;
 
-	err = app_bt_disable();
+	err = sid_ble_bt_disable();
 	if (err) {
 		LOG_ERR("Bluetooth disable failed (err %d %s)", err, strerror(err));
 		return err;