DC Motor with encoder, inconsistent reading

I have a dc Motor with a quadrature encoder attached. The reading are inconsistent and it often over shoots and sometimes it under shoots. At the lowest operating speed I can get it to be pretty consistent, 90+% it hits the mark. The higher the speed the worse the reading. When pwn is about 100 its all but useless.

I suspect it might be an issue with the interrupt since after completing the ISR it goes back to where it was in the loop. Meaning it might move a fraction of a second longer than it should before starting over and rechecking the condition. Is there a way to force the loop to restart? Or a better way to handle stopping the motors when certain conditions are meet?

#include "Motor.h"
#include "Delay.h"

//167 ticks per rotation.  
//Circumfrence of wheel 7
//resolution .04 inches
//degree per tick 2.2

#define rightEncoderA 0 
#define rightEncoderB 1 

#define leftEncoderA 2 
#define leftEncoderB 3 
int enablePinLeft = 5; //h bridge enable pin 1 for right motor
int leftInput1 = 4; // h bridge input 1 for right motor
int leftInput2 = 7; // h bridge input 2 for right motor

int enablePinRight = 6; //h bridge enable pin 2 for left motor
int rightInput1 = 8; // h bridge input 3 for left motor
int rightInput2 = 9; // h bridge input 4 for left motor

volatile double ticks;

bool move;

Motor rightMotor(enablePinRight, rightInput1, rightInput2, rightEncoderA, rightEncoderB, 5);
Motor leftMotor(enablePinLeft, leftInput1, leftInput2, leftEncoderA, leftEncoderB, 1);

CustomDelay customDelay;

ISR(INT1_vect)
{
	if (ticks < 80)
	{
		ticks++;
	}
	else
	{
		leftMotor.SetSpeed(0);
		leftMotor.SetDirection(OFF);
		move = false;
	}
}

void setup()
{
	Serial.begin(115200);
	ticks = 0;
	move = true;

	leftMotor.SetDirection(FORWARD);
	leftMotor.SetSpeed(75);

	DDRD = DDRD | B00000000;
	EICRA &= ~(bit(ISC10) | bit(ISC11));  // clear existing flags
	EICRA |= (bit(ISC10) | bit(ISC11));    // set wanted flags (falling level interrupt)
	EIMSK |= bit(INT1);
}

void loop()
{
	if (!move)
	{
		customDelay.Wait(1500);
		leftMotor.SetDirection(FORWARD);
		leftMotor.SetSpeed(75);
		move = true;
		ticks = 0;
	}
}

The code is below. Its just a wait that does not hold up the whole processor. Its only called after the motors are stopped. So the ISR stops the motors(the else part), then the loop waits 1.5 seconds, resets the counters and starts the motors again.

void CustomDelay::Wait(long interval)
{
	long timer = millis();
	while ((millis() - timer) < interval)
	{

	}
}

I plan to implement more in that later but right now I am just trying to make the motor control work before moving on.

I've wanted to add a tachometer/encoder to my interrupt routines and so I did and here you go:

This should work well My encoder has 400 clock steps per revolution and it works well.

#include "Interrupts.h"
#define ClockPin 2
#define DataPin 9
      // My Encoder has 400 Clock pulses per revolution
      // note that 150000.0 = (60 seonds * 1000000 microseconds)microseconds in a minute / 400 pulses in 1 revolution)
#define Multiplier 150000.0 // don't forget a decimal place to make this number a floating point number

InterruptsClass Interrupt;

volatile long EncoderCounter = 0;
volatile float SpeedInRPM = 0;

void onInterruptCallBackFunction(uint32_t Time, uint32_t PinsChanged, uint32_t Pins){
    Interrupt.PinCallBack(Time, PinsChanged, Pins);
}

void onPin2CallBackFunction(uint32_t Time, uint32_t PinsChanged, uint32_t Pins){
    int32_t DT;
    if ((DT = Interrupt.Tachometer( ClockPin, DataPin, Time, Pins))) { // returns time between pulses
      EncoderCounter += (Interrupt.CheckPin(DataPin)) ? 1 : -1; // Increments the encoder
      SpeedInRPM = Multiplier / DT;
    } 
 //   else EncoderCounter += (Interrupt.CheckPin(DataPin)) ? -1 : 1; // Increments the encoder 1/2 step
}

void setup() {
  Serial.begin(115200); 
  pinMode(DataPin, INPUT);
  Interrupt.onInterrupt(onInterruptCallBackFunction); // end of onInterrupt() function call
  Interrupt.onPin(ClockPin, INPUT,onPin2CallBackFunction );// End of onPin Function call
}

void loop() {
  static unsigned long SpamTimer;
  if ( (unsigned long)(millis() - SpamTimer) >= (100)) {
    SpamTimer = millis();
    Serial.print(EncoderCounter);
    Serial.print("\t");
    Serial.print(SpeedInRPM, 3);
    Serial.print(" RPM");
    Serial.println();
    SpeedInRPM = 0; // if no pulses occure in the next 100 miliseconds then we must assume that the motor has stopped
  }
}

Z
---------------- also -------------
You can pick any pin for your clock and data pin Example:
#define ClockPin A3
#define DataPin 4
And you can easily add additional encoders
There is code in my InterruptsClass to handle Ultrasonic Ping sensors, Switches and RC Radio Receiver input so look around :slight_smile:

Interrupts.cpp (6.38 KB)

Interrupts.h (5.39 KB)

I have not had a chance to test this, but looking at the code it looks like the clock pin would be the pin connected to my encoder pin A(or b I suppose) but what is the data pin connected to on the encoder?

AgentNoise:
I have not had a chance to test this, but looking at the code it looks like the clock pin would be the pin connected to my encoder pin A(or b I suppose) but what is the data pin connected to on the encoder?

Use a for Clock and B for Data or you can switch it they are alternating switches it doesn't matter :slight_smile:

Use a for Clock and B for Data or you can switch it they are alternating switches it doesn't matter :slight_smile:

Fantastic, thank you. Again, I have not tested this yet but could you provide a break down of why this is more efficient than the direct ISR solution I was trying. Would like to understand why the code works for future use.

AgentNoise:
Fantastic, thank you. Again, I have not tested this yet but could you provide a break down of why this is more efficient than the direct ISR solution I was trying. Would like to understand why the code works for future use.

My code gives you a encoder counter and RPM (my encoder I tested it with has 400 steps per revolution!) and you can use any pins you want. also you can add 3,4,5,6 encoders at 2 pins each and my code should be able to handle all!
It uses ISR's just not the dedicated hardware ones. It is close to as efficient as using the hardware ones and it is more flexible. If you can use it great I think you will like it. also if you need other interrupts like ping or RC Remote I have code included to handle those. let me know if you need an example of how to add them.

Is there a library that you installed?

digbaddy:
Is there a library that you installed?

Are you asking Me ZhomeSlice or the original poster AgentNoise?
My Libraries I offered AgentNoise for interrupts are provided on Post #6
Z

zhomeslice:
My code gives you a encoder counter and RPM (my encoder I tested it with has 400 steps per revolution!) and you can use any pins you want. also you can add 3,4,5,6 encoders at 2 pins each and my code should be able to handle all!
It uses ISR's just not the dedicated hardware ones. It is close to as efficient as using the hardware ones and it is more flexible. If you can use it great I think you will like it. also if you need other interrupts like ping or RC Remote I have code included to handle those. let me know if you need an example of how to add them.

I can't get your code to work. I get an error "Interrupts.h: 114:206: error: invalid conversion from 'void ()()' to 'InterruptsClass::voidFuncPtr {aka void ()(long unsigned int, long unsigned int, long unsigned int)}' [-fpermissive]
"

That line of code is

voidFuncPtr  PinCB[20] = {nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing, nothing}; // Key = Pin Val = Position List of pins that could be used as ping inputs:

Which does not see correct to me. I just copied the code you provided for a quick test and that's the error I get.

After editing it to change nothing to null I get this big ol' error dump

In file included from sketch\Interrupts.cpp:6:0:

sketch\Interrupts.h:96:0: warning: "Pin18" redefined [enabled by default]

 #define Pin18 22

 ^

sketch\Interrupts.h:94:0: note: this is the location of the previous definition

 #define Pin18 20

 ^

sketch\Interrupts.h:97:0: warning: "Pin19" redefined [enabled by default]

 #define Pin19 23

 ^

sketch\Interrupts.h:95:0: note: this is the location of the previous definition

 #define Pin19 21

 ^

sketch\Interrupts.cpp:12:31: error: invalid conversion from 'void (*)()' to 'voidFuncPtr {aka void (*)(long unsigned int, long unsigned int, long unsigned int)}' [-fpermissive]

 volatile voidFuncPtr Int_CB = nothing;

                               ^

In file included from D:\Program Files\Arduino\hardware\arduino\avr\cores\arduino/Arduino.h:30:0,

                 from sketch\Interrupts.cpp:2:

sketch\Interrupts.cpp: In function 'void PCINT2_vect()':

sketch\Interrupts.cpp:31:5: warning: 'PCINT2_vect' appears to be a misspelled signal handler [enabled by default]

 ISR(PCINT2_vect) { //this ISR pins 0-7

     ^

sketch\Interrupts.cpp: In function 'void PCINT1_vect()':

sketch\Interrupts.cpp:34:5: warning: 'PCINT1_vect' appears to be a misspelled signal handler [enabled by default]

 ISR(PCINT1_vect) { //this ISR s A0~A5

     ^

sketch\Interrupts.cpp: In function 'void TIMER2_COMPA_vect()':

sketch\Interrupts.cpp:69:5: warning: 'TIMER2_COMPA_vect' appears to be a misspelled signal handler [enabled by default]

 ISR(TIMER2_COMPA_vect)    

     ^

sketch\Interrupts.cpp: In function 'void TIMER2_OVF_vect()':

sketch\Interrupts.cpp:77:5: warning: 'TIMER2_OVF_vect' appears to be a misspelled signal handler [enabled by default]

 ISR(TIMER2_OVF_vect)  

     ^

sketch\Interrupts.cpp: In member function 'InterruptsClass& InterruptsClass::Timer2Enable()':

sketch\Interrupts.cpp:95:2: error: 'OCR2A' was not declared in this scope

  OCR2A = 0xAF;

  ^

exit status 1
Error compiling for board Arduino Leonardo.

AgentNoise:
After editing it to change nothing to null I get this big ol' error dump

In file included from sketch\Interrupts.cpp:6:0:

sketch\Interrupts.h:96:0: warning: "Pin18" redefined [enabled by default]

#define Pin18 22

^

sketch\Interrupts.h:94:0: note: this is the location of the previous definition

#define Pin18 20

^

sketch\Interrupts.h:97:0: warning: "Pin19" redefined [enabled by default]

#define Pin19 23

^

sketch\Interrupts.h:95:0: note: this is the location of the previous definition

#define Pin19 21

^

sketch\Interrupts.cpp:12:31: error: invalid conversion from 'void ()()' to 'voidFuncPtr {aka void ()(long unsigned int, long unsigned int, long unsigned int)}' [-fpermissive]

volatile voidFuncPtr Int_CB = nothing;

^

In file included from D:\Program Files\Arduino\hardware\arduino\avr\cores\arduino/Arduino.h:30:0,

from sketch\Interrupts.cpp:2:

sketch\Interrupts.cpp: In function 'void PCINT2_vect()':

sketch\Interrupts.cpp:31:5: warning: 'PCINT2_vect' appears to be a misspelled signal handler [enabled by default]

ISR(PCINT2_vect) { //this ISR pins 0-7

^

sketch\Interrupts.cpp: In function 'void PCINT1_vect()':

sketch\Interrupts.cpp:34:5: warning: 'PCINT1_vect' appears to be a misspelled signal handler [enabled by default]

ISR(PCINT1_vect) { //this ISR s A0~A5

^

sketch\Interrupts.cpp: In function 'void TIMER2_COMPA_vect()':

sketch\Interrupts.cpp:69:5: warning: 'TIMER2_COMPA_vect' appears to be a misspelled signal handler [enabled by default]

ISR(TIMER2_COMPA_vect)

^

sketch\Interrupts.cpp: In function 'void TIMER2_OVF_vect()':

sketch\Interrupts.cpp:77:5: warning: 'TIMER2_OVF_vect' appears to be a misspelled signal handler [enabled by default]

ISR(TIMER2_OVF_vect)

^

sketch\Interrupts.cpp: In member function 'InterruptsClass& InterruptsClass::Timer2Enable()':

sketch\Interrupts.cpp:95:2: error: 'OCR2A' was not declared in this scope

OCR2A = 0xAF;

^

exit status 1
Error compiling for board Arduino Leonardo.

My code is designed for the arduino UNO atmega328p and I haven't tested it on other boards. adapting it to work with other boards shouldn't be too difficult.
What board are you compiling it on?
Z

I am compiling it on a leonardo

AgentNoise:
I am compiling it on a leonardo

That's right.. I do not have a Leonardo to test my interupt code with. I'm sure the interrupt triggers are different. The setup of the interrupts isn't as difficult as it is confusing. So for me without something to test the code in I'm afraid I have little to offer. Thanks for letting me know that it isn't useful on a Leonardo.

Ill look at porting it to the Leonardo. Let you know when/if I get it working.

Hi,
What is your encoder?
Can you post a link to its data/specs?

I notice you do not turn PULL_UP ON for the encoder input pins, how is your encoder output configured?
Is the encoder open collector and/or have you got external PULL_UPs if they are needed.

Can you please post a copy of your circuit, in CAD or a picture of a hand drawn circuit in jpg, png?

Thamks.. Tom... :slight_smile: