From 1c682f8f222534aa4f683df93d29fec664c223d2 Mon Sep 17 00:00:00 2001 From: Cheney Ni Date: Tue, 5 May 2020 15:17:29 +0800 Subject: [PATCH] service: A2DP source callback to query the preferred codec In order to apply user's preferred codec while reconnecting, we need a callback method to talk to Bluetooth stack whether the mandatory codec has higher priority than others. This change implements a dummy method in the fluoride service, so is working with the stack. Bug: 134131114 Bug: 147572898 Test: bluetoothtbd_test Change-Id: Ib99cf12eba85450526e22723d1509405469c2ebc Merged-In: Ib99cf12eba85450526e22723d1509405469c2ebc (cherry picked from commit 3254942410f182533675645392d7f72d14bf7543) --- service/Android.bp | 1 + service/a2dp_source.cc | 7 + service/a2dp_source.h | 2 + service/hal/bluetooth_av_interface.cc | 22 ++- service/hal/bluetooth_av_interface.h | 2 + service/hal/fake_bluetooth_av_interface.cc | 66 +++++++- service/hal/fake_bluetooth_av_interface.h | 26 ++- service/test/a2dp_source_unittest.cc | 249 +++++++++++++++++++++++++++++ 8 files changed, 360 insertions(+), 15 deletions(-) create mode 100644 service/test/a2dp_source_unittest.cc diff --git a/service/Android.bp b/service/Android.bp index 4b21d0d58..2760b5bcc 100644 --- a/service/Android.bp +++ b/service/Android.bp @@ -63,6 +63,7 @@ btserviceBaseTestSrc = [ "hal/fake_bluetooth_gatt_interface.cc", "hal/fake_bluetooth_interface.cc", "test/a2dp_sink_unittest.cc", + "test/a2dp_source_unittest.cc", "test/adapter_unittest.cc", "test/advertise_data_unittest.cc", "test/fake_hal_util.cc", diff --git a/service/a2dp_source.cc b/service/a2dp_source.cc index 9e77007f1..d39f2029d 100644 --- a/service/a2dp_source.cc +++ b/service/a2dp_source.cc @@ -199,6 +199,13 @@ void A2dpSource::AudioConfigCallback( codecs_selectable_capabilities); } +bool A2dpSource::MandatoryCodecPreferredCallback(BluetoothAvInterface* iface, + const RawAddress& bd_addr) { + LockGuard lock(delegate_mutex_); + // Do nothing. Optional codecs are preferred by default. + return false; +} + // A2dpSourceFactory implementation // ======================================================== A2dpSourceFactory::A2dpSourceFactory() = default; diff --git a/service/a2dp_source.h b/service/a2dp_source.h index f4e3ab326..53da6bd27 100644 --- a/service/a2dp_source.h +++ b/service/a2dp_source.h @@ -82,6 +82,8 @@ class A2dpSource : public BluetoothInstance, const std::vector codecs_local_capabilities, const std::vector codecs_selectable_capabilities) override; + bool MandatoryCodecPreferredCallback(hal::BluetoothAvInterface* iface, + const RawAddress& bd_addr) override; // For |GetAppIdentifier|. const Uuid app_identifier_; diff --git a/service/hal/bluetooth_av_interface.cc b/service/hal/bluetooth_av_interface.cc index c21d93662..4700cce4e 100644 --- a/service/hal/bluetooth_av_interface.cc +++ b/service/hal/bluetooth_av_interface.cc @@ -46,11 +46,11 @@ GetA2dpSourceObservers(); base::ObserverList* GetA2dpSinkObservers(); -#define VERIFY_INTERFACE_OR_RETURN() \ +#define VERIFY_INTERFACE_OR_RETURN(...) \ do { \ if (!g_interface) { \ LOG(WARNING) << "Callback received while |g_interface| is NULL"; \ - return; \ + return __VA_ARGS__; \ } \ } while (0) @@ -88,6 +88,17 @@ void SourceAudioConfigCallback( } } +bool SourceMandatoryCodecPreferredCallback(const RawAddress& bd_addr) { + VERIFY_INTERFACE_OR_RETURN(false); + // The mandatory codec is preferred only when all observers disable their + // optional codecs. + for (auto& observer : *GetA2dpSourceObservers()) { + if (!observer.MandatoryCodecPreferredCallback(g_interface, bd_addr)) + return false; + } + return true; +} + void SinkConnectionStateCallback(const RawAddress& bd_addr, btav_connection_state_t state) { std::shared_lock lock(g_instance_lock); @@ -121,6 +132,7 @@ btav_source_callbacks_t av_source_callbacks = { .connection_state_cb = SourceConnectionStateCallback, .audio_state_cb = SourceAudioStateCallback, .audio_config_cb = SourceAudioConfigCallback, + .mandatory_codec_preferred_cb = SourceMandatoryCodecPreferredCallback, }; btav_sink_callbacks_t av_sink_callbacks = { @@ -297,6 +309,12 @@ void BluetoothAvInterface::A2dpSourceObserver::AudioConfigCallback( // Do nothing. } +bool BluetoothAvInterface::A2dpSourceObserver::MandatoryCodecPreferredCallback( + BluetoothAvInterface* iface, const RawAddress& bd_addr) { + // Do nothing. + return false; +} + void BluetoothAvInterface::A2dpSinkObserver::ConnectionStateCallback( BluetoothAvInterface* iface, const RawAddress& bd_addr, btav_connection_state_t state) { diff --git a/service/hal/bluetooth_av_interface.h b/service/hal/bluetooth_av_interface.h index 9346143ca..eda6476b1 100644 --- a/service/hal/bluetooth_av_interface.h +++ b/service/hal/bluetooth_av_interface.h @@ -41,6 +41,8 @@ class BluetoothAvInterface { const std::vector codecs_local_capabilities, const std::vector codecs_selectable_capabilities); + virtual bool MandatoryCodecPreferredCallback(BluetoothAvInterface* iface, + const RawAddress& bd_addr); protected: virtual ~A2dpSourceObserver() = default; diff --git a/service/hal/fake_bluetooth_av_interface.cc b/service/hal/fake_bluetooth_av_interface.cc index 39a6331d8..01c8ebef7 100644 --- a/service/hal/fake_bluetooth_av_interface.cc +++ b/service/hal/fake_bluetooth_av_interface.cc @@ -23,19 +23,30 @@ namespace { // The global test handler instances. We have to have globals since the HAL // interface methods all have to be global and their signatures don't allow us // to pass in user_data. +std::shared_ptr + g_a2dp_source_handler{}; std::shared_ptr - g_a2dp_sink_handler; + g_a2dp_sink_handler{}; -bt_status_t FakeInit(btav_sink_callbacks_t* callbacks) { +bt_status_t FakeSourceInit( + btav_source_callbacks_t* callbacks, int max_connected_audio_devices, + const std::vector& codec_priorities, + const std::vector& offloading_preference) { + return BT_STATUS_SUCCESS; +} + +bt_status_t FakeSinkInit(btav_sink_callbacks_t* callbacks) { return BT_STATUS_SUCCESS; } bt_status_t FakeConnect(const RawAddress& bd_addr) { + if (g_a2dp_source_handler) return g_a2dp_source_handler->Connect(bd_addr); if (g_a2dp_sink_handler) return g_a2dp_sink_handler->Connect(bd_addr); return BT_STATUS_FAIL; } bt_status_t FakeDisconnect(const RawAddress& bd_addr) { + if (g_a2dp_source_handler) return g_a2dp_source_handler->Disconnect(bd_addr); if (g_a2dp_sink_handler) return g_a2dp_sink_handler->Disconnect(bd_addr); return BT_STATUS_FAIL; } @@ -53,26 +64,36 @@ void FakeSetAudioTrackGain(float gain) { btav_source_interface_t fake_a2dp_source_interface = { .size = sizeof(btav_source_interface_t), - .init = nullptr, - .connect = nullptr, - .disconnect = nullptr, + .init = FakeSourceInit, + .connect = FakeConnect, + .disconnect = FakeDisconnect, + .set_silence_device = nullptr, + .set_active_device = nullptr, .config_codec = nullptr, - .cleanup = nullptr, + .cleanup = FakeCleanup, }; btav_sink_interface_t fake_a2dp_sink_interface = { .size = sizeof(btav_sink_interface_t), - .init = FakeInit, + .init = FakeSinkInit, .connect = FakeConnect, .disconnect = FakeDisconnect, .cleanup = FakeCleanup, .set_audio_focus_state = FakeSetAudioFocusState, .set_audio_track_gain = FakeSetAudioTrackGain, + .set_active_device = nullptr, }; } // namespace FakeBluetoothAvInterface::FakeBluetoothAvInterface( + std::shared_ptr a2dp_source_handler) { + CHECK(!g_a2dp_source_handler); + + if (a2dp_source_handler) g_a2dp_source_handler = a2dp_source_handler; +} + +FakeBluetoothAvInterface::FakeBluetoothAvInterface( std::shared_ptr a2dp_sink_handler) { CHECK(!g_a2dp_sink_handler); @@ -80,21 +101,50 @@ FakeBluetoothAvInterface::FakeBluetoothAvInterface( } FakeBluetoothAvInterface::~FakeBluetoothAvInterface() { - g_a2dp_sink_handler = nullptr; + g_a2dp_source_handler = {}; + g_a2dp_sink_handler = {}; } void FakeBluetoothAvInterface::NotifyConnectionState( const RawAddress& bda, btav_connection_state_t state) { + for (auto& observer : a2dp_source_observers_) { + observer.ConnectionStateCallback(this, bda, state); + } for (auto& observer : a2dp_sink_observers_) { observer.ConnectionStateCallback(this, bda, state); } } void FakeBluetoothAvInterface::NotifyAudioState(const RawAddress& bda, btav_audio_state_t state) { + for (auto& observer : a2dp_source_observers_) { + observer.AudioStateCallback(this, bda, state); + } for (auto& observer : a2dp_sink_observers_) { observer.AudioStateCallback(this, bda, state); } } +void FakeBluetoothAvInterface::NotifyAudioConfig( + const RawAddress& bda, const btav_a2dp_codec_config_t& codec_config, + const std::vector codecs_local_capabilities, + const std::vector + codecs_selectable_capabilities) { + for (auto& observer : a2dp_source_observers_) { + observer.AudioConfigCallback(this, bda, codec_config, + codecs_local_capabilities, + codecs_selectable_capabilities); + } +} +bool FakeBluetoothAvInterface::QueryMandatoryCodecPreferred( + const RawAddress& bda) { + // The mandatory codec is preferred only when all observers disable their + // optional codecs. + for (auto& observer : a2dp_source_observers_) { + if (!observer.MandatoryCodecPreferredCallback(this, bda)) { + return false; + } + } + return true; +} void FakeBluetoothAvInterface::NotifyAudioConfig(const RawAddress& bda, uint32_t sample_rate, uint8_t channel_count) { diff --git a/service/hal/fake_bluetooth_av_interface.h b/service/hal/fake_bluetooth_av_interface.h index c97943456..34aeac872 100644 --- a/service/hal/fake_bluetooth_av_interface.h +++ b/service/hal/fake_bluetooth_av_interface.h @@ -26,9 +26,16 @@ namespace hal { class FakeBluetoothAvInterface : public BluetoothAvInterface { public: - // Handles HAL Bluetooth A2DP sink API calls for testing. Test code can - // provide a fake or mock implementation of this and all calls will be routed - // to it. + // Handles HAL Bluetooth A2DP API calls for testing. Test code can provide a + // fake or mock implementation of this and all calls will be routed to it. + class TestA2dpSourceHandler { + public: + virtual bt_status_t Connect(RawAddress bda) = 0; + virtual bt_status_t Disconnect(RawAddress bda) = 0; + + protected: + virtual ~TestA2dpSourceHandler() = default; + }; class TestA2dpSinkHandler { public: virtual bt_status_t Connect(RawAddress bda) = 0; @@ -44,16 +51,26 @@ class FakeBluetoothAvInterface : public BluetoothAvInterface { // provide their own handlers or simply pass "nullptr" for the default // behavior in which BT_STATUS_FAIL will be returned from all calls. FakeBluetoothAvInterface( + std::shared_ptr a2dp_source_handler); + FakeBluetoothAvInterface( std::shared_ptr a2dp_sink_handler); ~FakeBluetoothAvInterface(); // The methods below can be used to notify observers with certain events and // given parameters. - // A2DP sink callbacks + // A2DP common callbacks void NotifyConnectionState(const RawAddress& bda, btav_connection_state_t state); void NotifyAudioState(const RawAddress& bda, btav_audio_state_t state); + // A2DP source callbacks + void NotifyAudioConfig( + const RawAddress& bda, const btav_a2dp_codec_config_t& codec_config, + const std::vector codecs_local_capabilities, + const std::vector + codecs_selectable_capabilities); + bool QueryMandatoryCodecPreferred(const RawAddress& bda); + // A2DP sink callbacks void NotifyAudioConfig(const RawAddress& bda, uint32_t sample_rate, uint8_t channel_count); @@ -73,7 +90,6 @@ class FakeBluetoothAvInterface : public BluetoothAvInterface { private: base::ObserverList a2dp_source_observers_; base::ObserverList a2dp_sink_observers_; - std::shared_ptr scanner_handler_; DISALLOW_COPY_AND_ASSIGN(FakeBluetoothAvInterface); }; diff --git a/service/test/a2dp_source_unittest.cc b/service/test/a2dp_source_unittest.cc new file mode 100644 index 000000000..5fb196131 --- /dev/null +++ b/service/test/a2dp_source_unittest.cc @@ -0,0 +1,249 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include "service/a2dp_source.h" +#include "service/hal/fake_bluetooth_av_interface.h" + +using ::testing::_; +using ::testing::Return; + +namespace bluetooth { +namespace { + +class MockA2dpSourceHandler + : public hal::FakeBluetoothAvInterface::TestA2dpSourceHandler { + public: + MockA2dpSourceHandler() = default; + ~MockA2dpSourceHandler() override = default; + + MOCK_METHOD1(Connect, bt_status_t(RawAddress)); + MOCK_METHOD1(Disconnect, bt_status_t(RawAddress)); + + private: + DISALLOW_COPY_AND_ASSIGN(MockA2dpSourceHandler); +}; + +class TestDelegate : public A2dpSource::Delegate { + public: + TestDelegate() = default; + ~TestDelegate() override = default; + + struct RequestData { + std::string device_address; + int state = -1; + int count = 0; + }; + + // A2dpSource::Delegate implementation: + void OnConnectionState(const std::string& device_address, + int state) override { + ++connection_state_.count; + connection_state_.device_address = device_address; + connection_state_.state = state; + } + void OnAudioState(const std::string& device_address, int state) override { + ++audio_state_.count; + audio_state_.device_address = device_address; + audio_state_.state = state; + } + void OnAudioConfig( + const std::string& device_address, A2dpCodecConfig codec_config, + const std::vector& codecs_local_capabilities, + const std::vector& codecs_selectable_capabilities) + override { + ++audio_config_.count; + audio_config_.device_address = device_address; + } + + const RequestData& connection_state() const { return connection_state_; } + const RequestData& audio_state() const { return audio_state_; } + const RequestData& audio_config() const { return audio_config_; } + + private: + RequestData connection_state_; + RequestData audio_state_; + RequestData audio_config_; +}; + +class A2dpSourceTest : public ::testing::Test { + public: + A2dpSourceTest() = default; + ~A2dpSourceTest() override = default; + + void SetUp() override { + mock_handler_.reset(new MockA2dpSourceHandler()); + fake_hal_av_iface_ = new hal::FakeBluetoothAvInterface(mock_handler_); + hal::BluetoothAvInterface::InitializeForTesting(fake_hal_av_iface_); + factory_.reset(new A2dpSourceFactory()); + } + + void TearDown() override { + factory_.reset(); + hal::BluetoothAvInterface::CleanUp(); + } + + protected: + hal::FakeBluetoothAvInterface* fake_hal_av_iface_; + std::shared_ptr mock_handler_; + std::unique_ptr factory_; + + private: + DISALLOW_COPY_AND_ASSIGN(A2dpSourceTest); +}; + +class A2dpSourcePostRegisterTest : public A2dpSourceTest { + public: + A2dpSourcePostRegisterTest() = default; + ~A2dpSourcePostRegisterTest() override = default; + + void SetUp() override { + A2dpSourceTest::SetUp(); + Uuid uuid = Uuid::GetRandom(); + auto callback = [&](BLEStatus status, const Uuid& in_uuid, + std::unique_ptr in_client) { + CHECK(in_uuid == uuid); + CHECK(in_client.get()); + CHECK(status == BLE_STATUS_SUCCESS); + + a2dp_source_ = std::unique_ptr( + static_cast(in_client.release())); + }; + + factory_->RegisterInstance(uuid, callback); + } + + void TearDown() override { + a2dp_source_ = nullptr; + A2dpSourceTest::TearDown(); + } + + protected: + void Connect(const std::string& addr) { + RawAddress hal_addr; + ASSERT_TRUE(RawAddress::FromString(addr, hal_addr)); + + EXPECT_CALL(*mock_handler_, Connect(hal_addr)) + .WillOnce(Return(BT_STATUS_SUCCESS)); + + EXPECT_TRUE(a2dp_source_->Connect(addr)); + } + + void Disconnect(const std::string& addr) { + RawAddress hal_addr; + ASSERT_TRUE(RawAddress::FromString(addr, hal_addr)); + + EXPECT_CALL(*mock_handler_, Disconnect(hal_addr)) + .WillOnce(Return(BT_STATUS_SUCCESS)); + + EXPECT_TRUE(a2dp_source_->Disconnect(addr)); + } + + std::unique_ptr a2dp_source_; + + private: + DISALLOW_COPY_AND_ASSIGN(A2dpSourcePostRegisterTest); +}; + +TEST_F(A2dpSourceTest, RegisterA2dpSource) { + // These will be asynchronously populate with a result when the callback + // executes. + BLEStatus status = BLE_STATUS_SUCCESS; + Uuid cb_uuid; + std::unique_ptr a2dp_source; + int callback_count = 0; + + auto callback = [&](BLEStatus in_status, const Uuid& uuid, + std::unique_ptr in_a2dp_source) { + status = in_status; + cb_uuid = uuid; + a2dp_source = std::unique_ptr( + static_cast(in_a2dp_source.release())); + callback_count++; + }; + + Uuid uuid0 = Uuid::GetRandom(); + + // This should always succeed. + EXPECT_TRUE(factory_->RegisterInstance(uuid0, callback)); + EXPECT_EQ(1, callback_count); + + testing::Mock::VerifyAndClearExpectations(mock_handler_.get()); + + ASSERT_TRUE(a2dp_source.get() != + nullptr); // Assert to terminate in case of error + EXPECT_EQ(BLE_STATUS_SUCCESS, status); + EXPECT_EQ(bluetooth::A2dpSource::kSingletonInstanceId, + a2dp_source->GetInstanceId()); + EXPECT_EQ(uuid0, a2dp_source->GetAppIdentifier()); + EXPECT_EQ(uuid0, cb_uuid); + + testing::Mock::VerifyAndClearExpectations(mock_handler_.get()); +} + +TEST_F(A2dpSourcePostRegisterTest, Connect) { + static const char kTestAddr[] = "AA:BB:CC:DD:EE:FF"; + Connect(kTestAddr); + Disconnect(kTestAddr); +} + +TEST_F(A2dpSourcePostRegisterTest, CallbackTest) { + static const char kTestAddr[] = "AA:BB:CC:DD:EE:FF"; + RawAddress hal_addr; + ASSERT_TRUE(RawAddress::FromString(kTestAddr, hal_addr)); + + TestDelegate delegate; + a2dp_source_->SetDelegate(&delegate); + Connect(kTestAddr); + + // OnConnectionState + const int kConnectionState = 2; + EXPECT_EQ(0, delegate.connection_state().count); + fake_hal_av_iface_->NotifyConnectionState( + hal_addr, static_cast(kConnectionState)); + EXPECT_EQ(1, delegate.connection_state().count); + EXPECT_EQ(kTestAddr, delegate.connection_state().device_address); + EXPECT_EQ(kConnectionState, delegate.connection_state().state); + + // OnAudioState + const int kAudioState = 1; + EXPECT_EQ(0, delegate.audio_state().count); + fake_hal_av_iface_->NotifyAudioState( + hal_addr, static_cast(kAudioState)); + EXPECT_EQ(1, delegate.audio_state().count); + EXPECT_EQ(kTestAddr, delegate.audio_state().device_address); + EXPECT_EQ(kAudioState, delegate.audio_state().state); + + // OnAudioConfig + const btav_a2dp_codec_config_t codec_config{}; + const std::vector codecs_local_capabilities(0); + const std::vector codecs_selectable_capabilities(0); + EXPECT_EQ(0, delegate.audio_config().count); + fake_hal_av_iface_->NotifyAudioConfig(hal_addr, codec_config, + codecs_local_capabilities, + codecs_selectable_capabilities); + EXPECT_EQ(1, delegate.audio_config().count); + EXPECT_EQ(kTestAddr, delegate.audio_config().device_address); + + fake_hal_av_iface_->QueryMandatoryCodecPreferred(hal_addr); + + Disconnect(kTestAddr); +} + +} // namespace +} // namespace bluetooth -- 2.11.0