Trouble with Counting Rotary Encoder

phillipstearns · December 8, 2021, 2:03am

I'm using this motor with a hall-effect encoder attached. I've contacted Pololu support and in response to a request for a datasheet to help understand how the sensor output is achieved from an engineering perspective, they only have this to say:

My controller is an Arduino Nano. I'm powering it directly on the 5V pin, bypassing the regulator. This same 5V is powering the encoder.

I'm using pin change interrupts to detect and decode the rotary encoder signals. I have a Raspberry Pi requesting count values over serial at a rate of 0.01s.

I'm wondering if there are conflicts between ATMega328 pin change interrupts and the current implementation of Arduino's Serial library. Specifically anything that would cause the Serial RX/TX to break pin change ISRs.

This code is working, but I experience drift when running for about an hour.

// encoder variables
volatile long enc1Count = 0;
volatile long enc2Count = 0;
volatile int enc1LastState = 0;
volatile int enc2LastState = 0;
const int stateTable[] = { 0, 1, -1, 0, -1, 0, 0, 1, 1, 0, 0, -1, 0, -1, 1, 0 };

// serial read write variables
const uint8_t buffSize = 3;
char buff[buffSize];
char request = 0;
char device = 0;

//////////////////////////////////////////////////////////////////////////////
// Pin Change Interrupt ISRs

ISR(PCINT0_vect) {
  // D8 = PINB0
  // D9 = PINB1
  volatile int enc1State = (PINB & 1) << 1 | (( PINB >> 1 ) & 1);
  volatile int stateCode = (enc1LastState << 2) | enc1State;
  volatile int encValue = stateTable[stateCode];
  if (encValue) {
    enc1LastState = enc1State;
    enc1Count += encValue;
  }
}

ISR(PCINT1_vect) {
  // D14 / ADC0 = PINC0
  // D15 / ADC1 = PINC1
  volatile int enc2State = (PINC & 1) << 1 | (( PINC >> 1 ) & 1);
  volatile int stateCode = (enc2LastState << 2) | enc2State;
  volatile int encValue = stateTable[stateCode];
  if (encValue) {
    enc2LastState = enc2State;
    enc2Count += encValue;
  }
}

//////////////////////////////////////////////////////////////////////////////
// Serial Communication

void pollSerial() {
  if (Serial.available() >= 3) {
    uint8_t bytesRead = Serial.readBytesUntil('\n', buff, buffSize);
    if (bytesRead == 2) {
      request = buff[0];
      device = buff[1];
      if (request == 'g' && device == '0') {
        Serial.print(enc1Count);
        Serial.print('|');
        Serial.println(enc2Count);
      } else if (request == 'g' && device == '1') {
        Serial.println(enc1Count);
      } else if (request == 'g' && device == '2') {
        Serial.println(enc2Count);
      } else if (request == 'c' && device == '0') {
        enc1Count = 0;
        enc2Count = 0;
      } else if (request == 'c' && device == '1') {
        enc1Count = 0;
      } else if (request == 'c' && device == '2') {
        enc2Count = 0;
      } else {
        Serial.println("err!");
      }
    }
    // clear the buffer;
    for (int i = 0; i < buffSize; ++i) {
      buff[i] = 0;
    }
  }
}

//////////////////////////////////////////////////////////////////////////////
// Setup

void setup() {
  PCICR |= 0b00000011; // enable pin change interrupts 0 and 1
  PCMSK0 = 0b00000011; // enable PCI00 (D8, PINB0) & PCI01 (D9, PINB1)
  PCMSK1 = 0b00000011; // enable PCI10 (ADC0, PINC0) & PCI11 (ADC1, PINC1)

  enc1LastState = (PINB & 1) << 1 | (( PINB >> 1 ) & 1); // PINB0 = D8, PINB1 = D9
  enc2LastState = (PINC & 1) << 1 | (( PINC >> 1 ) & 1); // PINC0 = D14/ADC0, PINC1 = D15/ADC1
  
  Serial.setTimeout(1);
  Serial.begin(115200);   // start serial for output for debugging
}

//////////////////////////////////////////////////////////////////////////////
// Loop

void loop() {
  pollSerial();
}

Should I limit the frequency that the serial input is polled? I'm really at a loss. This tracks encoder counts well enough for me to be fooled, but overtime, drifts, leading me to believe that I'm missing something either in:

the wiring of the encoder to the Arduino (something that might introduce interference)
the timing of the ISRs in relation to reading values off the PIN registers
the table based lookup used for detecting and responding to only valid encoder pin states
the timing of the Serial
any potential conflicts between the Serial library implementation and the pin change interrupt vectors I'm utilizing.

Any help would be wonderful. Thank you!

Grumpy_Mike · December 8, 2021, 4:08am

It is not the best way to do things as it takes longer to service interrupts with it, you are better off using interrupts 0 and 1 on pins 2 & 3.

What is the exact part number of your encoder?
Does it use internal or external magnets?

How do you know this and what is the size of this drift?

The code is not the best. For example you declare variables inside your ISR, this takes up more time than using global variables. You only need variables to be volatile if they are used both inside and outside the ISR.

The pin change interrupt is used with the software serial library but not the hardware one.

The code from what I see is interrupt driven you are not polling it. I think you mean sending the readings out through serial. This looks like a complex thing you are doing, and this itself could be causing the missing pulses.

You should be going through a state machine to ensure you only count valid pulses. Do you need to increase the count on each signal transition, it would be much better if you only counted and send out the value on a complete cycle of pulses.

groundFungus · December 8, 2021, 4:12am

Have you considered using a library like the Encoder library? You can have each of the encoders use one interrupt pin (interrupt 0 or 1) and one non-interrupt pin.

phillipstearns · December 8, 2021, 5:24am

Thanks for your comments and insight!

I have two motors with rotary encoders on one Nano. I could do a half count and only check the A pin for each and have them wired to INT0 and INT1. I'm trying to utilize the full count, detecting changes on both pins and responding. Using PCINT0 and PCINT1 was my solution given my constraints of one Arduino Nano, two encoders.

I'm looking at the PCINT timing diagram and it looks like it takes 3 complete clock cycles to set the interrupt flag. 187.5ns. I haven't dug deep enough to find out how many more cycles are consumed by jumping to the PCINT0_vect register and then on to executing the code therein, but it seems fast enough for me. Typically I'm dealing with a max of 1000 interrupts per second when these things are moving at their desired fastest speed.

I'm still waiting on Pololu to give me more information about the encoder attached to this motor.

Well if it's drifting it's clearly not "working", but it's doing almost what I would like.

I've been testing this code on two motors, having them spin ~270 degrees in one direction and then return to their starting point. Movement is set to take 15 seconds to complete, so 30 seconds per cycle out and back. Doing this over a half hour seems to produce the results I'm looking for. They return to where they started. However, after an hour or more, it becomes apparent that something is not right. The motors don't drift the same amount test after test. Between +/-5 and +/-15 degrees. And they seem to don't drift in the same direction test after test.

Noted. Will make that change.

I'll presume that means that the way I'm using PCINT and the standard Serial library shouldn't pose any problems?

If there's no conflict between the ISR and the Serial, then why would this cause missing counts? I'm requesting the counts from a Raspberry Pi using g0\n. Then the Arduino should respond with the counts as a string. I can pare this down considerably to use the cases I'm not using. I do however need to be able to clear the counts as well, hence the handling of the c character.

Just for clarity, the state of the encoder is saved in 2 bits. Encoder A pin state is the MSB, encoder B is LSB (0bAB). The new state is acquired by the PCINT ISRs using bit-wise operations and direct PIN register access. The last AB state and the new AB state are used to create a 4 bit value. This 4 bit value has 16 possible states, only 8 of which are valid, 4 correspond to CW motion, 4 correspond to CCW motion. This is what const int stateTable[] = { 0, 1, -1, 0, -1, 0, 0, 1, 1, 0, 0, -1, 0, -1, 1, 0 }; describes.

Are you suggesting I should setup a Switch to check only the valid states and then in each case increment or decrement the encoder count?

This is a good question. The encoder has 64 counts per revolution. The gearing is 131.25:1 so one complete revolution of the output shaft is 64 x 131.25 = 8400 counts. For my application, they can't deviate from one another by more than a degree during their range of motion. Generally the fastest angular velocity of the output shaft is about 17 degrees per second. This would produce a frequency of encoder events of roughly 400 Hz, or 2.5ms per count.

In a previous incarnation of this project, I was using an ArduinoMEGA and had 4x external interrupt pins at my disposal. The encoder counting routine only detected changes on the A pins of both, so I had half counts. That was enough, but the motion is definitely smoother with higher resolution. Halving my resolution again might not work for this project, but if it's my only option, I'll explore it.

phillipstearns · December 8, 2021, 5:35am

Thanks for linking this.

Having a look through the source code, it seems like option to try. I may not have enough time to do a thorough test before I reach my deadline, but it provides an example of the state machine @Grumpy_Mike suggested. The speed test example might be something I start with.

Grumpy_Mike · December 8, 2021, 9:40am

Thanks for clearing some things up.

That is odd, why should using less precise counts affect the movement of the motor?

The way you are testing this by turning a set angle and then returning it has three places where it could fail.

missing counts like you said.
the motor missing pulses because of the stall current being exceeded when you stop and start again in a new direction. You can try an overall slower step rate or / and a delay between stoping and reversing direction and / or accelerating from a standing start and decelerating when you are about to stop. Also to give you mor torque you could drive the motor with a higher voltage. I am assuming you have some sort of chopping regulated driver.
the problem is caused by the hysteresis of the gears. As there is such a reduction in the gears then this could take time to show.

There is nothing to stop the count changing while you are in the process of accessing the numbers to print out. As the count is an int then the processor has to generate two memory accesses to get both counts, and the interrupt can occur between those two accesses messing up the count.
Similarly when you clear the count you can get an interrupt between clearing one count byte and the next, again messing up the count. This is not considered a conflict but it is an interaction. Normally the interrupts are turned off before accessing multi byte data generated by an ISR.

JCA34F · December 8, 2021, 2:42pm

I believe a Pro Micro has 5 external interrupts.

phillipstearns · December 8, 2021, 5:02pm

That's excellent! Will keep that in mind for future projects.

phillipstearns · December 10, 2021, 12:08am

The power applied to the motors is based on the difference between the target position and the read position of the motors.

The code for calculating the target on the raspberry pi:

tCurrent = time()
tRemaining = tFinal - tCurrent
progress = 1 - ( tRemaining / tDuration )
progress = pow(sin(progress*PI/2),2)
target = (progress * (self.targetOpen - self.targetClose)) + self.targetClose)

The raspberry pi is driving the motors via this board.
The loop for updating the motor position fires with a period of 0.01s. The serial send/rcv between the raspberry pi and the arduino happen at that rate.

This might be possible... I don't understand the impact of stall current on the encoder count. The encoder is tied to the motor shaft. If it moves, the encoder state changes. Acceleration deceleration should matter. The motor might not get to exactly where it needs to be on every transition, but I don't see how this would be cumulative over time.

I'm fairly certain I'm not dealing with forces that should add up to +/-15 degrees of drift due to material hysteresis. Pololu states a 1 degree backlash, and this is generally not 15x cumulative either.

Yes, I see the counts can be changed during access between the serial print calls due to the nature of Interrupts. However, If there is no change for a period of time and the values are requested multiple times, this should not be a problem. On the other hand, I've no had an issue with the clearing of the counts. I never have received a result from the Arduino where one clears to zero and then the other on the next count request. Zeroing happens in a calibration state with zero motion. If they do not zero at the same time, it's not an issue.

This is most certainly the case. Somehow. In the previous incarnation. I was using external interrupts. The mystery that I'd like to clear is whether Serial communication would interfere with the ISR vectors PCINT0_vect or PCINT1_vect or if there are any other things that the Arduino IDE might introduce that are not immediately visible. Also, is it a better approach to use attachInterrupt() or ISR(PCINT0_vect) or, if using external interrupts, ISR(INT0_vect)?

I'm have physically altered the connections on the Arduino Nano so that each encoder A line triggers INT0 and INT1.

ISR(INT0_vect) { // attached to pin2
  enc1A = (PIND >> ENC1A_PIN) & 1; //triggered the interrupt
  enc1B = (PIND >> ENC1B_PIN) & 1;
  if (enc1A == enc1B) {
    --enc1Count;
  } else {
    ++enc1Count;
  }
}

In the setup:

EICRA |= 0b00000101; // set INT0 and INT1 to respond to any voltage changes
EIMSK |= 0b00000011; // enable INT0 and INT1

As per the datasheet.

I'm aware that I could use a switch instead of the if statement and save some CPU cycles, but in the previous version of the project, I was using digitalRead() in place of accessing the pin registers directly and did not experience drift. If it seems like I need to further optimize, I'll do it, but only if this fails.

Grumpy_Mike · December 10, 2021, 3:41am

With regard to that board it says
“ making use of the Raspberry Pi’s hardware PWM outputs”

Now I have been working with the Raspberry Pi since the original came out, and I know there is no hardware PWM in the machine. I am surprised at such a statement from this normally reliable source. All PWM is generated in software and as such can be delayed by the none real time version of Linux it runs.

If a current below the stall current is applied to the. It or then a step will not occur, the motor will stall and there will be no movement. But you will have applied a pulse, which if you are counting pulses will cause the code to thing more pulses have been given.

No they would not affect each other.

Makes no odds they both actually do the same thing.

For the record the only experience I have in this sort of setup, that is driving a LEGO motor, which has a built in encoder, as in the EV3 LEGO system. I have tried using the commercial system that basically has an Arduino and special build of Linux to make it more real time. I have never got the repeated left / right move to return to the same place over time. It drifts like you have found. After a lot of investigation I concluded it was the hysteresis of the gears that was causing the problem. My project was an R, Theta drawing arm, but while it functioned the results were not good enough for me so I never published anything on the project.

DaveEvans · December 10, 2021, 6:52pm

I wonder if counting pulses using input capture on two timers would eliminate the "drift" problem. Would need other than 328p...a mega or pro micro or...

phillipstearns · December 10, 2021, 11:53pm

The broadcom chip BCM2835 ARM manual lists PWM0 and PWM1 and associated GPIO PINs on page 140:
https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf

I've also used the hardware PWM via pigpio with great success for LED control.
https://abyz.me.uk/rpi/pigpio/python.html#hardware_PWM

I haven't cross referenced the pins with those supported by pigpio, but I suspect they're the same as listed in the BCM2835 PDF.

I still don't understand the thinking. My current understanding of the encoder, to shaft movement is: no movement, no pulse.

Allowing for backslash, of course there will be wiggle in the output shaft, but cumulative drift? There is also the possibility that a pulse is NOT counted if the shaft rotates by less than 0.086 degrees (360/4200 . If the slight overshot is consistent cycle through cycle, this may in fact add up, but not as quickly as I am observing in some cases.

I've modified my approach a little bit and tried to strip things down and protect sensitive variables while accessing:

/* Dual Quadrature Rotary Decoder
   Uses external interrupts of the Atmega328p
   Encoder 1 - A -> D2, B -> D4
   Encoder 2 - A -> D3, B -> D5
   refs:
   http://gammon.com.au/interrupts
*/

#define ENC1A_PIN 2
#define ENC1B_PIN 4
#define ENC2A_PIN 3
#define ENC2B_PIN 5

// motor position variables
volatile int enc1Count = 0;
volatile int enc2Count = 0;
int position1 = 0;
int position2 = 0;
int reg1 = 0;
int reg2 = 0;

// serial poll timing variables
long currentTime = 0;
long lastTime = 0;
long triggerTime = 10; // every 10ms

//////////////////////////////////////////////////////////////////////////////
// External Interrupt ISRs

ISR(INT0_vect) { // attached to pin2
  reg1 = PIND;
  if ( ((reg1 >> ENC1A_PIN) & 1) == ((reg1 >> ENC1B_PIN) & 1)) {
    --enc1Count;
  } else {
    ++enc1Count;
  }
}

ISR(INT1_vect) { // attached to pin3
  reg2 = PIND;
  if ( ((reg2 >> ENC2A_PIN) & 1) == ((reg2 >> ENC2B_PIN) & 1)) {
    --enc2Count;
  } else {
    ++enc2Count;
  }
}

//////////////////////////////////////////////////////////////////////////////
// Serial Communication

void sendCounts() {
  cli();
  position1=enc1Count;
  position2=enc2Count;
  sei();
  Serial.print(position1);
  Serial.print('|');
  Serial.println(position2);
}

//////////////////////////////////////////////////////////////////////////////
// Setup

void setup() {
  EICRA |= 0b00000101; // set INT0 and INT1 to respond to any voltage changes
  EIMSK |= 0b00000011; // enable INT0 and INT1

  //  according to pololu, the encoders don't need pullup resistors
  pinMode(ENC1A_PIN, INPUT);
  pinMode(ENC1B_PIN, INPUT);
  pinMode(ENC2A_PIN, INPUT);
  pinMode(ENC2B_PIN, INPUT);

  Serial.setTimeout(1);
  Serial.begin(115200);   // start serial for output for debugging
}

//////////////////////////////////////////////////////////////////////////////
// Loop

void loop() {
  currentTime = millis();
  if (currentTime - lastTime >= triggerTime) {
    // if we receive ANYTHING on the serial port, clear the counts
    // never be not sending
    if(Serial.available() > 0){
      while(Serial.read() != -1){
      }
      cli();
      enc1Count = 0;
      enc2Count = 0;
      sei();
    }
    sendCounts();
    lastTime = currentTime;
  }
}

Perhaps a bigger issue that I may have over looked is a third brushed DC motor in the setup. I'm using software PWM from the Raspberry Pi (GPIO_PIN 21) to drive a N-channel enhancement mode Power MOSFET with a 1.5v on voltage. I've got a flyback diode in place across the motor, but that does not seem to be enough. I'm not getting bad serial data so those lines do not appear impacted (that kind of error is rejected), but I DO get counting happening when there shouldn't be.

SO. As far as I can tell. The above code is not the primary source of the "drift" I'm seeing so much as this other motor causing EMI. I'm setting up a second power supply to create some isolation. I may engage internal pullups if this doesn't resolve the issue.