search

jasoncoon / esp8266-fastled-webserver

GNU General Public License v3.0
714 stars 359 forks source link

Enable polar noise patterns on more products. #229

Closed jasoncoon closed 2 years ago

jasoncoon commented 2 years ago

Enable for other products after discussion.

The polar noise patterns should be enabled on any product that has angle and radius (or a radius proxy).

Thoughts on consolidating HAS_POLAR_COORDS and HAS_RADIUS_PROXY completely?

jasoncoon commented 2 years ago

Should we change the constant data here, to be able to remove both RADII_SCALE_DIVISOR and RADII_SCALE_MULTIPLIER?

Yes, I think so. Whether we call it radius or radiusProxy, I would rather it be explicit, and always in a one byte (0-255) scale, like the other coords.

jasoncoon commented 2 years ago

I think we could actually get rid of the physicalToFibonacci arrays, as the only places they are used should be using either radius or fibonacciToPhysical instead. 😆

I'll try to get these changes made and tested before merging this PR.

henrygab commented 2 years ago

I think we could actually get rid of the physicalToFibonacci arrays, as the only places they are used should be using either radius or fibonacciToPhysical instead. 😆

OK, although the compiler will optimize them away if they are never referenced, so it shouldn't cause any change in the final binary size. Then again, since they are dead code, it makes sense to drop them.

jasoncoon commented 2 years ago

I still need to test on all the different products, but everything builds without errors or warnings.

henrygab commented 2 years ago

Very nice! I like how the standardized radius range (always 0..255) allows simplification of the clock hand calculations, and removing the HAS_POLAR_COORDS is another "Good Thing:tm:"

Nicely done!