Problems using arrays for FastLED colours

I've been working on a Halloween project, which uses a 500 LED strand for lighting, plus an MP3 player and motion sensor to activate a jump scare. 10 pulses of different colours travel down the strand It's 99% working but I've run into an issue I can't figure out.

If I use an array to show different colour pulses then the MP3 player ignores requests to play a different MP3. It's taken some trial and error to figure out exactly what's happening.

Here's the full code
.

I set the contents of the array using

uint32_t colour[] = {CRGB::Red, CRGB::OrangeRed, CRGB::Crimson, CRGB::MediumVioletRed, CRGB::Orange, CRGB::Blue};

I pass thiscolour as an integer and set the LEDs colour using

leds[thispos]=colour[thiscolour];

I tried some things.

  • It works fine if I ignore the array and just set the colour using =CRGB::Red
  • It works if I use the same colour array position for all pulses e.g. colour[1]
  • It fails if I use different colour array positions for all pulses. colour[1], colour[2] etc
  • It fails if I use different colour array positions for all pulses, even when the array colours are all the same.
  • It fails if I ignore the array and use a case statement to set the colours.
  • The problem occurs whether I use 50 or 500 LEDs.
  • I don't appear to be running out of RAM.

An earlier version of my code doesn't exhibit this problem, so I've gone through the diffs but haven't found anything. I think I'm about to just revert to that old code and then try and build my way back up, but I'm tired and likely to make a mistake right now.

If anyone has any idea why setting different colours using FastLED and an array of colours would stop an MP3 player from working properly then I would be very appreciative.

This sounds like undefined behavior, most likely an array out of bounds access.

Check all your indices. If you can't find anything by just reading the code, add checks for all array accesses.

The maximum valid index for an array of N elements is N-1.

Pieter

For starters....
In your reverseMap() function, you return NUM_LEDS if the value is less than 0 and then you use this value which is outside the array...

You also call your redpulse() function and then fade the leds and show them and then do it again which doesn't really do anything. It is not your problem, but is extraneous.

Thank you both.

blh64:
For starters....
In your reverseMap() function, you return NUM_LEDS if the value is less than 0 and then you use this value which is outside the array...

OMG. I think that's it. In the earlier, working version I returned NUM_LEDS - 1. I bet that's it. THANK YOU.

blh64:
You also call your redpulse() function and then fade the leds and show them and then do it again which doesn't really do anything. It is not your problem, but is extraneous.

I don't quite follow this. I only show them once as far as I can tell. Do you mean that I fade them more than once?

the best failsafe for writing beyond the bounds of the array is

 thispos = reverseMap(thispos);
    thispos=thispos % NUM_LEDS; 
    leds[thispos]=colour[thiscolour]; // set the led colour

And there is no need to switch variable types hereCRGB colour[] = {CRGB::Red, CRGB::OrangeRed, CRGB::Crimson, CRGB::MediumVioletRed, CRGB::Orange, CRGB::Blue};
And this

// insert a delay to keep the framerate modest
  FastLED.delay(1000 / FRAMES_PER_SECOND);

, Well if you look into the FastLED library you'll find that their .delay() function just calls .show() until the elapsed time exceeds the argument, which i find is just the dumbest thing to do with processor time, leaving interrupts turned off for that time sending the same info to a chip over and over again ! If you want to use delay() you should just use delay(), not FastLED.delay() !

DavidA1024:
I don't quite follow this. I only show them once as far as I can tell. Do you mean that I fade them more than once?

You call the fade function immediately after filling the array and then display it. You then finish loop() and once again fill the array and then fade it and show it. Typical use is to fill the array and then repeatedly fade the array so it slowly fades to black.

What you are doing is equivalent to

void loop() {
  int variable = 10;
  variable--;
}

Deva_Rishi:
the best failsafe for writing beyond the bounds of the array is

thispos = reverseMap(thispos);

thispos=thispos % NUM_LEDS;
   leds[thispos]=colour[thiscolour]; // set the led colour

Ah. Nice approach. I guess it causes it to wrap around the LED strand a little.

Deva_Rishi:
And there is no need to switch variable types here

CRGB colour[] = {CRGB::Red, CRGB::OrangeRed, CRGB::Crimson, CRGB::MediumVioletRed, CRGB::Orange, CRGB::Blue};

I'm not sure I know what you mean. I've read that I should use uint32_t.

Deva_Rishi:
their .delay() function just calls .show() until the elapsed time exceeds the argument

Oh. I didn't know that.

blh64:
You call the fade function immediately after filling the array and then display it. You then finish loop() and once again fill the array and then fade it and show it.

Ah yes. That's true. The first fade is to draw a pulse with faded ends, the second fade is because it became difficult to remember which LEDs I'd lit using the pulse in combination with a sin function. I actually liked the effect it caused so I didn't put in any more work to doing it properly.

Here's what it looks like in the flesh.

I'm not sure I know what you mean. I've read that I should use uint32_t.

Well the colors are defined as a CRGB struct, which is a 3-byte struct, you can of course fit that into a 4 byte variable, but you may as well declare the colour[] array to be of the CRGB type, which is the same as how the leds[] array is declared.

Ah. Nice approach. I guess it causes it to wrap around the LED strand a little.

In case of thispos>=NUM_LEDS yes it does, but anything is better than writing beyond the bounds of the array, which happens a lot when novices use the FastLED library. Just a function the writes to the leds[] array but first checks for the bounds is another good option.

So a quick follow up on this thread.

Yes, the bug was caused by an out of bounds array. Thanks particularly to blh64 who could look through my code, find the bug and tell me the fix within 90 minutes of my post.

And thanks to everyone else here who taught me some things and gave me some ideas. What a great community.

Anyway, this is sounding like an Oscars acceptance speech.

Thanks again!