Skip to content
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.

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

Toggle (after off with a transition) dont set brightness to previous level #58

Closed
htvekov opened this issue Mar 15, 2020 · 31 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@htvekov
Copy link
Contributor

htvekov commented Mar 15, 2020

Hi' Xavi

There's an issue with toggle function for Ikea bulbs on z2m integration (perhaps also other integrations as well. Haven't had time yet to check Hue bridge behaviour)

If lights are turned off with any transition period specified, then lights will only come on again at lowest brightness. HA state has correct brightness, but bulb has not !!

Example: Automation for a hallway sensor to turn off a group of hallway lights after a certain period with no movement. This is done with a transition period for eg. 1 seconds
If lights afterwards are turned on again via ControllerX app (toggle button), then previous brightness is not restored to bulbs - only HA states.

Solution: Always call toggle event with transition attribute set

    @action
    async def off(self, **attributes):
        if "transition" not in attributes:
            attributes["transition"] = self.transition / 1000
        self.call_service("homeassistant/turn_off", entity_id=self.light["name"], transition=attributes["transition"])

    @action
    async def toggle(self, **attributes):
        if "transition" not in attributes:
            attributes["transition"] = self.transition / 1000
        self.call_service("homeassistant/toggle", entity_id=self.light["name"], transition=attributes["transition"])

Above change to lightcontroller.py should handle this nicely.
Can't think of any other problems above change would do to other aspects of app ?

I've included transition to turn_off function as well, just to make it turn off nicely with a transition - if specified 😊

Tested for now with both Ikea WS (White Spectrum) and Ikea color bulbs on z2m.
Not tested yet: Ikea and Hue bulbs behavior on Hue bridge.

  • Devices involved:
  • Model: IKEA E1810 as Light Controller
  • Integration: z2m
  • AppDaemon version: v4.0.2.3
  • ControllerX version: v2.4.2
  • HACS version (if installed from there): v0.23.2
  • Home Assistant Core version: v0.106.6

AppDaemon app configuration

office_test:
  module: controllerx
  class: E1810Controller
  controller: sensor.0xec1bbdfffe1b99e8_action #Ikea 5 button remote
  integration: z2m
  #transition: 1000 #transition attribute works on Hue bridge
  smooth_power_on: true
  light: light.0x90fd9ffffe838cb3_light # Color bulb office
  actions:
    - arrow_left_hold
    - arrow_left_release
    - arrow_right_hold
    - arrow_right_release
    - arrow_right_click
    - arrow_left_click
    - brightness_up_click
    - brightness_down_click
    - brightness_up_release
    - brightness_down_release
    - brightness_up_hold
    - brightness_down_hold
    - toggle
    - toggle_hold
@htvekov htvekov added the bug Something isn't working label Mar 15, 2020
@xaviml
Copy link
Owner

xaviml commented Mar 15, 2020

Hi @htvekov

Thank you for spotting that out! Actually, this is a bug, the transition is only added to the on functionality and not to the off and toggle. So not only will this fix the bug you spotted, but also it will add a transition to off and_toggle_ as it should.

I will get back to this as soon as possible.

@htvekov
Copy link
Contributor Author

htvekov commented Mar 15, 2020

You're welcome, Xavi 😊

I'll test behavior on Hue bridge tomorrow and get back to you with the test results.

Ciao !

@xaviml
Copy link
Owner

xaviml commented Mar 16, 2020

Hi @htvekov

I created v2.4.4b0 with the code you suggested. You can test with Hue bridge integration with v2.4.3 and see if gets fixed with v2.4.4b0. Thank you :)

@htvekov
Copy link
Contributor Author

htvekov commented Mar 16, 2020

Hi' Xavi.

Just tested Ikea white spectrum bulb and Hue bulb on Hue bridge.
Both behaves similar regarding the 'missing brightness toggle' issue.

Sorry, it doesn't work. Brightness state will NOT be send to bulb only HA.
Not even if I strip all attributes and only send the transition attribute 😣

Toggle only works on Hue bridge, and set brightness state at bulb correctly, if toggle is called without any attributes at all. But then it won't work on z2m integration as the transition is needed there...

As brightness is omitted for bulbs turned off on Hue bridge, I assume that no brightness attribute is included when entity_id=self.light["name"], **attributes is used. But that Hue bridge want's 'all' attributes or 'nothing' was unexpected to me ?

Only way I believe to solve toggle working correctly on both z2m and Hue integration, is to determine if a bulb/group is belonging to a Hue integration and 'flag' entities respectively. Hue groups has the is_hue_group: true as identifier, but single bulbs do not have this attribute.
Best way I imagine is to check if valid brightness attribute can be read. If no valid brightness attribute can be read from entity, then 'assume' that bulb is Hue integration. And when toggle and off is called then all attributes should be omitted.

A more simple solution would to indicate in app that bulb/group was Hue integration and toggle accordingly without attributes. Not the most 'elegant' solution, but it will work.

What are your thoughts on this, Xavi ?

@htvekov
Copy link
Contributor Author

htvekov commented Mar 16, 2020

Hmm...

My brain was not fully in gear 10 min. ago 🙄😃

It will be complicated to determine a bulb's integration type based on that, when you can only destinguish between integration types when bulbs are off.

And you can't even just make a quick check on brightness value (present/not present) before calling toggle with/without attributes.

Because, when toggle (off) for a Hue bulb has been called with attributes, it WONT set brightness on next toggle (on) even if attributes is omitted.
If turned off with attributes, it has to be turned on again with attributes...

Can't see any other solution that to add line to app to indicate Hue bridge integration.

@htvekov
Copy link
Contributor Author

htvekov commented Mar 16, 2020

Hmm...

One final thought for now how to differentiate z2m from Hue integration is to check for STATE attribute. Not 'the state' of bulb but separate attribute visible as z2m attribute:

image

This attribute is never present in Hue integration.

Just a thought... 🤔😊

Ciao !

@htvekov
Copy link
Contributor Author

htvekov commented Mar 17, 2020

Hi' again, Xavi.

I've given this issue some more thoughts and tested some more.

It's the HA's Hue integration that actually is flawed and needs to be changed in order to keep brightness value for bulbs - even if they're off. Just as with z2m integration.

But if that can't be changed, there's still hope ! 😉
HA actually stores (or pulls from bridge ?) the previous brightness attribute when lights are turned on again/toggled on again. So you CAN actually read correct brightness attribute, just after HA state changes to on.

There's one solution that would work for def_toggle function:

- determine bulb state.
   - If on, then just call toggle service as usual and end.
   - if off, then read brightness attribute.
      - If brightness attribute present, then bulb is onz2m integration (and hopefully deCONZ, ZHA ?)
         - Hue flag: false
      - If brightness attribute not present.
         - Hue flag: true
- set transition attributes as usual
- call toggle service with attributes
   - if Hue flag: false
      - end
- wait for bulb state to change to on, just a few ms.  delay or perhaps not needed at all ?
- read brightness attribute for entity
- call turn_on service with brightness attribute

If I could write it in Python myself, I would have done it 😏

I've tested this process and 'emulated' via HA service calls and patched lightcontroller.py
It works !

I'm ready to test, if you wan't to give it a go, Xavi !

Cheers !

@htvekov
Copy link
Contributor Author

htvekov commented Mar 17, 2020

FYI

Brightness issue with Hue integration has now been filed at home-assistant/core.
home-assistant/core#32894

@xaviml
Copy link
Owner

xaviml commented Mar 17, 2020

Hi @htvekov,

I have been testing this issue today with z2m integration. I realised with ControllerX v2.4.4b0 the toggle does not work properly. Sometimes it turns on the light to different brightness that it was (what you describe in the issue).

However, if I go back to the stable release v2.4.3 I cannot reproduce the problem that you are describing. What I did for testing was to change the brightness through HA, then turn off the light through HA and then turn on the light with the controller. If I repeat this with v2.4.4b0 is when I spot the problem.

This means that the problem is only when adding the transition (as you are saying), but v2.4.3 did not add any transition for the toggle and off actions.

Since this seems to be a bug from the Hue bridge integration, I would prefer to leave the code as it is and not add any patch code in there. I would like to keep the code as clean as possible. What I do in the call is HA call services, so if that does not work, the problem will need to be fixed on the HA end and not from ControllerX.

Therefore, I will remove the transitions from the toggle and off actions.

Thank you for your comprehension and your help @htvekov !

@htvekov
Copy link
Contributor Author

htvekov commented Mar 17, 2020

Evening, Xavi.

I've come to the same conclusion as you, that the goal must be to get integrations fixed at HA end.
But the major problem lies within the vast possibilities and combinations of many different zigbee bulb firmwares, on at least 4 different integration platforms on HA. This will end up in hundreds of combinations which all not will act in exact same way via HA service calls.

It will be an impossible task to bugfix, write and maintain code onwards for each and every small 'tweaks' needed for them all to work as expected with ControllerX.

The point and 'goal' of ControllerX, I believe, is mainly to 'mimic' original remote features with proprietary systems such as Ikea Trådfri, Philips Signify Hue etc. And this you've done extremely well I must say 👍😃 It works perfectly 'on it's own' !! But problems begins when ControllerX is combined with all the automation possibilities within HA. Then the 'bugs' crawl out, as no integration apparently handles all service calls in the same way.

It's not only Hue integration or Ikea bulbs firmware that are 'flawed'. Also z2m integration sometimes works in mysterious ways.

In my use case, the 'toggle (off) with transition' issue works even as badly on z2m- as on Hue integration.

Test case: Ikea WS/color bulb on z2m and ControllerX v2.4.3
Toggle (off) called in HA with a transition attribute set
Requires toggle(on) with transition attribute set, for brightness to be restored correctly.

Test case: Ikea WS/color bulb on Hue and ControllerX v2.4.3
Toggle (off) called in HA with a transition attribute set
Requires toggle(on) with NO attributes at all, for brightness to be restored correctly.

I actually don't know what's 'worst' of the two mentioned test cases ??

Both Hue and Ikea Trådfri are quite widespread (here in Scandinavia at least) and has a large user base. It's really a shame that HA integrations regarding transition are not equally treated, as transitions is one of the visual details of 'smart lighting' that people notice and appreciate.

If current transition HA implementation is 'by design' or just a bug I can't tell 🤔
But nevertheless I'll try to get balloob to change the HA Hue integration code.
And perhaps I should open an issue as well on the z2m integration 😉

For now i'll ditch transitions in my automations for groups that coexists with a ControllerX app.
Or use my 'quick and dirty' patch with transition set to 1234, to distinguish between toggle with attributes or not. Then I can maintain transition on my x2m integration and use the Hue bridge 'standard' applied transition for lights on Hue. Best of 'two worlds' at the moment 😉

@action
    async def toggle(self, **attributes):
        if self.transition != 1234:
            attributes["transition"] = self.transition / 1000
        self.call_service(
            "homeassistant/toggle", entity_id=self.light["name"], **attributes
            )

But hopefully, one day, freely used transitions will get back if HA core integrations are bugfixed 🙏🙏 😋😎😎

THANK YOU, XAVI for all the hard work, all the fixes, beta's you've done !!
And last, but not least, thank you for reading all my extremely long and complicated comments and issues ! 😆😉👍

Ciao !

@htvekov
Copy link
Contributor Author

htvekov commented Mar 18, 2020

And reported Hue issue:

home-assistant/core#32894

@htvekov
Copy link
Contributor Author

htvekov commented Mar 18, 2020

Hi' Xavi.

Both issues reported now.
Are you on zigbee2mqtt-dev v1.11.0 as me or 'standard' branch ?

I've read some of the z2m issues regarding Ikea bulbs and transition and there seems to be some difference in how transition, brigthness and color/color_temp are called between the two branches.

That could perhaps explain, if you not always can replicate my findings ??

Anyway, fingers crossed about HA level fixes to the brightness issue 🤞🤞😉

@htvekov
Copy link
Contributor Author

htvekov commented Mar 21, 2020

Hi' Xavi.

One down - One to go 😃

Koenkk has made the neccesary changes in z2m converters.
I've tested this code change and brigtness will now be reset to previous level - even after off with a transition period.

Koenkk expects next zigbee release in appx. 2,5 weeks time or so.

Your need to re-add brightness to code for sync funtion to work after next zigbee release, though.
Otherwise brightness wont be restored from lights on - 'toggle_hold'.

This patch works (with latest zigbee2mqtt-edge version) for both Hue/Ikea lights on Hue and Ikea lights on z2m. All tested 😎🍻

@action
    async def sync(self):
        await self.on(brightness=self.max_brightness, transition=0)
        attributes = {}
        try:
            color_attribute = await self.get_attribute(LightController.ATTRIBUTE_COLOR)
            if color_attribute == LightController.ATTRIBUTE_COLOR_TEMP:
                attributes[color_attribute] = 370  # 2700K light
            else:
                self.index_color = 12  # white colour
                attributes[color_attribute] = self.colors[self.index_color]
        except:
            self.log("sync action will only change brightness", level="WARNING")
        if attributes != {}:
            #await self.on(xy_color = [0.323, 0.329], brightness = 255, transition = 1)
            await self.on(color_temp = 370 , brightness = 255, transition = 1)
            #await self.on(**attributes)

@xaviml
Copy link
Owner

xaviml commented Mar 26, 2020

Hi @htvekov,

I have been testing and I still experience the following either in z2m-stable and z2m-edge:

  • Turning the light from HA
  • Change the brightness to 50%
  • Turn off the light from HA
  • Call light.toggle service with transition = 0.3

Then the brightness is not set back to 50%, but to a different brightness (I did not pick up the pattern is following). However, if I remove the "transition" attribute, I do not experience this, it always goes back to the previous brightness. Do you experience this? Was this what you were experiencing or it was another thing?

Regarding the fix you are proposing and you say it works as expected, it has to be with a transition equal to 1 and send the brightness as well?

Thanks for debugging and testing! :)

@htvekov
Copy link
Contributor Author

htvekov commented Mar 26, 2020

Hi' Xavi.

Wow, we're GOOD at finding HA core bugs !! 🐛🐞😊
I've made loads and loads of tests, but I haven't noticed this one before.

This bug is probably the effect of an error in Koenkk's correction code, for the missing brightness attribute after off with a transition set (my z2m bug report).

There's actually a pattern to the brightness set error. The brightness set is ALWAYS the PREVIOUS brightness, until an off or toggle off is called with a transition attribute set. Then this brightness becomes new 'previous brightness'.

I'll raise a new issue at Zigbee converters Github for this, Xavi.

The issue I was reporting originally, was if lights off was called with a transition, then next on/toggle on specifically needed transition attribute set as well. Otherwise light would come on at brightness: 1

*** CUT - ORIGINAL ISSUE AT ZIGBEE CONVERTERS ***

  • light.turn_off/light.toggle(off) with a transition via HA service calls
  • light.turn_on/light.toggle(on) again via HA service call with transition attribute omitted completely
  • Previous brightness wont be restored to bulb, even though HA states correct attribute value.
    Brightness is actually at 1.

Expected behaviour: light.toggle(on)/light.turn_on without any attributes at all should restore bulb to previous state - color/color_temp and brightness.

*** END CUT ***

Besides this new bug (which I'm sure Koenkk will fix), the new Edge version works flawlessly with both E1810 and E1743 with Ikea bulbs on z2m integration. Only issue is sync from lights on needs a brigtness attribute set in call to work.
Hue/Ikea bulbs on Hue integration needs the brightness attribute set as well to revover correctly when sync is called. But I've raised that issue in Home Asssistant core - Hue integration as well (No answer yet though)

All working here with this patch of ControllerX v2.4.3:

@action
   async def on(self, **attributes):
       if self.transition != 1234:
           attributes["transition"] = self.transition / 1000
       self.call_service(
           "homeassistant/turn_on", entity_id=self.light["name"], **attributes
       )

   @action
   async def off(self, **attributes):
       if self.transition != 1234:
           attributes["transition"] = self.transition / 1000
       self.call_service(
           "homeassistant/turn_off", entity_id=self.light["name"], **attributes
       )

   @action
   async def toggle(self, **attributes):
       if self.transition != 1234:
           attributes["transition"] = self.transition / 1000
       self.call_service(
           "homeassistant/toggle", entity_id=self.light["name"], **attributes
           )
       # This included to test bulbs on Hue bridge - working with brightness attribute included
       #self.call_service(
       #    "homeassistant/turn_on", entity_id=self.light["name"], brightness = 247
       #)

@action
   async def sync(self):
       await self.on(brightness=self.max_brightness, transition=0)
       attributes = {}
       try:
           color_attribute = await self.get_attribute(LightController.ATTRIBUTE_COLOR)
           if color_attribute == LightController.ATTRIBUTE_COLOR_TEMP:
               attributes[color_attribute] = 370  # 2700K light
           else:
               self.index_color = 12  # white colour
               attributes[color_attribute] = self.colors[self.index_color]
       except:
           self.log("sync action will only change brightness", level="WARNING")
       if attributes != {}:
           #await self.on(xy_color = [0.323, 0.329], brightness = 255, transition = 1)

           # needed brightness attribute added for sync from lights on to work !
           await self.on(color_temp = 370 , brightness = 255, transition = 1)

           #await self.on(**attributes)

The transition: 1234 I use only for bulbs at Hue Bridge to remove transition attribute completely from calls. This fix is needed, until Balloob fixes the issue I've raised for the Hue integration.

In order for the sync funtion to work properly (Ikea bulbs on z2m-Edge) when toggled from on position, the brightness attribute has to be included in main call ( My ugly patch off course only works with dimmable only and wthite spectrum lights 😃)

Pheww, that was a long comment again, Xavi.

Sorry ! 😉🙋‍♂️

Ciao !

@htvekov
Copy link
Contributor Author

htvekov commented Mar 26, 2020

Forgot to answer your specific end question.

No just any transition attribute will do - Even transition: 0 will work 😊
But the attribute has to be present.

And the brightness attribute as well, of course. Here you can hardcode it, as you allways wants full brightness.

@htvekov
Copy link
Contributor Author

htvekov commented Mar 26, 2020

Just reported the issue now, Xavi

@htvekov
Copy link
Contributor Author

htvekov commented Mar 28, 2020

Hi' Xavi.

a small update.
Koenkk is on 'the new brightness/transition' issue 😃

Actually he gave this reply to why we experince these transition/brightness errors:

Some background on why you get these bugs when using transition: Zigbee does not have commands to turn on/off lights with a transition (besides one with a fixed 0.8 transition). Therefore we turning a bulb off with a transition, we set the brightness to 0. When the bulb is turned on again we need to restore this transition.

I had NO idea that transition (besides default transition: 0,8) was NOT handled by direct zigbee commands to bulbs own firmware...

No wonder, we experience irratic behaviour, when all is 'emulated' by zigbee converters.

Holy crap, what I mess I've started... 🙄🤔😏😁🤞🤞

Ciao !

@xaviml
Copy link
Owner

xaviml commented Mar 29, 2020

Hi @htvekov,

I just tried the fix from Koenkk and it is all fixed now. I will work on the change for the bug in the sync action.

Thanks for all the debugging. I will keep you posted :)

@htvekov
Copy link
Contributor Author

htvekov commented Mar 29, 2020

Your're welcome, Xavi 👍

Could you please leave a 'backdoor' (secret or official 😉) to completely remove transition attributes on light.on, light.off and toggle calls for lights connnected via Hue integration ?
Eg. via transition: 1234 as I did in my 'quick and ugly' patch ?
That would save me from patching your code over and over again 😁, until Balloob hopefully one day fixes the error in Hue integration.

Thanks Xavi and Ciao ! 🙋‍♂️

@xaviml
Copy link
Owner

xaviml commented Apr 8, 2020

Hi @htvekov,

Sorry for the delay, I have been busy with work. I just deployed v2.4.4b2 with a new attribute add_transition that by default is true. You can add it to false and transition won't be sent to the HA call services, so you can use it for your Hue integration. I also do not see any other problem with the sync functionality with z2m. Do you still experience (with this last release) it or it got fixed with the fix from Koenkk?

I hope you, your family and friends are safe and doing well.

@htvekov
Copy link
Contributor Author

htvekov commented Apr 8, 2020

Hi' Xavi.

Yep, luckily all family and friends are all safe and sound !
We have been 'hit' as well here in Denmark, but nowhere near as 'hard' as your home country or Italy for that matter 😢

I'm working in the private sector as Procurement Manager, so I've been working from home the last couple of weeks and will probably continue with that until May.

Hope you and your family are all safe as well, Xavi.

No worries about the delay. At the end of the day: It's 'just lights' ! 😉😁

Had a brief test with v2.4.4b2 and still with 'fixed' z2m_edge installed (all house has gone to bed so will have to test more tomorrow - Ikea bulbs on Hue bridge)

Hue bulbs on Hue bridge are all behaving nicely now with sync with the add_transition: false attribute added 👍😃🎉

But Ikea WS/Color bulbs on z2m still have 'issues' when sync from lights on (default transition)
Color is restored, but brightness remains at level before sync is called.
Below z2m log for sync from lights on:

zigbee2mqtt:debug 2020-04-08 23:05:59: Received MQTT message on 'zigbee2mqtt/Office bed lamp/set' with data '{"state": "ON", "transition": 0.0, "brightness": 255}'
zigbee2mqtt:debug 2020-04-08 23:05:59: Publishing 'set' 'brightness' to 'Office bed lamp'
zigbee2mqtt:debug 2020-04-08 23:06:00: Received MQTT message on 'zigbee2mqtt/Office bed lamp/set' with data '{"state": "ON", "color": {"x": 0.324, "y": 0.329}, "transition": 0.3}'
zigbee2mqtt:debug 2020-04-08 23:06:00: Publishing 'set' 'state' to 'Office bed lamp'
zigbee2mqtt:info 2020-04-08 23:06:00: MQTT publish: topic 'zigbee2mqtt/Office bed lamp', payload '{"state":"ON","color":{"x":0.477,"y":0.196},"brightness":255,"update_available":false,"linkquality":92}'
zigbee2mqtt:debug 2020-04-08 23:06:00: Publishing 'set' 'transition' to 'Office bed lamp'
zigbee2mqtt:info 2020-04-08 23:06:00: MQTT publish: topic 'zigbee2mqtt/Office bed lamp', payload '{"state":"ON","color":{"x":0.477,"y":0.196},"brightness":49,"update_available":false,"linkquality":92}'
zigbee2mqtt:debug 2020-04-08 23:06:00: Publishing 'set' 'color' to 'Office bed lamp'
zigbee2mqtt:debug 2020-04-08 23:06:00: Received Zigbee message from 'Office Ikea 5 button', type 'read', cluster 'genBasic', data '["appVersion"]' from endpoint 1 with groupID 901
zigbee2mqtt:debug 2020-04-08 23:06:00: No converter available for 'E1524/E1810' with cluster 'genBasic' and type 'read' and data '["appVersion"]'
zigbee2mqtt:info 2020-04-08 23:06:00: MQTT publish: topic 'zigbee2mqtt/Office bed lamp', payload '{"state":"ON","color":{"x":0.324,"y":0.329},"brightness":49,"update_available":false,"linkquality":92}'
zigbee2mqtt:debug 2020-04-08 23:06:00: Publishing 'set' 'transition' to 'Office bed lamp'

Can't really understand why ? But it seems to be that the ZIgbee converter defaults back to 'old' brightness level, if color command DOESN'T include the brigtness attribute ? I can't remember if this behaviour was also like that before Koenkk made the two bugfixes ? Seems to me that this is actually another bug ? I'll try and replicate behaviour tomorrow directly in HA via service calls.

Anyway, this is an easy fix, as I earlier in this thread wrote that brightness attribute MUST be set in 'main call' for Ikea bulbs to reset brightness correctly.

QUOTE CUT

In order for the sync funtion to work properly (Ikea bulbs on z2m-Edge) when toggled from on position, the brightness attribute has to be included in main call ( My ugly patch off course only works with dimmable only and wthite spectrum lights 😃

END QUOTE CUT

I patched your new beta, tested and Ikea bulbs on z2m works correctly now 😃

@action
    async def sync(self):
        await self.on(brightness=self.max_brightness, transition=0)
        attributes = {}
        try:
            color_attribute = await self.get_attribute(LightController.ATTRIBUTE_COLOR)
            if color_attribute == LightController.ATTRIBUTE_COLOR_TEMP:
                attributes[color_attribute] = 370  # 2700K light
            else:
                self.index_color = 12  # white colour
                attributes[color_attribute] = self.colors[self.index_color]
        except:
            self.log("sync action will only change brightness", level="WARNING")
        if attributes != {}:
            await self.on(**attributes, brightness = 255)

Adding the brightness as 'hardcoded' to main call as well, should not have any effect on other integrations to my knowledge. So I believe it's quite safe to make that change.

Ciao and keep safe, Xavi ! 🙂

@xaviml
Copy link
Owner

xaviml commented Apr 9, 2020

Interesting... Sorry, I did not see that you added the brightness in the second call as well. I removed it since it was called in the first line of the function. I will add that but instead of 255, I will add self.max_brightness which by default is 255, but it can be changed.

I will add this and see if this still works on my side too. I will keep you posted.

Thanks!

@htvekov
Copy link
Contributor Author

htvekov commented Apr 9, 2020

Hi' Xavi.

I'm getting old... 🙄
Forgot to mention that main call ALSO needs transition attribute (any value will do) as well, in order for brightness to be set correctly on Hue bulbs via Hue bridge.

So 'main call' has to include all attributes (color/color_temp, brightness and transition).
If transition is omitted, Hue bulbs will remain at brightness=1 when sync from on position is called.

Ikea bulbs on Hue bridge works with my old patch of v2.4.3 but I can't get it to work with latest beta, no matter what i do ? Really strange as def_sync function is unchanged, and same patch applied to new beta for no apparent reason don't work with Ikea bulbs on Hue bridge ?

@action
    async def sync(self):
        await self.on(brightness=self.max_brightness, transition=0)
        attributes = {}
        try:
            color_attribute = await self.get_attribute(LightController.ATTRIBUTE_COLOR)
            if color_attribute == LightController.ATTRIBUTE_COLOR_TEMP:
                attributes[color_attribute] = 370  # 2700K light
            else:
                self.index_color = 12  # white colour
                attributes[color_attribute] = self.colors[self.index_color]
        except:
            self.log("sync action will only change brightness", level="WARNING")
        if attributes != {}:
            #await self.on(**attributes)
            await self.on(color_temp = 370, brightness = 255, transition = 1)

Hmm... have to check this further...

Ciao !

@xaviml
Copy link
Owner

xaviml commented Apr 9, 2020

Hi @htvekov

So you need the transition attribute to be sent in there, but not for the other on, off, toggle actions for the Hue bridge integration?

By the way, good to hear you and your family are doing well :) My family and I are all good as well. At least, we got the inflection point, so now the restriction will start to relax a bit, but not too much.

@htvekov
Copy link
Contributor Author

htvekov commented Apr 9, 2020

Hi' Xavi.

Glad to hear that you're all well 😊👍🤞

Well this 'Ikea bulb on Hue bridge' really puzzles me ??
Bulb simply dont restore brightness - neither with sync from light on or off ?

I have tested and checked code again and again, but simply can't figure out why the hell my patch off v2.4.3 works perfectly, when same patch doesn't work on newest beta ?
All I'm doing is stripping attributes for toggle calls (v2.4.3) and adding brightness and transition to main call on both versions. How hard can it be ??

Same app, same environment, same everything....
Except of course that in patch i use transition: 1234 and in latest beta i use add_transition: false

This patch is working on v2.4.3. Same patch on latest beta (only the sync function) DOESN'T work ??

No attributes in toggle calls on both versions: Check !
Same HA toggle call in both versions: Check !
Exact same patch of sync function: Check !

WHY !!!?? 🤔😣😫🤪
I need a skilled programmer ! 😉😄

Things would be MUCH easier, if Baloob would fix the HA core Hue issue I've raised some three weeks ago. I assume that this minor bug has no priority in the big picture...

Feel free to tell me what I've missed, Xavi 😁

    @action
    async def toggle(self, **attributes):
        if self.transition != 1234:
            attributes["transition"] = self.transition / 1000
        self.call_service(
            "homeassistant/toggle", entity_id=self.light["name"], **attributes
            ) 

@action
    async def sync(self):
        await self.on(brightness=self.max_brightness, transition=0)
        attributes = {}
        try:
            color_attribute = await self.get_attribute(LightController.ATTRIBUTE_COLOR)
            if color_attribute == LightController.ATTRIBUTE_COLOR_TEMP:
                attributes[color_attribute] = 370  # 2700K light
            else:
                self.index_color = 12  # white colour
                attributes[color_attribute] = self.colors[self.index_color]
        except:
            self.log("sync action will only change brightness", level="WARNING")
        if attributes != {}:
            #await self.on(xy_color = [0.323, 0.329], brightness = 255, transition = 1) # THIS WORKS !!!
            await self.on(color_temp = 370 , brightness = 255, transition = 1)  # THIS WORKS !!!
            #await self.on(**attributes)

@htvekov
Copy link
Contributor Author

htvekov commented Apr 9, 2020

Argghh, once again I forgot to actually answer your questions..

For Hue bridge integration it's 'all or nothing' in all tests I've done.
You have to apply both color/color_temp, brightness and transition to call or totally omit all attributes.

And the Ikea bulbs fw can't do both color and brightness transition simultaniously. Hence the two calls needed for the z2m integration. First to set brightness quickly and second call to set color with an optional transition period.

@htvekov
Copy link
Contributor Author

htvekov commented Apr 9, 2020

Hmm...

Had a final go before I'm off to bed.

New approach:

Newest beta, no transition specified and no add_transition: false in app.

Changed your toggle call to direct HA toggle call (both homeassistant and light works) and now sync with Ikea bulbs on Hue bridge works flawlessly.

BUT, sync from light on with Ikea bulb on z2m now doesn't restore brightness.... 😫😫

@action
   async def toggle(self, **attributes):
       self.call_service(
           "light/toggle", entity_id=self.light["name"], **attributes
           )
       #await self.call_light_service("light/toggle", **attributes)

What's the difference in these two calls, Xavi ?

@htvekov
Copy link
Contributor Author

htvekov commented Apr 9, 2020

I'm an idiot!! 🙄

Forgot to 're add' the missing brightness and transition attribute in latest beta with all this flipping back and forth in versions...

If toggle call is replaced with:

@action
   async def toggle(self, **attributes):
       self.call_service(
           "light/toggle", entity_id=self.light["name"], **attributes
           )
       #await self.call_light_service("light/toggle", **attributes)

And brightness / transition is added to main call in sync function, then sync and toggle to correct brightness works here on both z2m and Hue bridge for Ikea bulbs.

@action
    async def sync(self):
        await self.on(brightness=self.max_brightness, transition=0)
        attributes = {}
        try:
            color_attribute = await self.get_attribute(LightController.ATTRIBUTE_COLOR)
            if color_attribute == LightController.ATTRIBUTE_COLOR_TEMP:
                attributes[color_attribute] = 370  # 2700K light
            else:
                self.index_color = 12  # white colour
                attributes[color_attribute] = self.colors[self.index_color]
        except:
            self.log("sync action will only change brightness", level="WARNING")
        if attributes != {}:
            await self.on(**attributes, brightness=self.max_brightness, transition=1) # Added brightness and transition attribute)

Signing off - Good night, Xavi ! 🥱😌😴😴

@xaviml
Copy link
Owner

xaviml commented Apr 12, 2020

Hi @htvekov,

I saw there was a bug in the sync function, so I added the brightness back to the call together with the color. I pushed a new release with this. It will probably work yet with your Hue integration, but it should work with your z2m integration. I tested it with a color temperature lightbulb and a color one, also from "off" state and "on" state. Everything seems to work as expected. I will close this issue and we will create a new one if needed since the original problem got solved with the new release of z2m.

Thanks for your help @htvekov :)

@xaviml xaviml closed this as completed Apr 12, 2020
@htvekov
Copy link
Contributor Author

htvekov commented Apr 13, 2020

Hi' Xavi.

Ikea bulbs on z2m, Ikea bulbs on Hue bridge and Ikea bulbs on z2m works 99% with sync and toggle to correct brightness now 🎉🎈✨🎉🎉😎👍

Only thing not working is sync from off position with Ikea bulbs on Hue bridge.
I believe it's because 'main call' is without transition attribute, as app is run with add_transition: false. The Ikea bulbs need this specific call including the transition attribute as well, even though transition attribute is stripped from toggle/on and off calls.

I believe that this transition attribute can be left out, if Balloob at some point finds the time to fix the core HA Hue integration issue I raised some 4 weeks ago. I doubt that it will be fixed anytime soon, though...

Ciao ! 🙋‍♂️😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants