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

Commit b45c4285 authored by Jakub Pawlowski's avatar Jakub Pawlowski Committed by Jakub Pawłowski
Browse files

Get rid of on_name_read discovery callback

Name read happens during device search, not service discovery.
State machines were separated, so name and service discovery are now
separate.

Name read can be scheduled by btm_sec while connecting to encrypted
Classic link. If name is read this way, pass it to btif layer directly.

Bug: 330675788
Bug: 335732980
Flag: com.android.bluetooth.flags.separate_service_and_device_discovery
Test: mma -j32
Change-Id: Idfc3ded8dad8b77f457f817a7c7ab8beb2d5a894
parent 79ef6b42
Loading
Loading
Loading
Loading
+10 −99
Original line number Diff line number Diff line
@@ -102,8 +102,6 @@ static void bta_dm_inq_results_cb(tBTM_INQ_RESULTS* p_inq, const uint8_t* p_eir,
                                  uint16_t eir_len);
static void bta_dm_inq_cmpl();
static void bta_dm_inq_cmpl_cb(void* p_result);
static void bta_dm_service_search_remname_cback(const RawAddress& bd_addr,
                                                DEV_CLASS dc, BD_NAME bd_name);
static void bta_dm_remname_cback(const tBTM_REMOTE_DEV_NAME* p);
static void bta_dm_find_services(const RawAddress& bd_addr);
static void bta_dm_discover_next_device(void);
@@ -477,14 +475,6 @@ static bool bta_dm_read_remote_device_name(const RawAddress& bd_addr,
    return (true);
  } else if (btm_status == BTM_BUSY) {
    log::verbose("BTM_ReadRemoteDeviceName is busy");

    /* Remote name discovery is on going now so BTM cannot notify through
     * "bta_dm_remname_cback" */
    /* adding callback to get notified that current reading remote name done */

    get_btm_client_interface().security.BTM_SecAddRmtNameNotifyCallback(
        &bta_dm_service_search_remname_cback);

    return (true);
  } else {
    log::warn("BTM_ReadRemoteDeviceName returns 0x{:02X}", btm_status);
@@ -545,10 +535,6 @@ static void bta_dm_remote_name_cmpl(
  }

  // Callback with this property
  if (bta_dm_search_cb.p_device_search_cback != nullptr ||
      bta_dm_search_cb.service_search_cbacks.on_name_read != nullptr) {
    // Both device and service search callbacks end up sending event to java.
    // It's enough to send callback to just one of them.
  if (bta_dm_search_cb.p_device_search_cback != nullptr) {
    tBTA_DM_SEARCH search_data = {
        .name_res = {.bd_addr = remote_name_msg.bd_addr, .bd_name = {}},
@@ -556,13 +542,7 @@ static void bta_dm_remote_name_cmpl(
    if (remote_name_msg.hci_status == HCI_SUCCESS) {
      bd_name_copy(search_data.name_res.bd_name, remote_name_msg.bd_name);
    }
      bta_dm_search_cb.p_device_search_cback(BTA_DM_NAME_READ_EVT,
                                             &search_data);
    } else if (bta_dm_search_cb.service_search_cbacks.on_name_read != nullptr) {
      bta_dm_search_cb.service_search_cbacks.on_name_read(
          remote_name_msg.bd_addr, remote_name_msg.hci_status,
          remote_name_msg.bd_name);
    }
    bta_dm_search_cb.p_device_search_cback(BTA_DM_NAME_READ_EVT, &search_data);
  } else {
    log::warn("Received remote name complete without callback");
  }
@@ -709,11 +689,8 @@ static void bta_dm_sdp_result(tBTA_DM_SDP_RESULT& sdp_event) {
          log::info("GATT services discovered using SDP");

          // send all result back to app
          BD_NAME bd_name;
          bd_name_from_char_pointer(bd_name, bta_dm_get_remname());

          bta_dm_search_cb.service_search_cbacks.on_gatt_results(
              bta_dm_search_cb.peer_bdaddr, bd_name, gatt_uuids,
              bta_dm_search_cb.peer_bdaddr, BD_NAME{}, gatt_uuids,
              /* transport_le */ false);
        }
      } else {
@@ -780,10 +757,6 @@ static void bta_dm_sdp_result(tBTA_DM_SDP_RESULT& sdp_event) {
    } else {
      /* callbacks */
      /* start next bd_addr if necessary */

      get_btm_client_interface().security.BTM_SecDeleteRmtNameNotifyCallback(
          &bta_dm_service_search_remname_cback);

      BTM_LogHistory(
          kBtmLogTag, bta_dm_search_cb.peer_bdaddr, "Discovery completed",
          base::StringPrintf("Result:%s services_found:0x%x service_index:0x%d",
@@ -840,9 +813,6 @@ static void bta_dm_sdp_result(tBTA_DM_SDP_RESULT& sdp_event) {
    if (bta_dm_search_cb.p_sdp_db)
      osi_free_and_reset((void**)&bta_dm_search_cb.p_sdp_db);

    get_btm_client_interface().security.BTM_SecDeleteRmtNameNotifyCallback(
        &bta_dm_service_search_remname_cback);

    auto msg = std::make_unique<tBTA_DM_MSG>(tBTA_DM_SVC_RES{});
    auto& disc_result = std::get<tBTA_DM_SVC_RES>(*msg);

@@ -919,10 +889,8 @@ static void bta_dm_service_discovery_cmpl() {
    if (bta_dm_search_cb.service_search_cbacks.on_gatt_results != nullptr) {
      log::info("Sending GATT results to upper layer");

      BD_NAME bd_name;
      bd_name_from_char_pointer(bd_name, bta_dm_get_remname());
      bta_dm_search_cb.service_search_cbacks.on_gatt_results(
          bta_dm_search_cb.peer_bdaddr, bd_name, gatt_services,
          bta_dm_search_cb.peer_bdaddr, BD_NAME{}, gatt_services,
          /* transport_le */ true);
    } else {
      log::warn("on_gatt_results is nullptr!");
@@ -1483,53 +1451,6 @@ static void bta_dm_inq_cmpl_cb(void* /* p_result */) {
  bta_dm_inq_cmpl();
}

/*******************************************************************************
 *
 * Function         bta_dm_service_search_remname_cback
 *
 * Description      Remote name call back from BTM during service discovery
 *
 * Returns          void
 *
 ******************************************************************************/
static void bta_dm_service_search_remname_cback(const RawAddress& bd_addr,
                                                DEV_CLASS /* dc */,
                                                BD_NAME bd_name) {
  tBTM_REMOTE_DEV_NAME rem_name = {};
  tBTM_STATUS btm_status;

  log::verbose("name=<{}>", reinterpret_cast<char const*>(bd_name));

  /* if this is what we are looking for */
  if (bta_dm_search_cb.peer_bdaddr == bd_addr) {
    rem_name.bd_addr = bd_addr;
    bd_name_copy(rem_name.remote_bd_name, bd_name);
    rem_name.status = BTM_SUCCESS;
    rem_name.hci_status = HCI_SUCCESS;
    bta_dm_remname_cback(&rem_name);
  } else {
    /* get name of device */
    btm_status = get_btm_client_interface().peer.BTM_ReadRemoteDeviceName(
        bta_dm_search_cb.peer_bdaddr, bta_dm_remname_cback,
        BT_TRANSPORT_BR_EDR);
    if (btm_status == BTM_BUSY) {
      /* wait for next chance(notification of remote name discovery done) */
      log::verbose("BTM_ReadRemoteDeviceName is busy");
    } else if (btm_status != BTM_CMD_STARTED) {
      /* if failed to start getting remote name then continue */
      log::warn("BTM_ReadRemoteDeviceName returns 0x{:02X}", btm_status);

      // needed so our response is not ignored, since this corresponds to the
      // actual peer_bdaddr
      rem_name.bd_addr = bta_dm_search_cb.peer_bdaddr;
      rem_name.remote_bd_name[0] = 0;
      rem_name.status = btm_status;
      rem_name.hci_status = HCI_SUCCESS;
      bta_dm_remname_cback(&rem_name);
    }
  }
}

/*******************************************************************************
 *
 * Function         bta_dm_remname_cback
@@ -1551,16 +1472,11 @@ static void bta_dm_remname_cback(const tBTM_REMOTE_DEV_NAME* p_remote_name) {
      p_remote_name->remote_bd_name[0],
      strnlen((const char*)p_remote_name->remote_bd_name, BD_NAME_LEN));

  if (bta_dm_search_cb.peer_bdaddr == p_remote_name->bd_addr) {
    get_btm_client_interface().security.BTM_SecDeleteRmtNameNotifyCallback(
        &bta_dm_service_search_remname_cback);
  } else {
  if (bta_dm_search_cb.peer_bdaddr != p_remote_name->bd_addr) {
    // if we got a different response, maybe ignore it
    // we will have made a request directly from BTM_ReadRemoteDeviceName so we
    // expect a dedicated response for us
    if (p_remote_name->hci_status == HCI_ERR_CONNECTION_EXISTS) {
      get_btm_client_interface().security.BTM_SecDeleteRmtNameNotifyCallback(
          &bta_dm_service_search_remname_cback);
      log::info(
          "Assume command failed due to disconnection hci_status:{} peer:{}",
          hci_error_code_text(p_remote_name->hci_status),
@@ -2414,11 +2330,6 @@ void bta_dm_queue_search(tBTA_DM_API_SEARCH& search) {
  ::bta_dm_queue_search(search);
}

void bta_dm_service_search_remname_cback(const RawAddress& bd_addr,
                                         DEV_CLASS dc, BD_NAME bd_name) {
  ::bta_dm_service_search_remname_cback(bd_addr, dc, bd_name);
}

void bta_dm_start_scan(uint8_t duration_sec, bool low_latency_scan = false) {
  ::bta_dm_start_scan(duration_sec, low_latency_scan);
}
+5 −1
Original line number Diff line number Diff line
@@ -283,13 +283,16 @@ typedef union {
/* Search callback */
typedef void(tBTA_DM_SEARCH_CBACK)(tBTA_DM_SEARCH_EVT event,
                                   tBTA_DM_SEARCH* p_data);

// TODO: delete bd_name parameter after separate_service_and_device_discovery
// rolls out
typedef void(tBTA_DM_GATT_DISC_CBACK)(RawAddress bd_addr, BD_NAME bd_name,
                                      std::vector<bluetooth::Uuid>& services,
                                      bool transport_le);
typedef void(tBTA_DM_DID_RES_CBACK)(RawAddress bd_addr, uint8_t vendor_id_src,
                                    uint16_t vendor_id, uint16_t product_id,
                                    uint16_t version);

// TODO: delete after separate_service_and_device_discovery rolls out
typedef void(tBTA_DM_NAME_READ_CBACK)(RawAddress bd_addr,
                                      tHCI_ERROR_CODE hci_status,
                                      const BD_NAME bd_name);
@@ -301,6 +304,7 @@ struct service_discovery_callbacks {
  /* legacy callback I'll tear apart and get rid of */
  tBTA_DM_GATT_DISC_CBACK* on_gatt_results;
  tBTA_DM_DID_RES_CBACK* on_did_received;
  // TODO: delete after separate_service_and_device_discovery rolls out
  tBTA_DM_NAME_READ_CBACK* on_name_read;
  tBTA_DM_DISC_CBACK* on_service_discovery_results;
};
+0 −23
Original line number Diff line number Diff line
@@ -51,8 +51,6 @@ void bta_dm_opportunistic_observe_results_cb(tBTM_INQ_RESULTS* p_inq,
                                             const uint8_t* p_eir,
                                             uint16_t eir_len);
void bta_dm_queue_search(tBTA_DM_API_SEARCH& search);
void bta_dm_service_search_remname_cback(const RawAddress& bd_addr,
                                         DEV_CLASS dc, BD_NAME bd_name);
void bta_dm_start_scan(uint8_t duration_sec, bool low_latency_scan = false);
void store_avrcp_profile_feature(tSDP_DISC_REC* sdp_rec);

@@ -158,27 +156,6 @@ TEST_F(BtaInitializedTest, bta_dm_read_remote_device_name) {
      kRawAddress, BT_TRANSPORT_BR_EDR);
}

TEST_F(BtaInitializedTest, bta_dm_service_search_remname_cback__expected_name) {
  DEV_CLASS dc;
  BD_NAME bd_name;
  tBTA_DM_SEARCH_CB& search_cb =
      bluetooth::legacy::testing::bta_dm_disc_search_cb();
  search_cb.peer_bdaddr = kRawAddress,
  bluetooth::legacy::testing::bta_dm_service_search_remname_cback(kRawAddress,
                                                                  dc, bd_name);
}

TEST_F(BtaInitializedTest,
       bta_dm_service_search_remname_cback__unexpected_name) {
  DEV_CLASS dc;
  BD_NAME bd_name;
  tBTA_DM_SEARCH_CB& search_cb =
      bluetooth::legacy::testing::bta_dm_disc_search_cb();
  search_cb.peer_bdaddr = RawAddress::kAny;
  bluetooth::legacy::testing::bta_dm_service_search_remname_cback(kRawAddress,
                                                                  dc, bd_name);
}

TEST_F(BtaInitializedTest, bta_dm_start_scan) {
  constexpr bool kLowLatencyScan = true;
  constexpr bool kHighLatencyScan = false;
+0 −19
Original line number Diff line number Diff line
@@ -329,16 +329,10 @@ TEST_F(BtaDmTest, bta_dm_remname_cback__typical) {
  };
  bd_name_from_char_pointer(name.remote_bd_name, kRemoteName);

  mock_btm_client_interface.security.BTM_SecDeleteRmtNameNotifyCallback =
      [](tBTM_RMT_NAME_CALLBACK*) -> bool {
    inc_func_call_count("BTM_SecDeleteRmtNameNotifyCallback");
    return true;
  };
  bluetooth::legacy::testing::bta_dm_remname_cback(&name);

  sync_main_handler();

  ASSERT_EQ(1, get_func_call_count("BTM_SecDeleteRmtNameNotifyCallback"));
  ASSERT_TRUE(
      bluetooth::legacy::testing::bta_dm_disc_search_cb().name_discover_done);
}
@@ -359,16 +353,9 @@ TEST_F(BtaDmTest, bta_dm_remname_cback__wrong_address) {
  };
  bd_name_from_char_pointer(name.remote_bd_name, kRemoteName);

  mock_btm_client_interface.security.BTM_SecDeleteRmtNameNotifyCallback =
      [](tBTM_RMT_NAME_CALLBACK*) -> bool {
    inc_func_call_count("BTM_SecDeleteRmtNameNotifyCallback");
    return true;
  };
  bluetooth::legacy::testing::bta_dm_remname_cback(&name);

  sync_main_handler();

  ASSERT_EQ(0, get_func_call_count("BTM_SecDeleteRmtNameNotifyCallback"));
}

TEST_F(BtaDmTest, bta_dm_remname_cback__HCI_ERR_CONNECTION_EXISTS) {
@@ -385,16 +372,10 @@ TEST_F(BtaDmTest, bta_dm_remname_cback__HCI_ERR_CONNECTION_EXISTS) {
  };
  bd_name_from_char_pointer(name.remote_bd_name, kRemoteName);

  mock_btm_client_interface.security.BTM_SecDeleteRmtNameNotifyCallback =
      [](tBTM_RMT_NAME_CALLBACK*) -> bool {
    inc_func_call_count("BTM_SecDeleteRmtNameNotifyCallback");
    return true;
  };
  bluetooth::legacy::testing::bta_dm_remname_cback(&name);

  sync_main_handler();

  ASSERT_EQ(1, get_func_call_count("BTM_SecDeleteRmtNameNotifyCallback"));
  ASSERT_TRUE(
      bluetooth::legacy::testing::bta_dm_disc_search_cb().name_discover_done);
}
+66 −44
Original line number Diff line number Diff line
@@ -297,6 +297,9 @@ static void btif_stats_add_bond_event(const RawAddress& bd_addr,
                                      bt_bond_function_t function,
                                      bt_bond_state_t state);

static void btif_on_name_read(RawAddress bd_addr, tHCI_ERROR_CODE hci_status,
                              const BD_NAME bd_name, bool during_device_search);

/******************************************************************************
 *  Externs
 *****************************************************************************/
@@ -1425,40 +1428,9 @@ static void btif_dm_search_devices_evt(tBTA_DM_SEARCH_EVT event,

  switch (event) {
    case BTA_DM_NAME_READ_EVT: {
      /* Remote name update */
      if (strlen((const char*)p_search_data->name_res.bd_name)) {
        /** Fix inquiry time too long @{ */
        bt_property_t properties[3];
        /** @} */
        bt_status_t status;

        properties[0].type = BT_PROPERTY_BDNAME;
        properties[0].val = p_search_data->name_res.bd_name;
        properties[0].len = strlen((char*)p_search_data->name_res.bd_name);
        RawAddress& bdaddr = p_search_data->name_res.bd_addr;

        status =
            btif_storage_set_remote_device_property(&bdaddr, &properties[0]);
        ASSERTC(status == BT_STATUS_SUCCESS,
                "failed to save remote device property", status);
        GetInterfaceToProfiles()->events->invoke_remote_device_properties_cb(
            status, bdaddr, 1, properties);
        /** Fix inquiry time too long @{ */
        uint32_t cod = get_cod(&bdaddr);
        if (cod != 0) {
          BTIF_STORAGE_FILL_PROPERTY(&properties[1], BT_PROPERTY_BDADDR, sizeof(bdaddr), &bdaddr);
          BTIF_STORAGE_FILL_PROPERTY(&properties[2], BT_PROPERTY_CLASS_OF_DEVICE, sizeof(uint32_t), &cod);
          log::debug("report new device to JNI");
          GetInterfaceToProfiles()->events->invoke_device_found_cb(3, properties);
        } else {
          log::info("Skipping RNR callback because cod is zero addr:{} name:{}",
                    bdaddr,
                    PRIVATE_NAME(reinterpret_cast<char const*>(
                        p_search_data->name_res.bd_name)));
        }
        /** @} */
      }
      /* TODO: Services? */
      btif_on_name_read(p_search_data->name_res.bd_addr, HCI_SUCCESS,
                        p_search_data->name_res.bd_name,
                        true /* duirng_device_search */);
    } break;

    case BTA_DM_INQ_RES_EVT: {
@@ -2019,7 +1991,8 @@ void btif_on_gatt_results(RawAddress bd_addr, BD_NAME bd_name,
  num_properties++;

  /* Remote name update */
  if (strnlen((const char*)bd_name, BD_NAME_LEN)) {
  if (!IS_FLAG_ENABLED(separate_service_and_device_discovery) &&
      strnlen((const char*)bd_name, BD_NAME_LEN)) {
    prop[1].type = BT_PROPERTY_BDNAME;
    prop[1].val = bd_name;
    prop[1].len = strnlen((char*)bd_name, BD_NAME_LEN);
@@ -2041,9 +2014,16 @@ void btif_on_gatt_results(RawAddress bd_addr, BD_NAME bd_name,
      BT_STATUS_SUCCESS, bd_addr, num_properties, prop);
}

void btif_on_name_read(RawAddress bd_addr, tHCI_ERROR_CODE hci_status,
                       const BD_NAME bd_name) {
  if (!IS_FLAG_ENABLED(rnr_present_during_service_discovery)) {
static void btif_on_name_read(RawAddress bd_addr, tHCI_ERROR_CODE hci_status,
                              const BD_NAME bd_name,
                              bool during_device_search) {
  // Differentiate between merged callbacks
  if (!during_device_search
      // New fix after refactor, this callback is needed for the fix to work
      && !IS_FLAG_ENABLED(separate_service_and_device_discovery)
      // Original fix, this callback should not be called if RNR should not be
      // called
      && !IS_FLAG_ENABLED(rnr_present_during_service_discovery)) {
    log::info("Skipping name read event - called on bad callback.");
    return;
  }
@@ -2057,21 +2037,48 @@ void btif_on_name_read(RawAddress bd_addr, tHCI_ERROR_CODE hci_status,
    log::warn("Received RNR event without valid name addr:{}", bd_addr);
    return;
  }
  bt_property_t properties[] = {{

  // Needs 3 properties if during_device_search is true
  bt_property_t properties[3] = {{
      .type = BT_PROPERTY_BDNAME,
      .len = (int)strnlen((char*)bd_name, BD_NAME_LEN),
      .val = (void*)bd_name,
  }};

  const bt_status_t status =
      btif_storage_set_remote_device_property(&bd_addr, properties);
  log::assert_that(status == BT_STATUS_SUCCESS,
                   "Failed to save remote device property status:{}",
                   bt_status_text(status));
  const size_t num_props = sizeof(properties) / sizeof(bt_property_t);
  GetInterfaceToProfiles()->events->invoke_remote_device_properties_cb(
      status, bd_addr, (int)num_props, properties);
      status, bd_addr, 1, properties);
  log::info("Callback for read name event addr:{} name:{}", bd_addr,
            PRIVATE_NAME(reinterpret_cast<char const*>(bd_name)));

  if (!during_device_search) {
    return;
  }

  uint32_t cod = get_cod(&bd_addr);
  if (cod != 0) {
    BTIF_STORAGE_FILL_PROPERTY(&properties[1], BT_PROPERTY_BDADDR,
                               sizeof(bd_addr), &bd_addr);
    BTIF_STORAGE_FILL_PROPERTY(&properties[2], BT_PROPERTY_CLASS_OF_DEVICE,
                               sizeof(uint32_t), &cod);
    log::debug("report new device to JNI");
    GetInterfaceToProfiles()->events->invoke_device_found_cb(3, properties);
  } else {
    log::info(
        "Skipping device found callback because cod is zero addr:{} name:{}",
        bd_addr, PRIVATE_NAME(reinterpret_cast<char const*>(bd_name)));
  }
}

void btif_on_name_read_from_btm(const RawAddress& bd_addr, DEV_CLASS /* dc */,
                                BD_NAME bd_name) {
  log::info("{} {}", bd_addr, reinterpret_cast<char const*>(bd_name));
  btif_on_name_read(bd_addr, HCI_SUCCESS, bd_name,
                    false /* duirng_device_search */);
}

void btif_on_did_received(RawAddress bd_addr, uint8_t vendor_id_src,
@@ -2177,6 +2184,10 @@ void BTIF_dm_enable() {
  log::info("Local BLE Privacy enabled:{}", ble_privacy_enabled);
  BTA_DmBleConfigLocalPrivacy(ble_privacy_enabled);

  if (IS_FLAG_ENABLED(separate_service_and_device_discovery)) {
    BTM_SecAddRmtNameNotifyCallback(btif_on_name_read_from_btm);
  }

  /* for each of the enabled services in the mask, trigger the profile
   * enable */
  tBTA_SERVICE_MASK service_mask = btif_get_enabled_services_mask();
@@ -2202,6 +2213,10 @@ void BTIF_dm_enable() {
}

void BTIF_dm_disable() {
  if (IS_FLAG_ENABLED(separate_service_and_device_discovery)) {
    BTM_SecDeleteRmtNameNotifyCallback(&btif_on_name_read_from_btm);
  }

  /* for each of the enabled services in the mask, trigger the profile
   * disable */
  tBTA_SERVICE_MASK service_mask = btif_get_enabled_services_mask();
@@ -3075,6 +3090,13 @@ bt_status_t btif_dm_get_adapter_property(bt_property_t* prop) {
  return BT_STATUS_SUCCESS;
}

static void btif_on_name_read_legacy(RawAddress bd_addr,
                                     tHCI_ERROR_CODE hci_status,
                                     const BD_NAME bd_name) {
  btif_on_name_read(bd_addr, hci_status, bd_name,
                    false /* during_device_search */);
};

/*******************************************************************************
 *
 * Function         btif_dm_get_remote_services
@@ -3097,7 +3119,7 @@ void btif_dm_get_remote_services(RawAddress remote_addr, const int transport) {
      service_discovery_callbacks{
          .on_gatt_results = btif_on_gatt_results,
          .on_did_received = btif_on_did_received,
          .on_name_read = btif_on_name_read,
          .on_name_read = btif_on_name_read_legacy,
          .on_service_discovery_results = btif_on_service_discovery_results},
      transport);
}
@@ -4229,8 +4251,8 @@ void bta_energy_info_cb(tBTM_BLE_TX_TIME_MS tx_time,
}

void btif_on_name_read(RawAddress bd_addr, tHCI_ERROR_CODE hci_status,
                       const BD_NAME bd_name) {
  ::btif_on_name_read(bd_addr, hci_status, bd_name);
                       const BD_NAME bd_name, bool during_device_search) {
  ::btif_on_name_read(bd_addr, hci_status, bd_name, during_device_search);
}

}  // namespace testing
Loading