Skip to content

spotify media player#6980

Merged
balloob merged 8 commits intohome-assistant:devfrom
happyleavesaoc:spotify
Apr 20, 2017
Merged

spotify media player#6980
balloob merged 8 commits intohome-assistant:devfrom
happyleavesaoc:spotify

Conversation

@happyleavesaoc
Copy link
Copy Markdown
Contributor

@happyleavesaoc happyleavesaoc commented Apr 8, 2017

Description:

Spotify recently opened up their "Connect" playback web api: https://developer.spotify.com/web-api/web-api-connect-endpoint-reference/

Now we can have a Spotify media player with standard playback controls, track metadata, etc. Also can switch devices (implemented via source_select).

Pending spotipy-dev/spotipy#182 on the spotipy library; temporarily depending on my fork.

Requirements:

I hope someone else can test this before it is merged.

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

Example entry for configuration.yaml (if applicable):

media_player:
  - platform: spotify
    client_id: !secret spotify_client_id
    client_secret: !secret spotify_client_secret

Checklist:

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

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

@mention-bot
Copy link
Copy Markdown

@happyleavesaoc, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line under-indented for visual indent

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undefined name 'STATE_IDLE'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undefined name 'STATE_IDLE'

@happyleavesaoc
Copy link
Copy Markdown
Contributor Author

A few users have tested this and confirmed it works. Ready to merge!

@robbiet480
Copy link
Copy Markdown
Contributor

This worked for me, although the redirect caused a file download in my browser. I believe it's because Spotify expects to POST back to /api/spotify but you only accept GETs.

Other than that very small note, everything seems to work!

@happyleavesaoc
Copy link
Copy Markdown
Contributor Author

@robbiet480 POST results in 405: Method Not Allowed since oauth sends a GET, I guess. I haven't been able to find any documentation on what HTTP verb and/or headers are required to not cause a file download in this case.

@balloob
Copy link
Copy Markdown
Member

balloob commented Apr 20, 2017

Sorry for taking so long to review. I've been busy.

I removed the calls to schedule_update_ha_state, your device does not overwrite should_poll and thus the default value True is active. When a device should be polled, Home Assistant will automatically call update after it calls a service.

This looks good 🐬

@balloob balloob merged commit e020d51 into home-assistant:dev Apr 20, 2017
@balloob balloob mentioned this pull request Apr 21, 2017
@pbros
Copy link
Copy Markdown

pbros commented Apr 24, 2017

Hello, I tried using this media player, but I get the following error :

File "/usr/local/lib/python3.5/dist-packages/homeassistant/components/media_player/spotify.py", line 156, in update
self.refresh_spotify_instance()
File "/usr/local/lib/python3.5/dist-packages/homeassistant/components/media_player/spotify.py", line 143, in refresh_spotify_instance
self._oauth.is_token_expired(self._token_info))
AttributeError: 'SpotifyOAuth' object has no attribute 'is_token_expired'

@happyleavesaoc
Copy link
Copy Markdown
Contributor Author

@pbros Make sure you've installed the correct version of spotipy. For future questions, please open an issue, post on the community forum, or visit us on gitter.

@home-assistant home-assistant locked and limited conversation to collaborators Apr 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants