KNX Cover tilt control · Pull Request #8159 · home-assistant/core

ghost · 2017-06-22T18:30:34Z

Description: Add TILT control to KNX cover.

This adds 2 new KNX addresses to the cover component to implement the TILT feature (in KNX terms usually called "angle"). Without configuring these addresses, the components acts as a simple shutter. If they are configured, the title feature is enabled.

Related issue (if applicable): none

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for `configuration.yaml` (if applicable):

- platform: knx
  name: Shutter
  updown_address: 2/20
  stop_address: 2/21
  setposition_address: 2/22
  getposition_address: 2/24
  setangle_address: 2/23
  getangle_address: 2/25
  invert_position: True
  invert_angle: True

Checklist:

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

… closed position

…into knxcover3

houndci-bot · 2017-06-22T18:30:43Z

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


blank line contains whitespace

houndci-bot · 2017-06-22T18:30:43Z

+
+
+    @property
+    def target_tilt(self):


too many blank lines (2)

houndci-bot · 2017-06-22T18:30:43Z


+
+    @property
+    def current_cover_tilt_position(self):


too many blank lines (2)

houndci-bot · 2017-06-22T18:30:43Z

+            _LOGGER.debug("%s: Tilt supported at addresses %s, %s", 
+                          self.name, config.config.get(CONF_SETANGLE_ADDRESS),
+                          config.config.get(CONF_GETANGLE_ADDRESS))
+            self._supported_features = self._supported_features | SUPPORT_SET_TILT_POSITION


line too long (91 > 79 characters)

houndci-bot · 2017-06-22T18:30:43Z

+        # Tilt is only supported, if there is a get and set address for the angle
+        if (config.config.get(CONF_SETANGLE_ADDRESS) is not None) and \
+           (config.config.get(CONF_GETANGLE_ADDRESS) is not None):
+            _LOGGER.debug("%s: Tilt supported at addresses %s, %s", 


trailing whitespace

houndci-bot · 2017-06-22T18:30:43Z

+        self._target_tilt = None
+        self._supported_features = SUPPORT_OPEN | SUPPORT_CLOSE | SUPPORT_SET_POSITION | SUPPORT_STOP
+
+        # Tilt is only supported, if there is a get and set address for the angle


line too long (81 > 79 characters)

houndci-bot · 2017-06-22T18:30:43Z

        self._target_pos = None
+        self._current_tilt = None
+        self._target_tilt = None
+        self._supported_features = SUPPORT_OPEN | SUPPORT_CLOSE | SUPPORT_SET_POSITION | SUPPORT_STOP


line too long (101 > 79 characters)

houndci-bot · 2017-06-22T18:30:43Z

 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


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

houndci-bot · 2017-06-22T18:34:00Z

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


continuation line over-indented for visual indent

houndci-bot · 2017-06-22T18:34:00Z

+        self._supported_features = SUPPORT_OPEN | SUPPORT_CLOSE | \
+                                   SUPPORT_SET_POSITION | SUPPORT_STOP
+
+        # Tilt is only supported, if there is a get and set address for the angle


line too long (81 > 79 characters)

houndci-bot · 2017-06-22T18:34:00Z

+        self._current_tilt = None
+        self._target_tilt = None
+        self._supported_features = SUPPORT_OPEN | SUPPORT_CLOSE | \
+                                   SUPPORT_SET_POSITION | SUPPORT_STOP


continuation line over-indented for visual indent

pvizeli · 2017-06-23T09:10:13Z

 CONF_STOP = 'stop_address'
 CONF_UPDOWN = 'updown_address'
 CONF_INVERT_POSITION = 'invert_position'
+CONF_INVERT_ANGLE = 'invert_angle'


You can not use the same invert flag for all?

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

pvizeli · 2017-06-23T10:03:02Z

    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,


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

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

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.

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).

https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/rest_command.py#L41-L42

pvizeli · 2017-06-23T10:04:16Z

+            SUPPORT_SET_POSITION | SUPPORT_STOP
+
+        # Tilt is only supported, if there is a angle get and set address
+        if (config.config.get(CONF_SETANGLE_ADDRESS) is not None) and \


With new vol check you can simplify that please.

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.

…into knxcover3

ghost · 2017-06-28T08:48:54Z

@pvizeli: The requested changed have been implemented.

* Added invert flag for position for actuators that uses 100% for fully closed position * Implementation of tilt functionality * Bugfix check tilt * Formatting * Formatting fixes * Formatting * Bugfix set_tilt * Minor modifications in configuration section * Formatting * Update knx.py

Daniel Matuschek added 5 commits June 22, 2017 09:15

Added invert flag for position for actuators that uses 100% for fully…

1a14ebe

… closed position

Merge branch 'dev' of https://github.com/home-assistant/home-assistant …

e36a7b6

…into knxcover3

Implementation of tilt functionality

b496a42

Bugfix check tilt

560050c

Formatting

b110988

homeassistant added platform: cover.knx cla-signed labels Jun 22, 2017

houndci-bot reviewed Jun 22, 2017

View reviewed changes

Formatting fixes

82ee0d4

houndci-bot reviewed Jun 22, 2017

View reviewed changes

ghost changed the title ~~Knxcover3~~ KNX Cover tilt control Jun 22, 2017

Formatting

7432031

homeassistant added the cla-signed label Jun 22, 2017

balloobbot added the platform label Jun 22, 2017

Bugfix set_tilt

d9108b1

homeassistant added the cla-signed label Jun 23, 2017

pvizeli reviewed Jun 23, 2017

View reviewed changes

pvizeli requested changes Jun 23, 2017

View reviewed changes

Daniel Matuschek added 3 commits June 23, 2017 14:20

Merge branch 'dev' of https://github.com/home-assistant/home-assistant …

0827a63

…into knxcover3

Minor modifications in configuration section

f1e7e3d

Formatting

c494ee3

Update knx.py

5b8755e

homeassistant added the cla-signed label Jun 28, 2017

pvizeli approved these changes Jun 28, 2017

View reviewed changes

pvizeli merged commit 6846a76 into home-assistant:dev Jun 28, 2017

balloob mentioned this pull request Jul 1, 2017

0.48 #8270

Merged

ghost deleted the knxcover3 branch July 20, 2017 11:52

home-assistant locked and limited conversation to collaborators Oct 20, 2017

ghost added integration: knx and removed platform: cover.knx labels Mar 21, 2019

Uh oh!

Conversation

ghost commented Jun 22, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description: Add TILT control to KNX cover.

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvizeli Jun 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvizeli Jun 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Jun 28, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghost commented Jun 22, 2017 •

edited by ghost

Loading

Example entry for `configuration.yaml` (if applicable):

pvizeli Jun 23, 2017 •

edited

Loading

pvizeli Jun 23, 2017 •

edited

Loading