Improve config schema of the blackbird component (#14007)

* Import moved, return values removed and redundant log message removed

* Improve config schema of the blackbird component

* Tests updated

* Handle updated

* Schema fixed

homeassistant/components/media_player/blackbird.py

@@ -5,6 +5,7 @@ For more details about this platform, please refer to the documentation at
 https://home-assistant.io/components/media_player.blackbird
 """
 import logging
+import socket
 
 import voluptuous as vol
 
@@ -50,71 +51,68 @@ ZONE_IDS = vol.All(vol.Coerce(int), vol.Range(min=1, max=8))
 # Valid source ids: 1-8
 SOURCE_IDS = vol.All(vol.Coerce(int), vol.Range(min=1, max=8))
 
-PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
-    vol.Required(CONF_TYPE): vol.In(['serial', 'socket']),
-    vol.Optional(CONF_PORT): cv.string,
-    vol.Optional(CONF_HOST): cv.string,
+PLATFORM_SCHEMA = vol.All(
+    cv.has_at_least_one_key(CONF_PORT, CONF_HOST),
+    PLATFORM_SCHEMA.extend({
+        vol.Exclusive(CONF_PORT, CONF_TYPE): cv.string,
+        vol.Exclusive(CONF_HOST, CONF_TYPE): cv.string,
         vol.Required(CONF_ZONES): vol.Schema({ZONE_IDS: ZONE_SCHEMA}),
         vol.Required(CONF_SOURCES): vol.Schema({SOURCE_IDS: SOURCE_SCHEMA}),
-})
+    }))
 
 
 # pylint: disable=unused-argument
 def setup_platform(hass, config, add_devices, discovery_info=None):
     """Set up the Monoprice Blackbird 4k 8x8 HDBaseT Matrix platform."""
+    if DATA_BLACKBIRD not in hass.data:
+        hass.data[DATA_BLACKBIRD] = {}
+
     port = config.get(CONF_PORT)
     host = config.get(CONF_HOST)
-    device_type = config.get(CONF_TYPE)
 
-    import socket
     from pyblackbird import get_blackbird
     from serial import SerialException
 
-    if device_type == 'serial':
-        if port is None:
-            _LOGGER.error("No port configured")
-            return
+    connection = None
+    if port is not None:
         try:
             blackbird = get_blackbird(port)
+            connection = port
         except SerialException:
             _LOGGER.error("Error connecting to the Blackbird controller")
             return
 
-    elif device_type == 'socket':
+    if host is not None:
         try:
-            if host is None:
-                _LOGGER.error("No host configured")
-                return
             blackbird = get_blackbird(host, False)
+            connection = host
         except socket.timeout:
             _LOGGER.error("Error connecting to the Blackbird controller")
             return
 
-    else:
-        _LOGGER.error("Incorrect device type specified")
-        return
-
     sources = {source_id: extra[CONF_NAME] for source_id, extra
                in config[CONF_SOURCES].items()}
 
-    hass.data[DATA_BLACKBIRD] = []
+    devices = []
     for zone_id, extra in config[CONF_ZONES].items():
         _LOGGER.info("Adding zone %d - %s", zone_id, extra[CONF_NAME])
-        hass.data[DATA_BLACKBIRD].append(BlackbirdZone(
-            blackbird, sources, zone_id, extra[CONF_NAME]))
+        unique_id = "{}-{}".format(connection, zone_id)
+        device = BlackbirdZone(blackbird, sources, zone_id, extra[CONF_NAME])
+        hass.data[DATA_BLACKBIRD][unique_id] = device
+        devices.append(device)
 
-    add_devices(hass.data[DATA_BLACKBIRD], True)
+    add_devices(devices, True)
 
     def service_handle(service):
         """Handle for services."""
         entity_ids = service.data.get(ATTR_ENTITY_ID)
         source = service.data.get(ATTR_SOURCE)
         if entity_ids:
-            devices = [device for device in hass.data[DATA_BLACKBIRD]
+            devices = [device for device in hass.data[DATA_BLACKBIRD].values()
                        if device.entity_id in entity_ids]
 
         else:
-            devices = hass.data[DATA_BLACKBIRD]
+            devices = hass.data[DATA_BLACKBIRD].values()
 
         for device in devices:
             if service.service == SERVICE_SETALLZONES:
@@ -146,14 +144,13 @@ class BlackbirdZone(MediaPlayerDevice):
         """Retrieve latest state."""
         state = self._blackbird.zone_status(self._zone_id)
         if not state:
-            return False
+            return
         self._state = STATE_ON if state.power else STATE_OFF
         idx = state.av
         if idx in self._source_id_name:
             self._source = self._source_id_name[idx]
         else:
             self._source = None
-        return True
 
     @property
     def name(self):
@@ -187,7 +184,6 @@ class BlackbirdZone(MediaPlayerDevice):
 
     def set_all_zones(self, source):
         """Set all zones to one source."""
-        _LOGGER.debug("Setting all zones")
         if source not in self._source_name_id:
             return
         idx = self._source_name_id[source]

tests/components/media_player/test_blackbird.py

@@ -59,7 +59,6 @@ class TestBlackbirdSchema(unittest.TestCase):
         """Test valid schema."""
         valid_schema = {
             'platform': 'blackbird',
-            'type': 'serial',
             'port': '/dev/ttyUSB0',
             'zones': {1: {'name': 'a'},
                       2: {'name': 'a'},
@@ -87,8 +86,7 @@ class TestBlackbirdSchema(unittest.TestCase):
         """Test valid schema."""
         valid_schema = {
             'platform': 'blackbird',
-            'type': 'socket',
-            'port': '192.168.1.50',
+            'host': '192.168.1.50',
             'zones': {1: {'name': 'a'},
                       2: {'name': 'a'},
                       3: {'name': 'a'},
@@ -109,10 +107,18 @@ class TestBlackbirdSchema(unittest.TestCase):
         schemas = (
             {},  # Empty
             None,  # None
-            # Missing type
+            # Port and host used concurrently
+            {
+                'platform': 'blackbird',
+                'port': '/dev/ttyUSB0',
+                'host': '192.168.1.50',
+                'name': 'Name',
+                'zones': {1: {'name': 'a'}},
+                'sources': {1: {'name': 'b'}},
+            },
+            # Port or host missing
             {
                 'platform': 'blackbird',
-                'port': 'aaa',
                 'name': 'Name',
                 'zones': {1: {'name': 'a'}},
                 'sources': {1: {'name': 'b'}},
@@ -120,8 +126,7 @@ class TestBlackbirdSchema(unittest.TestCase):
             # Invalid zone number
             {
                 'platform': 'blackbird',
-                'type': 'serial',
-                'port': 'aaa',
+                'port': '/dev/ttyUSB0',
                 'name': 'Name',
                 'zones': {11: {'name': 'a'}},
                 'sources': {1: {'name': 'b'}},
@@ -129,8 +134,7 @@ class TestBlackbirdSchema(unittest.TestCase):
             # Invalid source number
             {
                 'platform': 'blackbird',
-                'type': 'serial',
-                'port': 'aaa',
+                'port': '/dev/ttyUSB0',
                 'name': 'Name',
                 'zones': {1: {'name': 'a'}},
                 'sources': {9: {'name': 'b'}},
@@ -138,8 +142,7 @@ class TestBlackbirdSchema(unittest.TestCase):
             # Zone missing name
             {
                 'platform': 'blackbird',
-                'type': 'serial',
-                'port': 'aaa',
+                'port': '/dev/ttyUSB0',
                 'name': 'Name',
                 'zones': {1: {}},
                 'sources': {1: {'name': 'b'}},
@@ -147,21 +150,11 @@ class TestBlackbirdSchema(unittest.TestCase):
             # Source missing name
             {
                 'platform': 'blackbird',
-                'type': 'serial',
-                'port': 'aaa',
+                'port': '/dev/ttyUSB0',
                 'name': 'Name',
                 'zones': {1: {'name': 'a'}},
                 'sources': {1: {}},
             },
-            # Invalid type
-            {
-                'platform': 'blackbird',
-                'type': 'aaa',
-                'port': 'aaa',
-                'name': 'Name',
-                'zones': {1: {'name': 'a'}},
-                'sources': {1: {'name': 'b'}},
-            },
         )
         for value in schemas:
             with self.assertRaises(vol.MultipleInvalid):
@@ -181,7 +174,6 @@ class TestBlackbirdMediaPlayer(unittest.TestCase):
                         new=lambda *a: self.blackbird):
             setup_platform(self.hass, {
                 'platform': 'blackbird',
-                'type': 'serial',
                 'port': '/dev/ttyUSB0',
                 'zones': {3: {'name': 'Zone name'}},
                 'sources': {1: {'name': 'one'},
@@ -189,7 +181,7 @@ class TestBlackbirdMediaPlayer(unittest.TestCase):
                             2: {'name': 'two'}},
             }, lambda *args, **kwargs: None, {})
             self.hass.block_till_done()
-        self.media_player = self.hass.data[DATA_BLACKBIRD][0]
+        self.media_player = self.hass.data[DATA_BLACKBIRD]['/dev/ttyUSB0-3']
         self.media_player.hass = self.hass
         self.media_player.entity_id = 'media_player.zone_3'
 
@@ -203,7 +195,8 @@ class TestBlackbirdMediaPlayer(unittest.TestCase):
         self.assertTrue(self.hass.services.has_service(DOMAIN,
                                                        SERVICE_SETALLZONES))
         self.assertEqual(len(self.hass.data[DATA_BLACKBIRD]), 1)
-        self.assertEqual(self.hass.data[DATA_BLACKBIRD][0].name, 'Zone name')
+        self.assertEqual(self.hass.data[DATA_BLACKBIRD]['/dev/ttyUSB0-3'].name,
+                         'Zone name')
 
     def test_setallzones_service_call_with_entity_id(self):
         """Test set all zone source service call with entity id."""