Can some help me with this i want to press a switch to start a loop so that my led stays on for a while and then closes and i messed it up somehow

int ledPin = 3;
int buttonPin = 2;
int i;
void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  int i = 0;
}

void loop() {
  int buttonState;
  buttonState = digitalRead(buttonPin);
  delay(50);
  if (buttonState == HIGH) {
    for (i = 0; i >= 20; i = i + 1) {
      digitalWrite(ledPin, HIGH);
      delay(500);
      i = i + 1;

    }
  } else {
    digitalWrite(ledPin, LOW);
  }

}

Hello and welcome to the forum,

first thing to do is
You should post code by using code-tags
There is an automatic function for doing this in the Arduino-IDE
just three steps

  1. press Ctrl-T for autoformatting your code
  2. do a rightclick with the mouse and choose "copy for forum"
  3. paste clipboard into write-window of a posting

So do you still have the original version of the code that worked?
If no the lesson is: if a code has reached a certain state which is working safe a copy!"

I do this by saving this version with a 3 digit serial number.
The functionality of your code is
wait for button press
if button is pressed then turn on LED for n seconds
after n seconds switch LED off

So the name should tell the main aspect of the functionality
something like "Buttonpress-LED-On-Off.ino"
And including a version-number something like this
Buttonpress-LED-On-Off-001.ino
Saving the working version and immediately after that
Saving the same file with an increased version-number
Buttonpress-LED-On-Off-002.ino

This enables to roll back at any time to a working version.
If you roll back and want to start modifying from an earlier version
load older version and immediately safe with a new version number
to still keep the working version.
Another way would be to save a copy to a different folder named something like
"Working-Codes"

For analysing what is going on in your code the standard method is to use serial output.
You can print to the serial monitor almost anything

  • content of variables
  • entering / leaving if-conditions
    etc.

This makes visible what you code is doing and a lot of "think-work" with maybe wrong assumptions is replaced by what is really happening. And in 95% of cases this will give you ideas what to change and/or will point you to that place in the code that needs your attention.

So how much do you know about setting up the serial monitor?

best regards Stefan

1 Like

There are several issues with your code:

  • In order to take action you are testing for the input pin to be HIGH while it is always HIGH due to INPUT_PULLUP.
  • Your for() loop never runs, please read about the structure, specifically the "condition" part which in your case prevent the loop from running.
  • No need to declare i, you are not using it outside the for() loop.

Here is an example of working code:

const int ledPin = 3;
const int buttonPin = 2;

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
}

void loop() {
  if (!digitalRead(buttonPin)) {
    for (int i = 0; i <= 20; i++) {
      digitalWrite(ledPin, HIGH);
      delay(500);
    }
  } else {
    digitalWrite(ledPin, LOW);
  }
}

and here's a simplified version as you don't rally need the for() loop:

void loop() {
  if (!digitalRead(buttonPin)) {
    digitalWrite(ledPin, HIGH);
    delay(10000);
  } else {
    digitalWrite(ledPin, LOW);
  }
}

@ninora

Yes your simplified version of the code has the required functionality.
But it uses the command delay().
delay() is that command that delays beginners in learning to really program.

The command delay() sets beginners on the path programming is linear sequential - you can't do two things at the same time.

Well there is truth in saying "a computer can only do one thing at a time" (per CPU-core). But almost any porgramming that goes beyond switching a LED On/Off uses non-blocking timing based on function millis().

Once newbees have learned to use delay() and then try to switch over to millis() have a harder time to learn it because their thinking has get used to "delay-thinking"
So IMHO it is better to introduce non-blocking timing right from the start.

I do this by using a function that has a self-explaining name.
The purists will now scream "that this is not the standard!"

That's true my function is non-standard. But what you call "the standard" is much harder to understand.

So here is a code-version that uses non-blocking timing

// non-blocking timing-function
boolean TimePeriodIsOver (unsigned long &periodStartTime, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();  
  if ( currentMillis - periodStartTime >= TimePeriod )
  {
    periodStartTime = currentMillis; // set new expireTime
    return true;                // more time than TimePeriod) has elapsed since last time if-condition was true
  } 
  else return false;            // not expired
}

// millis()-timing-variables MUST be of type unsigned long
unsigned long MyTestTimer = 0; 
boolean buttonHasBeenPressed = false;

int ledPin    = 3
int buttonPin = 2

void setup() {
  // make sure to adjust the baud in the serial monitor 
  // to the number inside of the parenthesis
  Serial.begin(115200); 
  Serial.print( F("\n Setup-Start  \n") );

  pinMode(ledPin, OUTPUT);
  // using INPUT_PULLUP means unpressed button == IO-Pin HIGH
  // pressed button IO-Pin == LOW
  pinMode(buttonPin, INPUT_PULLUP);
  // boolean flag for remembering if button has been pressed
  buttonHasBeenPressed = false;
}


void loop() {
  int buttonState;
  buttonState = digitalRead(buttonPin);
  
  // if button has NOT been pressed (recently)
  // and button is pressed now (new)
  if ( (buttonHasBeenPressed == false) && (buttonState == LOW) ) {
    digitalWrite(ledPin, HIGH);

    Serial.print( F("Button pressed! \n") );
    buttonHasBeenPressed = true;
    MyTestTimer = millis(); // update timing variable
  }    

  // when the time specified in the function TimePeriodIsOver
  // has passed by the function TimePeriodIsOver results "true"
  if ( TimePeriodIsOver (MyTestTimer,5000) && buttonHasBeenPressed == true) {
    digitalWrite(ledPin, LOW);
    // actual "LED-is-on-time" is over reset boolean flag to false
    buttonHasBeenPressed = false; 
    Serial.print( F("Time is over \n") );
  }
}  

best regards Stefan

i think i messed everything up but... THANKS

Ya i changed my code a little there was a lot of logic error
THANKS

This one worked... i just didnt want to use delay

int ledPin = 3;
int buttonPin = 2;
int i;
void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
 }

void loop() {
  int buttonState;
  buttonState = digitalRead(buttonPin);
  if (buttonState != HIGH) {
    for (int i = 0; i <= 20; i = i + 1) {
      digitalWrite(ledPin, HIGH);
      delay(500);
      i = i + 1;

    }
  } else {
    digitalWrite(ledPin, LOW);
  }

}

but you do use delay() !

delay(500);

Now I'm curious when you will come up with a question like
"How can I do thing "A" and at the same time thing "B"

That's when non-blocking timing comes into play.

Your-code-version is more complicated than it has to be
combining a blocking for-loop with the blocking command delay()
incrementing the countervariable in two different places

for (int i = 0; i <= 20; i = i + 1) {

and here

 i = i + 1;

trying to be quick (in the beginning) turns out to slow you down in the end.

But of course you are free to code in any way you want.

best regards Stefan