Skip to content
Closed
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
50 changes: 48 additions & 2 deletions wled00/bus_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,17 @@ BusDigital::BusDigital(const BusConfig &bc, uint8_t nr)
_hasWhite = hasWhite(bc.type);
_hasCCT = hasCCT(bc.type);
uint16_t lenToCreate = bc.count;
if (bc.type == TYPE_SM16703_DUAL) lenToCreate = (bc.count + _skip) * 2; // two SM16703 chips per logical pixel (including 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, nr);
if (bc.type == TYPE_SM16703_DUAL) {
_busPtr = PolyBus::create(_iType, _pins, lenToCreate, nr); // lenToCreate already includes skip * 2
} else {
_busPtr = PolyBus::create(_iType, _pins, lenToCreate + _skip, nr);
}
_valid = (_busPtr != nullptr) && bc.count > 0;
// fix for wled#4759
if (_valid) for (unsigned i = 0; i < _skip; i++) {
unsigned skipHW = (bc.type == TYPE_SM16703_DUAL) ? _skip * 2 : _skip;
if (_valid) for (unsigned i = 0; i < skipHW; i++) {
Comment on lines +189 to +199

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.

⚠️ Potential issue | 🟠 Major

Update memory accounting for the doubled hardware length.

This constructor now allocates (count + skip) * 2 pixels for TYPE_SM16703_DUAL, but the existing BusConfig::memUsage() / BusManager::memUsage() paths still size the bus from logical pixels. That underestimates RAM/DMA usage for this type and can make initialization heuristics overly optimistic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/bus_manager.cpp` around lines 189 - 199, Bus memory accounting
underestimates RAM/DMA usage for TYPE_SM16703_DUAL because constructor creates
(count + _skip) * 2 hardware pixels; update the memory calculations in
BusConfig::memUsage() and BusManager::memUsage() to mirror the allocation logic
used for PolyBus::create (i.e., compute lenToCreate the same way: if type ==
TYPE_SM16703_DUAL then use (count + _skip) * 2, if type == TYPE_WS2812_1CH_X3
use NUM_ICS_WS2812_1CH_3X(count), otherwise use count + _skip) so reported
memUsage reflects the doubled hardware length (and continue to handle the
skip/_skip multiplier consistently).

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"),
Expand Down Expand Up @@ -293,6 +299,45 @@ void IRAM_ATTR BusDigital::setPixelColor(unsigned pix, uint32_t c) {
if (Bus::_cct >= 1900) c = colorBalanceFromKelvin(Bus::_cct, c); //color correction from CCT
c = color_fade(c, _bri, true); // apply brightness

if (_type == TYPE_SM16703_DUAL) {
// logical pixel uses two SM16703 chips: first RGB, second WW/CW (B unused)
uint8_t r = R(c), g = G(c), b = B(c);
uint8_t ww = 0, cw = 0;
if (hasCCT()) Bus::calculateCCT(c, ww, cw);

if (BusManager::_useABL) {
if (_milliAmpsPerLed < 255) {
_colorSum += r + g + b + ww + cw; // include both chips' channels for ABL
} else {
// wacky model not expected; fall back to max RGB
uint8_t maxRgb = (r > g) ? ((r > b) ? r : b) : ((g > b) ? g : b);
_colorSum += maxRgb;
}
Comment on lines +308 to +315

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.

⚠️ Potential issue | 🟠 Major

Use a 5-channel ABL divisor for this type.

_colorSum now includes r + g + b + ww + cw, but estimateCurrent() still normalizes white-capable buses with 4 * 255. For TYPE_SM16703_DUAL that overestimates current by 25% and dims earlier than necessary.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/bus_manager.cpp` around lines 308 - 315, estimateCurrent() currently
normalizes ABL white-capable buses using 4 * 255 but your _colorSum for
TYPE_SM16703_DUAL (updated in BusManager::_useABL branch where _colorSum += r +
g + b + ww + cw and _milliAmpsPerLed < 255) uses five channels; change the
normalization/divisor logic in estimateCurrent() to use 5 * 255 for this dual
white-capable bus (or detect when _useABL and both ww/cw are included) so
current is calculated against 5 channels instead of 4, referencing
BusManager::_useABL, _colorSum, _milliAmpsPerLed, estimateCurrent(), and
TYPE_SM16703_DUAL to locate the relevant code paths.

}

unsigned logicalPix = pix;
if (_reversed) logicalPix = _len - pix - 1;
logicalPix += _skip;
unsigned firstIdx = logicalPix * 2;
unsigned secondIdx = firstIdx + 1;

// bounds safeguard (should not trigger if lenToCreate set correctly)
unsigned hwLen = (_len + _skip) * 2;
if (secondIdx >= hwLen) return;

const uint8_t coFirst = _colorOrderMap.getPixelColorOrder(logicalPix + _start, _colorOrder);
PolyBus::setPixelColor(_busPtr, _iType, firstIdx, c, coFirst, 0);

// Second chip: R=WW, G=CW by default; respect WW/CW swap flag (upper nibble of color order)
const uint8_t coSecond = _colorOrderMap.getPixelColorOrder(logicalPix + _start, _colorOrder);
bool swapWhites = true; // hardware wiring has WW/CW reversed relative to CCT
if ((coSecond >> 4) & 0x01) swapWhites = !swapWhites; // UI swap toggles
if (swapWhites) { uint8_t tmp = ww; ww = cw; cw = tmp; }
uint32_t wwCwColor = RGBW32(ww, cw, 0, 0);
PolyBus::setPixelColor(_busPtr, _iType, secondIdx, wwCwColor, COL_ORDER_GRB, 0);
return;
Comment on lines +331 to +338

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.

⚠️ Potential issue | 🟠 Major

Potential issue with WW/CW swap detection.

The swap logic on line 334 checks (coSecond >> 4) & 0x01, but the WW/CW swap flag in the color order upper nibble is value 4 (as seen in bus_wrapper.h line 809: case 4: std::swap(cctWW, cctCW);). Checking bit 0 won't detect value 4:

Upper nibble Binary & 0x01 result
0 (no swap) 0000 false
1 (W&B) 0001 true
4 (WW&CW) 0100 false

If the intent is to respect the WW/CW swap setting from the UI, the condition should check for value 4 specifically:

-    bool swapWhites = true; // hardware wiring has WW/CW reversed relative to CCT
-    if ((coSecond >> 4) & 0x01) swapWhites = !swapWhites; // UI swap toggles
+    bool swapWhites = true; // hardware wiring has WW/CW reversed relative to CCT
+    if ((coSecond >> 4) == 4) swapWhites = !swapWhites; // UI WW/CW swap toggles

Please verify this matches the intended behavior for the LSC Smart Connect strips.

🤖 Prompt for AI Agents
In wled00/bus_manager.cpp around lines 331 to 338, the WW/CW swap check shifts
the upper nibble then tests bit 0 which misses the UI swap value 4; change the
condition to extract the upper nibble (coSecond >> 4) and compare it to 4 (or
test ( (coSecond >> 4) & 0x07 ) == 4 if you want to be explicit about possible
values) so the WW/CW swap is honored; keep the rest of the swap logic and pixel
write unchanged.

Comment on lines +302 to +338

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.

⚠️ Potential issue | 🔴 Critical

Mirror this 2:1 mapping in readback and ABL repaint paths.

This new path writes one logical pixel to two hardware indices, but getPixelColor() below still reads pix directly and applyBriLimit() still uses the generic 1:1 repaint logic. After pixel 0, readback resolves to the wrong chip, and ABL dimming will only touch part of the strip / wrong channels when it kicks in. Please extract the logical→hardware mapping into a shared helper and reuse it from set/get/applyBriLimit.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/bus_manager.cpp` around lines 302 - 338, The new SM16703_DUAL path
maps one logical pixel to two hardware indices (firstIdx/secondIdx using
logicalPix = (_reversed ? _len - pix - 1 : pix) + _skip and hw index =
logicalPix*2/ +1), but getPixelColor() and applyBriLimit()/ABL repaint still
assume 1:1 mapping; extract that mapping into a shared helper (e.g.,
MapLogicalToHwIndices or BusManager::mapLogicalPixel) that takes pix and returns
{logicalPix, firstIdx, secondIdx, hwLen} (and consults _reversed, _skip, _start,
_len, and _colorOrderMap/_colorOrder as needed), then update getPixelColor(),
applyBriLimit(), and any ABL repaint code to call that helper and operate on
both hardware indices (and both chips' channels / _colorSum adjustments) instead
of using pix directly so readback and ABL touch the correct chips.

}

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);
Expand Down Expand Up @@ -370,6 +415,7 @@ void BusDigital::setColorOrder(uint8_t colorOrder) {
std::vector<LEDType> BusDigital::getLEDTypes() {
return {
{TYPE_WS2812_RGB, "D", PSTR("WS281x")},
{TYPE_SM16703_DUAL, "D", PSTR("SM16703 RGB+CCT (2x)")},
{TYPE_SK6812_RGBW, "D", PSTR("SK6812/WS2814 RGBW")},
{TYPE_TM1814, "D", PSTR("TM1814")},
{TYPE_WS2811_400KHZ, "D", PSTR("400kHz")},
Expand Down
5 changes: 3 additions & 2 deletions wled00/bus_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,16 @@ 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_SM16703_DUAL || // 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
}
static constexpr bool hasCCT(uint8_t type) {
return type == TYPE_WS2812_2CH_X3 || type == TYPE_WS2812_WWA ||
type == TYPE_ANALOG_2CH || type == TYPE_ANALOG_5CH ||
type == TYPE_FW1906 || type == TYPE_WS2805 ||
type == TYPE_SM16825;
type == TYPE_SM16825 || type == TYPE_SM16703_DUAL;
}
static constexpr bool isTypeValid(uint8_t type) { return (type > 15 && type < 128); }
static constexpr bool isDigital(uint8_t type) { return (type >= TYPE_DIGITAL_MIN && type <= TYPE_DIGITAL_MAX) || is2Pin(type); }
Expand Down
2 changes: 2 additions & 0 deletions wled00/bus_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,7 @@ class PolyBus {
case TYPE_WS2812_2CH_X3:
case TYPE_WS2812_RGB:
case TYPE_WS2812_WWA:
case TYPE_SM16703_DUAL:
return I_8266_U0_NEO_3 + offset;
case TYPE_SK6812_RGBW:
return I_8266_U0_NEO_4 + offset;
Expand Down Expand Up @@ -1378,6 +1379,7 @@ class PolyBus {
case TYPE_WS2812_2CH_X3:
case TYPE_WS2812_RGB:
case TYPE_WS2812_WWA:
case TYPE_SM16703_DUAL:
return I_32_RN_NEO_3 + offset;
case TYPE_SK6812_RGBW:
return I_32_RN_NEO_4 + offset;
Expand Down
1 change: 1 addition & 0 deletions wled00/const.h
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,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_SM16703_DUAL 35 //dual SM16703: RGB chip + WW/CW chip (B unused)
#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