Rotary encoder - problem printing data to serial

Hello,
I hooked up an incremental rotary encoder to Arduino Uno R3 and with the following sketch I manage to make it work repeatably without losing steps.
But the strange thing is when I open the serial monitor there is nothing printed not even a single line. And when I spin the sensor the data show up on the serial. if I zero the position of the encoder with the reset switch and I don't touch the sensor nothing changes, the last printed data just stay in pause, only if I spin again the encoder it shows the zero value and start printing the new value.
The serial monitor acts like in sleep mode. If the sensor doesn't move nothing shows if the sensor move data appear on the serial.

#define ENC_RD PIND  //encoder port read
#define A_PIN 2      // pdip 4, associated with INT0 vector; PD2
#define B_PIN 3      // pdip 5, associated with INT1 vector; PD3
#define rstbtn 4     // reset button
long counter = 0;
const float R = 0.15;
float distance = 0;

void setup() {
  Serial.begin(115200);
  pinMode(A_PIN, INPUT_PULLUP);
  pinMode(B_PIN, INPUT_PULLUP);
  pinMode(rstbtn, INPUT_PULLUP);
  attachInterrupt(0, evaluateRotary, CHANGE);
  attachInterrupt(1, evaluateRotary, CHANGE);
}

void evaluateRotary() {
  /* encoder routine. Expects encoder with four state changes between detents */
  /* and both pins open on detent */

  static uint8_t old_AB = 3;                                                                          //lookup table index
  static int8_t encval = 0;                                                                           //encoder value
  static const int8_t enc_states[] PROGMEM = { 0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0 };  //encoder lookup table
  /**/
  old_AB <<= 2;  //remember previous state
  old_AB |= ((ENC_RD >> 2) & 0x03);
  encval += pgm_read_byte(&(enc_states[(old_AB & 0x0f)]));

  if (encval > 3) {  //four steps forward
    counter++;
    encval = 0;
  } else if (encval < -3) {  //four steps backwards
    counter--;
    encval = 0;
  }
}



void resetCounter() {
  noInterrupts();
  counter = 0;
  interrupts();
}

void loop() {
  static long lastCounter = 0;
  distance = R * counter;

  if (counter != lastCounter) {
    // Serial.print("Counter value: ");
    // Serial.println(counter, DEC);
    lastCounter = counter;
    //Serial.print("Track: ");
    Serial.println();
    Serial.print(distance);
  }
  if (digitalRead(rstbtn) == LOW) {
    resetCounter();
  }
}

Thank you

When Serial monitor is opened a reset is sent to the controller.

This is the reason:

  if (counter != lastCounter) {

Since counter is used in an ISR, it should be declared volatile:

volatile long counter;

Since counter is a multi byte variable, access to it outside of ISRs must be done with interrupts off.

Most ppl grab a copy real quick and use that elsewhere:

    noInterrupts();
    long myCounterCopy = counter;
    interrupts();

You may have been "getting away" without doing, but you won't always be so lucky.

Implement those simple changes and see if anything is different.

And what @Railroader pointed you at.

a7

Thanks, guys, the suggested changes work but now the reset button doesn't work. I am missing something small again. Would you be able to have a peek?
This is the edited code:

#define ENC_RD PIND  //encoder port read
#define A_PIN 2      // pdip 4, associated with INT0 vector; PD2
#define B_PIN 3      // pdip 5, associated with INT1 vector; PD3
#define rstbtn 4     // reset button
volatile long counter = 0;
const float R = 0.15;
float distance = 0;

void setup() {
  Serial.begin(115200);
  pinMode(A_PIN, INPUT_PULLUP);
  pinMode(B_PIN, INPUT_PULLUP);
  pinMode(rstbtn, INPUT_PULLUP);
  attachInterrupt(0, evaluateRotary, CHANGE);
  attachInterrupt(1, evaluateRotary, CHANGE);
}

void evaluateRotary() {
  /* encoder routine. Expects encoder with four state changes between detents */
  /* and both pins open on detent */

  static uint8_t old_AB = 3;                                                                          //lookup table index
  static int8_t encval = 0;                                                                           //encoder value
  static const int8_t enc_states[] PROGMEM = { 0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0 };  //encoder lookup table
  /**/
  old_AB <<= 2;  //remember previous state
  old_AB |= ((ENC_RD >> 2) & 0x03);
  encval += pgm_read_byte(&(enc_states[(old_AB & 0x0f)]));

  if (encval > 3) {  //four steps forward
    counter++;
    encval = 0;
  } 
  else if (encval < -3) {  //four steps backwards
    counter--;
    encval = 0;
  }
}



void resetCounter() {
  noInterrupts();
  long myCounterCopy = counter;
  interrupts();
}

void loop() {
  static long lastCounter = 0;
  distance = R * lastCounter;
  lastCounter = counter;
  Serial.println();
  Serial.print(distance);
  
  if (digitalRead(rstbtn) == LOW) {
    resetCounter();
  }
}

You had this correct in the earlier post.

The protected reading of the counter value should also be in loop.

 noInterrupts();
 lastCounter = counter;
  interrupts();

This, what I think @cattledog was pointing out, isn't just wrong, it's really wrong.

void resetCounter() {
  noInterrupts();
  long myCounterCopy = counter;
  interrupts();
}

Because myCounterCopy is a local variable, it gets set and then forgotten when the function exits. So the whole function amounts to nothing, well it turns interrupts off, then back on again.

a7

I got a bit confused here. I tried the cattledog suggestion and nothing happened. The reset button still doesn't work.

So, Alto777 excuses my lack of knowledge, but I am not sure what your advice is.

Post the latest version that compiles but does not do what you want.

Your original

void resetCounter() {
  noInterrupts();
  counter = 0;
  interrupts();
}

is fine. It protects the access to counter. And will set it to zero. The global variable named counter.

Your loop() does not protect access to counter. See

void loop() {
  static long lastCounter = 0;
  distance = R * lastCounter;


// this use of 'counter' must be protected from interruption:

  noInterrupts();
  lastCounter = counter;
  interrupts();


  Serial.println();
  Serial.print(distance);
  
  if (digitalRead(rstbtn) == LOW) {
    resetCounter();
  }
}

Not clear what you are trying to do, just focusing on the interrupt/noInterruot issue.

a7

Thanks alto777, got it now with your explanation. Thanks a lot. Now all is working as it should.
Here is the full working code if someone what to play with it.

#define ENC_RD PIND  //encoder port read
#define A_PIN 2      // pdip 4, associated with INT0 vector; PD2
#define B_PIN 3      // pdip 5, associated with INT1 vector; PD3
#define rstbtn 4     // reset button
volatile long counter = 0;
const float R = 0.15;
float distance = 0;

void setup() {
  Serial.begin(115200);
  pinMode(A_PIN, INPUT_PULLUP);
  pinMode(B_PIN, INPUT_PULLUP);
  pinMode(rstbtn, INPUT_PULLUP);
  attachInterrupt(0, evaluateRotary, CHANGE);
  attachInterrupt(1, evaluateRotary, CHANGE);
}

void resetCounter() {
  noInterrupts();
  counter = 0;
  interrupts();
}

void loop() {

  static long lastCounter = 0;
  distance = R * lastCounter;

  // this use of 'counter' must be protected from interruption:

  noInterrupts();
  lastCounter = counter;
  interrupts();

  Serial.println();
  Serial.print(distance);

  if (digitalRead(rstbtn) == LOW) {
    resetCounter();
  }
}

void evaluateRotary() {
  /* encoder routine. Expects encoder with four state changes between detents */
  /* and both pins open on detent */

  static uint8_t old_AB = 3;                                                                          //lookup table index
  static int8_t encval = 0;                                                                           //encoder value
  static const int8_t enc_states[] PROGMEM = { 0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0 };  //encoder lookup table
  /**/
  old_AB <<= 2;  //remember previous state
  old_AB |= ((ENC_RD >> 2) & 0x03);
  encval += pgm_read_byte(&(enc_states[(old_AB & 0x0f)]));

  if (encval > 3) {  //four steps forward
    counter++;
    encval = 0;
  } else if (encval < -3) {  //four steps backwards
    counter--;
    encval = 0;
  }
}

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.