OSDN Git Service

Make advertising instance count part of the HCI interface
authorJakub Pawlowski <jpawlowski@google.com>
Mon, 10 Oct 2016 21:25:52 +0000 (14:25 -0700)
committerJakub Pawlowski <jpawlowski@google.com>
Mon, 17 Oct 2016 23:00:41 +0000 (16:00 -0700)
The number of advertising instances will ultimately depend on the HCI
interface used. BleAdvertisingManagerImpl should not depend on a global
function for that.

Test: Covered by BleAdvertiseApiTest sl4a test
Bug: 30622771
Change-Id: I1399de3f4289708f7218eae9c00ac7372e4246db

stack/btm/ble_advertiser_hci_interface.cc
stack/btm/ble_advertiser_hci_interface.h
stack/btm/btm_ble_multi_adv.cc
stack/include/ble_advertiser.h
stack/test/ble_advertiser_test.cc

index dc8a9c6..494047e 100644 (file)
  ******************************************************************************/
 
 #include "ble_advertiser_hci_interface.h"
+#include <base/callback.h>
 #include <base/logging.h>
 #include <queue>
 #include <utility>
 #include "btm_api.h"
+#include "btm_ble_api.h"
 
 #define BTM_BLE_MULTI_ADV_SET_RANDOM_ADDR_LEN 8
 #define BTM_BLE_MULTI_ADV_ENB_LEN 3
@@ -70,6 +72,10 @@ class BleAdvertiserHciInterfaceImpl : public BleAdvertiserHciInterface {
     pending_ops->push(std::make_pair(param_buf[0], command_complete));
   }
 
+  void ReadInstanceCount(base::Callback<void(uint8_t /* inst_cnt*/)> cb) override {
+    cb.Run(BTM_BleMaxMultiAdvInstanceCount());
+  }
+
   void SetParameters(uint8_t adv_int_min, uint8_t adv_int_max,
                      uint8_t advertising_type, uint8_t own_address_type,
                      BD_ADDR own_address, uint8_t direct_address_type,
index b5faf43..0c09e4c 100644 (file)
@@ -44,6 +44,7 @@ class BleAdvertiserHciInterface {
                                            uint16_t connection_handle);
   };
 
+  virtual void ReadInstanceCount(base::Callback<void(uint8_t /* inst_cnt*/)> cb) = 0;
   virtual void SetParameters(uint8_t adv_int_min, uint8_t adv_int_max,
                              uint8_t advertising_type, uint8_t own_address_type,
                              BD_ADDR own_address, uint8_t direct_address_type,
index f60ca71..29a4cc1 100644 (file)
@@ -87,16 +87,23 @@ class BleAdvertisingManagerImpl
     : public BleAdvertisingManager,
       public BleAdvertiserHciInterface::AdvertisingEventObserver {
  public:
-  BleAdvertisingManagerImpl() {
-    adv_inst.reserve(BTM_BleMaxMultiAdvInstanceCount());
+  BleAdvertisingManagerImpl(BleAdvertiserHciInterface *interface) {
+    this->hci_interface = interface;
+    hci_interface->ReadInstanceCount(base::Bind(
+        &BleAdvertisingManagerImpl::ReadInstanceCountCb, base::Unretained(this)));
+  }
+
+  ~BleAdvertisingManagerImpl() { adv_inst.clear(); }
+
+  void ReadInstanceCountCb(uint8_t instance_count) {
+    this->inst_count = instance_count;
+    adv_inst.reserve(inst_count);
     /* Initialize adv instance indices and IDs. */
-    for (uint8_t i = 0; i < BTM_BleMaxMultiAdvInstanceCount(); i++) {
+    for (uint8_t i = 0; i < inst_count; i++) {
       adv_inst.emplace_back(i + 1);
     }
   }
 
-  ~BleAdvertisingManagerImpl() { adv_inst.clear(); }
-
   void OnRpaGenerationComplete(uint8_t inst_id, tBTM_RAND_ENC *p) {
 #if (SMP_INCLUDED == TRUE)
     AdvertisingInstance *p_inst = &adv_inst[inst_id - 1];
@@ -125,7 +132,7 @@ class BleAdvertisingManagerImpl
     p_inst->rpa[3] = output.param_buf[2];
 
     if (p_inst->inst_id != BTM_BLE_MULTI_ADV_DEFAULT_STD &&
-        p_inst->inst_id < BTM_BleMaxMultiAdvInstanceCount()) {
+        p_inst->inst_id < inst_count) {
       /* set it to controller */
       GetHciInterface()->SetRandomAddress(p_inst->rpa, p_inst->inst_id,
                                           Bind(DoNothing));
@@ -147,14 +154,14 @@ class BleAdvertisingManagerImpl
   void RegisterAdvertiser(
       base::Callback<void(uint8_t /* inst_id */, uint8_t /* status */)> cb)
       override {
-    if (BTM_BleMaxMultiAdvInstanceCount() == 0) {
+    if (inst_count == 0) {
       LOG(ERROR) << "multi adv not supported";
       cb.Run(0xFF, BTM_BLE_MULTI_ADV_FAILURE);
       return;
     }
 
     AdvertisingInstance *p_inst = &adv_inst[0];
-    for (uint8_t i = 0; i < BTM_BleMaxMultiAdvInstanceCount() - 1;
+    for (uint8_t i = 0; i < inst_count - 1;
          i++, p_inst++) {
       if (!p_inst->in_use) {
         p_inst->in_use = TRUE;
@@ -195,7 +202,7 @@ class BleAdvertisingManagerImpl
     AdvertisingInstance *p_inst = &adv_inst[inst_id - 1];
 
     VLOG(1) << __func__ << " inst_id: " << +inst_id << ", enable: " << enable;
-    if (BTM_BleMaxMultiAdvInstanceCount() == 0) {
+    if (inst_count == 0) {
       LOG(ERROR) << "multi adv not supported";
       return;
     }
@@ -228,12 +235,12 @@ class BleAdvertisingManagerImpl
 
     VLOG(1) << __func__ << " inst_id:" << +inst_id;
 
-    if (BTM_BleMaxMultiAdvInstanceCount() == 0) {
+    if (inst_count == 0) {
       LOG(ERROR) << "multi adv not supported";
       return;
     }
 
-    if (inst_id > BTM_BleMaxMultiAdvInstanceCount() || inst_id < 0 ||
+    if (inst_id > inst_count || inst_id < 0 ||
         inst_id == BTM_BLE_MULTI_ADV_DEFAULT_STD) {
       LOG(ERROR) << "bad instance id " << +inst_id;
       return;
@@ -286,7 +293,7 @@ class BleAdvertisingManagerImpl
 
     VLOG(1) << "inst_id = " << +inst_id << ", is_scan_rsp = " << is_scan_rsp;
 
-    if (BTM_BleMaxMultiAdvInstanceCount() == 0) {
+    if (inst_count == 0) {
       LOG(ERROR) << "multi adv not supported";
       return;
     }
@@ -317,7 +324,7 @@ class BleAdvertisingManagerImpl
       }
     }
 
-    if (inst_id > BTM_BleMaxMultiAdvInstanceCount() || inst_id < 0 ||
+    if (inst_id > inst_count || inst_id < 0 ||
         inst_id == BTM_BLE_MULTI_ADV_DEFAULT_STD) {
       LOG(ERROR) << "bad instance id " << +inst_id;
       return;
@@ -339,12 +346,12 @@ class BleAdvertisingManagerImpl
 
     VLOG(1) << __func__ << " inst_id: " << +inst_id;
 
-    if (BTM_BleMaxMultiAdvInstanceCount() == 0) {
+    if (inst_count == 0) {
       LOG(ERROR) << "multi adv not supported";
       return;
     }
 
-    if (inst_id > BTM_BleMaxMultiAdvInstanceCount() || inst_id < 0 ||
+    if (inst_id > inst_count || inst_id < 0 ||
         inst_id == BTM_BLE_MULTI_ADV_DEFAULT_STD) {
       LOG(ERROR) << "bad instance id " << +inst_id;
       return;
@@ -371,7 +378,7 @@ class BleAdvertisingManagerImpl
     }
 #endif
 
-    if (inst_id < BTM_BleMaxMultiAdvInstanceCount() &&
+    if (inst_id < inst_count &&
         inst_id != BTM_BLE_MULTI_ADV_DEFAULT_STD) {
       VLOG(1) << "reneabling advertising";
 
@@ -394,23 +401,20 @@ class BleAdvertisingManagerImpl
     }
   }
 
-  void SetHciInterface(BleAdvertiserHciInterface *interface) {
-    hci_interface = interface;
-  };
-
  private:
   BleAdvertiserHciInterface *GetHciInterface() { return hci_interface; }
 
   BleAdvertiserHciInterface *hci_interface = nullptr;
   std::vector<AdvertisingInstance> adv_inst;
+  uint8_t inst_count;
 };
 
 namespace {
 BleAdvertisingManager *instance;
 }
 
-void BleAdvertisingManager::Initialize() {
-  instance = new BleAdvertisingManagerImpl();
+void BleAdvertisingManager::Initialize(BleAdvertiserHciInterface *interface) {
+  instance = new BleAdvertisingManagerImpl(interface);
 }
 
 BleAdvertisingManager *BleAdvertisingManager::Get() {
@@ -440,9 +444,8 @@ void btm_ble_adv_raddr_timer_timeout(void *data) {
 **
 *******************************************************************************/
 void btm_ble_multi_adv_init() {
-  BleAdvertisingManager::Initialize();
   BleAdvertiserHciInterface::Initialize();
-  BleAdvertisingManager::Get()->SetHciInterface(
+  BleAdvertisingManager::Initialize(
       BleAdvertiserHciInterface::Get());
 }
 
index 0d7a470..ef01c74 100644 (file)
@@ -59,7 +59,7 @@ class BleAdvertisingManager {
  public:
   virtual ~BleAdvertisingManager() = default;
 
-  static void Initialize();
+  static void Initialize(BleAdvertiserHciInterface *interface);
   static void CleanUp();
   static BleAdvertisingManager *Get();
 
@@ -87,10 +87,6 @@ class BleAdvertisingManager {
 
   /*  This function disable a Multi-ADV instance */
   virtual void Unregister(uint8_t inst_id);
-
-  /* This is exposed for tests, and for initial configuration. Higher layers
-   * shouldn't have need to ever call it.*/
-  virtual void SetHciInterface(BleAdvertiserHciInterface *interface);
 };
 
 #endif  // BLE_ADVERTISER_H
index 9cb8130..d67fda9 100644 (file)
@@ -26,6 +26,7 @@
 using ::testing::_;
 using ::testing::Args;
 using ::testing::ElementsAreArray;
+using ::testing::Exactly;
 using ::testing::IsEmpty;
 using ::testing::SaveArg;
 using status_cb = BleAdvertiserHciInterface::status_cb;
@@ -35,7 +36,6 @@ const int num_adv_instances = 16;
 /* Below are methods that must be implemented if we don't want to compile the
  * whole stack. They will be removed, or changed into mocks one by one in the
  * future, as the refactoring progresses */
-uint8_t BTM_BleMaxMultiAdvInstanceCount() { return num_adv_instances; }
 bool BTM_BleLocalPrivacyEnabled() { return true; }
 uint16_t BTM_ReadConnectability(uint16_t *p_window, uint16_t *p_interval) {
   return true;
@@ -74,6 +74,7 @@ class AdvertiserHciMock : public BleAdvertiserHciInterface {
   AdvertiserHciMock() = default;
   ~AdvertiserHciMock() override = default;
 
+  MOCK_METHOD1(ReadInstanceCount, void(base::Callback<void(uint8_t /* inst_cnt*/)>));
   MOCK_METHOD4(SetAdvertisingData,
                void(uint8_t, uint8_t *, uint8_t, status_cb));
   MOCK_METHOD4(SetScanResponseData,
@@ -114,9 +115,18 @@ class BleAdvertisingManagerTest : public testing::Test {
   std::unique_ptr<AdvertiserHciMock> hci_mock;
 
   virtual void SetUp() {
-    BleAdvertisingManager::Initialize();
     hci_mock.reset(new AdvertiserHciMock());
-    BleAdvertisingManager::Get()->SetHciInterface(hci_mock.get());
+
+    base::Callback<void(uint8_t)> inst_cnt_Cb;
+    EXPECT_CALL(*hci_mock, ReadInstanceCount(_))
+      .Times(Exactly(1))
+      .WillOnce(SaveArg<0>(&inst_cnt_Cb));
+
+    BleAdvertisingManager::Initialize(hci_mock.get());
+
+    // we are a truly gracious fake controller, let the command succeed!
+    inst_cnt_Cb.Run(num_adv_instances);
+    ::testing::Mock::VerifyAndClearExpectations(hci_mock.get());
   }
 
   virtual void TearDown() {
@@ -180,7 +190,7 @@ TEST_F(BleAdvertisingManagerTest, test_android_flow) {
       base::Bind(&BleAdvertisingManagerTest::SetParametersCb,
                  base::Unretained(this)));
 
-  // we are a trully gracious fake controller, let the command succeed!
+  // we are a truly gracious fake controller, let the command succeed!
   set_params_cb.Run(0);
   EXPECT_EQ(BTM_BLE_MULTI_ADV_SUCCESS, set_params_status);
   ::testing::Mock::VerifyAndClearExpectations(hci_mock.get());