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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/_C013.ino
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ void C013_SendUDPTaskData(byte destUnit, byte sourceTaskIndex, byte destTaskInde
const userVarIndex_t userVarIndex = dataReply.sourceTaskIndex * VARS_PER_TASK + x;

if (validUserVarIndex(userVarIndex)) {
dataReply.Values[x] = UserVar[dataReply.sourceTaskIndex * VARS_PER_TASK + x];
dataReply.Values[x] = UserVar[userVarIndex];
}
}

Expand Down
17 changes: 7 additions & 10 deletions src/_P005_DHT.ino
Original file line number Diff line number Diff line change
Expand Up @@ -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.

case P005_DHT22: delay(2); break; // minimum 1ms
case P005_DHT12: delay(200); break; // minimum 200ms
case P005_AM2301: delayMicroseconds(900); break;
Expand All @@ -172,15 +172,12 @@ bool P005_do_plugin_read(struct EventStruct *event) {
delayMicroseconds(20);
break;
}

if(!P005_waitState(0)) {P005_log(event, P005_error_no_reading); return false; }
if(!P005_waitState(1)) {P005_log(event, P005_error_no_reading); return false; }

noInterrupts();
if(!P005_waitState(0)) {
interrupts();
P005_log(event, P005_error_no_reading);
return false;
}
if(!P005_waitState(0)) {interrupts(); P005_log(event, P005_error_no_reading); return false; }
if(!P005_waitState(1)) {interrupts(); P005_log(event, P005_error_no_reading); return false; }
if(!P005_waitState(0)) {interrupts(); P005_log(event, P005_error_no_reading); return false; }

bool readingAborted = false;
byte dht_dat[5];
for (i = 0; i < 5 && !readingAborted; i++)
Expand All @@ -196,7 +193,7 @@ bool P005_do_plugin_read(struct EventStruct *event) {
if (readingAborted)
return false;

// Checksum calculation is a Rollover Checksum by design!
// Checksum calculation is a Rollover Checksum by design!
byte dht_check_sum = (dht_dat[0] + dht_dat[1] + dht_dat[2] + dht_dat[3]) & 0xFF; // check check_sum
if (dht_dat[4] != dht_check_sum)
{
Expand Down