How can I streamline this code?

Forum 2005-2010 (read only) Software Syntax & Programs
1

Hey guys.
I'm putting together a prototype in Arduino that uses serially communicated characters sent by a processing program on my computer to trigger LEDs to go on, off, or blink. Well, I have about 6 LEDs and about 2-4 if statements for each one. I haven't put it on my Arduino yet because I don't have it around, but it seems a little big to me and I'm concerned if it'll fit. Is there any way I can streamline this?

int prepPin = 11; //Precipitation LED
int temp1Pin = 10;//Temp LED 1-20 Degrees
int temp2Pin = 9; //Temp LED 21-40 Degrees
int temp3Pin = 6; //Temp LED 41-60 Degrees
int temp4Pin = 5; //Temp LED 61-80 Degrees
int temp5Pin = 3; //Temp LED 81-100 Degrees
int val = 0;

void setup()
{
  pinMode(prepPin, OUTPUT);
  pinMode(temp1Pin, OUTPUT);
  pinMode(temp2Pin, OUTPUT);
  pinMode(temp3Pin, OUTPUT);
  pinMode(temp4Pin, OUTPUT);
  pinMode(temp5Pin, OUTPUT);
  Serial.begin(9600);
}

void loop(){
  if(Serial.available()){
  val = Serial.read();
  if (val == 'A'){    
    Serial.print('val');
    digitalWrite(temp1Pin, HIGH); //LED turns on
  }
  if (val == 'a'){
    Serial.print('val');
    digitalWrite(temp1Pin, LOW);
    delay(1000);
    digitalWrite(temp1Pin, HIGH);
    delay(1000); //LED blinks
  }
  if (val == 'B'){
    Serial.print('val');
    digitalWrite(temp2Pin, HIGH);
  }
  if (val == 'b'){
    Serial.print('val');
    digitalWrite(temp2Pin, LOW);
    delay(1000);
    digitalWrite(temp2Pin, HIGH);
    delay(1000); //LED blinks
  }  
    if (val == '2'){
    Serial.print('val');
    digitalWrite(temp2Pin, LOW);
  }
    if (val == 'C'){
    Serial.print('val');
    digitalWrite(temp3Pin, HIGH);
  }
  if (val == 'c'){
    Serial.print('val');
    digitalWrite(temp3Pin, LOW);
    delay(1000);
    digitalWrite(temp3Pin, HIGH);
    delay(1000); //LED blinks
  }  
    if (val == '3'){
    Serial.print('val');
    digitalWrite(temp3Pin, LOW);
  }
    if (val == 'D'){
    Serial.print('val');
    digitalWrite(temp4Pin, HIGH);
  }
  if (val == 'd'){
    Serial.print('val');
    digitalWrite(temp4Pin, LOW);
    delay(1000);
    digitalWrite(temp4Pin, HIGH);
    delay(1000); //LED blinks
  }  
    if (val == '4'){
    Serial.print('val');
    digitalWrite(temp4Pin, LOW);
  }
    if (val == 'E'){
    Serial.print('val');
    digitalWrite(temp5Pin, HIGH);
  }
  if (val == 'e'){
    Serial.print('val');
    digitalWrite(temp5Pin, LOW);
    delay(1000);
    digitalWrite(temp5Pin, HIGH);
    delay(1000); //LED blinks
  }  
  if (val == '5'){
    Serial.print('val');
    digitalWrite(temp5Pin, LOW);
  }
  if (val == '0')_
    Serial.print('val');
    digitalWrite(prepPin, LOW);
  }
  if (val == 'W'){
    Serial.print('val');
    digitalWrite(prepPin, LOW);
    delay(1000);
    digitalWrite(prepPin, HIGH);
    delay(1000); //LED blinks
  }
  if (val == 'X'){
    Serial.print('val');
    digitalWrite(prepPin, LOW);
    delay(500);
    digitalWrite(prepPin, HIGH);
    delay(500);
  }
  if (val == 'Y'){
    Serial.print('val');
    digitalWrite(prepPin, LOW);
    delay(250);
    digitalWrite(prepPin, HIGH);
    delay(250);
  }
  if (val == 'Z'){
    Serial.print('val');
    digitalWrite(prepPin, LOW);
    delay(125);
    digitalWrite(prepPin, HIGH);
    delay(125);
  }
}
}
2

The code looks like it will work the way it is. For what it's worth, I don't think this will be too big: it should run about 4K worth of program space. You can hit 'verify' on the arduino IDE and it should report the size in the bottom window. If it says it can't determine this size, hit verify again: sometimes it gets confused the first time.

There are several ways to streamline this code: program size, execution time and "neatness". The first thing that comes to mind is to replace all those 'if' statements with a 'switch'. However, it's been rumored that switch statements use more room on the arduino.

You can speed things up a little, since all the possibilities for the input are mutually exclusive. Use "else if" instead of "if" for the statements after the first.

The biggest gain would be to use something more meaningful than ascii characters as input, if possible. For example, divide the 8 bits in two, one half for the pin number and the other half for on, off, or blink. There are other schemes that will work as well.

If you have to use the current setup, then there are some tricks you can play with characters and their ascii numbers. If you had an array
byte pins[5] = { 10, 9, 6, 5, 3 };
Then you could say something like if val is between 'A' and 'E', then do digitalWrite(pin[val - 'A'], HIGH). Your prep pin would still need special processing, but the code would be more compact.

My suggestion would be to leave it as is, until and unless you run out of room. Then go back and optimize. "if it ain't broke..."

3

Everything ckiick says is quite correct. I'd also add that you have repeated the expression Serial.print("val") [and note that that should probably be DOUBLE quotes] quite a few times. You could condense the code quite a bit by doing this once just in front of the big if/else block.

Mikal

4

Thanks, guys! I totally forgot about compile/verify. smacks head It's something like 3.6 kb in the end. And here I was worried.

5

If you're still interested in alternate implementations, the regularity of the actions that you're taking strongly suggests using a table-driven approach as I've shown below (completely untested but it compiles without error). This is a more advanced programming technique that is often used in applications of all sizes.

The program below uses several more advanced concepts. The basic ideas is that you create a data table containing entries for each action that you want to take. Then, when a "command" is received, you step through the table until you find the matching entry and then perform the action indicated by the other values in the table entry. In this case, I've placed the table in Flash memory (using the PROGMEM attribute) and each entry in the table is a structure whose layout is defined by the typedef statement below.

The big advantage to using this technique is that once it is set up, it is very easy to add new command/action entries with very small impact to program size. If you need to perform a different type of action, it is easy to define a new action code and add a new case to the action switch statement.

#include <inttypes.h>
#include <avr/pgmspace.h>

const int prepPin  = 11; //Precipitation LED
const int temp1Pin = 10; //Temp LED 1-20 Degrees
const int temp2Pin =  9; //Temp LED 21-40 Degrees
const int temp3Pin =  6; //Temp LED 41-60 Degrees
const int temp4Pin =  5; //Temp LED 61-80 Degrees
const int temp5Pin =  3; //Temp LED 81-100 Degrees

// these values define the actions that can be used in the structure below
const uint8_t ACT_SET   = 0;     // set pin high or low
const uint8_t ACT_PULSE = 1;     // pulse pin low then high
const uint8_t ACT_NONE  = 100;   // no action

// this structure defines the content of each table entry
typedef struct
{
  uint8_t val;     // the character code for this entry
  uint8_t pin;     // the pin number for this entry
  uint8_t action;  // the action code (see above)
  uint16_t data;   // the data for the action (e.g. HIGH/LOW or delay time)
} PinData;

#define ELEM_CNT(x) (sizeof(x)/sizeof(x[0]))

// this is the command/action table (note that this is in Flash memory)
static PinData pinData[] PROGMEM =
{
 // code  pin        action      data value
  { 'A',  temp1Pin,  ACT_SET,    HIGH },
  { 'a',  temp1Pin,  ACT_PULSE,  1000 },
  { 'B',  temp2Pin,  ACT_SET,    HIGH },
  { 'b',  temp2Pin,  ACT_PULSE,  1000 },
  { '2',  temp2Pin,  ACT_SET,    LOW },
  { 'C',  temp3Pin,  ACT_SET,    HIGH },
  { 'c',  temp3Pin,  ACT_PULSE,  1000 },
  { '3',  temp3Pin,  ACT_SET,    LOW },
  { 'D',  temp4Pin,  ACT_SET,    HIGH },
  { 'd',  temp4Pin,  ACT_PULSE,  1000 },
  { '4',  temp4Pin,  ACT_SET,    LOW },
  { 'E',  temp5Pin,  ACT_SET,    HIGH },
  { 'e',  temp5Pin,  ACT_PULSE,  1000 },
  { '5',  temp5Pin,  ACT_SET,    LOW },
  { '0',  prepPin,   ACT_SET,    LOW },
  { 'W',  prepPin,   ACT_PULSE,  1000 },
  { 'X',  prepPin,   ACT_PULSE,  500 },
  { 'Y',  prepPin,   ACT_PULSE,  250 },
  { 'Z',  prepPin,   ACT_PULSE,  125 }
};

void setup()
{
  pinMode(prepPin, OUTPUT);
  pinMode(temp1Pin, OUTPUT);
  pinMode(temp2Pin, OUTPUT);
  pinMode(temp3Pin, OUTPUT);
  pinMode(temp4Pin, OUTPUT);
  pinMode(temp5Pin, OUTPUT);
  Serial.begin(9600);
}

void loop()
{
  if (Serial.available())
  {
    int val = Serial.read();
    for (int i = 0; i < ELEM_CNT(pinData); i++)
    {
      // see if this is the matching entry
      if (val == pgm_read_byte(&pinData[i].val))
      {
        PinData pd;

        // copy the data for this entry from Flash to RAM for easier access
        memcpy_P(&pd, pinData + i, sizeof(pd));

        Serial.print('val');

        // perform the specified action
        switch (pd.action)
        {
        case ACT_SET:     // set the pin high or low
          digitalWrite(pd.pin, pd.data);
          break;
         
        case ACT_PULSE:    // pulse the pin low then high
          digitalWrite(pd.pin, LOW);
          delay(pd.data);
          digitalWrite(pd.pin, HIGH);
          delay(pd.data);
          break;
        }

        // no need to examine any more entries, exit for loop
        break;
      }
    }
  }
}