Condensing code

daveydav27 · August 12, 2013, 7:52am

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 
}

system · August 12, 2013, 7:59am

A couple of arrays would reduce the code-bloat.

marco_c · August 12, 2013, 8:27am

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.

UKHeliBob · August 12, 2013, 9:25am

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.

daveydav27 · August 12, 2013, 10:39am

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;
  }
  }

system · August 12, 2013, 10:45am

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.

daveydav27 · August 12, 2013, 11:28am

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.

system · August 12, 2013, 11:30am

Not real sure what I am doing wrong

Without seeing your code, neither are we.

daveydav27 · August 12, 2013, 11:42am

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);
  }
 }
}

system · August 12, 2013, 11:44am

You need a separate "previous" for each LED.

Edit: And a separate "ledState"

daveydav27 · August 12, 2013, 11:54am

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);
  }
}
}

system · August 12, 2013, 11:56am

And a separate "ledState"

econjack · August 12, 2013, 12:15pm

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.

system · August 12, 2013, 12:24pm

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

daveydav27 · August 12, 2013, 12:59pm

AWOL:

And a separate "ledState"

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.

daveydav27 · August 12, 2013, 1:51pm

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]'

system · August 12, 2013, 1:57pm

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.

daveydav27 · August 12, 2013, 2:06pm

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?

system · August 12, 2013, 2:11pm

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, one after the other, 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.

daveydav27 · August 12, 2013, 2:14pm

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, one after the other, 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?

Topic		Replies	Views
replace delay() Programming	22	8870	May 5, 2021
Tasks with no delay Programming	12	5940	May 5, 2021
Simple pattern output via an array LEDs and Multiplexing	20	8001	May 6, 2021
Help with complicated "Blink Without Delay" Programming	13	3938	May 5, 2021
output states carried through multiple loops General Guidance	17	1715	May 5, 2021

Condensing code

Related topics