Repository: postlund/pyatv

Issue:
Default supported commands are not handled
**Describe the bug**
The `SetStateMessage` contains the commands currently supported by a player, e.g. play, skip, etc. But there's another message used to define a default set of supported commands, eliminating the need to include them in `SetStateMessage`: `SetDefaultSupportedCommands`.

I expect that this should be saved and anything included in `SetStateMessage` should be added to the default set and potentially also override existing supported commands. An example message might look like this:

```
2020-12-25 11:00:20 DEBUG: 10.0.10.254:56679<->10.0.10.81:49153 << Receive: Protobuf: type: SET_DEFAULT_SUPPORTED_COMMANDS_MESSAGE
errorCode: NoError
[setDefaultSupportedCommandsMessage] {
  supportedCommands {
    supportedCommands {
      command: Play
      enabled: true
    }
    supportedCommands {
      command: PrepareForSetQueue
      enabled: true
    }
    supportedCommands {
      command: SetPlaybackQueue
      enabled: true
      supportedPlaybackQueueTypes: 7
      supportedPlaybackQueueTypes: 3
      supportedPlaybackQueueTypes: 6
      supportedPlaybackQueueTypes: 2
      supportedPlaybackQueueTypes: 5
      supportedPlaybackQueueTypes: 1
      supportedPlaybackQueueTypes: 8
      supportedCustomQueueIdentifiers: "com.apple.music.playbackqueue.radio"
      supportedCustomQueueIdentifiers: "com.apple.music.playbackqueue.tracklist"
      supportedCustomQueueIdentifiers: "com.apple.MediaPlaybackCore.playbackContextArchive-v1.opack"
      supportedCustomQueueIdentifiers: "com.apple.mediaplayer.playbackcontext"
    }
    supportedCommands {
      command: SetPlaybackSession
    }
    supportedCommands {
    }
  }
  displayID: "com.apple.TVMusic"
  8 {
  }
  9 {
    2 {
      2: "com.apple.TVMusic"
    }
  }
}
```

I have a few new fields to decode as well. The "9" seems suspicious and I probably need that to map the commands to the correct app.

**To Reproduce**
Seems to be used more frequently in tvOS 14 now, but has been present in the protocol earlier than that. Easiest is just to play something, check the log for this message and a corresponding `SetStateMessage`. Find something included `SetDefaultSupportedCommands` which is not in `SetStateMessage` and verify inconsistencies in the features interface.

**Expected behavior**
Correct feature state should be returned by the features interface.

**System Setup (please complete the following information):**
 - OS: any
 - Python: any
 - pyatv: master / 0.7.5
- Apple TV: Apple TV 4K tvOS 14.3

**Additional context**


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/postlund/pyatv/pull/924?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@280de0f`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/postlund/pyatv/pull/924/graphs/tree.svg?width=650&height=150&src=pr&token=wb6azJjRSY)](https://codecov.io/gh/postlund/pyatv/pull/924?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master     #924   +/-   ##
=========================================
  Coverage          ?   95.73%           
=========================================
  Files             ?       38           
  Lines             ?     3332           
  Branches          ?        0           
=========================================
  Hits              ?     3190           
  Misses            ?      142           
  Partials          ?        0           
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `95.73% <0.00%> (?)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.


------

[Continue to review full report at Codecov](https://codecov.io/gh/postlund/pyatv/pull/924?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/postlund/pyatv/pull/924?src=pr&el=footer). Last update [280de0f...52f4ddd](https://codecov.io/gh/postlund/pyatv/pull/924?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pyatv/mrp/player_state.py b/pyatv/mrp/player_state.py
index ef59805e..d5e626cb 100644
--- a/pyatv/mrp/player_state.py
+++ b/pyatv/mrp/player_state.py
@@ -3,6 +3,7 @@
 import math
 import logging
 import weakref
+from itertools import chain
 from copy import deepcopy
 from typing import Dict, Optional, List
 
@@ -15,7 +16,7 @@ _LOGGER = logging.getLogger(__name__)
 class PlayerState:
     """Represent what is currently playing on a device."""
 
-    def __init__(self, player: pb.NowPlayingPlayer):
+    def __init__(self, parent, player: pb.NowPlayingPlayer):
         """Initialize a new PlayerState instance."""
         self._playback_state = None
         self.supported_commands: List[pb.CommandInfo] = []
@@ -24,6 +25,7 @@ class PlayerState:
 
         self.identifier: Optional[str] = player.identifier
         self.display_name: Optional[str] = None
+        self.parent = parent
         self.update(player)
 
     @property
@@ -87,7 +89,7 @@ class PlayerState:
 
     def command_info(self, command):
         """Return supported command info."""
-        for cmd in self.supported_commands:
+        for cmd in chain(self.supported_commands, self.parent.supported_commands):
             if cmd.command == command:
                 return cmd
         return None
@@ -132,8 +134,15 @@ class Client:
         self.bundle_identifier: str = client.bundleIdentifier
         self.display_name: Optional[str] = None
         self.players: Dict[str, PlayerState] = {}
+        self.supported_commands: List[pb.CommandInfo] = []
         self.update(client)
 
+    def handle_set_default_supported_commands(self, supported_commands):
+        """Update default supported commands for client."""
+        self.supported_commands = deepcopy(
+            supported_commands.supportedCommands.supportedCommands
+        )
+
     def update(self, client: pb.NowPlayingClient):
         """Update client metadata."""
         self.display_name = client.displayName or self.display_name
@@ -161,6 +170,7 @@ class PlayerStateManager:
             pb.UPDATE_CLIENT_MESSAGE: self._handle_update_client,
             pb.REMOVE_CLIENT_MESSAGE: self._handle_remove_client,
             pb.REMOVE_PLAYER_MESSAGE: self._handle_remove_player,
+            pb.SET_DEFAULT_SUPPORTED_COMMANDS_MESSAGE: self._handle_set_default_supported_commands,  # noqa
             pb.VOLUME_CONTROL_AVAILABILITY_MESSAGE: self._volume_control_availability,
         }
         for message, handler in listeners.items():
@@ -179,7 +189,7 @@ class PlayerStateManager:
 
         player_id = player_path.player.identifier
         if player_id not in client.players:
-            client.players[player_id] = PlayerState(player_path.player)
+            client.players[player_id] = PlayerState(client, player_path.player)
         return client.players[player_id]
 
     @property
@@ -213,7 +223,7 @@ class PlayerStateManager:
             )
             if default_player:
                 return default_player
-        return PlayerState(pb.NowPlayingPlayer())
+        return PlayerState(Client(pb.NowPlayingClient()), pb.NowPlayingPlayer())
 
     async def _handle_set_state(self, message, _):
         setstate = message.inner()
@@ -270,6 +280,7 @@ class PlayerStateManager:
         if player.is_valid:
             client = self.get_client(player_to_remove.client)
             del client.players[player.identifier]
+            player.parent = None
 
             removed = False
             if player == self._active_player:
@@ -282,6 +293,14 @@ class PlayerStateManager:
             if removed:
                 await self._state_updated()
 
+    async def _handle_set_default_supported_commands(self, message, _):
+        supported_commands = message.inner()
+
+        client = self.get_client(supported_commands.playerPath.client)
+        client.handle_set_default_supported_commands(supported_commands)
+
+        await self._state_updated()
+
     async def _handle_update_client(self, message, _):
         update_client = message.inner()
 


Test patch:
diff --git a/tests/fake_device/mrp.py b/tests/fake_device/mrp.py
index a3050cc9..18aaf21f 100644
--- a/tests/fake_device/mrp.py
+++ b/tests/fake_device/mrp.py
@@ -247,6 +247,16 @@ class FakeMrpState:
         msg.inner().volumeControlAvailable = available
         self._send(msg)
 
+    def default_supported_commands(self, commands):
+        msg = messages.create(protobuf.SET_DEFAULT_SUPPORTED_COMMANDS_MESSAGE)
+        supported_commands = msg.inner().supportedCommands.supportedCommands
+        for command in commands:
+            item = supported_commands.add()
+            item.command = command
+            item.enabled = True
+        msg.inner().playerPath.client.bundleIdentifier = PLAYER_IDENTIFIER
+        self._send(msg)
+
 
 class FakeMrpServiceFactory:
     def __init__(self, state, app, loop):
@@ -596,3 +606,7 @@ class FakeMrpUseCases:
         metadata = PlayingState(playback_state=PlaybackState.Interrupted)
         self.state.set_player_state(PLAYER_IDENTIFIER, metadata)
         self.state.set_active_player(PLAYER_IDENTIFIER)
+
+    def default_supported_commands(self, commands):
+        """Call to set default supported commands."""
+        self.state.default_supported_commands(commands)
diff --git a/tests/mrp/test_mrp_functional.py b/tests/mrp/test_mrp_functional.py
index 23f2493b..760f4cd3 100644
--- a/tests/mrp/test_mrp_functional.py
+++ b/tests/mrp/test_mrp_functional.py
@@ -425,3 +425,13 @@ class MRPFunctionalTest(common_functional_tests.CommonFunctionalTests):
         await self.playing(title="test")
         self.assertEqual(self.atv.metadata.app.name, APP_NAME)
         self.assertEqual(self.atv.metadata.app.identifier, TEST_PLAYER)
+
+    @unittest_run_loop
+    async def test_set_default_commands(self):
+        self.usecase.default_supported_commands(
+            [CommandInfo_pb2.Play, CommandInfo_pb2.Pause]
+        )
+        self.usecase.example_video()
+
+        await self.playing(title="dummy")
+        self.assertFeatures(FeatureState.Available, FeatureName.Play, FeatureName.Pause)
diff --git a/tests/mrp/test_player_state.py b/tests/mrp/test_player_state.py
index 700b16ea..7b4de2ff 100644
--- a/tests/mrp/test_player_state.py
+++ b/tests/mrp/test_player_state.py
@@ -433,3 +433,28 @@ async def test_volume_control_availability(psm, protocol, listener):
 
     assert listener.call_count == 2
     assert not psm.volume_controls_available
+
+
+@pytest.mark.asyncio
+async def test_set_default_supported_commands(psm, protocol, listener):
+    msg = messages.create(pb.SET_DEFAULT_SUPPORTED_COMMANDS_MESSAGE)
+    supported_commands = msg.inner().supportedCommands.supportedCommands
+    command = supported_commands.add()
+    command.command = pb.CommandInfo_pb2.Play
+    msg.inner().playerPath.client.bundleIdentifier = CLIENT_ID_1
+    await protocol.inject(msg)
+
+    msg = messages.create(pb.SET_NOW_PLAYING_CLIENT_MESSAGE)
+    client = msg.inner().client
+    client.bundleIdentifier = CLIENT_ID_1
+    await protocol.inject(msg)
+
+    # Default commands are set on client, so any player belonging to that client
+    # should have the supported command
+    player_path = pb.PlayerPath()
+    player_path.client.bundleIdentifier = CLIENT_ID_1
+    player_path.player.identifier = PLAYER_ID_1
+    player = psm.get_player(player_path)
+
+    assert player.command_info(pb.CommandInfo_pb2.Play)
+    assert listener.call_count == 2