[riot-notifications] [RIOT-OS/RIOT] drivers/dht: Bugfixes (#11876)

Sebastian Meiling notifications at github.com
Tue Jul 23 22:18:21 CEST 2019


smlng commented on this pull request.



>  
+/**
+ * @brief   Wait until the pin @p pin has level @p expect
+ *
+ * @param   pin     GPIO pin to wait for
+ * @param   expect  Wait until @p pin has this logic level
+ * @param   timeout Timeout in µs
+ *
+ * @retval  0       Success
+ * @retval  -1      Timeout occurred before level was reached
+ */
+static inline int _wait_for_level(gpio_t pin, int expect, unsigned timeout)
+{
+    while ((gpio_read(pin) != expect) && timeout) {

not sure this works, bc `gpio_read` may return anything `> 0` for HIGH, i.e., must not be `1` in all cases. That's why I had this rather ugly line `((gpio_read(pin) > 0) == set)` in my suggested code, that should work for cases there `gpio_read()` returns `42` (for example) for HIGH.

>  
+/**
+ * @brief   Wait until the pin @p pin has level @p expect
+ *
+ * @param   pin     GPIO pin to wait for
+ * @param   expect  Wait until @p pin has this logic level
+ * @param   timeout Timeout in µs
+ *
+ * @retval  0       Success
+ * @retval  -1      Timeout occurred before level was reached
+ */
+static inline int _wait_for_level(gpio_t pin, int expect, unsigned timeout)

again: I'm unsure if `unsigned timeout` is large enough on all platforms to hold `1.000.000` microseconds, i.e. verify with arduino-mega2560 for instance.

>  
+/**
+ * @brief   Wait until the pin @p pin has level @p expect
+ *
+ * @param   pin     GPIO pin to wait for
+ * @param   expect  Wait until @p pin has this logic level
+ * @param   timeout Timeout in µs
+ *
+ * @retval  0       Success
+ * @retval  -1      Timeout occurred before level was reached
+ */
+static inline int _wait_for_level(gpio_t pin, int expect, unsigned timeout)

on that arduino `int = unsigned = 16Bits`? Which would map 1sec to 65000 microseconds, IIRC?

>  
+/**
+ * @brief   Wait until the pin @p pin has level @p expect
+ *
+ * @param   pin     GPIO pin to wait for
+ * @param   expect  Wait until @p pin has this logic level
+ * @param   timeout Timeout in µs
+ *
+ * @retval  0       Success
+ * @retval  -1      Timeout occurred before level was reached
+ */
+static inline int _wait_for_level(gpio_t pin, int expect, unsigned timeout)

better use `int32_t` to be safe and also avoid underflow (which you correctly pointed out).

-- 
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/11876#pullrequestreview-265650285
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190723/bf33fbb8/attachment.htm>


More information about the notifications mailing list