Skip to content

Improve error handling #31

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

Merged
merged 3 commits into from
Apr 4, 2020
Merged

Conversation

michaellass
Copy link

adafruit/Adafruit_Blinka#210 demonstrated erroneous readings that can easily be avoided:

  1. If an error is detected in the data received from the sensor, temperature and humidity readings should not be updated. Otherwise they will actually be returned to the user if they query data within two seconds after the error occured.
  2. If we receive a humidity value below 0% or above 100% this is clearly bogus data.

@ladyada
Copy link
Member

ladyada commented Feb 2, 2020

hmm! nice idea - would you mind pylinting? you probably just need to add a '# pylint: disable' line

@michaellass
Copy link
Author

We can actually reduce the number of branches by checking for any missing bits first. Then the main decoding logic does not need to be wrapped by an if len(pulses) >= 80. Pylint seems to be happy now.

@ladyada
Copy link
Member

ladyada commented Feb 2, 2020

ok and ya tested on a pi? which ones? we'll test too :)

@michaellass
Copy link
Author

michaellass commented Feb 2, 2020

It's running here (in combination with adafruit/libgpiod_pulsein#1) on a Raspberry Pi 4B and taking one measurement each minute. Looks good so far. Of course the most recent version now only runs for an hour or so but I will continue observing it over the next days.

@michaellass
Copy link
Author

Just a quick follow-up: The most recent version has been running here for more than three days now, taking over 4000 successful measurements without returning any garbage. Of course it can still return garbage if just the wrong two bits flip in the communication. There's nothing we can do about that.

@ladyada
Copy link
Member

ladyada commented Feb 5, 2020

thanks, we're a little swamped, but not forgotten!

@heroldus
Copy link

@ladyada Could you please merge the fix.

@FoamyGuy
Copy link

I tested this with a Trinket M0 and a DHT11. Ran for about an hour taking readings every 2 seconds. I did see the bogus data error a few times. But I think it was my fault in those cases the sensor was not connected properly yet. Since getting connected correctly it's run successfully and been reporting temperature and humidity.

It does need to have some merge conflicts resolved now though. @michaellass if you are able to you can make a new commit here to resolve them.

If not, no worries I will try to work on getting it resolved later this week.

Currently, we update self._temperature and self._humidity, even if it
turns out that the data returned by the sensor was bogus. If the user
queries the data within two seconds after an error, they will actually
get wrong data.

Fix this by updating _temperature and _humidity attributes only if no
error was detected.
@michaellass
Copy link
Author

Thanks for testing! I just rebased the changes onto master.

@FoamyGuy
Copy link

FoamyGuy commented Mar 31, 2020

Thank you michaellass this contribution and for rebasing.

Following up on testing, my Trinket M0 + DHT11 continued to run all night over night with no serious issues.

It looks like this was originally made to resolve an issue when running under Pi / Blinka. I will try to get it tested that way later this week.

Copy link

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested successfully with Raspberry Pi 3B. This does properly filter out erroneous data reads from the sensor.

@FoamyGuy FoamyGuy merged commit 92a195b into adafruit:master Apr 4, 2020
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Apr 5, 2020
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.

4 participants