Encoder ISR hanging loop?

Hello all! I am making a USB media controller with an Arduino Pro Micro. It uses 2 rotary encoders, one for "Volume" and one for page scrolling "Scroll". I am using a laptop touchpad for mouse cursor movement along with 2 tactile momentary switches for "Next " track and "Previous" track. Everything works well until I start moving the volume encoder a few clicks or the scroll encoder. The rest of the loop stops responding and I can then only change volume or scroll. Media play/ pause (connected to press switch on encoder) along with Next, Previous and all mouse functions will no longer function. If I reset the Arduino and don't touch the encoders, all other functions work properly (Mouse, Next track, Previous and Play/Pause). When I print any int to the serial monitor, the monitor halts when an Interrupt Service Routine is triggered. I'm sure it is something I have overlooked. Any assistance is greatly appreciated, thank you in advance!

// Download the HID and FAST LED Library before compiling
#include <HID-Project.h>
#include <HID-Settings.h>
#include "PS2Mouse.h"
#define DATA0 3            // To Encoder Data 0 - Interrupt Pin Volume
#define DATA1 2            // To Encoder Data 1 - Interrupt Pin Volume
#define DATA2 1            // To Encoder Data 0 - Interrupt Pin Scroll
#define DATA3 0            // To Encoder Data 1 - Interrupt Pin Scroll
#define MUTE 7             // To Touch Switch Data pin Mute
#define NEXT 6             // To Touch Switch Data pin Mute
#define PAUSE 8            // To Touch Switch Data pin Mute
#define PREVIOUS 5         // To Touch Switch Data pin Mute
PS2Mouse PS2mouse(10, 9);  // 9,10 Mouse data and clock pins


int oldData0 = 0, oldData1 = 0, oldButtonState = 0, oldButtonState1 = 0, oldButtonState2 = 0, oldButtonState3 = 0;  // To store the state of encoder
int oldData2 = 0, oldData3 = 0;
int val;
int b;

void setup() {

  PS2mouse.begin();
  pinMode(MUTE, INPUT);
  pinMode(DATA0, INPUT);
  pinMode(DATA1, INPUT);
  pinMode(DATA2, INPUT);
  pinMode(DATA3, INPUT);
  pinMode(NEXT, INPUT);
  pinMode(PREVIOUS, INPUT);
  pinMode(PAUSE, INPUT);

  digitalWrite(DATA0, HIGH);
  digitalWrite(DATA1, HIGH);
  digitalWrite(DATA2, HIGH);
  digitalWrite(DATA3, HIGH);
  digitalWrite(NEXT, HIGH);
  digitalWrite(PREVIOUS, HIGH);
  digitalWrite(PAUSE, HIGH);

  attachInterrupt(digitalPinToInterrupt(DATA0), volume, CHANGE);
  attachInterrupt(digitalPinToInterrupt(DATA1), volume, CHANGE);
  attachInterrupt(digitalPinToInterrupt(DATA2), scroll, CHANGE);
  attachInterrupt(digitalPinToInterrupt(DATA3), scroll, CHANGE);
  delay(3000);

  Consumer.begin();
}


void loop() {
  {

    uint8_t stat;

    int x, y;

    PS2mouse.getPosition(stat, x, y);

    Mouse.move(x * 5, -y * 5, 0);

    if (bitRead(stat, 0) == 1) {

      Mouse.press();

    }


    else {

      Mouse.release();
    }
  }

  //MEDIA_NEXT right


  int newButtonState1 = digitalRead(NEXT);

  if (oldButtonState1 != newButtonState1) {
    // If it did and the new button state is 1 (button pressed down)
    if (newButtonState1 == 1) {
      // See HID Project documentation for more Consumer keys
      Consumer.write(MEDIA_NEXT);
    }

    oldButtonState1 = newButtonState1;
  }


  //MEDIA_PREVIOUS Left

  int newButtonState2 = digitalRead(PREVIOUS);

  if (oldButtonState2 != newButtonState2) {
    // If it did and the new button state is 1 (button pressed down)
    if (newButtonState2 == 1) {
      // See HID Project documentation for more Consumer keys
      Consumer.write(MEDIA_PREVIOUS);
    }

    oldButtonState2 = newButtonState2;
  }

  //Media PAUSE PLAY

  int newButtonState3 = digitalRead(PAUSE);

  if (oldButtonState3 != newButtonState3) {
    // If it did and the new button state is 1 (button pressed down)
    if (newButtonState3 == 1) {

      val = 0;
      Serial.println("Count reset!");
      // See HID Project documentation for more Consumer keys
      Consumer.write(MEDIA_PLAY_PAUSE);
      ;
    }

    oldButtonState3 = newButtonState3;
  }
}

void scroll()  // Encoder Interrupt Function
{
  //Initalise Timer
  TIMSK1 |= (1 << TOIE1);
  TCCR1A &= (~(1 << WGM10)) & (~(1 << WGM11));
  TCCR1B &= (~(1 << WGM12)) & (~(1 << WGM13));
  TCCR1B |= (1 << CS12) | (1 << CS10);
  TCCR1B &= (~(1 << CS11));
  TCNT1 = 0;

  int newData2 = digitalRead(DATA2);
  int newData3 = digitalRead(DATA3);

  if (newData2 != oldData2 || newData3 != oldData3) {
    // Check both data pins and check if the state changed
    // compared to the last cycle
    if (!oldData2 && !oldData3 && !newData2 && newData3) {
      Mouse.move(0, 0, 1);
    }


    if (!oldData2 && !oldData3 && newData2 && !newData3) {
      Mouse.move(0, 0, -1);
    }

    oldData2 = newData2;
    oldData3 = newData3;
  }
}



void volume()  // Encoder Interrupt Function
{
  //Initalise Timer
  TIMSK1 |= (1 << TOIE1);
  TCCR1A &= (~(1 << WGM10)) & (~(1 << WGM11));
  TCCR1B &= (~(1 << WGM12)) & (~(1 << WGM13));
  TCCR1B |= (1 << CS12) | (1 << CS10);
  TCCR1B &= (~(1 << CS11));
  TCNT1 = 0;

  int newData0 = digitalRead(DATA0);
  int newData1 = digitalRead(DATA1);

  if (newData0 != oldData0 || newData1 != oldData1) {
    // Check both data pins and check if the state changed
    // compared to the last cycle
    if (!oldData0 && !oldData1 && !newData0 && newData1) {

      Consumer.write(MEDIA_VOLUME_DOWN);
    }


    if (!oldData0 && !oldData1 && newData0 && !newData1) {
      Consumer.write(MEDIA_VOLUME_UP);
    }

    oldData0 = newData0;
    oldData1 = newData1;
  }
}

ISR(TIMER1_OVF_vect) {
  b = 16;
  TCNT1 = 0;

  TCCR1B &= ~(1 << CS12);
  TCCR1B &= ~(1 << CS11);
  TCCR1B &= ~(1 << CS10);
}

NEVER do serial I/O within an ISR. I/O depends on interrupts, which are off in the ISR. Therefore, the ISR hangs up.

Set a global flag (a byte, declared volatile), which you check and use to trigger I/O in the main loop.

2 Likes

This is exactly what I needed! Thank you SO MUCH! I created a flag for volume and scroll functions, created 2 new functions and called to change the flags within their ISR, resetting after the function is completed. All functions work perfectly now. Thank you! The repaired code below.


```cpp
// Download the HID and FAST LED Library before compiling
#include <HID-Project.h>
#include <HID-Settings.h>
#include "PS2Mouse.h"
#define DATA0 3            // To Encoder Data 0 - Interrupt Pin Volume
#define DATA1 2            // To Encoder Data 1 - Interrupt Pin Volume
#define DATA2 1            // To Encoder Data 0 - Interrupt Pin Scroll
#define DATA3 0            // To Encoder Data 1 - Interrupt Pin Scroll
#define MUTE 7             // To Touch Switch Data pin Mute
#define NEXT 6             // To Touch Switch Data pin Mute
#define PAUSE 8            // To Touch Switch Data pin Mute
#define PREVIOUS 5         // To Touch Switch Data pin Mute
PS2Mouse PS2mouse(10, 9);  // 9,10 Mouse data and clock pins

volatile bool volumeChange = false;
volatile bool scrollChange = false;
int oldData0 = 0, oldData1 = 0, oldButtonState = 0, oldButtonState1 = 0, oldButtonState2 = 0, oldButtonState3 = 0;  // To store the state of encoder
int oldData2 = 0, oldData3 = 0;
int val;
int b;

void setup() {
  Serial.begin(9600);
  PS2mouse.begin();
  pinMode(MUTE, INPUT);
  pinMode(DATA0, INPUT);
  pinMode(DATA1, INPUT);
  pinMode(DATA2, INPUT);
  pinMode(DATA3, INPUT);
  pinMode(NEXT, INPUT);
  pinMode(PREVIOUS, INPUT);
  pinMode(PAUSE, INPUT);

  digitalWrite(DATA0, HIGH);
  digitalWrite(DATA1, HIGH);
  digitalWrite(DATA2, HIGH);
  digitalWrite(DATA3, HIGH);
  digitalWrite(NEXT, HIGH);
  digitalWrite(PREVIOUS, HIGH);
  digitalWrite(PAUSE, HIGH);

  attachInterrupt(digitalPinToInterrupt(DATA0), volumeInterrupt, CHANGE);
  attachInterrupt(digitalPinToInterrupt(DATA1), volumeInterrupt, CHANGE);
  attachInterrupt(digitalPinToInterrupt(DATA2), scrollInterrupt, CHANGE);
  attachInterrupt(digitalPinToInterrupt(DATA3), scrollInterrupt, CHANGE);
  delay(3000);

  Consumer.begin();
}

void scroll()
{
    //Initalise Timer
  TIMSK1 |= (1 << TOIE1);
  TCCR1A &= (~(1 << WGM10)) & (~(1 << WGM11));
  TCCR1B &= (~(1 << WGM12)) & (~(1 << WGM13));
  TCCR1B |= (1 << CS12) | (1 << CS10);
  TCCR1B &= (~(1 << CS11));
  TCNT1 = 0;

  int newData2 = digitalRead(DATA2);
  int newData3 = digitalRead(DATA3);

  if (newData2 != oldData2 || newData3 != oldData3) {
    // Check both data pins and check if the state changed
    // compared to the last cycle
    if (!oldData2 && !oldData3 && !newData2 && newData3) {
      Mouse.move(0, 0, 1);
    }


    if (!oldData2 && !oldData3 && newData2 && !newData3) {
      Mouse.move(0, 0, -1);
    }

    oldData2 = newData2;
    oldData3 = newData3;
    scrollChange = false;
  }
}

void volume()
{
  //Initalise Timer
  TIMSK1 |= (1 << TOIE1);
  TCCR1A &= (~(1 << WGM10)) & (~(1 << WGM11));
  TCCR1B &= (~(1 << WGM12)) & (~(1 << WGM13));
  TCCR1B |= (1 << CS12) | (1 << CS10);
  TCCR1B &= (~(1 << CS11));
  TCNT1 = 0;

  int newData0 = digitalRead(DATA0);
  int newData1 = digitalRead(DATA1);

  if (newData0 != oldData0 || newData1 != oldData1) {
    // Check both data pins and check if the state changed
    // compared to the last cycle
    if (!oldData0 && !oldData1 && !newData0 && newData1) {
      Consumer.write(MEDIA_VOLUME_DOWN);
    }


    if (!oldData0 && !oldData1 && newData0 && !newData1) {
      Consumer.write(MEDIA_VOLUME_UP);
    }

    oldData0 = newData0;
    oldData1 = newData1;
    volumeChange = false;
  }
}

ISR(TIMER1_OVF_vect) {
  b = 16;
  TCNT1 = 0;

  TCCR1B &= ~(1 << CS12);
  TCCR1B &= ~(1 << CS11);
  TCCR1B &= ~(1 << CS10);
}
void loop() {
  {
Serial.begin(9600);
    uint8_t stat;

    int x, y;

    PS2mouse.getPosition(stat, x, y);

    Mouse.move(x * 5, -y * 5, 0);

    if (bitRead(stat, 0) == 1) {

      Mouse.press();

    }


    else {

      Mouse.release();
    }
      Serial.println(volumeChange);
  }

  //MEDIA_NEXT right


  int newButtonState1 = digitalRead(NEXT);

  if (oldButtonState1 != newButtonState1) {
    // If it did and the new button state is 1 (button pressed down)
    if (newButtonState1 == 1) {
      // See HID Project documentation for more Consumer keys
      Consumer.write(MEDIA_NEXT);
    }

    oldButtonState1 = newButtonState1;
  }


  //MEDIA_PREVIOUS Left

  int newButtonState2 = digitalRead(PREVIOUS);

  if (oldButtonState2 != newButtonState2) {
    // If it did and the new button state is 1 (button pressed down)
    if (newButtonState2 == 1) {
      // See HID Project documentation for more Consumer keys
      Consumer.write(MEDIA_PREVIOUS);
    }

    oldButtonState2 = newButtonState2;
  }

  //Media PAUSE PLAY

  int newButtonState3 = digitalRead(PAUSE);

  if (oldButtonState3 != newButtonState3) {
    // If it did and the new button state is 1 (button pressed down)
    if (newButtonState3 == 1) {

      val = 0;
      Serial.println("Count reset!");
      // See HID Project documentation for more Consumer keys
      Consumer.write(MEDIA_PLAY_PAUSE);
      ;
    }

    oldButtonState3 = newButtonState3;
  }
if(scrollChange == true)
{
  scroll();
}
if(volumeChange == true)
{
  volume();
}
}

void scrollInterrupt()  // Encoder Interrupt Function
{
scrollChange = true;
}



void volumeInterrupt()  // Encoder Interrupt Function
{
volumeChange = true;
}

Excellent!

Followup question if I may, if using all the functions as intended now the serial monitor crashes and USB stops responding at random. Sometimes in 30 seconds, sometimes after a few minutes moving the encoders in a normal manner. I need to get all the components soldered into an enclosure as I've noticed if the trackpad data or clock pins move it freezes the sketch/ serial monitor/ USB serial. Is this a coding error by me or just the Arduino working as designed? Id I am very careful to not touch the trackpad I can still however "get it to act up" by turning an encoder. Since the serial monitor freezes it's hard to look at different variables to find the problem. Any ideas?

Post the latest, revised code, using code tags.

This is a very strange construction to have in loop(). Serial.begin() should not be here. What is the purpose of making this bit into a code block?

 {
Serial.begin(9600);
    uint8_t stat;

    int x, y;

    PS2mouse.getPosition(stat, x, y);

    Mouse.move(x * 5, -y * 5, 0);

    if (bitRead(stat, 0) == 1) {

      Mouse.press();

    }

What are all these sorts of manipulations of timer control variables supposed to do? Changing timer variables on the fly can have many unfortunate consequences.

ISR(TIMER1_OVF_vect) {
  b = 16;
  TCNT1 = 0;

  TCCR1B &= ~(1 << CS12);
  TCCR1B &= ~(1 << CS11);
  TCCR1B &= ~(1 << CS10);
}

The up to date code is the code in post 3. I'm sorry I didn't mention that.
For some reason I have a hard time getting Serial.println(*); to work if Serial.begin(); isn't in loop.

I was unaware that x and y weren't available outside of this block. Should I make them global variables or place them in setup?

The timer code is from pieces of an HID example I have been trimming. Those are the only parts of that code I didn't understand and was next to be researched before uncommenting/ deleting.

Thank you so much for your time and responses as well as your patience.

Serial.begin() should not be called repeatedly, so something else is very wrong.

I was unaware that x and y weren't available outside of this block.

Variables declared within a block or a function are local variables and can be used only within the block or function. Since you don't seem to use x and y anywhere else, it does not matter.

Sorry, I don't understand what much of the code is supposed to do, and your description of the problem doesn't give me any ideas on what might be wrong, except for points I've already mentioned. You need to add comments to explain the intention of code lines.

I recommend to use the Arduino encoder library, rather than cobble something together.

Ok. I will try using the encoder library and see if that works better. Thank you

Thank you for the heads up, I wasn't aware that Serial.begin() shouldn't be in the loop. I've tried Serial.begin() in setup only and now it works fine so I don't know what I did wrong before (not having it in loop) but I know better now. I appreciate the guidance.

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