Reading 4 encoders

hello
please help me
Ι want to ask  for (Reading 4 encoders using pin-change interrupts,),Posted by Nick Gammon 
the encoder count until 32767
Ι change the int count to long count but nothing
how will I make it counter over  32767
thanks
kostas



// Wiring: Connect common pin of encoder to ground.

volatile bool fired;

const byte ENCODERS = 4;

typedef struct 
  {
  int aPin;     // which Arduino pin for the "A" side
  int bPin;     // which Arduino pin for the "B" side

  // values below are calculated at run-time
  volatile byte * aPort;  // which processor port the A side is plugged into
  volatile byte * bPort;  // which processor port the B side is plugged into
  byte aBitMask;   // which bit in the A port
  byte bBitMask;   // which bit in the B port
  byte whichInterrupt;  // which pin-change interrupt port (0, 1, 2)
  byte oldValue;   // old value of A port (to see if it changed)
  int count;       // current encoder counter
  } encoder;

 volatile encoder encoders [ENCODERS] =
  {

 // A   B  pins  (eg. D4 and D8, D5 and D9 and so on)

  { 4,  8 },
  { 5,  9 },
  { 6, 10 },
  { 7, 11 },

  };  // end of encoders


void checkForPinChange (const byte which)
  {
  for (byte i = 0; i < ENCODERS; i++)
    {
    if (encoders [i].whichInterrupt == which)
      {
      byte newValue = *(encoders [i].aPort) & encoders [i].aBitMask;
      if (newValue != encoders [i].oldValue)
        {
        bool up;
        byte bPort = *(encoders [i].bPort) & encoders [i].bBitMask;
        if (newValue)
          up = bPort;
        else
          up = !bPort;
        fired = true;
        if (up)
          encoders [i].count++;
        else
          encoders [i].count--;
        encoders [i].oldValue = newValue;
        }  // end of if value has changed
      }   // end of if this is the right interrupt number
    }     // end of for each encoder
  } // end of checkForPinChange

// handle pin change interrupt for D8 to D13 here
ISR (PCINT0_vect)
 {
 checkForPinChange (PCIE0);
 }  // end of PCINT0_vect

// handle pin change interrupt for A0 to A5 here
ISR (PCINT1_vect)
 {
 checkForPinChange (PCIE1);
 }  // end of PCINT1_vect

// handle pin change interrupt for D0 to D7 here
ISR (PCINT2_vect)
 {
 checkForPinChange (PCIE2);
 }  // end of PCINT2_vect

void setup ()
{
 Serial.begin (115200);
 Serial.println ("Starting ...");

 PCIFR  |= bit (PCIF0) | bit (PCIF1) | bit (PCIF2);   // clear any outstanding interrupts

 for (byte i = 0; i < ENCODERS; i++)
    {
    pinMode (encoders [i].aPin, INPUT_PULLUP);
    pinMode (encoders [i].bPin, INPUT_PULLUP);
    // convert pin number to port, mask, etc. for efficiency

    // Input port
    encoders [i].aPort = portInputRegister (digitalPinToPort (encoders [i].aPin));
    encoders [i].bPort = portInputRegister (digitalPinToPort (encoders [i].bPin));

    // Which bit in the port to test
    encoders [i].aBitMask = digitalPinToBitMask (encoders [i].aPin);
    encoders [i].bBitMask = digitalPinToBitMask (encoders [i].bPin);

    // Which interrupt number (0, 1, 2)
    encoders [i].whichInterrupt = digitalPinToPCICRbit (encoders [i].aPin);

    // activate this pin-change interrupt bit (eg. PCMSK0, PCMSK1, PCMSK2)
    volatile byte * ICRmaskPort = digitalPinToPCMSK (encoders [i].aPin);
    *ICRmaskPort  |= bit (digitalPinToPCMSKbit (encoders [i].aPin));

    // enable this pin-change interrupt
    PCICR |= bit (digitalPinToPCICRbit (encoders [i].aPin));

    } // end of for each encoder

}  // end of setup

void loop ()
{
  if (fired)
    {
    for (byte i = 0; i < ENCODERS; i++)
      {
      char buf [10];
      sprintf (buf, "%5d", encoders [i].count);
      Serial.print (buf);
      }
    Serial.println ();
    fired = false;
    }  // end if fired

}  // end of loop

Hi,

Please do not put your question inside code tags, that makes it difficult to read. Put only your code inside the code tags.

When using sprintf() with long variables, you need to use "ld" instead of "d" in the format string:

sprintf (buf, "%5ld", encoders [i].count);

thank you
i change "%5ld" and works fine

why not pass a reference to a struct encoder rather than search for it?

seems like a lot of excessive processing in checkForPinChange().

in code i've written, i use attachInterrupt() to associate the interrupt with one encoder pin and the ISR checks the state of the other encode pin to +/- a count

instead of using "fired" to see of there was a change, why not just report the encoder values every second or so (put a space in front (" %5d")

There can be up to 8 encoders using the same interrupt. The ISR doesn't know which encoder caused the pin change interrupt. The checkForPinChange() goes through all of the encoders, finds the ones that share this ISR, and checks the corresponding pin to see if it has changed. We know that one of them has.

there are unique ISRs for each interrupt that call a common function, checkForPinChange(). pass the encoder reference in the call the checkForPinChange().

this is what i do

// ------------------------------------------------
// read brake
inline void readEncoder (
    byte  dt,
    byte  clk,
    int & brkPos,
    int & encSt )
{
    byte  val = digitalRead (dt) << 1 | digitalRead (clk);

    switch (encAct [val][encSt])  {
    case For:
        brkPos++;
        break;

    case Rev:
        brkPos--;
        break;

    default:
        break;
    }

    encSt = val;

 // digitalWrite (LED, ! digitalRead (LED));
}

// -------------------------------------
void IRAM_ATTR isrEncA (void)
{
    readEncoder (Enc_A_Dt, Enc_A_Clk, encApos, encAst);
}

// -------------------------------------
void IRAM_ATTR isrEncB (void)
{
    readEncoder (Enc_B_Dt, Enc_B_Clk, encBpos, encBst);
}

But not a unique interrupt/ISR per pin. They are Pin Change Interrupts. One ISR per PORT. Each PORT can have up to 8 different Pin Change Interrupts enabled. SOMETHING has to determine which pin changed and which encoder that pin is for.

i don't understand based on what i see the code doing

void checkForPinChange (const byte which)
{
    for (byte i = 0; i < ENCODERS; i++)
    {
        if (encoders [i].whichInterrupt == which)

why not pass "i"?

what exactly is "which" (e.g. PCIE0), a port, and bit of a port? looks like it's a 1-to-1 mapping between a pin and an interrupt ID.

if the interrupt were for the port, wouldn't the code need to look at a register to determine which pin on that port has a pending interrupt?

Because the interrupt might be for any one of several i's. The ISR know which port it is for but not which bit within that port it is for.

In the example (assuming an UNO) pins 4, 5, 6, and 7 are pins PD4, PD5, PD6 and PD7. Those are all on PORTD which, I think, is supported by PCIE2. The PCINT2_vect ISR is called when any one of those pins changes. The ISR does not know which of the four it was.

It's just the PORT.
PORTB (PB0-PB5, Pins 8-13) -> PCIE0
PORTC (PC0-PC5, Pins A0-A5) -> PCIE1
PORTD (PD0-PD7, Pins 0-7) -> PCIE2

There is a separate ENABLE bit per pin but not a separate FLAG bit per pin. There is only the one FLAG bit for the whole port. The interrupt indicates that one (or more) of the enabled pins has changed. The purpose of checkForPinChange() is to determine which of the encoder pins is in that port and check those pins to see if they have changed.

Don't count on this to work for four encoders in practical applications, on an MCU like an Arduino Uno.

If the encoders are connected to shafts that rotate with significant speed, there won't be enough processing time to even handle all the interrupts, let alone do anything else.

so the one ISR event causes multiple encoders to be updated ????

if PCIE0 represents a port, how do check() distinguish the bit on the port?

It should be obvious that the ISR code has to maintain a copy of the most recent port bit configuration, and determine which bit(s) changed.

i'm sure it does.

but how does checkForPinChange() discern this from its argument, "which" when it's value is PCIE0?

The OP's code appears to be misguided, and therefore does not work.

digitalPinToPCICRbit(pin) returns the PCI Enable Bit (PCIEn) in the PCI Control Register (PCICR) that enables the interrupt for the port the pin is in.
PCIE0 -> PORTB
PCIE1 -> PORTC
PCIE2 -> PORTD

The checkForPinChange() goes through all of the encoders, finds the one(s) that share this PCI Enable, and checks the corresponding pin to see if it has changed:
if (newValue != encoders [i].oldValue)
We know that one of them has because the interrupt flag got set.

ok. i didn't realize that line was reading a port register indicating that the interrupt for that bit is set. but don't understand why it checks for a change, doesn't the interrupt bit get cleared during the processing of the bit. does the bit simply toggle whenever an interrupt occurs? isn't there a port register to clear the interrupt?

i wonder if checkForPinChange() would be simpler if it simply checked all possible registers and interrupt bits, processing all (and clearing all that are processed).

It doesn't. There isn't a separate interrupt flag for each pin. All of the pins on that port share a flag. The "newValue" is the current value of the INPUT pin:

byte newValue = *(encoders [i].aPort) & encoders [i].aBitMask;

For example, if encoders[i].aPort' is PORTDandencoders[i].aBitMaskis0b00010000` we would be looking at Pin 4 (PD4). If the pin is HIGH, newValue would be 0b00010000. If the pin is LOW, newValue would be 0b00000000.

We know that one of the pins on that port has changed but we don't know which one. Looking for a pin that has changed is the way to determine which pin (or pins) caused the Pin Change Interrupt .

ok
the interrupt indicates that an interrupt has occurred for one of the bits configured as an interrupt ... and it appears the application code is responsible for figuring out which. can the interrupt be configured as Change, Rising, Falling

what processor is this for?

i'm more familiar with a set of registers for gpio bits setting direction, value input value, enabling interrupts, type for interrupt, interrupt status, clearing an interrupt ... on any GPIO. hence my previous comments

No, just 'change'. That's why it's called a "Pin Change Interrupt". :slight_smile: