RobTillaart / DHTNew

Arduino library for DHT11 and DHT22 with automatic sensor recognition
MIT License
96 stars 15 forks source link

ESP32 error #76

Closed willy2764 closed 2 years ago

willy2764 commented 2 years ago

Dear Rob, first of all thank you very much for this wonderfull and working library! I was just testing your library on a ESP32 and from time to time a got the error -4 DHTLIB_ERROR_SENSOR_NOT_READY I checked all the signal with an oscilloscope and I couldn't find any faults on the signal. After searching a while on the web I found that the instruction " noInterrupts();" would probably not be implemented on an ESP32. So I gave it a try and inserted additionally the following command:

#if defined(ESP32)  
  if (_disableIRQ) { portDISABLE_INTERRUPTS();}
#endif

and later to allow again interrupts:

#if defined(ESP32)
  portENABLE_INTERRUPTS();
#endif

Since then I didn't get any more errors. I hope this helps somehow.

RobTillaart commented 2 years ago

Thanks for reporting this issue. The solution you propose sounds interesting and I need to investigate. Might take a few days as I am quite busy.

RobTillaart commented 2 years ago

A first look in the ESP32 package (1.0.6) shows in Arduino.h:

#define sei()
#define cli()
#define interrupts() sei()
#define noInterrupts() cli()

so it looks like the noInterrupts() is doing nothing on ESP32, so it will not help.

I will make a develop branch to fix the issue.

RobTillaart commented 2 years ago

Develop branch created - https://github.com/RobTillaart/DHTNew/tree/develop PR is building.

@willy2764 Please give it a try if you have time

willy2764 commented 2 years ago

@RobTillaart Just trying it. Interesting, your modification also gives errors from time to time. If I change the code as follows, leaving the "noInterrupts()" command in, I don't get anymore errors

#if defined(ESP32)  
    portDISABLE_INTERRUPTS();
#endif
    noInterrupts();

respectively

#if defined(ESP32)
  portENABLE_INTERRUPTS();
#endif
  interrupts();

So, seems that both commands are needed for the ESP32.

RobTillaart commented 2 years ago

interesting result, as noInterrupts() is defined as an empty statemen,

test compiling minimal sketch for UNO with and without noInterrupts() differs 4 bytes. For ESP32 with and without noInterrupts() differs 0 bytes.

If my test (and theory) is correct you should get also the same file size with your sketch, Can you check that?

I do not understand why inserting noInterrupts() (nothing) in your proposal solves the problem.

How often (approx) does the failure pops up?
1 in 10, 1 in 100, 1 in 1000?

RobTillaart commented 2 years ago

Q: which DHT sensor do you use?

RobTillaart commented 2 years ago

Note: I updated the develop branch code to your proposal but still do not understand why it should work.

willy2764 commented 2 years ago

Hmmm. you are right concerning the file size. With or without noInterrupts() it is the same. I use two DHT22. I am using also ESP32 package (1.0.6). Just running new tests. They take a while. An error appearance is easy to see. A non appearance takes more time to be sure. I am running the reading loop every 5 seconds. The error affects both sensors and appear randomly. Actual test is running with the original library: Always error -4 after an average of about 70 readings in a test of about 700 readings. Distributed about 50/50 between the sensors. Now running test with your first version and afterwards I'll do the test with the actual development version and I will comment...

willy2764 commented 2 years ago

Now since about 2h running the test. No error. Probably I mixed something up on the last trials. So, your first developement version excluding the noInterrupts() command for the ESP32 seems fine.

RobTillaart commented 2 years ago

Ok, will check code again later. Please let me know if you find a cause.

willy2764 commented 2 years ago

The root cause. That's easy. Somewhere between my ears..... :-)

RobTillaart commented 2 years ago

I reverted to the first version, build is running, will merge tomorrow unless new problems pop up.

RobTillaart commented 2 years ago

Merged into 0.4.13