You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "linguini1 (via GitHub)" <gi...@apache.org> on 2024/07/17 00:42:58 UTC

[PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

linguini1 opened a new pull request, #12715:
URL: https://github.com/apache/nuttx/pull/12715

   ## Summary
   
   This PR introduces a new driver for the SHT4x family of temperature and humidity sensors that operate over the I2C protocol. It provides a POSIX interface for all of the sensor's features: reading data, serial number, performing a reset and operating the sensor's heater within the safe usage limits.
   
   This PR also closes #12665.
   
   ## Impact
   
   An easier interface for future developers to interact with this family of sensors, rather than having to write their own driver.
   
   ## Testing
   
   In order to test the driver, I performed a series of basic checks on all of the implemented functionality.
   
   - First, I performed a simple `read` of the device driver path from the command line using `cat` to ensure that the character driver interface works as expected.
   - I performed a command line write with `echo` to see that no write executed (not implemented intentionally)
   - I created a small test suite binary which I ran to test all of the IOCTL commands and the driver restrictions, as described below:
   
   ### Reset
   
   The `SNIOC_RESET` command was tested by making the corresponding `ioctl` call and ensuring that no error was returned.
   
   ```c
     err = ioctl(sensor, SNIOC_RESET);
     if (err) {
       fprintf(stderr, "SNIOC_RESET: %s\n", strerror(errno));
     } else {
       puts("RESET success!");
     }
   ```
   
   ### Who Am I
   
   The `SNIOC_WHO_AM_I` command was tested by making the corresponding `ioctl` call and ensuring that the device serial number was returned in the `uint32_t *serialno` variable without any errors.
   
   ```c
     uint32_t serialno = 0;
     err = ioctl(sensor, SNIOC_WHO_AM_I, &serialno);
     if (err) {
       fprintf(stderr, "SNIOC_WHO_AM_I: %s\n", strerror(errno));
     } else {
       printf("Serial no: %lu\n", serialno);
     }
   ```
   
   ### Read Raw Data
   
   `SNIOC_READ_RAW_DATA` was tested by making the corresponding `ioctl` call and checking that raw data was present in the passed pointer argument.
   
   ```c
     struct sht4x_raw_data_s raw = {0};
     err = ioctl(sensor, SNIOC_READ_RAW_DATA, &raw);
     if (err) {
       fprintf(stderr, "SNIOC_READ_RAW_DATA: %s\n", strerror(errno));
     } else {
       printf("Temp: %u, Hum: %u\n", raw.raw_temp, raw.raw_hum);
     }
   ```
   
   ### Read Converted Data
   
   `SNIOC_MEASURE` and `SNIOC_READ_CONVERT_DATA` were tested by making both `ioctl` calls and ensuring that they had identical results. The converted data was verified against the ambient temperature and humidity of my environment (my desk).
   
   ```c
     // Read converted data
     struct sht4x_conv_data_s data = {0};
     err = ioctl(sensor, SNIOC_MEASURE, &data);
     if (err) {
       fprintf(stderr, "SNIOC_MEASURE: %s\n", strerror(errno));
     } else {
       printf("Temp: %ld, Hum: %d\n", data.temp, data.hum);
     }
   
     // Read again
     data.temp = 0;
     data.hum = 0;
     err = ioctl(sensor, SNIOC_READ_CONVERT_DATA, &data);
     if (err) {
       fprintf(stderr, "SNIOC_READ_CONVERT_DATA: %s\n", strerror(errno));
     } else {
       printf("Temp: %ld mC, Hum: %d %%RH\n", data.temp, data.hum / 100);
     }
   ```
   
   ### Change precision
   
   Verified by making the `ioctl` call with `CONFIG_SHT4X_DEBUG` enabled and checking that the corresponding debug log was made.
   
   ```c
     // Change precision
     err = ioctl(sensor, SNIOC_CONFIGURE, SHT4X_PREC_LOW);
     if (err) {
       fprintf(stderr, "SNIOC_CONFIGURE: %s\n", strerror(errno));
     } else {
       puts("Set precision low successfully");
     }
   ```
   
   ### Heat for 1 second
   
   Verified by making the `ioctl` call and ensuring that no error was returned and that the returned data had a slightly higher temperature than the previous `SNIOC_READ_CONVERT_DATA` call, since the heater had then been active.
   
   ```c
     // Try to heat for 1 second
     data.temp = SHT4X_HEATER_200MW_1;
     err = ioctl(sensor, SNIOC_HEAT, &data);
     if (err) {
       fprintf(stderr, "SNIOC_HEAT: %s\n", strerror(errno));
     } else {
       puts("Heat successfully (1st try).");
       printf("Temp: %ld mC, Hum: %d %%RH\n", data.temp, data.hum / 100);
     }
   ```
   
   ### Heat again before cooldown
   
   The SHT4X should not have its heater active more than 10% of the time, so the driver enforces a 1 second cooldown after each heating command. I tested this by performing a second `SNIOC_HEAT` `ioctl` call immediately after the first, which correctly resulted in an `EAGAIN` error instead of performing the heater operation.
   
   ```c
     // Immediately try to heat again for 0.1
     data.temp = SHT4X_HEATER_200MW_01;
     err = ioctl(sensor, SNIOC_HEAT, &data);
     if (err) {
       fprintf(stderr, "SNIOC_HEAT: %s\n", strerror(errno));
     } else {
       puts("Heated (2nd try) again, should not have been possible.");
       printf("Temp: %ld mC, Hum: %d %%RH\n", data.temp, data.hum / 100);
     }
   ```
   
   ### Test Output
   
   Here is the test console output:
   ```console
   26050 mC, 35 %RH
   RESET success!
   Serial no: 1806896678
   Temp: 26624, Hum: 22183
   Temp: 26050, Hum: 3525
   Temp: 26050 mC, Hum: 35 %RH
   Set precision low successfully
   Heat successfully (1st try).
   Temp: 74525 mC, Hum: 17 %RH
   SNIOC_HEAT: Unknown error 11
   ```
   `Unknown error 11` corresponds to `EGAIN`, it seems `strerror` was not working. Likely a config setting I missed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1682663834


##########
drivers/sensors/sht4x.c:
##########
@@ -0,0 +1,827 @@
+/****************************************************************************
+ * drivers/sensors/sht4x.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <debug.h>
+#include <nuttx/clock.h>
+#include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/i2c/i2c_master.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mutex.h>
+#include <nuttx/random.h>
+#include <nuttx/sensors/sht4x.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#if defined(CONFIG_I2C) && defined(CONFIG_SENSORS_SHT4X)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_DEBUG
+#define sht4x_dbg(x, ...) _info(x, ##__VA_ARGS__)
+#endif
+
+#if defined(CONFIG_SHT4X_FAHRENHEIT)
+#define SHT4X_TEMP_UNIT "F"
+#else
+#define SHT4X_TEMP_UNIT "C"
+#endif // defined(CONFIG_SHT4X_FAHRENHEIT)
+
+#ifndef CONFIG_SHT4X_I2C_FREQUENCY
+#define CONFIG_SHT4X_I2C_FREQUENCY 400000
+#endif // CONFIG_SHT4X_I2C_FREQUENCY
+
+#define SHT4X_CRC_INIT 0xFF /* Initial value of the calculated CRC. */
+#define SHT4X_CRC_POLY 0x31 /* CRC calculation polynomial. */
+
+#define SHT4X_SOFT_RESET 0x94     /* Soft reset command. */
+#define SHT4X_READ_SERIAL 0x89    /* Read serial number command. */
+#define SHT4X_READ_LOW_PREC 0xE0  /* Low precision read temp & humidity. */
+#define SHT4X_READ_MED_PREC 0xF6  /* Med precision read temp & humidity. */
+#define SHT4X_READ_HIGH_PREC 0xFD /* High precision read temp & humidity. */
+#define SHT4X_HEAT_200_1 0x39     /* Activate heater with 200mW for 1s. */
+#define SHT4X_HEAT_200_P1 0x32    /* Activate heater with 200mW for 0.1s. */
+#define SHT4X_HEAT_110_1 0x2F     /* Activate heater with 110mW for 1s. */
+#define SHT4X_HEAT_110_P1 0x24    /* Activate heater with 110mW for 0.1s. */
+#define SHT4X_HEAT_20_1 0x1E      /* Activate heater with 20mW for 1s. */
+#define SHT4X_HEAT_20_P1 0x15     /* Activate heater with 20mW for 0.1s. */
+
+/****************************************************************************
+ * Private
+ ****************************************************************************/
+
+struct sht4x_dev_s
+{
+  FAR struct i2c_master_s *i2c; /* I2C interface. */
+  uint8_t addr;                 /* I2C address. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  bool unlinked;                    /* True, driver has been unlinked. */
+#endif                              // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  struct timespec last_heat;        /* Last time heater was active. */
+  enum sht4x_precision_e precision; /* The precision for read operations. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  int16_t crefs; /* Number of open references. */
+#endif           // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  mutex_t devlock;
+};
+
+/* Easy unpacking of serial number from I2C packet. */
+
+union sht4x_serialno_t
+{
+  uint32_t full; /* Full serial number. */
+  struct
+  {
+    uint16_t msb; /* Most significant 2 bytes. */
+    uint16_t lsb; /* Least significant 2 bytes. */
+  } halves;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Commands for reading at different precisions */
+
+const uint8_t PRECISION_READ[] =
+{
+    [SHT4X_PREC_LOW] = SHT4X_READ_LOW_PREC,
+    [SHT4X_PREC_MED] = SHT4X_READ_MED_PREC,
+    [SHT4X_PREC_HIGH] = SHT4X_READ_HIGH_PREC,
+};
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/* CRC lookup table. */
+
+static const uint8_t CRC_LOOKUP[] =
+    {
+        0x0,  0x31, 0x62, 0x53, 0xc4, 0xf5, 0xa6, 0x97, 0xb9, 0x88, 0xdb,
+        0xea, 0x7d, 0x4c, 0x1f, 0x2e, 0x43, 0x72, 0x21, 0x10, 0x87, 0xb6,
+        0xe5, 0xd4, 0xfa, 0xcb, 0x98, 0xa9, 0x3e, 0xf,  0x5c, 0x6d, 0x86,
+        0xb7, 0xe4, 0xd5, 0x42, 0x73, 0x20, 0x11, 0x3f, 0xe,  0x5d, 0x6c,
+        0xfb, 0xca, 0x99, 0xa8, 0xc5, 0xf4, 0xa7, 0x96, 0x1,  0x30, 0x63,
+        0x52, 0x7c, 0x4d, 0x1e, 0x2f, 0xb8, 0x89, 0xda, 0xeb, 0x3d, 0xc,
+        0x5f, 0x6e, 0xf9, 0xc8, 0x9b, 0xaa, 0x84, 0xb5, 0xe6, 0xd7, 0x40,
+        0x71, 0x22, 0x13, 0x7e, 0x4f, 0x1c, 0x2d, 0xba, 0x8b, 0xd8, 0xe9,
+        0xc7, 0xf6, 0xa5, 0x94, 0x3,  0x32, 0x61, 0x50, 0xbb, 0x8a, 0xd9,
+        0xe8, 0x7f, 0x4e, 0x1d, 0x2c, 0x2,  0x33, 0x60, 0x51, 0xc6, 0xf7,
+        0xa4, 0x95, 0xf8, 0xc9, 0x9a, 0xab, 0x3c, 0xd,  0x5e, 0x6f, 0x41,
+        0x70, 0x23, 0x12, 0x85, 0xb4, 0xe7, 0xd6, 0x7a, 0x4b, 0x18, 0x29,
+        0xbe, 0x8f, 0xdc, 0xed, 0xc3, 0xf2, 0xa1, 0x90, 0x7,  0x36, 0x65,
+        0x54, 0x39, 0x8,  0x5b, 0x6a, 0xfd, 0xcc, 0x9f, 0xae, 0x80, 0xb1,
+        0xe2, 0xd3, 0x44, 0x75, 0x26, 0x17, 0xfc, 0xcd, 0x9e, 0xaf, 0x38,
+        0x9,  0x5a, 0x6b, 0x45, 0x74, 0x27, 0x16, 0x81, 0xb0, 0xe3, 0xd2,
+        0xbf, 0x8e, 0xdd, 0xec, 0x7b, 0x4a, 0x19, 0x28, 0x6,  0x37, 0x64,
+        0x55, 0xc2, 0xf3, 0xa0, 0x91, 0x47, 0x76, 0x25, 0x14, 0x83, 0xb2,
+        0xe1, 0xd0, 0xfe, 0xcf, 0x9c, 0xad, 0x3a, 0xb,  0x58, 0x69, 0x4,
+        0x35, 0x66, 0x57, 0xc0, 0xf1, 0xa2, 0x93, 0xbd, 0x8c, 0xdf, 0xee,
+        0x79, 0x48, 0x1b, 0x2a, 0xc1, 0xf0, 0xa3, 0x92, 0x5,  0x34, 0x67,
+        0x56, 0x78, 0x49, 0x1a, 0x2b, 0xbc, 0x8d, 0xde, 0xef, 0x82, 0xb3,
+        0xe0, 0xd1, 0x46, 0x77, 0x24, 0x15, 0x3b, 0xa,  0x59, 0x68, 0xff,
+        0xce, 0x9d, 0xac,
+}
+#endif // CONFIG_SHT4X_CRC_LOOKUP
+
+/* Measurement times for the various precisions, in microseconds. */
+
+static const uint16_t MEASUREMENT_TIMES[] =
+{
+    [SHT4X_PREC_LOW] = 1600,
+    [SHT4X_PREC_MED] = 4500,
+    [SHT4X_PREC_HIGH] = 8300,
+};
+
+/* Commands for the various heating options. */
+
+static const uint8_t HEAT_CMDS[] =
+{
+    [SHT4X_HEATER_200MW_1] = SHT4X_HEAT_200_1,
+    [SHT4X_HEATER_200MW_01] = SHT4X_HEAT_200_P1,
+    [SHT4X_HEATER_110MW_1] = SHT4X_HEAT_110_1,
+    [SHT4X_HEATER_110MW_01] = SHT4X_HEAT_110_P1,
+    [SHT4X_HEATER_20MW_1] = SHT4X_HEAT_20_1,
+    [SHT4X_HEATER_20MW_01] = SHT4X_HEAT_20_P1,
+};
+
+/* Timeouts for the various heating options in microseconds. */
+
+static const uint32_t HEAT_TIMES[] =
+{
+    [SHT4X_HEATER_200MW_1] = 1000000, [SHT4X_HEATER_200MW_01] = 100000,
+    [SHT4X_HEATER_110MW_1] = 1000000, [SHT4X_HEATER_110MW_01] = 100000,
+    [SHT4X_HEATER_20MW_1] = 1000000,  [SHT4X_HEATER_20MW_01] = 100000,
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int sht4x_open(FAR struct file *filep);
+static int sht4x_close(FAR struct file *filep);
+static ssize_t sht4x_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static ssize_t sht4x_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static int sht4x_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static int sht4x_unlink(FAR struct inode *inode);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations g_sht4xfops =
+{
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .open = sht4x_open,
+    .close = sht4x_close,
+#else
+    .open = NULL,
+    .close = NULL,
+#endif
+    .read = sht4x_read,
+    .write = sht4x_write,
+    .seek = NULL,
+    .ioctl = sht4x_ioctl,
+    .mmap = NULL,
+    .truncate = NULL,
+    .poll = NULL,
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .unlink = sht4x_unlink,
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/****************************************************************************
+ * Name: sht4x_crc_lookup
+ *
+ * Description:
+ *   Perform a CRC calculation using the lookup table.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_lookup(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc = CRC_LOOKUP[crc ^ buf[byte]];
+    }
+
+  return crc;
+}
+
+#endif
+
+/****************************************************************************
+ * Name: sht4x_crc_bitwise
+ *
+ * Description:
+ *   Perform a CRC calculation using the bitwise calculation.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_bitwise(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc ^= buf[byte];
+      for (uint8_t bit = 0; bit < 8; bit++)
+        {
+          if (crc & 0x80)
+            {
+              /* Discard the highest bit (implicit XOR), then divide by the
+               * polynomial.
+               */
+
+              crc <<= 1;
+              crc ^= SHT4X_CRC_POLY;
+            }
+          else
+            {
+              /* Continue until the highest bit is set. */
+
+              crc <<= 1;
+            }
+        }
+    }
+
+  return crc;
+}
+
+/****************************************************************************
+ * Name: sht4x_crc
+ *
+ * Description:
+ *   Perform a CRC calculation on the data to check for validity.
+ *   The `buf` param must be a data buffer that ends with its own 8 bit CRC.
+ *   The `nbytes` param is the length of the data including the 8 bit CRC.
+ *
+ ****************************************************************************/
+
+static int sht4x_crc(const uint8_t *buf, uint8_t nbytes)
+{
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+  if (sht4x_crc_lookup(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+#else
+  if (sht4x_crc_bitwise(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+
+#endif
+  return 0;
+}
+
+/****************************************************************************
+ * Name: sht4x_cmd
+ *
+ * Description:
+ *   I2C access helper for sending commands to the SHT4x which receive data
+ *   in response.
+ *
+ ****************************************************************************/
+
+static int sht4x_cmd(FAR struct sht4x_dev_s *priv, uint8_t command,
+                     uint32_t timeout, uint16_t *data1, uint16_t *data2)
+{
+  struct i2c_msg_s cmd;
+  struct i2c_msg_s read_data;
+  int err;
+
+  /* 2 byte data, 1 byte CRC, 2 byte data, 1 byte CRC. */
+
+  uint8_t combined_data[6];
+
+  cmd.frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  cmd.addr = priv->addr;
+  cmd.flags = 0;
+  cmd.buffer = &command;
+  cmd.length = sizeof(command);
+
+  /* Trigger measurement. */
+
+  err = I2C_TRANSFER(priv->i2c, &cmd, 1);
+  if (err) return err;
+
+  /* Wait for measurement to complete. Serial number command does not require
+   * wait time.
+   */
+
+  if (timeout > 0)
+    {
+      usleep(timeout);
+    }
+
+  read_data.frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  read_data.addr = priv->addr;
+  read_data.flags = I2C_M_READ;
+  read_data.buffer = combined_data;
+  read_data.length = sizeof(combined_data);
+
+  err = I2C_TRANSFER(priv->i2c, &read_data, 1);
+  if (err) return err;
+
+  /* Check CRC for first data packet */
+
+  err = sht4x_crc(combined_data, 3);
+  if (err) return err;
+
+  /* Check CRC for second data packet */
+
+  err = sht4x_crc(combined_data + 3, 3);
+  if (err) return err;
+
+  *data1 = (uint16_t)(combined_data[0] << 8) | combined_data[1];
+  *data2 = (uint16_t)(combined_data[3] << 8) | combined_data[4];
+  return err;
+}
+
+/****************************************************************************
+ * Name: sht4x_reset
+ *
+ * Description:
+ *   Perform a soft reset of the sensor. WARNING: Sensor is not available
+ *   until 1ms after this command is sent.
+ *
+ ****************************************************************************/
+
+static int sht4x_reset(FAR struct sht4x_dev_s *priv)
+{
+  struct i2c_msg_s msg[1];
+  uint8_t reset_cmd = SHT4X_SOFT_RESET;
+
+  msg[0].frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  msg[0].addr = priv->addr;
+  msg[0].flags = 0;
+  msg[0].buffer = &reset_cmd;
+  msg[0].length = sizeof(reset_cmd);
+
+  return I2C_TRANSFER(priv->i2c, msg, 1);
+}
+
+/****************************************************************************
+ * Name: sht4x_calc_temp
+ *
+ * Description:
+ *   Calculate the temperature from the SHT4X raw data.
+ *
+ ****************************************************************************/
+
+static int32_t sht4x_calc_temp(uint16_t temp)
+{
+#ifdef CONFIG_SHT4X_FAHRENHEIT
+  /* Millidegrees Fahrenheit */
+
+  return -49000 + 315 * ((temp * 1000) / 65535);
+#else
+  return -45000 + 175 * ((temp * 1000) / 65535); /* Millidegrees Celsius */
+#endif // CONFIG_SHT4X_FAHRENHEIT
+}
+
+/****************************************************************************
+ * Name: sht4x_calc_hum
+ *
+ * Description:
+ *   Calculate the humidity from the SHT4X raw data.
+ *
+ ****************************************************************************/
+
+static int16_t sht4x_calc_hum(uint16_t humidity)
+{
+  int16_t hum = -600 + 125 * ((humidity * 100) / 65535); /* 0.01 %RH */
+#ifdef CONFIG_SHT4X_LIMIT_HUMIDITY
+
+  /* Limit values of humidity, since values outside of 0-100 are invalid. */
+
+  if (hum < 0)
+    {
+      hum = 0;
+    }
+  else if (hum > 10000)
+    {
+      hum = 10000;
+    }
+
+#endif // CONFIG_SHT4X_LIMIT_HUMIDITY
+  return hum;
+}
+
+/****************************************************************************
+ * Name: has_time_passed
+ *
+ * Description:
+ *   Return true if curr >= start + secs_since_start
+ *
+ ****************************************************************************/
+
+static bool has_time_passed(struct timespec curr, struct timespec start,
+                            unsigned int secs_since_start)
+{
+  if ((long)((start.tv_sec + secs_since_start) - curr.tv_sec) == 0)
+    {
+      return start.tv_nsec <= curr.tv_nsec;
+    }
+
+  return (long)((start.tv_sec + secs_since_start) - curr.tv_sec) <= 0;
+}
+
+/****************************************************************************
+ * Name: sht4x_open
+ *
+ * Description:
+ *   This function is called whenever the SHT4X device is opened.
+ *
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int sht4x_open(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  int err;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+  /* Increment the count of open references on the driver */
+
+  priv->crefs++;
+  DEBUGASSERT(priv->crefs > 0);
+
+  nxmutex_unlock(&priv->devlock);
+  return 0;
+}
+#endif
+
+/****************************************************************************
+ * Name: sht4x_close
+ *
+ * Description:
+ *   This routine is called when the SHT4X device is closed.
+ *
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int sht4x_close(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  int err;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+  /* Decrement the count of open references on the driver */
+
+  DEBUGASSERT(priv->crefs > 0);
+  priv->crefs--;
+
+  /* If the count has decremented to zero and the driver has been unlinked,
+   * then free memory now.
+   */
+
+  if (priv->crefs <= 0 && priv->unlinked)
+    {
+      nxmutex_destroy(&priv->devlock);
+      kmm_free(priv);
+      return 0;
+    }
+
+  nxmutex_unlock(&priv->devlock);
+  return 0;
+}
+#endif
+
+/****************************************************************************
+ * Name: sht4x_read
+ *
+ * Description:
+ *     Character driver interface to sensor for debugging.
+ *
+ ****************************************************************************/
+
+static ssize_t sht4x_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  ssize_t length = 0;
+  struct sht4x_raw_data_s raw_data;
+  struct sht4x_conv_data_s data;
+  int err;
+
+  /* If file position is non-zero, then we're at the end of file. */
+
+  if (filep->f_pos > 0) return 0;
+
+  /* Get exclusive access */
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  if (priv->unlinked)
+    {
+      /* Do not allow operations on unlinked sensors. This allows
+       * sensor use on hot swappable I2C bus.
+       */
+
+      nxmutex_unlock(&priv->devlock);
+      return 0;
+    }
+#endif
+
+  err = sht4x_cmd(priv, PRECISION_READ[priv->precision],
+                  MEASUREMENT_TIMES[priv->precision], &raw_data.raw_temp,
+                  &raw_data.raw_hum);
+  if (err)
+    {
+#ifdef CONFIG_SHT4X_DEBUG
+      sht4x_dbg("Could not read device: %d\n", err);
+#endif // CONFIG_SHT4X_DEBUG
+      nxmutex_unlock(&priv->devlock);
+      return err;
+    }
+
+  /* Convert to proper units. */
+
+  data.temp = sht4x_calc_temp(raw_data.raw_temp);
+  data.hum = sht4x_calc_hum(raw_data.raw_hum);
+
+  length = snprintf(buffer, buflen, "%ld m" SHT4X_TEMP_UNIT ", %d %%RH\n",
+                    data.temp, data.hum / 100);
+  if (length > buflen) length = buflen;
+
+  filep->f_pos += length;
+  nxmutex_unlock(&priv->devlock);
+  return length;
+}
+
+/****************************************************************************
+ * Name: sht4x_write
+ *
+ * Description:
+ *     Not implemented.
+ ****************************************************************************/
+
+static ssize_t sht4x_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: sht4x_ioctl
+ ****************************************************************************/
+
+static int sht4x_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  int err;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  if (priv->unlinked)
+    {
+      /* Do not allow operations on unlinked sensors. This allows
+       * sensor use on hot swappable I2C bus.
+       */
+
+      nxmutex_unlock(&priv->devlock);
+      return -ENODEV;
+    }
+#endif
+
+  switch (cmd)
+    {
+    case SNIOC_RESET:
+      err = sht4x_reset(priv);
+      break;
+
+    case SNIOC_WHO_AM_I:
+      {
+        union sht4x_serialno_t serialno;
+        err = sht4x_cmd(priv, SHT4X_READ_SERIAL, 10, &serialno.halves.msb,
+                        &serialno.halves.lsb);
+        *((uint32_t *)(arg)) = serialno.full;
+      }
+      break;
+
+    case SNIOC_READ_RAW_DATA:
+      err = sht4x_cmd(priv, PRECISION_READ[priv->precision],
+                      MEASUREMENT_TIMES[priv->precision],
+                      &((struct sht4x_raw_data_s *)(arg))->raw_temp,
+                      &((struct sht4x_raw_data_s *)(arg))->raw_hum);
+      break;
+
+    case SNIOC_MEASURE:
+    case SNIOC_READ_CONVERT_DATA:
+      {
+        uint16_t raw_t;
+        uint16_t raw_h;
+
+        err = sht4x_cmd(priv, PRECISION_READ[priv->precision],
+                        MEASUREMENT_TIMES[priv->precision], &raw_t, &raw_h);
+        ((struct sht4x_conv_data_s *)(arg))->temp = sht4x_calc_temp(raw_t);
+        ((struct sht4x_conv_data_s *)(arg))->hum = sht4x_calc_hum(raw_h);
+      }
+      break;
+
+    case SNIOC_HEAT:
+      {
+        struct timespec now;
+        clock_systime_timespec(&now);
+
+        /* Check if it has been one second since the last heat command. */
+
+        if (!has_time_passed(now, priv->last_heat, 1))
+          {
+            err = -EAGAIN; /* Signal to try again in some time. */
+            break;
+          }
+
+        /* Caller must pass heater option in the temperature argument. */
+
+        uint16_t raw_t = ((struct sht4x_conv_data_s *)(arg))->temp;
+        uint16_t raw_h;
+
+        err = sht4x_cmd(priv, HEAT_CMDS[raw_t], HEAT_TIMES[raw_t], &raw_t,
+                        &raw_h);
+        ((struct sht4x_conv_data_s *)(arg))->temp = sht4x_calc_temp(raw_t);
+        ((struct sht4x_conv_data_s *)(arg))->hum = sht4x_calc_hum(raw_h);
+        clock_systime_timespec(&priv->last_heat); /* Update last heat time. */
+      }
+      break;
+
+    case SNIOC_CONFIGURE:
+
+      /* Caller must pass precision option as argument. */
+
+      priv->precision = arg;
+#ifdef CONFIG_SHT4X_DEBUG
+      sht4x_dbg("Precision set to %d\n", priv->precision);
+#endif // CONFIG_SHT4X_DEBUG
+
+      break;
+
+    default:
+      err = -EINVAL;
+      break;
+    }
+
+  nxmutex_unlock(&priv->devlock);
+  return err;
+}
+
+/****************************************************************************
+ * Name: sht4x_unlink
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int sht4x_unlink(FAR struct inode *inode)
+{
+  FAR struct sht4x_dev_s *priv;
+  int err;
+
+  DEBUGASSERT(inode->i_private != NULL);
+  priv = inode->i_private;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+  /* Are there open references to the driver data structure? */
+
+  if (priv->crefs <= 0)
+    {
+      nxmutex_destroy(&priv->devlock);
+      kmm_free(priv);
+      return 0;
+    }
+
+  /* No. Just mark the driver as unlinked and free the resources when
+   * the last client closes their reference to the driver.
+   */
+
+  priv->unlinked = true;
+  nxmutex_unlock(&priv->devlock);
+  return OK;
+}
+#endif
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: sht4x_register
+ *
+ * Description:
+ *   Register the SHT4X character device as 'devpath'
+ *
+ * Input Parameters:
+ *   devpath - The full path to the driver to register. E.g., "/dev/temp0"
+ *   i2c     - An instance of the I2C interface to use to communicate with
+ *             the SHT4X
+ *   addr    - The I2C address of the SHT4X. The I2C address is one of 0x44,
+ *0x45 and 0x46.
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int sht4x_register(FAR const char *devpath, FAR struct i2c_master_s *i2c,
+                   uint8_t addr)
+{
+  FAR struct sht4x_dev_s *priv;
+  int err;
+
+  DEBUGASSERT(i2c != NULL);
+  DEBUGASSERT(addr == 0x44 || addr == 0x45 || addr == 0x46);
+
+  /* Initialize the device structure */
+
+  priv = kmm_zalloc(sizeof(struct sht4x_dev_s));
+  if (priv == NULL)
+    {
+      snerr("ERROR: Failed to allocate instance.\n");
+      return -ENOMEM;
+    }
+
+  priv->i2c = i2c;
+  priv->addr = addr;
+  priv->precision = SHT4X_PREC_HIGH;
+  err = clock_systime_timespec(&priv->last_heat);
+
+  if (err)
+    {
+      snerr("ERROR: Failed to get timespec: %d\n", err);
+      kmm_free(priv);
+      return err;
+    }
+
+  /* Allow heat immediately after registration since in theory the sensor has
+   * never had its heater activated yet.
+   */
+
+  priv->last_heat.tv_sec -= 1;
+
+  err = nxmutex_init(&priv->devlock);
+  if (err)
+    {
+      snerr("ERROR: Failed to register SHT4X driver: %d\n", err);
+      kmm_free(priv);
+      return err;
+    }
+
+  /* Register the character driver */
+
+  err = register_driver(devpath, &g_sht4xfops, 0666, priv);
+  if (err < 0)

Review Comment:
   Do you know why the check is typically explicitly for negatives? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1684370463


##########
boards/arm/rp2040/common/src/rp2040_common_bringup.c:
##########
@@ -68,6 +68,11 @@
 #include "rp2040_bmp280.h"
 #endif
 
+#ifdef CONFIG_SENSORS_SHT4X
+#include <nuttx/sensors/sht4x.h>
+#include "rp2040_i2c.h"

Review Comment:
   I do use that function call, which is why I included the I2C header you mentioned in your last comment.
   
   The reason I left that function call in the rp2040 bring up is because it seems like the architecture specific logic should stay out of the driver, since I want it to work on all architectures with I2C. I have seen this done for the SHT3x and SHT21. It made more sense to me to keep it in the architecture bring-up logic.
   
   I was hoping to find some way to access the I2C driver structure without an architecture specific initialization function, but since I haven't been able to, I think this way ensures that the driver can run on all architectures.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#issuecomment-2244033717

   Hi @linguini1 I think you can try to post in more than one place, like LinkedIn, Hacker News: https://news.ycombinator.com/ , etc.
   
   In the past https://slashdot.org used to be the right place to post about it, but now few people follow them


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #12715:
URL: https://github.com/apache/nuttx/pull/12715


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1682193396


##########
drivers/sensors/sht4x.c:
##########
@@ -0,0 +1,827 @@
+/****************************************************************************
+ * drivers/sensors/sht4x.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <debug.h>
+#include <nuttx/clock.h>
+#include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/i2c/i2c_master.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mutex.h>
+#include <nuttx/random.h>
+#include <nuttx/sensors/sht4x.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#if defined(CONFIG_I2C) && defined(CONFIG_SENSORS_SHT4X)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_DEBUG
+#define sht4x_dbg(x, ...) _info(x, ##__VA_ARGS__)
+#endif
+
+#if defined(CONFIG_SHT4X_FAHRENHEIT)
+#define SHT4X_TEMP_UNIT "F"
+#else
+#define SHT4X_TEMP_UNIT "C"
+#endif // defined(CONFIG_SHT4X_FAHRENHEIT)
+
+#ifndef CONFIG_SHT4X_I2C_FREQUENCY
+#define CONFIG_SHT4X_I2C_FREQUENCY 400000
+#endif // CONFIG_SHT4X_I2C_FREQUENCY
+
+#define SHT4X_CRC_INIT 0xFF /* Initial value of the calculated CRC. */
+#define SHT4X_CRC_POLY 0x31 /* CRC calculation polynomial. */
+
+#define SHT4X_SOFT_RESET 0x94     /* Soft reset command. */
+#define SHT4X_READ_SERIAL 0x89    /* Read serial number command. */
+#define SHT4X_READ_LOW_PREC 0xE0  /* Low precision read temp & humidity. */
+#define SHT4X_READ_MED_PREC 0xF6  /* Med precision read temp & humidity. */
+#define SHT4X_READ_HIGH_PREC 0xFD /* High precision read temp & humidity. */
+#define SHT4X_HEAT_200_1 0x39     /* Activate heater with 200mW for 1s. */
+#define SHT4X_HEAT_200_P1 0x32    /* Activate heater with 200mW for 0.1s. */
+#define SHT4X_HEAT_110_1 0x2F     /* Activate heater with 110mW for 1s. */
+#define SHT4X_HEAT_110_P1 0x24    /* Activate heater with 110mW for 0.1s. */
+#define SHT4X_HEAT_20_1 0x1E      /* Activate heater with 20mW for 1s. */
+#define SHT4X_HEAT_20_P1 0x15     /* Activate heater with 20mW for 0.1s. */
+
+/****************************************************************************
+ * Private
+ ****************************************************************************/
+
+struct sht4x_dev_s
+{
+  FAR struct i2c_master_s *i2c; /* I2C interface. */
+  uint8_t addr;                 /* I2C address. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  bool unlinked;                    /* True, driver has been unlinked. */
+#endif                              // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  struct timespec last_heat;        /* Last time heater was active. */
+  enum sht4x_precision_e precision; /* The precision for read operations. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  int16_t crefs; /* Number of open references. */
+#endif           // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  mutex_t devlock;
+};
+
+/* Easy unpacking of serial number from I2C packet. */
+
+union sht4x_serialno_t
+{
+  uint32_t full; /* Full serial number. */
+  struct
+  {
+    uint16_t msb; /* Most significant 2 bytes. */
+    uint16_t lsb; /* Least significant 2 bytes. */
+  } halves;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Commands for reading at different precisions */
+
+const uint8_t PRECISION_READ[] =
+{
+    [SHT4X_PREC_LOW] = SHT4X_READ_LOW_PREC,
+    [SHT4X_PREC_MED] = SHT4X_READ_MED_PREC,
+    [SHT4X_PREC_HIGH] = SHT4X_READ_HIGH_PREC,
+};
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/* CRC lookup table. */
+
+static const uint8_t CRC_LOOKUP[] =
+    {
+        0x0,  0x31, 0x62, 0x53, 0xc4, 0xf5, 0xa6, 0x97, 0xb9, 0x88, 0xdb,
+        0xea, 0x7d, 0x4c, 0x1f, 0x2e, 0x43, 0x72, 0x21, 0x10, 0x87, 0xb6,
+        0xe5, 0xd4, 0xfa, 0xcb, 0x98, 0xa9, 0x3e, 0xf,  0x5c, 0x6d, 0x86,
+        0xb7, 0xe4, 0xd5, 0x42, 0x73, 0x20, 0x11, 0x3f, 0xe,  0x5d, 0x6c,
+        0xfb, 0xca, 0x99, 0xa8, 0xc5, 0xf4, 0xa7, 0x96, 0x1,  0x30, 0x63,
+        0x52, 0x7c, 0x4d, 0x1e, 0x2f, 0xb8, 0x89, 0xda, 0xeb, 0x3d, 0xc,
+        0x5f, 0x6e, 0xf9, 0xc8, 0x9b, 0xaa, 0x84, 0xb5, 0xe6, 0xd7, 0x40,
+        0x71, 0x22, 0x13, 0x7e, 0x4f, 0x1c, 0x2d, 0xba, 0x8b, 0xd8, 0xe9,
+        0xc7, 0xf6, 0xa5, 0x94, 0x3,  0x32, 0x61, 0x50, 0xbb, 0x8a, 0xd9,
+        0xe8, 0x7f, 0x4e, 0x1d, 0x2c, 0x2,  0x33, 0x60, 0x51, 0xc6, 0xf7,
+        0xa4, 0x95, 0xf8, 0xc9, 0x9a, 0xab, 0x3c, 0xd,  0x5e, 0x6f, 0x41,
+        0x70, 0x23, 0x12, 0x85, 0xb4, 0xe7, 0xd6, 0x7a, 0x4b, 0x18, 0x29,
+        0xbe, 0x8f, 0xdc, 0xed, 0xc3, 0xf2, 0xa1, 0x90, 0x7,  0x36, 0x65,
+        0x54, 0x39, 0x8,  0x5b, 0x6a, 0xfd, 0xcc, 0x9f, 0xae, 0x80, 0xb1,
+        0xe2, 0xd3, 0x44, 0x75, 0x26, 0x17, 0xfc, 0xcd, 0x9e, 0xaf, 0x38,
+        0x9,  0x5a, 0x6b, 0x45, 0x74, 0x27, 0x16, 0x81, 0xb0, 0xe3, 0xd2,
+        0xbf, 0x8e, 0xdd, 0xec, 0x7b, 0x4a, 0x19, 0x28, 0x6,  0x37, 0x64,
+        0x55, 0xc2, 0xf3, 0xa0, 0x91, 0x47, 0x76, 0x25, 0x14, 0x83, 0xb2,
+        0xe1, 0xd0, 0xfe, 0xcf, 0x9c, 0xad, 0x3a, 0xb,  0x58, 0x69, 0x4,
+        0x35, 0x66, 0x57, 0xc0, 0xf1, 0xa2, 0x93, 0xbd, 0x8c, 0xdf, 0xee,
+        0x79, 0x48, 0x1b, 0x2a, 0xc1, 0xf0, 0xa3, 0x92, 0x5,  0x34, 0x67,
+        0x56, 0x78, 0x49, 0x1a, 0x2b, 0xbc, 0x8d, 0xde, 0xef, 0x82, 0xb3,
+        0xe0, 0xd1, 0x46, 0x77, 0x24, 0x15, 0x3b, 0xa,  0x59, 0x68, 0xff,
+        0xce, 0x9d, 0xac,
+}
+#endif // CONFIG_SHT4X_CRC_LOOKUP
+
+/* Measurement times for the various precisions, in microseconds. */
+
+static const uint16_t MEASUREMENT_TIMES[] =
+{
+    [SHT4X_PREC_LOW] = 1600,
+    [SHT4X_PREC_MED] = 4500,
+    [SHT4X_PREC_HIGH] = 8300,
+};
+
+/* Commands for the various heating options. */
+
+static const uint8_t HEAT_CMDS[] =

Review Comment:
   ALL global variables must be lower case and start with g_ prefix



##########
drivers/sensors/sht4x.c:
##########
@@ -0,0 +1,827 @@
+/****************************************************************************
+ * drivers/sensors/sht4x.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <debug.h>
+#include <nuttx/clock.h>
+#include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/i2c/i2c_master.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mutex.h>
+#include <nuttx/random.h>
+#include <nuttx/sensors/sht4x.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#if defined(CONFIG_I2C) && defined(CONFIG_SENSORS_SHT4X)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_DEBUG
+#define sht4x_dbg(x, ...) _info(x, ##__VA_ARGS__)
+#endif
+
+#if defined(CONFIG_SHT4X_FAHRENHEIT)
+#define SHT4X_TEMP_UNIT "F"
+#else
+#define SHT4X_TEMP_UNIT "C"
+#endif // defined(CONFIG_SHT4X_FAHRENHEIT)
+
+#ifndef CONFIG_SHT4X_I2C_FREQUENCY
+#define CONFIG_SHT4X_I2C_FREQUENCY 400000
+#endif // CONFIG_SHT4X_I2C_FREQUENCY
+
+#define SHT4X_CRC_INIT 0xFF /* Initial value of the calculated CRC. */
+#define SHT4X_CRC_POLY 0x31 /* CRC calculation polynomial. */
+
+#define SHT4X_SOFT_RESET 0x94     /* Soft reset command. */
+#define SHT4X_READ_SERIAL 0x89    /* Read serial number command. */
+#define SHT4X_READ_LOW_PREC 0xE0  /* Low precision read temp & humidity. */
+#define SHT4X_READ_MED_PREC 0xF6  /* Med precision read temp & humidity. */
+#define SHT4X_READ_HIGH_PREC 0xFD /* High precision read temp & humidity. */
+#define SHT4X_HEAT_200_1 0x39     /* Activate heater with 200mW for 1s. */
+#define SHT4X_HEAT_200_P1 0x32    /* Activate heater with 200mW for 0.1s. */
+#define SHT4X_HEAT_110_1 0x2F     /* Activate heater with 110mW for 1s. */
+#define SHT4X_HEAT_110_P1 0x24    /* Activate heater with 110mW for 0.1s. */
+#define SHT4X_HEAT_20_1 0x1E      /* Activate heater with 20mW for 1s. */
+#define SHT4X_HEAT_20_P1 0x15     /* Activate heater with 20mW for 0.1s. */
+
+/****************************************************************************
+ * Private
+ ****************************************************************************/
+
+struct sht4x_dev_s
+{
+  FAR struct i2c_master_s *i2c; /* I2C interface. */
+  uint8_t addr;                 /* I2C address. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  bool unlinked;                    /* True, driver has been unlinked. */
+#endif                              // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  struct timespec last_heat;        /* Last time heater was active. */
+  enum sht4x_precision_e precision; /* The precision for read operations. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  int16_t crefs; /* Number of open references. */
+#endif           // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  mutex_t devlock;
+};
+
+/* Easy unpacking of serial number from I2C packet. */
+
+union sht4x_serialno_t
+{
+  uint32_t full; /* Full serial number. */
+  struct
+  {
+    uint16_t msb; /* Most significant 2 bytes. */
+    uint16_t lsb; /* Least significant 2 bytes. */
+  } halves;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Commands for reading at different precisions */
+
+const uint8_t PRECISION_READ[] =
+{
+    [SHT4X_PREC_LOW] = SHT4X_READ_LOW_PREC,
+    [SHT4X_PREC_MED] = SHT4X_READ_MED_PREC,
+    [SHT4X_PREC_HIGH] = SHT4X_READ_HIGH_PREC,
+};
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/* CRC lookup table. */
+
+static const uint8_t CRC_LOOKUP[] =
+    {
+        0x0,  0x31, 0x62, 0x53, 0xc4, 0xf5, 0xa6, 0x97, 0xb9, 0x88, 0xdb,
+        0xea, 0x7d, 0x4c, 0x1f, 0x2e, 0x43, 0x72, 0x21, 0x10, 0x87, 0xb6,
+        0xe5, 0xd4, 0xfa, 0xcb, 0x98, 0xa9, 0x3e, 0xf,  0x5c, 0x6d, 0x86,
+        0xb7, 0xe4, 0xd5, 0x42, 0x73, 0x20, 0x11, 0x3f, 0xe,  0x5d, 0x6c,
+        0xfb, 0xca, 0x99, 0xa8, 0xc5, 0xf4, 0xa7, 0x96, 0x1,  0x30, 0x63,
+        0x52, 0x7c, 0x4d, 0x1e, 0x2f, 0xb8, 0x89, 0xda, 0xeb, 0x3d, 0xc,
+        0x5f, 0x6e, 0xf9, 0xc8, 0x9b, 0xaa, 0x84, 0xb5, 0xe6, 0xd7, 0x40,
+        0x71, 0x22, 0x13, 0x7e, 0x4f, 0x1c, 0x2d, 0xba, 0x8b, 0xd8, 0xe9,
+        0xc7, 0xf6, 0xa5, 0x94, 0x3,  0x32, 0x61, 0x50, 0xbb, 0x8a, 0xd9,
+        0xe8, 0x7f, 0x4e, 0x1d, 0x2c, 0x2,  0x33, 0x60, 0x51, 0xc6, 0xf7,
+        0xa4, 0x95, 0xf8, 0xc9, 0x9a, 0xab, 0x3c, 0xd,  0x5e, 0x6f, 0x41,
+        0x70, 0x23, 0x12, 0x85, 0xb4, 0xe7, 0xd6, 0x7a, 0x4b, 0x18, 0x29,
+        0xbe, 0x8f, 0xdc, 0xed, 0xc3, 0xf2, 0xa1, 0x90, 0x7,  0x36, 0x65,
+        0x54, 0x39, 0x8,  0x5b, 0x6a, 0xfd, 0xcc, 0x9f, 0xae, 0x80, 0xb1,
+        0xe2, 0xd3, 0x44, 0x75, 0x26, 0x17, 0xfc, 0xcd, 0x9e, 0xaf, 0x38,
+        0x9,  0x5a, 0x6b, 0x45, 0x74, 0x27, 0x16, 0x81, 0xb0, 0xe3, 0xd2,
+        0xbf, 0x8e, 0xdd, 0xec, 0x7b, 0x4a, 0x19, 0x28, 0x6,  0x37, 0x64,
+        0x55, 0xc2, 0xf3, 0xa0, 0x91, 0x47, 0x76, 0x25, 0x14, 0x83, 0xb2,
+        0xe1, 0xd0, 0xfe, 0xcf, 0x9c, 0xad, 0x3a, 0xb,  0x58, 0x69, 0x4,
+        0x35, 0x66, 0x57, 0xc0, 0xf1, 0xa2, 0x93, 0xbd, 0x8c, 0xdf, 0xee,
+        0x79, 0x48, 0x1b, 0x2a, 0xc1, 0xf0, 0xa3, 0x92, 0x5,  0x34, 0x67,
+        0x56, 0x78, 0x49, 0x1a, 0x2b, 0xbc, 0x8d, 0xde, 0xef, 0x82, 0xb3,
+        0xe0, 0xd1, 0x46, 0x77, 0x24, 0x15, 0x3b, 0xa,  0x59, 0x68, 0xff,
+        0xce, 0x9d, 0xac,
+}
+#endif // CONFIG_SHT4X_CRC_LOOKUP
+
+/* Measurement times for the various precisions, in microseconds. */
+
+static const uint16_t MEASUREMENT_TIMES[] =
+{
+    [SHT4X_PREC_LOW] = 1600,
+    [SHT4X_PREC_MED] = 4500,
+    [SHT4X_PREC_HIGH] = 8300,
+};
+
+/* Commands for the various heating options. */
+
+static const uint8_t HEAT_CMDS[] =
+{
+    [SHT4X_HEATER_200MW_1] = SHT4X_HEAT_200_1,
+    [SHT4X_HEATER_200MW_01] = SHT4X_HEAT_200_P1,
+    [SHT4X_HEATER_110MW_1] = SHT4X_HEAT_110_1,
+    [SHT4X_HEATER_110MW_01] = SHT4X_HEAT_110_P1,
+    [SHT4X_HEATER_20MW_1] = SHT4X_HEAT_20_1,
+    [SHT4X_HEATER_20MW_01] = SHT4X_HEAT_20_P1,
+};
+
+/* Timeouts for the various heating options in microseconds. */
+
+static const uint32_t HEAT_TIMES[] =
+{
+    [SHT4X_HEATER_200MW_1] = 1000000, [SHT4X_HEATER_200MW_01] = 100000,
+    [SHT4X_HEATER_110MW_1] = 1000000, [SHT4X_HEATER_110MW_01] = 100000,
+    [SHT4X_HEATER_20MW_1] = 1000000,  [SHT4X_HEATER_20MW_01] = 100000,
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int sht4x_open(FAR struct file *filep);
+static int sht4x_close(FAR struct file *filep);
+static ssize_t sht4x_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static ssize_t sht4x_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static int sht4x_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static int sht4x_unlink(FAR struct inode *inode);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations g_sht4xfops =
+{
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .open = sht4x_open,
+    .close = sht4x_close,
+#else
+    .open = NULL,
+    .close = NULL,
+#endif
+    .read = sht4x_read,
+    .write = sht4x_write,
+    .seek = NULL,
+    .ioctl = sht4x_ioctl,
+    .mmap = NULL,
+    .truncate = NULL,
+    .poll = NULL,
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .unlink = sht4x_unlink,
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/****************************************************************************
+ * Name: sht4x_crc_lookup
+ *
+ * Description:
+ *   Perform a CRC calculation using the lookup table.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_lookup(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc = CRC_LOOKUP[crc ^ buf[byte]];
+    }
+
+  return crc;
+}
+
+#endif
+
+/****************************************************************************
+ * Name: sht4x_crc_bitwise
+ *
+ * Description:
+ *   Perform a CRC calculation using the bitwise calculation.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_bitwise(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc ^= buf[byte];
+      for (uint8_t bit = 0; bit < 8; bit++)
+        {
+          if (crc & 0x80)
+            {
+              /* Discard the highest bit (implicit XOR), then divide by the
+               * polynomial.
+               */
+
+              crc <<= 1;
+              crc ^= SHT4X_CRC_POLY;
+            }
+          else
+            {
+              /* Continue until the highest bit is set. */
+
+              crc <<= 1;
+            }
+        }
+    }
+
+  return crc;
+}
+
+/****************************************************************************
+ * Name: sht4x_crc
+ *
+ * Description:
+ *   Perform a CRC calculation on the data to check for validity.
+ *   The `buf` param must be a data buffer that ends with its own 8 bit CRC.
+ *   The `nbytes` param is the length of the data including the 8 bit CRC.
+ *
+ ****************************************************************************/
+
+static int sht4x_crc(const uint8_t *buf, uint8_t nbytes)
+{
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+  if (sht4x_crc_lookup(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+#else
+  if (sht4x_crc_bitwise(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+
+#endif
+  return 0;
+}
+
+/****************************************************************************
+ * Name: sht4x_cmd
+ *
+ * Description:
+ *   I2C access helper for sending commands to the SHT4x which receive data
+ *   in response.
+ *
+ ****************************************************************************/
+
+static int sht4x_cmd(FAR struct sht4x_dev_s *priv, uint8_t command,
+                     uint32_t timeout, uint16_t *data1, uint16_t *data2)
+{
+  struct i2c_msg_s cmd;
+  struct i2c_msg_s read_data;
+  int err;
+
+  /* 2 byte data, 1 byte CRC, 2 byte data, 1 byte CRC. */
+
+  uint8_t combined_data[6];
+
+  cmd.frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  cmd.addr = priv->addr;
+  cmd.flags = 0;
+  cmd.buffer = &command;
+  cmd.length = sizeof(command);
+
+  /* Trigger measurement. */
+
+  err = I2C_TRANSFER(priv->i2c, &cmd, 1);
+  if (err) return err;
+
+  /* Wait for measurement to complete. Serial number command does not require
+   * wait time.
+   */
+
+  if (timeout > 0)
+    {
+      usleep(timeout);
+    }
+
+  read_data.frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  read_data.addr = priv->addr;
+  read_data.flags = I2C_M_READ;
+  read_data.buffer = combined_data;
+  read_data.length = sizeof(combined_data);
+
+  err = I2C_TRANSFER(priv->i2c, &read_data, 1);
+  if (err) return err;
+
+  /* Check CRC for first data packet */
+
+  err = sht4x_crc(combined_data, 3);
+  if (err) return err;
+
+  /* Check CRC for second data packet */
+
+  err = sht4x_crc(combined_data + 3, 3);
+  if (err) return err;
+
+  *data1 = (uint16_t)(combined_data[0] << 8) | combined_data[1];
+  *data2 = (uint16_t)(combined_data[3] << 8) | combined_data[4];
+  return err;
+}
+
+/****************************************************************************
+ * Name: sht4x_reset
+ *
+ * Description:
+ *   Perform a soft reset of the sensor. WARNING: Sensor is not available
+ *   until 1ms after this command is sent.
+ *
+ ****************************************************************************/
+
+static int sht4x_reset(FAR struct sht4x_dev_s *priv)
+{
+  struct i2c_msg_s msg[1];
+  uint8_t reset_cmd = SHT4X_SOFT_RESET;
+
+  msg[0].frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  msg[0].addr = priv->addr;
+  msg[0].flags = 0;
+  msg[0].buffer = &reset_cmd;
+  msg[0].length = sizeof(reset_cmd);
+
+  return I2C_TRANSFER(priv->i2c, msg, 1);
+}
+
+/****************************************************************************
+ * Name: sht4x_calc_temp
+ *
+ * Description:
+ *   Calculate the temperature from the SHT4X raw data.
+ *
+ ****************************************************************************/
+
+static int32_t sht4x_calc_temp(uint16_t temp)
+{
+#ifdef CONFIG_SHT4X_FAHRENHEIT
+  /* Millidegrees Fahrenheit */
+
+  return -49000 + 315 * ((temp * 1000) / 65535);
+#else
+  return -45000 + 175 * ((temp * 1000) / 65535); /* Millidegrees Celsius */
+#endif // CONFIG_SHT4X_FAHRENHEIT
+}
+
+/****************************************************************************
+ * Name: sht4x_calc_hum
+ *
+ * Description:
+ *   Calculate the humidity from the SHT4X raw data.
+ *
+ ****************************************************************************/
+
+static int16_t sht4x_calc_hum(uint16_t humidity)
+{
+  int16_t hum = -600 + 125 * ((humidity * 100) / 65535); /* 0.01 %RH */
+#ifdef CONFIG_SHT4X_LIMIT_HUMIDITY
+
+  /* Limit values of humidity, since values outside of 0-100 are invalid. */
+
+  if (hum < 0)
+    {
+      hum = 0;
+    }
+  else if (hum > 10000)
+    {
+      hum = 10000;
+    }
+
+#endif // CONFIG_SHT4X_LIMIT_HUMIDITY
+  return hum;
+}
+
+/****************************************************************************
+ * Name: has_time_passed
+ *
+ * Description:
+ *   Return true if curr >= start + secs_since_start
+ *
+ ****************************************************************************/
+
+static bool has_time_passed(struct timespec curr, struct timespec start,
+                            unsigned int secs_since_start)
+{
+  if ((long)((start.tv_sec + secs_since_start) - curr.tv_sec) == 0)
+    {
+      return start.tv_nsec <= curr.tv_nsec;
+    }
+
+  return (long)((start.tv_sec + secs_since_start) - curr.tv_sec) <= 0;
+}
+
+/****************************************************************************
+ * Name: sht4x_open
+ *
+ * Description:
+ *   This function is called whenever the SHT4X device is opened.
+ *
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int sht4x_open(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  int err;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+  /* Increment the count of open references on the driver */
+
+  priv->crefs++;
+  DEBUGASSERT(priv->crefs > 0);
+
+  nxmutex_unlock(&priv->devlock);
+  return 0;
+}
+#endif
+
+/****************************************************************************
+ * Name: sht4x_close
+ *
+ * Description:
+ *   This routine is called when the SHT4X device is closed.
+ *
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int sht4x_close(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  int err;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+  /* Decrement the count of open references on the driver */
+
+  DEBUGASSERT(priv->crefs > 0);
+  priv->crefs--;
+
+  /* If the count has decremented to zero and the driver has been unlinked,
+   * then free memory now.
+   */
+
+  if (priv->crefs <= 0 && priv->unlinked)
+    {
+      nxmutex_destroy(&priv->devlock);
+      kmm_free(priv);
+      return 0;
+    }
+
+  nxmutex_unlock(&priv->devlock);
+  return 0;
+}
+#endif
+
+/****************************************************************************
+ * Name: sht4x_read
+ *
+ * Description:
+ *     Character driver interface to sensor for debugging.
+ *
+ ****************************************************************************/
+
+static ssize_t sht4x_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  ssize_t length = 0;
+  struct sht4x_raw_data_s raw_data;
+  struct sht4x_conv_data_s data;
+  int err;
+
+  /* If file position is non-zero, then we're at the end of file. */
+
+  if (filep->f_pos > 0) return 0;
+
+  /* Get exclusive access */
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  if (priv->unlinked)
+    {
+      /* Do not allow operations on unlinked sensors. This allows
+       * sensor use on hot swappable I2C bus.
+       */
+
+      nxmutex_unlock(&priv->devlock);
+      return 0;
+    }
+#endif
+
+  err = sht4x_cmd(priv, PRECISION_READ[priv->precision],
+                  MEASUREMENT_TIMES[priv->precision], &raw_data.raw_temp,
+                  &raw_data.raw_hum);
+  if (err)
+    {
+#ifdef CONFIG_SHT4X_DEBUG
+      sht4x_dbg("Could not read device: %d\n", err);
+#endif // CONFIG_SHT4X_DEBUG
+      nxmutex_unlock(&priv->devlock);
+      return err;
+    }
+
+  /* Convert to proper units. */
+
+  data.temp = sht4x_calc_temp(raw_data.raw_temp);
+  data.hum = sht4x_calc_hum(raw_data.raw_hum);
+
+  length = snprintf(buffer, buflen, "%ld m" SHT4X_TEMP_UNIT ", %d %%RH\n",
+                    data.temp, data.hum / 100);
+  if (length > buflen) length = buflen;
+
+  filep->f_pos += length;
+  nxmutex_unlock(&priv->devlock);
+  return length;
+}
+
+/****************************************************************************
+ * Name: sht4x_write
+ *
+ * Description:
+ *     Not implemented.
+ ****************************************************************************/
+
+static ssize_t sht4x_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: sht4x_ioctl
+ ****************************************************************************/
+
+static int sht4x_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct sht4x_dev_s *priv = inode->i_private;
+  int err;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  if (priv->unlinked)
+    {
+      /* Do not allow operations on unlinked sensors. This allows
+       * sensor use on hot swappable I2C bus.
+       */
+
+      nxmutex_unlock(&priv->devlock);
+      return -ENODEV;
+    }
+#endif
+
+  switch (cmd)
+    {
+    case SNIOC_RESET:
+      err = sht4x_reset(priv);
+      break;
+
+    case SNIOC_WHO_AM_I:
+      {
+        union sht4x_serialno_t serialno;
+        err = sht4x_cmd(priv, SHT4X_READ_SERIAL, 10, &serialno.halves.msb,
+                        &serialno.halves.lsb);
+        *((uint32_t *)(arg)) = serialno.full;
+      }
+      break;
+
+    case SNIOC_READ_RAW_DATA:
+      err = sht4x_cmd(priv, PRECISION_READ[priv->precision],
+                      MEASUREMENT_TIMES[priv->precision],
+                      &((struct sht4x_raw_data_s *)(arg))->raw_temp,
+                      &((struct sht4x_raw_data_s *)(arg))->raw_hum);
+      break;
+
+    case SNIOC_MEASURE:
+    case SNIOC_READ_CONVERT_DATA:
+      {
+        uint16_t raw_t;
+        uint16_t raw_h;
+
+        err = sht4x_cmd(priv, PRECISION_READ[priv->precision],
+                        MEASUREMENT_TIMES[priv->precision], &raw_t, &raw_h);
+        ((struct sht4x_conv_data_s *)(arg))->temp = sht4x_calc_temp(raw_t);
+        ((struct sht4x_conv_data_s *)(arg))->hum = sht4x_calc_hum(raw_h);
+      }
+      break;
+
+    case SNIOC_HEAT:
+      {
+        struct timespec now;
+        clock_systime_timespec(&now);
+
+        /* Check if it has been one second since the last heat command. */
+
+        if (!has_time_passed(now, priv->last_heat, 1))
+          {
+            err = -EAGAIN; /* Signal to try again in some time. */
+            break;
+          }
+
+        /* Caller must pass heater option in the temperature argument. */
+
+        uint16_t raw_t = ((struct sht4x_conv_data_s *)(arg))->temp;
+        uint16_t raw_h;
+
+        err = sht4x_cmd(priv, HEAT_CMDS[raw_t], HEAT_TIMES[raw_t], &raw_t,
+                        &raw_h);
+        ((struct sht4x_conv_data_s *)(arg))->temp = sht4x_calc_temp(raw_t);
+        ((struct sht4x_conv_data_s *)(arg))->hum = sht4x_calc_hum(raw_h);
+        clock_systime_timespec(&priv->last_heat); /* Update last heat time. */
+      }
+      break;
+
+    case SNIOC_CONFIGURE:
+
+      /* Caller must pass precision option as argument. */
+
+      priv->precision = arg;
+#ifdef CONFIG_SHT4X_DEBUG
+      sht4x_dbg("Precision set to %d\n", priv->precision);
+#endif // CONFIG_SHT4X_DEBUG
+
+      break;
+
+    default:
+      err = -EINVAL;
+      break;
+    }
+
+  nxmutex_unlock(&priv->devlock);
+  return err;
+}
+
+/****************************************************************************
+ * Name: sht4x_unlink
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int sht4x_unlink(FAR struct inode *inode)
+{
+  FAR struct sht4x_dev_s *priv;
+  int err;
+
+  DEBUGASSERT(inode->i_private != NULL);
+  priv = inode->i_private;
+
+  err = nxmutex_lock(&priv->devlock);
+  if (err) return err;
+
+  /* Are there open references to the driver data structure? */
+
+  if (priv->crefs <= 0)
+    {
+      nxmutex_destroy(&priv->devlock);
+      kmm_free(priv);
+      return 0;
+    }
+
+  /* No. Just mark the driver as unlinked and free the resources when
+   * the last client closes their reference to the driver.
+   */
+
+  priv->unlinked = true;
+  nxmutex_unlock(&priv->devlock);
+  return OK;
+}
+#endif
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: sht4x_register
+ *
+ * Description:
+ *   Register the SHT4X character device as 'devpath'
+ *
+ * Input Parameters:
+ *   devpath - The full path to the driver to register. E.g., "/dev/temp0"
+ *   i2c     - An instance of the I2C interface to use to communicate with
+ *             the SHT4X
+ *   addr    - The I2C address of the SHT4X. The I2C address is one of 0x44,
+ *0x45 and 0x46.
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int sht4x_register(FAR const char *devpath, FAR struct i2c_master_s *i2c,
+                   uint8_t addr)
+{
+  FAR struct sht4x_dev_s *priv;
+  int err;
+
+  DEBUGASSERT(i2c != NULL);
+  DEBUGASSERT(addr == 0x44 || addr == 0x45 || addr == 0x46);
+
+  /* Initialize the device structure */
+
+  priv = kmm_zalloc(sizeof(struct sht4x_dev_s));
+  if (priv == NULL)
+    {
+      snerr("ERROR: Failed to allocate instance.\n");
+      return -ENOMEM;
+    }
+
+  priv->i2c = i2c;
+  priv->addr = addr;
+  priv->precision = SHT4X_PREC_HIGH;
+  err = clock_systime_timespec(&priv->last_heat);
+
+  if (err)
+    {
+      snerr("ERROR: Failed to get timespec: %d\n", err);
+      kmm_free(priv);
+      return err;
+    }
+
+  /* Allow heat immediately after registration since in theory the sensor has
+   * never had its heater activated yet.
+   */
+
+  priv->last_heat.tv_sec -= 1;
+
+  err = nxmutex_init(&priv->devlock);
+  if (err)
+    {
+      snerr("ERROR: Failed to register SHT4X driver: %d\n", err);
+      kmm_free(priv);
+      return err;
+    }
+
+  /* Register the character driver */
+
+  err = register_driver(devpath, &g_sht4xfops, 0666, priv);
+  if (err < 0)

Review Comment:
   let's unify the error ccheck by `if (err < 0)` instead `if (err)`



##########
drivers/sensors/sht4x.c:
##########
@@ -0,0 +1,827 @@
+/****************************************************************************
+ * drivers/sensors/sht4x.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <debug.h>
+#include <nuttx/clock.h>
+#include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/i2c/i2c_master.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mutex.h>
+#include <nuttx/random.h>
+#include <nuttx/sensors/sht4x.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#if defined(CONFIG_I2C) && defined(CONFIG_SENSORS_SHT4X)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_DEBUG
+#define sht4x_dbg(x, ...) _info(x, ##__VA_ARGS__)
+#endif
+
+#if defined(CONFIG_SHT4X_FAHRENHEIT)
+#define SHT4X_TEMP_UNIT "F"
+#else
+#define SHT4X_TEMP_UNIT "C"
+#endif // defined(CONFIG_SHT4X_FAHRENHEIT)
+
+#ifndef CONFIG_SHT4X_I2C_FREQUENCY
+#define CONFIG_SHT4X_I2C_FREQUENCY 400000
+#endif // CONFIG_SHT4X_I2C_FREQUENCY
+
+#define SHT4X_CRC_INIT 0xFF /* Initial value of the calculated CRC. */
+#define SHT4X_CRC_POLY 0x31 /* CRC calculation polynomial. */
+
+#define SHT4X_SOFT_RESET 0x94     /* Soft reset command. */
+#define SHT4X_READ_SERIAL 0x89    /* Read serial number command. */
+#define SHT4X_READ_LOW_PREC 0xE0  /* Low precision read temp & humidity. */
+#define SHT4X_READ_MED_PREC 0xF6  /* Med precision read temp & humidity. */
+#define SHT4X_READ_HIGH_PREC 0xFD /* High precision read temp & humidity. */
+#define SHT4X_HEAT_200_1 0x39     /* Activate heater with 200mW for 1s. */
+#define SHT4X_HEAT_200_P1 0x32    /* Activate heater with 200mW for 0.1s. */
+#define SHT4X_HEAT_110_1 0x2F     /* Activate heater with 110mW for 1s. */
+#define SHT4X_HEAT_110_P1 0x24    /* Activate heater with 110mW for 0.1s. */
+#define SHT4X_HEAT_20_1 0x1E      /* Activate heater with 20mW for 1s. */
+#define SHT4X_HEAT_20_P1 0x15     /* Activate heater with 20mW for 0.1s. */
+
+/****************************************************************************
+ * Private
+ ****************************************************************************/
+
+struct sht4x_dev_s
+{
+  FAR struct i2c_master_s *i2c; /* I2C interface. */
+  uint8_t addr;                 /* I2C address. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  bool unlinked;                    /* True, driver has been unlinked. */
+#endif                              // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  struct timespec last_heat;        /* Last time heater was active. */
+  enum sht4x_precision_e precision; /* The precision for read operations. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  int16_t crefs; /* Number of open references. */
+#endif           // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  mutex_t devlock;
+};
+
+/* Easy unpacking of serial number from I2C packet. */
+
+union sht4x_serialno_t
+{
+  uint32_t full; /* Full serial number. */
+  struct
+  {
+    uint16_t msb; /* Most significant 2 bytes. */
+    uint16_t lsb; /* Least significant 2 bytes. */
+  } halves;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Commands for reading at different precisions */
+
+const uint8_t PRECISION_READ[] =

Review Comment:
   static



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "LuchianMihai (via GitHub)" <gi...@apache.org>.
LuchianMihai commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1681568483


##########
drivers/sensors/sht4x.c:
##########
@@ -0,0 +1,827 @@
+/****************************************************************************
+ * drivers/sensors/sht4x.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <debug.h>
+#include <nuttx/clock.h>
+#include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/i2c/i2c_master.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mutex.h>
+#include <nuttx/random.h>
+#include <nuttx/sensors/sht4x.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#if defined(CONFIG_I2C) && defined(CONFIG_SENSORS_SHT4X)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_DEBUG
+#define sht4x_dbg(x, ...) _info(x, ##__VA_ARGS__)
+#endif
+
+#if defined(CONFIG_SHT4X_FAHRENHEIT)
+#define SHT4X_TEMP_UNIT "F"
+#else
+#define SHT4X_TEMP_UNIT "C"
+#endif // defined(CONFIG_SHT4X_FAHRENHEIT)
+
+#ifndef CONFIG_SHT4X_I2C_FREQUENCY
+#define CONFIG_SHT4X_I2C_FREQUENCY 400000
+#endif // CONFIG_SHT4X_I2C_FREQUENCY
+
+#define SHT4X_CRC_INIT 0xFF /* Initial value of the calculated CRC. */
+#define SHT4X_CRC_POLY 0x31 /* CRC calculation polynomial. */
+
+#define SHT4X_SOFT_RESET 0x94     /* Soft reset command. */
+#define SHT4X_READ_SERIAL 0x89    /* Read serial number command. */
+#define SHT4X_READ_LOW_PREC 0xE0  /* Low precision read temp & humidity. */
+#define SHT4X_READ_MED_PREC 0xF6  /* Med precision read temp & humidity. */
+#define SHT4X_READ_HIGH_PREC 0xFD /* High precision read temp & humidity. */
+#define SHT4X_HEAT_200_1 0x39     /* Activate heater with 200mW for 1s. */
+#define SHT4X_HEAT_200_P1 0x32    /* Activate heater with 200mW for 0.1s. */
+#define SHT4X_HEAT_110_1 0x2F     /* Activate heater with 110mW for 1s. */
+#define SHT4X_HEAT_110_P1 0x24    /* Activate heater with 110mW for 0.1s. */
+#define SHT4X_HEAT_20_1 0x1E      /* Activate heater with 20mW for 1s. */
+#define SHT4X_HEAT_20_P1 0x15     /* Activate heater with 20mW for 0.1s. */
+
+/****************************************************************************
+ * Private
+ ****************************************************************************/
+
+struct sht4x_dev_s
+{
+  FAR struct i2c_master_s *i2c; /* I2C interface. */
+  uint8_t addr;                 /* I2C address. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  bool unlinked;                    /* True, driver has been unlinked. */
+#endif                              // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  struct timespec last_heat;        /* Last time heater was active. */
+  enum sht4x_precision_e precision; /* The precision for read operations. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  int16_t crefs; /* Number of open references. */
+#endif           // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  mutex_t devlock;
+};
+
+/* Easy unpacking of serial number from I2C packet. */
+
+union sht4x_serialno_t
+{
+  uint32_t full; /* Full serial number. */
+  struct
+  {
+    uint16_t msb; /* Most significant 2 bytes. */
+    uint16_t lsb; /* Least significant 2 bytes. */
+  } halves;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Commands for reading at different precisions */
+
+const uint8_t PRECISION_READ[] =
+{
+    [SHT4X_PREC_LOW] = SHT4X_READ_LOW_PREC,
+    [SHT4X_PREC_MED] = SHT4X_READ_MED_PREC,
+    [SHT4X_PREC_HIGH] = SHT4X_READ_HIGH_PREC,
+};
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/* CRC lookup table. */
+
+static const uint8_t CRC_LOOKUP[] =
+    {
+        0x0,  0x31, 0x62, 0x53, 0xc4, 0xf5, 0xa6, 0x97, 0xb9, 0x88, 0xdb,
+        0xea, 0x7d, 0x4c, 0x1f, 0x2e, 0x43, 0x72, 0x21, 0x10, 0x87, 0xb6,
+        0xe5, 0xd4, 0xfa, 0xcb, 0x98, 0xa9, 0x3e, 0xf,  0x5c, 0x6d, 0x86,
+        0xb7, 0xe4, 0xd5, 0x42, 0x73, 0x20, 0x11, 0x3f, 0xe,  0x5d, 0x6c,
+        0xfb, 0xca, 0x99, 0xa8, 0xc5, 0xf4, 0xa7, 0x96, 0x1,  0x30, 0x63,
+        0x52, 0x7c, 0x4d, 0x1e, 0x2f, 0xb8, 0x89, 0xda, 0xeb, 0x3d, 0xc,
+        0x5f, 0x6e, 0xf9, 0xc8, 0x9b, 0xaa, 0x84, 0xb5, 0xe6, 0xd7, 0x40,
+        0x71, 0x22, 0x13, 0x7e, 0x4f, 0x1c, 0x2d, 0xba, 0x8b, 0xd8, 0xe9,
+        0xc7, 0xf6, 0xa5, 0x94, 0x3,  0x32, 0x61, 0x50, 0xbb, 0x8a, 0xd9,
+        0xe8, 0x7f, 0x4e, 0x1d, 0x2c, 0x2,  0x33, 0x60, 0x51, 0xc6, 0xf7,
+        0xa4, 0x95, 0xf8, 0xc9, 0x9a, 0xab, 0x3c, 0xd,  0x5e, 0x6f, 0x41,
+        0x70, 0x23, 0x12, 0x85, 0xb4, 0xe7, 0xd6, 0x7a, 0x4b, 0x18, 0x29,
+        0xbe, 0x8f, 0xdc, 0xed, 0xc3, 0xf2, 0xa1, 0x90, 0x7,  0x36, 0x65,
+        0x54, 0x39, 0x8,  0x5b, 0x6a, 0xfd, 0xcc, 0x9f, 0xae, 0x80, 0xb1,
+        0xe2, 0xd3, 0x44, 0x75, 0x26, 0x17, 0xfc, 0xcd, 0x9e, 0xaf, 0x38,
+        0x9,  0x5a, 0x6b, 0x45, 0x74, 0x27, 0x16, 0x81, 0xb0, 0xe3, 0xd2,
+        0xbf, 0x8e, 0xdd, 0xec, 0x7b, 0x4a, 0x19, 0x28, 0x6,  0x37, 0x64,
+        0x55, 0xc2, 0xf3, 0xa0, 0x91, 0x47, 0x76, 0x25, 0x14, 0x83, 0xb2,
+        0xe1, 0xd0, 0xfe, 0xcf, 0x9c, 0xad, 0x3a, 0xb,  0x58, 0x69, 0x4,
+        0x35, 0x66, 0x57, 0xc0, 0xf1, 0xa2, 0x93, 0xbd, 0x8c, 0xdf, 0xee,
+        0x79, 0x48, 0x1b, 0x2a, 0xc1, 0xf0, 0xa3, 0x92, 0x5,  0x34, 0x67,
+        0x56, 0x78, 0x49, 0x1a, 0x2b, 0xbc, 0x8d, 0xde, 0xef, 0x82, 0xb3,
+        0xe0, 0xd1, 0x46, 0x77, 0x24, 0x15, 0x3b, 0xa,  0x59, 0x68, 0xff,
+        0xce, 0x9d, 0xac,
+}
+#endif // CONFIG_SHT4X_CRC_LOOKUP
+
+/* Measurement times for the various precisions, in microseconds. */
+
+static const uint16_t MEASUREMENT_TIMES[] =
+{
+    [SHT4X_PREC_LOW] = 1600,
+    [SHT4X_PREC_MED] = 4500,
+    [SHT4X_PREC_HIGH] = 8300,
+};
+
+/* Commands for the various heating options. */
+
+static const uint8_t HEAT_CMDS[] =
+{
+    [SHT4X_HEATER_200MW_1] = SHT4X_HEAT_200_1,
+    [SHT4X_HEATER_200MW_01] = SHT4X_HEAT_200_P1,
+    [SHT4X_HEATER_110MW_1] = SHT4X_HEAT_110_1,
+    [SHT4X_HEATER_110MW_01] = SHT4X_HEAT_110_P1,
+    [SHT4X_HEATER_20MW_1] = SHT4X_HEAT_20_1,
+    [SHT4X_HEATER_20MW_01] = SHT4X_HEAT_20_P1,
+};
+
+/* Timeouts for the various heating options in microseconds. */
+
+static const uint32_t HEAT_TIMES[] =
+{
+    [SHT4X_HEATER_200MW_1] = 1000000, [SHT4X_HEATER_200MW_01] = 100000,
+    [SHT4X_HEATER_110MW_1] = 1000000, [SHT4X_HEATER_110MW_01] = 100000,
+    [SHT4X_HEATER_20MW_1] = 1000000,  [SHT4X_HEATER_20MW_01] = 100000,
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int sht4x_open(FAR struct file *filep);
+static int sht4x_close(FAR struct file *filep);
+static ssize_t sht4x_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static ssize_t sht4x_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static int sht4x_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static int sht4x_unlink(FAR struct inode *inode);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations g_sht4xfops =
+{
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .open = sht4x_open,
+    .close = sht4x_close,
+#else
+    .open = NULL,
+    .close = NULL,
+#endif
+    .read = sht4x_read,
+    .write = sht4x_write,
+    .seek = NULL,
+    .ioctl = sht4x_ioctl,
+    .mmap = NULL,
+    .truncate = NULL,
+    .poll = NULL,
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .unlink = sht4x_unlink,
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/****************************************************************************
+ * Name: sht4x_crc_lookup
+ *
+ * Description:
+ *   Perform a CRC calculation using the lookup table.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_lookup(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc = CRC_LOOKUP[crc ^ buf[byte]];
+    }
+
+  return crc;
+}
+
+#endif
+
+/****************************************************************************
+ * Name: sht4x_crc_bitwise
+ *
+ * Description:
+ *   Perform a CRC calculation using the bitwise calculation.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_bitwise(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc ^= buf[byte];
+      for (uint8_t bit = 0; bit < 8; bit++)
+        {
+          if (crc & 0x80)
+            {
+              /* Discard the highest bit (implicit XOR), then divide by the
+               * polynomial.
+               */
+
+              crc <<= 1;
+              crc ^= SHT4X_CRC_POLY;
+            }
+          else
+            {
+              /* Continue until the highest bit is set. */
+
+              crc <<= 1;
+            }
+        }
+    }
+
+  return crc;
+}
+
+/****************************************************************************
+ * Name: sht4x_crc
+ *
+ * Description:
+ *   Perform a CRC calculation on the data to check for validity.
+ *   The `buf` param must be a data buffer that ends with its own 8 bit CRC.
+ *   The `nbytes` param is the length of the data including the 8 bit CRC.
+ *
+ ****************************************************************************/
+
+static int sht4x_crc(const uint8_t *buf, uint8_t nbytes)
+{
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+  if (sht4x_crc_lookup(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+#else
+  if (sht4x_crc_bitwise(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+
+#endif
+  return 0;
+}
+
+/****************************************************************************
+ * Name: sht4x_cmd
+ *
+ * Description:
+ *   I2C access helper for sending commands to the SHT4x which receive data
+ *   in response.
+ *
+ ****************************************************************************/
+
+static int sht4x_cmd(FAR struct sht4x_dev_s *priv, uint8_t command,
+                     uint32_t timeout, uint16_t *data1, uint16_t *data2)
+{
+  struct i2c_msg_s cmd;
+  struct i2c_msg_s read_data;
+  int err;
+
+  /* 2 byte data, 1 byte CRC, 2 byte data, 1 byte CRC. */
+
+  uint8_t combined_data[6];
+
+  cmd.frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  cmd.addr = priv->addr;
+  cmd.flags = 0;
+  cmd.buffer = &command;
+  cmd.length = sizeof(command);
+
+  /* Trigger measurement. */
+
+  err = I2C_TRANSFER(priv->i2c, &cmd, 1);
+  if (err) return err;

Review Comment:
   Hi, I am definitely not the right person to do an review, I'm as much of a new contributor as you are.
   But it happens to also develop an driver for an sensor, so I use your PR, and other sensors as guidelines.
   Here if will return true on all non zero err values, even negative ones, but throughout other sensor drivers I see explicit check of non zero negative values `if (err < 0)`. Also, I see errors returned as strictly negative values. I know your driver behave as expected because the success/OK value is defined as 0.
   Is this by design?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1684370463


##########
boards/arm/rp2040/common/src/rp2040_common_bringup.c:
##########
@@ -68,6 +68,11 @@
 #include "rp2040_bmp280.h"
 #endif
 
+#ifdef CONFIG_SENSORS_SHT4X
+#include <nuttx/sensors/sht4x.h>
+#include "rp2040_i2c.h"

Review Comment:
   @acassis I do use that function call, which is why I included the I2C header you mentioned in your last comment.
   
   The reason I left that function call in the rp2040 bring up is because it seems like the architecture specific logic should stay out of the driver, since I want it to work on all architectures with I2C. I have seen this done for the SHT3x and SHT21. It made more sense to me to keep it in the architecture bring-up logic.
   
   I was hoping to find some way to access the I2C driver structure without an architecture specific initialization function, but since I haven't been able to, I think this way ensures that the driver can run on all architectures.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#issuecomment-2242911349

   @jerpelea I've now added a description of the changes alongside my previous title!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1684272861


##########
boards/arm/rp2040/common/src/rp2040_common_bringup.c:
##########
@@ -68,6 +68,11 @@
 #include "rp2040_bmp280.h"
 #endif
 
+#ifdef CONFIG_SENSORS_SHT4X
+#include <nuttx/sensors/sht4x.h>
+#include "rp2040_i2c.h"

Review Comment:
   You can use rp2040_i2cbus_initialize(busno), it is not board specific. It is arch specific, in fact I think we need some chip_i2cbus_initialize() generic enough to let us move the common/ to the root of boards/



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#issuecomment-2244020428

   Hey @acassis, if I were to write some kind of post about my experience creating a driver for NuttX, what would be the best place to put that?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1683445954


##########
boards/arm/rp2040/common/src/rp2040_common_bringup.c:
##########
@@ -68,6 +68,11 @@
 #include "rp2040_bmp280.h"
 #endif
 
+#ifdef CONFIG_SENSORS_SHT4X
+#include <nuttx/sensors/sht4x.h>
+#include "rp2040_i2c.h"

Review Comment:
   @acassis Do you know how using the bus number I can acquire the `i2c_master_s` struct within the register function? I much prefer that solution but don't know how to access the i2c driver without using the board specific `i2cbus_initialize` function.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1681597340


##########
drivers/sensors/sht4x.c:
##########
@@ -0,0 +1,827 @@
+/****************************************************************************
+ * drivers/sensors/sht4x.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <debug.h>
+#include <nuttx/clock.h>
+#include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/i2c/i2c_master.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mutex.h>
+#include <nuttx/random.h>
+#include <nuttx/sensors/sht4x.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#if defined(CONFIG_I2C) && defined(CONFIG_SENSORS_SHT4X)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_DEBUG
+#define sht4x_dbg(x, ...) _info(x, ##__VA_ARGS__)
+#endif
+
+#if defined(CONFIG_SHT4X_FAHRENHEIT)
+#define SHT4X_TEMP_UNIT "F"
+#else
+#define SHT4X_TEMP_UNIT "C"
+#endif // defined(CONFIG_SHT4X_FAHRENHEIT)
+
+#ifndef CONFIG_SHT4X_I2C_FREQUENCY
+#define CONFIG_SHT4X_I2C_FREQUENCY 400000
+#endif // CONFIG_SHT4X_I2C_FREQUENCY
+
+#define SHT4X_CRC_INIT 0xFF /* Initial value of the calculated CRC. */
+#define SHT4X_CRC_POLY 0x31 /* CRC calculation polynomial. */
+
+#define SHT4X_SOFT_RESET 0x94     /* Soft reset command. */
+#define SHT4X_READ_SERIAL 0x89    /* Read serial number command. */
+#define SHT4X_READ_LOW_PREC 0xE0  /* Low precision read temp & humidity. */
+#define SHT4X_READ_MED_PREC 0xF6  /* Med precision read temp & humidity. */
+#define SHT4X_READ_HIGH_PREC 0xFD /* High precision read temp & humidity. */
+#define SHT4X_HEAT_200_1 0x39     /* Activate heater with 200mW for 1s. */
+#define SHT4X_HEAT_200_P1 0x32    /* Activate heater with 200mW for 0.1s. */
+#define SHT4X_HEAT_110_1 0x2F     /* Activate heater with 110mW for 1s. */
+#define SHT4X_HEAT_110_P1 0x24    /* Activate heater with 110mW for 0.1s. */
+#define SHT4X_HEAT_20_1 0x1E      /* Activate heater with 20mW for 1s. */
+#define SHT4X_HEAT_20_P1 0x15     /* Activate heater with 20mW for 0.1s. */
+
+/****************************************************************************
+ * Private
+ ****************************************************************************/
+
+struct sht4x_dev_s
+{
+  FAR struct i2c_master_s *i2c; /* I2C interface. */
+  uint8_t addr;                 /* I2C address. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  bool unlinked;                    /* True, driver has been unlinked. */
+#endif                              // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  struct timespec last_heat;        /* Last time heater was active. */
+  enum sht4x_precision_e precision; /* The precision for read operations. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  int16_t crefs; /* Number of open references. */
+#endif           // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  mutex_t devlock;
+};
+
+/* Easy unpacking of serial number from I2C packet. */
+
+union sht4x_serialno_t
+{
+  uint32_t full; /* Full serial number. */
+  struct
+  {
+    uint16_t msb; /* Most significant 2 bytes. */
+    uint16_t lsb; /* Least significant 2 bytes. */
+  } halves;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Commands for reading at different precisions */
+
+const uint8_t PRECISION_READ[] =
+{
+    [SHT4X_PREC_LOW] = SHT4X_READ_LOW_PREC,
+    [SHT4X_PREC_MED] = SHT4X_READ_MED_PREC,
+    [SHT4X_PREC_HIGH] = SHT4X_READ_HIGH_PREC,
+};
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/* CRC lookup table. */
+
+static const uint8_t CRC_LOOKUP[] =
+    {
+        0x0,  0x31, 0x62, 0x53, 0xc4, 0xf5, 0xa6, 0x97, 0xb9, 0x88, 0xdb,
+        0xea, 0x7d, 0x4c, 0x1f, 0x2e, 0x43, 0x72, 0x21, 0x10, 0x87, 0xb6,
+        0xe5, 0xd4, 0xfa, 0xcb, 0x98, 0xa9, 0x3e, 0xf,  0x5c, 0x6d, 0x86,
+        0xb7, 0xe4, 0xd5, 0x42, 0x73, 0x20, 0x11, 0x3f, 0xe,  0x5d, 0x6c,
+        0xfb, 0xca, 0x99, 0xa8, 0xc5, 0xf4, 0xa7, 0x96, 0x1,  0x30, 0x63,
+        0x52, 0x7c, 0x4d, 0x1e, 0x2f, 0xb8, 0x89, 0xda, 0xeb, 0x3d, 0xc,
+        0x5f, 0x6e, 0xf9, 0xc8, 0x9b, 0xaa, 0x84, 0xb5, 0xe6, 0xd7, 0x40,
+        0x71, 0x22, 0x13, 0x7e, 0x4f, 0x1c, 0x2d, 0xba, 0x8b, 0xd8, 0xe9,
+        0xc7, 0xf6, 0xa5, 0x94, 0x3,  0x32, 0x61, 0x50, 0xbb, 0x8a, 0xd9,
+        0xe8, 0x7f, 0x4e, 0x1d, 0x2c, 0x2,  0x33, 0x60, 0x51, 0xc6, 0xf7,
+        0xa4, 0x95, 0xf8, 0xc9, 0x9a, 0xab, 0x3c, 0xd,  0x5e, 0x6f, 0x41,
+        0x70, 0x23, 0x12, 0x85, 0xb4, 0xe7, 0xd6, 0x7a, 0x4b, 0x18, 0x29,
+        0xbe, 0x8f, 0xdc, 0xed, 0xc3, 0xf2, 0xa1, 0x90, 0x7,  0x36, 0x65,
+        0x54, 0x39, 0x8,  0x5b, 0x6a, 0xfd, 0xcc, 0x9f, 0xae, 0x80, 0xb1,
+        0xe2, 0xd3, 0x44, 0x75, 0x26, 0x17, 0xfc, 0xcd, 0x9e, 0xaf, 0x38,
+        0x9,  0x5a, 0x6b, 0x45, 0x74, 0x27, 0x16, 0x81, 0xb0, 0xe3, 0xd2,
+        0xbf, 0x8e, 0xdd, 0xec, 0x7b, 0x4a, 0x19, 0x28, 0x6,  0x37, 0x64,
+        0x55, 0xc2, 0xf3, 0xa0, 0x91, 0x47, 0x76, 0x25, 0x14, 0x83, 0xb2,
+        0xe1, 0xd0, 0xfe, 0xcf, 0x9c, 0xad, 0x3a, 0xb,  0x58, 0x69, 0x4,
+        0x35, 0x66, 0x57, 0xc0, 0xf1, 0xa2, 0x93, 0xbd, 0x8c, 0xdf, 0xee,
+        0x79, 0x48, 0x1b, 0x2a, 0xc1, 0xf0, 0xa3, 0x92, 0x5,  0x34, 0x67,
+        0x56, 0x78, 0x49, 0x1a, 0x2b, 0xbc, 0x8d, 0xde, 0xef, 0x82, 0xb3,
+        0xe0, 0xd1, 0x46, 0x77, 0x24, 0x15, 0x3b, 0xa,  0x59, 0x68, 0xff,
+        0xce, 0x9d, 0xac,
+}
+#endif // CONFIG_SHT4X_CRC_LOOKUP
+
+/* Measurement times for the various precisions, in microseconds. */
+
+static const uint16_t MEASUREMENT_TIMES[] =
+{
+    [SHT4X_PREC_LOW] = 1600,
+    [SHT4X_PREC_MED] = 4500,
+    [SHT4X_PREC_HIGH] = 8300,
+};
+
+/* Commands for the various heating options. */
+
+static const uint8_t HEAT_CMDS[] =
+{
+    [SHT4X_HEATER_200MW_1] = SHT4X_HEAT_200_1,
+    [SHT4X_HEATER_200MW_01] = SHT4X_HEAT_200_P1,
+    [SHT4X_HEATER_110MW_1] = SHT4X_HEAT_110_1,
+    [SHT4X_HEATER_110MW_01] = SHT4X_HEAT_110_P1,
+    [SHT4X_HEATER_20MW_1] = SHT4X_HEAT_20_1,
+    [SHT4X_HEATER_20MW_01] = SHT4X_HEAT_20_P1,
+};
+
+/* Timeouts for the various heating options in microseconds. */
+
+static const uint32_t HEAT_TIMES[] =
+{
+    [SHT4X_HEATER_200MW_1] = 1000000, [SHT4X_HEATER_200MW_01] = 100000,
+    [SHT4X_HEATER_110MW_1] = 1000000, [SHT4X_HEATER_110MW_01] = 100000,
+    [SHT4X_HEATER_20MW_1] = 1000000,  [SHT4X_HEATER_20MW_01] = 100000,
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int sht4x_open(FAR struct file *filep);
+static int sht4x_close(FAR struct file *filep);
+static ssize_t sht4x_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static ssize_t sht4x_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static int sht4x_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static int sht4x_unlink(FAR struct inode *inode);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations g_sht4xfops =
+{
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .open = sht4x_open,
+    .close = sht4x_close,
+#else
+    .open = NULL,
+    .close = NULL,
+#endif
+    .read = sht4x_read,
+    .write = sht4x_write,
+    .seek = NULL,
+    .ioctl = sht4x_ioctl,
+    .mmap = NULL,
+    .truncate = NULL,
+    .poll = NULL,
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .unlink = sht4x_unlink,
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/****************************************************************************
+ * Name: sht4x_crc_lookup
+ *
+ * Description:
+ *   Perform a CRC calculation using the lookup table.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_lookup(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc = CRC_LOOKUP[crc ^ buf[byte]];
+    }
+
+  return crc;
+}
+
+#endif
+
+/****************************************************************************
+ * Name: sht4x_crc_bitwise
+ *
+ * Description:
+ *   Perform a CRC calculation using the bitwise calculation.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_bitwise(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc ^= buf[byte];
+      for (uint8_t bit = 0; bit < 8; bit++)
+        {
+          if (crc & 0x80)
+            {
+              /* Discard the highest bit (implicit XOR), then divide by the
+               * polynomial.
+               */
+
+              crc <<= 1;
+              crc ^= SHT4X_CRC_POLY;
+            }
+          else
+            {
+              /* Continue until the highest bit is set. */
+
+              crc <<= 1;
+            }
+        }
+    }
+
+  return crc;
+}
+
+/****************************************************************************
+ * Name: sht4x_crc
+ *
+ * Description:
+ *   Perform a CRC calculation on the data to check for validity.
+ *   The `buf` param must be a data buffer that ends with its own 8 bit CRC.
+ *   The `nbytes` param is the length of the data including the 8 bit CRC.
+ *
+ ****************************************************************************/
+
+static int sht4x_crc(const uint8_t *buf, uint8_t nbytes)
+{
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+  if (sht4x_crc_lookup(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+#else
+  if (sht4x_crc_bitwise(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+
+#endif
+  return 0;
+}
+
+/****************************************************************************
+ * Name: sht4x_cmd
+ *
+ * Description:
+ *   I2C access helper for sending commands to the SHT4x which receive data
+ *   in response.
+ *
+ ****************************************************************************/
+
+static int sht4x_cmd(FAR struct sht4x_dev_s *priv, uint8_t command,
+                     uint32_t timeout, uint16_t *data1, uint16_t *data2)
+{
+  struct i2c_msg_s cmd;
+  struct i2c_msg_s read_data;
+  int err;
+
+  /* 2 byte data, 1 byte CRC, 2 byte data, 1 byte CRC. */
+
+  uint8_t combined_data[6];
+
+  cmd.frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  cmd.addr = priv->addr;
+  cmd.flags = 0;
+  cmd.buffer = &command;
+  cmd.length = sizeof(command);
+
+  /* Trigger measurement. */
+
+  err = I2C_TRANSFER(priv->i2c, &cmd, 1);
+  if (err) return err;

Review Comment:
   Hello! Thanks for your review! That's a good point, I'm not sure of the reason why other drivers explicitly check for negative values. My reasoning was that only a '0' is success, so I want to return the error for all non-zero values. Maybe there's something I'm missing though, seeing as the other examples seem to be explicitly checking for negative values.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1683389095


##########
boards/arm/rp2040/common/src/rp2040_common_bringup.c:
##########
@@ -68,6 +68,11 @@
 #include "rp2040_bmp280.h"
 #endif
 
+#ifdef CONFIG_SENSORS_SHT4X
+#include <nuttx/sensors/sht4x.h>
+#include "rp2040_i2c.h"

Review Comment:
   I think you don't need to include i2c header file here. Modify sht4x_register() to accept the busno instead of passing the i2c_master struct pointer to it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1683445954


##########
boards/arm/rp2040/common/src/rp2040_common_bringup.c:
##########
@@ -68,6 +68,11 @@
 #include "rp2040_bmp280.h"
 #endif
 
+#ifdef CONFIG_SENSORS_SHT4X
+#include <nuttx/sensors/sht4x.h>
+#include "rp2040_i2c.h"

Review Comment:
   Do you know how using the bus number I can acquire the `i2c_master_s` struct within the register function? I much prefer that solution but don't know how to access the i2c driver without using the board specific `i2cbus_initialize` function.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#issuecomment-2232112820

   I am only just now seeing that some sensors (such as the HYT271) implement a different interface which use the `sensor_*` types (in this case `sensor_humi` and `sensor_temp`) and a "lower" and "upper" half. Is there a preference as to which style is followed? I used the SHT21 and SHT3x sensor drivers as a guide to my own, but I am now wondering about this other interface style which even includes a polling thread for periodic measurement.
   
   I suppose there's a trade-off between having an interface which is generically listed as `/dev/temp0` for just wanting to read temperature data, versus something more descriptive such as `/dev/sht4x0` which allows for specialized `SNIOC_*` controls over things like a heater, etc.
   
   As another aside, is there a good way to document the IOCTL interface to a sensor driver? The SHT4X driver makes use of existing `SNIOC` commands, but uses different arguments when compared to some of the other sensors using the same commands. I don't feel like adding new set of `SNIOC` commands is a good solution, but I don't see any publicly facing docs for the sensor interfaces?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1683386489


##########
drivers/sensors/sht4x.c:
##########
@@ -0,0 +1,828 @@
+/****************************************************************************
+ * drivers/sensors/sht4x.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <debug.h>
+#include <nuttx/clock.h>
+#include <nuttx/config.h>
+#include <nuttx/fs/fs.h>
+#include <nuttx/i2c/i2c_master.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mutex.h>
+#include <nuttx/random.h>
+#include <nuttx/sensors/sht4x.h>
+
+#include <stdio.h>
+#include <unistd.h>
+
+#if defined(CONFIG_I2C) && defined(CONFIG_SENSORS_SHT4X)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_DEBUG
+#define sht4x_dbg(x, ...) _info(x, ##__VA_ARGS__)
+#endif
+
+#if defined(CONFIG_SHT4X_FAHRENHEIT)
+#define SHT4X_TEMP_UNIT "F"
+#else
+#define SHT4X_TEMP_UNIT "C"
+#endif // defined(CONFIG_SHT4X_FAHRENHEIT)
+
+#ifndef CONFIG_SHT4X_I2C_FREQUENCY
+#define CONFIG_SHT4X_I2C_FREQUENCY 400000
+#endif // CONFIG_SHT4X_I2C_FREQUENCY
+
+#define SHT4X_CRC_INIT 0xFF /* Initial value of the calculated CRC. */
+#define SHT4X_CRC_POLY 0x31 /* CRC calculation polynomial. */
+
+#define SHT4X_SOFT_RESET 0x94     /* Soft reset command. */
+#define SHT4X_READ_SERIAL 0x89    /* Read serial number command. */
+#define SHT4X_READ_LOW_PREC 0xE0  /* Low precision read temp & humidity. */
+#define SHT4X_READ_MED_PREC 0xF6  /* Med precision read temp & humidity. */
+#define SHT4X_READ_HIGH_PREC 0xFD /* High precision read temp & humidity. */
+#define SHT4X_HEAT_200_1 0x39     /* Activate heater with 200mW for 1s. */
+#define SHT4X_HEAT_200_P1 0x32    /* Activate heater with 200mW for 0.1s. */
+#define SHT4X_HEAT_110_1 0x2F     /* Activate heater with 110mW for 1s. */
+#define SHT4X_HEAT_110_P1 0x24    /* Activate heater with 110mW for 0.1s. */
+#define SHT4X_HEAT_20_1 0x1E      /* Activate heater with 20mW for 1s. */
+#define SHT4X_HEAT_20_P1 0x15     /* Activate heater with 20mW for 0.1s. */
+
+/****************************************************************************
+ * Private
+ ****************************************************************************/
+
+struct sht4x_dev_s
+{
+  FAR struct i2c_master_s *i2c; /* I2C interface. */
+  uint8_t addr;                 /* I2C address. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  bool unlinked;                    /* True, driver has been unlinked. */
+#endif                              // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  struct timespec last_heat;        /* Last time heater was active. */
+  enum sht4x_precision_e precision; /* The precision for read operations. */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  int16_t crefs; /* Number of open references. */
+#endif           // CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  mutex_t devlock;
+};
+
+/* Easy unpacking of serial number from I2C packet. */
+
+union sht4x_serialno_t
+{
+  uint32_t full; /* Full serial number. */
+  struct
+  {
+    uint16_t msb; /* Most significant 2 bytes. */
+    uint16_t lsb; /* Least significant 2 bytes. */
+  } halves;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Commands for reading at different precisions */
+
+static const uint8_t g_precision_read[] =
+{
+    [SHT4X_PREC_LOW] = SHT4X_READ_LOW_PREC,
+    [SHT4X_PREC_MED] = SHT4X_READ_MED_PREC,
+    [SHT4X_PREC_HIGH] = SHT4X_READ_HIGH_PREC,
+};
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/* CRC lookup table. */
+
+static const uint8_t g_crc_lookup[] =
+    {
+        0x0,  0x31, 0x62, 0x53, 0xc4, 0xf5, 0xa6, 0x97, 0xb9, 0x88, 0xdb,
+        0xea, 0x7d, 0x4c, 0x1f, 0x2e, 0x43, 0x72, 0x21, 0x10, 0x87, 0xb6,
+        0xe5, 0xd4, 0xfa, 0xcb, 0x98, 0xa9, 0x3e, 0xf,  0x5c, 0x6d, 0x86,
+        0xb7, 0xe4, 0xd5, 0x42, 0x73, 0x20, 0x11, 0x3f, 0xe,  0x5d, 0x6c,
+        0xfb, 0xca, 0x99, 0xa8, 0xc5, 0xf4, 0xa7, 0x96, 0x1,  0x30, 0x63,
+        0x52, 0x7c, 0x4d, 0x1e, 0x2f, 0xb8, 0x89, 0xda, 0xeb, 0x3d, 0xc,
+        0x5f, 0x6e, 0xf9, 0xc8, 0x9b, 0xaa, 0x84, 0xb5, 0xe6, 0xd7, 0x40,
+        0x71, 0x22, 0x13, 0x7e, 0x4f, 0x1c, 0x2d, 0xba, 0x8b, 0xd8, 0xe9,
+        0xc7, 0xf6, 0xa5, 0x94, 0x3,  0x32, 0x61, 0x50, 0xbb, 0x8a, 0xd9,
+        0xe8, 0x7f, 0x4e, 0x1d, 0x2c, 0x2,  0x33, 0x60, 0x51, 0xc6, 0xf7,
+        0xa4, 0x95, 0xf8, 0xc9, 0x9a, 0xab, 0x3c, 0xd,  0x5e, 0x6f, 0x41,
+        0x70, 0x23, 0x12, 0x85, 0xb4, 0xe7, 0xd6, 0x7a, 0x4b, 0x18, 0x29,
+        0xbe, 0x8f, 0xdc, 0xed, 0xc3, 0xf2, 0xa1, 0x90, 0x7,  0x36, 0x65,
+        0x54, 0x39, 0x8,  0x5b, 0x6a, 0xfd, 0xcc, 0x9f, 0xae, 0x80, 0xb1,
+        0xe2, 0xd3, 0x44, 0x75, 0x26, 0x17, 0xfc, 0xcd, 0x9e, 0xaf, 0x38,
+        0x9,  0x5a, 0x6b, 0x45, 0x74, 0x27, 0x16, 0x81, 0xb0, 0xe3, 0xd2,
+        0xbf, 0x8e, 0xdd, 0xec, 0x7b, 0x4a, 0x19, 0x28, 0x6,  0x37, 0x64,
+        0x55, 0xc2, 0xf3, 0xa0, 0x91, 0x47, 0x76, 0x25, 0x14, 0x83, 0xb2,
+        0xe1, 0xd0, 0xfe, 0xcf, 0x9c, 0xad, 0x3a, 0xb,  0x58, 0x69, 0x4,
+        0x35, 0x66, 0x57, 0xc0, 0xf1, 0xa2, 0x93, 0xbd, 0x8c, 0xdf, 0xee,
+        0x79, 0x48, 0x1b, 0x2a, 0xc1, 0xf0, 0xa3, 0x92, 0x5,  0x34, 0x67,
+        0x56, 0x78, 0x49, 0x1a, 0x2b, 0xbc, 0x8d, 0xde, 0xef, 0x82, 0xb3,
+        0xe0, 0xd1, 0x46, 0x77, 0x24, 0x15, 0x3b, 0xa,  0x59, 0x68, 0xff,
+        0xce, 0x9d, 0xac,
+}
+#endif // CONFIG_SHT4X_CRC_LOOKUP
+
+/* Measurement times for the various precisions, in microseconds. */
+
+static const uint16_t g_measurement_times[] =
+{
+    [SHT4X_PREC_LOW] = 1600,
+    [SHT4X_PREC_MED] = 4500,
+    [SHT4X_PREC_HIGH] = 8300,
+};
+
+/* Commands for the various heating options. */
+
+static const uint8_t g_heat_cmds[] =
+{
+    [SHT4X_HEATER_200MW_1] = SHT4X_HEAT_200_1,
+    [SHT4X_HEATER_200MW_01] = SHT4X_HEAT_200_P1,
+    [SHT4X_HEATER_110MW_1] = SHT4X_HEAT_110_1,
+    [SHT4X_HEATER_110MW_01] = SHT4X_HEAT_110_P1,
+    [SHT4X_HEATER_20MW_1] = SHT4X_HEAT_20_1,
+    [SHT4X_HEATER_20MW_01] = SHT4X_HEAT_20_P1,
+};
+
+/* Timeouts for the various heating options in microseconds. */
+
+static const uint32_t g_heat_times[] =
+{
+    [SHT4X_HEATER_200MW_1] = 1000000, [SHT4X_HEATER_200MW_01] = 100000,
+    [SHT4X_HEATER_110MW_1] = 1000000, [SHT4X_HEATER_110MW_01] = 100000,
+    [SHT4X_HEATER_20MW_1] = 1000000,  [SHT4X_HEATER_20MW_01] = 100000,
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int sht4x_open(FAR struct file *filep);
+static int sht4x_close(FAR struct file *filep);
+static ssize_t sht4x_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static ssize_t sht4x_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static int sht4x_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static int sht4x_unlink(FAR struct inode *inode);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations g_sht4xfops =
+{
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .open = sht4x_open,
+    .close = sht4x_close,
+#else
+    .open = NULL,
+    .close = NULL,
+#endif
+    .read = sht4x_read,
+    .write = sht4x_write,
+    .seek = NULL,
+    .ioctl = sht4x_ioctl,
+    .mmap = NULL,
+    .truncate = NULL,
+    .poll = NULL,
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+    .unlink = sht4x_unlink,
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+
+/****************************************************************************
+ * Name: sht4x_crc_lookup
+ *
+ * Description:
+ *   Perform a CRC calculation using the lookup table.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_lookup(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc = g_crc_lookup[crc ^ buf[byte]];
+    }
+
+  return crc;
+}
+
+#endif
+
+/****************************************************************************
+ * Name: sht4x_crc_bitwise
+ *
+ * Description:
+ *   Perform a CRC calculation using the bitwise calculation.
+ *
+ ****************************************************************************/
+
+uint8_t sht4x_crc_bitwise(const uint8_t *buf, uint8_t nbytes)
+{
+  uint8_t crc = SHT4X_CRC_INIT;
+  for (uint8_t byte = 0; byte < nbytes; byte++)
+    {
+      crc ^= buf[byte];
+      for (uint8_t bit = 0; bit < 8; bit++)
+        {
+          if (crc & 0x80)
+            {
+              /* Discard the highest bit (implicit XOR), then divide by the
+               * polynomial.
+               */
+
+              crc <<= 1;
+              crc ^= SHT4X_CRC_POLY;
+            }
+          else
+            {
+              /* Continue until the highest bit is set. */
+
+              crc <<= 1;
+            }
+        }
+    }
+
+  return crc;
+}
+
+/****************************************************************************
+ * Name: sht4x_crc
+ *
+ * Description:
+ *   Perform a CRC calculation on the data to check for validity.
+ *   The `buf` param must be a data buffer that ends with its own 8 bit CRC.
+ *   The `nbytes` param is the length of the data including the 8 bit CRC.
+ *
+ ****************************************************************************/
+
+static int sht4x_crc(const uint8_t *buf, uint8_t nbytes)
+{
+#ifdef CONFIG_SHT4X_CRC_LOOKUP
+  if (sht4x_crc_lookup(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+#else
+  if (sht4x_crc_bitwise(buf, nbytes) != 0)
+    {
+      return -EBADMSG;
+    }
+
+#endif
+  return 0;
+}
+
+/****************************************************************************
+ * Name: sht4x_cmd
+ *
+ * Description:
+ *   I2C access helper for sending commands to the SHT4x which receive data
+ *   in response.
+ *
+ ****************************************************************************/
+
+static int sht4x_cmd(FAR struct sht4x_dev_s *priv, uint8_t command,
+                     uint32_t timeout, uint16_t *data1, uint16_t *data2)
+{
+  struct i2c_msg_s cmd;
+  struct i2c_msg_s read_data;
+  int err;
+
+  /* 2 byte data, 1 byte CRC, 2 byte data, 1 byte CRC. */
+
+  uint8_t combined_data[6];
+
+  cmd.frequency = CONFIG_SHT4X_I2C_FREQUENCY;
+  cmd.addr = priv->addr;
+  cmd.flags = 0;
+  cmd.buffer = &command;
+  cmd.length = sizeof(command);
+
+  /* Trigger measurement. */
+
+  err = I2C_TRANSFER(priv->i2c, &cmd, 1);
+  if (err) return err;

Review Comment:
   Please include { } to if() even when single line, put the return err in a new line, see: https://github.com/apache/nuttx/issues/12144



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for the Sensirion SHT4x temperature and humidity sensor [nuttx]

Posted by "linguini1 (via GitHub)" <gi...@apache.org>.
linguini1 commented on code in PR #12715:
URL: https://github.com/apache/nuttx/pull/12715#discussion_r1684370463


##########
boards/arm/rp2040/common/src/rp2040_common_bringup.c:
##########
@@ -68,6 +68,11 @@
 #include "rp2040_bmp280.h"
 #endif
 
+#ifdef CONFIG_SENSORS_SHT4X
+#include <nuttx/sensors/sht4x.h>
+#include "rp2040_i2c.h"

Review Comment:
   The reason I left that function call in the rp2040 bring up is because it seems like the architecture specific logic should stay out of the driver, since I want it to work on all architectures with I2C. I have seen this done for the SHT3x and SHT21.
   
   I was hoping to find some way to access the i2c driver structure without an architecture specific initialization function, but since I haven't been able to, I think this way ensures that the driver can run on all architectures.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org