From 1836baf4ec7c8d4f6056925493fd0a7949ad7faf Mon Sep 17 00:00:00 2001 From: Tom Harris Date: Wed, 21 Feb 2018 00:09:41 -0500 Subject: [PATCH] Changes per code review --- .../components/binary_sensor/insteon_plm.py | 35 ++++++----- homeassistant/components/fan/insteon_plm.py | 43 ++++++------- homeassistant/components/insteon_plm.py | 18 ++---- homeassistant/components/light/insteon_plm.py | 31 +++++----- .../components/sensor/insteon_plm.py | 34 ++++++----- .../components/switch/insteon_plm.py | 60 ++++++++++--------- 6 files changed, 114 insertions(+), 107 deletions(-) diff --git a/homeassistant/components/binary_sensor/insteon_plm.py b/homeassistant/components/binary_sensor/insteon_plm.py index ecb688893b4..6ce9538060a 100644 --- a/homeassistant/components/binary_sensor/insteon_plm.py +++ b/homeassistant/components/binary_sensor/insteon_plm.py @@ -2,10 +2,10 @@ Support for INSTEON dimmers via PowerLinc Modem. For more details about this component, please refer to the documentation at -https://home-assistant.io/components/insteon_plm/ +https://home-assistant.io/components/binary_sensor.insteon_plm/ """ -import logging import asyncio +import logging from homeassistant.core import callback from homeassistant.components.binary_sensor import BinarySensorDevice @@ -24,7 +24,7 @@ SENSOR_TYPES = {'openClosedSensor': 'opening', @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Set up the INSTEON PLM device class for the hass platform.""" - state_list = [] + entities = [] plm = hass.data['insteon_plm'] for device_info in discovery_info: @@ -32,27 +32,21 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): device = plm.devices[address] state_key = device_info['state_key'] - state_list.append(InsteonPLMBinarySensor(hass, - device, - state_key)) + entities.append(InsteonPLMBinarySensor(device, state_key)) - async_add_devices(state_list) + async_add_devices(entities) class InsteonPLMBinarySensor(BinarySensorDevice): - """A Class for an Insteon device state.""" + """A Class for an Insteon device entity.""" - def __init__(self, hass, device, state_key): + def __init__(self, device, state_key): """Initialize the INSTEON PLM binary sensor.""" - self._hass = hass self._insteon_device_state = device.states[state_key] self._insteon_device = device self._sensor_type = SENSOR_TYPES.get(self._insteon_device_state.name, None) - self._insteon_device_state.register_updates( - self.async_binarysensor_update) - @property def should_poll(self): """No polling needed.""" @@ -63,6 +57,10 @@ class InsteonPLMBinarySensor(BinarySensorDevice): """Return the address of the node.""" return self._insteon_device.address.human + def group(self): + """Return the INSTEON group that the entity responds to.""" + return self._insteon_device_state.group + @property def name(self): """Return the name of the node (used for Entity_ID).""" @@ -78,13 +76,12 @@ class InsteonPLMBinarySensor(BinarySensorDevice): def device_state_attributes(self): """Provide attributes for display on device card.""" insteon_plm = get_component('insteon_plm') - return insteon_plm.common_attributes(self._insteon_device, - self._insteon_device_state) + return insteon_plm.common_attributes(self) @callback def async_binarysensor_update(self, deviceid, statename, val): """Receive notification from transport that new data exists.""" - self._hass.async_add_job(self.async_update_ha_state()) + self.async_schedule_update_ha_state() @property def device_class(self): @@ -96,3 +93,9 @@ class InsteonPLMBinarySensor(BinarySensorDevice): """Return the boolean response if the node is on.""" sensorstate = self._insteon_device_state.value return bool(sensorstate) + + @asyncio.coroutine + def async_added_to_hass(self): + """Register INSTEON update events.""" + self._insteon_device_state.register_updates( + self.async_binarysensor_update) diff --git a/homeassistant/components/fan/insteon_plm.py b/homeassistant/components/fan/insteon_plm.py index 2174f6aeef5..87964885b01 100644 --- a/homeassistant/components/fan/insteon_plm.py +++ b/homeassistant/components/fan/insteon_plm.py @@ -2,10 +2,10 @@ Support for INSTEON fans via PowerLinc Modem. For more details about this component, please refer to the documentation at -https://home-assistant.io/components/insteon_plm/ +https://home-assistant.io/components/fan.insteon_plm/ """ -import logging import asyncio +import logging from homeassistant.core import callback from homeassistant.components.fan import (SPEED_OFF, @@ -24,41 +24,36 @@ SPEED_TO_HEX = {SPEED_OFF: 0x00, SPEED_MEDIUM: 0xbe, SPEED_HIGH: 0xff} +FAN_SPEEDS = [STATE_OFF, SPEED_LOW, SPEED_MEDIUM, SPEED_HIGH] + _LOGGER = logging.getLogger(__name__) @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Set up the INSTEON PLM device class for the hass platform.""" - state_list = [] + entities = [] plm = hass.data['insteon_plm'] - for device_info in discovery_info: - address = device_info['address'] - device = plm.devices[address] - state_key = device_info['state_key'] + address = device_info['address'] + device = plm.devices[address] + state_key = device_info['state_key'] - state_list.append(InsteonPLMFan(hass, - device, - state_key, - SUPPORT_SET_SPEED)) + entities.append(InsteonPLMFan(device, state_key, SUPPORT_SET_SPEED)) - async_add_devices(state_list) + async_add_devices(entities) class InsteonPLMFan(FanEntity): """An INSTEON fan component.""" - def __init__(self, hass, device, state_key, + def __init__(self, device, state_key, supported_features: int, ) -> None: """Initialize the entity.""" - self._hass = hass self._insteon_device_state = device.states[state_key] self._insteon_device = device self._supported_features = supported_features - self._insteon_device_state.register_updates(self.async_fan_update) - @property def should_poll(self): """No polling needed.""" @@ -69,6 +64,10 @@ class InsteonPLMFan(FanEntity): """Return the address of the node.""" return self._insteon_device.address.human + def group(self): + """Return the INSTEON group that the entity responds to.""" + return self._insteon_device_state.group + @property def name(self): """Return the name of the node (used for Entity_ID).""" @@ -84,8 +83,7 @@ class InsteonPLMFan(FanEntity): def device_state_attributes(self): """Provide attributes for display on device card.""" insteon_plm = get_component('insteon_plm') - return insteon_plm.common_attributes(self._insteon_device, - self._insteon_device_state) + return insteon_plm.common_attributes(self) @property def speed(self) -> str: @@ -95,7 +93,7 @@ class InsteonPLMFan(FanEntity): @property def speed_list(self) -> list: """Get the list of available speeds.""" - return [STATE_OFF, SPEED_LOW, SPEED_MEDIUM, SPEED_HIGH] + return FAN_SPEEDS def async_turn_on(self, speed: str = None, **kwargs) -> None: """Turn on the entity.""" @@ -118,7 +116,7 @@ class InsteonPLMFan(FanEntity): @callback def async_fan_update(self, deviceid, statename, val): """Receive notification from transport that new data exists.""" - self.hass.async_add_job(self.async_update_ha_state()) + self.async_schedule_update_ha_state() @property def supported_features(self) -> int: @@ -135,3 +133,8 @@ class InsteonPLMFan(FanEntity): elif speed > 0: hex_speed = SPEED_LOW return hex_speed + + @asyncio.coroutine + def async_added_to_hass(self): + """Register INSTEON update events.""" + self._insteon_device_state.register_updates(self.async_fan_update) diff --git a/homeassistant/components/insteon_plm.py b/homeassistant/components/insteon_plm.py index 5c3a4bee769..edf2b9ba4d4 100644 --- a/homeassistant/components/insteon_plm.py +++ b/homeassistant/components/insteon_plm.py @@ -4,14 +4,13 @@ Support for INSTEON PowerLinc Modem. For more details about this component, please refer to the documentation at https://home-assistant.io/components/insteon_plm/ """ -import logging import asyncio import collections - +import logging import voluptuous as vol from homeassistant.core import callback -from homeassistant.const import (CONF_PORT, EVENT_HOMEASSISTANT_STOP) +from homeassistant.const import CONF_PORT, EVENT_HOMEASSISTANT_STOP import homeassistant.helpers.config_validation as cv from homeassistant.helpers import discovery @@ -58,8 +57,8 @@ def async_setup(hass, config): hass.async_add_job( discovery.async_load_platform( hass, platform, DOMAIN, - discovered=[{'address': device.address.hex, - 'state_key': state_key}], + discovered={'address': device.address.hex, + 'state_key': state_key}, hass_config=config)) _LOGGER.info("Looking for PLM on %s", port) @@ -102,16 +101,11 @@ def async_setup(hass, config): return True -def common_attributes(entity, state): +def common_attributes(entity): """Return the device state attributes.""" attributes = { 'INSTEON Address': entity.address.human, - 'Description': entity.description, - 'Model': entity.model, - 'Category': '{:02x}'.format(entity.cat), - 'Subcategory': '{:02x}'.format(entity.subcat), - 'Product Key / Firmware': '{:02x}'.format(entity.product_key), - 'Group': '{:02x}'.format(state.group) + 'INSTEON Group': entity.group } return attributes diff --git a/homeassistant/components/light/insteon_plm.py b/homeassistant/components/light/insteon_plm.py index 7fde9b36a5b..84968ec01a9 100644 --- a/homeassistant/components/light/insteon_plm.py +++ b/homeassistant/components/light/insteon_plm.py @@ -2,10 +2,10 @@ Support for Insteon lights via PowerLinc Modem. For more details about this component, please refer to the documentation at -https://home-assistant.io/components/insteon_plm/ +https://home-assistant.io/components/light.insteon_plm/ """ -import logging import asyncio +import logging from homeassistant.core import callback from homeassistant.components.light import ( @@ -22,7 +22,7 @@ MAX_BRIGHTNESS = 255 @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Set up the Insteon PLM device.""" - state_list = [] + entities = [] plm = hass.data['insteon_plm'] for device_info in discovery_info: @@ -30,24 +30,19 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): device = plm.devices[address] state_key = device_info['state_key'] - state_list.append(InsteonPLMDimmerDevice(hass, - device, - state_key)) + entities.append(InsteonPLMDimmerDevice(device, state_key)) - async_add_devices(state_list) + async_add_devices(entities) class InsteonPLMDimmerDevice(Light): """A Class for an Insteon device.""" - def __init__(self, hass, device, state_key): + def __init__(self, device, state_key): """Initialize the light.""" - self._hass = hass self._insteon_device_state = device.states[state_key] self._insteon_device = device - self._insteon_device_state.register_updates(self.async_light_update) - @property def should_poll(self): """No polling needed.""" @@ -58,6 +53,10 @@ class InsteonPLMDimmerDevice(Light): """Return the address of the node.""" return self._insteon_device.address.human + def group(self): + """Return the INSTEON group that the entity responds to.""" + return self._insteon_device_state.group + @property def name(self): """Return the name of the node (used for Entity_ID).""" @@ -89,13 +88,12 @@ class InsteonPLMDimmerDevice(Light): def device_state_attributes(self): """Provide attributes for display on device card.""" insteon_plm = get_component('insteon_plm') - return insteon_plm.common_attributes(self._insteon_device, - self._insteon_device_state) + return insteon_plm.common_attributes(self) @callback def async_light_update(self, entity_id, statename, val): """Receive notification from transport that new data exists.""" - self._hass.async_add_job(self.async_update_ha_state()) + self.async_schedule_update_ha_state() @asyncio.coroutine def async_turn_on(self, **kwargs): @@ -110,3 +108,8 @@ class InsteonPLMDimmerDevice(Light): def async_turn_off(self, **kwargs): """Turn device off.""" self._insteon_device_state.off() + + @asyncio.coroutine + def async_added_to_hass(self): + """Register INSTEON update events.""" + self._insteon_device_state.register_updates(self.async_light_update) diff --git a/homeassistant/components/sensor/insteon_plm.py b/homeassistant/components/sensor/insteon_plm.py index 7d2f4474d3a..413adfda855 100644 --- a/homeassistant/components/sensor/insteon_plm.py +++ b/homeassistant/components/sensor/insteon_plm.py @@ -2,10 +2,10 @@ Support for INSTEON dimmers via PowerLinc Modem. For more details about this component, please refer to the documentation at -https://home-assistant.io/components/insteon_plm/ +https://home-assistant.io/components/sensor.insteon_plm/ """ -import logging import asyncio +import logging from homeassistant.core import callback from homeassistant.helpers.entity import Entity @@ -19,7 +19,7 @@ _LOGGER = logging.getLogger(__name__) @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Set up the INSTEON PLM device class for the hass platform.""" - state_list = [] + entities = [] plm = hass.data['insteon_plm'] for device_info in discovery_info: @@ -27,24 +27,19 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): device = plm.devices[address] state_key = device_info['state_key'] - state_list.append(InsteonPLMSensorDevice(hass, - device, - state_key)) + entities.append(InsteonPLMSensorDevice(device, state_key)) - async_add_devices(state_list) + async_add_devices(entities) class InsteonPLMSensorDevice(Entity): """A Class for an Insteon device.""" - def __init__(self, hass, device, state_key): + def __init__(self, device, state_key): """Initialize the binarysensor.""" - self._hass = hass self._insteon_device_state = device.states[state_key] self._insteon_device = device - self._insteon_device_state.register_updates(self.async_sensor_update) - @property def should_poll(self): """No polling needed.""" @@ -55,6 +50,10 @@ class InsteonPLMSensorDevice(Entity): """Return the address of the node.""" return self._insteon_device.address.human + def group(self): + """Return the INSTEON group that the entity responds to.""" + return self._insteon_device_state.group + @property def name(self): """Return the name of the node (used for Entity_ID).""" @@ -69,17 +68,20 @@ class InsteonPLMSensorDevice(Entity): @property def state(self): """Return the state of the sensor.""" - sensorstate = self._insteon_device_state.value - return sensorstate + return self._insteon_device_state.value @property def device_state_attributes(self): """Provide attributes for display on device card.""" insteon_plm = get_component('insteon_plm') - return insteon_plm.common_attributes(self._insteon_device, - self._insteon_device_state) + return insteon_plm.common_attributes(self) @callback def async_sensor_update(self, deviceid, statename, val): """Receive notification from transport that new data exists.""" - self._hass.async_add_job(self.async_update_ha_state()) + self.async_schedule_update_ha_state() + + @asyncio.coroutine + def async_added_to_hass(self): + """Register INSTEON update events.""" + self._insteon_device_state.register_updates(self.async_sensor_update) diff --git a/homeassistant/components/switch/insteon_plm.py b/homeassistant/components/switch/insteon_plm.py index 95b0f5ba9ad..3e940e2ff43 100644 --- a/homeassistant/components/switch/insteon_plm.py +++ b/homeassistant/components/switch/insteon_plm.py @@ -2,10 +2,10 @@ Support for INSTEON dimmers via PowerLinc Modem. For more details about this component, please refer to the documentation at -https://home-assistant.io/components/insteon_plm/ +https://home-assistant.io/components/switch.insteon_plm/ """ -import logging import asyncio +import logging from homeassistant.core import callback from homeassistant.components.switch import (SwitchDevice) @@ -19,39 +19,31 @@ _LOGGER = logging.getLogger(__name__) @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Set up the INSTEON PLM device class for the hass platform.""" - state_list = [] + entities = [] plm = hass.data['insteon_plm'] - for device_info in discovery_info: - address = device_info['address'] - device = plm.devices[address] - state_key = device_info['state_key'] + address = discovery_info['address'] + device = plm.devices[address] + state_key = discovery_info['state_key'] - state_name = device.states[state_key].name + state_name = device.states[state_key].name - if state_name in ['lightOnOff', 'outletTopOnOff', 'outletBottomOnOff']: - state_list.append(InsteonPLMSwitchDevice(hass, - device, - state_key)) - elif state_name == 'openClosedRelay': - state_list.append(InsteonPLMOpenClosedDevice(hass, - device, - state_key)) + if state_name in ['lightOnOff', 'outletTopOnOff', 'outletBottomOnOff']: + entities.append(InsteonPLMSwitchDevice(device, state_key)) + elif state_name == 'openClosedRelay': + entities.append(InsteonPLMOpenClosedDevice(device, state_key)) - async_add_devices(state_list) + async_add_devices(entities) class InsteonPLMSwitchDevice(SwitchDevice): """A Class for an Insteon device.""" - def __init__(self, hass, device, state_key): + def __init__(self, device, state_key): """Initialize the switch.""" - self._hass = hass self._insteon_device_state = device.states[state_key] self._insteon_device = device - self._insteon_device_state.register_updates(self.async_switch_update) - @property def should_poll(self): """No polling needed.""" @@ -62,6 +54,10 @@ class InsteonPLMSwitchDevice(SwitchDevice): """Return the address of the node.""" return self._insteon_device.address.human + def group(self): + """Return the INSTEON group that the entity responds to.""" + return self._insteon_device_state.group + @property def name(self): """Return the name of the node (used for Entity_ID).""" @@ -83,13 +79,12 @@ class InsteonPLMSwitchDevice(SwitchDevice): def device_state_attributes(self): """Provide attributes for display on device card.""" insteon_plm = get_component('insteon_plm') - return insteon_plm.common_attributes(self._insteon_device, - self._insteon_device_state) + return insteon_plm.common_attributes(self) @callback def async_switch_update(self, deviceid, statename, val): """Receive notification from transport that new data exists.""" - self._hass.async_add_job(self.async_update_ha_state()) + self.async_schedule_update_ha_state() @asyncio.coroutine def async_turn_on(self, **kwargs): @@ -101,18 +96,20 @@ class InsteonPLMSwitchDevice(SwitchDevice): """Turn device off.""" self._insteon_device_state.off() + @asyncio.coroutine + def async_added_to_hass(self): + """Register INSTEON update events.""" + self._insteon_device_state.register_updates(self.async_switch_update) + class InsteonPLMOpenClosedDevice(SwitchDevice): """A Class for an Insteon device.""" - def __init__(self, hass, device, state_key): + def __init__(self, device, state_key): """Initialize the switch.""" - self._hass = hass self._insteon_device_state = device.states[state_key] self._insteon_device = device - self._insteon_device_state.register_updates(self.async_relay_update) - @property def should_poll(self): """No polling needed.""" @@ -150,7 +147,7 @@ class InsteonPLMOpenClosedDevice(SwitchDevice): @callback def async_relay_update(self, deviceid, statename, val): """Receive notification from transport that new data exists.""" - self._hass.async_add_job(self.async_update_ha_state()) + self.async_schedule_update_ha_state() @asyncio.coroutine def async_turn_on(self, **kwargs): @@ -161,3 +158,8 @@ class InsteonPLMOpenClosedDevice(SwitchDevice): def async_turn_off(self, **kwargs): """Turn device off.""" self._insteon_device_state.close() + + @asyncio.coroutine + def async_added_to_hass(self): + """Register INSTEON update events.""" + self._insteon_device_state.register_updates(self.async_relay_update)