First program works but needs to be smaller help

Hello, today I initially wrote my own program in an hour and tweaked it. I'm curious on how I could improve my code. Basically its just light in a diffusion box blending colours. Heres the code:

/* * Multi coloured led cube example wahey! by snapez 17th august 08 */

int red = 12; // LED RED int yellow = 11; // LED YELLOW int green = 10; // LED GREEN int msec = 5000; // Set Timing in milliseconds globally before next transition void setup() // run once, when the sketch starts { pinMode(red, OUTPUT); // sets the digital pin as output pinMode(yellow, OUTPUT); // sets the digital pin as output pinMode(green, OUTPUT); // sets the digital pin as output }

void loop() // run over and over again { digitalWrite(red, HIGH); // sets the LED on delay(msec); // waits for a second digitalWrite(red, LOW); // sets the LED off digitalWrite(yellow, HIGH); // sets the LED on delay(msec); // waits for a second digitalWrite(yellow, LOW); // sets the LED off digitalWrite(green, HIGH); // sets the LED on delay(msec); // waits for a second digitalWrite(green, LOW); // sets the LED off digitalWrite(red, HIGH); // sets the LED on digitalWrite(yellow, HIGH); // sets the LED on delay(msec); // waits for a second digitalWrite(red, LOW); // sets the LED off digitalWrite(yellow, LOW); // sets the LED off digitalWrite(red, HIGH); // sets the LED on digitalWrite(green, HIGH); // sets the LED on delay(msec); // waits for a second digitalWrite(red, LOW); // sets the LED off digitalWrite(green, LOW); // sets the LED off digitalWrite(yellow, HIGH); // sets the LED on digitalWrite(green, HIGH); // sets the LED on delay(msec); // waits for a second digitalWrite(yellow, LOW); // sets the LED off digitalWrite(green, LOW); // sets the LED off digitalWrite(red, HIGH); // sets the LED on digitalWrite(yellow, HIGH); // sets the LED on digitalWrite(green, HIGH); // sets the LED on delay(msec); // waits for a second digitalWrite(red, LOW); // sets the LED off digitalWrite(yellow, LOW); // sets the LED off digitalWrite(green, LOW); // sets the LED off

}

I eventually intend to put a mic on the analog input and use it for beat detection. Please suggest how I could compact the code if possible.

Snipez,

One way to compact code like this is to create a data table that you loop through:

unsigned char red_state[] = { HIGH, LOW,  LOW,  HIGH, HIGH, LOW,  HIGH, };
unsigned char yel_state[] = { LOW,  HIGH, LOW,  HIGH, LOW,  HIGH, HIGH, };
unsigned char grn_state[] = { LOW,  LOW,  HIGH, LOW,  HIGH, HIGH, HIGH, }; 

void loop()
{
  for (int i=0; i<sizeof(red_state); ++i)
  {
    digitalWrite(red, red_state[i]); // drive LEDs according to table
    digitalWrite(yellow, yel_state[i]);
    digitalWrite(green, grn_state[i]);
    delay(msec);
    digitalWrite(red, LOW);
    digitalWrite(yellow, LOW);
    digitalWrite(green, LOW);
  }
}

Be careful using this technique that you don’t make tables so big that they consume all your precious data space! There are ways to compact still further and to use less data space if you are interested. This is just the first example I came up with.

Mikal

:) Nice, I'm a noob! Though I think I understand your use of arrays, it is far more efficient coding. With the code adapted to your solution, it compiles to 1236 bytes. My sketch compiled to 1458 bytes. So a saving of 222bytes, roughly 15%. Well done ;D

Thanks! I enjoy optimization. This trick, using C bitwise operators, is even better at 1172!! ::slight_smile:

/* 
* Multi coloured led cube example wahey! by snapez 17th august 08 
*/ 

#define red 12
#define yellow 11
#define green 10
#define msec 5000

#define RED_ON 1
#define GRN_ON 2
#define YEL_ON 4

void setup()
{ 
  pinMode(red, OUTPUT);
  pinMode(yellow, OUTPUT);
  pinMode(green, OUTPUT);
}

unsigned char state[] = {
  RED_ON, 
  YEL_ON, 
  GRN_ON, 
  RED_ON|YEL_ON, 
  RED_ON|GRN_ON, 
  YEL_ON|GRN_ON, 
  RED_ON|YEL_ON|GRN_ON
};

void loop()
{
  for (int i=0; i<sizeof(state); ++i)
  {
    if (state[i] & RED_ON) digitalWrite(red, HIGH);
    if (state[i] & YEL_ON) digitalWrite(yellow, HIGH);
    if (state[i] & GRN_ON) digitalWrite(green, HIGH);
    delay(msec);
    digitalWrite(red, LOW);
    digitalWrite(yellow, LOW);
    digitalWrite(green, LOW);
  }
}

All the best,

Mikal

A saving of 20% impressive!