home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.23k stars 30.58k forks source link

TPLink light.turn_on with brightness does not turn on light #57265

Closed jshank closed 3 years ago

jshank commented 3 years ago

The problem

Calling service light.turn_on with a TPLink HS220 using either brightness_pct or brightness values will set the turn-on brightness level but will not actually turn on the light. light.turn_on without a brightness value has to be called a second time to actually turn on the light. This only happens when the light is previously in an off state. If the light is on and light.turn_on is called with a brightness state, the light will change immediately to the new brightness variable.

service: light.turn_on
target:
  entity_id: light.office_overheads
data:
  brightness_pct: 85
Status Behavior
Light is off Brightness set but light remains off
Light is on Light is changed to 85% brightness

The same behavior occurs with light.toggle making the following automation work incorrectly

- id: Office light toggle with office remote A
  alias: Office light on with remote

  trigger:
    platform: event
    event_type: isy994_control
    event_data:
      entity_id: sensor.office_remote_a
      control: 'DON'
  action:
    service: light.toggle
    data:
      entity_id: light.office_overheads
      brightness_pct: 100

What is version of Home Assistant Core has the issue?

core-2021.10.0

What was the last working version of Home Assistant Core?

core-2021.9.0

What type of installation are you running?

Home Assistant Container

Integration causing the issue

tpllink

Link to integration documentation on our website

https://www.home-assistant.io/integrations/tplink/

Example YAML snippet

service: light.turn_on
target:
  entity_id: light.office_overheads
data:
  brightness_pct: 85

Anything in the logs that might be useful for us?

No errors, just unexpected behavior.

Additional information

No response

probot-home-assistant[bot] commented 3 years ago

tplink documentation tplink source (message by IssueLinks)

probot-home-assistant[bot] commented 3 years ago

Hey there @rytilahti, @thegardenmonkey, mind taking a look at this issue as it has been labeled with an integration (tplink) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

rytilahti commented 3 years ago

Looks like the upstream handles this inconsistently, created https://github.com/python-kasa/python-kasa/issues/230

Could you try if it turns the light on if you specify transition to the service call?

jshank commented 3 years ago

It does not turn on correctly when adding transition to the service call. As an aside, I did confirm by rolling the docker image back to image: "homeassistant/home-assistant:2021.9" and the light behaves correctly.

service: light.turn_on
target:
  entity_id: light.office_overheads
data:
  transition: 2
  brightness_pct: 85
2021-10-07 18:52:51 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [140328414538048] Error handling message: Unknown error
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/websocket_api/decorators.py", line 26, in _handle_async_response
    await func(hass, connection, msg)
  File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 525, in handle_execute_script
    await script_obj.async_run(msg.get("variables"), context=context)
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 1219, in async_run
    await asyncio.shield(run.async_run())
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 353, in async_run
    await self._async_step(log_exceptions=False)
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 371, in _async_step
    await getattr(self, handler)()
  File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 571, in _async_call_service_step
    await service_task
  File "/usr/src/homeassistant/homeassistant/core.py", line 1491, in async_call
    task.result()
  File "/usr/src/homeassistant/homeassistant/core.py", line 1526, in _execute_service
    await handler.job.target(service_call)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 213, in handle_service
    await self.hass.helpers.service.entity_service_call(
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 658, in entity_service_call
    future.result()  # pop exception if have
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 830, in async_request_call
    await coro
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 695, in _handle_entity_call
    await result
  File "/usr/src/homeassistant/homeassistant/components/light/__init__.py", line 462, in async_handle_light_on_service
    await light.async_turn_on(**filter_turn_on_params(light, params))
  File "/usr/src/homeassistant/homeassistant/components/tplink/entity.py", line 24, in _async_wrap
    await func(self, *args, **kwargs)
  File "/usr/src/homeassistant/homeassistant/components/tplink/light.py", line 88, in async_turn_on
    await self.device.set_brightness(brightness, transition=transition)
  File "/usr/local/lib/python3.9/site-packages/kasa/smartdevice.py", line 76, in wrapped
    return await f(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/kasa/smartdimmer.py", line 81, in set_brightness
    return await self.set_dimmer_transition(brightness, transition)
  File "/usr/local/lib/python3.9/site-packages/kasa/smartdimmer.py", line 124, in set_dimmer_transition
    raise ValueError(
ValueError: ('Transition must be integer, not of %s.', <class 'float'>)
krazos commented 3 years ago

I am also having an issue with the HS220 dimmer switch where the light.turn_on service fails when including a transition parameter. It appears that the TP-Link integration requires an integer value for transition, whereas the light.turn_on service is sending a floating value. I noticed this immediately after the upgrade to 2021.10 because my Circadian Lighting custom component began filling my logs with errors:

Logger: homeassistant.core
Source: components/tplink/light.py:88
First occurred: 3:14:13 PM (1 occurrences)
Last logged: 3:14:13 PM

Error executing service: <ServiceCall light.turn_on (c:3e38788ef2f0761ea34a7e635e36be54): entity_id=['light.kitchen_lights'], params=transition=1.0, brightness=254>
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/core.py", line 1507, in catch_exceptions
    await coro_or_task
  File "/usr/src/homeassistant/homeassistant/core.py", line 1526, in _execute_service
    await handler.job.target(service_call)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 213, in handle_service
    await self.hass.helpers.service.entity_service_call(
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 658, in entity_service_call
    future.result()  # pop exception if have
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 830, in async_request_call
    await coro
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 695, in _handle_entity_call
    await result
  File "/usr/src/homeassistant/homeassistant/components/light/__init__.py", line 462, in async_handle_light_on_service
    await light.async_turn_on(**filter_turn_on_params(light, params))
  File "/usr/src/homeassistant/homeassistant/components/tplink/entity.py", line 24, in _async_wrap
    await func(self, *args, **kwargs)
  File "/usr/src/homeassistant/homeassistant/components/tplink/light.py", line 88, in async_turn_on
    await self.device.set_brightness(brightness, transition=transition)
  File "/usr/local/lib/python3.9/site-packages/kasa/smartdevice.py", line 76, in wrapped
    return await f(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/kasa/smartdimmer.py", line 81, in set_brightness
    return await self.set_dimmer_transition(brightness, transition)
  File "/usr/local/lib/python3.9/site-packages/kasa/smartdimmer.py", line 124, in set_dimmer_transition
    raise ValueError(
ValueError: ('Transition must be integer, not of %s.', <class 'float'>)
jshank commented 3 years ago

@rytilahti, does https://github.com/home-assistant/core/pull/57272 address the issue with the light not turning on correctly when brightness is added or only fixed the secondary transition error?

calisro commented 3 years ago

https://github.com/home-assistant/core/issues/57252

Same issue. Might want to close the dup.

rytilahti commented 3 years ago

@krazos @jshank the linked PR fixes only the incorrect passing of the transition. I suggested testing it out as the docstring (https://github.com/python-kasa/python-kasa/blob/master/kasa/smartdimmer.py#L62) states that the bulb gets turned on then.

Fixing this issue requires changes to the backend lib (or alternatively, modifying the integration to call turn_on() after just setting the brightness). @jshank the new implementation has diverged from the previous implementation so much, that this will require some investigation and someone with access to a device to test the fix. I'm thinking about making a hack to call turn_on() inside the integration as a quick fix, @bdraco, what do you think about that?

bdraco commented 3 years ago

@rytilahti If transition turns it on, how about setting brightness with the shortest transition possible if none is specified to force it on?

rytilahti commented 3 years ago

@bdraco that's the big question: whether it does that or not? If someone could try if kasa --host <address> raw-command smartlife.iot.dimmer set_dimmer_transition '{"brightness": 80, "duration": 1}' does indeed turn the light on, then that's a good stopgap solution.

bdraco commented 3 years ago

I don't have any dimmers hooked up at the moment, or any free test wire harnesses.

If someone can't test by tomorrow I'll do some wiring to get one in a testable state.

TheGardenMonkey commented 3 years ago

Tested against my hs220 and yes that turns it on and set to 80% brightness.

jshank commented 3 years ago

Also confirmed that it works against my HS220. I turned the light off, then executed the following command. The light turned on to 80% brightness (brightness: 204).

bash-5.1# kasa --host 10.1.12.220 raw-command smartlife.iot.dimmer set_dimmer_transition '{"brightness": 80, "duration": 1}'
No --strip nor --bulb nor --plug given, discovering..
{}

Just for posterity's sake, the hardware is TPLink HS220 Firmware: 1.5.11 Build 200214 Rel.152651

keystone16 commented 3 years ago

57286 was just closed but concerns a different TP-Link issue than odd dimmer behaviour:

The problem With the 2021.10.0, only 5 of my 28 TP-Link Kasa HS105 (smart plug --all firmware 1.5.6) & HS200 (light switch --all firmware 1.2.5) devices can be connected. The HS110's (smart monitoring plugs) are all found. None of these devices are dimmers.

I'm reverting back to 2021.9.7 until this can be resolved.

What is version of Home Assistant Core has the issue? 2021.10.0

What was the last working version of Home Assistant Core? 2021.9.7

What type of installation are you running? Home Assistant Supervised

Integration causing the issue TP-Link Kasa

rytilahti commented 3 years ago

Please re-test with the .1 release when it gets released. That release will also improve the behavior for automatically discovered devices as the discovery will automatically be retried every 15 minutes (see #57221), so please let it run some time. If it's still not working, please feel free to re-open the issue you linked with logs more details about your setup. If the improvements in .1 don't fix it, the cause is unknown but might be related to https://github.com/python-kasa/python-kasa/issues/229 which needs more input & investigation.

keystone16 commented 3 years ago

Wonderful, thank you. I will retest the connections later with .1

calisro commented 3 years ago

They definitely appear to be working better for turn ons. Will test more.

As an aside (not relevant to this ticket) but on discovery, they are still not replacing the old entities. They are creating new ones with a _2. This is a minor issue if the user knows how to delete the old ones and rename the new ones but I thought i'd mention it. Seems to happen only with the HS2xx ones which I thought was interesting.

keystone16 commented 3 years ago

No, this wasn't a case of new entities being creates for my HS200 and HS105 switches -- the integrations tab listed all Kasa devices with most not connected and no new entities were created.

I've quickly upgraded to 2021.10.1 and the devices have now been connected. I'll see how this goes through the day.

bdraco commented 3 years ago

As an aside (not relevant to this ticket) but on discovery, they are still not replacing the old entities. They are creating new ones with a _2. This is a minor issue if the user knows how to delete the old ones and rename the new ones but I thought i'd mention it. Seems to happen only with the HS2xx ones which I thought was interesting.

The old integration used different unique id formats for switch and light

Unfortunately it looks like since dimmers were special in the old code and they used the unique id format of switches instead of lights

https://github.com/home-assistant/core/blob/dev/homeassistant/components/tplink/switch.py#L57 https://github.com/home-assistant/core/blob/dev/homeassistant/components/tplink/light.py#L61

I think we can fix it by using the switch format for dimmers. Unfortunately everyone who already migrated their _2 entities will have to do it again since it will switch back. But at least those people know how to do it.

calisro commented 3 years ago

I think we can fix it by using the switch format for dimmers. Unfortunately everyone who already migrated their _2 entities will have to do it again since it will switch back. But at least those people know how to do it.

It took a few minutes. Would probably help the masses though.