Go Down

Topic: Reading an encoder using interrupts (Read 879 times) previous topic - next topic

NJavrGuy

Everyone,

I am trying to read from an encoder using interrupts.

For some reason it is only working in one direction, not the other direction. It never seems to negative decrement and for the life of me I cannot figure it out.

Would anyone mind taking a look at my code and seeing if you see something obvious?

The VNH5019MC is just a motor controller that causes it to move.

The way the MC works is a positive value between 1 and 400 goes in one direction, and -400 to -1 goes the opposite direction.

Either positive or negative values are working as my motor is going in the right direction, but my encoder counter is only going positive.

As an FYI I am resetting at 0 because this application will only have a 360 degree rotation limit when I get the code working properly.

Any help would be much appreciated.
Code: [Select]


#include "VNH5019MC.h"
VNH5019MC md;


boolean AisHigh,BisHigh;

#define encoderPinA 2
#define encoderPinB 3

short CPR=32; //counts per revolution base
short GR=131; //Gear Ratio

volatile int encoderCount = 0; //Will count my encoder ticks

short DD=2; //debounce delay

boolean rotating; //debouncing boolean




void setup() {
 
// digitalWrite(2, HIGH);  // turn on pullup resistor
  //digitalWrite(3, HIGH);  // turn on pullup resistor
 
  AisHigh=false; //set boolean for AisHigh = false as starting point
  BisHigh=false; // ""   "" for BisHigh
 
  attachInterrupt(0,riseA,RISING);
  attachInterrupt(0,fallA,FALLING);
  attachInterrupt(1,riseB,RISING);
  attachInterrupt(1,fallB,FALLING);
 
  Serial.begin(9600);
  md.init();
}



void loop() {
 
  rotating=true; //reset debounce boolean
 
 
  goToPos(500,-50); //make motor move CCW at neg 50
 
  Serial.println(encoderCount);
 
}


void goToPos(int encPos,int speed) {
 
    md.setM1Speed(speed);

 
}



void riseA() {
 
  // debounce
  if ( rotating ) delay (DD);  // wait a little until the bouncing is done
 
  AisHigh=true; //found rise set true
 
  iterate();
  /*
  if(BisHigh) {
    encoderCount+=1;
  }
  else {
    encoderCount-=1;
  }
  */
 
  rotating=false;
 
  //Serial.print("RiseA ");
  //Serial.println(AisHigh);
 
}

void fallA() {
  // debounce
  if ( rotating ) delay (DD);  // wait a little until the bouncing is done
 
  AisHigh=false;
 
  iterate();
 
  /*
  if(BisHigh) {
    encoderCount-=1;
  }
  else {
    encoderCount+=1;
  }
    */
   
  rotating=false;
 
  //Serial.print("FallA ");
  //Serial.println(AisHigh);
 
}


void riseB() {
  // debounce
  if ( rotating ) delay (DD);  // wait a little until the bouncing is done
 
  BisHigh=true;
 
  iterate();
  /*
   if(AisHigh) {
    encoderCount+=1;
  }
  else {
    encoderCount-=1;
  }
  */
  rotating=false;
 
  //Serial.print("RiseB ");
  //Serial.println(BisHigh);
 
}


void fallB() {
  // debounce
  if ( rotating ) delay (DD);  // wait a little until the bouncing is done
 
  BisHigh=false;
 
  iterate();
 
  /*
 
  if(AisHigh) {
    encoderCount-=1;
  }
  else {
    encoderCount+=1;
  }
 
  */
 
  rotating=false;
 
  // Serial.print("FallB ");
  //Serial.println(BisHigh);
}

void iterate() {
 
  if(AisHigh == BisHigh) {
    encoderCount+=1;
  }
  else {
    encoderCount-=1;
  }
 
  if(encoderCount>=CPR*GR || encoderCount<=CPR*GR*-1) {
    //implies we have gone one full rotation
    //Reset the encoderCount
    encoderCount=0;
  }
 
// Serial.println("I");
 
}



AWOL

Cañ you really have two ISRs attached to one interrupt source?
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

Vaclav

I did not study your code - is your direction detector signed variable?
I am using Gray code and  direction detection  works ( as it should) when encoder  A are B are attached to Int0 /int1.

retrolefty


Cañ you really have two ISRs attached to one interrupt source?


No. Only one defined trigger action allowed per attachment declaration.


Vaclav

Code: [Select]
volatile int encoderCount = 0; //Will count my encoder ticks
This needs to be signed int to detect negative rotation

AWOL

Quote
No. Only one defined trigger action allowed per attachment declaration.
(I knew that, it was just a hint to the OP to check for themself)



I did not study your code

We have enough problems just getting people to post code, so when they do, I think it is only courteous (there's that word again) to read it, don't you?
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

AWOL


Code: [Select]
volatile int encoderCount = 0; //Will count my encoder ticks
This needs to be signed int to detect negative rotation

{Facepalm}
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

lar3ry


Code: [Select]
volatile int encoderCount = 0; //Will count my encoder ticks
This needs to be signed int to detect negative rotation

I thought an int was always signed unless declared unsigned.
There are 10 kinds of people in the world,
those who understand binary, and those who don't.

NJavrGuy

Everyone, thanks.

I did not realize you could only attach one ISPR per interrupt, that does make sense though.

I will try to tweak this using a change.


PaulS

If you know which direction the motor is rotating, because you told it to rotate that way, do you really need to get direction from the encoder?

Some links to the motor controller and encoder, if it's a separate device, would be good.

NJavrGuy


If you know which direction the motor is rotating, because you told it to rotate that way, do you really need to get direction from the encoder?

Some links to the motor controller and encoder, if it's a separate device, would be good.


Paul, the reason I want to know direction is I am making an application with a home built linear actuator that I am hoping will NOT be backdriveable without very high forces applied. So, my hope is that I can turn off my motors at times, and if a force is very high it will actually back drive my motors, rather than destroy my motors. In this case I want to be able to note exact positioning and the only way I can see doing that would be by reading the encoder pos either positive or negative. Hopefully I am explaining this properly, and perhaps I am overcomplicating this and do not need both directions, but I cannot see how at this point.

Regarding the MC and encoder, I am using this Pololu MC:

http://www.pololu.com/product/1451

And this motor/encoder combo also from Pololu:

http://www.pololu.com/product/1447

I have re-written the code as follows, and I am now getting directional position, but I am quite sure that there is a problem somewhere. This encoder is supposed to be 64 CPR, and with a 131:1 gear box, I should be seeing about 8384 counts per output shaft rotation, and I am seeing about 1000 or so.

Code: [Select]
#include "VNH5019MC.h"
VNH5019MC md;


boolean AisHigh,BisHigh;

#define encoderPinA 2
#define encoderPinB 3

short CPR=64; //counts per revolution base
short GR=131; //Gear Ratio

volatile  int encoderCount = 0;

short DD=2; //debounce delay

boolean rotating; //debouncing boolean




void setup() {
  pinMode(encoderPinA, INPUT);
  pinMode(encoderPinB, INPUT);


  digitalWrite(encoderPinA, HIGH);  // turn on pullup resistor
  digitalWrite(encoderPinA, HIGH);  // turn on pullup resistor
 
  AisHigh=false;  //set AisHigh bool to false to set it to low as a start
  BisHigh=false;  //""  BisHigh ""
 
  attachInterrupt(0,changeA,CHANGE);
 
  attachInterrupt(1,changeB,CHANGE);
 
 
  Serial.begin(9600);
  md.init();
}



void loop() {
 
  rotating=true; // reset debunker boolean
 
 
  goToPos(500,50); //set speed and direction
 
  Serial.println(encoderCount);
 
}


void goToPos(int encPos,int speed) {
 
    md.setM1Speed(speed);

 
}

void changeA() {
 
  // debounce
  if ( rotating ) delay (DD);  // wait a little until the bouncing is done
 
  //first toggle the AisHigh boolean
 
  if(digitalRead(encoderPinA)==LOW) AisHigh=false;
  else AisHigh=true;
 
 
  iterate();
 
  rotating=false;
 
}

void changeB() {
 
  // debounce
  if ( rotating ) delay (DD);  // wait a little until the bouncing is done
 
  //first toggle the BisHigh boolean
  if(digitalRead(encoderPinB)==LOW) BisHigh=false;
  else BisHigh=true;
 
 
  iterate();
 
  rotating=false;

 
}



void iterate() {
 
  if(AisHigh == BisHigh) {
    encoderCount+=1;
  }
  else {
    encoderCount-=1;
  }
 
  if(encoderCount>=CPR*GR || encoderCount<=CPR*GR*-1) {
    //implies we have gone one full rotation
    //Reset the encoderCount
    encoderCount=0;
  }
 
// Serial.println("I");
 
}

PaulS

Code: [Select]
  attachInterrupt(0,changeA,CHANGE);
  attachInterrupt(1,changeB,CHANGE); 

So, changeA() and changeB() are interrupt service routines.

Code: [Select]
void changeA() {
 
  // debounce
  if ( rotating ) delay (DD);  // wait a little until the bouncing is done

You can't use delay() in an ISR. ISRs are supposed to be fast. delay() is the exact opposite of fast.


Code: [Select]
void iterate()
{
// Serial.println("I");
}

I know that Serial.print() is commented out, but Serial.print() relies on interrupts, which are disabled during an ISR. You should never call Serial.print() from an ISR. Not even for debugging purposes.

solderspot


Interestingly enough I just added encoders to my project.

The counts per revolution are very high. They will generate an interrupt around once every 120 microseconds. You do not want to be, and should not need to debounce the signals. If your counts seem low it's because you are not servicing the interrupts fast enough.

This is the code I used to read my encoder pololu encoder: http://www.pololu.com/product/1217

It uses a timer interrupt to sample the encoder output but in your case you would use the pin interrupts. The code is also setup to service two encoders, one for each of the bot's two motors.

Code: [Select]

//----------------------------------------
//
//----------------------------------------

volatile int16_t _lft_ticks = 0;
volatile int16_t _rht_ticks = 0;
volatile uint16_t _counter = 0;
volatile bool _error = false;
char _lApin;
char _lBpin;
char _rApin;
char _rBpin;

//----------------------------------------
//
//----------------------------------------

void setup_encoder( char lftPinA, char lftPinB, char rhtPinA, char rhtPinB )
{
  _init_pin( &_lApin, lftPinA);
  _init_pin( &_lBpin, lftPinB);
  _init_pin( &_rApin, rhtPinA);
  _init_pin( &_rBpin, rhtPinB);

  cli();
  // set timer2 interrupt at 1000 Hz
// we are assuming a clk speed of 16MHz
  TCCR2A = 0;
  TCCR2B = 0;
  TCNT2  = 0;
  OCR2A = 249;
  TCCR2A |= (1 << WGM21);
  TCCR2B |= (1 << CS22);   
  TIMSK2 |= (1 << OCIE2A);
  sei();
}

//----------------------------------------
//
//----------------------------------------

void _init_pin( char *pin, char value)
{
  *pin = value;
  pinMode(value, INPUT);
  digitalWrite( value, HIGH);
}

//----------------------------------------
//
//----------------------------------------

bool get_ticks_since_last( int16_t *lft, int16_t *rht, uint16_t *ms )
{
  cli();
  *lft = _lft_ticks;
  *rht = _rht_ticks;
  *ms = _counter;
  _lft_ticks = _rht_ticks = _counter = 0;
  char error = _error;
  _error = false;
  sei();

  return !error;
}

//----------------------------------------
//
//----------------------------------------

void clear_ticks()
{
  cli();
  _lft_ticks = _rht_ticks = _counter = 0;
  _error = false;
  sei();
}

//----------------------------------------
//
//----------------------------------------

ISR(TIMER2_COMPA_vect)
{
  // this routine gets called once every 1 millisecond
  _counter++;

  static char lastLA = 0;
  static char lastLB = 0;
  static char lastRA = 0;
  static char lastRB = 0;

  _process(_lApin, _lBpin, &lastLA, &lastLB, &_lft_ticks);
  _process(_rApin, _rBpin, &lastRA, &lastRB, &_rht_ticks);
}

//----------------------------------------
//
//----------------------------------------

void _process( char Apin, char Bpin, char *lastA, char *lastB, volatile int16_t *ticks )
{
  char A = (digitalRead( Apin) == HIGH) ? 1 : 0;
  char B = (digitalRead( Bpin) == HIGH) ? 1 : 0;
  char lA = *lastA;
  char lB = *lastB;
  char dA = A!=lA;
  char dB = B!=lB;

  if( dA && dB )
  {
    // both should not change at the same time
    _error = true;
  }
  else if ( dA || dB )
  {
    if (A^lB)
    {
      *ticks += 1;
    }
    else if(B^lA)
    {
      *ticks -= 1;
    }
  }
  *lastA = A;
  *lastB = B;
}



Hope that is somewhat helpful. I explain a lot of it here: http://wp.me/p493sy-7L
solderspot
journal: http://solderspot.wordpress.com
github: https://github.com/solderspot

Nick Gammon

Code: [Select]

  _lft_ticks = _rht_ticks = _counter = 0;
  _error = false;


What's with the leading underscores? Wouldn't it be easier, and less work, and more readable, to omit them?
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Nick Gammon

Code: [Select]

char _lApin;
char _lBpin;
char _rApin;
char _rBpin;


According to the C++ standard, variables in the global namespace, starting with an underscore, are reserved.

Quote

17.4.3.2.1 Global names

Certain sets of names and function signatures are always reserved to the implementation:

Each name that contains a double underscore (_ _) or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use.

Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Go Up