Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 01d2fcb6 authored by Jakub Pawlowski's avatar Jakub Pawlowski
Browse files

Fix initial advertising command order 

Set the address during SetParameters, for VSC implementation, and right
after setting parameters for BT5 implementation.

Test: sl4a ConcurrentBleAdvertisingTest
Bug: 33922058
Bug: 34962013
Change-Id: Idcc60a36884607de191066d8be1ee3bc459e86fc
parent 64b2fa73

system/stack/btm/ble_advertiser_hci_interface.cc

+15 −16
Original line number Diff line number Diff line
@@ -94,11 +94,11 @@ class BleAdvertiserVscHciInterfaceImpl : public BleAdvertiserHciInterface { 


  void SetParameters(uint8_t handle, uint16_t properties, uint32_t adv_int_min,

                     uint32_t adv_int_max, uint8_t channel_map,

                     uint8_t own_address_type, uint8_t peer_address_type,

                     BD_ADDR peer_address, uint8_t filter_policy,

                     int8_t tx_power, uint8_t primary_phy,

                     uint8_t secondary_max_skip, uint8_t secondary_phy,

                     uint8_t advertising_sid,

                     uint8_t own_address_type, BD_ADDR own_address,

                     uint8_t peer_address_type, BD_ADDR peer_address,

                     uint8_t filter_policy, int8_t tx_power,

                     uint8_t primary_phy, uint8_t secondary_max_skip,

                     uint8_t secondary_phy, uint8_t advertising_sid,

                     uint8_t scan_request_notify_enable,

                     status_cb command_complete) override {

    VLOG(1) << __func__;
@@ -124,7 +124,6 @@ class BleAdvertiserVscHciInterfaceImpl : public BleAdvertiserHciInterface { 
    }



    UINT8_TO_STREAM(pp, own_address_type);

    BD_ADDR own_address = {0, 0, 0, 0, 0, 0};

    BDADDR_TO_STREAM(pp, own_address);

    UINT8_TO_STREAM(pp, peer_address_type);

    BDADDR_TO_STREAM(pp, peer_address);
@@ -273,11 +272,11 @@ class BleAdvertiserLegacyHciInterfaceImpl : public BleAdvertiserHciInterface { 


  void SetParameters(uint8_t handle, uint16_t properties, uint32_t adv_int_min,

                     uint32_t adv_int_max, uint8_t channel_map,

                     uint8_t own_address_type, uint8_t peer_address_type,

                     BD_ADDR peer_address, uint8_t filter_policy,

                     int8_t tx_power, uint8_t primary_phy,

                     uint8_t secondary_max_skip, uint8_t secondary_phy,

                     uint8_t advertising_sid,

                     uint8_t own_address_type, BD_ADDR /* own_address */,

                     uint8_t peer_address_type, BD_ADDR peer_address,

                     uint8_t filter_policy, int8_t tx_power,

                     uint8_t primary_phy, uint8_t secondary_max_skip,

                     uint8_t secondary_phy, uint8_t advertising_sid,

                     uint8_t scan_request_notify_enable,

                     status_cb command_complete) override {

    VLOG(1) << __func__;
@@ -393,11 +392,11 @@ class BleAdvertiserHciExtendedImpl : public BleAdvertiserHciInterface { 


  void SetParameters(uint8_t handle, uint16_t properties, uint32_t adv_int_min,

                     uint32_t adv_int_max, uint8_t channel_map,

                     uint8_t own_address_type, uint8_t peer_address_type,

                     BD_ADDR peer_address, uint8_t filter_policy,

                     int8_t tx_power, uint8_t primary_phy,

                     uint8_t secondary_max_skip, uint8_t secondary_phy,

                     uint8_t advertising_sid,

                     uint8_t own_address_type, BD_ADDR /* own_address */,

                     uint8_t peer_address_type, BD_ADDR peer_address,

                     uint8_t filter_policy, int8_t tx_power,

                     uint8_t primary_phy, uint8_t secondary_max_skip,

                     uint8_t secondary_phy, uint8_t advertising_sid,

                     uint8_t scan_request_notify_enable,

                     status_cb command_complete) override {

    VLOG(1) << __func__;

system/stack/btm/ble_advertiser_hci_interface.h

+5 −4
Original line number Diff line number Diff line
@@ -51,10 +51,11 @@ class BleAdvertiserHciInterface { 
  virtual void SetParameters(uint8_t handle, uint16_t properties,

                             uint32_t adv_int_min, uint32_t adv_int_max,

                             uint8_t channel_map, uint8_t own_address_type,

                             uint8_t peer_address_type, BD_ADDR peer_address,

                             uint8_t filter_policy, int8_t tx_power,

                             uint8_t primary_phy, uint8_t secondary_max_skip,

                             uint8_t secondary_phy, uint8_t advertising_sid,

                             BD_ADDR own_address, uint8_t peer_address_type,

                             BD_ADDR peer_address, uint8_t filter_policy,

                             int8_t tx_power, uint8_t primary_phy,

                             uint8_t secondary_max_skip, uint8_t secondary_phy,

                             uint8_t advertising_sid,

                             uint8_t scan_request_notify_enable,

                             status_cb command_complete) = 0;

  virtual void SetAdvertisingData(uint8_t handle, uint8_t operation,

system/stack/btm/btm_ble_multi_adv.cc

+72 −50
Original line number Diff line number Diff line
@@ -32,29 +32,30 @@ 
#include "btm_int_types.h"



using base::Bind;

using multiadv_cb = base::Callback<void(uint8_t /* status */)>;

extern void btm_gen_resolvable_private_addr(

    base::Callback<void(uint8_t[8])> cb);



extern fixed_queue_t* btu_general_alarm_queue;



struct AdvertisingInstance {

  uint8_t inst_id;

  bool in_use;

  uint8_t advertising_event_properties;

  BD_ADDR rpa;

  alarm_t* adv_raddr_timer;

  int8_t tx_power;

  int timeout_s;

  alarm_t* timeout_timer;

  uint8_t own_address_type;

  BD_ADDR own_address;



  AdvertisingInstance(int inst_id)

      : inst_id(inst_id),

        in_use(false),

        advertising_event_properties(0),

        rpa{0},

        tx_power(0),

        timeout_s(0),

        timeout_timer(nullptr) {

        timeout_timer(nullptr),

        own_address_type(0),

        own_address{0} {

    adv_raddr_timer = alarm_new_periodic("btm_ble.adv_raddr_timer");

  }
@@ -126,7 +127,8 @@ class BleAdvertisingManagerImpl 
    }

  }



  void OnRpaGenerationComplete(uint8_t inst_id, uint8_t rand[8]) {

  void OnRpaGenerationComplete(uint8_t inst_id, base::Closure cb,

                               uint8_t rand[8]) {

    LOG(INFO) << "inst_id = " << +inst_id;



    AdvertisingInstance* p_inst = &adv_inst[inst_id];
@@ -134,9 +136,9 @@ class BleAdvertisingManagerImpl 
    rand[2] &= (~BLE_RESOLVE_ADDR_MASK);

    rand[2] |= BLE_RESOLVE_ADDR_MSB;



    p_inst->rpa[2] = rand[0];

    p_inst->rpa[1] = rand[1];

    p_inst->rpa[0] = rand[2];

    p_inst->own_address[2] = rand[0];

    p_inst->own_address[1] = rand[1];

    p_inst->own_address[0] = rand[2];



    BT_OCTET16 irk;

    BTM_GetDeviceIDRoot(irk);
@@ -146,19 +148,31 @@ class BleAdvertisingManagerImpl 
      LOG_ASSERT(false) << "SMP_Encrypt failed";



    /* set hash to be LSB of rpAddress */

    p_inst->rpa[5] = output.param_buf[0];

    p_inst->rpa[4] = output.param_buf[1];

    p_inst->rpa[3] = output.param_buf[2];

    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];



    /* set it to controller */

    GetHciInterface()->SetRandomAddress(p_inst->inst_id, p_inst->rpa,

                                        Bind(DoNothing));

    cb.Run();

  }



  void ConfigureRpa(uint8_t inst_id) {

  void GenerateRpa(uint8_t inst_id, base::Closure cb) {

    btm_gen_resolvable_private_addr(

        Bind(&BleAdvertisingManagerImpl::OnRpaGenerationComplete,

             base::Unretained(this), inst_id));

             base::Unretained(this), inst_id, 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 RegisterAdvertiser(
@@ -171,17 +185,30 @@ class BleAdvertisingManagerImpl 
      p_inst->in_use = true;



#if (BLE_PRIVACY_SPT == TRUE)

      // configure the address, and set up periodic timer to update it.

      ConfigureRpa(p_inst->inst_id);



      // set up periodic timer to update address.

      if (BTM_BleLocalPrivacyEnabled()) {

        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);

        p_inst->own_address_type = BLE_ADDR_RANDOM;

        GenerateRpa(p_inst->inst_id,

                    Bind(

                        [](AdvertisingInstance* p_inst,

                           base::Callback<void(uint8_t /* inst_id */,

                                               uint8_t /* status */)>

                               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));

      }

#endif

#else

      p_inst->own_address_type = BLE_ADDR_PUBLIC;

      memcpy(p_inst->own_address,

             controller_get_interface()->get_address()->address, BD_ADDR_LEN);



      cb.Run(p_inst->inst_id, BTM_BLE_MULTI_ADV_SUCCESS);

#endif

      return;

    }
@@ -231,6 +258,15 @@ class BleAdvertisingManagerImpl 
          return;

        }



        BD_ADDR *rpa = &c->self->adv_inst[c->inst_id].own_address;

        c->self->GetHciInterface()->SetRandomAddress(c->inst_id, *rpa, Bind(

          [](c_type c, uint8_t status) {

            if (status != 0) {

              LOG(ERROR) << "setting random address failed, status: " << +status;

              c->cb.Run(status);

              return;

            }



            c->self->SetData(c->inst_id, false, std::move(c->advertise_data), Bind(

              [](c_type c, uint8_t status) {

                if (status != 0) {
@@ -252,6 +288,7 @@ class BleAdvertisingManagerImpl 
                }, base::Passed(&c)));

            }, base::Passed(&c)));

        }, base::Passed(&c)));

    }, base::Passed(&c)));

    // clang-format on

  }
@@ -327,22 +364,6 @@ class BleAdvertisingManagerImpl 
    // TODO: disable only if was enabled, currently no use scenario needs that,

    // we always set parameters before enabling

    // GetHciInterface()->Enable(false, inst_id, Bind(DoNothing));



    uint8_t own_address_type = BLE_ADDR_PUBLIC;

    BD_ADDR own_address;



#if (BLE_PRIVACY_SPT == TRUE)

    if (BTM_BleLocalPrivacyEnabled()) {

      own_address_type = BLE_ADDR_RANDOM;

      memcpy(own_address, p_inst->rpa, BD_ADDR_LEN);

    } else {

#else

    {

#endif

      memcpy(own_address, controller_get_interface()->get_address()->address,

             BD_ADDR_LEN);

    }



    p_inst->advertising_event_properties =

        p_params->advertising_event_properties;

    p_inst->tx_power = p_params->tx_power;
@@ -351,8 +372,9 @@ class BleAdvertisingManagerImpl 
    GetHciInterface()->SetParameters(

        p_inst->inst_id, p_params->advertising_event_properties,

        p_params->adv_int_min, p_params->adv_int_max, p_params->channel_map,

        own_address_type, 0x00, peer_address, p_params->adv_filter_policy,

        p_inst->tx_power, p_params->primary_advertising_phy, 0x01, 0x01,

        p_inst->own_address_type, p_inst->own_address, 0x00, peer_address,

        p_params->adv_filter_policy, p_inst->tx_power,

        p_params->primary_advertising_phy, 0x01, 0x01,

        p_params->secondary_advertising_phy,

        p_params->scan_request_notification_enable, cb);
@@ -437,7 +459,7 @@ class BleAdvertisingManagerImpl 
#if (BLE_PRIVACY_SPT == TRUE)

    if (BTM_BleLocalPrivacyEnabled() &&

        advertising_handle <= BTM_BLE_MULTI_ADV_MAX) {

      btm_acl_update_conn_addr(connection_handle, p_inst->rpa);

      btm_acl_update_conn_addr(connection_handle, p_inst->own_address);

    }

#endif
@@ -486,7 +508,7 @@ void BleAdvertisingManager::CleanUp() { 


void btm_ble_adv_raddr_timer_timeout(void* data) {

  ((BleAdvertisingManagerImpl*)BleAdvertisingManager::Get())

      ->ConfigureRpa(((AdvertisingInstance*)data)->inst_id);

      ->ConfigureRpa((AdvertisingInstance*)data);

}



/**

system/stack/test/ble_advertiser_test.cc

+23 −22
Original line number Diff line number Diff line
@@ -80,24 +80,25 @@ class AdvertiserHciMock : public BleAdvertiserHciInterface { 


  MOCK_METHOD9(SetParameters1,

               void(uint8_t, uint16_t, uint32_t, uint32_t, uint8_t, uint8_t,

                    uint8_t, BD_ADDR, uint8_t));

  MOCK_METHOD7(SetParameters2, void(int8_t, uint8_t, uint8_t, uint8_t, uint8_t,

                                    uint8_t, status_cb));

                    BD_ADDR, uint8_t, BD_ADDR));

  MOCK_METHOD8(SetParameters2, void(uint8_t, int8_t, uint8_t, uint8_t, uint8_t,

                                    uint8_t, uint8_t, status_cb));



  void SetParameters(uint8_t handle, uint16_t properties, uint32_t adv_int_min,

                     uint32_t adv_int_max, uint8_t channel_map,

                     uint8_t own_address_type, uint8_t peer_address_type,

                     BD_ADDR peer_address, uint8_t filter_policy,

                     int8_t tx_power, uint8_t primary_phy,

                     uint8_t secondary_max_skip, uint8_t secondary_phy,

                     uint8_t advertising_sid,

                     uint8_t own_address_type, BD_ADDR own_address,

                     uint8_t peer_address_type, BD_ADDR peer_address,

                     uint8_t filter_policy, int8_t tx_power,

                     uint8_t primary_phy, uint8_t secondary_max_skip,

                     uint8_t secondary_phy, uint8_t advertising_sid,

                     uint8_t scan_request_notify_enable,

                     status_cb cmd_complete) override {

    SetParameters1(handle, properties, adv_int_min, adv_int_max, channel_map,

                   own_address_type, peer_address_type, peer_address,

                   filter_policy);

    SetParameters2(tx_power, primary_phy, secondary_max_skip, secondary_phy,

                   advertising_sid, scan_request_notify_enable, cmd_complete);

                   own_address_type, own_address, peer_address_type,

                   peer_address);

    SetParameters2(filter_policy, tx_power, primary_phy, secondary_max_skip,

                   secondary_phy, advertising_sid, scan_request_notify_enable,

                   cmd_complete);

  };



 private:
@@ -185,9 +186,9 @@ TEST_F(BleAdvertisingManagerTest, test_android_flow) { 
  tBTM_BLE_ADV_PARAMS params;

  EXPECT_CALL(*hci_mock, SetParameters1(advertiser_id, _, _, _, _, _, _, _, _))

      .Times(1);

  EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _))

  EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _, _))

      .Times(1)

      .WillOnce(SaveArg<6>(&set_params_cb));

      .WillOnce(SaveArg<7>(&set_params_cb));

  BleAdvertisingManager::Get()->SetParameters(

      advertiser_id, &params,

      base::Bind(&BleAdvertisingManagerTest::SetParametersCb,
@@ -250,9 +251,9 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_filling) { 
  params.tx_power = -15;

  EXPECT_CALL(*hci_mock, SetParameters1(advertiser_id, _, _, _, _, _, _, _, _))

      .Times(1);

  EXPECT_CALL(*hci_mock, SetParameters2(params.tx_power, _, _, _, _, _, _))

  EXPECT_CALL(*hci_mock, SetParameters2(_, params.tx_power, _, _, _, _, _, _))

      .Times(1)

      .WillOnce(SaveArg<6>(&set_params_cb));

      .WillOnce(SaveArg<7>(&set_params_cb));

  BleAdvertisingManager::Get()->SetParameters(

      advertiser_id, &params,

      base::Bind(&BleAdvertisingManagerTest::SetParametersCb,
@@ -301,9 +302,9 @@ TEST_F(BleAdvertisingManagerTest, test_adv_data_not_filling) { 
  EXPECT_CALL(*hci_mock, SetParameters1(advertiser_id, _, _, _, _, _, _, _, _))

      .Times(1);

  EXPECT_CALL(*hci_mock,

              SetParameters2((uint8_t)params.tx_power, _, _, _, _, _, _))

              SetParameters2(_, (uint8_t)params.tx_power, _, _, _, _, _, _))

      .Times(1)

      .WillOnce(SaveArg<6>(&set_params_cb));

      .WillOnce(SaveArg<7>(&set_params_cb));

  BleAdvertisingManager::Get()->SetParameters(

      advertiser_id, &params,

      base::Bind(&BleAdvertisingManagerTest::SetParametersCb,
@@ -379,9 +380,9 @@ TEST_F(BleAdvertisingManagerTest, test_start_advertising) { 
  status_cb enable_cb;

  EXPECT_CALL(*hci_mock, SetParameters1(advertiser_id, _, _, _, _, _, _, _, _))

      .Times(1);

  EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _))

  EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _, _))

      .Times(1)

      .WillOnce(SaveArg<6>(&set_params_cb));

      .WillOnce(SaveArg<7>(&set_params_cb));

  EXPECT_CALL(*hci_mock, SetAdvertisingData(advertiser_id, _, _, _, _, _))

      .Times(1)

      .WillOnce(SaveArg<5>(&set_data_cb));
@@ -431,9 +432,9 @@ TEST_F(BleAdvertisingManagerTest, test_start_advertising_set_params_failed) { 
  status_cb set_params_cb;

  EXPECT_CALL(*hci_mock, SetParameters1(advertiser_id, _, _, _, _, _, _, _, _))

      .Times(1);

  EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _))

  EXPECT_CALL(*hci_mock, SetParameters2(_, _, _, _, _, _, _, _))

      .Times(1)

      .WillOnce(SaveArg<6>(&set_params_cb));

      .WillOnce(SaveArg<7>(&set_params_cb));



  EXPECT_CALL(*hci_mock, SetAdvertisingData(advertiser_id, _, _, _, _, _))

      .Times(Exactly(0));