Fix for cfg exceeding LED limit (#4939)

* Safety Checks for UI, fix for cfg exceeding LED limit
* improvements to low heap check
* add `isPlaceholder()` to bus, some fixes
* remove `disableForceReconnect` for better future implementation
* add "glitch gating" for C3 and check heapy every 5 seconds instead of every secondd
* replace magic number with the correct define, more robust bus defer by look-ahead

In the event that a Bus fails to initialize, or the memory validation
fails, keep the configuration around so the settings contents don't
change out from under the user.

---------

Co-authored-by: Will Miles <will@willmiles.net>

Author: DedeHai

wled00/FX_fcn.cpp

@@ -1187,9 +1187,9 @@ void WS2812FX::finalizeInit() {
   // create buses/outputs
   unsigned mem = 0;
   unsigned maxI2S = 0;
-  for (const auto &bus : busConfigs) {
-    unsigned memB = bus.memUsage(Bus::isDigital(bus.type) && !Bus::is2Pin(bus.type) ? digitalCount++ : 0); // does not include DMA/RMT buffer
-    mem += memB;
+  for (auto bus : busConfigs) {
+    bool use_placeholder = false;
+    unsigned busMemUsage = bus.memUsage(Bus::isDigital(bus.type) && !Bus::is2Pin(bus.type) ? digitalCount++ : 0); // does not include DMA/RMT buffer
     // estimate maximum I2S memory usage (only relevant for digital non-2pin busses)
     #if !defined(CONFIG_IDF_TARGET_ESP32C3) && !defined(ESP8266)
       #if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S3)
@@ -1209,13 +1209,14 @@ void WS2812FX::finalizeInit() {
       if (i2sCommonSize > maxI2S) maxI2S = i2sCommonSize;
     }
     #endif
-    if (mem + maxI2S <= MAX_LED_MEMORY) {
-      BusManager::add(bus);
-      DEBUG_PRINTF_P(PSTR("Bus memory: %uB\n"), memB);
-    } else {
-      errorFlag = ERR_NORAM_PX; // alert UI
-      DEBUG_PRINTF_P(PSTR("Out of LED memory! Bus %d (%d) #%u not created."), (int)bus.type, (int)bus.count, digitalCount);
-      break;
+    if (mem + busMemUsage + maxI2S > MAX_LED_MEMORY) {
+      DEBUG_PRINTF_P(PSTR("Bus %d with %d LEDS memory usage exceeds limit\n"), (int)bus.type, bus.count);
+      errorFlag = ERR_NORAM; // alert UI  TODO: make this a distinct error: not enough memory for bus
+      use_placeholder = true;
+    }
+    if (BusManager::add(bus, use_placeholder) != -1) {
+      mem += BusManager::busses.back()->getBusSize();
+      if (Bus::isDigital(bus.type) && !Bus::is2Pin(bus.type) && BusManager::busses.back()->isPlaceholder()) digitalCount--; // remove placeholder from digital count
     }
   }
   DEBUG_PRINTF_P(PSTR("LED buffer size: %uB/%uB\n"), mem + maxI2S, BusManager::memUsage());
@@ -1824,6 +1825,10 @@ void WS2812FX::resetSegments() {
   if (isServicing()) return;
   _segments.clear();          // destructs all Segment as part of clearing
   _segments.emplace_back(0, isMatrix ? Segment::maxWidth : _length, 0, isMatrix ? Segment::maxHeight : 1);
+  if(_segments.size() == 0) {
+    _segments.emplace_back(); // if out of heap, create a default segment
+    errorFlag = ERR_NORAM_PX;
+  }
   _segments.shrink_to_fit();  // just in case ...
   _mainSegment = 0;
 }
@@ -1846,7 +1851,7 @@ void WS2812FX::makeAutoSegments(bool forceReset) {
 
     for (size_t i = s; i < BusManager::getNumBusses(); i++) {
       const Bus *bus = BusManager::getBus(i);
-      if (!bus || !bus->isOk()) break;
+      if (!bus) break;
 
       segStarts[s] = bus->getStart();
       segStops[s]  = segStarts[s] + bus->getLength();

wled00/bus_manager.cpp

@@ -1105,6 +1105,26 @@ size_t BusHub75Matrix::getPins(uint8_t* pinArray) const {
 #endif
 // ***************************************************************************
 
+BusPlaceholder::BusPlaceholder(const BusConfig &bc) 
+: Bus(bc.type, bc.start, bc.autoWhite, bc.count, bc.reversed, bc.refreshReq)
+, _colorOrder(bc.colorOrder)
+, _skipAmount(bc.skipAmount)
+, _frequency(bc.frequency)
+, _milliAmpsPerLed(bc.milliAmpsPerLed)
+, _milliAmpsMax(bc.milliAmpsMax)
+, _text(bc.text)
+{
+  memcpy(_pins, bc.pins, sizeof(_pins));
+}
+
+size_t BusPlaceholder::getPins(uint8_t* pinArray) const {
+  size_t nPins = Bus::getNumberOfPins(_type);
+  if (pinArray) {
+    for (size_t i = 0; i < nPins; i++) pinArray[i] = _pins[i];
+  }
+  return nPins;
+}
+
 //utility to get the approx. memory usage of a given BusConfig
 size_t BusConfig::memUsage(unsigned nr) const {
   if (Bus::isVirtual(type)) {
@@ -1148,7 +1168,7 @@ size_t BusManager::memUsage() {
   return size + maxI2S;
 }
 
-int BusManager::add(const BusConfig &bc) {
+int BusManager::add(const BusConfig &bc, bool placeholder) {
   DEBUGBUS_PRINTF_P(PSTR("Bus: Adding bus (p:%d v:%d)\n"), getNumBusses(), getNumVirtualBusses());
   unsigned digital = 0;
   unsigned analog  = 0;
@@ -1158,8 +1178,12 @@ int BusManager::add(const BusConfig &bc) {
     if (bus->isDigital() && !bus->is2Pin()) digital++;
     if (bus->is2Pin()) twoPin++;
   }
-  if (digital > WLED_MAX_DIGITAL_CHANNELS || analog > WLED_MAX_ANALOG_CHANNELS) return -1;
-  if (Bus::isVirtual(bc.type)) {
+  digital += (Bus::isDigital(bc.type) && !Bus::is2Pin(bc.type));
+  analog  += (Bus::isPWM(bc.type) ? Bus::numPWMPins(bc.type) : 0);
+  if (digital > WLED_MAX_DIGITAL_CHANNELS || analog > WLED_MAX_ANALOG_CHANNELS) placeholder = true; // TODO: add errorFlag here
+  if (placeholder) {
+    busses.push_back(make_unique<BusPlaceholder>(bc));
+  } else if (Bus::isVirtual(bc.type)) {
     busses.push_back(make_unique<BusNetwork>(bc));
 #ifdef WLED_ENABLE_HUB75MATRIX
   } else if (Bus::isHub75(bc.type)) {
@@ -1266,7 +1290,7 @@ void BusManager::on() {
   if (PinManager::getPinOwner(LED_BUILTIN) == PinOwner::BusDigital) {
     for (auto &bus : busses) {
       uint8_t pins[2] = {255,255};
-      if (bus->isDigital() && bus->getPins(pins)) {
+      if (bus->isDigital() && bus->getPins(pins) && bus->isOk()) {
         if (pins[0] == LED_BUILTIN || pins[1] == LED_BUILTIN) {
           BusDigital &b = static_cast<BusDigital&>(*bus);
           b.begin();
@@ -1361,7 +1385,7 @@ void BusManager::initializeABL() {
       _useABL = true; // at least one bus has ABL set
       uint32_t ESPshare = MA_FOR_ESP / numABLbuses; // share of ESP current per ABL bus
       for (auto &bus : busses) {
-        if (bus->isDigital()) {
+        if (bus->isDigital() && bus->isOk()) {
           BusDigital &busd = static_cast<BusDigital&>(*bus);
           uint32_t busLength = busd.getLength();
           uint32_t busDemand = busLength * busd.getLEDCurrent();

wled00/bus_manager.h

@@ -133,7 +133,7 @@ class Bus {
     virtual void     setColorOrder(uint8_t co)                  {}
     virtual uint32_t getPixelColor(unsigned pix) const          { return 0; }
     virtual size_t   getPins(uint8_t* pinArray = nullptr) const { return 0; }
-    virtual uint16_t getLength() const                          { return isOk() ? _len : 0; }
+    virtual uint16_t getLength() const                          { return _len; }
     virtual uint8_t  getColorOrder() const                      { return COL_ORDER_RGB; }
     virtual unsigned skippedLeds() const                        { return 0; }
     virtual uint16_t getFrequency() const                       { return 0U; }
@@ -152,6 +152,7 @@ class Bus {
     inline  bool     isPWM() const                              { return isPWM(_type); }
     inline  bool     isVirtual() const                          { return isVirtual(_type); }
     inline  bool     is16bit() const                            { return is16bit(_type); }
+    virtual bool     isPlaceholder() const                      { return false; }
     inline  bool     mustRefresh() const                        { return mustRefresh(_type); }
     inline  void     setReversed(bool reversed)                 { _reversed = reversed; }
     inline  void     setStart(uint16_t start)                   { _start = start; }
@@ -372,6 +373,39 @@ class BusNetwork : public Bus {
     #endif
 };
 
+// Placeholder for buses that we can't construct due to resource limitations
+// This preserves the configuration so it can be read back to the settings pages
+// Function calls "mimic" the replaced bus, isPlaceholder() can be used to identify a placeholder
+class BusPlaceholder : public Bus {
+  public:
+    BusPlaceholder(const BusConfig &bc);
+
+    // Actual calls are stubbed out
+    void setPixelColor(unsigned pix, uint32_t c) override {};
+    void show() override {};
+
+    // Accessors
+    uint8_t  getColorOrder() const override  { return _colorOrder; }
+    size_t   getPins(uint8_t* pinArray) const override;
+    unsigned skippedLeds() const override    { return _skipAmount; }
+    uint16_t getFrequency() const override   { return _frequency; }
+    uint16_t getLEDCurrent() const override  { return _milliAmpsPerLed; }
+    uint16_t getMaxCurrent() const override  { return _milliAmpsMax; }
+    const String getCustomText() const override { return _text; }
+    bool     isPlaceholder() const override  { return true; }
+
+    size_t   getBusSize() const override   { return sizeof(BusPlaceholder); }
+
+  private:
+    uint8_t _colorOrder;
+    uint8_t _skipAmount;
+    uint8_t _pins[OUTPUT_MAX_PINS];
+    uint16_t _frequency;
+    uint8_t _milliAmpsPerLed;
+    uint16_t _milliAmpsMax;
+    String _text;
+};
+
 #ifdef WLED_ENABLE_HUB75MATRIX
 class BusHub75Matrix : public Bus {
   public:
@@ -507,7 +541,7 @@ namespace BusManager {
 
   //do not call this method from system context (network callback)
   void removeAll();
-  int  add(const BusConfig &bc);
+  int  add(const BusConfig &bc, bool placeholder);
 
   void on();
   void off();

wled00/cfg.cpp

@@ -958,7 +958,7 @@ void serializeConfig(JsonObject root) {
   for (size_t s = 0; s < BusManager::getNumBusses(); s++) {
     DEBUG_PRINTF_P(PSTR("Cfg: Saving bus #%u\n"), s);
     const Bus *bus = BusManager::getBus(s);
-    if (!bus || !bus->isOk()) break;
+    if (!bus) break;  // Memory corruption, iterator invalid
     DEBUG_PRINTF_P(PSTR("  (%d-%d, type:%d, CO:%d, rev:%d, skip:%d, AW:%d kHz:%d, mA:%d/%d)\n"),
       (int)bus->getStart(), (int)(bus->getStart()+bus->getLength()),
       (int)(bus->getType() & 0x7F),

wled00/data/index.js

@@ -3455,4 +3455,4 @@ _C.addEventListener('touchstart', lock, false);
 
 _C.addEventListener('mouseout', move, false);
 _C.addEventListener('mouseup', move, false);
-_C.addEventListener('touchend', move, false);
+_C.addEventListener('touchend', move, false);

wled00/wled.cpp

@@ -40,8 +40,8 @@ void WLED::reset()
 
 void WLED::loop()
 {
-  static uint32_t      lastHeap = UINT32_MAX;
-  static unsigned long heapTime = 0;
+  static uint16_t      heapTime = 0;   // timestamp for heap check
+  static uint8_t       heapDanger = 0; // counter for consecutive low-heap readings
 #ifdef WLED_DEBUG
   static unsigned long lastRun = 0;
   unsigned long        loopMillis = millis();
@@ -169,19 +169,47 @@ void WLED::loop()
     correctPIN = false;
   }
 
-  // reconnect WiFi to clear stale allocations if heap gets too low
-  if (millis() - heapTime > 15000) {
-    uint32_t heap = getFreeHeapSize();
-    if (heap < MIN_HEAP_SIZE && lastHeap < MIN_HEAP_SIZE) {
-      DEBUG_PRINTF_P(PSTR("Heap too low! %u\n"), heap);      
-      strip.resetSegments(); // remove all but one segments from memory
-      if (!Update.isRunning()) forceReconnect = true;
-    } else if (heap < MIN_HEAP_SIZE) {
-      DEBUG_PRINTLN(F("Heap low, purging segments."));
-      strip.purgeSegments();
+   // free memory and reconnect WiFi to clear stale allocations if heap is too low for too long, check once every 5s
+  if ((uint16_t)(millis() - heapTime) > 5000) {
+    #ifdef ESP8266
+    uint32_t heap = getFreeHeapSize(); // ESP8266 needs ~8k of free heap for UI to work properly
+    #else
+    #ifdef CONFIG_IDF_TARGET_ESP32C3
+    // calling getContiguousFreeHeap() during led update causes glitches on C3
+    // this can (probably) be removed once RMT driver for C3 is fixed
+    unsigned t0 = millis();
+    while (strip.isUpdating() && (millis() - t0 < 15)) delay(1);    // be nice, but not too nice. Waits up to 15ms
+    #endif
+    uint32_t heap = getContiguousFreeHeap(); // ESP32 family needs ~10k of contiguous free heap for UI to work properly
+    #endif
+    if (heap < MIN_HEAP_SIZE - 1024) heapDanger+=5; // allow 1k of "wiggle room" for things that do not respect min heap limits
+    else heapDanger = 0;
+    switch (heapDanger) {
+      case 15: // 15 consecutive seconds
+        DEBUG_PRINTLN(F("Heap low, purging segments"));
+        strip.purgeSegments();
+        strip.setTransition(0); // disable transitions
+        for (unsigned i = 0; i < strip.getSegmentsNum(); i++) {
+          strip.getSegments()[i].setMode(FX_MODE_STATIC); // set static mode to free effect memory
+        }
+        errorFlag = ERR_NORAM; // alert UI  TODO: make this a distinct error: segment reset
+        break;
+      case 30: // 30 consecutive seconds
+        DEBUG_PRINTLN(F("Heap low, reset segments"));
+        strip.resetSegments(); // remove all but one segments from memory
+        errorFlag = ERR_NORAM; // alert UI  TODO: make this a distinct error: segment reset
+        break;
+      case 45: // 45 consecutive seconds
+        DEBUG_PRINTF_P(PSTR("Heap panic! Reset strip, reset connection\n"));
+        strip.~WS2812FX();      // deallocate strip and all its memory
+        new(&strip) WS2812FX(); // re-create strip object, respecting current memory limits
+        if (!Update.isRunning()) forceReconnect = true; // in case wifi is broken, make sure UI comes back, set disableForceReconnect = true to avert
+        errorFlag = ERR_NORAM; // alert UI  TODO: make this a distinct error: strip reset
+        break;
+      default:
+        break;
     }
-    lastHeap = heap;
-    heapTime = millis();
+    heapTime = (uint16_t)millis();
   }
 
   //LED settings have been saved, re-init busses

wled00/xml.cpp

@@ -315,7 +315,7 @@ void getSettingsJS(byte subPage, Print& settingsScript)
     unsigned sumMa = 0;
     for (size_t s = 0; s < BusManager::getNumBusses(); s++) {
       const Bus *bus = BusManager::getBus(s);
-      if (!bus || !bus->isOk()) break; // should not happen but for safety
+      if (!bus) break; // should not happen but for safety
       int offset = s < 10 ? '0' : 'A' - 10;
       char lp[4] = "L0"; lp[2] = offset+s; lp[3] = 0; //ascii 0-9 //strip data pin
       char lc[4] = "LC"; lc[2] = offset+s; lc[3] = 0; //strip length