1

I have a 20x20 matrix of neopixel LED's. I have a certain set of LED's on in different places at different times. Each one is set to mainColor. Right now mainColor is just set to strip.Color(r, g, b) but I would like to have those color fade through the spectrum. I decided the easiest way would just be to do something like this, and simply return strip.Color(r, g, b) to mainColor, then calling it to turn change the colors like so:

strip.setPixelColor(pixelNumber, mainColor(5));

Here is the code for mainColor: int r; int g; int b = 255; int _r; int _g; int _b = 255;

void Color (int FADESPEED)
{
    // fade from blue to violet
    for (_r = 0; _r < 256; _r++) {
        r = _r;

        mainColor = strip.Color(r, g, b);
        Clock ();

        delay(FADESPEED);
    }
    // fade from violet to red
    for (_b = 255; _b > 0; _b--) {
        b = _b;

        mainColor = strip.Color(r, g, b);
        Clock ();

        delay(FADESPEED);
    }
    // fade from red to yellow
    for (_g = 0; _g < 256; _g++) {
        g = _g;

        mainColor = strip.Color(r, g, b);
        Clock ();

        delay(FADESPEED);
    }
    // fade from yellow to green
    for (_r = 255; _r > 0; _r--) {
        r = _r;

        mainColor = strip.Color(r, g, b);
        Clock ();

        delay(FADESPEED);
    }
    // fade from green to teal
    for (_b = 0; _b < 256; _b++) {
        b = _b;

        mainColor = strip.Color(r, g, b);
        Clock ();

        delay(FADESPEED);
    }
    // fade from teal to blue
    for (_g = 255; _g > 0; _g--) {
        g = _g;

        mainColor = strip.Color(r, g, b);
        Clock ();

        delay(FADESPEED);
    }
}

This was good but it would only return r, g, b as it was on the last iteration of the last for loop, which obviously isn't what I want. But when I un-comment the Serial.print's the values were perfect. So then I put return strip.Color(r, g, b) into each for loop, however this only returns the first iteration of the first loop, which is also not what I want. Does anyone know how in the world I can return strip.Color(r, g, b) for every iteration of every for loop? Or if not, any alternatives that would work in this situation.

Thanks, Matthew

  • If you return something, the loop ends. I don't know why you would return something in every iteration. That doesn't make sense. they don't do anything until the for loops are done, which takes forever - well, get rid of the delay calls. In fact, a rewrite is called for. Step back from your code, think of what you want to do, write some pseudo-code on paper, and then code when you are happy you have got a good plan. – Nick Gammon May 22 '16 at 20:42
  • @NickGammon That is a good idea, but I need the delay because I want the colors to fade slowly, but I will try some pseudo code...In the mean time, any ideas? – mr-matt May 22 '16 at 22:28
  • 2
    Yes, but you don't have to use delays. See How to do multiple things at once ... like cook bacon and eggs for some ideas. – Nick Gammon May 23 '16 at 5:10
  • Thanks @NickGammon, that site is pretty useful, I will see what I can do with that. – mr-matt May 23 '16 at 6:03
1

There appears to be a number of problems with this code.

The variables only have scope with in this function and are not static. So they will not survive from call to call. Every time you call this function it will be like starting over.

Putting returns in the middle of code is not good coding practice. It is very difficult to follow and hard to debug. It would be best to arrange your code so that there is only 1 return at the end of the function.

This code (from Adafruit) only controls 1 RGB LED. If you are to control 40 RGB LEDs all at different stages of color changing you would need 40 copies of this function. This is not very efficient. Instead use this function to populate one lookup table with R G B data. Then track 40 variables, one for each RGB LED. Use the 40 variables to lookup a specific color combination in the lookup table for each of the 40 RGB LEDs.

  • I have updated the code so the scope is more suitable, though it was designed to not actually matter because the last iteration of the last loop is technically the same as the first iteration of the first loop. I have also removed the extra returns, however it still only is returning the last iteration of the last loop...so back to square one with that. As for the LED's, they are all to be colored the same so it shouldn't really matter, right? – mr-matt May 21 '16 at 22:08
  • All your LEDs are to be the same color? I didn't realize that. So, in the original Adafruit code, a call was made to the function analogWrite() inside each of the different color changing loops. You are trying, instead, to return that value each time you call your function mainColor(). This is very difficult to do! I would do what the code on Adafruit's web site does. Or, as you have already found out, if you print out the values they look correct. So why not call the function analogWrite() from the same locations as your commented out print statements? – st2000 May 21 '16 at 23:44
  • I am building a word clock, so they cannot all be on at the same time, if they were I would just use the rainbow cycle provided with neo pixels code library, that's why I would rather have the one color that I can call for each set of LEDs. – mr-matt May 22 '16 at 0:44
  • So, some LEDs are on and some are off. The ones which are on all change their color in unison. So, continuously call the change color function. But from that function call a function that choses which LED to light. Only from that function make the call to strip.setPixelColor() for the LEDs which are to be lit. – st2000 May 22 '16 at 3:08
  • Yes, I'm not home at the moment but I may have an idea. I have a for loop void that takes in an array of pixel id's, the length of that array, and also the color, I do that rather than doing a for loop every time just to make it easier for me. So every group of LEDs call that for loop void, including the color. But I just thought, seeing as though every group uses the same color, I might as well make put that color just in the for loop void, as well as the mainColor loops. I'll give that a go once I'm home, do you think that would make a difference? – mr-matt May 22 '16 at 3:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.