Improve reliability, especially at low clock speed

[tstarling]

I found that the library was not working reliably with my 8 MHz Seeeduino Stalker. Some investigation with an oscilloscope showed that the input polling loop was quite slow (maybe 8us), and timer interrupts were taking ~10us. This meant that the polling loop commonly missed the first sensor-driven transition after pullup, and any transitions which occur close to timer interrupts. So:

• Use pulseIn() instead of our own loop. This is a little bit faster
  than what was done before, is calibrated correctly, and could
  potentially be optimised upstream to provide better resolution in
  future. It makes the code a bit simpler and avoids the need to watch
  for the first sensor pulldown.
• Disable interrupts during pulseIn(), to provide accurate timing.
• Disabling interrupts, and the fact that I was using sleep mode,
  meant that millis() was not accurate. So instead of depending on
  millis() to determine whether to do a sensor read, require the caller
  to explicitly trigger a sensor read, and rely on the caller to wait
  the requisite sampling period. This means removing resetTimer() and
  making readSensor() public.

Also:

• Use INPUT_PULLUP, which was introduced in Arduino 1.0.1.

Tested on DHT11 and RHT03 (like DHT22) in autodetect mode. Gathered data from the RHT03 for 6 days, 1800 samples, with no timeout or checksum errors.

[markruys]

Looks good, disabling interrupts to get a more precise timing. Your changes alter the core of the original code, so I want to test it on the hardware I have too before merging it.
A pity that because you drop lastReadTime, you now have to explicitly call readSensor() before you call getHumidity() or getTemperature(). This will break existing code from others who use this library.

[tstarling]

I can split it up if you only want to merge part of it. It will more or less work with the old lastReadTime system, it's just that the sampling interval will be a few milliseconds longer due to the missed timer interrupts. And of course it won't work for me, since I am using sleep mode. In my application, the RTC wakes up the CPU every 5 minutes, with only a few milliseconds added to millis() each time. Explicitly triggering a read seems like a good policy, it is used by a lot of other sensor libraries, but if you really need backwards compatibility then I can fork instead.

Alternatively you could make a branch in your repo for the legacy interface, and merge the full patch into master.

[matthijskooijman]

I wonder if disabling interrupts is a good idea? Having interrupts
disabled for a few ms isn't usually a good idea, one of the selling
points of this particular DHT library is IMHO that it works with
interrupts enabled. Of course, having interrupts disabled makes the
readings sensitive to being interrupted. However, the difference between
a short and long pulse is 68-30 uS, buying 48uS of leeway. Assuming that
a digitalRead takes 10uS (measured on 8Mhz pro mini), that leaves 38uS
of interrupt time.

I was going to argue that changing the low-high threshold from > 30 to,
say, > 60 would drastically reduce the chance of interrupts messing up
the readings. I tried using a bit of code like below to force the race
condition to occur:

--- a/DHT.cpp
+++ b/DHT.cpp
@@ -134,6 +134,9 @@ void DHT::readSensor()
     delayMicroseconds(800);
   }

+  while(TCNT0 > 222) /* wait */;
+  while(TCNT0 < 222) /* wait */;
+
   pinMode(pin, INPUT);
   digitalWrite(pin, HIGH); // Switch bus to receive data

This didn't work as well as I'd expected. Additionally, raising the
threshold didn't work either. Looking more closely at the code, I
noticed there is a big chance that an interrupt happens between setting
age and reading the pin, which would actually cause age to be an
underestimate (there's also a smaller chance of the interrupt
happening just after the reading, which, if the end of the pulse is also
imminent, causes an over-estimation).

I think that some fiddlery with disabling interrupts for a part of the
loop (to force interrupts to happen either before or after reading the
pin) could help to ensure things are either underestimated or
overestimated (though the slow-ness of digitalRead() can still swing
both ways I think). I don't have any further time to investigate now (I
really should be getting my actual work done), though.

Thinking on this more closely, I realize that ensuring over- or under-
estimaton only helps if you apply the inverse on the start of pulse
detection (e.g. if you make sure that interrupts always cause
over-estimation of the timestamp of both the start and end edges, the
end result can still be both higher and lower).

So, perhaps disabling interrupts is the right approach after all... If
you do, I think using the pulseIn() function is a smart approach, since
it contains very tight architecture-optimized code, which should make it
accurate and portable.

A pity that because you drop lastReadTime, you now have to explicitly
call readSensor() before you call getHumidity() or getTemperature().
This will break existing code from others who use this library.

Is this really needed? Disabling interrupts prevents millis() from
counting, but a complete read shouldn't take more than a few ms?