From 01e1fc4aeea04b9ef87f98b31ff510f606248c17 Mon Sep 17 00:00:00 2001 From: "andrew@sayre.net" Date: Sun, 7 Apr 2019 21:14:27 -0500 Subject: [PATCH 1/4] Add check for supported features --- .../components/media_player/__init__.py | 65 +++++-------------- 1 file changed, 18 insertions(+), 47 deletions(-) diff --git a/homeassistant/components/media_player/__init__.py b/homeassistant/components/media_player/__init__.py index 3421551320e203..8b3c022fcf3d56 100644 --- a/homeassistant/components/media_player/__init__.py +++ b/homeassistant/components/media_player/__init__.py @@ -32,51 +32,20 @@ from homeassistant.loader import bind_hass from .const import ( - ATTR_APP_ID, - ATTR_APP_NAME, - ATTR_INPUT_SOURCE, - ATTR_INPUT_SOURCE_LIST, - ATTR_MEDIA_ALBUM_ARTIST, - ATTR_MEDIA_ALBUM_NAME, - ATTR_MEDIA_ARTIST, - ATTR_MEDIA_CHANNEL, - ATTR_MEDIA_CONTENT_ID, - ATTR_MEDIA_CONTENT_TYPE, - ATTR_MEDIA_DURATION, - ATTR_MEDIA_ENQUEUE, - ATTR_MEDIA_EPISODE, - ATTR_MEDIA_PLAYLIST, - ATTR_MEDIA_POSITION, - ATTR_MEDIA_POSITION_UPDATED_AT, - ATTR_MEDIA_SEASON, - ATTR_MEDIA_SEEK_POSITION, - ATTR_MEDIA_SERIES_TITLE, - ATTR_MEDIA_SHUFFLE, - ATTR_MEDIA_TITLE, - ATTR_MEDIA_TRACK, - ATTR_MEDIA_VOLUME_LEVEL, - ATTR_MEDIA_VOLUME_MUTED, - ATTR_SOUND_MODE, - ATTR_SOUND_MODE_LIST, - DOMAIN, - SERVICE_CLEAR_PLAYLIST, - SERVICE_PLAY_MEDIA, - SERVICE_SELECT_SOUND_MODE, - SERVICE_SELECT_SOURCE, - SUPPORT_PAUSE, - SUPPORT_SEEK, - SUPPORT_VOLUME_SET, - SUPPORT_VOLUME_MUTE, - SUPPORT_PREVIOUS_TRACK, - SUPPORT_NEXT_TRACK, - SUPPORT_PLAY_MEDIA, - SUPPORT_SELECT_SOURCE, - SUPPORT_STOP, - SUPPORT_CLEAR_PLAYLIST, - SUPPORT_PLAY, - SUPPORT_SHUFFLE_SET, - SUPPORT_SELECT_SOUND_MODE, -) + ATTR_APP_ID, ATTR_APP_NAME, ATTR_INPUT_SOURCE, ATTR_INPUT_SOURCE_LIST, + ATTR_MEDIA_ALBUM_ARTIST, ATTR_MEDIA_ALBUM_NAME, ATTR_MEDIA_ARTIST, + ATTR_MEDIA_CHANNEL, ATTR_MEDIA_CONTENT_ID, ATTR_MEDIA_CONTENT_TYPE, + ATTR_MEDIA_DURATION, ATTR_MEDIA_ENQUEUE, ATTR_MEDIA_EPISODE, + ATTR_MEDIA_PLAYLIST, ATTR_MEDIA_POSITION, ATTR_MEDIA_POSITION_UPDATED_AT, + ATTR_MEDIA_SEASON, ATTR_MEDIA_SEEK_POSITION, ATTR_MEDIA_SERIES_TITLE, + ATTR_MEDIA_SHUFFLE, ATTR_MEDIA_TITLE, ATTR_MEDIA_TRACK, + ATTR_MEDIA_VOLUME_LEVEL, ATTR_MEDIA_VOLUME_MUTED, ATTR_SOUND_MODE, + ATTR_SOUND_MODE_LIST, DOMAIN, SERVICE_CLEAR_PLAYLIST, SERVICE_PLAY_MEDIA, + SERVICE_SELECT_SOUND_MODE, SERVICE_SELECT_SOURCE, SUPPORT_CLEAR_PLAYLIST, + SUPPORT_NEXT_TRACK, SUPPORT_PAUSE, SUPPORT_PLAY, SUPPORT_PLAY_MEDIA, + SUPPORT_PREVIOUS_TRACK, SUPPORT_SEEK, SUPPORT_SELECT_SOUND_MODE, + SUPPORT_SELECT_SOURCE, SUPPORT_SHUFFLE_SET, SUPPORT_STOP, SUPPORT_TURN_OFF, + SUPPORT_TURN_ON, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET) from .reproduce_state import async_reproduce_states # noqa _LOGGER = logging.getLogger(__name__) @@ -463,7 +432,8 @@ def supported_features(self): def turn_on(self): """Turn the media player on.""" - raise NotImplementedError() + if self.supported_features & SUPPORT_TURN_ON: + raise NotImplementedError() def async_turn_on(self): """Turn the media player on. @@ -474,7 +444,8 @@ def async_turn_on(self): def turn_off(self): """Turn the media player off.""" - raise NotImplementedError() + if self.supported_features & SUPPORT_TURN_OFF: + raise NotImplementedError() def async_turn_off(self): """Turn the media player off. From ea850300ba6dce684ea76e85e23d3ea5da142f2a Mon Sep 17 00:00:00 2001 From: "andrew@sayre.net" Date: Sun, 7 Apr 2019 22:20:05 -0500 Subject: [PATCH 2/4] Move logic to service helper --- .../components/media_player/__init__.py | 47 ++++++++++--------- homeassistant/helpers/entity_component.py | 6 ++- homeassistant/helpers/service.py | 14 ++++-- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/homeassistant/components/media_player/__init__.py b/homeassistant/components/media_player/__init__.py index 8b3c022fcf3d56..5bc2d640e2bd56 100644 --- a/homeassistant/components/media_player/__init__.py +++ b/homeassistant/components/media_player/__init__.py @@ -166,74 +166,77 @@ async def async_setup(hass, config): component.async_register_entity_service( SERVICE_TURN_ON, MEDIA_PLAYER_SCHEMA, - 'async_turn_on' + 'async_turn_on', SUPPORT_TURN_ON ) component.async_register_entity_service( SERVICE_TURN_OFF, MEDIA_PLAYER_SCHEMA, - 'async_turn_off' + 'async_turn_off', SUPPORT_TURN_OFF ) component.async_register_entity_service( SERVICE_TOGGLE, MEDIA_PLAYER_SCHEMA, - 'async_toggle' + 'async_toggle', SUPPORT_TURN_OFF | SUPPORT_TURN_ON ) component.async_register_entity_service( SERVICE_VOLUME_UP, MEDIA_PLAYER_SCHEMA, - 'async_volume_up' + 'async_volume_up', SUPPORT_VOLUME_SET ) component.async_register_entity_service( SERVICE_VOLUME_DOWN, MEDIA_PLAYER_SCHEMA, - 'async_volume_down' + 'async_volume_down', SUPPORT_VOLUME_SET ) component.async_register_entity_service( SERVICE_MEDIA_PLAY_PAUSE, MEDIA_PLAYER_SCHEMA, - 'async_media_play_pause' + 'async_media_play_pause', SUPPORT_PLAY | SUPPORT_PAUSE ) component.async_register_entity_service( SERVICE_MEDIA_PLAY, MEDIA_PLAYER_SCHEMA, - 'async_media_play' + 'async_media_play', SUPPORT_PLAY ) component.async_register_entity_service( SERVICE_MEDIA_PAUSE, MEDIA_PLAYER_SCHEMA, - 'async_media_pause' + 'async_media_pause', SUPPORT_PAUSE ) component.async_register_entity_service( SERVICE_MEDIA_STOP, MEDIA_PLAYER_SCHEMA, - 'async_media_stop' + 'async_media_stop', SUPPORT_STOP ) component.async_register_entity_service( SERVICE_MEDIA_NEXT_TRACK, MEDIA_PLAYER_SCHEMA, - 'async_media_next_track' + 'async_media_next_track', SUPPORT_NEXT_TRACK ) component.async_register_entity_service( SERVICE_MEDIA_PREVIOUS_TRACK, MEDIA_PLAYER_SCHEMA, - 'async_media_previous_track' + 'async_media_previous_track', SUPPORT_PREVIOUS_TRACK ) component.async_register_entity_service( SERVICE_CLEAR_PLAYLIST, MEDIA_PLAYER_SCHEMA, - 'async_clear_playlist' + 'async_clear_playlist', SUPPORT_CLEAR_PLAYLIST ) component.async_register_entity_service( SERVICE_VOLUME_SET, MEDIA_PLAYER_SET_VOLUME_SCHEMA, lambda entity, call: entity.async_set_volume_level( - volume=call.data[ATTR_MEDIA_VOLUME_LEVEL]) + volume=call.data[ATTR_MEDIA_VOLUME_LEVEL]), + SUPPORT_VOLUME_SET ) component.async_register_entity_service( SERVICE_VOLUME_MUTE, MEDIA_PLAYER_MUTE_VOLUME_SCHEMA, lambda entity, call: entity.async_mute_volume( - mute=call.data[ATTR_MEDIA_VOLUME_MUTED]) + mute=call.data[ATTR_MEDIA_VOLUME_MUTED]), + SUPPORT_VOLUME_MUTE ) component.async_register_entity_service( SERVICE_MEDIA_SEEK, MEDIA_PLAYER_MEDIA_SEEK_SCHEMA, lambda entity, call: entity.async_media_seek( - position=call.data[ATTR_MEDIA_SEEK_POSITION]) + position=call.data[ATTR_MEDIA_SEEK_POSITION]), + SUPPORT_SEEK ) component.async_register_entity_service( SERVICE_SELECT_SOURCE, MEDIA_PLAYER_SELECT_SOURCE_SCHEMA, - 'async_select_source' + 'async_select_source', SUPPORT_SELECT_SOURCE ) component.async_register_entity_service( SERVICE_SELECT_SOUND_MODE, MEDIA_PLAYER_SELECT_SOUND_MODE_SCHEMA, - 'async_select_sound_mode' + 'async_select_sound_mode', SUPPORT_SELECT_SOUND_MODE ) component.async_register_entity_service( SERVICE_PLAY_MEDIA, MEDIA_PLAYER_PLAY_MEDIA_SCHEMA, @@ -241,11 +244,11 @@ async def async_setup(hass, config): media_type=call.data[ATTR_MEDIA_CONTENT_TYPE], media_id=call.data[ATTR_MEDIA_CONTENT_ID], enqueue=call.data.get(ATTR_MEDIA_ENQUEUE) - ) + ), SUPPORT_PLAY_MEDIA ) component.async_register_entity_service( SERVICE_SHUFFLE_SET, MEDIA_PLAYER_SET_SHUFFLE_SCHEMA, - 'async_set_shuffle' + 'async_set_shuffle', SUPPORT_SHUFFLE_SET ) return True @@ -432,8 +435,7 @@ def supported_features(self): def turn_on(self): """Turn the media player on.""" - if self.supported_features & SUPPORT_TURN_ON: - raise NotImplementedError() + raise NotImplementedError() def async_turn_on(self): """Turn the media player on. @@ -444,8 +446,7 @@ def async_turn_on(self): def turn_off(self): """Turn the media player off.""" - if self.supported_features & SUPPORT_TURN_OFF: - raise NotImplementedError() + raise NotImplementedError() def async_turn_off(self): """Turn the media player off. diff --git a/homeassistant/helpers/entity_component.py b/homeassistant/helpers/entity_component.py index 744cf36ea66c1d..7be3d906bfa83e 100644 --- a/homeassistant/helpers/entity_component.py +++ b/homeassistant/helpers/entity_component.py @@ -179,13 +179,15 @@ async def async_extract_from_service(self, service, expand_group=True): if entity.available and entity.entity_id in entity_ids] @callback - def async_register_entity_service(self, name, schema, func): + def async_register_entity_service(self, name, schema, func, + required_features=None): """Register an entity service.""" async def handle_service(call): """Handle the service.""" service_name = "{}.{}".format(self.domain, name) await self.hass.helpers.service.entity_service_call( - self._platforms.values(), func, call, service_name + self._platforms.values(), func, call, service_name, + required_features ) self.hass.services.async_register( diff --git a/homeassistant/helpers/service.py b/homeassistant/helpers/service.py index 3892dbb660733c..ea62d12c66c02e 100644 --- a/homeassistant/helpers/service.py +++ b/homeassistant/helpers/service.py @@ -216,7 +216,8 @@ def load_services_files(yaml_files): @bind_hass -async def entity_service_call(hass, platforms, func, call, service_name=''): +async def entity_service_call(hass, platforms, func, call, service_name='', + required_features=None): """Handle an entity service call. Calls all platforms simultaneously. @@ -295,7 +296,8 @@ async def entity_service_call(hass, platforms, func, call, service_name=''): platforms_entities.append(platform_entities) tasks = [ - _handle_service_platform_call(func, data, entities, call.context) + _handle_service_platform_call(func, data, entities, call.context, + required_features) for platform, entities in zip(platforms, platforms_entities) ] @@ -306,7 +308,8 @@ async def entity_service_call(hass, platforms, func, call, service_name=''): future.result() # pop exception if have -async def _handle_service_platform_call(func, data, entities, context): +async def _handle_service_platform_call(func, data, entities, context, + required_features): """Handle a function call.""" tasks = [] @@ -314,6 +317,11 @@ async def _handle_service_platform_call(func, data, entities, context): if not entity.available: continue + # Skip entities that don't have the required feature. + if required_features is not None \ + and not entity.supported_features & required_features: + continue + entity.async_set_context(context) if isinstance(func, str): From 4e1d4ff036a6d57bdeb6dc329ce18b5d6d9318bc Mon Sep 17 00:00:00 2001 From: "andrew@sayre.net" Date: Sun, 7 Apr 2019 22:41:25 -0500 Subject: [PATCH 3/4] Fix hacked in test for seek --- homeassistant/components/demo/media_player.py | 14 +++++++------- tests/components/demo/test_media_player.py | 14 +++++++++++--- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/demo/media_player.py b/homeassistant/components/demo/media_player.py index 5ad13b4e9953ae..cb3f3b5b46a60f 100644 --- a/homeassistant/components/demo/media_player.py +++ b/homeassistant/components/demo/media_player.py @@ -1,14 +1,13 @@ """Demo implementation of the media player.""" -from homeassistant.const import STATE_OFF, STATE_PAUSED, STATE_PLAYING -import homeassistant.util.dt as dt_util - from homeassistant.components.media_player import MediaPlayerDevice from homeassistant.components.media_player.const import ( MEDIA_TYPE_MOVIE, MEDIA_TYPE_MUSIC, MEDIA_TYPE_TVSHOW, SUPPORT_CLEAR_PLAYLIST, SUPPORT_NEXT_TRACK, SUPPORT_PAUSE, SUPPORT_PLAY, - SUPPORT_PLAY_MEDIA, SUPPORT_PREVIOUS_TRACK, SUPPORT_SELECT_SOUND_MODE, - SUPPORT_SELECT_SOURCE, SUPPORT_SHUFFLE_SET, SUPPORT_TURN_OFF, - SUPPORT_TURN_ON, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET) + SUPPORT_PLAY_MEDIA, SUPPORT_PREVIOUS_TRACK, SUPPORT_SEEK, + SUPPORT_SELECT_SOUND_MODE, SUPPORT_SELECT_SOURCE, SUPPORT_SHUFFLE_SET, + SUPPORT_TURN_OFF, SUPPORT_TURN_ON, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET) +from homeassistant.const import STATE_OFF, STATE_PAUSED, STATE_PLAYING +import homeassistant.util.dt as dt_util def setup_platform(hass, config, add_entities, discovery_info=None): @@ -30,7 +29,8 @@ def setup_platform(hass, config, add_entities, discovery_info=None): YOUTUBE_PLAYER_SUPPORT = \ SUPPORT_PAUSE | SUPPORT_VOLUME_SET | SUPPORT_VOLUME_MUTE | \ SUPPORT_TURN_ON | SUPPORT_TURN_OFF | SUPPORT_PLAY_MEDIA | SUPPORT_PLAY | \ - SUPPORT_SHUFFLE_SET | SUPPORT_SELECT_SOUND_MODE | SUPPORT_SELECT_SOURCE + SUPPORT_SHUFFLE_SET | SUPPORT_SELECT_SOUND_MODE | SUPPORT_SELECT_SOURCE | \ + SUPPORT_SEEK MUSIC_PLAYER_SUPPORT = \ SUPPORT_PAUSE | SUPPORT_VOLUME_SET | SUPPORT_VOLUME_MUTE | \ diff --git a/tests/components/demo/test_media_player.py b/tests/components/demo/test_media_player.py index 83acf8be601bd5..808e3ee2102a4b 100644 --- a/tests/components/demo/test_media_player.py +++ b/tests/components/demo/test_media_player.py @@ -184,9 +184,7 @@ def test_prev_next_track(self): state = self.hass.states.get(ent_id) assert 1 == state.attributes.get('media_episode') - @patch('homeassistant.components.demo.media_player.DemoYoutubePlayer.' - 'media_seek', autospec=True) - def test_play_media(self, mock_seek): + def test_play_media(self): """Test play_media .""" assert setup_component( self.hass, mp.DOMAIN, @@ -212,6 +210,16 @@ def test_play_media(self, mock_seek): state.attributes.get('supported_features')) assert 'some_id' == state.attributes.get('media_content_id') + @patch('homeassistant.components.demo.media_player.DemoYoutubePlayer.' + 'media_seek', autospec=True) + def test_seek(self, mock_seek): + """Test seek.""" + assert setup_component( + self.hass, mp.DOMAIN, + {'media_player': {'platform': 'demo'}}) + ent_id = 'media_player.living_room' + state = self.hass.states.get(ent_id) + assert state.attributes['supported_features'] & mp.SUPPORT_SEEK assert not mock_seek.called with pytest.raises(vol.Invalid): common.media_seek(self.hass, None, ent_id) From 048be292398834c38fc9df5f494cbee8065bea11 Mon Sep 17 00:00:00 2001 From: "andrew@sayre.net" Date: Mon, 8 Apr 2019 09:57:17 -0500 Subject: [PATCH 4/4] Test for service required features --- tests/helpers/test_service.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/helpers/test_service.py b/tests/helpers/test_service.py index f59a01ec268469..231ffddff3095c 100644 --- a/tests/helpers/test_service.py +++ b/tests/helpers/test_service.py @@ -37,11 +37,13 @@ def mock_entities(): entity_id='light.kitchen', available=True, should_poll=False, + supported_features=1, ) living_room = Mock( entity_id='light.living_room', available=True, should_poll=False, + supported_features=0, ) entities = OrderedDict() entities[kitchen.entity_id] = kitchen @@ -269,6 +271,19 @@ def test_async_get_all_descriptions(hass): assert 'fields' in descriptions[logger.DOMAIN]['set_level'] +async def test_call_with_required_features(hass, mock_entities): + """Test service calls invoked only if entity has required feautres.""" + test_service_mock = Mock(return_value=mock_coro()) + await service.entity_service_call(hass, [ + Mock(entities=mock_entities) + ], test_service_mock, ha.ServiceCall('test_domain', 'test_service', { + 'entity_id': 'all' + }), required_features=1) + assert len(mock_entities) == 2 + # Called once because only one of the entities had the required features + assert test_service_mock.call_count == 1 + + async def test_call_context_user_not_exist(hass): """Check we don't allow deleted users to do things.""" with pytest.raises(exceptions.UnknownUser) as err: