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

Add timeouts in read functions for reliability #2

Merged
merged 5 commits into from
Feb 15, 2021

Conversation

azerupi
Copy link
Contributor

@azerupi azerupi commented Feb 15, 2021

Hi!
As mentioned in #1, I have implemented a simple timeout strategy for the read functions. This should remove any possibility to end up in an infinite loop.

After some analysis of a bluepill with a DHT22 I have adapted the timeout values to be a bit tighter towards the measured values.

As can be seen in the pictures below the whole communication (after releasing the pin) takes less than 5ms and the individual state changes take between 26 and 78µs. Therefore I set the timeouts to 100µs. This hasn't given me any problems, but it might be worth to test it with a couple of other sensors and especially the DHT11 (I can't test because I don't have that variant).

I have also added the example for the bluepill that I used in the doc, however since the setup of crates, configuration, memory layout is different I marked it as ignore so that it does not throw any errors. I'm not sure how we can have examples for multiple boards that compile.

Let me know if I can improve something

Fixes #1

DS1Z_QuickPrint3
DS1Z_QuickPrint2
DS1Z_QuickPrint1

Timeouts avoid getting stuck in infinite loops if the sensor
does not respond as expected. Now if the sensor does not respond
we return a DhtError::Timeout and the user can choose how to
handle it.
Copy link
Owner

@michaelbeaumont michaelbeaumont left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for the PR! Which device are you using by the way?

src/read.rs Outdated Show resolved Hide resolved
src/read.rs Outdated Show resolved Hide resolved
src/read.rs Outdated Show resolved Hide resolved
src/read.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated
Comment on lines 67 to 74
//! ## Notes
//!
//! On the bluepill (STM32F103) the pin seems to be pulled low by default causing the sensor
//! to be confused when actually reading it for the first time. The same thing happens when
//! the sensor is polled too quickly in succession. In this case you will get a `DhtError::Timeout`
//!
//! To avoid this, you can pull the pin high when initializing it and polling the sensor with an
//! interval of at least 500ms.
Copy link
Owner

Choose a reason for hiding this comment

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

I think you can add this before the above ## Example heading, worded in a non chip specific way

src/lib.rs Outdated
//! }
//!
//! // Delay of at least 500ms before polling the sensor again, 1 second or more advised
//! delay.delay_ms(500_u16);
Copy link
Owner

Choose a reason for hiding this comment

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

You can also add this loop reading to both examples and call out the minimum delay there.

src/lib.rs Outdated
//! let mut dht = gpiob.pb15.into_open_drain_output(&mut gpiob.crh);
//!
//! // Pulling the pin high to avoid confusing the sensor when initializing
//! dht.set_high().ok();
Copy link
Owner

Choose a reason for hiding this comment

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

I think you can update the above, existing example and /examples/stm32f042.rs with a set_high().unwrap()

@azerupi
Copy link
Contributor Author

azerupi commented Feb 15, 2021

Awesome! Thanks for the PR! Which device are you using by the way?

My pleasure, thanks for the crate 😉
For the device, I assume you mean the oscilloscope? It's a Rigol DS1054Z, it's a nice toy for debugging electronic circuits like this.

I switched the polarity of the wait function, I think it reads better (nothing to do with this PR really), WDYT?

Yes, I especially like the for loop instead of the wile loop. 👍

I'll update the documentation with the suggested changes.

@michaelbeaumont
Copy link
Owner

For the device, I assume you mean the oscilloscope? It's a Rigol DS1054Z, it's a nice toy for debugging electronic circuits like this.

Yeah, good to know, I'll pick one up as my first scope once I can justify buying it to myself 😅
Thanks a lot for the PR!

@michaelbeaumont michaelbeaumont merged commit 28927cd into michaelbeaumont:master Feb 15, 2021
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.

bluepill example
2 participants