Repository: postlund/pyatv
Pull request #2006 (2023-05-24 06:56:40)

Make sure were sending and receiving volume messages for our device
… even if playing to other AirPlay devices in a group.

Fixes: #2005

Instead of updating `self._output_device_id` when we get a `VOLUME_CONTROL_CAPABILITIES_DID_CHANGE_MESSAGE` (which may be the UID of another grouped AirPlay device), just always use the one from device info which we get when the protocol starts.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/postlund/pyatv/pull/2006?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) Report
Patch coverage: **`100.00`**% and project coverage change: **`-0.29`** :warning:
> Comparison is base [(`0028ce2`)](https://app.codecov.io/gh/postlund/pyatv/commit/0028ce2cc68736abf7fb88621c2ca919d0535417?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) 87.66% compared to head [(`8e686a3`)](https://app.codecov.io/gh/postlund/pyatv/pull/2006?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) 87.37%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2006      +/-   ##
==========================================
- Coverage   87.66%   87.37%   -0.29%     
==========================================
  Files         151      151              
  Lines        9859     9856       -3     
==========================================
- Hits         8643     8612      -31     
- Misses       1216     1244      +28     
```


| [Impacted Files](https://app.codecov.io/gh/postlund/pyatv/pull/2006?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) | Coverage Δ | |
|---|---|---|
| [pyatv/protocols/mrp/\_\_init\_\_.py](https://app.codecov.io/gh/postlund/pyatv/pull/2006?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL21ycC9fX2luaXRfXy5weQ==) | `95.91% <100.00%> (-0.03%)` | :arrow_down: |

... and [5 files with indirect coverage changes](https://app.codecov.io/gh/postlund/pyatv/pull/2006/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/postlund/pyatv/pull/2006?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).

postlund: This looks reasonable to me, will merge it when it passes. I didn't find the time to make a release yesterday, but I can make it now pretty much. Is there anything else you want to have merged before the release?
michalmo: Just this one will allow me to make the changes to HA.

#1905 is still open when you have some time, but that can be later.

Patch:
diff --git a/pyatv/protocols/mrp/__init__.py b/pyatv/protocols/mrp/__init__.py
index 3a8dcde7..c4945272 100644
--- a/pyatv/protocols/mrp/__init__.py
+++ b/pyatv/protocols/mrp/__init__.py
@@ -726,16 +726,13 @@ class MrpAudio(Audio):
         self.protocol: MrpProtocol = protocol
         self.state_dispatcher = state_dispatcher
         self._volume_controls_available: bool = False
-        self._output_device_uid: Optional[str] = None
         self._volume: float = 0.0
         self._volume_event: asyncio.Event = asyncio.Event()
         self._add_listeners()
 
     @property
     def device_uid(self) -> Optional[str]:
-        """Return device UID for current active output device."""
-        if self._output_device_uid is not None:
-            return self._output_device_uid
+        """Return the UID of our device."""
         if self.protocol.device_info is not None:
             return self.protocol.device_info.inner().deviceUID  # type: ignore
         return None
@@ -764,8 +761,9 @@ class MrpAudio(Audio):
     async def _volume_control_changed(self, message) -> None:
         inner = message.inner()
 
-        self._output_device_uid = inner.outputDeviceUID
-        self._update_volume_controls(inner.capabilities)
+        # Make sure update is for our device (in case it changed for someone else)
+        if inner.outputDeviceUID == self.device_uid:
+            self._update_volume_controls(inner.capabilities)
 
     def _update_volume_controls(
         self, availabilty_message: protobuf.VolumeControlAvailabilityMessage
@@ -779,7 +777,7 @@ class MrpAudio(Audio):
         inner = message.inner()
 
         # Make sure update is for our device (in case it changed for someone else)
-        if inner.outputDeviceUID == self._output_device_uid:
+        if inner.outputDeviceUID == self.device_uid:
             self._volume = round(inner.volume * 100.0, 1)
             _LOGGER.debug("Volume changed to %0.1f", self.volume)
 


Test patch:
diff --git a/tests/protocols/mrp/test_mrp_interface.py b/tests/protocols/mrp/test_mrp_interface.py
index d2699d90..85527b01 100644
--- a/tests/protocols/mrp/test_mrp_interface.py
+++ b/tests/protocols/mrp/test_mrp_interface.py
@@ -25,6 +25,9 @@ async def volume_controls_changed(protocol, device_uid, controls_available):
 
 @pytest.fixture(name="audio")
 def audio_fixture(protocol_mock, mrp_state_dispatcher):
+    device_info = messages.device_information("test", "id")
+    device_info.inner().deviceUID = DEVICE_UID
+    protocol_mock.device_info = device_info
     yield MrpAudio(protocol_mock, mrp_state_dispatcher)
 
 
@@ -41,6 +44,7 @@ async def test_audio_volume_control_availability(protocol_mock, audio):
 @pytest.mark.parametrize(
     "device_uid,controls_available,controls_expected",
     [
+        ("foo", True, False),
         (DEVICE_UID, True, True),
     ],
 )
@@ -110,21 +114,6 @@ async def test_audio_volume_did_change_dispatches(
         assert math.isclose(message.value, expected_volume)
 
 
-async def test_audio_set_volume_only_device_info(protocol_mock, audio):
-    device_info = messages.device_information("test", "id")
-    device_info.inner().deviceUID = DEVICE_UID
-    protocol_mock.device_info = device_info
-
-    await audio.set_volume(0.0)
-
-    assert len(protocol_mock.sent_messages) == 1
-
-    message = protocol_mock.sent_messages.pop()
-    assert message.type == protobuf.SET_VOLUME_MESSAGE
-    assert message.inner().outputDeviceUID == DEVICE_UID
-    assert math.isclose(message.inner().volume, 0.0, rel_tol=1e-02)
-
-
 async def test_audio_set_volume(protocol_mock, audio):
     await volume_controls_changed(protocol_mock, DEVICE_UID, True)
     assert audio.is_available
@@ -139,6 +128,7 @@ async def test_audio_set_volume(protocol_mock, audio):
     assert math.isclose(message.inner().volume, 0.0, rel_tol=1e-02)
 
 
-async def test_audio_set_volume_no_output_device(audio):
+async def test_audio_set_volume_no_output_device(protocol_mock, audio):
+    protocol_mock.device_info = None
     with pytest.raises(exceptions.ProtocolError):
         await audio.set_volume(10)