Bad position of sensirion_sleep_usec prototype

[qcabrol]

Most of the driver code is written in a way that the _sensirion_i2chal files are not required and the required functions can be written in user application code with the actual driver code only importing _sensirioni2c.h.

The decoupling is nice but 'sensirion_sleep_usec' is used in say 'sht3x.c' and not defined in either the sensirion_common or sensirion_i2c files unlike all the other functions required for proper linkage.

This leads to compilation warnings as there is not proper linking.

[build] /home/workspace/drivers/sensirion/sht3x/sht3x.c: In function 'sht3x_read_serial':
[build] /home/workspace/drivers/sensirion/sht3x/sht3x.c:156:5: warning: implicit declaration of function 'sensirion_sleep_usec' [-Wimplicit-function-declaration]
[build]   156 |     sensirion_sleep_usec(SHT3X_CMD_DURATION_USEC);
[build]       |     ^~~~~~~~~~~~~~~~~~~~

Proposals:

• define an extern sensirion_sleep_usec(uint32_t) in each sensor driver header file or the sensirion_i2c file instead of within sensirion_i2c_hal
• include sensirion_i2c_hal within the includes statements of the driver file (e.g sht3x) so that the prototype exist before the driver is imported within the user code

[qcabrol]

Patch proposal

diff --git a/embedded-common b/embedded-common
index 1ac7c72..f26f6eb 160000
--- a/embedded-common
+++ b/embedded-common
@@ -1 +1 @@
-Subproject commit 1ac7c72c895d230c6f1375865f3b7161ce6b665a
+Subproject commit f26f6ebb34d5cc82794016edf6cad6f5a3bf82f1
diff --git a/sht3x/sht3x.c b/sht3x/sht3x.c
index bcb38cf..9fb8f27 100644
--- a/sht3x/sht3x.c
+++ b/sht3x/sht3x.c
@@ -42,6 +42,7 @@
 #include "sensirion_arch_config.h"
 #include "sensirion_common.h"
 #include "sensirion_i2c.h"
+#include "sensirion_i2c_hal.h"

 /* all measurement commands return T (CRC) RH (CRC) */
 #if USE_SENSIRION_CLOCK_STRETCHING
diff --git a/sht4x/sht4x.c b/sht4x/sht4x.c
index 18b6567..2c3d5bb 100644
--- a/sht4x/sht4x.c
+++ b/sht4x/sht4x.c
@@ -42,6 +42,7 @@
 #include "sensirion_arch_config.h"
 #include "sensirion_common.h"
 #include "sensirion_i2c.h"
+#include "sensirion_i2c_hal.h"

 /* all measurement commands return T (CRC) RH (CRC) */
 #define SHT4X_CMD_MEASURE_HPM 0xFD
diff --git a/shtc1/shtc1.c b/shtc1/shtc1.c
index 6f4b8d1..a8714cf 100644
--- a/shtc1/shtc1.c
+++ b/shtc1/shtc1.c
@@ -44,6 +44,7 @@
 #include "sensirion_arch_config.h"
 #include "sensirion_common.h"
 #include "sensirion_i2c.h"
+#include "sensirion_i2c_hal.h"

 /* all measurement commands return T (CRC) RH (CRC) */
 #if USE_SENSIRION_CLOCK_STRETCHING

[qcabrol]

I think it was useful for ease of integration. Not sure if the issue was just not seen ?

[rnestler]

We closed old inactive issues which didn't seem relevant anymore. But indeed it looks like nobody looked into this before, sorry for that.

Take a look at https://github.com/Sensirion/embedded-common where the HAL is defined. It got restructured quite a bit and I'm not sure if the issue is still applicable. If yes feel free to open an issue there or propose a pull request with the changes.