Merge "AVRCP: Change notification variable when the interim response is sent" (7a9f2349) · Commits · e / os / android_packages_modules_Bluetooth

system/profile/avrcp/device.cc

+29 −12

Original line number	Diff line number	Diff line
		@@ -205,21 +205,18 @@ void Device::HandleNotification(

		switch (pkt->GetEventRegistered()) {
		case Event::TRACK_CHANGED: {
		track_changed_ = Notification(true, label);
		media_interface_->GetNowPlayingList(
		base::Bind(&Device::TrackChangedNotificationResponse,
		weak_ptr_factory_.GetWeakPtr(), label, true));
		} break;

		case Event::PLAYBACK_STATUS_CHANGED: {
		play_status_changed_ = Notification(true, label);
		media_interface_->GetPlayStatus(
		base::Bind(&Device::PlaybackStatusNotificationResponse,
		weak_ptr_factory_.GetWeakPtr(), label, true));
		} break;

		case Event::PLAYBACK_POS_CHANGED: {
		play_pos_changed_ = Notification(true, label);
		play_pos_interval_ = pkt->GetInterval();
		media_interface_->GetPlayStatus(
		base::Bind(&Device::PlaybackPosNotificationResponse,
		@@ -227,13 +224,15 @@ void Device::HandleNotification(
		} break;

		case Event::NOW_PLAYING_CONTENT_CHANGED: {
		now_playing_changed_ = Notification(true, label);
		media_interface_->GetNowPlayingList(base::Bind(
		&Device::HandleNowPlayingNotificationResponse,
		weak_ptr_factory_.GetWeakPtr(), now_playing_changed_.second, true));
		media_interface_->GetNowPlayingList(
		base::Bind(&Device::HandleNowPlayingNotificationResponse,
		weak_ptr_factory_.GetWeakPtr(), label, true));
		} break;

		case Event::AVAILABLE_PLAYERS_CHANGED: {
		// TODO (apanicke): If we make a separate handler function for this, make
		// sure to register the notification in the interim response.

		// Respond immediately since this notification doesn't require any info
		avail_players_changed_ = Notification(true, label);
		auto response =
		@@ -243,13 +242,15 @@ void Device::HandleNotification(
		} break;

		case Event::ADDRESSED_PLAYER_CHANGED: {
		addr_player_changed_ = Notification(true, label);
		media_interface_->GetMediaPlayerList(
		base::Bind(&Device::AddressedPlayerNotificationResponse,
		weak_ptr_factory_.GetWeakPtr(), label, true));
		} break;

		case Event::UIDS_CHANGED: {
		// TODO (apanicke): If we make a separate handler function for this, make
		// sure to register the notification in the interim response.

		// Respond immediately since this notification doesn't require any info
		uids_changed_ = Notification(true, label);
		auto response =
		@@ -362,7 +363,9 @@ void Device::TrackChangedNotificationResponse(uint8_t label, bool interim,
		DEVICE_VLOG(1) << __func__;
		uint64_t uid = 0;

		if (!track_changed_.first) {
		if (interim) {
		track_changed_ = Notification(true, label);
		} else if (!track_changed_.first) {
		DEVICE_VLOG(0) << __func__ << ": Device not registered for update";
		return;
		}
		@@ -402,7 +405,9 @@ void Device::PlaybackStatusNotificationResponse(uint8_t label, bool interim,
		DEVICE_VLOG(1) << __func__;
		if (status.state == PlayState::PAUSED) play_pos_update_cb_.Cancel();

		if (!play_status_changed_.first) {
		if (interim) {
		play_status_changed_ = Notification(true, label);
		} else if (!play_status_changed_.first) {
		DEVICE_VLOG(0) << __func__ << ": Device not registered for update";
		return;
		}
		@@ -433,7 +438,9 @@ void Device::PlaybackPosNotificationResponse(uint8_t label, bool interim,
		PlayStatus status) {
		DEVICE_VLOG(4) << __func__;

		if (!play_pos_changed_.first) {
		if (interim) {
		play_pos_changed_ = Notification(true, label);
		} else if (!play_pos_changed_.first) {
		DEVICE_VLOG(3) << __func__ << ": Device not registered for update";
		return;
		}
		@@ -476,6 +483,14 @@ void Device::AddressedPlayerNotificationResponse(
		std::vector<MediaPlayerInfo> /* unused */) {
		DEVICE_VLOG(1) << __func__
		<< ": curr_player_id=" << (unsigned int)curr_player;

		if (interim) {
		addr_player_changed_ = Notification(true, label);
		} else if (!addr_player_changed_.first) {
		DEVICE_VLOG(3) << __func__ << ": Device not registered for update";
		return;
		}

		// If there is no set browsed player, use the current addressed player as the
		// default NOTE: Using any browsing commands before the browsed player is set
		// is a violation of the AVRCP Spec but there are some carkits that try too
		@@ -1200,7 +1215,9 @@ void Device::HandleNowPlayingUpdate() {
		void Device::HandleNowPlayingNotificationResponse(
		uint8_t label, bool interim, std::string curr_song_id,
		std::vector<SongInfo> song_list) {
		if (!now_playing_changed_.first) {
		if (interim) {
		now_playing_changed_ = Notification(true, label);
		} else if (!now_playing_changed_.first) {
		LOG(WARNING) << "Device is not registered for now playing updates";
		return;
		}

system/profile/avrcp/tests/avrcp_device_test.cc

+293 −7

Original line number	Diff line number	Diff line
		@@ -41,10 +41,11 @@ using TestAvrcpPacket = TestPacketType<Packet>;
		using TestBrowsePacket = TestPacketType<BrowsePacket>;

		using ::testing::_;
		using ::testing::MockFunction;
		using ::testing::Mock;
		using ::testing::MockFunction;
		using ::testing::NiceMock;
		using ::testing::Return;
		using ::testing::SaveArg;

		bool get_pts_avrcp_test(void) { return false; }

		@@ -189,7 +190,9 @@ TEST_F(AvrcpDeviceTest, playPositionTest) {

		test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);

		PlayStatus status1 = {0x1234, 0x5678, PlayState::PLAYING};
		// TODO (apanicke): Add an underlying message loop so we can test the playing
		// state.
		PlayStatus status1 = {0x1234, 0x5678, PlayState::PAUSED};
		PlayStatus status2 = {0x5678, 0x9ABC, PlayState::STOPPED};

		EXPECT_CALL(interface, GetPlayStatus(_))
		@@ -201,28 +204,311 @@ TEST_F(AvrcpDeviceTest, playPositionTest) {
		EXPECT_CALL(a2dp_interface, active_peer())
		.WillRepeatedly(Return(test_device->GetAddress()));

		// Test the interim response for play status changed
		// Test the interim response for play position changed
		auto interim_response =
		RegisterNotificationResponseBuilder::MakePlaybackStatusBuilder(
		true, PlayState::PLAYING);
		RegisterNotificationResponseBuilder::MakePlaybackPositionBuilder(true,
		0x1234);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(interim_response))))
		.Times(1);

		auto request = RegisterNotificationRequestBuilder::MakeBuilder(
		Event::PLAYBACK_STATUS_CHANGED, 0);
		Event::PLAYBACK_POS_CHANGED, 0);
		auto pkt = TestAvrcpPacket::Make();
		request->Serialize(pkt);
		SendMessage(1, pkt);

		// Test the changed response for play status changed
		// Test the changed response for play position changed
		auto changed_response =
		RegisterNotificationResponseBuilder::MakePlaybackPositionBuilder(false,
		0x5678);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(changed_response))))
		.Times(1);
		test_device->HandlePlayPosUpdate();
		}

		TEST_F(AvrcpDeviceTest, trackChangedBeforeInterimTest) {
		MockMediaInterface interface;
		NiceMock<MockA2dpInterface> a2dp_interface;

		test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);

		// Pretend the device is active
		EXPECT_CALL(a2dp_interface, active_peer())
		.WillRepeatedly(Return(test_device->GetAddress()));

		SongInfo info = {"test_id",
		{// The attribute map
		AttributeEntry(Attribute::TITLE, "Test Song"),
		AttributeEntry(Attribute::ARTIST_NAME, "Test Artist"),
		AttributeEntry(Attribute::ALBUM_NAME, "Test Album"),
		AttributeEntry(Attribute::TRACK_NUMBER, "1"),
		AttributeEntry(Attribute::TOTAL_NUMBER_OF_TRACKS, "2"),
		AttributeEntry(Attribute::GENRE, "Test Genre"),
		AttributeEntry(Attribute::PLAYING_TIME, "1000")}};
		std::vector<SongInfo> list = {info};

		MediaInterface::NowPlayingCallback interim_cb;
		MediaInterface::NowPlayingCallback changed_cb;

		EXPECT_CALL(interface, GetNowPlayingList(_))
		.Times(2)
		.WillOnce(SaveArg<0>(&interim_cb))
		.WillOnce(SaveArg<0>(&changed_cb));

		// Test that the changed response doesn't get sent before the interim
		::testing::InSequence s;
		auto interim_response =
		RegisterNotificationResponseBuilder::MakeTrackChangedBuilder(true, 0x01);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(interim_response))))
		.Times(1);
		auto changed_response =
		RegisterNotificationResponseBuilder::MakeTrackChangedBuilder(false, 0x01);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(changed_response))))
		.Times(1);

		// Register for the update, sets interim_cb
		auto request =
		RegisterNotificationRequestBuilder::MakeBuilder(Event::TRACK_CHANGED, 0);
		auto pkt = TestAvrcpPacket::Make();
		request->Serialize(pkt);
		SendMessage(1, pkt);

		// Try to send track changed update, should fail and do nothing
		test_device->HandleTrackUpdate();

		// Send the interim response
		interim_cb.Run("test_id", list);

		// Try to send track changed update, should succeed
		test_device->HandleTrackUpdate();
		changed_cb.Run("test_id", list);
		}

		TEST_F(AvrcpDeviceTest, playStatusChangedBeforeInterimTest) {
		MockMediaInterface interface;
		NiceMock<MockA2dpInterface> a2dp_interface;

		// Pretend the device is active
		EXPECT_CALL(a2dp_interface, active_peer())
		.WillRepeatedly(Return(test_device->GetAddress()));

		test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);

		MediaInterface::PlayStatusCallback interim_cb;
		MediaInterface::PlayStatusCallback changed_cb;

		EXPECT_CALL(interface, GetPlayStatus(_))
		.Times(2)
		.WillOnce(SaveArg<0>(&interim_cb))
		.WillOnce(SaveArg<0>(&changed_cb));

		// Test that the changed response doesn't get sent before the interim
		::testing::InSequence s;
		auto interim_response =
		RegisterNotificationResponseBuilder::MakePlaybackStatusBuilder(
		true, PlayState::PLAYING);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(interim_response))))
		.Times(1);
		auto changed_response =
		RegisterNotificationResponseBuilder::MakePlaybackStatusBuilder(
		false, PlayState::STOPPED);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(changed_response))))
		.Times(1);

		// Send the registration packet
		auto request = RegisterNotificationRequestBuilder::MakeBuilder(
		Event::PLAYBACK_STATUS_CHANGED, 0);
		auto pkt = TestAvrcpPacket::Make();
		request->Serialize(pkt);
		SendMessage(1, pkt);

		// Send a play status update, should be ignored since the interim response
		// hasn't been sent yet.
		test_device->HandlePlayStatusUpdate();

		// Send the interim response.
		PlayStatus status1 = {0x1234, 0x5678, PlayState::PLAYING};
		interim_cb.Run(status1);

		// Send the changed response, should succeed this time
		test_device->HandlePlayStatusUpdate();
		PlayStatus status2 = {0x1234, 0x5678, PlayState::STOPPED};
		changed_cb.Run(status2);
		}

		TEST_F(AvrcpDeviceTest, playPositionChangedBeforeInterimTest) {
		MockMediaInterface interface;
		NiceMock<MockA2dpInterface> a2dp_interface;

		// Pretend the device is active
		EXPECT_CALL(a2dp_interface, active_peer())
		.WillRepeatedly(Return(test_device->GetAddress()));

		test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);

		MediaInterface::PlayStatusCallback interim_cb;
		MediaInterface::PlayStatusCallback changed_cb;

		EXPECT_CALL(interface, GetPlayStatus(_))
		.Times(2)
		.WillOnce(SaveArg<0>(&interim_cb))
		.WillOnce(SaveArg<0>(&changed_cb));

		// Test that the changed response doesn't get sent before the interim
		::testing::InSequence s;
		auto interim_response =
		RegisterNotificationResponseBuilder::MakePlaybackPositionBuilder(true,
		0x1234);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(interim_response))))
		.Times(1);
		auto changed_response =
		RegisterNotificationResponseBuilder::MakePlaybackPositionBuilder(false,
		0x5678);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(changed_response))))
		.Times(1);

		// Send the registration packet
		auto request = RegisterNotificationRequestBuilder::MakeBuilder(
		Event::PLAYBACK_POS_CHANGED, 0);
		auto pkt = TestAvrcpPacket::Make();
		request->Serialize(pkt);
		SendMessage(1, pkt);

		// Send a play position update, should be ignored since the notification
		// isn't registered since no interim response has been sent.
		test_device->HandlePlayPosUpdate();

		// Run the interim callback for GetPlayStatus which should be pointing to the
		// GetPlayStatus call made by the update.
		PlayStatus status1 = {0x1234, 0x5678, PlayState::PAUSED};
		interim_cb.Run(status1);

		// Send a play position update, this one should succeed.
		test_device->HandlePlayPosUpdate();
		PlayStatus status2 = {0x5678, 0x9ABC, PlayState::STOPPED};
		changed_cb.Run(status2);
		}

		TEST_F(AvrcpDeviceTest, nowPlayingChangedBeforeInterim) {
		MockMediaInterface interface;
		NiceMock<MockA2dpInterface> a2dp_interface;

		test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);

		SongInfo info = {"test_id",
		{// The attribute map
		AttributeEntry(Attribute::TITLE, "Test Song"),
		AttributeEntry(Attribute::ARTIST_NAME, "Test Artist"),
		AttributeEntry(Attribute::ALBUM_NAME, "Test Album"),
		AttributeEntry(Attribute::TRACK_NUMBER, "1"),
		AttributeEntry(Attribute::TOTAL_NUMBER_OF_TRACKS, "2"),
		AttributeEntry(Attribute::GENRE, "Test Genre"),
		AttributeEntry(Attribute::PLAYING_TIME, "1000")}};
		std::vector<SongInfo> list = {info};

		MediaInterface::NowPlayingCallback interim_cb;
		MediaInterface::NowPlayingCallback changed_cb;

		EXPECT_CALL(interface, GetNowPlayingList(_))
		.Times(2)
		.WillOnce(SaveArg<0>(&interim_cb))
		.WillOnce(SaveArg<0>(&changed_cb));

		// Test that the changed response doesn't get sent before the interim
		::testing::InSequence s;
		auto interim_response =
		RegisterNotificationResponseBuilder::MakeNowPlayingBuilder(true);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(interim_response))))
		.Times(1);
		auto changed_response =
		RegisterNotificationResponseBuilder::MakeNowPlayingBuilder(false);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(changed_response))))
		.Times(1);

		// Send the registration packet
		auto request = RegisterNotificationRequestBuilder::MakeBuilder(
		Event::NOW_PLAYING_CONTENT_CHANGED, 0);
		auto pkt = TestAvrcpPacket::Make();
		request->Serialize(pkt);
		SendMessage(1, pkt);

		// Send now playing changed, should fail since the interim response hasn't
		// been sent
		test_device->HandleNowPlayingUpdate();

		// Send the data needed for the interim response
		interim_cb.Run("test_id", list);

		// Send now playing changed, should succeed
		test_device->HandleNowPlayingUpdate();
		changed_cb.Run("test_id", list);
		}

		TEST_F(AvrcpDeviceTest, addressPlayerChangedBeforeInterim) {
		MockMediaInterface interface;
		NiceMock<MockA2dpInterface> a2dp_interface;

		test_device->RegisterInterfaces(&interface, &a2dp_interface, nullptr);

		MediaInterface::MediaListCallback interim_cb;
		MediaInterface::MediaListCallback changed_cb;

		EXPECT_CALL(interface, GetMediaPlayerList(_))
		.Times(2)
		.WillOnce(SaveArg<0>(&interim_cb))
		.WillOnce(SaveArg<0>(&changed_cb));

		// Test that the changed response doesn't get sent before the interim
		::testing::InSequence s;
		auto interim_response =
		RegisterNotificationResponseBuilder::MakeAddressedPlayerBuilder(true, 0,
		0);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(interim_response))))
		.Times(1);
		auto changed_response =
		RegisterNotificationResponseBuilder::MakeAddressedPlayerBuilder(false, 0,
		0);
		EXPECT_CALL(response_cb,
		Call(1, false, matchPacket(std::move(changed_response))))
		.Times(1);
		// TODO (apanicke): Remove this expectation once b/110957802 is fixed and
		// we don't try to reject notifications that aren't registered.
		auto rejected_response = RejectBuilder::MakeBuilder(
		CommandPdu::REGISTER_NOTIFICATION, Status::ADDRESSED_PLAYER_CHANGED);
		EXPECT_CALL(response_cb,
		Call(_, false, matchPacket(std::move(rejected_response))))
		.Times(4);

		// Send the registration packet
		auto request = RegisterNotificationRequestBuilder::MakeBuilder(
		Event::ADDRESSED_PLAYER_CHANGED, 0);
		auto pkt = TestAvrcpPacket::Make();
		request->Serialize(pkt);
		SendMessage(1, pkt);

		// Send addressed player update, should fail since the interim response
		// hasn't been sent
		test_device->HandleAddressedPlayerUpdate();

		// Send the data needed for the interim response
		MediaPlayerInfo info = {0, "Test Player", true};
		std::vector<MediaPlayerInfo> list = {info};
		interim_cb.Run(0, list);

		// Send addressed player update, should succeed
		test_device->HandleAddressedPlayerUpdate();
		changed_cb.Run(0, list);
		}

		TEST_F(AvrcpDeviceTest, nowPlayingTest) {