Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transfer playback when changing device #408

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

TimotheeGerber
Copy link
Contributor

Warning: This PR will not work before PR #90 in rspotify has been merged and a new version has been released. If you want to test it, you can clone my patched version of rspotify and ask cargo to use it instead of the one in crates.io by adding the following in your Cargo.toml:

[patch.crates-io]
rspotify = { path = "../relative/path/to/patched_rspotify" }

Currently, selecting another device changes the device in the internal representation but does not transfer playback to the selected device. This is counter intuitive and lead to strange situations.

Imagine I have 2 devices, receiver_1 and receiver_2, and I am currently playing a song on receiver_1. If I select receiver_2, receiver_1 continues to play the song. Nevertheless, commands (pause, next, previous, etc) are sent to receiver_2. So, a pause command will not pause the song because receiver_1 does not get the command. receiver_2 gets the command and cannot do anything with it.

This PR changes that behavior and transfer playback when selecting a device.

I hope that the modifications I have made are OK. Don't hesitate to comment them if needed!

Closes #366

Copy link
Owner

@Rigellute Rigellute left a comment

Choose a reason for hiding this comment

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

Good work @TimotheeGerber 👍 Thank you.

Will wait until rspotify has released and then will merge this.

@@ -1144,7 +1144,17 @@ impl<'a> Network<'a> {
}
}

async fn set_device_id_in_config(&mut self, device_id: String) {
async fn transfert_playback_to_device(&mut self, device_id: String) {
Copy link
Owner

Choose a reason for hiding this comment

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

Nice 👍

@@ -421,7 +421,7 @@ This table shows all that is possible with the Spotify API, what is implemented
| device | Yes | Get a User’s Available Devices | Yes |
| current_playback | Yes | Get Information About The User’s Current Playback | Yes |
| current_playing | No | Get the User’s Currently Playing Track | No |
| transfer_playback | No | Transfer a User’s Playback | No |
| transfer_playback | Yes | Transfer a User’s Playback | Yes |
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for updating this

@TimotheeGerber TimotheeGerber changed the title WIP: Transfer playback when changing device Transfer playback when changing device Oct 14, 2020
@TimotheeGerber
Copy link
Contributor Author

A new version of rspotify has been released this summer and spotify-tui already use it. This PR can now be integrated. It still works and will improve playback transfer for people using multiple receivers.

@Rigellute
Copy link
Owner

@TimotheeGerber awesome, thanks

@Rigellute Rigellute merged commit 00b1acd into Rigellute:master Oct 21, 2020
@TimotheeGerber TimotheeGerber deleted the transfer_playback branch October 21, 2020 09:32
lanej pushed a commit to lanej/spotify-tui that referenced this pull request Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot change device.
2 participants