From 0e6e3b368bccc6b019257bfc79c9f64dba494ced Mon Sep 17 00:00:00 2001
From: Alexandre Abadie <alexandre.abadie@inria.fr>
Date: Mon, 20 Mar 2017 09:58:13 +0100
Subject: [PATCH] drivers/bmp180: driver cleanup

---
 drivers/bmp180/Makefile                |   2 -
 drivers/bmp180/bmp180.c                | 101 ++++++++++++-------------
 drivers/bmp180/bmp180_saul.c           |   9 +--
 drivers/bmp180/include/bmp180_params.h |  13 +++-
 drivers/include/bmp180.h               |  98 ++++++++++++------------
 5 files changed, 105 insertions(+), 118 deletions(-)

diff --git a/drivers/bmp180/Makefile b/drivers/bmp180/Makefile
index 43df70d8c6..48422e909a 100644
--- a/drivers/bmp180/Makefile
+++ b/drivers/bmp180/Makefile
@@ -1,3 +1 @@
-MODULE = bmp180
-
 include $(RIOTBASE)/Makefile.base
diff --git a/drivers/bmp180/bmp180.c b/drivers/bmp180/bmp180.c
index c943a8b863..5dfb036547 100644
--- a/drivers/bmp180/bmp180.c
+++ b/drivers/bmp180/bmp180.c
@@ -30,6 +30,10 @@
 #define ENABLE_DEBUG        (0)
 #include "debug.h"
 
+#define DEV_I2C      (dev->params.i2c_dev)
+#define DEV_ADDR     (dev->params.i2c_addr)
+#define OVERSAMPLING (dev->params.oversampling)
+
 /* Internal function prototypes */
 static int _read_ut(bmp180_t *dev, int32_t *ut);
 static int _read_up(bmp180_t *dev, int32_t *up);
@@ -39,34 +43,31 @@ static int _compute_b5(bmp180_t *dev, int32_t ut, int32_t *b5);
  *                          BMP180 Core API                                 *
  *---------------------------------------------------------------------------*/
 
-int bmp180_init(bmp180_t *dev, i2c_t i2c, uint8_t mode)
+int bmp180_init(bmp180_t *dev, const bmp180_params_t *params)
 {
-    dev->i2c_dev = i2c;
+    dev->params = *params;
 
     /* Clamp oversampling mode */
-    if (mode > BMP180_ULTRAHIGHRES) {
-        mode = BMP180_ULTRAHIGHRES;
+    if (OVERSAMPLING > BMP180_ULTRAHIGHRES) {
+        OVERSAMPLING = BMP180_ULTRAHIGHRES;
     }
 
-    /* Setting oversampling mode */
-    dev->oversampling = mode;
-
     /* Initialize I2C interface */
-    if (i2c_init_master(dev->i2c_dev, I2C_SPEED_NORMAL)) {
+    if (i2c_init_master(DEV_I2C, I2C_SPEED_NORMAL)) {
         DEBUG("[Error] I2C device not enabled\n");
-        return -1;
+        return -BMP180_ERR_NOI2C;
     }
 
     /* Acquire exclusive access */
-    i2c_acquire(dev->i2c_dev);
+    i2c_acquire(DEV_I2C);
 
     /* Check sensor ID */
     uint8_t checkid;
-    i2c_read_reg(dev->i2c_dev, BMP180_ADDR, BMP180_REGISTER_ID, &checkid);
+    i2c_read_reg(DEV_I2C, DEV_ADDR, BMP180_REGISTER_ID, &checkid);
     if (checkid != 0x55) {
         DEBUG("[Error] Wrong device ID\n");
-        i2c_release(dev->i2c_dev);
-        return -1;
+        i2c_release(DEV_I2C);
+        return -BMP180_ERR_NODEV;
     }
 
     /* adding delay before reading calibration values to avoid timing issues */
@@ -74,10 +75,10 @@ int bmp180_init(bmp180_t *dev, i2c_t i2c, uint8_t mode)
 
     uint8_t buffer[22] = {0};
     /* Read calibration values, using contiguous register addresses */
-    if (i2c_read_regs(dev->i2c_dev, BMP180_ADDR, BMP180_CALIBRATION_AC1, buffer, 22) < 0) {
+    if (i2c_read_regs(DEV_I2C, DEV_ADDR, BMP180_CALIBRATION_AC1, buffer, 22) < 0) {
         DEBUG("[Error] Cannot read calibration registers.\n");
-        i2c_release(dev->i2c_dev);
-        return -1;
+        i2c_release(DEV_I2C);
+        return -BMP180_ERR_NOCAL;
     }
     dev->calibration.ac1 = (int16_t)(buffer[0] << 8)   | buffer[1];
     dev->calibration.ac2 = (int16_t)(buffer[2] << 8)   | buffer[3];
@@ -92,7 +93,7 @@ int bmp180_init(bmp180_t *dev, i2c_t i2c, uint8_t mode)
     dev->calibration.md  = (int16_t)(buffer[20] << 8)  | buffer[21];
 
     /* Release I2C device */
-    i2c_release(dev->i2c_dev);
+    i2c_release(DEV_I2C);
 
     DEBUG("AC1: %i\n", (int)dev->calibration.ac1);
     DEBUG("AC2: %i\n", (int)dev->calibration.ac2);
@@ -108,49 +109,51 @@ int bmp180_init(bmp180_t *dev, i2c_t i2c, uint8_t mode)
     return 0;
 }
 
-int bmp180_read_temperature(bmp180_t *dev, int32_t *temperature)
+int16_t bmp180_read_temperature(bmp180_t *dev)
 {
     int32_t ut, b5;
     /* Acquire exclusive access */
-    i2c_acquire(dev->i2c_dev);
+    i2c_acquire(DEV_I2C);
 
     /* Read uncompensated value */
     _read_ut(dev, &ut);
 
+    /* Release I2C device */
+    i2c_release(DEV_I2C);
+
     /* Compute true temperature value following datasheet formulas */
     _compute_b5(dev, ut, &b5);
-    *temperature = (b5 + 8) >> 4;
 
-    /* Release I2C device */
-    i2c_release(dev->i2c_dev);
-
-    return 0;
+    return (int16_t)((b5 + 8) >> 4);
 }
 
-int bmp180_read_pressure(bmp180_t *dev, int32_t *pressure)
+uint32_t bmp180_read_pressure(bmp180_t *dev)
 {
     int32_t ut = 0, up = 0, x1, x2, x3, b3, b5, b6, p;
     uint32_t b4, b7;
 
     /* Acquire exclusive access */
-    i2c_acquire(dev->i2c_dev);
+    i2c_acquire(DEV_I2C);
 
     /* Read uncompensated values: first temperature, second pressure */
     _read_ut(dev, &ut);
     _read_up(dev, &up);
 
+    /* release I2C device */
+    i2c_release(DEV_I2C);
+
     /* Compute true pressure value following datasheet formulas */
     _compute_b5(dev, ut, &b5);
     b6 = b5 - 4000;
     x1 = ((int32_t)dev->calibration.b2 * ((b6 * b6) >> 12)) >> 11;
     x2 = ((int32_t)dev->calibration.ac2 * b6) >> 11;
     x3 = x1 + x2;
-    b3 = ((((int32_t)dev->calibration.ac1*4 + x3) << dev->oversampling) + 2) >> 2;
+    b3 = ((((int32_t)dev->calibration.ac1*4 + x3) << OVERSAMPLING) + 2) >> 2;
     x1 = ((int32_t)dev->calibration.ac3 * b6) >> 13;
     x2 = ((int32_t)dev->calibration.b1 * (b6 * b6) >> 12) >> 16;
     x3 = ((x1 + x2) + 2) >> 2;
     b4 = (int32_t)dev->calibration.ac4 * (uint32_t)(x3+32768) >> 15;
-    b7 = ((uint32_t)up - b3) * (uint32_t)(50000UL >> dev->oversampling);
+    b7 = ((uint32_t)up - b3) * (uint32_t)(50000UL >> OVERSAMPLING);
     if (b7 < 0x80000000) {
         p = (b7 * 2) / b4;
     }
@@ -161,32 +164,22 @@ int bmp180_read_pressure(bmp180_t *dev, int32_t *pressure)
     x1 = (p >> 8) * (p >> 8);
     x1 = (x1 * 3038) >> 16;
     x2 = (-7357 * p) >> 16;
-    *pressure = p + ((x1 + x2 + 3791) >> 4);
 
-    /* release I2C device */
-    i2c_release(dev->i2c_dev);
-
-    return 0;
+    return (uint32_t)(p + ((x1 + x2 + 3791) >> 4));
 }
 
-int bmp180_altitude(bmp180_t *dev, int32_t pressure_0, int32_t *altitude)
+int16_t bmp180_altitude(bmp180_t *dev, uint32_t pressure_0)
 {
-    int32_t p;
-    bmp180_read_pressure(dev, &p);
+    uint32_t p = bmp180_read_pressure(dev);
 
-    *altitude = (int32_t)(44330.0 * (1.0 - pow((double)p / pressure_0, 0.1903)));
-
-    return 0;
+    return (int16_t)(44330.0 * (1.0 - pow((double)p / pressure_0, 0.1903)));;
 }
 
-int bmp180_sealevel_pressure(bmp180_t *dev, int32_t altitude, int32_t *pressure_0)
+uint32_t bmp180_sealevel_pressure(bmp180_t *dev, int16_t altitude)
 {
-    int32_t p;
-    bmp180_read_pressure(dev, &p);
+    uint32_t p = bmp180_read_pressure(dev);
 
-    *pressure_0 = (int32_t)((double)p / pow(1.0 - (altitude / 44330.0), 5.255));
-
-    return 0;
+    return (uint32_t)((double)p / pow(1.0 - (altitude / 44330.0), 5.255));;
 }
 
 /*------------------------------------------------------------------------------------*/
@@ -198,11 +191,11 @@ static int _read_ut(bmp180_t *dev, int32_t *output)
     /* Read UT (Uncompsensated Temperature value) */
     uint8_t ut[2] = {0};
     uint8_t control[2] = { BMP180_REGISTER_CONTROL, BMP180_TEMPERATURE_COMMAND };
-    i2c_write_bytes(dev->i2c_dev, BMP180_ADDR, control, 2);
+    i2c_write_bytes(DEV_I2C, DEV_ADDR, control, 2);
     xtimer_usleep(BMP180_ULTRALOWPOWER_DELAY);
-    if (i2c_read_regs(dev->i2c_dev, BMP180_ADDR, BMP180_REGISTER_DATA, ut, 2) < 0) {
+    if (i2c_read_regs(DEV_I2C, DEV_ADDR, BMP180_REGISTER_DATA, ut, 2) < 0) {
         DEBUG("[Error] Cannot read uncompensated temperature.\n");
-        i2c_release(dev->i2c_dev);
+        i2c_release(DEV_I2C);
         return -1;
     }
     *output = ( ut[0] << 8 ) | ut[1];
@@ -216,9 +209,9 @@ static int _read_up(bmp180_t *dev, int32_t *output)
 {
     /* Read UP (Uncompsensated Pressure value) */
     uint8_t up[3] = {0};
-    uint8_t control[2] = { BMP180_REGISTER_CONTROL, BMP180_PRESSURE_COMMAND | (dev->oversampling & 0x3) << 6 };
-    i2c_write_bytes(dev->i2c_dev, BMP180_ADDR, control, 2);
-    switch (dev->oversampling) {
+    uint8_t control[2] = { BMP180_REGISTER_CONTROL, BMP180_PRESSURE_COMMAND | (OVERSAMPLING & 0x3) << 6 };
+    i2c_write_bytes(DEV_I2C, DEV_ADDR, control, 2);
+    switch (OVERSAMPLING) {
     case BMP180_ULTRALOWPOWER:
         xtimer_usleep(BMP180_ULTRALOWPOWER_DELAY);
         break;
@@ -235,13 +228,13 @@ static int _read_up(bmp180_t *dev, int32_t *output)
         xtimer_usleep(BMP180_ULTRALOWPOWER_DELAY);
         break;
     }
-    if (i2c_read_regs(dev->i2c_dev, BMP180_ADDR, BMP180_REGISTER_DATA, up, 3) < 0) {
+    if (i2c_read_regs(DEV_I2C, DEV_ADDR, BMP180_REGISTER_DATA, up, 3) < 0) {
         DEBUG("[Error] Cannot read uncompensated pressure.\n");
-        i2c_release(dev->i2c_dev);
+        i2c_release(DEV_I2C);
         return -1;
     }
 
-    *output = ((up[0] << 16) | (up[1] << 8) | up[2]) >> (8 - dev->oversampling);
+    *output = ((up[0] << 16) | (up[1] << 8) | up[2]) >> (8 - OVERSAMPLING);
 
     DEBUG("UP: %i\n", (int)*output);
 
diff --git a/drivers/bmp180/bmp180_saul.c b/drivers/bmp180/bmp180_saul.c
index a12a52f59e..e918a1b3bb 100644
--- a/drivers/bmp180/bmp180_saul.c
+++ b/drivers/bmp180/bmp180_saul.c
@@ -25,14 +25,11 @@
 #include "bmp180.h"
 #include "xtimer.h"
 
-static int32_t temperature, pressure;
-
 static int read_temperature(void *dev, phydat_t *res)
 {
     bmp180_t *d = (bmp180_t *)dev;
 
-    bmp180_read_temperature(d, &temperature);
-    res->val[0] = temperature;
+    res->val[0] = bmp180_read_temperature(d);
     res->unit = UNIT_TEMP_C;
     res->scale = -1;
     return 1;
@@ -42,9 +39,7 @@ static int read_pressure(void *dev, phydat_t *res)
 {
     bmp180_t *d = (bmp180_t *)dev;
 
-    bmp180_read_pressure(d, &pressure);
-
-    res->val[0] = pressure/10;
+    res->val[0] = bmp180_read_pressure(d) / 10;
     res->unit = UNIT_PA;
     res->scale = 1;
     return 1;
diff --git a/drivers/bmp180/include/bmp180_params.h b/drivers/bmp180/include/bmp180_params.h
index db9449207a..24a9222bf7 100644
--- a/drivers/bmp180/include/bmp180_params.h
+++ b/drivers/bmp180/include/bmp180_params.h
@@ -23,6 +23,7 @@
 
 #include "board.h"
 #include "bmp180.h"
+#include "bmp180_internals.h"
 #include "saul_reg.h"
 
 #ifdef __cplusplus
@@ -36,12 +37,16 @@ extern "C" {
 #ifndef BMP180_PARAM_I2C_DEV
 #define BMP180_PARAM_I2C_DEV         I2C_DEV(0)
 #endif
-#ifndef BMP180_PARAM_MODE
-#define BMP180_PARAM_MODE            BMP180_ULTRALOWPOWER
+#ifndef BMP180_PARAM_I2C_ADDR
+#define BMP180_PARAM_I2C_ADDR        BMP180_ADDR
+#endif
+#ifndef BMP180_PARAM_OVERSAMPLING
+#define BMP180_PARAM_OVERSAMPLING    BMP180_ULTRALOWPOWER
 #endif
 
-#define BMP180_PARAMS_DEFAULT        { .i2c_dev = BMP180_PARAM_I2C_DEV,  \
-                                       .mode    = BMP180_PARAM_MODE }
+#define BMP180_PARAMS_DEFAULT        { .i2c_dev      = BMP180_PARAM_I2C_DEV,  \
+                                       .i2c_addr     = BMP180_PARAM_I2C_ADDR, \
+                                       .oversampling = BMP180_PARAM_OVERSAMPLING }
 /**@}*/
 
 /**
diff --git a/drivers/include/bmp180.h b/drivers/include/bmp180.h
index 830a2b36c9..da3e6843f3 100644
--- a/drivers/include/bmp180.h
+++ b/drivers/include/bmp180.h
@@ -32,112 +32,108 @@ extern "C" {
  * @name Oversampling modes
  * @{
  */
-#define BMP180_ULTRALOWPOWER          (0)
-#define BMP180_STANDARD               (1)
-#define BMP180_HIGHRES                (2)
-#define BMP180_ULTRAHIGHRES           (3)
+typedef enum {
+    BMP180_ULTRALOWPOWER = 0,
+    BMP180_STANDARD,
+    BMP180_HIGHRES,
+    BMP180_ULTRAHIGHRES
+} bmp180_oversampling_mode_t;
 /** @} */
 
 /**
  * @brief Calibration struct for the BMP180 sensor
  */
 typedef struct {
-    int16_t ac1;          /**< ac1 coefficient */
-    int16_t ac2;          /**< ac2 coefficient */
-    int16_t ac3;          /**< ac3 coefficient */
-    int16_t b1;           /**< b1 coefficient */
-    int16_t b2;           /**< b2 coefficient */
-    int16_t mb;           /**< mb coefficient */
-    int16_t mc;           /**< mc coefficient */
-    int16_t md;           /**< md coefficient */
-    uint16_t ac4;         /**< ac4 coefficient */
-    uint16_t ac5;         /**< ac5 coefficient */
-    uint16_t ac6;         /**< ac6 coefficient */
+    int16_t ac1;                                /**< ac1 coefficient */
+    int16_t ac2;                                /**< ac2 coefficient */
+    int16_t ac3;                                /**< ac3 coefficient */
+    int16_t b1;                                 /**< b1 coefficient */
+    int16_t b2;                                 /**< b2 coefficient */
+    int16_t mb;                                 /**< mb coefficient */
+    int16_t mc;                                 /**< mc coefficient */
+    int16_t md;                                 /**< md coefficient */
+    uint16_t ac4;                               /**< ac4 coefficient */
+    uint16_t ac5;                               /**< ac5 coefficient */
+    uint16_t ac6;                               /**< ac6 coefficient */
 } bmp180_calibration_t;
 
 /**
- * @brief Device descriptor for the BMP180 sensor
+ * @brief Device initialization parameters
  */
 typedef struct {
-    i2c_t i2c_dev;                     /**< I2C device which is used */
-    bmp180_calibration_t calibration;  /**< Device calibration */
-    uint8_t oversampling;              /**< Oversampling mode */
-} bmp180_t;
-
+    i2c_t i2c_dev;                              /**< I2C device which is used */
+    uint8_t i2c_addr;                           /**< I2C address */
+    bmp180_oversampling_mode_t oversampling;    /**< Oversampling mode */
+} bmp180_params_t;
 
 /**
- * @brief Device initialization parameters
+ * @brief Device descriptor for the BMP180 sensor
  */
 typedef struct {
-    i2c_t i2c_dev;   /**< I2C device which is used */
-    uint8_t mode;    /**< Oversampling mode */
-} bmp180_params_t;
+    bmp180_params_t      params;                /**< Device initialization parameters */
+    bmp180_calibration_t calibration;           /**< Device calibration */
+} bmp180_t;
 
 /**
- * @brief export SAUL endpoints
- * @{
+ * @brief   Status and error return codes
  */
-extern const saul_driver_t bmp180_temperature_saul_driver;
-extern const saul_driver_t bmp180_pressure_saul_driver;
-/** @} */
+enum {
+    BMP180_OK = 0,                              /**< everything was fine */
+    BMP180_ERR_NOI2C,                           /**< error initializing the I2C bus */
+    BMP180_ERR_NODEV,                           /**< did not detect BMP180 */
+    BMP180_ERR_NOCAL,                           /**< error when reading calibration values */
+};
 
 /**
  * @brief Initialize the given BMP180 device
  *
  * @param[out] dev          Initialized device descriptor of BMP180 device
- * @param[in]  i2c          I2C bus the sensor is connected to
- * @param[in]  mode         BMP180 oversampling mode
+ * @param[in]  params       Initialization parameters
  *
- * @return                  0 on success
- * @return                  -1 if given I2C is not enabled in board config
+ * @return                  BMP180_OK on success
+ * @return                  -BMP180_ERR_NOI2C if given I2C is not enabled in board config
+ * @return                  -BMP180_ERR_NODEV if not a BMP180 at given address
+ * @return                  -BMP180_ERR_NOCAL if an error occured when reading calibration values
  */
-int bmp180_init(bmp180_t *dev, i2c_t i2c, uint8_t mode);
+int bmp180_init(bmp180_t *dev, const bmp180_params_t *params);
 
 /**
  * @brief Read temperature value from the given BMP180 device, returned in d°C
  *
  * @param[in] dev           Device descriptor of BMP180 device to read from
- * @param[out] temperature  Temperature in d°C
  *
- * @return                  0 on success
- * @return                  -1 if device's I2C is not enabled in board config
+ * @return                  Temperature in d°C
  */
-int bmp180_read_temperature(bmp180_t *dev, int32_t *temperature);
+int16_t bmp180_read_temperature(bmp180_t *dev);
 
 /**
  * @brief Read pressure value from the given BMP180 device, returned in Pa
  *
  * @param[in]  dev          Device descriptor of BMP180 device to read from
- * @param[out] pressure     Pressure in Pa
  *
- * @return                  0 on success
- * @return                  -1 if device's I2C is not enabled in board config
+ * @return                  Pressure in Pa
  */
-int bmp180_read_pressure(bmp180_t *dev, int32_t *pressure);
+uint32_t bmp180_read_pressure(bmp180_t *dev);
 
 /**
  * @brief Compute altitude, returned in m.
  *
  * @param[in]  dev          Device descriptor of BMP180 device to read from
  * @param[in]  pressure_0   The pressure at sea level in Pa
- * @param[out] altitude     Altitude in m
  *
- * @return                  0 on success
- * @return                  -1 if device's I2C is not enabled in board config
+ * @return                  Altitude in m
  */
-int bmp180_altitude(bmp180_t *dev, int32_t pressure_0, int32_t *altitude);
+int16_t bmp180_altitude(bmp180_t *dev, uint32_t pressure_0);
 
 /**
  * @brief Compute pressure at sea level, returned in Pa.
  *
  * @param[in]  dev          Device descriptor of BMP180 device to read from
  * @param[in]  altitude     Altitude in m
- * @param[out] pressure_0   Pressure at sea level in Pa
  *
- * @return                  0 on success
- * @return                  -1 if device's I2C is not enabled in board config
+ * @return                  Pressure at sea level in Pa
  */
-int bmp180_sealevel_pressure(bmp180_t *dev, int32_t altitude, int32_t *pressure_0);
+uint32_t bmp180_sealevel_pressure(bmp180_t *dev, int16_t altitude);
 
 #ifdef __cplusplus
 }
-- 
GitLab