More efficient code?

Hi, is there anyway to make this code more efficient? I am quite new
on arduino :slight_smile:

//Made by Redkite for the "L298N Stepper Motor Driver Controller Board"
//from www.dealextreme.com.(sku 120542)
void setup() {
  Serial.begin(9600);          //initialize serial
  pinMode(2, OUTPUT);//Enable B, must be on for turning motor.
  pinMode(3, OUTPUT);//In 3
  pinMode(4, OUTPUT);//In 4
  pinMode(5, OUTPUT);//Enable A, must be on for forward/reverse motor.
  pinMode(6, OUTPUT);//In 1
  pinMode(7, OUTPUT);//In 2
}
 
void loop() {
  while(Serial.available()){  //is there anything to read?
        char getData = Serial.read();  //if yes, read it
 
      switch (getData) {
    case 'F':   //Forward 
      digitalWrite(5, HIGH);
      digitalWrite(6, HIGH);
      digitalWrite(7, LOW);
      break;
    case 'B':    //Reverse
      digitalWrite(5, HIGH);
      digitalWrite(6, LOW);
      digitalWrite(7, HIGH);
      break;
    case 'L':    //Left
      digitalWrite(2, HIGH);
      digitalWrite(3, HIGH);
      digitalWrite(4, LOW);
      break;
    case 'R':    //Right
      digitalWrite(2, HIGH);
      digitalWrite(3, LOW);
      digitalWrite(4, HIGH);
      break;
      case 'G': //Forward Left   
      digitalWrite(5, HIGH);  //Enables movement motor
      digitalWrite(6, HIGH);  //6 High and 7 Low = Forward movement.
      digitalWrite(7, LOW);  //
      digitalWrite(2, HIGH);  //Enables turining motor
      digitalWrite(3, HIGH);  //3 High and 4 Low = Left turning
      digitalWrite(4, LOW);  //
      break;
      case 'I': //Forward Right   
      digitalWrite(5, HIGH);  //Enables movement motor
      digitalWrite(6, HIGH);  //6 High and 7 Low = Forward movement.
      digitalWrite(7, LOW);  //
      digitalWrite(2, HIGH);  //Enables turining motor
      digitalWrite(3, LOW);  //3 Low and 4 High = Right turning
      digitalWrite(4, HIGH);  //
      break;
      case 'H': //Backward Left   
      digitalWrite(5, HIGH);  //Enables movement motor
      digitalWrite(6, LOW);  //6 Low and 7 High = Backward movement.
      digitalWrite(7, HIGH);  //
      digitalWrite(2, HIGH);  //Enables turining motor
      digitalWrite(3, HIGH);  //3 High and 4 Low = Left turning
      digitalWrite(4, LOW);  //
      break;
      case 'J': //Backward Right   
      digitalWrite(5, HIGH);  //Enables movement motor
      digitalWrite(6, LOW);  //6 Low and 7 High = Backward movement.
      digitalWrite(7, HIGH);  //
      digitalWrite(2, HIGH);  //Enables turining motor
      digitalWrite(3, LOW);  //3 Low and 4 High = Right turning
      digitalWrite(4, HIGH);  //
      break;
      default:  //Do this when nothing is recieved
      digitalWrite(2, LOW);  //Turn off all movement/inputs.
      digitalWrite(3, LOW);
      digitalWrite(4, LOW);
      digitalWrite(5, LOW);
      digitalWrite(6, LOW);
      digitalWrite(7, LOW);
      }
  }
}

For starters use arrays and/or loops for all those pinModes() and digitalWrites().

For example

for (int i = 2; i < 7; i++)
pinMode (i, OUTPUT);


Rob

Use arrays to hold the bit patterns
http://www.thebox.myzen.co.uk/Tutorial/Arrays.html
Why hav a comment In1 against an output?

It won't make your code any more or less efficient, but giving the pins sensible names will allow you to make changes more easily.

Grumpy_Mike:
Use arrays to hold the bit patterns
http://www.thebox.myzen.co.uk/Tutorial/Arrays.html
Why hav a comment In1 against an output?

The In1~4 are the names on the Stepper Motor board :slight_smile:

Thanks for replys. I have never used Array's before, but i shall look at it :wink:

One more thing.
How can i implement a "failsafe" feuture? If i lose my bluetooth connection all the OUTPUT'S are set to LOW?

Thanks in advance :slight_smile:

Remember the last time you got a valid transmission (using millis()), then check how long it's been every loop and take action if it's been too long.


Rob

Graynomad:
Remember the last time you got a valid transmission (using millis()), then check how long it's been every loop and take action if it's been too long.


Rob

Dont know exactly what you mean. Can you provide an example?

millis returns the number of milliseconds (more or less) since the last reset.
If you call it when you get an event, and record the time returned, then every time through your loop, you can call millis again, and if the difference of the two times (the time now, and the time you last recorded a reception event) is greater than some predetermined value, then take your failsafe action.

More than being efficient, you should worry about the code being correct. And you also need to be clearer about what 'efficient' means. Use fewer instructions? Less memory? Run faster? I don't think this sketch will use that much program space, and not that much memory either. You could use some tricks to make the program smaller, but they make the code more complicated and less readable.

Here are a couple of problems with the sketch.

You only digitalWrite to some of the driver pins for F, B, L, R. Consider the case where an 'F' is received immediately followed by an 'R'. The case for F enables A and sets pins 6 and 7. But the case for 'L' does not turn them off. If you are only using one motor, you need to explicitly disable the other.

The default case says "Do this when nothing is recieved." However, the default case will only be hit if there IS serial input and it's not one of the recognized letters. If there is no input, nothing is done and the output stays at whatever settings they were last set to. You need to think about what to do in the case where it gets an unforeseen character. Stop? Ignore it?

You don't have a stop command. Or, from above, you have a bunch of them, just nothing especially defined as stop.

I applaud your idea to have a failsafe mode if communication is lost. Runaway robots are such a cliche :blush:.

One more thing to think about. If you want to go forward, does the sender have to keep sending 'F's until it wants to stop, or does it send one 'F' and the robot keeps going until it gets another command? The first case is safer for the robot, but you might lose some speed if the serial link is slow.

here's what I would do:
[ I have not compiled or run this code. Use at your own risk.]

#define MOVE_EN         5
#define TURN_EN         2
#define MOVE_A          6
#define MOVE_B          7
#define TURN_A          3
#define TURN_B          4

#define FAILSAFE        2000    // two seconds
unsigned long last;

void setup() {
  Serial.begin(9600);          //initialize serial
  pinMode(TURN_EN, OUTPUT);//Enable B, must be on for turning motor.
  pinMode(TURN_A, OUTPUT);//In 3
  pinMode(TURN_B, OUTPUT);//In 4
  pinMode(MOVE_EN, OUTPUT);//Enable A, must be on for forward/reverse motor.
  pinMode(MOVE_A, OUTPUT);//In 1
  pinMode(MOVE_B, OUTPUT);//In 2
  last = millis();
}

void move_fwd()
{
      digitalWrite(MOVE_EN, HIGH);
      digitalWrite(MOVE_A, HIGH);
      digitalWrite(MOVE_B, LOW);
}

void move_bck()
{
      digitalWrite(MOVE_EN, HIGH);
      digitalWrite(MOVE_A, LOW);
      digitalWrite(MOVE_B, HIGH);
}

void move_stop()
{
      digitalWrite(MOVE_EN, LOW);
      digitalWrite(MOVE_A, LOW);
      digitalWrite(MOVE_B, LOW);
}

void turn_rt()
{
      digitalWrite(TURN_EN, HIGH);
      digitalWrite(TURN_A, LOW);
      digitalWrite(TURN_B, HIGH);
}

void turn_lft()
{
      digitalWrite(TURN_EN, HIGH);
      digitalWrite(TURN_A, HIGH);
      digitalWrite(TURN_B, LOW);
}

void turn_stop()
{
      digitalWrite(TURN_EN, LOW);
      digitalWrite(TURN_A, LOW);
      digitalWrite(TURN_B, LOW);
}

void loop() {
  if (Serial.available()) {  //is there anything to read?
        char getData = Serial.read();  //if yes, read it
        last = millis();

      switch (getData) {
    case 'F':   //Forward
        turn_stop();
        move_fwd();
      break;
    case 'B':    //Reverse
        turn_stop();
        move_bck();
      break;
    case 'L':    //Left
        move_stop();
        turn_lft();
      break;
    case 'R':    //Right
        move_stop();
        turn_rt();
      break;
      case 'G': //Forward Left
        move_fwd();
        turn_lft();
      break;
      case 'I': //Forward Right
        move_fwd();
        turn_rt();
      break;
      case 'H': //Backward Left
        move_bck();
        turn_lft();
      break;
      case 'J': //Backward Right
        move_bck();
        turn_rt();
      break;
      case 'S':  // Stop
        move_stop();
        turn_stop();
      default:  // ignore garbage
        break;
      }
  } else {
        if (millis() - last) > FAILSAFE)
                move_stop();
                turn_stop();
        }
}