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

Default transition time not applied to commands arriving during an ongoing transition #4514

Open
1 task done
protyposis opened this issue Jan 22, 2025 · 14 comments
Open
1 task done

Comments

@protyposis
Copy link

protyposis commented Jan 22, 2025

What happened?

When the state, e.g., on or brightness, is changed with a temporary transition time, this transition time seems to persist and apply to subsequent commands arriving during the transition. This contrasts the "applies to just the current API call" description in the docs of the state object and can lead to unexpected behavior, e.g., turning off a light may take a long time if a previous command used a lengthy transition.

An example where this becomes noticeable is using Home Assistant with this Adaptive Lighting integration, which adjusts brightness and color temperature throughout the day using long transitions (defaulting to 45 seconds). Consequently, it takes 45 seconds for lights to turn off after pressing a wall switch. A workaround is to always specify the desired transition time with every command, but this is inconvenient or even impossible when using a combination of 3rd party integrations whose behavior cannot be entirely customized. This setup uses the Websocket JSON API with the tt (temporary transition) parameter.

To Reproduce Bug

Imagine a WLED instance with a default transition time of 1000 ms configured.

The first example demonstrates the described issue, where turning off takes unexpectedly long to apply since it reuses the temporary transition time of the previous command (similar behavior with brightness change, etc.):

curl -X POST "http://[WLED-IP]/json/state" -d '{"on":true,"bri":255}' -H "Content-Type: application/json"
sleep 2
curl -X POST "http://[WLED-IP]/json/state" -d '{"on":true,"bri":10,"tt":300}' -H "Content-Type: application/json"
sleep 2
# Takes 28 seconds to turn off
curl -X POST "http://[WLED-IP]/json/state" -d '{"on":false}' -H "Content-Type: application/json"

Example two works as expected, because the previous transition is first awaited before the light is turned off:

curl -X POST "http://[WLED-IP]/json/state" -d '{"on":true,"bri":255}' -H "Content-Type: application/json"
sleep 2
curl -X POST "http://[WLED-IP]/json/state" -d '{"on":true,"bri":10,"tt":300}' -H "Content-Type: application/json"
sleep 30
# Turns off immediately with 1-second default transition time
curl -X POST "http://[WLED-IP]/json/state" -d '{"on":false}' -H "Content-Type: application/json"

Example three also works as expected, because the turn-off command explicitly overwrites the previous temporary transition time:

curl -X POST "http://[WLED-IP]/json/state" -d '{"on":true,"bri":255}' -H "Content-Type: application/json"
sleep 2
curl -X POST "http://[WLED-IP]/json/state" -d '{"on":true,"bri":10,"tt":300}' -H "Content-Type: application/json"
sleep 2
# Turns off immediately with 1-second transition time
curl -X POST "http://[WLED-IP]/json/state" -d '{"on":false,"tt":10}' -H "Content-Type: application/json"

Expected Behavior

The default transition time is used for all commands which don't contain a transition time. In the second reproduction example, this would mean that the third command turns the light off with the default transition time instead of reusing the previous temporary transition time.

I assume that's what the default transition time configuration in WLED is supposed to do, because that's how previous 0.14.x versions of WLED behaved (0.13.x, too, afair). I noticed the changed behavior after upgrading to 0.15.0.

Install Method

Binary from WLED.me

What version of WLED?

WLED 0.15.0 (build 2412100)

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@protyposis protyposis added the bug label Jan 22, 2025
@DedeHai
Copy link
Collaborator

DedeHai commented Jan 22, 2025

This is by design: there is one transition time, which when set, persists. IMHO the default value in the settings has become a bit meaningless: it is only used at bootup until a new value is written.
You are looking at machine controls, but how about user interface: how or when would you go back to the default value? If I set the transition to say 2 seconds, then click on some colors and brightnesses, should it reset to the default value after 10 seconds? 10minutes? or after 1 click? 2 clicks? The same goes for presets and commands: they all access the same variable in the background, keeping track of seperate variables for each interface is just not feasible (neither code complexity wise nor ram wise, there are many of these parameters).

@protyposis
Copy link
Author

I don't fully understand your argument, probably because I’m not familiar with WLED’s internal architecture. Could you clarify if the behavior change was intentional or a side effect of architectural changes? I opened this ticket because I was speculating this might have been an unintentional change, since it wasn’t mentioned in the release notes of 0.15 and worked differently in previous versions (0.13, 0.14).

how or when would you go back to the default value?

Any (new) transition would use the default value unless an explicit value is supplied by the initiator (e.g., through the API command). If transitions overlap, meaning a new transition starts before the previous one completes, the new transition replaces the old one and uses either the new explicit value or the default. (That's also how it seems to work in Home Assistant, ZHA, Zigbee2Mqtt.)

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 22, 2025

I do not know what the intentions behind it is, I was confused about this at the beginning as well (why is there a default value if it is not used?)

what you describe will not hold up when using the UI. Since you are actually using an external controller: why not have it send transition time along with the json command? then your issue would be solved.
its less of a WLED problem but the external controls not using the right commands.

@protyposis protyposis changed the title Transition time does not reset to default transition time Default transition time not applied to commands arriving during an ongoing transition Jan 23, 2025
@protyposis
Copy link
Author

I updated my initial report and title to describe the issue more clearly. I originally described that the transition parameter unexpectedly updates the default transition time, but that's what it's supposed to do and correct behavior. It's the tt parameter (temporary transition) which unexpectedly carries over to subsequent commands.

what you describe will not hold up when using the UI

I assume the situation has changed with my updated description, but for the sake of completeness, Home Assistant uses exactly the same interface and the same commands as the UI (Websocket with JSON state objects), so I don't see how this makes any difference for the UI. Also, the solution you proposed is already described in my initial post. It is of course one way to solve it, but I'd still like to make the authors aware of a potentially unexpected behavior.

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 23, 2025

that is beyond my knowledge of the code.

@protyposis
Copy link
Author

I took a quick look at the code and might have identified the root cause.

When a JSON state update request is received, the transition time is updated with the (temporary) time ("tt", or "transition" a few lines above), but only if provided. If not provided, nothing happens and the previous setting remains:

https://github.com/Aircoookie/WLED/blob/f2caf14d6a785923089b867f0920b05af88d09ae/wled00/json.cpp#L344-L349

Later, after the transition has finished because the configured time has passed (ti/tr > 0), the time is reset to the default (transitionDelay):
https://github.com/Aircoookie/WLED/blob/f2caf14d6a785923089b867f0920b05af88d09ae/wled00/led.cpp#L169-L183

There are no other places where the transition time is restored, which explains the behavior of the provided reproduction cases.

It seems this behavioral change was introduced by b88344a, which would explain the new behavior in 0.15.0. In earlier versions, the transition time was reset on every state update, i.e., with every state change request, even when it did not contain tt/transition. @blazoncek would you mind taking a look whether this is intended behavior?

@blazoncek
Copy link
Collaborator

IMO behaviour is correct. If the transition is still running and is not overridden it should continue to run until it is finished.

@deviantintegral
Copy link

I also ran into this and tracked it down to the Adaptive Lighting integration. Turning off the light could take up to 45 seconds to actually happen. However, I also saw the problem in 0.14.4 (and 0.15.0 and the 0.16 nightlies). Admittedly with 0.14.4 I was using Athom's default firmware, so perhaps the above commit had been cherry-picked in.

My workaround is to set the transition time in Adaptive Lighting to 1 second.

From a user standpoint, this whole behaviour feels a bit odd. With other lighting systems I've used, a second command overrides any existing running effect.

If the existing behaviour is what the project wants to maintain, I suppose that's OK. But in that case, it would be good to surface an "is transitioning" status in the UI. As is, you press off in the Web UI, it shows off, but the light stays on. That leads folks to think there is a bug in WLED, since the UI clearly doens't show the actual state of the like.

@blazoncek
Copy link
Collaborator

a second command overrides any existing running effect

And it can. By providing override in API call.

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 26, 2025

@deviantintegral if using the WLED UI (no external apps) and this behaviour is seen, I would definitely call it a bug. if an external application is used and it does not use the API correctly, there is not much that can be done.

@blazoncek
Copy link
Collaborator

@DedeHai even UI will not override currently running transition (as it has no "tt" command).

However, what can be considered "override for current transition" (except specifying "tt")? Is it effect? Or color? Or non state changing command? And does it need to override for each segment or just currently selected or main segment?

@DedeHai
Copy link
Collaborator

DedeHai commented Jan 27, 2025

even UI will not override currently running transition (as it has no "tt" command).

then I misunderstood.

From a user standpoint, this whole behaviour feels a bit odd. With other lighting systems I've used, a second command overrides any existing running effect.

because this is true for the ui: if I start a transition with say 30s, change brightness, then change transition time and set a new brightness, it uses the new transition value, so it does "override any existing running effect"

@blazoncek
Copy link
Collaborator

From a user standpoint, this whole behaviour feels a bit odd. With other lighting systems I've used, a second command overrides any existing running effect.

Also, not every "other lighting system" is as flexible as WLED is or offers so many different ways to set up lighting as WLED does in a single software package. Unfortunately that also means not every user's wish/preference can be accommodated.

@deviantintegral
Copy link

deviantintegral commented Jan 29, 2025

Thanks for the ideas. I tried:

  • Setting Adaptive Lighting back to a 45 second transition. Adaptive Lighting is adjusting colour and brightness only.
  • Setting my “off” command in the Home Assistant assistant to explicitly include a 1 second transition.

However, if the light is in the middle of an Adaptive Lighting transition, it won’t turn off until it’s complete.

edit oops, I take that back. I set the transition for “on” instead of “off” in my automation. Time to try again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants