drivers/dht: busy wait reimplementation

[hugueslarrive]

Contribution description

In PR #19674, I also provided quick and dirty fixes to restore functionality on esp8266 and enable operation on AVR. While reviewing PR #18591, it became apparent to me that this driver needed a refresh, particularly its migration to ztimer.

The cause of the malfunction on esp8266 was that since the default switch to ztimer as the backend for xtimer, XTIMER_BACKOFF was no longer taken into account. Therefore, the correction I provided in PR #19674 simply made explicit what was previously done implicitly with xtimer and now needs to be done explicitly with ztimer (spinning instead of sleeping).

Moreover, it was unnecessarily complex to measure the pulse duration in a busy-wait implementation, which required 2 calls to ztimer_now() and 32-bit operations expensive on 8-bit architecture. Instead, it is sufficient to read the state of the bus at the threshold moment.

Finally, in practice, it is possible to reduce the read interval (down to less than 0.5s for DHT22) by "harassing" the DHT with start signals until it responds.

This re-implementation brings the following improvements:

• Many backports from @maribu's IRQ based implementation (drivers/dht: use IRQs for decoding #18591):
  • Use of ztimer
  • Use of errno.h
  • Use of a dht_data structure to pass arguments, to facilitate integration
  • Adaptation of the bit parsing technique to parse bits into the data array
• Reintroduction of DHT11/DHT22 differentiation.
• Separation of dht_read() steps into functions for better readability and the ability to share certain functions among different implementations
• Sensor presence check in dht_init()
• Automatic adjustment to a minimum data hold time
• Default input mode changed to open drain (a pull-up resistor should be placed close to the output if necessary but not close to the input)
• AVR support without platform-specific handling by avoiding ztimer_spin() and using the overflow of an 8-bit variable as a pre-timeout to minimize time-consuming ztimer_now() calls

Regarding the changes in the start signal sequence and the removal of the _reset() function:

In the previous implementation, there was an unnecessary spike at the beginning of the signal sequence, corresponding to START_HIGH_TIME. This spike has been removed in the re-implementation, as it is unnecessary. Instead, the MCU now simply pulls the signal low for START_LOW_TIME and then releases the bus, which is sufficient for initiating communication with the DHT sensor. Actually, it is necessary to raise the bus level; otherwise, the _wait_for_level() called immediately after to check the response of the DHT may read the port before the signal level is raised, resulting in a false positive.

Additionally, the previous implementation had an issue where the MCU switched back to output mode and went high immediately after reading the 40 bits of data. However, the DHT sensor was still transmitting 2 or 3 additional bytes of '0' at that point, causing a conflict. This issue has been resolved in the re-implementation:

Regarding the optimization for AVR, I have performed measurements of _wait_for_level() until timeout (85 loops):
- on esp8266-esp-12x: 264 µs, which is 3.11 µs per loop
- on nucleo-f303k8: 319 µs, which is 3.75 µs per loop
- on arduino-nano: 3608 µs, which is 42.45 µs per loop
Duration measurements on the Arduino Nano:

ztimer_spin(ZTIMER_USEC, 1); // 40 µs
_delay_us(1); // 0.93 µs

ztimer_spin(ZTIMER_USEC, 40); // 77.1 µs
_delay_us(40); // 40 µs

gpio_read(GPIO_PIN(PORT_B,1)); // 2.25 µs
(_SFR_MEM8(p_in_addr) & p_in_mask); // 0.08 µs

_wait_for_level() until timeout optimized on arduino-nano: 120 µs, which is 1.41 µs per loop

Regarding slow 8-bit MCUs like AVR, the issue was that they spent 20 times more time checking their watch for timeouts than actually watching the input. This problem was addressed by simply adding an 8-bit overflow counter before checking the timer for a timeout. It is not excessive for slower devices (255 * 3 µs + 18) * 80 = approximately 63 ms, and faster devices will check the timer anyway. Moreover, there is no more spinning on the timer; it only occurs on the input, which is more rational in any case.

Testing procedure

I used tests/drivers/dht with AM2301 (DHT22) sensors and 3 different boards: nucleo-f303k8, esp8266-esp-12x, and arduino-nano.

I used a 5K1 resistor to test sensor presence verification in the dht_init() function.

Issues/PRs references

Replace #19674, see also #18591.

[maribu]

See inline for suggestions how to avoid platform specific handling while still remaining compatible with AVR (or at least I hope it will work :D)

[hugueslarrive]

Does anyone have the datasheet for the new DHT11 sensors with the detailed data format to verify if the negative temperature bit is indeed in the fourth byte instead of the third byte like the DHT22 sensors?

[maribu]

Thx, this is a mayor improvement of the driver. Some minor comments inline, some optional (as state in the comment).

@OlegHahm With this PR the driver works again on the nRF52840 DK. (It doesn't work on P0.0 (the default config), though. I didn't track this down, but I assume a pin conflict.)

[riot-ci]

Murdock results

✔️ PASSED

a582da1 drivers/dht: busy wait reimplementation

Success	Failures	Total	Runtime
6934	0	6934	10m:21s

Artifacts

• Documentation preview

[maribu]

This also needs

diff --git a/drivers/dht/Kconfig b/drivers/dht/Kconfig
index a21f124720..dca40c956c 100644
--- a/drivers/dht/Kconfig
+++ b/drivers/dht/Kconfig
@@ -10,7 +10,7 @@ config MODULE_DHT
     depends on HAS_PERIPH_GPIO
     depends on TEST_KCONFIG
     select MODULE_PERIPH_GPIO
-    select MODULE_XTIMER
+    select ZTIMER_USEC
 
 config HAVE_DHT
     bool
diff --git a/tests/drivers/dht/app.config.test b/tests/drivers/dht/app.config.test
index 934b6b207c..492e5016c7 100644
--- a/tests/drivers/dht/app.config.test
+++ b/tests/drivers/dht/app.config.test
@@ -1,4 +1,3 @@
 # this file enables modules defined in Kconfig. Do not use this file for
 # application configuration. This is only needed during migration.
 CONFIG_MODULE_DHT=y
-CONFIG_MODULE_XTIMER=y

[hugueslarrive]

diff --git a/tests/drivers/dht/app.config.test b/tests/drivers/dht/app.config.test
index 934b6b207c..492e5016c7 100644
--- a/tests/drivers/dht/app.config.test
+++ b/tests/drivers/dht/app.config.test
@@ -1,4 +1,3 @@
 # this file enables modules defined in Kconfig. Do not use this file for
 # application configuration. This is only needed during migration.
 CONFIG_MODULE_DHT=y
-CONFIG_MODULE_XTIMER=y

I replaced it with CONFIG_ZTIMER_USEC here as well to maintain consistency with the Makefile.

[maribu]

I replaced it with CONFIG_ZTIMER_USEC here as well to maintain consistency with the Makefile.

Makes sense. After all main.c is also using ztimer and the explicit dependency would ensure it remains operationally even e.g. when a, say, RP2040 state machine DHT driver that doesn't need ztimer would be used.

[maribu]

Thanks for the PR. In addition to fixing the interpretation of DHT2x data, this is major improvement in code quality :)

Please squash

[maribu]

Still not yet fully squashed 😉

[kfessel]

Why?

What is the reason to default to no PU?

asking 'cause i want to suggest to remove .in_mode from all structs and allways go for IN_PU

[hugueslarrive]

Firstly, because the DHT already has an internal pull-up resistor, it is not necessary to unnecessarily increase power consumption. Additionally, the nominal voltage of DHT is 5V, and some 3.3V MCUs have their I/Os tolerant to 5V, provided that the internal pull-up resistor is not activated.

[kfessel]

so we can remove the runtime <dht_params_t>.in_mode information since it is mcu/board specific and not likely to change at runtime -> we can use the define.

you may ignore this

[maribu]

please squash

[maribu]

lgtm. Two nitpicks inline. If you take them, please squash/amend right away.

[maribu]

bors merge

[benpicco]

bors cancel
bors merge

[bors]

Canceled.

[hugueslarrive]

Thank you, @maribu. 🍻 No more excuses now to delay the backporting of #18591 since we have a solid fallback.

[hugueslarrive]

Oops! My apologies for jumping the gun!

[bors]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• Murdock
• static-tests
• tools-build-success

[maribu]

Thanks for the PR! :)