user10448: 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

user7508: 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();
}


10448: <p>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.</p>

<p>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. </p>

<p>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.</p>

<p>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!</p>

<p>Any suggestions are welcome.</p>

<p>Thanks in advance.</p>

<p>Example code:</p>

<pre><code>#include &lt;Wire.h&gt;
#include &lt;Adafruit_MotorShield.h&gt;

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-&gt;setSpeed(255); 
    MotorMotor1-&gt;run(FORWARD); 
    Motor1StartMillis = millis();
    Motor1Running = true;
  }


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

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


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

}
</code></pre>

<p>Thanks for reading, and thank you for any input you may have.
Steve</p>
 7508: <p>You are perfectly right, a class would help simplify your code. Here is
a tentative class that should work for what you are doing:</p>



<pre class="lang-c++ prettyprint-override"><code>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;
};
</code></pre>

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

<p>Beware that the name of the motor is copied as a pointer. Thus you
should better call the constructor with a literal string (like
<code>"the name"</code>) 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.</p>

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

<pre class="lang-c++ prettyprint-override"><code>void Motor::update()
{
    if (startNow) {
        Serial.print("Run ");
        Serial.print(name);
        Serial.println('!');
        startNow = false;
        motor-&gt;setSpeed(255);
        motor-&gt;run(FORWARD);
        startTime = millis();
        running = true;
    }

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

<p>With this class, your code would now look like:</p>

<pre class="lang-c++ prettyprint-override"><code>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();
}
</code></pre>

<p>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:</p>

<pre class="lang-c++ prettyprint-override"><code>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 &lt; NUM_MOTORS; i++)
        motors[i].start();
}

void loop()
{
    for (unsigned int i = 0; i &lt; NUM_MOTORS; i++)
        motors[i].update();
}
</code></pre>
