The code below does not return 0 within the timeout period. It works fine if there is a pulse. It just seems to hang until there is activity on the pin?
int total = 0;
for (int i = 0; i < 5; )
{
pHigh = pulseIn(pulsePin, HIGH, 10000);
pLow = pulseIn(pulsePin, LOW, 50000);
Rinterval = pHigh + pLow;
if (Rinterval > 10000)
{
Serial.print(pHigh);
Serial.print("\t:\t");
Serial.print(pLow);
RPS = (1000000UL / Rinterval);
Serial.print("\tRPS: ");
Serial.println(RPS);
total = total + RPS;
i++;
}
HOWEVER JUST ON ITS OWN LIKE THIS:
unsigned long Rinterval = pulseIn(pulsePin, HIGH, 10000) + pulseIn(pulsePin, LOW, 50000);
RPS = (1000000UL / Rinterval);
will return 0 and allow to program to continue.
I have started reading that this is a bug in ardiuno?
or is it just me..
It's hard to tell for sure from your incomplete code, but it seems like the for loop will never end because i will always be 0 when both pulseIns return 0. So you've got an infinite loop there.
void starter_engaged() { // starter_engaged()
int total = 0;
for (int i = 10; i < 15; )
{
pHigh = pulseIn(pulsePin, HIGH, 10000);
pLow = pulseIn(pulsePin, LOW, 50000);
Rinterval = pHigh + pLow;
if (Rinterval > 10000)
{
Serial.print(pHigh);
Serial.print("\t:\t");
Serial.print(pLow);
RPS = (1000000UL / Rinterval);
Serial.print("\tRPS: ");
Serial.println(RPS);
total = total + RPS;
i++;
}
}
int averageRPS = total / 5;
if (averageRPS > 25) {/*
unsigned long Rinterval = pulseIn(pulsePin, HIGH) + pulseIn(pulsePin, LOW);
RPS = (1000000UL / Rinterval);
if (RPS > 25)*/
digitalWrite(start_ctrl, LOW);
Serial.println("STARTER OFF");
Serial.print("RPS: ");
Serial.println(averageRPS);
Serial.println("ENGINE RUNNING");
delay(10);
disengage_starter();
}
else if ((start_time + 3300) < millis()) {
digitalWrite(star_ctrl, LOW);
Serial.println("STAR OFF");
//digitalWrite(cdiff_ctrl, LOW);
//Serial.println("Cdiff OFF");
disengage_starter_timeout();
}
else {
starter_engaged();
}
}
Yep, it's an infinite loop. Here's what happens:
- set i to 10
- enter the body of the for loop because i is less than 15
- set pHigh to 0 (because pulseIn returns 0 due to timeout)
- set pLow to 0 (because pulseIn returns 0 due to timeout)
- set Rinterval to 0 (because 0 + 0 = 0)
- skip the body of the if statement because Rinterval is not greater than 10000 (it's 0)
- go to step 2
Nothing breaks out of the for loop. Under what condition(s) do you want to leave the for loop?
I want to leave the loop if the condition is met, i.e there are pulses to be read, (single cylinder ignition), this happens as it is with no problems. The statement if (Rinterval > 10000) is there to remove false triggers.
if there are no pulses just exit the for loop (based on the pulseIn timeouts) and continue until millis() has exceeded 3300ms.
So basically its reading an ignition spark, each pulseIn() (high & low) will represent 1 rev.
I want to average over 5 revs to give a true value for the revs per second.
the crank speed is well under 15 RPS. Anything over 25 RPS then exit the loop - disengage_starter() - this indicates the engine is running
Anything under 25 RPS then continue until millis() has timed out and we can carry on with the rest of the program - disengage_starter_timeout()
Ok thanks christop
Ill just have to include an else statement after the if to let the loop exit.
You're allowing only 10 milliseconds (10,000 microseconds) for a HIGH pulse to go by. That means at least 100 RPS (6000 RPM) to be assured that a pulse goes by in time. You should use the minimum acceptable pulse rate and double it to set your timeout. If you want to measure down to 600 RPM (10 RPS) you should use a timeout closer to 200,000 microseconds for both the HIGH and LOW pulses.
You may also want to ignore the pulses where one of the halves is missing:
if (pHigh > 0 && pLow > 0 && Rinterval > 10000)
So if the 'if' condition is not met does it immediately check the 'else' then return into the for loop?
void starter_engaged() { // start_time=millis() has been called, start_ctrl = HIGH
total = 0;
for (int i = 0; i < 5;)
{
pHigh = pulseIn(pulsePin, HIGH, 200000);
pLow = pulseIn(pulsePin, LOW, 200000);
unsigned long Rinterval = pHigh + pLow;
if (pHigh > 0 && pLow > 0 && Rinterval > 10000) // set condition for capturing reading
{
Serial.print (pHigh);
Serial.print ("\t:\t");
Serial.print (pLow);
RPS = (1000000UL / Rinterval);
Serial.print ("\tRPS: ");
Serial.println (RPS);
total = total + RPS;
i++;
}
else if ((start_time + 3300) < millis()) { // Test if 3.3 seconds has passed since the starter was engaged
digitalWrite(start_ctrl, LOW);
Serial.println ("STARTER OFF");
disengage_starter_timeout();
}
}
int averageRPS = total / 5;
if (averageRPS > 25) {
digitalWrite(start_ctrl, LOW);
Serial.println ("STARTER OFF");
Serial.print ("RPS: ");
Serial.println (averageRPS);
Serial.println ("ENGINE RUNNING");// Send "Engine Running" message after engine has started
delay(10);
disengage_starter(); // Go to disengage_starter after engine is running
}
else {
starter_engaged(); // Repeat this function if engine has not started or 3.3 seconds has not elapsed
}
}
andyroid:
So if the 'if' condition is not met does it immediately check the 'else' then return into the for loop?
void starter_engaged() // start_time=millis() has been called, start_ctrl = HIGH
{
total = 0;
for (int i = 0; i < 5;)
{
pHigh = pulseIn(pulsePin, HIGH, 200000);
pLow = pulseIn(pulsePin, LOW, 200000);
unsigned long Rinterval = pHigh + pLow;
if (pHigh > 0 && pLow > 0 && Rinterval > 10000) // set condition for capturing reading
{
Serial.print (pHigh);
Serial.print ("\t:\t");
Serial.print (pLow);
RPS = (1000000UL / Rinterval);
Serial.print ("\tRPS: ");
Serial.println (RPS);
total = total + RPS;
i++;
}
else
if ((start_time + 3300) < millis()) // Test if 3.3 seconds has passed since the starter was engaged
{
digitalWrite(start_ctrl, LOW);
Serial.println ("STARTER OFF");
disengage_starter_timeout();
}
}
int averageRPS = total / 5;
if (averageRPS > 25)
{
digitalWrite(start_ctrl, LOW);
Serial.println ("STARTER OFF");
Serial.print ("RPS: ");
Serial.println (averageRPS);
Serial.println ("ENGINE RUNNING");// Send "Engine Running" message after engine has started
delay(10);
disengage_starter(); // Go to disengage_starter after engine is running
}
else
{
starter_engaged(); // Repeat this function if engine has not started or 3.3 seconds has not elapsed
}
}
It's a little hard to read without comments to show intent. The 'if' will either add the RPS to the total and add one to the count OR turn off the starter. The 'for' loop will repeat the 'if' until 5 valid RPS samples have accumulated.
WARNING: If any RPS sample fails, the starter will be turned off??? That appears to be what the 'else' is doing.
Your time comparison should always be: "newer time" - "older time" compared to "time interval"; The two times must be 'unsigned long' and the interval should be 'unsigned' or 'unsigned long'.
It is very unlikely that starter_engaged() should be calling starter_engaged(). Recursive calls take up stack space and often cause a crash if they are not carefully designed.