Go Down

Topic: Why doesn't the pulseIn display duration more than 170ms??? (Read 3415 times) previous topic - next topic

Mr.Debugger

Oct 09, 2011, 09:43 am Last Edit: Oct 09, 2011, 09:47 am by Mrxnoxious991 Reason: 1
Hey,

I am trying to measure the duration of an active LOW signal using pulseIn function.
The results are pretty confusing!
The controller only displays results of pulses that are fast ( i.e. less than 170ms) and returns "0" if the duration of the pulse exceeds more than 170. The max. value I could capture was 169250 usec.
I suspect it has something to do with the the overflow or the time out function.

Any help would be appreciated

Thanks!

Here's the code:
Code: [Select]
unsigned long duration;
int counter = 0;
void setup()
{
pinMode (5, INPUT);
Serial.begin(9600);
}
void loop()
{
  duration = pulseIn(5, LOW);
{
  Serial.println (duration);
   
}
}



robtillaart


In wiring_pulse.c  (its in the Arduino distribution ) the pulsein function is defined. It returns 0 if there is a timeout, so that is not the case as you get nr > 0.

The signature is :   unsigned long pulseIn(uint8_t pin, uint8_t state, unsigned long timeout)

you call it with 2 parameters so where does the 3rd param (timeout)  comes from.... I even wondered that it compiled ....

Can you explicitly pass a timeout value?


Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

robtillaart

see also - http://arduino.cc/forum/index.php/topic,74642.0.html -
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

PaulS

Quote
you call it with 2 parameters so where does the 3rd param (timeout)  comes from.... I even wondered that it compiled ....

You need to look at the header file for the function, too. The timeout argument is defined with a default value, making it optional.

The pulseIn function is designed to measure events that occur within a reasonably short period of time. 169250 microseconds is not what pulseIn was designed to measure. For servo PPM pulses, for example, that time-frame would indicate that the pulse sender was dead in the water.

robtillaart


OK, forgot about the headerfile and the optional parameter  :smiley-red: :smiley-red: :smiley-red: :)

But stil, the default value for the parameter = 1.000000L  which is 5x bigger that the max value the OP got.

Code: [Select]

  ...
  return clockCyclesToMicroseconds(width * 21 + 16);
}

#define clockCyclesToMicroseconds(a) ( ((a) * 1000L) / (F_CPU / 1000L) )


These two lines together cause an overflow...   Looking at the math it can be replaced with something simpler

return ((width*21+16) / clockCyclesPerMicrosecond() );


which overflows less fast as the factors 1000 (twice) is removed from the equation. This makes the PulseIn() function "behave better" in a larger range. Probably the division will be optimized to a shift so it will be a few cycles faster too.

Reported as bug/enhancement  in - http://code.google.com/p/arduino/issues/detail?id=675 -






Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Mr.Debugger

Quote
169250 microseconds is not what pulseIn was designed to measure


Then why does it say that the pulseIn function works on pulses b/w 10usec to 3min in length.

http://www.arduino.cc/en/Reference/PulseIn

Nick Gammon

Confirmed. I am having trouble measuring pulses much higher than 1.8 mS (varying the timeout only leads to confusing results).

Anyway I am tempted to suggest you use a CHANGE interrupt rather than pulseIn.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

robtillaart


Quote
Then why does it say that the pulseIn function works on pulses b/w 10usec to 3min in length.


That should be possible but the math prevents this. I expect the function has changed a bit over the different versions and that "this bug" creeped in ....

you may patch your instance of pulsein() with the above change - C:\Program Files (x86)\arduino-0022\hardware\arduino\cores\arduino\wiring_pulse.c - that should expand the working range of the function to at least 1 minute I estimate.

Rob
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Mr.Debugger

So, what you are suggesting is that I should replace the following code:

Code: [Select]

return clockCyclesToMicroseconds(width * 21 + 16);
}

With this ?
Code: [Select]

return ((width*21+16) / clockCyclesPerMicrosecond() );

Where did this come from:
Code: [Select]

#define clockCyclesToMicroseconds(a) ( ((a) * 1000L) / (F_CPU / 1000L) )

The above code is not present in the header file of wiring_pulse

This is what I basically did but I didn't see any difference. :(

robtillaart

Quote
#define clockCyclesToMicroseconds(a) ( ((a) * 1000L) / (F_CPU / 1000L) )


Comes from - C:\Program Files (x86)\arduino-0022\hardware\arduino\cores\arduino\wiring.h - on my win 7 machine

Strange you did not see any difference .. I'm gonna search for a free arduino to test ...

Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

robtillaart

Back again, my testcode
Code: [Select]

unsigned long duration;
int counter = 0;

unsigned long before = 0;
void setup()
{
  pinMode (5, INPUT);
  Serial.begin(9600);
}
void loop()
{
  duration = pulseIn(5, LOW, 10000000L); // yes 10.000.000
  {
    Serial.print("D: ");
    Serial.println (duration);
    Serial.print("M: ");
    Serial.println(millis()-before);
    before = millis();
  }
}


Got strange results indeed, ...think think think... review PulseIn() again, found it used the function  microsecondsToClockCycles()  also in the code to determine the timeout.

rewrote PulseIn() to get at least a better timeout

Code: [Select]
/* Measures the length (in microseconds) of a pulse on the pin; state is HIGH
* or LOW, the type of pulse to measure.  Works on pulses from 2-3 microseconds
* to 3 minutes in length, but must be called at least a few dozen microseconds
* before the start of the pulse. */
unsigned long pulseIn(uint8_t pin, uint8_t state, unsigned long timeout)
{
// cache the port and bit of the pin in order to speed up the
// pulse width measuring loop and achieve finer resolution.  calling
// digitalRead() instead yields much coarser resolution.
uint8_t bit = digitalPinToBitMask(pin);
uint8_t port = digitalPinToPort(pin);
uint8_t stateMask = (state ? bit : 0);
unsigned long width = 0; // keep initialization out of time critical area

// convert the timeout from microseconds to a number of times through
// the initial loop; it takes 16 clock cycles per iteration.
unsigned long numloops = 0;
unsigned long maxloops = timeout * clockCyclesPerMicrosecond() / 16;     //unsigned long maxloops =  microsecondsToClockCycles(timeout) / 16;

// wait for any previous pulse to end
while ((*portInputRegister(port) & bit) == stateMask)
if (numloops++ == maxloops)
return 0;

// wait for the pulse to start
while ((*portInputRegister(port) & bit) != stateMask)
if (numloops++ == maxloops)
return 0;

// wait for the pulse to stop
while ((*portInputRegister(port) & bit) == stateMask) {
if (numloops++ == maxloops)
return 0;
width++;
}

// convert the reading to microseconds. The loop has been determined
// to be 20 clock cycles long and have about 16 clocks between the edge
// and the start of the loop. There will be some error introduced by
// the interrupt handlers.
return (width * 21 + 16)/clockCyclesPerMicrosecond();    // return clockCyclesToMicroseconds(width * 21 + 16);
}


Seems to work a lot better!  If it works better I will report it as bug ...

Please give it a try...




Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Mr.Debugger

The code seems to work fine after the correction :)
Also check out the latest version of Arduino 0023.

robtillaart

#12
Nov 13, 2011, 09:44 am Last Edit: Nov 13, 2011, 10:15 am by robtillaart Reason: 1
(rewritten this post a few times)

reread this thread after a few weeks, the error comes up in pulseIn() but its rootcause is in the macro's in wiring.h
=> C:\Program Files (x86)\arduino-0022\hardware\arduino\cores\arduino\wiring.h

Code: [Select]

#define clockCyclesPerMicrosecond() ( F_CPU / 1000000L )
#define clockCyclesToMicroseconds(a) ( ((a) * 1000L) / (F_CPU / 1000L) )
#define microsecondsToClockCycles(a) ( ((a) * (F_CPU / 1000L)) / 1000L )


The above macros should be rewitten to increase their working range preventing overflow for "relative small" values

Code: [Select]

#define clockCyclesToMicroseconds(a) ( (a) / clockCyclesPerMicrosecond() )
#define microsecondsToClockCycles(a) ( ((a) * clockCyclesPerMicrosecond() )


Drawback is that CPU with frequencies that are not a whole multiple of 1Mhz get an error, but afaik there are only 8, 16 and 20 MHz versions of Arduino.

By changing the macros the original pulseIn() code would not need to be changed as the problem is solved at its root cause.

I'll update this in the bugreport - http://code.google.com/p/arduino/issues/detail?id=675 - too
Rob Tillaart

Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -
(Please do not PM for private consultancy)

Bret

This had me really vexed!!!!

I found a example of a simple camera shutter speed tester and while I could manage to get usable readings down to about 1/30 sec below that was nothing. I also found it very hard to get the function to wait for a longer timeout so I tried making my own simple function that worked fine for lower speeds but seemed to fail above about 1/250 sec shutters, so I'm not sure what to try here.....

I have simple IR emitter/Receiver setup with the following code

Code: [Select]
const int readPin=12;

long pulseLength;
int pinStatus;
long speed=0;

void setup(){
  Serial.begin(9600);
}


void loop(){
  //Serial.print("Pin:");
  //Serial.println(digitalRead(readPin));
  speed=testShutter(readPin);

  //Serial.println(speed);
  //on Button go a wait for Read to go low and count how long
}

long testShutter(int rPin){
  unsigned long now;
  unsigned long done;

  do
{
  pinStatus=digitalRead(rPin);
  if (pinStatus==LOW){//wait for LOW
  Serial.println("counting");
    //LOW start counting
    now=micros();
      while (digitalRead(rPin)==LOW){
      //wait for High
      }
      done=micros();
     pulseLength=done-now;
     Serial.print ("Started:");
     Serial.println(now);
     Serial.print ("ended:");
     Serial.println(done);
     Serial.print("difference is pulse:");
     Serial.println(pulseLength);
     Serial.print("****");
     Serial.print(pulseLength/1000000ul);
     Serial.println("seconds");
     
  }
  return pulseLength;
 
  break;

} while (speed=0);

/*do {
  pulseLength=pulseIn(rPin,LOW);
}while(pulseLength==0);
return pulseLength;*/
}


-Bret Lanius

Nick Gammon

#14
Jan 14, 2012, 10:50 pm Last Edit: Jan 14, 2012, 10:52 pm by Nick Gammon Reason: 1
Don't use pulsein, use interrupts. This sketch:

Code: [Select]
volatile boolean started;
volatile unsigned long startTime;
volatile unsigned long endTime;

// interrupt service routine
void shutter ()
{
 if (started)  
   endTime = micros ();   // shutter close time
 else
   startTime = micros ();  // shutter open time

 started = !started;   // toggle flag
}  // end of shutter

void setup ()
{
 Serial.begin (115200);
 Serial.println ("Shutter test ...");
 attachInterrupt (0, shutter, CHANGE);
}  // end of setup

void loop ()
{

 if (endTime)
   {
   Serial.print ("Shutter open for ");
   Serial.print (endTime - startTime);
   Serial.println (" microseconds.");
   endTime = 0;
   }  

} // end of loop


You need to connect the shutter to D2 (one of the pins that takes a change interrupt). I am successfully measuring a pulse of 50 uS with that code (1/20000 of a second).

Just invert the result to get the shutter speed. eg.

For 50 microseconds take 1/ 0.000050 giving 20000 (shutter speed of 1/20000).
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Go Up