Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 48 additions & 10 deletions wled00/bus_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,16 @@ BusDigital::BusDigital(const BusConfig &bc)
_hasWhite = hasWhite(bc.type);
_hasCCT = hasCCT(bc.type);
uint16_t lenToCreate = bc.count;
uint16_t skipToCreate = _skip;
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);
if (bc.type == TYPE_WS2812_RGBW_PAIR) { // one logical RGBW bulb is backed by two WS281x pixels
lenToCreate *= 2;
skipToCreate *= 2;
}
_busPtr = PolyBus::create(_iType, _pins, lenToCreate + skipToCreate);
_valid = (_busPtr != nullptr) && bc.count > 0;
// fix for wled#4759
if (_valid) for (unsigned i = 0; i < _skip; i++) {
if (_valid) for (unsigned i = 0; i < skipToCreate; i++) {
PolyBus::setPixelColor(_busPtr, _iType, i, 0, COL_ORDER_GRB); // set sacrificial pixels to black (CO does not matter here)
}
else {
Expand Down Expand Up @@ -202,7 +207,12 @@ void BusDigital::estimateCurrent() {
}
// _colorSum has all the values of color channels summed, max would be getLength()*(3*255 + (255 if hasWhite()): convert to milliAmps
uint32_t clrUnitsPerChannel = hasWhite() ? 4*255 : 3*255;
_milliAmpsTotal = ((uint64_t)_colorSum * actualMilliampsPerLed) / clrUnitsPerChannel + getLength(); // add 1mA standby current per LED to total (WS2812: ~0.7mA, WS2815: ~2mA)
uint16_t standbyMilliamps = getPhysicalLength(); // add 1mA standby current per LED to total (WS2812: ~0.7mA, WS2815: ~2mA)
if (_type == TYPE_WS2812_RGBW_PAIR && _milliAmpsPerLed < 255) {
actualMilliampsPerLed *= 2;
clrUnitsPerChannel = 6*255;
}
_milliAmpsTotal = ((uint64_t)_colorSum * actualMilliampsPerLed) / clrUnitsPerChannel + standbyMilliamps;
}

void BusDigital::applyBriLimit(uint8_t newBri) {
Expand All @@ -212,15 +222,16 @@ void BusDigital::applyBriLimit(uint8_t newBri) {
if (_milliAmpsLimit == 0 || _milliAmpsTotal == 0) return; // ABL not used for this bus
newBri = 255;

if (_milliAmpsLimit > getLength()) { // each LED uses about 1mA in standby
const uint16_t standbyMilliamps = getPhysicalLength(); // each LED uses about 1mA in standby
if (_milliAmpsLimit > standbyMilliamps) {
if (_milliAmpsTotal > _milliAmpsLimit) {
// scale brightness down to stay in current limit
newBri = ((uint32_t)_milliAmpsLimit * 255) / _milliAmpsTotal + 1; // +1 to avoid 0 brightness
_milliAmpsTotal = _milliAmpsLimit;
}
} else {
newBri = 1; // limit too low, set brightness to 1, this will dim down all colors to minimum since we use video scaling
_milliAmpsTotal = getLength(); // estimate bus current as minimum
_milliAmpsTotal = standbyMilliamps; // estimate bus current as minimum
}
}

Expand All @@ -229,8 +240,10 @@ void BusDigital::applyBriLimit(uint8_t newBri) {
uint16_t wwcw = 0;
unsigned hwLen = _len;
if (_type == TYPE_WS2812_1CH_X3) hwLen = NUM_ICS_WS2812_1CH_3X(_len); // only needs a third of "RGB" LEDs for NeoPixelBus
if (_type == TYPE_WS2812_RGBW_PAIR) hwLen = (_len + _skip) * 2;
for (unsigned i = 0; i < hwLen; i++) {
uint8_t co = _colorOrderMap.getPixelColorOrder(i+_start, _colorOrder); // need to revert color order for correct color scaling and CCT calc in case white is swapped
uint8_t co = _colorOrderMap.getPixelColorOrder((_type == TYPE_WS2812_RGBW_PAIR ? i/2 : i)+_start, _colorOrder); // need to revert color order for correct color scaling and CCT calc in case white is swapped
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (_type == TYPE_WS2812_RGBW_PAIR) co &= 0x0F; // paired physical pixels are RGB-only WS281x chips
uint32_t c = PolyBus::getPixelColor(_busPtr, _iType, i, co); // Note: if ABL would be calculated as a seperate loop (as it was before) it is slower but could use original color, making it more color-accurate
if (hasCCT()) {
uint8_t cctWW, cctCW;
Expand Down Expand Up @@ -284,14 +297,26 @@ void IRAM_ATTR BusDigital::setPixelColor(unsigned pix, uint32_t c) {
if (BusManager::_useABL) {
// if using ABL, sum all color channels to estimate current and limit brightness in show()
uint8_t r = R(c), g = G(c), b = B(c);
if (_milliAmpsPerLed < 255) { // normal ABL
if (_type == TYPE_WS2812_RGBW_PAIR && _milliAmpsPerLed < 255) {
_colorSum += r + g + b + 3 * W(c);
} else if (_milliAmpsPerLed < 255) { // normal ABL
_colorSum += r + g + b + W(c);
} else { // wacky WS2815 power model, ignore white channel, use max of RGB (issue #549)
_colorSum += ((r > g) ? ((r > b) ? r : b) : ((g > b) ? g : b));
}
}

if (_reversed) pix = _len - pix -1;
if (_type == TYPE_WS2812_RGBW_PAIR) {
// AI: below section was generated by an AI
// Map one logical RGBW bulb onto two WS281x pixels observed as RGB first, WW second.
unsigned physicalPix = (pix + _skip) * 2;
const uint8_t co = _colorOrderMap.getPixelColorOrder(pix+_skip+_start, _colorOrder) & 0x0F;
PolyBus::setPixelColor(_busPtr, _iType, physicalPix, RGBW32(R(c), G(c), B(c), 0), co);
PolyBus::setPixelColor(_busPtr, _iType, physicalPix + 1, RGBW32(W(c), W(c), W(c), 0), co);
// AI: end
return;
}
pix += _skip;
const uint8_t co = _colorOrderMap.getPixelColorOrder(pix+_start, _colorOrder);
if (_type == TYPE_WS2812_1CH_X3) { // map to correct IC, each controls 3 LEDs
Expand All @@ -312,6 +337,17 @@ void IRAM_ATTR BusDigital::setPixelColor(unsigned pix, uint32_t c) {
uint32_t IRAM_ATTR BusDigital::getPixelColor(unsigned pix) const {
if (!_valid) return 0;
if (_reversed) pix = _len - pix -1;
if (_type == TYPE_WS2812_RGBW_PAIR) {
// AI: below section was generated by an AI
// Reconstruct one logical RGBW value from the paired RGB and WW physical pixels.
unsigned physicalPix = (pix + _skip) * 2;
const uint8_t co = _colorOrderMap.getPixelColorOrder(pix+_skip+_start, _colorOrder) & 0x0F;
uint32_t rgb = restoreColorLossy(PolyBus::getPixelColor(_busPtr, _iType, physicalPix, co), _NPBbri);
uint32_t ww = restoreColorLossy(PolyBus::getPixelColor(_busPtr, _iType, physicalPix + 1, co), _NPBbri);
uint8_t w = std::max(R(ww), std::max(G(ww), B(ww)));
// AI: end
return RGBW32(R(rgb), G(rgb), B(rgb), w);
}
pix += _skip;
const uint8_t co = _colorOrderMap.getPixelColorOrder(pix+_start, _colorOrder);
uint32_t c = restoreColorLossy(PolyBus::getPixelColor(_busPtr, _iType, (_type==TYPE_WS2812_1CH_X3) ? IC_INDEX_WS2812_1CH_3X(pix) : pix, co),_NPBbri);
Expand Down Expand Up @@ -363,6 +399,7 @@ std::vector<LEDType> BusDigital::getLEDTypes() {
{TYPE_FW1906, "D", PSTR("FW1906/WS2811 RGBCCT")},
{TYPE_WS2805, "D", PSTR("WS2805 RGBCCT")},
{TYPE_SM16825, "D", PSTR("SM16825 RGBCCT")},
{TYPE_WS2812_RGBW_PAIR, "D", PSTR("WS281x RGB+WW Pair")},
{TYPE_WS2812_1CH_X3, "D", PSTR("WS2811 White")},
{TYPE_WS2812_WWA, "D", PSTR("WS281x WWA")}, // amber ignored
{TYPE_WS2801, "2P", PSTR("WS2801 RGB")},
Expand Down Expand Up @@ -1270,7 +1307,8 @@ size_t BusConfig::memUsage() const {
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
mem += sizeof(BusDigital) + PolyBus::memUsage(count + skipAmount, iType);
const uint16_t physicalCount = (type == TYPE_WS2812_RGBW_PAIR) ? (count + skipAmount) * 2 : count + skipAmount;
mem += sizeof(BusDigital) + PolyBus::memUsage(physicalCount, iType);
} else if (Bus::isOnOff(type)) {
mem += sizeof(BusOnOff);
} else {
Expand Down Expand Up @@ -1494,7 +1532,7 @@ void BusManager::initializeABL() {
for (auto &bus : busses) {
if (bus->isDigital() && bus->isOk()) {
BusDigital &busd = static_cast<BusDigital&>(*bus);
uint32_t busLength = busd.getLength();
uint32_t busLength = busd.getPhysicalLength();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? This seems to affect all busses, not just the new paired type.

@coderabbitai does this change create side-effects for other LED drivers?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

header=$(fd -a '^bus_manager\.h$' . | head -n 1)
printf '%s\n' '--- BusDigital declaration and getPhysicalLength implementation ---'
rg -n -A12 -B12 'getPhysicalLength|class BusDigital' "$header" wled00/bus_manager.cpp

printf '%s\n' '--- All getPhysicalLength call sites ---'
rg -n -A3 -B3 '\bgetPhysicalLength\s*\(' wled00

printf '%s\n' '--- Relevant PR diff ---'
git diff --unified=30 HEAD^ HEAD -- wled00/bus_manager.cpp wled00/bus_manager.h

Length of output: 19861


@softhack007, I verified the implementation.

getPhysicalLength() returns _len for every existing digital LED type. It returns _len * 2 only for TYPE_WS2812_RGBW_PAIR.

Therefore, Line 1535 has no functional side effect for other LED drivers. Their ABL calculation remains unchanged.

For TYPE_WS2812_RGBW_PAIR, the doubled value is required because each logical RGBW bulb has two physical WS281x LEDs. The value sets both:

  • The per-bus ABL demand: busLength * getLEDCurrent().
  • The minimum standby-current allowance: one mA per physical LED.

This change is correct. I withdraw the concern.

@softhack007 I agree that this finding should be withdrawn, but I couldn't resolve this review thread on the repository platform, so it remains open. Please retry or resolve it manually.

You are interacting with an AI system.

uint32_t busDemand = busLength * busd.getLEDCurrent();
uint32_t busMax = busd.getMaxCurrent();
if (busMax > ESPshare) busMax -= ESPshare;
Expand All @@ -1518,7 +1556,7 @@ void BusManager::applyABL() {
if (_gMilliAmpsMax == 0)
busd.applyBriLimit(0); // apply per bus ABL limit, updates _milliAmpsTotal if limit reached
milliAmpsSum += busd.getUsedCurrent();
totalLEDs += busd.getLength(); // sum total number of LEDs for global Limit
totalLEDs += busd.getPhysicalLength(); // sum total number of physical LEDs for global Limit

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here - this change affects all LED drivers

}
}
// check global current limit and apply global ABL limit, total current is summed above
Expand Down
4 changes: 3 additions & 1 deletion wled00/bus_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ class Bus {
static constexpr bool hasWhite(uint8_t type) {
return (type >= TYPE_WS2812_1CH && type <= TYPE_WS2812_WWA) ||
type == TYPE_SK6812_RGBW || type == TYPE_TM1814 || type == TYPE_UCS8904 ||
type == TYPE_FW1906 || type == TYPE_WS2805 || type == TYPE_SM16825 || // digital types with white channel
type == TYPE_FW1906 || type == TYPE_WS2805 || type == TYPE_SM16825 ||
type == TYPE_WS2812_RGBW_PAIR || // digital types with white channel
(type > TYPE_ONOFF && type <= TYPE_ANALOG_5CH && type != TYPE_ANALOG_3CH) || // analog types with white channel
type == TYPE_NET_DDP_RGBW || type == TYPE_NET_ARTNET_RGBW; // network types with white channel
}
Expand Down Expand Up @@ -263,6 +264,7 @@ class BusDigital : public Bus {
uint16_t getLEDCurrent() const override { return _milliAmpsPerLed; }
uint16_t getUsedCurrent() const override { return _milliAmpsTotal; }
uint16_t getMaxCurrent() const override { return _milliAmpsMax; }
uint16_t getPhysicalLength() const { return (_type == TYPE_WS2812_RGBW_PAIR) ? _len * 2 : _len; }
uint8_t getDriverType() const override { return _driverType; }
void setCurrentLimit(uint16_t milliAmps) { _milliAmpsLimit = milliAmps; }
void estimateCurrent(); // estimate used current from summed colors
Expand Down
2 changes: 2 additions & 0 deletions wled00/bus_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,7 @@ class PolyBus {
if (offset > 3) offset = 3;
switch (busType) {
case TYPE_WS2812_1CH_X3:
case TYPE_WS2812_RGBW_PAIR:
case TYPE_WS2812_RGB:
case TYPE_WS2812_WWA:
t = I_8266_U0_NEO_3 + offset; break;
Expand Down Expand Up @@ -1370,6 +1371,7 @@ class PolyBus {
// Now determine actual bus type with the chosen offset
switch (busType) {
case TYPE_WS2812_1CH_X3:
case TYPE_WS2812_RGBW_PAIR:
case TYPE_WS2812_RGB:
case TYPE_WS2812_WWA:
t = I_32_RN_NEO_3 + offset; break;
Expand Down
1 change: 1 addition & 0 deletions wled00/const.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit");
#define TYPE_WS2805 32 //RGB + WW + CW
#define TYPE_TM1914 33 //RGB
#define TYPE_SM16825 34 //RGB + WW + CW
#define TYPE_WS2812_RGBW_PAIR 35 // paired WS281x pixels: WW, then RGB
#define TYPE_DIGITAL_MAX 39 // last usable digital type
//"Analog" types (40-47)
#define TYPE_ONOFF 40 //binary output (relays etc.; NOT PWM)
Expand Down
Loading