DS18S20 has wrong resolution

[broddo]

This might only be a small thing, but I noticed that the function getResolution() returns a value of 12 bits if a DS18S20 is selected. However, according to the datasheet the DS18S20 is a 9-bit device. This might have an impact on the temperature calculation.

https://github.com/milesburton/Arduino-Temperature-Control-Library/blob/57a88b9dbbbdac10667904069697449c89363c98/DallasTemperature.cpp#L347

[RobTillaart]

Checked the datasheet and indeed the resolution of the sensor is 9 bits.

Quick check of places where DS18S20MODEL is used

• getResolution -- returns 12 (that is the issue)
• setResolution not affected
• writeScratchPad not affected
• validFamily not affected
• calculateTemperature is not affected

call trace requestTemperaturesByAddress calls GetResolution uses it for calling blockTillConversionComplete(bitResolution); which calls millisToWaitForConversion(bitResolution); ==> it waits for 750 (12 bit) instead of 94 (9 bit) milliseconds

From Datasheet DS18S20,- page 3 - table AC Electrical Characteristics Temperature Conversion Time Tconv (Note 12) 750 ms (note 12 is just a timing diagram)

So the time it waits is correct because of incorrect resolution

Analysis in short The returned resolution is indeed wrong, so good catch, The good news is that the math (as far as analyzed) is correct due to the wrong resolution.

The underlying error is that the bitResolution is mapped directly on the conversion time and while that mapping is correct for the DS18B20, this mapping is incorrect for the DS18S20.

*Fixing the problem ? There are a number of scenario's to investigate:

1. not fixing it and describe the incorrect resolution problem in the documentation,
2. fix getResolution and add a model check wherever needed to fix what breaks.
3. add a new variable conversionTime that goes through all the code, often side by side with bitResolution Quite some work, but it would be the correct
4. stop support for the DS18S20 in this library and give it its own derived class.
5. ....

---

update documentation Should be done, as this gives time for proper analysis and redesign

---

Adding model info where needed Fixing getResolution() is simple, however

The lowest level function millisToWaitForConversion(bitResolution); is only correct for DS18B20. To make it correct for the DS18S20 too it should have an extra parameter model or so.

millisToWaitForConversion(bitResolution); is a public function so changing it is at least an interface break. Within the library millisToWaitForConversion is called just on one place, from blockTillConversionComplete(bitResolution); That function does not have the address info either. Also a public function so that would break the interface too

blockTillConversionComplete is called in two places, by requestTemperaturesByAddress, we have the address so here it could be fixed. Unfortunately blockTillConversionComplete is also called by requestTemperatures which explicitly is made to be called without address. Dead end?

So the path of adding the model to solve this issue, seems to be a dead end as requestTemperatures has no address info.

So should there be two signatures for millisToWaitForConversion(bitResolution);?

This would make the solution more complex I expect, imho bigger than the problem.

---

Add conversionTime Quite some work, did not investigate but it would be the best cleanest solution as that is the information needed and used. The use of bitResolution as carrier of the conversionTime worked well, but apparently not 100%

Think this is the way to go.

---

Split off DS18S20 This is also a major action and it would create problems from usage point of view when the user has one bus with a mix of sensors, to name the most obvious one.

IMHO not an option.

My conclusion For now update the documentation and in time start redesign of how to implement proper conversionTime.

[gitkomodo]

Yes, the temperature register of the DS18S20 is 9-bit. BUT the 'detail' of the temperatures measured by the DS18S20 is actually equivalent to the DS18B20 at 12-bit resolution, also reflected in the conversion time. So in practice it is functioning as a 12-bit device. Because of this imho no change is needed.

[RobTillaart]

Because of this imho no change is needed. it should at least be documented, so people know.

[gitkomodo]

it should at least be documented, so people know.

Yes. In that documentation I wouldn't phrase it as an 'incorrect resolution' but explain that 12-bit is returned because in both temperature detail and conversion time the DS18S20 behaves equivalent to other family members set to 12-bit.

[broddo]

Indeed - just highlighting it would help as it would prevent people like myself raising another issue after spotting a discrepancy!