Using Internal Timer Interrupt to Read Analog Sensor

Question

I'm having difficultly using an internal timer interrupt with my Arduino Nano to properly capture a specific value from a linear potentiometer and then instantly stopping the actuator.

I basically have the wiper of a linear potentiometer connected to the end of a linear actuator. The actuator extends/retracts via a L298N module being controlled by a Nano. From where its positioned, the potentiometer reads around "80" when the actuator is fully extended, and around "25" when fully retracted. I'm entering numbers via the serial monitor to prompt the actuator to move up or down, these numbers are being stored in the Nano's EEPROM.

My interrupt seems not to work while the actuator is in motion. For example, if I set the interrupt to stop the motor when the potentiometer reads "50", it just skips past this number without stopping the actuator. But if I set the interrupt to stop the motor when the potentiometer reads the minimum or maximum (where the actuator is currently located before activation), the actuator correctly doesn't extract or retract. I would think my interrupt is fast enough at 8Khz, so I feel there is something else prohibiting the actuator from stopping at a specific value between the max and min.

The my current code is below, any help would be much appreciated.

    //timer2 will interrupt at 8kHz
    
    #include <EEPROM.h>
    #define EEPROM_SIZE
    
    long number;
    long x;
    
    int toggle=0;
    int old_x=0;
    
    const int linPot = A1;
    
    int MotorSpeed=255;
    
    int enA = 9;
    int in1 = 8;
    int in2 = 7;
    
    int currentPosition;
    
    enum {IdleState, UpState, DownState} State;
    
    void setup(){
      
      Serial.begin(9600);
     
      number = EEPROM.read(0);
      
      x=number;
    
      pinMode(enA, OUTPUT);
      pinMode(in1, OUTPUT);
      pinMode(in2, OUTPUT);
    
      State = IdleState;
    
    //cli();//stop interrupts
    
    //set timer2 interrupt at 8kHz
      TCCR2A = 0;// set entire TCCR2A register to 0
      TCCR2B = 0;// same for TCCR2B
      TCNT2  = 0;//initialize counter value to 0
      // set compare match register for 8khz increments
      OCR2A = 249;// = (16*10^6) / (8000*8) - 1 (must be <256)
      // turn on CTC mode
      TCCR2A |= (1 << WGM21);
      // Set CS21 bit for 8 prescaler
      TCCR2B |= (1 << CS21);   
      // enable timer compare interrupt
      TIMSK2 |= (1 << OCIE2A);
    
    
    sei();//allow interrupts
    
    }//end setup
    
    
    ISR(TIMER2_COMPA_vect){//timer1 interrupt 8kHz 
    //generates pulse wave of frequency 8kHz/2 = 4kHz 
    
    if(currentPosition==50){
    
      HALT();
    
    }
      
    }
    
    void loop(){
    
    if ((Serial.available()&&toggle==0)){
    
            x=Serial.parseInt();
            old_x=x;
            toggle=1;        
          }
    
          if((Serial.available())&&(toggle==1)&&(Serial.parseInt()!=old_x)){
              toggle=0;
          }
    
            if((toggle==1)&&(Serial.parseInt()==0)){
              toggle=0;
          }   
    
          if(x>78){
            x=78;
          }
    
           if(x<0){
            x=0;
          }
    
        EEPROM.write(0, x);
        Serial.print("X :");
        Serial.println(x);
    
        int data = analogRead(linPot);
        currentPosition = map(data, 0, 1023, 0, 100);
        Serial.print("Potentiometer at ");
        Serial.print(currentPosition);
        Serial.println("%");
        
        long difference = currentPosition-x;
        Serial.print("Difference :");
        Serial.println(difference);
        
    if(difference>0){
    
        State = UpState;
      
    }
    
    if(difference>0){
    
       State = DownState;
    }
    
    
    if(difference==0){
    
       State = IdleState;
    }
    
    
     switch (State) {
                
                   case IdleState:
                   Serial.println("Idle State");
                    HALT();
                    if((currentPosition-number)<0){
                     State = UpState; 
                    }
                    if((currentPosition-number)>0){
                     State = DownState; 
                    }
                    break;
                 
                  case UpState:
                  Serial.println("Up State");
                    //Need to move down
                    DOWN(); 
                    if((currentPosition-number)>0){
                     State = DownState; 
                    }
                    if((currentPosition-number)==0){
                     State = IdleState; 
                    }
                    break;
            
                  case DownState:
                  Serial.println("Down State");
                    //Need to move up
                    UP();
                    if((currentPosition-number)<0){
                     State = UpState; 
                    }     
                    if((currentPosition-number)==0){
                     State = IdleState; 
                    } 
                    break;
                    
     }
    
    }
    
    void DOWN(){
        analogWrite(enA, MotorSpeed);
        digitalWrite(in1, HIGH);
        digitalWrite(in2, LOW);
      }
    
    void UP() {
        analogWrite(enA, MotorSpeed);
        digitalWrite(in1, LOW);
        digitalWrite(in2, HIGH);
    
      }
    
    void HALT () {
        digitalWrite(in1, LOW);
        digitalWrite(in2, LOW);
    }

Answer 1

First, read Nick Gammon's answer: all his points are perfectly valid. He notes that the serial prints can be quite slow. And indeed, as I counted the characters, I find that the prints should take at least 54 ms per loop iteration. This is really very slow. In addition to this, the calls to parseInt() can be much slower yet, especially considering that one of them is not conditioned on Serial.available().

Currently, the code that takes care of reading the potentiometer and stopping the actuator looks like this:

int currentPosition;

// Triggered every 125 µs.
ISR(TIMER2_COMPA_vect) {
    if (currentPosition == 50) HALT();
}

void loop() {
    // Potentially extremely slow code...
    currentPosition = map(analogRead(linPot), 0, 1023, 0, 100);
    // Definitely very slow code: takes at least 54 ms.
}

This is reading the potentiometer at best every 54 ms, and potentially much more slowly. On every read, currentPosition is updated. In parallel, the value of currentPosition is being tested every 125 µs.

Note that there is really no point in testing currentPosition that often. Once the value has been tested, testing it again before it is updated serves no purpose. Your code would actually be a tiny bit faster if you test the value right after updating it:

void loop() {
    // Potentially extremely slow code...
    currentPosition = map(analogRead(linPot), 0, 1023, 0, 100);
    if (currentPosition >= 50) HALT();
    // Definitely very slow code: takes at least 54 ms.
}

Note that I changed the equality test to an inequality test, for a reason explained in Nick Gammon's answer.

You still have the problem that the loop is very slow. If you want the potentiometer to be tested more frequently, you have several options.

1. The bad option: do it in the interrupt handler:

   // Triggered every 125 µs.
   ISR(TIMER2_COMPA_vect) {
       currentPosition = map(analogRead(linPot), 0, 1023, 0, 100);
       if (currentPosition >= 50) HALT();
   }
   

   This is a bad option because analogRead() takes at least 112 µs, which means that the ISR will eat 98.6% of the CPU cycles. You could make this work kind of reasonably if you lower the interrupt frequency to something like 1 kHz or below. Still, such a long ISR is not really good practice.

2. The better option: make loop non-blocking:

   void loop() {
       // Non-blocking, very fast code.
       currentPosition = map(analogRead(linPot), 0, 1023, 0, 100);
       if (currentPosition >= 50) HALT();
       // More very fast code.
   }
   

   To read the serial port in a non-blocking way, see Reading Serial on the Arduino. For writing to the serial port in a non-blocking way, simply refrain to print very frequently.

3. The hard option: configure the ADC to read continuously.

   You can program the ADC in “self-running mode”: it will then take a reading every 104 µs. You can program it to also deliver an interrupt every time a new reading is available: you can test the reading in this interrupt handler. This will give you the best possible response time:

   void setup() {
       // Set the ADC to free running mode.
       // Enable the ADC interrupt.
   }
   
   volatile int raw_adc_reading;
   
   // Triggered on every fresh ADC reading.
   ISR(ADC_vect) {
       raw_adc_reading = ADC;
       if (raw_adc_reading >= RAW_THRESHOLD) STOP();
   }
   

   Note that I prefer to test the raw reading rather than the converted one. This is because map() is slow (it involves a long division) and an ISR should ideally be as fast as possible.

   You will have to dig into the MCU datasheet to learn how to configure the ADC in free running mode.

Answer 2

For one thing, currentPosition is not declared volatile so the compiler may be keeping that value in a register, which is not consulted inside the ISR.

I'm also concerned about all the serial prints at 9600 baud (quite slow) which may make the position skip from 49 to 51 while you are busy printing values.

Comparing to be exactly equal in order to do something is always fraught.

---

   x=Serial.parseInt();
    old_x=x;
    toggle=1;        
  }

  if((Serial.available())&&(toggle==1)&&(Serial.parseInt()!=old_x)){
      toggle=0;
  }

    if((toggle==1)&&(Serial.parseInt()==0)){
      toggle=0;
  } 

Are you expecting three integers from serial?