You guys have really helped me out so I firgured I would ask another question about another problem. Same project, different end.
I am counting the revolutions of a shaft per min which works fine but even while the shaft is not moving the serial still prints the last rpm that is registered before stopping. Also even if nothing is moving and I start the program the serial sends out -1 when it should be 0. Im sure this is an easy fix and I'm over looking something I know you pros will see probably immediately.
const int SENSOR_0 = 2; //Moved to interrupt pin
const int SENSOR_1 = 3; //Moved to interrupt pin
const int pulsesPerRev = 2; //Number of magnets on the shaft
const long sendingInterval = 1000; //ms between serial messages
long averageRpm_0 = 0;
long averageRpm_1 = 0;
long previousMillisSend = 0;
unsigned long currentMillis = 0;
volatile long pulseDuration_0 = 0;
volatile long pulseDuration_1 = 0;
volatile long previousPulseTime_0 = 0;
volatile long previousPulseTime_1 = 0;
void setup()
{
pinMode(SENSOR_0, INPUT);
digitalWrite(SENSOR_0, HIGH); //Set pullup resistor on input pin
attachInterrupt(0, sensorISR_0, RISING);
pinMode(SENSOR_1, INPUT);
digitalWrite(SENSOR_1, HIGH); //Set pullup resistor on input pin
attachInterrupt(1, sensorISR_1, RISING);
Serial.begin( 57600 );
}
void loop()
{
currentMillis = millis();
if(currentMillis - previousMillisSend > sendingInterval){ //Send rpm update to Adobe Air program
previousMillisSend = currentMillis;
averageRpm_0 = 1000 * 60 / pulsesPerRev / pulseDuration_0;
averageRpm_1 = 1000 * 60 / pulsesPerRev / pulseDuration_1;
//**counts to rpm math**
Serial.print(averageRpm_0, DEC);
Serial.print("\t");
Serial.print(averageRpm_1, DEC);
Serial.println();
}
}
void sensorISR_0()
{
pulseDuration_0 = currentMillis - previousPulseTime_0;
previousPulseTime_0 = currentMillis;
}
void sensorISR_1()
{
pulseDuration_1 = currentMillis - previousPulseTime_1;
previousPulseTime_1 = currentMillis;
}
Do you get actual/meaningfull values when you print pulseDuration_0 & pulseDuration_1. The average beeing always -1 sounds like a overflow because of divide_by_0 or something like that.
You're dividing by a value which you haven't tested against zero. A division by zero is not defined and your -1 may result from there. Also you're not resetting the pulseDuration_X variables, so they hold the last value till the next edge is reached.
Ok, made the sketch zero out pulseDuration_X is there has not been any pulses for greater than 5sec.
const int SENSOR_0 = 2; //Moved to interrupt pin
const int SENSOR_1 = 3; //Moved to interrupt pin
const int pulsesPerRev = 2; //Number of magnets on the shaft
const long sendingInterval = 1000; //ms between serial messages
long averageRpm_0 = 0;
long averageRpm_1 = 0;
long previousMillisSend = 0;
unsigned long currentMillis = 0;
volatile long pulseDuration_0 = 0;
volatile long pulseDuration_1 = 0;
volatile long previousPulseTime_0 = 0;
volatile long previousPulseTime_1 = 0;
void setup()
{
pinMode(SENSOR_0, INPUT);
digitalWrite(SENSOR_0, HIGH); //Set pullup resistor on input pin
attachInterrupt(0, sensorISR_0, RISING);
pinMode(SENSOR_1, INPUT);
digitalWrite(SENSOR_1, HIGH); //Set pullup resistor on input pin
attachInterrupt(1, sensorISR_1, RISING);
Serial.begin(57600);
}
void loop()
{
currentMillis = millis();
if(currentMillis - previousMillisSend > sendingInterval){ //Send rpm update to AIR program
previousMillisSend = currentMillis;
averageRpm_0 = (1000 * 60 / pulsesPerRev / pulseDuration_0)*(-1);
averageRpm_1 = (1000 * 60 / pulsesPerRev / pulseDuration_1)*(-1);
//**counts to rpm math**
Serial.print(averageRpm_0, DEC);
Serial.print("\t");
Serial.print(averageRpm_1, DEC);
Serial.println();
if(currentMillis - previousPulseTime_0 > 5000)
{
pulseDuration_0=0;
}
if(currentMillis - previousPulseTime_1 > 5000)
{
pulseDuration_1=0;
}
}
}
void sensorISR_0()
{
pulseDuration_0 = currentMillis - previousPulseTime_0;
previousPulseTime_0 = currentMillis;
}
void sensorISR_1()
{
pulseDuration_1 = currentMillis - previousPulseTime_1;
previousPulseTime_1 = currentMillis;
}
Now if I spin the shaft it gives the proper values as well as stops sending crap when the shaft stops moving.
At little nit pick problem still is...
If the shaft slows and stops on its own, the program stops reading signal and zeros out the rpm reading. (As it should)
BUT... if the shaft is spinning and I bring it to a sudden stop then the program will continue to register a floating rpm value and does not zero out until I spin the shaft just enough to get a pulse, the it zeros out.
Hope I am being clear on what I'm trying to explain.
currentMillis is not volatile because the value is not being changed in the ISR. I was under the impression it only needed to be volatile if the variable was changed in a ISR. Maybe I should look up the definition of volatile and when to use it.
And why currentMillis is not in the ISR is because I don't know exactly what I'm doing..Learning as I go...
But from what I read in the reference section (http://www.arduino.cc/en/Reference/AttachInterrupt) It says: "Inside the attached function, delay() won't work and the value returned by millis() will not increment."
So I did not put currentMillis into the ISR due to that, maybe I am misunderstanding that comment. And I am using the currentMillis for a timer to send the serial data. If I move that variable to the ISR then will my sending interval timer even function properly since currentMillis would only be updated when an interrupt is triggered?
Now that you have brought up that question, it makes me think that just cause it won't increment, it will still give me the "exact" time that the interrupt was tripped.
This is being multiplied by -1 so that my rpm comes out positive, otherwise I get a negative RPM reading. I could just take abs() but multiplying by -1 was so easy.
It says: "Inside the attached function, delay() won't work and the value returned by millis() will not increment."
This means that if your ISR takes a long time, the time returned by millis() at the start and the time returned by millis() at the end will be the same. It doesn't mean that you can't call millis() in the ISR.
Doing so would assure that the time associated with the last pulse was always correct, and currentMillis would not be needed as a global or volatile variable.
Thanks for clearing that up for me about the millis() in an ISR.
I moved currentMillis to the ISR's and added a currentMillisT variable for my sending interval timer. Also made currentMillis a volatile long and made the new currentMillisT an unsigned long.
To make things look better. Have to get into the practice of proper coding, not make shift patching..
Do yall think there is a better way (I know there is, just don't know how to do it.) to implement the counting of the RPMs and to make it say 0 RPM when the shaft is not moving? It still has a small bug where if the shaft comes to a sudden stop it will keep reading an RPM although nothing is there.
UPDATE::
After testing my changes I just got random results. Here is the complete change
const int SENSOR_0 = 2; //Moved to interrupt pin
const int SENSOR_1 = 3; //Moved to interrupt pin
const int pulsesPerRev = 2; //Number of magnets on the shaft
const long sendingInterval = 1000; //ms between serial messages
unsigned long averageRpm_0 = 0;
unsigned long averageRpm_1 = 0;
long previousMillisSend = 0;
unsigned long currentMillisT = 0;
volatile long currentMillis = 0;
volatile long currentMillis1 = 0;
volatile long pulseDuration_0 = 0;
volatile long pulseDuration_1 = 0;
volatile long previousPulseTime_0 = 0;
volatile long previousPulseTime_1 = 0;
void setup()
{
pinMode(SENSOR_0, INPUT);
digitalWrite(SENSOR_0, HIGH); //Set pullup resistor on input pin
attachInterrupt(0, sensorISR_0, RISING);
pinMode(SENSOR_1, INPUT);
digitalWrite(SENSOR_1, HIGH); //Set pullup resistor on input pin
attachInterrupt(1, sensorISR_1, RISING);
Serial.begin(57600);
}
void loop()
{
currentMillisT = millis();
if(currentMillisT - previousMillisSend > sendingInterval){ //Send rpm update to AIR program
previousMillisSend = currentMillisT;
averageRpm_0 = abs(1000 * 60 / pulsesPerRev / pulseDuration_0);
averageRpm_1 = abs(1000 * 60 / pulsesPerRev / pulseDuration_1);
//**counts to rpm math**
Serial.print(averageRpm_0, DEC);
Serial.print("\t");
Serial.print(averageRpm_1, DEC);
Serial.println();
Serial.flush();
if(currentMillisT - previousPulseTime_0 > 5000)
{
pulseDuration_0=0;
}
if(currentMillisT - previousPulseTime_1 > 5000)
{
pulseDuration_1=0;
}
}
}
void sensorISR_0()
{
currentMillis = millis();
pulseDuration_0 = currentMillis - previousPulseTime_0;
previousPulseTime_0 = currentMillis;
}
void sensorISR_1()
{
currentMillis1 = millis();
pulseDuration_1 = currentMillis1 - previousPulseTime_1;
previousPulseTime_1 = currentMillis1;
}
millis() returns an unsigned long. All your time operations should be unsigned, with unsigned math you can subtract a bigger number from a smaller number and get a correct positive result.
It's like with a 12 hour clock, present time 3 - start time 10 = 5 hours difference. We don't care that it is -5 hours since we are looking at hours -ago- just by subtracting start from present. Unsigned math takes care of that for us; so in base 12 (12 hour clock), 10 - 3 = 7, 3 - 10 = 5.
With 32 bits you have base 4,294,967,296. That many millseconds takes over 47 days to roll over but when it does, signed math will fail without extra code that unsigned math doesn't need.
You want, I'll post a sketch made to experiment with that shows how the bits come out.
1000 is an int.
1000 * 60 will cause 60 to be treated as an int.
1000*60 is 60000 which is too large for an int (should be less than or equal to 32767) and will appear as a negative int.
You need to perform your arithmetic in a different order to ensure that you do not overflow.
Multiplying and dividing positive numbers and getting a negative result should be a clear sign (pun intended) that something is wrong. Multiplying by -1 or taking the absolute value is not the correct approach.
89ls1:
I was under the impression it only needed to be volatile if the variable was changed in a ISR.
Not quite. If the variable is changed outside an ISR, the compiler might just keep it in a register (not put it back into the actual variable). Thus, the ISR won't notice the change.