Go Down

Topic: Arduino Interrupt pins problem (Read 924 times) previous topic - next topic

Robin2

#15
Jan 15, 2019, 12:06 pm Last Edit: Jan 15, 2019, 12:07 pm by Robin2
Pin 2 is not stable and gives the higher rpm. even with your nice code it will happens.
When you said (in Reply #8) "I used your code. It is working well on both of pins 2,3" I had assumed there were no problems.

Please post the version of my code that is giving rise to the incorrect readings when using pin2 and working properly when using pin3.

Also please post some samples of the output in each case.

...R

Two or three hours spent thinking and reading documentation solves most programming problems.

ebarash

Also please post some samples of the output in each case.

...R


well, this is your code and I just added marked line with #### to show the number of pulses in 1 second


const byte fwdPin = 9;
const byte revPin = 10;
const byte potPin = A1;

int potVal;
int pwmVal;

unsigned long revMicros;
unsigned long prevRevMicros;
unsigned long revDuration;
unsigned long revCount;


unsigned long prevDisplayMillis;
unsigned long  displayInterval = 1000;

    // variables for the ISR
volatile unsigned long isrMicros;
volatile unsigned long isrCount;
volatile bool newIsrMicros = false;



void setup() {
    Serial.begin(115200);
    Serial.println("SimpleISRdemo.ino");
    pinMode (fwdPin, OUTPUT);
    pinMode (revPin, OUTPUT);

    isrCount = 0;
    attachInterrupt(1, revDetectorISR, RISING);
}

//==========

void loop() {
    getIsrData();
    if (millis() - prevDisplayMillis >= displayInterval) {
        prevDisplayMillis += displayInterval;
        showData();
        isrCount=0;                                       //#### 
        readPot();
        updateMotorSpeed();
    }
}

//===========

 void readPot() {
    potVal = analogRead(potPin);
}

//===========

void updateMotorSpeed() {
    pwmVal = potVal >> 2;

    digitalWrite(revPin,LOW);
    analogWrite(fwdPin, pwmVal);
 }

//===========

void getIsrData() {
    if (newIsrMicros == true) {
        prevRevMicros = revMicros; // save the previous value
        noInterrupts();
            revMicros = isrMicros;
            revCount = isrCount;
            newIsrMicros = false;
        interrupts();
        revDuration = revMicros - prevRevMicros;
    }
}

//===========

void showData() {
    Serial.println();
    Serial.println("===============");
    Serial.print("PWM Val "); Serial.println(pwmVal);
    Serial.print("  Rev Duration ");
    Serial.print(revDuration);
    Serial.print("  Rev Count ");
    Serial.print(revCount);
    Serial.println();
}

//===========

void revDetectorISR() {
    isrMicros = micros();
    isrCount ++;
    newIsrMicros = true;

}

--------------------------------------------------------------------------------------------------

and this the result with both pins 2 and 3

Rev Duration 44644  Rev Count 22

**********************************************************************
**********************************************************************

Now I changed it just a little:

changes marked with //####




const byte fwdPin = 9;
const byte revPin = 10;
const byte potPin = A1;

int potVal;
int pwmVal;

unsigned long revMicros;
unsigned long prevRevMicros;
unsigned long revDuration;
unsigned long revCount;
unsigned long lastMillis=0;                          //####

unsigned long prevDisplayMillis;
unsigned long  displayInterval = 1000;

    // variables for the ISR
volatile unsigned long isrMicros;
volatile unsigned long isrCount;
volatile bool newIsrMicros = false;



void setup() {
    Serial.begin(115200);
    Serial.println("SimpleISRdemo.ino");
    pinMode (fwdPin, OUTPUT);
    pinMode (revPin, OUTPUT);

    isrCount = 0;
    attachInterrupt(1, revDetectorISR, RISING);
}

//==========

void loop() {
    getIsrData();
 //   if (millis() - prevDisplayMillis >= displayInterval) {  //####
        prevDisplayMillis += displayInterval;
        showData();
        readPot();
        updateMotorSpeed();
  //  }                                                                       //####
}

//===========

 void readPot() {
    potVal = analogRead(potPin);
}

//===========

void updateMotorSpeed() {
    pwmVal = potVal >> 2;

    digitalWrite(revPin,LOW);
    analogWrite(fwdPin, pwmVal);
 }

//===========

void getIsrData() {
    if (newIsrMicros == true) {
        prevRevMicros = revMicros; // save the previous value
        noInterrupts();
            revMicros = isrMicros;
           if(millis() - lastMillis>=1000){     //####
            revCount = isrCount;                //####
            lastMillis += 1000;                   //####
            isrCount=0;                             //####
           }                                             //####
            newIsrMicros = false;
        interrupts();
        revDuration = revMicros - prevRevMicros;
    }
}

//===========

void showData() {
    Serial.println();
    Serial.println("===============");
    Serial.print("PWM Val "); Serial.println(pwmVal);
    Serial.print("  Rev Duration ");
    Serial.print(revDuration);
    Serial.print("  Rev Count ");
    Serial.print(revCount);
    Serial.println();
}

//===========

void revDetectorISR() {
    isrMicros = micros();
    isrCount ++;
    newIsrMicros = true;

}
------------------------------------------------------------------------------------

this is the result with arduino pin 3


Rev Duration 44660  Rev Count 22


but for pin 2

Rev Duration 44684  Rev Count 37

************************************************************
************************************************************

You can see by same code I have defferent results.

But if I apply delay(1000) in void loop, or unmark this line

//   if (millis() - prevDisplayMillis >= displayInterval) {  //####

the result will be same again.

Robin2

To make it easy for people to help you please modify your post and use the code button </>
Code: [Select]
so your code looks like this and is easy to copy to a text editor. See How to use the Forum

Your code is too long for me to study quickly without copying to my text editor. The text editor shows line numbers, identifies matching brackets and allows me to search for things like all instances of a particular variable or function.

Also please use the AutoFormat tool to indent your code for easier reading.

...R
Two or three hours spent thinking and reading documentation solves most programming problems.

ebarash

Quote
To make it easy for people to help you please modify your post and use the code button </>

Sorry if I wrote like that, I am new.

this is your code and I just added marked line with #### to show the number of pulses in 1 second

and this the result with both pins 2 and 3

Rev Duration 44644  Rev Count 22



Code: [Select]

const byte fwdPin = 9;
const byte revPin = 10;
const byte potPin = A1;

int potVal;
int pwmVal;

unsigned long revMicros;
unsigned long prevRevMicros;
unsigned long revDuration;
unsigned long revCount;


unsigned long prevDisplayMillis;
unsigned long  displayInterval = 1000;

    // variables for the ISR
volatile unsigned long isrMicros;
volatile unsigned long isrCount;
volatile bool newIsrMicros = false;



void setup() {
    Serial.begin(115200);
    Serial.println("SimpleISRdemo.ino");
    pinMode (fwdPin, OUTPUT);
    pinMode (revPin, OUTPUT);

    isrCount = 0;
    attachInterrupt(1, revDetectorISR, RISING);
}

//==========

void loop() {
    getIsrData();
    if (millis() - prevDisplayMillis >= displayInterval) {
        prevDisplayMillis += displayInterval;
        showData();
        isrCount=0;                                       //#### 
        readPot();
        updateMotorSpeed();
    }
}

//===========

 void readPot() {
    potVal = analogRead(potPin);
}

//===========

void updateMotorSpeed() {
    pwmVal = potVal >> 2;

    digitalWrite(revPin,LOW);
    analogWrite(fwdPin, pwmVal);
 }

//===========

void getIsrData() {
    if (newIsrMicros == true) {
        prevRevMicros = revMicros; // save the previous value
        noInterrupts();
            revMicros = isrMicros;
            revCount = isrCount;
            newIsrMicros = false;
        interrupts();
        revDuration = revMicros - prevRevMicros;
    }
}

//===========

void showData() {
    Serial.println();
    Serial.println("===============");
    Serial.print("PWM Val "); Serial.println(pwmVal);
    Serial.print("  Rev Duration ");
    Serial.print(revDuration);
    Serial.print("  Rev Count ");
    Serial.print(revCount);
    Serial.println();
}

//===========

void revDetectorISR() {
    isrMicros = micros();
    isrCount ++;
    newIsrMicros = true;

}


ebarash

and this is with a little change:

changes marked with //####

this is the result with arduino pin 3


Rev Duration 44660  Rev Count 22


but for pin 2

Rev Duration 44684  Rev Count 37

You can see by same code I have defferent results.

But if I apply delay(1000) in void loop, or unmark this line

//   if (millis() - prevDisplayMillis >= displayInterval) {  //####

the result will be same again.

Code: [Select]

const byte fwdPin = 9;
const byte revPin = 10;
const byte potPin = A1;

int potVal;
int pwmVal;

unsigned long revMicros;
unsigned long prevRevMicros;
unsigned long revDuration;
unsigned long revCount;
unsigned long lastMillis=0;                          //####

unsigned long prevDisplayMillis;
unsigned long  displayInterval = 1000;

    // variables for the ISR
volatile unsigned long isrMicros;
volatile unsigned long isrCount;
volatile bool newIsrMicros = false;



void setup() {
    Serial.begin(115200);
    Serial.println("SimpleISRdemo.ino");
    pinMode (fwdPin, OUTPUT);
    pinMode (revPin, OUTPUT);

    isrCount = 0;
    attachInterrupt(1, revDetectorISR, RISING);
}

//==========

void loop() {
    getIsrData();
 //   if (millis() - prevDisplayMillis >= displayInterval) {  //####
        prevDisplayMillis += displayInterval;
        showData();
        readPot();
        updateMotorSpeed();
  //  }                                                                       //####
}

//===========

 void readPot() {
    potVal = analogRead(potPin);
}

//===========

void updateMotorSpeed() {
    pwmVal = potVal >> 2;

    digitalWrite(revPin,LOW);
    analogWrite(fwdPin, pwmVal);
 }

//===========

void getIsrData() {
    if (newIsrMicros == true) {
        prevRevMicros = revMicros; // save the previous value
        noInterrupts();
            revMicros = isrMicros;
           if(millis() - lastMillis>=1000){     //####
            revCount = isrCount;                //####
            lastMillis += 1000;                   //####
            isrCount=0;                             //####
           }                                             //####
            newIsrMicros = false;
        interrupts();
        revDuration = revMicros - prevRevMicros;
    }
}

//===========

void showData() {
    Serial.println();
    Serial.println("===============");
    Serial.print("PWM Val "); Serial.println(pwmVal);
    Serial.print("  Rev Duration ");
    Serial.print(revDuration);
    Serial.print("  Rev Count ");
    Serial.print(revCount);
    Serial.println();
}

//===========

void revDetectorISR() {
    isrMicros = micros();
    isrCount ++;
    newIsrMicros = true;

}

Robin2

#20
Jan 15, 2019, 08:20 pm Last Edit: Jan 15, 2019, 08:21 pm by Robin2
Two things occur to me regarding the programs in Reply #18

in your first program this line is bad practice
Code: [Select]
        isrCount=0;                                       //#### 
Because isrCount is a 4-byte variable there is a risk that one of the bytes will be changed by the ISR while you are trying to set it to 0. You should update it with interrupts supended - like this

Code: [Select]
    noInterrupts();
       isrCount = 0;
    interrupts();


However, IMHO, there is nothing to be gained by setting the number back to 0.


And in the second program I would not have this line
Code: [Select]
          if(millis() - lastMillis>=1000){
inside the section of code where interrupts are disabled. You should ensure the interrupts are disabled for the shortest possible time.

Without trying your program (which I don't have time to do) I can't understand why it makes a difference which interrupt pin is used, although I assume the two pins have different priorities if you study the Atmega 328 datasheet closely. (It's a good read at bedtime :) )

...R
Two or three hours spent thinking and reading documentation solves most programming problems.

ebarash

Quote
I assume the two pins have different priorities
Dear Robin,

I am very thankful for your time and advices. Maybe this is the answare of my question. I just wondered about the difference between two pins with the same code. I will read closely the datasheet later to know more about it.

And yes, you are right about those two lines. However, it was a sample to show the difference.

regards   

Go Up