diff --git a/subsys/sal/sid_pal/src/sid_ble_advert.c b/subsys/sal/sid_pal/src/sid_ble_advert.c index 1d11df2708..a6cc704e27 100644 --- a/subsys/sal/sid_pal/src/sid_ble_advert.c +++ b/subsys/sal/sid_pal/src/sid_ble_advert.c @@ -84,6 +84,20 @@ static struct bt_le_adv_param adv_param_slow = { typedef enum { BLE_ADV_DISABLE, BLE_ADV_FAST, BLE_ADV_SLOW } sid_ble_adv_state_t; static atomic_t adv_state = ATOMIC_INIT(BLE_ADV_DISABLE); +static const char *adv_state_str(sid_ble_adv_state_t state) +{ + switch (state) { + case BLE_ADV_DISABLE: + return "DISABLE"; + case BLE_ADV_FAST: + return "FAST"; + case BLE_ADV_SLOW: + return "SLOW"; + default: + return "UNKNOWN"; + } +} + static void adv_interval_change(struct k_work *); K_WORK_DELAYABLE_DEFINE(change_adv_work, adv_interval_change); @@ -184,7 +198,7 @@ static uint8_t adv_manuf_data_copy(uint8_t *data, uint8_t data_len) int sid_ble_advert_init(void) { - /* No initialization needed, left for backward compatibility */ + atomic_set(&adv_state, BLE_ADV_DISABLE); return 0; } @@ -234,20 +248,32 @@ int sid_ble_advert_update(uint8_t *data, uint8_t data_len) ad[ADV_DATA_MANUF_DATA].data_len = adv_manuf_data_copy(data, data_len); - int err = 0; + if (BLE_ADV_DISABLE == state) { + return 0; + } - 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(adv_set, ad, ARRAY_SIZE(ad), sd, ARRAY_SIZE(sd)); + if (adv_set == NULL) { + LOG_WRN("update: stale state=%s adv_set=NULL, cached mfg data only", + adv_state_str(state)); + atomic_set(&adv_state, BLE_ADV_DISABLE); + return 0; } - return err; + return bt_le_ext_adv_set_data(adv_set, ad, ARRAY_SIZE(ad), sd, ARRAY_SIZE(sd)); } void sid_ble_advert_notify_connection(void) { - LOG_DBG("Connection has been made, cancel change adv"); + sid_ble_adv_state_t state = atomic_get(&adv_state); + + LOG_DBG("Connection made, cancel change adv (state=%s adv_set=%p)", adv_state_str(state), + adv_set); k_work_cancel_delayable(&change_adv_work); + if (state != BLE_ADV_DISABLE) { + LOG_DBG("notify_connection: state %s -> DISABLE (adv stopped by stack)", + adv_state_str(state)); + atomic_set(&adv_state, BLE_ADV_DISABLE); + } } int sid_ble_advert_params_set(sid_ble_advert_params_t *params) @@ -293,14 +319,36 @@ int sid_ble_advert_stop(void) int sid_ble_advert_deinit(void) { + struct k_work_sync sync; + sid_ble_adv_state_t prev_state = atomic_get(&adv_state); + + (void)k_work_cancel_delayable_sync(&change_adv_work, &sync); + + if (prev_state != BLE_ADV_DISABLE || adv_set != NULL) { + LOG_INF("deinit: state=%s adv_set=%p", adv_state_str(prev_state), adv_set); + } + if (adv_set) { - int err = bt_le_ext_adv_delete(adv_set); + int err = bt_le_ext_adv_stop(adv_set); + if (err) { - LOG_ERR("Failed to delete adv_set_fast errno %d (%s)", err, strerror(err)); + LOG_DBG("deinit: bt_le_ext_adv_stop err %d (continuing to delete)", err); + } + err = bt_le_ext_adv_delete(adv_set); + + if (err) { + LOG_ERR("Failed to delete adv_set errno %d (%s)", err, strerror(err)); + adv_set = NULL; + atomic_set(&adv_state, BLE_ADV_DISABLE); return err; } adv_set = NULL; } + if (prev_state != BLE_ADV_DISABLE) { + LOG_WRN("deinit: reset stale adv_state %s -> DISABLE", adv_state_str(prev_state)); + } + atomic_set(&adv_state, BLE_ADV_DISABLE); + return 0; } diff --git a/tests/unit_tests/sid_ble_advert/src/main.c b/tests/unit_tests/sid_ble_advert/src/main.c index b26e5adc9e..d39baf95bd 100644 --- a/tests/unit_tests/sid_ble_advert/src/main.c +++ b/tests/unit_tests/sid_ble_advert/src/main.c @@ -54,12 +54,28 @@ static int bt_le_ext_adv_create_custom_fake(const struct bt_le_adv_param *a, return bt_le_ext_adv_create_fake.return_val; } +static void advert_start_with_fakes(void) +{ + bt_le_ext_adv_create_fake.custom_fake = bt_le_ext_adv_create_custom_fake; + bt_le_ext_adv_create_fake.return_val = 0; + bt_le_ext_adv_start_fake.return_val = ESUCCESS; + bt_le_ext_adv_set_data_fake.return_val = ESUCCESS; + bt_le_ext_adv_stop_fake.return_val = ESUCCESS; + bt_le_ext_adv_delete_fake.return_val = ESUCCESS; + zassert_equal(ESUCCESS, sid_ble_advert_start()); +} + static void before_test(void *fixture) { ARG_UNUSED(fixture); FFF_FAKES_LIST(RESET_FAKE); FFF_RESET_HISTORY(); + + bt_le_ext_adv_stop_fake.return_val = ESUCCESS; + bt_le_ext_adv_delete_fake.return_val = ESUCCESS; + (void)sid_ble_advert_deinit(); + FFF_RESET_HISTORY(); } ZTEST_SUITE(ble_advert, NULL, NULL, before_test, NULL, NULL); @@ -68,12 +84,13 @@ ZTEST(ble_advert, test_sid_ble_advert_start) { size_t adv_start_call_count = 0; - bt_le_ext_adv_start_fake.return_val = ESUCCESS; - zassert_equal(ESUCCESS, sid_ble_advert_start()); - adv_start_call_count++; - zassert_equal(adv_start_call_count, bt_le_ext_adv_start_fake.call_count); + advert_start_with_fakes(); + adv_start_call_count = bt_le_ext_adv_start_fake.call_count; + bt_le_ext_adv_create_fake.custom_fake = bt_le_ext_adv_create_custom_fake; + bt_le_ext_adv_create_fake.return_val = 0; bt_le_ext_adv_start_fake.return_val = -ENOENT; + bt_le_ext_adv_set_data_fake.return_val = ESUCCESS; zassert_equal(-ENOENT, sid_ble_advert_start()); adv_start_call_count++; zassert_equal(adv_start_call_count, bt_le_ext_adv_start_fake.call_count); @@ -108,10 +125,8 @@ ZTEST(ble_advert, test_sid_ble_advert_update) uint8_t test_data[] = "Lorem ipsum."; size_t adv_update_call_count = 0; - bt_le_ext_adv_start_fake.return_val = ESUCCESS; - bt_le_ext_adv_set_data_fake.return_val = ESUCCESS; - zassert_equal(ESUCCESS, sid_ble_advert_start()); - adv_update_call_count++; + advert_start_with_fakes(); + adv_update_call_count = bt_le_ext_adv_set_data_fake.call_count; zassert_equal(ESUCCESS, sid_ble_advert_update(test_data, sizeof(test_data))); adv_update_call_count++; zassert_equal(adv_update_call_count, bt_le_ext_adv_set_data_fake.call_count); @@ -134,7 +149,7 @@ ZTEST(ble_advert, test_sid_ble_advert_update_in_every_state) bt_le_ext_adv_start_fake.return_val = ESUCCESS; bt_le_ext_adv_set_data_fake.return_val = ESUCCESS; - zassert_equal(ESUCCESS, sid_ble_advert_start()); + advert_start_with_fakes(); zassert_equal(ESUCCESS, sid_ble_advert_update(test_data, sizeof(test_data))); bt_le_ext_adv_stop_fake.return_val = ESUCCESS; @@ -174,8 +189,7 @@ ZTEST(ble_advert, test_sid_ble_advert_update_before_start) bt_le_ext_adv_set_data_fake.return_val = ESUCCESS; zassert_equal(ESUCCESS, sid_ble_advert_update(test_data, sizeof(test_data))); - bt_le_ext_adv_start_fake.return_val = ESUCCESS; - zassert_equal(ESUCCESS, sid_ble_advert_start()); + advert_start_with_fakes(); advert_data = bt_le_ext_adv_set_data_fake.arg1_val; advert_data_size = bt_le_ext_adv_set_data_fake.arg2_val; @@ -199,7 +213,7 @@ static void check_sid_ble_advert_update(uint8_t *data, uint8_t data_len) bt_le_ext_adv_start_fake.return_val = ESUCCESS; bt_le_ext_adv_set_data_fake.return_val = ESUCCESS; - zassert_equal(ESUCCESS, sid_ble_advert_start()); + advert_start_with_fakes(); zassert_equal(ESUCCESS, sid_ble_advert_update(data, data_len)); advert_data = bt_le_ext_adv_set_data_fake.arg1_val; @@ -252,6 +266,99 @@ ZTEST(ble_advert, test_sid_ble_advert_params_set_get) ZTEST(ble_advert, test_sid_ble_advert_notify_connection) { + uint8_t test_data[] = "connected"; + size_t set_data_calls; + + advert_start_with_fakes(); + set_data_calls = bt_le_ext_adv_set_data_fake.call_count; + sid_ble_advert_notify_connection(); - /* No crash; implementation cancels delayed work. */ + + zassert_equal(ESUCCESS, sid_ble_advert_update(test_data, sizeof(test_data))); + zassert_equal(set_data_calls, bt_le_ext_adv_set_data_fake.call_count, + "update must not touch HCI after connection"); +} + +ZTEST(ble_advert, test_sid_ble_advert_deinit_idle) +{ + zassert_equal(ESUCCESS, sid_ble_advert_deinit()); + zassert_equal(0, bt_le_ext_adv_stop_fake.call_count); + zassert_equal(0, bt_le_ext_adv_delete_fake.call_count); +} + +ZTEST(ble_advert, test_sid_ble_advert_deinit_active) +{ + size_t stop_before; + size_t delete_before; + + advert_start_with_fakes(); + stop_before = bt_le_ext_adv_stop_fake.call_count; + delete_before = bt_le_ext_adv_delete_fake.call_count; + zassert_equal(ESUCCESS, sid_ble_advert_deinit()); + zassert_equal(stop_before + 1, bt_le_ext_adv_stop_fake.call_count, + "deinit must stop active advertising once"); + zassert_equal(delete_before + 1, bt_le_ext_adv_delete_fake.call_count, + "deinit must delete active advertising set once"); +} + +ZTEST(ble_advert, test_sid_ble_advert_deinit_stop_error) +{ + advert_start_with_fakes(); + bt_le_ext_adv_stop_fake.return_val = -EIO; + bt_le_ext_adv_delete_fake.return_val = ESUCCESS; + + zassert_equal(ESUCCESS, sid_ble_advert_deinit()); + zassert_equal(1, bt_le_ext_adv_stop_fake.call_count); + zassert_equal(1, bt_le_ext_adv_delete_fake.call_count); +} + +ZTEST(ble_advert, test_sid_ble_advert_deinit_delete_fails) +{ + advert_start_with_fakes(); + bt_le_ext_adv_delete_fake.return_val = -EINVAL; + + zassert_equal(-EINVAL, sid_ble_advert_deinit()); + zassert_equal(1, bt_le_ext_adv_stop_fake.call_count); + zassert_equal(1, bt_le_ext_adv_delete_fake.call_count); +} + +ZTEST(ble_advert, test_sid_ble_advert_connected_deinit_restart) +{ + uint8_t test_data[] = "reinit"; + size_t set_data_calls; + + advert_start_with_fakes(); + sid_ble_advert_notify_connection(); + + zassert_equal(ESUCCESS, sid_ble_advert_deinit()); + zassert_equal(1, bt_le_ext_adv_stop_fake.call_count); + zassert_equal(1, bt_le_ext_adv_delete_fake.call_count); + + set_data_calls = bt_le_ext_adv_set_data_fake.call_count; + zassert_equal(ESUCCESS, sid_ble_advert_update(test_data, sizeof(test_data))); + zassert_equal(set_data_calls, bt_le_ext_adv_set_data_fake.call_count, + "update must cache mfg data only before restart"); + + advert_start_with_fakes(); +} + +ZTEST(ble_advert, test_sid_ble_advert_stop_fail_deinit_restart) +{ + uint8_t test_data[] = "after stop fail"; + size_t set_data_calls; + + advert_start_with_fakes(); + bt_le_ext_adv_stop_fake.return_val = -EIO; + zassert_equal(-EIO, sid_ble_advert_stop()); + + bt_le_ext_adv_delete_fake.return_val = ESUCCESS; + zassert_equal(ESUCCESS, sid_ble_advert_deinit()); + + set_data_calls = bt_le_ext_adv_set_data_fake.call_count; + zassert_equal(ESUCCESS, sid_ble_advert_update(test_data, sizeof(test_data))); + zassert_equal(set_data_calls, bt_le_ext_adv_set_data_fake.call_count, + "deinit must reset stale state after failed stop"); + + bt_le_ext_adv_stop_fake.return_val = ESUCCESS; + advert_start_with_fakes(); }