[riot-notifications] [RIOT-OS/RIOT] drivers: add gp2y10xx dust sensor (#15445)

benpicco notifications at github.com
Tue Nov 17 23:02:23 CET 2020


@benpicco commented on this pull request.

one more nitpick: defining a type for the level (instead of using `int`) makes the params struct easier to understand. 

> + * @brief   Driver error values
+ */
+enum {
+    GP2Y10XX_OK         = 0,    /**< Everything is ok */
+    GP2Y10XX_ERR_ADC    = -1,   /**< ADC error */
+    GP2Y10XX_ERR_ILED   = -2,   /**< ILED pin error */
+};
+
+/**
+ * @brief   ILED pin level.
+ *
+ * This specifies how the ILED pin behaves, if it's on when it's
+ * active-low/high. Waveshare breakout board is active-high, that is, that the
+ * voltage is 3.3v to turn ILED on (logic 1) and 0v to turn it off (logic 0).
+ */
+enum {

```suggestion
typedef enum {
```

> +    GP2Y10XX_OK         = 0,    /**< Everything is ok */
+    GP2Y10XX_ERR_ADC    = -1,   /**< ADC error */
+    GP2Y10XX_ERR_ILED   = -2,   /**< ILED pin error */
+};
+
+/**
+ * @brief   ILED pin level.
+ *
+ * This specifies how the ILED pin behaves, if it's on when it's
+ * active-low/high. Waveshare breakout board is active-high, that is, that the
+ * voltage is 3.3v to turn ILED on (logic 1) and 0v to turn it off (logic 0).
+ */
+enum {
+    GP2Y10XX_ILED_LEVEL_HIGH,   /**< Active high */
+    GP2Y10XX_ILED_LEVEL_LOW,    /**< Active low */
+};

```suggestion
} gp2y10xx_level_t;
```

> + */
+enum {
+    GP2Y10XX_ILED_LEVEL_HIGH,   /**< Active high */
+    GP2Y10XX_ILED_LEVEL_LOW,    /**< Active low */
+};
+
+/**
+ * @brief   GP2Y10xx device parameters
+ */
+typedef struct {
+    adc_t aout;         /**< ADC line connected to device AOUT pin. */
+    adc_res_t adc_res;  /**< ADC line resolution. */
+    uint16_t vref;      /**< Reference voltage used for the VCC supply of the
+                             sensor, in mV. */
+    gpio_t iled_pin;    /**< ILED pin */
+    int iled_level;     /**< ILED pin level */

```suggestion
    gp2y10xx_level_t iled_level; /**< ILED pin level */
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15445#pullrequestreview-532846167
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20201117/efd66749/attachment.htm>


More information about the notifications mailing list