DS1820 driver returns undocumented error values instead of raising an exception

andreasdotorg commented 7 years ago

Reading the temperature value from a DS1820 sensor can return one of the values -9997.0, -9998.0 or -9999.0, if something goes wrong during onewire communication. Instead, an exception should be raised.

jolivepetrus commented 7 years ago

Hi @andreas23, thanks for report this.

Can you tell us the steps you follow for reproduce the situation?

andreasdotorg commented 7 years ago

Mhh... it happens if there are read errors on the bus, while the device is working otherwise. I can't reproduce it except by waiting for a few hours for a random error, and the one I got was -9998.0.

It's pretty obvious from the code though where exactly this happens, see:

https://github.com/whitecatboard/Lua-RTOS-ESP32/blob/b01f76a0e7c10e649e4d554a0593efb8cac2cd18/components/lua_rtos/sensors/ds1820.c#L708 https://github.com/whitecatboard/Lua-RTOS-ESP32/blob/b01f76a0e7c10e649e4d554a0593efb8cac2cd18/components/lua_rtos/sensors/ds1820.c#L727 https://github.com/whitecatboard/Lua-RTOS-ESP32/blob/b01f76a0e7c10e649e4d554a0593efb8cac2cd18/components/lua_rtos/sensors/ds1820.c#L739

aheik commented 6 years ago

Hi,

we have any of the DS1820 onewire sensors and all of them returns 85.0. The sensor are connected as parasite power.

/ > sensor.enumerate(sensor.OWire, pio.GPIO4)
SENSOR      DEVICE   ADDRESS             MODEL         
-------------------------------------------------------
DS1820      01       1089d62d0308003a    DS18S20

/ > s = sensor.attach("DS1820",pio.GPIO4,1)
/ > s:read("all")
85.0

After analyzing the communication protocol on the onewire bus, I checked the conversion time with the datasheet. The data sheet specifies 750ms, but the read scratchpad sequence starts after 150ms. In my opinion, the problem is a wrong resolution for the DS18S20_FAMILY. The default resolution of 9 bits configures a measure_time of 150ms.

My DS1820 sensors works fine after changing the resolution for the DS18S20_FAMILY_CODE in the _set_resolution() function to TM_DS18B20_Resolution_12bits.

diff --git a/components/sys/sensors/ds1820.c b/components/sys/sensors/ds1820.c
index c84a61df..5f24413d 100644
--- a/components/sys/sensors/ds1820.c
+++ b/components/sys/sensors/ds1820.c
@@ -605,7 +605,7 @@ static uint8_t _set_resolution(uint8_t ds_res, uint8_t dev, uint8_t ds_dev) {
                res = TM_DS18B20_Resolution_10bits;
        }
        if (ow_devices[dev].roms[ds_dev-1][0] == DS18S20_FAMILY_CODE) {
-               res = TM_DS18B20_Resolution_9bits;
+               res = TM_DS18B20_Resolution_12bits;
        }
        else {
                res = TM_DS18B20_SetResolution(dev, ow_devices[dev].roms[ds_dev-1], (TM_DS18B20_Resolution_t)res);

the0ne commented 5 years ago

@jolivepetrus shall we take over the solution that @aheik has supposed? I do not have any of those sensors for testing, but @aheik mentiones testing with [m]any of those successfully after using 12bits resolution.

we have [m]any of the DS1820 onewire sensors and all of them returns 85.0.

the0ne commented 5 years ago

@aheik can you create a PR?

whitecatboard / Lua-RTOS-ESP32

DS1820 driver returns undocumented error values instead of raising an exception #69