-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Improved bus handling: free choice of bus driver in any order and improved memory calculations #5303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
DedeHai
wants to merge
29
commits into
wled:main
Choose a base branch
from
DedeHai:LEDdriverSelect
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Improved bus handling: free choice of bus driver in any order and improved memory calculations #5303
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
9ad0d97
WIP: make bus driver (RMT/I2S) user-selectable
Copilot 8d9e775
define new constants instead of using local variable (WIP)
DedeHai 766097c
Simplify bus allocation by storing iType in BusConfig
Copilot b518c19
Fix parallel I2S DMA buffer estimation (8x multiplier)
Copilot 6934100
improvements to getI()
DedeHai cd1148d
Improve UI validation logic for RMT/I2S bus management
Copilot 044506f
Refactor UI validation: extract helper function and fix duplicate res…
Copilot 5ebf650
Apply I2S validation rules to finalizeInit() matching UI behavior
Copilot 89f07fd
add globals to UI
DedeHai d389806
remove obsolete I2S user select option, bugfix in getI(), better UI v…
DedeHai 02393b2
remove "600 LEDs max" restriction, bugfixes in mem calc, other fixes
DedeHai cb24aec
add proper restriction to RMT/I2S buses for legacy configs, still nee…
DedeHai ae2912d
cleanup, remove unused/duplicate code
DedeHai bf4a066
cleanup, simplified memory calculation code by removing clutter
DedeHai 0954f6c
cleanup, add ifdefs for ESP8266 to save some ram (UI for ESP8266 is b…
DedeHai e4f89e5
bugfix in UI: mux max buses instead of RMT+I2S which are zero on ESP8…
DedeHai 94974fc
fix in UI if all RMT buses are used (restrict type)
DedeHai 5231b27
fixes in memory calculations, added buffers to memory use, updated ma…
DedeHai 33bacc1
removed some debug outputs
DedeHai 0ea65d0
bugfixes, added ptr=nullptr to p_free() and d_free() for robustness
DedeHai 0a724bd
bugfixes, removed annoying debug output
DedeHai 8c10915
improvements for S2
DedeHai ead95e8
update for C3, removed max LED restriction, increased MAX_LED_MEMORY
DedeHai 7554783
use CONFIG_IDF_TARGET_ESP32 instead of ARDUINO_ARCH
DedeHai ff34848
revert nullpointer mishap
DedeHai bb7764c
Merge branch 'main' into LEDdriverSelect
DedeHai a87fd3a
Merge branch 'main' into LEDdriverSelect
DedeHai d2d11fe
revert nullptr change
DedeHai 9e1a864
merge fixes
DedeHai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,6 @@ | |
|
|
||
| extern char cmDNS[]; | ||
| extern bool cctICused; | ||
| extern bool useParallelI2S; | ||
|
|
||
| // functions to get/set bits in an array - based on functions created by Brandon for GOL | ||
| // toDo : make this a class that's completely defined in a header file | ||
|
|
@@ -117,15 +116,17 @@ uint32_t Bus::autoWhiteCalc(uint32_t c) const { | |
| } | ||
|
|
||
|
|
||
| BusDigital::BusDigital(const BusConfig &bc, uint8_t nr) | ||
| BusDigital::BusDigital(const BusConfig &bc) | ||
| : Bus(bc.type, bc.start, bc.autoWhite, bc.count, bc.reversed, (bc.refreshReq || bc.type == TYPE_TM1814)) | ||
| , _skip(bc.skipAmount) //sacrificial pixels | ||
| , _colorOrder(bc.colorOrder) | ||
| , _milliAmpsPerLed(bc.milliAmpsPerLed) | ||
| , _milliAmpsMax(bc.milliAmpsMax) | ||
| , _driverType(bc.driverType) // Store driver preference (0=RMT, 1=I2S) | ||
| { | ||
| DEBUGBUS_PRINTLN(F("Bus: Creating digital bus.")); | ||
| if (!isDigital(bc.type) || !bc.count) { DEBUGBUS_PRINTLN(F("Not digial or empty bus!")); return; } | ||
|
|
||
| if (!PinManager::allocatePin(bc.pins[0], true, PinOwner::BusDigital)) { DEBUGBUS_PRINTLN(F("Pin 0 allocated!")); return; } | ||
| _frequencykHz = 0U; | ||
| _colorSum = 0; | ||
|
|
@@ -139,28 +140,32 @@ BusDigital::BusDigital(const BusConfig &bc, uint8_t nr) | |
| _pins[1] = bc.pins[1]; | ||
| _frequencykHz = bc.frequency ? bc.frequency : 2000U; // 2MHz clock if undefined | ||
| } | ||
| _iType = PolyBus::getI(bc.type, _pins, nr); | ||
|
|
||
| _iType = bc.iType; // reuse the iType that was determined by polyBus in getI() in finalizeInit() | ||
| if (_iType == I_NONE) { DEBUGBUS_PRINTLN(F("Incorrect iType!")); return; } | ||
| _hasRgb = hasRGB(bc.type); | ||
| _hasWhite = hasWhite(bc.type); | ||
| _hasCCT = hasCCT(bc.type); | ||
| uint16_t lenToCreate = bc.count; | ||
| if (bc.type == TYPE_WS2812_1CH_X3) lenToCreate = NUM_ICS_WS2812_1CH_3X(bc.count); // only needs a third of "RGB" LEDs for NeoPixelBus | ||
| _busPtr = PolyBus::create(_iType, _pins, lenToCreate + _skip, nr); | ||
| _busPtr = PolyBus::create(_iType, _pins, lenToCreate + _skip); | ||
| _valid = (_busPtr != nullptr) && bc.count > 0; | ||
| // fix for wled#4759 | ||
| if (_valid) for (unsigned i = 0; i < _skip; i++) { | ||
| PolyBus::setPixelColor(_busPtr, _iType, i, 0, COL_ORDER_GRB); // set sacrificial pixels to black (CO does not matter here) | ||
| } | ||
| DEBUGBUS_PRINTF_P(PSTR("Bus: %successfully inited #%u (len:%u, type:%u (RGB:%d, W:%d, CCT:%d), pins:%u,%u [itype:%u] mA=%d/%d)\n"), | ||
| _valid?"S":"Uns", | ||
| (int)nr, | ||
| else { | ||
| cleanup(); | ||
| } | ||
|
Comment on lines
+144
to
+159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Release allocated pins when Line 145 returns early on 🛠️ Suggested fix _pins[0] = bc.pins[0];
+ _pins[1] = 255U; // default for 1‑pin buses
if (is2Pin(bc.type)) {
if (!PinManager::allocatePin(bc.pins[1], true, PinOwner::BusDigital)) {
cleanup();
DEBUGBUS_PRINTLN(F("Pin 1 allocated!"));
return;
}
_pins[1] = bc.pins[1];
_frequencykHz = bc.frequency ? bc.frequency : 2000U; // 2MHz clock if undefined
}
_iType = bc.iType; // reuse the iType that was determined by polyBus in getI() in finalizeInit()
- if (_iType == I_NONE) { DEBUGBUS_PRINTLN(F("Incorrect iType!")); return; }
+ if (_iType == I_NONE) {
+ DEBUGBUS_PRINTLN(F("Incorrect iType!"));
+ cleanup(); // release pins allocated above
+ return;
+ }🤖 Prompt for AI Agents |
||
| DEBUGBUS_PRINTF_P(PSTR("Bus len:%u, type:%u (RGB:%d, W:%d, CCT:%d), pins:%u,%u [itype:%u, driver:%s] mA=%d/%d %s\n"), | ||
| (int)bc.count, | ||
| (int)bc.type, | ||
| (int)_hasRgb, (int)_hasWhite, (int)_hasCCT, | ||
| (unsigned)_pins[0], is2Pin(bc.type)?(unsigned)_pins[1]:255U, | ||
| (unsigned)_iType, | ||
| (int)_milliAmpsPerLed, (int)_milliAmpsMax | ||
| isI2S() ? "I2S" : "RMT", | ||
| (int)_milliAmpsPerLed, (int)_milliAmpsMax, | ||
| _valid ? " " : "FAILED" | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -351,6 +356,10 @@ std::vector<LEDType> BusDigital::getLEDTypes() { | |
| }; | ||
| } | ||
|
|
||
| bool BusDigital::isI2S() { | ||
| return (_iType & 0x01) == 0; // I2S types have even iType values | ||
| } | ||
DedeHai marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| void BusDigital::begin() { | ||
| if (!_valid) return; | ||
| PolyBus::begin(_busPtr, _iType, _pins, _frequencykHz); | ||
|
|
@@ -1105,7 +1114,7 @@ size_t BusHub75Matrix::getPins(uint8_t* pinArray) const { | |
| #endif | ||
| // *************************************************************************** | ||
|
|
||
| BusPlaceholder::BusPlaceholder(const BusConfig &bc) | ||
| BusPlaceholder::BusPlaceholder(const BusConfig &bc) | ||
| : Bus(bc.type, bc.start, bc.autoWhite, bc.count, bc.reversed, bc.refreshReq) | ||
| , _colorOrder(bc.colorOrder) | ||
| , _skipAmount(bc.skipAmount) | ||
|
|
@@ -1125,47 +1134,20 @@ size_t BusPlaceholder::getPins(uint8_t* pinArray) const { | |
| return nPins; | ||
| } | ||
|
|
||
| //utility to get the approx. memory usage of a given BusConfig | ||
| size_t BusConfig::memUsage(unsigned nr) const { | ||
| //utility to get the approx. memory usage of a given BusConfig inclduding segmentbuffer and global buffer (4 bytes per pixel) | ||
| size_t BusConfig::memUsage() const { | ||
| size_t mem = (count + skipAmount) * 8; // 8 bytes per pixel for segment + global buffer | ||
| if (Bus::isVirtual(type)) { | ||
| return sizeof(BusNetwork) + (count * Bus::getNumberOfChannels(type)); | ||
| mem += sizeof(BusNetwork) + (count * Bus::getNumberOfChannels(type)); // note: getNumberOfChannels() includes CCT channel if applicable but virtual buses do not use CCT channel buffer | ||
| } else if (Bus::isDigital(type)) { | ||
| // if any of digital buses uses I2S, there is additional common I2S DMA buffer not accounted for here | ||
| return sizeof(BusDigital) + PolyBus::memUsage(count + skipAmount, PolyBus::getI(type, pins, nr)); | ||
| mem += sizeof(BusDigital) + PolyBus::memUsage(count + skipAmount, iType); | ||
| } else if (Bus::isOnOff(type)) { | ||
| return sizeof(BusOnOff); | ||
| mem += sizeof(BusOnOff); | ||
| } else { | ||
| return sizeof(BusPwm); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| size_t BusManager::memUsage() { | ||
| // when ESP32, S2 & S3 use parallel I2S only the largest bus determines the total memory requirements for back buffers | ||
| // front buffers are always allocated per bus | ||
| unsigned size = 0; | ||
| unsigned maxI2S = 0; | ||
| #if !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(ESP8266) | ||
| unsigned digitalCount = 0; | ||
| #endif | ||
| for (const auto &bus : busses) { | ||
| size += bus->getBusSize(); | ||
| #if !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(ESP8266) | ||
| if (bus->isDigital() && !bus->is2Pin()) { | ||
| digitalCount++; | ||
| if ((PolyBus::isParallelI2S1Output() && digitalCount <= 8) || (!PolyBus::isParallelI2S1Output() && digitalCount == 1)) { | ||
| #ifdef NPB_CONF_4STEP_CADENCE | ||
| constexpr unsigned stepFactor = 4; // 4 step cadence (4 bits per pixel bit) | ||
| #else | ||
| constexpr unsigned stepFactor = 3; // 3 step cadence (3 bits per pixel bit) | ||
| #endif | ||
| unsigned i2sCommonSize = stepFactor * bus->getLength() * bus->getNumberOfChannels() * (bus->is16bit()+1); | ||
| if (i2sCommonSize > maxI2S) maxI2S = i2sCommonSize; | ||
| } | ||
| } | ||
| #endif | ||
| mem += sizeof(BusPwm); | ||
| } | ||
| return size + maxI2S; | ||
| return mem; | ||
| } | ||
|
|
||
| int BusManager::add(const BusConfig &bc, bool placeholder) { | ||
|
|
@@ -1190,7 +1172,7 @@ int BusManager::add(const BusConfig &bc, bool placeholder) { | |
| busses.push_back(make_unique<BusHub75Matrix>(bc)); | ||
| #endif | ||
| } else if (Bus::isDigital(bc.type)) { | ||
| busses.push_back(make_unique<BusDigital>(bc, Bus::is2Pin(bc.type) ? twoPin : digital)); | ||
| busses.push_back(make_unique<BusDigital>(bc)); | ||
| } else if (Bus::isOnOff(bc.type)) { | ||
| busses.push_back(make_unique<BusOnOff>(bc)); | ||
| } else { | ||
|
|
@@ -1228,49 +1210,35 @@ String BusManager::getLEDTypesJSONString() { | |
| return json; | ||
| } | ||
|
|
||
| void BusManager::useParallelOutput() { | ||
| DEBUGBUS_PRINTLN(F("Bus: Enabling parallel I2S.")); | ||
| PolyBus::setParallelI2S1Output(); | ||
| uint8_t BusManager::getI(uint8_t busType, const uint8_t* pins, uint8_t driverPreference) { | ||
| return PolyBus::getI(busType, pins, driverPreference); | ||
| } | ||
|
|
||
| bool BusManager::hasParallelOutput() { | ||
| return PolyBus::isParallelI2S1Output(); | ||
| } | ||
|
|
||
| //do not call this method from system context (network callback) | ||
| void BusManager::removeAll() { | ||
| DEBUGBUS_PRINTLN(F("Removing all.")); | ||
| //prevents crashes due to deleting busses while in use. | ||
| while (!canAllShow()) yield(); | ||
| busses.clear(); | ||
| PolyBus::setParallelI2S1Output(false); | ||
| #ifndef ESP8266 | ||
| // Reset channel tracking for fresh allocation | ||
| PolyBus::resetChannelTracking(); | ||
| #endif | ||
| } | ||
|
|
||
| #ifdef ESP32_DATA_IDLE_HIGH | ||
| // #2478 | ||
| // If enabled, RMT idle level is set to HIGH when off | ||
| // to prevent leakage current when using an N-channel MOSFET to toggle LED power | ||
| // since I2S outputs are known only during config of buses, lets just assume RMT is used for digital buses | ||
| // unused RMT channels should have no effect | ||
| void BusManager::esp32RMTInvertIdle() { | ||
| bool idle_out; | ||
| unsigned rmt = 0; | ||
| unsigned u = 0; | ||
| for (auto &bus : busses) { | ||
| if (bus->getLength()==0 || !bus->isDigital() || bus->is2Pin()) continue; | ||
| #if defined(CONFIG_IDF_TARGET_ESP32C3) // 2 RMT, only has 1 I2S but NPB does not support it ATM | ||
| if (u > 1) return; | ||
| rmt = u; | ||
| #elif defined(CONFIG_IDF_TARGET_ESP32S2) // 4 RMT, only has 1 I2S bus, supported in NPB | ||
| if (u > 3) return; | ||
| rmt = u; | ||
| #elif defined(CONFIG_IDF_TARGET_ESP32S3) // 4 RMT, has 2 I2S but NPB does not support them ATM | ||
| if (u > 3) return; | ||
| rmt = u; | ||
| #else | ||
| unsigned numI2S = !PolyBus::isParallelI2S1Output(); // if using parallel I2S, RMT is used 1st | ||
| if (numI2S > u) continue; | ||
| if (u > 7 + numI2S) return; | ||
| rmt = u - numI2S; | ||
| #endif | ||
| if (static_cast<BusDigital*>(bus.get())->isI2S()) continue; | ||
| if (u >= WLED_MAX_RMT_CHANNELS) return; | ||
| //assumes that bus number to rmt channel mapping stays 1:1 | ||
| rmt_channel_t ch = static_cast<rmt_channel_t>(rmt); | ||
| rmt_idle_level_t lvl; | ||
|
|
@@ -1445,9 +1413,15 @@ void BusManager::applyABL() { | |
|
|
||
| ColorOrderMap& BusManager::getColorOrderMap() { return _colorOrderMap; } | ||
|
|
||
|
|
||
| #ifndef ESP8266 | ||
| // PolyBus channel tracking for dynamic allocation | ||
| bool PolyBus::_useParallelI2S = false; | ||
|
|
||
| uint8_t PolyBus::_rmtChannelsAssigned = 0; // number of RMT channels assigned durig getI() check | ||
| uint8_t PolyBus::_rmtChannel = 0; // number of RMT channels actually used during bus creation in create() | ||
| uint8_t PolyBus::_i2sChannelsAssigned = 0; // number of I2S channels assigned durig getI() check | ||
| uint8_t PolyBus::_parallelBusItype = 0; // type I_NONE | ||
| uint8_t PolyBus::_2PchannelsAssigned = 0; | ||
| #endif | ||
| // Bus static member definition | ||
| int16_t Bus::_cct = -1; | ||
| uint8_t Bus::_cctBlend = 0; // 0 - 127 | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.