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

Commit 0d52530e authored by Rahul Arya's avatar Rahul Arya
Browse files

[Non-Discoverable Mode] Hold ConnectComplete callback until local_address is known 

If we get an incoming LE connection, we don't know the local_address
until the associated advertising set terminates. Rather than returning a
placeholder value and *hoping* that we backfill in time, delay the event out
of GD until the value is ready.

Bug: 254314964
Test: atest pts-bot
Change-Id: I1d2d2f04a6c88c229a891705bf519b9ca5c69342
parent 4b7304f2

system/gd/hci/acl_manager.cc

+8 −2
Original line number Diff line number Diff line
@@ -308,9 +308,15 @@ void AclManager::WriteDefaultLinkPolicySettings(uint16_t default_link_policy_set 
  CallOn(pimpl_->classic_impl_, &classic_impl::write_default_link_policy_settings, default_link_policy_settings);

}



void AclManager::OnAdvertisingSetTerminated(ErrorCode status, uint16_t conn_handle, hci::AddressWithType adv_address) {

void AclManager::OnAdvertisingSetTerminated(

    ErrorCode status, uint16_t conn_handle, uint8_t adv_set_id, hci::AddressWithType adv_address) {

  if (status == ErrorCode::SUCCESS) {

    CallOn(pimpl_->le_impl_, &le_impl::UpdateLocalAddress, conn_handle, adv_address);

    CallOn(

        pimpl_->le_impl_,

        &le_impl::OnAdvertisingSetTerminated,

        conn_handle,

        adv_set_id,

        adv_address);

  }

}

system/gd/hci/acl_manager.h

+2 −1
Original line number Diff line number Diff line
@@ -129,7 +129,8 @@ public: 
 virtual void WriteDefaultLinkPolicySettings(uint16_t default_link_policy_settings);



 // Callback from Advertising Manager to notify the advitiser (local) address

 virtual void OnAdvertisingSetTerminated(ErrorCode status, uint16_t conn_handle, hci::AddressWithType adv_address);

 virtual void OnAdvertisingSetTerminated(

     ErrorCode status, uint16_t conn_handle, uint8_t adv_set_id, hci::AddressWithType adv_address);



 // In order to avoid circular dependency use setter rather than module dependency.

 virtual void SetSecurityModule(security::SecurityModule* security_module);

system/gd/hci/acl_manager/le_acl_connection.cc

+59 −14
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */



#include "hci/acl_manager/le_acl_connection.h"



#include "hci/acl_manager/le_connection_management_callbacks.h"

#include "os/metrics.h"
@@ -73,9 +74,6 @@ class LeAclConnectionTracker : public LeConnectionManagementCallbacks { 
  void OnPhyUpdate(hci::ErrorCode hci_status, uint8_t tx_phy, uint8_t rx_phy) override {

    SAVE_OR_CALL(OnPhyUpdate, hci_status, tx_phy, rx_phy);

  }

  void OnLocalAddressUpdate(AddressWithType address_with_type) override {

    SAVE_OR_CALL(OnLocalAddressUpdate, address_with_type);

  }

  void OnLeSubrateChange(

      hci::ErrorCode hci_status,

      uint16_t subrate_factor,
@@ -115,20 +113,19 @@ struct LeAclConnection::impl { 
};



LeAclConnection::LeAclConnection()

    : AclConnection(), local_address_(Address::kEmpty, AddressType::PUBLIC_DEVICE_ADDRESS),

      remote_address_(Address::kEmpty, AddressType::PUBLIC_DEVICE_ADDRESS) {}

    : AclConnection(),

      remote_address_(Address::kEmpty, AddressType::PUBLIC_DEVICE_ADDRESS),

      role_specific_data_(DataAsUninitializedPeripheral{}) {}



LeAclConnection::LeAclConnection(

    std::shared_ptr<Queue> queue,

    LeAclConnectionInterface* le_acl_connection_interface,

    uint16_t handle,

    AddressWithType local_address,

    AddressWithType remote_address,

    Role role)

    RoleSpecificData role_specific_data,

    AddressWithType remote_address)

    : AclConnection(queue->GetUpEnd(), handle),

      local_address_(local_address),

      remote_address_(remote_address),

      role_(role) {

      role_specific_data_(role_specific_data) {

  pimpl_ = new LeAclConnection::impl(le_acl_connection_interface, std::move(queue), handle);

}
@@ -137,6 +134,43 @@ LeAclConnection::~LeAclConnection() { 
  delete pimpl_;

}



AddressWithType LeAclConnection::GetLocalAddress() const {

  return std::visit(

      [](auto&& data) {

        using T = std::decay_t<decltype(data)>;

        if constexpr (std::is_same_v<T, DataAsUninitializedPeripheral>) {

          // This case should never happen outside of acl_manager.cc, since once the connection is

          // passed into the OnConnectSuccess callback, it should be fully initialized.

          LOG_ALWAYS_FATAL("Attempted to read the local address of an uninitialized connection");

          return AddressWithType{};

        } else {

          return data.local_address;

        }

      },

      role_specific_data_);

}



Role LeAclConnection::GetRole() const {

  return std::visit(

      [](auto&& data) {

        using T = std::decay_t<decltype(data)>;

        if constexpr (std::is_same_v<T, DataAsCentral>) {

          return Role::CENTRAL;

        } else if constexpr (

            std::is_same_v<T, DataAsPeripheral> ||

            std::is_same_v<T, DataAsUninitializedPeripheral>) {

          return Role::PERIPHERAL;

        } else {

          static_assert(!sizeof(T*), "missing case");

        }

      },

      role_specific_data_);

}



const RoleSpecificData& LeAclConnection::GetRoleSpecificData() const {

  return role_specific_data_;

}



void LeAclConnection::RegisterCallbacks(LeConnectionManagementCallbacks* callbacks, os::Handler* handler) {

  return pimpl_->tracker.RegisterCallbacks(callbacks, handler);

}
@@ -178,15 +212,26 @@ LeConnectionManagementCallbacks* LeAclConnection::GetEventCallbacks( 
  return pimpl_->GetEventCallbacks(std::move(invalidate_callbacks));

}



bool LeAclConnection::LeConnectionUpdate(uint16_t conn_interval_min, uint16_t conn_interval_max, uint16_t conn_latency,

                                         uint16_t supervision_timeout, uint16_t min_ce_length, uint16_t max_ce_length) {

bool LeAclConnection::LeConnectionUpdate(

    uint16_t conn_interval_min,

    uint16_t conn_interval_max,

    uint16_t conn_latency,

    uint16_t supervision_timeout,

    uint16_t min_ce_length,

    uint16_t max_ce_length) {

  if (!check_connection_parameters(conn_interval_min, conn_interval_max, conn_latency, supervision_timeout)) {

    LOG_ERROR("Invalid parameter");

    return false;

  }

  pimpl_->tracker.le_acl_connection_interface_->EnqueueCommand(

      LeConnectionUpdateBuilder::Create(handle_, conn_interval_min, conn_interval_max, conn_latency,

                                        supervision_timeout, min_ce_length, max_ce_length),

      LeConnectionUpdateBuilder::Create(

          handle_,

          conn_interval_min,

          conn_interval_max,

          conn_latency,

          supervision_timeout,

          min_ce_length,

          max_ce_length),

      pimpl_->tracker.client_handler_->BindOnce([](CommandStatusView status) {

        ASSERT(status.IsValid());

        ASSERT(status.GetCommandOpCode() == OpCode::LE_CONNECTION_UPDATE);

system/gd/hci/acl_manager/le_acl_connection.h

+38 −16
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ 


#include <atomic>

#include <memory>

#include <variant>



#include "hci/acl_manager/acl_connection.h"

#include "hci/acl_manager/le_connection_management_callbacks.h"
@@ -29,6 +30,26 @@ namespace bluetooth { 
namespace hci {

namespace acl_manager {



struct DataAsCentral {

  // the address used when initiating the connection

  AddressWithType local_address;

};



struct DataAsPeripheral {

  // the address of the advertising set that the peer connected to

  AddressWithType local_address;

  // the advertising set ID that the peer connected to - in LE, our role is peripheral iff the peer

  // initiated a connection to our advertisement

  std::optional<uint8_t> advertising_set_id;

};



// when we know it's a peripheral, but we don't yet have all the data about the set it connected to

// this state should never remain after the connection is fully populated

struct DataAsUninitializedPeripheral {};



using RoleSpecificData =

    std::variant<DataAsUninitializedPeripheral, DataAsCentral, DataAsPeripheral>;



class LeAclConnection : public AclConnection {

 public:

  LeAclConnection();
@@ -36,30 +57,27 @@ class LeAclConnection : public AclConnection { 
      std::shared_ptr<Queue> queue,

      LeAclConnectionInterface* le_acl_connection_interface,

      uint16_t handle,

      AddressWithType local_address,

      AddressWithType remote_address,

      Role role);

      RoleSpecificData role_specific_data,

      AddressWithType remote_address);

  LeAclConnection(const LeAclConnection&) = delete;

  LeAclConnection& operator=(const LeAclConnection&) = delete;



  ~LeAclConnection();



  virtual AddressWithType GetLocalAddress() const {

    return local_address_;

  }

  virtual AddressWithType GetLocalAddress() const;



  virtual void UpdateLocalAddress(AddressWithType address) {

    local_address_ = address;

  virtual Role GetRole() const;



  const RoleSpecificData& GetRoleSpecificData() const;



  void UpdateRoleSpecificData(RoleSpecificData role_specific_data) {

    role_specific_data_ = role_specific_data;

  }



  virtual AddressWithType GetRemoteAddress() const {

    return remote_address_;

  }



  virtual Role GetRole() const {

    return role_;

  }



  // The peer address and type returned from the Connection Complete Event

  AddressWithType peer_address_with_type_;

  Address remote_initiator_address_;
@@ -90,8 +108,13 @@ class LeAclConnection : public AclConnection { 
  virtual void RegisterCallbacks(LeConnectionManagementCallbacks* callbacks, os::Handler* handler);

  virtual void Disconnect(DisconnectReason reason);



  virtual bool LeConnectionUpdate(uint16_t conn_interval_min, uint16_t conn_interval_max, uint16_t conn_latency,

                                  uint16_t supervision_timeout, uint16_t min_ce_length, uint16_t max_ce_length);

  virtual bool LeConnectionUpdate(

      uint16_t conn_interval_min,

      uint16_t conn_interval_max,

      uint16_t conn_latency,

      uint16_t supervision_timeout,

      uint16_t min_ce_length,

      uint16_t max_ce_length);



  virtual bool ReadRemoteVersionInformation() override;

  virtual bool LeReadRemoteFeatures();
@@ -105,9 +128,8 @@ class LeAclConnection : public AclConnection { 
  virtual LeConnectionManagementCallbacks* GetEventCallbacks(std::function<void(uint16_t)> invalidate_callbacks);



 protected:

  AddressWithType local_address_;

  AddressWithType remote_address_;

  Role role_;

  RoleSpecificData role_specific_data_;



 private:

  void OnLeSubrateRequestStatus(CommandStatusView status);

system/gd/hci/acl_manager/le_acl_connection_test.cc

+5 −2
Original line number Diff line number Diff line
@@ -76,7 +76,6 @@ class TestLeConnectionManagementCallbacks : public hci::acl_manager::LeConnectio 
      hci::ErrorCode hci_status, uint8_t lmp_version, uint16_t manufacturer_name, uint16_t sub_version) override {}

  virtual void OnLeReadRemoteFeaturesComplete(hci::ErrorCode hci_status, uint64_t features) override {}

  virtual void OnPhyUpdate(hci::ErrorCode hci_status, uint8_t tx_phy, uint8_t rx_phy) override {}

  virtual void OnLocalAddressUpdate(hci::AddressWithType address_with_type) override {}

  MOCK_METHOD(

      void,

      OnLeSubrateChange,
@@ -172,7 +171,11 @@ class LeAclConnectionTest : public ::testing::Test { 
    queue_ = std::make_shared<LeAclConnection::Queue>(kQueueSize);

    sync_handler();

    connection_ = new LeAclConnection(

        queue_, &le_acl_connection_interface_, kConnectionHandle, address_1, address_2, Role::CENTRAL);

        queue_,

        &le_acl_connection_interface_,

        kConnectionHandle,

        DataAsCentral{address_1},

        address_2);

    connection_->RegisterCallbacks(&callbacks_, handler_);

  }