For loop not working as expected

Using Arduino Programming Questions
1

Im writing a program in which a 7 segment display shows a number that increases with a button press up to 5 and then back to 1. The number on the display determines the speed of a sequence of lights flashing which are triggered by another button. So far I am just working on the code for the 7 segment display but it is not working as expected. The commented section works fine by itself but the non commented section doesnt output anything at all. There are no error messages when compiled.

void loop()
{
//while(digitalRead(button2)==LOW){
//  one();
//}
//two();

int i;
for(int i=1; 1 <= i <= 5; i++)
  {
  switch(i)
    {
    case 1:
      while(digitalRead(button2)==LOW){
        one;
        }
        break;
    case 2:
      while(digitalRead(button2)==LOW){
        two;
        }
        break;    
    case 3:
      while(digitalRead(button2)==LOW){
        three;
        }
        break;
    case 4:
      while(digitalRead(button2)==LOW){
        four;
        }
        break;
    case 5:
      while(digitalRead(button2)==LOW){
        five;
        }
        break;
    }
  }
}

If you would like to see all of the functions and other code, the file is attached below. I am fairly new to coding (and using an arduino) so any help is greatly appreciated.

7_seg_display.ino.ino (3.95 KB)

2 
for(int i=1; 1 <= i <= 5; i++)

That is a valid C statement which is why the compiler didn't spit out an error message. But that doesn't mean that it is a sensible statement which does what you intend.
Try this:

for(int i=1; i <= 5; i++)

But it probably won't improve things much. What is this supposed to do?

        one;

There's no need to attach a sketch that is less than 9kB. Just post all of it in code tags.

Pete

3

It is supposed to go into the case statement so that pressing the button will cause the display to go up by increments of 1 up to 5 then back to 1

4

This program is supposed to display a number on a 7 segment display and increase by 1 till it hits 5 when a button is pressed, but when I press the button it displays a random number instead of increasing by 1. Im fairly new to coding so help is appreciated

int switchPin = 10;
int button2 = 9;
int ledPin = 13;
int ledPin2 = 12;
int ledPin3 = 11;
int a = 0;
int b = 1;
int c = 2;
int d = 3;
int e = 4;
int f = 5;
int g = 7;
int Decimalpoint = 8;

void setup()
{
  pinMode(switchPin, INPUT);
  pinMode(ledPin, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(ledPin3, OUTPUT);
  pinMode(a, OUTPUT);
  pinMode(b, OUTPUT);
  pinMode(c, OUTPUT);
  pinMode(d, OUTPUT);
  pinMode(e, OUTPUT);
  pinMode(f, OUTPUT);
  pinMode(g, OUTPUT);
  pinMode(Decimalpoint, OUTPUT);
  pinMode(button2, INPUT);
  
}

void one()
{
  digitalWrite(a,LOW);
  digitalWrite(b,LOW);
  digitalWrite(c,LOW);
  digitalWrite(d,HIGH);
  digitalWrite(e,LOW);
  digitalWrite(f,LOW);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);

}
void two()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,LOW);
  digitalWrite(c,HIGH);
  digitalWrite(d,HIGH);
  digitalWrite(e,HIGH);
  digitalWrite(f,HIGH);
  digitalWrite(g,LOW);
  digitalWrite(Decimalpoint,HIGH);
}
void three()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,LOW);
  digitalWrite(c,HIGH);
  digitalWrite(d,HIGH);
  digitalWrite(e,LOW);
  digitalWrite(f,HIGH);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);
}
void four()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,HIGH);
  digitalWrite(c,LOW);
  digitalWrite(d,HIGH);
  digitalWrite(e,LOW);
  digitalWrite(f,LOW);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);
}
void five()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,HIGH);
  digitalWrite(c,HIGH);
  digitalWrite(d,LOW);
  digitalWrite(e,LOW);
  digitalWrite(f,HIGH);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);
}

void loop()
{
for(int i = 1 ; i <= 5; i++)
  {
  switch(i)
    {
    case 1:
      while(digitalRead(button2)==LOW){
        one();
        }
        break;
    case 2:
      while(digitalRead(button2)==LOW){
        two();
        }
        break;    
    case 3:
      while(digitalRead(button2)==LOW){
        three();
        }
        break;
    case 4:
      while(digitalRead(button2)==LOW){
        four();
        }
        break;
    case 5:
      while(digitalRead(button2)==LOW){
        five();
        }
        break;
    }
  }
}
5 
      while(digitalRead(button2)==LOW){
        three();
        }

That will execute the three() function many thousands of times per second while the button is held down. Why do you want to do that?

6

@rfarrell961, please do not cross-post. Threads merged.

7

By the way, it's happening because your switch contacts bounce when they make and break. (in response to #3)

8

[quote author=Coding Badly link=msg=3537497 date=1514324618]
@rfarrell961, please do not cross-post. Threads merged.

[/quote] My other post was a different question I had after fixing some simple errors that significantly changed the program

9

...about the same project. Keeping the two threads together provides context and background. Which helps those trying to help you.

10

aarg:
By the way, it's happening because your switch contacts bounce when they make and break. (in response to #3)

How would I go about fixing this?

11

Ok so I now have figured out the 7 segment display. My next problem is that I need to make it so that if the display shows a certain number, pressing another button will cause 3 lights to flash at speeds that depend on the display. I think that I will need to use threading but I'm uncertain as to which approach to use. here is my code so far:

int switchPin = 10;
int button2 = 9;
int ledPin = 13;
int ledPin2 = 12;
int ledPin3 = 11;
int a = 0;
int b = 1;
int c = 2;
int d = 3;
int e = 4;
int f = 5;
int g = 7;
int Decimalpoint = 8;

void setup()
{
  pinMode(switchPin, INPUT);
  pinMode(ledPin, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(ledPin3, OUTPUT);
  pinMode(a, OUTPUT);
  pinMode(b, OUTPUT);
  pinMode(c, OUTPUT);
  pinMode(d, OUTPUT);
  pinMode(e, OUTPUT);
  pinMode(f, OUTPUT);
  pinMode(g, OUTPUT);
  pinMode(Decimalpoint, OUTPUT);
  pinMode(button2, INPUT);
  
}

void one()
{
  digitalWrite(a,LOW);
  digitalWrite(b,LOW);
  digitalWrite(c,LOW);
  digitalWrite(d,HIGH);
  digitalWrite(e,LOW);
  digitalWrite(f,LOW);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);

}
void two()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,LOW);
  digitalWrite(c,HIGH);
  digitalWrite(d,HIGH);
  digitalWrite(e,HIGH);
  digitalWrite(f,HIGH);
  digitalWrite(g,LOW);
  digitalWrite(Decimalpoint,HIGH);
}
void three()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,LOW);
  digitalWrite(c,HIGH);
  digitalWrite(d,HIGH);
  digitalWrite(e,LOW);
  digitalWrite(f,HIGH);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);
}
void four()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,HIGH);
  digitalWrite(c,LOW);
  digitalWrite(d,HIGH);
  digitalWrite(e,LOW);
  digitalWrite(f,LOW);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);
}
void five()
{
  digitalWrite(a,HIGH);
  digitalWrite(b,HIGH);
  digitalWrite(c,HIGH);
  digitalWrite(d,LOW);
  digitalWrite(e,LOW);
  digitalWrite(f,HIGH);
  digitalWrite(g,HIGH);
  digitalWrite(Decimalpoint,HIGH);
}

void speed1()
{
  if (digitalRead(switchPin) == HIGH)
  {
    digitalWrite(ledPin, HIGH);
    delay(100);
    digitalWrite(ledPin, LOW);
    delay(100);
    digitalWrite(ledPin2, HIGH);
    delay(100);
    digitalWrite(ledPin2, LOW);
    delay(100);
    digitalWrite(ledPin3, HIGH);
    delay(100); 
    digitalWrite(ledPin3, LOW);  
    delay(100); 
    }   
}
void speed2()
{
  if (digitalRead(switchPin) == HIGH)
  {
    digitalWrite(ledPin, HIGH);
    delay(200);
    digitalWrite(ledPin, LOW);
    delay(200);
    digitalWrite(ledPin2, HIGH);
    delay(200);
    digitalWrite(ledPin2, LOW);
    delay(200);
    digitalWrite(ledPin3, HIGH);
    delay(200); 
    digitalWrite(ledPin3, LOW);  
    delay(200); 
    }  
}
void speed3()
{
  if (digitalRead(switchPin) == HIGH)
  {
    digitalWrite(ledPin, HIGH);
    delay(300);
    digitalWrite(ledPin, LOW);
    delay(300);
    digitalWrite(ledPin2, HIGH);
    delay(300);
    digitalWrite(ledPin2, LOW);
    delay(300);
    digitalWrite(ledPin3, HIGH);
    delay(300); 
    digitalWrite(ledPin3, LOW);  
    delay(300); 
    }   
}
void speed4()
{
  if (digitalRead(switchPin) == HIGH)
  {
    digitalWrite(ledPin, HIGH);
    delay(400);
    digitalWrite(ledPin, LOW);
    delay(400);
    digitalWrite(ledPin2, HIGH);
    delay(400);
    digitalWrite(ledPin2, LOW);
    delay(400);
    digitalWrite(ledPin3, HIGH);
    delay(400); 
    digitalWrite(ledPin3, LOW);  
    delay(400); 
    }  
}
void speed5()
{
  if (digitalRead(switchPin) == HIGH)
  {
    digitalWrite(ledPin, HIGH);
    delay(500);
    digitalWrite(ledPin, LOW);
    delay(500);
    digitalWrite(ledPin2, HIGH);
    delay(500);
    digitalWrite(ledPin2, LOW);
    delay(500);
    digitalWrite(ledPin3, HIGH);
    delay(500); 
    digitalWrite(ledPin3, LOW);  
    delay(500); 
    }  
}
void loop()
{
for(int i=1; 1 <= i <= 5; i++)
  {
  switch(i)
    {
    case 1:
      while(digitalRead(button2)==LOW){
        one();
        }
        delay(100);
        if (digitalRead(button2)==LOW) {
          break; 
        }
    case 2:
      while(digitalRead(button2)==LOW){
        two();
        }
        delay(100);
        if (digitalRead(button2)==LOW) {
          break; 
        }   
    case 3:
      while(digitalRead(button2)==LOW){
        three();
        }
        delay(100);
        if (digitalRead(button2)==LOW) {
          break; 
        }
    case 4:
      while(digitalRead(button2)==LOW){
        four();
        }
        delay(100);
        if (digitalRead(button2)==LOW) {
          break; 
        }
    case 5:
      while(digitalRead(button2)==LOW){
        five();
        }
        delay(100);
        if (digitalRead(button2)==LOW) {
          break; 
        }
    }
  }
}
12

rfarrell961:
I think that I will need to use threading

No. There is an obvious solution that would use threading but it would be inefficient and the wrong choice. Instead of creating two blocking threads, create one non-blocking thread that does both things. Non-blocking means no while loops or for loops waiting for something to complete and absolutely no use of delay.

Before you start going into that you have some things to learn first. Go do a basic C++ tutorial on functions. Writing out a separate function for each possibly value of the delay is not something sustainable. That should be one function that takes an argument that tells it what number to use in the delay.

void speed4()
{
  if (digitalRead(switchPin) == HIGH)
  {
    digitalWrite(ledPin, HIGH);
    delay(400);
    digitalWrite(ledPin, LOW);
    delay(400);
    digitalWrite(ledPin2, HIGH);
    delay(400);
    digitalWrite(ledPin2, LOW);
    delay(400);
    digitalWrite(ledPin3, HIGH);
    delay(400); 
    digitalWrite(ledPin3, LOW);  
    delay(400); 
    }  
}

All those functions could all be one function.

void speed(int delayTime)
{
  if (digitalRead(switchPin) == HIGH)
  {
    digitalWrite(ledPin, HIGH);
    delay(delayTime);
    digitalWrite(ledPin, LOW);
    delay(delayTime);
    digitalWrite(ledPin2, HIGH);
    delay(delayTime);
    digitalWrite(ledPin2, LOW);
    delay(delayTime);
    digitalWrite(ledPin3, HIGH);
    delay(delayTime); 
    digitalWrite(ledPin3, LOW);  
    delay(delayTime); 
    }  
}

But that is purely academic. To get what you are really after even that function won't work because it blocks with delay for a long time waiting to finish all those blinks. You need to do some reading on writing non-blocking code to make this actually work.