2

I want to make a countdown timer with a LilyPad Arduino for 3 seconds with red (1 second), then orange(1 second), then yellow (1 second), then green (3 seconds), all being triggered by a button. I want to check to see if I have the right code. I am a self taught programmer, so apologies if this is a little messy. (or a lot)

#include <Adafruit_NeoPixel.h>
const int inputPin = 3;

void setup() {
  // put your setup code here, to run once:
  pinMode(inputPin, INPUT_PULLUP);
  strip.begin();
  strip.show();
}

void loop() {
  // put your main code here, to run repeatedly:
  If (digitalRead (inputPin) == LOW) {    
    For (int i = 0; i < 12; I++) {
      Int pixelNum1 = 1;
      strip.setPixelColor( pixelNum1, 244, 0, 0);
      strip.show();
      delay (27);
      pixelNum1 ++;
    }

    For (int i = 0; i < 24; I++) {
      Int pixelNum2 = 1;
      strip.setPixelColor( pixelNum, 247, 247, 0)
      strip.show();
      delay (27);
      pixelNum ++;
    }

    For (int i = 0; i < 12; I++) {
      Int pixelNum3 = 1;
      strip.setPixelColor( pixelNum, 0, 247, 41)
      strip.show();
      delay (27);
      pixelNum ++;
    }
  }
}
4
  • 1
    edit your question. Select all the code and hit <ctrl-k>. Read editing-help for more info. – Johnny Mopp Sep 27 '17 at 2:14
  • Also, If, For, and Int are not valid C++ keywords (unless you have some defines/typedefs). Please post the actual code. – Johnny Mopp Sep 27 '17 at 2:15
  • Does the code do what you want it to? – Delta_G Sep 27 '17 at 3:49
  • variables are case sensitive so i and I are not the same variable – ratchet freak Sep 27 '17 at 8:17
2
For (int i = 0; i < 12; I++) {
      Int pixelNum1 = 1;
      strip.setPixelColor( pixelNum1, 244, 0, 0);
      strip.show();
      delay (27);
      pixelNum1 ++;
    }

Assuming you fix the upper-case lower-case issues here first, you've still got a scope issue. Since pixelNum1 is defined inside the for loop, it will go out of scope at the end of the for loop. So it will be 1 on every iteration and it won't increment like you seem to think it will.

Why don't you just get rid of it altogether and use i+1 instead?

2

In addition to what has already been said, I would add these two recommendations:

  1. Do not repeat yourself. If you are repeating the same code, only with different data, then put that data into an array and loop through it.
  2. Define all your constants at the beginning, instead of having them as “magic numbers” within the code itself.

Note that those are only good programming practices. They are not critical for the functioning of your code, but they make it easier to read and to maintain.

Applying those recommendations, here is my throw at this. Note that the Color struct is only meant to make it easier to define colors as constants:

#include <Adafruit_NeoPixel.h>

struct Color {
    uint8_t r, g, b;
};

const uint8_t  INPUT_PIN  =  3;
const uint8_t  STRIP_PIN  =  6;
const int      LED_COUNT  = 12;
const uint32_t LED_DELAY  = 27;
const int      NUM_COLORS =  4;
const Color    COLORS[NUM_COLORS] = {
                   {244,   0,  0},  // red
                   {247, 122,  0},  // orange
                   {247, 247,  0},  // yellow
                   {  0, 247, 41}   // green
               };

Adafruit_NeoPixel strip(LED_COUNT, STRIP_PIN);

void setup()
{
    pinMode(INPUT_PIN, INPUT_PULLUP);
    strip.begin();
    strip.show();
}

void loop()
{
    // Nothing to do until the button is pressed.
    if (digitalRead(INPUT_PIN) == HIGH)
        return;

    // Cycle through the colors.
    for (int i = 0; i < NUM_COLORS; i++) {
        Color color = COLORS[i];
        for (int led = 0; led < LED_COUNT; led++) {
            strip.setPixelColor(led, color.r, color.g, color.b);
            strip.show();
            delay(LED_DELAY);
        }
    }

    // Switch off.
    strip.clear();
    strip.show();
}

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.