Correct method of creating library?

Projects Programming
1

Hi, I'm trying to convert this sketch into a library to make things more manageable...

/* Defines the vales passed to the function vStepMotor() 
   to specify which direction you would like the motor to 
   turn in. */

#define FORWARD true
#define REVERSE false

/* Defines the digital IO pins used to control each of 
   the stepper motors coils. */

#define MOTOR_COIL_1_DIO 8
#define MOTOR_COIL_2_DIO 9
#define MOTOR_COIL_3_DIO 10
#define MOTOR_COIL_4_DIO 11

/* The number of steps for 1 revolution of this stepper 
   motor is 360 degrees divided by the motor step angle 
   of 5.625 multiplied by the gear ratio 1/64 multiplied 
   by the number of coils 4 divided by two. 
   Therefore: 360 / (5.625 * (1 / 64)) = 512 */

#define STEPSPER1REV 512


/* Set the 4 DIO pins used to drive the stepper motor to outputs */
void setup() 
{
  pinMode(MOTOR_COIL_1_DIO, OUTPUT);
  pinMode(MOTOR_COIL_2_DIO, OUTPUT);
  pinMode(MOTOR_COIL_3_DIO, OUTPUT);
  pinMode(MOTOR_COIL_4_DIO, OUTPUT);
}

/* MAIN PROGRAM */
void loop() 
{
  /* Used for counting the number of steps the motor has made */
  word u16StepCounter;
 
  /* Rotate the stepper motor one full forward revolution */
  for (u16StepCounter = 0; u16StepCounter < STEPSPER1REV; u16StepCounter++)
  {
    vStepMotor(FORWARD, 3);
  }
  
  /* Rotate the stepper motor one full reverse revolution */
  for (u16StepCounter = 0; u16StepCounter < STEPSPER1REV; u16StepCounter++)
  {
    vStepMotor(REVERSE, 3);
  }
}

/* Function will pulse all four coils of the sterrper motor in either the forward 
   or reverse directions. The function accepts to inputs, a true or a false denoting 
   the direction the motor should turn in (true = forward, false = reverse), and a step 
   delay time in milliseconds between each step. */

void vStepMotor(boolean bDirection, word u16StepDelay)
{
  /* Holds which DIO pin is currently associated with which motor coil */
  byte bMotorCoil_1;
  byte bMotorCoil_2;
  byte bMotorCoil_3;
  byte bMotorCoil_4;
  
  /* Set the order of the DIO pins depending on the direction the 
     motor is requires to turn in. */
  if (bDirection == true)
  {
    bMotorCoil_1 = MOTOR_COIL_1_DIO;
    bMotorCoil_2 = MOTOR_COIL_2_DIO;
    bMotorCoil_3 = MOTOR_COIL_3_DIO;
    bMotorCoil_4 = MOTOR_COIL_4_DIO;
  }else
  {
    bMotorCoil_1 = MOTOR_COIL_4_DIO;
    bMotorCoil_2 = MOTOR_COIL_3_DIO;
    bMotorCoil_3 = MOTOR_COIL_2_DIO;
    bMotorCoil_4 = MOTOR_COIL_1_DIO;
  } 


  /* Pulse each of the stepper motors coils in a sequential order */
  digitalWrite(bMotorCoil_1, HIGH);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, HIGH);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, HIGH);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, HIGH);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
}

I've managed to create a header file and a cpp file for it i wanted to make sure I've done it correctly.
Header...

//header
/** Motor control library... **/

#ifndef Motor_h
#define Motor_h

#include "Arduino.h"

class Motor
{
   public:
     Motor(int MOTOR_COIL_1_DIO, int MOTOR_COIL_2_DIO, int MOTOR_COIL_3_DIO, int MOTOR_COIL_4_DIO);
     void vStepMotor(boolean bDirection, word u16StepDelay);
     private:
     int _pin1;
     int _pin2;
     int _pin3;
     int _pin4;
};

#endif

CPP

/*
 Motor.cpp
*/

#include "Arduino.h"
#include "Motor.h"

Motor::Motor(int MOTOR_COIL_1_DIO, int MOTOR_COIL_2_DIO, int MOTOR_COIL_3_DIO, int MOTOR_COIL_4_DIO);
{
  pinMode(MOTOR_COIL_1_DIO, OUTPUT,MOTOR_COIL_2_DIO, OUTPUT,MOTOR_COIL_3_DIO, OUTPUT,MOTOR_COIL_4_DIO, OUTPUT);
   _pin1 = MOTOR_COIL_1_DIO;
   _pin2 = MOTOR_COIL_2_DIO;
   _pin3 = MOTOR_COIL_3_DIO;
   _pin4 = MOTOR_COIL_4_DIO;
}

void Motor::(boolean bDirection, word u16StepDelay)
{
  /* Holds which DIO pin is currently associated with which motor coil */
  byte bMotorCoil_1;
  byte bMotorCoil_2;
  byte bMotorCoil_3;
  byte bMotorCoil_4;
  
  /* Set the order of the DIO pins depending on the direction the 
     motor is requires to turn in. */
  if (bDirection == true)
  {
    bMotorCoil_1 = MOTOR_COIL_1_DIO;
    bMotorCoil_2 = MOTOR_COIL_2_DIO;
    bMotorCoil_3 = MOTOR_COIL_3_DIO;
    bMotorCoil_4 = MOTOR_COIL_4_DIO;
  }else
  {
    bMotorCoil_1 = MOTOR_COIL_4_DIO;
    bMotorCoil_2 = MOTOR_COIL_3_DIO;
    bMotorCoil_3 = MOTOR_COIL_2_DIO;
    bMotorCoil_4 = MOTOR_COIL_1_DIO;
  } 


  /* Pulse each of the stepper motors coils in a sequential order */
  digitalWrite(bMotorCoil_1, HIGH);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, HIGH);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, HIGH);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, HIGH);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
}
2

This is not possible:

pinMode(MOTOR_COIL_1_DIO, OUTPUT,MOTOR_COIL_2_DIO, OUTPUT,MOTOR_COIL_3_DIO, OUTPUT,MOTOR_COIL_4_DIO, OUTPUT);

Try:

pinMode(MOTOR_COIL_1_DIO, OUTPUT);
pinMode(MOTOR_COIL_2_DIO, OUTPUT);
pinMode(MOTOR_COIL_3_DIO, OUTPUT);
pinMode(MOTOR_COIL_4_DIO, OUTPUT);

You could (but dont have to) simplify this:

  /* Pulse each of the stepper motors coils in a sequential order */
  digitalWrite(bMotorCoil_1, HIGH);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, HIGH);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, HIGH);
  digitalWrite(bMotorCoil_4, LOW);
  
  delay(u16StepDelay);
  
  digitalWrite(bMotorCoil_1, LOW);
  digitalWrite(bMotorCoil_2, LOW);
  digitalWrite(bMotorCoil_3, LOW);
  digitalWrite(bMotorCoil_4, HIGH);
  
  delay(u16StepDelay);

To this:

for (byte i = 1; i < 16; i <<=1){
  digitalWrite(bMotorCoil_1, (i&1));
  digitalWrite(bMotorCoil_2, (i&2));
  digitalWrite(bMotorCoil_3, (i&4));
  digitalWrite(bMotorCoil_4, (i&8));
  
  delay(u16StepDelay);
}
3

i wanted to make sure I've done it correctly.

Sorry, no. The header file is fine. The constructor is not.

The constructors for all global class instances will be called, then the init() method, then the setup() method, and, finally, in an infinite loop, loop() (and serialEvent(), if defined) will be called.

Setting the mode of pins in the constructor will be a waste of time, because init() will undo whatever you have done when it sets up all the hardware.

You need a begin() method in which you do the hardware-specific stuff. That method gets called in setup(), just like Serial.begin().

void Motor::(boolean bDirection, word u16StepDelay)

An anonymous method? Not allowed.

    bMotorCoil_1 = MOTOR_COIL_1_DIO;
    bMotorCoil_2 = MOTOR_COIL_2_DIO;
    bMotorCoil_3 = MOTOR_COIL_3_DIO;
    bMotorCoil_4 = MOTOR_COIL_4_DIO;

These names are not known in this method. You need to be using _pin1, etc.

Why are the values stored in ints in the class, and in bytes in the method? How many pins does your Arduino have? None of mine have more than 255 pins.

4

My strategy when I wrote my first library was base it on an existing library - so I add a keywords.txt and examples directory.

5

Thanks for the replies, so i looked back and tried to correct a few things...
My header file is now the following...

//header
/** Motor control library... **/

#ifndef Motor_h
#define Motor_h

#include "Arduino.h"

class Motor
{
   public:
     Motor();
     ~Motor();
     void start(byte MOTOR_COIL_1_DIO, byte MOTOR_COIL_2_DIO, byte MOTOR_COIL_3_DIO, byte MOTOR_COIL_4_DIO);
     void vStepMotor(boolean bDirection, word u16StepDelay);
     private:
     byte _pin1;
     byte _pin2;
     byte _pin3;
     byte _pin4;
};

#endif

I removed the arguments of the constructor, added a destructor.
Moved all my hardware part of the code to a function called start and corrected the data types of my private variables.

For the Source

/*
 Motor.cpp
*/

#include "Arduino.h"
#include "Motor.h"

Motor::Motor(); //constructer
{}
Motor::~Motor(); //destructer
{}

void Motor::start(byte MOTOR_COIL_1_DIO, byte MOTOR_COIL_2_DIO, byte MOTOR_COIL_3_DIO, byte MOTOR_COIL_4_DIO) //sets output pins, called in setup() method.
{  _pin1 = MOTOR_COIL_1_DIO;
   _pin2 = MOTOR_COIL_2_DIO;
   _pin3 = MOTOR_COIL_3_DIO;
   _pin4 = MOTOR_COIL_4_DIO;
  pinMode(_pin1, OUTPUT);
  pinMode(_pin2, OUTPUT);
  pinMode(_pin3, OUTPUT);
  pinMode(_pin4, OUTPUT);
   
}

void Motor::vStepMotor(boolean bDirection, word u16StepDelay)
{
  /* Holds which DIO pin is currently associated with which motor coil */
  byte bMotorCoil_1;
  byte bMotorCoil_2;
  byte bMotorCoil_3;
  byte bMotorCoil_4;
  
  /* Set the order of the DIO pins depending on the direction the 
     motor is requires to turn in. */
  if (bDirection == true)
  {
    bMotorCoil_1 = _pin1;
    bMotorCoil_2 = _pin2;
    bMotorCoil_3 = _pin3;
    bMotorCoil_4 = _pin4;
  }else
   {
    bMotorCoil_1 = _pin4;
    bMotorCoil_2 = _pin3;
    bMotorCoil_3 = _pin2;
    bMotorCoil_4 = _pin1;
   } 


  /* Pulse each of the stepper motors coils in a sequential order */
  for (byte i = 1; i < 16; i <<=1) {
  digitalWrite(bMotorCoil_1, (i&1));
  digitalWrite(bMotorCoil_2, (i&2));
  digitalWrite(bMotorCoil_3, (i&4));
  digitalWrite(bMotorCoil_4, (i&8));
  
  delay(u16StepDelay);
        }
}

The constructor and destructor now do nothing, all the pins are setup in the start function..
(about the start function)
I use these names as the arguments for my function...

void Motor::start(byte MOTOR_COIL_1_DIO, byte MOTOR_COIL_2_DIO, byte MOTOR_COIL_3_DIO, byte MOTOR_COIL_4_DIO)

then I assign each one to a private variable...

_pin1 = MOTOR_COIL_1_DIO;
   _pin2 = MOTOR_COIL_2_DIO;
   _pin3 = MOTOR_COIL_3_DIO;
   _pin4 = MOTOR_COIL_4_DIO;

is this necessary can i just call my arguments _pin1, _pin2 and ect and skip having to assign them later?
(carrying on with changes)
void Motor::vStepMotor(boolean bDirection, word u16StepDelay)
The function I forgot to name is now fixed and private variables declared in the header are now used instead.
I also replaced setting each pin high and low individually with the suggested for loop
can sombody explain how that works?
two bits in it confuse me the (i&1) and the i <<=1 bits specifically..

Thanks..

6

is this necessary can i just call my arguments _pin1, _pin2 and ect and skip having to assign them later?

Yes, it is. No, you can't.

7 
i <<= 1;

Is a shorthand way of writing

i = i << 1;

Which says bit shift i one bit to the left and save back in i. For example if you have the binary number:

11000100

it is made of 8 bits (assuming it is a byte). If we shift it 1 bit to the left, what we do is essentially remove the 8th bit, and add a zero on the end:

remove 8th: 1000100
add a zero: 10001000

If you have a look at the two side by side:
11000100
10001000
Can you see that as the name suggests, all bits have shifted one space to the left (apart from the last bit which has been removed)?

Now to the (i&1). That says, do the bitwise and of the value in the variable 'i' and the literal number 1.
What is a Bitwise AND? Well, first you need to know what a logical AND is:

A B | Q
----+--
0 0 | 0
0 1 | 0
1 0 | 0
1 1 | 1

If you take two inputs A and B, and then do the logical AND, what it says is 'the output is a 1 if and only if all inputs are a 1'

Now consider the bitwise AND. It is the same principal, but it applies it to all bits in a number. Lets say A=0b01011101 and B=0b11110000, then the calculation goes as follows:

01011101
11110000 &

01010000

Can you see that a bit in the output is a 1 if and only if both corresponding bits in the input are a 1.

Now lets put it together in the code I gave:

for (byte i = 1; i < 16; i <<=1){
  digitalWrite(bMotorCoil_1, (i&1));
  digitalWrite(bMotorCoil_2, (i&2));
  digitalWrite(bMotorCoil_3, (i&4));
  digitalWrite(bMotorCoil_4, (i&8));
  
  delay(u16StepDelay);
}

Initially the number i=1 ("byte i = 1"). At this point i is less than 16, so we proceed with the loop.
(i&1) = 0b00001 & 0b00001 = 0b00001. This is not 0, so is condered HIGH.
(i&2) = 0b00001 & 0b00010 = 0b00000. This is 0, so considered a LOW
(i&4) = 0b00001 & 0b00100 = 0b00000. This is 0, so considered a LOW
(i&8) = 0b00001 & 0b01000 = 0b00000. This is 0, so considered a LOW
So for that loop, only the first is set high.
At the end of the loop, i is bit shifted 1 to the left. So i becomes:
1<<1 = 0b00001 << 1 = 0b00010 = 2

At this point i is again less than 16, so we proceed with the loop.
(i&1) = 0b00010 & 0b00001 = 0b00000. This is 0, so considered a LOW
(i&2) = 0b00010 & 0b00010 = 0b00010. This is not 0, so considered a HIGH
(i&4) = 0b00010 & 0b00100 = 0b00000. This is 0, so considered a LOW
(i&8) = 0b00010 & 0b01000 = 0b00000. This is 0, so considered a LOW
So for that loop, only the second is set high.

And so on. If you work through you will see in the next loop only the 3rd is high. In the fourth loop only the 4th is high.
At the end of the 4th loop, i = 8. i is then bit shifted 1 to the left. So i becomes:
1<<1 = 0b01000 << 1 = 0b10000 = 16
This is not less than 16, so the loop exits.

8

Thanks thats a very good explanation,
I also tried to extend this to make it work with 2 motors...
It is somewhat long but a quick skim should be all thats needed to know whether it'll work or not.

//header
/** Motor control library... **/

#ifndef Motor_h
#define Motor_h

#include "Arduino.h"

class Motor
{
   public:
     Motor();
     ~Motor();
     void start(byte M1, byte M2, byte M3, byte M4, byte MA, byte MB, byte MC, byte MD);
     void forb(boolean bDirection, word u16StepDelay);
     void left(boolean bDirection, word u16StepDelay);
     void right(boolean bDirection, word u16StepDelay);
  private:
     byte _pin1[2];
     byte _pin2[2];
     byte _pin3[2];
     byte _pin4[2];
     /* Holds which DIO pin is currently associated with which motor coil */
    byte bMotorCoil_1[2];
    byte bMotorCoil_2[2];
    byte bMotorCoil_3[2];
    byte bMotorCoil_4[2];
    
};

#endif

and the source...

/*
 Motor.cpp
*/

#include "Arduino.h"
#include "Motor.h"

Motor::Motor(); //constructer
{}
Motor::~Motor(); //destructer
{}

void Motor::start(byte M1, byte M2, byte M3, byte M4, byte MA, byte MB, byte MC, byte MD) //sets output pins, called in setup() method.
{  _pin1[0] = M1;
   _pin2[0] = M2;
   _pin3[0] = M3;
   _pin4[0] = M4;
   _pin1[1] = MA;
   _pin2[2] = MB;
   _pin3[3] = MC;
   _pin4[4] = MD;
 for (i=0; i<=1; i++){
 pinMode(_pin1[i], OUTPUT);
 }
 for (i=0; i<=1; i++){
 pinMode(_pin2[i], OUTPUT);
 }
 for (i=0; i<=1; i++){
 pinMode(_pin3[i], OUTPUT);
 }
 for (i=0; i<=1; i++){
 pinMode(_pin4[i], OUTPUT);
 }
   
}

void Motor::forb(boolean bDirection, word u16StepDelay)
{
  /* Set the order of the DIO pins depending on the direction the 
     motor is requires to turn in. */
  if (bDirection == true)
  {
    for (i=0; i<=1; i++){
    bMotorCoil_1[i] = _pin1[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_2[i] = _pin2[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_3[i] = _pin3[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_4[i] = _pin4[i];
    }
    
  }else
   {
    if (bDirection == true)
  {
    for (i=0; i<=1; i++){
    bMotorCoil_1[i] = _pin4[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_2[i] = _pin3[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_3[i] = _pin2[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_4[i] = _pin1[i];
    }
   } 


  /* Pulse each of the stepper motors coils in a sequential order */
  for (byte a = 1; a < 16; a <<=1) {
    for (int i=0; i<=1; i++){
  digitalWrite(bMotorCoil_1[i], (a&1));
    }
  for (i=0; i<=1; i++){
  digitalWrite(bMotorCoil_2[i], (a&2));
    }
    for (i=0; i<=1; i++){
  digitalWrite(bMotorCoil_3[i], (a&4));
    }
    for (i=0; i<=1; i++){
  digitalWrite(bMotorCoil_4[i], (a&8));
    }
  
  delay(u16StepDelay);
        }
}

void Motor::left(boolean bDirection, word u16StepDelay)
{

  
  /* Set the order of the DIO pins depending on the direction the 
     motor is requires to turn in. */
  if (bDirection == true)
  {
    for (i=0; i<=1; i++){
    bMotorCoil_1[i] = _pin1[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_2[i] = _pin2[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_3[i] = _pin3[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_4[i] = _pin4[i];
    }
    
  }else
      {
    for (i=0; i<=1; i++){
    bMotorCoil_1[i] = _pin4[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_2[i] = _pin3[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_3[i] = _pin2[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_4[i] = _pin1[i];
    }
   } 


  /* Pulse each of the stepper motors coils in a sequential order */
  for (byte a = 1; a < 16; a <<=1) {
  digitalWrite(bMotorCoil_1[1], (a&1));
  digitalWrite(bMotorCoil_2[1], (a&2));
  digitalWrite(bMotorCoil_3[1], (a&4));
  digitalWrite(bMotorCoil_4[1], (a&8));
  delay(u16StepDelay);
        }

}

void Motor::right(boolean bDirection, word u16StepDelay)
{

  
  /* Set the order of the DIO pins depending on the direction the 
     motor is requires to turn in. */
  if (bDirection == true)
  {
    for (i=0; i<=1; i++){
    bMotorCoil_1[i] = _pin1[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_2[i] = _pin2[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_3[i] = _pin3[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_4[i] = _pin4[i];
    }
    
  } else
   {
    for (i=0; i<=1; i++){
    bMotorCoil_1[i] = _pin4[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_2[i] = _pin3[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_3[i] = _pin2[i];
    }
    for (i=0; i<=1; i++){
    bMotorCoil_4[i] = _pin1[i];
    }
  }


  /* Pulse each of the stepper motors coils in a sequential order */
  for (byte a = 1; a < 16; a <<=1) {
    digitalWrite(bMotorCoil_1[2], (a&1));
    digitalWrite(bMotorCoil_2[2], (a&2));
    digitalWrite(bMotorCoil_3[2], (a&4));
    digitalWrite(bMotorCoil_4[2], (a&8));
    delay(u16StepDelay);
        }
   }
9

I also tried to extend this to make it work with 2 motors...

I don't think that is a good idea. One instance of the Motor class should deal with ONE motor. Two motors? Two instances!

10

but then how do i get both motors moving simultaneously

11 
  _pin1[0] = M1;
   _pin2[0] = M2;
   _pin3[0] = M3;
   _pin4[0] = M4;
   _pin1[1] = MA;
   _pin2[2] = MB;
   _pin3[3] = MC;
   _pin4[4] = MD;

These index are all wrong.

but then how do i get both motors moving simultaneously

Are these stepper motors? There is already a Stepper library that has a step() method that makes the stepper step once. Simply create an instance for each one. Step the first one. Step the second one. Repeat until all steps have been completed.

Your class makes it look like you are using stepper motors as regular DC motors. That is not there intended use.

12

yes im using this http://www.ebay.co.uk/itm/140890074174?_trksid=p5197.c0.m619

13 
 _pin1[0] = M1;
   _pin2[0] = M2;
   _pin3[0] = M3;
   _pin4[0] = M4;
   _pin1[1] = MA;
   _pin2[1] = MB;
   _pin3[1] = MC;
   _pin4[1] = MD;

would that be correct or is there more wrong involved?

14

Thats the correct indexing, yes.