Mlx90393 sensor driver #4

ristaum · 2024-02-07T09:39:23Z

Contribution description

Testing procedure

Issues/PRs references

gschorcht

Ich habe mal ein erstes mal darüber geschaut. Um die Zahl der Anmerkungen überschaubar zu halten, erst einmal bis hierher. Es gibt natürlich weitere Anmerkungen.

Bitte zunächst auch make static-test in riot/riotbuild ausführen und die Fehler fixen. Das spart weitere zahlreiche Kommentare.

gschorcht · 2024-02-08T10:28:34Z

drivers/mlx90393/include/mlx90393_constants.h

+#define MLX90393_CONSTANTS_H
+
+#ifdef __cplusplus
+extern "C" //{


Suggested change

extern "C" //{

extern "C" {

gschorcht · 2024-02-08T10:28:52Z

drivers/mlx90393/include/mlx90393_constants.h

+#define MLX90393_T_CONV_END         120     /**< Time to end analog active mode in us **/
+
+#ifdef __cplusplus
+//}


Suggested change

//}

}

drivers/include/mlx90393.h

drivers/mlx90393/mlx90393.c

drivers/mlx90393/include/mlx90393_constants.h

drivers/include/mlx90393.h

gschorcht · 2024-02-09T09:42:56Z

Sie sollten am besten git commit --fixup verwenden um die vorhandenen Commits zu ergänzen. Alternativ könnten Sie am Ende die Commits neu aufteilen.

gschorcht · 2024-02-09T09:44:40Z

Hatten Sie jetzt bereits make static-test laufen lassen und die Fehler mit bereinigt oder kommt das noch?

ristaum · 2024-02-09T09:55:35Z

Hatten Sie jetzt bereits make static-test laufen lassen und die Fehler mit bereinigt oder kommt das noch?

Habe ich laufen lassen und die Fehler behoben, welche von meinen Dateien kamen, z.B Whitespace Fehler und Macros nicht kommentiert. Es kommen noch weitere Fehler, aber da würde ich vermuten, dass die nicht von meinen Änderungen kommen.

gschorcht · 2024-02-09T10:04:31Z

Um diese Fehler müssen Sie sich nicht kümmern.

drivers/include/mlx90393.h

gschorcht · 2024-02-09T11:04:04Z

Bei liefern die statischen Tests noch Fehler:

riotbuild@324e3a6e1e43:~$ make static-test
./dist/tools/ci/static_tests.sh
Running "./dist/tools/whitespacecheck/check.sh master" x
Command output:

	drivers/mlx90393/include/mlx90393_constants.h:83: trailing whitespace.
	+ * 
	drivers/mlx90393/mlx90393.c:627: new blank line at EOF.
	drivers/mlx90393/include/mlx90393_constants.h:83: trailing whitespace.
	+ * 
	drivers/mlx90393/mlx90393.c:627: new blank line at EOF.
	ERROR: This change introduces new whitespace errors

Running "./dist/tools/licenses/check.sh" ✓
Running "./dist/tools/licenses/check.sh" ✓
Running "./dist/tools/doccheck/check.sh" x
Command output:

	ERROR: Doxygen generates the following warnings:
	drivers/include/mlx90393.h:144: warning: end of comment block while expecting command </center>
	drivers/include/mlx90393.h:144: warning: end of comment block while expecting command </center>

Running "./dist/tools/externc/check.sh" ✓
Running "./dist/tools/vera++/check.sh" ✓
Command output:
	drivers/mlx90393/include/mlx90393_constants.h:83: warning: trailing whitespace
	drivers/mlx90393/mlx90393.c:54: warning: too many consecutive empty lines
	drivers/mlx90393/mlx90393.c:226: warning: keyword 'if' not followed by a single space
	drivers/mlx90393/mlx90393.c:284: warning: keyword 'if' not followed by a single space
	drivers/mlx90393/mlx90393.c:627: warning: trailing empty line(s)

Running "./dist/tools/coccinelle/check.sh" ✓
Running "./dist/tools/flake8/check.sh" ✓
Running "./dist/tools/headerguards/check.sh" ✓
Running "./dist/tools/buildsystem_sanity_check/check.sh" 
Running "./dist/tools/codespell/check.sh" x
Command output:

	There are typos in the following files:
	
	drivers/include/mlx90393.h:31: tresholds ==> thresholds
	drivers/include/mlx90393.h:77: Absolut ==> Absolute
	drivers/include/mlx90393.h:270: Tresholds ==> Thresholds
	drivers/include/mlx90393.h:273: Treshold ==> Threshold
	drivers/include/mlx90393.h:297: treshold ==> threshold
	drivers/include/mlx90393.h:297: Treshold ==> Threshold
	drivers/mlx90393/include/mlx90393_params.h:138: treshold ==> threshold
	drivers/mlx90393/include/mlx90393_params.h:173: treshold ==> threshold
	drivers/mlx90393/mlx90393.c:105: avaiable ==> available
	drivers/mlx90393/mlx90393.c:200: tresholds ==> thresholds
	drivers/mlx90393/mlx90393.c:202: treshold ==> threshold
	drivers/mlx90393/mlx90393.c:204: treshold ==> threshold
	drivers/mlx90393/mlx90393.c:206: treshold ==> threshold
	If those are false positives, add them to dist/tools/codespell/ignored_words.txt

Running "./dist/tools/uncrustify/uncrustify.sh --check" ✓
Command output:

	All files are uncrustified!

Running "./dist/tools/shellcheck/check.sh" ✓

drivers/mlx90393/include/mlx90393_constants.h

ristaum · 2024-02-09T11:55:59Z

Bei liefern die statischen Tests noch Fehler:

riotbuild@324e3a6e1e43:~$ make static-test
./dist/tools/ci/static_tests.sh
Running "./dist/tools/whitespacecheck/check.sh master" x
Command output:

	drivers/mlx90393/include/mlx90393_constants.h:83: trailing whitespace.
	+ * 
	drivers/mlx90393/mlx90393.c:627: new blank line at EOF.
	drivers/mlx90393/include/mlx90393_constants.h:83: trailing whitespace.
	+ * 
	drivers/mlx90393/mlx90393.c:627: new blank line at EOF.
	ERROR: This change introduces new whitespace errors

Running "./dist/tools/licenses/check.sh" ✓
Running "./dist/tools/licenses/check.sh" ✓
Running "./dist/tools/doccheck/check.sh" x
Command output:

	ERROR: Doxygen generates the following warnings:
	drivers/include/mlx90393.h:144: warning: end of comment block while expecting command </center>
	drivers/include/mlx90393.h:144: warning: end of comment block while expecting command </center>

Running "./dist/tools/externc/check.sh" ✓
Running "./dist/tools/vera++/check.sh" ✓
Command output:
	drivers/mlx90393/include/mlx90393_constants.h:83: warning: trailing whitespace
	drivers/mlx90393/mlx90393.c:54: warning: too many consecutive empty lines
	drivers/mlx90393/mlx90393.c:226: warning: keyword 'if' not followed by a single space
	drivers/mlx90393/mlx90393.c:284: warning: keyword 'if' not followed by a single space
	drivers/mlx90393/mlx90393.c:627: warning: trailing empty line(s)

Running "./dist/tools/coccinelle/check.sh" ✓
Running "./dist/tools/flake8/check.sh" ✓
Running "./dist/tools/headerguards/check.sh" ✓
Running "./dist/tools/buildsystem_sanity_check/check.sh" 
Running "./dist/tools/codespell/check.sh" x
Command output:

	There are typos in the following files:
	
	drivers/include/mlx90393.h:31: tresholds ==> thresholds
	drivers/include/mlx90393.h:77: Absolut ==> Absolute
	drivers/include/mlx90393.h:270: Tresholds ==> Thresholds
	drivers/include/mlx90393.h:273: Treshold ==> Threshold
	drivers/include/mlx90393.h:297: treshold ==> threshold
	drivers/include/mlx90393.h:297: Treshold ==> Threshold
	drivers/mlx90393/include/mlx90393_params.h:138: treshold ==> threshold
	drivers/mlx90393/include/mlx90393_params.h:173: treshold ==> threshold
	drivers/mlx90393/mlx90393.c:105: avaiable ==> available
	drivers/mlx90393/mlx90393.c:200: tresholds ==> thresholds
	drivers/mlx90393/mlx90393.c:202: treshold ==> threshold
	drivers/mlx90393/mlx90393.c:204: treshold ==> threshold
	drivers/mlx90393/mlx90393.c:206: treshold ==> threshold
	If those are false positives, add them to dist/tools/codespell/ignored_words.txt

Running "./dist/tools/uncrustify/uncrustify.sh --check" ✓
Command output:

	All files are uncrustified!

Running "./dist/tools/shellcheck/check.sh" ✓

Ich habe make static-test im RIOT root Verzeichnis ausgeführt. Ein riotbuild Ordner habe ich nicht in meinem repository. Kann das der Grund für die unterschiedlichen ausgaben sein?

gschorcht · 2024-02-09T11:57:38Z

Sie müssen make static-test in docker ausführen:

docker run -i -t --privileged -v /dev:/dev -u $UID -v $(pwd):/data/riotbuild riot/riotbuild

ristaum · 2024-02-09T12:29:18Z

Sie müssen make static-test in docker ausführen:
docker run -i -t --privileged -v /dev:/dev -u $UID -v $(pwd):/data/riotbuild riot/riotbuild

Jetzt müssten alle Fehler weg sein.

gschorcht · 2024-02-15T15:32:14Z

drivers/include/mlx90393.h

+ */
+
+/**
+ * @defgroup    drivers_mlx90393 MLX90393


Suggested change

* @defgroup drivers_mlx90393 MLX90393

* @defgroup drivers_mlx90393 MLX90393 3-axis magnetometer

gschorcht · 2024-02-16T11:05:42Z

drivers/include/mlx90393.h

+ *       - OSR_0 and DIG_FILT_0
+ *       - OSR_0 and DIG_FILT_1
+ *       - OSR_1 and DIG_FILT_0


?

Suggested change

* - OSR_0 and DIG_FILT_0

* - OSR_0 and DIG_FILT_1

* - OSR_1 and DIG_FILT_0

* - #MLX90393_OSR_0 and #MLX90393_DIG_FILT_0

* - #MLX90393_OSR_0 and #MLX90393_DIG_FILT_1

* - #MLX90393_OSR_1 and #MLX90393_DIG_FILT_0```

gschorcht · 2024-02-16T12:10:41Z

tests/drivers/mlx90393/main.c

+
+int main(void)
+{
+    puts("MAG3110 magnetometer driver test application\n\r");


Suggested change

puts("MAG3110 magnetometer driver test application\n\r");

puts("MLX90393 magnetometer driver test application\n\r");

gschorcht · 2024-02-16T12:11:13Z

tests/drivers/mlx90393/main.c

+    mlx90393_t dev;
+    int error = 0;
+    if ((error = mlx90393_init(&dev, &mlx90393_params[0])) != 0) {


Suggested change

mlx90393_t dev;

int error = 0;

if ((error = mlx90393_init(&dev, &mlx90393_params[0])) != 0) {

mlx90393_t dev;

int error = 0;

if ((error = mlx90393_init(&dev, &mlx90393_params[0])) != 0) {

gschorcht · 2024-02-16T12:12:21Z

tests/drivers/mlx90393/main.c

+    unsigned count = 0;
+
+    mlx90393_data_t data;
+    puts("Starting read data from the device");
+    while (1) {


Suggested change

unsigned count = 0;

mlx90393_data_t data;

puts("Starting read data from the device");

while (1) {

unsigned count = 0;

mlx90393_data_t data;

puts("Starting read data from the device");

while (1) {

gschorcht · 2024-02-16T12:15:08Z

drivers/mlx90393/include/mlx90393_constants.h

+#define MLX90393_TEMP_RESOLUTION    452     /**< Temperature sensor resolution
+                                            * (45.2 * 10 for avoiding float values) */


Ich denke, so wäre es besser als die misaligned Variante

Suggested change

#define MLX90393_TEMP_RESOLUTION 452 /**< Temperature sensor resolution

* (45.2 * 10 for avoiding float values) */

#define MLX90393_TEMP_RESOLUTION 452 /**< Temperature sensor resolution

(45.2 * 10 for avoiding float values) */

gschorcht · 2024-02-16T12:32:26Z

drivers/mlx90393/mlx90393_saul.c

+{
+    /* Maximum value that can be achieved is +- 106480 uT according to Table 17
+    and 21 from Datasheet. To fit in signed 16 bit only the 16 MSB are passed. */
+    mlx90393_data_t data;


Suggested change

mlx90393_data_t data;

mlx90393_data_t data;

gschorcht · 2024-02-16T13:03:20Z

drivers/mlx90393/mlx90393_saul.c

+        res->val[0] = (int16_t) data.x_axis >> 2;
+        res->val[1] = (int16_t) data.y_axis >> 2;
+        res->val[2] = (int16_t) data.z_axis >> 2;
+        res->unit = UNIT_T;
+        res->scale = -6;


Laut documentation liefert mlx90393_read die Daten in uTesla. Wenn Sie die aber durch 4 Teilen, kommt etwas komisches heraus, aber nicht uTesla.
Zudem würde in Ihrem Fall erst der Type Cast (Narrowing Conversion) und dann die Division erfolgen.

Wenn Sie ein Problem mit dem Wertebereich haben, wäre eine mögliche Lösung um möglichst wenig Genauigkeit zu verlieren aus meiner Sicht:

Suggested change

res->val[0] = (int16_t) data.x_axis >> 2;

res->val[1] = (int16_t) data.y_axis >> 2;

res->val[2] = (int16_t) data.z_axis >> 2;

res->unit = UNIT_T;

res->scale = -6;

res->val[0] = (int16_t)(data.x_axis / 10);

res->val[1] = (int16_t)(data.y_axis / 10);

res->val[2] = (int16_t)(data.z_axis / 10);

res->unit = UNIT_T;

res->scale = -5;

Oder alternativ gleich in mTesla:

Suggested change

res->val[0] = (int16_t) data.x_axis >> 2;

res->val[1] = (int16_t) data.y_axis >> 2;

res->val[2] = (int16_t) data.z_axis >> 2;

res->unit = UNIT_T;

res->scale = -6;

res->val[0] = (int16_t) data.x_axis / 1000;

res->val[1] = (int16_t) data.y_axis / 1000;

res->val[2] = (int16_t) data.z_axis / 1000;

res->unit = UNIT_T;

res->scale = -3;

Dazu fehlt mir aber das Gefühl in welchem Wertebreich sich die Werte bewegen und wie genau das Ergebnis sein muss.

gschorcht · 2024-02-16T13:05:21Z

drivers/include/mlx90393.h

+ *
+ * The MLX90393 sensor can be shutdown into idle mode when no continuous measurements are
+ * required using the function #mlx90393_stop_cont. The power consumption is then reduced to
+ * max. 5 uA. To restart the MLX90393 in previous continuous measurement mode,


Suggested change

* max. 5 uA. To restart the MLX90393 in previous continuous measurement mode,

* a maxiumum of 5 uA. To restart the MLX90393 in previous continuous measurement mode,

gschorcht · 2024-02-16T13:25:49Z

drivers/mlx90393/mlx90393.c

+        }
+    }
+    /* wake up on change mode */
+    else if (DEV_MODE == MLX90393_MODE_WOC_ABSOLUTE || DEV_MODE == MLX90393_MODE_WOC_RELATIVE) {


Ich überlege noch, ob man den Code weiter optimieren könnte, wenn man ein Pseudomodule mlx90393_int einführt, mit dem die Interrupt-Nutzung aktiviert wird.

Erstens würde das den kompletten else-Zweig heraus optimieren, wenn das Module nicht genutzt wird

Suggested change

else if (DEV_MODE == MLX90393_MODE_WOC_ABSOLUTE || DEV_MODE == MLX90393_MODE_WOC_RELATIVE) {

else if (IS_USED(MODULE_MLX90393_INT) &&

(DEV_MODE == MLX90393_MODE_WOC_ABSOLUTE || DEV_MODE == MLX90393_MODE_WOC_RELATIVE)) {

was nicht passiert, wenn nur auf dev->mode getestet wird, weil zur Compile-Zeit nicht klar ist, welchen Wert dev->mode hat.

Zweitens könnte man den Member mlx90393_params_t::int_pin sparen, wenn es nicht benutzt wird bzw. den Member mlx90393_t::conversion_time sowie den gesamten Code mit Berechnung und Nutzung von _calculate_conv_time heraus optimieren, wenn mlx90393_irq benutzt wird.

Wenn mlx90393_int enabled ist, reicht ein einfaches assert(gpio_is_valid(params->int_pin)) um sicherzustellen, dass das Interrupt Pin für die Interrupt-Nutzung definiert ist, was wiederum Code spart.

gschorcht · 2024-02-16T13:30:22Z

drivers/mlx90393/mlx90393.c

+            return error;
+        }
+    }
+    if (DEV_MODE == MLX90393_MODE_SINGLE_MEASUREMENT && !gpio_is_valid(DEV_INT_PIN)) {


Siehe Anmerkung zu Pseudomodule mlx90393_int:

Suggested change

if (DEV_MODE == MLX90393_MODE_SINGLE_MEASUREMENT && !gpio_is_valid(DEV_INT_PIN)) {

if (!ISUSED(MODULE_MLX90393) && (DEV_MODE == MLX90393_MODE_SINGLE_MEASUREMENT)) {

ristaum · 2024-03-10T21:05:28Z

Ich hätte da noch eine Frage. Soll es bei der Verwendung des Pseudo Moduls mlx90393_woc trotzdem möglich sein den Single Measurement und Burst Mode zu nutzen oder werden diese dann wegoptimiert?

gschorcht · 2024-03-11T07:20:25Z

Ich hätte da noch eine Frage. Soll es bei der Verwendung des Pseudo Moduls mlx90393_woc trotzdem möglich sein den Single Measurement und Burst Mode zu nutzen oder werden diese dann wegoptimiert?

Die Frage ist, wie wäre der Use Case? Aus meiner Sicht wäre es so, dass eine Anwendung WOC nutzt, um getriggert zu werden, wenn ein Grenzwert überschritten wird, würde dann aber auch die Werte wissen wollen.

Hier zunächst die Frage: Sind die Messwerte im Sensor nach einem WOC Interrupt gültig, d.h könnte man die Messwerte ohne erneute Messung auslesen oder müsste man auf den nächsten Messewert warten. Im ersteren Fall müssten wir nochmal über den Ansatz mit mlx90393_star_cont und mlx90393_read nachdenken.

Bei Nutzung von mlx90393_int wird Single Measurement und Burst Mode mit Interrupt genutzt, Polling dann aber nicht mehr unterstützt.

ristaum · 2024-03-11T10:44:55Z

Die Messwerte sind nach neinem WOC Interrupt gültig. Man wartet mit mlx90393_read darauf, dass der Interrupt auslöst und liest direkt die Messung aus, welche den Grenzwert überschritten hat.

gschorcht · 2024-03-11T12:45:07Z

Man wartet mit mlx90393_read darauf, dass der Interrupt auslöst und liest direkt die Messung aus, welche den Grenzwert überschritten hat.

Hm, da tut sich vielleicht noch ein Design-Problem auf, wenn man die Use Cases betrachtet, dass wir so bisher vielleicht nicht richtig betrachtet haben.

Use Case 1: App liest gezielt Werte im Single Measurement oder Burst Mode mittels Interrupt oder Polling.
Use Case 2: WOC soll als Ereignis behandelt werden und die App darauf reagieren.

In Use Case 1 würde der laufende Thread eine Messung triggern, wenn er den Messwert braucht und "kurz" auf das Ergebnis warten (Single Measurement Mode) bzw. periodisch Messwerte auslesen (Burst Mode).

In Use Case 2 soll jedoch auf ein Ereignis reagiert werden, welches selten oder auch nie eintritt. In diesem Fall würde der Thread dauerhaft blockieren, was dazu führt, dass man einen eigenen Thread nur für das potentielle Eintreten eines solchen Ereignisses bräuchte. Ein Thread ist teuer im Sinne des verfügbaren RAMs.

Insofern wäre es vielleicht besser, die API doch noch wie folgt etwas zu ändern

typedef void (*mlx90393_cb_t)(void *);

int mlx90393_start_cont(mlx90393_t *dev);
int mlx90393_stop_cont(mlx90393_t *dev);
int mlx90393_read(mlx90393_t *dev, mlx90393_data_t *data);

int mlx90393_enable_woc(mlx90393_t *dev, mlx90393_cb_t cb, void *arg);

mlx90393_cb_t wäre eine Callback-Funktion der Anwendung, die anstelle von _isr in gpio_init_int als Callback-Funktion registriert wird. Wichtig wäre dann mit @warning darauf hinzuweisen, dass diese im Interrupt Context aufgerufen wird und daher nicht auf den Sensor zugegriffen werden darf, also auch die Werte nicht gelesen werden dürfen und auch nichts time-consuming ausgeführt werden darf.

Ein übliche Nutzung wäre aber in der Callback-Function einfach ein Flag des App-Threads zu setzen, welches in der Abarbeitung im App-Thread dann ausgewertet wird und dann im Falle eines gesetzten Flags die App im Thread-Context die Sensor-Werte bei Bedarf mit mlx90393_read auslesen kann oder irgendetwas anderes tun kann, wenn sie nicht die konkreten Werte braucht sondern nur das Signal, dass Schwellwerte überschritten wurden.

Mit mlx90393_read würde dann

im Single Measurement Mode eine Messung gestartet, gewartet und gelesen werden
im Burst Mode gewartet und gelesen werden
im WOC Mode einfach nur gelesen werden

Eine Frage noch zum Verständnis, wenn eine Single Measurement in mlx90393_read gestartet wird und so schnell fertig ist, dass der DRDY Interrupt bereits kommt bevor das Interrupt-Pin mit gpio_init_int initialisiert und das Mutex gelockt ist, geht der Interrupt dann verloren und der Thread blockiert dauerhaft?

ristaum · 2024-03-11T14:01:23Z

Okay und heißt das jetzt, dass bei der Verwendung des mlx90393_woc Pseudomodules die Single Measurement und Burst Modes nicht mehr unterstützt werden?

Eine Frage noch zum Verständnis, wenn eine Single Measurement in mlx90393_read gestartet wird und so schnell fertig ist, dass der DRDY Interrupt bereits kommt bevor das Interrupt-Pin mit gpio_init_int initialisiert und das Mutex gelockt ist, geht der Interrupt dann verloren und der Thread blockiert dauerhaft?

Ja das wäre wohl möglich. Ist das realistisch bei einer minimalen Dauer einer Messung im Single Measurement Mode von 1772 us?

gschorcht · 2024-03-11T14:55:51Z

Okay und heißt das jetzt, dass bei der Verwendung des mlx90393_woc Pseudomodules die Single Measurement und Burst Modes nicht mehr unterstützt werden?

Die Nutzung eines Pseudomodules ist in diesem Fall als Freischalten zusätzlicher Features zu verstehen. Es ist ja nicht auszuschließen, dass eine Anwendung beispielsweise für eine bestimmte Zeit regelmäßig weiter messen will, sobald ein WOC eingetreten ist, bevor sie wieder in den WOC Mode wechselt. Die Funktionalität des Single Measurement Mode und des Burst Mode im Polling würde ich als Standardfunktionalität betrachten, WOC und Interrupt als zusätzliche Funktionalität. Natürlich könnte man auch mlx90393_sm und mlx90393_bm als Pseudomodule definieren und beispielsweise mlx90393_bm standardmäßig aktivieren wenn kein anderes mlx90393_* Pseudomodule aktiviert ist. Das würde es aber unnötig verkomplizieren.

Ja das wäre wohl möglich. Ist das realistisch bei einer minimalen Dauer einer Messung im Single Measurement Mode von 1772 us?

Nun ja, ich weiß nicht, wie lange ein ATmega 328p mit 8 MHz Takt dafür braucht ein GPIO und ein Mutex zu initialisieren und zu locken.

gschorcht · 2024-08-16T10:54:03Z

drivers/mlx90393/include/mlx90393_params.h

                                .gain           = MLX90393_PARAM_GAIN,          \
                                .resolution     = MLX90393_PARAM_RES,           \
                                .odr            = MLX90393_PARAM_ODR,           \
                                .oversampling   = MLX90393_PARAM_OSR,           \
                                .dig_filt       = MLX90393_PARAM_DIG_FILT,      \
-                                .threshold      = MLX90393_PARAM_THRESHOLD      \
+                                MLX90393_PARAM_INT_PIN                          \


Suggested change

MLX90393_PARAM_INT_PIN \

MLX90393_PARAM_INT \

gschorcht · 2024-08-16T11:02:02Z

drivers/mlx90393/include/mlx90393_params.h

                                .gain           = MLX90393_PARAM_GAIN,          \
                                .resolution     = MLX90393_PARAM_RES,           \
                                .odr            = MLX90393_PARAM_ODR,           \
                                .oversampling   = MLX90393_PARAM_OSR,           \
                                .dig_filt       = MLX90393_PARAM_DIG_FILT,      \
-                                .threshold      = MLX90393_PARAM_THRESHOLD      \
+                                MLX90393_PARAM_INT_PIN                          \


Suggested change

MLX90393_PARAM_INT_PIN \

MLX90393_PARAM_INT \

gschorcht · 2024-08-16T11:32:49Z

drivers/mlx90393/Kconfig

@@ -0,0 +1,15 @@
+# Copyright (c) 2023 mi6527ri


Suggested change

# Copyright (c) 2023 mi6527ri

# Copyright (C) 2023 Michael Ristau

gschorcht · 2024-08-16T11:33:13Z

drivers/mlx90393/include/mlx90393_constants.h

@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2023 mi6527ri


Suggested change

* Copyright (C) 2023 mi6527ri

* Copyright (C) 2023 Michael Ristau

gschorcht · 2024-08-16T12:26:49Z

drivers/mlx90393/Kconfig

+# Copyright (c) 2023 mi6527ri
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+config MODULE_MLX90393
+    bool "drivers_mlx90393"
+    depends on TEST_KCONFIG
+    depends on HAS_PERIPH_GPIO
+    depends on HAS_PERIPH_GPIO_IRQ
+    depends on HAS_PERIPH_I2C
+    select MODULE_PERIPH_GPIO
+    select MODULE_PERIPH_GPIO_IRQ
+    select MODULE_PERIPH_I2C


Suggested change

# Copyright (c) 2023 mi6527ri

#

# This file is subject to the terms and conditions of the GNU Lesser

# General Public License v2.1. See the file LICENSE in the top level

# directory for more details.

config MODULE_MLX90393

bool "drivers_mlx90393"

depends on TEST_KCONFIG

depends on HAS_PERIPH_GPIO

depends on HAS_PERIPH_GPIO_IRQ

depends on HAS_PERIPH_I2C

select MODULE_PERIPH_GPIO

select MODULE_PERIPH_GPIO_IRQ

select MODULE_PERIPH_I2C

Im aktuellen Master erfolgt die Dependency-Modellierung nicht mehr über Kconfig. Das File kann komplett entfallen.

Allerdings wäre es hilfreich, wenn einige der Parameter per Kconfig einstellbar wären, wie beispielsweise die Thresholds im WOC mode. Das würde alle Sensor-Parameter betreffen, die nicht hardware-mäßig vorgegeben sind, also alles ab .mode mit Ausnahme des Interrupt Pins.

gschorcht · 2024-08-16T12:33:40Z

drivers/mlx90393/include/mlx90393_params.h

+#if IS_USED(MODULE_MLX90393_INT) || DOXYGEN
+#ifndef MLX90393_PARAM_INT_PIN
+/**
+ * @brief   Default interrupt pin
+ */
+#define MLX90393_PARAM_INT_PIN          .int_pin = (GPIO_PIN(PORT_C, 8)),
+#endif
+#else
+#define MLX90393_PARAM_INT_PIN
+#endif


So kann man das leider nicht realisieren, kein Mensch wird an der make-Kommandozeile ein solches Konstrukt angeben können/wollen, um den Default-Wert zu überschreiben.

Suggested change

#if IS_USED(MODULE_MLX90393_INT) || DOXYGEN

#ifndef MLX90393_PARAM_INT_PIN

/**

* @brief Default interrupt pin

*/

#define MLX90393_PARAM_INT_PIN .int_pin = (GPIO_PIN(PORT_C, 8)),

#endif

#else

#define MLX90393_PARAM_INT_PIN

#endif

/**

* @brief Default interrupt pin

*/

#ifndef MLX90393_PARAM_INT_PIN

#define MLX90393_PARAM_INT_PIN (GPIO_PIN(PORT_C, 8))

#endif

#if IS_USED(MODULE_MLX90393_INT) || DOXYGEN

#define MLX90393_PARAM_INT .int_pin = MLX90393_PARAM_INT_PIN,

#else

#define MLX90393_PARAM_INT

#endif

In der Definition von MLX90393_PARAM_SPI bzw. MLX90393_PARAM_I2C wird dann einfach grundsätzlich MLX90393_PARAM_INT hinzugefügt, was entweder leer ist oder ebent die Deklaration des int_pin Members enthält, siehe unten.

gschorcht · 2024-08-16T12:36:07Z

drivers/mlx90393/include/mlx90393_params.h

+#if IS_USED(MODULE_MLX90393_WOC) || DOXYGEN
+#ifndef MLX90393_PARAM_THRESHOLD
+/**
+ * @brief   Default thresholds for Wake-up On Change mode
+ */
+#define MLX90393_PARAM_THRESHOLD                    \
+.threshold =                                        \
+{                                                   \
+    .xy             = 0xFFFF,                       \
+    .z              = 1000,                         \
+    .temp           = 0xFFFF                        \
+},
+#endif
+#else
+#define MLX90393_PARAM_THRESHOLD
+#endif


Das Gleiche wie bei der Definition des Interrupt-Pin, sowas kann man nicht überschreiben, zumindest nicht an der Make-Kommandozeile. Daher wie folgt:

Suggested change

#if IS_USED(MODULE_MLX90393_WOC) || DOXYGEN

#ifndef MLX90393_PARAM_THRESHOLD

/**

* @brief Default thresholds for Wake-up On Change mode

*/

#define MLX90393_PARAM_THRESHOLD \

.threshold = \

{ \

.xy = 0xFFFF, \

.z = 1000, \

.temp = 0xFFFF \

},

#endif

#else

#define MLX90393_PARAM_THRESHOLD

#endif

#ifndef MLX90393_PARAM_THRESHOLD_XY

#define MLX90393_PARAM_THRESHOLD_XY 0xFFFF

#endif

#ifndef MLX90393_PARAM_THRESHOLD_Z

#define MLX90393_PARAM_THRESHOLD_Z 1000

#endif

#ifndef MLX90393_PARAM_THRESHOLD_TEMP

#define MLX90393_PARAM_THRESHOLD_TEMP 0xFFFF

#endif

#if IS_USED(MODULE_MLX90393_WOC) || DOXYGEN

#ifndef MLX90393_PARAM_THRESHOLD

/**

* @brief Default thresholds for Wake-up On Change mode

*/

#define MLX90393_PARAM_THRESHOLD \

.threshold = { \

.xy = MLX90393_PARAM_THRESHOLD_XY, \

.z = MLX90393_PARAM_THRESHOLD_Z, \

.temp = MLX90393_PARAM_THRESHOLD_TEMP, \

},

#endif

#else

#define MLX90393_PARAM_THRESHOLD

#endif

gschorcht · 2024-08-16T12:39:02Z

drivers/mlx90393/include/mlx90393_params.h

+#ifndef MLX90393_PARAM_OSR
+/**
+ * @brief   Default oversampling ratio
+ */
+#define MLX90393_PARAM_OSR                          \
+{                                                   \
+    .mag            = MLX90393_OSR_1,               \
+    .temp           = MLX90393_OSR_1                \
+}
+#endif


Das Gleiche wie bei der Definition des Interrupt-Pin, sowas kann man nicht überschreiben, zumindest nicht an der Make-Kommandozeile. Daher wie folgt:

Suggested change

#ifndef MLX90393_PARAM_OSR

/**

* @brief Default oversampling ratio

*/

#define MLX90393_PARAM_OSR \

{ \

.mag = MLX90393_OSR_1, \

.temp = MLX90393_OSR_1 \

}

#endif

#ifndef MLX90393_PARAM_OSR_MAG

#define MLX90393_PARAM_OSR_MAG MLX90393_OSR_1

#endif

#ifndef MLX90393_PARAM_OSR_TEMP

#define MLX90393_PARAM_OSR_TEMP MLX90393_OSR_1

#endif

/**

* @brief Default oversampling ratio

*/

#define MLX90393_PARAM_OSR { \

.mag = MLX90393_PARAM_OSR_MAG, \

.temp = MLX90393_PARAM_OSR_TEMP, \

}

mi6527ri added 8 commits January 1, 2024 16:21

basic i2c functionality

a96f6af

spi support

499f354

avoid calculating with float values

c108f30

default params file + connectivity test function

f590ad1

some changes

c7b5ceb

saul implementation and test application

fda54ac

removed shell from test application

7b872d1

small changes and added documentation

a4520fe

github-actions bot added Area: doc Area: drivers Area: Kconfig Area: tests labels Feb 7, 2024

gschorcht requested changes Feb 8, 2024

View reviewed changes

drivers/mlx90393: added UNIX error codes and codestyle changes

2ae152e

gschorcht reviewed Feb 9, 2024

View reviewed changes

drivers/include/mlx90393.h Outdated Show resolved Hide resolved

gschorcht reviewed Feb 9, 2024

View reviewed changes

drivers/mlx90393/include/mlx90393_constants.h Outdated Show resolved Hide resolved

fixup! drivers/mlx90393: added UNIX error codes and codestyle changes

9eeb5df

gschorcht reviewed Feb 15, 2024

View reviewed changes

gschorcht reviewed Feb 16, 2024

View reviewed changes

drivers/mlx90393: added mlx90393_int and mlx90393_woc pseudo modules

4844ad3

ristaum force-pushed the mlx90393-sensor-driver branch from 5db7b6c to 4844ad3 Compare March 12, 2024 15:11

fixup! saul implementation and test application

019814d

gschorcht requested changes Aug 16, 2024

View reviewed changes

	* @defgroup drivers_mlx90393 MLX90393
	* @defgroup drivers_mlx90393 MLX90393 3-axis magnetometer

	puts("MAG3110 magnetometer driver test application\n\r");
	puts("MLX90393 magnetometer driver test application\n\r");

		#define MLX90393_TEMP_RESOLUTION 452 /**< Temperature sensor resolution
		* (45.2 * 10 for avoiding float values) */

	* max. 5 uA. To restart the MLX90393 in previous continuous measurement mode,
	* a maxiumum of 5 uA. To restart the MLX90393 in previous continuous measurement mode,

	else if (DEV_MODE == MLX90393_MODE_WOC_ABSOLUTE \|\| DEV_MODE == MLX90393_MODE_WOC_RELATIVE) {
	else if (IS_USED(MODULE_MLX90393_INT) &&
	(DEV_MODE == MLX90393_MODE_WOC_ABSOLUTE \|\| DEV_MODE == MLX90393_MODE_WOC_RELATIVE)) {

	if (DEV_MODE == MLX90393_MODE_SINGLE_MEASUREMENT && !gpio_is_valid(DEV_INT_PIN)) {
	if (!ISUSED(MODULE_MLX90393) && (DEV_MODE == MLX90393_MODE_SINGLE_MEASUREMENT)) {

	# Copyright (c) 2023 mi6527ri
	# Copyright (C) 2023 Michael Ristau

	* Copyright (C) 2023 mi6527ri
	* Copyright (C) 2023 Michael Ristau

-#ifndef MLX90393_PARAM_OSR
-/**
- * @brief   Default oversampling ratio
- */
-#define MLX90393_PARAM_OSR                          \
-{                                                   \
-    .mag            = MLX90393_OSR_1,               \
-    .temp           = MLX90393_OSR_1                \
-}
-#endif
+#ifndef MLX90393_PARAM_OSR_MAG
+#define MLX90393_PARAM_OSR_MAG          MLX90393_OSR_1
+#endif
+#ifndef MLX90393_PARAM_OSR_TEMP
+#define MLX90393_PARAM_OSR_TEMP         MLX90393_OSR_1
+#endif
+/**
+ * @brief   Default oversampling ratio
+ */
+#define MLX90393_PARAM_OSR {                        \
+    .mag            = MLX90393_PARAM_OSR_MAG,       \
+    .temp           = MLX90393_PARAM_OSR_TEMP,      \
+}

Mlx90393 sensor driver #4

Are you sure you want to change the base?

Mlx90393 sensor driver #4

Conversation

ristaum commented Feb 7, 2024

Contribution description

Testing procedure

Issues/PRs references

gschorcht left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht commented Feb 9, 2024

gschorcht commented Feb 9, 2024

ristaum commented Feb 9, 2024

gschorcht commented Feb 9, 2024

gschorcht commented Feb 9, 2024 • edited Loading

ristaum commented Feb 9, 2024

gschorcht commented Feb 9, 2024

ristaum commented Feb 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ristaum commented Mar 10, 2024

gschorcht commented Mar 11, 2024

ristaum commented Mar 11, 2024

gschorcht commented Mar 11, 2024 • edited Loading

ristaum commented Mar 11, 2024

gschorcht commented Mar 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht Aug 16, 2024 • edited Loading

Choose a reason for hiding this comment

gschorcht commented Feb 9, 2024 •

edited

Loading

gschorcht commented Mar 11, 2024 •

edited

Loading

gschorcht Aug 16, 2024 •

edited

Loading