Skip to content
62 changes: 60 additions & 2 deletions homeassistant/components/cover/knx.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
import voluptuous as vol

from homeassistant.components.cover import (
CoverDevice, PLATFORM_SCHEMA, ATTR_POSITION, DEVICE_CLASSES_SCHEMA
CoverDevice, PLATFORM_SCHEMA, ATTR_POSITION, DEVICE_CLASSES_SCHEMA,
SUPPORT_OPEN, SUPPORT_CLOSE, SUPPORT_SET_POSITION, SUPPORT_STOP,SUPPORT_SET_TILT_POSITION

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

missing whitespace after ','
line too long (93 > 79 characters)

)
from homeassistant.components.knx import (KNXConfig, KNXMultiAddressDevice)
from homeassistant.const import (CONF_NAME, CONF_DEVICE_CLASS)
Expand All @@ -19,9 +20,12 @@

CONF_GETPOSITION_ADDRESS = 'getposition_address'
CONF_SETPOSITION_ADDRESS = 'setposition_address'
CONF_GETANGLE_ADDRESS = 'getangle_address'
CONF_SETANGLE_ADDRESS = 'setangle_address'
CONF_STOP = 'stop_address'
CONF_UPDOWN = 'updown_address'
CONF_INVERT_POSITION = 'invert_position'
CONF_INVERT_ANGLE = 'invert_angle'

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 can not use the same invert flag for all?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, there might be cases were one is inverted, but not the other.


DEFAULT_NAME = 'KNX Cover'
DEPENDENCIES = ['knx']
Expand All @@ -34,6 +38,9 @@
vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
vol.Optional(CONF_SETPOSITION_ADDRESS): cv.string,
vol.Optional(CONF_INVERT_POSITION, default=False): cv.boolean,
vol.Optional(CONF_GETANGLE_ADDRESS): cv.string,
vol.Optional(CONF_SETANGLE_ADDRESS): cv.string,

@pvizeli pvizeli Jun 23, 2017

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.

use vol.Inclusive(..., 'angle'): cv... that make that the user need set this options.
EDIT:
If user choise one options, he need set both.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thee options are optional. There are shutter with and without the angle feature. The code supports both now.

@pvizeli pvizeli Jun 23, 2017

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.

Yes that is what I mean with this change. It would be also nice to use vol.Match(r"...") for address instead cv.string but that is optional.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In this case, I don't understand how the code should look like.
Can you give me a full example for this?

While it would be possible to define regexps for the addresses, these would become quite ugly (there are multiple address formats).

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.

vol.Optional(CONF_INVERT_ANGLE, default=False): cv.boolean,
})


Expand All @@ -50,19 +57,39 @@ def __init__(self, hass, config):
KNXMultiAddressDevice.__init__(
self, hass, config,
['updown', 'stop'], # required
optional=['setposition', 'getposition']
optional=['setposition', 'getposition',
'getangle', 'setangle']
)
self._device_class = config.config.get(CONF_DEVICE_CLASS)
self._invert_position = config.config.get(CONF_INVERT_POSITION)
self._invert_angle = config.config.get(CONF_INVERT_ANGLE)
self._hass = hass
self._current_pos = None
self._target_pos = None
self._current_tilt = None
self._target_tilt = None
self._supported_features = SUPPORT_OPEN | SUPPORT_CLOSE | SUPPORT_SET_POSITION | SUPPORT_STOP

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (101 > 79 characters)


# Tilt is only supported, if there is a get and set address for the angle

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (81 > 79 characters)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (81 > 79 characters)

if (config.config.get(CONF_SETANGLE_ADDRESS) is not None) and \

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.

With new vol check you can simplify that please.

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.

Remove also _LOGGER.debug("%s: Tilt not supported", self.name) that is not needed. It is loaded or not, you have debug output already there.

(config.config.get(CONF_GETANGLE_ADDRESS) is not None):
_LOGGER.debug("%s: Tilt supported at addresses %s, %s",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

trailing whitespace

self.name, config.config.get(CONF_SETANGLE_ADDRESS),
config.config.get(CONF_GETANGLE_ADDRESS))
self._supported_features = self._supported_features | SUPPORT_SET_TILT_POSITION

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (91 > 79 characters)

else:
_LOGGER.debug("%s: Tilt not supported", self.name)

@property
def should_poll(self):
"""Polling is needed for the KNX cover."""
return True

@property
def supported_features(self):
"""Flag supported features."""
return self._supported_features

@property
def is_closed(self):
"""Return if the cover is closed."""
Expand All @@ -85,6 +112,21 @@ def target_position(self):
"""Return the position we are trying to reach: 0 - 100."""
return self._target_pos


@property

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

too many blank lines (2)

def current_cover_tilt_position(self):
"""Return current position of cover.

None is unknown, 0 is closed, 100 is fully open.
"""
return self._current_tilt


@property

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

too many blank lines (2)

def target_tilt(self):
"""Return the tilt angle (in %) we are trying to reach: 0 - 100."""
return self._target_tilt

def set_cover_position(self, **kwargs):
"""Set new target position."""
position = kwargs.get(ATTR_POSITION)
Expand All @@ -108,6 +150,14 @@ def update(self):
self._current_pos = 100-value
_LOGGER.debug("%s: position = %d", self.name, value)

if self._supported_features & SUPPORT_SET_TILT_POSITION:
value = self.get_percentage('getangle')
if value is not None:
self._current_tilt = value
if self._invert_angle:
self._current_tilt = 100-value
_LOGGER.debug("%s: tilt = %d", self.name, value)

def open_cover(self, **kwargs):
"""Open the cover."""
_LOGGER.debug("%s: open: updown = 0", self.name)
Expand All @@ -123,6 +173,14 @@ def stop_cover(self, **kwargs):
_LOGGER.debug("%s: stop: stop = 1", self.name)
self.set_int_value('stop', 1)

def set_cover_tilt_position(self, tilt_position, **kwargs):
"""Move the cover til to a specific position."""
if self._invert_angle:
tilt_position = 100-tilt_position

self._set_tilt_position = round(tilt_position, -1)
self.set_percentage('setangle', tilt_position)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

blank line contains whitespace

@property
def device_class(self):
"""Return the class of this device, from component DEVICE_CLASSES."""
Expand Down