Skip to content

Conversation

@DaanCMP
Copy link
Contributor

@DaanCMP DaanCMP commented Oct 19, 2025

Problem

The backlight loop effect (case 255) was causing ESP8266 crashes after completing one full cycle. The issue occurred because:

  • loopIndex variable incremented indefinitely without bounds checking
  • When loopIndex exceeded BACKLIGHT_NUM_LEDS, it caused array bounds violation
  • This resulted in accessing invalid memory, causing the ESP8266 to crash
  • The first LED would stay lit at full brightness before the crash

Solution

Added bounds checking and proper loop reset:

  • Added bounds check: if (loopIndex >= BACKLIGHT_NUM_LEDS)
  • Reset loopIndex to 0 when it reaches the end of the LED array
  • Separated array access and increment for clarity and safety

Changes

  • Backlight.cpp: Added bounds checking in the loop effect (case 255)
  • Prevents array bounds violation when loop completes one cycle
  • Ensures continuous looping without crashes

Testing

  • Loop effect now safely cycles through all backlight LEDs continuously
  • No more crashes or memory violations
  • Loop resets properly and continues indefinitely

- Add bounds checking for loopIndex to prevent array bounds violation
- Reset loopIndex to 0 when it reaches BACKLIGHT_NUM_LEDS
- Prevents ESP8266 crash when loop effect completes one cycle
- Fixes issue where first LED would stay lit and device would crash

The loop effect now safely cycles through all backlight LEDs continuously
without memory violations.
Copy link
Owner

@Winston-Lu Winston-Lu left a comment

Choose a reason for hiding this comment

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

use a modulus instead to be a bit more compact and not need to branch (would likely be optimized out anyways)

@Winston-Lu
Copy link
Owner

appreciate the work though in fixing something i never tested (i got lazy)

Using winston's suggestion

Co-authored-by: Winston Lu <33874247+Winston-Lu@users.noreply.github.com>
@Winston-Lu
Copy link
Owner

could you also confirm that the changes work? Gave my own shelf away since i was moving to a different country so I dont have a means of testing it myself anymore

@DaanCMP
Copy link
Contributor Author

DaanCMP commented Oct 19, 2025

Haha i got the feeling that this was untested already. I might refracture/update it tonight because there are some more things broken. (like the brightness control not working at all).

I'd like to add some functions like a countdown clock or something but i'm right at the edge of IRAM. I'm thinking of moving the grid to progmem, might be slower but i'm wondering what the actual difference will be.
I could also try to calculate the grid on-demand, but idk if thats feasable

@DaanCMP
Copy link
Contributor Author

DaanCMP commented Oct 19, 2025

could you also confirm that the changes work? Gave my own shelf away since i was moving to a different country so I dont have a means of testing it myself anymore

Will let you know later tonight

@Winston-Lu
Copy link
Owner

feel free to try whatever, this project was done in my early days of C++ before i had any of my code reviewed before, but likely a lot of memory is being taken up by the arrays of the RGB pixels. We should still have enough budget to calculate things on runtime if needed, the worst one for runtime is the gradient since I calculate each pixel every time instead of caching or saving results, but the rest should run at 30+fps

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.

2 participants