Gbertaz / NonBlockingDallas

Arduino library for the DS18B20 temperature sensor. Avoid blocking the sketch while reading the sensor.
MIT License
11 stars 4 forks source link

update() function stopped working #6

Closed K-IF closed 1 year ago

K-IF commented 1 year ago

Hello there !

I've configured time interval parameter in begin() function to 1500 ms and I've noticed that every time my sensor after some time stopped responding.

I did some investigation with debug messages and I've discovered that this line has to be changed, otherwise the state machine of yours inside update() function never escapes state "waitingNextReading".

I've initialized your wrapper like this:

// DS18B20
//Initialize the sensor passing the resolution, unit of measure and reading interval [milliseconds]
sensorDs18b20.begin(NonBlockingDallas::resolution_12, NonBlockingDallas::unit_C, TIME_INTERVAL);
//Callbacks
sensorDs18b20.onIntervalElapsed(handleIntervalElapsed);
sensorDs18b20.onTemperatureChange(handleTemperatureChange);

And I'm using sensorDs18b20.update(); function inside in my main.

I believe that this check is not correct:

if(_lastReadingMillis != 0 && (millis() - _lastReadingMillis < _tempInterval - _conversionMillis)) return;

Correct me if I'm wrong but at some point I end up having this message at the debug (I've added some custom Serial.prints):

_lastReadingMillis= 2315064 _conversionMillis= 20024 _tempInterval - _conversionMillis= 4294948772 if case: (_lastReadingMillis != 0 && (millis() - _lastReadingMillis < _tempInterval - _conversionMillis))= 1 if case: (_lastReadingMillis != 0)= 1 if case: (boolean)(millis() - _lastReadingMillis < _tempInterval - _conversionMillis)= 1

sensor state 1

This led me to think that you should simply check if time interval has passed and not to check all the above sections. I found no guard that guaranties that _conversionMillis wont get higher value of _tempInterval. Probably there is some blocking code in my program that causes the problem. I can't imagine how else this could end up wrong.

In order to fix this I've changed your if case to this:

if(millis() - _lastReadingMillis < _tempInterval) return;
_lastReadingMillis= millis();
requestTemperature();

And I've commended out _lastReadingMillis = millis(); inside function readSensors()

I will try this tonight and see tomorrow if the error persists.

Kind regards, kif

Gbertaz commented 1 year ago

Hello @K-IF

Removing this condition _lastReadingMillis != 0 you will have to wait for time interval to elapse before getting the very first reading of the sensor. It is not convenient especially in case interval is very high (i.e. 30 minutes or more)

And I've commended out _lastReadingMillis = millis(); inside function readSensors()

How are you supposed to save the time then? The reading of the sensor will be always triggered regardless of time interval.

The value of _conversionMillis that I can see from your debug is too high, it shouldn't be more than 750ms. I have the feeling that there is some other issue in your sketch. I am not sure how you are using the library but I would suggest not to add too much code in the body of the callbacks to avoid blocking the execution.

K-IF commented 1 year ago

Good morning @Gbertaz . I'm sorry for the late response.

Regarding the update of _lastReadingMillis variable, I'm reading the value of millis right after passing the if statement at waitNextReading function. Library's waitNextReading function:

void NonBlockingDallas::waitNextReading(){
    if(_lastReadingMillis != 0 && (millis() - _lastReadingMillis < _tempInterval - _conversionMillis)) return;
    requestTemperature();
}

The change that I've done:

void NonBlockingDallas::waitNextReading(){
    if( (_lastReadingMillis != 0) && ((millis() - _lastReadingMillis) < _tempInterval) ) 
        {  
           return;
        }
    _lastReadingMillis= millis();
    requestTemperature();
}

I agree on using the (_lastReadingMillis != 0) condition. I didn't think of the first reading issue for longer time intervals. Usually I'm using an init flag for such things.

Regarding the second part of your answer I believe that either the wifi handler or the MQTT connection handler is stalling the program that ends up the increase of _conversionMillis variable. In my case I just removed conversionMillis from the conditional statement and I have no more issue there. If you think it is an action that occurs every X time interval + almost a fixed conversionMillis delay.

Thank you for your time and for the great wrapper that you've developed !

Gbertaz commented 1 year ago

Glad to know you solved the problem @K-IF. I will consider adding a fix to avoid the same problem in future. Thank you for using the library.