Seed FastLED's RNG #3552

TripleWhy · 2023-11-26T23:02:08Z

This allows effects or color palettes that use FastLED's RNG to be different each time the device is powered on.

I also made sure that all effects use use FastLED's random functions instead of arduino's for more consistency.

blazoncek · 2023-11-27T06:43:32Z

This will inevitably change the way some effects work and may break certain presets or playlists.
Please re-base this PR for 0_15 branch because of that.

This allows effects or color palettes that use FastLED's RNG to be different each time the device is powered on.

TripleWhy · 2023-11-27T17:51:27Z

Rebased.

It does indeed change some effects, if they are configured as boot preset. But that's the point. I would like effects involving randomness to be different each time.

The PR does not affect playlist shuffles as they don't use FastLED's random function, but Arduino's instead. I don't know of other ways how this could affect playlists.

blazoncek · 2023-11-28T11:03:44Z

@softhack007 if you have no objections I propose we merge.

softhack007 · 2023-11-28T13:03:58Z

@softhack007 if you have no objections I propose we merge.

Hi, thanks for the PR, generally looks good to me. Maybe let's spend a few thoughts about point 2 below.

I've checked that this would be (basically) compatible with ongoing work in MM - synchronous effects on several devices (see Replace usage of millis() in effects with strip.now - sync effects across wled instances MoonModules/WLED#72). MM is also using fastled random16 and synchronizes seeds, so it should be ok (@ewoudwijma what do you think?)
one point to check: are there other places in the WLED core source code where arduino random() should be replaced by fastLED random? I'm particularly thinking about playlist "shuffle", maybe also other code in the core would benefit from using fastLED random functions?

WLED/wled00/playlist.cpp

Line 35 in 1dab26b

int randomIndex = random(0, currentIndex);

softhack007 · 2023-11-28T13:09:01Z

wled00/wled.cpp

@@ -495,6 +495,16 @@ void WLED::setup()
  initServer();
  DEBUG_PRINT(F("heap ")); DEBUG_PRINTLN(ESP.getFreeHeap());

+  // Seed FastLED random functions with an esp random value, which already works properly at this point.
+#if defined(ARDUINO_ARCH_ESP32)
+  const uint32_t seed32 = esp_random();


minor: please check if the header file for esp_random is already included.

#if defined(ARDUINO_ARCH_ESP32) #include "esp_random.h" #endif

Should I also include something for the esp8266?

I'm not sure ... is there a header on 8266 that defines RANDOM_REG32 ?

blazoncek · 2023-11-28T13:44:49Z

I'd like to distinguish 2 aspects of random():

true (pseudo) randomness
fastest execution while providing adequate randomness

For effects I'd like the second but for everything else the first should be chosen IMO.
ESP32 documentation says this. Which let's me believe that not all ESPs may have it enabled properly to be trustworthy as a true randomness.

FastLED uses simple prime multiplication/addition requiring seed to be truly randomised. If you want to avoid misconfigured bootloader then randomize seed by reading unconnected analog input on boot. What are the chances of misconfigured bootloader? IDK
Ardiuino's function may be the same in that regard (haven't looked at the source).

There's also this.

ewoudwijma · 2023-11-28T14:08:15Z

@softhack007 replacing random with random8/16 👍

softhack007 · 2023-11-28T14:26:27Z

@blazoncek good point 👍
Maybe the important difference is
a) needing "fast" random number (fastLED), or
b) needing a good quality random number (standard random())

Actually - on esp32 - random() is implemented using esp_random(), so its a fair/good quality random number already. And its not based on standard libc "rand()" which has low quality. I only found references to rand() in AsyncWebserver.

~~Maybe them we could just add randomSeed(unsigned long seed) directly after random16_set_seed() (wled.cpp) for slightly improving overall randomness.~~

EDIT: in the latest framework, calling randomSeed() switches back to "soft random" with low quality 🤦 .
So better not add randomSeed().

softhack007

OK for me. It seems that "seeding" the standard arduino random() would be a bad move on esp32, so using fastLED random8/16 in effects, and seeding it once from esp_random(), is best.

softhack007 · 2023-11-28T14:48:36Z

wled00/wled.cpp

+  const uint32_t seed32 = esp_random();
+#elif defined(ARDUINO_ARCH_ESP8266)
+  const uint32_t seed32 = RANDOM_REG32;
+#else


is this "else" branch needed? We only support esp32 variants (ARDUINO_ARCH_ESP32) and 8266 (ARDUINO_ARCH_ESP8266).

No, but it doesn't hurt, does it? If someone were to come along and try to port WLED to a different architecture, this would be one less thing to worry about. I don't have strong feelings about this though, and can remove it if you

I'd prefer if you remove it - its unused code and we don't have any plans for supporting non-esp architectures.

blazoncek · 2023-11-29T09:03:18Z

It crossed my mind that not everyone may want truly random seed at start. Some may prefer a known seed and known or predictable random() output.
So, perhaps it might be good to add configuration item (UI?) where seed can be entered. 0 (default) would mean random boot seed. Thoughts?

@TripleWhy please comment on the discussion above.

TripleWhy · 2023-11-29T10:22:41Z

are there other places in the WLED core source code where arduino random() should be replaced by fastLED random?

I don't really have strong opinion on which random function is better. On the ESP8266 it looks to me like arduino's unseeded(!) random is faster, more random, and slightly biased, but on the ESP32 it contains a (theoretically) unlimited loop. (At least, in the version that VS Code shows me. The link that softhack007 posted does not contain loops.)

ESP32 documentation says this. Which let's me believe that not all ESPs may have it enabled properly to be trustworthy as a true randomness.

I saw that too, but I assumed initializing those systems would be part of WLED somewhere, so I just checked if esp_random returns different values on my device, at the point where I added it. I does that, so I assumed it would be fine to use it. Even if the output isn't truly random at that point, it only really matters that the output is different each boot.
If there are devices where it does not return different values, at least my version is not any worse that it was before.

Ardiuino's function may be the same in that regard (haven't looked at the source).

It uses esp_random() without additional checks.

There's also this.

Feel free to create a better seed, I just wanted to propose seeding and one way to do it, instead of just submitting a feature request.

OK for me. It seems that "seeding" the standard arduino random() would be a bad move on esp32

Same for ESP8266.

It crossed my mind that not everyone may want truly random seed at start.

Someone who wants predictable results probably wouldn't choose an effect containing randomness in the first place...
There is also no way to reset the randomness, so to truly preview such a repeatable random effect he would have to save it, configure it as boot effect, and reboot. And then there is the issue that (without my PR) not all effects use repeatable random functions, some use true random functions.

blazoncek · 2023-11-29T12:16:32Z

@TripleWhy please don't feel offended or provoked. I just wanted feedback and your thoughts.

As mentioned above I'm ok with PR as-is. If there is a request to produce less random random() in the future we can always add custom, fixed seed at a later point.

I am going to merge as-is.

P.S. @softhack007 the only other random() call is in playlist.cpp there are no other random() calls in WLED core. That one can be substituted with random8() at any time.

softhack007 · 2023-11-29T12:26:57Z

P.S. @softhack007 the only other random() call is in playlist.cpp there are no other random() calls in WLED core. That one can be substituted with random8() at any time.

@blazoncek thanks for checking - considering the "lower quality" of random8/16 functions, I think its best to only use them in effects.

TripleWhy · 2023-11-29T12:52:52Z

@TripleWhy please don't feel offended or provoked. I just wanted feedback and your thoughts.

I wasn't. Sorry if I made it sound that way.

I am going to merge as-is.

Well. Seems like I'm not able to address the code comments after all ^^

Seed FastLED's RNG

TripleWhy added 2 commits November 27, 2023 18:18

Use FastLED's random functions instead of arduino's for all effects

528b8b1

Seed FastLED's random functions during startup

25c5e82

This allows effects or color palettes that use FastLED's RNG to be different each time the device is powered on.

TripleWhy force-pushed the seedRandom16 branch from 6a8b5d4 to 25c5e82 Compare November 27, 2023 17:44

TripleWhy changed the base branch from main to 0_15 November 27, 2023 17:52

blazoncek added this to the 0.15.0 candidate milestone Nov 28, 2023

softhack007 self-requested a review November 28, 2023 13:03

softhack007 reviewed Nov 28, 2023

View reviewed changes

softhack007 approved these changes Nov 28, 2023

View reviewed changes

softhack007 reviewed Nov 28, 2023

View reviewed changes

blazoncek merged commit bf4b29b into Aircoookie:0_15 Nov 29, 2023
12 checks passed

softhack007 pushed a commit to MoonModules/WLED that referenced this pull request Dec 12, 2023

Merge pull request Aircoookie#3552 from TripleWhy/seedRandom16

451243c

Seed FastLED's RNG

Djelibeybi pushed a commit to Djelibeybi/WLED-MM that referenced this pull request Jan 15, 2024

Merge pull request Aircoookie#3552 from TripleWhy/seedRandom16

0717bb9

Seed FastLED's RNG

softhack007 mentioned this pull request Oct 25, 2024

Replace PRNG with hardware RNG #4225

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Seed FastLED's RNG #3552

Seed FastLED's RNG #3552

TripleWhy commented Nov 26, 2023

blazoncek commented Nov 27, 2023

TripleWhy commented Nov 27, 2023

blazoncek commented Nov 28, 2023

softhack007 commented Nov 28, 2023 •

edited

Loading

softhack007 Nov 28, 2023

TripleWhy Nov 29, 2023

softhack007 Nov 29, 2023

blazoncek commented Nov 28, 2023

ewoudwijma commented Nov 28, 2023

softhack007 commented Nov 28, 2023 •

edited

Loading

softhack007 left a comment

softhack007 Nov 28, 2023

TripleWhy Nov 29, 2023

softhack007 Nov 29, 2023

blazoncek commented Nov 29, 2023

TripleWhy commented Nov 29, 2023 •

edited

Loading

blazoncek commented Nov 29, 2023

softhack007 commented Nov 29, 2023

TripleWhy commented Nov 29, 2023

Seed FastLED's RNG #3552

Seed FastLED's RNG #3552

Conversation

TripleWhy commented Nov 26, 2023

blazoncek commented Nov 27, 2023

TripleWhy commented Nov 27, 2023

blazoncek commented Nov 28, 2023

softhack007 commented Nov 28, 2023 • edited Loading

softhack007 Nov 28, 2023

Choose a reason for hiding this comment

TripleWhy Nov 29, 2023

Choose a reason for hiding this comment

softhack007 Nov 29, 2023

Choose a reason for hiding this comment

blazoncek commented Nov 28, 2023

ewoudwijma commented Nov 28, 2023

softhack007 commented Nov 28, 2023 • edited Loading

softhack007 left a comment

Choose a reason for hiding this comment

softhack007 Nov 28, 2023

Choose a reason for hiding this comment

TripleWhy Nov 29, 2023

Choose a reason for hiding this comment

softhack007 Nov 29, 2023

Choose a reason for hiding this comment

blazoncek commented Nov 29, 2023

TripleWhy commented Nov 29, 2023 • edited Loading

blazoncek commented Nov 29, 2023

softhack007 commented Nov 29, 2023

TripleWhy commented Nov 29, 2023

softhack007 commented Nov 28, 2023 •

edited

Loading

softhack007 commented Nov 28, 2023 •

edited

Loading

TripleWhy commented Nov 29, 2023 •

edited

Loading