Skip to content

Add kelvin/brightness_pct alternatives to light.turn_on #7596

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.

Sign up for GitHub

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
balloob merged 7 commits into from
May 17, 2017
Merged

Add kelvin/brightness_pct alternatives to light.turn_on #7596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7804f45
Refactor color profiles to a class
amelchio May 14, 2017
04d4b1a
Refactor into preprocess_turn_on_alternatives
amelchio May 14, 2017
3fadbfc
LIFX: use light.preprocess_turn_on_alternatives
amelchio May 14, 2017
39b6748
Add kelvin parameter to light.turn_on
amelchio May 14, 2017
a247a2c
Add brightness_pct parameter to light.turn_on
amelchio May 14, 2017
bcb48cc
LIFX: accept brightness_pct in effects
amelchio May 14, 2017
687d330
Add test of kelvin/brightness_pct conversions
amelchio May 14, 2017
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
131 changes: 84 additions & 47 deletions homeassistant/components/light/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@
ATTR_RGB_COLOR = "rgb_color"
ATTR_XY_COLOR = "xy_color"
ATTR_COLOR_TEMP = "color_temp"
ATTR_KELVIN = "kelvin"
ATTR_MIN_MIREDS = "min_mireds"
ATTR_MAX_MIREDS = "max_mireds"
ATTR_COLOR_NAME = "color_name"
ATTR_WHITE_VALUE = "white_value"

# int with value 0 .. 255 representing brightness of the light.
# Brightness of the light, 0..255 or percentage
ATTR_BRIGHTNESS = "brightness"
ATTR_BRIGHTNESS_PCT = "brightness_pct"

# String representing a profile (built-in ones or external defined).
ATTR_PROFILE = "profile"
Expand Down Expand Up @@ -92,18 +94,21 @@
# Service call validation schemas
VALID_TRANSITION = vol.All(vol.Coerce(float), vol.Clamp(min=0, max=6553))
VALID_BRIGHTNESS = vol.All(vol.Coerce(int), vol.Clamp(min=0, max=255))
VALID_BRIGHTNESS_PCT = vol.All(vol.Coerce(float), vol.Range(min=0, max=100))

LIGHT_TURN_ON_SCHEMA = vol.Schema({
ATTR_ENTITY_ID: cv.entity_ids,
ATTR_PROFILE: cv.string,
ATTR_TRANSITION: VALID_TRANSITION,
ATTR_BRIGHTNESS: VALID_BRIGHTNESS,
ATTR_BRIGHTNESS_PCT: VALID_BRIGHTNESS_PCT,
ATTR_COLOR_NAME: cv.string,
ATTR_RGB_COLOR: vol.All(vol.ExactSequence((cv.byte, cv.byte, cv.byte)),
vol.Coerce(tuple)),
ATTR_XY_COLOR: vol.All(vol.ExactSequence((cv.small_float, cv.small_float)),
vol.Coerce(tuple)),
ATTR_COLOR_TEMP: vol.All(vol.Coerce(int), vol.Range(min=1)),
ATTR_KELVIN: vol.All(vol.Coerce(int), vol.Range(min=0)),
ATTR_WHITE_VALUE: vol.All(vol.Coerce(int), vol.Range(min=0, max=255)),
ATTR_FLASH: vol.In([FLASH_SHORT, FLASH_LONG]),
ATTR_EFFECT: cv.string,
Expand Down Expand Up @@ -142,30 +147,33 @@ def is_on(hass, entity_id=None):


def turn_on(hass, entity_id=None, transition=None, brightness=None,
rgb_color=None, xy_color=None, color_temp=None, white_value=None,
brightness_pct=None, rgb_color=None, xy_color=None,
color_temp=None, kelvin=None, white_value=None,
profile=None, flash=None, effect=None, color_name=None):
"""Turn all or specified light on."""
hass.add_job(
async_turn_on, hass, entity_id, transition, brightness,
rgb_color, xy_color, color_temp, white_value,
async_turn_on, hass, entity_id, transition, brightness, brightness_pct,
rgb_color, xy_color, color_temp, kelvin, white_value,
profile, flash, effect, color_name)


@callback
def async_turn_on(hass, entity_id=None, transition=None, brightness=None,
rgb_color=None, xy_color=None, color_temp=None,
white_value=None, profile=None, flash=None, effect=None,
color_name=None):
brightness_pct=None, rgb_color=None, xy_color=None,
color_temp=None, kelvin=None, white_value=None,
profile=None, flash=None, effect=None, color_name=None):
"""Turn all or specified light on."""
data = {
key: value for key, value in [
(ATTR_ENTITY_ID, entity_id),
(ATTR_PROFILE, profile),
(ATTR_TRANSITION, transition),
(ATTR_BRIGHTNESS, brightness),
(ATTR_BRIGHTNESS_PCT, brightness_pct),
(ATTR_RGB_COLOR, rgb_color),
(ATTR_XY_COLOR, xy_color),
(ATTR_COLOR_TEMP, color_temp),
(ATTR_KELVIN, kelvin),
(ATTR_WHITE_VALUE, white_value),
(ATTR_FLASH, flash),
(ATTR_EFFECT, effect),
Expand Down Expand Up @@ -207,6 +215,27 @@ def toggle(hass, entity_id=None, transition=None):
hass.services.call(DOMAIN, SERVICE_TOGGLE, data)


def preprocess_turn_on_alternatives(params):
"""Processing extra data for turn light on request."""
profile = Profiles.get(params.pop(ATTR_PROFILE, None))
Copy link
Copy Markdown
Member

@balloob balloob May 15, 2017

Choose a reason for hiding this comment

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

I wonder if anyone knows about this and uses it.

Copy link
Copy Markdown
Member

@balloob balloob May 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe we should deprecate it now that we have scenes, scripts etc. (out of scope of this PR)

Copy link
Copy Markdown
Contributor Author

@amelchio amelchio May 15, 2017

Choose a reason for hiding this comment

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

I have actually been thinking about extending it instead :-). I hope to make it possible to have multiple colors in a profile. This would create a "mood" that could be applied to a group of lights so each light gets one of the colors (without them all being identical). It could also be used for an animation effect where lights randomly change between colors in the profile.

if profile is not None:
params.setdefault(ATTR_XY_COLOR, profile[:2])
params.setdefault(ATTR_BRIGHTNESS, profile[2])

color_name = params.pop(ATTR_COLOR_NAME, None)
if color_name is not None:
params[ATTR_RGB_COLOR] = color_util.color_name_to_rgb(color_name)
Copy link
Copy Markdown
Member

@balloob balloob May 15, 2017

Choose a reason for hiding this comment

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

Would it make sense to just mark rgb_color and color_name exclusive in the voluptuous schema? That way they can never be specified together. Now it might be confusing to the user why one parameter would overrule the other.

Copy link
Copy Markdown
Contributor Author

@amelchio amelchio May 15, 2017

Choose a reason for hiding this comment

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

Yes, I think that would make sense but I would prefer it to be a different PR?

The xy_color, color_temp, profile and maybe more parameters are also mutually exclusive.


kelvin = params.pop(ATTR_KELVIN, None)
if kelvin is not None:
mired = color_util.color_temperature_kelvin_to_mired(kelvin)
params[ATTR_COLOR_TEMP] = mired

brightness_pct = params.pop(ATTR_BRIGHTNESS_PCT, None)
if brightness_pct is not None:
params[ATTR_BRIGHTNESS] = int(255 * brightness_pct/100)


@asyncio.coroutine
def async_setup(hass, config):
"""Expose light control via statemachine and services."""
Expand All @@ -215,10 +244,8 @@ def async_setup(hass, config):
yield from component.async_setup(config)

# load profiles from files
profiles = yield from hass.loop.run_in_executor(
None, _load_profile_data, hass)

if profiles is None:
profiles_valid = yield from Profiles.load_profiles(hass)
if not profiles_valid:
return False

@asyncio.coroutine
Expand All @@ -231,17 +258,7 @@ def async_handle_light_service(service):
target_lights = component.async_extract_from_service(service)
params.pop(ATTR_ENTITY_ID, None)

# Processing extra data for turn light on request.
profile = profiles.get(params.pop(ATTR_PROFILE, None))

if profile:
params.setdefault(ATTR_XY_COLOR, profile[:2])
params.setdefault(ATTR_BRIGHTNESS, profile[2])

color_name = params.pop(ATTR_COLOR_NAME, None)

if color_name is not None:
params[ATTR_RGB_COLOR] = color_util.color_name_to_rgb(color_name)
preprocess_turn_on_alternatives(params)

for light in target_lights:
if service.service == SERVICE_TURN_ON:
Expand Down Expand Up @@ -287,31 +304,51 @@ def async_handle_light_service(service):
return True


def _load_profile_data(hass):
"""Load built-in profiles and custom profiles."""
profile_paths = [os.path.join(os.path.dirname(__file__),
LIGHT_PROFILES_FILE),
hass.config.path(LIGHT_PROFILES_FILE)]
profiles = {}

for profile_path in profile_paths:
if not os.path.isfile(profile_path):
continue
with open(profile_path) as inp:
reader = csv.reader(inp)

# Skip the header
next(reader, None)

try:
for rec in reader:
profile, color_x, color_y, brightness = PROFILE_SCHEMA(rec)
profiles[profile] = (color_x, color_y, brightness)
except vol.MultipleInvalid as ex:
_LOGGER.error("Error parsing light profile from %s: %s",
profile_path, ex)
return None
return profiles
class Profiles:
"""Representation of available color profiles."""

_all = None

@classmethod
@asyncio.coroutine
def load_profiles(cls, hass):
"""Load and cache profiles."""
def load_profile_data(hass):
"""Load built-in profiles and custom profiles."""
profile_paths = [os.path.join(os.path.dirname(__file__),
LIGHT_PROFILES_FILE),
hass.config.path(LIGHT_PROFILES_FILE)]
profiles = {}

for profile_path in profile_paths:
if not os.path.isfile(profile_path):
continue
with open(profile_path) as inp:
reader = csv.reader(inp)

# Skip the header
next(reader, None)

try:
for rec in reader:
profile, color_x, color_y, brightness = \
PROFILE_SCHEMA(rec)
profiles[profile] = (color_x, color_y, brightness)
except vol.MultipleInvalid as ex:
_LOGGER.error(
"Error parsing light profile from %s: %s",
profile_path, ex)
return None
return profiles

cls._all = yield from hass.loop.run_in_executor(
None, load_profile_data, hass)
return cls._all is not None

@classmethod
def get(cls, name):
"""Return a named profile."""
return cls._all.get(name)


class Light(ToggleEntity):
Expand Down
9 changes: 4 additions & 5 deletions homeassistant/components/light/lifx/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@

from homeassistant.components.light import (
Light, DOMAIN, PLATFORM_SCHEMA, LIGHT_TURN_ON_SCHEMA,
ATTR_BRIGHTNESS, ATTR_COLOR_NAME, ATTR_RGB_COLOR,
ATTR_BRIGHTNESS, ATTR_RGB_COLOR,
ATTR_XY_COLOR, ATTR_COLOR_TEMP, ATTR_TRANSITION, ATTR_EFFECT,
SUPPORT_BRIGHTNESS, SUPPORT_COLOR_TEMP, SUPPORT_RGB_COLOR,
SUPPORT_XY_COLOR, SUPPORT_TRANSITION, SUPPORT_EFFECT)
SUPPORT_XY_COLOR, SUPPORT_TRANSITION, SUPPORT_EFFECT,
preprocess_turn_on_alternatives)
from homeassistant.config import load_yaml_config_file
from homeassistant.util.color import (
color_temperature_mired_to_kelvin, color_temperature_kelvin_to_mired)
Expand Down Expand Up @@ -434,9 +435,7 @@ def find_hsbk(self, **kwargs):
if hsbk is not None:
return [hsbk, True]

color_name = kwargs.pop(ATTR_COLOR_NAME, None)
if color_name is not None:
kwargs[ATTR_RGB_COLOR] = color_util.color_name_to_rgb(color_name)
preprocess_turn_on_alternatives(kwargs)
Copy link
Copy Markdown

@houndci-bot houndci-bot May 14, 2017

Choose a reason for hiding this comment

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

undefined name 'preprocess_turn_on_alternatives'

Copy link
Copy Markdown
Member

@balloob balloob May 15, 2017

Choose a reason for hiding this comment

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

Why would LIFX need this logic?

Copy link
Copy Markdown
Contributor Author

@amelchio amelchio May 15, 2017

Choose a reason for hiding this comment

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

This is because LIFX effects (and also lifx_set_state now) accept the same parameter names as light.turn_on, without actually going through the turn_on handler.

I previously wrote it out for color_name, but now with four alternative names I thought it to be too much duplication.


if ATTR_RGB_COLOR in kwargs:
hue, saturation, brightness = \
Expand Down
11 changes: 7 additions & 4 deletions homeassistant/components/light/lifx/effects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import voluptuous as vol

from homeassistant.components.light import (
DOMAIN, ATTR_BRIGHTNESS, ATTR_COLOR_NAME, ATTR_RGB_COLOR, ATTR_EFFECT,
ATTR_TRANSITION)
DOMAIN, ATTR_BRIGHTNESS, ATTR_BRIGHTNESS_PCT, ATTR_COLOR_NAME,
ATTR_RGB_COLOR, ATTR_EFFECT, ATTR_TRANSITION,
VALID_BRIGHTNESS, VALID_BRIGHTNESS_PCT)
from homeassistant.const import (ATTR_ENTITY_ID)
import homeassistant.helpers.config_validation as cv

Expand Down Expand Up @@ -36,7 +37,8 @@
})

LIFX_EFFECT_BREATHE_SCHEMA = LIFX_EFFECT_SCHEMA.extend({
ATTR_BRIGHTNESS: vol.All(vol.Coerce(int), vol.Clamp(min=0, max=255)),
ATTR_BRIGHTNESS: VALID_BRIGHTNESS,
ATTR_BRIGHTNESS_PCT: VALID_BRIGHTNESS_PCT,
ATTR_COLOR_NAME: cv.string,
ATTR_RGB_COLOR: vol.All(vol.ExactSequence((cv.byte, cv.byte, cv.byte)),
vol.Coerce(tuple)),
Expand All @@ -49,7 +51,8 @@
LIFX_EFFECT_PULSE_SCHEMA = LIFX_EFFECT_BREATHE_SCHEMA

LIFX_EFFECT_COLORLOOP_SCHEMA = LIFX_EFFECT_SCHEMA.extend({
ATTR_BRIGHTNESS: vol.All(vol.Coerce(int), vol.Clamp(min=0, max=255)),
ATTR_BRIGHTNESS: VALID_BRIGHTNESS,
ATTR_BRIGHTNESS_PCT: VALID_BRIGHTNESS_PCT,
vol.Optional(ATTR_PERIOD, default=60):
vol.All(vol.Coerce(float), vol.Clamp(min=0.05)),
vol.Optional(ATTR_CHANGE, default=20):
Expand Down
10 changes: 9 additions & 1 deletion homeassistant/components/light/services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ turn_on:

color_temp:
description: Color temperature for the light in mireds
example: '250'
example: 250

kelvin:
description: Color temperature for the light in Kelvin
example: 4000

white_value:
description: Number between 0..255 indicating level of white
Expand All @@ -36,6 +40,10 @@ turn_on:
description: Number between 0..255 indicating brightness
example: 120

brightness_pct:
description: Number between 0..100 indicating percentage of full brightness
example: 47

profile:
description: Name of a light profile to use
example: relax
Expand Down
5 changes: 5 additions & 0 deletions tests/components/light/test_demo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ def test_state_attributes(self):
self.assertEqual(154, state.attributes.get(light.ATTR_MIN_MIREDS))
self.assertEqual(500, state.attributes.get(light.ATTR_MAX_MIREDS))
self.assertEqual('none', state.attributes.get(light.ATTR_EFFECT))
light.turn_on(self.hass, ENTITY_LIGHT, kelvin=4000, brightness_pct=50)
self.hass.block_till_done()
state = self.hass.states.get(ENTITY_LIGHT)
self.assertEqual(250, state.attributes.get(light.ATTR_COLOR_TEMP))
self.assertEqual(127, state.attributes.get(light.ATTR_BRIGHTNESS))

def test_turn_off(self):
"""Test light turn off method."""
Expand Down