From 4bfdb1433e95dfe504e376ca082def5257c23bcb Mon Sep 17 00:00:00 2001 From: jjlawren Date: Tue, 28 Jun 2022 15:19:27 -0500 Subject: [PATCH] Optimize Sonos unjoin behavior when using `media_player.unjoin` (#74086) * Coalesce Sonos unjoins to process together * Refactor for readability * Skip unjoin call if already ungrouped * Store unjoin data in a dedicated dataclass * Revert import adjustment --- homeassistant/components/sonos/__init__.py | 10 +++++++ .../components/sonos/media_player.py | 30 ++++++++++++++++--- homeassistant/components/sonos/speaker.py | 2 ++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/sonos/__init__.py b/homeassistant/components/sonos/__init__.py index 2e2290dff04..e3f65d754dd 100644 --- a/homeassistant/components/sonos/__init__.py +++ b/homeassistant/components/sonos/__init__.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio from collections import OrderedDict +from dataclasses import dataclass, field import datetime from functools import partial import logging @@ -74,6 +75,14 @@ CONFIG_SCHEMA = vol.Schema( ) +@dataclass +class UnjoinData: + """Class to track data necessary for unjoin coalescing.""" + + speakers: list[SonosSpeaker] + event: asyncio.Event = field(default_factory=asyncio.Event) + + class SonosData: """Storage class for platform global data.""" @@ -89,6 +98,7 @@ class SonosData: self.boot_counts: dict[str, int] = {} self.mdns_names: dict[str, str] = {} self.entity_id_mappings: dict[str, SonosSpeaker] = {} + self.unjoin_data: dict[str, UnjoinData] = {} async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: diff --git a/homeassistant/components/sonos/media_player.py b/homeassistant/components/sonos/media_player.py index 0f766f33e6f..7b18b102919 100644 --- a/homeassistant/components/sonos/media_player.py +++ b/homeassistant/components/sonos/media_player.py @@ -44,8 +44,9 @@ from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import config_validation as cv, entity_platform, service from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity_platform import AddEntitiesCallback +from homeassistant.helpers.event import async_call_later -from . import media_browser +from . import UnjoinData, media_browser from .const import ( DATA_SONOS, DOMAIN as SONOS_DOMAIN, @@ -777,6 +778,27 @@ class SonosMediaPlayerEntity(SonosEntity, MediaPlayerEntity): await self.hass.async_add_executor_job(self.speaker.join, speakers) async def async_unjoin_player(self): - """Remove this player from any group.""" - async with self.hass.data[DATA_SONOS].topology_condition: - await self.hass.async_add_executor_job(self.speaker.unjoin) + """Remove this player from any group. + + Coalesces all calls within 0.5s to allow use of SonosSpeaker.unjoin_multi() + which optimizes the order in which speakers are removed from their groups. + Removing coordinators last better preserves playqueues on the speakers. + """ + sonos_data = self.hass.data[DATA_SONOS] + household_id = self.speaker.household_id + + async def async_process_unjoin(now: datetime.datetime) -> None: + """Process the unjoin with all remove requests within the coalescing period.""" + unjoin_data = sonos_data.unjoin_data.pop(household_id) + await SonosSpeaker.unjoin_multi(self.hass, unjoin_data.speakers) + unjoin_data.event.set() + + if unjoin_data := sonos_data.unjoin_data.get(household_id): + unjoin_data.speakers.append(self.speaker) + else: + unjoin_data = sonos_data.unjoin_data[household_id] = UnjoinData( + speakers=[self.speaker] + ) + async_call_later(self.hass, 0.5, async_process_unjoin) + + await unjoin_data.event.wait() diff --git a/homeassistant/components/sonos/speaker.py b/homeassistant/components/sonos/speaker.py index 729d1a1457f..f4d1d89aa2f 100644 --- a/homeassistant/components/sonos/speaker.py +++ b/homeassistant/components/sonos/speaker.py @@ -906,6 +906,8 @@ class SonosSpeaker: @soco_error() def unjoin(self) -> None: """Unjoin the player from a group.""" + if self.sonos_group == [self]: + return self.soco.unjoin() self.coordinator = None