-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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
KNX Cover tilt control #8159
Conversation
|
||
self._set_tilt_position = round(tilt_position, -1) | ||
self.set_percentage('setangle', tilt_position) | ||
|
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
CONF_STOP = 'stop_address' | ||
CONF_UPDOWN = 'updown_address' | ||
CONF_INVERT_POSITION = 'invert_position' | ||
CONF_INVERT_ANGLE = 'invert_angle' |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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 \ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@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
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):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass