DHT11 / clearWatch error weird behavior

Hoping someone can help. Having issue with reading DHT11 that I don't understand.

If I never call updateDisplay() then DHT11 returns "temp:-1,rh:-1,err:true,checksumError:false" forever (tried leaving it for days).

If I call updateDisplay() then it returns -1, but after a few iterations I get a clearWatch() error message, then DHT11 temp/rh is read correctly forever.

Compacting Flash...
Calculating Size...
Writing..
Compressed 36800 bytes to 11307
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Temp is -1 and RH is -1, Err: true, ChecksumError: false
Uncaught Error: clearWatch(undefined) not allowed. Use clearWatch() instead.
at line 1 col 19
clearWatch(a.watch);delete a.watch;var e=parseInt(b.substr(2...
^
in function called from system
Temp is 25 and RH is 58
Temp is 26 and RH is 55
Temp is 25 and RH is 54
Temp is 25 and RH is 54
Temp is 25 and RH is 54
Temp is 25 and RH is 54
Temp is 26 and RH is 55

My code:

var displayReady=false;
var dht = require("DHT11").connect(D15);
var spi = new SPI();
spi.setup({mosi:D23 /* sda */, sck:D22 /* scl */, baud: 4000000});
var g = require("ILI9163").connect(spi, D21 /* DC */, D18 /* CE */, D19 /* RST */, function() { 
  displayReady = true; 
  console.log("Display ready.");
});
require("Font8x12").add(Graphics);

var temp = "N/A";
var rh = "N/A";


function updateDisplay() {
  //return;
  if (!displayReady)
    return;
        
    g.clear();
    g.setColor(1,1,1);
    g.setFont8x12();
    g.setFontVector(8);
    g.setRotation(1);
    g.drawString("Test",0,0);
    
    g.setFontVector(20);
  
    g.setColor(1,0.5,1);
    g.drawString("Temp: " + temp.toString(),0,10);
    
    g.setColor(0,1,1);
    g.drawString("Humid: " + rh.toString(),0,40);
}

function readSensor() {
  dht.read(function (a) {
    temp = a.temp;
    rh = a.rh;
    if (a.temp != -1)
      console.log("Temp is "+a.temp.toString()+" and RH is "+a.rh.toString());
    else
      console.log("Temp is "+a.temp.toString()+" and RH is "+a.rh.toString() + ", Err: " + a.err.toString() + ", ChecksumError: "+ a.checksumError.toString());
    
    updateDisplay();
  });
}

setInterval(function() {
  readSensor();
}, 10000);

In my opinion a bug in the dht11.js software module. You are not the first to have issues, see also:
http://forum.espruino.com/conversations/316733/#comment14104546
I don't have a fix, and no DHT11 to test.

If it is the DHT11 then it seems weird that it works once clearWatch() causes error. Also if I copy in the DHT11 code (slightly modified so it runs) then it consistently gets correct data from the start. So I doubt it is the DHT11 hardware. (Though now it crashes later on due to "New interpreter error: MEMORY")

var ht={pin: D27};

function DHT11read(cb, n) {
  if (!n) n=10;
  var d = "";
//  var ht = this;
  digitalWrite(ht.pin, 0);
  pinMode(ht.pin,"output"); // force pin state to output
  // start watching for state change
  this.watch = setWatch(function(t) {
    d+=0|(t.time-t.lastTime>0.00005);
  }, ht.pin, {edge:'falling',repeat:true} );
  // raise pulse after 1ms
  setTimeout(function() {pinMode(ht.pin,'input_pullup');pinMode(ht.pin);},20);
  // stop looking after 50ms
  setTimeout(function() {
    if(ht.watch){ ht.watch = clearWatch(ht.watch); }
    var cks =
        parseInt(d.substr(2,8),2)+
        parseInt(d.substr(10,8),2)+
        parseInt(d.substr(18,8),2)+
        parseInt(d.substr(26,8),2);
    if (cks&&((cks&0xFF)==parseInt(d.substr(34,8),2))) {
      cb({
        raw : d,
        rh : parseInt(d.substr(2,8),2),
        temp : parseInt(d.substr(18,8),2)
      });
    } else {
      //if (n>1) setTimeout(function() {ht.read(cb,--n);},500);
      if (n>1) setTimeout(function() {DHT11read(cb,--n);},500);
      else cb({err:true, checksumError:cks>0, raw:d, temp:-1, rh:-1});
    }
  }, 50);
}

var displayReady=false;
//var dht = require("DHT11").connect(D27);

var spi = new SPI();
spi.setup({mosi:D23 /* sda */, sck:D22 /* scl */});
var g = require("ILI9163").connect(spi, D21 /* DC */, D18 /* CE */, D19 /* RST */, function() { 
  displayReady = true; });
require("Font8x12").add(Graphics);

var temp = "N/A";
var rh = "N/A";

function updateDisplay() {
  return;
  if (!displayReady)
    return;
    console.log("ILI9163");
    g.clear();
    g.setColor(1,1,1);
    //g.setFont8x12();
    g.setFontVector(8);
    g.setRotation(1);
    g.drawString("Mensa Norge Landstreff FTW!",0,0);
    
    g.setFontVector(20);
  
    g.setColor(1,0.5,1);
    g.drawString("Temp: " + temp.toString(),0,10);
    
    g.setColor(0,1,1);
    g.drawString("Humid: " + rh.toString(),0,40);
     
}

function readSensor() {
    console.log("DHT11 pre read");
  //dht.read(
  DHT11read(
    function (a) {
    console.log("DHT11 post read");
    temp = a.temp;
    rh = a.rh;
    console.log(a);
    updateDisplay();
  });
}


setInterval(function() {
  readSensor();
}, 10000);

Result:

| |_ ___ ___ _ ||___ ___
| |_ -| . | _| | | | | . |
||_| || |_|||_|_|

     |_| espruino.com

2v04 (c) 2019 G.Williams
Espruino is Open Source. Our work is supported
only by sales of official boards and donations:
http://espruino.com/Donate

DHT11 pre read
DHT11 post read
{
"raw": "010011001000000000000110000000010001001110",
"rh": 50, "temp": 24 }
DHT11 pre read
DHT11 post read
{
"raw": "010011010000000000000101110000100101010100",
"rh": 52, "temp": 23 }
DHT11 pre read
DHT11 post read
{
"raw": "010011001000000000000101110000100101010010",
"rh": 50, "temp": 23 }
DHT11 pre read
DHT11 post read
{
"raw": "010011000100000000000110010000100101010011",
"rh": 49, "temp": 25 }
DHT11 pre read
DHT11 post read
{
"raw": "010011000100000000000110000000000001001001",
"rh": 49, "temp": 24 }
DHT11 pre read
DHT11 post read
{
"raw": "010011000000000000000110000000000001001000",
"rh": 48, "temp": 24 }
DHT11 pre read
DHT11 post read
{
"raw": "010011000000000000000110000000000001001000",
"rh": 48, "temp": 24 }
DHT11 pre read
DHT11 post read
{
"raw": "010011000100000000000110000000000001001001",
"rh": 49, "temp": 24 }
DHT11 pre read
DHT11 post read
{ "err": true, "checksumError": false,
"raw": "0",
"temp": -1, "rh": -1 }
DHT11 pre read
DHT11 post read
{ "err": true, "checksumError": false,
"raw": "0",
"temp": -1, "rh": -1 }
DHT11 pre read
Execution Interrupted during event processing.
New interpreter error: MEMORY
DHT11 pre read
ERROR: Ctrl-C while processing interval - removing it.
Execution Interrupted during event processing.
New interpreter error: CALLBACK

Sorry, edited my post. I wanted to say, the DHT11.js software module has most likely a bug, not the hardware.

Sun 2019.09.01

Hi @th to assist in locating where in the code the memory leak is occurring:
Around L86

process.memory().free

https://www.espruino.com/Reference#t_l_process_memory

See #3 "New interpreter error: MEMORY"

http://forum.espruino.com/conversations/321537/

So it seems scoping of setWatch affects readings. I'm not familiar with watch, but it seems to be a "special" citizen, defying normal expectations and capable of crashing everything after repeated use.

This works for some time before it fails with MEMORY.

var dht = require("https://raw.githubusercontent.com/tedd/Espruino_DHT11/master/DHT11_test.js").connect(D27);

function readSensor() {
  dht.read(
    function (a) {
    console.log(a);
    setTimeout(readSensor, 5000);
  });
}

readSensor();

To fix the out-of-memory error, replace these lines (refering to post #3)

this.watch = setWatch(function(t) {
    if(ht.watch){ ht.watch = clearWatch(ht.watch); }

with these lines

var w = setWatch(function(t) {
clearWatch(w);

It would be better to set the watch only once (as well as some other setup tasks), instead of setting it in every run (as it is done in this .js module).

Edit: ok, it's posted in the ESP32 forum. Stupid me...
Similar error in this thread DHT11 / DHT22 uses same communication "protocol"

@maze1980 Thanks, will try that!

@AkosLukacs ESP32 (dev board), yes.

One thing you could try to identify the bug in the DHT11.js module: Print the read "raw" value, using your code in #1.
tldr:
I'd assume it's length differs from the length in #3, having one extra bit. And removing this bit would fix the module for the ESP32, and would break it for all other boards having a different timing. A rewrite of the module with the setup of the watch only once at the "connect" would be the best solution, and should fix it for all platforms.

Thanks @maze1980, that did the trick.

Moved setWatch to ctor, writes to shared buffer. Set a cap on how many readings it will buffer (in case read is never called), and clear buffer upon fresh read. I'll look into making it more efficient later. As long as we have enough data "continously" there is no need for waiting, so it could return immediately.

var dht = require("https://raw.githubusercontent.com/tedd/Espruino_DHT11/master/DHT11_test.js?3").connect(D27);

function readSensor() {
  dht.read(
    function (a) {
    console.log(a);
    setTimeout(readSensor, 5000);
  });
}

readSensor();

Looks good, just a lil' bit hacked ( var _this = this; and later var ht = this;). I'd keep it the same variable in both lines - I like to use "var self = this;" as it works without underscore.

Maybe a disconnect function would be nice, to remove the watch when no longer needed. But memory is limited, and I don't think that anyone would ever use it. Unless trying to connect different devices to the same pin, which would be a bad design, better using I2C instead.

You should set the pin mode in the connect function to pinMode(ht.pin, 'input_pullup');, to avoid random signals to be received.

And you could replace a code of code as follows, mainly for better comments, from

...
  digitalWrite(ht.pin, 0);
  pinMode(ht.pin,"output"); // force pin state to output
  // raise pulse after 1ms
  setTimeout(function() { pinMode(ht.pin, 'input_pullup'); pinMode(ht.pin); }, 20);
  // stop looking after 50ms
...

to

...
 //pull signal to ground for 20 ms to request data from DHT11
  pinMode(ht.pin,"output");
  digitalWrite(ht.pin, 0);
  setTimeout(function() { pinMode(ht.pin, 'input_pullup'); }, 20);

  //parse the received data after 50ms
...

You invoke clearWatch(handle) with invalid handle. Behavior is absolutely correct.

Best pattern for dealing with setWatch() and clearWatch() is:

var watchIdX = undefined;
...
watchIdX = setWatch(...);
...
watchIdX = clearWatch(watchIdX);
...
// and to make it safe:
if (watchIdX) watchIdX = clearWatch(watchIdX);

If your watch is a single shot, you also have to clear the watch handle...

var watchIdX = undefined;
...
watchIdX = setWatch(function(...) {
    watchIdX = undefined;
  ... }, pinY, {repeat:false, ...} );

(EDITED) Before a recent release, clearWatch(undefined) and clearInterval(undefined) were handled lick without argument and that lead to unintentional clearing of all watches, timeouts and intervals. Clearing all watche, intervals and timeouts produces unpredictable results, as it is easy to understand in a (timer) interrupt driven context. (thanks @AkosLukacs - see post #16).

The other challenge you may face in your code is asynchronous behavior: some code issues a command to a (peripheral) device to prepare some data, then you have to wait for sending another command to receive - pull / get - the prepared data and then process that data. Even pulling the prepared data involve the pattern of repeated, asynchronous pulling of pieces of data until the data gram is complete. The basic solution technique is callbacks, and in elegant form it is Promises. Trying to delay is always a lottery in regard to delay enough... and if it works, it slows overall application flow unnecessarily down.

Another safe approach I choose is to start active code within the onInit(){ ... } function in order to let the upload complete BEFORE acting code (with setWatch, setInterval, setTimeOut) - code on level 0 / immediate code - is executed.

Thanks. I checked the DHT11 spec to see what the timings are. Looks like the 50ms is overkill, fail can be determined earlier than 500ms and parsing can be optimized a bit.

It has been running for a few hours and seems to work fine now. I'll fix up the code in a few days from now. Good to get it working since I have ordered 35 ESP32+DHT11+TFT displays for a small intro course I'm holding next month. Would be a bit too fiddly if we had to go for C in Ardunio IDE. :)

Re: clearWatch / clearInterval:

• calling clearWatch without arguments is valid, and clears all watches
  
• calling clearWatch(undefined) throws an error. To avoid the mistake when you had a watch / interval number stored, cleared it, cleared the number, but call it again, but now accidentally clear all watches
  

Ended up rewriting a bit today. Will finish later, but so far:

• It works (on my computer)
  
• Has higher accuracy (fractions are used)
  
• Is faster (more accurate delay, no loop)
  
• Simpler code (no recursion)
  
• Code is commented so it should be easy to debug
  
  

https://github.com/tedd/Espruino_DHT11/blob/master/DHT11_new.js

@th, sorry for the confusion - and thanks @AkosLukacs to point it out (I remembered it kind of reverse...).... Before clearWatch(undefined) did the same as clearWatch()... which was bad... (To be clear, I'll edit my previous post w/ remark).

The change to prevent defaulting undefined to no argument also fixed issue of clearing an already cleared watch / interval / timeout again with the same - defined - handle. Before it complained, now it just ignores. Even though cleared handles get at one point in time reused it is of no real concern (when coding properly).

Nice rewrite, I really like it. Maybe you could

1. add pinMode(self.pin, 'input_pullup'); to function DHT11(pin) before setting the watch. In my opinion not having this is the reason that the first bit is zero in the first run, and one in the consecutive runs that you observed (line 15).
   
2. replace self.d += 0 | (t.time - t.lastTime > 0.00005); with self.d += Number(t.time - t.lastTime > 0.00005); (line 19).
   
3. remove pinMode(self.pin); (line 40). The signal should be pulled all time, line 39 is sufficient.
   
4. Remove the 10 consecutive measurements, already done.
   
5. fix // Two first bytes are header ... to bits (line 63).
   
6. add err: false in the callback for a successful measurement (line 79).
   
7. change to temp: undefined, rh: undefined in the callback for a failed measurement (line 89). Because -1 is a valid temperature it's not an optimal return value for a failed measurement, and also not good for further processing (you can store -1 for temp and rh in a database, if you don't test)
   
8. Test t, tf, rh, rhf to be numbers before calling parseFloat. If self.d is less than 42 bits the parseInt will eventually return "NaN", so I would suggest to test the length of self.d in line 63.
   
   

5, 6, 7 and 8:

	setTimeout(function() {
		let error = true;
		let temp = undefined;
		let relh = undefined;
		if (self.d.length === 42) { //Only if we got 42 bits
			// Two first bits are header. First bit is 0 on first, 1 on consecutive. Second bit is always 1.
			let h = self.d.substr(1,1);				 	// Header, always "1" - we'll include this in checksum test
			let rh = parseInt(self.d.substr(2,8),2);	// Relative humidity
			let rhf = parseInt(self.d.substr(10,8),2);  // Relative humidity fraction
			let t = parseInt(self.d.substr(18,8),2);	// Temperature
			let tf = parseInt(self.d.substr(26,8),2);   // Temperature fraction
			let csum = parseInt(self.d.substr(34,8),2); // Checksum
	
			// Calculate checksum
			let cks = rh + rhf + t + tf;
		
			// Check checksum:
			// - It is not zero (temp and humidity exactly zero?)
			// - Sum all data, compare last byte to checksum byte
			if (h == "1" && cks && ((cks & 0xFF) == csum)) {
				error = false;
				temp = parseFloat(t + "." + tf);
				relh = parseFloat(rh + "." + rhf);
			}
		}
		// Callback
		cb({ err: error, raw: self.d, temp: temp, rh: relh });
	}, 30);

I did't try the code not having a DHT11, watch out for typos and other errors. But I guess you'll get the idea and catch them.
Anyway, you write the module so do what you need. Others can adjust the code to their needs, as it is now much cleaner and easier to understand compared to the original module.

What I do not like about the rewrite is that as soon as the DHT11 instance is created, the setWatch() is activated and will collect "0" and "1" in the instance variable d... The collection of "0"s and "1"s should only be enabled within the expected window - after the request of data and within both the time and count as expected. Since multiple sensors can be on a single line, it creates another issue: all these DHT11s are concurrently active and listen, where as only one would have to...

The collection of "0"s and "1"s should only be enabled within the expected window

Why, what's the benefit?
For me setting the watch once is more effective, as it is only done once. The pin can't be used for anything else but a DHT11 sensor anyway.

Since multiple sensors can be on a single line

No. That is impossible. Or did I miss something?

You did not miss anything... I missed missed something: there is no address involved in any of the data grams. Therefore - as you state - only one sensor can be on the data line or pin.

Content of data - string variable d - is collecting data also between the read cycles. If the data line is stable - no noise - no data-runaway is to expect. I'm also aware of the fact that d is cleared just before the read cycle, so that it would be possible to de- and re-power the sensor between read phases to conserve energy, assuming read frequency is very low. Much lower than the max frequency defined by the period of power on, stabilize, allow measurement and collection, and power off.