From e690f6e206caab7d4ec0ffc47d773a295f122a6d Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Thu, 8 Mar 2018 04:13:24 +0100 Subject: [PATCH 1/7] Add websocket-based non-polling variant for songpal --- .../components/media_player/songpal.py | 123 +++++++++++++++--- requirements_all.txt | 2 +- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/media_player/songpal.py b/homeassistant/components/media_player/songpal.py index 83b10997c314eb..33c0785b17a925 100644 --- a/homeassistant/components/media_player/songpal.py +++ b/homeassistant/components/media_player/songpal.py @@ -4,7 +4,9 @@ For more details about this platform, please refer to the documentation at https://home-assistant.io/components/media_player.songpal/ """ +import asyncio import logging +from collections import OrderedDict import voluptuous as vol @@ -12,15 +14,17 @@ DOMAIN, PLATFORM_SCHEMA, SUPPORT_SELECT_SOURCE, SUPPORT_TURN_OFF, SUPPORT_TURN_ON, SUPPORT_VOLUME_MUTE, SUPPORT_VOLUME_SET, SUPPORT_VOLUME_STEP, MediaPlayerDevice) -from homeassistant.const import ATTR_ENTITY_ID, CONF_NAME, STATE_OFF, STATE_ON +from homeassistant.const import ( + ATTR_ENTITY_ID, CONF_NAME, STATE_OFF, STATE_ON, EVENT_HOMEASSISTANT_STOP) from homeassistant.exceptions import PlatformNotReady import homeassistant.helpers.config_validation as cv -REQUIREMENTS = ['python-songpal==0.0.8'] +REQUIREMENTS = ['python-songpal==0.0.9'] _LOGGER = logging.getLogger(__name__) CONF_ENDPOINT = 'endpoint' +CONF_POLL = 'poll' PARAM_NAME = 'name' PARAM_VALUE = 'value' @@ -36,6 +40,7 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ vol.Optional(CONF_NAME): cv.string, vol.Required(CONF_ENDPOINT): cv.string, + vol.Optional(CONF_POLL, default=False): cv.boolean, }) SET_SOUND_SCHEMA = vol.Schema({ @@ -62,7 +67,12 @@ async def async_setup_platform( else: name = config.get(CONF_NAME) endpoint = config.get(CONF_ENDPOINT) - device = SongpalDevice(name, endpoint) + poll = config.get(CONF_POLL) + device = SongpalDevice(name, endpoint, poll) + + if endpoint in hass.data[PLATFORM]: + _LOGGER.debug("The endpoint exists already, skipping setup.") + return try: await device.initialize() @@ -96,12 +106,13 @@ async def async_service_handler(service): class SongpalDevice(MediaPlayerDevice): """Class representing a Songpal device.""" - def __init__(self, name, endpoint): + def __init__(self, name, endpoint, poll): """Init.""" - import songpal + from songpal import Device self._name = name - self.endpoint = endpoint - self.dev = songpal.Device(self.endpoint) + self._endpoint = endpoint + self._poll = poll + self.dev = Device(self._endpoint) self._sysinfo = None self._state = False @@ -114,13 +125,82 @@ def __init__(self, name, endpoint): self._volume = 0 self._is_muted = False - self._sources = [] + self._active_source = None + self._sources = {} + + @property + def should_poll(self): + return self._poll async def initialize(self): """Initialize the device.""" await self.dev.get_supported_methods() self._sysinfo = await self.dev.get_system_info() + async def async_activate_websocket(self): + """Activate websocket for listening if wanted.""" + _LOGGER.info("Activating websocket connection..") + from songpal import (VolumeChange, ContentChange, + PowerChange, ConnectChange) + + async def volume_changed(x: VolumeChange): + """Volume changed callback.""" + _LOGGER.debug("Volume changed: %s", x) + self._volume = x.volume + self._is_muted = x.mute + await self.async_update_ha_state() + + async def source_changed(x: ContentChange): + """Source changed callback.""" + _LOGGER.debug("Source changed: %s", x) + if x.is_input: + self._active_source = self._sources[x.source] + _LOGGER.debug("New active source: %s", self._active_source) + await self.async_update_ha_state() + else: + _LOGGER.warning("Got non-handled content change: %s" % x) + + async def power_changed(x: PowerChange): + """Power changed callback.""" + _LOGGER.debug("Power changed: %s", x) + self._state = x.status + await self.async_update_ha_state() + + async def try_reconnect(x: ConnectChange): + """Callback to reconnect back.""" + _LOGGER.error("Got disconnected with %s, trying to reconnect.", + x.exception) + self._available = False + self.dev.clear_notification_callbacks() + await self.async_update_ha_state() + + # Try to reconnect forever, a successful reconnect will initialize + # the websocket connection again. + while not self._available: + await asyncio.sleep(10) + await self.async_update() + # We need to inform HA about the state in case we are coming + # back from a disconnected state. + await self.async_update_ha_state() + + self.dev.on_notification(VolumeChange, volume_changed) + self.dev.on_notification(ContentChange, source_changed) + self.dev.on_notification(PowerChange, power_changed) + self.dev.on_notification(ConnectChange, try_reconnect) + + async def listen_events(): + await self.dev.listen_notifications() + + async def handle_stop(event): + nonlocal remove_hass_stop_listener + remove_hass_stop_listener = None + await self.dev.stop_listen_notifications() + + remove_hass_stop_listener = self.hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_STOP, handle_stop) + + self.hass.loop.create_task(listen_events()) + @property def name(self): """Return name of the device.""" @@ -169,18 +249,27 @@ async def async_update(self): inputs = await self.dev.get_inputs() _LOGGER.debug("Got ins: %s", inputs) - self._sources = inputs + + self._sources = OrderedDict() + for input in inputs: + self._sources[input.uri] = input + if input.active: + self._active_source = input + + _LOGGER.debug("Active source: %s", self._active_source) self._available = True + + # activate notifications if wanted + if not self._poll: + await self.hass.async_add_job(self.async_activate_websocket) except SongpalException as ex: - # if we were available, print out the exception - if self._available: - _LOGGER.error("Got an exception: %s", ex) + _LOGGER.error("Unable to update: %s", ex) self._available = False async def async_select_source(self, source): """Select source.""" - for out in self._sources: + for out in self._sources.values(): if out.title == source: await out.activate() return @@ -190,7 +279,7 @@ async def async_select_source(self, source): @property def source_list(self): """Return list of available sources.""" - return [x.title for x in self._sources] + return [src.title for src in self._sources.values()] @property def state(self): @@ -202,11 +291,7 @@ def state(self): @property def source(self): """Return currently active source.""" - for out in self._sources: - if out.active: - return out.title - - return None + return self._active_source.title @property def volume_level(self): diff --git a/requirements_all.txt b/requirements_all.txt index aa2e52e13ba650..d6580713dd7488 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1256,7 +1256,7 @@ python-roku==3.1.5 python-sochain-api==0.0.2 # homeassistant.components.media_player.songpal -python-songpal==0.0.8 +python-songpal==0.0.9 # homeassistant.components.sensor.synologydsm python-synology==0.2.0 From 281f21002478b27211aa414d7b6a897865fe2ac5 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Sat, 8 Dec 2018 23:50:43 +0100 Subject: [PATCH 2/7] linting fixes --- .../components/media_player/songpal.py | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/homeassistant/components/media_player/songpal.py b/homeassistant/components/media_player/songpal.py index 33c0785b17a925..fbc804c73f054f 100644 --- a/homeassistant/components/media_player/songpal.py +++ b/homeassistant/components/media_player/songpal.py @@ -106,7 +106,7 @@ async def async_service_handler(service): class SongpalDevice(MediaPlayerDevice): """Class representing a Songpal device.""" - def __init__(self, name, endpoint, poll): + def __init__(self, name, endpoint, poll=False): """Init.""" from songpal import Device self._name = name @@ -130,6 +130,7 @@ def __init__(self, name, endpoint, poll): @property def should_poll(self): + """Return True if the device should be polled.""" return self._poll async def initialize(self): @@ -143,31 +144,28 @@ async def async_activate_websocket(self): from songpal import (VolumeChange, ContentChange, PowerChange, ConnectChange) - async def volume_changed(x: VolumeChange): - """Volume changed callback.""" - _LOGGER.debug("Volume changed: %s", x) - self._volume = x.volume - self._is_muted = x.mute + async def _volume_changed(volume: VolumeChange): + _LOGGER.debug("Volume changed: %s", volume) + self._volume = volume.volume + self._is_muted = volume.mute await self.async_update_ha_state() - async def source_changed(x: ContentChange): - """Source changed callback.""" - _LOGGER.debug("Source changed: %s", x) - if x.is_input: - self._active_source = self._sources[x.source] + async def _source_changed(content: ContentChange): + _LOGGER.debug("Source changed: %s", content) + if content.is_input: + self._active_source = self._sources[content.source] _LOGGER.debug("New active source: %s", self._active_source) await self.async_update_ha_state() else: - _LOGGER.warning("Got non-handled content change: %s" % x) + _LOGGER.warning("Got non-handled content change: %s", + content) - async def power_changed(x: PowerChange): - """Power changed callback.""" - _LOGGER.debug("Power changed: %s", x) - self._state = x.status + async def _power_changed(power: PowerChange): + _LOGGER.debug("Power changed: %s", power) + self._state = power.status await self.async_update_ha_state() - async def try_reconnect(x: ConnectChange): - """Callback to reconnect back.""" + async def _try_reconnect(x: ConnectChange): _LOGGER.error("Got disconnected with %s, trying to reconnect.", x.exception) self._available = False @@ -183,10 +181,10 @@ async def try_reconnect(x: ConnectChange): # back from a disconnected state. await self.async_update_ha_state() - self.dev.on_notification(VolumeChange, volume_changed) - self.dev.on_notification(ContentChange, source_changed) - self.dev.on_notification(PowerChange, power_changed) - self.dev.on_notification(ConnectChange, try_reconnect) + self.dev.on_notification(VolumeChange, _volume_changed) + self.dev.on_notification(ContentChange, _source_changed) + self.dev.on_notification(PowerChange, _power_changed) + self.dev.on_notification(ConnectChange, _try_reconnect) async def listen_events(): await self.dev.listen_notifications() @@ -251,10 +249,10 @@ async def async_update(self): _LOGGER.debug("Got ins: %s", inputs) self._sources = OrderedDict() - for input in inputs: - self._sources[input.uri] = input - if input.active: - self._active_source = input + for input_ in inputs: + self._sources[input_.uri] = input_ + if input_.active: + self._active_source = input_ _LOGGER.debug("Active source: %s", self._active_source) From 40b70617283d564fb2ea4fa5d97233f5b6f9bda1 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Sun, 9 Dec 2018 15:28:50 +0100 Subject: [PATCH 3/7] changes based on Martin's feedback --- homeassistant/components/media_player/songpal.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/media_player/songpal.py b/homeassistant/components/media_player/songpal.py index fbc804c73f054f..5849efa7670aa0 100644 --- a/homeassistant/components/media_player/songpal.py +++ b/homeassistant/components/media_player/songpal.py @@ -176,10 +176,9 @@ async def _try_reconnect(x: ConnectChange): # the websocket connection again. while not self._available: await asyncio.sleep(10) - await self.async_update() # We need to inform HA about the state in case we are coming # back from a disconnected state. - await self.async_update_ha_state() + await self.async_update_ha_state(force_refresh=True) self.dev.on_notification(VolumeChange, _volume_changed) self.dev.on_notification(ContentChange, _source_changed) @@ -190,12 +189,9 @@ async def listen_events(): await self.dev.listen_notifications() async def handle_stop(event): - nonlocal remove_hass_stop_listener - remove_hass_stop_listener = None await self.dev.stop_listen_notifications() - remove_hass_stop_listener = self.hass.bus.async_listen_once( - EVENT_HOMEASSISTANT_STOP, handle_stop) + self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, handle_stop) self.hass.loop.create_task(listen_events()) @@ -260,7 +256,7 @@ async def async_update(self): # activate notifications if wanted if not self._poll: - await self.hass.async_add_job(self.async_activate_websocket) + await self.hass.async_create_task(self.async_activate_websocket()) except SongpalException as ex: _LOGGER.error("Unable to update: %s", ex) self._available = False From 4e0d5300cfd3f6fae67fca999a6e3ebfde908011 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Sun, 9 Dec 2018 15:56:15 +0100 Subject: [PATCH 4/7] Fix linting --- homeassistant/components/media_player/songpal.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/media_player/songpal.py b/homeassistant/components/media_player/songpal.py index 5849efa7670aa0..152a0c81da60cd 100644 --- a/homeassistant/components/media_player/songpal.py +++ b/homeassistant/components/media_player/songpal.py @@ -256,7 +256,8 @@ async def async_update(self): # activate notifications if wanted if not self._poll: - await self.hass.async_create_task(self.async_activate_websocket()) + await self.hass.async_create_task( + self.async_activate_websocket()) except SongpalException as ex: _LOGGER.error("Unable to update: %s", ex) self._available = False From 55c7c4667f4c76ad6a23534b157aed96f300961b Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Sun, 9 Dec 2018 16:38:13 +0100 Subject: [PATCH 5/7] add backoff timer for reconnects, fix variable naming (I thought that this wouldn't matter for internals..) --- homeassistant/components/media_player/songpal.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/media_player/songpal.py b/homeassistant/components/media_player/songpal.py index 152a0c81da60cd..79b0f90b8212de 100644 --- a/homeassistant/components/media_player/songpal.py +++ b/homeassistant/components/media_player/songpal.py @@ -165,20 +165,23 @@ async def _power_changed(power: PowerChange): self._state = power.status await self.async_update_ha_state() - async def _try_reconnect(x: ConnectChange): + async def _try_reconnect(connect: ConnectChange): _LOGGER.error("Got disconnected with %s, trying to reconnect.", - x.exception) + connect.exception) self._available = False self.dev.clear_notification_callbacks() await self.async_update_ha_state() # Try to reconnect forever, a successful reconnect will initialize # the websocket connection again. + delay = 10 while not self._available: - await asyncio.sleep(10) + _LOGGER.debug("Trying to reconnect in %s seconds", delay) + await asyncio.sleep(delay) # We need to inform HA about the state in case we are coming # back from a disconnected state. await self.async_update_ha_state(force_refresh=True) + delay = min(2*delay, 300) self.dev.on_notification(VolumeChange, _volume_changed) self.dev.on_notification(ContentChange, _source_changed) From 086ff9c8d825b448253d98a869975bed179f59fa Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Sun, 9 Dec 2018 16:46:14 +0100 Subject: [PATCH 6/7] Remove poll configuration variable --- homeassistant/components/media_player/songpal.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/homeassistant/components/media_player/songpal.py b/homeassistant/components/media_player/songpal.py index 79b0f90b8212de..a08e571214e1e5 100644 --- a/homeassistant/components/media_player/songpal.py +++ b/homeassistant/components/media_player/songpal.py @@ -24,7 +24,6 @@ _LOGGER = logging.getLogger(__name__) CONF_ENDPOINT = 'endpoint' -CONF_POLL = 'poll' PARAM_NAME = 'name' PARAM_VALUE = 'value' @@ -40,7 +39,6 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ vol.Optional(CONF_NAME): cv.string, vol.Required(CONF_ENDPOINT): cv.string, - vol.Optional(CONF_POLL, default=False): cv.boolean, }) SET_SOUND_SCHEMA = vol.Schema({ @@ -67,8 +65,7 @@ async def async_setup_platform( else: name = config.get(CONF_NAME) endpoint = config.get(CONF_ENDPOINT) - poll = config.get(CONF_POLL) - device = SongpalDevice(name, endpoint, poll) + device = SongpalDevice(name, endpoint, poll=False) if endpoint in hass.data[PLATFORM]: _LOGGER.debug("The endpoint exists already, skipping setup.") From 8ee828a6e27800f95a64d737364fd143caf54fb5 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Sun, 9 Dec 2018 21:28:43 +0100 Subject: [PATCH 7/7] bump the version just to be sure, the previous release lacked a version file (required for setup.py) --- homeassistant/components/media_player/songpal.py | 2 +- requirements_all.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/media_player/songpal.py b/homeassistant/components/media_player/songpal.py index a08e571214e1e5..c79470a82fefe6 100644 --- a/homeassistant/components/media_player/songpal.py +++ b/homeassistant/components/media_player/songpal.py @@ -19,7 +19,7 @@ from homeassistant.exceptions import PlatformNotReady import homeassistant.helpers.config_validation as cv -REQUIREMENTS = ['python-songpal==0.0.9'] +REQUIREMENTS = ['python-songpal==0.0.9.1'] _LOGGER = logging.getLogger(__name__) diff --git a/requirements_all.txt b/requirements_all.txt index d6580713dd7488..95b335bc8cbe77 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1256,7 +1256,7 @@ python-roku==3.1.5 python-sochain-api==0.0.2 # homeassistant.components.media_player.songpal -python-songpal==0.0.9 +python-songpal==0.0.9.1 # homeassistant.components.sensor.synologydsm python-synology==0.2.0