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

Commit 6096680d authored by Michal Belusiak's avatar Michal Belusiak Committed by Michal Belusiak (xWF)
Browse files

le_periodic_sync_manager: Check if established sync was pending 

That check prevents from executing operations on periodic_sync which
was not started yet. That is possible when there are two requests for
the same broadcaster, typical on pair of buds.

Fail scenario with inactive broadcaster:
1. HandleStartSyncRequest with periodic_sync A
2. OnStartSyncTimeout inform upper layer about A failure, cancel and
remove A
3. HandleStartSyncRequest with periodic_sync B (the same broadcaster)
4. HandleLePeriodicAdvertisingSyncEstablished with error 68 because of
canceling A, informing upper layer about B failure because A is already
removed, removing B.

Bug: 354890418
Test: atest PeriodicSyncManagerTest
Flag: Exempt, trivial fix covered with unit test
Change-Id: I67f4949e2cdd02c5e9757515393f6ce43ca627db
parent 8a754ad9

system/gd/hci/le_periodic_sync_manager.h

+14 −10
Original line number Diff line number Diff line
@@ -337,6 +337,7 @@ public: 
      AdvanceRequest();

      return;

    }

    if (periodic_sync->sync_state == PERIODIC_SYNC_STATE_PENDING) {

      periodic_sync->sync_handle = event_view.GetSyncHandle();

      periodic_sync->sync_state = PERIODIC_SYNC_STATE_ESTABLISHED;

      callbacks_->OnPeriodicSyncStarted(periodic_sync->request_id, (uint8_t)event_view.GetStatus(),
@@ -349,6 +350,9 @@ public: 
          periodic_syncs_.erase(periodic_sync);

        }

      }

    } else {

      log::debug("[PSync]: Wrong sync state={}", (uint8_t)(periodic_sync->sync_state));

    }



    AdvanceRequest();

  }

system/gd/hci/le_periodic_sync_manager_test.cc

+63 −0
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 
#include <flag_macros.h>

#include <gmock/gmock.h>

#include <gtest/gtest.h>

#include <log/log.h>



#include "hci/le_scanning_callback.h"

#include "hci/le_scanning_interface.h"
@@ -136,6 +137,7 @@ private: 
class PeriodicSyncManagerTest : public ::testing::Test {

protected:

  void SetUp() override {

    __android_log_set_minimum_priority(ANDROID_LOG_VERBOSE);

    thread_ = new os::Thread("thread", os::Thread::Priority::NORMAL);

    handler_ = new os::Handler(thread_);

    test_le_scanning_interface_ = new TestLeScanningInterface();
@@ -814,6 +816,67 @@ TEST_F(PeriodicSyncManagerTest, 
  sync_handler();

}



TEST_F(PeriodicSyncManagerTest, syncEstablished_pendingCheckToCorrectTheOrder) {

  uint16_t sync_handle = 0x12;

  uint8_t advertiser_sid = 0x02;

  Address address;

  Address::FromString("00:11:22:33:44:55", address);

  AddressWithType address_with_type = AddressWithType(address, AddressType::PUBLIC_DEVICE_ADDRESS);



  // start scan

  int request_id_1 = 0x01;

  PeriodicSyncStates request{

          .request_id = request_id_1,

          .advertiser_sid = advertiser_sid,

          .address_with_type = address_with_type,

          .sync_handle = sync_handle,

          .sync_state = PeriodicSyncState::PERIODIC_SYNC_STATE_IDLE,

  };

  periodic_sync_manager_->StartSync(request, 0x04, 0x0A);



  EXPECT_CALL(

          mock_callbacks_,

          OnPeriodicSyncStarted(request_id_1, static_cast<uint8_t>(ErrorCode::ADVERTISING_TIMEOUT),

                                _, _, _, _, _))

          .Times(1);



  // First timeout

  periodic_sync_manager_->OnStartSyncTimeout();



  // Second request with the same data but different id

  int request_id_2 = 0x02;

  request.request_id = request_id_2;

  periodic_sync_manager_->StartSync(request, 0x04, 0x0A);



  // Get LePeriodicAdvertisingSyncEstablished for the first request

  auto builder = LePeriodicAdvertisingSyncEstablishedBuilder::Create(

          ErrorCode::OPERATION_CANCELLED_BY_HOST, sync_handle, advertiser_sid,

          address_with_type.GetAddressType(), address_with_type.GetAddress(),

          SecondaryPhyType::LE_1M, 0xFF, ClockAccuracy::PPM_250);

  auto event_view = LePeriodicAdvertisingSyncEstablishedView::Create(

          LeMetaEventView::Create(EventView::Create(GetPacketView(std::move(builder)))));

  periodic_sync_manager_->HandleLePeriodicAdvertisingSyncEstablished(event_view);



  EXPECT_CALL(

          mock_callbacks_,

          OnPeriodicSyncStarted(request_id_2, static_cast<uint8_t>(ErrorCode::ADVERTISING_TIMEOUT),

                                _, _, _, _, _))

          .Times(1);



  // Second timeout

  periodic_sync_manager_->OnStartSyncTimeout();



  // Get LePeriodicAdvertisingSyncEstablished for the second request

  auto builder2 = LePeriodicAdvertisingSyncEstablishedBuilder::Create(

          ErrorCode::OPERATION_CANCELLED_BY_HOST, sync_handle, advertiser_sid,

          address_with_type.GetAddressType(), address_with_type.GetAddress(),

          SecondaryPhyType::LE_1M, 0xFF, ClockAccuracy::PPM_250);

  event_view = LePeriodicAdvertisingSyncEstablishedView::Create(

          LeMetaEventView::Create(EventView::Create(GetPacketView(std::move(builder2)))));

  periodic_sync_manager_->HandleLePeriodicAdvertisingSyncEstablished(event_view);

  sync_handler();

}



TEST_F(PeriodicSyncManagerTest, handle_periodic_advertising_report_test) {

  uint16_t sync_handle = 0x12;

  uint8_t advertiser_sid = 0x02;