From 47f405577a0944825a10147453bf59c9ce7d2364 Mon Sep 17 00:00:00 2001 From: azogue Date: Thu, 11 May 2017 19:57:34 +0200 Subject: [PATCH 1/4] Fix Kodi specific services, add descriptions, add more handled exceptions - Fixes issue #7528 - Add descriptions for Kodi specific services in services.yaml. - Error handling in Kodi API errors. - Make compatible the existent specific service `media_player.kodi_set_shuffle` with the general `media_player.shuffle_set` service (both use the same method but with different named parameter, I think the Kodi specific service should be eliminated, since it is not) --- homeassistant/components/media_player/kodi.py | 98 +++++++++++++------ .../components/media_player/services.yaml | 54 ++++++++++ 2 files changed, 123 insertions(+), 29 deletions(-) diff --git a/homeassistant/components/media_player/kodi.py b/homeassistant/components/media_player/kodi.py index 10d13002625caa..d55aaa8c1caa30 100644 --- a/homeassistant/components/media_player/kodi.py +++ b/homeassistant/components/media_player/kodi.py @@ -9,16 +9,18 @@ import logging import urllib import re +import os import aiohttp import voluptuous as vol +from homeassistant.config import load_yaml_config_file from homeassistant.components.media_player import ( SUPPORT_NEXT_TRACK, SUPPORT_PAUSE, SUPPORT_PREVIOUS_TRACK, SUPPORT_SEEK, SUPPORT_PLAY_MEDIA, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET, SUPPORT_STOP, - SUPPORT_TURN_OFF, SUPPORT_PLAY, SUPPORT_VOLUME_STEP, MediaPlayerDevice, - PLATFORM_SCHEMA, MEDIA_TYPE_MUSIC, MEDIA_TYPE_TVSHOW, MEDIA_TYPE_VIDEO, - MEDIA_TYPE_PLAYLIST, MEDIA_PLAYER_SCHEMA, DOMAIN) + SUPPORT_TURN_OFF, SUPPORT_PLAY, SUPPORT_VOLUME_STEP, SUPPORT_SHUFFLE_SET, + MediaPlayerDevice, PLATFORM_SCHEMA, MEDIA_TYPE_MUSIC, MEDIA_TYPE_TVSHOW, + MEDIA_TYPE_VIDEO, MEDIA_TYPE_PLAYLIST, MEDIA_PLAYER_SCHEMA, DOMAIN) from homeassistant.const import ( STATE_IDLE, STATE_OFF, STATE_PAUSED, STATE_PLAYING, CONF_HOST, CONF_NAME, CONF_PORT, CONF_SSL, CONF_PROXY_SSL, CONF_USERNAME, CONF_PASSWORD, @@ -61,8 +63,9 @@ } SUPPORT_KODI = SUPPORT_PAUSE | SUPPORT_VOLUME_SET | SUPPORT_VOLUME_MUTE | \ - SUPPORT_PREVIOUS_TRACK | SUPPORT_NEXT_TRACK | SUPPORT_SEEK | \ - SUPPORT_PLAY_MEDIA | SUPPORT_STOP | SUPPORT_PLAY | SUPPORT_VOLUME_STEP + SUPPORT_PREVIOUS_TRACK | SUPPORT_NEXT_TRACK | SUPPORT_SEEK | \ + SUPPORT_PLAY_MEDIA | SUPPORT_STOP | SUPPORT_SHUFFLE_SET | \ + SUPPORT_PLAY | SUPPORT_VOLUME_STEP PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ vol.Required(CONF_HOST): cv.string, @@ -78,7 +81,9 @@ }) SERVICE_ADD_MEDIA = 'kodi_add_to_playlist' -SERVICE_SET_SHUFFLE = 'kodi_set_shuffle' +SERVICE_SET_SHUFFLE = 'kodi_set_shuffle' # this is the same as mp.shuffle_set + +DATA_KODI = 'kodi' ATTR_MEDIA_TYPE = 'media_type' ATTR_MEDIA_NAME = 'media_name' @@ -86,14 +91,15 @@ ATTR_MEDIA_ID = 'media_id' MEDIA_PLAYER_SET_SHUFFLE_SCHEMA = MEDIA_PLAYER_SCHEMA.extend({ - vol.Required('shuffle_on'): cv.boolean, + vol.Exclusive('shuffle_on', 'shuffle'): cv.boolean, + vol.Exclusive('shuffle', 'shuffle'): cv.boolean, }) MEDIA_PLAYER_ADD_MEDIA_SCHEMA = MEDIA_PLAYER_SCHEMA.extend({ vol.Required(ATTR_MEDIA_TYPE): cv.string, vol.Optional(ATTR_MEDIA_ID): cv.string, - vol.Optional(ATTR_MEDIA_NAME): cv.string, - vol.Optional(ATTR_MEDIA_ARTIST_NAME): cv.string, + vol.Inclusive(ATTR_MEDIA_NAME, 'media_search'): cv.string, + vol.Inclusive(ATTR_MEDIA_ARTIST_NAME, 'media_search'): cv.string, }) SERVICE_TO_METHOD = { @@ -109,6 +115,8 @@ @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Set up the Kodi platform.""" + if DATA_KODI not in hass.data: + hass.data[DATA_KODI] = [] host = config.get(CONF_HOST) port = config.get(CONF_PORT) tcp_port = config.get(CONF_TCP_PORT) @@ -130,7 +138,11 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): password=config.get(CONF_PASSWORD), turn_off_action=config.get(CONF_TURN_OFF_ACTION), websocket=websocket) + hass.data[DATA_KODI].append(entity) async_add_devices([entity], update_before_add=True) + descriptions = yield from hass.loop.run_in_executor( + None, load_yaml_config_file, os.path.join( + os.path.dirname(__file__), 'services.yaml')) @asyncio.coroutine def async_service_handler(service): @@ -141,23 +153,30 @@ def async_service_handler(service): params = {key: value for key, value in service.data.items() if key != 'entity_id'} - - yield from getattr(entity, method['method'])(**params) + entity_ids = service.data.get('entity_id') + if entity_ids: + target_players = [player for player in hass.data[DATA_KODI] + if player.entity_id in entity_ids] + else: + target_players = hass.data[DATA_KODI] update_tasks = [] - if entity.should_poll: - update_coro = entity.async_update_ha_state(True) - update_tasks.append(update_coro) + for player in target_players: + yield from getattr(player, method['method'])(**params) + + for player in target_players: + if player.should_poll: + update_coro = player.async_update_ha_state(True) + update_tasks.append(update_coro) if update_tasks: yield from asyncio.wait(update_tasks, loop=hass.loop) for service in SERVICE_TO_METHOD: - schema = SERVICE_TO_METHOD[service].get( - 'schema', MEDIA_PLAYER_SCHEMA) + schema = SERVICE_TO_METHOD[service].get('schema') hass.services.async_register( DOMAIN, service, async_service_handler, - description=None, schema=schema) + description=descriptions.get(service), schema=schema) def cmd(func): @@ -657,12 +676,13 @@ def async_play_media(self, media_type, media_id, **kwargs): {"item": {"file": str(media_id)}}) @asyncio.coroutine - def async_set_shuffle(self, shuffle_on): + def async_set_shuffle(self, shuffle_on=None, shuffle=None): """Set shuffle mode, for the first player.""" + shuffle = shuffle if shuffle is not None else shuffle_on if len(self._players) < 1: raise RuntimeError("Error: No active player.") yield from self.server.Player.SetShuffle( - {"playerid": self._players[0]['playerid'], "shuffle": shuffle_on}) + {"playerid": self._players[0]['playerid'], "shuffle": shuffle}) @asyncio.coroutine def async_add_media_to_playlist( @@ -675,13 +695,14 @@ def async_add_media_to_playlist( All the albums of an artist can be added with media_name="ALL" """ + import jsonrpc_base + params = {"playlistid": 0} if media_type == "SONG": if media_id is None: media_id = yield from self.async_find_song( media_name, artist_name) - - yield from self.server.Playlist.Add( - {"playlistid": 0, "item": {"songid": int(media_id)}}) + if media_id: + params["item"] = {"songid": int(media_id)} elif media_type == "ALBUM": if media_id is None: @@ -691,12 +712,22 @@ def async_add_media_to_playlist( media_id = yield from self.async_find_album( media_name, artist_name) + if media_id: + params["item"] = {"albumid": int(media_id)} - yield from self.server.Playlist.Add( - {"playlistid": 0, "item": {"albumid": int(media_id)}}) else: raise RuntimeError("Unrecognized media type.") + if media_id is not None: + try: + yield from self.server.Playlist.Add(params) + except jsonrpc_base.jsonrpc.ProtocolError as e: + result = e.args[2]['error'] + _LOGGER.error('Run API method %s.Playlist.Add(%s) error: %s', + self.entity_id, media_type, result) + else: + _LOGGER.warning('No media detected for Playlist.Add') + @asyncio.coroutine def async_add_all_albums(self, artist_name): """Add all albums of an artist to default playlist (i.e. playlistid=0). @@ -734,9 +765,13 @@ def async_get_albums(self, artist_id=None): def async_find_artist(self, artist_name): """Find artist by name.""" artists = yield from self.async_get_artists() - out = self._find( - artist_name, [a['artist'] for a in artists['artists']]) - return artists['artists'][out[0][0]]['artistid'] + try: + out = self._find( + artist_name, [a['artist'] for a in artists['artists']]) + return artists['artists'][out[0][0]]['artistid'] + except KeyError: + _LOGGER.warning('No artists were found: %s', artist_name) + return None @asyncio.coroutine def async_get_songs(self, artist_id=None): @@ -769,8 +804,13 @@ def async_find_album(self, album_name, artist_name=''): artist_id = yield from self.async_find_artist(artist_name) albums = yield from self.async_get_albums(artist_id) - out = self._find(album_name, [a['label'] for a in albums['albums']]) - return albums['albums'][out[0][0]]['albumid'] + try: + out = self._find(album_name, [a['label'] for a in albums['albums']]) + return albums['albums'][out[0][0]]['albumid'] + except KeyError: + _LOGGER.warning('No albums were found with artist: %s, album: %s', + artist_name, album_name) + return None @staticmethod def _find(key_word, words): diff --git a/homeassistant/components/media_player/services.yaml b/homeassistant/components/media_player/services.yaml index ae90e141289ce4..7407424ff39eba 100644 --- a/homeassistant/components/media_player/services.yaml +++ b/homeassistant/components/media_player/services.yaml @@ -269,3 +269,57 @@ soundtouch_remove_zone_slave: slaves: description: Name of slaves entities to remove from the existing zone example: 'media_player.soundtouch_bedroom' + +kodi_add_to_playlist: + description: Add music to the default playlist (i.e. playlistid=0). + + fields: + entity_id: + description: Name(s) of the Kodi entities where to add the media. + example: 'media_player.living_room_kodi' + media_type: + description: Media type identifier. It must be one of SONG or ALBUM. + example: ALBUM + media_id: + description: Unique Id of the media entry to add (`songid` or albumid`). If not defined, `media_name` and `artist_name` are needed to search the Kodi music library. + example: 123456 + media_name: + description: Optional media name for filtering media. Can be 'ALL' when `media_type` is 'ALBUM' and `artist_name` is specified, to add all songs from one artist. + example: 'Highway to Hell' + artist_name: + description: Optional artist name for filtering media. + example: 'AC/DC' + +kodi_set_shuffle: # == shuffle_set + description: Set shuffling state + + fields: + entity_id: + description: Name(s) of entities to set + example: 'media_player.spotify' +# shuffle: + shuffle_on: + description: True/false for enabling/disabling shuffle + example: true + +kodi_execute_addon: + description: 'Run a Kodi Addon with optional parameters. Results of the Kodi API call, if any, will be redirected in a Home Assistant event: `kodi_run_addon_result`.' + + fields: + entity_id: + description: Name(s) of the Kodi entities where to run the (pre-installed) Kodi Addon. + example: 'media_player.living_room_kodi' + addonid: + description: Name of the Kodi addon. + example: 'script.json-cec' + +kodi_run_method: + description: 'Run a Kodi JSONRPC API method with optional parameters. Results of the Kodi API call will be redirected in a Home Assistant event: `kodi_run_method_result`.' + + fields: + entity_id: + description: Name(s) of the Kodi entities where to run the API method. + example: 'media_player.living_room_kodi' + method: + description: Name of the Kodi JSONRPC API method to be called. + example: 'VideoLibrary.GetRecentlyAddedEpisodes' From 1966cfeaed888385cb8867158b543181d9b4ed47 Mon Sep 17 00:00:00 2001 From: azogue Date: Thu, 11 May 2017 20:15:38 +0200 Subject: [PATCH 2/4] fix line too long --- homeassistant/components/media_player/kodi.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/media_player/kodi.py b/homeassistant/components/media_player/kodi.py index d55aaa8c1caa30..4cc7956557a2ae 100644 --- a/homeassistant/components/media_player/kodi.py +++ b/homeassistant/components/media_player/kodi.py @@ -805,7 +805,8 @@ def async_find_album(self, album_name, artist_name=''): albums = yield from self.async_get_albums(artist_id) try: - out = self._find(album_name, [a['label'] for a in albums['albums']]) + out = self._find( + album_name, [a['label'] for a in albums['albums']]) return albums['albums'][out[0][0]]['albumid'] except KeyError: _LOGGER.warning('No albums were found with artist: %s, album: %s', From 8e4c0ac4b7e4872523a116ee7273337cecc57765 Mon Sep 17 00:00:00 2001 From: azogue Date: Thu, 11 May 2017 20:59:22 +0200 Subject: [PATCH 3/4] removed new services (for another PR); removed `kodi_set_shuffle` service --- .../components/media_player/services.yaml | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/homeassistant/components/media_player/services.yaml b/homeassistant/components/media_player/services.yaml index 7407424ff39eba..4d5f85c05eb825 100644 --- a/homeassistant/components/media_player/services.yaml +++ b/homeassistant/components/media_player/services.yaml @@ -289,37 +289,3 @@ kodi_add_to_playlist: artist_name: description: Optional artist name for filtering media. example: 'AC/DC' - -kodi_set_shuffle: # == shuffle_set - description: Set shuffling state - - fields: - entity_id: - description: Name(s) of entities to set - example: 'media_player.spotify' -# shuffle: - shuffle_on: - description: True/false for enabling/disabling shuffle - example: true - -kodi_execute_addon: - description: 'Run a Kodi Addon with optional parameters. Results of the Kodi API call, if any, will be redirected in a Home Assistant event: `kodi_run_addon_result`.' - - fields: - entity_id: - description: Name(s) of the Kodi entities where to run the (pre-installed) Kodi Addon. - example: 'media_player.living_room_kodi' - addonid: - description: Name of the Kodi addon. - example: 'script.json-cec' - -kodi_run_method: - description: 'Run a Kodi JSONRPC API method with optional parameters. Results of the Kodi API call will be redirected in a Home Assistant event: `kodi_run_method_result`.' - - fields: - entity_id: - description: Name(s) of the Kodi entities where to run the API method. - example: 'media_player.living_room_kodi' - method: - description: Name of the Kodi JSONRPC API method to be called. - example: 'VideoLibrary.GetRecentlyAddedEpisodes' From 29e840bfce086e70abc803e4d04221b9f43796e5 Mon Sep 17 00:00:00 2001 From: azogue Date: Fri, 12 May 2017 01:23:46 +0200 Subject: [PATCH 4/4] requested changes - Removed `kodi_set_shuffle` service. - Optional `media_name` and `artist_name` parameters. `media_name` defaults to 'ALL'. - Guard clause to check if the services are already registered. --- homeassistant/components/media_player/kodi.py | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/media_player/kodi.py b/homeassistant/components/media_player/kodi.py index 4cc7956557a2ae..18c01c396acdbb 100644 --- a/homeassistant/components/media_player/kodi.py +++ b/homeassistant/components/media_player/kodi.py @@ -81,7 +81,6 @@ }) SERVICE_ADD_MEDIA = 'kodi_add_to_playlist' -SERVICE_SET_SHUFFLE = 'kodi_set_shuffle' # this is the same as mp.shuffle_set DATA_KODI = 'kodi' @@ -90,25 +89,17 @@ ATTR_MEDIA_ARTIST_NAME = 'artist_name' ATTR_MEDIA_ID = 'media_id' -MEDIA_PLAYER_SET_SHUFFLE_SCHEMA = MEDIA_PLAYER_SCHEMA.extend({ - vol.Exclusive('shuffle_on', 'shuffle'): cv.boolean, - vol.Exclusive('shuffle', 'shuffle'): cv.boolean, -}) - MEDIA_PLAYER_ADD_MEDIA_SCHEMA = MEDIA_PLAYER_SCHEMA.extend({ vol.Required(ATTR_MEDIA_TYPE): cv.string, vol.Optional(ATTR_MEDIA_ID): cv.string, - vol.Inclusive(ATTR_MEDIA_NAME, 'media_search'): cv.string, - vol.Inclusive(ATTR_MEDIA_ARTIST_NAME, 'media_search'): cv.string, + vol.Optional(ATTR_MEDIA_NAME): cv.string, + vol.Optional(ATTR_MEDIA_ARTIST_NAME): cv.string, }) SERVICE_TO_METHOD = { SERVICE_ADD_MEDIA: { 'method': 'async_add_media_to_playlist', 'schema': MEDIA_PLAYER_ADD_MEDIA_SCHEMA}, - SERVICE_SET_SHUFFLE: { - 'method': 'async_set_shuffle', - 'schema': MEDIA_PLAYER_SET_SHUFFLE_SCHEMA}, } @@ -140,9 +131,6 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): hass.data[DATA_KODI].append(entity) async_add_devices([entity], update_before_add=True) - descriptions = yield from hass.loop.run_in_executor( - None, load_yaml_config_file, os.path.join( - os.path.dirname(__file__), 'services.yaml')) @asyncio.coroutine def async_service_handler(service): @@ -172,8 +160,15 @@ def async_service_handler(service): if update_tasks: yield from asyncio.wait(update_tasks, loop=hass.loop) + if hass.services.has_service(DOMAIN, SERVICE_ADD_MEDIA): + return + + descriptions = yield from hass.loop.run_in_executor( + None, load_yaml_config_file, os.path.join( + os.path.dirname(__file__), 'services.yaml')) + for service in SERVICE_TO_METHOD: - schema = SERVICE_TO_METHOD[service].get('schema') + schema = SERVICE_TO_METHOD[service]['schema'] hass.services.async_register( DOMAIN, service, async_service_handler, description=descriptions.get(service), schema=schema) @@ -676,9 +671,8 @@ def async_play_media(self, media_type, media_id, **kwargs): {"item": {"file": str(media_id)}}) @asyncio.coroutine - def async_set_shuffle(self, shuffle_on=None, shuffle=None): + def async_set_shuffle(self, shuffle): """Set shuffle mode, for the first player.""" - shuffle = shuffle if shuffle is not None else shuffle_on if len(self._players) < 1: raise RuntimeError("Error: No active player.") yield from self.server.Player.SetShuffle( @@ -686,7 +680,7 @@ def async_set_shuffle(self, shuffle_on=None, shuffle=None): @asyncio.coroutine def async_add_media_to_playlist( - self, media_type, media_id=None, media_name='', artist_name=''): + self, media_type, media_id=None, media_name='ALL', artist_name=''): """Add a media to default playlist (i.e. playlistid=0). First the media type must be selected, then @@ -721,8 +715,8 @@ def async_add_media_to_playlist( if media_id is not None: try: yield from self.server.Playlist.Add(params) - except jsonrpc_base.jsonrpc.ProtocolError as e: - result = e.args[2]['error'] + except jsonrpc_base.jsonrpc.ProtocolError as exc: + result = exc.args[2]['error'] _LOGGER.error('Run API method %s.Playlist.Add(%s) error: %s', self.entity_id, media_type, result) else: