8000 KNX Cover tilt control · Pull Request #8159 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

KNX Cover tilt control #8159

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.

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

Merged
merged 12 commits into from Jun 28, 2017
Merged

KNX Cover tilt control #8159

merged 12 commits into from Jun 28, 2017

Conversation

ghost
Copy link
@ghost ghost commented Jun 22, 2017

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:

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.


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

Choose a reason for hiding this comment

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

blank line contains whitespace

return self._current_tilt


@property

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)

@@ -85,6 +112,21 @@ def target_position(self):
"""Return the position we are trying to reach: 0 - 100."""
return self._target_pos


@property

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)

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

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)

# 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",

Choose a reason for hiding this comment

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

trailing whitespace

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

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)

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

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)

@@ -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

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)

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

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

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

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)

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

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

@ghost ghost changed the title Knxcover3 KNX Cover tilt control Jun 22, 2017
CONF_STOP = 'stop_address'
CONF_UPDOWN = 'updown_address'
CONF_INVERT_POSITION = 'invert_position'
CONF_INVERT_ANGLE = 'invert_angle'
Copy link
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
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.

@@ -34,6 +39,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,
Copy link
Member
@pvizeli pvizeli Jun 23, 2017

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

Copy link
Member
@pvizeli pvizeli Jun 23, 2017

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
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
Member

Choose a reason for hiding this comment

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

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 \
Copy link
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
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.

@ghost
Copy link
Author
ghost commented Jun 28, 2017

@pvizeli: The requested changed have been implemented.

@pvizeli pvizeli merged commit 6846a76 into home-assistant:dev Jun 28, 2017
@balloob balloob mentioned this pull request Jul 1, 2017
@ghost ghost deleted the knxcover3 branch July 20, 2017 11:52
dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
* 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
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0