[SOLVED] Rotary Encoder slow update

[anishkgt]

I had tested the rotary encoder separately using

int PinA = 13;
int PinB = 12;
int Counter = 0;
int PinALastState = LOW;
int pinAState = LOW;
int prevRotaryVal;

void setup() {
  pinMode (PinA, INPUT_PULLUP);
  pinMode (PinB, INPUT_PULLUP);
  Serial.begin (9600);
}

int rotaryVal()
{
  pinAState = digitalRead(PinA);
  if (pinAState != PinALastState)
  {
    if (digitalRead(PinB) != pinAState) {
      Counter++;
    } else {
      Counter--;
    }
  }
  PinALastState = pinAState;
  if (Counter >= 500)
  {
    Counter = 500;
  }
  if (Counter <= 0)
  {
    Counter = 0;
  }
  return Counter >> 1;
}

void loop()
{
  if (prevRotaryVal != rotaryVal())
  {
    prevRotaryVal = rotaryVal();
    Serial.println(rotaryVal() * 2);
  }
}

and it works well.

now using the same concept in my project (sketch attached) it just acts weird. Sometimes it updates to serial monitor as 1 0 several times after turning the knob but applying a little downward pressure to the knob it updates better but very slow. I even tried all the delay's and still no change.

Could somebody please help me here.

Thanks in advance

sketch.txt (16.1 KB)

[jpk]

I had the same problem, here is a working solution, which works also for multiple encoders:

const uint8_t pinA = 4;
const uint8_t pinB = 5;

int counter = 0;

Pio* portA = g_APinDescription[pinA].pPort;
Pio* portB = g_APinDescription[pinB].pPort;
const uint32_t maskA = g_APinDescription[pinA].ulPin;
const uint32_t maskB = g_APinDescription[pinB].ulPin;

boolean chanA;
boolean chanB;

int8_t oldstate = 3;
int enc = 0;
int val = 0;
int old = 0;

const uint8_t detents = 3;

const int8_t enctable[] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0};

// half step:
// const int8_t enctable[] = {0, 0, -1, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, -1, 0, 0};

void setup() {
  Serial.begin(115200);
  pinMode(pinA, INPUT_PULLUP);
  pinMode(pinB, INPUT_PULLUP);
  attachInterrupt(pinA, isr, CHANGE);
  attachInterrupt(pinB, isr, CHANGE);
}

void loop() {
 update();
}

void isr() {
  chanA = portA -> PIO_PDSR & maskA; // instead of digitalRead(pinA);
  chanB = portB -> PIO_PDSR & maskB; // instead of digitalRead(pinB);
  int8_t state = chanA | (chanB << 1);
  if (oldstate != state) {
    enc += enctable[state | (oldstate << 2)];
    if (state == detents)
      val = enc >> 2;
    oldstate = state;
  }
}

void update() {
  if (val > old) {
    counter++;
    Serial.println(val);
    old = val;
  }
  else if (val < old) {
    counter--;
    Serial.println(val);
    old = val;
  }
}

Without interrupt just poll the pins in loop(). Tested on DUE.

[anishkgt]

Thanks mate but am not very good with the arduino and this code is a bit hard for me to understand.

still looking for a simper one.

[Robin2]

anishkgt:
and it works well.

now using the same concept in my project (sketch attached) it just acts weird.

You have added a huge amount of code that is slowing down loop() - all the LCD stuff and all the delay()s.

You probably need to use an interrupt to read the encoder - but something simpler than proposed in Reply #1 may be sufficient. I think this might work

void encoderISR()
{
  // we know pinA has just gone from LOW to HIGH
 
  if (digitalRead(PinB) == LOW) {
      Counter++;
  } else {
      Counter--;
  }
}

assuming pinA is connected to pin 2 and in setup() you have

attachInterrupt(0, encoderISR, RISING);

and at the top of the program you have

volatile int counter;

and the code in loop() to read the value of counter should be

noInterrupts();
  counterCopy = counter;
interrupts();

and then do all your calculations using counterCopy

...R

[anishkgt]

I feared so, i was trying to avoid component rearrangement as out of two intterupts i have one used for Zero cross detection and the second pin is already assigned to switch on a triac.

[Robin2]

You can use pinChange interrupts on most pins. They are just a little more complex as they trigger for LOW to HIGH and HIGH to LOW so you need to test the value of the pin that triggers the interrupt.

...R

[dougp]

A brief discussion on Robin2's suggestion.

[Southpark]

dougp:
A brief discussion on Robin2's suggestion.

Very nice discussion! I think a helpful addition there is maybe something like..... external interrupt pin is associated with a pin operating as an input pin. And the rest of those other interrupts assignable to various pins are associated with pins operating as output pins.

[cattledog]

@anishkgt

Given the architecture of your program, you may want to think through the human interface in some detail, rather than just patch in an encoder into the main loop. What values in the program are you wanting to adjust with the rotary encoder setting? Is the rotary encoder value replacing a potentiometer value?

You may wish to create a setting adjustment routine which pulls the adjustments, possibly a confirmation of the adjustment, and the display of the changes into a separate loop, possible one which is part of the configuration mode entered by the longer button press.

Putting the encoder on pin change interrupts, and setting a flag to be responded to changes in loop is possible, but given the display requirements, its not clear to me that is the best human interface to the settings.

If you do go to pin change interrupts, I recommend Nico Hood's pinchange interrupt library
which is available through the library manager. The syntax is analagous to that for external interrupts, it can be set to respond to RISING,FALLING or CHANGE. It is very simple to use, and does not require you to work out any pin changes.

[Robin2]

Southpark:
And the rest of those other interrupts assignable to various pins are associated with pins operating as output pins.

I don't think that is quite right.

For the sort of application the OP has in mind the pinChange interrupt pin will be an INPUT or INPUT_PULLUP.

You can have a pinChange interrupt on a pin set as OUTPUT if you want to create a software interrupt by changing the state of the pin with digitalWrite() - but I suspect that would be an unusual use of a pinChange interrupt.

...R

[Southpark]

Robin2:
I don't think that is quite right.

For the sort of application the OP has in mind the pinChange interrupt pin will be an INPUT or INPUT_PULLUP.

You can have a pinChange interrupt on a pin set as OUTPUT if you want to create a software interrupt by changing the state of the pin with digitalWrite() - but I suspect that would be an unusual use of a pinChange interrupt.

...R

Thanks Robin! But..... external interrupts are associated with particular pins on the arduino that are configured as input pins right? That is, an external signal is needed for triggering the interrupt.

The rest of the pin interrupts are based on the arduino's own pin states in output mode right?

[Robin2]

Southpark:
The rest of the pin interrupts are based on the arduino's own pin states in output mode right?

That's true IF they are set as OUTPUT. But they can also be used as INPUT or INPUT_PULLUP in which case they behave exactly the same as an external interrupt in CHANGE mode.

I can't recall if an external interrupt can be triggered if its pin is set as OUTPUT - maybe not.

...R

[cattledog]

I can't recall if an external interrupt can be triggered if its pin is set as OUTPUT

Yes the interrupt will be triggered if the pin is in OUTPUT mode.

byte pulseOut = 2;//output on pin2
unsigned long currentMicros;
unsigned long nextMicros;
unsigned long duration = 25UL; // flip every 50uS = 20KHz pulse
unsigned long elapsedMicros;
unsigned long lastSecond;
volatile unsigned long count;

void countISR()
{
  count++;
}

void setup() {

  pinMode (pulseOut, OUTPUT);
  attachInterrupt(0, countISR, RISING); //INT0 pin2 (Atmega328)
  Serial.begin(115200);
  Serial.println("starting interrupt count");
}
void loop() {
  currentMicros = micros();
  elapsedMicros = currentMicros - nextMicros;
  if (elapsedMicros >= duration) {
    nextMicros = nextMicros + duration;
    PIND = PIND | 0b00000100; // toggle D2 by writing to Input register
  }
  if (micros() - lastSecond >= 1000000L)
  {
    lastSecond += 1000000;
    noInterrupts();
    unsigned long copyCount = count;
    count = 0;
    interrupts();
    Serial.println(copyCount);
  }
}

[Robin2]

cattledog:
Yes the interrupt will be triggered if the pin is in OUTPUT mode.

Thanks. However I don't think your code will have any value for the OP

...R

[cattledog]

However I don't think your code will have any value for the OP

None whatsoever. I figured it was a better answer than simply Yes

[anishkgt]

cattledog:
None whatsoever. I figured it was a better answer than simply Yes

Thank you Cattledog.

The project has three pots, one 10K POT and two trim pots. A turn on the pot was very sensitive when i had to get readings between 1 and 10. Hence thought of getting an encoder when the readings are more accurate with a turn. Moreover that would eliminate the trim pots as well reducing cost as well.

So going forward with encoder, i was reading Gammon Forum about his library and to how much brain can process i understand that just adding this string of code

attachInterrupt(digitalPinToInterrupt (PinA), rotaryVal, CHANGE);

Would solve my problem. yet TO try this out because am at work now.

[Robin2]

anishkgt:
to how much brain can process i understand that just adding this string of code

Pretty much the same as Reply #3 ?

...R

[anishkgt]

i just could to get back home so downloaded the IDE and compiled and it compiles ok but with the below log

In file included from sketch\RotaryTestCommonCathode.ino.cpp:1:0:

C:\Program Files (x86)\Arduino\hardware\arduino\avr\cores\arduino/Arduino.h:150:6: note: initializing argument 2 of 'void attachInterrupt(uint8_t, void (*)(), int)'

 void attachInterrupt(uint8_t, void (*)(void), int mode);

Should this be of any concern in the long run for the project ?

[anishkgt]

It even shows on the test sketch as well

#include <PinChangeInterrupt.h>
int PinA = 13;
int PinB = 12;
int Counter = 0;
int PinALastState = LOW;
int pinAState = LOW;
int prevRotaryVal;

void setup() {
  pinMode (PinA, INPUT_PULLUP);
  pinMode (PinB, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt (12), rotaryVal, CHANGE);
  Serial.begin (9600);
}

int rotaryVal()
{
  pinAState = digitalRead(PinA);
  if (pinAState != PinALastState)
  {
    if (digitalRead(PinB) != pinAState) {
      Counter++;
    } else {
      Counter--;
    }
  }
  PinALastState = pinAState;
  if (Counter >= 500)
  {
    Counter = 500;
  }
  if (Counter <= 0)
  {
    Counter = 0;
  }
  return Counter >> 1;
}

void loop()
{
  if (prevRotaryVal != rotaryVal())
  {
    prevRotaryVal = rotaryVal();
    Serial.println(rotaryVal());
  }
}

[cattledog]

attachInterrupt(digitalPinToInterrupt (12), rotaryVal, CHANGE);

Pin 12 is not an external interrupt pin. Did you install the recommended pin change interrupt library?

If you do go to pin change interrupts, I recommend Nico Hood's pinchange interrupt library
which is available through the library manager. The syntax is analagous to that for external interrupts, it can be set to respond to RISING,FALLING or CHANGE. It is very simple to use, and does not require you to work out any pin changes.

If so, then use
attachPCINT(digitalPinToPCINT(12), rotaryVal, CHANGE);