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

Commit 52e2f6d0 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

leaudio: Prevent from infinite loop on additng to background connect 

Till now, whenever GATT Open failed, the GATT client always received
GATT_ERROR status. Usually this happens only when remote device is not
in a range and it is safe to put device into background connect.

However, it could happen that GATT Client is trying to do background
connect to the device which is not properly paired GATT_Connect does not
go down to connection manager at all. In such case, GATT_ERROR status
was also used which is missleading for the GATT CLient and would endup
in infinite loop.

With this change, such situation is detected and GATT Client can take
decision to not try to connect to the remote device which is incorrectly
paired

This patch also adds additional log to BTM_BackgroundConnectAddressKnown
which helps to understand why provided address is considered as invalid
parameter

Bug: 285191013
Test: atest BluetoothInstrumentationTests
Tag: #feature
Change-Id: Idb70fb12d14042bff624130991690f0912e6741a
parent 349be0e7

system/bta/gatt/bta_gattc_act.cc

+3 −2
Original line number Diff line number Diff line
@@ -418,8 +418,9 @@ static void bta_gattc_init_bk_conn(const tBTA_GATTC_API_OPEN* p_data, 
                    p_data->connection_type, p_data->transport, false)) {

    LOG_ERROR("Unable to connect to remote bd_addr=%s",

              ADDRESS_TO_LOGGABLE_CSTR(p_data->remote_bda));

    bta_gattc_send_open_cback(p_clreg, GATT_ERROR, p_data->remote_bda,

                              GATT_INVALID_CONN_ID, BT_TRANSPORT_LE, 0);

    bta_gattc_send_open_cback(p_clreg, GATT_ILLEGAL_PARAMETER,

                              p_data->remote_bda, GATT_INVALID_CONN_ID,

                              BT_TRANSPORT_LE, 0);

    return;

  }

system/bta/le_audio/client.cc

+6 −5
Original line number Diff line number Diff line
@@ -1897,9 +1897,10 @@ class LeAudioClientImpl : public LeAudioClient { 


    if (status != GATT_SUCCESS) {

      /* autoconnect connection failed, that's ok */

      if (leAudioDevice->GetConnectionState() ==

      if (status != GATT_ILLEGAL_PARAMETER &&

          (leAudioDevice->GetConnectionState() ==

               DeviceConnectState::CONNECTING_AUTOCONNECT ||

          leAudioDevice->autoconnect_flag_) {

           leAudioDevice->autoconnect_flag_)) {

        LOG_INFO("Device not available now, do background connect.");

        leAudioDevice->SetConnectionState(DeviceConnectState::DISCONNECTED);

        AddToBackgroundConnectCheckGroupConnected(leAudioDevice);
@@ -1908,8 +1909,8 @@ class LeAudioClientImpl : public LeAudioClient { 


      leAudioDevice->SetConnectionState(DeviceConnectState::DISCONNECTED);



      LOG(ERROR) << "Failed to connect to LeAudio leAudioDevice, status: "

                 << +status;

      LOG_ERROR("Failed to connect to LeAudio leAudioDevice, status: 0x%02x",

                status);

      callbacks_->OnConnectionState(ConnectionState::DISCONNECTED, address);

      le_audio::MetricsCollector::Get()->OnConnectionStateChanged(

          leAudioDevice->group_id_, address, ConnectionState::CONNECTED,

system/bta/le_audio/le_audio_client_test.cc

+130 −0
Original line number Diff line number Diff line
@@ -2615,6 +2615,136 @@ TEST_F(UnicastTest, ConnectTwoEarbudsCsisGroupUnknownAtConnect) { 
  DisconnectLeAudio(test_address1, 2);

}



TEST_F(UnicastTestNoInit, ConnectFailedDueToInvalidParameters) {

  // Prepare two devices

  uint8_t group_size = 2;

  uint8_t group_id = 2;



  /* Prepare  mock to not inject connect event so the device can stay in

   * CONNECTING state*/

  ON_CALL(mock_gatt_interface_, Open(_, _, BTM_BLE_DIRECT_CONNECTION, false))

      .WillByDefault(DoAll(Return()));



  const RawAddress test_address0 = GetTestAddress(0);

  SetSampleDatabaseEarbudsValid(

      1, test_address0, codec_spec_conf::kLeAudioLocationFrontLeft,

      codec_spec_conf::kLeAudioLocationFrontLeft, default_channel_cnt,

      default_channel_cnt, 0x0004,

      /* source sample freq 16khz */ true, /*add_csis*/

      true,                                /*add_cas*/

      true,                                /*add_pacs*/

      default_ase_cnt,                     /*add_ascs_cnt*/

      group_size, 1);



  const RawAddress test_address1 = GetTestAddress(1);

  SetSampleDatabaseEarbudsValid(

      2, test_address1, codec_spec_conf::kLeAudioLocationFrontRight,

      codec_spec_conf::kLeAudioLocationFrontRight, default_channel_cnt,

      default_channel_cnt, 0x0004,

      /* source sample freq 16khz */ true, /*add_csis*/

      true,                                /*add_cas*/

      true,                                /*add_pacs*/

      default_ase_cnt,                     /*add_ascs_cnt*/

      group_size, 2);



  // Load devices from the storage when storage API is called

  bool autoconnect = true;



  /* Common storage values */

  std::vector<uint8_t> handles;

  LeAudioClient::GetHandlesForStorage(test_address0, handles);



  std::vector<uint8_t> ases;

  LeAudioClient::GetAsesForStorage(test_address0, ases);



  std::vector<uint8_t> src_pacs;

  LeAudioClient::GetSourcePacsForStorage(test_address0, src_pacs);



  std::vector<uint8_t> snk_pacs;

  LeAudioClient::GetSinkPacsForStorage(test_address0, snk_pacs);



  EXPECT_CALL(mock_storage_load, Call()).WillOnce([&]() {

    do_in_main_thread(

        FROM_HERE,

        base::Bind(&LeAudioClient::AddFromStorage, test_address0, autoconnect,

                   codec_spec_conf::kLeAudioLocationFrontLeft,

                   codec_spec_conf::kLeAudioLocationFrontLeft, 0xff, 0xff,

                   std::move(handles), std::move(snk_pacs), std::move(src_pacs),

                   std::move(ases)));

    do_in_main_thread(

        FROM_HERE,

        base::Bind(&LeAudioClient::AddFromStorage, test_address1, autoconnect,

                   codec_spec_conf::kLeAudioLocationFrontRight,

                   codec_spec_conf::kLeAudioLocationFrontRight, 0xff, 0xff,

                   std::move(handles), std::move(snk_pacs), std::move(src_pacs),

                   std::move(ases)));

  });



  // Expect stored device0 to connect automatically (first directed connection )

  EXPECT_CALL(mock_gatt_interface_,

              Open(gatt_if, test_address0, BTM_BLE_DIRECT_CONNECTION, _))

      .Times(1);



  // Expect stored device1 to connect automatically (first direct connection)

  EXPECT_CALL(mock_gatt_interface_,

              Open(gatt_if, test_address1, BTM_BLE_DIRECT_CONNECTION, _))

      .Times(1);



  ON_CALL(mock_btm_interface_, BTM_IsEncrypted(test_address1, _))

      .WillByDefault(DoAll(Return(true)));

  ON_CALL(mock_btm_interface_, BTM_IsEncrypted(test_address0, _))

      .WillByDefault(DoAll(Return(true)));



  ON_CALL(mock_groups_module_, GetGroupId(_, _))

      .WillByDefault(DoAll(Return(group_id)));



  ON_CALL(mock_btm_interface_,

          GetSecurityFlagsByTransport(test_address0, NotNull(), _))

      .WillByDefault(

          DoAll(SetArgPointee<1>(BTM_SEC_FLAG_ENCRYPTED), Return(true)));



  std::vector<::bluetooth::le_audio::btle_audio_codec_config_t>

      framework_encode_preference;



  // Initialize

  BtaAppRegisterCallback app_register_callback;

  ON_CALL(mock_gatt_interface_, AppRegister(_, _, _))

      .WillByDefault(DoAll(SaveArg<0>(&gatt_callback),

                           SaveArg<1>(&app_register_callback)));

  LeAudioClient::Initialize(

      &mock_audio_hal_client_callbacks_,

      base::Bind([](MockFunction<void()>* foo) { foo->Call(); },

                 &mock_storage_load),

      base::Bind([](MockFunction<bool()>* foo) { return foo->Call(); },

                 &mock_hal_2_1_verifier),

      framework_encode_preference);

  if (app_register_callback) app_register_callback.Run(gatt_if, GATT_SUCCESS);



  // We need to wait for the storage callback before verifying stuff

  SyncOnMainLoop();

  ASSERT_TRUE(LeAudioClient::IsLeAudioClientRunning());

  Mock::VerifyAndClearExpectations(&mock_gatt_interface_);



  // Simulate connect parameters are invalid and phone does not fallback

  // to background connect.

  EXPECT_CALL(mock_gatt_interface_,

              Open(gatt_if, test_address0,

                   BTM_BLE_BKG_CONNECT_TARGETED_ANNOUNCEMENTS, _))

      .Times(0);



  EXPECT_CALL(mock_gatt_interface_,

              Open(gatt_if, test_address1,

                   BTM_BLE_BKG_CONNECT_TARGETED_ANNOUNCEMENTS, _))

      .Times(0);



  // Devices not found

  InjectConnectedEvent(test_address0, 0, GATT_ILLEGAL_PARAMETER);

  InjectConnectedEvent(test_address1, 0, GATT_ILLEGAL_PARAMETER);



  SyncOnMainLoop();

  Mock::VerifyAndClearExpectations(&mock_gatt_interface_);

}



TEST_F(UnicastTestNoInit, LoadStoredEarbudsCsisGrouped) {

  // Prepare two devices

  uint8_t group_size = 2;

system/stack/btm/btm_ble_bgconn.cc

+6 −0
Original line number Diff line number Diff line
@@ -166,6 +166,9 @@ bool BTM_BackgroundConnectAddressKnown(const RawAddress& address) { 
  if (p_dev_rec == NULL || (p_dev_rec->device_type & BT_DEVICE_TYPE_BLE) == 0)

    return true;



  LOG_WARN("%s, device type not BLE: 0x%02x", ADDRESS_TO_LOGGABLE_CSTR(address),

           p_dev_rec->device_type);



  // bonded device with identity address known

  if (!p_dev_rec->ble.identity_address_with_type.bda.IsEmpty()) {

    return true;
@@ -177,6 +180,9 @@ bool BTM_BackgroundConnectAddressKnown(const RawAddress& address) { 
    return true;

  }



  LOG_WARN("%s, the address type is 0x%02x", ADDRESS_TO_LOGGABLE_CSTR(address),

           p_dev_rec->ble.AddressType());



  // Only Resolvable Private Address (RPA) is known, we don't allow it into

  // the background connection procedure.

  return false;