fill_rainbow() - Unexpected red pixel

[Tim-AQ]

I'm using the builtin fill_rainbow function for generating a rainbow spectrum on an LED strip. The issue is, I get a random red pixel midway through the yellow region. I've tried it on 3 different Arduino nano's, and two different LED-strips. Here's the code:

#include <FastLED.h>

#define DATA_PIN 2
#define NUM_LEDS 114
CRGB leds[NUM_LEDS];

uint8_t hue = 0;

void setup() {
  Serial.begin(115200);
  FastLED.addLeds<WS2812B, DATA_PIN, GRB>(leds, NUM_LEDS);

  fill_rainbow( leds, NUM_LEDS, hue, 1);
  for(int i=0; i<NUM_LEDS; i++) {
    Serial.print(i);
    Serial.print(' ');
    Serial.print(leds[i].r);
    Serial.print(' ');
    Serial.print(leds[i].g);
    Serial.print(' ');
    Serial.println(leds[i].b);
  }

  CHSV hsv;
  CRGB rgb;
  //rgb = hsv;
}

void loop() {
  fill_rainbow( leds, NUM_LEDS, hue, 1);
  FastLED.show();
}

Here pixel number 60 gets the rgb code 160, 0, 0. When it should have something like 160, 150, 0.

The strangest part of all is that I can get rid of the problem in two ways, either by uncommenting the line rgb = hsv;, or by commenting out the fill_rainbow function in the loop function. Any ideas whats going wrong?

[kriegsman]

Great bug report, thank you I’ll take a look.

[Tim-AQ]

Could be worth mentioning that its not always there. Depending on the value of hue it sometimes there and sometimes not. So one pixel (which physical pixel changes, and follows the yellow region of the spectrum) is flickering between red and yellow when the variable hue is slowly increased.

[kriegsman]

Something funny is going on here.

Sometimes I get 'smooth' color outputs, and sometimes I'm seeing a glitch around hue=60. Making small ("unrelated") changes to the sketch change the results. This looks like a compiler-level issue, which is weird already. (Maybe it's possibly related to ambiguities in the treatment of inline asm? Maybe? Investigating...)

Here's a segment out output showing R G B, and also the delta-R, delta-G, and delta-B between successive hues, when the glitch hits (me).

hue R dR G dG B dB 56 160 0 140 -3 0 0
57 160 0 143 -3 0 0
58 160 0 144 -1 0 0
59 160 0 147 -3 0 0
60 160 0 160 -13 0 0 <-- that's a big jump in G 61 160 0 152 8 0 0 <-- and then G jumps back again 62 160 0 155 -3 0 0
63 160 0 158 -3 0 0
64 160 0 160 -2 0 0

Making any number of small changes to the sketch itself can make this problem go away; the numbers change.

[kriegsman]

Just rendering the data to show the glitch. It's not a math error per se, and the fact that re-arranging the source code has an effect on it is indicative of a certain kind of problem.

[kriegsman]

Just for my own reference, here's 'my' version of the code which shows a funky value at pixel 60 (aka hue = 60) repeatably, at least on an Leonardo clone, under avr-gcc 5.4.0 (Arduino 1.8.6 hourly build 2018/07/20... which is questionable, but still)

`#include

define DATA_PIN 13

define NUM_LEDS 114

CRGB leds[NUM_LEDS];

uint8_t hue = 0;

void setup() { Serial.begin(115200); FastLED.addLeds<WS2812B, DATA_PIN, GRB>(leds, NUM_LEDS); delay(3000); }

void loop() { fill_rainbow( leds, NUM_LEDS, hue, 1);

static bool dump = true; if( dump) { dump = false;

for(int i=56; i<65; i++) {
  Serial.print(i);
  Serial.print(' ');
  Serial.print(leds[i].r);
  Serial.print(' ');
  Serial.print(leds[i].g);
  Serial.print(' ');
  Serial.println(leds[i].b);
}

}

FastLED.show(); } ` Output =

56 160 140 0 57 160 143 0 58 160 144 0 59 160 147 0 60 160 160 0 <-- 'G' is too big here 61 160 152 0 62 160 155 0 63 160 158 0 64 160 160 0

PS I hate github Markdown. I'll get over it.

[kriegsman]

What version of Arduino are you running, @Tim-AQ, and on what platform?

[Tim-AQ]

Im using Arduino version 1.8.7 , and the platform is Manjaro Linux.

When the glitch occurs for me, this is the output i get: hue R G B 58 160 144 0 59 160 147 0 60 160 0 0 61 160 152 0 62 160 155 0

[reyemxela]

I was just coming to the issues section to submit one of my own when I found this thread. Running Arduino 1.8.7 on Windows 10.

I was playing around with a color cycle animation, and trying to use CHSV(i, 200, 255) for a more desaturated look. Because of that, I'm getting the hiccup in a slightly different spot on the hue wheel, but still the same problem.

hue-r---g---b 47 146 107 12 48 146 110 12 49 146 112 12 50 146 114 12 51 146 145 12 <--- 52 146 118 12 53 146 120 12 54 146 122 12

[kriegsman]

Thank you for the addition data point. I am definitely seeing some weird stuff going on — and it’s not “just” a quirk in the arithmetic. Something else is up, I think. I’m going to keep trying to isolate it.

On Thu, Dec 6, 2018 at 10:37 AM reyemxela notifications@github.com wrote:

I was just coming to the issues section to submit one of my own when I found this thread. Running Arduino 1.8.7 on Windows 10.

I was playing around with a color cycle animation, and trying to use CHSV(i, 200, 255) for a more desaturated look. Because of that, I'm getting the hiccup in a slightly different spot on the hue wheel, but still the same problem.

hue-r---g---b 47 146 107 12 48 146 110 12 49 146 112 12 50 146 114 12 51 146 145 12 <--- 52 146 118 12 53 146 120 12 54 146 122 12

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/FastLED/FastLED/issues/668#issuecomment-444913897, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRdatNfW_9T3QMRMr9QjxD4J8og86I0ks5u2TmegaJpZM4XilEo .

[sabifa]

Is this issue still present? I'm not quite sure why but I'm facing this problem right now.

My rainbow code looks like the following:

uint8_t hue = 0;

(..)

void LightFading()
{
    thishue++;
    fill_rainbow(leds, NUM_PIXELS, hue, 4);
    FastLED.show();
}

If I delete following code (which is in an entirely different method) it works fine:

long color = strtol(colorString.c_str(), NULL, 16);

Do you have any idea why this is caused and how I am able to fix the problem?

[quakec]

@kriegsman

I had just had the exact same issue.

My setup: Arduino Uno and a WS2812 strip on a pcb ring.

I suspected the problem lie somewhere with a value being floored from an integer division. So I looked at the fill_rainbow function in colorutils.cpp

void fill_rainbow( struct CRGB * pFirstLED, int numToFill,
                  uint8_t initialhue,
                  uint8_t deltahue )
{
    CHSV hsv;
    hsv.hue = initialhue;
    hsv.val = 255;
    hsv.sat = 240;
    for( int i = 0; i < numToFill; i++) {
        pFirstLED[i] = hsv;
        hsv.hue += deltahue;
    }
}

void fill_rainbow( struct CHSV * targetArray, int numToFill,
                  uint8_t initialhue,
                  uint8_t deltahue )
{
    CHSV hsv;
    hsv.hue = initialhue;
    hsv.val = 255;
    hsv.sat = 240;
    for( int i = 0; i < numToFill; i++) {
        targetArray[i] = hsv;
        hsv.hue += deltahue;
    }
}

I noticed the line with hsv.sat = 240; has a number that doesn't fit into 255 ( I know right, I never even looked beyond this ). Anyway changing to hsv.sat = 255; seems to have fixed the issue at my end.

Hope this helps you in the quest to isolate this.

James

[quakec]

@kriegsman

Okay further updates, I had modified the sig for the fill_rainbow function to accept a uint8_t sat parameter.

void fill_rainbow( struct CRGB * pFirstLED, int numToFill,
                  uint8_t initialhue,
                  uint8_t deltahue,
                  uint8_t sat)
{
    CHSV hsv;
    hsv.hue = initialhue;
    hsv.val = 255;
    hsv.sat = sat;
    for( int i = 0; i < numToFill; i++) {
        pFirstLED[i] = hsv;
        hsv.hue += deltahue;
    }
}

void fill_rainbow( struct CHSV * targetArray, int numToFill,
                  uint8_t initialhue,
                  uint8_t deltahue,
                  uint8_t sat)
{
    CHSV hsv;
    hsv.hue = initialhue;
    hsv.val = 255;
    hsv.sat = sat;
    for( int i = 0; i < numToFill; i++) {
        targetArray[i] = hsv;
        hsv.hue += deltahue;
    }
}

What is weird is that I'm passing 240 in the sat parameter of fill_rainbow now and it's working without the red pixel. But this only works if I have code present which alters the value of sat...

  while (Serial.available() > 0) {
    int inChar = Serial.read();
    if (isDigit(inChar)) {
      // convert the incoming byte to a char and add it to the string:
      inString += (char)inChar;
    }
    // if you get a newline, print the string, then the string's value:
    if (inChar == '\n') {

      sat = inString.toInt();

      Serial.print("Value:");
      Serial.println(sat);

      inString = "";

    }    
  }

If I remove this last section of code, having a hard coded sat value the problem still exists. How odd.

[kriegsman]

Thank you for the continued help and investigations. This is still on my list to look into further. Much appreciated.

[Hardi-St]

Hi together,

first of all I’d like to thank you for the wonderful FastLED library. I’m using it in “my” MobaLedLib library which could be found here: https://github.com/Hardi-St The library controls the lights and more on a model railway. The functions are described here: https://www.stummiforum.de/viewtopic.php?f=7&t=165060&sd=a&start=0

I have been struggling over the same problem in the function “fill_rainbow()” where a wrong color is generated if hue = 60.

During my investigations I detected a strange behavior. The generated code size differs if the number of leds is changed in the “fill_rainbow()” call: fill_rainbow( leds, 69, hue, 1); => 4786 Bytes No Error fill_rainbow( leds, 70, hue, 1); => 4844 Bytes Error !!

For the tests I use GCC 7.03.00, FastLED version 3.003.002. But the effect is similar with GCC 5.04.00

Next I compared the generated ASM code of the two builds. For some reasons there are a lot of changes in the ASM code. One change may cause the problem. It affects the call to “scale8_LEAVING_R1_DIRTY”. For me it looks like the compiler has changed the sequence of the R1 usage. It’s shown in this picture: https://abload.de/image.php?img=compare_fill_rainbow_afjvu.jpg

Unfortunately I have no experience with the AVR assembler code. I hope that this may help to fix the problem.

Here is the code I use for the tests:

#include <FastLED.h>

#define DATA_PIN 6
#define NUM_LEDS 114
CRGB leds[NUM_LEDS];

uint8_t hue = 59;

void setup() {
  Serial.begin(115200);
  FastLED.addLeds<WS2812B, DATA_PIN, GRB>(leds, NUM_LEDS);

  fill_rainbow( leds, 70 /*NUM_LEDS*/, hue, 1);
  for(int i=0; i<NUM_LEDS; i++) {
    Serial.print(i);
    Serial.print(' ');
    Serial.print(leds[i].r);
    Serial.print(' ');
    Serial.print(leds[i].g);
    Serial.print(' ');
    Serial.println(leds[i].b);
  }
  FastLED.show();
  CHSV hsv;
  CRGB rgb;
  //rgb = hsv;
}
/*
    6 LEDs 4786 Bytes No Error
   60 LEDs 4786 Bytes No Error
   65 LEDs 4786 Bytes No Error
   67 LEDs 4786 Bytes No Error
   68 LEDs 4786 Bytes No Error
   69 LEDs 4786 Bytes No Error
   70 LEDs 4844 Bytes Error
   75 LEDs 4844 Bytes Error
   96 LEDs 4844 Bytes Error
*/

void loop() {
//  fill_rainbow( leds, NUM_LEDS, hue, 1);
//  FastLED.show();
}

In my test program I have noticed that the wrong LED could also be pink if NUM_LEDS is modified. RGB values 160 147 0 160 0 160 <= Error pink LED 160 152 0 160 155 0 160 158 0 160 160 0 But I have not been able to generate this failure with the code above.

As a work around I have changed the “hsv.sat = 240;” to 242. This has also been mentioned by jamesgohl.

Hardi

[kriegsman]

Thank you... very helpful details.

[decarb]

Has any progress been made on this issue? I'm having exactly the same problems using a WS2812B strip with an Arduino Uno and the default IDE. The error occurs at a hue level of 60.

[kriegsman]

Progress on everything on the library has been on hold since September, when we lost @focalintent in an accident. I hope to have things moving a bit by the end of the year, and this compiler/code issue is still high on my list.

[ddesborough]

Hello @kriegsman . I also ran into this. One thing I have found is that the exact same code loaded through Arduino Create (the web editor) works without the red pixel flash. I noticed that when I uploaded the first time it had to update some drivers on my machine, but that didn't help the native Arduino client. It still uploads code that has the issue. The code size between the two is different, so that seems in-line with prior findings that things that change the image size seem to matter. I have been trying to compare avrdude.conf and anything else I can think of, but no leads so far.

So, to anyone who is blocked by this, try loading your code through Arduino Create as a possible work around for now.

-Davin

[decarb]

Hi @kriegsman,

I managed to create my own workaround for this issue. By copying the rainbow function from the source to my sketch file and changing the sat value to 255, it seems to be working for now. Very strange issue and I'd love to hear if you are able to solve it!

If you are having this issue yourself you can either try using the online editor as @ddesborough suggested or you can copy the source code for the fill_rainbow function to your sketch and use it from there.

Devin

[ddesborough]

Much like @DMJoubert, I had conducted a simple test of having my own fill_rainbow like function in my sketch. I simply did the following:

void testRainbow() 
{
  for(int i = 0; i < NUM_LEDS; i++){
    leds[i].setHue(gHue + 8*i);
  }
}

This works without issue. Change the 8 to whatever hue step you would like to take. Obviously gHue is a global. I was running this in the context of the DemoReel100 example.

The root cause would be interesting to understand. It makes me wonder if there are other things going slightly astray that are not so noticeable (in projects in general, not just FastLED).

Anyway, there are plenty of ways to work around this until Mark is able to return.

[quakec]

I’m starting to think the whole problem is caused by a compiler optimisation issue. I had the same code not working on a windows 10 Arduino IDE / compiler but when compiled on a Mac it worked perfectly fine.

On Thu, 5 Dec 2019 at 03:04, ddesborough notifications@github.com wrote:

Much like @DMJoubert https://github.com/DMJoubert, I had conducted a simple test of having my own fill_rainbow like function in my sketch. I simply did the following:

void testRainbow() { for(int i = 0; i < NUM_LEDS; i++){ leds[i].setHue(gHue + 8*i); } }

This works without issue. Change the 8 to whatever hue step you would like to take. Obviously gHue is a global. I was running this in the context of the DemoReel100 example.

The root cause would be interesting to understand. It makes me wonder if there are other things going slightly astray that are not so noticeable (in projects in general, not just FastLED).

Anyway, there are plenty of ways to work around this until Mark is able to return.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/FastLED/FastLED/issues/668?email_source=notifications&email_token=AANUS37GBVRXPJ4SAZ4VUZ3QXBVTRA5CNFSM4F4KKEUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF7J4KQ#issuecomment-561946154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANUS337R6P2FXP2IK6AZATQXBVTRANCNFSM4F4KKEUA .

-- Sent from my iPhone 11 Pro

[ddesborough]

OK, some findings after playing with compiler options. The file I am making the compiler changes in is D:\Program Files (x86)\Arduino\hardware\arduino\avr\platform.txt.

The lines in that file that I am editing are: compiler.c.flags=-c -g -Os {compiler.warning_flags} -std=gnu11 -ffunction-sections -fdata-sections -MMD -flto -fno-fat-lto-objects

and

compiler.cpp.flags=-c -g -Os {compiler.warning_flags} -std=gnu++11 -fpermissive -fno-exceptions -ffunction-sections -fdata-sections -fno-threadsafe-statics

The specific parameter I am changing is the -Os (that is the letter O).

Using -Os (the default value) produces the red flash we have seen. I then went through the following process:

1. Close the Arduino IDE
2. Edit the -O option on the two lines in platform.txt and save the file
3. Start the IDE
4. Compile and upload.

Here are my results with various settings:

-Os
avrdude: 4138 bytes of flash verified (causes the red flash)

-O1
avrdude: 4558 bytes of flash verified (animation is fine)

-O2
avrdude: 5448 bytes of flash verified (animation is fine)

-O3
avrdude: 6452 bytes of flash verified (animation is fine)

-Os optimizes purely for size. The other options start trading size for better performance. I did not do any sort of performance benchmarking, but you can see the image size is growing with each optimization step.

So, until there is some better answer, you can change the optimization flags in platform.txt to get around this issue. Credit to @jamesgohl for his post which made me go down this path.

-Davin

[Hardi-St]

Hi Davin,

That’s great. It confirms my assumption that there is a problem with the compiler optimization. The problem seems to be the “for()-loop” optimization in combination with the “scale8_LEAVING_R1_DIRTY()” function. The compiler rearranges the ASM code inside a for()-loop depending on the loop counts if the loop counts is fixed. The “fill_rainbow( leds, 70, hue, 1)” line generates such a fixed loop count.

That’s what I showed in this comparison: https://abload.de/image.php?img=compare_fill_rainbow_afjvu.jpg

But to globally change the optimization is not the best solution. The real problem is the “scale8_LEAVING_R1_DIRTY()” function which is used in the HSV to RGB conversion function “hsv2rgb_rainbow()”.

There are two methods to solve the problem:

1. We could locally disable the optimization in the “fill_rainbow()” function by adding a “#pragma GCC optimize ("-O3")” line:

#pragma GCC push_options
#pragma GCC optimize ("-O3")     // Disable the size optimization to prevent the “red pixel” problem
void fill_rainbow( struct CRGB * pFirstLED, int numToFill,
                  uint8_t initialhue,
                  uint8_t deltahue )
{
    CHSV hsv;
    hsv.hue = initialhue;
    hsv.val = 255;
    hsv.sat = 240;
    for( int i = 0; i < numToFill; i++) {
        pFirstLED[i] = hsv;
        hsv.hue += deltahue;
    }
}
#pragma GCC pop_options // Enable the prior optimization

2. Wie could disable the “scale8_LEAVING_R1_DIRTY()” function by setting “#define SCALE8_C 1” in the file “lib8tion.h”.

   
   #if !defined(LIB8_ATTINY)
   #define SCALE8_C 1     // 08.12.19:  Old: #define SCALE8_C 0

In my opinion **the second method is better** because then all potential HSV to RGB conversions are save. In addition, the usage of the “#pragma GCC” switch is only valid if as GCC compiler is used. => Additional #if statements would be needed.
If the “#define SCALE8_C 1” expression is used the code size is also reduced which is a nice effect with the small FLASH amount of an Arduino. The “#pragma GCC optimize” method on the other hand increases the used FLASH.

But both solutions would (potentially) slow down the program. This should be measured!

There is a third method to solve this problem without affecting the speed of the program. In the HSV to RGB conversation function “hsv2rgb_rainbow()” there are three “scale8_LEAVING_R1_DIRTY” calls. These three lines could be packed into one ASM statement. Then the compiler will not rearrange the code. 

### @ Mark Kriegsman: 
May by the “#define SCALE8_C” could be changed to 1 in the next release. 

Hardi

[kriegsman]

I continue to believe that it is a compiler optimization issue, and that what’s essentially needed is to make the code more explicit about what the compile can and can’t move around — if we’re having this issue here, we’re probably having it other places too but we just haven’t isolated those bugs yet. So, I want to actually fix it. Hoping to be able to in the next two weeks.

[quakec]

Hmmm. Compiler options are there for a reason. If your code isn’t working under a specific setting that does not necessarily mean your code is wrong. I think seasoned cpp programmers become very purist.

On Sun, 8 Dec 2019 at 11:27, Mark Kriegsman notifications@github.com wrote:

I continue to believe that it is a compiler optimization issue, and that what’s essentially needed is to make the code more explicit about what the compile can and can’t move around — if we’re having this issue here, we’re probably having it other places too but we just haven’t isolated those bugs yet. So, I want to actually fix it. Hoping to be able to in the next two weeks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/FastLED/FastLED/issues/668?email_source=notifications&email_token=AANUS335ITN7R6XBCTM2RXDQXTKYZA5CNFSM4F4KKEUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGG33OQ#issuecomment-562937274, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANUS326SQLH5AR5YIFYOR3QXTKYZANCNFSM4F4KKEUA .

-- Sent from my iPhone 11 Pro

[kriegsman]

If the code isn’t working, its worth it to me to understand exactly why that is, what changed in the compilation process, so that I can understand where else to be aware of it. Everyone has been very helpful here with this one, especially during a very hard personal time, and I really appreciate all of it.

[OneEyeRick]

I am experiencing this problem. I am a coding novice, so I don't know the significance of my findings. If you use delay() the red pixel shows. However if you use millis() it does not.

The following code creates a red pixel in the yellowish region:

fill_rainbow( leds, NUM_LEDS, hue, 1);
  FastLED.show();
  hue++;
delay(30);  //any delay causes red pixel

The following code does not create a red pixel:

if(millis() > time_now + period){ 
  time_now = millis();
  fill_rainbow( leds, NUM_LEDS, hue, 1);
  FastLED.show();
  hue++;
 }

I don't know if this helps solve the problem. Hopefully someone can use it to get rid of their red pixel.

[ddesborough]

@OneEyeRick - Please see my comment where I discuss the compiler options: https://github.com/FastLED/FastLED/issues/668#issuecomment-562912641

This is a fairly simple change (a small change to two lines in a text file) and it will let you work around the issue for now. Use the -O1 option and you should be good to go. This route will let you write "normal" code that you won't need to change later on.

I almost forgot, you can also try using Arduino create (the web based version) rather than a local install as it seems to compile differently and not have the issue. At least it did back when I was testing this.