1

I am a beginner in Arduino programming. This is my program for 5 LEDs and 1 Button. The LEDs are supposed to light before I press the button. But the LEDs start immediately, even without pressing the button. How is it possible?

int sw7=7;
int timer=500;
int start=0;
int stopp=100;
void setup()
{
  for (int opin=1;opin<6;opin++)
  {
    pinMode(opin,OUTPUT);
    pinMode(sw7,INPUT);
  }
}

void loop()
{
  if (digitalRead (sw7)==LOW)
  {
    while (start<=stopp);
  }
  {
    for (int opin=1;opin<=5;opin++)
    {
      digitalWrite(opin,HIGH);
      delay(timer);
      digitalWrite(opin,LOW);
     } 
  }
}
  • 1
    how did you wire the button? has it pull-down? – Juraj Feb 15 at 14:34
  • It's a large tack switch. 1 foot is on pin 7 in series with the other foot connected to 5v and the foot beside the foot that is connected in pin 7 is connected to ground. Is there nothing wrong with the program? Thank you. – user54017 Feb 15 at 14:37
  • I aligned your code. do you see how the for loop is outside of the if for the button? – Juraj Feb 15 at 14:38
  • 1
    no resistor? wire it this way and use mode INPUT_PULLUP arduino.cc/en/Tutorial/InputPullupSerial – Juraj Feb 15 at 14:41
  • 1
    @Gerben. void loop() { if (digitalRead (sw7)==LOW) { while (start<=stopp); } else { for (int opin=1;opin<=5;opin++) { like this? – user54017 Feb 15 at 14:53
2

Several things:

You need to explicitly set your output pins to LOW in your setup method.

The code to set the sw7 to input should not be inside the loop.

You should probably connect your switch between sw7 and ground and use INPUT_PULLUP mode (in INPUT_PULLUP mode the line is connected to +5V through a "pull-up resistor" so that it reads HIGH until you press the button, and then it reads LOW. If you don't use either an input pull-up or pull-down resistor then the input tends to "float" and give bad readings.)

You say "The LEDs are supposed to light before I press the button. But the LEDs start immediately, even without pressing the button."

I'm not sure what you mean. What do you want to happen, exactly? Do you want the LEDs to be lit steadily, and then turn off or flash when you press the button?

The code below will turn off all the LEDs initially, then toggle between off and flashing on each (debounced) button press. Switch the value of idleLEDState to HIGH if you want the LEDs to be lit when they are not flashing.

#define flashDelay 50 //Delay between on and off state for LEDs
#define switchBounceDelay 100


uint8_t idleLEDState = LOW; //Change to HIGH 
                            //if you want the LEDs to be lit when not flashing
bool flashLEDs = false;
bool ledsOn = false; 
unsigned long nextFlashTime = 0;
unsigned long nextSwitchCheckTime = 0;

void setup() {
  pinMode(sw7, INPUT_PULLUP); //Connect the switch between sw7 and ground
  for (int opin=1;opin<6;opin++)
  {
    pinMode(opin,OUTPUT);
    digitalWrite(opin, idleLEDState); 
  }
}

void loop() {

  //If it's time to check the switch, and it's being pulled LOW, toggle flashLEDs
  if (mills() > nextSwitchCheckTime && digitalRead(sw7) == LOW) {
    flashLEDs = !flashLEDs;
    nextSwitchCheckTime = millis() + switchBounceDelay;
  }

  if (!flashLEDs) {
    //IF we're not supposed to flash the LEDs, set them to their idle state 
    for (int opin=1;opin<6;opin++) {
        digitalWrite(opin, idleLEDState); 
     }
  } else if millis() > nextFlashTime {
    nextTime = millis() + flashDelay;
    ledsOn = !ledsOn;
    for (int opin=1;opin<6;opin++) {
        uint8_t newState = ledsOn ? HIGH : LOW;
        digitalWrite(opin, newState); 
     }
}

I banged out the code above in the Stack Exchange editor. It likely contains a few typos. Please exert some effort to fix it and adapt it to your needs before posting back "Your code didn't work. I got error XYZ."

0

This is a sketch according to your requirements:

// pin mapping
int pin_map[] = { 
  1,  // 0: led in pin 1
  2,  // 1: led in pin 2
  3,  // 2: led in pin 3
  4,  // 3: led in pin 4 
  5,  // 4: led in pin 5
  7   // 5: button (pull-up)
};

int timeout_ms = 500;
int blink_ms = 100;

void setup()
{
  // setup pins
  for (int opin = 0; opin < 5; opin++) pinMode(pin_map[opin], OUTPUT);
  pinMode(pin_map[5], INPUT_PULLUP);

  // wait for activation
  while (millis() < timeout_ms)
  {
    // activate by button (off:pull-up on:GND)
    if (digitalRead (pin_map[5]) == LOW) break;
  }
}


void loop()
{
  for (int opin = 0; opin < 5; opin++) 
  {
    // toggle LED
    int pin = pin_map[opin];
    pinMode(pin, !digitalRead(pin));
  }

  delay(blink_ms);
}

You will need avoid delay() usage if you need asynchronous tasks. Probably, you should study C language and practice with some hundred of sketchs to improve your code skills.

  • Putting the LED pins and the switch pins into the same array seems confusing. They do different things. I do see the value in using an array to store the LED pin assignments though. That way you can use any pins for the LEDs, even discontinuous pin numbers. – Duncan C Feb 16 at 15:11

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.