Implement templates for covers#8100
Conversation
| self.hass.start() | ||
| self.hass.block_till_done() | ||
|
|
||
| core.cover.set_cover_tilt_position(self.hass, 42, 'cover.test_template_cover') |
There was a problem hiding this comment.
line too long (86 > 79 characters)
| state = self.hass.states.get('cover.test_template_cover') | ||
| assert state.state == STATE_OPEN | ||
|
|
||
| core.cover.set_cover_position(self.hass, 42, 'cover.test_template_cover') |
There was a problem hiding this comment.
line too long (81 > 79 characters)
| 'covers': { | ||
| 'test_template_cover': { | ||
| 'position_template': | ||
| "{{ states.cover.test_state.attributes.position }}", |
There was a problem hiding this comment.
line too long (84 > 79 characters)
There was a problem hiding this comment.
I am not sure how to best fix this one without breaking the indentation
There was a problem hiding this comment.
Since it is a string, wondering if you can split it in two lines, like:
"{{states.cover.test_state."
"attributes.position}}",
There was a problem hiding this comment.
You can find some other approaches to handle this here
There was a problem hiding this comment.
Thanks.
I decided to shorten the state-variable name. I only needed 4 characters.. I think readability is better.
|
|
||
| state = self.hass.states.get('cover.test_template_cover') | ||
| assert state.state == STATE_CLOSED | ||
| def test_template_state_boolean(self): |
| from homeassistant.const import STATE_ON, STATE_OFF, STATE_OPEN, STATE_CLOSED | ||
| from homeassistant.helpers.restore_state import DATA_RESTORE_CACHE | ||
|
|
||
| from tests.common import ( |
There was a problem hiding this comment.
'tests.common.mock_component' imported but unused
| state = float(self._position_template.async_render()) | ||
| if state < 0 or state > 100: | ||
| self._position = None | ||
| _LOGGER.error("Cover position value must be between 0 and 100. Value was: %.2f", state) |
There was a problem hiding this comment.
line too long (108 > 79 characters)
| try: | ||
| state = self._template.async_render().lower() | ||
| if state in _VALID_STATES: | ||
| self._position = 100 if state in ('true', STATE_OPEN) else 0 |
There was a problem hiding this comment.
line too long (80 > 79 characters)
| self._tilt_template.hass = self.hass | ||
| if self._icon_template is not None: | ||
| self._icon_template.hass = self.hass | ||
| @asyncio.coroutine |
| if ((position_template is None and state_template is None) or | ||
| (position_template is not None and state_template is not None)): | ||
| _LOGGER.error('Must specify only one of: %s', | ||
| ', '.join([CONF_VALUE_TEMPLATE, CONF_VALUE_TEMPLATE])) |
There was a problem hiding this comment.
line too long (80 > 79 characters)
| tilt_action = device_config.get(TILT_ACTION) | ||
|
|
||
| if ((position_template is None and state_template is None) or | ||
| (position_template is not None and state_template is not None)): |
There was a problem hiding this comment.
line too long (80 > 79 characters)
| state = float(self._position_template.async_render()) | ||
| if state < 0 or state > 100: | ||
| self._position = None | ||
| _LOGGER.error("Cover position value must be between 0 and 100." |
There was a problem hiding this comment.
line too long (83 > 79 characters)
| self.hass.stop() | ||
|
|
||
| def test_template_state_text(self): | ||
| """"Test the state text of a template.""" |
There was a problem hiding this comment.
You have an extra double quote here.
| assert state.state == STATE_CLOSED | ||
|
|
||
| def test_template_state_boolean(self): | ||
| """"Test the state text of a template.""" |
| assert state.state == STATE_OPEN | ||
|
|
||
| def test_template_position(self): | ||
| """"Test the state text of a template.""" |
| assert state.state == STATE_CLOSED | ||
|
|
||
| def test_template_tilt(self): | ||
| """"Test the state text of a template.""" |
| assert state.attributes.get('current_tilt_position') == 42.0 | ||
|
|
||
| def test_template_out_of_bounds(self): | ||
| """"Test the state text of a template.""" |
| assert self.hass.states.all() == [] | ||
|
|
||
| def test_template_non_numeric(self): | ||
| """"Test the state text of a template.""" |
There was a problem hiding this comment.
Same. Just search for """" and replace by """ throughout the document
|
You may have to push another commit to fix the travis timeout error. |
| if ((position_template is None and state_template is None) or | ||
| (position_template is not None and | ||
| state_template is not None)): | ||
| _LOGGER.error('Must specify only one of: %s', |
There was a problem hiding this comment.
Instead of testing like this, use the voluptuous Exclusive validator to make sure only 1 is passed in. Example: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/wink.py#L53-L56
There was a problem hiding this comment.
I've changed this, however I still need the check because voluptuous doesn't seem to offer one-hot exclusivity.
| def async_added_to_hass(self): | ||
| """Register callbacks.""" | ||
| state = yield from async_get_last_state(self.hass, self.entity_id) | ||
| if state: |
There was a problem hiding this comment.
This is a weird check. You want to check the value too. Now you just check that a state exists.
There was a problem hiding this comment.
I don't understand the issue. I check the value in the next line:
if state:
self._position = 100 if state.state else 0
Do you prefer that I remove the ternary check?
There was a problem hiding this comment.
Sorry, added comment to the wrong line. But state.state is going to be either STATE_OPEN or STATE_CLOSED. Both are string values and thus will always be truthy, setting the position to 100.
There was a problem hiding this comment.
Thank you for taking the time to explain the issue.
|
Great work! This is very close to be ready to be merged 💃 |
* Implement templates for covers * Fix a few remaining pylint warnings * Fix hound line-length warnings * Fix one more hound line-length warning * Fix quadruple-quotes an line length code-quality issues * Irrelevant change to retrigger travis due to timeout * Use volutuous Exclusive to check for mutex condition * Fix incorrect state check
Description:
Implement templates for cover elements
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2842
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass