add mute to notificationHandler

[ColdFireIce]

I think this was missing. Player.state.mute didn't change for me. Could find it in the code either.

I'm not sure if its ok to introduce a new event for mute-change, but I thought it wouldn't fit the volume-change event.

I would like to implement a GroupMute similar to GroupVolume and recalulateGroupMute similar to recalulateGroupVolume. Is this ok? Or should it be part of the GroupVolume functiion?

[jishi]

Yeah, this was part of the old version actually, so it makes sense to re-add it.

Regarding groupMute, that was also available in the old version. It worked slightly different than volume since it uses the builtin GroupMute-functionality in Sonos, you can see it here:

https://github.com/jishi/node-sonos-discovery/blob/legacy/lib/player.js#L640

It also keeps state depending on evented info, not according to mute-state of the players, so recalculation* might be unecessary.

For volume, this approach is unpredictable, but for mute it might be OK.

[ColdFireIce]

I'm not quite sure what you mean with:

It also keeps state depending on evented info, not according to mute-state of the players, so recalculation* might be unecessary.

For volume, this approach is unpredictable, but for mute it might be OK.

So setting GroupMute should use the soap call. But updating groupState.mute would be handled how?

[ColdFireIce]

I would thing recalculateGroupMute would look something like:

function recalculateGroupMute() {

  const zone = this.system.zones.find(zone => zone.uuid === this.uuid);

  this.groupState.mute = !!zone.members.find(player => !player.state.mute).length;
}

and would be invoked in the Player.notificationHandler when 'data' cotains the 'mute' object. Similar to the data.volume case. Or is there an separate event for groupMute?

[jishi]

Yeah, one could do that, but as I said, there is a GroupMute event as well which one could use for simplicity, would be more streamlined since you wouldn't recalculate for every player that receives a mute signal.

[jishi]

See here, https://github.com/jishi/node-sonos-discovery/blob/legacy/lib/player.js#L155-L159

And also https://github.com/jishi/node-sonos-discovery/blob/legacy/lib/player.js#L450-L452

[ColdFireIce]

Ah... I see. So this separate event should be implemented. Something like this? NotificationListener.js:

    sax.on('tag:groupmute', (property) => {
      logger.trace(property.$text);
      _this.emit('group-mute', uuid, property.$text);
    });

Player.js

  listener.on('group-mute', (val) => _this.coordinator.groupState.mute = val === '1');

Or is this not the right way?

[jishi]

I need to investigate it further to be sure, and also setup correct mocks and tests for it. If you fix the setting of group volume, then I can fix the rest.

[ColdFireIce]

Ok will do tomorrow :) I'll implement the setGroupMute action via a soap action And update the groupState.mute using the GroupRenderingControl/Event I'll create a new Pull Request then.

Greetings

[ColdFireIce]

This pull request (adding mute to the notificationHandler) is ok as is, right? So this could be merged?

[jishi]

Yes, this should be fine!