Flicker on switching on gen 2 switches

[bigbadblunt]

Flicker on switching on gen 2 switches

[v-sante]

Just to say I've turned my hand to this in the light.py file. Note, I know nothing about python so just trying to work it out from scratch.

Seems there are two instructions, one to turn on and one to match any existing setting regarding brightness. This double instruction starts with setting the brightness and then turning on. This seems to be the reason for the double flash. The first is a brightness adjustment, the second actually turns it on.

Clearly the other ways of turning the light on (app, other integrations in openhab) don't have this issue.

What I've tried so far is reversing the order, so it turns on and then adjusts the brightness. There's still a double flash, but consistent with my reasoning, it is different. The led comes on and stays on (as the light has been told to be on) and then there is a flicker as the brightness adjustment comes through, but it doesn't revert to an "off" colour in between.

I guess what we (or I) need to find is a way of covering these two instructions in one instruction to the switch (and whether both are in fact needed). I will test and tinker further. Would be interested if this is just a "me" problem.

[bigbadblunt]

Thanks for looking into this.

I'm pretty sure two messages are needed to set brightness and to change state from on to off - I don't believe there is a way to do both in a single combined message.

If I use the official app, start with a light which is off, and change the brightness, I get the same behaviour (i.e. two flashes on the switch led as the light turns on).

If I use the official app, start with a light which is off and turn it on, I agree the behaviour differs.

I'm minded not to change this. For gen1 kit HA doesn't know the status of the switch, so needs to send both messages to make sure it operates correctly. For gen2 I could test to see if the brightness is changing, and only send both messages if the brightness level has changed, but I'm a bit worried about HA getting out of sync with the switch and not sending a message when it needs to.

if it really bugs you, the code change to only send the brightness message if the brightness level has changed is below. This just adds an extra test to the if statement, and moves the "set_brightness" call inside the if.

if ATTR_BRIGHTNESS in kwargs and (kwarg[ATTR_BRIGHTNESS] != self._brightness):
            self._brightness = kwargs[ATTR_BRIGHTNESS]
            _LOGGER.debug("Setting brightness %s %s", self._brightness, int(self._brightness / 255 * 100))
            await self._lwlink.async_set_brightness_by_featureset_id(
                self._featureset_id, int(round(self._brightness / 255 * 100)))

[v-sante]

Hi there - thanks so much for the pointer.

I tried your code with no initial luck but I’ve got it to work with the following two adjustments which I will mention if anyone else is considering.

1. Typo of kwargs of kwarg in the first line.

2. I think there was a problem with a straight comparison of kwargs[ATTR_BRIGHTNESS] and self._brightness as I think the first is out of 100 and the latter is out of 255. So I adjusted the != line to divide the former by 100 and multiply by 255. For completeness and consistency with the rest of the code I also adjusted self._brightness to int(round(.

Seems to be working perfectly.

Was wondering about your concerns about backwards compatibility. Couldn’t this build in a further if condition based on whether the device is gen2 or not. I seem to recall seeing this property is known.

Also, I’ve tested and not detected any lack of immediate syncing if I adjust brightness on other apps. So it may be this change could be integrated without any loss of gen1 or gen2 function with the right further if condition?

Apologies don’t know how to quote code but I will try to put it into my fork (which is a mess at the moment).

Separately I noticed some deprecations around the use of Light instead of LightEntity that seems to be a recent change in HA. No apparent impact on function. Just FYI.

[bigbadblunt]

Hi - that's the problem with writing code without testing it! Apols.

kwargs[ATTR_BRIGHTNESS] is definitely out of 255. But interestingly, HA doesn't pass the ATTR_BRIGHTNESS if it doesn't think the brightness has changed. So just moving the async_set_brightness_by_featureset_id inside the if is enough to fix the behaviour.

Thinking about it a bit more, I think this is probably the right behaviour; if you dimmed the lights but HA didn't know about it, then later used HA to turn them on, I don't think you'd want HA overwriting your brightness. So I've changed my mind and pushed a new version that fixes this issue.

The new version also fixes the deprecation warnings, and introduces an options configuration flow. Let me know if you have any problems, I will package it for HACS asap.

[bigbadblunt]

PS as always, let me know if the new code works!

[v-sante]

Brilliant. Not sure why I was still getting flicker - possible I didn't reset properly.

New code tested and working in respect of the turning off and on (no double flicker, goes straight to preset brightness). However actually adjusting the brightness on its own once the light is on is a bit weird and laggy. I've observed it a bit and I would say the following:

1. Sometimes adjusting the brightness works, sometimes it doesn't.
2. When it doesn't work, after a short while (maybe 10 secs), presumably on a property update in HA, it resets to the pre-adjustment brightness as if the brightness adjustment instruction had never been given.
3. When it works, it works with varying degrees of responsiveness.

I didn't notice this issue last night (albeit I only had a little time with it as only got it to work around midnight). So I've just checked the differences in my code and yours.

I tried (with little understanding of why it might help given your explanation above) about adding back in the extra if statement and made no difference. No surprise!

I then noticed a reversal of the await instruction and the self._brightness definition. I undid this reversal and it seems to have fixed the issue. Amended code below which I've tested and is fine:

` async def async_turn_on(self, **kwargs):
"""Turn the LightWave light on."""
_LOGGER.debug("HA light.turn_on received, kwargs: %s", kwargs)

    if ATTR_BRIGHTNESS in kwargs:
        _LOGGER.debug("Changing brightness from %s to %s (%s%%)", self._brightness, kwargs[ATTR_BRIGHTNESS], int(kwargs[ATTR_BRIGHTNESS] / 255 * 100))
        self._brightness = kwargs[ATTR_BRIGHTNESS]
        await self._lwlink.async_set_brightness_by_featureset_id(
            self._featureset_id, int(round(self._brightness / 255 * 100)))

    self._state = True
    await self._lwlink.async_turn_on_by_featureset_id(self._featureset_id)

    self.async_schedule_update_ha_state()`

Final q, curious why isn't adjusting brightess a separate async def altogether from turning on? Tried to work it out but couldn't.

[bigbadblunt]

OOOOPS. I've no idea how I managed to move that line down. Fixed in the latest code.

As to why there's only one function that does both the turning on and the setting of the brightness, that just the way HA does it. If you're interested, the specification of light interface is at:

https://developers.home-assistant.io/docs/core/entity/light

There are only two methods, one for turning on and one for turning off - apparently HA considers a change in brightness to just be turning the light on again with a different brightness level.