Spotify aliases by Teagan42 · Pull Request #7702 · home-assistant/core

Teagan42 · 2017-05-22T03:49:04Z

Description:

Spotify some times doesn't respect device names, or just names them terribly. This adds ability to alias the media player instances and logs out new devices to info for ease of aliasing.

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2684

Example entry for `configuration.yaml` (if applicable):

media_player:
  - platform: spotify
    client_id: <your client id>
    client_secret: <your client secret>
    aliases:
        abc123def456: 'Living Room'
        9183abas000: 'Bed Room'

Checklist:

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
New dependencies are only imported inside functions that use them ([example][ex-import]).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

houndci-bot · 2017-05-22T03:49:10Z

+            device_diff = {name:id for name, id in self._devices.items()
+                           if old_devices.get(name, None) is None}
+            if len(device_diff) > 0:
+              _LOGGER.info("New Devices: %s", str(device_diff))


indentation is not a multiple of four

houndci-bot · 2017-05-22T03:49:10Z

+                                    device.get('id')
                             for device in devices}
+            device_diff = {name:id for name, id in self._devices.items()
+                           if old_devices.get(name, None) is None}


undefined name 'old_devices'

houndci-bot · 2017-05-22T03:49:10Z

+                                               device.get('name')):
+                                    device.get('id')
                             for device in devices}
+            device_diff = {name:id for name, id in self._devices.items()


missing whitespace after ':'

houndci-bot · 2017-05-22T03:49:10Z

+            current_device_keys = self._devices.keys()
+            self._devices = {self._aliases.get(device.get('id'),
+                                               device.get('name')):
+                                    device.get('id')


continuation line over-indented for visual indent

houndci-bot · 2017-05-22T03:49:10Z

        devices = self._player.devices().get('devices')
        if devices is not None:
-            self._devices = {device.get('name'): device.get('id')
+            current_device_keys = self._devices.keys()


local variable 'current_device_keys' is assigned to but never used

houndci-bot · 2017-05-22T03:49:10Z

    vol.Optional(CONF_NAME): cv.string,
-    vol.Optional(CONF_CACHE_PATH): cv.string
+    vol.Optional(CONF_CACHE_PATH): cv.string,
+    vol.Optional(CONF_ALIASES, {}): { cv.string: cv.string }


whitespace after '{'
whitespace before '}'

pvizeli · 2017-05-29T14:15:19Z

What is bad with friendly name on customize?

happyleavesaoc · 2017-05-30T00:41:01Z

@pvizeli This is media player source option friendly names, not entity names

bieniu · 2017-05-31T18:04:51Z

I have error with this PR when HA starts:

2017-05-31 20:01:25 ERROR (MainThread) [homeassistant.components.media_player] Error while setting up platform spotify
Traceback (most recent call last):
  File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/helpers/entity_component.py", line 155, in _async_setup_platform
    entity_platform.schedule_add_entities, discovery_info
  File "/usr/lib/python3.4/asyncio/futures.py", line 388, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.4/asyncio/tasks.py", line 286, in _wakeup
    value = future.result()
  File "/usr/lib/python3.4/asyncio/futures.py", line 277, in result
    raise self._exception
  File "/usr/lib/python3.4/concurrent/futures/thread.py", line 54, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/components/media_player/spotify.py", line 95, in setup_platform
    config[CONF_ALIASES])
KeyError: 'aliases'

happyleavesaoc · 2017-05-31T23:32:53Z

    vol.Optional(CONF_NAME): cv.string,
-    vol.Optional(CONF_CACHE_PATH): cv.string
+    vol.Optional(CONF_CACHE_PATH): cv.string,
+    vol.Optional(CONF_ALIASES, {}): {cv.string: cv.string}


vol.Optional(CONF_ALIASES, default={}) or similar per @bieniu

@happyleavesaoc With your change the component starts with HA and in log there is information about new device. But when I add this device as an alias in configuration and turn off all Spotify players in log every minute appears this error:

2017-06-01 10:53:17 ERROR (MainThread) [homeassistant.helpers.entity] Update for media_player.spotify fails Traceback (most recent call last): File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/helpers/entity.py", line 225, in async_update_ha_state None, self.update) File "/usr/lib/python3.4/asyncio/futures.py", line 388, in __iter__ yield self # This tells Task to wait for completion. File "/usr/lib/python3.4/asyncio/tasks.py", line 286, in _wakeup value = future.result() File "/usr/lib/python3.4/asyncio/futures.py", line 277, in result raise self._exception File "/usr/lib/python3.4/concurrent/futures/thread.py", line 54, in run result = self.fn(*self.args, **self.kwargs) File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/components/media_player/spotify.py", line 161, in update devices = self._player.devices().get('devices') AttributeError: 'NoneType' object has no attribute 'get'

And after HA restart there is no Spotify component at all but only error:

2017-06-01 10:39:29 ERROR (MainThread) [homeassistant.core] Error doing job: Task exception was never retrieved Traceback (most recent call last): File "/usr/lib/python3.4/asyncio/tasks.py", line 233, in _step result = coro.throw(exc) File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/helpers/entity_component.py", line 361, in async_process_entity new_entity, self, update_before_add=update_before_add File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/helpers/entity_component.py", line 191, in async_add_entity yield from self.hass.loop.run_in_executor(None, entity.update) File "/usr/lib/python3.4/asyncio/futures.py", line 388, in __iter__ yield self # This tells Task to wait for completion. File "/usr/lib/python3.4/asyncio/tasks.py", line 286, in _wakeup value = future.result() File "/usr/lib/python3.4/asyncio/futures.py", line 277, in result raise self._exception File "/usr/lib/python3.4/concurrent/futures/thread.py", line 54, in run result = self.fn(*self.args, **self.kwargs) File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/components/media_player/spotify.py", line 161, in update devices = self._player.devices().get('devices') AttributeError: 'NoneType' object has no attribute 'get'

I added a None check and the default try the most recent commit.

Teagan42 · 2017-06-01T18:44:17Z

Will address this evening

…

On Thu, Jun 1, 2017 at 2:57 AM, Maciej Bieniek ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In homeassistant/components/media_player/spotify.py <#7702 (comment)> : > @@ -52,7 +53,8 @@ vol.Required(CONF_CLIENT_ID): cv.string, vol.Required(CONF_CLIENT_SECRET): cv.string, vol.Optional(CONF_NAME): cv.string, - vol.Optional(CONF_CACHE_PATH): cv.string + vol.Optional(CONF_CACHE_PATH): cv.string, + vol.Optional(CONF_ALIASES, {}): {cv.string: cv.string} @happyleavesaoc <https://github.com/happyleavesaoc> With your change the component starts with HA and in log there is information about new device. But when I add this device as an alias in configuration and turn off all Spotify players in log every minute appears this error: 2017-06-01 10:53:17 ERROR (MainThread) [homeassistant.helpers.entity] Update for media_player.spotify fails Traceback (most recent call last): File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/helpers/entity.py", line 225, in async_update_ha_state None, self.update) File "/usr/lib/python3.4/asyncio/futures.py", line 388, in __iter__ yield self # This tells Task to wait for completion. File "/usr/lib/python3.4/asyncio/tasks.py", line 286, in _wakeup value = future.result() File "/usr/lib/python3.4/asyncio/futures.py", line 277, in result raise self._exception File "/usr/lib/python3.4/concurrent/futures/thread.py", line 54, in run result = self.fn(*self.args, **self.kwargs) File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/components/media_player/spotify.py", line 161, in update devices = self._player.devices().get('devices') AttributeError: 'NoneType' object has no attribute 'get' And after HA restart there is no Spotify component at all but only error: 2017-06-01 10:39:29 ERROR (MainThread) [homeassistant.core] Error doing job: Task exception was never retrieved Traceback (most recent call last): File "/usr/lib/python3.4/asyncio/tasks.py", line 233, in _step result = coro.throw(exc) File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/helpers/entity_component.py", line 361, in async_process_entity new_entity, self, update_before_add=update_before_add File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/helpers/entity_component.py", line 191, in async_add_entity yield from self.hass.loop.run_in_executor(None, entity.update) File "/usr/lib/python3.4/asyncio/futures.py", line 388, in __iter__ yield self # This tells Task to wait for completion. File "/usr/lib/python3.4/asyncio/tasks.py", line 286, in _wakeup value = future.result() File "/usr/lib/python3.4/asyncio/futures.py", line 277, in result raise self._exception File "/usr/lib/python3.4/concurrent/futures/thread.py", line 54, in run result = self.fn(*self.args, **self.kwargs) File "/srv/homeassistant/homeassistant_venv/lib/python3.4/site-packages/homeassistant/components/media_player/spotify.py", line 161, in update devices = self._player.devices().get('devices') AttributeError: 'NoneType' object has no attribute 'get' — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7702 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC2fZUOGj9nJwCqaeVf8tK8BmigYdMcjks5r_nz4gaJpZM4Nh2gl> .

Teagan Glenn added 2 commits May 21, 2017 20:52

Alias support for spotify devices

79113cb

Fix log

ff6ffdc

homeassistant added platform: media_player.spotify cla-signed labels May 22, 2017

houndci-bot reviewed May 22, 2017

View reviewed changes

Teagan42 mentioned this pull request May 22, 2017

Spotify Aliases home-assistant/home-assistant.io#2684

Merged

Formatting/Fixes

8b24157

balloobbot added the platform label May 22, 2017

Remove default arg

1da2811

homeassistant added the cla-signed label May 22, 2017

happyleavesaoc reviewed May 31, 2017

View reviewed changes

Add default keyword

dee3050

homeassistant added the cla-signed label Jun 1, 2017

None check

9c8a869

balloob approved these changes Jun 2, 2017

View reviewed changes

balloob merged commit cefacf9 into home-assistant:dev Jun 2, 2017

balloob mentioned this pull request Jun 2, 2017

0.46 #7866

Merged

home-assistant locked and limited conversation to collaborators Sep 4, 2017

ghost added integration: spotify and removed platform: media_player.spotify labels Mar 21, 2019

Uh oh!

Conversation

Teagan42 commented May 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvizeli commented May 29, 2017

Uh oh!

happyleavesaoc commented May 30, 2017

Uh oh!

bieniu commented May 31, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Teagan42 commented Jun 1, 2017 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Teagan42 commented May 22, 2017 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):