Condensing code

Hello folks,

I am working on a project, I have it all wired up and got the flash pattern the way I want it. However, the code is really long and bloated. I was trying to find a way to use psudothread, But I just can't seem to wrap my head around it, and spent a few days trying to figure it out. I went on ahead and wrote this code, but it seems really bloated and in efficient. Can anyone help me with this? Thank you in advance.

 const int leda1 = 2;
const int leda2 = 3;
const int leda3 = 4;
const int leda4 = 5;
const int ledC = 6;
const int ledK = 7;
const int ledc = 8;
const int ledh = 9;
const int ledi = 10;
const int lede = 11;
const int ledf = 12;
const int leds = 13;

int timer = 50;


void setup () {
  pinMode(ledc, OUTPUT);
  pinMode(ledh, OUTPUT);
  pinMode(ledi, OUTPUT);
  pinMode(lede, OUTPUT);
  pinMode(ledf, OUTPUT);
  pinMode(leds, OUTPUT);
  pinMode(ledK, OUTPUT);
  pinMode(ledC, OUTPUT);
  pinMode(leda1, OUTPUT);
  pinMode(leda2, OUTPUT);
  pinMode(leda3, OUTPUT);
  pinMode(leda4, OUTPUT);
}

void loop() {
  
   
   digitalWrite(leda1, HIGH);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
   
   
    digitalWrite(ledc, HIGH);
      digitalWrite(ledK, HIGH);
   
delay(timer);//50

   digitalWrite(leda1, LOW);
   digitalWrite(leda2, HIGH);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
delay(timer);//100
   
   
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, HIGH);
   digitalWrite(leda4, LOW);
   
     
delay(timer);//150
   
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, HIGH);
   
   digitalWrite(ledc, LOW);
     digitalWrite(ledh, HIGH);
delay(timer);//200
   
   
   digitalWrite(leda1, HIGH);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
   
        
delay(timer);//250
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, HIGH);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
    
    
    

delay(timer);//300 
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, HIGH);
   digitalWrite(leda4, LOW);

    digitalWrite(ledh, LOW);
     digitalWrite(ledi, HIGH);
    
delay(timer);//350


   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, HIGH);

delay(timer);//400
   digitalWrite(leda1, HIGH);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);


delay(timer);//450
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, HIGH);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
   
     digitalWrite(ledi, LOW);
     digitalWrite(lede, HIGH);

delay(timer);//500
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, HIGH);
   digitalWrite(leda4, LOW);
   
   digitalWrite(ledK, LOW);
     digitalWrite(ledC, HIGH);
        
delay(timer);//550
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, HIGH);
   
   
delay(timer);//600
   
   digitalWrite(leda1, HIGH);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
   
   digitalWrite(lede, LOW);
     digitalWrite(ledf, HIGH);
    

delay(timer);//650

   digitalWrite(leda1, LOW);
   digitalWrite(leda2, HIGH);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
   
delay(timer);//700   
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, HIGH);
   digitalWrite(leda4, LOW);

delay(timer);//750   
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, HIGH);

delay(timer);//800   
   digitalWrite(leda1, HIGH);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
    
    digitalWrite(ledf, LOW);
     digitalWrite(leds, HIGH);

delay(timer);//850   
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, HIGH);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);

delay(timer);//900
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, HIGH);
   digitalWrite(leda4, LOW);
   
delay(timer);//950
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, HIGH);
   
     digitalWrite(ledC, LOW);
   digitalWrite(leds, LOW);
delay(timer);//1000 
}

A couple of arrays would reduce the code-bloat.

Have a look at the Blink without delay example to get the concept of not using delay and then at my Multiblink sketch in the repository, link below, for data driven programs. Not beginners code but you can learn from it.

Turning this code into a function would considerably reduce the number of lines of code

   digitalWrite(leda1, HIGH);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);

As there is a pattern as to which LEDs are on/off you would not need to use any parameters, merely move to the next state.

AWOL:
A couple of arrays would reduce the code-bloat.

When I originally started writing the code for this, I had 3 arrays.
pinArray1 = {8, 9, 10, 11, 12, 13,};
pinArray2 = {7, 6,};
pinArray3 = {2, 3, 4, 5,};
But the problem that I was having with them, was I needed pinArray1 to cycle through with a 700ms delay (turn on 8, wait 700ms, turn off 8 and turn on 9, wait 700ms, turn off 9 turn on 10 And so on).

And i needed pinArray2 to cycle through at 1000ms.
And pinArray 3 to cycle through at 70ms.

I could not figure out how to make them cycle through at the needed speeds, And not have them interfere with the timing of one another.
here is my original code before I scraped it and wrote the one I originally posted.
I know there is case and switch statements in this code, I was going to have it do more patterns if I could have gotten it to work.

 int pattern = 0;
const int leda1 = 2;
const int leda2 = 3;
const int leda3 = 4;
const int leda4 = 5;
const int ledC = 6;
const int ledK = 7;
const int ledc = 8;
const int ledh = 9;
const int ledi = 10;
const int lede = 11;
const int ledf = 12;
const int leds = 13;
int pinArray1[] = {8, 9, 10, 11, 12, 13,};
int pinArray2[] = {7, 6, 7, 6, 7, 6,};
int pinArray3[] = {2, 3, 4, 5,};
int count = 0;
int count1 = 0;
int count2 = 0;
int timer1 = 700;
int timer2 = 150;
int timer3 = 70;
void setup () {
  pinMode(ledc, OUTPUT);
  pinMode(ledh, OUTPUT);
  pinMode(ledi, OUTPUT);
  pinMode(lede, OUTPUT);
  pinMode(ledf, OUTPUT);
  pinMode(leds, OUTPUT);
  pinMode(ledK, OUTPUT);
  pinMode(ledC, OUTPUT);
  pinMode(leda1, OUTPUT);
  pinMode(leda2, OUTPUT);
  pinMode(leda3, OUTPUT);
  pinMode(leda4, OUTPUT);
}
  void loop() { 
  int pattern = random(3);
  for (int nextRandom = 1; nextRandom <=10; nextRandom++)  //a dealy that has each case run 10 times before switching.
  switch (pattern){
  case 0:

   digitalWrite(ledc, LOW);
   digitalWrite(ledh, LOW);
   digitalWrite(ledi, LOW);
   digitalWrite(lede, LOW);
   digitalWrite(ledf, LOW);
   digitalWrite(leds, LOW);
   digitalWrite(ledK, LOW);
   digitalWrite(ledC, LOW);
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);
      
  for (count=0;count<6;count++) {

   digitalWrite(pinArray1[count], LOW);
   digitalWrite(pinArray1[count], HIGH);
   delay(timer1);
   digitalWrite(pinArray1[count], HIGH);
   
  for (count1=0;count1<2;count1++){
   digitalWrite(pinArray2[count1], LOW);
   digitalWrite(pinArray2[count1], HIGH);
   delay(timer2);
   digitalWrite(pinArray2[count1], LOW);
   
  for (count2=0;count2<4;count2++){
    digitalWrite(pinArray3[count2], LOW);
   digitalWrite(pinArray3[count2], HIGH);
   delay(timer3);
   digitalWrite(pinArray3[count2], LOW);
  } 
  }
   
    }
    
  break;
  }
  }

If you've got a problem of different rates, you can either figure out what is the lowest-common denominator of your delays, and work in units of that, or you can get rid of delays altogether, by looking at the blink without delay example for clues.

AWOL: If you've got a problem of different rates, you can either figure out what is the lowest-common denominator of your delays, and work in units of that, or you can get rid of delays altogether, by looking at the blink without delay example for clues.

I have looked at and played around with the "blink without delay" example, And I guess there is something that I am just not wrapping my head around with it. I have tried to change the example to do use LED's at different intervals, and was going to expand from that, But when I tried what looked correct, it just hung. Not real sure what I am doing wrong.

Not real sure what I am doing wrong

Without seeing your code, neither are we.

AWOL:

Not real sure what I am doing wrong

Without seeing your code, neither are we.

Here is the "blink without delay" code that I had tried. It is not hanging now, However, It is now completely ignoring pin12.

 /* Blink without Delay

 Turns on and off a light emitting diode(LED) connected to a digital  
 pin, without using the delay() function.  This means that other code
 can run at the same time without being interrupted by the LED code.

 The circuit:
 * LED attached from pin 13 to ground.
 * Note: on most Arduinos, there is already an LED on the board
 that's attached to pin 13, so no hardware is needed for this example.


 created 2005
 by David A. Mellis
 modified 8 Feb 2010
 by Paul Stoffregen

 This example code is in the public domain.


 http://www.arduino.cc/en/Tutorial/BlinkWithoutDelay
 */

// constants won't change. Used here to 
// set pin numbers:
const int ledPin =  13;      // the number of the LED pin
const int ledPin2 = 12;
// Variables will change:
int ledState = LOW;             // ledState used to set the LED
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 1000;           // interval at which to blink (milliseconds)
long interval2 = 500;
void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
  pinMode(ledPin2, OUTPUT);  
}

void loop()
{
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  unsigned long currentMillis = millis();

  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  
  if(currentMillis - previousMillis > interval2) {
     previousMillis = currentMillis;   
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    digitalWrite(ledPin2, ledState);
  }
 }
}

You need a separate "previous" for each LED.

Edit: And a separate "ledState"

AWOL: You need a separate "previous" for each LED.

Edit: And a separate "ledState"

I added a second previousMillis (previousMillis2) and pointed to it, And now it hangs with 13 stuck on HIGH.

 /* Blink without Delay

 Turns on and off a light emitting diode(LED) connected to a digital  
 pin, without using the delay() function.  This means that other code
 can run at the same time without being interrupted by the LED code.

 The circuit:
 * LED attached from pin 13 to ground.
 * Note: on most Arduinos, there is already an LED on the board
 that's attached to pin 13, so no hardware is needed for this example.


 created 2005
 by David A. Mellis
 modified 8 Feb 2010
 by Paul Stoffregen

 This example code is in the public domain.


 http://www.arduino.cc/en/Tutorial/BlinkWithoutDelay
 */

// constants won't change. Used here to 
// set pin numbers:
const int ledPin =  13;      // the number of the LED pin
const int ledPin2 = 12;
// Variables will change:
int ledState = LOW;             // ledState used to set the LED
long previousMillis = 0;        // will store last time LED was updated
long previousMillis2 = 0;
// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 1000;           // interval at which to blink (milliseconds)
long interval2 = 500;
void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
  pinMode(ledPin2, OUTPUT);  
}

void loop()
{
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  unsigned long currentMillis = millis();

  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  
  if(currentMillis - previousMillis2 > interval2) {
    // save the last time you blinked the LED 
    previousMillis2 = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin2, ledState);
  }
}
}

[u]And a separate "ledState[/u]"

Although similar to rearranging the deck furniture on the Titanic, you can simplify:

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

to

   ledState = !ledState;

I would also change it from an int to a boolean.

Or, skip it altogether: digitalWrite(ledPin2, !digitalRead (ledPin2));

AWOL:

[u]And a separate "ledState[/u]"

Awesome, Thank you AWOL. I now have the "blink without delay" running 2 LED's at different intervals. Now to see if I can get it to advance through an array.

OK, Not sure what I have wrong here. I have tried to impliment arrays with this, and I am getting compile errors, and not real sure where the problem is. I really do appreciate the help that has been given so far.

Here is the code that I have added the arrays to.

 /* Blink without Delay
 
 Turns on and off a light emitting diode(LED) connected to a digital  
 pin, without using the delay() function.  This means that other code
 can run at the same time without being interrupted by the LED code.
 
 The circuit:
 * LED attached from pin 13 to ground.
 * Note: on most Arduinos, there is already an LED on the board
 that's attached to pin 13, so no hardware is needed for this example.
 
 
 created 2005
 by David A. Mellis
 modified 8 Feb 2010
 by Paul Stoffregen
 
 This example code is in the public domain.

 
 http://www.arduino.cc/en/Tutorial/BlinkWithoutDelay
 */

// constants won't change. Used here to 
// set pin numbers:
int pattern = 0;
const int leda1 = 2;
const int leda2 = 3;
const int leda3 = 4;
const int leda4 = 5;
const int ledC = 6;
const int ledK = 7;
const int ledc = 8;
const int ledh = 9;
const int ledi = 10;
const int lede = 11;
const int ledf = 12;
const int leds = 13;
int pinArray1[] = {8, 9, 10, 11, 12, 13,};
int pinArray2[] = {7, 6,};
int pinArray3[] = {2, 3, 4, 5,};
int count = 0;
int count1 = 0;
int count2 = 0;
                 // the number of the LED pin

// Variables will change:
long previousMillis = 0;        // will store last time LED was updated
long previousMillis2 = 0;
// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 1000;           // interval at which to blink (milliseconds)
long interval2 = 300;
void setup() {
  // set the digital pins as output:
  pinMode(ledc, OUTPUT);
  pinMode(ledh, OUTPUT);
  pinMode(ledi, OUTPUT);
  pinMode(lede, OUTPUT);
  pinMode(ledf, OUTPUT);
  pinMode(leds, OUTPUT);
  pinMode(ledK, OUTPUT);
  pinMode(ledC, OUTPUT);
  pinMode(leda1, OUTPUT);
  pinMode(leda2, OUTPUT);
  pinMode(leda3, OUTPUT);
  pinMode(leda4, OUTPUT);
}


void loop()
{
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  unsigned long currentMillis = millis();
  unsigned long currentMillis2 = millis();
   digitalWrite(ledc, LOW);
   digitalWrite(ledh, LOW);
   digitalWrite(ledi, LOW);
   digitalWrite(lede, LOW);
   digitalWrite(ledf, LOW);
   digitalWrite(leds, LOW);
   digitalWrite(ledK, LOW);
   digitalWrite(ledC, LOW);
   digitalWrite(leda1, LOW);
   digitalWrite(leda2, LOW);
   digitalWrite(leda3, LOW);
   digitalWrite(leda4, LOW);

      

  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
   if (count = 0; count<6;count++); 
     if (pinArray1 = LOW)
       pinArray1 = HIGH;
     else pinArray1 = LOW;  

    
 
  
  }
}

and here are the errors Im getting

sketch_aug12a.ino: In function ‘void loop()’:
sketch_aug12a:102: error: expected )' before ';' token sketch_aug12a:102: error: expected ;’ before ‘)’ token
sketch_aug12a:103: error: incompatible types in assignment of ‘int’ to ‘int [6]’
sketch_aug12a:104: error: incompatible types in assignment of ‘int’ to ‘int [6]’
sketch_aug12a:105: error: incompatible types in assignment of ‘int’ to ‘int [6]’

unsigned long currentMillis = millis();
  unsigned long currentMillis2 = millis();

The time returned is unlikely to have changed between those two calls - you only really need one “current”.

if (count = 0; count<6;count++);

Lose the semicolon.
In fact, lose the “for” loop / “if” mashup.

 if (pinArray1 = LOW)

That’s not how you assign to an array element.
It isn’t how you compare an array element.
It looks to me like you’re confusing a pin number with the state of that pin.

AWOL:

if (count = 0; count<6;count++);

Lose the semicolon.
In fact, lose the “for” loop / “if” mashup.

 if (pinArray1 = LOW)

That’s not how you assign to an array element.
It isn’t how you compare an array element.
It looks to me like you’re confusing a pin number with the state of that pin.

OK, Now I’m really confused. I did have

for (count=0;count<6;count++) {
   digitalWrite(pinArray1[count], LOW);
   digitalWrite(pinArray1[count], HIGH);
   delay(timer1);
   digitalWrite(pinArray1[count], HIGH);

But that has a delay in it, and that is what I am trying to get away from correct?

Correct. You can't use "delay" A side-effect of that is that a "for" loop isn't the way to go either. Think about it; a "for" loop says "I want to do these things, [u]one after the other[/u], this many times".

That's not what you want here, so you have to manage the loop (the count, the test and the increment) separately.

AWOL: Correct. You can't use "delay" A side-effect of that is that a "for" loop isn't the way to go either. Think about it; a "for" loop says "I want to do these things, [u]one after the other[/u], this many times".

That's not what you want here, so you have to manage the loop (the count, the test and the increment) separately.

OK, So the "for" loops are out. Can you point me in the direction that I need to go to be able to have it cycle through an array?