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

Send media_stop() on turn_off() #7940

Merged
merged 1 commit into from Jun 7, 2017

Conversation

Projects
None yet
5 participants
@Juggels
Copy link
Contributor

commented Jun 7, 2017

Description:

Instead of pausing playback upon sending 'turn_off()', now stop playback. This will prevent live streams from continuing playback when a turn_off command is sent to the speaker.

Related issue (if applicable): fixes #7925

Checklist:

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
@mention-bot

This comment has been minimized.

Copy link

commented Jun 7, 2017

@Juggels, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pvizeli, @rhooper and @bjarniivarsson to be potential reviewers.

@lwis

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2017

Which speakers have you tested this against?

@Juggels

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2017

Just successfully ran the following tests:

  • Playing a live (radio) stream
    • Single Play:1
    • Grouped Play:1 and Play:5, sent command to coordinator
    • Grouped Play:1 and Play:5, sent command to slave
  • Playing from music library (turn_on will restart playing the current playlist, instead of resuming)
    • Single Play:1
    • Grouped Play:1 and Play:5, sent command to coordinator
    • Grouped Play:1 and Play:5, sent command to slave
@pvizeli

pvizeli approved these changes Jun 7, 2017

Copy link
Member

left a comment

I think this change are legitim. Please a second review on that

@lwis

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2017

LGTM! Thanks 🐊

@lwis lwis merged commit 36eb0ce into home-assistant:dev Jun 7, 2017

4 checks passed

cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 93.546%
Details
hound No violations found. Woof!

@balloob balloob referenced this pull request Jun 16, 2017

Merged

0.47 #8055

@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017

@Juggels Juggels deleted the Juggels:sonos branch Mar 13, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.