attachinterrupt, erroneous interrupts. [solved]

Hi,

I am having a problem with the standard attachInterrupt function on my mega.
My very simple ISR (increments a counter when it is called) is being called ~10 times too many in every ~50.

The encoder and the pulses it generates are 100% as expected and if I just monitor the pin with a while loop and digitalRead I get exactly what I expect. However, when I attach an interrupt to the pin instead of polling it continuously, I get erroneous calls to the ISR, resulting in more counts than actually happened.

Therefore, as far as I can see, the problem must be with the arduino interrupt library unless I'm missing something here?
Has anyone else come across this or does anyone else have any ideas as to what would be causing the ISR to fire a few more times than it should have?

(also, I've used a scope and other devices to monitor the encoder output and they are all as expected, only the arduino interrupt is firing too many times)

Any info would be greatly appreciated.

Thanks.

Have you set the horizontal timebase on your scope to a value that would allow you to see pulses in the sub millisecond range? Because if not, you might just have a bouncy signal. The while loop and digital read (and the oscope setting) might be too slow to catch it, but the interrupt may not be.

Please post your code.

aarg:
Please post your code.

Have you got the correct mode of LOW | CHANGE | RISING | FALLING?

Thanks for the replies.

re the CHANGE, RISING etc, I've tried all of them and all seemed to give me problems.

re the bouncy signal, that may be a possibility but I thought that the schmitt trigger in the encoder would debounce the signal?

The code is below, sorry I thought it was so basic (and untidy atm) it wasn't worth posting:

const int _pinMotorEnableLeft = 5;
const int _pinMotorEnableRight = 7;
const int _pinMotorDirectionLeft = 4;
const int _pinMotorDirectionRight = 6;
const int _pinEncoderLeft = 20;
const int _pinEncoderRight = 21;

int _leftMotorSpeed = 0;
int _rightMotorSpeed = 0;

volatile int _leftCount = 0;
volatile int _rightCount = 0;

void UpdateLeftCount() {
  ++_leftCount;
}

void UpdateRightCount() {
  ++_rightCount;
}

void setup()  { 
     
  //   Motors
  pinMode(_pinMotorEnableLeft, OUTPUT);
  pinMode(_pinMotorEnableRight, OUTPUT);
  pinMode(_pinMotorDirectionLeft, OUTPUT);
  pinMode(_pinMotorDirectionRight, OUTPUT);
  pinMode(_pinEncoderLeft, INPUT);
  pinMode(_pinEncoderRight, INPUT);
  
 
  
  // Attach interrupts. 
  attachInterrupt(3, UpdateLeftCount, CHANGE);
attachInterrupt(2, UpdateRightCount, CHANGE);
  
  // Init serial comms at 9600 baud rate.
  Serial.begin(9600); 
  
  // Init pin values
  digitalWrite(_pinMotorEnableLeft, LOW);
  digitalWrite(_pinMotorEnableRight, LOW);
  digitalWrite(_pinMotorDirectionLeft, LOW);    
  digitalWrite(_pinMotorDirectionRight, LOW);
     
   Serial.println("- Setup Complete - ");
} 

void loop()  { 

//// temp
  int encoderCount = 0;
  bool oldState = LOW;
 
 
 ////
 
  char incomingByte;

  if (Serial.available() > 0) {
      // read the incoming byte from the serial port.
      incomingByte = Serial.read();
      
      switch (incomingByte) {
          case 'w':         
            Serial.println("fwd");
            Forward();
            RampUp();
            break;
            
         case 'a':         
            Serial.println("left");
            Left();
            RampUp();
            break;
            
         case 'd':         
            Serial.println("right");
            Right();
            RampUp();
            break;
            
         case 's':         
            Serial.println("back");
            Back();
            RampUp();
            break;
            
          
// a temp function to test my interrupt problems.
// my pin monitor that works fine, but it obv not the correct way of doing things.
          case 'p':         
            Serial.println("scanning pin 21");
            Forward();             
             analogWrite(_pinMotorEnableRight, 100);              
            while (!(Serial.available() > 0)) {
                     if (oldState != digitalRead(21)) {
                       encoderCount++;
                       oldState = digitalRead(21);
                     
                     }
            }
            Serial.println(encoderCount);
            break;
            
// halt
          case 'h':         
            Serial.println("stop");
            Serial.println("Left: ");
            Serial.println(_leftCount);
            Serial.println("Right: ");
            Serial.println(_rightCount);
            _leftCount = 0;
            _rightCount = 0;
            Stop();
       
            delay(500);
            break;
      }
 }
}

void ResetCounters(){
 _leftCount = 0;
 _rightCount = 0;

}

void  Forward() {
digitalWrite(_pinMotorEnableLeft, LOW);
digitalWrite(_pinMotorEnableRight, LOW);
delay(300);
 
digitalWrite(_pinMotorDirectionLeft, HIGH);
digitalWrite(_pinMotorDirectionRight, LOW);
ZeroMotorSpeeds();
}

void  Back() {
digitalWrite(_pinMotorEnableLeft, LOW);
digitalWrite(_pinMotorEnableRight, LOW);
delay(300);
 
digitalWrite(_pinMotorDirectionLeft, LOW);
digitalWrite(_pinMotorDirectionRight, HIGH);
ZeroMotorSpeeds();
}

void  Right() {
digitalWrite(_pinMotorEnableLeft, LOW);
digitalWrite(_pinMotorEnableRight, LOW);
delay(300);
 
digitalWrite(_pinMotorDirectionLeft, HIGH);
digitalWrite(_pinMotorDirectionRight, HIGH);
ZeroMotorSpeeds();
}


void  Left() {
digitalWrite(_pinMotorEnableLeft, LOW);
digitalWrite(_pinMotorEnableRight, LOW);
delay(300);
 
digitalWrite(_pinMotorDirectionLeft, LOW);
digitalWrite(_pinMotorDirectionRight, LOW);
ZeroMotorSpeeds();
}

void ZeroMotorSpeeds() {
_leftMotorSpeed = 0;
_rightMotorSpeed = 0;

}

void Stop() {
_leftMotorSpeed = 0;
_rightMotorSpeed = 0;
digitalWrite(_pinMotorEnableLeft, LOW);
digitalWrite(_pinMotorEnableRight, LOW);

}

// RAMP UP
void RampUp() {
int leftLimit = 255;
int rightLimit = 255;
while(_leftMotorSpeed < leftLimit || _rightMotorSpeed < rightLimit) {
      if (_leftMotorSpeed < leftLimit) {
        _leftMotorSpeed++;
        analogWrite(_pinMotorEnableLeft, _leftMotorSpeed); 
      }
        if (_rightMotorSpeed < rightLimit) {
        _rightMotorSpeed++;
        analogWrite(_pinMotorEnableRight, _rightMotorSpeed); 
      } 
      delay(1);

}

}

EDIT put in code tags (they weren't there on the quick reply so I ddn't know they were there, thanks =) )

I'd find a nice clean source of interrupts, like another I/O pin (possibly even on another Arduino), and program that to generate interrupts at close to (or exceeding) the rate you expect, and see how likely you conclusion is.

Please remember to use CODE TAGS when posting code.

How are the encoders wired?
External pullups or pull-downs?

a22c:
re the bouncy signal, that may be a possibility but I thought that the schmitt trigger in the encoder would debounce the signal?

It has one? What encoder is it?

I'd find a nice clean source of interrupts, like another I/O pin (possibly even on another Arduino), and program that to generate interrupts at close to (or exceeding) the rate you expect, and see how likely you conclusion is.

Based on all the info I have the only common part when there is a problem is the attach interrupt lib. I know these libraries are almost certainly going to be correct but if everything else reads the signals fine (including the arduino itself) when doing anything other than the ISR I honestly can't think of anything else it could be. I doubt there is a bug in the lib but it's all I can think of at the moment.

It has one? What encoder is it?

Yes there are two each wired to a separate pin. (20 and 21)
the wheel RPM is about 30 so their ticks are at ~300 ms intervals.

I'm at work atm and I can't remember the exact model but they're visually similar to the H9730 encoders.

The encoders are OPB 666T http://www.farnell.com/datasheets/86991.pdf

How are the encoders wired?
External pullups or pull-downs?

I don't think there is a problem with the encoders all my other devices (including a PIC) see the output fine as expected. however when I get home I'll have a quick look at them but I'm pretty sure the Schmitt output goes directly to the pin (20 and 21, for each encoder) They are pulled up as well.

You mean like the HEDS 9730 encoder? There's no mention of any schmidt in the datasheet.

With quadrature detection, you get a degree of built in debounce in the absence of hysteresis and/or filtering. In that, although it can dither up/down on a state transition, this will never result in an incorrect count (as long as the value isn't recognized until the other phase changes state). At least, that is the way my sleep deprived mind remembers it now.

I think I've even seen a logic circuit to produce reliable up/down pulses this way.

I know it's not real debouncing, because on a transition it will bounce, it's more of an error guard. Or whatever you want to call it. Because repetitive bounces will not generate truly erroneous counts.

aarg:
You mean like the HEDS 9730 encoder? There's no mention of any schmidt in the datasheet.

With quadrature detection, you get a degree of built in debounce. In that, although it can dither up/down on a state transition, this will never result in an incorrect count.

That kind of encoder yes, ah ok sorry I was told that they had them internally. I'll find the correct model number shortly.

What you said makes sense as the signals are clean on the scope and I've got no reason to think they're bouncing or doing anything strange as everything else read them fine.

Could there be some kind of interference on the interrupt pins? e.g from serial comms or something? I know 20 and 21 aren't the rx and tx pins but is there something similar I need to disable on those pins for example?

Could the change be happening too slowly and the interrupts on the arduino are firing incorrectly mid change on about 5% of all count?

EDIT: One last thing I thought of is that these encoders and the wiring from them go past my (large-ish) motors and 12v lead acid battery, could this be putting noise spikes on the wires that the interrupt code sees but nothing else is fast enough?
aarg, is that what you meant by changing my scope time base, to check for spikes? Would the arduino fire if there was a small spike or does it have proper filtering on that pin?

a22c:
T
What you said makes sense as the signals are clean on the scope

Actually, from what I said, they might not have to be clean (in other words, great if they're clean but there's nothing in what I mentioned that would make them clean). An encoder of this type can put out unclean, bouncy signals and still function correctly. It's the external decoding circuit that ensures that.

a22c:
Would the arduino fire if there was a small spike or does it have proper filtering on that pin?

I only have the Mega spec in front of me, with it, the interrupt input circuit is clocked by the system clock. That should give you a clue. If the spike happened to fall on a clock edge, the answer would be yes, I believe.

But what would "proper" filtering be?

aarg:
Actually, from what I said, they might not have to be clean.

Oh I see, I misunderstood you completely there then, sorry. Alright thanks, I'll take another look at the signals from the encoders tonight when I get home and post my findings here either way. I'll also post the part number of those encoders as well. I found the part number they are below.

I only have the Mega spec in front of me, with it, the interrupt input circuit is clocked by the system clock. That should give you a clue. If the spike happened to fall on a clock edge, the answer would be yes, I believe.

But what would "proper" filtering be?

So if the spike was at the correct time it would fire an interrupt, hmm ok thanks for that.

I would assume that the interrupt input circuit would have some form of filtering or even just a schmitt trigger after the pin as it is a dedicated hardware interrupt. Having thought about it, I doubt they can do much filtering as noise for me might be a valid signal for others. I'll read the datasheet on this at lunch I think.

I found the correct encoders and they do indeed have a Schmitt trigger on internally, so this should remove all bounce I'm guessing.

I've got a the correct model number now, the encoder is a OPB 666T, the data sheet is here: http://www.farnell.com/datasheets/86991.pdf

External pullups or pull-downs?

I have got a small board between the encoder and the arduino that has a pull up for the open-collector chip output and a series protection resistors to prevent cross-logic levels from killing either the sensor or the CPU pin.

When I check things again this evening I'm hoping that I see a couple of obvious problems and I just have to write an embarrassed post here saying it was my bad circuits that were to blame because at least that is easily solvable!

I looked at the schematic for the mega and the pins I'm using are pulled up, the pdf:

Would this affect interrupt on pins 20 and 21 int0 and int1 (44 and 43 on the chip)? my electronics knowledge is limited at best.

EDIT:
Another bit of information is that when I completely unplug the arduino interrupt pins they still occasionally fire.

I got 7 counts from a disconnected pin 21 ISR (21 and 20 seem to be pulled up so I have no idea why the arduino interrupts are firing) I had assumed they were floating until I saw they were pulled up.

The pullup's are NOT on by default. You need to turn them on in your code see pinmode();

Mark

Thanks Mark, I'll turn on those pull ups and see if that makes a difference.

EDIT - I just checked the schematic again and they're actually hardwired with 10k resistors to 5v. Are you sure that the pinmode function affects these 2 pins (which are the only interrupts that are pulled up in the schematic) please see the SCL SDA pins on the mega schematic.
I think you may be thinking of the standard inbuilt 20k pullups, sorry if I'm talking nonsense this is my first trip into avrs.

I suspect the output signal is too weak ... try driving the IRLED with about 15mA with this suggested circuit:

const int _pinMotorEnableLeft = 5;
const int _pinMotorEnableRight = 7;
const int _pinMotorDirectionLeft = 4;
const int _pinMotorDirectionRight = 6;
const int _pinEncoderLeft = 20;
const int _pinEncoderRight = 21;

_What _is _it _with _the _leading _underscores _everywhere? _Do _you _think _it _makes _the _code _easier _to _read?

According to the C++ standard:

In C++ these variable names are reserved:

Reserved in any scope, including for use as implementation macros:

  • identifiers beginning with an underscore and an uppercase letter
  • identifiers containing adjacent underscores (or "double underscore")

Reserved in the global namespaces:

  • identifiers beginning with an underscore

So in global namespaces (such as you are using) such variable names are reserved. Don't use them. We can't stop you, but it is against the Standard. It is confusing because it makes them look like compiler reserved names.

... the problem must be with the arduino interrupt library ...

There isn't an interrupt library. attachInterrupt basically just remembers the function you supply, which is then called by the base ISR.

There is no way it sometimes calls the ISR more often, that would cause all sorts of grief.

_What _is _it _with _the _leading _underscores _everywhere? _Do _you _think _it _makes _the _code _easier _to _read?

Really? That's the most constructive thing you have to offer? You'll find that underscores or similar prefix e.g. g_ are a perfectly valid global scope identifier according to many standards, Google's included.

dlloyd:
I suspect the output signal is too weak ... try driving the IRLED with about 15mA with this suggested circuit:

Your post, as far as I can tell has nothing to do with my problems. Wrong thread perhaps?