3

I am using Adafruit motor shield, I am running multiple DC motors for different amounts of time, but to start at the same time - I have researched that to do this I have to monitor the elapsed time of each motor in the loop instead of using the delay to avoid blocking if I want them to run at the same time.

Eventually, There will be around 10 DC motors running off this, however it seems very sloppy to me to have 10 repeat entries of very similar code.

Is there a way to create a class or something for the following code so I am not repeating it all as although it works it seems like the way I am currently doing it is bad practice(?) and could be done more eloquently! Forgive me if this is a strange question as I am new to arduino.

In this example there are only 2 motors, however when it gets to around 10, there will be a lot of code just for this bit!

Any suggestions are welcome.

Thanks in advance.

Example code:

#include <Wire.h>
#include <Adafruit_MotorShield.h>

boolean runMotor1 = false;
boolean Motor1Running = false;
unsigned long Motor1StartMillis;
unsigned long Motor1RunTime = 5000;

boolean runMotor2 = false;
boolean Motor2Running = false;
unsigned long Motor2StartMillis;
unsigned long Motor2RunTime = 10000;

//etc etc etc up to 10 Motors

Adafruit_MotorShield AFMS = Adafruit_MotorShield(); 
Adafruit_DCMotor *MotorMotor1 = AFMS.getMotor(1);
Adafruit_DCMotor *MotorMotor2 = AFMS.getMotor(2);

void setup() 
{ 
  Serial.begin(9600);      
  AFMS.begin();
  runMotor1 = true;  // set condition to true for testing
  runMotor2 = true;
} 
void loop()
{
/// Motor1 START
  if (runMotor1 == true) {
     Serial.println("Run Motor 1!");
     runMotor1 = false;
    MotorMotor1->setSpeed(255); 
    MotorMotor1->run(FORWARD); 
    Motor1StartMillis = millis();
    Motor1Running = true;
  }


  if( Motor1Running && (millis()-Motor1StartMillis > Motor1RunTime) )
  {
    Serial.println("Motor1 Time Elapsed!");
    MotorMotor1->setSpeed(0); 
    MotorMotor1->run(RELEASE);
    runMotor1 = false;
    Motor1Running = false;
  }

  /// Motor 2
    if (runMotor2 == true) {
     Serial.println("Run Motor 2!");
     runMotor2 = false;
    MotorMotor2->setSpeed(255); 
    MotorMotor2->run(FORWARD); 
    Motor2StartMillis = millis();
    Motor2Running = true;
  }


  if( Motor2Running && (millis()-Motor2StartMillis > Motor2RunTime) )
  {
    Serial.println("Motor2 Time Elapsed!");
    MotorMotor2->setSpeed(0); 
    MotorMotor2->run(RELEASE);
    runMotor2 = false;
    Motor2Running = false;
  }

}

Thanks for reading, and thank you for any input you may have. Steve

  • It's not in the least bit bad practice: that's what classes are for! And it's exactly how the vast majority of all Arduino add-ons, your Adafruit motor shield included, are represented in software. – CharlieHanson Jun 4 '15 at 2:52
  • Yes, I meant, The way I am doing it is bad practice, and should I be using a class or something. Let me change it to clarify :) – Steve Jun 4 '15 at 2:56
  • Aha! If you can hold on for a few hours I can give you a fuller answer - you might benefit from extending the Adafruit class rather than reinventing the wheel. – CharlieHanson Jun 4 '15 at 3:06
3

You are perfectly right, a class would help simplify your code. Here is a tentative class that should work for what you are doing:

class Motor
{
public:
    Motor(const char *_name, Adafruit_DCMotor *_motor,
          unsigned long _runTime)
    : name(_name), startNow(false), running(false),
      runTime(_runTime), motor(_motor) { }
    void start() { startNow = true; }
    void update();
private:
    const char *name;
    boolean startNow;
    boolean running;
    unsigned long startTime;
    unsigned long runTime;
    Adafruit_DCMotor *motor;
};

Most of this is almost copy-paste from your actual code, except for your runMotorX variables that I renamed startNow, which seemed more appropriate.

Beware that the name of the motor is copied as a pointer. Thus you should better call the constructor with a literal string (like "the name") as a first parameter. Otherwise it would be more appropriate to allocate memory and copy the characters themselves, but I like to avoid dynamic memory allocation on an Arduino if I can.

Now, all the code that you have in loop() for each motor would be the Motor's update() method:

void Motor::update()
{
    if (startNow) {
        Serial.print("Run ");
        Serial.print(name);
        Serial.println('!');
        startNow = false;
        motor->setSpeed(255);
        motor->run(FORWARD);
        startTime = millis();
        running = true;
    }

    if (running && (millis()-startTime > runTime))
    {
        Serial.print(name);
        Serial.println(" Time Elapsed!");
        motor->setSpeed(0);
        motor->run(RELEASE);
        running = false;
    }
}

With this class, your code would now look like:

Adafruit_MotorShield AFMS = Adafruit_MotorShield();
Motor m1("Motor 1", AFMS.getMotor(1),  5000);
Motor m2("Motor 2", AFMS.getMotor(2), 10000);
//etc etc etc up to 10 Motors

void setup()
{
    Serial.begin(9600);
    AFMS.begin();
    m1.start();
    m2.start();
}

void loop()
{
    m1.update();
    m2.update();
}

Of course, when you get to 10 motors, this would still be annoying repetition. You can simplify further by putting the motors in an array and looping through it:

Adafruit_MotorShield AFMS = Adafruit_MotorShield();
Motor motors[] = {
    Motor("Motor 1", AFMS.getMotor(1),  5000),
    Motor("Motor 2", AFMS.getMotor(2), 10000)
    //etc etc etc up to 10 Motors
};
#define NUM_MOTORS (sizeof motors / sizeof motors[0])

void setup()
{
    Serial.begin(9600);
    AFMS.begin();
    for (unsigned int i = 0; i < NUM_MOTORS; i++)
        motors[i].start();
}

void loop()
{
    for (unsigned int i = 0; i < NUM_MOTORS; i++)
        motors[i].update();
}
  • Excellent, thanks a lot, this is easy enough to understand and is exactly what I was looking for. Thanks for going the extra mile with the loop example too! – Steve Jun 4 '15 at 20:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.