From d48e4adcd4a131d41db52b0afe5bf0503ba3557f Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Thu, 13 Apr 2023 12:16:32 +0200 Subject: [PATCH 1/5] CI build fix seems that NPB 2.7.4 introduces new incompatibilities that break our gh build action. --- platformio.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platformio.ini b/platformio.ini index 949858598..873592413 100644 --- a/platformio.ini +++ b/platformio.ini @@ -314,7 +314,7 @@ build_flags = -g lib_deps = ${env.lib_deps} ;; NeoPixelBus 2.7.1 is the first that has official support for ESP32-S3 - makuna/NeoPixelBus @ ~2.7.3 + makuna/NeoPixelBus @ 2.7.3 https://github.com/pbolduc/AsyncTCP.git @ 1.2.0 From 996d0415810c8583bfdb9d350a9cec1989c568c5 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Fri, 14 Apr 2023 11:39:12 +0200 Subject: [PATCH 2/5] bugfix for art-net transmit art-net transmit was not sending out LEDs, but only transmitted headers repeatedly (thanks @troyhacks for noticing). Actually such problems can be found by newer compilers, so i've added the warning option to [esp32_idf_V4]. wled00/udp.cpp: In function 'uint8_t realtimeBroadcast(uint8_t, IPAddress, uint16_t, uint8_t*, uint8_t, bool)': wled00/udp.cpp:824:40: warning: declaration of 'byte buffer [12]' shadows a parameter [-Wshadow=compatible-local] byte buffer[ART_NET_HEADER_SIZE]; ^ wled00/udp.cpp:720:85: note: shadowed declaration is here uint8_t realtimeBroadcast(uint8_t type, IPAddress client, uint16_t length, uint8_t *buffer, uint8_t bri, bool isRGBW) { --- platformio.ini | 1 + wled00/udp.cpp | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/platformio.ini b/platformio.ini index 873592413..a20dab2c3 100644 --- a/platformio.ini +++ b/platformio.ini @@ -247,6 +247,7 @@ lib_deps = platform = espressif32@5.2.0 platform_packages = build_flags = -g + -Wshadow=compatible-local ;; emit warning in case a local variable "shadows" another local one -DARDUINO_ARCH_ESP32 -DESP32 #-DCONFIG_LITTLEFS_FOR_IDF_3_2 -D CONFIG_ASYNC_TCP_USE_WDT=0 diff --git a/wled00/udp.cpp b/wled00/udp.cpp index 6d7d28202..afe7b3356 100644 --- a/wled00/udp.cpp +++ b/wled00/udp.cpp @@ -821,9 +821,9 @@ uint8_t realtimeBroadcast(uint8_t type, IPAddress client, uint16_t length, uint8 } } - byte buffer[ART_NET_HEADER_SIZE]; - memcpy_P(buffer, ART_NET_HEADER, ART_NET_HEADER_SIZE); - ddpUdp.write(buffer, ART_NET_HEADER_SIZE); // This doesn't change. Hard coded ID, OpCode, and protocol version. + byte header_buffer[ART_NET_HEADER_SIZE]; + memcpy_P(header_buffer, ART_NET_HEADER, ART_NET_HEADER_SIZE); + ddpUdp.write(header_buffer, ART_NET_HEADER_SIZE); // This doesn't change. Hard coded ID, OpCode, and protocol version. ddpUdp.write(sequenceNumber & 0xFF); // sequence number. 1..255 ddpUdp.write(0x00); // physical - more an FYI, not really used for anything. 0..3 ddpUdp.write((currentPacket) & 0xFF); // Universe LSB. 1 full packet == 1 full universe, so just use current packet number. From 4a3bc486d00549c9f471f9f154d6ffa93e91a786 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Fri, 14 Apr 2023 13:09:25 +0200 Subject: [PATCH 3/5] two more "shadowed locals" In these case, there seem to be no bug, but simply renaming the "inner" variables improves code readability. --- wled00/FX_fcn.cpp | 6 +++--- wled00/pin_manager.cpp | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index a9dabafee..95298c72a 100644 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -1181,12 +1181,12 @@ void WS2812FX::estimateCurrentAndLimitBri() { uint32_t powerSum = 0; - for (uint8_t b = 0; b < busses.getNumBusses(); b++) { - Bus *bus = busses.getBus(b); + for (uint_fast8_t bNum = 0; bNum < busses.getNumBusses(); bNum++) { + Bus *bus = busses.getBus(bNum); if (bus->getType() >= TYPE_NET_DDP_RGB) continue; //exclude non-physical network busses uint16_t len = bus->getLength(); uint32_t busPowerSum = 0; - for (uint16_t i = 0; i < len; i++) { //sum up the usage of each LED + for (uint_fast16_t i = 0; i < len; i++) { //sum up the usage of each LED uint32_t c = bus->getPixelColor(i); byte r = R(c), g = G(c), b = B(c), w = W(c); diff --git a/wled00/pin_manager.cpp b/wled00/pin_manager.cpp index c800c807d..a1ac59ce7 100644 --- a/wled00/pin_manager.cpp +++ b/wled00/pin_manager.cpp @@ -300,10 +300,10 @@ byte PinManagerClass::allocateLedc(byte channels) if (ca >= channels) { //enough free channels byte in = (i + 1) - ca; for (byte j = 0; j < ca; j++) { - byte b = in + j; - byte by = b >> 3; - byte bi = b - 8*by; - bitWrite(ledcAlloc[by], bi, true); + byte bChan = in + j; + byte byChan = bChan >> 3; + byte biChan = bChan - 8*byChan; + bitWrite(ledcAlloc[byChan], biChan, true); } return in; } From 246d945f39166c7b4b519ed43723cb985afdfee5 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Fri, 14 Apr 2023 14:13:45 +0200 Subject: [PATCH 4/5] another "inner var shadows outer var" Seems this is not causing bugs, however its still bad style to re-define existing vars in an inner loop. Solved to improve code readability. --- wled00/FX.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wled00/FX.cpp b/wled00/FX.cpp index 2f5e9fdc6..2b5c81418 100644 --- a/wled00/FX.cpp +++ b/wled00/FX.cpp @@ -4653,8 +4653,8 @@ uint16_t mode_2DColoredBursts() { // By: ldirko https://editor.so byte ysteps = abs8(x2 - y2) + 1; byte steps = xsteps >= ysteps ? xsteps : ysteps; //Draw gradient line - for (size_t i = 1; i <= steps; i++) { - uint8_t rate = i * 255 / steps; + for (size_t j = 1; j <= steps; j++) { + uint8_t rate = j * 255 / steps; byte dx = lerp8by8(x1, y1, rate); byte dy = lerp8by8(x2, y2, rate); //SEGMENT.setPixelColorXY(dx, dy, grad ? color.nscale8_video(255-rate) : color); // use addPixelColorXY for different look From d10daf0991b008767e7ef03331816ad721886673 Mon Sep 17 00:00:00 2001 From: Blaz Kristan Date: Mon, 17 Apr 2023 16:25:05 +0200 Subject: [PATCH 5/5] Bugfix - skip regular button handling while waiting for analog read --- wled00/button.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/wled00/button.cpp b/wled00/button.cpp index fce21424d..d51040a87 100644 --- a/wled00/button.cpp +++ b/wled00/button.cpp @@ -225,7 +225,6 @@ void handleButton() { static unsigned long lastRead = 0UL; static unsigned long lastRun = 0UL; - bool analog = false; unsigned long now = millis(); //if (strip.isUpdating()) return; // don't interfere with strip updates. Our button will still be there in 1ms (next cycle) @@ -241,14 +240,18 @@ void handleButton() if (usermods.handleButton(b)) continue; // did usermod handle buttons - if ((buttonType[b] == BTN_TYPE_ANALOG || buttonType[b] == BTN_TYPE_ANALOG_INVERTED) && now - lastRead > ANALOG_BTN_READ_CYCLE) { // button is not a button but a potentiometer - analog = true; - handleAnalog(b); continue; + if (buttonType[b] == BTN_TYPE_ANALOG || buttonType[b] == BTN_TYPE_ANALOG_INVERTED) { // button is not a button but a potentiometer + if (now - lastRead > ANALOG_BTN_READ_CYCLE) { + handleAnalog(b); + lastRead = now; + } + continue; } //button is not momentary, but switch. This is only suitable on pins whose on-boot state does not matter (NOT gpio0) if (buttonType[b] == BTN_TYPE_SWITCH || buttonType[b] == BTN_TYPE_PIR_SENSOR) { - handleSwitch(b); continue; + handleSwitch(b); + continue; } //momentary button logic @@ -305,7 +308,6 @@ void handleButton() shortPressAction(b); } } - if (analog) lastRead = now; } // If enabled, RMT idle level is set to HIGH when off