Getting out of a cycle in a for loop

[8juancho]

Hi everyone, I'm programming a program to start a sequence with five leds using a push button then turn off the leds with the same push button, I'm making the sequence with cycle for but when I push again leds don't turn off

const byte led1=8;
const byte led2=9;
const byte led3=10;
const byte led4=11;
const byte led5=12;
const byte ST=A5;
byte flag=0;
void setup() {

pinMode (led1,OUTPUT);
pinMode (led2,OUTPUT);
pinMode (led3,OUTPUT);
pinMode (led4,OUTPUT);
pinMode (led5,OUTPUT); 
pinMode (ST, INPUT);
void loop(){
if(digitalRead (ST)==1) {
flag= !flag;
  //ELIMINATE BOUNCING
delay(30);
//wait to release the pushbutton
while (digitalRead (ST));
delay(30);
}

//algoritmmo principal
if(flag==1){
  for (int i=8;i<=12;i++){
    digitalWrite(i,HIGH);
    delay(500);
  }
  for (int i=12;i>=8;i--){
    digitalWrite(i,LOW);
    delay(500);
  }
 
}else{
digitalWrite(led1,LOW);
digitalWrite(led2,LOW);
digitalWrite(led3,LOW);
digitalWrite(led4,LOW);
digitalWrite(led5,LOW);
}

}

The leds keep on the cycle, do you see a mistake here?
Thank you

[pert]

I moved your topic to an appropriate forum category @8juancho.

In the future, please take some time to pick the forum category that best suits the subject of your topic. There is an "About the _____ category" topic at the top of each category that explains its purpose.

This is an important part of responsible forum usage, as explained in the "How to get the best out of this forum" guide. The guide contains a lot of other useful information. Please read it.

Thanks in advance for your cooperation.

Due to your history of forum rule violations, I am giving you a short account suspension. I hope that when you return to the forum you'll be more respectful of our community.

[Delta_G]

Follow the execution of your loop line by line and think about it. It reads the button, but then it gets to the for loop. Inside the for loop nothing reads the button and at the end of the for loop it goes back to the beginning of the for loop. SO for that entire duration, you've not coded for anything involving that button.

Don't block with a for loop. Let the loop function be the loop. Use if statements and variables to keep count and work out how to take one step at a time. Use millis to see how long since the last step so you can decide if it is time to take another step as in the famous Blink Without Delay example sketch.

[J-M-L]

Said differently, This takes 5 seconds to execute, time during which the button is not checked at all.

@8juancho

for (int i=8;i<=12;i++){
    digitalWrite(i,HIGH);
    delay(500);
  }
  for (int i=12;i>=8;i--){
    digitalWrite(i,LOW);
    delay(500);
  }

So you need to get rid of those delays. For extra information and examples look at

• Using millis() for timing. A beginners guide
• Several things at the same time
• Flashing multiple LEDs at the same time

[paulpaulson]

Hello 8juancho

I assume that you have written the programme by yourself, then it is quite easy to find the error.

There's a trick to figuring out why something isn't working:

Use a logic analyzer to see what happens.
Put Serial.print statements at various places in the code to see the values of variables and determine whether they meet your expectations.

Take a view here to check the proper wiring of the button.

Have a nice day and enjoy coding in C++.

p.s. the sketch won´t compile.

[gcjr]

look this over
need to make processing of leds and monitoring of buttons independent of one another
more of an architecture than coding question (how do you make the different pieces work together)

const byte ButPin    = A5;
const byte LedPin [] = { 8, 9, 10, 11, 12 };

const int  Nled      = sizeof(LedPin);

byte butState;
int  flag;
int  idx;
unsigned long msecLast;

enum { Off = HIGH, On = LOW };

void loop ()
{
    unsigned long msec = millis ();

    if (flag) {
        if (msec - msecLast >= 500)  {
            msecLast = msec;
            digitalWrite (LedPin [idx], ! digitalRead (LedPin [idx]));
            if (Nled <= ++ idx)
                idx = 0;
        }
    }
    else  {
        for (int n = 0; n < Nled; n++)
            digitalWrite (LedPin [n], Off);
    }

    byte but = digitalRead (ButPin);
    if (butState != but)  {         // change
        butState = but;
        delay (20);                 // debounce
        if (LOW == but)             // pressed
            flag = ! flag;
    }
}

void setup ()
{
    Serial.begin (9600);

    pinMode (ButPin, INPUT_PULLUP);
    butState = digitalRead (ButPin);

    for (int n = 0; n < Nled; n++)  {
        pinMode (LedPin [n], OUTPUT);
        digitalWrite (LedPin [n], Off);
    }
}

[StefanL38]

As already mentioned as long as you are inside the for-loop you are inside the for-loop and the lines of code

      for (int i = 8; i <= 12; i++) {
        digitalWrite(i, HIGH);
        delay(500);
      }
      for (int i = 12; i >= 8; i--) {
        digitalWrite(i, LOW);
        delay(500);
      }

etc.

You are asking for a functionality that requires pretty complex coding
The code postd below uses multiple things that all need their own time to learn

important:

button must be wired between IO-pin and GROUND

• using arrays
• using functions with and without parameters
• using serial printing for debugging / observing / explaining what the code is doing
• using flags
• using non-blocking timing based on millis()

/*
  const byte led1 = 8;
  const byte led2 = 9;
  const byte led3 = 10;
  const byte led4 = 11;
  const byte led5 = 12;
*/

//           IO-pin-numbers 8  9  10  11  12
const byte myLEDs1to5[5] = {8, 9, 10, 11, 12};

const byte StartButton = A5;
boolean runMySequence = false;
boolean sequenceSwitchOn = true;

byte buttonState;
byte lastButtonState;
int  IndexNr = 0;

void setup() {
  Serial.begin(115200);
  Serial.println( F("Setup-Start") );

  for (byte IndexNr = 0; IndexNr < 5; IndexNr++) {
    explainArrayUse1(IndexNr);
    pinMode (myLEDs1to5[IndexNr], OUTPUT);
  }

  pinMode (StartButton, INPUT_PULLUP);
}


void loop() {
  buttonState = digitalRead(StartButton);

  // check if buttonstate has CHANGED
  if (lastButtonState != buttonState) {

    if (buttonState == LOW) {
      Serial.println( F("Button pressed starting sequence") );
      IndexNr = 0;
      runMySequence = true;
      //ELIMINATE BOUNCING
      delay(30);
      //wait to release the pushbutton
      while (!digitalRead (StartButton));
      delay(30);
    }
  }

  //algoritmmo principal
  if (runMySequence) {
      switchLedsOn();
  }

  lastButtonState = buttonState;
}



void explainArrayUse1(int myParameter) {

  int myArrayElementNr = myParameter;

  Serial.print( F("using array-element with IndexNr ") );
  Serial.print( myArrayElementNr );
  Serial.print( F(" which contains value ") );
  Serial.print( myLEDs1to5[myArrayElementNr] );
  Serial.println( F(" as OUTPUT") );
  Serial.print( F("") );
  Serial.print( F("pinMode(") );
  Serial.print( myLEDs1to5[myArrayElementNr] );
  Serial.println( F(", OUTPUT);") );
}


void explainArrayUse2(int myParameter) {

  int myArrayElementNr = myParameter;

  Serial.print( F("using array-element with IndexNr ") );
  Serial.print( myArrayElementNr );
  Serial.print( F(" which contains value ") );
  Serial.print( myLEDs1to5[myArrayElementNr] );
  Serial.println( F(" and switch IO-pin HIGH") );
  Serial.print( F("") );
  Serial.print( F("digitalWrite(") );
  Serial.print( myLEDs1to5[myArrayElementNr] );
  Serial.println( F(", HIGH);") );
}


void explainArrayUse3(int myParameter) {

  int myArrayElementNr = myParameter;

  Serial.print( F("using array-element with IndexNr ") );
  Serial.print( myArrayElementNr );
  Serial.print( F(" which contains value ") );
  Serial.print( myLEDs1to5[myArrayElementNr] );
  Serial.println( F(" and switch IO-pin LOW") );
  Serial.print( F("") );
  Serial.print( F("digitalWrite(") );
  Serial.print( myLEDs1to5[myArrayElementNr] );
  Serial.println( F(", LOW);") );
}


void switchLedsOn() {
  static unsigned long myTimer;

  // check if 500 milliseconds of time have passed by
  // if not do nothing
  if ( TimePeriodIsOver(myTimer, 500) ) {
    // when REALLY 500 milliseconds of time have passe by
    if (sequenceSwitchOn) {
      digitalWrite (myLEDs1to5[IndexNr], HIGH);
      explainArrayUse2(IndexNr);
      IndexNr++;

      if (IndexNr >= 5) {
        sequenceSwitchOn = false;
        IndexNr--;
      }
    }

    // if switching on LEDs 1,2,3,4,5 is done
    else {  // sequenceSwitchOn is false
      digitalWrite (myLEDs1to5[IndexNr], LOW);
      explainArrayUse3(IndexNr);
      IndexNr--;
      if (IndexNr <= -1) {
        runMySequence = false; // stop executing the sequence
        sequenceSwitchOn = true;
        Serial.println( F("sequence finished") );
        IndexNr = 0;
      }
    }
  }
}



void switchLedsOFF() {
  static unsigned long myTimer;

  // check if 500 milliseconds of time have passed by
  // if not do nothing
  if ( TimePeriodIsOver(myTimer, 500) ) {
    // when REALLY 500 milliseconds of time have passe by
    digitalWrite (myLEDs1to5[IndexNr], LOW);
    explainArrayUse3(IndexNr);
    IndexNr++;

    if (IndexNr >= 5) {
      runMySequence = false; // stop executing the sequence
      Serial.println( F("sequence finished") );
      IndexNr = 0;
    }
  }
}


// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}

understanding all this in detail requires learning different coding techniques
a tutorial about non-blocking timing that is used here

Example-code for timing based on millis() easier to understand through the use of example-numbers / avoiding delay()

non-blocking timing. Execute code only from time to time. There are a lot of different ways to learn programming. This thread wants to add another approach that is different to the yet existing ones. UPDATE 06.01.2023 if you are mainly interested in applying non-blocking timing you can do a quick read of this short tutorial / demonstration If you are intersted in understanding the details how it works go on reading here. I use an everyday analogon in this post to explain the basic princip…

best regards Stefan

[alto777]

@8juancho

when I push again leds don't turn off

As you have figured out, the for loops are not looking at the pushbutton, so they will run to completion no matter you less it or not.

You can fix his in a way you might understand readily, viz:

  for (int i=8;i<=12;i++){
    digitalWrite(i,HIGH);
    delay(500);

    if (digitalRead(ST)) break;
  }

Assuming your button reads HIGH (true), thus will cause the loop to be exited when you press the button.

It will still be a bit sluggish, as it may be as long as one half second (500 milliseconds) before it actually reads the button.

Put a check like that inside each for loop.

Of course breaking out it the loops like that will leave your LEDs in the middle of the sequence, so watch out for that.

a7

[StefanL38]

The code in post #7 does switch on sequentially led 1,2,3,4,5 and then switch them OFF 5,4,3,2,1

But this code-version does not yet stop the sequence at any earlier point.
then full sequence has ran down.

I tried to write the code with this immidiately stopping using flags.
But it turned out to be pretty complicated at least with the aproach I tried to make it work.

But there is a much easier approach:
using a state-machine
You can imagine a state-machine as putting the device in different modes of operation

mode 1: with all LEDs off waiting for a buttonpress
if button is pressed switch to mode 2

mode 2: switch LEDs sequentially on 1,2,3,4,5 with delay 500 milliseconds
automatically switch to mode 3

mode 3: switch LEDs sequentially off 5,4,3,2,1 with delay 500 milliseconds
if sequence is finished switch automatically to mode 1

mode 4: switch all LEDs off immidiately without any (noticable) delay
then switch automatically to mode 1

the button is always checked for presses
with checking an additional condition

if (sequenceIsActive) {
switch to mode 4 (immidiately OFF)

if the sequence is active at any point simply change mode of operation to mode 4 (immidiately OFF)

this is done using the switch-case-break;-statement

The switch-case-break-statement reduces the if-conditions and flag-variables to a minimum because in each mode only that code that belongs to that mode of operation is executed. This means you simply don't need additional flags

These modes of operation are called "states".
Hence the name state-machine. This word was chosen from highly sophisticated theoretical informatic scientists but is etablished as the common name for it.

Here is a tutorial for state-machines

A Demo-Code explaining the switch-case state-machine and how to do things (almost) in parallel

Hi everybody, if you look at the end of this first posting you see a list of links that show where this tutorial was used as a link. Go through this list to find additional information on how to apply this code This democode requires only the pure microcontroller without any hardware. You just open the serial monitor to watch the behaviour of the code. From time to time I enjoy writing demo-codes that want to explain programming-functionalities. There are a lot of different approachs to expl…

best regards Stefan

[gcjr]

@StefanL38

I tried to write the code with this immidiately stopping using flags.
But it turned out to be pretty complicated at least with the aproach I tried to make it work.

the approach i posted demonstrates a fairly simple approach (50 lines) using a single flag to indicate on/off and using millis() rather than delay() so that a button press can be detected immediately and turn things off

[StefanL38]

@gcjr

if (Nled <= ++ idx)
                idx = 0;

as long as idx is less or equal than Nled set idx to zero???

[gcjr]

@StefanL38

as long as idx is less or equal than Nled set idx to zero???

no
after increment idx greater than or equal to Nled, reset idx to zero

[alto777]

@StefanL38

as long as

Well it's an if statement, so there's no "as long as" to it.

Sometimes seen as

   if (++ixd >= Nled)
    idx = 0;

also seen

   idx++;  // or ++idx, who cares
   if (ixd >= Nled)
      idx = 0;

a7