Repository: postlund/pyatv
Pull request #1361 (2021-09-30 20:23:57)

airplay: Handle semver version in osvers
Relates to #1359

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



codecov[bot]: # [Codecov](https://codecov.io/gh/postlund/pyatv/pull/1361?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) Report
> Merging [#1361](https://codecov.io/gh/postlund/pyatv/pull/1361?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) (eb184a6) into [master](https://codecov.io/gh/postlund/pyatv/commit/65b40bd2d8df81a479b36e3dfaae54e88d56089f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) (65b40bd) will **decrease** coverage by `0.22%`.
> The diff coverage is `90.90%`.

[![Impacted file tree graph](https://codecov.io/gh/postlund/pyatv/pull/1361/graphs/tree.svg?width=650&height=150&src=pr&token=wb6azJjRSY&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)](https://codecov.io/gh/postlund/pyatv/pull/1361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)

```diff
@@            Coverage Diff             @@
##           master    #1361      +/-   ##
==========================================
- Coverage   92.71%   92.48%   -0.23%     
==========================================
  Files         149      149              
  Lines        9207     9198       -9     
==========================================
- Hits         8536     8507      -29     
- Misses        671      691      +20     
```


| [Impacted Files](https://codecov.io/gh/postlund/pyatv/pull/1361?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/airplay/remote\_control.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL2FpcnBsYXkvcmVtb3RlX2NvbnRyb2wucHk=) | `38.15% <ø> (-3.02%)` | :arrow_down: |
| [pyatv/protocols/airplay/\_\_init\_\_.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL2FpcnBsYXkvX19pbml0X18ucHk=) | `80.15% <66.66%> (ø)` | |
| [pyatv/protocols/airplay/utils.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL2FpcnBsYXkvdXRpbHMucHk=) | `100.00% <100.00%> (ø)` | |
| [pyatv/protocols/raop/raop.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL3Jhb3AvcmFvcC5weQ==) | `88.14% <0.00%> (-3.17%)` | :arrow_down: |
| [pyatv/protocols/mrp/connection.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL21ycC9jb25uZWN0aW9uLnB5) | `83.48% <0.00%> (-1.84%)` | :arrow_down: |
| [pyatv/support/net.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvc3VwcG9ydC9uZXQucHk=) | `91.66% <0.00%> (-1.67%)` | :arrow_down: |
| [pyatv/protocols/mrp/protocol.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL21ycC9wcm90b2NvbC5weQ==) | `91.89% <0.00%> (-1.40%)` | :arrow_down: |
| [pyatv/support/http.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvc3VwcG9ydC9odHRwLnB5) | `92.73% <0.00%> (-1.33%)` | :arrow_down: |
| [pyatv/core/mdns.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvY29yZS9tZG5zLnB5) | `90.14% <0.00%> (-0.71%)` | :arrow_down: |
| [pyatv/protocols/raop/audio\_source.py](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvcHJvdG9jb2xzL3Jhb3AvYXVkaW9fc291cmNlLnB5) | `86.18% <0.00%> (-0.10%)` | :arrow_down: |
| ... and [3 more](https://codecov.io/gh/postlund/pyatv/pull/1361/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/postlund/pyatv/pull/1361?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/postlund/pyatv/pull/1361?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl). Last update [65b40bd...eb184a6](https://codecov.io/gh/postlund/pyatv/pull/1361?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).


Patch:
diff --git a/pyatv/protocols/airplay/__init__.py b/pyatv/protocols/airplay/__init__.py
index 0ef41d6d..a4ca233b 100644
--- a/pyatv/protocols/airplay/__init__.py
+++ b/pyatv/protocols/airplay/__init__.py
@@ -22,7 +22,6 @@ from pyatv.interface import (
     Stream,
 )
 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.mrp_connection import AirPlayMrpConnection
 from pyatv.protocols.airplay.pairing import (
@@ -30,10 +29,12 @@ from pyatv.protocols.airplay.pairing import (
     get_preferred_auth_type,
 )
 from pyatv.protocols.airplay.player import AirPlayPlayer
+from pyatv.protocols.airplay.remote_control import RemoteControl
 from pyatv.protocols.airplay.utils import (
     AirPlayFlags,
     get_pairing_requirement,
     is_password_required,
+    is_remote_control_supported,
     parse_features,
 )
 from pyatv.support import net
@@ -216,14 +217,14 @@ def setup(  # pylint: disable=too-many-locals
     credentials = extract_credentials(service)
 
     # Set up remote control channel if it is supported
-    if not remote_control.is_supported(service):
+    if not is_remote_control_supported(service):
         _LOGGER.debug("Remote control not supported by device")
     elif credentials.type not in [AuthenticationType.HAP, AuthenticationType.Transient]:
         _LOGGER.debug("%s not supported by remote control channel", credentials.type)
     else:
         _LOGGER.debug("Remote control channel is supported")
 
-        control = remote_control.RemoteControl(device_listener)
+        control = RemoteControl(device_listener)
         control_port = service.port
 
         # When tunneling, we don't have any identifier or port available at this stage
diff --git a/pyatv/protocols/airplay/remote_control.py b/pyatv/protocols/airplay/remote_control.py
index 42ef9833..189caf31 100644
--- a/pyatv/protocols/airplay/remote_control.py
+++ b/pyatv/protocols/airplay/remote_control.py
@@ -9,7 +9,6 @@ from uuid import uuid4
 from pyatv.auth.hap_channel import setup_channel
 from pyatv.auth.hap_pairing import HapCredentials, PairVerifyProcedure
 from pyatv.core.protocol import heartbeater
-from pyatv.interface import BaseService
 from pyatv.protocols.airplay.auth import verify_connection
 from pyatv.protocols.airplay.channels import DataStreamChannel, EventChannel
 from pyatv.support.http import HttpConnection, http_connect
@@ -30,24 +29,6 @@ DATASTREAM_OUTPUT_INFO = "DataStream-Output-Encryption-Key"
 DATASTREAM_INPUT_INFO = "DataStream-Input-Encryption-Key"
 
 
-# TODO: It is not fully understood how to determine if a device supports remote control
-# over AirPlay, so this method makes a pure guess. We know that Apple TVs running tvOS
-# X (X>=13?) support it as well as HomePods, something we can identify from the model
-# string. This implementation should however be improved when it's properly known how
-# to check for support.
-def is_supported(service: BaseService) -> bool:
-    """Return if device supports remote control tunneling."""
-    model = service.properties.get("model", "")
-    if model.startswith("AudioAccessory"):
-        return True
-
-    if not model.startswith("AppleTV"):
-        return False
-
-    version = service.properties.get("osvers", "0.0")
-    return float(version) >= 13.0
-
-
 class RemoteControl:
     """MRP remote control session over AirPlay."""
 
diff --git a/pyatv/protocols/airplay/utils.py b/pyatv/protocols/airplay/utils.py
index e3813ab1..fcb3d13f 100644
--- a/pyatv/protocols/airplay/utils.py
+++ b/pyatv/protocols/airplay/utils.py
@@ -128,3 +128,21 @@ def get_pairing_requirement(service: BaseService) -> PairingRequirement:
         return PairingRequirement.Mandatory
 
     return PairingRequirement.NotNeeded
+
+
+# TODO: It is not fully understood how to determine if a device supports remote control
+# over AirPlay, so this method makes a pure guess. We know that Apple TVs running tvOS
+# X (X>=13?) support it as well as HomePods, something we can identify from the model
+# string. This implementation should however be improved when it's properly known how
+# to check for support.
+def is_remote_control_supported(service: BaseService) -> bool:
+    """Return if device supports remote control tunneling."""
+    model = service.properties.get("model", "")
+    if model.startswith("AudioAccessory"):
+        return True
+
+    if not model.startswith("AppleTV"):
+        return False
+
+    version = service.properties.get("osvers", "0.0").split(".", maxsplit=1)[0]
+    return float(version) >= 13.0


Test patch:
diff --git a/tests/protocols/airplay/test_utils.py b/tests/protocols/airplay/test_utils.py
index 6b24c0c1..31369528 100644
--- a/tests/protocols/airplay/test_utils.py
+++ b/tests/protocols/airplay/test_utils.py
@@ -7,6 +7,7 @@ from pyatv.protocols.airplay.utils import (
     AirPlayFlags,
     get_pairing_requirement,
     is_password_required,
+    is_remote_control_supported,
     parse_features,
 )
 
@@ -85,3 +86,19 @@ def test_is_password_required(properties, requires_password):
 async def test_get_pairing_requirement(props, expected_req):
     service = MutableService("id", Protocol.AirPlay, 0, props)
     assert get_pairing_requirement(service) == expected_req
+
+
+@pytest.mark.parametrize(
+    "props,expected_supported",
+    [
+        ({}, False),
+        ({"model": "AudioAccessory1,2"}, True),
+        ({"model": "Foo"}, False),
+        ({"osvers": "13.0"}, False),
+        ({"osvers": "13.0", "model": "AppleTV5,6"}, True),
+        ({"osvers": "8.4.4", "model": "AppleTV5,6"}, False),
+    ],
+)
+def test_is_remote_control_supported(props, expected_supported):
+    service = MutableService("id", Protocol.AirPlay, 0, props)
+    assert is_remote_control_supported(service) == expected_supported