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

Commit b255f3b6 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

leaudio: Fix invalid state report from group state machine 

Before sending STREAMING status to client.cc, make sure that Stop group
has not been initiated. Otherwise client.cc would get unexpected message
from state machine and assert.

This patch also adds test which reproduce the issue, and also change the
way state machine handles multiple devices when release ASEs.
From now on, Release command will be send simultaneously, to avoid
getting state machine in wrong state, when  one of the device disconnects
during the Release operation.

Bug: 271957165
Tag: #feature
Test: atest BluetoothInstrumentationTests
Test: atest bluetooth_le_audio_test
Change-Id: If4a917f7347b84a19b115b199e624c40eac7ad35
parent 727d001a

system/bta/le_audio/state_machine.cc

+44 −36
Original line number Diff line number Diff line
@@ -287,20 +287,11 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
      return;

    }



    LeAudioDevice* leAudioDevice = group->GetFirstActiveDevice();

    if (leAudioDevice == nullptr) {

      LOG(ERROR) << __func__

                 << " Shouldn't be called without an active device.";

      state_machine_callbacks_->StatusReportCb(group->group_id_,

                                               GroupStreamStatus::IDLE);

      return;

    }



    /* All Ases should aim to achieve target state */

    SetTargetState(group, AseState::BTA_LE_AUDIO_ASE_STATE_IDLE);

    PrepareAndSendRelease(leAudioDevice);

    state_machine_callbacks_->StatusReportCb(group->group_id_,

                                             GroupStreamStatus::RELEASING);



    auto status = PrepareAndSendReleaseToTheGroup(group);

    state_machine_callbacks_->StatusReportCb(group->group_id_, status);

  }



  void ProcessGattNotifEvent(uint8_t* value, uint16_t len, struct ase* ase,
@@ -661,7 +652,9 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
     * contexts table.

     */

    if (group->IsAnyDeviceConnected() && !group->HaveAllCisesDisconnected()) {

      if (group->GetState() == AseState::BTA_LE_AUDIO_ASE_STATE_STREAMING) {

      if ((group->GetState() == AseState::BTA_LE_AUDIO_ASE_STATE_STREAMING) &&

          (group->GetTargetState() ==

           AseState::BTA_LE_AUDIO_ASE_STATE_STREAMING)) {

        /* We keep streaming but want others to let know user that it might be

         * need to update offloader with new CIS configuration

         */
@@ -1490,7 +1483,6 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
        }

        break;

      case AseState::BTA_LE_AUDIO_ASE_STATE_RELEASING: {

        LeAudioDevice* leAudioDeviceNext;

        ase->state = AseState::BTA_LE_AUDIO_ASE_STATE_IDLE;

        ase->active = false;

        ase->configured_for_context_type =
@@ -1514,16 +1506,16 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
          return;

        }



        leAudioDeviceNext = group->GetNextActiveDevice(leAudioDevice);

        if (!group->HaveAllActiveDevicesAsesTheSameState(

                AseState::BTA_LE_AUDIO_ASE_STATE_IDLE)) {

          LOG_DEBUG("Waiting for more devices to get into idle state");

          return;

        }



        /* Configure ASEs for next device in group */

        if (leAudioDeviceNext) {

          PrepareAndSendRelease(leAudioDeviceNext);

        } else {

        /* Last node is in releasing state*/

        group->SetState(AseState::BTA_LE_AUDIO_ASE_STATE_IDLE);



        group->PrintDebugState();



        /* If all CISes are disconnected, notify upper layer about IDLE state,

         * otherwise wait for */

        if (!group->HaveAllCisesDisconnected()) {
@@ -1539,7 +1531,7 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
        ReleaseCisIds(group);

        state_machine_callbacks_->StatusReportCb(group->group_id_,

                                                 GroupStreamStatus::IDLE);

        }



        break;

      }

      default:
@@ -2050,6 +2042,22 @@ class LeAudioGroupStateMachineImpl : public LeAudioGroupStateMachine { 
                                      GATT_WRITE_NO_RSP, NULL, NULL);

  }



  GroupStreamStatus PrepareAndSendReleaseToTheGroup(LeAudioDeviceGroup* group) {

    LOG_INFO("group_id: %d", group->group_id_);

    LeAudioDevice* leAudioDevice = group->GetFirstActiveDevice();

    if (!leAudioDevice) {

      LOG_ERROR(" Shouldn't be called without an active device.");

      return GroupStreamStatus::IDLE;

    }



    for (; leAudioDevice;

         leAudioDevice = group->GetNextActiveDevice(leAudioDevice)) {

      PrepareAndSendRelease(leAudioDevice);

    }



    return GroupStreamStatus::RELEASING;

  }



  void PrepareAndSendRelease(LeAudioDevice* leAudioDevice) {

    ase* ase = leAudioDevice->GetFirstActiveAse();

    LOG_ASSERT(ase) << __func__ << " shouldn't be called without an active ASE";

system/bta/le_audio/state_machine_test.cc

+108 −10
Original line number Diff line number Diff line
@@ -546,6 +546,12 @@ class StateMachineTest : public Test { 
    return &(*group);

  }



  void InjectAclDisconnected(LeAudioDeviceGroup* group,

                             LeAudioDevice* leAudioDevice) {

    LeAudioGroupStateMachine::Get()->ProcessHciNotifAclDisconnected(

        group, leAudioDevice);

  }



  void InjectAseStateNotification(types::ase* ase, LeAudioDevice* device,

                                  LeAudioDeviceGroup* group, uint8_t new_state,

                                  void* new_state_params) {
@@ -1139,10 +1145,11 @@ class StateMachineTest : public Test { 
  }



  void PrepareReleaseHandler(LeAudioDeviceGroup* group,

                             int verify_ase_count = 0) {

                             int verify_ase_count = 0,

                             bool inject_disconnect_device = false) {

    ase_ctp_handlers[ascs::kAseCtpOpcodeRelease] =

        [group, verify_ase_count, this](LeAudioDevice* device,

                                        std::vector<uint8_t> value,

        [group, verify_ase_count, inject_disconnect_device, this](

            LeAudioDevice* device, std::vector<uint8_t> value,

            GATT_WRITE_OP_CB cb, void* cb_data) {

          auto num_ase = value[1];
@@ -1150,6 +1157,11 @@ class StateMachineTest : public Test { 
          if (verify_ase_count) ASSERT_EQ(verify_ase_count, num_ase);

          ASSERT_EQ(value.size(), 2ul + num_ase);



          if (inject_disconnect_device) {

            InjectAclDisconnected(group, device);

            return;

          }



          // Inject Releasing & Idle ASE state notification for each ASE

          auto* ase_p = &value[2];

          for (auto i = 0u; i < num_ase; ++i) {
@@ -2514,6 +2526,11 @@ TEST_F(StateMachineTest, testReleaseMultiple) { 


  InjectInitialIdleNotification(group);



  EXPECT_CALL(mock_callbacks_,

              StatusReportCb(leaudio_group_id,

                             bluetooth::le_audio::GroupStreamStatus::STREAMING))

      .Times(1);



  // Start the configuration and stream Media content

  LeAudioGroupStateMachine::Get()->StartStream(

      group, context_type,
@@ -2535,6 +2552,10 @@ TEST_F(StateMachineTest, testReleaseMultiple) { 
  EXPECT_CALL(mock_callbacks_,

              StatusReportCb(leaudio_group_id,

                             bluetooth::le_audio::GroupStreamStatus::IDLE));

  EXPECT_CALL(mock_callbacks_,

              StatusReportCb(leaudio_group_id,

                             bluetooth::le_audio::GroupStreamStatus::STREAMING))

      .Times(0);



  // Stop the stream

  LeAudioGroupStateMachine::Get()->StopStream(group);
@@ -2544,6 +2565,89 @@ TEST_F(StateMachineTest, testReleaseMultiple) { 
  ASSERT_EQ(1, mock_function_count_map["alarm_cancel"]);

}



TEST_F(StateMachineTest, testReleaseMultiple_DeviceDisconnectedDuringRelease) {

  const auto context_type = kContextTypeMedia;

  const auto leaudio_group_id = 6;

  const auto num_devices = 2;



  // Prepare multiple fake connected devices in a group

  auto* group =

      PrepareSingleTestDeviceGroup(leaudio_group_id, context_type, num_devices);

  ASSERT_EQ(group->Size(), num_devices);



  PrepareConfigureCodecHandler(group);

  PrepareConfigureQosHandler(group);

  PrepareEnableHandler(group);

  PrepareDisableHandler(group);



  /* Here we inject device disconnection during release */

  PrepareReleaseHandler(group, 0, true);



  auto* leAudioDevice = group->GetFirstDevice();

  auto expected_devices_written = 0;

  while (leAudioDevice) {

    EXPECT_CALL(gatt_queue,

                WriteCharacteristic(leAudioDevice->conn_id_,

                                    leAudioDevice->ctp_hdls_.val_hdl, _,

                                    GATT_WRITE_NO_RSP, _, _))

        .Times(AtLeast(4));

    expected_devices_written++;

    leAudioDevice = group->GetNextDevice(leAudioDevice);

  }

  ASSERT_EQ(expected_devices_written, num_devices);



  EXPECT_CALL(*mock_iso_manager_, CreateCig(_, _)).Times(1);

  EXPECT_CALL(*mock_iso_manager_, EstablishCis(_)).Times(1);

  EXPECT_CALL(*mock_iso_manager_, SetupIsoDataPath(_, _)).Times(2);

  EXPECT_CALL(*mock_iso_manager_, RemoveIsoDataPath(_, _)).Times(0);

  EXPECT_CALL(*mock_iso_manager_, DisconnectCis(_, _)).Times(0);

  EXPECT_CALL(*mock_iso_manager_, RemoveCig(_, _)).Times(1);



  InjectInitialIdleNotification(group);



  EXPECT_CALL(mock_callbacks_,

              StatusReportCb(leaudio_group_id,

                             bluetooth::le_audio::GroupStreamStatus::STREAMING))

      .Times(1);



  // Start the configuration and stream Media content

  LeAudioGroupStateMachine::Get()->StartStream(

      group, context_type,

      {.sink = types::AudioContexts(context_type),

       .source = types::AudioContexts(context_type)});



  // Check if group has transitioned to a proper state

  ASSERT_EQ(group->GetState(),

            types::AseState::BTA_LE_AUDIO_ASE_STATE_STREAMING);



  ASSERT_EQ(1, mock_function_count_map["alarm_cancel"]);

  mock_function_count_map["alarm_cancel"] = 0;



  testing::Mock::VerifyAndClearExpectations(&mock_iso_manager_);



  // Validate GroupStreamStatus

  EXPECT_CALL(

      mock_callbacks_,

      StatusReportCb(leaudio_group_id,

                     bluetooth::le_audio::GroupStreamStatus::RELEASING));

  EXPECT_CALL(mock_callbacks_,

              StatusReportCb(leaudio_group_id,

                             bluetooth::le_audio::GroupStreamStatus::IDLE));

  EXPECT_CALL(mock_callbacks_,

              StatusReportCb(leaudio_group_id,

                             bluetooth::le_audio::GroupStreamStatus::STREAMING))

      .Times(0);



  // Stop the stream

  LeAudioGroupStateMachine::Get()->StopStream(group);



  testing::Mock::VerifyAndClearExpectations(&mock_iso_manager_);



  // Check if group has transitioned to a proper state

  ASSERT_EQ(group->GetState(), types::AseState::BTA_LE_AUDIO_ASE_STATE_IDLE);

  ASSERT_EQ(1, mock_function_count_map["alarm_cancel"]);

}



TEST_F(StateMachineTest, testReleaseBidirectional) {

  /* Device is banded headphones with 2x snk + 1x src ase

   * (1x bidirectional + 1xunidirectional CIS)
@@ -3026,12 +3130,6 @@ TEST_F(StateMachineTest, testConfigureDataPathForAdsp) { 
       .source = types::AudioContexts(context_type)}));

}



static void InjectAclDisconnected(LeAudioDeviceGroup* group,

                                  LeAudioDevice* leAudioDevice) {

  LeAudioGroupStateMachine::Get()->ProcessHciNotifAclDisconnected(

      group, leAudioDevice);

}



TEST_F(StateMachineTest, testStreamConfigurationAdspDownMix) {

  const auto context_type = kContextTypeConversational;

  const int leaudio_group_id = 4;