hass-music-assistant icon indicating copy to clipboard operation
hass-music-assistant copied to clipboard

Turning off snapcast group player won't unlink all players

Open ronluna opened this issue 10 months ago • 21 comments

What version of Music Assistant has the issue?

2.0.0b133

What version of the Home Assistant Integration have you got installed?

n/a

Have you tried everything in the Troubleshooting FAQ and reviewed the Open and Closed Issues and Discussions to resolve this yourself?

  • [X] Yes

The problem

If a snapcast group player has more than 4 members, turning off the group player would leave one of the members linked to the first member of the group

How to reproduce

Create a snapcast group player with over 4 members in it. turn on the group (All members get linked) . Turn off the group (One of the members will remain linked to the first member of that group)

Music Providers

Spotify

Player Providers

Snapcast

Full log output

nothing in the logs

Additional information

No response

What version of Home Assistant Core are your running

n/a

What type of installation are you running?

Home Assistant OS

On what type of hardware are you running?

Alternative

ronluna avatar Apr 09 '24 13:04 ronluna

So just confirming if there are four or less in the group then everything works ok?

OzGav avatar Apr 09 '24 13:04 OzGav

Well I just test with 3 players and same results. So it is safe to say that anything above 2 players won't get completely unlink.

2 players works as expected!

ronluna avatar Apr 09 '24 14:04 ronluna

Explain how you are grouping the players as MA has three ways to do that. You need to supply far more detail.

OzGav avatar Apr 09 '24 22:04 OzGav

Apologies for the lack of detail,

1- Click on the gear icon 2- Click on the Player tab 3- Click on Add group player 4- Select Snapcast 5- Enter a name for the group player and select the players from the list then save.

Then when the group player is turned on. all the players get automatically linked! image

but when the group player is turned off. one of the players remains linked to the group: image

ronluna avatar Apr 10 '24 00:04 ronluna

@marcelveldt is this to be solved at the provider level or at the core level?

SantiagoSotoC avatar May 30 '24 21:05 SantiagoSotoC

This is a possible solution , https://github.com/music-assistant/server/pull/1347

SantiagoSotoC avatar Jun 11 '24 03:06 SantiagoSotoC

@ronluna when you can, would you like to test the latest beta, to confirm the solution

SantiagoSotoC avatar Jun 11 '24 21:06 SantiagoSotoC

Hello @SantiagoSotoC , Volume control seems to be behaving correctly now, however I did manage to encounter a new bug. Currently MA only allow 2 players to be link at once as seen on this video.

https://github.com/music-assistant/hass-music-assistant/assets/9822111/c5b5a7e1-a646-4930-b7c6-2eddc5816887

and the logs show the following:

2024-06-11 22:25:40.313 ERROR (MainThread) [snapcast.control.group] snapclient1 already in group 04cec120-249b-8269-e87b-72fb992eafa2
2024-06-11 22:27:13.521 ERROR (MainThread) [snapcast.control.group] snapclient1 already in group 30d23894-f101-79e9-efb0-084847cee7ed
2024-06-11 22:27:23.153 ERROR (MainThread) [snapcast.control.group] snapclient0#0 already in group 30d23894-f101-79e9-efb0-084847cee7ed
2024-06-11 22:27:23.153 ERROR (MainThread) [snapcast.control.group] snapclient1 already in group 30d23894-f101-79e9-efb0-084847cee7ed
2024-06-11 22:27:31.703 ERROR (MainThread) [snapcast.control.group] snapclient3#3 already in group 30d23894-f101-79e9-efb0-084847cee7ed
2024-06-11 22:27:31.704 ERROR (MainThread) [snapcast.control.group] snapclient0#0 already in group 30d23894-f101-79e9-efb0-084847cee7ed
2024-06-11 22:27:31.704 ERROR (MainThread) [snapcast.control.group] snapclient1 already in group 30d23894-f101-79e9-efb0-084847cee7ed

ronluna avatar Jun 11 '24 22:06 ronluna

I'll check tomorrow, I have an idea of what it might be

SantiagoSotoC avatar Jun 11 '24 23:06 SantiagoSotoC

@ronluna can you test the change in addon dev ? you have to configure the branch as fix_snapcast_player_unsync_when_master_off_2

SantiagoSotoC avatar Jun 13 '24 22:06 SantiagoSotoC

I've been deploying the beta image using the following stack:

version: "3"
services:
  music-assistant-server:
    image: ghcr.io/music-assistant/server:beta # <<< Desired release version here (change beta to latest once there is a stable release)
    container_name: music-assistant-server
    restart: unless-stopped
    # Network mode must be set to host for MA to work correctly
    network_mode: host
    volumes:
      - ${USERDIR:-$HOME}/docker/music-assistant-server/data:/data/
    # privileged caps needed to mount smb folders within the container
    cap_add:
      - SYS_ADMIN
      - DAC_READ_SEARCH
    privileged: true
    environment:
      # Provide logging level as environment variable.
      # default=info, possible=(critical, error, warning, info, debug, verbose)
      - LOG_LEVEL=info

I'm not sure how to properly pull and deplay fix_snapcast_player_unsync_when_master_off_2 as I can't find an image for it.

any ideas?

ronluna avatar Jun 13 '24 22:06 ronluna

I've been deploying the beta image using the following stack:

version: "3"
services:
  music-assistant-server:
    image: ghcr.io/music-assistant/server:beta # <<< Desired release version here (change beta to latest once there is a stable release)
    container_name: music-assistant-server
    restart: unless-stopped
    # Network mode must be set to host for MA to work correctly
    network_mode: host
    volumes:
      - ${USERDIR:-$HOME}/docker/music-assistant-server/data:/data/
    # privileged caps needed to mount smb folders within the container
    cap_add:
      - SYS_ADMIN
      - DAC_READ_SEARCH
    privileged: true
    environment:
      # Provide logging level as environment variable.
      # default=info, possible=(critical, error, warning, info, debug, verbose)
      - LOG_LEVEL=info

I'm not sure how to properly pull and deplay fix_snapcast_player_unsync_when_master_off_2 as I can't find an image for it.

any ideas?

Don't worry, I thought you were using HA addons, I'll try my best, and in the next beta, try it out

SantiagoSotoC avatar Jun 13 '24 23:06 SantiagoSotoC

Yes, fix that and duplicate players

SantiagoSotoC avatar Jun 14 '24 12:06 SantiagoSotoC

Although it appears that the changes in 1360 don't fix the issue as I manually replace init.py with 1360's changes and the linking the player were having the same problem. I only replace init.py file as it was the only file that I've noticed had changes on.

ronluna avatar Jun 14 '24 13:06 ronluna

@ronluna The latest beta should fix this, I await your response to close the isssue

SantiagoSotoC avatar Jun 20 '24 02:06 SantiagoSotoC

Hello @SantiagoSotoC ,

Creating groups are somewhat working. No error in the logs though but the groups are created properly only the first time after MA has started and in order to get it working again the docker instance needs to get restarted.

https://github.com/music-assistant/hass-music-assistant/assets/9822111/545440af-0ba6-4ae4-8dc3-52860cb8c209

ronluna avatar Jun 20 '24 03:06 ronluna

We are talking about two separate things

  1. Turning off snapcast group player won't unlink all players
  2. The creation of groups from the ui

Let's go with the first one, and we'll see the other in another issue, otherwise everything gets mixed up here.

SantiagoSotoC avatar Jun 20 '24 04:06 SantiagoSotoC

Hello @SantiagoSotoC ,

Creating groups are somewhat working. No error in the logs though but the groups are created properly only the first time after MA has started and in order to get it working again the docker instance needs to get restarted.

Screen.Recording.2024-06-19.at.11.34.37.PM.420.mov

The group in snapweb is created well, it is a display issue, or some incorrect value, but it is better to open another discussion for that.

SantiagoSotoC avatar Jun 20 '24 04:06 SantiagoSotoC

Understood, Although its kinda mixed up... As yes, currently turning off Group players (The one that have been created using the "add group player" button from within the setting menu) area properly getting unlink but unlinking group players that have been created on the fly (by using the chain icon on the player's side bar) are not linking/unlinking correctly.

ronluna avatar Jun 20 '24 04:06 ronluna

I do have a question though. What would be the expected behavior when interacting with group players that have been pre created from the settings screen where some snapcast players are members on multiple groups.

Example:

Common areas members: (Kitchen, Dining, Living, Terrace) Entire House: (Master Bed, Family, Kitchen, Dining, Living, Terrace)

As seen in that above groups some player are member on both groups and currently MA can have both group players turned on and are shown as properly link but does not properly reflects what snapweb is displaying and only that last one that have been turned on is the one effectively active and linked. and most of the time seems like MA gets confused and does not know what to link or unlink in this scenario.

ronluna avatar Jun 20 '24 04:06 ronluna

I just found out that this function exists, I guess it will be implemented in the time I was gone. But I guess hand created groups have priority. Always snapweb is the real source, the ma model is not perfectly coupled to snapcast, the provider is like a translation and there are a lot of edge cases.

I will have to install some clients with squeezelite to know what the expected experience is like.

I think that in the commit I deleted a very necessary line, for the next beta I have added it back.

I would still like to dedicate a few days to anailize several things on the blackboard, I understand things about the library that at the time I did not understand and now it is a patch on top of another because of the chaos of these decisions.

SantiagoSotoC avatar Jun 20 '24 05:06 SantiagoSotoC

Hello @SantiagoSotoC , just a quick update while testing version 2.1.0b10 and this is what I've noticed:

  • Manually linking/unlinking snapcast players from the sidebar on MA is not available (the little chain is nowhere to be found)
  • When turning off predefined groups that might have snapcast players that are members in different predefined group won't unlink any of the players, instead it shows the master player with all its members linked to it.

I think when dealing with snapcast players that are members in multiple predefined groups, MA should check if any of the members is part of any predefined group and if it is then turn off that group so all the members of group get release before adding them to the new predefined group... Just an idea.

ronluna avatar Jul 05 '24 13:07 ronluna

@ronluna ... 100% ACK. This should be somehow the ideal behaviour.

DerPicknicker avatar Jul 10 '24 15:07 DerPicknicker

Only one predefined group does not work well, and going to two is too much for the current state The idea of a predefined group is that there is no sync master, more like what a snapcast group really is. I don't know how it was thought to work when a player is part of many groups, it is a problem for later, it is more important that you can sync and unsync from the UI, without problems and that when a song is paused it remembers the position. Things are simpler one step at a time, when the rest is solved we see this

SantiagoSotoC avatar Jul 10 '24 20:07 SantiagoSotoC

@ronluna the problem with sync chain should be solved in rc2. Maybe now the predefined groups behave better, but I didn't test it, but there were some things wrong in the provider and also in the ui

SantiagoSotoC avatar Jul 15 '24 02:07 SantiagoSotoC

@ronluna We will close this soon assuming it is fixed unless we hear otherwise

OzGav avatar Jul 18 '24 04:07 OzGav

Just had a little time to test yesterday and today and for far this is what I've found:

Not sure if this is related to this bug but the total amount of player does not seem correct when players are linked:

image

The Living player shows that has +4 linked player when in reality has +5 .

When manually unlinking players (using the chain next to the player) it does not unlink all the player. the log shows the following: 2024-07-18 10:48:50.450 ERROR (MainThread) [music_assistant.webserver] Error handling message: players/cmd/unsync_many: 'Player' object has no attribute 'group_child'

Predefined groups become unavailable/dissapear from the player's side bar list and needs to be recreated after turning them on/off .

ronluna avatar Jul 18 '24 10:07 ronluna

Predefined groups become unavailable/dissapear from the player's side bar list and needs to be recreated after turning them on/off .

@OzGav We would have to test this with other providers

SantiagoSotoC avatar Jul 18 '24 16:07 SantiagoSotoC

The Living player shows that has +4 linked player when in reality has +5 .

@marcelveldt this may be another problem in ui?

SantiagoSotoC avatar Jul 18 '24 16:07 SantiagoSotoC

2024-07-18 10:48:50.450 ERROR (MainThread) [music_assistant.webserver] Error handling message: players/cmd/unsync_many: 'Player' object has no attribute 'group_child'

I found the error, in a while I do a PR, it was a typo in the revisions of the last PR

Edit: Repaired at the next rc

SantiagoSotoC avatar Jul 18 '24 16:07 SantiagoSotoC