Nano Every Encoder Problem

Hey Guys.

I got an original brand new Nano Every ATMEGA4809. I want to build a DRO for my lathe and I choose the Nano Every because I need at least 8 Interrupt Pins, for 4 rotary Encoders.

I've tested my code with a standard chinese Nano Clone and it worked like a charm, no missing steps, simply perfect. As rotary encoder I used this kind (600 p/r):
https://www.amazon.com/Aideepen-Incremental-Rotary-Encoder-Voltage/dp/B08352H8S7

I wired it up like this:
Nano 5V -> Enc 5V
Nano GND -> Enc GND
Nano D2 -> Enc A
Nano D3 -> Enc B

I give out the result via Serial Port.

Now my problem is, as long as I rotate the encoder very slow the Nano Every is counting the steps without a problem, but if I rotate to fast or change direction to fast it immediately stops counting until I restart the Serial Monitor.

//NANO EVERY Config

int encoderPinA = 2;
int encoderPinB = 3;

float encoderPos = 0;
float lastReportedPos = 1;
static boolean rotating = false;

boolean A_set = false;            
boolean B_set = false;

float metricMM = 0.00;

void setup() {
  pinMode(encoderPinA, INPUT); 
  pinMode(encoderPinB, INPUT); 
  
  digitalWrite(encoderPinA, HIGH);
  digitalWrite(encoderPinB, HIGH);

  attachInterrupt(digitalPinToInterrupt(encoderPinA), doEncoderA, CHANGE);
  attachInterrupt(digitalPinToInterrupt(encoderPinB), doEncoderB, CHANGE);

  Serial.begin(115200);
}

void loop() { 
  rotating = true;

  if (lastReportedPos != encoderPos) {
    stepsToMM();
    getSerialData();
    lastReportedPos = encoderPos;
  }
}

void stepsToMM() {
  metricMM = ((4 * encoderPos) / 600);
}


void doEncoderA() {
  if ( rotating ) delay (1);
  if( digitalRead(encoderPinA) != A_set ) {
    A_set = !A_set;

    if ( A_set && !B_set ) 
      encoderPos += 1;
    rotating = false;
  }
}

void doEncoderB() {
  if ( rotating ) delay (1);
  if( digitalRead(encoderPinB) != B_set ) {
    B_set = !B_set;
    if( B_set && !A_set ) 
      encoderPos -= 1;
    rotating = false;
  }
}


void getSerialData() {
  Serial.print(encoderPos);
  Serial.print("     ");
  Serial.print(metricMM, 2);
  Serial.println();
}

Hope someone can help. It must be a fault of the Nano Every bcause al works great on common Nanos, even on clones.

King regards

This does not solve your issue but it is something you should know.

Interrupt Service Routines should be quick. Putting an intentional delay (even if it is only one millisecond) into an Interrupt Service Routine violates this guideline.

Besides, I thought (perhaps incorrectly) that the delay(...) function should not be in an Interrupt Service Routine because it depends upon interrupts and thus will occasionally fail.

you shouldn't call delay() in your interrupt routine

All variables that are used in both the ISR and the remainder of your code should be declared as volatile.

When accessing a variable that is also used in an ISR, you should temporarily disable interrupts to prevent an interrupt occurring during the access (the processor can only access a variable one byte at a time, you are using a float which is four bytes, leaving ample opportunity for an interrupt). Since you are using encoderPos several times, it would be better to copy the value to another variable, then work from that, otherwise you have no guarantee of a consistent value through the calculations and printing.

Why are you using a float for encoderPos? The encoder position will never change by a fractional amount.

david_2018:
All variables that are used in both the ISR and the remainder of your code should be declared as volatile.

why? i don't in my ISRs

we use to only declare pointers to registers as volatile because only the hardware knew when the value changed

gcjr:
why? i don't in my ISRs

we use to only declare pointers to registers as volatile because only the hardware knew when the value changed

Because an interrupt can occur at any time, the compiler cannot possibly know when an interrupt occurs and which values in memory might have changed. When an ISR occurs, a value in memory could have changed, but the old value might still be cached in a register, resulting in inconsistent results. It gets even worse when dealing with multibyte data, as explained by david_2018.
By marking a variable as 'volatile', you inform the compiler that its value might change at any time, and it should load it from memory each time it is accessed in the code, instead of keeping it cached in a register, or optimizing it out.

I'm going to copy the rest from an example I posted earlier:

PieterP:
The compiler doesn't know what happens in interrupts, it assumes variables can't just change at any time, that would be silly. When you have variables that can change without the compiler knowing, you have to specifically mark them volatile.

Consider this:

bool flag = false;

void loop() {
 while (!flag) { /* wait */ }
 // Other stuff
}

ISR(something) {
 flag = true;
}



If you look at just the loop function, the compiler doesn't see any reason why flag would ever change. If flag doesn't change, why waste time reading it from memory each time? Without taking the ISR into account, the code is equivalent to.



void loop() {
 if (!flag)
   while (true); // endless loop
 // Other stuff
}




That changes when you mark the flag volatile, because it forces the compiler to read the flag from memory on each iteration, and evaluate the condition again. It's no longer an unconditional endless loop.

Pieter

thanks for your advice to delete the "delay". Only doing this, did the trick