Repository: ikalchev/HAP-python

Issue:
New Accessory Typs + Services
It seems their might have been new accessories added with iOS version 12.2 beta 1.
I recently saw a feature request to add a TV service and found this afterwards:
https://github.com/KhaosT/HAP-NodeJS/pull/638

I tried to download the current HAP specifications, but it seems they aren't available at the moment. At least I always get an error message. Maybe (Hopefully) they are being updated now.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/ikalchev/HAP-python/pull/187?src=pr&el=h1) Report
> Merging [#187](https://codecov.io/gh/ikalchev/HAP-python/pull/187?src=pr&el=desc) into [dev](https://codecov.io/gh/ikalchev/HAP-python/commit/5ac5b8307b1c66481958c60fb84691ea4842aca8?src=pr&el=desc) will **increase** coverage by `0.07%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##              dev     #187      +/-   ##
==========================================
+ Coverage   60.45%   60.52%   +0.07%     
==========================================
  Files          16       16              
  Lines        1669     1672       +3     
  Branches      169      169              
==========================================
+ Hits         1009     1012       +3     
  Misses        622      622              
  Partials       38       38
```

| [Impacted Files](https://codecov.io/gh/ikalchev/HAP-python/pull/187?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pyhap/const.py](https://codecov.io/gh/ikalchev/HAP-python/pull/187/diff?src=pr&el=tree#diff-cHloYXAvY29uc3QucHk=) | `100% <100%> (ø)` | :arrow_up: |

adrum: I have this functional in HomeAssistant in [this branch](https://github.com/adrum/home-assistant/tree/add-homekit-television?files=1). I'll submit a PR there once this has been merged and a new version has been released. 
cdce8p: Hi, awesome that you've put in the work to manually copy the chars and services :+1: 
I haven't had the time to look at you're changes in detail yet.

Some things that might help you:
- If a char has `uint8` as its format, it will only look at the corresponding `valid_values`. `Min` and `max` are in therefore redundant.
- I saw you've added the distinction between `primary` and `linked` services. If I recall correctly, and I hope I do (it has been a while), the `HAP` protocol automatically interprets the first added service as the `primary` one. All others are automatically the `linked` ones.
ikalchev: Hey, so great that you made this change! Let me know when it is ready to merge it. I think I will do a pip release with the merge as well.

Best,
Ivan
adrum: @cdce8p I've removed the redundant min/max values where all the valid values were filled out. I wasn't sure what to do with ones like `RemoteKey` since there were some missing. This is how it is in the HAP-NodeJS project as well. 

When I was testing my HomeAssistant branch with my changes for this library, my Input Sources weren't properly showing to be associated with the television until I added the ability to set the services as linked in the `to_HAP` function. I wasn't too sure about the primary service part of it, so I went ahead and added it just in case.
adrum: @cdce8p I just tested removing the `add_linked_service` and `is_primary_service` and the source inputs would not show up as services on the tv services. Based on my testing, these seem to be required for the `Television` service to work properly.
ikalchev: @adrum I am Ok to merge this if you are ready.
adrum: @ikalchev I think this is ready to go.

Patch:
diff --git a/pyhap/accessory.py b/pyhap/accessory.py
index ed103a6..f5115b1 100644
--- a/pyhap/accessory.py
+++ b/pyhap/accessory.py
@@ -115,6 +115,12 @@ class Accessory:
         self.add_service(service)
         return service
 
+    def set_primary_service(self, primary_service):
+        """Set the primary service of the acc."""
+        for service in self.services:
+            service.is_primary_service = service.type_id == \
+                primary_service.type_id
+
     def config_changed(self):
         """Notify the accessory about configuration changes.
 
diff --git a/pyhap/const.py b/pyhap/const.py
index e6ce1ca..13aab7f 100644
--- a/pyhap/const.py
+++ b/pyhap/const.py
@@ -45,6 +45,8 @@ CATEGORY_SPEAKER = 26
 CATEGORY_SPRINKLER = 28
 CATEGORY_FAUCET = 29
 CATEGORY_SHOWER_HEAD = 30
+CATEGORY_TELEVISION = 31
+CATEGORY_TARGET_CONTROLLER = 32  # Remote Controller
 
 
 # ### HAP Permissions ###
@@ -52,6 +54,7 @@ HAP_PERMISSION_HIDDEN = 'hd'
 HAP_PERMISSION_NOTIFY = 'ev'
 HAP_PERMISSION_READ = 'pr'
 HAP_PERMISSION_WRITE = 'pw'
+HAP_PERMISSION_WRITE_RESPONSE = 'wr'
 
 
 # ### HAP representation ###
@@ -63,7 +66,9 @@ HAP_REPR_FORMAT = 'format'
 HAP_REPR_IID = 'iid'
 HAP_REPR_MAX_LEN = 'maxLen'
 HAP_REPR_PERM = 'perms'
+HAP_REPR_PRIMARY = 'primary'
 HAP_REPR_SERVICES = 'services'
+HAP_REPR_LINKED = 'linked'
 HAP_REPR_STATUS = 'status'
 HAP_REPR_TYPE = 'type'
 HAP_REPR_VALUE = 'value'
diff --git a/pyhap/resources/characteristics.json b/pyhap/resources/characteristics.json
index 691753b..9128162 100644
--- a/pyhap/resources/characteristics.json
+++ b/pyhap/resources/characteristics.json
@@ -23,6 +23,15 @@
          "Inactive": 0
       }
    },
+   "ActiveIdentifier": {
+      "Format": "uint32",
+      "Permissions": [
+         "pr",
+         "pw",
+         "ev"
+      ],
+      "UUID": "000000E7-0000-1000-8000-0026BB765291"
+   },
    "AdministratorOnlyAccess": {
       "Format": "bool",
       "Permissions": [
@@ -182,6 +191,19 @@
          "NotCharging": 0
       }
    },
+   "ClosedCaptions": {
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "pw",
+         "ev"
+      ],
+      "UUID": "000000DD-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Disabled": 0,
+         "Enabled": 1
+      }
+   },
    "ColorTemperature": {
       "Format": "uint32",
       "Permissions": [
@@ -194,6 +216,15 @@
       "minStep": 1,
       "minValue": 140
    },
+   "ConfiguredName": {
+      "Format": "string",
+      "Permissions": [
+         "pr",
+         "pw",
+         "ev"
+      ],
+      "UUID": "000000E3-0000-1000-8000-0026BB765291"
+   },
    "ContactSensorState": {
       "Format": "uint8",
       "Permissions": [
@@ -324,6 +355,20 @@
          "Inactive": 0
       }
    },
+   "CurrentMediaState": {
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "ev"
+      ],
+      "UUID": "000000E0-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Play": 0,
+         "Pause": 1,
+         "Stop": 2,
+         "Unknown": 3
+      }
+   },
    "CurrentPosition": {
       "Format": "uint8",
       "Permissions": [
@@ -397,6 +442,18 @@
       "minValue": -90,
       "unit": "arcdegrees"
    },
+   "CurrentVisibilityState":{
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "ev"
+      ],
+      "UUID": "00000135-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Shown": 0,
+         "Hidden": 1
+      }
+   },
    "DigitalZoom": {
       "Format": "float",
       "Permissions": [
@@ -406,6 +463,15 @@
       ],
       "UUID": "0000011D-0000-1000-8000-0026BB765291"
    },
+   "DisplayOrder": {
+      "Format": "tlv8",
+      "Permissions": [
+         "pr",
+         "pw",
+         "ev"
+      ],
+      "UUID": "00000136-0000-1000-8000-0026BB765291"
+   },
    "FilterChangeIndication": {
       "Format": "uint8",
       "Permissions": [
@@ -476,6 +542,15 @@
       "minValue": 0,
       "unit": "arcdegrees"
    },
+   "Identifier": {
+      "Format": "uint32",
+      "Permissions": [
+         "pr"
+      ],
+      "UUID": "000000E6-0000-1000-8000-0026BB765291",
+      "minStep": 1,
+      "minValue": 0
+   },
    "Identify": {
       "Format": "bool",
       "Permissions": [
@@ -505,6 +580,43 @@
       "minValue": 0,
       "unit": "arcdegrees"
    },
+   "InputSourceType":{
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "ev"
+      ],
+      "UUID": "000000DB-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Other": 0,
+         "HomeScreen": 1,
+         "Tuner": 2,
+         "HDMI": 3,
+         "CompositeVideo": 4,
+         "SVideo": 5,
+         "ComponentVideo": 6,
+         "DVI": 7,
+         "AirPlay": 8,
+         "USB": 9,
+         "Application": 10
+      }
+   },
+   "InputDeviceType":{
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "ev"
+      ],
+      "UUID": "000000DC-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Other": 0,
+         "TV": 1,
+         "Recording": 2,
+         "Tuner": 3,
+         "Playback": 4,
+         "AudioSystem": 5
+      }
+   },
    "InUse": {
       "Format": "uint8",
       "Permissions": [
@@ -794,6 +906,27 @@
       ],
       "UUID": "00000050-0000-1000-8000-0026BB765291"
    },
+   "PictureMode":{
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "pw",
+         "ev"
+      ],
+      "UUID": "000000E2-0000-1000-8000-0026BB765291",
+      "maxValue": 13,
+      "minValue": 0,
+      "ValidValues": {
+         "Other": 0,
+         "Standard": 1,
+         "Calibrated": 2,
+         "CalibratedDark": 3,
+         "Vivid": 4,
+         "Game": 5,
+         "Computer": 6,
+         "Custom": 7
+      }
+   },
    "PositionState": {
       "Format": "uint8",
       "Permissions": [
@@ -807,6 +940,17 @@
          "Stopped": 2
       }
    },
+   "PowerModeSelection":{
+      "Format": "uint8",
+      "Permissions": [
+         "pw"
+      ],
+      "UUID": "000000DF-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Show": 0,
+         "Hide": 1
+      }
+   },
    "ProgramMode": {
       "Format": "uint8",
       "Permissions": [
@@ -870,6 +1014,30 @@
       "minStep": 1,
       "minValue": 0
    },
+   "RemoteKey":{
+      "Format": "uint8",
+      "Permissions": [
+         "pw"
+      ],
+      "UUID": "000000E1-0000-1000-8000-0026BB765291",
+      "maxValue": 16,
+      "minValue": 0,
+      "ValidValues": {
+         "Rewind": 0,
+         "FastForward": 1,
+         "NextTrack": 2,
+         "PreviousTrack": 3,
+         "ArrowUp": 4,
+         "ArrowDown": 5,
+         "ArrowLeft": 6,
+         "ArrowRight": 7,
+         "Select": 8,
+         "Back": 9,
+         "Exit": 10,
+         "PlayPause": 11,
+         "Information": 15
+      }
+   },
    "ResetFilterIndication": {
       "Format": "uint8",
       "Permissions": [
@@ -1028,6 +1196,18 @@
          "Vertical": 1
       }
    },
+   "SleepDiscoveryMode": {
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "ev"
+      ],
+      "UUID": "000000E8-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "NotDiscoverable": 0,
+         "AlwaysDiscoverable": 1
+      }
+   },
    "SmokeDetected": {
       "Format": "uint8",
       "Permissions": [
@@ -1258,6 +1438,20 @@
          "HumidifierorDehumidifier": 0
       }
    },
+   "TargetMediaState": {
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "pw",
+         "ev"
+      ],
+      "UUID": "00000137-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Play": 0,
+         "Pause": 1,
+         "Stop": 2
+      }
+   },
    "TargetPosition": {
       "Format": "uint8",
       "Permissions": [
@@ -1336,6 +1530,19 @@
       "minValue": -90,
       "unit": "arcdegrees"
    },
+   "TargetVisibilityState":{
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "pw",
+         "ev"
+      ],
+      "UUID": "00000134-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Shown": 0,
+         "Hidden": 1
+      }
+   },
    "TemperatureDisplayUnits": {
       "Format": "uint8",
       "Permissions": [
@@ -1396,6 +1603,31 @@
       "minValue": 0,
       "unit": "percentage"
    },
+   "VolumeControlType": {
+      "Format": "uint8",
+      "Permissions": [
+         "pr",
+         "ev"
+      ],
+      "UUID": "000000E9-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "None": 0,
+         "Relative": 1,
+         "RelativeWithCurrent": 2,
+         "Absolute": 3
+      }
+   },
+   "VolumeSelector": {
+      "Format": "uint8",
+      "Permissions": [
+         "pw"
+      ],
+      "UUID": "000000EA-0000-1000-8000-0026BB765291",
+      "ValidValues": {
+         "Increment": 0,
+         "Decrement": 1
+      }
+   },
    "WaterLevel": {
       "Format": "float",
       "Permissions": [
diff --git a/pyhap/resources/services.json b/pyhap/resources/services.json
index a2b829c..3358e47 100644
--- a/pyhap/resources/services.json
+++ b/pyhap/resources/services.json
@@ -250,6 +250,21 @@
       ],
       "UUID": "00000082-0000-1000-8000-0026BB765291"
    },
+   "InputSource": {
+      "OptionalCharacteristics": [
+         "Identifier",
+         "InputDeviceType",
+         "TargetVisibilityState",
+         "Name"
+      ],
+      "RequiredCharacteristics": [
+         "ConfiguredName",
+         "InputSourceType",
+         "IsConfigured",
+         "CurrentVisibilityState"
+      ],
+      "UUID": "000000D9-0000-1000-8000-0026BB765291"
+   },
    "IrrigationSystem": {
       "OptionalCharacteristics": [
          "Name",
@@ -451,6 +466,38 @@
       ],
       "UUID": "00000049-0000-1000-8000-0026BB765291"
    },
+   "Television": {
+      "OptionalCharacteristics": [
+         "Brightness",
+         "ClosedCaptions",
+         "DisplayOrder",
+         "CurrentMediaState",
+         "TargetMediaState",
+         "PictureMode",
+         "PowerModeSelection",
+         "RemoteKey"
+      ],
+      "RequiredCharacteristics": [
+         "Active",
+         "ActiveIdentifier",
+         "ConfiguredName",
+         "SleepDiscoveryMode"
+      ],
+      "UUID": "000000D8-0000-1000-8000-0026BB765291"
+   },
+   "TelevisionSpeaker": {
+      "OptionalCharacteristics": [
+         "Active",
+         "Volume",
+         "VolumeControlType",
+         "VolumeSelector",
+         "Name"
+      ],
+      "RequiredCharacteristics": [
+         "Mute"
+      ],
+      "UUID": "00000113-0000-1000-8000-0026BB765291"
+   },
    "TemperatureSensor": {
       "OptionalCharacteristics": [
          "StatusActive",
diff --git a/pyhap/service.py b/pyhap/service.py
index 69cb6b2..c68686d 100644
--- a/pyhap/service.py
+++ b/pyhap/service.py
@@ -1,7 +1,9 @@
 """This module implements the HAP Service."""
 from uuid import UUID
 
-from pyhap.const import HAP_REPR_CHARS, HAP_REPR_IID, HAP_REPR_TYPE
+from pyhap.const import (
+    HAP_REPR_CHARS, HAP_REPR_IID,
+    HAP_REPR_LINKED, HAP_REPR_PRIMARY, HAP_REPR_TYPE)
 
 
 class Service:
@@ -11,14 +13,17 @@ class Service:
     TemperatureSensor service has the characteristic CurrentTemperature.
     """
 
-    __slots__ = ('broker', 'characteristics', 'display_name', 'type_id')
+    __slots__ = ('broker', 'characteristics', 'display_name', 'type_id',
+                 'linked_services', 'is_primary_service')
 
     def __init__(self, type_id, display_name=None):
         """Initialize a new Service object."""
         self.broker = None
         self.characteristics = []
+        self.linked_services = []
         self.display_name = display_name
         self.type_id = type_id
+        self.is_primary_service = None
 
     def __repr__(self):
         """Return the representation of the service."""
@@ -26,6 +31,13 @@ class Service:
             .format(self.display_name,
                     {c.display_name: c.value for c in self.characteristics})
 
+    def add_linked_service(self, service):
+        """Add the given service as "linked" to this Service."""
+        if not any(self.broker.iid_manager.get_iid(service) ==
+                   self.broker.iid_manager.get_iid(original_service)
+                   for original_service in self.linked_services):
+            self.linked_services.append(service)
+
     def add_characteristic(self, *chars):
         """Add the given characteristics as "mandatory" for this Service."""
         for char in chars:
@@ -70,12 +82,23 @@ class Service:
         :return: A HAP representation.
         :rtype: dict.
         """
-        return {
+        hap = {
             HAP_REPR_IID: self.broker.iid_manager.get_iid(self),
             HAP_REPR_TYPE: str(self.type_id).upper(),
             HAP_REPR_CHARS: [c.to_HAP() for c in self.characteristics],
         }
 
+        if self.is_primary_service is not None:
+            hap[HAP_REPR_PRIMARY] = self.is_primary_service
+
+        if self.linked_services:
+            hap[HAP_REPR_LINKED] = []
+            for linked_service in self.linked_services:
+                hap[HAP_REPR_LINKED].append(
+                    linked_service.broker.iid_manager.get_iid(linked_service))
+
+        return hap
+
     @classmethod
     def from_dict(cls, name, json_dict, loader):
         """Initialize a service object from a dict.


Test patch:
diff --git a/tests/test_accessory.py b/tests/test_accessory.py
index 3d47137..3b9b977 100644
--- a/tests/test_accessory.py
+++ b/tests/test_accessory.py
@@ -33,6 +33,20 @@ def test_acc_set_services_compatible(mock_driver):
     assert acc.get_service('TemperatureSensor') is not None
 
 
+def test_acc_set_primary_service(mock_driver):
+    """Test method set_primary_service."""
+    acc = Accessory(mock_driver, 'Test Accessory')
+    service = acc.driver.loader.get_service('Television')
+    acc.add_service(service)
+    linked_service = acc.driver.loader.get_service('TelevisionSpeaker')
+    acc.add_service(linked_service)
+    assert acc.get_service('Television').is_primary_service is None
+    assert acc.get_service('TelevisionSpeaker').is_primary_service is None
+    acc.set_primary_service(service)
+    assert acc.get_service('Television').is_primary_service is True
+    assert acc.get_service('TelevisionSpeaker').is_primary_service is False
+
+
 # #### Bridge ############
 # execute with `-k bridge`
 # ########################
diff --git a/tests/test_service.py b/tests/test_service.py
index c07494d..d739c7a 100644
--- a/tests/test_service.py
+++ b/tests/test_service.py
@@ -93,6 +93,31 @@ def test_configure_char():
         new_setter_callback
 
 
+def test_is_primary_service():
+    """Test setting is_primary_service on a service."""
+    service = Service(uuid1(), 'Test Service')
+
+    assert service.is_primary_service is None
+
+    service.is_primary_service = True
+    assert service.is_primary_service is True
+
+    service.is_primary_service = False
+    assert service.is_primary_service is False
+
+
+def test_add_linked_service():
+    """Test adding linked service to a service."""
+    service = Service(uuid1(), 'Test Service')
+    assert len(service.linked_services) == 0
+
+    linked_service = Service(uuid1(), 'Test Linked Service')
+    service.add_linked_service(linked_service)
+
+    assert len(service.linked_services) == 1
+    assert service.linked_services[0] == linked_service
+
+
 def test_to_HAP():
     """Test created HAP representation of a service."""
     uuid = uuid1()
@@ -115,6 +140,58 @@ def test_to_HAP():
     }
 
 
+def test_linked_service_to_HAP():
+    """Test created HAP representation of a service."""
+    uuid = uuid1()
+    pyhap_char_to_HAP = 'pyhap.characteristic.Characteristic.to_HAP'
+
+    service = Service(uuid, 'Test Service')
+    linked_service = Service(uuid1(), 'Test Linked Service')
+    service.add_linked_service(linked_service)
+    service.characteristics = get_chars()
+    with patch(pyhap_char_to_HAP) as mock_char_HAP, \
+            patch.object(service, 'broker') as mock_broker, \
+            patch.object(linked_service, 'broker') as mock_linked_broker:
+        mock_iid = mock_broker.iid_manager.get_iid
+        mock_iid.return_value = 2
+        mock_linked_iid = mock_linked_broker.iid_manager.get_iid
+        mock_linked_iid.return_value = 3
+        mock_char_HAP.side_effect = ('Char 1', 'Char 2')
+        hap_repr = service.to_HAP()
+        mock_iid.assert_called_with(service)
+
+    assert hap_repr == {
+        'iid': 2,
+        'type': str(uuid).upper(),
+        'characteristics': ['Char 1', 'Char 2'],
+        'linked': [mock_linked_iid()],
+    }
+
+
+def test_is_primary_service_to_HAP():
+    """Test created HAP representation of primary service."""
+    uuid = uuid1()
+    pyhap_char_to_HAP = 'pyhap.characteristic.Characteristic.to_HAP'
+
+    service = Service(uuid, 'Test Service')
+    service.characteristics = get_chars()
+    service.is_primary_service = True
+    with patch(pyhap_char_to_HAP) as mock_char_HAP, \
+            patch.object(service, 'broker') as mock_broker:
+        mock_iid = mock_broker.iid_manager.get_iid
+        mock_iid.return_value = 2
+        mock_char_HAP.side_effect = ('Char 1', 'Char 2')
+        hap_repr = service.to_HAP()
+        mock_iid.assert_called_with(service)
+
+    assert hap_repr == {
+        'iid': 2,
+        'type': str(uuid).upper(),
+        'characteristics': ['Char 1', 'Char 2'],
+        'primary': True
+    }
+
+
 def test_from_dict():
     """Test creating a service from a dictionary."""
     uuid = uuid1()