Encoder sensor and rotary encoder working separately but not together

Hi

Please see my full code below. On an UNO, I’ve successfully run two sections of code separately from BrainyBits website, speed sensor and rotary encoder dial. Note that I used an L298N driver.

When I combine the code, however, the motors won’t spin. Both parts of the code uses interrupts, so I’m using pins digital pins 2 and 3 as interrupts. The following section of code seems to be interfering with the spin of the motors, because when I comment it out, the motors spin again (but I lose the encoder sensor reading).

     Timer1.initialize(1000000); // set timer for 1sec
     attachInterrupt(0, docount, RISING);  // interrupt 0 is pin 2 - increase counter when speed sensor pin goes High
     Timer1.attachInterrupt( timerIsr ); // enable the timer

How, please, can I make these 2 bits of awesome code work together? I have tried putting “volatile” prefixes on all variables as apparently I need them when variables are used in both the interrupt and other parts of the program. This had no effect so I removed the volatile commands that weren’t already there. Please note that I am doing this to try to implement PID speed control after I fix this. If there is an easier way to do all this then please let me know!

In fact, are there actually 3 interrupts? timer (internal), encoder sensor, rotary encoder (external)?

// for motors
    #define Motor1Dir1 7  // control direction for motor (2 pins required)
    #define Motor1Dir2 8  // control direction for motor (2 pins required)
    #define Motor1PWM 9 // PWM output to motor

//for timer
    #include <TimerOne.h>
    unsigned int counter=0;

//for rotary encoder dial
    volatile boolean TurnDetected;  // need volatile for Interrupts
    volatile boolean up;

    const int PinCLK=3;   // Generating interrupts using CLK signal
    const int PinDT=5;    // Reading DT signal
    const int PinSW=4;    // Reading Push Button switch

    // Interrupt routine runs if CLK goes from HIGH to LOW
    void isr ()  {
     delay(4);  // delay for Debouncing
     if (digitalRead(PinCLK))
       up = digitalRead(PinDT);
     else
       up = !digitalRead(PinDT);
     TurnDetected = true;
    }

//for timer
    void docount()  // counts from the speed sensor
    {
      counter++;  // increase +1 the counter value
    } 

    void timerIsr()
    {
      Timer1.detachInterrupt();  //stop the timer
      Serial.print("Motor Speed: "); 
      //try to get 2 decimal places. if I can't, make it rpm. Or maybe it's because the sensor output is digital (change to analog?)
      int rotation = (counter / 20);  // divide by number of holes in Disc and correct factor (6)
      Serial.print(rotation,DEC);  
      Serial.println(" rps"); 
      counter=0;  //  reset counter to zero
      Timer1.attachInterrupt( timerIsr );  //enable the timer
     }

void setup ()  {
     Serial.begin (9600);

//for timer
     Timer1.initialize(1000000); // set timer for 1sec
     attachInterrupt(0, docount, RISING);  // interrupt 0 is pin 2 - increase counter when speed sensor pin goes High
     Timer1.attachInterrupt( timerIsr ); // enable the timer

//for rotary encoder dial
     pinMode(PinCLK,INPUT);
     pinMode(PinDT,INPUT);  
     pinMode(PinSW,INPUT);
     digitalWrite(PinSW, HIGH); // Pull-Up resistor for switch
 
     attachInterrupt (1,isr,FALLING); // CHANGING IT TO interrupt 1 (pin 3)

     Serial.println("Start");
     pinMode(Motor1Dir1, OUTPUT ); pinMode(Motor1Dir2, OUTPUT );
     pinMode(Motor1PWM, OUTPUT );
     digitalWrite(Motor1Dir1, LOW ); digitalWrite(Motor1Dir2, LOW );// Set motor to off
     digitalWrite(Motor1PWM, LOW );
}

void loop ()  {
 static long RotaryPosition=0;    // STATIC to count correctly
 if (!(digitalRead(PinSW))) {   // check if button is pressed
   if (RotaryPosition == 0) {  // check if button was already pressed
   } else {
       RotaryPosition=0; // if YES, then reset position to ZERO
       digitalWrite( Motor1Dir1, LOW ); digitalWrite( Motor1Dir2, LOW );// turn motor off
       analogWrite( Motor1PWM, LOW ); 
       Serial.print ("Reset = ");
       Serial.println (RotaryPosition);
   }
 }
 
 // Runs if rotation was detected
 if (TurnDetected)  {
   if (up) {
     if (RotaryPosition >= 100) { // Max value set to 100
       RotaryPosition = 100;
     }
     else {
         RotaryPosition=RotaryPosition+2;
     }
   }
   else {
     if (RotaryPosition <= -100) { 
       // Max value set to -100        
       RotaryPosition = -100;
     }        
     else {          
       RotaryPosition=RotaryPosition-2;
      }
    }    
    TurnDetected = false;  // do NOT repeat IF loop until new rotation detected
    Serial.print ("Speed = "); 
    Serial.println (RotaryPosition);  
    
    if (RotaryPosition >= 0) { 
      digitalWrite( Motor1Dir1, HIGH ); digitalWrite( Motor1Dir2, LOW ); // direction = reverse
      analogWrite( Motor1PWM, RotaryPosition);
    } 

    if (RotaryPosition <0) {
      digitalWrite( Motor1Dir1, LOW ); digitalWrite( Motor1Dir2, HIGH); // direction = forward
      analogWrite( Motor1PWM, -RotaryPosition);
    }
  }
}
    // Interrupt routine runs if CLK goes from HIGH to LOW
    void isr ()  {
     delay(4);  // delay for Debouncing

You can NOT use delay() in an ISR.

      counter++;  // increase +1 the counter value

Why isn’t counter volatile?

This had no effect so I removed the volatile commands that weren’t already there.

There is no such thing as a “volatile command”. There is a volatile KEYWORD.

It is not at all obvious what the timer is doing. timerISR is a crappy name. There is some reason you have used the timer. The ISR name should reflect that reason.