Repository: postlund/pyatv (python)

Issue:
Enhance feature detection of play_url and improve error handling
### What feature would you like?

Feature availability of `play_url` is solely based on credentials being available or not. This is generally the case as credentials are needed for AirPlay on the Apple TV (which supports `play_url`) and not on the HomePod or other AirPlay receivers (which does *not*  support `play_url`). An edge case is older Apple TV devices with device verification turned off, where it will reported as unsupported even if is supported.

### Describe the solution you'd like

A better way is to check if `SupportsAirPlayVideoV1` or `SupportsAirPlayVideoV2` is present in the AirPlay feature flags.

An additional enhancement is to check availability of that feature in `play_url` and raise an exception in case it is not. This will make it more clear to users calling that function, expecting it to work on the HomePod. Preferably in the facade.

### Any other information to share?

This should be included in 0.9.0 as I find it important.

Pull request description:
airplay: Improve feature availability support and error handling
Base feature availability of `play_url` from feature flag in zeroconf. Also raise exception if call is made to `play_url` when it's not supported.

Fixes #1308

<a href="https://gitpod.io/#https://github.com/postlund/pyatv/pull/1310"><img src="https://gitpod.io/button/open-in-gitpod.svg"/></a>



Patch:
diff --git a/pyatv/core/facade.py b/pyatv/core/facade.py
index 1a531d7a..17ad213d 100644
--- a/pyatv/core/facade.py
+++ b/pyatv/core/facade.py
@@ -287,9 +287,10 @@ class FacadePower(Relayer, interface.Power, interface.PowerListener):
 class FacadeStream(Relayer, interface.Stream):  # pylint: disable=too-few-public-methods
     """Facade implementation for stream functionality."""
 
-    def __init__(self):
+    def __init__(self, features: interface.Features):
         """Initialize a new FacadeStream instance."""
         super().__init__(interface.Stream, DEFAULT_PRIORITIES)
+        self._features = features
 
     def close(self) -> None:
         """Close connection and release allocated resources."""
@@ -297,6 +298,9 @@ class FacadeStream(Relayer, interface.Stream):  # pylint: disable=too-few-public
 
     async def play_url(self, url: str, **kwargs) -> None:
         """Play media from an URL on the device."""
+        if not self._features.in_state(FeatureState.Available, FeatureName.PlayUrl):
+            raise exceptions.NotSupportedError("play_url is not supported")
+
         await self.relay("play_url")(url, **kwargs)
 
     async def stream_file(self, file: Union[str, io.BufferedReader], **kwargs) -> None:
@@ -376,7 +380,7 @@ class FacadeAppleTV(interface.AppleTV):
             interface.Metadata: FacadeMetadata(),
             interface.Power: FacadePower(),
             interface.PushUpdater: self._push_updates,
-            interface.Stream: FacadeStream(),
+            interface.Stream: FacadeStream(self._features),
             interface.Apps: FacadeApps(),
             interface.Audio: FacadeAudio(),
         }
diff --git a/pyatv/protocols/airplay/__init__.py b/pyatv/protocols/airplay/__init__.py
index a4fdbf9b..ceab13d6 100644
--- a/pyatv/protocols/airplay/__init__.py
+++ b/pyatv/protocols/airplay/__init__.py
@@ -25,6 +25,7 @@ from pyatv.interface import (
 from pyatv.protocols import mrp
 from pyatv.protocols.airplay import remote_control
 from pyatv.protocols.airplay.auth import extract_credentials, verify_connection
+from pyatv.protocols.airplay.features import AirPlayFlags, parse
 from pyatv.protocols.airplay.mrp_connection import AirPlayMrpConnection
 from pyatv.protocols.airplay.pairing import (
     AirPlayPairingHandler,
@@ -47,11 +48,14 @@ class AirPlayFeatures(Features):
     def __init__(self, service: conf.AirPlayService) -> None:
         """Initialize a new AirPlayFeatures instance."""
         self.service = service
+        self._features = parse(self.service.properties.get("features", "0x0"))
 
     def get_feature(self, feature_name: FeatureName) -> FeatureInfo:
         """Return current state of a feature."""
-        has_credentials = self.service.credentials
-        if feature_name == FeatureName.PlayUrl and has_credentials:
+        if feature_name == FeatureName.PlayUrl and (
+            AirPlayFlags.SupportsAirPlayVideoV1 in self._features
+            or AirPlayFlags.SupportsAirPlayVideoV2 in self._features
+        ):
             return FeatureInfo(FeatureState.Available)
 
         return FeatureInfo(FeatureState.Unavailable)
diff --git a/pyatv/protocols/airplay/auth/__init__.py b/pyatv/protocols/airplay/auth/__init__.py
index e497c960..cc1fe4af 100644
--- a/pyatv/protocols/airplay/auth/__init__.py
+++ b/pyatv/protocols/airplay/auth/__init__.py
@@ -122,8 +122,8 @@ def extract_credentials(service: BaseService) -> HapCredentials:
 
     features = ft.parse(service.properties.get("features", "0x0"))
     if (
-        ft.AirPlayFeatures.SupportsSystemPairing in features
-        or ft.AirPlayFeatures.SupportsCoreUtilsPairingAndEncryption in features
+        ft.AirPlayFlags.SupportsSystemPairing in features
+        or ft.AirPlayFlags.SupportsCoreUtilsPairingAndEncryption in features
     ):
         return TRANSIENT_CREDENTIALS
 
diff --git a/pyatv/protocols/airplay/features.py b/pyatv/protocols/airplay/features.py
index 743f8228..4b23791b 100644
--- a/pyatv/protocols/airplay/features.py
+++ b/pyatv/protocols/airplay/features.py
@@ -5,7 +5,7 @@ import re
 # pylint: disable=invalid-name
 
 
-class AirPlayFeatures(IntFlag):
+class AirPlayFlags(IntFlag):
     """Features supported by AirPlay."""
 
     SupportsAirPlayVideoV1 = 1 << 0
@@ -54,7 +54,7 @@ class AirPlayFeatures(IntFlag):
 # pylint: enable=invalid-name
 
 
-def parse(features: str) -> AirPlayFeatures:
+def parse(features: str) -> AirPlayFlags:
     """Parse an AirPlay feature string and return what is supported.
 
     A feature string have one of the following formats:
@@ -68,4 +68,4 @@ def parse(features: str) -> AirPlayFeatures:
     value, upper = match.groups()
     if upper is not None:
         value = upper + value
-    return AirPlayFeatures(int(value, 16))
+    return AirPlayFlags(int(value, 16))
diff --git a/pyatv/protocols/airplay/pairing.py b/pyatv/protocols/airplay/pairing.py
index 2c2d50bc..3b02eb8d 100644
--- a/pyatv/protocols/airplay/pairing.py
+++ b/pyatv/protocols/airplay/pairing.py
@@ -6,7 +6,7 @@ from pyatv import conf, exceptions
 from pyatv.auth.hap_pairing import PairSetupProcedure
 from pyatv.interface import BaseService, PairingHandler
 from pyatv.protocols.airplay.auth import AuthenticationType, pair_setup
-from pyatv.protocols.airplay.features import AirPlayFeatures, parse
+from pyatv.protocols.airplay.features import AirPlayFlags, parse
 from pyatv.support import error_handler
 from pyatv.support.http import ClientSessionManager, HttpConnection, http_connect
 
@@ -18,7 +18,7 @@ def get_preferred_auth_type(service: BaseService) -> AuthenticationType:
     features_string = service.properties.get("features")
     if features_string:
         features = parse(features_string)
-        if AirPlayFeatures.SupportsCoreUtilsPairingAndEncryption in features:
+        if AirPlayFlags.SupportsCoreUtilsPairingAndEncryption in features:
             return AuthenticationType.HAP
     return AuthenticationType.Legacy
 
diff --git a/pyatv/protocols/raop/__init__.py b/pyatv/protocols/raop/__init__.py
index 65b38b06..fffe3caf 100644
--- a/pyatv/protocols/raop/__init__.py
+++ b/pyatv/protocols/raop/__init__.py
@@ -506,7 +506,7 @@ def pair(
         raise exceptions.NotSupportedError("pairing not required")
 
     parsed = ap_features.parse(features)
-    if ap_features.AirPlayFeatures.SupportsLegacyPairing not in parsed:
+    if ap_features.AirPlayFlags.SupportsLegacyPairing not in parsed:
         raise exceptions.NotSupportedError("legacy pairing not supported")
 
     return AirPlayPairingHandler(


Test patch:
diff --git a/tests/common_functional_tests.py b/tests/common_functional_tests.py
index 8e015459..1cc143d2 100644
--- a/tests/common_functional_tests.py
+++ b/tests/common_functional_tests.py
@@ -518,9 +518,10 @@ class CommonFunctionalTests(AioHTTPTestCase):
 
     @unittest_run_loop
     async def test_features_play_url(self):
-        # TODO: The test always sets up AirPlay, so PlayUrl will always be available.
-        # In the future (after migrating to pytest fixtures), I will add a test where
-        # AirPlay is not available.
+        # TODO: As availability is based on zeroconf properties, this test just
+        # verifies that PlayUrl is available. It's hard to change zeroconf properties
+        # between test runs here, so better tests will be written when dedicated
+        # functional tests for AirPlay are written.
         self.assertFeatures(FeatureState.Available, FeatureName.PlayUrl)
 
     @unittest_run_loop
diff --git a/tests/conftest.py b/tests/conftest.py
index ac7921e5..e78414eb 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -82,7 +82,7 @@ def stub_heartbeat_loop(request):
 async def session_manager():
     session_manager = await create_session()
     yield session_manager
-    session_manager.close()
+    await session_manager.close()
 
 
 @pytest.fixture
diff --git a/tests/core/test_facade.py b/tests/core/test_facade.py
index 6bfe99f2..da7df953 100644
--- a/tests/core/test_facade.py
+++ b/tests/core/test_facade.py
@@ -21,8 +21,11 @@ from pyatv.interface import (
     FeatureState,
     Power,
     PushUpdater,
+    Stream,
 )
 
+TEST_URL = "http://test"
+
 pytestmark = pytest.mark.asyncio
 
 
@@ -129,6 +132,14 @@ class DummyDeviceListener(DeviceListener):
         self.closed_calls += 1
 
 
+class DummyStream(Stream):
+    def __init__(self) -> None:
+        self.url = None
+
+    async def play_url(self, url: str, **kwargs) -> None:
+        self.url = url
+
+
 @pytest.fixture(name="register_interface")
 def register_interface_fixture(facade_dummy):
     def _register_func(feature: FeatureName, instance, protocol: Protocol):
@@ -415,3 +426,27 @@ async def tests_device_info_from_multiple_protocols(facade_dummy, register_inter
     assert dev_info.operating_system == OperatingSystem.TvOS
     assert dev_info.version == "1.0"
     assert dev_info.build_number == "ABC"
+
+
+async def test_stream_play_url_not_available(facade_dummy, register_interface):
+    stream, _ = register_interface(FeatureName.Volume, DummyStream(), Protocol.RAOP)
+
+    await facade_dummy.connect()
+
+    with pytest.raises(exceptions.NotSupportedError):
+        await facade_dummy.stream.play_url(TEST_URL)
+
+
+async def test_stream_play_url_available(facade_dummy, register_interface):
+    stream, _ = register_interface(FeatureName.PlayUrl, DummyStream(), Protocol.RAOP)
+
+    # play_url requires FeatureName.PlayUrl to be available, so add the feature interface
+    register_interface(
+        FeatureName.PlayUrl, DummyFeatures(FeatureName.PlayUrl), Protocol.DMAP
+    )
+
+    await facade_dummy.connect()
+
+    await facade_dummy.stream.play_url(TEST_URL)
+
+    assert stream.url == TEST_URL
diff --git a/tests/fake_udns.py b/tests/fake_udns.py
index bf5e1c43..7074b2c8 100644
--- a/tests/fake_udns.py
+++ b/tests/fake_udns.py
@@ -58,7 +58,7 @@ def airplay_service(
         name=atv_name,
         addresses=addresses,
         port=port,
-        properties={"deviceid": deviceid.encode("utf-8")},
+        properties={"deviceid": deviceid.encode("utf-8"), "features": b"0x1"},
         model=model,
     )
     return ("_airplay._tcp.local", service)
diff --git a/tests/protocols/airplay/test_airplay_interface.py b/tests/protocols/airplay/test_airplay_interface.py
new file mode 100644
index 00000000..814ff899
--- /dev/null
+++ b/tests/protocols/airplay/test_airplay_interface.py
@@ -0,0 +1,23 @@
+"""Unit tests for interface implementations in pyatv.protocols.airplay."""
+
+import pytest
+
+from pyatv.conf import AirPlayService
+from pyatv.const import FeatureName, FeatureState
+from pyatv.protocols.airplay import AirPlayFeatures
+
+# AirPlayFeatures
+
+
+@pytest.mark.parametrize(
+    "flags,expected_state",
+    [
+        ("0x0,0x0", FeatureState.Unavailable),
+        ("0x1,0x0", FeatureState.Available),  # VideoV1
+        ("0x00000000,0x20000", FeatureState.Available),  # VideoV2
+    ],
+)
+def test_feature_play_url(flags, expected_state):
+    service = AirPlayService("id", properties={"features": flags})
+    features = AirPlayFeatures(service)
+    assert features.get_feature(FeatureName.PlayUrl).state == expected_state
diff --git a/tests/protocols/airplay/test_features.py b/tests/protocols/airplay/test_features.py
index 2f8f554f..921d6634 100644
--- a/tests/protocols/airplay/test_features.py
+++ b/tests/protocols/airplay/test_features.py
@@ -1,26 +1,26 @@
 """Unit tests for pyatv.protocols.airplay.features."""
 import pytest
 
-from pyatv.protocols.airplay.features import AirPlayFeatures, parse
+from pyatv.protocols.airplay.features import AirPlayFlags, parse
 
 
 @pytest.mark.parametrize(
     "flags,output",
     [
         # Single feature flag
-        ("0x00000001", AirPlayFeatures.SupportsAirPlayVideoV1),
+        ("0x00000001", AirPlayFlags.SupportsAirPlayVideoV1),
         (
             "0x40000003",
-            AirPlayFeatures.HasUnifiedAdvertiserInfo
-            | AirPlayFeatures.SupportsAirPlayPhoto
-            | AirPlayFeatures.SupportsAirPlayVideoV1,
+            AirPlayFlags.HasUnifiedAdvertiserInfo
+            | AirPlayFlags.SupportsAirPlayPhoto
+            | AirPlayFlags.SupportsAirPlayVideoV1,
         ),
         # Dual feature flag
         (
             "0x00000003,0x00000001",
-            AirPlayFeatures.IsCarPlay
-            | AirPlayFeatures.SupportsAirPlayPhoto
-            | AirPlayFeatures.SupportsAirPlayVideoV1,
+            AirPlayFlags.IsCarPlay
+            | AirPlayFlags.SupportsAirPlayPhoto
+            | AirPlayFlags.SupportsAirPlayVideoV1,
         ),
     ],
 )
diff --git a/tests/protocols/dmap/test_dmap_functional.py b/tests/protocols/dmap/test_dmap_functional.py
index a1f7d878..8afc3489 100644
--- a/tests/protocols/dmap/test_dmap_functional.py
+++ b/tests/protocols/dmap/test_dmap_functional.py
@@ -77,7 +77,10 @@ class DMAPFunctionalTest(common_functional_tests.CommonFunctionalTests):
     async def get_connected_device(self, hsgid):
         self.dmap_service = DmapService("dmapid", hsgid, port=self.server.port)
         self.airplay_service = AirPlayService(
-            "airplay_id", self.server.port, DEVICE_CREDENTIALS
+            "airplay_id",
+            self.server.port,
+            DEVICE_CREDENTIALS,
+            properties={"features": "0x1"},  # AirPlayVideoV1 supported
         )
         self.conf = AppleTV(ipaddress.IPv4Address("127.0.0.1"), "Apple TV")
         self.conf.add_service(self.dmap_service)
diff --git a/tests/protocols/mrp/test_mrp_functional.py b/tests/protocols/mrp/test_mrp_functional.py
index 13f620c7..c639912b 100644
--- a/tests/protocols/mrp/test_mrp_functional.py
+++ b/tests/protocols/mrp/test_mrp_functional.py
@@ -54,7 +54,12 @@ class MRPFunctionalTest(common_functional_tests.CommonFunctionalTests):
             MrpService("mrp_id", self.fake_atv.get_port(Protocol.MRP))
         )
         self.conf.add_service(
-            AirPlayService("airplay_id", self.server.port, DEVICE_CREDENTIALS)
+            AirPlayService(
+                "airplay_id",
+                self.server.port,
+                DEVICE_CREDENTIALS,
+                properties={"features": "0x1"},  # AirPlayVideoV1 supported
+            )
         )
         self.atv = await self.get_connected_device()