From 33663f95026eb7a6fd3b9dcc716a7b88029c1a10 Mon Sep 17 00:00:00 2001 From: Martin Hjelmare Date: Tue, 1 Aug 2017 05:52:39 +0200 Subject: [PATCH] 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 | 36 +++++---- homeassistant/components/remote/apple_tv.py | 13 ++-- homeassistant/components/remote/demo.py | 13 ++++ homeassistant/components/remote/harmony.py | 20 +++-- homeassistant/components/remote/itach.py | 9 +-- homeassistant/components/remote/kira.py | 27 ++----- tests/components/remote/test_demo.py | 86 +++++---------------- 7 files changed, 85 insertions(+), 119 deletions(-) diff --git a/homeassistant/components/remote/__init__.py b/homeassistant/components/remote/__init__.py index 64a5c77e5dd..e975460be58 100755 --- a/homeassistant/components/remote/__init__.py +++ b/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_DELAY_SECS = '0.4' +DEFAULT_NUM_REPEATS = 1 +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_DELAY_SECS, default=DEFAULT_DELAY_SECS): cv.string + vol.Optional(ATTR_DEVICE): 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} - if entity_id: - data[ATTR_ENTITY_ID] = entity_id + data = { + key: value for key, value in [ + (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)) diff --git a/homeassistant/components/remote/apple_tv.py b/homeassistant/components/remote/apple_tv.py index 50f31f57a5b..36eee4b284e 100644 --- a/homeassistant/components/remote/apple_tv.py +++ b/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(): - for command in kwargs[ATTR_COMMAND]: - if not hasattr(self._atv.remote_control, command): + def _send_commands(): + for single_command in 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() diff --git a/homeassistant/components/remote/demo.py b/homeassistant/components/remote/demo.py index 6976c116be9..bc67c1646b2 100644 --- a/homeassistant/components/remote/demo.py +++ b/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() diff --git a/homeassistant/components/remote/harmony.py b/homeassistant/components/remote/harmony.py index 9c551e5cbc6..19dcc10c545 100755 --- a/homeassistant/components/remote/harmony.py +++ b/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, - ATTR_ACTIVITY, ATTR_NUM_REPEATS, ATTR_DELAY_SECS) + PLATFORM_SCHEMA, DOMAIN, ATTR_DEVICE, ATTR_ACTIVITY, ATTR_NUM_REPEATS, + 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], - kwargs[ATTR_COMMAND], int(kwargs[ATTR_NUM_REPEATS]), - float(kwargs[ATTR_DELAY_SECS])) + self._token, self.host, self._port, device, command, **kwargs) def sync(self): """Sync the Harmony device with the web service.""" diff --git a/homeassistant/components/remote/itach.py b/homeassistant/components/remote/itach.py index 5f6ee0c6ad7..eefa1ed79af 100644 --- a/homeassistant/components/remote/itach.py +++ b/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 ( - PLATFORM_SCHEMA, ATTR_COMMAND) +from homeassistant.components.remote import PLATFORM_SCHEMA 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]: - self.itachip2ir.send(self._name, command, 1) + for single_command in command: + self.itachip2ir.send(self._name, single_command, 1) def update(self): """Update the device.""" diff --git a/homeassistant/components/remote/kira.py b/homeassistant/components/remote/kira.py index 6f167bfdd05..c1a04718d33 100755 --- a/homeassistant/components/remote/kira.py +++ b/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): - code_tuple = (command, + for singel_command in 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)) diff --git a/tests/components/remote/test_demo.py b/tests/components/remote/test_demo.py index 0c6fada4748..ed9968c2d10 100755 --- a/tests/components/remote/test_demo.py +++ b/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 ( - ATTR_ENTITY_ID, STATE_ON, STATE_OFF, CONF_PLATFORM, - SERVICE_TURN_ON, SERVICE_TURN_OFF) -from tests.common import get_test_home_assistant, mock_service +from homeassistant.const import STATE_ON, STATE_OFF +from tests.common import get_test_home_assistant -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.""" - self.assertTrue( - 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') - + """Test if services call the entity methods as expected.""" + remote.turn_on(self.hass, entity_id=ENTITY_ID) 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)) - 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') - + remote.turn_off(self.hass, entity_id=ENTITY_ID) 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)) - 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') - + remote.turn_on(self.hass, entity_id=ENTITY_ID) 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)) - call = send_command_calls[-1] - - self.assertEqual(remote.DOMAIN, call.domain) - self.assertEqual(SERVICE_SEND_COMMAND, call.service) - self.assertEqual('entity_id_val', call.data[ATTR_ENTITY_ID]) + remote.send_command(self.hass, 'test', entity_id=ENTITY_ID) + self.hass.block_till_done() + state = self.hass.states.get(ENTITY_ID) + self.assertEqual( + state.attributes, + {'friendly_name': 'Remote One', 'last_command_sent': 'test'})