sensors: ds18b20: reliablilty improvements

drivers/sensor/maxim/ds18b20/ds18b20.c

@@ -15,16 +15,66 @@
  * Parasite power configuration is not supported by the driver.
  */
 
+#include <stdbool.h>
+
+#include <zephyr/device.h>
+#include <zephyr/devicetree.h>
 #include <zephyr/drivers/sensor.h>
 #include <zephyr/drivers/sensor/w1_sensor.h>
+#include "zephyr/drivers/w1.h"
+#include <zephyr/kernel.h>
 #include <zephyr/logging/log.h>
 #include <zephyr/sys/__assert.h>
-
-#include "ds18b20.h"
+#include <zephyr/sys/util_macro.h>
 
 LOG_MODULE_REGISTER(DS18B20, CONFIG_SENSOR_LOG_LEVEL);
 
+#define DS18B20_CMD_CONVERT_T         0x44
+#define DS18B20_CMD_WRITE_SCRATCHPAD  0x4E
+#define DS18B20_CMD_READ_SCRATCHPAD   0xBE
+#define DS18B20_CMD_COPY_SCRATCHPAD   0x48
+#define DS18B20_CMD_RECALL_EEPROM     0xB8
+#define DS18B20_CMD_READ_POWER_SUPPLY 0xB4
+
+/* resolution is set using bit 5 and 6 of configuration register
+ * macro only valid for values 9 to 12
+ */
+#define DS18B20_RESOLUTION_POS		5
+#define DS18B20_RESOLUTION_MASK		(BIT_MASK(2) << DS18B20_RESOLUTION_POS)
+/* convert resolution in bits to scratchpad config format */
+#define DS18B20_RESOLUTION(res)		((res - 9) << DS18B20_RESOLUTION_POS)
+/* convert resolution in bits to array index (for resolution specific elements) */
+#define DS18B20_RESOLUTION_INDEX(res)	(res - 9)
+
+#define DS18B20_FAMILYCODE 0x28
+#define DS18S20_FAMILYCODE 0x10
+
+enum chip_type {type_ds18b20, type_ds18s20};
+
+struct ds18b20_scratchpad {
+	int16_t temp;
+	uint8_t alarm_temp_high;
+	uint8_t alarm_temp_low;
+	uint8_t config;
+	uint8_t res[3];
+	uint8_t crc;
+} __packed;
+
+struct ds18b20_config {
+	const struct device *bus;
+	uint8_t family;
+	uint8_t resolution;
+	enum chip_type chip;
+};
+
+struct ds18b20_data {
+	struct w1_slave_config config;
+	struct ds18b20_scratchpad scratchpad;
+	bool lazy_loaded;
+};
+
 static int ds18b20_configure(const struct device *dev);
+static int ds18b20_read_scratchpad(const struct device *dev, struct ds18b20_scratchpad *scratchpad);
 
 /* measure wait time for 9-bit, 10-bit, 11-bit, 12-bit resolution respectively */
 static const uint16_t measure_wait_ds18b20_ms[4] = { 94, 188, 376, 750 };
@@ -48,41 +98,90 @@ static inline void ds18b20_temperature_from_raw(const struct device *dev,
 	}
 }
 
+static inline bool slave_responded(uint8_t *rx_buf, size_t len)
+{
+	uint8_t cmp_byte = 0xff;
+
+	for (int i = 0; i < len; i++) {
+		cmp_byte &= rx_buf[i];
+	}
+
+	return (cmp_byte == 0xff) ? false : true;
+}
+
 /*
  * Write scratch pad, read back, then copy to eeprom
  */
 static int ds18b20_write_scratchpad(const struct device *dev,
 				    struct ds18b20_scratchpad scratchpad)
 {
 	struct ds18b20_data *data = dev->data;
-	const struct device *bus = ds18b20_bus(dev);
+	const struct ds18b20_config *cfg = dev->config;
+	const struct device *bus = cfg->bus;
+	int ret;
 	uint8_t sp_data[4] = {
 		DS18B20_CMD_WRITE_SCRATCHPAD,
 		scratchpad.alarm_temp_high,
 		scratchpad.alarm_temp_low,
 		scratchpad.config
 	};
 
-	return w1_write_read(bus, &data->config, sp_data, sizeof(sp_data), NULL, 0);
+	ret = w1_write_read(bus, &data->config, sp_data, sizeof(sp_data), NULL, 0);
+	if (ret != 0) {
+		return ret;
+	}
+
+	ret = ds18b20_read_scratchpad(dev, &scratchpad);
+	if (ret != 0) {
+		return ret;
+	}
+
+	if ((sp_data[3] & DS18B20_RESOLUTION_MASK) !=
+	    (scratchpad.config & DS18B20_RESOLUTION_MASK)) {
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static int ds18b20_read_scratchpad(const struct device *dev,
 				   struct ds18b20_scratchpad *scratchpad)
 {
 	struct ds18b20_data *data = dev->data;
-	const struct device *bus = ds18b20_bus(dev);
+	const struct ds18b20_config *cfg = dev->config;
+	const struct device *bus = cfg->bus;
+	int ret;
 	uint8_t cmd = DS18B20_CMD_READ_SCRATCHPAD;
+	uint8_t crc;
+
+	memset(scratchpad, 0, sizeof(*scratchpad));
+	ret = w1_write_read(bus, &data->config, &cmd, 1,
+			     (uint8_t *)scratchpad, sizeof(*scratchpad));
+	if (ret != 0) {
+		return ret;
+	}
+
+	if (!slave_responded((uint8_t *)scratchpad, sizeof(*scratchpad))) {
+		LOG_WRN("Slave not reachable");
+		return -ENODEV;
+	}
 
-	return w1_write_read(bus, &data->config, &cmd, 1,
-			     (uint8_t *)&scratchpad[0], 9);
+	crc = w1_crc8((uint8_t *)scratchpad, sizeof(*scratchpad) - 1);
+	if (crc != scratchpad->crc) {
+		LOG_WRN("CRC does not match");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 /* Starts sensor temperature conversion without waiting for completion. */
 static int ds18b20_temperature_convert(const struct device *dev)
 {
 	int ret;
 	struct ds18b20_data *data = dev->data;
-	const struct device *bus = ds18b20_bus(dev);
+	const struct ds18b20_config *cfg = dev->config;
+	const struct device *bus = cfg->bus;
 
 	(void)w1_lock_bus(bus);
 	ret = w1_reset_select(bus, &data->config);

samples/sensor/ds18b20/src/main.c

@@ -37,19 +37,30 @@ static const struct device *get_ds18b20_device(void)
 int main(void)
 {
 	const struct device *dev = get_ds18b20_device();
+	int res;
 
 	if (dev == NULL) {
 		return 0;
 	}
 
-	while (1) {
+	while (true) {
 		struct sensor_value temp;
 
-		sensor_sample_fetch(dev);
-		sensor_channel_get(dev, SENSOR_CHAN_AMBIENT_TEMP, &temp);
+		res = sensor_sample_fetch(dev);
+		if (res != 0) {
+			printk("sample_fetch() failed: %d\n", res);
+			return res;
+		}
+
+		res = sensor_channel_get(dev, SENSOR_CHAN_AMBIENT_TEMP, &temp);
+		if (res != 0) {
+			printk("channel_get() failed: %d\n", res);
+			return res;
+		}
 
 		printk("Temp: %d.%06d\n", temp.val1, temp.val2);
 		k_sleep(K_MSEC(2000));
 	}
+
 	return 0;
 }