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

Commit b72f776f authored by Mariusz Skamra's avatar Mariusz Skamra
Browse files

btm: btm_iso: Fix handling cancelled Create CIS procedure 

This fixes kIsoEventCisDisconnected event that was not generated
when the CIS creation has been cancelled by local host.

As per Core 5.4 Vol 4, Part E, 7.1.6 Disconnect Command:
"If this command is issued for a CIS on the Central and the CIS
is successfully terminated before being established, then an
HCI_LE_CIS_Established event shall also be sent for this CIS with
the Status Operation Cancelled by Host (0x44)."

Bug: 357472821
Flag: Exempt; regression covered by unit test added
Test: atest net_test_btm_iso
Change-Id: I7512e5ff26f40ef4f58f0b7f1473ba07170a7582
parent db44a2c7

system/stack/btm/btm_iso_impl.h

+12 −3
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ static constexpr uint8_t kStateFlagIsConnecting = 0x01; 
static constexpr uint8_t kStateFlagIsConnected = 0x02;

static constexpr uint8_t kStateFlagHasDataPathSet = 0x04;

static constexpr uint8_t kStateFlagIsBroadcast = 0x10;

static constexpr uint8_t kStateFlagIsCancelled = 0x20;



constexpr char kBtmLogTag[] = "ISO";
@@ -282,8 +283,9 @@ struct iso_impl { 
    for (auto& el : conn_params.conn_pairs) {

      auto cis = GetCisIfKnown(el.cis_conn_handle);

      log::assert_that(cis, "No such cis: {}", el.cis_conn_handle);

      log::assert_that(!(cis->state_flags & (kStateFlagIsConnected | kStateFlagIsConnecting)),

                       "cis: {} is already connected or connecting flags: {}, "

      log::assert_that(!(cis->state_flags &

                         (kStateFlagIsConnected | kStateFlagIsConnecting | kStateFlagIsCancelled)),

                       "cis: {} is already connected/connecting/cancelled flags: {}, "

                       "num of cis params: {}",

                       el.cis_conn_handle, cis->state_flags, conn_params.conn_pairs.size());
@@ -307,6 +309,12 @@ struct iso_impl { 
    log::assert_that(

            cis->state_flags & kStateFlagIsConnected || cis->state_flags & kStateFlagIsConnecting,

            "Not connected");



    if (cis->state_flags & kStateFlagIsConnecting) {

      cis->state_flags &= ~kStateFlagIsConnecting;

      cis->state_flags |= kStateFlagIsCancelled;

    }



    bluetooth::legacy::hci::GetInterface().Disconnect(cis_handle, static_cast<tHCI_STATUS>(reason));



    BTM_LogHistory(kBtmLogTag, cis_hdl_to_addr[cis_handle], "Disconnect CIS ",
@@ -603,7 +611,7 @@ struct iso_impl { 
                                      hci_error_code_text((tHCI_REASON)(reason)).c_str()));

    cis_hdl_to_addr.erase(handle);



    if (cis->state_flags & kStateFlagIsConnected) {

    if (cis->state_flags & kStateFlagIsConnected || cis->state_flags & kStateFlagIsCancelled) {

      cis_disconnected_evt evt = {

              .reason = reason,

              .cig_id = cis->cig_id,
@@ -612,6 +620,7 @@ struct iso_impl { 


      cig_callbacks_->OnCisEvent(kIsoEventCisDisconnected, &evt);

      cis->state_flags &= ~kStateFlagIsConnected;

      cis->state_flags &= ~kStateFlagIsCancelled;



      /* return used credits */

      iso_credits_ += cis->used_credits;

system/stack/include/hci_error_code.h

+3 −1
Original line number Diff line number Diff line
@@ -68,8 +68,9 @@ typedef enum : uint8_t { 
  HCI_ERR_ADVERTISING_TIMEOUT = 0x3C,        // stack/btm/btm_ble

  HCI_ERR_CONN_FAILED_ESTABLISHMENT = 0x3E,  // GATT_CONN_FAIL_ESTABLISH

  HCI_ERR_LIMIT_REACHED = 0x43,              // stack/btm/btm_ble_multi_adv.cc

  HCI_ERR_CANCELLED_BY_LOCAL_HOST = 0x44,    // stack/btm/btm_iso_impl.h



  _HCI_ERR_MAX_ERR = 0x43,

  _HCI_ERR_MAX_ERR = 0x44,

  HCI_ERR_UNDEFINED = 0xff,

} tHCI_ERROR_CODE;
@@ -117,6 +118,7 @@ inline std::string hci_error_code_text(const tHCI_ERROR_CODE& error_code) { 
    CASE_RETURN_TEXT(HCI_ERR_ADVERTISING_TIMEOUT);

    CASE_RETURN_TEXT(HCI_ERR_CONN_FAILED_ESTABLISHMENT);

    CASE_RETURN_TEXT(HCI_ERR_LIMIT_REACHED);

    CASE_RETURN_TEXT(HCI_ERR_CANCELLED_BY_LOCAL_HOST);

    default:

      return base::StringPrintf("UNKNOWN[0x%02hx]", error_code);

  }

system/stack/test/btm_iso_test.cc

+167 −1
Original line number Diff line number Diff line
@@ -932,7 +932,7 @@ TEST_F(IsoManagerDeathTest, ConnectSameCisTwice) { 
  IsoManager::GetInstance()->EstablishCis(params);



  ASSERT_EXIT(IsoManager::GetInstance()->IsoManager::GetInstance()->EstablishCis(params),

              ::testing::KilledBySignal(SIGABRT), "already connected or connecting");

              ::testing::KilledBySignal(SIGABRT), "already connected/connecting/cancelled");

}



TEST_F(IsoManagerDeathTest, EstablishCisInvalidResponsePacket) {
@@ -1133,6 +1133,172 @@ TEST_F(IsoManagerTest, EstablishCisLateArrivingCallback) { 
  std::move(iso_cb).Run(buf.data(), buf.size());

}



TEST_F(IsoManagerTest, CancelPendingCreateCis_EstablishedThenDisconnected) {

  /**

   * Verify the HCI Disconnect command will cancel pending CIS creation.

   * As the Core is not strict about event order, in this scenario HCI CIS Established event comes

   * before HCI Disconnection Complete event.

   *

   * Scenario:

   * 1. Issue the HCI LE Create CIS command.

   * 2. Issue HCI Disconnect command with CIS connection handle parameter before the HCI CIS

   *    Established event is received.

   * 3. Verify the kIsoEventCisEstablishCmpl event is generated once HCI CIS Established event is

   *    received with Operation Cancelled By Local Host error.

   * 4. Verify the kIsoEventCisDisconnected event is generated once HCI Disconnection Complete event

   *    is received.

   */



  IsoManager::GetInstance()->CreateCig(volatile_test_cig_create_cmpl_evt_.cig_id,

                                       kDefaultCigParams);

  ON_CALL(hcic_interface_, CreateCis)

          .WillByDefault([](uint8_t, const EXT_CIS_CREATE_CFG*,

                            base::OnceCallback<void(uint8_t*, uint16_t)> /* cb */) {

            /* We override default mock. Nothing to do here */

          });



  ON_CALL(hcic_interface_, Disconnect).WillByDefault([](uint16_t, uint8_t) {

    /* We override default mock. Nothing to do here */

  });



  EXPECT_CALL(*cig_callbacks_,

              OnCisEvent(bluetooth::hci::iso_manager::kIsoEventCisEstablishCmpl, _))

          .Times(kDefaultCigParams.cis_cfgs.size())

          .WillRepeatedly([this](uint8_t /* type */, void* data) {

            auto* event = static_cast<bluetooth::hci::iso_manager::cis_establish_cmpl_evt*>(data);

            ASSERT_EQ(event->status, HCI_ERR_CANCELLED_BY_LOCAL_HOST);

            ASSERT_EQ(event->cig_id, volatile_test_cig_create_cmpl_evt_.cig_id);

            ASSERT_TRUE(std::find(volatile_test_cig_create_cmpl_evt_.conn_handles.begin(),

                                  volatile_test_cig_create_cmpl_evt_.conn_handles.end(),

                                  event->cis_conn_hdl) !=

                        volatile_test_cig_create_cmpl_evt_.conn_handles.end());

          });



  EXPECT_CALL(*cig_callbacks_, OnCisEvent(bluetooth::hci::iso_manager::kIsoEventCisDisconnected, _))

          .Times(kDefaultCigParams.cis_cfgs.size())

          .WillRepeatedly([this](uint8_t /* type */, void* data) {

            auto* event = static_cast<bluetooth::hci::iso_manager::cis_disconnected_evt*>(data);

            ASSERT_EQ(event->reason, HCI_ERR_CONN_CAUSE_LOCAL_HOST);

            ASSERT_EQ(event->cig_id, volatile_test_cig_create_cmpl_evt_.cig_id);

            ASSERT_TRUE(std::find(volatile_test_cig_create_cmpl_evt_.conn_handles.begin(),

                                  volatile_test_cig_create_cmpl_evt_.conn_handles.end(),

                                  event->cis_conn_hdl) !=

                        volatile_test_cig_create_cmpl_evt_.conn_handles.end());

          });



  EXPECT_CALL(hcic_interface_, CreateCis).Times(1);



  // Establish all CISes before setting up their data paths

  bluetooth::hci::iso_manager::cis_establish_params params;

  for (auto& handle : volatile_test_cig_create_cmpl_evt_.conn_handles) {

    params.conn_pairs.push_back({handle, 1});

  }

  IsoManager::GetInstance()->EstablishCis(params);



  EXPECT_CALL(hcic_interface_, Disconnect).Times(kDefaultCigParams.cis_cfgs.size());



  /* Cancel pending HCI LE Create CIS command */

  for (auto& handle : volatile_test_cig_create_cmpl_evt_.conn_handles) {

    IsoManager::GetInstance()->DisconnectCis(handle, HCI_ERR_CONN_CAUSE_LOCAL_HOST);

  }



  for (auto& handle : volatile_test_cig_create_cmpl_evt_.conn_handles) {

    std::vector<uint8_t> buf(28, 0);

    uint8_t* p = buf.data();

    UINT8_TO_STREAM(p, HCI_ERR_CANCELLED_BY_LOCAL_HOST);

    UINT16_TO_STREAM(p, handle);



    /* inject HCI LE CIS Established event */

    IsoManager::GetInstance()->HandleHciEvent(HCI_BLE_CIS_EST_EVT, buf.data(), buf.size());



    /* followed by HCI Disconnection Complete event */

    IsoManager::GetInstance()->HandleDisconnect(handle, HCI_ERR_CONN_CAUSE_LOCAL_HOST);

  }

}



TEST_F(IsoManagerTest, CancelPendingCreateCis_DisconnectedThenEstablished) {

  /**

   * Verify the HCI Disconnect command will cancel pending CIS creation.

   * As the Core is not strict about event order, in this scenario HCI Disconnection Complete event

   * comes before HCI CIS Established event.

   *

   * Scenario:

   * 1. Issue the HCI LE Create CIS command.

   * 2. Issue HCI Disconnect command with CIS connection handle parameter before the HCI CIS

   *    Established event is received.

   * 3. Verify the kIsoEventCisEstablishCmpl event is generated once HCI CIS Established event is

   *    received with Operation Cancelled By Local Host error.

   * 4. Verify the kIsoEventCisDisconnected event is generated once HCI Disconnection Complete event

   *    is received.

   */



  IsoManager::GetInstance()->CreateCig(volatile_test_cig_create_cmpl_evt_.cig_id,

                                       kDefaultCigParams);

  ON_CALL(hcic_interface_, CreateCis)

          .WillByDefault([](uint8_t, const EXT_CIS_CREATE_CFG*,

                            base::OnceCallback<void(uint8_t*, uint16_t)> /* cb */) {

            /* We override default mock. Nothing to do here */

          });



  ON_CALL(hcic_interface_, Disconnect).WillByDefault([](uint16_t, uint8_t) {

    /* We override default mock. Nothing to do here */

  });



  EXPECT_CALL(*cig_callbacks_,

              OnCisEvent(bluetooth::hci::iso_manager::kIsoEventCisEstablishCmpl, _))

          .Times(kDefaultCigParams.cis_cfgs.size())

          .WillRepeatedly([this](uint8_t /* type */, void* data) {

            auto* event = static_cast<bluetooth::hci::iso_manager::cis_establish_cmpl_evt*>(data);

            ASSERT_EQ(event->status, HCI_ERR_CANCELLED_BY_LOCAL_HOST);

            ASSERT_EQ(event->cig_id, volatile_test_cig_create_cmpl_evt_.cig_id);

            ASSERT_TRUE(std::find(volatile_test_cig_create_cmpl_evt_.conn_handles.begin(),

                                  volatile_test_cig_create_cmpl_evt_.conn_handles.end(),

                                  event->cis_conn_hdl) !=

                        volatile_test_cig_create_cmpl_evt_.conn_handles.end());

          });



  EXPECT_CALL(*cig_callbacks_, OnCisEvent(bluetooth::hci::iso_manager::kIsoEventCisDisconnected, _))

          .Times(kDefaultCigParams.cis_cfgs.size())

          .WillRepeatedly([this](uint8_t /* type */, void* data) {

            auto* event = static_cast<bluetooth::hci::iso_manager::cis_disconnected_evt*>(data);

            ASSERT_EQ(event->reason, HCI_ERR_CONN_CAUSE_LOCAL_HOST);

            ASSERT_EQ(event->cig_id, volatile_test_cig_create_cmpl_evt_.cig_id);

            ASSERT_TRUE(std::find(volatile_test_cig_create_cmpl_evt_.conn_handles.begin(),

                                  volatile_test_cig_create_cmpl_evt_.conn_handles.end(),

                                  event->cis_conn_hdl) !=

                        volatile_test_cig_create_cmpl_evt_.conn_handles.end());

          });



  EXPECT_CALL(hcic_interface_, CreateCis).Times(1);



  // Establish all CISes before setting up their data paths

  bluetooth::hci::iso_manager::cis_establish_params params;

  for (auto& handle : volatile_test_cig_create_cmpl_evt_.conn_handles) {

    params.conn_pairs.push_back({handle, 1});

  }

  IsoManager::GetInstance()->EstablishCis(params);



  EXPECT_CALL(hcic_interface_, Disconnect).Times(kDefaultCigParams.cis_cfgs.size());



  /* Cancel pending HCI LE Create CIS command */

  for (auto& handle : volatile_test_cig_create_cmpl_evt_.conn_handles) {

    IsoManager::GetInstance()->DisconnectCis(handle, HCI_ERR_CONN_CAUSE_LOCAL_HOST);

  }



  for (auto& handle : volatile_test_cig_create_cmpl_evt_.conn_handles) {

    /* inject HCI Disconnection Complete event */

    IsoManager::GetInstance()->HandleDisconnect(handle, HCI_ERR_CONN_CAUSE_LOCAL_HOST);



    std::vector<uint8_t> buf(28, 0);

    uint8_t* p = buf.data();

    UINT8_TO_STREAM(p, HCI_ERR_CANCELLED_BY_LOCAL_HOST);

    UINT16_TO_STREAM(p, handle);



    /* followed by inject HCI LE CIS Established event */

    IsoManager::GetInstance()->HandleHciEvent(HCI_BLE_CIS_EST_EVT, buf.data(), buf.size());

  }

}



TEST_F(IsoManagerTest, ReconnectCisValid) {

  IsoManager::GetInstance()->CreateCig(volatile_test_cig_create_cmpl_evt_.cig_id,

                                       kDefaultCigParams);