From cd845954291a9539613c660ee273ecf0cc5e94a3 Mon Sep 17 00:00:00 2001 From: Greg Dowling Date: Tue, 27 Apr 2021 22:55:29 +0100 Subject: [PATCH] Rework roon media player grouping to use media_player base services (#49667) * Add group/join status attributes to roon player. * Rework join/unjoin code to use base media_player services. * Switch join and unjoin to be sync. --- homeassistant/components/roon/manifest.json | 2 +- homeassistant/components/roon/media_player.py | 71 ++++++------------- homeassistant/components/roon/server.py | 6 ++ homeassistant/components/roon/services.yaml | 20 ------ requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- 6 files changed, 29 insertions(+), 74 deletions(-) diff --git a/homeassistant/components/roon/manifest.json b/homeassistant/components/roon/manifest.json index 875294310d9..09fcaad5f1f 100644 --- a/homeassistant/components/roon/manifest.json +++ b/homeassistant/components/roon/manifest.json @@ -3,7 +3,7 @@ "name": "RoonLabs music player", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/roon", - "requirements": ["roonapi==0.0.32"], + "requirements": ["roonapi==0.0.36"], "codeowners": ["@pavoni"], "iot_class": "local_push" } diff --git a/homeassistant/components/roon/media_player.py b/homeassistant/components/roon/media_player.py index 773028da2d3..ff55c0fb1fb 100644 --- a/homeassistant/components/roon/media_player.py +++ b/homeassistant/components/roon/media_player.py @@ -7,6 +7,7 @@ import voluptuous as vol from homeassistant.components.media_player import MediaPlayerEntity from homeassistant.components.media_player.const import ( SUPPORT_BROWSE_MEDIA, + SUPPORT_GROUPING, SUPPORT_NEXT_TRACK, SUPPORT_PAUSE, SUPPORT_PLAY, @@ -42,6 +43,7 @@ from .media_browser import browse_media SUPPORT_ROON = ( SUPPORT_BROWSE_MEDIA + | SUPPORT_GROUPING | SUPPORT_PAUSE | SUPPORT_VOLUME_SET | SUPPORT_STOP @@ -59,12 +61,8 @@ SUPPORT_ROON = ( _LOGGER = logging.getLogger(__name__) -SERVICE_JOIN = "join" -SERVICE_UNJOIN = "unjoin" SERVICE_TRANSFER = "transfer" -ATTR_JOIN = "join_ids" -ATTR_UNJOIN = "unjoin_ids" ATTR_TRANSFER = "transfer_id" @@ -75,16 +73,6 @@ async def async_setup_entry(hass, config_entry, async_add_entities): # Register entity services platform = entity_platform.current_platform.get() - platform.async_register_entity_service( - SERVICE_JOIN, - {vol.Required(ATTR_JOIN): vol.All(cv.ensure_list, [cv.entity_id])}, - "join", - ) - platform.async_register_entity_service( - SERVICE_UNJOIN, - {vol.Optional(ATTR_UNJOIN): vol.All(cv.ensure_list, [cv.entity_id])}, - "unjoin", - ) platform.async_register_entity_service( SERVICE_TRANSFER, {vol.Required(ATTR_TRANSFER): cv.entity_id}, @@ -164,6 +152,13 @@ class RoonDevice(MediaPlayerEntity): """Flag media player features that are supported.""" return SUPPORT_ROON + @property + def group_members(self): + """Return the grouped players.""" + + roon_names = self._server.roonapi.grouped_zone_names(self._output_id) + return [self._server.entity_id(roon_name) for roon_name in roon_names] + @property def device_info(self): """Return the device info.""" @@ -491,8 +486,8 @@ class RoonDevice(MediaPlayerEntity): path_list, ) - def join(self, join_ids): - """Add another Roon player to this player's join group.""" + def join_players(self, group_members): + """Join `group_members` as a player group with the current player.""" zone_data = self._server.roonapi.zone_by_output_id(self._output_id) if zone_data is None: @@ -511,7 +506,7 @@ class RoonDevice(MediaPlayerEntity): sync_available[zone["display_name"]] = output["output_id"] names = [] - for entity_id in join_ids: + for entity_id in group_members: name = self._server.roon_name(entity_id) if name is None: _LOGGER.error("No roon player found for %s", entity_id) @@ -531,43 +526,17 @@ class RoonDevice(MediaPlayerEntity): [self._output_id] + [sync_available[name] for name in names] ) - def unjoin(self, unjoin_ids=None): - """Remove a Roon player to this player's join group.""" + def unjoin_player(self): + """Remove this player from any group.""" - zone_data = self._server.roonapi.zone_by_output_id(self._output_id) - if zone_data is None: - _LOGGER.error("No zone data for %s", self.name) + if not self._server.roonapi.is_grouped(self._output_id): + _LOGGER.error( + "Can't unjoin player %s because it's not in a group", + self.name, + ) return - join_group = { - output["display_name"]: output["output_id"] - for output in zone_data["outputs"] - if output["display_name"] != self.name - } - - if unjoin_ids is None: - # unjoin everything - names = list(join_group) - else: - names = [] - for entity_id in unjoin_ids: - name = self._server.roon_name(entity_id) - if name is None: - _LOGGER.error("No roon player found for %s", entity_id) - return - - if name not in join_group: - _LOGGER.error( - "Can't unjoin player %s from %s because it's not in the joined group %s", - name, - self.name, - list(join_group), - ) - return - names.append(name) - - _LOGGER.debug("Unjoining %s from %s", names, self.name) - self._server.roonapi.ungroup_outputs([join_group[name] for name in names]) + self._server.roonapi.ungroup_outputs([self._output_id]) async def async_transfer(self, transfer_id): """Transfer playback from this roon player to another.""" diff --git a/homeassistant/components/roon/server.py b/homeassistant/components/roon/server.py index 83b620e176e..d216dca419d 100644 --- a/homeassistant/components/roon/server.py +++ b/homeassistant/components/roon/server.py @@ -28,6 +28,7 @@ class RoonServer: self.offline_devices = set() self._exit = False self._roon_name_by_id = {} + self._id_by_roon_name = {} async def async_setup(self, tries=0): """Set up a roon server based on config parameters.""" @@ -78,11 +79,16 @@ class RoonServer: def add_player_id(self, entity_id, roon_name): """Register a roon player.""" self._roon_name_by_id[entity_id] = roon_name + self._id_by_roon_name[roon_name] = entity_id def roon_name(self, entity_id): """Get the name of the roon player from entity_id.""" return self._roon_name_by_id.get(entity_id) + def entity_id(self, roon_name): + """Get the id of the roon player from the roon name.""" + return self._id_by_roon_name.get(roon_name) + def stop_roon(self): """Stop background worker.""" self.roonapi.stop() diff --git a/homeassistant/components/roon/services.yaml b/homeassistant/components/roon/services.yaml index ec096effe5b..6622d9b4c31 100644 --- a/homeassistant/components/roon/services.yaml +++ b/homeassistant/components/roon/services.yaml @@ -1,23 +1,3 @@ -join: - description: Group players together. - fields: - entity_id: - description: id of the player that will be the master of the group. - example: "media_player.study" - join_ids: - description: id(s) of the players that will join the master. - example: "['media_player.bedroom', 'media_player.kitchen']" - -unjoin: - description: Remove players from a group. - fields: - entity_id: - description: id of the player that is the master of the group.. - example: "media_player.study" - unjoin_ids: - description: Optional id(s) of the players that will be unjoined from the group. If not specified, all players will be unjoined from the master. - example: "['media_player.bedroom', 'media_player.kitchen']" - transfer: description: Transfer playback from one player to another. fields: diff --git a/requirements_all.txt b/requirements_all.txt index dac6f3c3549..2e5d7581749 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2000,7 +2000,7 @@ rokuecp==0.8.1 roombapy==1.6.3 # homeassistant.components.roon -roonapi==0.0.32 +roonapi==0.0.36 # homeassistant.components.rova rova==0.2.1 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index bf695a0eeb3..3bddefb76e9 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1067,7 +1067,7 @@ rokuecp==0.8.1 roombapy==1.6.3 # homeassistant.components.roon -roonapi==0.0.32 +roonapi==0.0.36 # homeassistant.components.rpi_power rpi-bad-power==0.1.0