[riot-notifications] [RIOT-OS/RIOT] drivers/lpsxxx: refactor lps331ap and add support for lps25hb + lps22hb (#10695)

Sebastian Meiling notifications at github.com
Mon Feb 4 19:44:56 CET 2019


smlng requested changes on this pull request.

code wise this looks all-in-all good, I don't have the hardware to test ...

> +        DEBUG("[lpsxxx] read_pres: cannot read PRES_OUT_H register\n");
+        return -LPSXXX_ERR_I2C;
+    }
+    i2c_release(DEV_I2C);
+
+    val |= ((uint32_t)tmp << 16);
+
+    DEBUG("[lpsxxx] read_pres: raw data %08" PRIx32 "\n", (uint32_t)val);
+
+    /* see if value is negative */
+    if (tmp & 0x80) {
+        val |= ((uint32_t)0xff << 24);
+    }
+
+    /* compute actual pressure value in mbar */
+    *pres = (uint16_t)(((float)val) / PRES_DIVIDER);

IMHO `>> 12` would be more efficient than `/ 4096`

> +        DEBUG("[lpsxxx] read_pres: cannot read PRES_OUT_H register\n");
+        return -LPSXXX_ERR_I2C;
+    }
+    i2c_release(DEV_I2C);
+
+    val |= ((uint32_t)tmp << 16);
+
+    DEBUG("[lpsxxx] read_pres: raw data %08" PRIx32 "\n", (uint32_t)val);
+
+    /* see if value is negative */
+    if (tmp & 0x80) {
+        val |= ((uint32_t)0xff << 24);
+    }
+
+    /* compute actual pressure value in mbar */
+    *pres = (uint16_t)(((float)val) / PRES_DIVIDER);

or `#define PRES_DIVIDER (12U)`above and `>> PRES_DIVIDER` here

> +        lpsxxx_enable(&dev);
+        xtimer_sleep(1); /* wait a bit for the measurements to complete */
+
+        lpsxxx_read_temp(&dev, &temp);
+        lpsxxx_read_pres(&dev, &pres);
+        lpsxxx_disable(&dev);
+
+        int pres_abs = pres / 1000;
+        pres -= pres_abs * 1000;
+        int temp_abs = temp / 100;
+        temp -= temp_abs * 100;
+
+        printf("Pressure value: %2i.%03i bar - Temperature: %2i.%02i °C\n",
+               pres_abs, pres, temp_abs, temp);
+
+        xtimer_sleep(1);

can be omitted? Sleeps already one second above after `enable`

-- 
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/10695#pullrequestreview-199741947
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190204/492e1028/attachment.html>


More information about the notifications mailing list