Position Control of A DC Motor

Hi guys,

I started my first ever project on Thursday night.
This is for a university project.
I'm studying mechanical engineering so I'm out of my comfort zone at the moment but keen to learn.
I have no previous coding experience.

The basic ides of my project is :

To detect the 8 positions (N, NE, E, SE, S, SW, W, NW) of a DC motor.

I will be using 2 optical sensors and a disk encoder to detect the motor position.
The encoder is concentric with the output shaft of the motor, contains 4 holes about its perimeter and also has a single nipple (for a loss of other words) extruded from its outer edge, parallel with the disk face.
One optical sensor will detect the 4 holes and the other optical sensor will detect the nipple.
The motors output shaft is connected to a 2:1 gear box and then to a compass dial.
Therefore, a combination of the optical eye readings will give the desired position ( 1,1 = N, 1,2 = NE .... 2,3 = W, 2,4 = NW)

For the purpose of this exercise I will be using 2 push buttons instead of the optical sensors to simulate the position of the encoder and I will also be using LED's to signal the state of the motor (RED = ON, GREEN OFF) (Counter-intuitive, I know), and finally I will be using the serial port to command the desired position (ideally a wind direction sensor will provide the direction but will keep the serial port command control for demonstration purposes).

Problems encounter / desired outcome:

I am using a switch-case statement to differentiate between the different serial commands. Each serial command begins by turning on the motor, reading the the current position of the motor, reacting accordingly (if not at desired position stay on, if at desired position turn off) and then exits the switch-case statement.

My problem is that this set up doesn't allow time for the encoder to reach the desired position before reacting.

Ideally, I would like to enter the desired switch-case statement, loop through this switch-case statement until the correct position has been met, stop the motor at this point and return to main loop.

I have studied the examples and many tutorials which I thought were relevant and have spent hours trawling through the net to find a similar previously asked question to no avail.
I have been staring at the computer for 12 hours a day since Thursday and maybe it might just take a fresh pair of eyes to help me understand.
If anyone can reference me to some relevant material or shed light on this via their experience or otherwise I would greatly appreciate it.

Thank you.

const int  buttonPin1 = PUSH2;    
const int ledPin1 = GREEN_LED;       
const int  buttonPin2 = PUSH1;    
const int ledPin2 = BLUE_LED;       
const int motor = RED_LED;
int buttonPushCounter1 = 0;   
int buttonState1 = 0;         
int lastButtonState1 = 0;     
int buttonPushCounter2 = 0;   
int buttonState2 = 0;        
int lastButtonState2 = 0;    
int motorstate = 0;
int lastmotorstate = 0;
int ByteReceived;

void setup() {
  Serial.begin(9600);
  Serial.println(" Type Direction in Box above, . ");
  pinMode(buttonPin1, INPUT);
  pinMode(ledPin1, OUTPUT);
  pinMode(buttonPin2, INPUT_PULLUP);
  pinMode(ledPin2, OUTPUT);
  pinMode(motor, OUTPUT);
}

void loop() {
  buttonState1 = digitalRead(buttonPin1);
  if (buttonState1 != lastButtonState1) {
    if (buttonState1 == HIGH) {
      buttonPushCounter1++;
      Serial.println("on");
      Serial.print("number of button2 pushes:  ");
      Serial.println(buttonPushCounter1);
    } 
    else {
      Serial.println("off"); 
    }
    if(buttonPushCounter1 >= 2) buttonPushCounter1 =0;
  }
  {buttonState2 = digitalRead(buttonPin2);

    if (buttonState2 != lastButtonState2) {
      if (buttonState2 == HIGH) {
        buttonPushCounter2++;
        Serial.println("on");
        Serial.print("number of button1 pushes:  ");
        Serial.println(buttonPushCounter2);
      }
      else {
        Serial.println("off"); 
      }
      if(buttonPushCounter2 >= 4) buttonPushCounter2 =0;
    }
    lastButtonState1 = buttonState1;
    lastButtonState2 = buttonState2;

    if (Serial.available() > 0)
    {
      int ByteReceived = Serial.read();
      Serial.print(ByteReceived);   
      Serial.print("        ");      
      Serial.print(ByteReceived, HEX);
      Serial.print("       ");     
      Serial.print(char(ByteReceived));

      switch (ByteReceived) {
      case 'o':
        digitalWrite(motor, HIGH);
        Serial.print(" motor ON ");
        break;
      case 'x':
        digitalWrite(motor, LOW);
        Serial.print(" motor OFF ");
        break;
      case '1':
        digitalWrite(motor, HIGH);
        Serial.print(" motor ON ");
          {motorstate = digitalRead(motor);
          if ((buttonPushCounter1 % 1 == 0)&&(buttonPushCounter2 % 1 == 0)) {
            digitalWrite(ledPin1, HIGH);
            digitalWrite(motor, LOW);
            if (motorstate != lastmotorstate) {
              Serial.print(" motor OFF ");
            }
            else {
              digitalWrite(ledPin1, LOW);
            }
            lastmotorstate = motorstate;
            break;
          case '2':
            digitalWrite(motor, HIGH);
            Serial.print(" motor ON ");

            {motorstate = digitalRead(motor);
              if ((buttonPushCounter1 % 2 == 0)&&(buttonPushCounter2 % 1 == 0)) {
                digitalWrite(ledPin1, HIGH);
                digitalWrite(motor, LOW);
                if (motorstate != lastmotorstate) {
                  Serial.print(" motor OFF ");
                }
                else {
                  digitalWrite(ledPin1, LOW);
                }
                lastmotorstate = motorstate;
                break;
              case '3':
                digitalWrite(motor, HIGH);
                Serial.print(" motor ON ");

                { 
                  motorstate = digitalRead(motor);
                  if ((buttonPushCounter1 % 3 == 0)&&(buttonPushCounter2 % 1 == 0)) {
                    digitalWrite(ledPin1, HIGH);
                    digitalWrite(motor, LOW);
                    if (motorstate != lastmotorstate) {
                      Serial.print(" motor OFF ");
                    }
                    else {
                      digitalWrite(ledPin1, LOW);
                    }
                    lastmotorstate = motorstate;
                    break;
//... Continues on until case 8
                                    }
                                  }
                                }
                              }
                            }
                          }
                        }
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}

First, your first 5 assignment statements using the constants (e.g., PUSH2) need to be defined...the compiler has no clue what PUSH2 is. Normally, I would expect to see something like:

#define PUSH2 2 // Use pin 2...or whatever

Second, you have a lot of unnecessary braces that make it hard to determine what you're really trying to do. For example:

      { motorstate = digitalRead(motor);

This leading brace is not needed, but I don't know how you want to group the statements in each case statement. You need to review how braces are used in a switch/case statement block.

Finally, you can use Ctrl-T in the Arduino IDE to reformat your code to a standard coding style. That might help us read it.

Ideally, I would like to enter the desired switch-case statement, loop through this switch-case statement until the correct position has been met, stop the motor at this point and return to main loop.

There is no reason why you can't have a while loop inside switch/case.

switch (targetPosition)
{
  case NORTH:
    while (direction != NORTH)
      {
        //do stuff to move to North
      }
    //stop the motor
  break;

  case EAST:
    while (direction != EAST)
      {
        //do stuff to move to East
      }
    //stop the motor
  break;
etc, etc
}

But note that this is blocking code and would be better written to move towards the desired direction then check each time through loop() whether it has been reached

Unless the motor is rather slow, and/or the pulses from your sensors are pretty wide, you're likely to have a hard time getting it to stop where you want, without over-shooting. It's unlikely to have good positional accuracy and repeatability in any case. It would be much easier to position accurately using a proportional control on the motor (i.e. - PWM control) and a PID controller. Does the motor only turn one direction?

Regards,
Ray L.

About the motor - which is a simple DC variety.
Its quite easy to bring it to an immediate halt, with no overshoot at all.
Simply use a dynamic brake, which could be a change-over relay, that shorts out the motor terminals.
To give the relay contacts a bit longer life, a series resistor could be inserted, to limit the brake shunt current.
The relay could also be solid state realys, which are faster acting than their mechanical counterparts.
This principle is used all over in the industry.

About the position encoder, I would suggest using a gray code scheme, with 4 detector LED's.
You can then get a 4-bit gray code which ultimately translates into 16 different positions.

Google gray code encoders and understand the overlapping scheme of detector slots/holes in your detector disc.
You will need 4 circular hole patterns, each pattern circle double the hole count as the previous circle.

Anders53:
About the motor - which is a simple DC variety.
Its quite easy to bring it to an immediate halt, with no overshoot at all.
Simply use a dynamic brake, which could be a change-over relay, that shorts out the motor terminals.
To give the relay contacts a bit longer life, a series resistor could be inserted, to limit the brake shunt current.
The relay could also be solid state realys, which are faster acting than their mechanical couterparts.
This principle is used all over in the industry.

About the position encoder, I would suggest using a gray code scheme, with 4 detecor LED's.
You can then get a 4-bit gray code which ultimately translates into 16 different positions.

Google gray code encoders and understand the overlapping scheme of detector slots/holes in your detector disc.
You will need 4 circular hole patterns, each pattern circle double the hole count as the previous circle.

Or you could use just two sensors, and have quadrature, a single ring of holes, and get 4X as many encoder counts as there are holes in the disk, plus be able to sense direction as well as position...

Regards,
Ray L.

Or you could just use a stepper motor and be done with it, or a servo as long as you are prepared to go the long way round to the target position about 50% of the time.

Unfortunately the OP has a DC motor and a very simple position encoder.

econjack:
First, your first 5 assignment statements using the constants (e.g., PUSH2) need to be defined...the compiler has no clue what PUSH2 is. Normally, I would expect to see something like:

#define PUSH2 2 // Use pin 2...or whatever

Second, you have a lot of unnecessary braces that make it hard to determine what you're really trying to do. For example:

      { motorstate = digitalRead(motor);

This leading brace is not needed, but I don't know how you want to group the statements in each case statement. You need to review how braces are used in a switch/case statement block.

Finally, you can use Ctrl-T in the Arduino IDE to reformat your code to a standard coding style. That might help us read it.

Thank you for your post. I have updated my original post with the standard coding style.

The board I am using has 2 push buttons installed in it already and they are defined as push 1 and push 2 so there's no problem there.

In regards the excess braces, it seems my program will not compile with any less? even the one you have pointed out?

UKHeliBob:
There is no reason why you can't have a while loop inside switch/case.

switch (targetPosition)

{
  case NORTH:
    while (direction != NORTH)
      {
        //do stuff to move to North
      }
    //stop the motor
  break;

case EAST:
    while (direction != EAST)
      {
        //do stuff to move to East
      }
    //stop the motor
  break;
etc, etc
}



But note that this is blocking code and would be better written to move towards the desired direction then check each time through loop() whether it has been reached

Thank you for your post. I have attempted to include a while loop (probably crudely) as you suggested but it doesn't seem to give me the required result. Could you please review my attempt if you get a chance.

Thank you.

 case '1':
      while ((buttonPushCounter1 != 1)&&(buttonPushCounter2 != 1)){
        digitalWrite(motor, HIGH);
        Serial.print(" motor ON ");}

          {motorstate = digitalRead(motor);
          if ((buttonPushCounter1 % 1 == 0)&&(buttonPushCounter2 % 1 == 0)) {
            digitalWrite(ledPin1, HIGH);
            digitalWrite(motor, LOW);
            if (motorstate != lastmotorstate) {
              Serial.print(" motor OFF ");
            }
            else {
              digitalWrite(ledPin1, LOW);
            }
            lastmotorstate = motorstate;

            //do something when var equals 2
            break;

RayLivingston:
Unless the motor is rather slow, and/or the pulses from your sensors are pretty wide, you're likely to have a hard time getting it to stop where you want, without over-shooting. It's unlikely to have good positional accuracy and repeatability in any case. It would be much easier to position accurately using a proportional control on the motor (i.e. - PWM control) and a PID controller. Does the motor only turn one direction?

Regards,
Ray L.

Thank you for your post. Yes, the motor is only required to turn clockwise. Upon completion of this coding, testing and analysis of the project performance with be undertaken where PID controllers etc. will be considered as an addition. As I mentioned, this is a university project so the more to write about the better.

RayLivingston:
Or you could use just two sensors, and have quadrature, a single ring of holes, and get 4X as many encoder counts as there are holes in the disk, plus be able to sense direction as well as position...

Regards,
Ray L.

UKHeliBob:
Or you could just use a stepper motor and be done with it, or a servo as long as you are prepared to go the long way round to the target position about 50% of the time.

Unfortunately the OP has a DC motor and a very simple position encoder.

yes I agree with the above two quotations but unfortunately these suggestions are beyond the scope of the project as it is entitled: A Demonstration of the Position Control Capabilities of a DC motor. I had to build everything from scratch (including the encoder) and I was give a general purpose motor from a computer Cd drive

RayLivingston:
Or you could use just two sensors, and have quadrature, a single ring of holes, and get 4X as many encoder counts as there are holes in the disk, plus be able to sense direction as well as position...

Regards,
Ray L.

Agreed - but you still need some kind of an index.
Otherwise you are just counting from the start position.

Given a project, you are limited to the resources herein, which is not too bad, as it squeezes the last drop out of your knowledge, observations and fantasy.
Which is the general idea with a student project :slight_smile:

This code works fine FOR ONE serial command,

To simplify the question,

How can I apply this code to 8 different serial commands

// this constant won't change:
const int  buttonPin1 = PUSH2;    // the pin that the pushbutton is attached to
const int ledPin1 = GREEN_LED;       // the pin that the LED is attached to
const int  buttonPin2 = PUSH1;    // the pin that the pushbutton is attached to
const int ledPin2 = BLUE_LED;       // the pin that the LED is attached to
const int motor = RED_LED;
// Variables will change:
int buttonPushCounter1 = 0;   // counter for the number of button presses
int buttonState1 = 0;         // current state of the button
int lastButtonState1 = 0;     // previous state of the button
int buttonPushCounter2 = 0;   // counter for the number of button presses
int buttonState2 = 0;         // current state of the button
int lastButtonState2 = 0;     // previous state of the button
int motorstate = 0;
int lastmotorstate = 0;
int ByteReceived;

void setup() {
  Serial.begin(9600);
  Serial.println(" Type Direction in Box above, . ");
  // initialize the button pin as a input:
  pinMode(buttonPin1, INPUT);
  // initialize the LED as an output:
  pinMode(ledPin1, OUTPUT);
  // initialize the button pin as a input:
  pinMode(buttonPin2, INPUT_PULLUP);
  // initialize the LED as an output:
  pinMode(ledPin2, OUTPUT);
  // initialize serial communication:
  pinMode(motor, OUTPUT);
}


void loop() {

  if (Serial.available() > 0)
  {
    ByteReceived = Serial.read();
    Serial.print(ByteReceived);   
    Serial.print("        ");      
    Serial.print(ByteReceived, HEX);
    Serial.print("       ");     
    Serial.print(char(ByteReceived));

    if(ByteReceived == 'o') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }
    if(ByteReceived == '6') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }

    if(ByteReceived == 'x') // Single Quote! This is a character.
    {
      digitalWrite(motor, LOW);
      Serial.print(" motor OFF ");
    }
  }

  // read the pushbutton input pin:
  buttonState1 = digitalRead(buttonPin1);

  // compare the buttonState to its previous state
  if (buttonState1 != lastButtonState1) {
    // if the state has changed, increment the counter
    if (buttonState1 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter1++;
      Serial.println("on");
      Serial.print("number of button2 pushes:  ");
      Serial.println(buttonPushCounter1);
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
    }
    if(buttonPushCounter1 >= 2) buttonPushCounter1 =0;
  }
  {
    buttonState2 = digitalRead(buttonPin2);

    // compare the buttonState to its previous state
    if (buttonState2 != lastButtonState2) {
      // if the state has changed, increment the counter
      if (buttonState2 == HIGH) {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter2++;
        Serial.println("on");
        Serial.print("number of button1 pushes:  ");
        Serial.println(buttonPushCounter2);
      }
      else {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off"); 
      }
      if(buttonPushCounter2 >= 4) buttonPushCounter2 =0;
    }
    // save the current state as the last state, 
    //for next time through the loop
    lastButtonState1 = buttonState1;
    lastButtonState2 = buttonState2;

    // turns on the LED every four button pushes by 
    // checking the modulo of the button push counter.
    // the modulo function gives you the remainder of 
    // the division of two numbers:



    { 
      motorstate = digitalRead(motor);
      if ((buttonPushCounter1 % 2 == 0)&&(buttonPushCounter2 % 2 == 0)) {
        digitalWrite(ledPin1, HIGH);
        digitalWrite(motor, LOW);
        if (motorstate != lastmotorstate) {
          Serial.print(" motor OFF ");
        }
        else {
          digitalWrite(ledPin1, LOW);
        }
      }
    }

I had to reformat your code text, as it was difficult to follow the embraced statements.
I found a couple of extra braces, which I have removed.

Try compile it again, and see what comes up.

// this constant won't change:
const int buttonPin1 = PUSH2;    // the pin that the pushbutton is attached to
const int ledPin1 = GREEN_LED;   // the pin that the LED is attached to
const int buttonPin2 = PUSH1;    // the pin that the pushbutton is attached to
const int ledPin2 = BLUE_LED;    // the pin that the LED is attached to
const int motor = RED_LED;

// Variables will change:
int buttonPushCounter1 = 0;      // counter for the number of button presses
int buttonState1 = 0;            // current state of the button
int lastButtonState1 = 0;        // previous state of the button
int buttonPushCounter2 = 0;      // counter for the number of button presses
int buttonState2 = 0;            // current state of the button
int lastButtonState2 = 0;        // previous state of the button
int motorstate = 0;
int lastmotorstate = 0;
int ByteReceived;

void setup() {
  Serial.begin(9600);
  Serial.println(" Type Direction in Box above, . ");
  // initialize the button pin as a input:
  pinMode(buttonPin1, INPUT);
  // initialize the LED as an output:
  pinMode(ledPin1, OUTPUT);
  // initialize the button pin as a input:
  pinMode(buttonPin2, INPUT_PULLUP);
  // initialize the LED as an output:
  pinMode(ledPin2, OUTPUT);
  // initialize serial communication:
  pinMode(motor, OUTPUT);
}

void loop() {
  if (Serial.available() > 0)
  {
    ByteReceived = Serial.read();
    Serial.print(ByteReceived);
    Serial.print("        ");
    Serial.print(ByteReceived, HEX);
    Serial.print("       ");
    Serial.print(char(ByteReceived));

    if (ByteReceived == 'o') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }
    if (ByteReceived == '6') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }
    if (ByteReceived == 'x') // Single Quote! This is a character.
    {
      digitalWrite(motor, LOW);
      Serial.print(" motor OFF ");
    }
  }

  // read the pushbutton input pin:
  buttonState1 = digitalRead(buttonPin1);
  // compare the buttonState to its previous state
  if (buttonState1 != lastButtonState1) {
    // if the state has changed, increment the counter
    if (buttonState1 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter1++;
      Serial.println("on");
      Serial.print("number of button2 pushes:  ");
      Serial.println(buttonPushCounter1);
    }
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    if (buttonPushCounter1 >= 2) buttonPushCounter1 = 0;
  }
  
  // read the second pushbutton input pin:
  buttonState2 = digitalRead(buttonPin2);
  // compare the buttonState to its previous state
  if (buttonState2 != lastButtonState2) {
    // if the state has changed, increment the counter
    if (buttonState2 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter2++;
      Serial.println("on");
      Serial.print("number of button1 pushes:  ");
      Serial.println(buttonPushCounter2);
    }
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    if (buttonPushCounter2 >= 4) buttonPushCounter2 = 0;
  }
  
  // save the current state as the last state,
  //for next time through the loop
  lastButtonState1 = buttonState1;
  lastButtonState2 = buttonState2;

  // turns on the LED every four button pushes by
  // checking the modulo of the button push counter.
  // the modulo function gives you the remainder of
  // the division of two numbers:

  motorstate = digitalRead(motor);
  if ((buttonPushCounter1 % 2 == 0) && (buttonPushCounter2 % 2 == 0)) {
    digitalWrite(ledPin1, HIGH);
    digitalWrite(motor, LOW);
    if (motorstate != lastmotorstate) Serial.print(" motor OFF ");
    else digitalWrite(ledPin1, LOW);
  }
}

Anders53:
I had to reformat your code text, as it was difficult to follow the embraced statements.
I found a couple of extra braces, which I have removed.

Try compile it again, and see what comes up.

// this constant won't change:

const int buttonPin1 = PUSH2;    // the pin that the pushbutton is attached to
const int ledPin1 = GREEN_LED;  // the pin that the LED is attached to
const int buttonPin2 = PUSH1;    // the pin that the pushbutton is attached to
const int ledPin2 = BLUE_LED;    // the pin that the LED is attached to
const int motor = RED_LED;

// Variables will change:
int buttonPushCounter1 = 0;      // counter for the number of button presses
int buttonState1 = 0;            // current state of the button
int lastButtonState1 = 0;        // previous state of the button
int buttonPushCounter2 = 0;      // counter for the number of button presses
int buttonState2 = 0;            // current state of the button
int lastButtonState2 = 0;        // previous state of the button
int motorstate = 0;
int lastmotorstate = 0;
int ByteReceived;

void setup() {
  Serial.begin(9600);
  Serial.println(" Type Direction in Box above, . ");
  // initialize the button pin as a input:
  pinMode(buttonPin1, INPUT);
  // initialize the LED as an output:
  pinMode(ledPin1, OUTPUT);
  // initialize the button pin as a input:
  pinMode(buttonPin2, INPUT_PULLUP);
  // initialize the LED as an output:
  pinMode(ledPin2, OUTPUT);
  // initialize serial communication:
  pinMode(motor, OUTPUT);
}

void loop() {
  if (Serial.available() > 0)
  {
    ByteReceived = Serial.read();
    Serial.print(ByteReceived);
    Serial.print("        ");
    Serial.print(ByteReceived, HEX);
    Serial.print("      ");
    Serial.print(char(ByteReceived));

if (ByteReceived == 'o') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }
    if (ByteReceived == '6') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }
    if (ByteReceived == 'x') // Single Quote! This is a character.
    {
      digitalWrite(motor, LOW);
      Serial.print(" motor OFF ");
    }
  }

// read the pushbutton input pin:
  buttonState1 = digitalRead(buttonPin1);
  // compare the buttonState to its previous state
  if (buttonState1 != lastButtonState1) {
    // if the state has changed, increment the counter
    if (buttonState1 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter1++;
      Serial.println("on");
      Serial.print("number of button2 pushes:  ");
      Serial.println(buttonPushCounter1);
    }
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    if (buttonPushCounter1 >= 2) buttonPushCounter1 = 0;
  }
 
  // read the second pushbutton input pin:
  buttonState2 = digitalRead(buttonPin2);
  // compare the buttonState to its previous state
  if (buttonState2 != lastButtonState2) {
    // if the state has changed, increment the counter
    if (buttonState2 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter2++;
      Serial.println("on");
      Serial.print("number of button1 pushes:  ");
      Serial.println(buttonPushCounter2);
    }
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    if (buttonPushCounter2 >= 4) buttonPushCounter2 = 0;
  }
 
  // save the current state as the last state,
  //for next time through the loop
  lastButtonState1 = buttonState1;
  lastButtonState2 = buttonState2;

// turns on the LED every four button pushes by
  // checking the modulo of the button push counter.
  // the modulo function gives you the remainder of
  // the division of two numbers:

motorstate = digitalRead(motor);
  if ((buttonPushCounter1 % 2 == 0) && (buttonPushCounter2 % 2 == 0)) {
    digitalWrite(ledPin1, HIGH);
    digitalWrite(motor, LOW);
    if (motorstate != lastmotorstate) Serial.print(" motor OFF ");
    else digitalWrite(ledPin1, LOW);
  }
}

Thank you for your post. This cleaned up version also complies.
Thank you for increasing the readability of my code.

I was wondering if it was possible to repeat this code several times with different serial input commands such as the one mentioned in the original post, I cant seem to get it to work

Jarigho:
.....

I was wondering if it was possible to repeat this code several times with different serial input commands such as the one mentioned in the original post, I cant seem to get it to work

Of course you can do that.

Split up the various blocks of code in your loop() and make them as external functions, which you call from within loop()
A function is most simply explained as a subroutine, which holds a set of code that you use several times in your program.

Example : all the contigous code you are doing when reading your button could be put into a function.
You simply put the code within such a structure :

void readMyButton(here you put variables specific to button and pins you are working with when you call )
{
// put your code here to be executed by a call
}

The functions you are defining are commonly placed at the bottom of your code, outside and after the loop() section

In your setup() you will insert a call to that function like this :
readMyButton (here you put necessary start variable values specific to button and pins you are working with when you call)

In your loop() you will insert a call to that function like this :
readMyButton (here you put variables specific to button and pins you are working with when you call)

for the next button and or pins to be worked with, you call again with new Button and pin variables :
readMyButton (here you put next set of variables specific to button and pins you are working with when you call)

Very quickly you have reduced the same code structures to only appear once, and then re-using it by the function calls.

Hope it makes sense to you.

Anders53:
Of course you can do that.

Split up the various blocks of code in your loop() and make them as external functions, which you call from within loop()
A function is most simply explained as a subroutine, which holds a set of code that you use several times in your program.

Example : all the contigous code you are doing when reading your button could be put into a function.
You simply put the code within such a structure :

void readMyButton(here you put variables specific to button and pins you are working with when you call )
{
// put your code here to be executed by a call
}

The functions you are defining are commonly placed at the bottom of your code, outside and after the loop() section

In your setup() you will insert a call to that function like this :
readMyButton (here you put necessary start variable values specific to button and pins you are working with when you call)

In your loop() you will insert a call to that function like this :
readMyButton (here you put variables specific to button and pins you are working with when you call)

for the next button and or pins to be worked with, you call again with new Button and pin variables :
readMyButton (here you put next set of variables specific to button and pins you are working with when you call)

Very quickly you have reduced the same code structures to only appear once, and then re-using it by the function calls.

Hope it makes sense to you.

Thank you for your post.
I have edited my code as you have suggested but a similar problem has been encountered in that I would like the motor to stay on until the buttoncounter1 =1 and buttoncounter2 = 1 (this would be for north). instead the reaction im getting seems to be that the programme only reads the position of the motor ONCE at the same instant that the serial is sent.

Is there a way of the programme looping about this set of code until the button counters read the required amount as opposed to just reading it once.

// this constant won't change:
const int buttonPin1 = PUSH2;    // the pin that the pushbutton is attached to
const int ledPin1 = GREEN_LED;   // the pin that the LED is attached to
const int buttonPin2 = PUSH1;    // the pin that the pushbutton is attached to
const int ledPin2 = BLUE_LED;    // the pin that the LED is attached to
const int motor = RED_LED;

// Variables will change:
int buttonPushCounter1 = 0;      // counter for the number of button presses
int buttonState1 = 0;            // current state of the button
int lastButtonState1 = 0;        // previous state of the button
int buttonPushCounter2 = 0;      // counter for the number of button presses
int buttonState2 = 0;            // current state of the button
int lastButtonState2 = 0;        // previous state of the button
int motorstate = 0;
int lastmotorstate = 0;
int ByteReceived;

void setup() {
  Serial.begin(9600);
  Serial.println(" Type Direction in Box above, . ");
  // initialize the button pin as a input:
  pinMode(buttonPin1, INPUT);
  // initialize the LED as an output:
  pinMode(ledPin1, OUTPUT);
  // initialize the button pin as a input:
  pinMode(buttonPin2, INPUT_PULLUP);
  // initialize the LED as an output:
  pinMode(ledPin2, OUTPUT);
  // initialize serial communication:
  pinMode(motor, OUTPUT);
}

void loop() {
  if (Serial.available() > 0)
  {
    ByteReceived = Serial.read();
    Serial.print(ByteReceived);
    Serial.print("        ");
    Serial.print(ByteReceived, HEX);
    Serial.print("       ");
    Serial.print(char(ByteReceived));

    if (ByteReceived == 'o') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }
    if (ByteReceived == '1') // Single Quote! This is a character.
    {
      North();
    }
    if (ByteReceived == 'x') // Single Quote! This is a character.
    {
      digitalWrite(motor, LOW);
      Serial.print(" motor OFF ");
    }
  }

  // read the pushbutton input pin:
  buttonState1 = digitalRead(buttonPin1);
  // compare the buttonState to its previous state
  if (buttonState1 != lastButtonState1) {
    // if the state has changed, increment the counter
    if (buttonState1 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter1++;
      Serial.println("on");
      Serial.print("number of button2 pushes:  ");
      Serial.println(buttonPushCounter1);
    }
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    if (buttonPushCounter1 >= 2) buttonPushCounter1 = 0;
  }

  // read the second pushbutton input pin:
  buttonState2 = digitalRead(buttonPin2);
  // compare the buttonState to its previous state
  if (buttonState2 != lastButtonState2) {
    // if the state has changed, increment the counter
    if (buttonState2 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter2++;
      Serial.println("on");
      Serial.print("number of button1 pushes:  ");
      Serial.println(buttonPushCounter2);
    }
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off");
    }
    if (buttonPushCounter2 >= 4) buttonPushCounter2 = 0;
  }

  // save the current state as the last state,
  //for next time through the loop
  lastButtonState1 = buttonState1;
  lastButtonState2 = buttonState2;

  // turns on the LED every four button pushes by
  // checking the modulo of the button push counter.
  // the modulo function gives you the remainder of
  // the division of two numbers:


  lastmotorstate = motorstate;
  if (buttonPushCounter2 % 4 == 0) { 
    digitalWrite(ledPin2, HIGH); 
  }
  else {
    digitalWrite(ledPin2, LOW); 
  }
}


void North (){
  digitalWrite(motor, HIGH);
  Serial.print(" motor ON ");
  motorstate = digitalRead(motor);
  if ((buttonPushCounter1 == 1) && (buttonPushCounter2 == 1)) {
    digitalWrite(ledPin1, HIGH);
    digitalWrite(motor, LOW);
    if (motorstate != lastmotorstate) Serial.print(" motor OFF ");
    else digitalWrite(ledPin1, LOW);
  }
}

Even some sort of repeating serial command function would solve this problem.. if that is a thing?

I think ive cracked it!

But only if its made possible for my programme to read the button counters properly.. they way I have it at the moment the program doesn't read the counters

Any help with this would be great.. thanks everyone for there support

   motorstate = digitalRead(motor);
    if (buttonPushCounter1 == 1 && buttonPushCounter2 == 2) { // This seems to be inccorect
      digitalWrite(ledPin1, HIGH);
      digitalWrite(motor, LOW);
      if (motorstate != lastmotorstate) Serial.print(" motor OFF ");
      else digitalWrite(ledPin1, LOW);
    }
  }
}

Ok Guys this is my latest code, and I'm still having trouble getting the programme to read both the counters (Same line as the code mention in the above comment) and thus stop the motor. I just took the button counter code from the examples and there doesn't seem to be anything else this on-line or in the tutorials. Has anyone had this problem before or understand how to communicate this so the programme can understand what i'm trying to achieve?
Thank you.

/ this constant won't change:
const int buttonPin1 = PUSH2;    // the pin that the pushbutton is attached to
const int ledPin1 = GREEN_LED;   // the pin that the LED is attached to
const int buttonPin2 = PUSH1;    // the pin that the pushbutton is attached to
const int ledPin2 = BLUE_LED;    // the pin that the LED is attached to
const int motor = RED_LED;

// Variables will change:
int buttonPushCounter1 = 0;      // counter for the number of button presses
int buttonState1 = 0;            // current state of the button
int lastButtonState1 = 0;        // previous state of the button
int buttonPushCounter2 = 0;      // counter for the number of button presses
int buttonState2 = 0;            // current state of the button
int lastButtonState2 = 0;        // previous state of the button
int motorstate = 0;
int lastmotorstate = 0;
int ByteReceived;

void setup() {
  Serial.begin(9600);
  Serial.println(" Type Direction in Box above, . ");
  // initialize the button pin as a input:
  pinMode(buttonPin1, INPUT);
  // initialize the LED as an output:
  pinMode(ledPin1, OUTPUT);
  // initialize the button pin as a input:
  pinMode(buttonPin2, INPUT_PULLUP);
  // initialize the LED as an output:
  pinMode(ledPin2, OUTPUT);
  // initialize serial communication:
  pinMode(motor, OUTPUT);
}

void loop() {
  if (Serial.available() > 0)
  {
    ByteReceived = Serial.read();
    Serial.print(ByteReceived);
    Serial.print("        ");
    Serial.print(ByteReceived, HEX);
    Serial.print("       ");
    Serial.print(char(ByteReceived));

    if (ByteReceived == 'o') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }

    if (ByteReceived == '2') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
      NorthEast();
    }
    if (ByteReceived == '3') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
      East();
    }
    if (ByteReceived == 'x') // Single Quote! This is a character.
    {
      digitalWrite(motor, LOW);
      Serial.print(" motor OFF ");
    }
  }

  NorthEast();
  East();
}


void NorthEast(){
  while (ByteReceived == '2') // Single Quote! This is a character.
  { // read the pushbutton input pin:
    buttonState1 = digitalRead(buttonPin1);
    // compare the buttonState to its previous state
    if (buttonState1 != lastButtonState1) {
      // if the state has changed, increment the counter
      if (buttonState1 == HIGH) {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter1++;
        Serial.println("on");
        Serial.print("number of button2 pushes:  ");
        Serial.println(buttonPushCounter1);
      }
      else {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter1 >= 2) buttonPushCounter1 = 0;
    }

    // read the second pushbutton input pin:
    buttonState2 = digitalRead(buttonPin2);
    // compare the buttonState to its previous state
    if (buttonState2 != lastButtonState2) {
      // if the state has changed, increment the counter
      if (buttonState2 == HIGH) {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter2++;
        Serial.println("on");
        Serial.print("number of button1 pushes:  ");
        Serial.println(buttonPushCounter2);
      }
      else {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter2 >= 4) buttonPushCounter2 = 0;
    }

    // save the current state as the last state,
    //for next time through the loop
    lastButtonState1 = buttonState1;
    lastButtonState2 = buttonState2;

    motorstate = digitalRead(motor);
    if (buttonPushCounter1 == 1 && buttonPushCounter2  == 2) { // this is the line im having trouble with 
      digitalWrite(ledPin1, HIGH);
      digitalWrite(motor, LOW);
      if (motorstate != lastmotorstate) Serial.print(" motor OFF ");
      else digitalWrite(ledPin1, LOW);
    }
  }
}

void East(){
  while (ByteReceived == '3') // Single Quote! This is a character.
  { // read the pushbutton input pin:
    buttonState1 = digitalRead(buttonPin1);
    // compare the buttonState to its previous state
    if (buttonState1 != lastButtonState1) {
      // if the state has changed, increment the counter
      if (buttonState1 == HIGH) {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter1++;
        Serial.println("on");
        Serial.print("number of button2 pushes:  ");
        Serial.println(buttonPushCounter1);
      }
      else {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter1 >= 2) buttonPushCounter1 = 0;
    }

    // read the second pushbutton input pin:
    buttonState2 = digitalRead(buttonPin2);
    // compare the buttonState to its previous state
    if (buttonState2 != lastButtonState2) {
      // if the state has changed, increment the counter
      if (buttonState2 == HIGH) {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter2++;
        Serial.println("on");
        Serial.print("number of button1 pushes:  ");
        Serial.println(buttonPushCounter2);
      }
      else {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter2 >= 4) buttonPushCounter2 = 0;
    }

    // save the current state as the last state,
    //for next time through the loop
    lastButtonState1 = buttonState1;
    lastButtonState2 = buttonState2;

    motorstate = digitalRead(motor);
    if (buttonPushCounter1 == 1 && buttonPushCounter2 == 3) {
      digitalWrite(ledPin1, HIGH);
      digitalWrite(motor, LOW);
      if (motorstate != lastmotorstate) Serial.print(" motor OFF ");
      else digitalWrite(ledPin1, LOW);
    }
  }
}

Have a look on the changes I have commented in your code.

void loop() {
  if (Serial.available() > 0)
  {
    ByteReceived = Serial.read();
    Serial.print(ByteReceived);
    Serial.print("        ");
    Serial.print(ByteReceived, HEX);
    Serial.print("       ");
    Serial.println(char(ByteReceived)); //           < ----------------------------------------------------------line feed

    if (ByteReceived == 'o') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
    }

    if (ByteReceived == '2') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
      NorthEast();
    }
    if (ByteReceived == '3') // Single Quote! This is a character.
    {
      digitalWrite(motor, HIGH);
      Serial.print(" motor ON ");
      East();
    }
    if (ByteReceived == 'x') // Single Quote! This is a character.
    {
      digitalWrite(motor, LOW);
      Serial.print(" motor OFF ");
    }
    Serial.println(" ");      // prints a linefeed          < ----------------------------------------------------------line feed
  }

  NorthEast();
  East();
}


void NorthEast(){
  while (ByteReceived == '2') // Single Quote! This is a character.
  { // read the pushbutton input pin:
    buttonState1 = digitalRead(buttonPin1);
    // compare the buttonState to its previous state
    if (buttonState1 != lastButtonState1)
    {
      // if the state has changed, increment the counter
      if (buttonState1 == HIGH)
      {
        // if the current state is HIGH then the button
        // went from off to on:
        buttonPushCounter1++;
        Serial.println("on");
        Serial.print("number of button2 pushes:  ");
        Serial.println(buttonPushCounter1);
      }
      else
      {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter1 >= 2) buttonPushCounter1 = 0;
      lastButtonState1 = buttonState1;  // update memorised button  < ----------------------------------------------------------update button state
    }

    // read the second pushbutton input pin:
    buttonState2 = digitalRead(buttonPin2);
    // compare the buttonState to its previous state
    if (buttonState2 != lastButtonState2)
    {
      // if the state has changed, increment the counter
      if (buttonState2 == HIGH)
      {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter2++;
        Serial.println("on");
        Serial.print("number of button1 pushes:  ");
        Serial.println(buttonPushCounter2);
      }
      else
      {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter2 >= 4) buttonPushCounter2 = 0;
      lastButtonState2 = buttonState2;  // update memorised button  < ----------------------------------------------------------update button state
    }

    // save the current state as the last state,
    //for next time through the loop
    // lastButtonState1 = buttonState1;  //
    // lastButtonState2 = buttonState2;  // wrong, only update these if changed  < -------------------------------------------------dont update state

    motorstate = digitalRead(motor);
    if (buttonPushCounter1 == 1 && buttonPushCounter2  == 2)
    { // this is the line im having trouble with 
      digitalWrite(ledPin1, HIGH);
      digitalWrite(motor, LOW);
      if (motorstate != lastmotorstate)
      {
        Serial.print(" motor OFF ");
        lastmotorstate = motorstate;  // Update your motor state < ---------------------------------------------------------------- update motor state
      }
      else digitalWrite(ledPin1, LOW);
    }
  }
}

void East(){
  while (ByteReceived == '3') // Single Quote! This is a character.
  { // read the pushbutton input pin:
    buttonState1 = digitalRead(buttonPin1);
    // compare the buttonState to its previous state
    if (buttonState1 != lastButtonState1)
    {
      // if the state has changed, increment the counter
      if (buttonState1 == HIGH)
      {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter1++;
        Serial.println("on");
        Serial.print("number of button2 pushes:  ");
        Serial.println(buttonPushCounter1);
      }
      else
      {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter1 >= 2) buttonPushCounter1 = 0;
      lastButtonState1 = buttonState1;  // update memorised button  < ----------------------------------------------------------update button state
    }

    // read the second pushbutton input pin:
    buttonState2 = digitalRead(buttonPin2);
    // compare the buttonState to its previous state
    if (buttonState2 != lastButtonState2)
    {
      // if the state has changed, increment the counter
      if (buttonState2 == HIGH)
      {
        // if the current state is HIGH then the button
        // wend from off to on:
        buttonPushCounter2++;
        Serial.println("on");
        Serial.print("number of button1 pushes:  ");
        Serial.println(buttonPushCounter2);
      }
      else
      {
        // if the current state is LOW then the button
        // wend from on to off:
        Serial.println("off");
      }
      if (buttonPushCounter2 >= 4) buttonPushCounter2 = 0;
      lastButtonState2 = buttonState2;  // update memorised button  < ----------------------------------------------------------update button state
    }

    // save the current state as the last state,
    //for next time through the loop
    lastButtonState1 = buttonState1;  //
    lastButtonState2 = buttonState2;  // wrong, only update these if changed  < ----------------------------------------------------dont update state

    motorstate = digitalRead(motor);
    if (buttonPushCounter1 == 1 && buttonPushCounter2 == 3) {
      digitalWrite(ledPin1, HIGH);
      digitalWrite(motor, LOW);
      if (motorstate != lastmotorstate)
      {
        Serial.print(" motor OFF ");
        lastmotorstate = motorstate;  // Update your motor state < ---------------------------------------------------------------- update motor state
      }
      else digitalWrite(ledPin1, LOW);
    }
  }
}