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

Problem with acquireAndWait(timeout) #11

Open
ScruffR opened this issue Dec 11, 2016 · 3 comments
Open

Problem with acquireAndWait(timeout) #11

ScruffR opened this issue Dec 11, 2016 · 3 comments

Comments

@ScruffR
Copy link

ScruffR commented Dec 11, 2016

The way the waiting is done in that function presents two problems.
If the wayiting time is rather long and the sensor slow to respond, the missing Particle.process() might cause the cloud connection to drop.
But the main issue is this part

int PietteTech_DHT::acquireAndWait(uint32_t timeout) {
    ...
    uint32_t start = millis();
    ...
    while(acquiring() && (timeout == 0 || (millis() > start && (millis()-start) < timeout)));
    ...
}

Since those two lines will be execeuted within the same millisecond in almost every case, the millis() > start will almost always be false because they will be equal causing to drop out of the wait immediately before the acutal measurement will be finished.

But in fact that condition is superfluous all together - as are other wrap round "precautions" - as unsigned integer calculation will be tolerant to such conditions.

So a (for me) working implementation of that function would look like this

int PietteTech_DHT::acquireAndWait(uint32_t timeout) {
    acquire();
    uint32_t start = millis();
    uint32_t wrapper;
    while(acquiring() && (timeout == 0 || ((millis()-start) < timeout))) Particle.process();
    if (acquiring())  
    { // if we are still acquiring, we've had a timeout
// ---- most likely not needed, but haven't thought into it too deeply ;-) ----
        if (millis() < start) // millis counter wrapped
        {
            wrapper = (~start)+1;   // Compute elapsed seconds between "start" and counter-wrap to 0
            timeout -= wrapper;     // Subtract elapsed seconds to 0 from timeout
        }
// ----------------------------------------------------------------------------
        _status = DHTLIB_ERROR_RESPONSE_TIMEOUT;
    }
    return getStatus();
}
@brettski
Copy link

brettski commented Jul 7, 2017

Removing millis() > start && corrected my issues using a timeout parameter in the acquireAndWait function as well. When using any timeout value an initial read error of not started (DHTLIB_ERROR_NOTSTARTED) would be thrown and no subsequent reads return a value (though no errors).

The if (acquiring()) section changed by @ScruffR makes sense as well, plus the status DHTLIB_ERROR_RESPONSE_TIMEOUT is getting set which isn't being done in the current release version.

@ScruffR
Copy link
Author

ScruffR commented Jul 8, 2017

@brettski, which release version do yo refer to?
Since this repo seems to have gone stale (no issue reports addressed for a long time), I took the liberty to get the "Particle ownership" to the library transferred to the Particle Elite repo.
https://build.particle.io/libs/PietteTech_DHT/0.0.5

Suggestions, IRs and PRs are always welcome there
https://github.com/eliteio/PietteTech_DHT

@brettski
Copy link

brettski commented Jul 8, 2017

@ScruffR I am using the one from this repository, I wasn't aware of the version you linked. I may add that one into my project and retest. I actually didn't know about those libraries. Thanks for the information!

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

No branches or pull requests

2 participants