Potential divide by zero in DallasTemperature::calculateTemperature()

[vortex64]

if scratchPad[COUNT_PER_C] is ever zero a divide by zero will occur. Saw this happen on a ESP32. I just put a check on scratchPad[COUNT_PER_C] and set it to 1 if it is ever zero.

if (deviceAddress[0] == DS18S20MODEL) { fpTemperature = ((fpTemperature & 0xfff0) << 3) - 32

• (((scratchPad[COUNT_PER_C] - scratchPad[COUNT_REMAIN]) << 7) / scratchPad[COUNT_PER_C]); }

[RobTillaart]

scratchPad[COUNT_PER_C] is never zero.
See https://datasheets.maximintegrated.com/en/ds/DS18S20.pdf page 6

Resolutions greater than 9 bits can be calculated using the data from the temperature, COUNT REMAIN and COUNT PER °C registers in the scratchpad. Note that the COUNT PER °C register is hard-wired to 16 (10h).

I do not know how a hard coded value could become zero. Can you post a minimal example that reproduces the problem?

It could be caused by hardware as 1000 is read as 0000. This would normally give some CRC error. Two questions:

• What cable length do you use?
• What pull up resistors do you use?

But given the above info from the datasheet, the value should better be hard coded in the library as 16 which would allow the compiler to optimize the division with a << 4.

[vortex64]

Probably hardware issue then.  My apologies.  All I know is the esp32 gave a divide by zero error on the serial port and the only division I had in the code was the code in question. On Monday, January 27, 2020 Rob Tillaart reply@reply.github.com wrote:

scratchPad[COUNT_PER_C] is never zero. See https://datasheets.maximintegrated.com/en/ds/DS18S20.pdf page 6

Resolutions greater than 9 bits can be calculated using the data from the temperature, COUNT REMAIN and COUNT PER °C registers in the scratchpad. Note that the COUNT PER °C register is hard-wired to 16 (10h).

I do not know how a hard coded value could become zero. Can you post a minimal example that reproduces the problem?

It could be caused by hardware as 1000 is read as 0000. This would normally give some CRC error. Two questions:

• What cable length do you use?
• What pull up resistors do you use?

But given the above info from the datasheet, the value should better be hard coded in the library as 16 which would allow the compiler to optimize the division with a << 4.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[vortex64]

Forgot to answer your questions.  30 ft, at 3.3v with a 2k pull up.  Which may explain the possible error.  I will turn on crc.  Otherwise I am getting good temperature readings. I only saw the divide by zero error once.  On Monday, January 27, 2020 Rob Tillaart reply@reply.github.com wrote:

scratchPad[COUNT_PER_C] is never zero. See https://datasheets.maximintegrated.com/en/ds/DS18S20.pdf page 6

Resolutions greater than 9 bits can be calculated using the data from the temperature, COUNT REMAIN and COUNT PER °C registers in the scratchpad. Note that the COUNT PER °C register is hard-wired to 16 (10h).

I do not know how a hard coded value could become zero. Can you post a minimal example that reproduces the problem?

It could be caused by hardware as 1000 is read as 0000. This would normally give some CRC error. Two questions:

• What cable length do you use?
• What pull up resistors do you use?

But given the above info from the datasheet, the value should better be hard coded in the library as 16 which would allow the compiler to optimize the division with a << 4.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[RobTillaart]

@vortex64 I have read - http://midondesign.com/Documents/DS1820.PDF - page 4

The DS1820 has a COUNT PER C register that is temperature dependent, where DS18S20 has a fixed value. The DS1820 datasheet does not mention its range or possible values. So theoretically it could be zero. As the model ID (0x10) for the DS1820 is identical to the DS18S20 an extra if statement would prevent a divide by zero.

    if (deviceAddress[0] == DS18S20MODEL) {
        fpTemperature = ((fpTemperature & 0xfff0) << 3) - 32
            + (((scratchPad[COUNT_PER_C] - scratchPad[COUNT_REMAIN]) << 7)
                        / scratchPad[COUNT_PER_C]);
        }
    }

would become

    if ((deviceAddress[0] == DS18S20MODEL) && (scratchPad[COUNT_PER_C] != 0) ) {
        fpTemperature = ((fpTemperature & 0xfff0) << 3) - 32
            + (((scratchPad[COUNT_PER_C] - scratchPad[COUNT_REMAIN]) << 7)
                        / scratchPad[COUNT_PER_C]);
        }
    }

I'll make a pull request.

[vortex64]

FWIW, I was using the older ds1820.  Basically I was interfacing and old Dallas one wire weather station to an ESP32.  This is not the best situation since the ESP32 in 3.3v.  I am using a 2k pull-up. I would also mention that my early code was not utilizing the CRC.  My newer code now does.

In any case I would not want a hardware issue to cause a div by zero in the code.  Just seems to me to err on the side of caution when doing so.  I've been in embedded a long time and cant count how many times I have been bitten by div by zero. 

I now automatically put a divisor check when dividing. On Monday, February 10, 2020 Rob Tillaart reply@reply.github.com wrote:

@vortex64 I have read - http://midondesign.com/Documents/DS1820.PDF - page 4

The DS1820 has a COUNT PER C register that is temperature dependent, where DS18S20 has a fixed value. The DS1820 datasheet does not mention its range or possible values. So theoretically it could be zero. As the model ID (0x10) for the DS1820 is identical to the DS18S20 an extra if statement would prevent a divide by zero. if (deviceAddress[0] == DS18S20MODEL) { fpTemperature = ((fpTemperature & 0xfff0) << 3) - 32

• (((scratchPad[COUNT_PER_C] - scratchPad[COUNT_REMAIN]) << 7) / scratchPad[COUNT_PER_C]); } }

would become if ((deviceAddress[0] == DS18S20MODEL) && (scratchPad[COUNT_PER_C] != 0) ) { fpTemperature = ((fpTemperature & 0xfff0) << 3) - 32

• (((scratchPad[COUNT_PER_C] - scratchPad[COUNT_REMAIN]) << 7) / scratchPad[COUNT_PER_C]); } }

I'll make a pull request.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[RobTillaart]

Thanks for confirming it was an DS1820, so the code fix now prevents this div by zero.