From b5725f8f19987daba3fed20540b2822d7c7a6831 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Thu, 25 Apr 2019 23:42:39 -0500 Subject: [PATCH] Fix supported features gates in media_player volume up/down services (#23419) * Correct media player feature gates * Fix failing test * Lint... --- .../components/media_player/__init__.py | 47 ++++++++++--------- homeassistant/helpers/service.py | 3 +- .../media_player/test_async_helpers.py | 14 ++++++ tests/helpers/test_service.py | 2 +- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/homeassistant/components/media_player/__init__.py b/homeassistant/components/media_player/__init__.py index b23d95ab625..ccfa968fa9a 100644 --- a/homeassistant/components/media_player/__init__.py +++ b/homeassistant/components/media_player/__init__.py @@ -45,7 +45,8 @@ from .const import ( 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) + SUPPORT_TURN_ON, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET, + SUPPORT_VOLUME_STEP) from .reproduce_state import async_reproduce_states # noqa _LOGGER = logging.getLogger(__name__) @@ -174,77 +175,77 @@ async def async_setup(hass, config): component.async_register_entity_service( SERVICE_TURN_ON, MEDIA_PLAYER_SCHEMA, - 'async_turn_on', SUPPORT_TURN_ON + 'async_turn_on', [SUPPORT_TURN_ON] ) component.async_register_entity_service( SERVICE_TURN_OFF, MEDIA_PLAYER_SCHEMA, - 'async_turn_off', SUPPORT_TURN_OFF + 'async_turn_off', [SUPPORT_TURN_OFF] ) component.async_register_entity_service( SERVICE_TOGGLE, MEDIA_PLAYER_SCHEMA, - 'async_toggle', SUPPORT_TURN_OFF | SUPPORT_TURN_ON + 'async_toggle', [SUPPORT_TURN_OFF | SUPPORT_TURN_ON] ) component.async_register_entity_service( SERVICE_VOLUME_UP, MEDIA_PLAYER_SCHEMA, - 'async_volume_up', SUPPORT_VOLUME_SET + 'async_volume_up', [SUPPORT_VOLUME_SET, SUPPORT_VOLUME_STEP] ) component.async_register_entity_service( SERVICE_VOLUME_DOWN, MEDIA_PLAYER_SCHEMA, - 'async_volume_down', SUPPORT_VOLUME_SET + 'async_volume_down', [SUPPORT_VOLUME_SET, SUPPORT_VOLUME_STEP] ) component.async_register_entity_service( SERVICE_MEDIA_PLAY_PAUSE, MEDIA_PLAYER_SCHEMA, - 'async_media_play_pause', SUPPORT_PLAY | SUPPORT_PAUSE + 'async_media_play_pause', [SUPPORT_PLAY | SUPPORT_PAUSE] ) component.async_register_entity_service( SERVICE_MEDIA_PLAY, MEDIA_PLAYER_SCHEMA, - 'async_media_play', SUPPORT_PLAY + 'async_media_play', [SUPPORT_PLAY] ) component.async_register_entity_service( SERVICE_MEDIA_PAUSE, MEDIA_PLAYER_SCHEMA, - 'async_media_pause', SUPPORT_PAUSE + 'async_media_pause', [SUPPORT_PAUSE] ) component.async_register_entity_service( SERVICE_MEDIA_STOP, MEDIA_PLAYER_SCHEMA, - 'async_media_stop', SUPPORT_STOP + 'async_media_stop', [SUPPORT_STOP] ) component.async_register_entity_service( SERVICE_MEDIA_NEXT_TRACK, MEDIA_PLAYER_SCHEMA, - 'async_media_next_track', SUPPORT_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', SUPPORT_PREVIOUS_TRACK + 'async_media_previous_track', [SUPPORT_PREVIOUS_TRACK] ) component.async_register_entity_service( SERVICE_CLEAR_PLAYLIST, MEDIA_PLAYER_SCHEMA, - 'async_clear_playlist', SUPPORT_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]), - SUPPORT_VOLUME_SET + [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]), - SUPPORT_VOLUME_MUTE + [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]), - SUPPORT_SEEK + [SUPPORT_SEEK] ) component.async_register_entity_service( SERVICE_SELECT_SOURCE, MEDIA_PLAYER_SELECT_SOURCE_SCHEMA, - 'async_select_source', SUPPORT_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', SUPPORT_SELECT_SOUND_MODE + 'async_select_sound_mode', [SUPPORT_SELECT_SOUND_MODE] ) component.async_register_entity_service( SERVICE_PLAY_MEDIA, MEDIA_PLAYER_PLAY_MEDIA_SCHEMA, @@ -252,11 +253,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 + ), [SUPPORT_PLAY_MEDIA] ) component.async_register_entity_service( SERVICE_SHUFFLE_SET, MEDIA_PLAYER_SET_SHUFFLE_SCHEMA, - 'async_set_shuffle', SUPPORT_SHUFFLE_SET + 'async_set_shuffle', [SUPPORT_SHUFFLE_SET] ) return True @@ -701,7 +702,8 @@ class MediaPlayerDevice(Entity): await self.hass.async_add_job(self.volume_up) return - if self.volume_level < 1: + if self.volume_level < 1 \ + and self.supported_features & SUPPORT_VOLUME_SET: await self.async_set_volume_level(min(1, self.volume_level + .1)) async def async_volume_down(self): @@ -714,7 +716,8 @@ class MediaPlayerDevice(Entity): await self.hass.async_add_job(self.volume_down) return - if self.volume_level > 0: + if self.volume_level > 0 \ + and self.supported_features & SUPPORT_VOLUME_SET: await self.async_set_volume_level( max(0, self.volume_level - .1)) diff --git a/homeassistant/helpers/service.py b/homeassistant/helpers/service.py index 8c576f58c14..7eb72a66c8b 100644 --- a/homeassistant/helpers/service.py +++ b/homeassistant/helpers/service.py @@ -327,7 +327,8 @@ async def _handle_service_platform_call(func, data, entities, context, # Skip entities that don't have the required feature. if required_features is not None \ - and not entity.supported_features & required_features: + and not any(entity.supported_features & feature_set + for feature_set in required_features): continue entity.async_set_context(context) diff --git a/tests/components/media_player/test_async_helpers.py b/tests/components/media_player/test_async_helpers.py index 1c4a2fa84a2..aa3d1eff209 100644 --- a/tests/components/media_player/test_async_helpers.py +++ b/tests/components/media_player/test_async_helpers.py @@ -29,6 +29,13 @@ class AsyncMediaPlayer(mp.MediaPlayerDevice): """Volume level of the media player (0..1).""" return self._volume + @property + def supported_features(self): + """Flag media player features that are supported.""" + return mp.const.SUPPORT_VOLUME_SET | mp.const.SUPPORT_PLAY \ + | mp.const.SUPPORT_PAUSE | mp.const.SUPPORT_TURN_OFF \ + | mp.const.SUPPORT_TURN_ON + @asyncio.coroutine def async_set_volume_level(self, volume): """Set volume level, range 0..1.""" @@ -74,6 +81,13 @@ class SyncMediaPlayer(mp.MediaPlayerDevice): """Volume level of the media player (0..1).""" return self._volume + @property + def supported_features(self): + """Flag media player features that are supported.""" + return mp.const.SUPPORT_VOLUME_SET | mp.const.SUPPORT_VOLUME_STEP \ + | mp.const.SUPPORT_PLAY | mp.const.SUPPORT_PAUSE \ + | mp.const.SUPPORT_TURN_OFF | mp.const.SUPPORT_TURN_ON + def set_volume_level(self, volume): """Set volume level, range 0..1.""" self._volume = volume diff --git a/tests/helpers/test_service.py b/tests/helpers/test_service.py index 647ca981da3..81cdd097855 100644 --- a/tests/helpers/test_service.py +++ b/tests/helpers/test_service.py @@ -280,7 +280,7 @@ async def test_call_with_required_features(hass, mock_entities): Mock(entities=mock_entities) ], test_service_mock, ha.ServiceCall('test_domain', 'test_service', { 'entity_id': 'all' - }), required_features=1) + }), 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