Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ omit =
homeassistant/components/climate/oem.py
homeassistant/components/climate/proliphix.py
homeassistant/components/climate/radiotherm.py
homeassistant/components/climate/sensibo.py
homeassistant/components/cover/garadget.py
homeassistant/components/cover/homematic.py
homeassistant/components/cover/myq.py
Expand Down
10 changes: 10 additions & 0 deletions homeassistant/components/climate/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
ATTR_MIN_TEMP = 'min_temp'
ATTR_TARGET_TEMP_HIGH = 'target_temp_high'
ATTR_TARGET_TEMP_LOW = 'target_temp_low'
ATTR_TARGET_TEMP_STEP = 'target_temp_step'
ATTR_AWAY_MODE = 'away_mode'
ATTR_AUX_HEAT = 'aux_heat'
ATTR_FAN_MODE = 'fan_mode'
Expand Down Expand Up @@ -419,6 +420,10 @@ def state_attributes(self):
ATTR_TEMPERATURE:
self._convert_for_display(self.target_temperature),
}

if self.target_temperature_step is not None:
data[ATTR_TARGET_TEMP_STEP] = self.target_temperature_step

target_temp_high = self.target_temperature_high
if target_temp_high is not None:
data[ATTR_TARGET_TEMP_HIGH] = self._convert_for_display(
Expand Down Expand Up @@ -505,6 +510,11 @@ def target_temperature(self):
"""Return the temperature we try to reach."""
return None

@property
def target_temperature_step(self):
"""Return the supported step of target temperature."""
return None

@property
def target_temperature_high(self):
"""Return the highbound target temperature we try to reach."""
Expand Down
235 changes: 235 additions & 0 deletions homeassistant/components/climate/sensibo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
"""
Support for Sensibo wifi-enabled home thermostats.

For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/climate.sensibo/
"""

import asyncio
import logging

import aiohttp
import voluptuous as vol

from homeassistant.const import (
ATTR_TEMPERATURE, CONF_API_KEY, CONF_ID, TEMP_CELSIUS, TEMP_FAHRENHEIT)
from homeassistant.components.climate import (
ATTR_CURRENT_HUMIDITY, ClimateDevice, PLATFORM_SCHEMA)
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers.aiohttp_client import async_get_clientsession
from homeassistant.util.temperature import convert as convert_temperature

REQUIREMENTS = ['pysensibo==1.0.0']

_LOGGER = logging.getLogger(__name__)

ALL = 'all'

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_API_KEY): cv.string,
vol.Optional(CONF_ID, default=ALL): vol.All(cv.ensure_list, cv.string),
})

_FETCH_FIELDS = ','.join([
'room{name}', 'measurements', 'remoteCapabilities',
'acState', 'connectionStatus{isAlive}'])
_INITIAL_FETCH_FIELDS = 'id,' + _FETCH_FIELDS


@asyncio.coroutine
def async_setup_platform(hass, config, async_add_devices, discovery_info=None):
"""Set up Sensibo devices."""
import pysensibo

client = pysensibo.SensiboClient(
config[CONF_API_KEY], session=async_get_clientsession(hass))
devices = []
try:
for dev in (
yield from client.async_get_devices(_INITIAL_FETCH_FIELDS)):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please wrap this in async_timeout.timeout to make sure that we don't wait forever.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is a default timeout on operations of 5 minutes: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/client.py#L37

I'll pass in lower timeout.

Would it be cleaner if the client session additionally had a default read and connect timeouts? https://github.com/aio-libs/aiohttp/blob/master/aiohttp/client.py#L55

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are many things that can cause a request to take forever. If you look at the intro of the docs, they suggest to use async_timeout.timeout: http://aiohttp.readthedocs.io/en/stable/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

if config[CONF_ID] == ALL or dev['id'] in config[CONF_ID]:
devices.append(SensiboClimate(client, dev))
except aiohttp.client_exceptions.ClientConnectorError:
_LOGGER.exception('Failed to connct to Sensibo servers.')
return False

if devices:
async_add_devices(devices)


class SensiboClimate(ClimateDevice):
"""Representation os a Sensibo device."""

def __init__(self, client, data):
"""Build SensiboClimate.

client: aiohttp session.
data: initially-fetched data.
"""
self._client = client
self._id = data['id']
self._do_update(data)

def _do_update(self, data):
print(data)
self._name = data['room']['name']
self._measurements = data['measurements']
self._capabilities = data['remoteCapabilities']
self._ac_states = data['acState']
self._status = data['connectionStatus']['isAlive']
self._operations = sorted(self._capabilities['modes'].keys())
self._current_capabilities = self._capabilities[
'modes'][self.current_operation]
self._temperature_unit = TEMP_CELSIUS if self._ac_states[
'temperatureUnit'] == 'C' else TEMP_FAHRENHEIT

@property
def device_state_attributes(self):
"""Return the state attributes."""
return {ATTR_CURRENT_HUMIDITY: self.current_humidity}

@property
def temperature_unit(self):
"""Return the unit of measurement which this thermostat uses."""
return self._temperature_unit

@property
def available(self):
"""Return True if entity is available."""
return self._status

@property
def target_temperature(self):
"""Return the temperature we try to reach."""
return self._ac_states['targetTemperature']

@property
def target_temperature_step(self):
"""Return the supported step of target temperature."""
return 1

@property
def current_operation(self):
"""Return current operation ie. heat, cool, idle."""
return self._ac_states['mode']

@property
def current_humidity(self):
"""Return the current humidity."""
return self._measurements['humidity']

@property
def current_temperature(self):
"""Return the current temperature."""
# This field is not affected by temperature_unit.
# It is always in C / nativeTemperatureUnit
if 'nativeTemperatureUnit' not in self._ac_states:
return self._measurements['temperature']
return convert_temperature(
self._measurements['temperature'],
TEMP_CELSIUS,
self.temperature_unit)

@property
def operation_list(self):
"""List of available operation modes."""
return self._operations

@property
def current_fan_mode(self):
"""Return the fan setting."""
return self._ac_states['fanLevel']

@property
def fan_list(self):
"""List of available fan modes."""
return self._current_capabilities['fanLevels']

@property
def current_swing_mode(self):
"""Return the fan setting."""
return self._ac_states['swing']

@property
def swing_list(self):
"""List of available swing modes."""
return self._current_capabilities['swing']

@property
def name(self):
"""Return the name of the entity."""
return self._name

@property
def is_aux_heat_on(self):
"""Return true if AC is on."""
return self._ac_states['on']

@property
def min_temp(self):
"""Return the minimum temperature."""
return self._current_capabilities['temperatures'][
'C' if self.unit_of_measurement == TEMP_CELSIUS else 'F'][
'values'][0]

@property
def max_temp(self):
"""Return the maximum temperature."""
return self._current_capabilities['temperatures'][
'C' if self.unit_of_measurement == TEMP_CELSIUS else 'F'][

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should not have to do these conversions yourself. If you return the unit that the device uses as temperature_unit, HASS will take care of converting it correctly.

(this comment applies to all other conversions too)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the device uses C internally, but the API uses the unit selected in the app. That's the unit I return in temperature_unit.

The device API also returns min/max temperature range in both C and F.
I'm picking an appropriate JSON field here, not converting anything.

The only place I'm converting units is current_temperature because that field is always in C unlike target_temperature which is according to temperature_unit

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since both fields are available, I would suggest you just always pick C and let Home Assistant do the conversion if necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually the JSON looks like this

"C": {"isNative": True, "values": [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30]},
"F": {"isNative": False, "values": [61, 63, 64, 66, 68, 70, 72, 73, 75, 77, 79, 81, 82, 84, 86]}}

So not all whole-F values are accepted.
I'll add a fix to make sure correct F temperature is set.

As for min and max - it would be safest to stick to the provided list. What is the advantage of converting?

'values'][-1]

@asyncio.coroutine
def async_set_temperature(self, **kwargs):
"""Set new target temperature."""
temperature = kwargs.get(ATTR_TEMPERATURE)
if temperature is None:
return
yield from self._client.async_set_ac_state_property(
self._id, 'targetTemperature', int(temperature))
yield from self.async_update_ha_state(True)

@asyncio.coroutine
def async_set_fan_mode(self, fan):
"""Set new target fan mode."""
yield from self._client.async_set_ac_state_property(
self._id, 'fanLevel', fan)
yield from self.async_update_ha_state(True)

@asyncio.coroutine
def async_set_operation_mode(self, operation_mode):
"""Set new target operation mode."""
yield from self._client.async_set_ac_state_property(
self._id, 'mode', operation_mode)
yield from self.async_update_ha_state(True)

@asyncio.coroutine
def async_set_swing_mode(self, swing_mode):
"""Set new target swing operation."""
yield from self._client.async_set_ac_state_property(
self._id, 'swing', swing_mode)
yield from self.async_update_ha_state(True)

@asyncio.coroutine
def async_turn_aux_heat_on(self):
"""Turn Sensibo unit on."""
yield from self._client.async_set_ac_state_property(
self._id, 'on', True)
yield from self.async_update_ha_state(True)

@asyncio.coroutine
def async_turn_aux_heat_off(self):
"""Turn Sensibo unit on."""
yield from self._client.async_set_ac_state_property(
self._id, 'on', False)
yield from self.async_update_ha_state(True)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should_poll is not defined and so defaults to True, this means that Hass will call this for you after calling any service. You can remove all these statements.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Could you point me to the code where this happens?
I would like to get an immediate update, i.e not as part of 15s update cycle here: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/helpers/entity_component.py#L377

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks!


@asyncio.coroutine
def async_update(self):
"""Retrieve latest state."""
try:
data = yield from self._client.async_get_device(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a timeout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added to _client constructor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That should be using async_timeout.timeout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

self._id, _FETCH_FIELDS)
self._do_update(data)
except aiohttp.client_exceptions.ClientConnectorError:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what about all the other errors? Shouldn't you just listen for the base error ClientError

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

_LOGGER.warning('Failed to connect to Sensibo servers.')
3 changes: 3 additions & 0 deletions requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,9 @@ pyowm==2.6.1
# homeassistant.components.qwikswitch
pyqwikswitch==0.4

# homeassistant.components.climate.sensibo
pysensibo==1.0.0

# homeassistant.components.switch.acer_projector
pyserial==3.1.1

Expand Down