From 1efd58a8d4ce2a4aa9750ed66f496364c33270d4 Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Wed, 12 Jan 2022 23:33:19 +0000 Subject: [PATCH 1/7] Utilize Registry() helper to simplify data update coordinator --- .../components/overkiz/coordinator.py | 124 ++++++++++-------- 1 file changed, 69 insertions(+), 55 deletions(-) diff --git a/homeassistant/components/overkiz/coordinator.py b/homeassistant/components/overkiz/coordinator.py index 45d98cef28692..0139d2372fdf3 100644 --- a/homeassistant/components/overkiz/coordinator.py +++ b/homeassistant/components/overkiz/coordinator.py @@ -13,16 +13,19 @@ NotAuthenticatedException, TooManyRequestsException, ) -from pyoverkiz.models import Device, Place +from pyoverkiz.models import Device, Event, Place from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed +from homeassistant.util.decorator import Registry from .const import DOMAIN, UPDATE_INTERVAL _LOGGER = logging.getLogger(__name__) +EVENT_HANDLERS = Registry() + class OverkizDataUpdateCoordinator(DataUpdateCoordinator[dict[str, Device]]): """Class to manage fetching data from Overkiz platform.""" @@ -56,7 +59,7 @@ def __init__( for device in devices ) self.executions: dict[str, dict[str, str]] = {} - self.areas = self.places_to_area(places) + self.areas = self._places_to_area(places) self._config_entry_id = config_entry_id async def _async_update_data(self) -> dict[str, Device]: @@ -88,57 +91,8 @@ async def _async_update_data(self) -> dict[str, Device]: for event in events: _LOGGER.debug(event) - if event.name == EventName.DEVICE_AVAILABLE: - self.devices[event.device_url].available = True - - elif event.name in [ - EventName.DEVICE_UNAVAILABLE, - EventName.DEVICE_DISABLED, - ]: - self.devices[event.device_url].available = False - - elif event.name in [ - EventName.DEVICE_CREATED, - EventName.DEVICE_UPDATED, - ]: - self.hass.async_create_task( - self.hass.config_entries.async_reload(self._config_entry_id) - ) - - elif event.name == EventName.DEVICE_REMOVED: - base_device_url, *_ = event.device_url.split("#") - registry = dr.async_get(self.hass) - - if registered_device := registry.async_get_device( - {(DOMAIN, base_device_url)} - ): - registry.async_remove_device(registered_device.id) - - del self.devices[event.device_url] - - elif event.name == EventName.DEVICE_STATE_CHANGED: - for state in event.device_states: - device = self.devices[event.device_url] - - if (device_state := device.states[state.name]) is None: - device_state = state - device.states[state.name] = device_state - - device_state.value = state.value - - elif event.name == EventName.EXECUTION_REGISTERED: - if event.exec_id not in self.executions: - self.executions[event.exec_id] = {} - - if not self.is_stateless: - self.update_interval = timedelta(seconds=1) - - elif ( - event.name == EventName.EXECUTION_STATE_CHANGED - and event.exec_id in self.executions - and event.new_state in [ExecutionState.COMPLETED, ExecutionState.FAILED] - ): - del self.executions[event.exec_id] + if event_handler := EVENT_HANDLERS.get(event.name): + await event_handler(self, event) if not self.executions: self.update_interval = UPDATE_INTERVAL @@ -150,7 +104,7 @@ async def _get_devices(self) -> dict[str, Device]: _LOGGER.debug("Fetching all devices and state via /setup/devices") return {d.device_url: d for d in await self.client.get_devices(refresh=True)} - def places_to_area(self, place: Place) -> dict[str, str]: + def _places_to_area(self, place: Place) -> dict[str, str]: """Convert places with sub_places to a flat dictionary.""" areas = {} if isinstance(place, Place): @@ -158,6 +112,66 @@ def places_to_area(self, place: Place) -> dict[str, str]: if isinstance(place.sub_places, list): for sub_place in place.sub_places: - areas.update(self.places_to_area(sub_place)) + areas.update(self._places_to_area(sub_place)) return areas + + @EVENT_HANDLERS.register(EventName.DEVICE_AVAILABLE) + async def on_device_available(self, event: Event) -> None: + """Handle device available event.""" + if event.device_url: + self.devices[event.device_url].available = True + + @EVENT_HANDLERS.register((EventName.DEVICE_UNAVAILABLE, EventName.DEVICE_DISABLED)) + async def on_device_unavailable_disabled(self, event: Event) -> None: + """Handle device unavailable / dispabled event.""" + if event.device_url: + self.devices[event.device_url].available = False + + @EVENT_HANDLERS.register(EventName.DEVICE_STATE_CHANGED) + async def on_device_state_changed(self, event: Event) -> None: + """Handle device state changed event.""" + if not event.device_url: + return + + for state in event.device_states: + device = self.devices[event.device_url] + + if (device_state := device.states[state.name]) is None: + device_state = state + device.states[state.name] = device_state + + device_state.value = state.value + + @EVENT_HANDLERS.register(EventName.DEVICE_REMOVED) + async def on_device_removed(self, event: Event) -> None: + """Handle device removed event.""" + if not event.device_url: + return + + base_device_url = event.device_url.split("#")[0] + registry = dr.async_get(self.hass) + + if registered_device := registry.async_get_device({(DOMAIN, base_device_url)}): + registry.async_remove_device(registered_device.id) + + if event.device_url: + del self.devices[event.device_url] + + @EVENT_HANDLERS.register(EventName.EXECUTION_REGISTERED) + async def on_execution_registered(self, event: Event) -> None: + """Handle execution registered event.""" + if event.exec_id and event.exec_id not in self.executions: + self.executions[event.exec_id] = {} + + if not self.is_stateless: + self.update_interval = timedelta(seconds=1) + + @EVENT_HANDLERS.register(EventName.EXECUTION_STATE_CHANGED) + async def on_execution_state_changed(self, event: Event) -> None: + """Handle execution changed event.""" + if event.exec_id in self.executions and event.new_state in [ + ExecutionState.COMPLETED, + ExecutionState.FAILED, + ]: + del self.executions[event.exec_id] From 127ed950fc579b11c97517210a754c79318c69e0 Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Thu, 13 Jan 2022 09:43:28 +0000 Subject: [PATCH 2/7] Small documentation changs --- homeassistant/components/overkiz/coordinator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/overkiz/coordinator.py b/homeassistant/components/overkiz/coordinator.py index 0139d2372fdf3..0305f3c2aea28 100644 --- a/homeassistant/components/overkiz/coordinator.py +++ b/homeassistant/components/overkiz/coordinator.py @@ -101,11 +101,11 @@ async def _async_update_data(self) -> dict[str, Device]: async def _get_devices(self) -> dict[str, Device]: """Fetch devices.""" - _LOGGER.debug("Fetching all devices and state via /setup/devices") + _LOGGER.debug("Fetching all devices and state via /setup/devices.") return {d.device_url: d for d in await self.client.get_devices(refresh=True)} def _places_to_area(self, place: Place) -> dict[str, str]: - """Convert places with sub_places to a flat dictionary.""" + """Convert places with sub_places to a flat dictionary [placeoid, label]).""" areas = {} if isinstance(place, Place): areas[place.oid] = place.label @@ -124,7 +124,7 @@ async def on_device_available(self, event: Event) -> None: @EVENT_HANDLERS.register((EventName.DEVICE_UNAVAILABLE, EventName.DEVICE_DISABLED)) async def on_device_unavailable_disabled(self, event: Event) -> None: - """Handle device unavailable / dispabled event.""" + """Handle device unavailable / disabled event.""" if event.device_url: self.devices[event.device_url].available = False From 2de4127926f340debf6f13842228d3d819fb793a Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Thu, 13 Jan 2022 09:55:31 +0000 Subject: [PATCH 3/7] Simplify state --- homeassistant/components/overkiz/coordinator.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/homeassistant/components/overkiz/coordinator.py b/homeassistant/components/overkiz/coordinator.py index 0305f3c2aea28..e8d1345b3cba4 100644 --- a/homeassistant/components/overkiz/coordinator.py +++ b/homeassistant/components/overkiz/coordinator.py @@ -136,12 +136,7 @@ async def on_device_state_changed(self, event: Event) -> None: for state in event.device_states: device = self.devices[event.device_url] - - if (device_state := device.states[state.name]) is None: - device_state = state - device.states[state.name] = device_state - - device_state.value = state.value + device.states[state.name] = state @EVENT_HANDLERS.register(EventName.DEVICE_REMOVED) async def on_device_removed(self, event: Event) -> None: From bbe8b8d65708298966c3eb1b19fc0c9568a56c74 Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Thu, 13 Jan 2022 10:00:09 +0000 Subject: [PATCH 4/7] Fix pylint --- homeassistant/components/overkiz/coordinator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/overkiz/coordinator.py b/homeassistant/components/overkiz/coordinator.py index e8d1345b3cba4..bfecdcd5a46c1 100644 --- a/homeassistant/components/overkiz/coordinator.py +++ b/homeassistant/components/overkiz/coordinator.py @@ -101,7 +101,7 @@ async def _async_update_data(self) -> dict[str, Device]: async def _get_devices(self) -> dict[str, Device]: """Fetch devices.""" - _LOGGER.debug("Fetching all devices and state via /setup/devices.") + _LOGGER.debug("Fetching all devices and state via /setup/devices") return {d.device_url: d for d in await self.client.get_devices(refresh=True)} def _places_to_area(self, place: Place) -> dict[str, str]: From 9140767977b4816b6b39506df0a57e7e153fc29f Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Thu, 13 Jan 2022 11:37:06 +0000 Subject: [PATCH 5/7] Move event handlers to module level --- .../components/overkiz/coordinator.py | 126 ++++++++++-------- 1 file changed, 72 insertions(+), 54 deletions(-) diff --git a/homeassistant/components/overkiz/coordinator.py b/homeassistant/components/overkiz/coordinator.py index bfecdcd5a46c1..56854b4b6ae20 100644 --- a/homeassistant/components/overkiz/coordinator.py +++ b/homeassistant/components/overkiz/coordinator.py @@ -116,57 +116,75 @@ def _places_to_area(self, place: Place) -> dict[str, str]: return areas - @EVENT_HANDLERS.register(EventName.DEVICE_AVAILABLE) - async def on_device_available(self, event: Event) -> None: - """Handle device available event.""" - if event.device_url: - self.devices[event.device_url].available = True - - @EVENT_HANDLERS.register((EventName.DEVICE_UNAVAILABLE, EventName.DEVICE_DISABLED)) - async def on_device_unavailable_disabled(self, event: Event) -> None: - """Handle device unavailable / disabled event.""" - if event.device_url: - self.devices[event.device_url].available = False - - @EVENT_HANDLERS.register(EventName.DEVICE_STATE_CHANGED) - async def on_device_state_changed(self, event: Event) -> None: - """Handle device state changed event.""" - if not event.device_url: - return - - for state in event.device_states: - device = self.devices[event.device_url] - device.states[state.name] = state - - @EVENT_HANDLERS.register(EventName.DEVICE_REMOVED) - async def on_device_removed(self, event: Event) -> None: - """Handle device removed event.""" - if not event.device_url: - return - - base_device_url = event.device_url.split("#")[0] - registry = dr.async_get(self.hass) - - if registered_device := registry.async_get_device({(DOMAIN, base_device_url)}): - registry.async_remove_device(registered_device.id) - - if event.device_url: - del self.devices[event.device_url] - - @EVENT_HANDLERS.register(EventName.EXECUTION_REGISTERED) - async def on_execution_registered(self, event: Event) -> None: - """Handle execution registered event.""" - if event.exec_id and event.exec_id not in self.executions: - self.executions[event.exec_id] = {} - - if not self.is_stateless: - self.update_interval = timedelta(seconds=1) - - @EVENT_HANDLERS.register(EventName.EXECUTION_STATE_CHANGED) - async def on_execution_state_changed(self, event: Event) -> None: - """Handle execution changed event.""" - if event.exec_id in self.executions and event.new_state in [ - ExecutionState.COMPLETED, - ExecutionState.FAILED, - ]: - del self.executions[event.exec_id] + +@EVENT_HANDLERS.register(EventName.DEVICE_AVAILABLE) +async def on_device_available( + coordinator: OverkizDataUpdateCoordinator, event: Event +) -> None: + """Handle device available event.""" + if event.device_url: + coordinator.devices[event.device_url].available = True + + +@EVENT_HANDLERS.register((EventName.DEVICE_UNAVAILABLE, EventName.DEVICE_DISABLED)) +async def on_device_unavailable_disabled( + coordinator: OverkizDataUpdateCoordinator, event: Event +) -> None: + """Handle device unavailable / disabled event.""" + if event.device_url: + coordinator.devices[event.device_url].available = False + + +@EVENT_HANDLERS.register(EventName.DEVICE_STATE_CHANGED) +async def on_device_state_changed( + coordinator: OverkizDataUpdateCoordinator, event: Event +) -> None: + """Handle device state changed event.""" + if not event.device_url: + return + + for state in event.device_states: + device = coordinator.devices[event.device_url] + device.states[state.name] = state + + +@EVENT_HANDLERS.register(EventName.DEVICE_REMOVED) +async def on_device_removed( + coordinator: OverkizDataUpdateCoordinator, event: Event +) -> None: + """Handle device removed event.""" + if not event.device_url: + return + + base_device_url = event.device_url.split("#")[0] + registry = dr.async_get(coordinator.hass) + + if registered_device := registry.async_get_device({(DOMAIN, base_device_url)}): + registry.async_remove_device(registered_device.id) + + if event.device_url: + del coordinator.devices[event.device_url] + + +@EVENT_HANDLERS.register(EventName.EXECUTION_REGISTERED) +async def on_execution_registered( + coordinator: OverkizDataUpdateCoordinator, event: Event +) -> None: + """Handle execution registered event.""" + if event.exec_id and event.exec_id not in coordinator.executions: + coordinator.executions[event.exec_id] = {} + + if not coordinator.is_stateless: + coordinator.update_interval = timedelta(seconds=1) + + +@EVENT_HANDLERS.register(EventName.EXECUTION_STATE_CHANGED) +async def on_execution_state_changed( + coordinator: OverkizDataUpdateCoordinator, event: Event +) -> None: + """Handle execution changed event.""" + if event.exec_id in coordinator.executions and event.new_state in [ + ExecutionState.COMPLETED, + ExecutionState.FAILED, + ]: + del coordinator.executions[event.exec_id] From e1f13a79d65d6c97e776741f871c07d3d200184c Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Thu, 13 Jan 2022 12:06:22 +0000 Subject: [PATCH 6/7] Address feedback --- homeassistant/components/overkiz/coordinator.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/overkiz/coordinator.py b/homeassistant/components/overkiz/coordinator.py index 56854b4b6ae20..34d52c2b423fc 100644 --- a/homeassistant/components/overkiz/coordinator.py +++ b/homeassistant/components/overkiz/coordinator.py @@ -126,7 +126,8 @@ async def on_device_available( coordinator.devices[event.device_url].available = True -@EVENT_HANDLERS.register((EventName.DEVICE_UNAVAILABLE, EventName.DEVICE_DISABLED)) +@EVENT_HANDLERS.register(EventName.DEVICE_UNAVAILABLE) +@EVENT_HANDLERS.register(EventName.DEVICE_DISABLED) async def on_device_unavailable_disabled( coordinator: OverkizDataUpdateCoordinator, event: Event ) -> None: @@ -135,6 +136,17 @@ async def on_device_unavailable_disabled( coordinator.devices[event.device_url].available = False +@EVENT_HANDLERS.register(EventName.DEVICE_CREATED) +@EVENT_HANDLERS.register(EventName.DEVICE_UPDATED) +async def on_device_created_updated( + coordinator: OverkizDataUpdateCoordinator, event: Event +) -> None: + """Handle device unavailable / disabled event.""" + coordinator.hass.async_create_task( + coordinator.hass.config_entries.async_reload(coordinator._config_entry_id) + ) + + @EVENT_HANDLERS.register(EventName.DEVICE_STATE_CHANGED) async def on_device_state_changed( coordinator: OverkizDataUpdateCoordinator, event: Event From 1aed36fd8b4d5a91e312650e23923f009411fa3b Mon Sep 17 00:00:00 2001 From: Mick Vleeshouwer Date: Thu, 13 Jan 2022 12:21:45 +0000 Subject: [PATCH 7/7] Make config_entry_id not protected --- homeassistant/components/overkiz/coordinator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/overkiz/coordinator.py b/homeassistant/components/overkiz/coordinator.py index 34d52c2b423fc..3d3121ab134c4 100644 --- a/homeassistant/components/overkiz/coordinator.py +++ b/homeassistant/components/overkiz/coordinator.py @@ -60,7 +60,7 @@ def __init__( ) self.executions: dict[str, dict[str, str]] = {} self.areas = self._places_to_area(places) - self._config_entry_id = config_entry_id + self.config_entry_id = config_entry_id async def _async_update_data(self) -> dict[str, Device]: """Fetch Overkiz data via event listener.""" @@ -143,7 +143,7 @@ async def on_device_created_updated( ) -> None: """Handle device unavailable / disabled event.""" coordinator.hass.async_create_task( - coordinator.hass.config_entries.async_reload(coordinator._config_entry_id) + coordinator.hass.config_entries.async_reload(coordinator.config_entry_id) )