Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Re-implement HomematicIP cloud to async #13468

Merged
merged 15 commits into from Apr 25, 2018
Merged

Re-implement HomematicIP cloud to async #13468

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
bd9d104
Recode to async version of homematicip-rest-api
worm-ee Mar 25, 2018
ea2a1ac
Remove blank line
worm-ee Mar 25, 2018
4b8fd19
Cleanup of access point status class
worm-ee Mar 25, 2018
7e09231
Fix to loong line
worm-ee Mar 26, 2018
8df9e54
Fix import errors
worm-ee Mar 26, 2018
d697637
Bugfix missing wait the _retry_task for sleep command
worm-ee Apr 14, 2018
bf448ab
Update comment
worm-ee Apr 16, 2018
74bcd8c
Updates after review
worm-ee Apr 17, 2018
cbab26f
Small updates of logging and property name
worm-ee Apr 17, 2018
637ef04
Fix DOMAIN and revert back to lowercase snakecase strings
worm-ee Apr 17, 2018
5d37888
Fix intention and tripple double quotes
worm-ee Apr 17, 2018
138c341
Fix travis build
worm-ee Apr 18, 2018
bb23d7c
Remove unnecessary state attributes
worm-ee Apr 23, 2018
335aa0c
Fix optional name in configuration
worm-ee Apr 24, 2018
5249110
Further reduction of state attributes
worm-ee Apr 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
217 changes: 125 additions & 92 deletions homeassistant/components/homematicip_cloud.py
View file
Open in desktop
Expand Up @@ -5,143 +5,181 @@
https://home-assistant.io/components/homematicip_cloud/
"""

import asyncio
import logging
from socket import timeout

import voluptuous as vol

from homeassistant.core import callback
from homeassistant.const import EVENT_HOMEASSISTANT_STOP
from homeassistant.helpers.aiohttp_client import async_get_clientsession
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.dispatcher import (dispatcher_send,
async_dispatcher_connect)
from homeassistant.helpers.discovery import load_platform
from homeassistant.helpers.discovery import async_load_platform
from homeassistant.helpers.entity import Entity

REQUIREMENTS = ['homematicip==0.8']
REQUIREMENTS = ['homematicip==0.9.1']

_LOGGER = logging.getLogger(__name__)

DOMAIN = 'homematicip_cloud'

COMPONENTS = [
'sensor'
]

CONF_NAME = 'name'
CONF_ACCESSPOINT = 'accesspoint'
CONF_AUTHTOKEN = 'authtoken'

CONFIG_SCHEMA = vol.Schema({
vol.Optional(DOMAIN): [vol.Schema({
vol.Optional(CONF_NAME, default=''): cv.string,
vol.Optional(DOMAIN, default=[]): vol.All(cv.ensure_list, [vol.Schema({
vol.Optional(CONF_NAME): vol.Any(None, cv.string),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't allow None.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to keep None there as I use it als a prefix for the component names if someone like me has more access points to distinguish them and having the main without prefix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not specifying a name will have the same result.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I need you expertise, so far I have two access points in my config, like:

homematicip_cloud:
- name:
accesspoint: !secret homematicip_accesspoint
authtoken: !secret homematicip_authtoken
- name: loc2
accesspoint: !secret homematicip_accesspoint-loc2
authtoken: !secret homematicip_authtoken-loc2

First one without a prefix for the components, second one with HUB2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should never have users specifying a key without a value. The following config should give you the same result.

homematicip_cloud:
  - accesspoint: !secret homematicip_accesspoint
    authtoken: !secret homematicip_authtoken
  - name: loc2
    accesspoint: !secret homematicip_accesspoint-loc2
    authtoken: !secret homematicip_authtoken-loc2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your help, I would never have tried this syntax :-).
I will fix this...

vol.Required(CONF_ACCESSPOINT): cv.string,
vol.Required(CONF_AUTHTOKEN): cv.string,
})],
})]),
}, extra=vol.ALLOW_EXTRA)

EVENT_HOME_CHANGED = 'homematicip_home_changed'
EVENT_DEVICE_CHANGED = 'homematicip_device_changed'
EVENT_GROUP_CHANGED = 'homematicip_group_changed'
EVENT_SECURITY_CHANGED = 'homematicip_security_changed'
EVENT_JOURNAL_CHANGED = 'homematicip_journal_changed'
HMIP_ACCESS_POINT = 'Access Point'
HMIP_HUB = 'HmIP-HUB'

ATTR_HOME_ID = 'home_id'
ATTR_HOME_LABEL = 'home_label'
ATTR_HOME_NAME = 'home_name'
ATTR_DEVICE_ID = 'device_id'
ATTR_DEVICE_LABEL = 'device_label'
ATTR_STATUS_UPDATE = 'status_update'
ATTR_FIRMWARE_STATE = 'firmware_state'
ATTR_UNREACHABLE = 'unreachable'
ATTR_LOW_BATTERY = 'low_battery'
ATTR_MODEL_TYPE = 'model_type'
ATTR_GROUP_TYPE = 'group_type'
ATTR_DEVICE_RSSI = 'device_rssi'
ATTR_DUTY_CYCLE = 'duty_cycle'
ATTR_CONNECTED = 'connected'
ATTR_SABOTAGE = 'sabotage'
ATTR_RSSI = 'rssi'
ATTR_TYPE = 'type'
ATTR_OPERATION_LOCK = 'operation_lock'


def setup(hass, config):
async def async_setup(hass, config):
"""Set up the HomematicIP component."""
# pylint: disable=import-error, no-name-in-module
from homematicip.home import Home
from homematicip.base.base_connection import HmipConnectionError

hass.data.setdefault(DOMAIN, {})
homes = hass.data[DOMAIN]
accesspoints = config.get(DOMAIN, [])

def _update_event(events):
"""Handle incoming HomeMaticIP events."""
for event in events:
etype = event['eventType']
edata = event['data']
if etype == 'DEVICE_CHANGED':
dispatcher_send(hass, EVENT_DEVICE_CHANGED, edata.id)
elif etype == 'GROUP_CHANGED':
dispatcher_send(hass, EVENT_GROUP_CHANGED, edata.id)
elif etype == 'HOME_CHANGED':
dispatcher_send(hass, EVENT_HOME_CHANGED, edata.id)
elif etype == 'JOURNAL_CHANGED':
dispatcher_send(hass, EVENT_SECURITY_CHANGED, edata.id)
return True

for device in accesspoints:
name = device.get(CONF_NAME)
accesspoint = device.get(CONF_ACCESSPOINT)
authtoken = device.get(CONF_AUTHTOKEN)

home = Home()
if name.lower() == 'none':
name = ''
home.label = name
for conf in accesspoints:
_websession = async_get_clientsession(hass)
_hmip = HomematicipConnector(hass, conf, _websession)
try:
home.set_auth_token(authtoken)
home.init(accesspoint)
if home.get_current_state():
_LOGGER.info("Connection to HMIP established")
else:
_LOGGER.warning("Connection to HMIP could not be established")
return False
except timeout:
_LOGGER.warning("Connection to HMIP could not be established")
await _hmip.init()
except HmipConnectionError:
_LOGGER.error('Failed to connect to the HomematicIP server, %s.',
conf.get(CONF_ACCESSPOINT))
return False
homes[home.id] = home
home.onEvent += _update_event
home.enable_events()
_LOGGER.info('HUB name: %s, id: %s', home.label, home.id)

for component in ['sensor']:
load_platform(hass, component, DOMAIN, {'homeid': home.id}, config)
home = _hmip.home
home.name = conf.get(CONF_NAME)
home.label = HMIP_ACCESS_POINT
home.modelType = HMIP_HUB

hass.data[DOMAIN][home.id] = home
_LOGGER.info('Connected to the HomematicIP server, %s.',
conf.get(CONF_ACCESSPOINT))
homeid = {ATTR_HOME_ID: home.id}
for component in COMPONENTS:
hass.async_add_job(async_load_platform(hass, component, DOMAIN,
homeid, config))

hass.loop.create_task(_hmip.connect())
return True


class HomematicipConnector:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this below async_setup to follow our normal pattern.

"""Manages HomematicIP http and websocket connection."""

def __init__(self, hass, config, websession):
"""Initialize HomematicIP cloud connection."""
from homematicip.async.home import AsyncHome
self._hass = hass
self._ws_close_requested = False
self._retry_task = None
self._tries = 0
self._accesspoint = config.get(CONF_ACCESSPOINT)
_authtoken = config.get(CONF_AUTHTOKEN)

self.home = AsyncHome(hass.loop, websession)
self.home.set_auth_token(_authtoken)

hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, self.close())

async def init(self):
"""Initialize connection."""
await self.home.init(self._accesspoint)
await self.home.get_current_state()

async def _handle_connection(self):
"""Handle websocket connection."""
from homematicip.base.base_connection import HmipConnectionError

await self.home.get_current_state()
hmip_events = await self.home.enable_events()
try:
await hmip_events
except HmipConnectionError:
return

async def connect(self):
"""Start websocket connection."""
self._tries = 0
while True:
await self._handle_connection()
if self._ws_close_requested:
break
self._ws_close_requested = False
self._tries += 1
try:
self._retry_task = self._hass.async_add_job(asyncio.sleep(
2 ** min(9, self._tries), loop=self._hass.loop))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to wait the _retry_task otherwise the sleep command just wanders off and does not make the loop wait.

await self._retry_task

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

await self._retry_task
except asyncio.CancelledError:
break
_LOGGER.info('Reconnect (%s) to the HomematicIP cloud server.',
self._tries)

async def close(self):
"""Close the websocket connection."""
self._ws_close_requested = True
if self._retry_task is not None:
self._retry_task.cancel()
await self.home.disable_events()
_LOGGER.info("Closed connection to HomematicIP cloud server.")


class HomematicipGenericDevice(Entity):
"""Representation of an HomematicIP generic device."""

def __init__(self, home, device):
def __init__(self, home, device, post=None):
"""Initialize the generic device."""
self._home = home
self._device = device
self.post = post
_LOGGER.info('Setting up %s (%s)', self.name,
self._device.modelType)

async def async_added_to_hass(self):
"""Register callbacks."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ever called ?

async_dispatcher_connect(
self.hass, EVENT_DEVICE_CHANGED, self._device_changed)
self._device.on_update(self._device_changed)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just add this to the init method.


@callback
def _device_changed(self, deviceid):
def _device_changed(self, json, **kwargs):
"""Handle device state changes."""
if deviceid is None or deviceid == self._device.id:
_LOGGER.debug('Event device %s', self._device.label)
self.async_schedule_update_ha_state()

def _name(self, addon=''):
"""Return the name of the device."""
name = ''
if self._home.label != '':
name += self._home.label + ' '
name += self._device.label
if addon != '':
name += ' ' + addon
return name
_LOGGER.debug('Event %s (%s)', self.name, self._device.modelType)
self.async_schedule_update_ha_state()

@property
def name(self):
"""Return the name of the generic device."""
return self._name()
name = self._device.label
if self._home.name is not None:
name = "{} {}".format(self._home.name, name)
if self.post is not None:
name = "{} {}".format(name, self.post)
return name

@property
def should_poll(self):
Expand All @@ -153,24 +191,19 @@ def available(self):
"""Device available."""
return not self._device.unreach

def _generic_state_attributes(self):
@property
def device_state_attributes(self):
"""Return the state attributes of the generic device."""
laststatus = ''
if self._device.lastStatusUpdate is not None:
laststatus = self._device.lastStatusUpdate.isoformat()
return {
ATTR_HOME_LABEL: self._home.label,
ATTR_HOME_NAME: self._home.name,
ATTR_DEVICE_LABEL: self._device.label,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We re storing way too many things in the attributes here. Attributes should only be used to store things that describe the STATE of a device. I am not sure how all these other things came in here, but they should be removed.

Example of a good attribute: a light has the state "on". The attributes explain that the color is red.

Anything that does not explain the state, should be removed. Examples are unreachable, firmware state or status update (this one is already described by looking at the last_updated attribute of the state).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove all unnecessary things from the sensors as well as the status sensor for components. You are right it might confuse users.

ATTR_HOME_ID: self._device.homeId,
ATTR_DEVICE_ID: self._device.id.lower(),
ATTR_STATUS_UPDATE: laststatus,
ATTR_FIRMWARE_STATE: self._device.updateState.lower(),
ATTR_UNREACHABLE: self._device.unreach,
ATTR_LOW_BATTERY: self._device.lowBat,
ATTR_RSSI: self._device.rssiDeviceValue,
ATTR_TYPE: self._device.modelType
ATTR_DEVICE_RSSI: self._device.rssiDeviceValue,
ATTR_MODEL_TYPE: self._device.modelType
}

@property
def device_state_attributes(self):
"""Return the state attributes of the generic device."""
return self._generic_state_attributes()