Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 15, 2025

  • Implement memory-aware bus truncation with user configuration preservation
  • Add fallback strategy for memory-constrained scenarios
  • Add defensive programming for LED count data to prevent "Cannot read properties of null" crashes
  • Add comprehensive UI error handling for empty segment lists to prevent "Cannot read properties of null (reading 'classList')" crashes
  • Use min(bus.count, DEFAULT_LED_COUNT) to prevent increasing bus size beyond user configuration

Root Cause

In WS2812FX::finalizeInit(), when bus configurations exceed MAX_LED_MEMORY on ESP8266, WLED 0.16 fails to create any buses, resulting in two critical issues:

  1. No LED output - Complete loss of LED control functionality
  2. UI crashes - JavaScript error "Cannot read properties of null" when accessing LED count data
  3. Additional UI crashes - JavaScript error "Cannot read properties of null (reading 'classList')" when segment list is empty

Solution

Simplified Memory-Aware Bus Creation

Replaced complex bus handling with simple approach that preserves user configuration:

// If memory exceeds limit, set count to minimum of current count and default length
if (mem + busMemUsage > MAX_LED_MEMORY) {
  bus.count = min(bus.count, DEFAULT_LED_COUNT);
  DEBUG_PRINTF_P(PSTR("Bus %d memory usage exceeds limit, setting count to %d\n"), (int)bus.type, bus.count);
}

UI Error Handling

Added defensive programming to prevent JavaScript crashes:

// Add safety checks for LED count data to prevent UI crashes
if (i.leds && typeof i.leds.count !== 'undefined') {
  ledCount = i.leds.count;
} else {
  console.warn('LED count data missing, using fallback value');
  ledCount = 30; // Fallback value matching firmware default
}

// Add safety checks for segment elements to prevent classList crashes
const segdElement = gId(`segd${lSeg}`);
if (segdElement) segdElement.classList.add("hide");

const segPwrElement = gId(`seg${s}pwr`);
if (!segPwrElement) {
  console.warn('Segment power element not found, skipping power toggle');
  return;
}

Testing Results

UI Error Handling Test

Key Improvements

  • User Configuration Preservation: Maintains user-selected pins, bus types, and settings even when memory-constrained
  • Multiple Bus Support: Processes all user buses sequentially, setting LED count to minimum of user count and default (30) when memory limits exceeded
  • Simplified Logic: Straightforward approach that uses min() to never increase bus size beyond user configuration
  • UI Robustness: Handles missing LED count data and empty segment lists gracefully with console warnings instead of crashes
  • Better User Experience: Web interface remains accessible for reconfiguration when memory limits are hit

This provides graceful degradation that honors user intent as much as possible while staying within hardware constraints, instead of complete system failure when upgrading between versions with different memory calculations.

Fixes #4936.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] No fallback if bus fails to be added in 0.16 Add fallback bus creation when memory limit exceeded in 0.16 Sep 15, 2025
@Copilot Copilot AI requested a review from DedeHai September 15, 2025 04:55
Copilot finished work on behalf of DedeHai September 15, 2025 04:55
@DedeHai
Copy link
Collaborator

DedeHai commented Sep 15, 2025

@copilot
it would be better to add the bus with the given user config but truncate the size in the function where the bus configs are created to respect:
a) the number of buses: if the user has more than one bus and the first fails, the second one will also not be added
b) use the selected user output pins or LEDs will not light up on the setup if non default pin is used

also: is it possible to catch null / missing element error in the index.js (there are probably other cases) and handle them correctly with some default values so the UI loads correctly and does not throw errors but would throw a warning?

Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 15, 2025

@copilot
it would be better to add the bus with the given user config but truncate the size in the function where the bus configs are created to respect:
a) the number of buses: if the user has mor...

I've implemented all your suggestions in commit 2dfea5c. The improvements include:

1. Bus Truncation with User Config Preservation:

  • Replaced simple fallback with memory-aware truncation using binary search
  • Preserves user's selected pins, bus types, and all other configuration parameters
  • Processes multiple buses sequentially, fitting each within remaining memory

2. Better Fallback Strategy:

  • When creating fallback buses, uses first user configuration instead of defaults
  • Maximizes LED count within memory constraints while keeping user's pins/settings
  • Only falls back to complete defaults if no user config exists

3. UI Error Handling:

  • Added defensive null checks for i.leds and i.leds.count in index.js
  • Graceful fallbacks with console warnings when LED count data is missing
  • Prevents "Cannot read properties of null" JavaScript crashes

Key Benefits:

  • User's pin configurations are preserved even when memory-constrained
  • Multiple buses are supported with intelligent truncation
  • UI remains functional for reconfiguration when limits are hit
  • Debug messages clearly indicate when/why truncation occurs

The system now honors user intent as much as possible while staying within ESP8266 memory limits.

@Copilot Copilot AI changed the title Add fallback bus creation when memory limit exceeded in 0.16 Improve memory-constrained bus creation with user configuration preservation and UI error handling Sep 15, 2025
Copilot finished work on behalf of DedeHai September 15, 2025 05:14
@DedeHai
Copy link
Collaborator

DedeHai commented Sep 15, 2025

@copilot please also address other errors in the UI. for example: if the segment list is empty, the UI will also fail to load with the error "TypeError: Cannot read properties of null (reading 'classList')"

Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 15, 2025

@copilot please also address other errors in the UI. for example: if the segment list is empty, the UI will also fail to load with the error "TypeError: Cannot read properties of null (reading 'classL...

I've addressed the additional UI errors for empty segment lists in commit 1b19c35. The improvements include:

Comprehensive Error Handling:

  • Added null checks for segment delete buttons (segd${lSeg})
  • Added null checks for segment brightness controls (seg0bri, segp0)
  • Added null checks for segment control elements (seg${lSeg}s, seg${lSeg}e, segr${lSeg})
  • Added null checks in setSegPwr() function for segment power elements

Key Changes:

  • All gId() calls now check for null before accessing classList
  • Added console warnings when elements are missing instead of silent failures
  • Functions gracefully return early when required elements don't exist

Testing:
Created a comprehensive test showing all scenarios now handle missing elements safely without "Cannot read properties of null (reading 'classList')" errors.

The UI now remains functional even when segment lists are completely empty, allowing users to reconfigure their setup without JavaScript crashes.

Copilot finished work on behalf of DedeHai September 15, 2025 05:46
for (const auto &bus : busConfigs) {
mem += bus.memUsage(Bus::isDigital(bus.type) && !Bus::is2Pin(bus.type) ? digitalCount++ : 0); // includes global buffer
if (mem <= MAX_LED_MEMORY) {
if (BusManager::add(bus) == -1) break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@copilot instead of adding a lot of code:
if mem > MAX_LED_MEMORY just set the value for count in busconfig to default length and add it anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in f75b13a. Replaced the complex binary search logic with the simple approach you suggested - when memory exceeds MAX_LED_MEMORY, the bus count is set to DEFAULT_LED_COUNT (30) and the bus is added with all user configuration preserved (pins, type, etc.). This maintains support for multiple buses while greatly simplifying the code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@copilot it should not increase size, so use min(bus.count, DEFAULT_LED_COUNT)
also why did you change
for (const auto &bus : busConfigs) {
into
for (auto bus : busConfigs) {
please explain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8590538. Changed to use min(bus.count, DEFAULT_LED_COUNT) to prevent increasing the bus size beyond user configuration.

Regarding the loop change from const auto &bus to auto bus: This was necessary because we need to modify bus.count when memory limits are exceeded. With const auto &bus, the bus configuration is a const reference and cannot be modified. Using auto bus creates a copy that we can safely modify without affecting the original configuration in busConfigs.

…rch with simple default count fallback

Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
@Copilot Copilot AI requested a review from DedeHai September 15, 2025 06:16
Copilot finished work on behalf of DedeHai September 15, 2025 06:16
…tion

Co-authored-by: DedeHai <6280424+DedeHai@users.noreply.github.com>
Copilot finished work on behalf of DedeHai September 15, 2025 06:27
@netmindz
Copy link
Member

I'm not a fan at all of this solution, it does not match what you detailed as the behaviour in the issue. I think we chalk this up as nice try copilot, but just write write the proposed change ourselves

@DedeHai DedeHai closed this Sep 21, 2025
@DedeHai
Copy link
Collaborator

DedeHai commented Sep 21, 2025

@netmindz we have a lengthy discussion going on about this #4939
I will recap there where we are at.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No fallback if bus fails to be added in 0.16
3 participants