replace delay()

I am new to arduino (and to programming), so I have a very basic question. I have searched the web for solutions, but nothing really answered my question.

I'm working on a light system for an RC drift car. I want to blink 10 different leds, but in more different ways. I'm using a simple button to surf between the different led blinking modes. My problem is, that I'm using the delay() function to delay the blinking time. This stops my code and stops reading the button. I could probably solve it like it is in the "Blink without Delay" example, but I have to toggle 10 different leds in 10 different ways and that would make my code very complicated and very long. I have already written some of it, but I'm stuck now.

const int buttonPin = 2; //pushbutton
const int L1 = 3; //led 1
const int L2 = 4; //led 2
const int L3 = 5;
const int L4 = 6;
const int L5 = 7;
const int L6 = 8;
const int L7 = 9;
const int L8 = 10;
const int L9 = 11;
const int L10 = 12;

int testL = 13; //only for debugging, will be deleted

int reading;
int state;
int lastButtonState = HIGH;


//for blink mode2
int time = 100;

void setup() {
  pinMode(buttonPin, INPUT_PULLUP);  //set button as input

  for (int i=3; i<=12; i++) //set leds as outputs
    pinMode(i, OUTPUT);

  pinMode(Lpin, OUTPUT);  //only for debugging, will be deleted

  Serial.begin(9600);  //only for debugging, will be deleted
}

void loop() {

  reading = digitalRead(buttonPin);

  if (reading == LOW && reading != lastButtonState) {
    state++;
    if (state >= 9)
      state = 0;
    //state changes between 0 and 9 if button is pressed
    //these will be the different blinking modes
  } 
  lastButtonState = reading;

  Serial.println(state); //to know which mode is active

    switch (state){  //the different modes:

  case 0: //all leds off
    for (int i=3; i<=12; i++)
      digitalWrite(i, LOW);
    break;

  case 1: //double blink for all leds with 70 millis delay
    for (int i=3; i<=12; i++)
      digitalWrite(i, HIGH);
    delay(70);
    for (int i=3; i<=12; i++)
      digitalWrite(i, LOW);
    delay(70);

    break;

  case 2: //blink mode 2
    digitalWrite(L1, HIGH);
    digitalWrite(L4, HIGH);
    digitalWrite(L7, HIGH);
    digitalWrite(L10, HIGH);
    digitalWrite(L5, HIGH);
    digitalWrite(L6, HIGH);
    delay(time);
    digitalWrite(L1, LOW);
    digitalWrite(L4, LOW);
    digitalWrite(L7, LOW);
    digitalWrite(L10, LOW);
    digitalWrite(L5, LOW);
    digitalWrite(L6, LOW);
    delay(time);
    digitalWrite(L1, HIGH);
    digitalWrite(L4, HIGH);
    digitalWrite(L7, HIGH);
    digitalWrite(L10, HIGH);
    digitalWrite(L5, HIGH);
    digitalWrite(L6, HIGH);
    delay(time);
    digitalWrite(L1, LOW);
    digitalWrite(L4, LOW);
    digitalWrite(L7, LOW);
    digitalWrite(L10, LOW);
    digitalWrite(L5, LOW);
    digitalWrite(L6, LOW);
    delay(time);


    digitalWrite(L2, HIGH);
    digitalWrite(L3, HIGH);
    digitalWrite(L8, HIGH);
    digitalWrite(L9, HIGH);
    delay(time);
    digitalWrite(L2, LOW);
    digitalWrite(L3, LOW);
    digitalWrite(L8, LOW);
    digitalWrite(L9, LOW);
    delay(time);
    digitalWrite(L2, HIGH);
    digitalWrite(L3, HIGH);
    digitalWrite(L8, HIGH);
    digitalWrite(L9, HIGH);
    delay(time);
    digitalWrite(L2, LOW);
    digitalWrite(L3, LOW);
    digitalWrite(L8, LOW);
    digitalWrite(L9, LOW);
    delay(time);


    break;
  case 3: //blink mode 3

      break;
  case 4:

    break;
  case 5:

    break;
  case 6:

    break;
  case 7:

    break;
  case 8:

    break;
  case 9:

    break;
  }
}

So my question is: How can I read the button constantly without having to write hundreds of pages of code?

The blink without delay is the way to go. With arrays there's no reason the code should be very long at all.

The first thing you should learn is using array's. Then you can code many things more efficient.
Write a separate functions for the blinking patterns without delays

Here (quite) some code snippets that should get you started, some are not super efficient but I think the application is not time critical :slight_smile:

int leds[10] = {3,4,5,6,7,8,9,10,11,12};  // array of the led numbers used

int state = 0;
int lastState = 0;

setup()
{
  for (int i=0; i<10; i++)
  {
    pinMode(led[i], INPUT_PULLUP);
  }
...
}

void loop()
{
  reading = digitalRead(buttonPin);
  if (reading == LOW && reading != lastButtonState) 
  {
    state++;
    if (state >= 9) state = 0;
  } 
  lastButtonState = reading;

  switch(state)
  {
    case 0: Pattern0(); break;
    case 1: Pattern1(); break;
    case 2: Pattern2(); break;
    ...
  }
}

////////////////////////////////////////////////////////
//
// PATTERN FUNCTIONS
//
void Pattern0()
{
  for (int i=0; i< 10; i++) digitalWrite(Leds[i], LOW);
}

void Pattern1()
{
  for (int i=0; i< 10; i++) digitalWrite(Leds[i], HIGH);
}

void Pattern2()
{
  static int interval = 100;    // each pattern 
  static unsigned long lastTime;
  if (state != lastState) // first call to this pattern?
  {
    lastState = state;
    // setup the initial state of the leds
    for (int i=0; i< 10; i+= 2) // NOTE STEPSIZE 2
    {
     digitalWrite(Leds[i], HIGH);
     digitalWrite(Leds[i+1], LOW);
    }
    lastTime= millis();
  }
  // generic blink if interval has passed since last call
  if (millis() - lastTime >= interval)
  {
    for (int i=0; i< 10; i++) digitalWrite(Leds[i], !digitalRead(Leds[i]);  // blink;
    lastTime= millis();
  }
}

void Pattern3()
{
  static int interval = 50;    // each pattern 
  static unsigned long lastTime;
  if (state != lastState) // first call to this pattern?
  {
    lastState = state;
    // setup the initial state of the leds
    for (int i=0; i< 10; i++) digitalWrite(Leds[i+1], LOW);
    digitalWrite(Leds[0], HIGH);
    digitalWrite(Leds[5], HIGH);
    lastTime= millis();
  }
  // generic shift if interval has passed since last call
  if (millis() - lastTime >= interval)
  {
    int temp = digitalRead(0);
    for (int i=0; i< 9; i++) digitalWrite(Leds[i], digitalRead(Leds[ i+1]);
    digitalWrite(leds[9], temp);
 
    lastTime= millis();
  }
}

You see the repeating pattern of patterns? (code not tested)

Look at the MultiBlink code in the Playground. It is designed to do what you seem to need.

Thanks for your support, especially to robtillaart! I once used arrays, but I found it a bit difficult to understand them. I will work on that.

marco_c: I looked at that code and wit that, I can not make a double blink (or I don't know how). But thanks for the advice.

Thanks guys! Now I understand a lot more from arduino and how to write programs. You just have to think in a logical way :D.

Now there are 14 different patterns and I'm planning to do 20. Here is a video of the current state: Rc drift car lights prototype V2 - YouTube

Can you post your final code for the future when other people finding this thread?

robtillaart:
Can you post your final code for the future when other people finding this thread?

If it's ready I will!

So, it is getting ready, but I have a big problem. I want to flash the program on a Antmega8, but the code is a bit longer, than that the flash memory can hold. By exaclty 252 bytes. :smiley:
How can I shrink it? :smiley:

P.S. I had to upload the sketch, because it couldn't fit in a single post. :smiley:

RC_Drift_lights.ino (27.9 KB)

How can I shrink it?

you can remove the - static unsigned long lastmillis = 0; - from the patterns()
and use one global unsigned long lastmillis = 0;
That will save you 4 bytes per pattern

you might reuse state as a global var instead of local statics. (might introduce some less defined behavior)

static int count = 0; idem

global vars can be used in all functions but they share state, where a static function has its scope only within the function it is defined in.

int interval = 90; => uint8_t interval = 90; use 8 bit variables where possible saves a byte per place
==> also in for (uint8_t i=0; i< 10; i++){

should get you started.

Now, I deleted all static unsigned long lastmillis = 0; - from the patterns() and declared a global one, but that didn't shrink the size with no bit!

I can't use count, because the blinkings go crazy.

int interval = 90; => uint8_t interval = 90; use 8 bit variables where possible saves a byte per place
==> also in for (uint8_t i=0; i< 10; i++){

So uint8_t is the same as byte?

edit:
I replaced all ints to bytes (and uint8_ts), but that only saved me 8 bytes.

I think you misunderstood me. The flash memory is too small not the SRAM. If this isn't the case, my apologies.

Ah, the delights of shrinking programs!

The first question is: why do you need to fit it on an atmega8? Using an atmega16 instead will save you the bother of shrinking the code and leave you room for expansion.

If you really do want to shrink that code, there are plenty of ways you can do it. For example, you can rewrite Pattern13, making use of the similarity between cases:

void Pattern13()  //leds go around
{
  static int interval = 70;
  static unsigned long lastmillis = 0;
  static int state;
  static boolean onoff;
  static int count = 0;

  if (firstRun){  //initialize leds and states only once
    onoff = LOW;
    for (int i=0; i< 10; i++) digitalWrite(Leds[i], LOW);
    digitalWrite(Leds[4], HIGH);
    digitalWrite(Leds[5], HIGH);
    count = 0;
  }
  else{
    if (millis() - lastmillis >= interval){  //switch case if interval has passed

      switch (state){
      case 0:  //blink first leds twice
      case 1:
      case 2:
      case 3:
        onoff = !onoff;
        digitalWrite(Leds[state], onoff);
        break;
      case 4:  //blink last leds twice
      case 5:
      case 6:
      case 7:
        onoff = !onoff;
        digitalWrite(Leds[13 - state], onoff);
        break;
      }
      count++;
      if (count >= 2){  //if programm has passed 2 times (on, off) go to the sixth part
        state = (state + 1) & 7;
        count = 0;
      }
      lastmillis = millis();
    }
  }
}

PS - if you factor out the lastmillis check/update code you can get it down to 7400 bytes or less.

Here's something I'd try. You use this pattern a lot:

    digitalWrite(Leds[0], LOW);

I wonder if it generates less code to replace all those calls with calls to a function to do the dereference:

    // global LED control function
    void setLed(byte led, byte lohi) { 
        digitalWrite(Leds[led], lohi);
    }

    // replace all digitalWrite(Leds[0], LOW); with:
    setLed(0, LOW);

It would be easy to try on a few cases to see what the space impact is.

-br

Thanks guys problem solved. :smiley:

billroy:
I wonder if it generates less code to replace all those calls with calls to a function to do the dereference:

    // global LED control function

void setLed(byte led, byte lohi) {
        digitalWrite(Leds[led], lohi);
    }

// replace all digitalWrite(Leds[0], LOW); with:
    setLed(0, LOW);



It would be easy to try on a few cases to see what the space impact is.

I also tried this out, but it only saved me about 20 bytes...

Another idea. There are a lot of patterns like this:

        digitalWrite(Leds[1], HIGH);
        digitalWrite(Leds[2], HIGH);
        digitalWrite(Leds[7], HIGH);
        digitalWrite(Leds[8], HIGH);

Would it be smaller to do this?

        setLeds("1278", HIGH);

In other words, would passing a string constant with the LEDs to set to a custom led-setting routine be cheaper than 4 separate calls?

Just a thought.

-br

billroy:
Another idea. There are a lot of patterns like this:

        digitalWrite(Leds[1], HIGH);

digitalWrite(Leds[2], HIGH);
        digitalWrite(Leds[7], HIGH);
        digitalWrite(Leds[8], HIGH);




Would it be smaller to do this?



setLeds("1278", HIGH);




In other words, would passing a string constant with the LEDs to set to a custom led-setting routine be cheaper than 4 separate calls?

Just a thought. 

-br

I guess if your goal is a smaller source file then functions like this might help, but I would assume that you would be just making the compiled code larger and probably slower. 'Pretty' source files at the expense of slower and larger object files sounds kind of backwards thinking to me? Discuss.

Lefty

'Pretty' source files at the expense of slower and larger object files sounds kind of backwards thinking to me? Discuss.

If memory and/or performance is not the problem readability for maintainability is a very important point.
I have seen too much "write only" code that could not be read by mere humans and it really is no fun to reverse engineer the meaning of it.
So use proper descriptive names for variables ==> proper named vars need far less comments!
and use the proper types as far as possible, datatypes were invented (among others) for the reason to match :wink:

So I don't like this

void setLeds(char * str, byte lohi)
{
  for (byte i=0; i <strlen(str)) digitalWrite(str[i]-'0', lohi);
}

but I prefer meaningful names and proper types.

void setLeds(byte pin[], byte size, byte lohi)
{
  for (byte i=0; i <size) digitalWrite(pin[i], lohi);
}

...

Passing in a set of pin numbers as a string seems like a backwards step to me since it requires code to construct and parse the string.

If that set of pins is used repeatedly as a set then it'd be tempting to define a global variable or constant with those numbers in and pass that to the function to set them all. If it's dynamic then it would be possible to use a variable length array, or varargs, to pass in the set of numbers.

Maybe I am doing it the wrong way, but: Why you don't use millis() instead?

I mean, when you set led1 ON, you do:
digitalWrite(led1,HIGH);
time1=millis();

and for example if you want it ON for 250 ms:
if (millis>time1+250){
digitalWrite (led1,LOW);}

This way the loop it's never stopped.
NO?

I was thinking of something as simple as this:

void setLeds(char *leds, byte value) {
	while (*leds) {
		digitalWrite(led[*leds++ - '0'], value);
	}
}

setLeds("1457", onoff);

I think the feedback that there are bigger patterns you can factor in to common routines is probably more important, though.

-br