svrooij / node-sonos-ts

:speaker: Sonos control library, use this library in your own appliction.
https://sonos-ts.svrooij.io/
MIT License
84 stars 18 forks source link

PlayNotification to all devices #54

Closed Enc3ph4l0n closed 4 years ago

Enc3ph4l0n commented 4 years ago

Hi Stephan,

Is it possible to play a notification through all speakers as a group? Unless I've misunderstood, the only way I can see to achieve this is to loop through the speakers which seems to be exactly how you do it with sonos2mqtt:

return Promise.all(this.sonosManager.Devices.map(d => d.PlayNotification(payload)))

However, this results in an echo as the speakers are playing out of sync. I can correct for it to a certain degree but it's not perfect.

I tried several other approaches such as coordinator.PlayNotification() but this only plays through the targetted speaker. I tried grouping the speakers first before playing, but I suspect I've misunderstood the architecture of how this works.

Prior to this library I was using Jishi's node-http-api with clipall. As far as I can see, it seems to capture the state of each player, groups the speakers, coordinator then plays the notification and then attempts to restore the speakers to what they were. However, your library fits better into my setup and I thought I could achieve the same result.

I think it's likely I've completely underestimated what's possible with grouping and coordinator functionality or I've totally missed the point!

Would be grateful for your thoughts/help!

Thanks!

svrooij commented 4 years ago

I totally agree with you that this could use an update.

What happens if you do the following:

  1. Group some speakers
  2. Pick the coordinator (this will be the first player in the Sonos app)
  3. Send the play notification command to this player only.

What I would expect is that the notification would play on all grouped players in sync.

A solution for this could be to extend the manager to support this. It should temporary group all the players together with the play notification command with a timeout set and the group uri set (grouping is just sending the correct SetAVTranportURI to a speaker).

Then you could play the notification on the group coordinator. And have the timeout on the member player restore the original playback.

What do you think?

Enc3ph4l0n commented 4 years ago

Hi Stephan,

What happens if you do the following:

1. Group some speakers

2. Pick the coordinator (this will be the first player in the Sonos app)

3. Send the play notification command to this player only.

What I would expect is that the notification would play on all grouped players in sync.

That's what I thought too, but I didn't think it was working. Having tried again I see it does work, as you would expect. I was thrown by differing volumes across rooms, and probably by the fact it was playing in sync!

A solution for this could be to extend the manager to support this. It should temporary group all the players together with the play notification command with a timeout set and the group uri set (grouping is just sending the correct SetAVTranportURI to a speaker).

Then you could play the notification on the group coordinator. And have the timeout on the member player restore the original playback.

Having now a firmer grip on the Sonos architecture and the implementation of your library I realise using:

new SonosDevice(<DEVICE>).PlayNotification();

Doesn't actually play to just the individual speaker but also to any speakers it's grouped with. The issue then extends both to playing to an individual speaker and to an entire group.

The solution you outline seems reasonable but should encompass both scenarios; furthermore, it should ascertain pre-existing groups as well as volume etc. so they can be restored too.

Whilst a timeout to restore original playback is a solution, I wonder if the length of the track is required for it to work more efficiently especially in regards to stacked notifications...

I started to mock up a solution last night but with all the elements it was getting too involved for one evening. I think I'm inclined to put this issue on ice for now purely in consideration to the imminent arrival of "Sonos S2". Would you agree? I really hope it doesn't break too much of the library!

Thanks!

svrooij commented 4 years ago

I wouldn't worry about the Sonos S2 stuff.

If your players aren't grouped, or near each other. We would only have to make it play on all group coordinators.

The SonosManager seems like a reasonable place to put this extra stuff, since it already knows all the groups anyway.

So quick solution is to make a PlayNotification method on the manager that would send it to all the know groups. Maybe the volume part should be changed to change the group volume instead of the player volume.

I'm for sure accepting a PR for this, because it would come in handy in my sonos-cli and sonos2mqtt.

chronikum commented 4 years ago

Is someone already working on this? I recently switched to node from an python project for my doorbell and I'd appreciate a synced doorbell greatly :) If not, I could do that in a few week maybe and do a PR

svrooij commented 4 years ago

I haven't started with this, but my suggestion it to use the following. It will play the notification on all separate groups currently available. So grouped speakers stay grouped and individual players will play it on their own.

Promise.all(sonosManager.Devices
            .filter(d => d.Coordinator.Uuid === d.Uuid)
            .map(d => d.PlayNotification(payload))
          )

My idea was to implement this on the SonosManager class, since it knows all the devices already.

github-actions[bot] commented 4 years ago

:tada: This issue has been resolved in version 2.0.9-beta.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 4 years ago

:tada: This issue has been resolved in version 2.0.9 :tada:

The release is available on:

Your semantic-release bot :package::rocket: