From f4d65e3751a0532498aa5e1177b571ba5ce367e1 Mon Sep 17 00:00:00 2001 From: micha91 Date: Mon, 26 Jul 2021 20:48:20 +0200 Subject: [PATCH] Musiccast grouping fixes (#52339) Co-authored-by: Martin Hjelmare --- .../components/yamaha_musiccast/manifest.json | 2 +- .../yamaha_musiccast/media_player.py | 122 ++++++++++-------- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- 4 files changed, 69 insertions(+), 59 deletions(-) diff --git a/homeassistant/components/yamaha_musiccast/manifest.json b/homeassistant/components/yamaha_musiccast/manifest.json index 501e3b8a00b..46fae870e5e 100644 --- a/homeassistant/components/yamaha_musiccast/manifest.json +++ b/homeassistant/components/yamaha_musiccast/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/yamaha_musiccast", "requirements": [ - "aiomusiccast==0.8.0" + "aiomusiccast==0.8.2" ], "ssdp": [ { diff --git a/homeassistant/components/yamaha_musiccast/media_player.py b/homeassistant/components/yamaha_musiccast/media_player.py index c3269df6ca5..77bc7a21b85 100644 --- a/homeassistant/components/yamaha_musiccast/media_player.py +++ b/homeassistant/components/yamaha_musiccast/media_player.py @@ -34,6 +34,7 @@ from homeassistant.const import ( STATE_PAUSED, STATE_PLAYING, ) +from homeassistant.core import callback from homeassistant.exceptions import HomeAssistantError import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entity import Entity @@ -148,22 +149,28 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): self._cur_track = 0 self._repeat = REPEAT_MODE_OFF - self.coordinator.entities.append(self) async def async_added_to_hass(self): """Run when this Entity has been added to HA.""" await super().async_added_to_hass() + self.coordinator.entities.append(self) # Sensors should also register callbacks to HA when their state changes self.coordinator.musiccast.register_callback(self.async_write_ha_state) self.coordinator.musiccast.register_group_update_callback( self.update_all_mc_entities ) + self.coordinator.async_add_listener(self.async_schedule_check_client_list) async def async_will_remove_from_hass(self): """Entity being removed from hass.""" await super().async_will_remove_from_hass() + self.coordinator.entities.remove(self) # The opposite of async_added_to_hass. Remove any registered call backs here. self.coordinator.musiccast.remove_callback(self.async_write_ha_state) + self.coordinator.musiccast.remove_group_update_callback( + self.update_all_mc_entities + ) + self.coordinator.async_remove_listener(self.async_schedule_check_client_list) @property def should_poll(self): @@ -571,12 +578,18 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): return self - async def update_all_mc_entities(self): + async def update_all_mc_entities(self, check_clients=False): """Update the whole musiccast system when group data change.""" - for entity in self.get_all_mc_entities(): - if entity.is_server: + # First update all servers as they provide the group information for their clients + for entity in self.get_all_server_entities(): + if check_clients or self.coordinator.musiccast.group_reduce_by_source: await entity.async_check_client_list() - entity.async_write_ha_state() + else: + entity.async_write_ha_state() + # Then update all other entities + for entity in self.get_all_mc_entities(): + if not entity.is_server: + entity.async_write_ha_state() # Services @@ -585,7 +598,7 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): Creates a new group if necessary. Used for join service. """ - _LOGGER.info( + _LOGGER.debug( "%s wants to add the following entities %s", self.entity_id, str(group_members), @@ -597,6 +610,9 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): if entity.entity_id in group_members ] + if self.state == STATE_OFF: + await self.async_turn_on() + if not self.is_server and self.musiccast_zone_entity.is_server: # The MusicCast Distribution Module of this device is already in use. To use it as a server, we first # have to unjoin and wait until the servers are updated. @@ -609,38 +625,40 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): if self.is_server else uuid.random_uuid_hex().upper() ) + + ip_addresses = set() # First let the clients join for client in entities: if client != self: try: - await client.async_client_join(group, self) + network_join = await client.async_client_join(group, self) except MusicCastGroupException: _LOGGER.warning( "%s is struggling to update its group data. Will retry perform the update", client.entity_id, ) - await client.async_client_join(group, self) + network_join = await client.async_client_join(group, self) - await self.coordinator.musiccast.mc_server_group_extend( - self._zone_id, - [ - entity.ip_address - for entity in entities - if entity.ip_address != self.ip_address - ], - group, - self.get_distribution_num(), - ) + if network_join: + ip_addresses.add(client.ip_address) + + if ip_addresses: + await self.coordinator.musiccast.mc_server_group_extend( + self._zone_id, + list(ip_addresses), + group, + self.get_distribution_num(), + ) _LOGGER.debug( "%s added the following entities %s", self.entity_id, str(entities) ) - _LOGGER.info( + _LOGGER.debug( "%s has now the following musiccast group %s", self.entity_id, str(self.musiccast_group), ) - await self.update_all_mc_entities() + await self.update_all_mc_entities(True) async def async_unjoin_player(self): """Leave the group. @@ -654,15 +672,15 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): else: await self.async_client_leave_group() - await self.update_all_mc_entities() + await self.update_all_mc_entities(True) # Internal client functions - async def async_client_join(self, group_id, server): + async def async_client_join(self, group_id, server) -> bool: """Let the client join a group. If this client is a server, the server will stop distributing. If the client is part of a different group, - it will leave that group first. + it will leave that group first. Returns True, if the server has to add the client on his side. """ # If we should join the group, which is served by the main zone, we can simply select main_sync as input. _LOGGER.debug("%s called service client join", self.entity_id) @@ -672,14 +690,16 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): if server.zone == DEFAULT_ZONE: await self.async_select_source(ATTR_MAIN_SYNC) server.async_write_ha_state() - return + return False # It is not possible to join a group hosted by zone2 from main zone. - raise Exception("Can not join a zone other than main of the same device.") + raise HomeAssistantError( + "Can not join a zone other than main of the same device." + ) if self.musiccast_zone_entity.is_server: # If one of the zones of the device is a server, we need to unjoin first. - _LOGGER.info( + _LOGGER.debug( "%s is a server of a group and has to stop distribution " "to use MusicCast for %s", self.musiccast_zone_entity.entity_id, @@ -688,11 +708,11 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): await self.musiccast_zone_entity.async_server_close_group() elif self.is_client: - if self.coordinator.data.group_id == server.coordinator.data.group_id: + if self.is_part_of_group(server): _LOGGER.warning("%s is already part of the group", self.entity_id) - return + return False - _LOGGER.info( + _LOGGER.debug( "%s is client in a different group, will unjoin first", self.entity_id, ) @@ -705,20 +725,14 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): ): # The device is already part of this group (e.g. main zone is also a client of this group). # Just select mc_link as source - await self.async_select_source(ATTR_MC_LINK) - # As the musiccast group has changed, we need to trigger the servers ha state. - # In other cases this happens due to the callback after the dist updated message. - server.async_write_ha_state() - return + await self.coordinator.musiccast.zone_join(self._zone_id) + return False _LOGGER.debug("%s will now join as a client", self.entity_id) await self.coordinator.musiccast.mc_client_join( server.ip_address, group_id, self._zone_id ) - - # Ensure that mc link is selected. If main sync was selected previously, it's possible that this does not - # happen automatically - await self.async_select_source(ATTR_MC_LINK) + return True async def async_client_leave_group(self, force=False): """Make self leave the group. @@ -728,18 +742,9 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): _LOGGER.debug("%s client leave called", self.entity_id) if not force and ( self.source == ATTR_MAIN_SYNC - or len( - [entity for entity in self.other_zones if entity.source == ATTR_MC_LINK] - ) - > 0 + or [entity for entity in self.other_zones if entity.source == ATTR_MC_LINK] ): - # If we are only syncing to main or another zone is also using the musiccast module as client, don't - # kill the client session, just select a dummy source. - save_inputs = self.coordinator.musiccast.get_save_inputs(self._zone_id) - if len(save_inputs): - await self.async_select_source(save_inputs[0]) - # Then turn off the zone - await self.async_turn_off() + await self.coordinator.musiccast.zone_unjoin(self._zone_id) else: servers = [ server @@ -747,14 +752,11 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): if server.coordinator.data.group_id == self.coordinator.data.group_id ] await self.coordinator.musiccast.mc_client_unjoin() - if len(servers): + if servers: await servers[0].coordinator.musiccast.mc_server_group_reduce( servers[0].zone_id, [self.ip_address], self.get_distribution_num() ) - for server in self.get_all_server_entities(): - await server.async_check_client_list() - # Internal server functions async def async_server_close_group(self): @@ -762,7 +764,7 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): Should only be called for servers. """ - _LOGGER.info("%s closes his group", self.entity_id) + _LOGGER.debug("%s closes his group", self.entity_id) for client in self.musiccast_group: if client != self: await client.async_client_leave_group() @@ -770,6 +772,9 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): async def async_check_client_list(self): """Let the server check if all its clients are still part of his group.""" + if not self.is_server or self.coordinator.data.group_update_lock.locked(): + return + _LOGGER.debug("%s updates his group members", self.entity_id) client_ips_for_removal = [] for expected_client_ip in self.coordinator.data.group_client_list: @@ -779,8 +784,8 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): # The client is no longer part of the group. Prepare removal. client_ips_for_removal.append(expected_client_ip) - if len(client_ips_for_removal) > 0: - _LOGGER.info( + if client_ips_for_removal: + _LOGGER.debug( "%s says good bye to the following members %s", self.entity_id, str(client_ips_for_removal), @@ -793,3 +798,8 @@ class MusicCastMediaPlayer(MusicCastDeviceEntity, MediaPlayerEntity): await self.async_server_close_group() self.async_write_ha_state() + + @callback + def async_schedule_check_client_list(self): + """Schedule async_check_client_list.""" + self.hass.create_task(self.async_check_client_list()) diff --git a/requirements_all.txt b/requirements_all.txt index c348e03b06a..2256474d3b6 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -212,7 +212,7 @@ aiolyric==1.0.7 aiomodernforms==0.1.8 # homeassistant.components.yamaha_musiccast -aiomusiccast==0.8.0 +aiomusiccast==0.8.2 # homeassistant.components.keyboard_remote aionotify==0.2.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 6c3fe00b068..6d1c0afd6b2 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -137,7 +137,7 @@ aiolyric==1.0.7 aiomodernforms==0.1.8 # homeassistant.components.yamaha_musiccast -aiomusiccast==0.8.0 +aiomusiccast==0.8.2 # homeassistant.components.notion aionotion==3.0.2