[Solved] Help using port manipulation / registers

So I'm trying to control the speed of a motor using a rotary encoder and interrupts. The code I'm using I managed to find online, I've adapted to my application and it works great.

However I just want to change the interrupt pins from digital pins 2 & 3 to pins 18 & 19 on the Mega2560.

Here's the code that works when using pins 2 & 3:

#ifdef __AVR_ATmega2560__
    #define  PIN   PINE
  #define ABMASK  0b00110000
  #define AMASK 0b00010000
  #define BMASK 0b00100000
#endif

#define motor 8

/*#ifdef __AVR_ATmega328P__
    #define PIN   PIND
  #define ABMASK  0b00001100
  #define AMASK 0b00000100
  #define BMASK 0b00001000
#endif
*/

static int pinA = 2; // Our first hardware interrupt pin is digital pin 2
static int pinB = 3; // Our second hardware interrupt pin is digital pin 3
volatile long pinCignoreCycles = 0;         // the last time the output pin was sampled
volatile byte aFlag = 0; // let's us know when we're expecting a rising edge on pinA to signal that the encoder has arrived at a detent
volatile byte bFlag = 0; // let's us know when we're expecting a rising edge on pinB to signal that the encoder has arrived at a detent (opposite direction to when aFlag is set)
volatile byte encoderPos = 0; //this variable stores our current value of encoder position. Change to int or uin16_t instead of byte if you want to record a larger range than 0-255
volatile byte oldEncPos = 0; //stores the last encoder position value so we can compare to the current reading and see if it has changed (so we know when to print to the serial monitor)
volatile byte reading = 0; //somewhere to store the direct values we read from our interrupt pins before checking to see if we have moved a whole detent


void setup() {
    delay(3000); // 3 second delay for recovery

  pinMode(pinA, INPUT_PULLUP); // set pinA as an input, pulled HIGH to the logic voltage (5V or 3.3V for most cases)
  pinMode(pinB, INPUT_PULLUP); // set pinB as an input, pulled HIGH to the logic voltage (5V or 3.3V for most cases)
  attachInterrupt(0,PinA,RISING); // set an interrupt on PinA, looking for a rising edge signal and executing the "PinA" Interrupt Service Routine (below)
  attachInterrupt(1,PinB,RISING); // set an interrupt on PinB, looking for a rising edge signal and executing the "PinB" Interrupt Service Routine (below)
  Serial.begin(115200); // start the serial monitor link
}


void PinA()
{
  reading = PIN & ABMASK;                       
  if(reading == ABMASK && aFlag)                
  {
    encoderPos ++;                             
    bFlag = 0;
    aFlag = 0;                                 
  }
  else if (reading == AMASK) bFlag = 1;         
}

void PinB()
{
  reading = PIN & ABMASK;
  if (reading == ABMASK && bFlag) 
  {
    encoderPos --;                            
    bFlag = 0;
    aFlag = 0;
  }
  else if (reading == BMASK) aFlag = 1;
}

void loop(){

if(oldEncPos != encoderPos) {
    Serial.println(encoderPos);
    oldEncPos = encoderPos;
  }
    analogWrite(motor, encoderPos);
}

And here's the code that I've tried to modify to work with pins 18 & 19:

#ifdef __AVR_ATmega2560__
    #define  PIN   PIND
  #define ABMASK  0b00110000
  #define AMASK 0b00010000
  #define BMASK 0b00100000
#endif

#define motor 8

static int pinA = 18; // Our first hardware interrupt pin is digital pin 2
static int pinB = 19; // Our second hardware interrupt pin is digital pin 3
volatile long pinCignoreCycles = 0;         // the last time the output pin was sampled
volatile byte aFlag = 0; // let's us know when we're expecting a rising edge on pinA to signal that the encoder has arrived at a detent
volatile byte bFlag = 0; // let's us know when we're expecting a rising edge on pinB to signal that the encoder has arrived at a detent (opposite direction to when aFlag is set)
volatile byte encoderPos = 0; //this variable stores our current value of encoder position. Change to int or uin16_t instead of byte if you want to record a larger range than 0-255
volatile byte oldEncPos = 0; //stores the last encoder position value so we can compare to the current reading and see if it has changed (so we know when to print to the serial monitor)
volatile byte reading = 0; //somewhere to store the direct values we read from our interrupt pins before checking to see if we have moved a whole detent


void setup() {
    delay(3000); // 3 second delay for recovery

  pinMode(pinA, INPUT_PULLUP); // set pinA as an input, pulled HIGH to the logic voltage (5V or 3.3V for most cases)
  pinMode(pinB, INPUT_PULLUP); // set pinB as an input, pulled HIGH to the logic voltage (5V or 3.3V for most cases)
  attachInterrupt(5,PinA,RISING); // set an interrupt on PinA, looking for a rising edge signal and executing the "PinA" Interrupt Service Routine (below)
  attachInterrupt(4,PinB,RISING); // set an interrupt on PinB, looking for a rising edge signal and executing the "PinB" Interrupt Service Routine (below
  Serial.begin(115200); // start the serial monitor link
}


void PinA()
{
  reading = PIN & ABMASK;
  if(reading == ABMASK && aFlag) 
  {
    encoderPos ++;
    bFlag = 0;
    aFlag = 0;
  }
  else if (reading == AMASK) bFlag = 1;
}

void PinB()
{
  reading = PIN & ABMASK;
  if (reading == ABMASK && bFlag) 
  {
    encoderPos --;
    bFlag = 0;
    aFlag = 0;
  }
  else if (reading == BMASK) aFlag = 1;
}

void loop(){

if(oldEncPos != encoderPos) {
    Serial.println(encoderPos);
    oldEncPos = encoderPos;
  }
    analogWrite(motor, encoderPos);
}

As you can see I've updated the interrupt lines and changed PINE to PIND.

But still no luck. Any ideas where I'm going wrong?

Instead of using hard-coded interrupts, like this:

attachInterrupt(5,PinA,RISING);

Try to use this method:

attachInterrupt(digitalPinToInterrupt(pinNumber), handler, mode);

I'll be sure to do this from now on, thanks. Still not working though, Is it possible that the fault lies within this section of code?

#define  PIN   PIND
  #define ABMASK  0b00110000
  #define AMASK 0b00010000
  #define BMASK 0b00100000
#endif

Yes, that should problebly be:

#define  PIN   PIND
  #define ABMASK  0b00001100
  #define AMASK 0b00000100
  #define BMASK 0b00001000
#endif

EDIT: A bit of a miss.. :wink:

Still nothing :confused:

So just to make sure I understand how the registers work, does:

ABMASK  0b00000110 mean that pins 18 and 19 are HIGH

AMASK 0b00000010 mean that only pin 19 is HIGH

and
BMASK 0b00000100 mean only pin 18 is HIGH?

And so these are used to read all the input pins at once to determine each pulse of the encoder?

Yes, you could just as well use digitalRead(pinNumber) to get a result - which would be more reliable.

Ideally I want to try and solve what's stopping it from working with the port register control.

I can see how the code you sent should work as opposed to what it was before, but there must be something else that needs changing for it to work with pins 18 and 19.

Issue Solved.

Upon taking a closer look at the pin out for the Mega I realised that the first 4 pins for port D go in the reverse order of the physical pins. So pin 21 = portD pin 0, pin 22 = portD pin 1, pin 19 = portD pin 3 etc.

Anyway here's the correct lines of code to get it working for pins 18 and 19 (hopefully this will prove useful for anyone who experienced a similar issue as what I did)

  #define ABMASK  0b00001100
  #define AMASK 0b00000100
  #define BMASK 0b00001000