From 8bdea4b19d0b65331e3a28abf735e49ea99a4e75 Mon Sep 17 00:00:00 2001
From: Ajay Panicker <apanicke@google.com>
Date: Tue, 3 Apr 2018 22:28:06 -0700
Subject: [PATCH] Disable absolute volume if the remote device rejects
registration
Instead of trying to re-register for a rejected volume changed
notification, disable absolute volume. This prevents spinning if a
remote device continuously rejects all attempts to register. A volume
level of -2 will be used to represent that the volume notification was
rejected.
Bug: 77238060
Test: Run host native test net_test_avrcp
Change-Id: I228524fb30348ca691d0792f0c7bcc4653d1fcef
---
.../packet/tests/avrcp/avrcp_test_packets.h | 4 +++
system/profile/avrcp/device.cc | 25 ++++++++++++++++---
.../profile/avrcp/tests/avrcp_device_test.cc | 19 ++++++++++++++
3 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/system/packet/tests/avrcp/avrcp_test_packets.h b/system/packet/tests/avrcp/avrcp_test_packets.h
index cd07de375c1..f5c914e2c01 100644
--- a/system/packet/tests/avrcp/avrcp_test_packets.h
+++ b/system/packet/tests/avrcp/avrcp_test_packets.h
@@ -134,6 +134,10 @@ std::vector<uint8_t> interim_uids_notificaiton = {0x0f, 0x48, 0x00, 0x00, 0x19,
std::vector<uint8_t> interim_volume_changed_notification = {
0x0f, 0x48, 0x00, 0x00, 0x19, 0x58, 0x31, 0x00, 0x00, 0x02, 0x0d, 0x47};
+// AVRCP Rejected Volume Changed Notification with volume at 0%
+std::vector<uint8_t> rejected_volume_changed_notification = {
+ 0x0a, 0x48, 0x00, 0x00, 0x19, 0x58, 0x31, 0x00, 0x00, 0x02, 0x0d, 0x00};
+
// AVRCP Changed Volume Changed Notification with volume at 55% (0x47)
std::vector<uint8_t> changed_volume_changed_notification = {
0x0d, 0x48, 0x00, 0x00, 0x19, 0x58, 0x31, 0x00, 0x00, 0x02, 0x0d, 0x47};
diff --git a/system/profile/avrcp/device.cc b/system/profile/avrcp/device.cc
index 734dc241246..e66caa7b075 100644
--- a/system/profile/avrcp/device.cc
+++ b/system/profile/avrcp/device.cc
@@ -25,6 +25,9 @@ namespace avrcp {
#define DEVICE_LOG(LEVEL) LOG(LEVEL) << address_.ToString() << " : "
#define DEVICE_VLOG(LEVEL) VLOG(LEVEL) << address_.ToString() << " : "
+#define VOL_NOT_SUPPORTED -1
+#define VOL_REGISTRATION_FAILED -2
+
Device::Device(
const RawAddress& bdaddr, bool avrcp13_compatibility,
base::Callback<void(uint8_t label, bool browse,
@@ -253,6 +256,14 @@ void Device::HandleVolumeChanged(
DEVICE_VLOG(1) << __func__ << ": interim=" << pkt->IsInterim();
if (volume_interface_ == nullptr) return;
+ if (pkt->GetCType() == CType::REJECTED) {
+ // Disable Absolute Volume
+ active_labels_.erase(label);
+ volume_interface_ = nullptr;
+ volume_ = VOL_REGISTRATION_FAILED;
+ return;
+ }
+
// We only update on interim and just re-register on changes.
if (!pkt->IsInterim()) {
active_labels_.erase(label);
@@ -261,7 +272,7 @@ void Device::HandleVolumeChanged(
}
// Handle the first volume update.
- if (volume_ == -1) {
+ if (volume_ == VOL_NOT_SUPPORTED) {
volume_ = pkt->GetVolume();
volume_interface_->DeviceConnected(
GetAddress(), base::Bind(&Device::SetVolume, base::Unretained(this)));
@@ -290,6 +301,7 @@ void Device::SetVolume(int8_t volume) {
}
}
+ volume_ = volume;
send_message_cb_.Run(label, false, std::move(request));
}
@@ -1055,6 +1067,13 @@ void Device::DeviceDisconnected() {
volume_interface_->DeviceDisconnected(GetAddress());
}
+static std::string volumeToStr(int8_t volume) {
+ if (volume == VOL_NOT_SUPPORTED) return "Absolute Volume not supported";
+ if (volume == VOL_REGISTRATION_FAILED)
+ return "Volume changed notification was rejected";
+ return std::to_string(volume);
+}
+
std::ostream& operator<<(std::ostream& out, const Device& d) {
out << "Avrcp Device: Address=" << d.address_.ToString() << std::endl;
out << " â”” isActive: " << (d.IsActive() ? "YES" : "NO") << std::endl;
@@ -1072,7 +1091,7 @@ std::ostream& operator<<(std::ostream& out, const Device& d) {
out << " â”” UIDs Changed: " << d.uids_changed_.first << std::endl;
out << " â”” Last Song Sent ID: " << d.last_song_info_.media_id << std::endl;
out << " â”” Last Play State: " << d.last_play_status_.state << std::endl;
- out << " â”” Current Volume: " << d.volume_ << std::endl;
+ out << " â”” Current Volume: " << volumeToStr(d.volume_) << std::endl;
out << " â”” Current Folder: " << d.CurrentFolder();
out << " â”” Control MTU Size: " << d.ctrl_mtu_ << std::endl;
out << " â”” Browse MTU Size: " << d.browse_mtu_ << std::endl;
@@ -1083,4 +1102,4 @@ std::ostream& operator<<(std::ostream& out, const Device& d) {
}
} // namespace avrcp
-} // namespace bluetooth
\ No newline at end of file
+} // namespace bluetooth
diff --git a/system/profile/avrcp/tests/avrcp_device_test.cc b/system/profile/avrcp/tests/avrcp_device_test.cc
index e9dde53b077..2207bb07cc0 100644
--- a/system/profile/avrcp/tests/avrcp_device_test.cc
+++ b/system/profile/avrcp/tests/avrcp_device_test.cc
@@ -678,5 +678,24 @@ TEST_F(AvrcpDeviceTest, volumeChangedTest) {
SendMessage(1, response);
}
+TEST_F(AvrcpDeviceTest, volumeRejectedTest) {
+ MockMediaInterface interface;
+ NiceMock<MockA2dpInterface> a2dp_interface;
+ MockVolumeInterface vol_interface;
+
+ test_device->RegisterInterfaces(&interface, &a2dp_interface, &vol_interface);
+
+ auto reg_notif =
+ RegisterNotificationRequestBuilder::MakeBuilder(Event::VOLUME_CHANGED, 0);
+ EXPECT_CALL(response_cb, Call(_, false, matchPacket(std::move(reg_notif))))
+ .Times(1);
+ test_device->RegisterVolumeChanged();
+
+ auto response = TestAvrcpPacket::Make(rejected_volume_changed_notification);
+ SendMessage(1, response);
+
+ EXPECT_CALL(response_cb, Call(_, _, _)).Times(0);
+}
+
} // namespace avrcp
} // namespace bluetooth
\ No newline at end of file
--
GitLab