Full code of program doesnt work corectly.

Hello, im beginner programmer but i have to do , a little project.
This project have 6leds and 2 buttons…
First button make first sequence.
Second button make second sequence…
I write this code and he going to compile and flash to my arduino… But problem is when i try run (click any button) program… Doesnt work corectly… I dont know what’s happend with this… somebody can looks at my code and tell me any suggestion and resolve problem with me?

This is code;

//Define all leds on which pins
#define leda 2
#define ledb 3
#define ledc 4
#define ledd 5
#define lede 6
#define ledf 7
#define buttona 12
#define buttonb 14

int ledPins = leda + ledb + ledc + ledd + lede + ledf;
int delayTime[] = {1000, 20000, 3000, 7000, 3000, 52000, 5000, 5000, 4000, 19000, 2000};
int pinCount = 6;

char seq1[] = {ledb, ledb, ledd, lede, lede, ledd, ledc, ledf, ledf ,ledc, leda};
char output1[] = {HIGH, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, LOW};
int time1[] =  {2000,20000,3000,7000,3000,52000,5000,5000,4000,19000,2000}; //11 +START
char seq2[] = {ledc, ledc, ledd, ledd, ledc, lede, lede, ledc, ledd, lede, lede, ledd, ledc, ledf, ledf, ledc, ledb};
char output2[] = {HIGH, LOW, HIGH, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, LOW};
int time2[] = {1000,9000,2000,8000,3000,7000,2000,18000,2000,2000,2000,29000,5000,5000,2000,19000,2000}; //18 + START
int buttons = buttona + buttonb;

byte buttonState;
byte lastState = LOW;
byte count = 0;

int program_running = 0; // 0 - no program running, 1 - first program running (firts button pressed), 2 - ....
int start_milis = 0;
int seq_step = 0;
int interval = millis() - start_milis;

void setup() {
  
  //Define all led and set all on OFF
pinMode(ledPins, OUTPUT);
  //Safe implementation
 
pinMode(buttona,INPUT);
pinMode(buttonb,INPUT);

}
 
void loop() {

  if( program_running == 1 ) {
    int interval = millis() - start_milis;
      sequence();
    
  } 
  if( program_running == 2) {
    int interval = millis() - start_milis;
      sequence2();
    
  }


  //If pushbuttonA == sequence();
    if(digitalRead(buttona) == LOW)
    {
      program_running = 1;
      start_milis = millis();
          count = 1;
          sequence();
          ButtonCounter();
    }
  //If pushbuttonB == sequence();
      if(digitalRead(buttonb) == LOW)
      {
        program_running = 2;
        start_milis = millis();
          count = 1;        
          sequence2();
          ButtonCounter();
      }

}

void sequence(){
  //First sequence:
      digitalWrite(leda,HIGH);
      if(interval > 2000)
      {
        interval = 0;
        for(int forCount = 0; forCount < 12; forCount++)
        {
         
          if( interval > time1[seq_step])
          {
            digitalWrite(seq1[forCount],output1[forCount]);
            seq_step++;
            interval = 0;
          }
        };
      }
        
    //Complete sequence:
    digitalWrite(ledPins,HIGH);
  

   do
    {
      digitalWrite(ledPins,HIGH);
    }while(ledPins == HIGH); loop();

          
  //If push button twice time go to stoploop();
    // if(pushbutton == down && sequence == run())
}

void sequence2(){
        //Second sequence
      digitalWrite(ledb,HIGH);
      if(interval > 2000)
      {
        interval = 0;
        for(int forCountB = 0; forCountB < 12; forCountB++)
        {
         
          if( interval > time1[seq_step])
          {
            digitalWrite(seq2[forCountB],output2[forCountB]);
            seq_step++;
            interval = 0;
          }
        };
      }
        
   //Complete sequence:
  digitalWrite(ledPins,HIGH);

   do
    {
      digitalWrite(ledPins,HIGH);
    }while(ledPins == HIGH); loop();

  
  //If push button twice time go to stoploop();
   // if(pushbutton == down && sequence == run())
}
void stoploop(){

  //Stop sequence

  //Choose button and run sequence first or second
    //If pushbuttonA == sequence();
    if(digitalRead(buttona) == LOW)
    {
          count = 1;
          sequence();
          ButtonCounter();
    }
  //If pushbuttonB == sequence();
      if(digitalRead(buttonb) == LOW)
      {
          count = 1;
          sequence2();
          ButtonCounter();
      }

}

void ButtonCounter(){

  buttonState = digitalRead(buttons);
  if(buttonState == buttonState != lastState )
  {
    count++;
   if(count >= 2)
   do
   {
    //STOP sequence
    loop();
   } while(count >= 2);
  }


}

Please explain exactly what you mean by "Doesnt work corectly".

Do you have external pull up resistors on the button pins?

#define leda 2
#define ledb 3
#define ledc 4
#define ledd 5
#define lede 6
#define ledf 7
#define buttona 12
#define buttonb 14

int ledPins = leda + ledb + ledc + ledd + lede + ledf;

...

//Define all led and set all on OFF
pinMode(ledPins, OUTPUT);

What on earth makes you believe this will do anything remotely useful?? Exactly what pin do you think that pinMode call is going to configure??

Regards,
Ray L.

int ledPins = leda + ledb + ledc + ledd + lede + ledf;

?

I stopped reading after that.

I have resistors 10kOmh on buttons and 4.7kOmh on each leds.

@RayLivingston I mean all pins set on OUTPUT.

@UKHeliBob I said "I'm begginer" I mean it's good. in 100% dont trust compiler..

firtus:
I have resistors 10kOmh on buttons and 4.7kOmh on each leds.

@RayLivingston I mean all pins set on OUTPUT.

@UKHeliBob I said "I'm begginer" I mean it's good. in 100% dont trust compiler..

Being a beginner is no excuse for not taking a little time to learn, rather than making things up. Programming requires care and precision, and understanding the functions you're using. The first argument to pinMode HAS to be a SINGLE pin number and not a whole pile of pin numbers added together. If you asked me for the time of day, would you find it useful if my response was to add the hour and minutes and seconds all together, and give you a single number, instead of the hours, minutes and seconds? If you pass pinMode 7 as the pin number, how on earth is pinMode supposed to know you mean BOTH pin 3 and pin 4, rather than pin 7? The compiler cannot read your mind.

Regards,
Ray L.

@UKHeliBob I said "I'm begginer" I mean it's good. in 100% dont trust compiler..

I would trust the compiler to do what I told it if it could.

So after

int ledPins = leda + ledb + ledc + ledd + lede + ledf;

ledPins will have a value of the sum of the led pin numbers. This perfectly valid C but it is not what you want.

You have arrays in your code so why not an array of LED pins ?

I corected my code, now looks better ?

//Define all leds on which pins
#define leda 2
#define ledb 3
#define ledc 4
#define ledd 5
#define lede 6
#define ledf 7
#define buttona 12
#define buttonb 14

int delayTime[] = {1000, 20000, 3000, 7000, 3000, 52000, 5000, 5000, 4000, 19000, 2000};
int pinCount = 6;

char seq1[] = {ledb, ledb, ledd, lede, lede, ledd, ledc, ledf, ledf ,ledc, leda};
char output1[] = {HIGH, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, LOW};
int time1[] =  {2000,20000,3000,7000,3000,52000,5000,5000,4000,19000,2000}; //11 +START
char seq2[] = {ledc, ledc, ledd, ledd, ledc, lede, lede, ledc, ledd, lede, lede, ledd, ledc, ledf, ledf, ledc, ledb};
char output2[] = {HIGH, LOW, HIGH, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, LOW};
int time2[] = {1000,9000,2000,8000,3000,7000,2000,18000,2000,2000,2000,29000,5000,5000,2000,19000,2000}; //18 + START


byte buttonState;
byte lastState = LOW;
byte count = 0;

int program_running = 0; // 0 - no program running, 1 - first program running (firts button pressed), 2 - ....
int start_milis = 0;
int seq_step = 0;
int interval = millis() - start_milis;

void setup() {
  
  //Define all led and set all on OFF
pinMode(leda, OUTPUT);
pinMode(ledb, OUTPUT);
pinMode(ledc, OUTPUT);
pinMode(ledd, OUTPUT);
pinMode(lede, OUTPUT);
pinMode(ledf, OUTPUT);
 
pinMode(buttona,INPUT);
pinMode(buttonb,INPUT);

}
 
void loop() {

  if( program_running == 1 ) {
    int interval = millis() - start_milis;
      sequence();
    
  } 
  if( program_running == 2) {
    int interval = millis() - start_milis;
      sequence2();
    
  }


  //If pushbuttonA == sequence();
    if(digitalRead(buttona) == LOW)
    {
      program_running = 1;
      start_milis = millis();
          count = 1;
          sequence();
          ButtonCounter();
    }
  //If pushbuttonB == sequence();
      if(digitalRead(buttonb) == LOW)
      {
        program_running = 2;
        start_milis = millis();
          count = 1;        
          sequence2();
          ButtonCounter();
      }

}

void sequence(){
  //First sequence:
      digitalWrite(leda,HIGH);
      if(interval > 2000)
      {
        interval = 0;
        for(int forCount = 0; forCount < 12; forCount++)
        {
         
          if( interval > time1[seq_step])
          {
            digitalWrite(seq1[forCount],output1[forCount]);
            seq_step++;
            interval = 0;
          }
        };
      }
        
    
  //If push button twice time go to stoploop();
    // if(pushbutton == down && sequence == run())
}

void sequence2(){
        //Second sequence
      digitalWrite(ledb,HIGH);
      if(interval > 2000)
      {
        interval = 0;
        for(int forCountB = 0; forCountB < 12; forCountB++)
        {
         
          if( interval > time1[seq_step])
          {
            digitalWrite(seq2[forCountB],output2[forCountB]);
            seq_step++;
            interval = 0;
          }
        };
      }
        
  //If push button twice time go to stoploop();
   // if(pushbutton == down && sequence == run())
}
void stoploop(){

  //Stop sequence

  //Choose button and run sequence first or second
    //If pushbuttonA == sequence();
    if(digitalRead(buttona) == LOW)
    {
          count = 1;
          sequence();
          ButtonCounter();
    }
  //If pushbuttonB == sequence();
      if(digitalRead(buttonb) == LOW)
      {
          count = 1;
          sequence2();
          ButtonCounter();
      }

}

void ButtonCounter(){

  buttonState = digitalRead(buttona) + digitalRead(buttonb);

  if(buttonState == buttonState != lastState )
  {
    count++;
   if(count >= 2)
   do
   {
    //STOP sequence
    loop();
   } while(count >= 2);
  }


}

Declare all of your timing variables as unsigned long because that is what millis() returns.

You declare interval several times in the program, each with different scope. As a result, when you use the variable it may not be the one that you intended. I suggest that you declare it globally and don't declare it again.

U mean like this unsigned long time1 = {2000,20000,3000,7000,3000,52000,5000,5000,4000,19000,2000}; ?
And variable of interval was correct.

U mean like this unsigned long time1 = {2000,20000,3000,7000,3000,52000,5000,5000,4000,19000,2000}; ?

Yes, and any other variables associated with timing using millis()

And variable of interval was correct.

I would declare interval unsigned long too to prevent potential problems, but only declare it once.

I have only to arrays with time, so only two changes...
And about interval, i declare like global variable..

firtus:
I have only to arrays with time, so only two changes...
And about interval, i declare like global variable..

What about start_milis ?

Post the code as it is now and describe the current problems.

Corrected code :

And now it’s the same as before, exactly pushing button A or B each sequence dont running. I dont see anything strange, what is a problem with this…

//Define all leds on which pins
#define leda 2
#define ledb 3
#define ledc 4
#define ledd 5
#define lede 6
#define ledf 7
#define buttona 12
#define buttonb 14

int pinCount = 6;

char seq1[] = {ledb, ledb, ledd, lede, lede, ledd, ledc, ledf, ledf ,ledc, leda};
char output1[] = {HIGH, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, LOW};
unsigned long time1[] =  {2000,20000,3000,7000,3000,52000,5000,5000,4000,19000,2000}; //11 +START
char seq2[] = {ledc, ledc, ledd, ledd, ledc, lede, lede, ledc, ledd, lede, lede, ledd, ledc, ledf, ledf, ledc, ledb};
char output2[] = {HIGH, LOW, HIGH, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, HIGH, HIGH, LOW, LOW, LOW};
unsigned long time2[] = {1000,9000,2000,8000,3000,7000,2000,18000,2000,2000,2000,29000,5000,5000,2000,19000,2000}; //18 + START


byte buttonState;
byte lastState = LOW;
byte count = 0;

int program_running = 0; // 0 - no program running, 1 - first program running (firts button pressed), 2 - ....
int start_milis = 0;
int seq_step = 0;
int interval = millis() - start_milis;

void setup() {
  
  //Define all led and set all on OFF
pinMode(leda, OUTPUT);
pinMode(ledb, OUTPUT);
pinMode(ledc, OUTPUT);
pinMode(ledd, OUTPUT);
pinMode(lede, OUTPUT);
pinMode(ledf, OUTPUT);
 
pinMode(buttona,INPUT);
pinMode(buttonb,INPUT);

}
 
void loop() {

  if( program_running == 1 ) {
    interval = millis() - start_milis;
      sequence();
    
  } 
  if( program_running == 2) {
    interval = millis() - start_milis;
      sequence2();
    
  }
 
  
  //If pushbuttonA == sequence();
    if(digitalRead(buttona) == LOW)
    {
      program_running = 1;
      start_milis = millis();
          count = 1;
          sequence();
          ButtonCounter();
    }
  //If pushbuttonB == sequence();
      if(digitalRead(buttonb) == LOW)
      {
        program_running = 2;
        start_milis = millis();
          count = 1;        
          sequence2();
          ButtonCounter();
      }

}

void sequence(){
  //First sequence:
      digitalWrite(leda,HIGH);
      if(interval > 2000)
      {
        interval = 0;
        for(int forCount = 0; forCount < 12; forCount++)
        {
         
          if( interval > time1[seq_step])
          {
            digitalWrite(seq1[forCount],output1[forCount]);
            seq_step++;
            interval = 0;
          }
        };
      }
        
    
  //If push button twice time go to stoploop();
    // if(pushbutton == down && sequence == run())
}

void sequence2(){
        //Second sequence
      digitalWrite(ledb,HIGH);
      if(interval > 2000)
      {
        interval = 0;
        for(int forCountB = 0; forCountB < 12; forCountB++)
        {
         
          if( interval > time1[seq_step])
          {
            digitalWrite(seq2[forCountB],output2[forCountB]);
            seq_step++;
            interval = 0;
          }
        };
      }
        
  //If push button twice time go to stoploop();
   // if(pushbutton == down && sequence == run())
}
void stoploop(){

  //Stop sequence

  //Choose button and run sequence first or second
    //If pushbuttonA == sequence();
    if(digitalRead(buttona) == LOW)
    {
          count = 1;
          sequence();
          ButtonCounter();
    }
  //If pushbuttonB == sequence();
      if(digitalRead(buttonb) == LOW)
      {
          count = 1;
          sequence2();
          ButtonCounter();
      }

}

void ButtonCounter(){

  buttonState = digitalRead(buttona) + digitalRead(buttonb);

  if(buttonState == buttonState != lastState )
  {
    count++;
   if(count >= 2)
   do
   {
    //STOP sequence
    loop();
   } while(count >= 2);
  }


}

Start serial communication and try to Serial.print() values of your variables and watch whats going wrong.
For example:

  if(buttonState == buttonState != lastState )
  {
    count++;
    if(count >= 2)
      do
    {
      //STOP sequence
      loop();
    } 
    while(count >= 2);
  }

Im sure this doesnt do what you think it does. Also, you don use your stoploop() anywhere - perhaps thats what you wanted to be in place of loop call?

Whatever... Now i try use other way to start working program, i change the number of pins and they start lighting... What else... Probably void loop() doesnt work exactly, i put simple code to loop and dont work.. and try to put in setup and work... Currently i make this project with old programmer and he got shock and he didnt know what happend with this.. I will send later more information about progress and probably i will need ur help. But currently thanks :slight_smile: