JBL partybox-like soundreactive effect #67
Conversation
modified: wled00/FX.h
wled00/FX.cpp
Outdated
| uint16_t pos = 0; | ||
| uint8_t hue = 0; | ||
| int hueDelay = 0; |
There was a problem hiding this comment.
Maybe change the scope of those variables to be static inside mode_jbl or put them in a struct or change their names to something less generic.
It is very likely that other fx developers will use variables with the same name. In the best case scenario this will just spam a "shadows variable" warning. In the worst case they will use your variables by accident and strange stuff happens.
There was a problem hiding this comment.
see my proposal below ... I agree these should not be global. Better to use SEGENV.aux0, SEGENV.aux1, SEGENV.step.
|
Hi, thanks for the PR :-) Yes there is a lot of optimization potential with your code. In fact it should not depend on global variables, as effects may run in several instances in parallel, especially they will be used with several segments. For a start:
Don't worry, we will guide you through improving your code. Also this guide is worth reading (however a bit outdated): https://github.com/atuline/WLED/wiki/WLED-Programming-Notes#important-wled-variables I'll make a few more proposals once I find a bit of time to read your source code in detail. |
|
All things you noted above should be fixed in the latest commit |
thanks, looks much cleaner already 👍 |
|
After uploading the "new" code on my esp32, it looks like the animation is much slower than before (with the initial commit) |
|
If you used latest mdev, it could be due to a bug in audioreactive, that caused the microphone to switch off and you would only see "simulated sound". I've fixed that one just an hour ago. Please try again with the latest source code. If your effect still seems too slow, please verify that you compare at similar FPS (led settings "target fps"). I can take a closer look on the weekend, maybe there is some timing problem due to integer arithmetics. |
|
I use the latest (pulled few minutes ago) source code, the mic is working, and the fps is the same, but it's fine i added a slider to change/correct the speed |
| @@ -1941,7 +2019,7 @@ static const char _data_FX_MODE_JUGGLE[] PROGMEM = "Juggle@!,Trail;;!;;sx=64,ix= | |||
|
|
|||
|
|
|||
| uint16_t mode_palette() { | |||
| uint16_t counter = 0; | |||
| uint16_t counter = 0; | |||
There was a problem hiding this comment.
Same here, this seems like not related to your new effect.
|
As a minor thing, please think about the name of your effect |
|
"Party Beatbox" maybe? |
|
how exactly should i refer to this effect? I plan to make the other effects this speaker has |
looks best with all sliders in the middle, added option to change the effect speed multiplier
Well one of the effects looks exactly like our "gravimeter" (mirrored) just without the peak dot. So you don't need to replicate that one. Not sure about the others - these stuttering rotating rings look ugly to me, maybe call that "party jerk" :-) In fact it's your call to find nice names, I'm just saying avoid the word JBL to keep JBL legal far away from us. |
Yeah i was thinking about that too
Party jerk it is
I think it's almost the hardest part about it |
|
are there any more problems or potential enchancements with this effect? |
Give me a week or so to come back to your effect. I was very busy the last weeks with helping to push out upstream -b5. |
Okay, no problem take your time, i was just wondering |
|
Hi, I'm very busy@job currently, will come back to this PR in October. |
|
Hi, I'll merge this PR. I have a few ideas for improvement, but that can also be done later:
this way your new effect will make it into the next MM release. |
this currently only works with the "color gradient" and the "colors 1&2" palettes