Clean up remote component (#8728)

* Clean up remote component

* Don't have device be required in send_command service and method.
* Don't have entity_id be required in the base service schema.
* Don't always add activity in the data dict for a service call.
* Update harmony remote platform according to new service schema.
* Remove not needed properties and attributes from the Kira remote
  platform.
* Add send_command method to demo platform.
* Add tests and remove duplicate tests.

* Break out required argument as positional argument

homeassistant/components/remote/__init__.py

@@ -45,11 +45,11 @@ MIN_TIME_BETWEEN_SCANS = timedelta(seconds=10)
 SERVICE_SEND_COMMAND = 'send_command'
 SERVICE_SYNC = 'sync'
 
-DEFAULT_NUM_REPEATS = '1'
+DEFAULT_NUM_REPEATS = 1
-DEFAULT_DELAY_SECS = '0.4'
+DEFAULT_DELAY_SECS = 0.4
 
 REMOTE_SERVICE_SCHEMA = vol.Schema({
-    vol.Required(ATTR_ENTITY_ID): cv.entity_ids,
+    vol.Optional(ATTR_ENTITY_ID): cv.entity_ids,
 })
 
 REMOTE_SERVICE_ACTIVITY_SCHEMA = REMOTE_SERVICE_SCHEMA.extend({
@@ -57,10 +57,12 @@ REMOTE_SERVICE_ACTIVITY_SCHEMA = REMOTE_SERVICE_SCHEMA.extend({
 })
 
 REMOTE_SERVICE_SEND_COMMAND_SCHEMA = REMOTE_SERVICE_SCHEMA.extend({
-    vol.Required(ATTR_DEVICE): cv.string,
     vol.Required(ATTR_COMMAND): vol.All(cv.ensure_list, [cv.string]),
-    vol.Optional(ATTR_NUM_REPEATS, default=DEFAULT_NUM_REPEATS): cv.string,
+    vol.Optional(ATTR_DEVICE): cv.string,
-    vol.Optional(ATTR_DELAY_SECS, default=DEFAULT_DELAY_SECS): cv.string
+    vol.Optional(
+        ATTR_NUM_REPEATS, default=DEFAULT_NUM_REPEATS): cv.positive_int,
+    vol.Optional(
+        ATTR_DELAY_SECS, default=DEFAULT_DELAY_SECS): vol.Coerce(float)
 })
 
 
@@ -74,9 +76,11 @@ def is_on(hass, entity_id=None):
 @bind_hass
 def turn_on(hass, activity=None, entity_id=None):
     """Turn all or specified remote on."""
-    data = {ATTR_ACTIVITY: activity}
+    data = {
-    if entity_id:
+        key: value for key, value in [
-        data[ATTR_ENTITY_ID] = entity_id
+            (ATTR_ACTIVITY, activity),
+            (ATTR_ENTITY_ID, entity_id),
+        ] if value is not None}
     hass.services.call(DOMAIN, SERVICE_TURN_ON, data)
 
 
@@ -107,13 +111,16 @@ def toggle(hass, activity=None, entity_id=None):
 
 
 @bind_hass
-def send_command(hass, device, command, entity_id=None,
+def send_command(hass, command, entity_id=None, device=None,
                  num_repeats=None, delay_secs=None):
     """Send a command to a device."""
-    data = {ATTR_DEVICE: str(device), ATTR_COMMAND: command}
+    data = {ATTR_COMMAND: command}
     if entity_id:
         data[ATTR_ENTITY_ID] = entity_id
 
+    if device:
+        data[ATTR_DEVICE] = device
+
     if num_repeats:
         data[ATTR_NUM_REPEATS] = num_repeats
 
@@ -194,13 +201,14 @@ def async_setup(hass, config):
 class RemoteDevice(ToggleEntity):
     """Representation of a remote."""
 
-    def send_command(self, **kwargs):
+    def send_command(self, command, **kwargs):
         """Send a command to a device."""
         raise NotImplementedError()
 
-    def async_send_command(self, **kwargs):
+    def async_send_command(self, command, **kwargs):
         """Send a command to a device.
 
         This method must be run in the event loop and returns a coroutine.
         """
-        return self.hass.async_add_job(ft.partial(self.send_command, **kwargs))
+        return self.hass.async_add_job(ft.partial(
+            self.send_command, command, **kwargs))

homeassistant/components/remote/apple_tv.py

@@ -8,7 +8,6 @@ import asyncio
 
 from homeassistant.components.apple_tv import (
     ATTR_ATV, ATTR_POWER, DATA_APPLE_TV)
-from homeassistant.components.remote import ATTR_COMMAND
 from homeassistant.components import remote
 from homeassistant.const import (CONF_NAME, CONF_HOST)
 
@@ -75,18 +74,18 @@ class AppleTVRemote(remote.RemoteDevice):
         """
         self._power.set_power_on(False)
 
-    def async_send_command(self, **kwargs):
+    def async_send_command(self, command, **kwargs):
         """Send a command to one device.
 
         This method must be run in the event loop and returns a coroutine.
         """
         # Send commands in specified order but schedule only one coroutine
         @asyncio.coroutine
-        def _send_commads():
+        def _send_commands():
-            for command in kwargs[ATTR_COMMAND]:
+            for single_command in command:
-                if not hasattr(self._atv.remote_control, command):
+                if not hasattr(self._atv.remote_control, single_command):
                     continue
 
-                yield from getattr(self._atv.remote_control, command)()
+                yield from getattr(self._atv.remote_control, single_command)()
 
-        return _send_commads()
+        return _send_commands()

homeassistant/components/remote/demo.py

@@ -25,6 +25,7 @@ class DemoRemote(RemoteDevice):
         self._name = name or DEVICE_DEFAULT_NAME
         self._state = state
         self._icon = icon
+        self._last_command_sent = None
 
     @property
     def should_poll(self):
@@ -46,6 +47,12 @@ class DemoRemote(RemoteDevice):
         """Return true if remote is on."""
         return self._state
 
+    @property
+    def device_state_attributes(self):
+        """Return device state attributes."""
+        if self._last_command_sent is not None:
+            return {'last_command_sent': self._last_command_sent}
+
     def turn_on(self, **kwargs):
         """Turn the remote on."""
         self._state = True
@@ -55,3 +62,9 @@ class DemoRemote(RemoteDevice):
         """Turn the remote off."""
         self._state = False
         self.schedule_update_ha_state()
+
+    def send_command(self, command, **kwargs):
+        """Send a command to a device."""
+        for com in command:
+            self._last_command_sent = com
+        self.schedule_update_ha_state()

homeassistant/components/remote/harmony.py

@@ -15,8 +15,8 @@ import homeassistant.helpers.config_validation as cv
 from homeassistant.const import (
     CONF_NAME, CONF_HOST, CONF_PORT, ATTR_ENTITY_ID)
 from homeassistant.components.remote import (
-    PLATFORM_SCHEMA, DOMAIN, ATTR_DEVICE, ATTR_COMMAND,
+    PLATFORM_SCHEMA, DOMAIN, ATTR_DEVICE, ATTR_ACTIVITY, ATTR_NUM_REPEATS,
-    ATTR_ACTIVITY, ATTR_NUM_REPEATS, ATTR_DELAY_SECS)
+    ATTR_DELAY_SECS)
 from homeassistant.util import slugify
 from homeassistant.config import load_yaml_config_file
 
@@ -207,13 +207,21 @@ class HarmonyRemote(remote.RemoteDevice):
         import pyharmony
         pyharmony.ha_power_off(self._token, self.host, self._port)
 
-    def send_command(self, **kwargs):
+    def send_command(self, command, **kwargs):
         """Send a set of commands to one device."""
         import pyharmony
+        device = kwargs.pop(ATTR_DEVICE, None)
+        if device is None:
+            _LOGGER.error("Missing required argument: device")
+            return
+        num_repeats = kwargs.pop(ATTR_NUM_REPEATS, None)
+        if num_repeats is not None:
+            kwargs[ATTR_NUM_REPEATS] = num_repeats
+        delay_secs = kwargs.pop(ATTR_DELAY_SECS, None)
+        if delay_secs is not None:
+            kwargs[ATTR_DELAY_SECS] = delay_secs
         pyharmony.ha_send_commands(
-            self._token, self.host, self._port, kwargs[ATTR_DEVICE],
+            self._token, self.host, self._port, device, command, **kwargs)
-            kwargs[ATTR_COMMAND], int(kwargs[ATTR_NUM_REPEATS]),
-            float(kwargs[ATTR_DELAY_SECS]))
 
     def sync(self):
         """Sync the Harmony device with the web service."""

homeassistant/components/remote/itach.py

@@ -14,8 +14,7 @@ import homeassistant.components.remote as remote
 from homeassistant.const import (
     DEVICE_DEFAULT_NAME, CONF_NAME, CONF_MAC, CONF_HOST, CONF_PORT,
     CONF_DEVICES)
-from homeassistant.components.remote import (
+from homeassistant.components.remote import PLATFORM_SCHEMA
-    PLATFORM_SCHEMA, ATTR_COMMAND)
 
 REQUIREMENTS = ['pyitachip2ir==0.0.6']
 
@@ -108,10 +107,10 @@ class ITachIP2IRRemote(remote.RemoteDevice):
         self.itachip2ir.send(self._name, "OFF", 1)
         self.schedule_update_ha_state()
 
-    def send_command(self, **kwargs):
+    def send_command(self, command, **kwargs):
         """Send a command to one device."""
-        for command in kwargs[ATTR_COMMAND]:
+        for single_command in command:
-            self.itachip2ir.send(self._name, command, 1)
+            self.itachip2ir.send(self._name, single_command, 1)
 
     def update(self):
         """Update the device."""

homeassistant/components/remote/kira.py

@@ -11,9 +11,7 @@ import homeassistant.components.remote as remote
 from homeassistant.helpers.entity import Entity
 
 from homeassistant.const import (
-    STATE_UNKNOWN,
+    CONF_DEVICE, CONF_NAME)
-    CONF_DEVICE,
-    CONF_NAME)
 
 DOMAIN = 'kira'
 
@@ -40,8 +38,6 @@ class KiraRemote(Entity):
         """Initialize KiraRemote class."""
         _LOGGER.debug("KiraRemote device init started for: %s", name)
         self._name = name
-        self._state = STATE_UNKNOWN
-
         self._kira = kira
 
     @property
@@ -49,30 +45,21 @@ class KiraRemote(Entity):
         """Return the Kira device's name."""
         return self._name
 
-    @property
-    def device_state_attributes(self):
-        """Add platform specific attributes."""
-        return {}
-
-    @property
-    def is_on(self):
-        """Return True. Power state doesn't apply to this device."""
-        return True
-
     def update(self):
         """No-op."""
 
-    def send_command(self, **kwargs):
+    def send_command(self, command, **kwargs):
         """Send a command to one device."""
-        for command in kwargs.get(remote.ATTR_COMMAND):
+        for singel_command in command:
-            code_tuple = (command,
+            code_tuple = (singel_command,
                           kwargs.get(remote.ATTR_DEVICE))
             _LOGGER.info("Sending Command: %s to %s", *code_tuple)
             self._kira.sendCode(code_tuple)
 
-    def async_send_command(self, **kwargs):
+    def async_send_command(self, command, **kwargs):
         """Send a command to a device.
 
         This method must be run in the event loop and returns a coroutine.
         """
-        return self.hass.async_add_job(ft.partial(self.send_command, **kwargs))
+        return self.hass.async_add_job(ft.partial(
+            self.send_command, command, **kwargs))

tests/components/remote/test_demo.py

@@ -4,12 +4,10 @@ import unittest
 
 from homeassistant.setup import setup_component
 import homeassistant.components.remote as remote
-from homeassistant.const import (
+from homeassistant.const import STATE_ON, STATE_OFF
-    ATTR_ENTITY_ID, STATE_ON, STATE_OFF, CONF_PLATFORM,
+from tests.common import get_test_home_assistant
-    SERVICE_TURN_ON, SERVICE_TURN_OFF)
-from tests.common import get_test_home_assistant, mock_service
 
-SERVICE_SEND_COMMAND = 'send_command'
+ENTITY_ID = 'remote.remote_one'
 
 
 class TestDemoRemote(unittest.TestCase):
@@ -29,71 +27,25 @@ class TestDemoRemote(unittest.TestCase):
         self.hass.stop()
 
     def test_methods(self):
-        """Test if methods call the services as expected."""
+        """Test if services call the entity methods as expected."""
-        self.assertTrue(
+        remote.turn_on(self.hass, entity_id=ENTITY_ID)
-            setup_component(self.hass, remote.DOMAIN,
-                            {remote.DOMAIN: {CONF_PLATFORM: 'demo'}}))
-
-        # Test is_on
-        self.hass.states.set('remote.demo', STATE_ON)
-        self.assertTrue(remote.is_on(self.hass, 'remote.demo'))
-
-        self.hass.states.set('remote.demo', STATE_OFF)
-        self.assertFalse(remote.is_on(self.hass, 'remote.demo'))
-
-        self.hass.states.set(remote.ENTITY_ID_ALL_REMOTES, STATE_ON)
-        self.assertTrue(remote.is_on(self.hass))
-
-        self.hass.states.set(remote.ENTITY_ID_ALL_REMOTES, STATE_OFF)
-        self.assertFalse(remote.is_on(self.hass))
-
-    def test_services(self):
-        """Test the provided services."""
-        # Test turn_on
-        turn_on_calls = mock_service(
-            self.hass, remote.DOMAIN, SERVICE_TURN_ON)
-
-        remote.turn_on(
-            self.hass,
-            entity_id='entity_id_val')
-
         self.hass.block_till_done()
+        state = self.hass.states.get(ENTITY_ID)
+        self.assertEqual(state.state, STATE_ON)
 
-        self.assertEqual(1, len(turn_on_calls))
+        remote.turn_off(self.hass, entity_id=ENTITY_ID)
-        call = turn_on_calls[-1]
-
-        self.assertEqual(remote.DOMAIN, call.domain)
-
-        # Test turn_off
-        turn_off_calls = mock_service(
-            self.hass, remote.DOMAIN, SERVICE_TURN_OFF)
-
-        remote.turn_off(
-            self.hass, entity_id='entity_id_val')
-
         self.hass.block_till_done()
+        state = self.hass.states.get(ENTITY_ID)
+        self.assertEqual(state.state, STATE_OFF)
 
-        self.assertEqual(1, len(turn_off_calls))
+        remote.turn_on(self.hass, entity_id=ENTITY_ID)
-        call = turn_off_calls[-1]
-
-        self.assertEqual(remote.DOMAIN, call.domain)
-        self.assertEqual(SERVICE_TURN_OFF, call.service)
-        self.assertEqual('entity_id_val', call.data[ATTR_ENTITY_ID])
-
-        # Test send_command
-        send_command_calls = mock_service(
-            self.hass, remote.DOMAIN, SERVICE_SEND_COMMAND)
-
-        remote.send_command(
-            self.hass, entity_id='entity_id_val',
-            device='test_device', command=['test_command'],
-            num_repeats='2', delay_secs='0.8')
-
         self.hass.block_till_done()
+        state = self.hass.states.get(ENTITY_ID)
+        self.assertEqual(state.state, STATE_ON)
 
-        self.assertEqual(1, len(send_command_calls))
+        remote.send_command(self.hass, 'test', entity_id=ENTITY_ID)
-        call = send_command_calls[-1]
+        self.hass.block_till_done()
-
+        state = self.hass.states.get(ENTITY_ID)
-        self.assertEqual(remote.DOMAIN, call.domain)
+        self.assertEqual(
-        self.assertEqual(SERVICE_SEND_COMMAND, call.service)
+            state.attributes,
-        self.assertEqual('entity_id_val', call.data[ATTR_ENTITY_ID])
+            {'friendly_name': 'Remote One', 'last_command_sent': 'test'})