search

Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
15.01k stars 3.24k forks source link

Fix default pin assignement for unassigned buttons, take 2 #4098

Open PaoloTK opened 3 months ago

PaoloTK commented 3 months ago

Follow up on https://github.com/Aircoookie/WLED/issues/3953 Fill btnPin array with -1 unless corresponding member of buttonType array has been set to a value != 0 (BTN_TYPE_NONE). 0.14 branch had a similar implementation where it filled all elements except the first with -1, but since in 0.15 you can define multiple button pins at compile time it was removed. Because of this change it is no longer necessary to specify the second pin and type on the default BTNPIN and BTNTYPE defs.

Credit to @softhack007 for the idea.

blazoncek commented 3 months ago

If you want to do it, do it correctly. This is the place for this kind of initialisation: https://github.com/Aircoookie/WLED/blob/3615ab535b3ebfb77a175a4cb2949d0a4a516143/wled00/cfg.cpp#L255-L269

PaoloTK commented 3 months ago

If you want to do it, do it correctly. This is the place for this kind of initialisation:

https://github.com/Aircoookie/WLED/blob/3615ab535b3ebfb77a175a4cb2949d0a4a516143/wled00/cfg.cpp#L255-L269

While as always I truly appreciate the help and guidance, there's no need to be rude to people trying to help, so you could have done without the first sentence.

I spent a long time testing the code block you linked and could never get it to trigger for reasons beyond my understanding. I asked in the Discord and got no answers. So I did the next best thing I could think of, found where that behavior was implemented in the previous release and placed the code there.

If you look at the block you quoted, if it was to trigger the fix I implemented should not be needed since one of the conditions to set btnPin to -1 is buttonType[s] == BTN_TYPE_NONE.

If the block above should always be triggered on first boot and it's not due to a bug, it's obviously better to figure out why that is rather than patch the code somewhere else, so I'm willing to look into that instead. However, fixing this bug is actually quite crucial since if it isn't (and when someone raised the issue in https://github.com/Aircoookie/WLED/issues/3953 it was marked "won't fix") once 0.15 ships ALL configurations that use pin 0 for LED output on ESP32 will break. I tried implementing a quick fix for that instead previously but was shut down.