From 1f62c122e908573497a8c69ccd7bd829ce02a0b9 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowski Date: Tue, 4 Apr 2017 04:08:19 -0700 Subject: [PATCH] Advertising manager improvements * Keep track wether advertiser is enabled. * Make sure that random address for connectable advertisers is updated when advertising is disabled, as per ESR11-E7716. * Make sure that the local variable holding the address is properly updated after the address is pushed to the controller. * Use "LE Remove Advertising Set" command to free advertiser after use. Bug: 35935853 Bug: 30622771 Test: manual Change-Id: I1415f7272dd99e5e81ce1e2b7ef2bf98f7229cf9 --- stack/btm/ble_advertiser_hci_interface.cc | 13 ++ stack/btm/btm_ble_multi_adv.cc | 207 +++++++++++++++-------- stack/test/ble_advertiser_test.cc | 265 ++++++++++++++++++++++-------- 3 files changed, 349 insertions(+), 136 deletions(-) diff --git a/stack/btm/ble_advertiser_hci_interface.cc b/stack/btm/ble_advertiser_hci_interface.cc index 886770ea5..d7b217bb2 100644 --- a/stack/btm/ble_advertiser_hci_interface.cc +++ b/stack/btm/ble_advertiser_hci_interface.cc @@ -26,6 +26,7 @@ #include "btm_ble_api.h" #include "btm_int_types.h" #include "device/include/controller.h" +#include "hcidefs.h" #define BTM_BLE_MULTI_ADV_SET_RANDOM_ADDR_LEN 8 #define BTM_BLE_MULTI_ADV_ENB_LEN 3 @@ -209,6 +210,12 @@ class BleAdvertiserVscHciInterfaceImpl : public BleAdvertiserHciInterface { uint8_t max_extended_advertising_events, status_cb command_complete) override { VLOG(1) << __func__; + + if (max_extended_advertising_events) { + command_complete.Run(HCI_ERR_ILLEGAL_PARAMETER_FMT); + return; + } + uint8_t param[BTM_BLE_MULTI_ADV_ENB_LEN]; memset(param, 0, BTM_BLE_MULTI_ADV_ENB_LEN); @@ -399,6 +406,12 @@ class BleAdvertiserLegacyHciInterfaceImpl : public BleAdvertiserHciInterface { uint8_t max_extended_advertising_events, status_cb command_complete) override { VLOG(1) << __func__; + + if (max_extended_advertising_events) { + command_complete.Run(HCI_ERR_ILLEGAL_PARAMETER_FMT); + return; + } + uint8_t param[HCIC_PARAM_SIZE_WRITE_ADV_ENABLE]; uint8_t* pp = param; diff --git a/stack/btm/btm_ble_multi_adv.cc b/stack/btm/btm_ble_multi_adv.cc index 3183a19dc..27a392db2 100644 --- a/stack/btm/btm_ble_multi_adv.cc +++ b/stack/btm/btm_ble_multi_adv.cc @@ -1,5 +1,6 @@ /****************************************************************************** * + * Copyright (C) 2017 The Android Open Source Project * Copyright (C) 2014 Broadcom Corporation * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -42,17 +43,40 @@ extern fixed_queue_t* btu_general_alarm_queue; constexpr int ADV_DATA_LEN_MAX = 251; +namespace { + +bool is_connectable(uint16_t advertising_event_properties) { + return advertising_event_properties & 0x01; +} + struct AdvertisingInstance { uint8_t inst_id; bool in_use; uint8_t advertising_event_properties; alarm_t* adv_raddr_timer; int8_t tx_power; - int duration; + uint16_t duration; + uint8_t maxExtAdvEvents; alarm_t* timeout_timer; uint8_t own_address_type; BD_ADDR own_address; MultiAdvCb timeout_cb; + bool address_update_required; + + /* When true, advertising set is enabled, or last scheduled call to "LE Set + * Extended Advertising Set Enable" is to enable this advertising set. Any + * command scheduled when in this state will execute when the set is enabled, + * unless enabling fails. + * + * When false, advertising set is disabled, or last scheduled call to "LE Set + * Extended Advertising Set Enable" is to disable this advertising set. Any + * command scheduled when in this state will execute when the set is disabled. + */ + bool enable_status; + + bool IsEnabled() { return enable_status; } + + bool IsConnectable() { return is_connectable(advertising_event_properties); } AdvertisingInstance(int inst_id) : inst_id(inst_id), @@ -62,7 +86,9 @@ struct AdvertisingInstance { duration(0), timeout_timer(nullptr), own_address_type(0), - own_address{0} { + own_address{0}, + address_update_required(false), + enable_status(false) { adv_raddr_timer = alarm_new_periodic("btm_ble.adv_raddr_timer"); } @@ -74,18 +100,9 @@ struct AdvertisingInstance { void btm_ble_adv_raddr_timer_timeout(void* data); -namespace { - void DoNothing(uint8_t) {} void DoNothing2(uint8_t, uint8_t) {} -bool is_connectable(uint16_t advertising_event_properties) { - if ((advertising_event_properties & 0x01) != 0) { - return true; - } - return false; -} - struct closure_data { base::Closure user_task; tracked_objects::Location posted_from; @@ -150,18 +167,18 @@ class BleAdvertisingManagerImpl } } - void OnRpaGenerationComplete(uint8_t inst_id, base::Closure cb, + void OnRpaGenerationComplete(base::Callback cb, uint8_t rand[8]) { - LOG(INFO) << "inst_id = " << +inst_id; + VLOG(1) << __func__; - AdvertisingInstance* p_inst = &adv_inst[inst_id]; + bt_bdaddr_t bda; rand[2] &= (~BLE_RESOLVE_ADDR_MASK); rand[2] |= BLE_RESOLVE_ADDR_MSB; - p_inst->own_address[2] = rand[0]; - p_inst->own_address[1] = rand[1]; - p_inst->own_address[0] = rand[2]; + bda.address[2] = rand[0]; + bda.address[1] = rand[1]; + bda.address[0] = rand[2]; BT_OCTET16 irk; BTM_GetDeviceIDRoot(irk); @@ -171,31 +188,66 @@ class BleAdvertisingManagerImpl LOG_ASSERT(false) << "SMP_Encrypt failed"; /* set hash to be LSB of rpAddress */ - p_inst->own_address[5] = output.param_buf[0]; - p_inst->own_address[4] = output.param_buf[1]; - p_inst->own_address[3] = output.param_buf[2]; + bda.address[5] = output.param_buf[0]; + bda.address[4] = output.param_buf[1]; + bda.address[3] = output.param_buf[2]; - cb.Run(); + cb.Run(bda); } - void GenerateRpa(uint8_t inst_id, base::Closure cb) { + void GenerateRpa(base::Callback cb) { btm_gen_resolvable_private_addr( Bind(&BleAdvertisingManagerImpl::OnRpaGenerationComplete, - base::Unretained(this), inst_id, std::move(cb))); + base::Unretained(this), std::move(cb))); } - void ConfigureRpa(AdvertisingInstance* p_inst) { - GenerateRpa(p_inst->inst_id, - Bind( - [](AdvertisingInstance* p_inst) { - /* set it to controller */ - ((BleAdvertisingManagerImpl*)BleAdvertisingManager::Get()) - ->GetHciInterface() - ->SetRandomAddress(p_inst->inst_id, - p_inst->own_address, - Bind(DoNothing)); - }, - p_inst)); + void ConfigureRpa(AdvertisingInstance* p_inst, MultiAdvCb configuredCb) { + /* Connectable advertising set must be disabled when updating RPA */ + bool restart = p_inst->IsEnabled() && p_inst->IsConnectable(); + + // If there is any form of timeout on the set, schedule address update when + // the set stops, because there is no good way to compute new timeout value. + // Maximum duration value is around 10 minutes, so this is safe. + if (restart && (p_inst->duration || p_inst->maxExtAdvEvents)) { + p_inst->address_update_required = true; + configuredCb.Run(0x01); + return; + } + + GenerateRpa(Bind( + [](AdvertisingInstance* p_inst, MultiAdvCb configuredCb, + bt_bdaddr_t bda) { + /* Connectable advertising set must be disabled when updating RPA */ + bool restart = p_inst->IsEnabled() && p_inst->IsConnectable(); + + auto hci_interface = + ((BleAdvertisingManagerImpl*)BleAdvertisingManager::Get()) + ->GetHciInterface(); + + if (restart) { + p_inst->enable_status = false; + hci_interface->Enable(false, p_inst->inst_id, 0x00, 0x00, + Bind(DoNothing)); + } + + /* set it to controller */ + hci_interface->SetRandomAddress( + p_inst->inst_id, p_inst->own_address, + Bind( + [](AdvertisingInstance* p_inst, bt_bdaddr_t bda, + MultiAdvCb configuredCb, uint8_t status) { + memcpy(p_inst->own_address, &bda, BD_ADDR_LEN); + configuredCb.Run(0x00); + }, + p_inst, bda, configuredCb)); + + if (restart) { + p_inst->enable_status = true; + hci_interface->Enable(true, p_inst->inst_id, 0x00, 0x00, + Bind(DoNothing)); + } + }, + p_inst, std::move(configuredCb))); } void RegisterAdvertiser( @@ -211,19 +263,20 @@ class BleAdvertisingManagerImpl // set up periodic timer to update address. if (BTM_BleLocalPrivacyEnabled()) { p_inst->own_address_type = BLE_ADDR_RANDOM; - GenerateRpa(p_inst->inst_id, - Bind( - [](AdvertisingInstance* p_inst, - base::Callback - cb) { - alarm_set_on_queue(p_inst->adv_raddr_timer, - BTM_BLE_PRIVATE_ADDR_INT_MS, - btm_ble_adv_raddr_timer_timeout, - p_inst, btu_general_alarm_queue); - cb.Run(p_inst->inst_id, BTM_BLE_MULTI_ADV_SUCCESS); - }, - p_inst, cb)); + GenerateRpa(Bind( + [](AdvertisingInstance* p_inst, + base::Callback + cb, + bt_bdaddr_t bda) { + memcpy(p_inst->own_address, &bda, BD_ADDR_LEN); + + alarm_set_on_queue(p_inst->adv_raddr_timer, + BTM_BLE_PRIVATE_ADDR_INT_MS, + btm_ble_adv_raddr_timer_timeout, p_inst, + btu_general_alarm_queue); + cb.Run(p_inst->inst_id, BTM_BLE_MULTI_ADV_SUCCESS); + }, + p_inst, cb)); } #else p_inst->own_address_type = BLE_ADDR_PUBLIC; @@ -474,7 +527,6 @@ class BleAdvertisingManagerImpl // Run the regular enable callback enable_cb.Run(status); - p_inst->duration = duration; p_inst->timeout_timer = alarm_new("btm_ble.adv_timeout"); base::Closure cb = Bind(&BleAdvertisingManagerImpl::Enable, @@ -482,7 +534,7 @@ class BleAdvertisingManagerImpl std::move(timeout_cb), 0, 0, base::Bind(DoNothing)); // schedule disable when the timeout passes - alarm_set_closure_on_queue(FROM_HERE, p_inst->timeout_timer, duration * 100, + alarm_set_closure_on_queue(FROM_HERE, p_inst->timeout_timer, duration * 10, std::move(cb), btu_general_alarm_queue); } @@ -503,17 +555,34 @@ class BleAdvertisingManagerImpl } if (enable && (duration || maxExtAdvEvents)) { - p_inst->timeout_cb = timeout_cb; + p_inst->timeout_cb = std::move(timeout_cb); + } + + p_inst->duration = duration; + p_inst->maxExtAdvEvents = maxExtAdvEvents; + + if (enable && p_inst->address_update_required) { + p_inst->address_update_required = false; + ConfigureRpa(p_inst, base::Bind(&BleAdvertisingManagerImpl::EnableFinish, + base::Unretained(this), p_inst, enable, + std::move(cb))); + return; } - if (enable && duration) { + EnableFinish(p_inst, enable, std::move(cb), 0); + } + + void EnableFinish(AdvertisingInstance* p_inst, bool enable, MultiAdvCb cb, + uint8_t status) { + if (enable && p_inst->duration) { + p_inst->enable_status = enable; // TODO(jpawlowski): HCI implementation that can't do duration should // emulate it, not EnableWithTimerCb. GetHciInterface()->Enable( - enable, p_inst->inst_id, duration, maxExtAdvEvents, + enable, p_inst->inst_id, p_inst->duration, p_inst->maxExtAdvEvents, Bind(&BleAdvertisingManagerImpl::EnableWithTimerCb, - base::Unretained(this), inst_id, std::move(cb), duration, - std::move(timeout_cb))); + base::Unretained(this), p_inst->inst_id, std::move(cb), + p_inst->duration, p_inst->timeout_cb)); } else { if (p_inst->timeout_timer) { @@ -522,8 +591,9 @@ class BleAdvertisingManagerImpl p_inst->timeout_timer = nullptr; } - GetHciInterface()->Enable(enable, p_inst->inst_id, duration, - maxExtAdvEvents, cb); + p_inst->enable_status = enable; + GetHciInterface()->Enable(enable, p_inst->inst_id, p_inst->duration, + p_inst->maxExtAdvEvents, std::move(cb)); } } @@ -696,11 +766,15 @@ class BleAdvertisingManagerImpl return; } - // TODO(jpawlowski): only disable when enabled or enabling - GetHciInterface()->Enable(false, inst_id, 0x00, 0x00, Bind(DoNothing)); + if (adv_inst[inst_id].IsEnabled()) { + p_inst->enable_status = false; + GetHciInterface()->Enable(false, inst_id, 0x00, 0x00, Bind(DoNothing)); + } alarm_cancel(p_inst->adv_raddr_timer); p_inst->in_use = false; + GetHciInterface()->RemoveAdvertisingSet(inst_id, Bind(DoNothing)); + p_inst->address_update_required = false; } void OnAdvertisingSetTerminated( @@ -713,6 +787,8 @@ class BleAdvertisingManagerImpl if (status == 0x43 || status == 0x3C) { // either duration elapsed, or maxExtAdvEvents reached + p_inst->enable_status = false; + if (p_inst->timeout_cb.is_null()) { LOG(INFO) << __func__ << "No timeout callback"; return; @@ -735,8 +811,7 @@ class BleAdvertisingManagerImpl // TODO(jpawlowski): we don't really allow to do directed advertising // right now. This should probably be removed, check with Andre. if ((p_inst->advertising_event_properties & 0x0C) == - 0 /* directed advertising bits not set - */) { + 0 /* directed advertising bits not set */) { GetHciInterface()->Enable(true, advertising_handle, 0x00, 0x00, Bind(DoNothing)); } else { @@ -755,7 +830,12 @@ class BleAdvertisingManagerImpl }; BleAdvertisingManager* instance; + +void btm_ble_adv_raddr_timer_timeout(void* data) { + ((BleAdvertisingManagerImpl*)BleAdvertisingManager::Get()) + ->ConfigureRpa((AdvertisingInstance*)data, base::Bind(DoNothing)); } +} // namespace void BleAdvertisingManager::Initialize(BleAdvertiserHciInterface* interface) { instance = new BleAdvertisingManagerImpl(interface); @@ -771,11 +851,6 @@ void BleAdvertisingManager::CleanUp() { instance = nullptr; }; -void btm_ble_adv_raddr_timer_timeout(void* data) { - ((BleAdvertisingManagerImpl*)BleAdvertisingManager::Get()) - ->ConfigureRpa((AdvertisingInstance*)data); -} - /** * This function initialize the advertising manager. **/ diff --git a/stack/test/ble_advertiser_test.cc b/stack/test/ble_advertiser_test.cc index f612ddb5d..222bacea8 100644 --- a/stack/test/ble_advertiser_test.cc +++ b/stack/test/ble_advertiser_test.cc @@ -29,6 +29,7 @@ using ::testing::ElementsAreArray; using ::testing::Exactly; using ::testing::IsEmpty; using ::testing::SaveArg; +using base::Bind; using status_cb = BleAdvertiserHciInterface::status_cb; using parameters_cb = BleAdvertiserHciInterface::parameters_cb; @@ -54,9 +55,15 @@ void btm_gen_resolvable_private_addr(base::Callback cb) { uint8_t fake_rand[8] = {0, 0, 0, 0, 0, 0, 0, 0}; cb.Run(fake_rand); } + +alarm_callback_t last_alarm_cb = nullptr; +void* last_alarm_data = nullptr; void alarm_set_on_queue(alarm_t* alarm, period_ms_t interval_ms, alarm_callback_t cb, void* data, fixed_queue_t* queue) { + last_alarm_cb = cb; + last_alarm_data = data; } + void alarm_cancel(alarm_t* alarm) {} alarm_t* alarm_new_periodic(const char* name) { return nullptr; } alarm_t* alarm_new(const char* name) { return nullptr; } @@ -65,6 +72,15 @@ const controller_t* controller_get_interface() { return nullptr; } fixed_queue_t* btu_general_alarm_queue = nullptr; namespace { +void DoNothing(uint8_t) {} + +void DoNothing2(uint8_t, uint8_t) {} + +void TriggerRandomAddressUpdate() { + // Call to StartAdvertisingSet set the last_alarm_cb to random address timeout + // callback. Call it now in order to trigger address update + last_alarm_cb(last_alarm_data); +} constexpr uint8_t INTERMEDIATE = 0x00; // Intermediate fragment of fragmented data @@ -93,6 +109,7 @@ class AdvertiserHciMock : public BleAdvertiserHciInterface { void(uint8_t, uint8_t, uint8_t, uint8_t*, status_cb)); MOCK_METHOD3(SetPeriodicAdvertisingEnable, void(uint8_t, uint8_t, status_cb)); MOCK_METHOD2(RemoveAdvertisingSet, void(uint8_t, status_cb)); + MOCK_METHOD1(ClearAdvertisingSets, void(status_cb)); MOCK_METHOD9(SetParameters1, void(uint8_t, uint16_t, uint32_t, uint32_t, uint8_t, uint8_t, @@ -133,6 +150,10 @@ class BleAdvertisingManagerTest : public testing::Test { int set_data_status = -1; int enable_status = -1; int start_advertising_status = -1; + int start_advertising_set_advertiser_id = -1; + int start_advertising_set_tx_power = -1; + int start_advertising_set_status = -1; + std::unique_ptr hci_mock; virtual void SetUp() { @@ -167,29 +188,38 @@ class BleAdvertisingManagerTest : public testing::Test { void SetDataCb(uint8_t status) { set_data_status = status; } void EnableCb(uint8_t status) { enable_status = status; } void StartAdvertisingCb(uint8_t status) { start_advertising_status = status; } + void StartAdvertisingSetCb(uint8_t advertiser_id, int8_t tx_power, + uint8_t status) { + start_advertising_set_advertiser_id = advertiser_id; + start_advertising_set_tx_power = tx_power; + start_advertising_set_status = status; + } }; TEST_F(BleAdvertisingManagerTest, test_registration) { for (int i = 0; i < num_adv_instances; i++) { - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( + BleAdvertisingManager::Get()->RegisterAdvertiser(Bind( &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); EXPECT_EQ(i, reg_inst_id); } // This call should return an error - no more advertisers left. - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(ADVERTISE_FAILED_TOO_MANY_ADVERTISERS, reg_status); // Don't bother checking inst_id, it doesn't matter - // This will currently trigger a mock message about a call to Enable(). This - // should be fixed in the future- we shouldn't disable non-enabled scan. + status_cb remove_cb; + EXPECT_CALL(*hci_mock, RemoveAdvertisingSet(_, _)) + .Times(1) + .WillOnce(SaveArg<1>(&remove_cb)); BleAdvertisingManager::Get()->Unregister(5); + remove_cb.Run(0); // One advertiser was freed, so should be able to register one now - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); EXPECT_EQ(5, reg_inst_id); } @@ -197,8 +227,8 @@ TEST_F(BleAdvertisingManagerTest, test_registration) { /* This test verifies that the following flow is working correctly: register, * set parameters, set data, enable, ... (advertise) ..., unregister*/ TEST_F(BleAdvertisingManagerTest, test_android_flow) { - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); int advertiser_id = reg_inst_id; @@ -210,9 +240,8 @@ TEST_F(BleAdvertisingManagerTest, test_android_flow) { .Times(1) .WillOnce(SaveArg<7>(&set_params_cb)); BleAdvertisingManager::Get()->SetParameters( - advertiser_id, ¶ms, - base::Bind(&BleAdvertisingManagerTest::SetParametersCb, - base::Unretained(this))); + advertiser_id, ¶ms, Bind(&BleAdvertisingManagerTest::SetParametersCb, + base::Unretained(this))); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); // we are a truly gracious fake controller, let the command succeed! @@ -225,8 +254,7 @@ TEST_F(BleAdvertisingManagerTest, test_android_flow) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, std::vector(), - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); set_data_cb.Run(0); @@ -238,8 +266,8 @@ TEST_F(BleAdvertisingManagerTest, test_android_flow) { .WillOnce(SaveArg<4>(&enable_cb)); BleAdvertisingManager::Get()->Enable( advertiser_id, true, - base::Bind(&BleAdvertisingManagerTest::EnableCb, base::Unretained(this)), - 0, 0, base::Callback()); + Bind(&BleAdvertisingManagerTest::EnableCb, base::Unretained(this)), 0, 0, + base::Callback()); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); enable_cb.Run(0); @@ -250,17 +278,22 @@ TEST_F(BleAdvertisingManagerTest, test_android_flow) { EXPECT_CALL(*hci_mock, Enable(0x00 /* disable */, advertiser_id, _, _, _)) .Times(1) .WillOnce(SaveArg<4>(&enable_cb)); + status_cb remove_cb; + EXPECT_CALL(*hci_mock, RemoveAdvertisingSet(_, _)) + .Times(1) + .WillOnce(SaveArg<1>(&remove_cb)); BleAdvertisingManager::Get()->Unregister(advertiser_id); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); enable_cb.Run(0); + remove_cb.Run(0); } /* This test verifies that when advertising data is set, tx power and flags will * be properly filled. */ TEST_F(BleAdvertisingManagerTest, test_adv_data_filling) { - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); int advertiser_id = reg_inst_id; @@ -275,9 +308,8 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_filling) { .Times(1) .WillOnce(SaveArg<7>(&set_params_cb)); BleAdvertisingManager::Get()->SetParameters( - advertiser_id, ¶ms, - base::Bind(&BleAdvertisingManagerTest::SetParametersCb, - base::Unretained(this))); + advertiser_id, ¶ms, Bind(&BleAdvertisingManagerTest::SetParametersCb, + base::Unretained(this))); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); // let the set parameters command succeed! @@ -298,8 +330,7 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_filling) { BleAdvertisingManager::Get()->SetData( advertiser_id, false, std::vector({0x02 /* len */, 0x0A /* tx_power */, 0x00}), - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); set_data_cb.Run(0); @@ -309,8 +340,8 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_filling) { /* This test verifies that when advertising is non-connectable, flags will not * be added. */ TEST_F(BleAdvertisingManagerTest, test_adv_data_not_filling) { - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); int advertiser_id = reg_inst_id; @@ -326,9 +357,8 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_not_filling) { .Times(1) .WillOnce(SaveArg<7>(&set_params_cb)); BleAdvertisingManager::Get()->SetParameters( - advertiser_id, ¶ms, - base::Bind(&BleAdvertisingManagerTest::SetParametersCb, - base::Unretained(this))); + advertiser_id, ¶ms, Bind(&BleAdvertisingManagerTest::SetParametersCb, + base::Unretained(this))); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); // let the set parameters command succeed! @@ -345,8 +375,7 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_not_filling) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, std::vector({0x02 /* len */, 0xFF, 0x01}), - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); set_data_cb.Run(0); @@ -354,8 +383,8 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_not_filling) { } TEST_F(BleAdvertisingManagerTest, test_reenabling) { - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); EXPECT_EQ(0, reg_inst_id); @@ -384,51 +413,52 @@ TEST_F(BleAdvertisingManagerTest, test_reenabling_disabled_instance) { /* This test verifies that the only flow that is currently used on Android, is * working correctly in happy case scenario. */ -TEST_F(BleAdvertisingManagerTest, test_start_advertising) { - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); - EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); - int advertiser_id = reg_inst_id; - +TEST_F(BleAdvertisingManagerTest, test_start_advertising_set) { std::vector adv_data; std::vector scan_resp; tBTM_BLE_ADV_PARAMS params; + tBLE_PERIODIC_ADV_PARAMS periodic_params; + periodic_params.enable = false; + std::vector periodic_data; parameters_cb set_params_cb; status_cb set_address_cb; status_cb set_data_cb; status_cb set_scan_resp_data_cb; status_cb enable_cb; - EXPECT_CALL(*hci_mock, SetParameters1(advertiser_id, _, _, _, _, _, _, _, _)) - .Times(1); + EXPECT_CALL(*hci_mock, SetParameters1(_, _, _, _, _, _, _, _, _)).Times(1); EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _, _)) .Times(1) .WillOnce(SaveArg<7>(&set_params_cb)); - EXPECT_CALL(*hci_mock, SetRandomAddress(advertiser_id, _, _)) + EXPECT_CALL(*hci_mock, SetRandomAddress(_, _, _)) .Times(1) .WillOnce(SaveArg<2>(&set_address_cb)); - EXPECT_CALL(*hci_mock, SetAdvertisingData(advertiser_id, _, _, _, _, _)) + EXPECT_CALL(*hci_mock, SetAdvertisingData(_, _, _, _, _, _)) .Times(1) .WillOnce(SaveArg<5>(&set_data_cb)); - EXPECT_CALL(*hci_mock, SetScanResponseData(advertiser_id, _, _, _, _, _)) + EXPECT_CALL(*hci_mock, SetScanResponseData(_, _, _, _, _, _)) .Times(1) .WillOnce(SaveArg<5>(&set_scan_resp_data_cb)); - EXPECT_CALL(*hci_mock, Enable(0x01 /* enable */, advertiser_id, _, _, _)) + EXPECT_CALL(*hci_mock, Enable(0x01 /* enable */, _, _, _, _)) .Times(1) .WillOnce(SaveArg<4>(&enable_cb)); - BleAdvertisingManager::Get()->StartAdvertising( - advertiser_id, base::Bind(&BleAdvertisingManagerTest::StartAdvertisingCb, - base::Unretained(this)), - ¶ms, adv_data, scan_resp, 0, base::Callback()); + BleAdvertisingManager::Get()->StartAdvertisingSet( + Bind(&BleAdvertisingManagerTest::StartAdvertisingSetCb, + base::Unretained(this)), + ¶ms, adv_data, scan_resp, &periodic_params, periodic_data, + 0 /* duration */, 0 /* maxExtAdvEvents */, Bind(DoNothing2)); // we are a truly gracious fake controller, let the commands succeed! - set_params_cb.Run(0, 0); + int selected_tx_power = -15; + set_params_cb.Run(0, selected_tx_power); set_address_cb.Run(0); set_data_cb.Run(0); set_scan_resp_data_cb.Run(0); enable_cb.Run(0); - EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, start_advertising_status); + EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, start_advertising_set_status); + EXPECT_EQ(selected_tx_power, start_advertising_set_tx_power); + int advertiser_id = start_advertising_set_advertiser_id; ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); // ... advertising ... @@ -438,15 +468,20 @@ TEST_F(BleAdvertisingManagerTest, test_start_advertising) { EXPECT_CALL(*hci_mock, Enable(0x00 /* disable */, advertiser_id, _, _, _)) .Times(1) .WillOnce(SaveArg<4>(&disable_cb)); + status_cb remove_cb; + EXPECT_CALL(*hci_mock, RemoveAdvertisingSet(advertiser_id, _)) + .Times(1) + .WillOnce(SaveArg<1>(&remove_cb)); BleAdvertisingManager::Get()->Unregister(advertiser_id); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); disable_cb.Run(0); + remove_cb.Run(0); } TEST_F(BleAdvertisingManagerTest, test_start_advertising_set_params_failed) { - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); int advertiser_id = reg_inst_id; @@ -465,8 +500,8 @@ TEST_F(BleAdvertisingManagerTest, test_start_advertising_set_params_failed) { .Times(Exactly(0)); BleAdvertisingManager::Get()->StartAdvertising( - advertiser_id, base::Bind(&BleAdvertisingManagerTest::StartAdvertisingCb, - base::Unretained(this)), + advertiser_id, Bind(&BleAdvertisingManagerTest::StartAdvertisingCb, + base::Unretained(this)), ¶ms, adv_data, scan_resp, 0, base::Callback()); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); @@ -483,8 +518,8 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { std::vector data(max_data_size); for (int i = 0; i < max_data_size; i++) data[i] = i; - BleAdvertisingManager::Get()->RegisterAdvertiser(base::Bind( - &BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); + BleAdvertisingManager::Get()->RegisterAdvertiser( + Bind(&BleAdvertisingManagerTest::RegistrationCb, base::Unretained(this))); EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, reg_status); int advertiser_id = reg_inst_id; @@ -501,8 +536,7 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, data, - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); for (int i = 0; i < 7; i++) { set_data_cb.Run(0x00); } @@ -524,8 +558,7 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, data, - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); for (int i = 0; i < 3; i++) { set_data_cb.Run(0x00); } @@ -543,8 +576,7 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, data, - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); for (int i = 0; i < 2; i++) { set_data_cb.Run(0x00); } @@ -562,8 +594,7 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, data, - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); for (int i = 0; i < 2; i++) { set_data_cb.Run(0x00); } @@ -579,8 +610,7 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, data, - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); set_data_cb.Run(0x00); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); // Expect the whole flow to succeed @@ -594,8 +624,7 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, data, - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); set_data_cb.Run(0x00); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); // Expect the whole flow to succeed @@ -609,10 +638,106 @@ TEST_F(BleAdvertisingManagerTest, test_data_sender) { .WillOnce(SaveArg<5>(&set_data_cb)); BleAdvertisingManager::Get()->SetData( advertiser_id, false, data, - base::Bind(&BleAdvertisingManagerTest::SetDataCb, - base::Unretained(this))); + Bind(&BleAdvertisingManagerTest::SetDataCb, base::Unretained(this))); set_data_cb.Run(0x00); ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); // Expect the whole flow to succeed EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, set_data_status); } + +/* This test makes sure that conectable advertisment with timeout will get it's + * address updated once the timeout passes and one tries to enable it again.*/ +TEST_F(BleAdvertisingManagerTest, + test_connectable_address_update_during_timeout) { + std::vector adv_data; + std::vector scan_resp; + tBTM_BLE_ADV_PARAMS params; + params.advertising_event_properties = 0x1 /* connectable */; + tBLE_PERIODIC_ADV_PARAMS periodic_params; + periodic_params.enable = false; + std::vector periodic_data; + + uint8_t maxExtAdvEvents = 50; + + parameters_cb set_params_cb; + status_cb set_address_cb; + status_cb set_data_cb; + status_cb set_scan_resp_data_cb; + status_cb enable_cb; + EXPECT_CALL(*hci_mock, SetParameters1(_, _, _, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _, _)) + .Times(1) + .WillOnce(SaveArg<7>(&set_params_cb)); + EXPECT_CALL(*hci_mock, SetRandomAddress(_, _, _)) + .Times(1) + .WillOnce(SaveArg<2>(&set_address_cb)); + EXPECT_CALL(*hci_mock, SetAdvertisingData(_, _, _, _, _, _)) + .Times(1) + .WillOnce(SaveArg<5>(&set_data_cb)); + EXPECT_CALL(*hci_mock, SetScanResponseData(_, _, _, _, _, _)) + .Times(1) + .WillOnce(SaveArg<5>(&set_scan_resp_data_cb)); + EXPECT_CALL(*hci_mock, Enable(0x01 /* enable */, _, _, maxExtAdvEvents, _)) + .Times(1) + .WillOnce(SaveArg<4>(&enable_cb)); + + BleAdvertisingManager::Get()->StartAdvertisingSet( + Bind(&BleAdvertisingManagerTest::StartAdvertisingSetCb, + base::Unretained(this)), + ¶ms, adv_data, scan_resp, &periodic_params, periodic_data, + 0 /* duration */, maxExtAdvEvents, Bind(DoNothing2)); + + // we are a truly gracious fake controller, let the commands succeed! + int selected_tx_power = -15; + set_params_cb.Run(0, selected_tx_power); + set_address_cb.Run(0); + set_data_cb.Run(0); + set_scan_resp_data_cb.Run(0); + enable_cb.Run(0); + EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, start_advertising_set_status); + EXPECT_EQ(selected_tx_power, start_advertising_set_tx_power); + int advertiser_id = start_advertising_set_advertiser_id; + ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); + + // ... advertising ... + + // No HCI calls should be triggered, becuase there is a timeout on a + // connectable advertisement. + TriggerRandomAddressUpdate(); + ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); + + // Set terminated because we advertised maxExtAdvEvents times! + BleAdvertisingManager::Get()->OnAdvertisingSetTerminated( + 0x43 /*status */, advertiser_id, 0x00 /* conn_handle*/, maxExtAdvEvents); + ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); + + // Try to Enable the advertiser. It should first update it's random address. + EXPECT_CALL(*hci_mock, SetRandomAddress(_, _, _)) + .Times(1) + .WillOnce(SaveArg<2>(&set_address_cb)); + EXPECT_CALL(*hci_mock, Enable(0x01 /* enable */, _, _, maxExtAdvEvents, _)) + .Times(1) + .WillOnce(SaveArg<4>(&enable_cb)); + BleAdvertisingManager::Get()->Enable( + advertiser_id, true, + Bind(&BleAdvertisingManagerTest::EnableCb, base::Unretained(this)), 0, + maxExtAdvEvents, Bind(DoNothing)); + set_address_cb.Run(0); + enable_cb.Run(0); + ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); + + // Disable advertiser + status_cb disable_cb; + EXPECT_CALL(*hci_mock, Enable(0x00 /* disable */, advertiser_id, _, _, _)) + .Times(1) + .WillOnce(SaveArg<4>(&disable_cb)); + status_cb remove_cb; + EXPECT_CALL(*hci_mock, RemoveAdvertisingSet(advertiser_id, _)) + .Times(1) + .WillOnce(SaveArg<1>(&remove_cb)); + BleAdvertisingManager::Get()->Unregister(advertiser_id); + ::testing::Mock::VerifyAndClearExpectations(hci_mock.get()); + + disable_cb.Run(0); + remove_cb.Run(0); +} -- 2.11.0