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

Equality comparison with Float ! #9

Closed mr-miky closed 5 months ago

mr-miky commented 1 year ago

`void NonBlockingDallas::readTemperatures(int deviceIndex){

bool validReadout = (temp != 85.0 && temp != (-127.0));

}`

You cannot do this type of comparison with floats because, due to the approximations, you have a very high probability of having a result that is always true.

You must use (temp <= 84.95 && temp >= -126.95)

Gbertaz commented 1 year ago

Hello @mr-miky,

Those are the values returned by DallasTemperature library in case of error. Why should there be an approximation?

Please check the implementation of getTempC function here and declaration of DEVICE_DISCONNECTED_C here.

mr-miky commented 1 year ago

Because using equal ( == ) and not equal ( != ) with float and double can give wrong results because of the approximations.

A barely decent C/C++ IDE immediately highlights that you are using == and != with non-integer numbers...

https://www.geeksforgeeks.org/problem-in-comparing-floating-point-numbers-and-how-to-compare-them-correctly/ https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/

mr-miky commented 1 year ago
void NonBlockingDallas::readTemperatures(int deviceIndex){
    int16_t itemp = _dallasTemp->getTemp(_sensorAddresses[deviceIndex]);
    float temp = 0;
    switch(_unitsOM){
        case unit_C:
            temp = _dallasTemp->getTempC(_sensorAddresses[deviceIndex]);
        break;
        case unit_F:
            temp = _dallasTemp->getTempF(_sensorAddresses[deviceIndex]);
        break;
    }
    bool validReadout = (itemp != DEVICE_DISCONNECTED_RAW);
    if(cb_onIntervalElapsed)(*cb_onIntervalElapsed)(temp, validReadout, deviceIndex);

    if(itemp != _temperatures[deviceIndex]){
        _temperatures[deviceIndex] = itemp;
        if(cb_onTemperatureChange)(*cb_onTemperatureChange)(temp, validReadout, deviceIndex);
    }
}

obviously _temperatures[deviceIndex] must be declared int16_t

Gbertaz commented 1 year ago

Thanks for your suggestions @mr-miky.

I introduced a new callback in case of device disconnected, I think it doesn't make sense to return the temperature when it's not valid. Also, I added the conversion to the unit of measure in place to avoid reading the bus two times, and the calculation happens only if there is a valid reading.