drivers/vl53l1x: VL53L1X ToF ranging sensor added as package #10363

gschorcht · 2018-11-10T18:49:07Z

Contribution description

This PR was originally intended only as an example for the discussion on how to add a device driver as a package. In the meantime I finished the driver. It is fully functional and therefore ready for review.

Overview

This PR adds a device driver for ST VL53L1X Time-of-Flight laser-ranging sensor. The VL53L1X allows accurate and fast ranging up to 4 m and with a frequency of up to 50 Hz.

There are three variants of the driver which differ in functionality and size:

Module	Driver	Short Description
`vl53l1x`	Standard driver	Common functionality and medium size
`vl53l1x_st_api`	ST VL53L1X API driver	Complete functionality and large size
`vl53l1x_basic`	Basic driver	Minimum functionality and small size

This vl53l1x_st_api variant of the driver uses the ST STSW-IMG007 VL53L1X API as a package (pkg/driver_vl53l1x_st_api). In this case, the driver is simply a wrapper for the API which provides a driver interface that is compatible with the other driver variants. Since the ST STSW-IMG007 VL53L1X API package is quite complex and very large, this driver variant can only be used when memory requirements are not an issue. It has to be used for best accuracy, for calibration and if threshold interrupts shall be used.

The vl53l1x_std driver variant (default) is a compromise of size and functionality. It provides the application with most functionality of the sensor. The driver can be used when memory requirements are important and most of the functionality should be used.

The vl53l1x_basic driver variant is the smallest driver variant which only provides some basic functions such as the distance measurement and the data-ready interrupt.

All driver variants provide SAUL capabilities and data-ready interrupt functionality. However, only the vl53l1x_st_api driver variant allows to use threshold interrupts.

Function / Property	`vl53l1x_basic`	`vl53l1x_std` (default)	`vl53l1x_st_api`
Distance results in mm	X	X	X
Signal rate results in MCPS		X	X
Measurement status information		X	X
SAUL capability	X	X	X
Distance mode configuration		X	X
Timing budget configuration		X	X
Inter-measurement period configuration		X	X
Region of Interest (ROI) configuration		X	X
Data-ready interrupts	X	X	X
Threshold interrupts			X
Calibration functions			X [1]
Limit check configuration			X [1]
Accuracy	medium	high	high
Size on reference platform in kByte [2]	1.0	2.9	19.8

[1] These functions are available by using the ST VL53L1X API directly.
[2] Reference platform: STM32F411RE

Testing procedure

The driver variants can be tested by using the test application with different modules where the standard variant vl53l1x_std is used by default:

make flash -C tests/driver_vl53l1x BOARD=...

To use the other driver variants, module vl53l1x_st_api or module vl53l1x_basic
have to be specified at make command line

USEMODULE=vl53l1x_st_api make flash -C tests/driver_vl53l1x BOARD=...

or

USEMODULE=vl53l1x_basic make flash -C tests/driver_vl53l1x BOARD=...

If the configuration parameter VL53L1X_PARAM_PIN_INT is defined, e.g.,

CFLAGS='-DVL53L1X_PARAM_PIN_INT=\(GPIO\(0,\1\)' make flash -C tests/driver_vl53l1x BOARD=...

interrupts are used to get data instead of polling for new data. In the case of driver variant vl53l1x_st_api, threshold interrupts are configured.

In all cases, the sensor is configured with following default parameters:

timing budget of 50 ms
intermeasurement period of 100 ms

Issues/PRs references

Some weeks ago @dylad was providing a sensor driver for the Bosch BME680 gas sensor, see PR #9909. Since the calculations are quite complex, Bosch provides a ready-to-use API. Therefore, he and @aabadie decided that it would be better to add Bosch's BME680 API driver as package.

Since the ST VL53L1X is even more complex, I decided to try it in the same way. This PR is a try to structure a driver as a package.

See also PR #9909

stale · 2019-08-10T03:08:17Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

gschorcht · 2019-08-10T12:11:55Z

It's waiting for review.

stale · 2020-02-11T12:15:14Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

javierfileiv · 2020-12-21T20:51:42Z

Hi! This is a great PR! Thanks @gschorcht ! I was working on a native VL53L0X, but it was turning into a burden with all this i2c settings that come out of nowhere. I'ḿ not sure if your porting is compatible with my breakbord. Otherwise I can port it in a similar way as you did. Thanks!

maribu · 2021-01-15T16:05:09Z

I do have some VL53L0X sensors lying around. I assume those are incompatible, is this right?

Update: I ordered two VL53L1X now. So I should be able to review and test this.

maribu

I haven't seen all files yet, but some comments inline. I can resume review when I have the hardware for testing.

drivers/vl53l1x/include/vl53l1x_params.h

maribu · 2021-01-15T16:22:19Z

drivers/vl53l1x/include/vl53l1x_register_map.h

@@ -0,0 +1,11881 @@
+/*


Would it be easily possible to provide this header via a pkg, similar to the way it is done with the vendor code for STM? But if that is non-trivial to do, I'm very much fine with having a copy in our repo.

This file is part of the ST VL53L1X driver package. Maybe we could pull the package even if it is not used.

maribu · 2021-01-15T16:24:16Z

drivers/include/vl53l1x.h

+    #if MODULE_VL53L1X_ST_API
+    VL53L1X_DIST_SHORT  = VL53L1_DISTANCEMODE_SHORT,  /**< up to 1.3 m */
+    VL53L1X_DIST_MEDIUM = VL53L1_DISTANCEMODE_MEDIUM, /**< up to 2 m */
+    VL53L1X_DIST_LONG   = VL53L1_DISTANCEMODE_LONG,   /**< up to 4 m */
+    #else
+    VL53L1X_DIST_SHORT,     /**< up to 1.3 m */
+    VL53L1X_DIST_MEDIUM,    /**< up to 2 m */
+    VL53L1X_DIST_LONG,      /**< up to 4 m */
+    #endif


Would the be a downside to just unconditionally using the constants which are compatible with the ST API in any case?

These symbols are defined in pkg/driver_vl53l1x_st_api/vl53l1x-st-api/vl53l1_def.h. They are available only if the ST VL53L1X driver is used by enabling the vl53l1x_st_api module.

maribu · 2021-01-15T16:26:19Z

drivers/include/vl53l1x.h

+ */
+int vl53l1x_read_mm(vl53l1x_t *dev, int16_t *mm);
+
+#if !MODULE_VL53L1X_BASIC


Suggested change

#if !MODULE_VL53L1X_BASIC

#if !MODULE_VL53L1X_BASIC || DOXYGEN

It shouldn't be necessary because MODULE_VL53L1X_BASIC isn't defined when doxygen is executed so that the condition is true.

drivers/vl53l1x/vl53l1x.c

gschorcht · 2021-01-16T10:26:44Z

@maribu Ups, I have to appologize, my plan was to improve the code a bit before because the code is quite old and contains a lot of code style violations 😟 Anyway, thanks for starting the review.

maribu · 2022-01-05T08:24:13Z

I'm still on vacation and reviewing larger PRs from my smartphone is cumbersome, also I cannot do testing. But it has a prominent position in my todo list ;)

maribu

I only did a quick look, but code looks good so far.

drivers/include/vl53l1x.h

maribu · 2022-01-05T14:28:51Z

drivers/vl53l1x/vl53l1x.c

+#if IS_USED(MODULE_VL53L1X_ST_API)
+
+    EXEC_RET(VL53L1_SetMeasurementTimingBudgetMicroSeconds(&dev->dev,
+                                                           budget_us));
+#else


Is there a chance that this would work as C conditionals? That way we can be sure that both ST-API and pololu variant can be compiled without having to provide two test applications. (More instances above and below.)

Hm, there are a lot of defines and symbol mappings dependent on whether the ST API is used or not that I can't imaging that compilation works if we use C conditionals. I can try it, but I'm not optimistic.

I think there is no chance in this case. The problem is that the ST API is used as a package, so of course the header files with a lot of defines and declarations required during compilation are not available if the package is not used by the driver:

drivers/vl53l1x/vl53l1x.c:525:14: error: implicit declaration of function 'VL53L1_SetMeasurementTimingBudgetMicroSeconds' [-Werror=implicit-function-declaration] EXEC_RET(VL53L1_SetMeasurementTimingBudgetMicroSeconds(&dev->dev, ^

To solve this problem we would have to pull the package in any case and insert the headers always, even if they are not used by the driver. IMHO, that would be the wrong approach just to use C Conditionals.

Even if we would do it in that way, there would raise another compilation problem because the driver defines some mappings to be able to use same code independent on whether the ST API is used or not. These mappings would result in a conflict if the ST API is used.

I fully agree that the CI coverage is an argument to use C conditionals. But in this case I would prefer to keep the preprocessor conditionals and to provide a second test application. Would it be ok for you?

maribu · 2022-01-05T14:35:23Z

drivers/vl53l1x/vl53l1x.c

+        }
+        ztimer_sleep(ZTIMER_MSEC, 1);
+    }
+    if (timeout == 0) {


Suggested change

if (timeout == 0) {

if (timeout == UINT_MAX) {

The postdecrement will still kick in, see:

~ $ cat test.c #include <stdio.h> int main(void) { unsigned foo = 3; while (foo--) { } printf("foo = %u\n", foo); return 0; } ~ $ gcc -o test test.c ~ $ ./test foo = 4294967295

But IMO it would be more readable to just go for

while (timeout) { timeout--; ...

instead.

The postdecrement will still kick in, see:

Yeah of course. Good catch. Fixed.

stale · 2022-07-10T11:11:58Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

riot-ci · 2022-12-13T08:49:24Z

Murdock results

❌ FAILED

d52ab8f fixup! tests/driver_vl53l1x: add test app for vl53l1x_st_api driver

Success	Failures	Total	Runtime
6703	0	7970	10m:51s

Artifacts

gschorcht · 2023-09-10T11:40:49Z

Rebased another time to fix the conflicts and updated to be compatible with the tests/drivers directory structure.

gschorcht added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Nov 10, 2018

gschorcht force-pushed the drivers_vl53l1x_st branch 2 times, most recently from 4afebb6 to f54431c Compare November 12, 2018 12:12

gschorcht added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: pkg Area: External package ports Area: drivers Area: Device drivers labels Nov 12, 2018

gschorcht force-pushed the drivers_vl53l1x_st branch from f54431c to 14043fc Compare November 15, 2018 14:30

gschorcht changed the title ~~drivers/vl53l1x: VL53L1X ToF rangins sensor added as package~~ drivers/vl53l1x: VL53L1X ToF ranging sensor added as package Nov 17, 2018

gschorcht added the Area: SAUL Area: Sensor/Actuator Uber Layer label Nov 18, 2018

gschorcht mentioned this pull request Nov 29, 2018

pkg/bme680 driver: add support for I2C/SPI BME680 driver #10502

Closed

dylad mentioned this pull request Nov 29, 2018

[RFC] Introduce drivers as package #10506

Closed

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

gschorcht mentioned this pull request Sep 9, 2019

sys/arduino: Using Arduino libraries #12180

Merged

gschorcht mentioned this pull request Nov 15, 2019

driver/bme680: add I2C/SPI driver for BME680 device #12717

Merged

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Feb 11, 2020

gschorcht removed the State: stale State: The issue / PR has no activity for >185 days label Feb 11, 2020

miri64 requested a review from benpicco June 24, 2020 13:57

gschorcht mentioned this pull request Jan 15, 2021

drivers/bme680: replace pkg with module #15738

Closed

maribu reviewed Jan 15, 2021

View reviewed changes

maribu reviewed Jan 5, 2022

View reviewed changes

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jul 10, 2022

gschorcht added State: don't stale State: Tell state-bot to ignore this issue and removed State: stale State: The issue / PR has no activity for >185 days State: don't stale State: Tell state-bot to ignore this issue labels Jul 10, 2022

gschorcht force-pushed the drivers_vl53l1x_st branch from 3e982fa to b06ad42 Compare December 13, 2022 01:56

gschorcht requested a review from MrKevinWeiss as a code owner December 13, 2022 01:56

gschorcht added 18 commits September 10, 2023 13:32

drivers/vl53l1x: add driver for ST VL53L1X sensor

55b315f

pkg/driver_vl53l1x_st_api: vendor API used by vl53l1x driver

31cbc2a

drivers/vl53l1x: SAUL integration

b85d14e

tests/driver_vl53l1x: add test app for vl53l1x driver

7df6f4a

driver/vl53l1x: add Kconfig

2812f88

fixup! drivers/vl53l1x: add driver for ST VL53L1X sensor

b4e47ea

fixup! drivers/vl53l1x: SAUL integration

39ce37c

fixup! pkg/driver_vl53l1x_st_api: vendor API used by vl53l1x driver

a61ebaa

fixup! pkg/driver_vl53l1x_st_api: vendor API used by vl53l1x driver

963837a

fixup! drivers/vl53l1x: add driver for ST VL53L1X sensor

3f7f28a

fixup! drivers/vl53l1x: add driver for ST VL53L1X sensor

ca6ca0e

fixup! tests/driver_vl53l1x: add test app for vl53l1x driver

23a0cb2

fixup! driver/vl53l1x: add Kconfig

9947675

tests/driver_vl53l1x: add test app for vl53l1x_st_api driver

e2abaf0

fixup! driver/vl53l1x: add Kconfig

4682545

fixup! tests/driver_vl53l1x: add test app for vl53l1x_st_api driver

6e9ed3a

fixup! tests/driver_vl53l1x: add test app for vl53l1x driver

0c1209f

fixup! tests/driver_vl53l1x: add test app for vl53l1x_st_api driver

d52ab8f

gschorcht force-pushed the drivers_vl53l1x_st branch from ecf1139 to d52ab8f Compare September 10, 2023 11:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/vl53l1x: VL53L1X ToF ranging sensor added as package #10363

drivers/vl53l1x: VL53L1X ToF ranging sensor added as package #10363

gschorcht commented Nov 10, 2018 •

edited

Loading

stale bot commented Aug 10, 2019

gschorcht commented Aug 10, 2019

stale bot commented Feb 11, 2020

javierfileiv commented Dec 21, 2020

maribu commented Jan 15, 2021 •

edited

Loading

maribu left a comment

maribu Jan 15, 2021

gschorcht Dec 7, 2021

maribu Jan 15, 2021

gschorcht Dec 7, 2021

maribu Jan 15, 2021

gschorcht Dec 7, 2021

gschorcht commented Jan 16, 2021

maribu commented Jan 5, 2022

maribu left a comment

maribu Jan 5, 2022

gschorcht Dec 15, 2022

gschorcht Dec 15, 2022

maribu Jan 5, 2022

gschorcht Dec 15, 2022

stale bot commented Jul 10, 2022

riot-ci commented Dec 13, 2022 •

edited

Loading

gschorcht commented Sep 10, 2023

	#if !MODULE_VL53L1X_BASIC
	#if !MODULE_VL53L1X_BASIC \|\| DOXYGEN

drivers/vl53l1x: VL53L1X ToF ranging sensor added as package #10363

Are you sure you want to change the base?

drivers/vl53l1x: VL53L1X ToF ranging sensor added as package #10363

Conversation

gschorcht commented Nov 10, 2018 • edited Loading

Contribution description

Overview

Testing procedure

Issues/PRs references

stale bot commented Aug 10, 2019

gschorcht commented Aug 10, 2019

stale bot commented Feb 11, 2020

javierfileiv commented Dec 21, 2020

maribu commented Jan 15, 2021 • edited Loading

maribu left a 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

gschorcht commented Jan 16, 2021

maribu commented Jan 5, 2022

maribu left a 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

stale bot commented Jul 10, 2022

riot-ci commented Dec 13, 2022 • edited Loading

Murdock results

Artifacts

gschorcht commented Sep 10, 2023

gschorcht commented Nov 10, 2018 •

edited

Loading

maribu commented Jan 15, 2021 •

edited

Loading

riot-ci commented Dec 13, 2022 •

edited

Loading