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

DHT & optimizations #2748

Merged
merged 3 commits into from
Nov 18, 2019
Merged

DHT & optimizations #2748

merged 3 commits into from
Nov 18, 2019

Conversation

uzi18
Copy link
Contributor

@uzi18 uzi18 commented Nov 16, 2019

@@ -151,7 +151,7 @@ bool P005_do_plugin_read(struct EventStruct *event) {
digitalWrite(Plugin_005_DHT_Pin, LOW); // Pull low

switch (Par3) {
case P005_DHT11: delay(18); break; // minimum 18ms
case P005_DHT11: delay(19); break; // minimum 18ms
Copy link
Member

Choose a reason for hiding this comment

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

I always get a bit nervous by these kind of fixes.
Delay from 18 to 19 msec....
If this really is needed, then the timing is way too critical.

Copy link
Contributor Author

@uzi18 uzi18 Nov 16, 2019

Choose a reason for hiding this comment

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

we discussed this once, it should be 18 minimum, so 19 is not big deal, for dht22 minimum is 1ms and 2ms works ok,
actually it is hard to buy dht11

in fact in one wire protocols timings are critical

we can add delay(0) before turning of interrupts, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

we can add delay(0) before turning of interrupts, what do you think?

That sounds like a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TD-er about 1ms more, it is also better for longer wires

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TD-er bad idea with delay, because we have already delays just before

Copy link
Member

Choose a reason for hiding this comment

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

Well, it depends, is the delay from the switch statement just critical as minimum, or also for the maximum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TD-er first delay is start condition - request for data and it is minimum critical, every next should not be more than in documentation so critical for maximum.
we still don't know how accurate delays are in esp8266 from poor quality boards to nice ones.

@TD-er TD-er merged commit 8588353 into letscontrolit:mega Nov 18, 2019
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

3 participants