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

Switch will mute/unmute instead of play/stop #46

Merged
merged 6 commits into from
Dec 11, 2016

Conversation

amitgandhinz
Copy link
Contributor

@amitgandhinz amitgandhinz commented Dec 8, 2016

Play/Stop doesnt make sense on a Sonos Playbar, and this.device.play will return an error on the Playbar.

This change modifies the switch behavior to instead Mute/Unmute the speaker.

Amit Gandhi added 2 commits December 7, 2016 21:57
Play/Stop doesnt make sense on a Sonos Playbar, and device.play will return an error on the Playbar.

This change modifies the switch behavior to instead Mute/Unmute the speaker.

TODO: support both playbar vs play 1 speakers based on device description
Switch will mute/unmute instead of play/stop
@nfarina
Copy link
Owner

nfarina commented Dec 9, 2016

A simple/dumb solution might be to introduce a config option for muting instead of pausing. Since you have to add one Accessory per room - just add an option like mute: true to the accessory representing your Playbar:

{
  "accessories": [
    {
      "accessory": "Sonos",
      "name": "Living Room Speakers",
      "room": "Living Room",
      "mute": true,
    }
  ]
}

Play/Stop doesnt make sense on a Sonos Playbar, and device.play will return an error on the Playbar.

This change modifies the switch behavior to instead Mute/Unmute the speaker based on a config option.
Play/Stop doesnt make sense on a Sonos Playbar, and device.play will return an error on the Playbar.

This change modifies the switch behavior to instead Mute/Unmute the speaker based on a config option.
@amitgandhinz
Copy link
Contributor Author

amitgandhinz commented Dec 10, 2016

Thanks for the tip @nfarina . I have added the config option.

@nfarina
Copy link
Owner

nfarina commented Dec 11, 2016

Awesome! Ready to merge?

@amitgandhinz
Copy link
Contributor Author

Yes sir!

@nfarina nfarina merged commit 8f1f7bd into nfarina:master Dec 11, 2016
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.

2 participants