FOR Loop Within IF Statement

The past few days I’ve had trouble with a FOR loop iteration within an IF statement.

I have a 12V DC geared motor with magnetic encoder being driven by an L298N motor driver which is controlled by an Arduino. Most of the code works, being that if button 2 is pressed, the motor rotates clockwise; if button 32 is pressed, the motor rotates counter-clockwise.

However, if button 1 is pressed, I want to motor to rotate clockwise to a specified limit (for which position is reported to the Arudino from encoder), then counter clockwise to another specified limit, and continually repeat this back-and-forth sequence. This FOR loop iteration will correctly work if it (the back-and-forth sequence) alone is placed inside the void loop, but if it is nested within the IF statement, it correctly rotates clockwise but then incorrectly rotates counter-clockwise continuously (never returning to the clockwise direction).

The code is below, any help is appreciated and my apologies for the wordy explanation.

#define enA 9
#define in1 6
#define in2 7

const int button1Pin = 8;     
const int button2Pin = 12;
const int button3Pin = 13;

int button1State = 0;     
int button2State = 0;  
int button3State = 0;  

int rotDirection = 0;


volatile long temp, counter = 0; 

void setup() {
  
  Serial.begin (9600);
  pinMode(2, INPUT_PULLUP); 
  pinMode(3, INPUT_PULLUP); 

  attachInterrupt(0, ai0, RISING);
  attachInterrupt(1, ai1, RISING);
  
  pinMode(enA, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);
  
  digitalWrite(in1, LOW);
  digitalWrite(in2, LOW);
  
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  pinMode(button3Pin, INPUT);

}

void loop() {

  analogWrite(enA, 255);

  if( counter != temp ){
  Serial.println (counter);
  temp = counter;
  }

  button1State = digitalRead(button1Pin);
  button2State = digitalRead(button2Pin);
  button3State = digitalRead(button3Pin);

 
 if (button1State == HIGH) {
  
              for (counter =0; counter<2000; counter++) {
              clockwise();
              }             
                      
              for (counter= 2000; counter>0; counter--){
              counterclockwise();
              }             
        }

 if (button2State == HIGH) {
  
              clockwise();
        }

 if (button3State == HIGH) {
  
              counterclockwise();
        }     
}

 void ai0() {
  if(digitalRead(3)==LOW) {
  counter++;}
  
  else{
  counter--;}
  }
   
  void ai1() {
  if(digitalRead(2)==LOW) {
  counter--;}
  
  else{
  counter++;}
  }

void clockwise () {
      digitalWrite(in1, HIGH);
      digitalWrite(in2, LOW);
      rotDirection = 1;   
      delay(20);
}

void counterclockwise () {
      digitalWrite(in1, LOW);
      digitalWrite(in2, HIGH);
      rotDirection = 0;   
      delay(20);    
}

Why are the interruptions messing around with your for loop counter (which are multiple bytes being incremented or decremented in a non protected manner in the for loop)

How do you handle bouncing ?

Describe your wiring in details

what kind of motor is this DC, brushless or steppermotor?

At this stage I haven't focused on debouncing; not that it's the same, but I am using a 1K ohm-resistor as pull-down with each tactile button.

My buttons are tactile buttons on breadboard (connected to pins 8, 12 and 13 of an Arduino Uno), with 5V supply from Arduino.

Pins 6 of the Arduino is connected input 1 of the L298N motor driver, pin 7 of Arduino connected to input 2 of the motor driver, and pin 9 of the Adruino connected to the enable of the motor driver. Them motor driver is being supplied by an external 12V supply which is also power the 12V DC geared motor with encoder.

Given that the back-and-forth motion works correctly when it alone is in the void loop (without any buttons), wouldn't the problem be related to how the FOR command is nested within the IF statement?

again, Why are the interruptions messing around with your for loop counter ? at the very minimum if your interrupt stuff makes sense, counter++ and counter-- in the for loops would have to be in critical sections

The first thing I would do is to print the values before testing them in the if statement. Are they what you expect based on whether the button or buttons are pressed ?

From the serial monitor, the counter seems to be working properly.

I'm guessing my problem lies in your question, but I'm not savvy enough to answer. I suppose an interrupt allows one execution to complete and then return to normal process. Just experimenting, I removed one of the interrupts from the code and you are correct, no real difference (removing two causes the wrong behaviors). I've also tried to detach the interrupt in the FOR loop and VOID loop but have had no luck.

so you did not write that code ?

I used extracts from similar examples for what I'm trying to do; but isn't that how you start? Similarly, since I initially posted my problem, I've used various examples to debounce. But that doesn't mean I'm not studying interrupts, debouncing, etc, and trying to learn from this experience.

but isn't that how you start

No. you extract code you understand and need, you don't go in a code fishing expedition hoping you'll get the right code in the end...

Here it seems you included interrupts that are driving a rotary encoder and you use the same variable in the for loop as is used to maintain the rotary encoder position... I'm still trying to understand why.

what's the purpose of the rotary encoder? it gives you the position of the shaft, then why do you need the for loop? just launch the motor and await until the rotary encoder reaches the right position ?

As previously stated, the design of your code is faulty. If you want to run the motors for a number of encoder counts, then you do not want to be changing the count numbers in a for loop as well as in an encoder interrupt.

An if or while construct based on the count would be more appropriate.