pulseIn() bug?

pulseIn() seems to display a strange bug of unstable output when read from two different input pins. I've found a workaround, but it's not very elegant. Here's the problem:

I'm using two TSL230 chips to read two different light sources and I'm using pulseIn() to read them like so:

SENSOR1_duration = pulseIn(FREQ1_PIN, HIGH); // read the first TSL230 chip
SENSOR2_duration = pulseIn(FREQ2_PIN, HIGH); // read the second TSL230 chip

This should give me consistent data from each sensor if the light is not changing, but instead I get a stream of data like this:

S1=30349      S2=20614      
S1=19241      S2=2356            <--- note the weird values here from S2
S1=30217      S2=20832      
S1=15929      S2=5609      
S1=30466      S2=20769      
S1=12717      S2=8887            <--- and here from S1 and S2
S1=30639      S2=20939      
S1=9767      S2=11907            <--- and here from S1
S1=30455      S2=21000      
S1=6613      S2=14913      
S1=27073      S2=20935      
S1=3819      S2=17583      
S1=24214      S2=20877      
S1=560      S2=20959      
S1=21479      S2=2274      
S1=30465      S2=20801      
S1=16194      S2=5524

Note the values jittering around.

On the other hand, if I simply read one sensor only (no change in hardware, just commenting out the other pulseIn()), I get steady, consistent output like this:

S1=30407
S1=30284
S1=30443
S1=30467
S1=30492
S1=30597
S1=30681
S1=30746
S1=30569
S1=30477
S1=30594
S1=30964
S1=30474
S1=30847

If I read the other sensor alone, I get similarly stable values. It's only when reading both pulseIn()s that I get the unstable output.

The workaround--which works perfectly--is to read each sensor twice and throw away the first value. Code looks like this:

SENSOR1_duration = pulseIn(FREQ1_PIN, HIGH); // gets thrown away.
SENSOR1_duration = pulseIn(FREQ1_PIN, HIGH); // re-do the pulseIn()
SENSOR2_duration = pulseIn(FREQ2_PIN, HIGH); // gets thrown away.
SENSOR2_duration = pulseIn(FREQ2_PIN, HIGH); //re-do the pulseIn()

This gives the expected output with no jumping around:

S1=30407      S2=20879
S1=30284      S2=21087
S1=30443      S2=20994
S1=30467      S2=20842
S1=30492      S2=20914
S1=30597      S2=20857
S1=30681      S2=20902
S1=30746      S2=20777
S1=30569      S2=20700
S1=30477      S2=20953
S1=30594      S2=20767
S1=30964      S2=20823
S1=30474      S2=20844
S1=30847      S2=20707
S1=30824      S2=20989
S1=30496      S2=20834
S1=30465      S2=20746
S1=31029      S2=20771

I've tried various other ways to work around this weirdness (turning off interrupts, adding delays, etc.), but this is the only way that works, yet it shouldn't be necessary. Am I missing something? Is the arduino team missing something? Is the Wiring code underlying pulseIn() buggy?

Have you tried just measuring the pulse yourself? I haven't experienced your issue but I had issues with it never timing out because of a never-ending pulse, so I decided to measure it myself and not be concerned about the minimal difference in accuracy.

I think I did it something like this at work:

long pulseCount(int timeOut, int thisPin){
  boolean running = true;
  long millisStart = millis();
  long millisMeasureStart = 0;

  while(running){

    if(millis() > (millisStart + timeOut)){
      running = false;
    }

    if (digitalRead(thisPin)==HIGH){
      running = false;
      millisMeasureStart = millis();
      
      while(digitalRead(thisPin)==HIGH){
        // Wait until it ends
      }
      
      return millis() - millisMeasureStart;
      
    }
  }

  return 0;
}

That can probably be made much more efficient. I can't remember how I've actually done it at work, I don't remember it being that bloated. Anyway, you get my point!

Cheers,
Scott

Edit: Fixed code and remember my reasoning.

This is the code behind pulseIn in case you were curious:

/*
  wiring_pulse.c - pulseIn() function
  Part of Arduino - http://www.arduino.cc/

  Copyright (c) 2005-2006 David A. Mellis

  This library is free software; you can redistribute it and/or
  modify it under the terms of the GNU Lesser General Public
  License as published by the Free Software Foundation; either
  version 2.1 of the License, or (at your option) any later version.

  This library is distributed in the hope that it will be useful,
  but WITHOUT ANY WARRANTY; without even the implied warranty of
  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  Lesser General Public License for more details.

  You should have received a copy of the GNU Lesser General
  Public License along with this library; if not, write to the
  Free Software Foundation, Inc., 59 Temple Place, Suite 330,
  Boston, MA  02111-1307  USA

  $Id: wiring.c 248 2007-02-03 15:36:30Z mellis $
*/

#include "wiring_private.h"
#include "pins_arduino.h"

/* 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 = microsecondsToClockCycles(timeout) / 16;
      
      // 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)
            width++;

      // convert the reading to microseconds. The loop has been determined
      // to be 10 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 clockCyclesToMicroseconds(width * 10 + 16); 
}

I think the problem is that pulseIn() doesn't wait for a pulse to start (i.e. a LOW to HIGH transition). If the pin is already high when you call pulseIn(), it starts timing immediately, losing whatever portion of the pulse happened before you called the function. In Arduino 0013 this has been changed so that the function waits for a LOW then a HIGH (or vice-versa), which should fix the problem.

@Mellis: Yeah, sounds right. As you posted your message I was editing my original message because I realised that my issue was from a pulse that never ended (which in reality was because I hadn't tied the input to ground, so it just floated along all day). We live and we learn!

Mellis is correct regarding pulseIn, also note, that pulseIn() is a fairly poor way of working with the TSL230R. (Not very accurate =) A much more accurate, easier, and non-blocking method is to use an interrupt and count rising pulses - so that you have an actual frequency counter, rather than extrapolating frequency from a single measurement. I did a write-up in my intro to the TSL230R w/ arduino:

Hi drone, Nice write-up!

An alternative to using interrupts to count pulses is to use the input capture feature of timer1. input capture measures the width of a pulse in hardware so it works completely in the background. Its a little tricky to set up the timer but there is an example in the this thread:
http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1201890734

Thanks mem! I'll have to keep that trick in the ol' folder for the next project =)

!c

@scootabug: Thanks for your ideas, but the pulses I'm measuring range from 1 second to 1 µs, so the counter loop you propose wouldn't handle it fast enough. But it's a nice thought.

@mellis: This seems like the most likely issue. I'm running this on Arduino 12. I didn't realize that Arduino 13 is out (is it? I'll have to look.)

@drone: I had actually seen your article...excellent, btw...and ordinarily that's how I'd want to do it. I note that you even mention the potential limitations of pulseIn(). The problem is that I have more than two TSL230s (I know, I know, above I said two...since that's all there is for the moment, but the final project will have three or four). AFAIK, there are only two interrupt lines on the Arduino Duemilanove...hence my problem...unless there's some way to have more interrupt lines. Is there? (without getting too crazy complicated...I'm a bit of a newb' at this).

@mem: Yours is an interesting idea. I haven't looked into this at all, but it might be something to try. Thanks!

Anyway, my kludge of calling it twice seems to work perfectly, but it's certainly inelegant. Thanks, everyone, and I'll let you know what I find with your various approaches.

--faceman

Face - that is a limitation with my method. Do you have to read them all at the same time though?

If not, you can attach them all to the same input pin, and use the OE (output enable pin) on each chip to control which one you're reading. If you have three, you would set OE high on two, and read from the third. If you can handle reading them 1 second apart, you could switch chips as part of reading frequency function.

But otherwise, yeah, it looks like you'll have to rely on pulseIn() for more than two, afaik.

!c

@drone: that's freaking brilliant! [slaps forehead] Yes, that's a great solution! They don't need to be read at exactly the same time. Even reading them within a second or two would work just fine. Right now, I just have OE jumpered to ground on all of them, but this would be an easy switch.

I could set 'em all high, then pull chip 1 low, read it with the interrupt handler, set it high again, and move through the other two the same way (to avoid ever having two chips low at the same time). Too bad it's getting a bit late 'cause I can't wait to try this.

Thanks!

I was having this very problem with an mems2125 accelerometer. One axis or the other was always erratic when I tried to read them together, but alone they would be fine. I tried faceman's fix of doubling up the pulseIn() reads on each axis and now it's perfect. I'll play around with other polling methods to see if something else works, too. But, this tip saved me from running out to replace my 2125... I honestly thought it was bad. Thanks.

I had a fair amount of trouble placing drone's method into a subroutine so i could read multiple sensors from the same chip.

i thought i'd post my solution here to save anyone else the trouble, it basically consists of detatching the interrupt at the end of the subroutine.

//  Jonathan Isaiah 3/28/2010  TSL light to frequency sensor code, in subroutine
//  Adapted from tutorial by the roaming drone found @ (forum wont let me insert link, totorial is linked above anyway)

//            Pin Mappings
//   ______________________________
//  |__TSL230_Pin__|__Arduino_Pin__|
//  |       6      |       2       |
//  |       1      |       7       |
//  |       2      |       6       |
//  |       7      |       4       |
//  |       8      |       5       |
//   ------------------------------
#define TSL_OUTPUT_PIN              2 

#define SAMPLE_DURATION          250 //quarter second matches problem fix time below
 
unsigned long pulseCount          = 0;
unsigned long currentSampleTime   = millis();
unsigned long previousSampleTime  = currentSampleTime;
unsigned int sampleTimeDifference = 0;
unsigned int tsl230scalingFactor  = 1;
unsigned int tsl230sensitivity    = 1;
unsigned long freq; 
float irradiance;

void setup() {
  Serial.begin(9600);//start serial communication
 
  
 
  pinMode(TSL_OUTPUT_PIN, INPUT);
 

}
 

void loop(){
  
getfreq();
Serial.print("Sensor Frequency: ");
Serial.print(freq);
Serial.println();
Serial.print(irradiance);
Serial.print(" Watts / Meter sq");
Serial.println();
Serial.println();
delay(500);
}
 
void getfreq() { // output the current irradiance every SAMPLE_PERIOD ms
  attachInterrupt(0, incrementPulseCount, RISING);// add an intterupt on pin 2, trigger on rising pulse (0->5V rising edge)
  freq = 0;
  delay(250);  //delay 250 (unknown problem fix)
  previousSampleTime  = currentSampleTime; //set the previous time value
  currentSampleTime   = millis(); //get the current time value
 
  if( currentSampleTime > previousSampleTime ) {  // calculate how much time has passed. if the current time value is later longer than the last one, add the add the difference
    sampleTimeDifference += currentSampleTime - previousSampleTime;
  }
  else if( currentSampleTime < previousSampleTime ) { //otherwise, it has rolled over, so subtract from the maximum
    sampleTimeDifference += ( currentSampleTime + ( 34359737 - previousSampleTime )); //since millis() rolls over after ~9.5 hours, 34359737
  } 
 
  if( sampleTimeDifference >= SAMPLE_DURATION ) {// if SAMPLE_DURATION ms have passed, output a reading
    sampleTimeDifference = 0;  // re-set the sample time counter
    unsigned long frequency = getTsl230Frequency(); // get the current frequency reading
   float irradianceLocal = calculateWattsPerMeterSquared(frequency); //calculate irradiance
   //Serial.println(irradianceLocal); //output irradiance
   //Serial.println(frequency);
   freq = frequency;
   irradiance = irradianceLocal;
  }
detachInterrupt(0);
}
 
void incrementPulseCount() {
  pulseCount++;// increment the pulse count
  return;
}
 
unsigned long getTsl230Frequency() {
  unsigned long frequencyValue = pulseCount * tsl230scalingFactor;// copy the pulse count value and multiply by the scaling factor.
  pulseCount = 0;// re-set pulse counter
 
  return(frequencyValue); //return frequency value
}
 
float calculateWattsPerMeterSquared(unsigned long frequency) { // return approx irradiance, assume monochromatic 640 [nm] source as per datasheet graph of Output Frequency vs Irradiance
  float wattsPerMeterSquared = ( (float) (frequency/4) / ((float) 10 * (float)tsl230sensitivity)) * 0.01; //equation as per the datasheet, factor of 0.01 to go from uW/cm^2 to W/m^2  also (frequency/4 makes up for quarter second sample time
 
  return(wattsPerMeterSquared);
}