Replace usage of millis() in effects with strip.now - sync effects across wled instances

[arneboe]

Is your feature request related to a problem? Please describe.
Some/most effects in FX.cpp directly use millis(). This causes problems when running several wled fixtures
that should all display the same effect. The animations of the fixtures will be out of sync because the timing on each esp is slightly different (e.g. they did not start at exactly the same microsecond).

Describe the solution you'd like
strip.now already exists and is updated on every frame. All effects should use strip.now instead of millis().
This way we gain the ability to manipulate the time.
In a second step a method to reset the time to a certain value could be implemented.
Using this method, usermods can be implemented to achieve time sync between several wled fixtures (e.g. via artnet/dmx/wifi).

[ewowi]

Sounds like a good improvement. I see 73 calls to millis() in FX.cpp. @softhack007, okay to replace them?
cc @netmindz with regards to artnet / dmx
cc @troyhacks with regards to multiple esps collaborating on one effect (future spec)

See also #73

[netmindz]

ArtNet and sACN don't have time as such, but they do have frame counters that could be used a a form of sync

[ewoudwijma]

@arneboe , what about the random functions ? In order to run effects more predictable, should we also seed the random function in a way we can predict random values ?

[troyhacks]

There are functions to set the random seed in FastLED:

random16_get_seed()
random16_set_seed( uint16_t seed)
random16_add_entropy( uint16_t entropy)

Ideally those could be synchronized somehow, along with synchronizing strip.now across multiple instances

[troyhacks]

@arneboe - this was a stellar idea and happened to come along right when I was considering the same things.

This image is "Distortion Waves" rendered on 4 controllers (each 16x16) and each board is mirrored via a combination of the "reverse" settings in the segment options. The Distortion Waves effect didn't correlate across instances at all before I made this change.

Works very well. I'm setting strip.now via NTP milliseconds every X seconds, which seems accurate enough for this test.

I will look into this more closely and generate a proper PR that isn't as heavy handed as what I did to my test installations. 🤣

[arneboe]

@troyhacks nice! :)
I did some experiments using dmx and reset strip.now to zero every time the effect id changed via dmx.
This seems to work reasonably well for my use-case. There is still a little time offset because the dmx data is not handled at exactly the same time but I think I can improve on that by storing a timestamp when the break is received.

[arneboe]

@ewoudwijma Good idea!
Synchronizing them could be done by occasionally resetting all seeds to a hard coded value. E.g. every time the effect id is changed. This has the down side that effects will look a bit repetitive.
If ntp is availble the seeds could be set to the current time every minute.

All of this assuming that the RNG implementation is deterministic (i.e. it does not use any input from pins, clock, etc).

[ewoudwijma]

Like @troyhacks said, we also need this for syncing multiple devices each working on one effect (so we can scale up nr of leds to infinity 😱🙂).

So we need deterministic effects, if current random functions use pins etc for randomness we will replace these with more deterministic ones (maybe/ probably with a setting to choose, in case predictability is noticeable / annoying)

Funny multiple people want this for different reasons now 🙂

@arneboe , are you on discord? Then you could join the discussions on this topic

[softhack007]

@troyhacks

I'm setting strip.now via NTP milliseconds every X seconds, which seems accurate enough for this test.

@arneboe

I did some experiments using dmx and reset strip.now to zero every time the effect id changed via dmx.

While the overall idea of adjusting the timekeeping base of a controller sounds promising, I would strongly advise against directly fiddeling with strip.now. Two reasons:

A) some effects store the current strip.now in a variable, and on the next run use that to calculate elapsed time. This ensures smoothness in some effects. So resetting strip.now means these effects will run into underflow and their time keeping will be confused.
B) strip.now is regularly overwritten with the value of millis(). And no, don't try to modify the time base of millis().
C) a time base should not be reset to zero; in best case it should only move forward.

WLED/wled00/FX_fcn.cpp

Lines 1608 to 1610 in 0ba8402

	void WS2812FX::service() {
	unsigned long nowUp = millis(); // Be aware, millis() rolls over every 49 days // WLEDMM avoid losing precision
	now = nowUp + timebase;

Edit: if you don't want NTP for time synchronization, it maybe sufficient to add a small DS3231 RTC board. Look into the RTC usermod for details.

[softhack007]

I see 73 calls to millis() in FX.cpp. @softhack007, okay to replace them?
cc @netmindz with regards to artnet / dmx

Agreed, maybe it's even a good PR for upstream.
Can we use SEGENV.now to avoid directly accessing the strip object?

[ewowi]

I am not in favour of SEGENV.now as it is not segment related (I think) and increased memory usage

And what about timebase? that is used in calculating strip.now, I see also millis() + strip.timebase is used sometimes, if we replace mills() by strip.now, we should also replace mills() + strip.timebase by strip.now ? I am not sure I understand (I mean I am sure I don't understand ;-) )

[softhack007]

About timebase ... seems it's time I look deeper into the code.

The idea would be to keep calculating strip.now = timebase + millis(). We cannot adjust millis() directly, so timebase could be our tool to get a number of controllers into perfect sync. Maybe it should be mmTimebase - so we don't interfere with other hackwork done by the WLED core.

long mmTimebase = 0;
// adjust mmTimebase wherever we think is appropriate
// 
void WS2812FX::service() { 
long long nowUp = millis(); // Be aware, millis() rolls over every 49 days // WLEDMM avoid losing precision 
now = nowUp + mmTimebase + timebase; // mmTimebase is allowed to be negative, so we can set the "clock" forward or backward, as needed to keep devices in sync.

[ewoudwijma]

Sounds good! Maybe we can even use the existing timebase to manipulate time, I will take a look this weekend

Something like: if a new effect is chosen set timebase equal to -millis() (or -nowUp) so strip.now starts with 0

[netmindz]

What was the existing timebase added for?

[softhack007]

What was the existing timebase added for?

Good question, I found a commit from 4 years ago, and it looks like the intention was to sync "timebase" between controllers in the same group. Unfortunately there is no further explanation ... maybe we'd find something in the KB...

Aircoookie@d4c921e

Additionally there is a hint "Timebase reset when turned off" in the 0.9.1 release notes.

So most likely we need to reverse engineer the use case / requirements...