Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use micros() instead of cycle counting. #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rtwfroody
Copy link

In the ESP32 Arduino library, noInterrupts() is a nop, so interrupts
still occur while receiving a value. On my board there is at least one
interrupt every time we try to read a value, which messes up the
reading. Instead of counting cycles, use micros() to keep track of time.
This increases the success rate of receiving data to 90% or so.

A better fix would be to implement noInterrupts() for the ESP32, but
that looks a lot harder. The new code path is probably not suitable for
slower micros, but hopefully those use the __AVR code path anyway.

Thank you for creating a pull request to contribute to Adafruit's GitHub code!
Before you open the request please review the following guidelines and tips to
help it be more easily integrated:

I'm not sure this should be merged as-is, but I want it to be available to people who are running into the same problem.

In the ESP32 Arduino library, noInterrupts() is a nop, so interrupts
still occur while receiving a value. On my board there is at least one
interrupt every time we try to read a value, which messes up the
reading. Instead of counting cycles, use micros() to keep track of time.
This increases the success rate of receiving data to 90% or so.

A better fix would be to implement noInterrupts() for the ESP32, but
that looks a lot harder. The new code path is probably not suitable for
slower micros, but hopefully those use the __AVR code path anyway.
@khjoen
Copy link

khjoen commented Nov 13, 2017 via email

@rtwfroody
Copy link
Author

@khjoen, that's good to know! So this code must definitely not be merged. It's working for me because interrupts aren't actually being disabled, but that is really the bug that should be fixed.

@phylor
Copy link

phylor commented Jul 25, 2018

I can confirm the same issues using an ESP-WROOM-32. The changes in this branch solved them.

Issues with unmodified code:

I connected the DHT22 to pin 15 of the ESP32 and its 3.3V pin, using a 10k pullup resistor between data and 3.3V. I'm using the Arduino IDE to upload. My sketch consists of the basic DHT example.

The data is read fine from the sensor. The checksum is consistently read incorrectly though. Example output of received data:

1, 13, 1, 21, B6 =? 36

In all of these readouts, the checksum read from the sensor (e.g. B6) consistently differs from the calculated checksum (36) in the first bit. It seems that a 1 is read, although a 0 is expected. This is a consistent behavior across all readings.

How I solved it:

  • As my issues were only with the checksum, removing the check for the correct checksum got me a legit output of temperature and humidity.
  • Applying the changes in this branch also fixed the issue, as the read checksum conformed with the calculated one.

I guess that although the changes cannot be merged as is, it would be great if these could be supplied as an option to the constructor.

@cr1st1p
Copy link

cr1st1p commented Sep 1, 2019

Not sure if it helps, but you can take a look at https://github.com/beegee-tokyo/DHTesp/blob/master/DHTesp.cpp and see how it does the 'stop task switching' on esp32.
I did not play with esp32 yet so I can't say if that part looks ok or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants