Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
339 changes: 339 additions & 0 deletions homeassistant/components/fan/template.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,339 @@
"""
Support for Template fans.

For more details about this platform, please refer to the documentation
https://home-assistant.io/components/fan.template/
"""
import logging

import voluptuous as vol

from homeassistant.core import callback
from homeassistant.const import (
CONF_FRIENDLY_NAME, CONF_VALUE_TEMPLATE, CONF_ENTITY_ID,
STATE_ON, STATE_OFF, MATCH_ALL, EVENT_HOMEASSISTANT_START,
STATE_UNKNOWN)

from homeassistant.exceptions import TemplateError
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.config_validation import PLATFORM_SCHEMA
from homeassistant.helpers.entity import ToggleEntity
from homeassistant.components.fan import (SPEED_LOW, SPEED_MEDIUM,
SPEED_HIGH, SUPPORT_SET_SPEED,
SUPPORT_OSCILLATE, FanEntity,
ATTR_SPEED, ATTR_OSCILLATING,
ENTITY_ID_FORMAT)

from homeassistant.helpers.entity import async_generate_entity_id
from homeassistant.helpers.script import Script

_LOGGER = logging.getLogger(__name__)

CONF_FANS = 'fans'
CONF_SPEED_LIST = 'speeds'
CONF_SPEED_TEMPLATE = 'speed_template'
CONF_OSCILLATING_TEMPLATE = 'oscillating_template'
CONF_ON_ACTION = 'turn_on'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should use the consts COMMAND_ON and COMMAND_OFF to match the other templates.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

light/template is also using same naming convention and some people might already use my PR so I prefer not to change this.

CONF_OFF_ACTION = 'turn_off'
CONF_SET_SPEED_ACTION = 'set_speed'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think you should add _ACTION here, just CONF_SET_SPEED, etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am using same naming convention as in light/template so prefer to keep them if u don't might

CONF_SET_OSCILLATING_ACTION = 'set_oscillating'

_VALID_STATES = [STATE_ON, STATE_OFF, 'true', 'false']

FAN_SCHEMA = vol.Schema({
vol.Optional(CONF_FRIENDLY_NAME): cv.string,
vol.Required(CONF_VALUE_TEMPLATE): cv.template,
vol.Optional(CONF_SPEED_TEMPLATE): cv.template,
vol.Optional(CONF_OSCILLATING_TEMPLATE): cv.template,

vol.Required(CONF_ON_ACTION): cv.SCRIPT_SCHEMA,
vol.Required(CONF_OFF_ACTION): cv.SCRIPT_SCHEMA,

vol.Optional(CONF_SET_SPEED_ACTION): cv.SCRIPT_SCHEMA,
vol.Optional(CONF_SET_OSCILLATING_ACTION): cv.SCRIPT_SCHEMA,

vol.Optional(
CONF_SPEED_LIST,
default=[SPEED_LOW, SPEED_MEDIUM, SPEED_HIGH]
): cv.ensure_list,

vol.Optional(CONF_ENTITY_ID): cv.entity_ids
})

PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_FANS): vol.Schema({cv.slug: FAN_SCHEMA}),
})


async def async_setup_platform(
hass, config, async_add_devices, discovery_info=None
):
"""Set up the Template Fans."""
fans = []

for device, device_config in config[CONF_FANS].items():
friendly_name = device_config.get(CONF_FRIENDLY_NAME, device)

state_template = device_config[CONF_VALUE_TEMPLATE]
speed_template = device_config.get(CONF_SPEED_TEMPLATE)
oscillating_template = device_config.get(
CONF_OSCILLATING_TEMPLATE
)

on_action = device_config[CONF_ON_ACTION]
off_action = device_config[CONF_OFF_ACTION]
set_speed_action = device_config.get(CONF_SET_SPEED_ACTION)
set_oscillating_action = device_config.get(CONF_SET_OSCILLATING_ACTION)

speed_list = device_config[CONF_SPEED_LIST]

template_entity_ids = set()

temp_ids = state_template.extract_entities()
if temp_ids != MATCH_ALL:
template_entity_ids |= set(temp_ids)

if speed_template:
temp_ids = speed_template.extract_entities()
if temp_ids != MATCH_ALL:

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.

This still won't work. The moment you get a MATCH_ALL, you should just match on MATCH_ALL.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry but I dont get it, could u give the correct code.
Thanks

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.

template_entity_ids |= set(temp_ids)

if oscillating_template:
temp_ids = oscillating_template.extract_entities()

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.

if one is MATCH_ALL, they should all be MATCH_ALL. This is not currently the case. If the oscillating_template returns MATCH_ALL, it will just use extracted entities of the other templates.

if temp_ids != MATCH_ALL:
template_entity_ids |= set(temp_ids)

if not template_entity_ids:
template_entity_ids = MATCH_ALL

entity_ids = device_config.get(CONF_ENTITY_ID, template_entity_ids)

fans.append(
TemplateFan(
hass, device, friendly_name,
state_template, speed_template, oscillating_template,
on_action, off_action, set_speed_action,
set_oscillating_action, speed_list, entity_ids
)
)

async_add_devices(fans)


class TemplateFan(FanEntity):
"""A template fan component."""

def __init__(self, hass, device_id, friendly_name,
state_template, speed_template, oscillating_template,
on_action, off_action, set_speed_action,
set_oscillating_action, speed_list, entity_ids):
"""Initialize the fan."""
self.hass = hass
self.entity_id = async_generate_entity_id(
ENTITY_ID_FORMAT, device_id, hass=hass)
self._name = friendly_name

self._template = state_template
self._speed_template = speed_template
self._oscillating_template = oscillating_template
self._supported_features = 0

self._on_script = Script(hass, on_action)
self._off_script = Script(hass, off_action)

self._set_speed_script = None
if set_speed_action:
self._set_speed_script = Script(hass, set_speed_action)

self._set_oscillating_script = None
if set_oscillating_action:
self._set_oscillating_script = Script(hass, set_oscillating_action)

self._state = False
self._speed = None
self._oscillating = None

self._template.hass = self.hass
if self._speed_template:
self._speed_template.hass = self.hass
self._supported_features |= SUPPORT_SET_SPEED
if self._oscillating_template:
self._oscillating_template.hass = self.hass
self._supported_features |= SUPPORT_OSCILLATE

self._entities = entity_ids
# List of valid speeds
self._speed_list = speed_list

@property
def name(self):
"""Return the display name of this fan."""
return self._name

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

@property
def speed_list(self: ToggleEntity) -> list:
"""Get the list of available speeds."""
return self._speed_list

@property
def is_on(self):
"""Return true if device is on."""
return self._state

@property
def speed(self):
"""Return the current speed."""
return self._speed

@property
def oscillating(self):
"""Return the oscillation state."""
return self._oscillating

@property
def should_poll(self):
"""Return the polling state."""
return False

# pylint: disable=arguments-differ
async def async_turn_on(self, speed: str = None) -> None:
"""Turn on the fan.

This method is a coroutine.

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.

I think by now (especially with async def) we don't need to plaster "This method is a coroutine." everywhere where it's abundantly clear from the context.

"""
self._state = True
await self._on_script.async_run()

if speed:
await self.async_set_speed(speed)

# pylint: disable=arguments-differ
async def async_turn_off(self) -> None:
"""Turn off the fan.

This method is a coroutine.
"""
self._state = False

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.

Shouldn't we only set the state to False once the off script terminates? + If the off script raises an exception I would expect the state to remain in the previous state.

await self._off_script.async_run()

async def async_set_speed(self, speed: str) -> None:
"""Set the speed of the fan.

This method is a coroutine.
"""
if self._set_speed_script is None:
return

if speed in self._speed_list:
self._speed = speed
await self._set_speed_script.async_run({ATTR_SPEED: speed})
else:
_LOGGER.error(
'Received invalid speed: %s. ' +
'Expected: %s.',
speed, self._speed_list)

async def async_oscillate(self, oscillating: bool) -> None:
"""Set oscillation of the fan.

This method is a coroutine.
"""
if self._set_oscillating_script is None:
return

if oscillating is True or oscillating is False:
self._oscillating = oscillating
await self._set_oscillating_script.async_run(
{ATTR_OSCILLATING: oscillating}
)
else:
_LOGGER.error(

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.

This check is already handled by the service schema:

https://github.com/home-assistant/home-assistant/blob/678f284015a2c52f96a7687979cfd9f785e4527a/homeassistant/components/fan/__init__.py#L78-L81

Also if it were needed, it should definitely not go into this platform, but rather in the core fan definition.

'Received invalid oscillating: %s. ' +
'Expected True/False.', oscillating)

async def async_added_to_hass(self):
"""Register callbacks."""
@callback
def template_fan_state_listener(entity, old_state, new_state):
"""Handle target device state changes."""
self.async_schedule_update_ha_state(True)

@callback
def template_fan_startup(event):
"""Update template on startup."""
self.hass.helpers.event.async_track_state_change(
self._entities, template_fan_state_listener)

self.async_schedule_update_ha_state(True)

self.hass.bus.async_listen_once(
EVENT_HOMEASSISTANT_START, template_fan_startup)

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.

I don't understand why we should only call this on EVENT_HOMEASSISTANT_START. This code would break once we start dynamically loading/unloading entities. Maybe have a look at how it's done in the light group platform:

https://github.com/home-assistant/home-assistant/blob/ca5f4709564773c8cebd6ee40aa7cc1095b19105/homeassistant/components/light/group.py#L72-L82

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am doing the same as in light/template and sensor/template. Using the code in light group did not work. Fan is no changed when other entities changed


async def async_update(self):
"""Update the state from the template."""
_LOGGER.info('Updating fan %s', self._name)

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.

This seems like quite the unnecessary logging statement. Even more it's info and not debug.


# Update state
try:
state = self._template.async_render().lower()
except TemplateError as ex:
_LOGGER.error(ex)
self._state = None

# Validate state
if state in _VALID_STATES:

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.

I think if the previous call produces a TemplateError, state will not be set, resulting in NameError: name 'state' is not defined.

self._state = state in ('true', STATE_ON)

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.

I think we should only allow STATE_ON, at some point people will want 1, yes, enable and so on...

elif state == STATE_UNKNOWN:
self._state = None
else:
_LOGGER.error(
'Received invalid fan is_on state: %s. ' +
'Expected: %s.',
state, ', '.join(_VALID_STATES))
self._state = None

# Update speed if 'speed_template' is configured
if self._speed_template:

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.

PEP8 recommend using is not None:

if self._speed_template is not None:

try:
speed = self._speed_template.async_render().lower()
except TemplateError as ex:
_LOGGER.error(ex)
self._state = None

# Validate speed
if speed in self._speed_list:

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.

Same here.

self._speed = speed
elif speed == STATE_UNKNOWN:
self._speed = None
else:
_LOGGER.error(
'Received invalid speed: %s. ' +
'Expected: %s.',
speed, self._speed_list)
self._speed = None

# Update oscillating if 'oscillating_template' is configured
if self._oscillating_template:
try:
oscillating = self._oscillating_template.async_render().lower()
except TemplateError as ex:
_LOGGER.error(ex)
self._state = None

# Validate osc
if oscillating == 'true':

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.

Same here

self._oscillating = True
elif oscillating == 'false':
self._oscillating = False
elif oscillating == STATE_UNKNOWN:
self._oscillating = None
else:
_LOGGER.error(
'Received invalid oscillating: %s. ' +
'Expected True/False.', oscillating)

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.

We do allow on/off/true/false for state, but only true/false for oscillating; that just seems very user unfriendly to me. I would just strictly allow on/off for both

self._oscillating = None
Loading