🚀 ESP-IDF V5 #4838
🚀 ESP-IDF V5 #4838
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughRefactors build and platform compatibility for ESP32/ESP-IDF v5: gates NeoEsp32RmtHI compilation by WLED_USE_SHARED_RMT, renames Network → WLEDNetwork across the codebase, updates LEDC/ETH/UDP/MDNS/NTP APIs for IDF v5, adds SHA-1 shim, DMX RDM personality changes, and updates platformio/requirements. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I'm not sure if renaming Network is the right approach to fix the conflict @willmmiles |
|
I am also unsure why we are seeing linker errors for things like mbedtls_sha1_starts |
Hm, it looks like it's just a bag of stateless utility functions. Probably it should be a namespace instead of a class. I don't think there's a practical solution to avoid the collision other than renaming, unless the newer core offers these utilities itself. (In fact I'd also suggest pulling them in to wled00/network.cpp , given the use of WLED-specific #defines.) (Best pratice would've been that the upstream libraries put everything they do in a namespace, eg.
I believe that the Tasmota platform omits mbedtls to save flash space, as they've gone over to BearSSL. Looks like they're still exposing the headers though - I don't know if that's an IDF bug or something wrong with their build process. AsyncWebServer needs an SHA1 implementation as it is required for WebSockets. It looks like the upstream AWS folks have switched over to IDFv5 functions and vendored in Espressif's implementation when building on IDFv4. I'll see if I can pull those patches. |
|
With few fixes it builds. MQTT mbedTLS WiFi wled.cpp Compare IPAddress to a value, not to 0 (prevents pointer overload → memcmp on nullptr) and clamp index (prevents out-of-range access cascading into bad compares). @@ void WLED::initConnection()
- if (multiWiFi[selectedWiFi].staticIP != 0U && multiWiFi[selectedWiFi].staticGW != 0U) {
- WiFi.config(multiWiFi[selectedWiFi].staticIP, multiWiFi[selectedWiFi].staticGW, multiWiFi[selectedWiFi].staticSN, dnsAddress);
- } else {
- WiFi.config(IPAddress((uint32_t)0), IPAddress((uint32_t)0), IPAddress((uint32_t)0));
- }
+ if (multiWiFi.empty()) { // guard: handle empty WiFi list safely
+ WiFi.config(IPAddress((uint32_t)0), IPAddress((uint32_t)0), IPAddress((uint32_t)0));
+ } else {
+ if (selectedWiFi >= multiWiFi.size()) selectedWiFi = 0; // guard: ensure valid index
+ if (multiWiFi[selectedWiFi].staticIP != IPAddress((uint32_t)0) &&
+ multiWiFi[selectedWiFi].staticGW != IPAddress((uint32_t)0)) { // guard: compare as IPAddress to avoid pointer overload
+ WiFi.config(multiWiFi[selectedWiFi].staticIP, multiWiFi[selectedWiFi].staticGW, multiWiFi[selectedWiFi].staticSN, dnsAddress);
+ } else {
+ WiFi.config(IPAddress((uint32_t)0), IPAddress((uint32_t)0), IPAddress((uint32_t)0));
+ }
+ }ESP32 MAC fallback (fix Info tab showing 000000000000) @@ (top of file)
+#ifdef ARDUINO_ARCH_ESP32
+#include <esp_mac.h> // fallback MAC read from efuse
+#endif@@ // generate module IDs must be done before AP setup
-escapedMac = WiFi.macAddress();
+escapedMac = WiFi.macAddress();
+#ifdef ARDUINO_ARCH_ESP32
+if (escapedMac == "00:00:00:00:00:00") { // fix: fallback to efuse MAC if WiFi not started
+ uint8_t m[6] = {0};
+ if (esp_read_mac(m, ESP_MAC_WIFI_STA) == ESP_OK) {
+ char buf[18];
+ sprintf(buf, "%02X:%02X:%02X:%02X:%02X:%02X", m[0], m[1], m[2], m[3], m[4], m[5]);
+ escapedMac = buf;
+ }
+}
+#endif
escapedMac.replace(":", "");
escapedMac.toLowerCase();network.cpp
@@ int findWiFi(...)
- if (multiWiFi.size() <= 1) {
- DEBUG_PRINTF_P(PSTR("WiFi: Defaulf SSID (%s) used.\n"), multiWiFi[0].clientSSID);
- return 0;
- }
+ if (multiWiFi.size() <= 1) {
+ if (multiWiFi.empty()) return 0; // guard: handle empty list safely
+ DEBUG_PRINTF_P(PSTR("WiFi: Defaulf SSID (%s) used.\n"), multiWiFi[0].clientSSID);
+ return 0;
+ }
@@ for (size_t n = 0; n < multiWiFi.size(); n++)
- bool foundBSSID = memcmp(multiWiFi[n].bssid, WiFi.BSSID(o), 6) == 0;
+ const uint8_t* _scanBSSID = WiFi.BSSID(o);
+ bool _cfgHasBSSID = false; for (int _i = 0; _i < 6; _i++) _cfgHasBSSID |= multiWiFi[n].bssid[_i];
+ bool foundBSSID = (_scanBSSID && _cfgHasBSSID) && memcmp(multiWiFi[n].bssid, _scanBSSID, 6) == 0; // guard: safe memcmp against nullptr/empty BSSIDUsermods Modified um_manager.cpp: #include "wled.h"
#include <assert.h>
/*
* Registration and management utility for v2 usermods
*
* Discovery paths, in this order:
* 1) Legacy dtors window: .dtors.tbl.usermods.0 ... .99 (older toolchains)
* 2) Dynamic fallback: usermods call registerUsermod(Usermod*) from a ctor
*/
// -------- Legacy dtors window (keep for backward compatibility) --------
static Usermod * const _usermod_table_begin[0]
__attribute__((section(".dtors.tbl.usermods.0"), unused)) = {};
static Usermod * const _usermod_table_end[0]
__attribute__((section(".dtors.tbl.usermods.99"), unused)) = {};
struct UMSpan {
Usermod* const* begin;
Usermod* const* end;
};
static inline UMSpan reg_dtors_span() {
UMSpan s{_usermod_table_begin, _usermod_table_end};
return s;
}
// ----- Dynamic fallback: usermods can push themselves here at boot -----
static Usermod* g_dynMods[16];
static size_t g_dynCnt = 0;
// Weak, so a stronger definition elsewhere (if any) wins.
// Usermods may call this from a constructor to guarantee registration.
extern "C" void registerUsermod(Usermod* m) __attribute__((weak));
extern "C" void registerUsermod(Usermod* m) {
if (m && g_dynCnt < (sizeof(g_dynMods)/sizeof(g_dynMods[0]))) {
g_dynMods[g_dynCnt++] = m;
}
}
// ----- Common iteration helpers -----
// Accept any callable (incl. capturing lambdas/functors)
template<typename Fn>
static inline void forEachMod(Fn&& fn) {
// 1) legacy dtors table
{
UMSpan s = reg_dtors_span();
for (auto p = s.begin; p < s.end; ++p) {
Usermod* m = *p;
if (m) fn(m);
}
}
// 2) dynamic list
for (size_t i = 0; i < g_dynCnt; ++i) {
if (g_dynMods[i]) fn(g_dynMods[i]);
}
}
static inline size_t countMods() {
size_t count = 0;
// dtors window
{
UMSpan s = reg_dtors_span();
count += (size_t)(s.end - s.begin);
}
// dynamic
count += g_dynCnt;
return count;
}
// --------- Usermod Manager methods (iterate via forEachMod) ---------
void UsermodManager::setup() { forEachMod([](Usermod* m){ m->setup(); }); }
void UsermodManager::connected() { forEachMod([](Usermod* m){ m->connected(); }); }
void UsermodManager::loop() { forEachMod([](Usermod* m){ m->loop(); }); }
void UsermodManager::handleOverlayDraw() { forEachMod([](Usermod* m){ m->handleOverlayDraw(); }); }
void UsermodManager::appendConfigData(Print& dest) {
forEachMod([&](Usermod* m){ m->appendConfigData(dest); });
}
bool UsermodManager::handleButton(uint8_t b) {
bool overrideIO = false;
forEachMod([&](Usermod* m){
if (m->handleButton(b)) overrideIO = true;
});
return overrideIO;
}
bool UsermodManager::getUMData(um_data_t **data, uint8_t mod_id) {
bool got = false;
forEachMod([&](Usermod* m){
if (got) return;
if (mod_id > 0 && m->getId() != mod_id) return;
if (m->getUMData(data)) got = true;
});
return got;
}
void UsermodManager::addToJsonState(JsonObject& obj) {
forEachMod([&](Usermod* m){ m->addToJsonState(obj); });
}
void UsermodManager::addToJsonInfo(JsonObject& obj) {
auto um_id_list = obj.createNestedArray("um");
forEachMod([&](Usermod* m){
um_id_list.add(m->getId());
m->addToJsonInfo(obj);
});
}
void UsermodManager::readFromJsonState(JsonObject& obj) {
forEachMod([&](Usermod* m){ m->readFromJsonState(obj); });
}
void UsermodManager::addToConfig(JsonObject& obj) {
forEachMod([&](Usermod* m){ m->addToConfig(obj); });
}
bool UsermodManager::readFromConfig(JsonObject& obj) {
bool allComplete = true;
forEachMod([&](Usermod* m){
if (!m->readFromConfig(obj)) allComplete = false;
});
return allComplete;
}
#ifndef WLED_DISABLE_MQTT
void UsermodManager::onMqttConnect(bool sessionPresent) {
forEachMod([&](Usermod* m){ m->onMqttConnect(sessionPresent); });
}
bool UsermodManager::onMqttMessage(char* topic, char* payload) {
bool handled = false;
forEachMod([&](Usermod* m){
if (!handled && m->onMqttMessage(topic, payload)) handled = true;
});
return handled;
}
#endif // WLED_DISABLE_MQTT
#ifndef WLED_DISABLE_ESPNOW
bool UsermodManager::onEspNowMessage(uint8_t* sender, uint8_t* payload, uint8_t len) {
bool handled = false;
forEachMod([&](Usermod* m){
if (!handled && m->onEspNowMessage(sender, payload, len)) handled = true;
});
return handled;
}
#endif // WLED_DISABLE_ESPNOW
void UsermodManager::onUpdateBegin(bool init) {
forEachMod([&](Usermod* m){ m->onUpdateBegin(init); });
}
void UsermodManager::onStateChange(uint8_t mode) {
forEachMod([&](Usermod* m){ m->onStateChange(mode); });
}
Usermod* UsermodManager::lookup(uint16_t mod_id) {
Usermod* found = nullptr;
forEachMod([&](Usermod* m){
if (!found && m->getId() == mod_id) found = m;
});
return found;
}
size_t UsermodManager::getModCount() {
return countMods();
}
/* Usermod v2 interface shim for oappend */
Print* Usermod::oappend_shim = nullptr;
void Usermod::appendConfigData(Print& settingsScript) {
assert(!oappend_shim);
oappend_shim = &settingsScript;
this->appendConfigData();
oappend_shim = nullptr;
}At the end of usermod_v2_RF433.cpp I added: // --- Registration via dynamic fallback (no custom sections) ---
static RF433Usermod usermod_v2_RF433;
REGISTER_USERMOD(usermod_v2_RF433); // OK to keep for legacy dtors builds
extern "C" void registerUsermod(Usermod*) __attribute__((weak));
__attribute__((constructor, used))
static void __rf433_ctor_register(void) {
if (registerUsermod) registerUsermod(&usermod_v2_RF433);
}NeoPixelBus --- a/src/internal/methods/ESP/ESP32/NeoEsp32RmtXMethod.h
+++ b/src/internal/methods/ESP/ESP32/NeoEsp32RmtXMethod.h
@@ -1,5 +1,7 @@
#pragma once
+// Needed for ESP_* macros that take a log_tag
+#include "esp_log.h"
+static const char* TAG = "NeoEsp32RmtX";And I added a temporary shim in wled00/bus_wrapper.h. Since ESP-IDF 5 switched to the new RMT “driver_v2,” Makuna renamed all of the “N” (legacy/driver_v1) variants to “X” (v2) under internal/methods/ESP/ESP32/* // --- temporary shim for NeoPixelBus CORE3 / RMT driver_v2 ------------------
#if __has_include(<NeoPixelBus.h>)
#define NeoEsp32RmtNWs2812xMethod NeoEsp32RmtXWs2812xMethod
#define NeoEsp32RmtNSk6812Method NeoEsp32RmtXSk6812Method
#define NeoEsp32RmtN400KbpsMethod NeoEsp32RmtX400KbpsMethod
#define NeoEsp32RmtNTm1814Method NeoEsp32RmtXTm1814Method
#define NeoEsp32RmtNTm1829Method NeoEsp32RmtXTm1829Method
#define NeoEsp32RmtNTm1914Method NeoEsp32RmtXTm1914Method
#define NeoEsp32RmtNApa106Method NeoEsp32RmtXApa106Method
#define NeoEsp32RmtNWs2805Method NeoEsp32RmtXWs2805Method
#endif
// --------------------------------------------------------------------------- |
|
Thanks for helping with this @sombree Could you please open a PR against the V5 branch and we can then pull in your fixes that way, maintaining attribution for your work |
…(ESP-IDF 5.3.4 / arduino-esp32 v3.1.4). Based on wled#4838.
…(ESP-IDF 5.3.4 / arduino-esp32 v3.1.4). Based on wled#4838.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wled00/util.cpp (1)
1135-1229:⚠️ Potential issue | 🟠 MajorBlanket IDF v5 exclusion disables all three functions unnecessarily —
computeSHA1andgetDeviceIdcan work with targeted API guards.The
#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)guard removescomputeSHA1,generateDeviceFingerprint, andgetDeviceIdon IDF v5, but only two compatibility issues exist:
mbedtls API rename (lines 1147–1149): In IDF v5's mbedtls 3.x, the
_retsuffix was removed. Use conditional compilation to select the correct function names:
- Old (IDF <5):
mbedtls_sha1_starts_ret(),mbedtls_sha1_update_ret(),mbedtls_sha1_finish_ret()- New (IDF ≥5):
mbedtls_sha1_starts(),mbedtls_sha1_update(),mbedtls_sha1_finish()(still returnint)esp_adc_cal.h removal (line 1164): Replaced by
esp_adc/adc_cali.handesp_adc/adc_cali_scheme.hwith a redesigned API; only affectsgenerateDeviceFingerprint.A targeted approach allows
computeSHA1andgetDeviceIdto work on IDF v5 immediately:Suggested fix for computeSHA1
String computeSHA1(const String& input) { `#ifdef` ESP8266 return sha1(input); `#else` unsigned char shaResult[20]; mbedtls_sha1_context ctx; mbedtls_sha1_init(&ctx); + `#if` ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) + mbedtls_sha1_starts(&ctx); + mbedtls_sha1_update(&ctx, (const unsigned char*)input.c_str(), input.length()); + mbedtls_sha1_finish(&ctx, shaResult); + `#else` mbedtls_sha1_starts_ret(&ctx); mbedtls_sha1_update_ret(&ctx, (const unsigned char*)input.c_str(), input.length()); mbedtls_sha1_finish_ret(&ctx, shaResult); + `#endif` mbedtls_sha1_free(&ctx); // ... hex conversion unchanged `#endif` }
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Around line 278-283: The build_unflags assignment is wrong: it uses
${esp32_idf_V5.build_flags} which cancels flags; change the value to
${esp32_idf_V5.build_unflags} so build_unflags references the unflag list from
the esp32_idf_V5 section; update the platformio.ini entry that defines
build_unflags (near the lines with platform, build_flags, lib_deps) to use
${esp32_idf_V5.build_unflags} instead of ${esp32_idf_V5.build_flags}.
- Line 130: The build flag WLED_USE_SHARED_RMT is defined multiple times; remove
the definitions from the common build_flags and keep a single definition in the
esp32_idf_V5 build_flags block (or if you want it to apply everywhere, remove
the duplicates in esp32_idf_V5 and keep it only in common). Edit the
platformio.ini to ensure WLED_USE_SHARED_RMT appears exactly once and only
within the intended scope by deleting the extra entries in the
[common].build_flags or the duplicate(s) in [esp32_idf_V5].build_flags.
- Around line 387-404: The [esp32s3] environment is missing the proper unflags
and lib exclusion: replace the incorrect reference to ${common.build_unflags}
with ${esp32_idf_V5.build_unflags} (so the -Wno-volatile unflag is applied to C
files) and add the same lib_ignore entry used in other ESP32 envs (e.g.
${esp32_idf_V5.lib_ignore}) to exclude NeoESP32RmtHI from S3 builds to avoid RMT
driver conflicts; update the build_unflags and add lib_ignore under the
[esp32s3] block.
In `@wled00/dmx_input.cpp`:
- Around line 95-129: Define a named constant like DMX_PERSONALITY_COUNT and
replace the hardcoded 10 in the static array declaration (currently static
dmx_personality_t personalities[10];), in createPersonalities usage, and in
calls to dmx_driver_install and rdm_register_dmx_personality so all three sites
use DMX_PERSONALITY_COUNT; update any loops or bounds that assume 10 to
reference DMX_PERSONALITY_COUNT and ensure the constant is visible where
dmx_driver_install and rdm_register_dmx_personality are invoked.
- Around line 86-93: In createConfig(), config.software_version_label is set to
DmxVersionString.c_str() which leaves a dangling pointer when the local
std::string DmxVersionString is destroyed; fix by copying the string into the
config's fixed-size char buffer (e.g., use
strncpy(config.software_version_label, DmxVersionString.c_str(),
sizeof(config.software_version_label)-1) and set the final byte to '\0') so the
label remains valid after createConfig() returns; alternatively, if
config.software_version_label is a const char* and you prefer a static lifetime,
replace DmxVersionString with a static std::string or a static char array, but
prefer the strncpy approach to avoid global state.
In `@wled00/network.cpp`:
- Around line 234-241: The ETH.begin(...) call uses the v5/Arduino-ESP32
parameter order and needs a conditional to preserve v4 compatibility; wrap the
call to ETH.begin in a version/build guard (e.g., check CONFIG_IDF_VERSION_MAJOR
or the esp32dev_V4 define) and call ETH.begin with the v5 signature when IDF v5
is detected (using es.eth_type, es.eth_address, es.eth_mdc, es.eth_mdio,
es.eth_power, es.eth_clk_mode) but use the v4 signature when building for
esp32dev_V4/IDF v4 (placing phy_addr/eth_address as the first parameter and
ordering the rest per the v4 API) so compilation succeeds on both IDF versions.
In `@wled00/src/dependencies/espalexa/Espalexa.h`:
- Line 273: The lambda passed to serverAsync->on that handles HTTP_GET still
uses an implicit-capture `[=]`; change its capture to `[this]` to match the
other handlers and avoid implicit `this` capture warnings — i.e. update the
lambda used in the serverAsync->on("/espalexa", HTTP_GET, ...) callback that
assigns to `server` and calls `servePage()` so it captures `this` explicitly
(use `[this]`), leaving the parameter `AsyncWebServerRequest *request`
unchanged.
🧹 Nitpick comments (5)
wled00/src/dependencies/espalexa/Espalexa.h (1)
21-26: Dummy macro value is misleading but currently harmless.The real
ESP_IDF_VERSION_VALencodes as(major << 16) | (minor << 8) | patch, soESP_IDF_VERSION_VAL(5,0,0)=327680, not500. Since the only comparison site (line 389) is guarded bydefined(ARDUINO_ARCH_ESP32), this value is never evaluated on ESP8266, so it won't cause a bug today. But if anyone later adds a version check without the ESP32 guard, the result will be silently wrong.Suggested fix
-#define ESP_IDF_VERSION_VAL(n1,n2,n3) 500 +#define ESP_IDF_VERSION_VAL(n1,n2,n3) ((n1 << 16) | (n2 << 8) | (n3))platformio.ini (2)
504-522:esp32dev_V4unflagsWLED_ENABLE_DMX_INPUTwhich may not exist at that point.Line 510 uses
build_unflagswith-D WLED_ENABLE_DMX_INPUT, butbuild_unflagson line 509 inherits${common.build_unflags}which is empty. The DMX flag comes from${esp32_idf_V4.build_flags}(line 313), so it's being added and unflagged in the same env. This works in PlatformIO but the intent would be clearer by simply not including the flag in the first place via a V4-specific override.Otherwise the V4 regression test environment looks reasonable.
13-39: Several entries indefault_envsare both commented and uncommented.
esp32s3dev_8MB_opiappears uncommented on line 36 and commented on line 31.esp32s3dev_16MB_opiandesp32s3_4M_qspisimilarly appear commented in two places (lines 30+35 and 32+38). This makes it hard to tell which S3 builds are actually active. Consider cleaning up to have each env listed once.wled00/fcn_declare.h (1)
5-10: DummyESP_IDF_VERSION_VALreturns a misleading magic number.The shim defines
ESP_IDF_VERSION_VAL(n1,n2,n3)as500, which doesn't match the real macro's formula((n1<<16)|(n2<<8)|n3). While it currently works because ESP8266 guards short-circuit before evaluating IDF version comparisons, any future direct use on ESP8266 (e.g.,ESP_IDF_VERSION_VAL(1,0,0)) would yield500instead of65536, leading to silently wrong comparisons.Consider matching the real formula or using a clearly-below-everything sentinel like
0:Suggested fix
// dummy macro for 8266 `#ifndef` ARDUINO_ARCH_ESP32 `#ifndef` ESP_IDF_VERSION_VAL -#define ESP_IDF_VERSION_VAL(n1,n2,n3) 500 +#define ESP_IDF_VERSION_VAL(n1,n2,n3) ((n1 << 16) | (n2 << 8) | n3) +#endif +#ifndef ESP_IDF_VERSION +#define ESP_IDF_VERSION 0 `#endif` `#endif`wled00/dmx_input.cpp (1)
100-128: Usesizeof(personalities[0].description)instead of hardcoded32for strncpy buffer size.The
descriptionfield indmx_personality_tis 33 bytes (32 chars + null terminator). While the current code happens to work becausepersonalitiesis zero-initialized and all strings are shorter than 32 characters, usingsizeof(personalities[0].description)would be more defensive and prevent potential issues if string lengths change in the future.
| #else | ||
| // fallback code | ||
| const std::string DmxVersionString = "WLED_V" + std::to_string(VERSION); | ||
| config.software_version_label = DmxVersionString.c_str(); | ||
| #endif | ||
|
|
||
| return config; | ||
| } |
There was a problem hiding this comment.
Dangling pointer: config.software_version_label points to destroyed local std::string.
DmxVersionString is a local std::string inside createConfig(). Assigning .c_str() to config.software_version_label and then returning config leaves the pointer dangling — the string's buffer is freed when the function returns. This is undefined behavior and will likely cause garbage or a crash when the label is read.
Proposed fix — use `strncpy` into a fixed-size field, or make the string `static`
`#else`
// fallback code
- const std::string DmxVersionString = "WLED_V" + std::to_string(VERSION);
- config.software_version_label = DmxVersionString.c_str();
+ static std::string DmxVersionString = "WLED_V" + std::to_string(VERSION);
+ config.software_version_label = DmxVersionString.c_str();
`#endif`🤖 Prompt for AI Agents
In `@wled00/dmx_input.cpp` around lines 86 - 93, In createConfig(),
config.software_version_label is set to DmxVersionString.c_str() which leaves a
dangling pointer when the local std::string DmxVersionString is destroyed; fix
by copying the string into the config's fixed-size char buffer (e.g., use
strncpy(config.software_version_label, DmxVersionString.c_str(),
sizeof(config.software_version_label)-1) and set the final byte to '\0') so the
label remains valid after createConfig() returns; alternatively, if
config.software_version_label is a const char* and you prefer a static lifetime,
replace DmxVersionString with a static std::string or a static char array, but
prefer the strncpy approach to avoid global state.
| static dmx_personality_t personalities[10]; | ||
|
|
||
| static void createPersonalities() | ||
| { | ||
| // Initialize personalities array | ||
| strncpy(personalities[0].description, "SINGLE_RGB", 32); | ||
| personalities[0].footprint = 3; | ||
|
|
||
| strncpy(personalities[1].description, "SINGLE_DRGB", 32); | ||
| personalities[1].footprint = 4; | ||
|
|
||
| strncpy(personalities[2].description, "EFFECT", 32); | ||
| personalities[2].footprint = 15; | ||
|
|
||
| strncpy(personalities[3].description, "MULTIPLE_RGB", 32); | ||
| personalities[3].footprint = std::min(512, int(strip.getLengthTotal()) * 3); | ||
|
|
||
| strncpy(personalities[4].description, "MULTIPLE_DRGB", 32); | ||
| personalities[4].footprint = std::min(512, int(strip.getLengthTotal()) * 3 + 1); | ||
|
|
||
| strncpy(personalities[5].description, "MULTIPLE_RGBW", 32); | ||
| personalities[5].footprint = std::min(512, int(strip.getLengthTotal()) * 4); | ||
|
|
||
| strncpy(personalities[6].description, "EFFECT_W", 32); | ||
| personalities[6].footprint = 18; | ||
|
|
||
| strncpy(personalities[7].description, "EFFECT_SEGMENT", 32); | ||
| personalities[7].footprint = std::min(512, strip.getSegmentsNum() * 15); | ||
|
|
||
| strncpy(personalities[8].description, "EFFECT_SEGMENT_W", 32); | ||
| personalities[8].footprint = std::min(512, strip.getSegmentsNum() * 18); | ||
|
|
||
| strncpy(personalities[9].description, "PRESET", 32); | ||
| personalities[9].footprint = 1; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Magic number 10 repeated for personality count — extract a named constant.
The literal 10 appears in the static array size (Line 95), dmx_driver_install (Line 151), and rdm_register_dmx_personality (Line 167). If one site is updated but not the others, it silently breaks. A single constant keeps them in sync.
Proposed refactor
+static constexpr size_t DMX_PERSONALITY_COUNT = 10;
+
-static dmx_personality_t personalities[10];
+static dmx_personality_t personalities[DMX_PERSONALITY_COUNT];Then use DMX_PERSONALITY_COUNT in dmx_driver_install and rdm_register_dmx_personality calls as well.
Also applies to: 148-151, 167-167
🤖 Prompt for AI Agents
In `@wled00/dmx_input.cpp` around lines 95 - 129, Define a named constant like
DMX_PERSONALITY_COUNT and replace the hardcoded 10 in the static array
declaration (currently static dmx_personality_t personalities[10];), in
createPersonalities usage, and in calls to dmx_driver_install and
rdm_register_dmx_personality so all three sites use DMX_PERSONALITY_COUNT;
update any loops or bounds that assume 10 to reference DMX_PERSONALITY_COUNT and
ensure the constant is visible where dmx_driver_install and
rdm_register_dmx_personality are invoked.
to check if 8266 code was broken accidentally
|
@coderabbitai i need a concise summary of open points in this PR. |
|
Tip For best results, initiate chat on the files or code changes.
🔧 Core WLED Code Issues1. util.cpp - Device Identification FunctionsStatus: ❌ Broken in V5 builds (conditionally compiled out)
2. ADC Deprecated ConstantsStatus:
🏗️ Build System & Tooling3. PlatformIO InstabilityStatus:
4. Usermod CI BuildsStatus: ⏸️ Generating ~100 extra builds
5. Platform Package DecisionStatus: 🤔 Strategy discussion needed
🔌 Usermods Compatibility6. pwm_outputs UsermodStatus: ❌ Missing LEDC API guards
7. deep_sleep UsermodStatus:
8. audioreactive UsermodStatus: 🚧 Disabled for V5
9. Usermod Registration MechanismStatus: ✅ Fixed with hybrid approach
📚 External Dependencies10. NeoPixelBus/RMT Driver ConflictsStatus: ✅ Mitigated with build flag
11. mbedtls SHA1 FunctionsStatus: ✅ Fixed with shim
🎯 Action Items SummaryCritical (breaks builds):
Important (warnings/future-proofing): Strategic decisions needed: Would you like me to create GitHub issues for the critical action items, or generate patch proposals for items 🧠 Learnings used |
softhack007
added
enhancement
major
|
Would be great to get your input on this @troyhacks |
@DedeHai do you think we can replace deprecated functions as the rabbit recommends? |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Around line 540-547: The debug env `[env:esp32dev_debug]` currently overrides
build_unflags and drops inherited unflags from `${esp32_idf_V5.build_unflags}`;
update the `build_unflags` setting in `env:esp32dev_debug` to preserve inherited
unflags by appending or merging `${esp32_idf_V5.build_unflags}` (e.g., include
both the existing `-D WLED_RELEASE_NAME=...` unflag and
`${esp32_idf_V5.build_unflags}`) so you don't re-enable flags previously removed
by the esp32_idf_V5 configuration.
In `@wled00/util.cpp`:
- Around line 1139-1140: The preprocessor guard around computeSHA1() and
getDeviceId() in util.cpp must match the established pattern used in
fcn_declare.h and json.cpp; replace the current simplistic ESP_IDF_VERSION check
with the explicit guard used elsewhere: check for either non-ESP32 builds or
ESP-IDF < 5.0 (i.e., the pattern !defined(ARDUINO_ARCH_ESP32) ||
(ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)) ) so the
declarations/definitions of computeSHA1 and getDeviceId are consistently
enabled/disabled; keep the TODO comment about getDeviceId unavailability on
ESP-IDF v5 intact.
🧹 Nitpick comments (1)
wled00/util.cpp (1)
16-19: Indentation is inconsistent with surrounding includes.Lines 8-15 have no leading whitespace for
#includedirectives, but lines 16-19 have varying indentation. This creates visual inconsistency within the same#elseblock.🧹 Suggested fix to align with surrounding code style
`#include` "mbedtls/sha1.h" // for SHA1 on ESP32 `#include` "esp_efuse.h" - `#include` "esp_chip_info.h" - `#if` ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) - `#include` <esp_mac.h> // V5 requirement - `#endif` +#include "esp_chip_info.h" +#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) +#include <esp_mac.h> // V5 requirement +#endif `#endif`
| #if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) // ToDO: find a solution to make this work in V5 | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for ESP_IDF_VERSION_VAL definitions
rg -n "ESP_IDF_VERSION_VAL" --type-add 'header:*.h' -t header
# Check how ESP_IDF_VERSION is used in headers
rg -n "ESP_IDF_VERSION" wled00/ -t cpp -t header -B 2 -A 2
# Check the actual code around lines 1139-1140
sed -n '1130,1150p' wled00/util.cppRepository: wled/WLED
Length of output: 2765
🏁 Script executed:
# Check for version compatibility macros in the codebase
rg -n "defined.*ESP_IDF_VERSION" wled00/Repository: wled/WLED
Length of output: 730
🏁 Script executed:
# Look for any fallback definitions
rg -n "#define.*ESP_IDF_VERSION" wled00/Repository: wled/WLED
Length of output: 206
🏁 Script executed:
sed -n '1139,1250p' wled00/util.cppRepository: wled/WLED
Length of output: 3556
🏁 Script executed:
# Also check if there's similar code in json.cpp mentioned in results
sed -n '700,715p' wled00/json.cppRepository: wled/WLED
Length of output: 858
🏁 Script executed:
# Check fcn_declare.h to see how the same pattern is used there
sed -n '420,440p' wled00/fcn_declare.hRepository: wled/WLED
Length of output: 1395
🏁 Script executed:
# Check if ESP_IDF_VERSION is defined on ESP8266
rg -n "ESP_IDF_VERSION" --type cpp | grep -i "define\|ifdef" | head -20
# Look at the learning about USB CDC which mentions ARDUINO_USB_CDC_ON_BOOT vs explicit CONFIG_IDF_TARGET checks
# This suggests relying on Arduino framework flags rather than IDF checks
# Check what the standard pattern is
rg -n "ARDUINO_ARCH_ESP32" wled00/ -t cpp | head -10
# Verify if ESP8266 includes or defines ESP_IDF_VERSION
fd "board" . | head -5
rg -n "ESP_IDF_VERSION" . --type-add 'board:*.json' | head -5Repository: wled/WLED
Length of output: 2218
🏁 Script executed:
# Search for where ESP_IDF_VERSION might be set for ESP8266
rg -n "ESP_IDF_VERSION\s*=" wled00/
# Check if there's version checking in network.cpp that shows the correct pattern
sed -n '230,240p' wled00/network.cppRepository: wled/WLED
Length of output: 440
🏁 Script executed:
# Check the exact guard pattern in util.cpp at the beginning
head -20 wled00/util.cpp
# Verify what files include or define ESP_IDF_VERSION
rg -n "include.*esp_idf_version" wled00/ -t cpp
# Check if the guard pattern is consistent across files
rg -B1 "computeSHA1\|getDeviceId" wled00/ -t cpp -t header | grep -E "^[^:]*:`#if`"Repository: wled/WLED
Length of output: 718
🏁 Script executed:
# Let's verify: Check if ESP_IDF_VERSION is used without defined() check in util.cpp
sed -n '1,30p' wled00/util.cpp | grep -A5 -B5 "ESP_IDF_VERSION"
# Count unsafe vs safe patterns in the codebase
echo "=== Unsafe patterns (no defined() check): ==="
rg "#if ESP_IDF_VERSION" wled00/ -t cpp | grep -v "defined(ESP_IDF_VERSION)"
echo ""
echo "=== Safe patterns (with defined() or platform check): ==="
rg "#if.*defined.*ARDUINO_ARCH.*ESP_IDF_VERSION" wled00/ -t cpp
rg "#if defined(ESP_IDF_VERSION)" wled00/ -t cppRepository: wled/WLED
Length of output: 2080
Use the established guard pattern for consistency with declarations and other usage sites.
The guard at line 1139 is missing the explicit platform check used elsewhere in the codebase. The same computeSHA1() and getDeviceId() functions are declared in fcn_declare.h:427 and used in json.cpp:703 with an explicit pattern: #if !defined(ARDUINO_ARCH_ESP32) || (ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)). The guard here should match that pattern for consistency and clarity.
Additionally, getDeviceId() remains unavailable on ESP-IDF v5 due to mbedTLS API changes—the TODO indicates this is known but unresolved.
🛠️ Suggested fix
-#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) // ToDO: find a solution to make this work in V5
+#if !defined(ARDUINO_ARCH_ESP32) || (ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)) // TODO: Update mbedTLS SHA1 API for ESP-IDF v5 support📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) // ToDO: find a solution to make this work in V5 | |
| `#if` !defined(ARDUINO_ARCH_ESP32) || (ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)) // TODO: Update mbedTLS SHA1 API for ESP-IDF v5 support |
🤖 Prompt for AI Agents
In `@wled00/util.cpp` around lines 1139 - 1140, The preprocessor guard around
computeSHA1() and getDeviceId() in util.cpp must match the established pattern
used in fcn_declare.h and json.cpp; replace the current simplistic
ESP_IDF_VERSION check with the explicit guard used elsewhere: check for either
non-ESP32 builds or ESP-IDF < 5.0 (i.e., the pattern
!defined(ARDUINO_ARCH_ESP32) || (ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0))
) so the declarations/definitions of computeSHA1 and getDeviceId are
consistently enabled/disabled; keep the TODO comment about getDeviceId
unavailability on ESP-IDF v5 intact.
First off, move I2C to IDF calls. That will solve a lot of headaches right off that bat. I'll have more as I think about it. A good test is to comment out Arduino.h and see what breaks. 😁 |
|
Flash size comparison:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@softhack007 now do it with LTO. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@platformio.ini`:
- Around line 505-525: The esp32dev_V4 regression environment contains test-only
build flags (WLED_USE_ETHERNET and the pin overrides RLYPIN and BTNPIN) which
should be removed from the default_envs to avoid skewing V4 regression results;
remove those flags from the [env:esp32dev_V4] build_flags and create a separate
test-only env (e.g., [env:esp32dev_V4_ethernet_test]) that sets -D
WLED_USE_ETHERNET -D RLYPIN=-1 -D BTNPIN=-1 (and any related lib_ignore entries
such as esp_dmx) so the regression env remains standard while the test env
captures the temporary Ethernet/pin overrides.
In `@wled00/bus_manager.cpp`:
- Around line 421-427: The code currently calls ledcAttach(...) under IDF v5
which auto-allocates channels; change this to explicitly bind the expected
channel index (channel = _ledcStart + i) using ledcAttachChannel (or the v5
equivalent that takes a channel argument) so that the assigned channel matches
the code's later assumptions (references: ledcAttach, channel, _ledcStart,
_pins, and the timer reset and direct register writes around lines noted).
Ensure you compute/assign channel = _ledcStart + i before attaching and replace
the ledcAttach(...) call with ledcAttachChannel(channel, _pins[i],
/*...params...*/) so timer reset and PWM duty writes target the correct channel.
| [env:esp32dev_V4] | ||
| ;; uses V4 framework - for checking that the code still builds in V4 | ||
| board = esp32dev | ||
| platform = ${esp32_idf_V4.platform} | ||
| platform_packages = ${esp32_idf_V4.platform_packages} | ||
| build_unflags = ${common.build_unflags} | ||
| custom_usermods = audioreactive | ||
| build_flags = ${common.build_flags} ${esp32_idf_V4.build_flags} -D WLED_RELEASE_NAME=\"ESP32\" #-D WLED_DISABLE_BROWNOUT_DET | ||
| -D WLED_ENABLE_DMX_INPUT ;; TODO: fix lots of compile errors in dmx_input.cpp | ||
| custom_usermods = ${common.default_usermods} | ||
| build_flags = ${common.build_flags} ${esp32_idf_V4.build_flags} -D WLED_RELEASE_NAME=\"ESP32_V4\" #-D WLED_DISABLE_BROWNOUT_DET | ||
| -DARDUINO_USB_CDC_ON_BOOT=0 ;; this flag is mandatory for "classic ESP32" when building with arduino-esp32 >=2.0.3 | ||
| -D WLED_DISABLE_INFRARED ;; TODO: remove once we have updated for V5 | ||
| -D WLED_USE_SHARED_RMT ;; TODO: disabled NeoEsp32RmtMethodIsr | ||
| -D WLED_USE_ETHERNET -D RLYPIN=-1 -D BTNPIN=-1 ;; TODO: this is just for testing - remove before merging to main | ||
| lib_deps = ${esp32_idf_V4.lib_deps} | ||
| lib_ignore = | ||
| NeoESP32RmtHI ;; TODO: disabled NeoEsp32RmtMethodIsr | ||
| esp_dmx ;; TODO: fix lots of compile errors in dmx_input.cpp | ||
| monitor_filters = esp32_exception_decoder | ||
| board_build.partitions = ${esp32.default_partitions} | ||
| board_build.flash_mode = dio | ||
|
|
There was a problem hiding this comment.
Remove the testing-only Ethernet flags before merge.
WLED_USE_ETHERNET plus pin overrides are marked TODO-only in the V4 regression env, but this env is in default_envs. That can skew V4 regression results away from the standard build. Consider moving these flags to an override or a dedicated test env.
Do you want me to draft a minimal test-only env block or open a tracking issue for this cleanup?
🤖 Prompt for AI Agents
In `@platformio.ini` around lines 505 - 525, The esp32dev_V4 regression
environment contains test-only build flags (WLED_USE_ETHERNET and the pin
overrides RLYPIN and BTNPIN) which should be removed from the default_envs to
avoid skewing V4 regression results; remove those flags from the
[env:esp32dev_V4] build_flags and create a separate test-only env (e.g.,
[env:esp32dev_V4_ethernet_test]) that sets -D WLED_USE_ETHERNET -D RLYPIN=-1 -D
BTNPIN=-1 (and any related lib_ignore entries such as esp_dmx) so the regression
env remains standard while the test env captures the temporary Ethernet/pin
overrides.
| #if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) | ||
| ledcSetup(channel, _frequency, _depth - (dithering*4)); // with dithering _frequency doesn't really matter as resolution is 8 bit | ||
| ledcAttachPin(_pins[i], channel); | ||
| #else | ||
| ledcAttach(_pins[i], _frequency, _depth - (dithering*4)); | ||
| // LEDC timer reset credit @dedehai | ||
| #endif |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Arduino-ESP32 v3 ledcAttach ledcAttachChannel channel allocation API documentation
💡 Result:
In Arduino-ESP32 v3.x (ESP-IDF 5.x–based core), the LEDC API was redesigned so channels can be auto-assigned to pins via the Peripheral Manager (instead of you manually calling ledcSetup() + ledcAttachPin(), which were removed). [2]
API docs (Arduino-ESP32 “latest”)
-
Auto channel allocation
bool ledcAttach(uint8_t pin, uint32_t freq, uint8_t resolution);
Sets up PWM on
pinand selects an LEDC channel automatically. Returnstrueon success. [1] -
Manual channel selection
bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t channel);
Sets up PWM on
pinusing the specific LEDCchannelyou provide. Returnstrueon success. [1]
Notes relevant to “channel allocation”
- The docs explicitly state:
ledcAttach()selects the channel automatically, whileledcAttachChannel()lets you pick the channel. [1] - Available LEDC channel count varies by chip (e.g., ESP32: 16; ESP32-S3: 8; ESP32-C3: 6). [1]
- v3 migration note:
ledcSetup()andledcAttachPin()removed, replaced byledcAttach(), and many APIs that previously took achannelnow take apin. [2]
Sources: LEDC API reference [1], Arduino-ESP32 v2→v3 migration guide (LEDC section) [2].
🏁 Script executed:
# Find and examine bus_manager.cpp to understand channel usage context
fd bus_manager.cpp -x head -n 50 {}
# Check the specific lines mentioned (421-427) and surrounding code to see how channel is used
fd bus_manager.cpp -x sed -n '410,500p' {}Repository: wled/WLED
Length of output: 5613
🏁 Script executed:
# Search for how ledcAttach is used elsewhere in the codebase and check if ledcAttachChannel is available
rg "ledcAttach" wled00/ -A 3 -B 1
# Also search for any existing uses of ledcAttachChannel
rg "ledcAttachChannel" wled00/Repository: wled/WLED
Length of output: 563
🌐 Web query:
Arduino-ESP32 ledcAttachChannel API availability ESP-IDF 5.0 documentation
💡 Result:
ledcAttachChannel() is not an ESP-IDF 5.0 LEDC API function, so you won’t find it in the ESP-IDF v5.0 LEDC documentation. In ESP-IDF v5.0, LEDC setup is done with APIs such as ledc_timer_config() and ledc_channel_config() (and related ledc_set_duty(), ledc_update_duty(), etc.). [1]
ledcAttachChannel() does exist in Arduino-ESP32, as part of Arduino-ESP32’s own LEDC wrapper API (documented under Arduino-ESP32 “LED Control (LEDC)”). [2]
Sources:
[1] ESP-IDF v5.0 LEDC (PWM) API docs (no ledcAttachChannel, uses ledc_timer_config, ledc_channel_config, etc.)
[2] Arduino-ESP32 LEDC API docs (includes ledcAttachChannel(pin, freq, resolution, channel))
🏁 Script executed:
# Get more context around the timer reset code to see if it's executed on both code paths
fd bus_manager.cpp -x sed -n '415,440p' {}
# Check if there are other LEDC-related operations that depend on the channel value
fd bus_manager.cpp -x grep -n "LEDC\|channel\|_ledcStart" {}Repository: wled/WLED
Length of output: 5098
🌐 Web query:
Arduino-ESP32 v3 ledcAttach auto-allocated channel number how to retrieve which channel was assigned
💡 Result:
In Arduino-ESP32 v3.x, ledcAttach(pin, freq, resolution) auto-selects a channel but only returns bool, and the public Arduino LEDC API does not provide a “get channel for this pin” function. The API is intentionally pin-based (e.g., ledcWrite(pin, duty)). [1]
What you can do instead:
-
If you need to know/control the channel: use manual assignment
ledcAttachChannel(pin, freq, resolution, channel);[1]
-
If you just want to see what got auto-assigned (debug-only): enable core debug logging (e.g., Arduino IDE “Core Debug Level: Verbose”). The core prints lines like:
ledcAttachChannel(): LEDC attached to pin X (channel Y, resolution ...)
which reveals the chosen channel. [2]
🏁 Script executed:
# Verify ledcAttachChannel signature in Arduino-ESP32 headers to confirm it supports the proposed fix
rg "ledcAttachChannel" --type-list | head -5
# Search for any existing uses or documentation of ledcAttachChannel in WLED
rg "ledcAttachChannel\|ledcAttach" wled00/ -B 2 -A 2Repository: wled/WLED
Length of output: 442
🌐 Web query:
Arduino-ESP32 ledcAttachChannel function signature parameters pin channel frequency resolution
💡 Result:
ledcAttachChannel (Arduino-ESP32 LEDC API) signature:
bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, uint8_t channel);[1]
Parameters:
- pin (
uint8_t): GPIO pin to output PWM on. [1] - freq (
uint32_t): PWM frequency (Hz). [1] - resolution (
uint8_t): PWM duty resolution in bits (docs note 1–14 bits, or 1–20 bits on ESP32). [1] - channel (
uint8_t): LEDC channel number to use. [1]
Returns true on success, false on failure. [1]
Sources:
[1] (espressif-docs.readthedocs-hosted.com)
Citations:
Use ledcAttachChannel() to ensure deterministic LEDC channel assignment on IDF v5.
ledcAttach() auto-allocates a channel in Arduino-ESP32 v3, but the code assumes channel == _ledcStart + i for timer reset (line 429) and direct LEDC register writes (lines 549–555). If the auto-allocated channel differs from the assumed value, timer reset targets the wrong timer and PWM duty writes corrupt the wrong LEDC channels, breaking phase synchronization and LED output.
Proposed fix
`#if` ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)
ledcSetup(channel, _frequency, _depth - (dithering*4)); // with dithering _frequency doesn't really matter as resolution is 8 bit
ledcAttachPin(_pins[i], channel);
`#else`
- ledcAttach(_pins[i], _frequency, _depth - (dithering*4));
+ ledcAttachChannel(_pins[i], _frequency, _depth - (dithering*4), channel);
// LEDC timer reset credit `@dedehai`
`#endif`🤖 Prompt for AI Agents
In `@wled00/bus_manager.cpp` around lines 421 - 427, The code currently calls
ledcAttach(...) under IDF v5 which auto-allocates channels; change this to
explicitly bind the expected channel index (channel = _ledcStart + i) using
ledcAttachChannel (or the v5 equivalent that takes a channel argument) so that
the assigned channel matches the code's later assumptions (references:
ledcAttach, channel, _ledcStart, _pins, and the timer reset and direct register
writes around lines noted). Ensure you compute/assign channel = _ledcStart + i
before attaching and replace the ledcAttach(...) call with
ledcAttachChannel(channel, _pins[i], /*...params...*/) so timer reset and PWM
duty writes target the correct channel.
Update the code to handle at least being able to compile against V5
Edit (softhack007): espressif has "silently" changed a lot of APIs, and - in many cases - replacements are not behaving the same way as before. We are lucky that a few "legacy APIs" are still available, like the "V4" I2S driver. So even when "it compiles", there may be a lot more adaptations needed before "it really works".
Helpful information and migration guidance
coding guides
We should make sure that the "V5" branch will still compile for "V4" as a backup solution.
When adding code that only works in the new framework, it should be conditionally compiled, like in the examples below
related
Summary by CodeRabbit
New Features
Bug Fixes
Chores