FastLED interfering with variables

So I know the first thing I will be asked is "post your code" but in this situation that will not be possible due to this project being made with the intention of selling it. (also the rest of the code is irrelevant and incredibly long) However, I can post some parts of the code for you to analyze.

Info: Arduino Mega 2560, sketch uses 10% program storage space, 25% dynamic variable space.

Essentially, this project is using two 10-length addressable RGB strips for aesthetics, and the RGB strips use a single wire chipset, which can cause problems with interrupts being disabled according to the FastLED wiki. However, it is not serial or i2c or spi or servo or any other communication that is being broken by FastLED, but a variable. There is a password that must be set with a keypad, which is then displayed on an i2c LCD. Later in the program, the password is required again, but the original password is displayed so that it can be entered. However, when a specific block of FastLED code is added to the code, instead of the original password being displayed, all four characters of the password are replaced by a custom LCD character that is used elsewhere in the code. Depending on where the block of fastLED code is placed, only 2 or 3 of the password's characters are corrupted.

For reference, the password is stored in a four-length char array, and the custom character is stored in an 8 length byte array. These are completely unrelated and strangely, when the custom character is commented out, the password is corrupted by fully filled character spaces on the LCD.

When this specific block of FastLED code is removed, everything works fine. When it is in the code, depending on where in the code it is, some or all of the password characters corrupt.

Also, when the password corrupts, the code does not recognize the correct password when it is inputted.

(yes I realize it gives the user the password and then asks them to input it. It's supposed to be easy.)

Back to the interrupt disabling done by the fastLED library, I have tried using the #define FASTLED_ALLOW_INTERRUPTS to both 0 and 1, neither of which fixed the problem. I also tried setting the FASTLED_INTERRUPT_RETRY_COUNT to 1.

Here is the offending block of code:

currentTime = millis();
  if (currentTime >= lastTimePing + 3000){
    lastTimePing = millis();
    leds[leadLed] = CRGB (20,0,0);
      leds[leadLed-3] = CRGB (0,0,0);
   
             fill_solid(&(leds[20]), 2, CRGB (0,0,0));
       FastLED.show();
       leadLed++;
       if (leadLed == 23){
        leadLed = 0;
       
       }
  }

"leds" and "leadLed" are variables custom made to control the addressable LED strip with the FastLED library.

millis() and currentTime and lastTimePing are used to keep track of the time when this function is called in void loop().

If anyone has any ideas, I will be very grateful. I have spent hours pulling my hair out over this code, and I have eliminated a few other unrelated bugs in the code, none of which fixed the problem. There may be another bug in the code, but I do not think there is.

It seems quite obvious that this...

leds[leadLed-3] = CRGB (0,0,0);

..will access outside of the array when...

leadLed = 0;

Edit: Try...

if (leadLed >= 3) {
  leds[leadLed-3] = CRGB (0,0,0);
}

(deleted)

Is this part of the forum not about open source help and support rather than commercial activity. Someone in gigs and collaborations could be contracted to help and not reveal your code.

themodernwizard:
So I know the first thing I will be asked is "post your code" but in this situation that will not be possible due to this project being made with the intention of selling it. (also the rest of the code is irrelevant and incredibly long) However, I can post some parts of the code for you to analyze.

I hope you realize you will be legally obligated to distribute a copy of the MIT license that FastLED uses, with every device that you sell.

I suggest, it is not common that project which doesn't really contain significant innovation, can make money. Apart from that, it is not feasible in such cases to prevent reverse engineering which makes proprietary efforts futile. I have discovered that proprietary actions usually limit sales rather than guaranteeing them. You should consider instead capitalizing on your idea by being first on the market, and then moving on because most such ideas have a very short shelf life.

leds[leadLed-3] = CRGB (0,0,0);

Instead of subtracting 3 from 'leadLed' you could add 20 and use modulus to prevent overflow when 'leadLed > 2'.

leds[(leadLed + 20) % 23] = CRGB (0, 0, 0);

aarg:
I hope you realize you will be legally obligated to distribute a copy of the MIT license that FastLED uses, with every device that you sell.

I suggest, it is not common that project which doesn't really contain significant innovation, can make money. Apart from that, it is not feasible in such cases to prevent reverse engineering which makes proprietary efforts futile. I have discovered that proprietary actions usually limit sales rather than guaranteeing them. You should consider instead capitalizing on your idea by being first on the market, and then moving on because most such ideas have a very short shelf life.

I actually was not aware of that obligation, but upon further research, you are correct. Are there any examples of Fastled in commercial use out there? - #4 by Jason_Coon - FastLED Archive - Maker Forums

I will note that in documentation when this product is sold.

As for "significant innovation", it is not the code itself that is being sold, but a product made with an arduino running the code. This is not a representative of a large company speaking, but a maker working with a friend to build and sell a few of these things to make some money on the side and have fun doing it.

Although what we are doing has been done in some before by other makers selling one or two of these products, there is not to my knowledge any patent/copyright/etcetera on the design. To be clear, we are designing this entirely on our own and using those other projects as inspirations.

Reverse engineering - we are reverse-engineering the code based on inspiration from these other projects. Others can be inspired by our project and reverse engineer our code, but we will hold on to our original code so that they can take pride in their own work.

Anyways, thank you for enlightening me as to the nature of FastLED's licensing!

pmagowan:
Is this part of the forum not about open source help and support rather than commercial activity. Someone in gigs and collaborations could be contracted to help and not reveal your code.

I figured I could get some helpful answers simply based on the part of the code that I posted, which I did. As I mentioned in another reply, I am not a business, just a maker working on the side with a friend to make cool stuff and sell it. An open-source forum such as this is free, anything else would be way out of our budget.

pcbbc:
It seems quite obvious that this...

leds[leadLed-3] = CRGB (0,0,0);

..will access outside of the array when...

leadLed = 0;

Edit: Try...

if (leadLed >= 3) {

leds[leadLed-3] = CRGB (0,0,0);
}

Thank you! I believe you may have found the issue.

themodernwizard:
Thank you! I believe you may have found the issue.

themodernwizard:
Thank you! I believe you may have found the issue.

Great.

Although note that if you had been/are writing outside the bounds of the array in the other direction I would have no way of knowing as you haven’t thought to supply the code that created the LEDs array.

In that regard you were lucky, and such is the peril of providing only snippets.

But as a general rule of C/C++ programming, if you experience “undefined behaviour” (variables resetting, not holding their value) first thing to do is look for buffer/array under/overruns.

pcbbc:
Great.

Although note that if you had been/are writing outside the bounds of the array in the other direction I would have no way of knowing as you haven’t thought to supply the code that created the LEDs array.

In that regard you were lucky, and such is the peril of providing only snippets.

But as a general rule of C/C++ programming, if you experience “undefined behaviour” (variables resetting, not holding their value) first thing to do is look for buffer/array under/overruns.

Oh I didn’t think to give you that code here you go:

#include <FastLED.h>
#define LED_PIN  3 
#define NUM_LEDS   22

CRGB leds[NUM_LEDS];

int leadLed = 0;

FastLED.addLeds<WS2812, LED_PIN, GRB>(leds, NUM_LEDS);
  fill_solid(&(leds[0]), 20, CRGB (0,0,0));
          FastLED.show();

And lo and behold your code IS broken in the other direction as well...

This code...

 #define NUM_LEDS   22
CRGB leds[NUM_LEDS];

...creates an array with 22 elements. The elements are numbered 0..21. There is NOT an element 22.

This code...

leadLed++;
if (leadLed == 23){
  leadLed = 0;
}

...constrains leadLed to be in the range 0..22 inclusive. Therefore the index can reach 22, which is out of bounds of the array and so illegal.

You’d do well to avoid “magic numbers”...

// although why you are not filling the last 2 elements I do not understand...
fill_solid(&(leds[0]), NUM_LEDS - 2, CRGB (0,0,0));

if (leadLed ==NUM_LEDS){

NB: A magic number is any constant in your code which occurs in more than one place, or is dependent on some other constant. These should be replaced with constexpr, const or #define.

pcbbc:
And lo and behold your code IS broken in the other direction as well...

This code...

 #define NUM_LEDS   22

CRGB leds[NUM_LEDS];



...creates an array with 22 elements. The elements are numbered 0..21. There is **NOT** an element 22.

This code...


leadLed++;
if (leadLed == 23){
  leadLed = 0;
}



...constrains leadLed to be in the range 0..22 **inclusive**. Therefore the index **can** reach 22, which is out of bounds of the array and so illegal.

You’d do well to avoid “magic numbers”...


// although why you are not filling the last 2 elements I do not understand...
fill_solid(&(leds[0]), NUM_LEDS - 2, CRGB (0,0,0));

if (leadLed ==NUM_LEDS){



NB: A magic number is any constant in your code which occurs in more than one place, or is dependent on some other constant. These should be replaced with constexpr, const or #define.

May I ask exactly what each of the parameters in fill_solid do? I am not able to find anywhere what each parameter does, except the last parameter (color).

Also which of my constants is a magic number?

themodernwizard:
May I ask exactly what each of the parameters in fill_solid do? I am not able to find anywhere what each parameter does, except the last parameter (color).

Look at the library documentation.

I’m assuming:
Address of first led to set colour of
Number of LEDs to set colour
Colour to set LEDs to

Also which of my constants is a magic number?

23 and 20 were “magic numbers” in your original sketch in relation to the lines I corrected. Here are the original uncorrected lines...

 fill_solid(&(leds[0]), 20, CRGB (0,0,0));

if (leadLed == 23){

As I explained they are “magic” because they can be derived from NUM_LEDS which is #defined as 22 at the start of your code, but aren’t. Your code just hard codes tge value again (as 20 or 23).

That means if you ever changed the number of LEDs in your strip you might update the #define but forget to search your entire code for all the magic numbers (or not find some). If you instead always use NUM_LEDS you won’t have that problem: change it in one place, everywhere else uses the correct number.