MS8607 (PHT Click) sensor data over I2C and a driver on nRF52840 DongleReading

Hello!

I have started building an IoT test node using nrf52840 Dongle and MicroE MS8607 (PHT Click) sensors (please see the setup below). The functionality we are interested in is reading the sensor data over i2c and sending it via UART to the PC. 

The whole solution will be realized under Zephyr RTOS, therefore i2c configuration needs to be adapted as per Zephyr official docs: I2C Zephyr Project Documentation ; Zephyr API for i2c Bus

At the current stage it will be at user space, using kernel functions, i2c and UART for serial communication. I am developing under Linux Ubuntu 22.04 LTS, using Sublime Editor for code writing, Linux console for building an app and CuteCom for serial communication testing. nRF Connect for Desktop and it's tools as well as VS Code IDE aren't used. 

Since I am pretty new into the embedded software development field (mostly did web-app programming), there are main things I am confused about:

  • How to declare ic2 device combining nrf libraries, Zephyr packages and taking into account the manufacturer's recommended settings.
  • What is an approach and key-points of writing a driver for this particular solution?
  • The main confusion lays within where to start optimally as I am a bit with how to ensure all the compatibility as well as I haven't seen any sample solutions or demos for this sensor. 

* Native PHT Driver from manufacturer (MikroE):

pht.zip

Has anyone out here already worked with MS8607 (PHT Click) sensor on nRF52840 Dongle (PCA10059) through Zephyr RTOS?

Would appreciate any guidelines and approach. 

Thank you in advance!

p.s: Undoubtedly, it would be better to start with something more user clear (such as nRF52840 Development Kit or Arduino) but this time it is not an option, and I need to develop a solution using only the above mentioned hardware. 

Sincerely
Ingrid

Setup

Sensor Data sheet:
ENG_DS_MS8607-02BA01__C3.pdf

Parents
  • Update on the progress with this case. 
    After several studies and hard work (I customized the working setup, used nRF52840DK, corrected physical connections, etc.), 
    I managed to create an app for reading the relevant sensor data. My app was made following this structure: https://github.com/nrfconnect/ncs-example-application

    As we can see, the I2C communication is working correctly but there is an issue with the reading temperature:



    The code for the relevant pressure and temperature driver (ms8607pt.c) is shown below.

    Could you please advise where might be a problem with the driver, and how to correct this issue?

    Thank you!

    p.s: the driver was created as per this template (as recommended above): https://github.com/TEConnectivity/MS8607_Generic_C_Driver/blob/master/ms8607.c.

    Pressure and temperature driver ms8607pt.c

    #define DT_DRV_COMPAT te_ms8607pt
    
    #include <zephyr/device.h>
    #include <zephyr/drivers/i2c.h>
    #include <zephyr/drivers/sensor.h>
    #include <zephyr/logging/log.h>
    LOG_MODULE_REGISTER(ms8607pt, CONFIG_MS8607_LOG_LEVEL);
    
    #include "ms8607.h"
    
    struct ms8607pt_data {
    	const struct device *dev;
    	uint16_t eeprom_coeff[COEFFICIENT_NUMBERS+1];
    	bool coeff_is_ready;
    	bool ready;
    	enum ms8607_pressure_resolution resolution;
    	float temperature;
    	float pressure;
    };
    
    struct ms8607pt_config {
    	struct i2c_dt_spec i2c;
    };
    
    static uint32_t psensor_conversion_time[6] = {	PSENSOR_CONVERSION_TIME_OSR_256,
    												PSENSOR_CONVERSION_TIME_OSR_512,
    												PSENSOR_CONVERSION_TIME_OSR_1024,
    												PSENSOR_CONVERSION_TIME_OSR_2048,
    												PSENSOR_CONVERSION_TIME_OSR_4096,
    												PSENSOR_CONVERSION_TIME_OSR_8192
    												};
    
    static int read_register(const struct device *dev, uint8_t *val, uint8_t num_bytes)
    {
    	const struct ms8607pt_config *cfg = dev->config;
    	int rc = i2c_read_dt(&cfg->i2c, val, num_bytes);
    	return rc;
    }
    
    static int write_command(const struct device *dev, uint8_t command)
    {
    	const struct ms8607pt_config *cfg = dev->config;
    	int rc = 0;
    	uint8_t tx_buf[1] = {command};
    	rc = i2c_write_dt(&cfg->i2c, tx_buf, sizeof(tx_buf));
    	k_msleep(1);
    	return rc;
    }
    
    static uint8_t ms8607pt_crc_check (uint16_t *n_prom)
    {
    	uint8_t cnt, n_bit;
    	uint16_t n_rem, crc_read;
    	
    	n_rem = 0x00;
    	crc_read = n_prom[0];
    	n_prom[COEFFICIENT_NUMBERS] = 0;
    	n_prom[0] = (0x0FFF & (n_prom[0]));    // Clear the CRC byte
    
    	for( cnt = 0 ; cnt < (COEFFICIENT_NUMBERS+1)*2 ; cnt++ ) {
    
    		// Get next byte
    		if (cnt%2 == 1)
    			n_rem ^=  n_prom[cnt>>1] & 0x00FF ;
    		else
    			n_rem ^=  n_prom[cnt>>1]>>8 ;
    
    		for( n_bit = 8; n_bit > 0 ; n_bit-- ) {
    
    			if( n_rem & 0x8000 )
    				n_rem = (n_rem << 1) ^ 0x3000;
    			else
    				n_rem <<= 1;
    		}
    	}
    	n_rem >>= 12;
    	n_prom[0] = crc_read;
    	
    	return  (uint8_t)(n_rem);
    }
    
    static int ms8607pt_read_eeprom_coeff(const struct device *dev, uint8_t command, uint16_t *coeff)
    {
    	uint8_t buffer[2];
    	int rc = 0;
    	buffer[0] = 0;
    	buffer[1] = 0;
    
    	rc = write_command(dev, command);
    	if (rc != 0) {
    		LOG_ERR("Failed to call command 0x%02x - error %d", command, rc);
    		return rc;
    	}
    	
    	rc = read_register(dev, buffer, sizeof(buffer));
    	if (rc != 0) {
    		LOG_ERR("Failed to read buffer - error %d", rc);
    		return rc;
    	}
    
    	*coeff = (buffer[0] << 8) | buffer[1];
    	if (*coeff == 0) {
    		return -EINVAL;
    	}
    
    	return 0;
    }
    
    static int ms8607pt_read_eeprom(const struct device *dev) 
    {
    	struct ms8607pt_data *data = dev->data;
    	int rc = 0;
    	for(int i = 0 ; i < COEFFICIENT_NUMBERS ;i++)
    	{
    		rc = ms8607pt_read_eeprom_coeff(dev, PROM_ADDRESS_READ_ADDRESS_0 + i * 2, data->eeprom_coeff + i);
    		if(rc != 0)
    		{
    			return rc;
    		}
    	}
    
    	uint8_t cal_crc = ms8607pt_crc_check(data->eeprom_coeff);
    	uint8_t crc = (data->eeprom_coeff[CRC_INDEX] & 0xF000) >> 12;
    
    	if (cal_crc != crc) {
    		LOG_ERR("Invalid CRC 0x%2x - 0x%02x", cal_crc, crc);
    		return -EINVAL;
    	}
    	data->coeff_is_ready = true;
    	return 0;
    }
    
    static int ms8607pt_conversion_and_read_adc(const struct device *dev, uint8_t cmd, uint32_t *adc) 
    {
    	int rc = 0;
    	uint8_t buffer[3];
    
    	buffer[0] = 0;
    	buffer[1] = 0;
    	buffer[2] = 0;
    
    	rc = write_command(dev, cmd);
    	if (rc != 0) {
    		LOG_ERR("Failed to call COMMAND 0x%02x - error %d", cmd, rc);
    		return rc;
    	}
    	// 20ms wait for conversion
    	k_msleep(psensor_conversion_time[(cmd & PSENSOR_CONVERSION_OSR_MASK)/2 ]/1000 );
    	
    	rc = write_command(dev, PSENSOR_READ_ADC);
    	if (rc != 0) {
    		LOG_ERR("Failed to call COMMAND 0x%02x - error %d", cmd, rc);
    		return rc;
    	}
    
    	rc = read_register(dev, buffer, sizeof(buffer));
    	if (rc != 0) {
    		LOG_ERR("Failed to read buffer from READ PT %d", rc);
    		return rc;
    	}
    
    	*adc = ((uint32_t)buffer[0] << 16) | ((uint32_t)buffer[1] << 8) | buffer[2];
    	return 0;
    }
    
    static int ms8607pt_sample_fetch(const struct device *dev,
    				      enum sensor_channel chan)
    {
    	struct ms8607pt_data *data = dev->data;
    
    	if (chan != SENSOR_CHAN_ALL) {
    		return -ENOTSUP;
    	}
    
    	uint32_t adc_temperature = 0, adc_pressure = 0;
    	int32_t dT, TEMP;
    	int64_t OFF, SENS, P, T2, OFF2, SENS2;
    	uint8_t cmd;
    
    	int rc = 0;
    	if (data->coeff_is_ready == false) {
    		rc = ms8607pt_read_eeprom(dev);
    		if (rc != 0) {
    			LOG_ERR("Failed to read eeprom");
    			return rc;
    		}
    	}
    
    	// First read temperature
    	cmd = data->resolution * 2;
    	cmd |= PSENSOR_START_TEMPERATURE_ADC_CONVERSION;
    	rc = ms8607pt_conversion_and_read_adc(dev, cmd, &adc_temperature);
    	if (rc != 0) {
    		LOG_ERR("Failed to read temperature error %d", rc);
    		return rc;
    	}
    
    	// Now read pressure
    	cmd = data->resolution * 2;
    	cmd |= PSENSOR_START_PRESSURE_ADC_CONVERSION;
    	rc = ms8607pt_conversion_and_read_adc(dev, cmd, &adc_temperature);
    	if (rc != 0) {
    		LOG_ERR("Failed to read pressure error %d", rc);
    		return rc;
    	}
    
    	if (adc_temperature == 0 || adc_pressure == 0) {
    		LOG_ERR("Invalid value from ADC of temperature and pressure");
    		return -EINVAL;
    	}
    
    	// Difference between actual and reference temperature = D2 - Tref
    	dT = (int32_t)adc_temperature - ( (int32_t)data->eeprom_coeff[REFERENCE_TEMPERATURE_INDEX] <<8 );
    	
    	// Actual temperature = 2000 + dT * TEMPSENS
    	TEMP = 2000 + ((int64_t)dT * (int64_t)data->eeprom_coeff[TEMP_COEFF_OF_TEMPERATURE_INDEX] >> 23) ;
    
    	// Second order temperature compensation
    	if( TEMP < 2000 )
    	{
    		T2 = ( 3 * ( (int64_t)dT  * (int64_t)dT  ) ) >> 33;
    		OFF2 = 61 * ((int64_t)TEMP - 2000) * ((int64_t)TEMP - 2000) / 16 ;
    		SENS2 = 29 * ((int64_t)TEMP - 2000) * ((int64_t)TEMP - 2000) / 16 ;
    		
    		if( TEMP < -1500 )
    		{
    			OFF2 += 17 * ((int64_t)TEMP + 1500) * ((int64_t)TEMP + 1500) ;
    			SENS2 += 9 * ((int64_t)TEMP + 1500) * ((int64_t)TEMP + 1500) ;
    		}
    	}
    	else
    	{
    		T2 = ( 5 * ( (int64_t)dT  * (int64_t)dT  ) ) >> 38;
    		OFF2 = 0 ;
    		SENS2 = 0 ;
    	}
    	
    	// OFF = OFF_T1 + TCO * dT
    	OFF = ( (int64_t)(data->eeprom_coeff[PRESSURE_OFFSET_INDEX]) << 17 ) + ( ( (int64_t)(data->eeprom_coeff[TEMP_COEFF_OF_PRESSURE_OFFSET_INDEX]) * dT ) >> 6 ) ;
    	OFF -= OFF2 ;
    	
    	// Sensitivity at actual temperature = SENS_T1 + TCS * dT
    	SENS = ( (int64_t)data->eeprom_coeff[PRESSURE_SENSITIVITY_INDEX] << 16 ) + ( ((int64_t)data->eeprom_coeff[TEMP_COEFF_OF_PRESSURE_SENSITIVITY_INDEX] * dT) >> 7 ) ;
    	SENS -= SENS2 ;
    	
    	// Temperature compensated pressure = D1 * SENS - OFF
    	P = ( ( (adc_pressure * SENS) >> 21 ) - OFF ) >> 15 ;
    	data->temperature = ( (float)TEMP - T2 ) / 100;
    	data->pressure  = (float)P / 100;
    	data->ready = true;
    	return 0;
    }
    
    static int ms8607pt_channel_get(const struct device *dev,
    				     enum sensor_channel chan,
    				     struct sensor_value *val)
    {
    	struct ms8607pt_data *data = dev->data;
    	if (chan != SENSOR_CHAN_PRESS || chan != SENSOR_CHAN_AMBIENT_TEMP) {
    		return -ENOTSUP;
    	}
    
    	switch (chan) {
    		case SENSOR_CHAN_AMBIENT_TEMP:
    			val->val1 = (int32_t)data->temperature;
    			val->val2 = (int32_t)((data->temperature - (float)val->val1) * 1000000.0);
    			break;
    		case SENSOR_CHAN_PRESS:
    			val->val1 = (int32_t)data->pressure;
    			val->val2 = (int32_t)((data->pressure - (float)val->val1) * 1000000.0);
    			break;
    		default:
    			return -EINVAL;
    	}
    	
    	return 0;
    }
    
    static int ms8607pt_attr_set(const struct device *dev, enum sensor_channel chan,
    			   enum sensor_attribute attr,
    			   const struct sensor_value *val)
    {
    	struct ms8607pt_data *data = dev->data;
    	if (attr == SENSOR_ATTR_FULL_SCALE && chan == SENSOR_CHAN_PRESS) {
    		data->resolution = (enum ms8607_pressure_resolution)val->val1;
    		return 0;
    	}
    
    	return -EINVAL;
    }
    
    static const struct sensor_driver_api ms8607pt_api = {
    	.sample_fetch = ms8607pt_sample_fetch,
    	.channel_get = ms8607pt_channel_get,
    	.attr_set = ms8607pt_attr_set,
    };
    
    static int ms8607pt_init(const struct device *dev)
    {
    	struct ms8607pt_data *data = dev->data;
    	const struct ms8607pt_config *cfg = dev->config;
    	int rc = 0;
    	if (!i2c_is_ready_dt(&cfg->i2c)) {
    		LOG_ERR("I2C bus device not ready");
    		return -ENODEV;
    	}
    	k_msleep(5);
    	rc = write_command(dev, PSENSOR_RESET_COMMAND);
    	if (rc != 0) {
    		LOG_ERR("Failed to call RESET command %d", rc);
    		return rc;
    	}
    	k_msleep(5);
    	data->coeff_is_ready = false;
    	data->pressure = 0;
    	data->temperature = 0;
    	data->ready = false;
    	data->resolution = ms8607_pressure_resolution_osr_8192;
    
    	rc = ms8607pt_read_eeprom(dev);
    	if (rc != 0) {
    		LOG_ERR("Failed to erad EEPROM %d", rc);
    		return rc;
    	}
    
    	return 0;
    }
    
    
    static struct ms8607pt_data ms8607pt_data;
    static const struct ms8607pt_config ms8607pt_config = { 
    	.i2c = I2C_DT_SPEC_INST_GET(0),	
    };
    
    // Device definition
    DEVICE_DT_INST_DEFINE(0, ms8607pt_init, NULL, &ms8607pt_data,
    		&ms8607pt_config, POST_KERNEL,
    		CONFIG_SENSOR_INIT_PRIORITY, &ms8607pt_api);

    Humidity driver ms8607h.c

    #ifndef MS8607_H_INCLUDED
    #define MS8607_H_INCLUDED
    
    enum ms8607_humidity_i2c_master_mode {
    	ms8607_i2c_hold,
    	ms8607_i2c_no_hold
    };
    
    enum ms8607_status {
    	ms8607_status_ok,
    	ms8607_status_no_i2c_acknowledge,
    	ms8607_status_i2c_transfer_error,
    	ms8607_status_crc_error,
    	ms8607_status_heater_on_error
    };
    
    enum ms8607_humidity_resolution {
    	ms8607_humidity_resolution_12b = 0,
    	ms8607_humidity_resolution_8b,
    	ms8607_humidity_resolution_10b,
    	ms8607_humidity_resolution_11b
    };
    
    enum ms8607_battery_status {
    	ms8607_battery_ok,
    	ms8607_battery_low
    };
    
    enum ms8607_heater_status {
    	ms8607_heater_off,
    	ms8607_heater_on
    };
    
    enum ms8607_pressure_resolution {
    	ms8607_pressure_resolution_osr_256 = 0,
    	ms8607_pressure_resolution_osr_512,
    	ms8607_pressure_resolution_osr_1024,
    	ms8607_pressure_resolution_osr_2048,
    	ms8607_pressure_resolution_osr_4096,
    	ms8607_pressure_resolution_osr_8192
    };
    
    // HSENSOR device address
    #define HSENSOR_ADDR										0x40 //0b1000000
    
    // HSENSOR device commands
    #define HSENSOR_RESET_COMMAND								0xFE
    #define HSENSOR_READ_HUMIDITY_W_HOLD_COMMAND				0xE5
    #define HSENSOR_READ_HUMIDITY_WO_HOLD_COMMAND				0xF5
    #define HSENSOR_READ_SERIAL_FIRST_8BYTES_COMMAND			0xFA0F
    #define HSENSOR_READ_SERIAL_LAST_6BYTES_COMMAND				0xFCC9
    #define HSENSOR_WRITE_USER_REG_COMMAND						0xE6
    #define HSENSOR_READ_USER_REG_COMMAND						0xE7
    
    // Processing constants
    #define HSENSOR_TEMPERATURE_COEFFICIENT						(float)(-0.15)
    #define HSENSOR_CONSTANT_A									(float)(8.1332)
    #define HSENSOR_CONSTANT_B									(float)(1762.39)
    #define HSENSOR_CONSTANT_C									(float)(235.66)
    
    // Coefficients for temperature computation
    #define TEMPERATURE_COEFF_MUL								(175.72)
    #define TEMPERATURE_COEFF_ADD								(-46.85)
    
    // Coefficients for relative humidity computation
    #define HUMIDITY_COEFF_MUL									(125)
    #define HUMIDITY_COEFF_ADD									(-6)
    
    // Conversion timings
    #define HSENSOR_CONVERSION_TIME_12b							16000
    #define HSENSOR_CONVERSION_TIME_10b							5000
    #define HSENSOR_CONVERSION_TIME_8b							3000
    #define HSENSOR_CONVERSION_TIME_11b							9000    
    
    #define HSENSOR_RESET_TIME									15       // ms value
    
    // HSENSOR User Register masks and bit position
    #define HSENSOR_USER_REG_RESOLUTION_MASK					0x81
    #define HSENSOR_USER_REG_END_OF_BATTERY_MASK				0x40
    #define HSENSOR_USER_REG_ENABLE_ONCHIP_HEATER_MASK			0x4
    #define HSENSOR_USER_REG_DISABLE_OTP_RELOAD_MASK			0x2
    #define HSENSOR_USER_REG_RESERVED_MASK						(~(		HSENSOR_USER_REG_RESOLUTION_MASK				\
    																|	HSENSOR_USER_REG_END_OF_BATTERY_MASK			\
    																|	HSENSOR_USER_REG_ENABLE_ONCHIP_HEATER_MASK	\
    																|	HSENSOR_USER_REG_DISABLE_OTP_RELOAD_MASK ))
    
    // HTU User Register values
    // Resolution
    #define HSENSOR_USER_REG_RESOLUTION_12b						0x00
    #define HSENSOR_USER_REG_RESOLUTION_11b						0x81
    #define HSENSOR_USER_REG_RESOLUTION_10b						0x80
    #define HSENSOR_USER_REG_RESOLUTION_8b						0x01
    
    // End of battery status
    #define HSENSOR_USER_REG_END_OF_BATTERY_VDD_ABOVE_2_25V		0x00
    #define HSENSOR_USER_REG_END_OF_BATTERY_VDD_BELOW_2_25V		0x40
    // Enable on chip heater
    #define HSENSOR_USER_REG_ONCHIP_HEATER_ENABLE				0x04
    #define HSENSOR_USER_REG_OTP_RELOAD_DISABLE					0x02
    
    // PSENSOR device address
    #define PSENSOR_ADDR										0x76 //0b1110110
    
    // PSENSOR device commands
    #define PSENSOR_RESET_COMMAND								0x1E
    #define PSENSOR_START_PRESSURE_ADC_CONVERSION				0x40
    #define PSENSOR_START_TEMPERATURE_ADC_CONVERSION			0x50
    #define PSENSOR_READ_ADC									0x00
    
    #define PSENSOR_CONVERSION_OSR_MASK							0x0F
    
    #define PSENSOR_CONVERSION_TIME_OSR_256						1000
    #define PSENSOR_CONVERSION_TIME_OSR_512						2000
    #define PSENSOR_CONVERSION_TIME_OSR_1024					3000
    #define PSENSOR_CONVERSION_TIME_OSR_2048					5000
    #define PSENSOR_CONVERSION_TIME_OSR_4096					9000
    #define PSENSOR_CONVERSION_TIME_OSR_8192					18000
    
    // PSENSOR commands
    #define PROM_ADDRESS_READ_ADDRESS_0							0xA0
    #define PROM_ADDRESS_READ_ADDRESS_1							0xA2
    #define PROM_ADDRESS_READ_ADDRESS_2							0xA4
    #define PROM_ADDRESS_READ_ADDRESS_3							0xA6
    #define PROM_ADDRESS_READ_ADDRESS_4							0xA8
    #define PROM_ADDRESS_READ_ADDRESS_5							0xAA
    #define PROM_ADDRESS_READ_ADDRESS_6							0xAC
    #define PROM_ADDRESS_READ_ADDRESS_7							0xAE
    
    // Coefficients indexes for temperature and pressure computation
    #define CRC_INDEX											0
    #define PRESSURE_SENSITIVITY_INDEX							1
    #define PRESSURE_OFFSET_INDEX								2
    #define TEMP_COEFF_OF_PRESSURE_SENSITIVITY_INDEX			3
    #define TEMP_COEFF_OF_PRESSURE_OFFSET_INDEX					4
    #define REFERENCE_TEMPERATURE_INDEX							5
    #define TEMP_COEFF_OF_TEMPERATURE_INDEX						6
    #define COEFFICIENT_NUMBERS									7
    
    #define MAX_CONVERSION_TIME									HSENSOR_CONVERSION_TIME_12b
    
    #endif /* MS8607_H_INCLUDED */
    

    main.c

    #include <stdio.h>
    #include <zephyr/kernel.h>
    #include <zephyr/drivers/sensor.h>
    
    #include "app_version.h"
    #include <zephyr/logging/log.h>
    LOG_MODULE_REGISTER(main, CONFIG_APP_LOG_LEVEL);
    
    #include <sensor/ms8607.h>
    
    void main(void)
    {
    	int ret;
    	const struct device *ms8607_pt_dev;
    	const struct device *ms8607_h_dev;
    	struct sensor_value humidity, pressure, temperature;
    	enum ms8607_battery_status bat_status;
    	LOG_INF("MS8607 PHT Sensor Example Application - Version %s", APP_VERSION_STR);
    
    	ms8607_h_dev = DEVICE_DT_GET_ONE(te_ms8607h);
    	if (!device_is_ready(ms8607_h_dev)) {
    		LOG_ERR("Sensor %s not ready", ms8607_h_dev->name);
    		return;
    	}
    
    	ms8607_pt_dev = DEVICE_DT_GET_ONE(te_ms8607pt);
    	if (!device_is_ready(ms8607_pt_dev)) {
    		LOG_ERR("Sensor %s not ready", ms8607_pt_dev->name);
    		return;
    	}
    
    	ret = ms8607_enable_heater(ms8607_h_dev);
    	if (ret != 0) {
    		LOG_ERR("Failed to enable heater");
    		return;
    	}
    
    	k_sleep(K_SECONDS(1));
    
    	ret = ms8607_disable_heater(ms8607_h_dev);
    	if (ret != 0) {
    		LOG_ERR("Failed to enable heater");
    		return;
    	}
    
    	ret = ms8607_get_battery_status(ms8607_h_dev, &bat_status);
    	if (ret != 0) {
    		LOG_ERR("Failed to get battery status");
    		return;
    	}
    
    	LOG_INF("Battery status %s", bat_status == ms8607_battery_ok ? "Okay" : "Not Okay");
    
    	while (1) {
    		ret = sensor_sample_fetch(ms8607_pt_dev);
    		if (ret < 0) {
    			LOG_ERR("Could not fetch sample (%d)", ret);
    			return;
    		}
    
    		ret = sensor_sample_fetch(ms8607_h_dev);
    		if (ret < 0) {
    			LOG_ERR("Could not fetch sample (%d)", ret);
    			return;
    		}
    
    		sensor_channel_get(ms8607_pt_dev, SENSOR_CHAN_AMBIENT_TEMP, &temperature);
    		sensor_channel_get(ms8607_pt_dev, SENSOR_CHAN_PRESS, &pressure);
    		sensor_channel_get(ms8607_h_dev, SENSOR_CHAN_HUMIDITY, &humidity);
    		printf("MS8607: %.2f Temp ; %0.2f RH ; %0.2f Pressure \n",
    		       sensor_value_to_double(&temperature),
    		       sensor_value_to_double(&humidity),
    		       sensor_value_to_double(&pressure));
    		k_sleep(K_MSEC(1000));
    	}
    }
    
    

  • Note:
    You're mixing ENOTSUP and EINVAL. EINVAL means 'invalid param' and ENOTSUP means 'invalid value'. See error code definitions in zephyr\lib\libc\armstdc\include\errno.h. 

    Can you verify what the value of 'cmd' is before you call ms8607pt_conversion_and_read_adc? 


    The way that dev->data.resolution is used is a bit convoluted. Why can't you use the hardcoded enums?

    Will dev->data.resolution at any point in time equal ms8607_pressure_resolution_osr_8192 when "cmd = data->resolution * 2;" is executed? If so then it seems like 'cmd' will have an invalid value (5 * 2 = 10). 

  • Hello!

    Appreciate your notes and comments.

    Unfortunately, I probably can not answer your questions as do not fully understand this part to be honest (as I stated earlier, I am a novice in embedded systems & sensor drivers development fighting out as much as  I can on my own) so the code is born using available samples and occasional consultations with those who are somehow familiar. 

    Are there any suggestions on how to correct the pressure and temperature driver ms8607pt.c? Or how the right code should look (based on my starts)?

    Thank you!

Reply
  • Hello!

    Appreciate your notes and comments.

    Unfortunately, I probably can not answer your questions as do not fully understand this part to be honest (as I stated earlier, I am a novice in embedded systems & sensor drivers development fighting out as much as  I can on my own) so the code is born using available samples and occasional consultations with those who are somehow familiar. 

    Are there any suggestions on how to correct the pressure and temperature driver ms8607pt.c? Or how the right code should look (based on my starts)?

    Thank you!

Children
  • You're doing pretty good tbh, it seems you've got a good understanding of how to write a device driver for zephyr. I think what's missing is mostly down to debugging/verification. 

    For instance, you should verify that the commands you send to the sensor are valid. I suggest you scope the serial communication to the sensor and verify that the data you transmit and receive are as expected. 

    About enums and bit-fields: 

    if a given bit-field is represented numerically by a set of enums, it is vital that you never assign a numerical value to variable containing such a bit-field other than what can be represented by the given enum: 

    enum ms8607_pressure_resolution {           // Numerical value:
        ms8607_pressure_resolution_osr_256 = 0, // 0
        ms8607_pressure_resolution_osr_512,     // 1
        ms8607_pressure_resolution_osr_1024,    // 2
        ms8607_pressure_resolution_osr_2048,    // 3
        ms8607_pressure_resolution_osr_4096,    // 4
        ms8607_pressure_resolution_osr_8192     // 5
    };

    From "Figure 3: Command structure for pressure and temperature sensing" in the sensor's datasheet we find the OSR bit-feld in the command byte for the pressure and temp sensor:

    Command byte hex value
    Bit number 7 6   5   4 3 2 1 0
    bit name PROM CONV - Typ

    Ad2/
    Os2

    Ad1/
    Os1
    Ad0/
    Os0
    Stop
    Command
    Convert D1 (OSR=2048) 0 1 0 0 0 1 1 0 0x46

    Here you can see that the 3 bits named Os0 - Os2 makes a bit-field that represents OSR, and where an OSR of 2048 is represented numerically as the binary number 0b011, or 3 in decimal. 

    I've only included the one command in the table above, but from the complete table it will be clear that if you assign a numerical value to the OSR bitfield that is larger than 5 (0b101) the command byte will be undefined. Worse, if you assign a numerical value to the OSR bit-field greater than what can be represented by 3 bits ( > 7dec, or 0b111) you can overwrite the other bits in the command byte. 

    F.ex. 
    cmd = ms8607_pressure_resolution_osr_8192  * 2; // 5 * 2 = 10

    Here cmd will have the value of 10dec or 0b1010. This means that now the Typ bit has the value 0b1 and the OSR bit-field has the value 0b10. This command byte might still be a valid command according to the datasheet, but it will the one you want.

    The lession here is that you need to validate, or at least sanetize input parameters, and that can easily be done with enums. You can for instance add a line to ms8607pt_conversion_and_read_adc() to check whether OSR is of a valid value or not:

    if((cmd & (0x7 << 1)) > (ms8607_pressure_resolution_osr_8192 << 1)){
        // Throw error! OSR > 5 is invalid!
    }
    
    // (0x7 << 1) == 0b00001110
    // (ms8607_pressure_resolution_osr_8192 << 1) == 0b00001010


  • Thank you very much  ! really appreciate your help, consideration, and explanation. Will try to improve the solution considering the suggested approach.

Related