Skip to content

[RFC] Set color/brightness for turned off lights#7477

Closed
amelchio wants to merge 4 commits into
home-assistant:devfrom
amelchio:light-off_state
Closed

[RFC] Set color/brightness for turned off lights#7477
amelchio wants to merge 4 commits into
home-assistant:devfrom
amelchio:light-off_state

Conversation

@amelchio
Copy link
Copy Markdown
Contributor

@amelchio amelchio commented May 7, 2017

Description:

Background

The LIFX light API has a very handy feature: setting the color/brightness without actually turning the light on. Philips Hue apparently does not support this functionality natively though it is frequently requested, like here: https://developers.meethue.com/content/changing-color-while-onfalse

This PR is a proposal to add that feature to Home Assistant.

Setting the color of a light that is turned off might sound odd at first and I have seen it rejected as absurd elsewhere. However, this functionality is very useful for decoupling color and power changes. If the above link is not convincing, I can give several other examples.

The easiest way to see that it is not absurd is probably to compare to media players where the volume can be set even with nothing playing.

Implementation

A new power_on attribute is added to the light.turn_on call. Setting this to False will send just the color change, not the power on command. Thus, the light stays at its current power state.

As this is not supported by all lights, a SUPPORT_OFF_STATE flag is also added. This is used by the Flux switch to fix long-standing issues like #4005 (but only for supporting lights).

Using power_on: False without SUPPORT_OFF_STATE can be handled in a couple of ways. I think the best alternative is probably to leave it undefined, for the platform to handle.

Alternatives

I think light.turn_on is already too busy and this change makes it even worse. Also, to turn_on a light without powering it on sounds a bit weird.

It would IMHO be better to split the current functionality into light.turn_on, light.set_color, light.effect_start etc. The proposal would then be to add optional power control to set_color. I would be happy to work on such a split but it would be quite disruptive so I did not go down that path yet.

Because I actually don’t think this proposal is the best solution in an absolute sense I have marked the PR with [RFC]. If you feel like shooting down the implementation, please indicate whether you at least agree that the fundamental feature itself is useful.

Further work

Personally, I think that this feature is so useful that partial SUPPORT_OFF_STATE could be considered for hue and other platforms as well. This would involve a platform cache that adds the last set color to the turn_on call that eventually turns a light on.

It would only be a partial solution because the cache would not be applied if an external switch toggles a light without going through HA. It could, however, fix #4005 for Hue as well.

Related issue (if applicable): fixes #

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):

automation:
  # Reset all lights from our evening fun, but do not light up the whole house.
  - alias: Bright morning light
    trigger:
      platform: time
      after: '06:00'
    action:
      - service: light.turn_on
        data:
          color_temp: 200
          brightness: 200
          transition: 180
          power_on: false

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.

@mention-bot
Copy link
Copy Markdown

@amelchio, thanks for your PR! By analyzing the history of the files in this pull request, we identified @HBDK, @jawilson and @nkgilley to be potential reviewers.

Comment thread homeassistant/components/switch/flux.py Outdated
Copy link
Copy Markdown

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)

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,
vol.Optional(ATTR_POWER_ON, default=True): cv.boolean,
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 just doesn't make much sense… turn on with power off? 😛 I wonder if we should extend the turn_off schema instead?

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.

btw, this idea came up before and I have shot it down because of proposed implementations (things like a set_state command etc).

As far as I know LIFX is the only light that allows changing settings while it's off. So I suggest you add it as a LIFX only service. If we ever have more lights than we can maybe generalize it.

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.

Yeah, I think I touched on both points in the "Alternatives" section. The idea comes up repeatedly because it's really useful and we miss it when converting from IFTTT 😄

I'm cool with starting out with a LIFX only service (though that will not fix the Flux+motion sensor race). So that will indeed be something like lifx_set_state – or maybe lifx_set_color. I will look into that.

If I am creating a new service call, will it be acceptable to use the units that are well-known from the LIFX smartphone app? This is Kelvin for temperature and percentage for brightness.

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.

It's a lifx only service so I don't really care. But I feel like it would make sense to keep them in sync with the rest of Hass. So rather not use the app units.

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.

Oh, and I must admit that I did not read the whole PR intro…

@amelchio amelchio closed this May 8, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Aug 12, 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.

Light component always sending the 'on' attribute, making it impossible to use physical switches together with HA.

6 participants