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

DHT11 sampling fix (tested on ESP8266 and Mega2560 r3) #22

Merged
merged 9 commits into from
Aug 18, 2018

Conversation

t-w
Copy link
Contributor

@t-w t-w commented Jul 13, 2018

Changed 2 things for DHT11 :

  • Timing in sample() and the way of measuring the time of expected pin state (level).
    Note that this is very sensitive, different MCUs might need adjustments (as in the commit ca06c42). Eventually, if necessary, the MCU-specific thresholds could be stored. Also, any change in levelTime(), levelTimePrecise() might imply adjustments in time thresholds as with time measurements of this precision (microseconds) CPU time apparently matters...
    (for DHT22 timing/sampling is not changed and not tested!)

  • Minor improvement in the class interface (with preserving the old way, so that all legacy code, examples etc. should work with no changes). This is also done for DHT22.

As you see, I was not using the confirm(), which actually gives shorter code - but to make measurements when something fails, it was easier to have methods returning the measured time (levelTime()).

After these changes, the readings for the 2 mentioned boards are very stable with no errors (for me at least...).

t-w added 9 commits July 5, 2018 00:04
Added method for more precise time measurement of the level states.
Simplified interface dropping 'pin' from most methods (keeping overloaded methods for interface compatibility).


Adjusting pin level times for DHT11, needed after the fix above (-> need to adjust times after any change in the time measurement code...), #3
@winlinvip
Copy link
Owner

Why change the pin to the member? It changes the API of library. We should keep API compatible.

@winlinvip
Copy link
Owner

Sorry, it's compatible API, I got it.
I only need to test on Arduino UNO R3, then I will merge it.
Thanks for your good work.

@t-w
Copy link
Contributor Author

t-w commented Aug 17, 2018

Initially, I haven't actually been thinking that it will be ever used by someone other than me... and I decided to simplify the interface. I added the backward compatibility calls to the interface, so it should be fine for all the existing code. I was aware that it can cause doubts, but also I think the updated library is even simpler to use and more clear (might be good for beginners that use it to play).

While testing, note that the timing is very delicate here. It may be necessary to measure and adjust the thresholds if it fails on UNO R3 (and test it again on other devices). Eventually, if necessary, the thresholds could be set different depending on the device (I tried to avoid this). It is not by the book at all - but it seems the sensor just works like this...

@winlinvip
Copy link
Owner

Verified on Arduino UNO R3. Merged to master.

winlinvip added a commit that referenced this pull request Aug 18, 2018
winlinvip added a commit that referenced this pull request Aug 18, 2018
winlinvip added a commit that referenced this pull request Aug 18, 2018
winlinvip added a commit that referenced this pull request Aug 18, 2018
winlinvip added a commit that referenced this pull request Aug 18, 2018
@winlinvip
Copy link
Owner

@t-w I refined the code, please test 1.0.9 on ESP8266 and Mega2560 r3 again.

@t-w
Copy link
Contributor Author

t-w commented Aug 30, 2018

Tested on ESP8266 and Mega2560r3 (1.0.11) - works fine! (Thanks for the merge!)

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

Successfully merging this pull request may close these issues.

None yet

2 participants