From d57488b623fec81c0dc46c8c0600a549aea022de Mon Sep 17 00:00:00 2001
From: UncleRus <unclerus@gmail.com>
Date: Thu, 17 Dec 2020 07:02:14 +0500
Subject: [PATCH] bh1750, bmp180: Fix possible race condition

---
 components/bh1750/bh1750.c | 13 ++++++++++---
 components/bmp180/bmp180.c | 23 +++++++++--------------
 components/bmp180/bmp180.h | 31 ++++++++++++++-----------------
 3 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/components/bh1750/bh1750.c b/components/bh1750/bh1750.c
index d2f02638..03b2309d 100644
--- a/components/bh1750/bh1750.c
+++ b/components/bh1750/bh1750.c
@@ -41,10 +41,15 @@ static const char *TAG = "BH1750";
 #define CHECK(x) do { esp_err_t __; if ((__ = x) != ESP_OK) return __; } while (0)
 #define CHECK_ARG(VAL) do { if (!(VAL)) return ESP_ERR_INVALID_ARG; } while (0)
 
+inline static esp_err_t send_command_nolock(i2c_dev_t *dev, uint8_t cmd)
+{
+    return i2c_dev_write(dev, NULL, 0, &cmd, 1);
+}
+
 static esp_err_t send_command(i2c_dev_t *dev, uint8_t cmd)
 {
     I2C_DEV_TAKE_MUTEX(dev);
-    I2C_DEV_CHECK(dev, i2c_dev_write(dev, NULL, 0, &cmd, 1));
+    I2C_DEV_CHECK(dev, send_command_nolock(dev, cmd));
     I2C_DEV_GIVE_MUTEX(dev);
 
     return ESP_OK;
@@ -114,8 +119,10 @@ esp_err_t bh1750_set_measurement_time(i2c_dev_t *dev, uint8_t time)
 {
     CHECK_ARG(dev);
 
-    CHECK(send_command(dev, OPCODE_MT_HI | (time >> 5)));
-    CHECK(send_command(dev, OPCODE_MT_LO | (time & 0x1f)));
+    I2C_DEV_TAKE_MUTEX(dev);
+    I2C_DEV_CHECK(dev, send_command_nolock(dev, OPCODE_MT_HI | (time >> 5)));
+    I2C_DEV_CHECK(dev, send_command_nolock(dev, OPCODE_MT_LO | (time & 0x1f)));
+    I2C_DEV_GIVE_MUTEX(dev);
 
     return ESP_OK;
 }
diff --git a/components/bmp180/bmp180.c b/components/bmp180/bmp180.c
index 5553c80e..1d7a2341 100644
--- a/components/bmp180/bmp180.c
+++ b/components/bmp180/bmp180.c
@@ -130,14 +130,18 @@ esp_err_t bmp180_init(bmp180_dev_t *dev)
 {
     CHECK_ARG(dev);
 
-    if (!bmp180_is_available(&dev->i2c_dev))
+
+    I2C_DEV_TAKE_MUTEX(&dev->i2c_dev);
+
+    uint8_t id;
+    I2C_DEV_CHECK(&dev->i2c_dev, i2c_dev_read_reg(&dev->i2c_dev, BMP180_VERSION_REG, &id, 1));
+    if (id != BMP180_CHIP_ID)
     {
-        ESP_LOGE(TAG, "Device not found");
+        I2C_DEV_GIVE_MUTEX(&dev->i2c_dev);
+        ESP_LOGE(TAG, "Invalid device ID: 0x%02x", id);
         return ESP_ERR_NOT_FOUND;
     }
 
-    I2C_DEV_TAKE_MUTEX(&dev->i2c_dev);
-
     I2C_DEV_CHECK(&dev->i2c_dev, bmp180_read_reg_16(&dev->i2c_dev, BMP180_CALIBRATION_REG + 0, &dev->AC1));
     I2C_DEV_CHECK(&dev->i2c_dev, bmp180_read_reg_16(&dev->i2c_dev, BMP180_CALIBRATION_REG + 2, &dev->AC2));
     I2C_DEV_CHECK(&dev->i2c_dev, bmp180_read_reg_16(&dev->i2c_dev, BMP180_CALIBRATION_REG + 4, &dev->AC3));
@@ -167,16 +171,7 @@ esp_err_t bmp180_init(bmp180_dev_t *dev)
     return ESP_OK;
 }
 
-bool bmp180_is_available(i2c_dev_t *i2c_dev)
-{
-    if (!i2c_dev)
-        return false;
-
-    uint8_t id;
-    if (i2c_dev_read_reg(i2c_dev, BMP180_VERSION_REG, &id, 1) != ESP_OK)
-        return false;
-    return id == BMP180_CHIP_ID;
-}
+///////////////////////////////////////////////////////////////////////////////
 
 esp_err_t bmp180_measure(bmp180_dev_t *dev, float *temperature, uint32_t *pressure, bmp180_mode_t oss)
 {
diff --git a/components/bmp180/bmp180.h b/components/bmp180/bmp180.h
index b7d7ed0f..85989b42 100644
--- a/components/bmp180/bmp180.h
+++ b/components/bmp180/bmp180.h
@@ -62,42 +62,39 @@ typedef enum
 
 /**
  * @brief Initialize device descriptior
+ *
  * @param[out] dev Pointer to device descriptor
- * @param[in] port I2C port number
- * @param[in] sda_gpio GPIO pin number for SDA
- * @param[in] scl_gpio GPIO pin number for SCL
- * @return ESP_OK if no errors occured
+ * @param port I2C port number
+ * @param sda_gpio GPIO pin number for SDA
+ * @param scl_gpio GPIO pin number for SCL
+ * @return `ESP_OK` on success
  */
 esp_err_t bmp180_init_desc(bmp180_dev_t *dev, i2c_port_t port, gpio_num_t sda_gpio, gpio_num_t scl_gpio);
 
 /**
  * @brief Free device descriptor
+ *
  * @param dev Pointer to BMP180 device descriptor
  * @return `ESP_OK` on success
  */
 esp_err_t bmp180_free_desc(bmp180_dev_t *dev);
 
 /**
- * Init bmp180 driver
+ * @brief Init bmp180 device
+ *
  * @param dev Pointer to BMP180 device descriptor
- * @return ESP_OK on success
+ * @return `ESP_OK` on success
  */
 esp_err_t bmp180_init(bmp180_dev_t *dev);
 
 /**
- * Check BMP180 availability
- * @param i2c_dev I2C device descriptor
- * @return true if bmp180 is available
- */
-bool bmp180_is_available(i2c_dev_t *i2c_dev);
-
-/**
- * Measure temperature and pressure
+ * @brief Measure temperature and pressure
+ *
  * @param dev Pointer to BMP180 device descriptor
- * @param temperature Temperature in degrees Celsius
- * @param pressure Pressure in Pa
+ * @param[out] temperature Temperature in degrees Celsius
+ * @param[out] pressure Pressure in Pa
  * @param oss Measurement mode
- * @return ESP_OK on success
+ * @return `ESP_OK` on success
  */
 esp_err_t bmp180_measure(bmp180_dev_t *dev, float *temperature, uint32_t *pressure, bmp180_mode_t oss);