DC motor control using single bit motor encoder

Using Arduino Motors, Mechanics, Power and CNC
#1

I got my first Arduino kit (Arduino Mega 2560) and i am trying to control the position a 24V DC motor using a single bit encoder. The encoder has got a low resolution of 30 pulses/rev. I am using a dual DC motor driver to drive the motor. I have done the coding as shown below. But the motor is not working. I have provided common ground to the DC motor driver, encoder and Arduino by connecting the ground pins of motor driver and encoder to the Arduino. The encoder which needs 5V is provided from the Arduino 5V pin. Please see the code. It is supposed to stop the motor after one rotation is completd.

int motor1PinA = 14;
int motor1PinB = 15;
int motorpwm = 3;
int encoder1Pin = 2;
volatile int encoder1Pos = 0;
int theta1 = 360;
void setup() {
  pinMode(motor1PinA, OUTPUT);
  pinMode(motor1PinB, OUTPUT);
  pinMode(motorpwm, OUTPUT);
  pinMode(encoder1Pin, INPUT);
  attachInterrupt(0, encoder1svc, HIGH);
  digitalWrite(motor1PinA, LOW);
  digitalWrite(motor1PinB, LOW);
  digitalWrite(motorpwm, LOW);
}

void loop() {
  digitalWrite(motor1PinA, HIGH);
  if (encoder1Pos*12 >= theta1)
  digitalWrite(motor1PinB, HIGH);
  encoder1Pos = 0;
}
void encoder1svc()
{
  if (digitalRead(encoder1Pin) == HIGH)
    encoder1Pos++;
}
#2

It would help if you provide a link to the specifications for your motor and encoder AND provide a diagram of how everything is connected. A photo of a clear pencil sketch will be fine.

Would it be better to trigger the interrupt with a RISING pulse? I don't know if your present code will give several interrupts while the encoder output remains HIGH?

At first glance your code looks more suited to a stepper motor than a DC motor. I don't know whether it is possible to get a DC motor to move exactly to a particular step as there would be nothing to hold it there.

Wouldn't it be much easier to do encoder1Pos += 12; and not bother with multiplication.

...R

#3

It pays to read the documentation before using a function:

  attachInterrupt(0, encoder1svc, HIGH);

The correct 2nd argument to attachInterrupt is either:
LOW, CHANGE, FALLING or RISING. "HIGH" is meaningless in this context,
but by chance will behave as "CHANGE"

Using "LOW" will cause the interrupt to continuously retrigger while the pin is low,
the others fire just once for each appropriate event.

#4