Mega 2560 interrupts/libraries for optical sensors

I am trying to use PCINT19 - PCINT23, on a Mega2560 pro, to monitor opto sensors.  I had done this without interrupts, and was getting results that were not always consistent. So I decided to try interrupts.  Now I'm getting completely random results.  
Since I have to use 5 pcint pins on the same port, it seems as though I was unable to use any other libraries to detect the pin changed, besides "#include <avr/interrupt.h>".  If this is not the case, please let me know.
I went through a video that stated that, "if(PINK & B00001000)" was a way to write if(digitalRead(PCINT23)>0);  This is a condition that is supposed to tell me which pin in the port/vector that triggered the interrupt.  I was unable to find other libraries that would let me identify a pin from a port, rather than just identify only which port triggered the interrupt.  
Either way, I cannot get consistent results from the code that I wrote, and was wondering if I could get advice or some kind of hint as to what may be the problem.  

Relevant code related to interrupts

#include <avr/interrupt.h>

const byte sensorPins[5] = {69,68,67,66,65};
int optosCounts[5]={};
bool optosStatus[5]={};
void setup(){
	Serial.begin(9600);
	Serial.println(__FILE__ __DATE__);
	cli();
	PCICR |= 0b00000100; // Enables PortK Pin Change Interrupts
	PCMSK2 |= 0b11111000; // PCINT0
	//PCMSK2 |= (1 << PCINT23) | (1 << PCINT22) | (1 << PCINT21) | (1 << PCINT20) | (1 << PCINT19);
	sei();

}
ISR(PCINT2_vect){
  //cli();
	
  if(optosStatus[4]==0 && PINK & B00001000){
    //Serial.println("inside of if #4");
    optosStatus[4] = false;
    optosCounts[4]++;
  }else if(optosStatus[4]==1 && !PINK & B00001000){
    //Serial.println("inside of if else #4");
     optosStatus[4] = true;
  }
  if(optosStatus[3]==0 && PINK & B00010000){
    optosStatus[3] = false;
    optosCounts[3]++;//Serial.println("inside of if #3");
  }else if(optosStatus[3]==1 && !PINK & B00010000){
     optosStatus[3] = true;//Serial.println("inside of if else #3");
  }
  if(optosStatus[2]==0 && PINK & B00100000){
    optosStatus[2] = false;
    optosCounts[2]++;//Serial.println("inside of if #2");
  }
  else if(optosStatus[2]==1 && !PINK & B00100000){
     optosStatus[2] = true;//Serial.println("inside of if else #2");
  }
  if(optosStatus[1]==0 && PINK & B01000000){
    optosStatus[1] = false;
    optosCounts[1]++;//Serial.println("inside of if #1");
  }
  else if(optosStatus[1]==1 && !PINK & B01000000){
     optosStatus[1] = true;//Serial.println("inside of if else #1");
  }
  if(optosStatus[0]==0 && PINK & B10000000){
    optosStatus[0] = false;
    optosCounts[0]++;//Serial.println("inside of if #0");
  }
  else if(optosStatus[0]==1 && !PINK & B10000000){
     optosStatus[0] = true;//Serial.println("inside of if else #0");
  }
  //sei();
  for(int i = 0; i < numReels; i++){
    if(optosCounts[i] > 23){
      optosCounts[i]=1;
    }
  }
}

If I need to upload all 400 lines of my sketch, let me know.
Thanks in advance

I recommend the PinChangeInterupt library by Nico Hood. It makes the Pin Change interrupts work like External interrupts. It handles the interrupts and calls your ISR on RISING, FALLING or CHANGE.

I had looked that library over, but I am confused as to how to know which pin triggered the interrupt. Also, now that I have it in my code, I get this error:
"PinChangeInterrupt2.cpp.o (symbol from plugin): In function attachPinChangeInterrupt2()': (.text+0x0): multiple definition of __vector_11' "

You do it the old fashioned way we did it when there was only ONE interrupt on the microprocessor. You poll the pins and see which one has changed.

Please provide a Minimal, Complete, Verifiable example of your code which produces that error.

I am confused as to how to know which pin triggered the interrupt.

With the Nico Hood library each pin has a separate attachinterrupt.

The error I got is from leaving my ISR() function in the code, while using the new import. I removed it and it was all good.

The code you asked about is here:

#include <AccelStepper.h>
#include <Wire.h>
#include "Adafruit_MCP23X17.h"
#include "PinChangeInterrupt.h"
#include <avr/wdt.h>
const byte sensorPins[5] = {69,68,67,66,65};
int optosCounts[5]={};
bool optosStatus[5]={};
void setup(){
	Serial.begin(9600);
	Serial.println(__FILE__ __DATE__);
	for(int i=65;i<70;i++){
      pinMode(i, INPUT_PULLUP);
    }
    attachPCINT(digitalPinToPCINT(69), countOptos1, RISING);
    attachPCINT(digitalPinToPCINT(68), countOptos2, RISING);
    attachPCINT(digitalPinToPCINT(67), countOptos3, RISING);
    attachPCINT(digitalPinToPCINT(66), countOptos4, RISING);
    attachPCINT(digitalPinToPCINT(65), countOptos5, RISING);
}
void countOptos1(void){
  optosCounts[0]++;
}
void countOptos2(void){
  optosCounts[1]++;
}
void countOptos3(void){
  optosCounts[2]++;
}
void countOptos4(void){
  optosCounts[3]++;
}
void countOptos5(void){
  optosCounts[4]++;
}
void loop() {
  //probe for serial input
  //receive command
  checkForData();
  
  if(cmdWaiting==true or spinValidated==false){
    
    cueMotors();
    
  }
  
}
void cueMotors(){ 
  if(cmdWaiting == true){
    sendCommand();
  }//EndIf
  steppers[index].run();//should be index, not 0
  index++;
  if(index==numReels){
    index=0;
  }
  if(optosCounts[index]>23){//24th opto is 0????
    optosCounts[index]=1;
  }
}
void sendCommand(){
  for(int i=0; i<numReels; i++) {
    int fullRotations = i+1;
    // Set *relative* position for each target
    steppers[i].moveTo((fullRotations*NUM_STEPS)+(targetValues[i]*STEPS_PER_VALUE));//was moveTo
  }//EndFor
  cmdWaiting = false;
  spinValidated = false;
}

No matter what way I choose to go, I keep getting inconsistant results, even though I send the same command. I send 5 values that each are a target for stepper motor locations. I run the steppers at speed of 250, acceleration of 500, at 4.5454545 STEPS_PER_VALUE, and 200 total steps. (not sure if that helps clarity)

Each pin has a separate callback function that you set when you 'attach', like with external interrupts:
attachPinChangeInterrupt(Pin, callback, FALLING)
The library enables and handles the interrupt. If "Pin" went from HIGH to LOW between the previous interrupt and the current interrupt, it calls your callback. If you select "CHANGE" it will call you callback if the pin changed state.

That means that some other library has defined the PCInt2 interrupt already. Are you using SoftwareSerial?

Would the speed of my reels(stepper motor) be a problem for the interrupts?

Maybe.

I notice you don't declare the "optosCounts" array as 'volatile'.

You don't show the "checkForData()" function so there may be problems there, too.

If your optoCounts are limited to 24, you don't need a full int to store them. I would use a byte. Also, the overflow from 23 to 0 might best be done in the ISR using the modulo operator:

void countOptos1(void)
{
  optosCounts[0] = (optosCounts[0] + 1) % 24;
}

I appreciate your suggestion. And threw it in the code. I did some print lines inside of the callbacks, and verified that interrupts from neighboring reels are being triggered from vibration. Would you have a suggestion on this, or should I ask on a new thread?

I think you need sensors that are not sensitive to vibration.

Lot going on here, but the code is here

#include <AccelStepper.h>
#include <Wire.h>
#include "Adafruit_MCP23X17.h"
#include "PinChangeInterrupt.h"
#include <avr/wdt.h>
#define NUM_STEPS 200
#define STEPS_PER_VALUE 4.5454545
#define ENX 41
#define RST 39

// Instance of MCP23017 library
Adafruit_MCP23X17 mcp;
uint8_t values[15] = {};
uint8_t addressesExpected[5][3] = {
  {0,0,1},
  {0,1,1},
  {1,0,1},
  {1,1,1},
  {1,1,0}
};
uint8_t reelAddresses[5][3] = {};
bool pendingAddressCheckSuccess = false;
const byte numReels = 5;
const byte sensorPins[5] = {69,68,67,66,65};
int stepperPins[5]={5,57,58,59,2};
int dirPins[5]={60,54,55,56,61};
AccelStepper stepper1(AccelStepper::DRIVER, stepperPins[0], dirPins[0]);
AccelStepper stepper2(AccelStepper::DRIVER, stepperPins[1], dirPins[1]);
AccelStepper stepper3(AccelStepper::DRIVER, stepperPins[2], dirPins[2]);
AccelStepper stepper4(AccelStepper::DRIVER, stepperPins[3], dirPins[3]);
AccelStepper stepper5(AccelStepper::DRIVER, stepperPins[4], dirPins[4]);//AccelStepper stepperY(AccelStepper::DRIVER, stepperPin2, dirPin2);
AccelStepper steppers[5] = {stepper1,stepper2,stepper3,stepper4,stepper5};
int maxSpeed = 250;//250
int maxAccel = 500;//500
int cmdSpecifics = 0;                                          /*<cmdSpecifics, reelStop1, reelStop2, reelStop3, reelStop4, reelStop5>*/
bool reelHomed[5]={1,1,1,1,1};
bool newData = false;
const byte numChars = 21;
char receivedChars[numChars];
char tempChars[numChars];
bool cmdValid = false;
bool spinValidated = true;
bool cmdSent = false;
bool cmdWaiting=false;
int targetValues[5] = {};
unsigned long timeOfLastHigh;
unsigned long timeOfFirstHigh;
unsigned long lengthOfHigh;
volatile int optosCounts[5]={};
volatile bool optosStatus[5]={};
bool isHigh = false;    
int index=0;
void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);  Serial3.begin(9600);
  Serial.println(__FILE__ __DATE__);
  wdt_disable();
  mcp.begin_I2C();
  pinMode(ENX, OUTPUT);
  digitalWrite(ENX, LOW);
  digitalWrite(RST, HIGH);
  
  for(int i=0;i<numReels;i++){
    pinMode(sensorPins[i], INPUT_PULLUP);
    //attachPCINT(digitalPinToPCINT(i), countOptos, RISING);
  }
  attachPCINT(digitalPinToPCINT(69), countOptos1, RISING);
  attachPCINT(digitalPinToPCINT(68), countOptos2, RISING);
  attachPCINT(digitalPinToPCINT(67), countOptos3, RISING);
  attachPCINT(digitalPinToPCINT(66), countOptos4, RISING);
  attachPCINT(digitalPinToPCINT(65), countOptos5, RISING);
  if(pendingAddressCheckSuccess == false){
    parseAddresses();
    delay(100);
    verifyAddresses();
    delay(100);
    Serial.println(pendingAddressCheckSuccess);
  }
  for(int i=0;i<numReels;i++){//need 2 here
    //pinMode(sensorPins[i], INPUT);
    steppers[i].setMaxSpeed(maxSpeed);
    steppers[i].setMinPulseWidth(20);
    steppers[i].setAcceleration(maxAccel);
    CalibrateReel(i);
  }
  
}

void countOptos1(void){
  optosCounts[0] = (optosCounts[0] + 1) % 24;
}
void countOptos2(void){
  optosCounts[1] = (optosCounts[1] + 1) % 24;
}
void countOptos3(void){
  optosCounts[2] = (optosCounts[2] + 1) % 24;
}
void countOptos4(void){
  optosCounts[3] = (optosCounts[3] + 1) % 24;
}
void countOptos5(void){
  optosCounts[4] = (optosCounts[4] + 1) % 24;
}

void loop() {
  //probe for serial input
  //receive command
  checkForData();
  
  if(cmdWaiting==true or spinValidated==false){
    cueMotors();
  }
}
void cueMotors(){
  bool allHomed = checkReelState();
  digitalWrite(ENX, LOW);
  // Sending 9 re-calibrates the reel
  
  if(cmdSpecifics == 9 or allHomed == false){
    wdt_reset();
    reset();
  }
  if(cmdWaiting == true){
    sendCommand();
  }//EndIf
  steppers[index].run();//should be index, not 0
  index++;
  if(index==numReels){
    index=0;
  }
  if(isSpinFinished()==true){
    //Serial.println("spin is finished");
    if(spinValidated == false){
      validateSpin();//needs a lot of work
    }
  }
}
void parseAddresses(){
  int pinIndex = 0;
  for(int i = 0; i < 5; i++){
    for (int x = 0; x < 3; x++){
      char val = mcp.digitalRead(pinIndex);
      reelAddresses[i][x] = val;
      pinIndex++;
    }
  }
}
void verifyAddresses(){
  for(int i = 0; i < 5; i++){
    for (int x = 0; x < 3; x++){
      (reelAddresses[i][x]==addressesExpected[i][x])?pendingAddressCheckSuccess=true:pendingAddressCheckSuccess=false;
	}
  }
}
void checkForData(){
  
  recvWithStartEndMarkers();
  //assign values to appropriate variables
  if(newData==true){
    strcpy(tempChars, receivedChars);
    parseData();
    newData=false;
    Serial.flush();
    if(cmdValid==true){
      Serial.println(200);
      cmdWaiting=true;
    }
    else{
      Serial.println(404);
      cmdWaiting=false;
    }
    
  }
}
void reset(){
  wdt_enable(WDTO_8S);
  while(1);
}
bool checkReelState(){
  bool allHomed = false;
  for(int i = 0; i < numReels; i++){
    if(reelHomed[i]==0){
      allHomed=false;
      return allHomed;
    }
    else(allHomed = true);
  }
  return allHomed;
}
bool isSpinFinished(){
  if(steppers[0].distanceToGo()==0 and steppers[1].distanceToGo()==0 and steppers[2].distanceToGo()==0 and steppers[3].distanceToGo()==0 and steppers[4].distanceToGo()==0){// and steppers[1].distanceToGo()==0 and steppers[2].distanceToGo()==0 and steppers[3].distanceToGo()==0 and steppers[4].distanceToGo()==0
    return true;
  }else{
    return false;
  }
}
void validateSpin(){
  long thisRotation;
  long thisTarget;
  for(int i=0; i<numReels; i++) {
    int fullRotations = i+1;
    thisRotation = fullRotations*NUM_STEPS;
    thisTarget = targetValues[i]*STEPS_PER_VALUE;
    if(steppers[i].currentPosition() == (thisRotation+thisTarget)){
      spinValidated=true;
      steppers[i].setCurrentPosition((thisRotation-thisTarget)*-1);
    }//EndIf
    else{
      spinValidated=false;
    }//EndElse
  }//EndFor

}
void sendCommand(){
  for(int i=0; i<numReels; i++) {
    int fullRotations = i+1;
    // Set *relative* position for each target
    steppers[i].moveTo((fullRotations*NUM_STEPS)+(targetValues[i]*STEPS_PER_VALUE));//was moveTo
  }//EndFor
  cmdWaiting = false;
  spinValidated = false;
}
void CalibrateReel(int stepperNum){
  reelHomed[stepperNum] = false;
  // Set the stepper to a slow speed
  steppers[stepperNum].setMaxSpeed(100);
  // Make at most one complete rotation from the current position
  steppers[stepperNum].move(NUM_STEPS);
  while(steppers[stepperNum].distanceToGo() != 0){
    steppers[stepperNum].run();
  }
  steppers[stepperNum].stop();
  steppers[stepperNum].move(NUM_STEPS*4);
  timeTheOptos(stepperNum);
  // Stop the motor and declare this the zero position
  steppers[stepperNum].stop();
  steppers[stepperNum].setCurrentPosition(0);
  // Now that calibration is complete, send to default position
  steppers[stepperNum].move(19*STEPS_PER_VALUE);
  while(steppers[stepperNum].distanceToGo() != 0){
    steppers[stepperNum].run();
  }
  steppers[stepperNum].setMaxSpeed(maxSpeed);
  steppers[stepperNum].setCurrentPosition(0);
  optosCounts[stepperNum]=0;
}


void timeTheOptos(int stepperNum){
  while(reelHomed[stepperNum] == false){
    steppers[stepperNum].run();
    int detectState=digitalRead(sensorPins[stepperNum]);
    if(detectState==1){
      if(isHigh==false){
        timeOfFirstHigh = millis();
        isHigh=true;
      }
    }
    else if(detectState==0){
      if(isHigh==true){
        timeOfLastHigh=millis();
        lengthOfHigh=timeOfLastHigh-timeOfFirstHigh;
        isHigh=false;
        if(lengthOfHigh<=20UL){//20UL
          reelHomed[stepperNum]=true;
        }
      }
    }
  }
}
void recvWithStartEndMarkers(){
  static boolean recvInProgress=false;
  static byte ndx=0;
  char startMarker = '<';
  char endMarker = '>';
  char rc;
  
  while(Serial.available()>0 && newData==false){
    rc=Serial.read();

    if(recvInProgress==true){
      if(rc!=endMarker){
        receivedChars[ndx]=rc;
        ndx++;
        if(ndx>=numChars){
          ndx=numChars-1;
        }
      }
      else{
        receivedChars[ndx]='\0';//terminate the string
        recvInProgress=false;
        ndx=0;
        newData=true;
      }
    }
    else if(rc==startMarker){
      recvInProgress=true;
    }
  }
  Serial.flush();
  
}
void parseData(){
  int numParams = 6; //<x,x,x,x,x,x> (SEE ABOVE)
  Serial.println("parseData");
  char * strtokIndx;
  for(int i=0;i<numParams;i++){
    if(i==0){
      strtokIndx=strtok(tempChars,",");
    }else{
      strtokIndx=strtok(NULL,",");
    }
    cmdValid = validateChar(atoi(strtokIndx),i);
    if(cmdValid==false){
      memset(targetValues,0,sizeof(targetValues));
      return;
    }
  }
}
bool validateChar(int convertedChar, int index){
  bool results = true;
  
  if(index == 0){
    if(convertedChar == 0 or convertedChar == 1 or convertedChar == 9){
      Serial.print("convertedChar inside if = ");
      Serial.println(convertedChar);
      cmdSpecifics = convertedChar;
    }else results=false;
  }else{
    if(convertedChar >=0 and convertedChar<44){
      targetValues[index-1]=convertedChar;
    }else results=false;    
  }
  return results;
}

Not sure if will help with problem or not, and sorry i'm not the cleanest coder.

How are your sensors picking up vibrations?


I have 5 reels all mounted to a chassis, and when the stepper motor gets to going, it has a bit of vibration.

But motor vibration should have nothing to do with false triggers from your optical sensors. I think you need to figure out where the false triggers are coming from. You might need an oscilloscope to watch for glitches on the encoder outputs.

Can you please attach a data sheet for the optocal sensors you are using and a schematic of how they are wired and connected to the Arduino.

Boss told me that the 2.2k resistor squiggle mark is supposed to be after the vertical 5V line.

I'm not sure I understand your circuit.

The optical sensors do not appear to be connected directly to your pin change interrupt pins, but rather are going through a SN74ALS240AN. Is that correct? Can you please clarify the connections of the sensors, the buffer chip, and the Mega.

The sensors are open collector outputs. Do you have 5v pullups on the outputs from the sensors?

To my understanding, it gets pulled up at the 2.2k bussed resistor.


I really can provide any more advice on the hardware.

In a previous project where an encoder was subject to vibrational noise, some suppression could be achieved by looking for a stable transition of the pin in the isr. Depending on how fast the reels are going and the width of the slots something like this might work.

attachPCINT(digitalPinToPCINT(69), countOptos1, RISING);

void countOptos1(void)
{
   delayMicroseconds(200); //increase for best filtering,  max is approx 16,000
   if(digitalRead(69) == HIGH)
      optosCounts[0] = (optosCounts[0] + 1) % 24;
}

As previously suggested, getting a scope on the interrupt pins will tell you alot.

You might also consider putting an rc filter on the interrupt inputs if slowing things down won't loose data. Do you have a sense of how fast the encoder wheels are turning and the dimensions of the open/blocked areas?