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

Commit 813e46b5 authored by Rahul Arya's avatar Rahul Arya Committed by Gerrit Code Review
Browse files

Merge "Fix missing advertising set callbacks"

parents da573fcc 212357f2

system/gd/hci/le_advertising_manager.cc

+42 −42
Original line number Diff line number Diff line
@@ -61,7 +61,6 @@ struct Advertiser { 
  int8_t tx_power;

  uint16_t duration;

  uint8_t max_extended_advertising_events;

  bool pending_start = false;  // whether we have started but are still in the queue

  bool started = false;

  bool connectable = false;

  bool directed = false;
@@ -348,7 +347,6 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
          enable_advertiser(id, true, 0, 0);

        } else {

          enabled_sets_[id].advertising_handle_ = id;

          advertising_sets_[id].pending_start = true;

        }

      } break;

      case (AdvertisingApiType::ANDROID_HCI): {
@@ -374,7 +372,6 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
          enable_advertiser(id, true, 0, 0);

        } else {

          enabled_sets_[id].advertising_handle_ = id;

          advertising_sets_[id].pending_start = true;

        }

      } break;

      case (AdvertisingApiType::EXTENDED): {
@@ -486,9 +483,6 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
    if (!paused) {

      enable_advertiser(id, true, duration, max_ext_adv_events);

    } else {

      // invoke callbacks upon OnResume()

      advertising_sets_[id].pending_start = true;



      EnabledSet curr_set;

      curr_set.advertising_handle_ = id;

      curr_set.duration_ = duration;
@@ -933,11 +927,6 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
      return;

    }



    if (enable) {

      // so that callbacks get invoked on command complete

      advertising_sets_[advertiser_id].pending_start = true;

    }



    switch (advertising_api_type_) {

      case (AdvertisingApiType::LEGACY): {

        le_advertising_interface_->EnqueueCommand(
@@ -946,22 +935,29 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
                this,

                &impl::on_set_advertising_enable_complete<LeSetAdvertisingEnableCompleteView>,

                enable,

                enabled_sets));

                enabled_sets,

                true /* trigger callbacks */));

      } break;

      case (AdvertisingApiType::ANDROID_HCI): {

        le_advertising_interface_->EnqueueCommand(

            hci::LeMultiAdvtSetEnableBuilder::Create(enable_value, advertiser_id),

            module_handler_->BindOnceOn(

                this, &impl::on_set_advertising_enable_complete<LeMultiAdvtCompleteView>, enable, enabled_sets));

                this,

                &impl::on_set_advertising_enable_complete<LeMultiAdvtCompleteView>,

                enable,

                enabled_sets,

                true /* trigger callbacks */));

      } break;

      case (AdvertisingApiType::EXTENDED): {

        le_advertising_interface_->EnqueueCommand(

            hci::LeSetExtendedAdvertisingEnableBuilder::Create(enable_value, enabled_sets),

            module_handler_->BindOnceOn(

                this,

                &impl::on_set_extended_advertising_enable_complete<LeSetExtendedAdvertisingEnableCompleteView>,

                &impl::on_set_extended_advertising_enable_complete<

                    LeSetExtendedAdvertisingEnableCompleteView>,

                enable,

                enabled_sets));

                enabled_sets,

                true /* trigger callbacks */));

      } break;

    }
@@ -1128,13 +1124,17 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
        case (AdvertisingApiType::LEGACY): {

          le_advertising_interface_->EnqueueCommand(

              hci::LeSetAdvertisingEnableBuilder::Create(Enable::ENABLED),

              common::init_flags::trigger_advertising_callbacks_on_first_resume_after_pause_is_enabled()

              common::init_flags::

                      trigger_advertising_callbacks_on_first_resume_after_pause_is_enabled()

                  ? module_handler_->BindOnceOn(

                        this,

                        &impl::on_set_advertising_enable_complete<LeSetAdvertisingEnableCompleteView>,

                        &impl::on_set_advertising_enable_complete<

                            LeSetAdvertisingEnableCompleteView>,

                        true,

                        enabled_sets)

                  : module_handler_->BindOnce(impl::check_status<LeSetAdvertisingEnableCompleteView>));

                        enabled_sets,

                        false /* trigger_callbacks */)

                  : module_handler_->BindOnce(

                        impl::check_status<LeSetAdvertisingEnableCompleteView>));

        } break;

        case (AdvertisingApiType::ANDROID_HCI): {

          for (size_t i = 0; i < enabled_sets_.size(); i++) {
@@ -1142,12 +1142,14 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
            if (id != kInvalidHandle) {

              le_advertising_interface_->EnqueueCommand(

                  hci::LeMultiAdvtSetEnableBuilder::Create(Enable::ENABLED, id),

                  common::init_flags::trigger_advertising_callbacks_on_first_resume_after_pause_is_enabled()

                  common::init_flags::

                          trigger_advertising_callbacks_on_first_resume_after_pause_is_enabled()

                      ? module_handler_->BindOnceOn(

                            this,

                            &impl::on_set_advertising_enable_complete<LeMultiAdvtCompleteView>,

                            true,

                            enabled_sets)

                            enabled_sets,

                            false /* trigger_callbacks */)

                      : module_handler_->BindOnce(impl::check_status<LeMultiAdvtCompleteView>));

            }

          }
@@ -1156,14 +1158,17 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
          if (enabled_sets.size() != 0) {

            le_advertising_interface_->EnqueueCommand(

                hci::LeSetExtendedAdvertisingEnableBuilder::Create(Enable::ENABLED, enabled_sets),

                common::init_flags::trigger_advertising_callbacks_on_first_resume_after_pause_is_enabled()

                common::init_flags::

                        trigger_advertising_callbacks_on_first_resume_after_pause_is_enabled()

                    ? module_handler_->BindOnceOn(

                          this,

                          &impl::on_set_extended_advertising_enable_complete<

                              LeSetExtendedAdvertisingEnableCompleteView>,

                          true,

                          enabled_sets)

                    : module_handler_->BindOnce(impl::check_status<LeSetExtendedAdvertisingEnableCompleteView>));

                          enabled_sets,

                          false /* trigger_callbacks */)

                    : module_handler_->BindOnce(

                          impl::check_status<LeSetExtendedAdvertisingEnableCompleteView>));

          }

        } break;

      }
@@ -1230,7 +1235,11 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
  }



  template <class View>

  void on_set_advertising_enable_complete(bool enable, std::vector<EnabledSet> enabled_sets, CommandCompleteView view) {

  void on_set_advertising_enable_complete(

      bool enable,

      std::vector<EnabledSet> enabled_sets,

      bool trigger_callbacks,

      CommandCompleteView view) {

    ASSERT(view.IsValid());

    auto complete_view = View::Create(view);

    ASSERT(complete_view.IsValid());
@@ -1259,15 +1268,9 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
      }



      if (started) {

        // This event can be triggered from OnPause / OnResume. If so, only invoke callbacks if we were initially paused

        // (pending_start) after an API invocation (i.e. StartAdvertising -> currently paused -> OnResume -> enabled

        // [callback], but StartAdvertising -> enabled [callback] -> resumed -> OnPause -> OnResume [NO callback]

        if (!advertising_sets_[enabled_set.advertising_handle_].pending_start) {

          continue;

        }

        if (trigger_callbacks) {

          advertising_callbacks_->OnAdvertisingEnabled(id, enable, advertising_status);

        // since we have started, we are no longer pending

        advertising_sets_[enabled_set.advertising_handle_].pending_start = false;

        }

      } else {

        advertising_sets_[enabled_set.advertising_handle_].started = true;

        advertising_callbacks_->OnAdvertisingSetStarted(reg_id, id, le_physical_channel_tx_power_, advertising_status);
@@ -1277,7 +1280,10 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 


  template <class View>

  void on_set_extended_advertising_enable_complete(

      bool enable, std::vector<EnabledSet> enabled_sets, CommandCompleteView view) {

      bool enable,

      std::vector<EnabledSet> enabled_sets,

      bool trigger_callbacks,

      CommandCompleteView view) {

    ASSERT(view.IsValid());

    auto complete_view = LeSetExtendedAdvertisingEnableCompleteView::Create(view);

    ASSERT(complete_view.IsValid());
@@ -1309,15 +1315,9 @@ struct LeAdvertisingManager::impl : public bluetooth::hci::LeAddressManagerCallb 
      }



      if (started) {

        // This event can be triggered from OnPause / OnResume. If so, only invoke callbacks if we were initially paused

        // (pending_start) after an API invocation (i.e. StartAdvertising -> currently paused -> OnResume -> enabled

        // [callback], but StartAdvertising -> enabled [callback] -> resumed -> OnPause -> OnResume [NO callback]

        if (!advertising_sets_[enabled_set.advertising_handle_].pending_start) {

          continue;

        }

        if (trigger_callbacks) {

          advertising_callbacks_->OnAdvertisingEnabled(id, enable, advertising_status);

        // since we have started, we are no longer pending

        advertising_sets_[enabled_set.advertising_handle_].pending_start = false;

        }

      } else {

        advertising_sets_[enabled_set.advertising_handle_].started = true;

        advertising_callbacks_->OnAdvertisingSetStarted(reg_id, id, tx_power, advertising_status);

system/gd/hci/le_advertising_manager_test.cc

+87 −6
Original line number Diff line number Diff line
@@ -36,8 +36,13 @@ namespace bluetooth { 
namespace hci {

namespace {



using namespace std::literals;



using packet::RawBuilder;



using testing::_;

using testing::InSequence;



class TestController : public Controller {

 public:

  bool IsSupported(OpCode op_code) const override {
@@ -228,11 +233,7 @@ class LeAdvertisingManagerTest : public ::testing::Test { 
  void on_set_terminated(ErrorCode error_code, uint8_t, uint8_t) {}



  void sync_client_handler() {

    std::promise<void> promise;

    auto future = promise.get_future();

    client_handler_->Post(common::BindOnce(&std::promise<void>::set_value, common::Unretained(&promise)));

    auto future_status = future.wait_for(std::chrono::seconds(1));

    ASSERT_EQ(future_status, std::future_status::ready);

    ASSERT(thread_.GetReactor()->WaitForIdle(2s));

  }



  class MockAdvertisingCallback : public AdvertisingCallback {
@@ -1003,6 +1004,47 @@ TEST_F(LeExtendedAdvertisingAPITest, disable_enable_advertiser_test) { 
  test_hci_layer_->IncomingEvent(LeSetExtendedAdvertisingEnableCompleteBuilder::Create(uint8_t{1}, ErrorCode::SUCCESS));

}



TEST_F(LeExtendedAdvertisingAPITest, disable_after_enable) {

  // we expect Started -> Enable(false) -> Enable(true) -> Enable(false)



  // setup already arranges everything and starts the advertiser



  // expect

  InSequence s;

  EXPECT_CALL(mock_advertising_callback_, OnAdvertisingEnabled(_, false, _));

  EXPECT_CALL(mock_advertising_callback_, OnAdvertisingEnabled(_, true, _));

  EXPECT_CALL(mock_advertising_callback_, OnAdvertisingEnabled(_, false, _));

  EXPECT_CALL(mock_advertising_callback_, OnAdvertisingEnabled(_, true, _));



  // act



  // disable

  le_advertising_manager_->EnableAdvertiser(advertiser_id_, false, 0x00, 0x00);

  test_hci_layer_->GetCommand();

  test_hci_layer_->IncomingEvent(

      LeSetExtendedAdvertisingEnableCompleteBuilder::Create(uint8_t{1}, ErrorCode::SUCCESS));



  // enable

  le_advertising_manager_->EnableAdvertiser(advertiser_id_, true, 0x00, 0x00);

  test_hci_layer_->GetCommand();

  test_hci_layer_->IncomingEvent(

      LeSetExtendedAdvertisingEnableCompleteBuilder::Create(uint8_t{1}, ErrorCode::SUCCESS));



  // disable

  le_advertising_manager_->EnableAdvertiser(advertiser_id_, false, 0x00, 0x00);

  test_hci_layer_->GetCommand();

  test_hci_layer_->IncomingEvent(

      LeSetExtendedAdvertisingEnableCompleteBuilder::Create(uint8_t{1}, ErrorCode::SUCCESS));



  // enable

  le_advertising_manager_->EnableAdvertiser(advertiser_id_, true, 0x00, 0x00);

  test_hci_layer_->GetCommand();

  test_hci_layer_->IncomingEvent(

      LeSetExtendedAdvertisingEnableCompleteBuilder::Create(uint8_t{1}, ErrorCode::SUCCESS));



  sync_client_handler();

}



TEST_F(LeExtendedAdvertisingAPITest, set_periodic_parameter) {

  PeriodicAdvertisingParameters advertising_config{};

  advertising_config.max_interval = 0x1000;
@@ -1132,7 +1174,6 @@ TEST_F(LeExtendedAdvertisingAPITest, trigger_advertiser_callbacks_if_started_whi 
      [](std::promise<uint8_t> promise, uint8_t id, uint8_t _status) { promise.set_value(id); },

      std::move(id_promise)));

  sync_client_handler();

  LOG_INFO("start");

  auto set_id = id_future.get();



  auto status_promise = std::promise<ErrorCode>{};
@@ -1175,6 +1216,46 @@ TEST_F(LeExtendedAdvertisingAPITest, trigger_advertiser_callbacks_if_started_whi 


  // assert

  EXPECT_EQ(status_future.get(), ErrorCode::SUCCESS);



  sync_client_handler();

}



TEST_F(LeExtendedAdvertisingAPITest, no_callbacks_on_pause) {

  // arrange

  auto test_le_address_manager = (TestLeAddressManager*)test_acl_manager_->GetLeAddressManager();



  // expect

  EXPECT_CALL(mock_advertising_callback_, OnAdvertisingEnabled(_, _, _)).Times(0);



  // act

  LOG_INFO("pause");

  test_le_address_manager->client_->OnPause();

  test_hci_layer_->GetCommand();

  test_hci_layer_->IncomingEvent(

      LeSetExtendedAdvertisingEnableCompleteBuilder::Create(1, ErrorCode::SUCCESS));



  sync_client_handler();

}



TEST_F(LeExtendedAdvertisingAPITest, no_callbacks_on_resume) {

  // arrange

  auto test_le_address_manager = (TestLeAddressManager*)test_acl_manager_->GetLeAddressManager();

  test_le_address_manager->client_->OnPause();

  test_hci_layer_->GetCommand();

  test_hci_layer_->IncomingEvent(

      LeSetExtendedAdvertisingEnableCompleteBuilder::Create(1, ErrorCode::SUCCESS));

  sync_client_handler();



  // expect

  EXPECT_CALL(mock_advertising_callback_, OnAdvertisingEnabled(_, _, _)).Times(0);



  // act

  test_le_address_manager->client_->OnResume();

  test_hci_layer_->GetCommand();

  test_hci_layer_->IncomingEvent(

      LeSetExtendedAdvertisingEnableCompleteBuilder::Create(1, ErrorCode::SUCCESS));



  sync_client_handler();

}



}  // namespace