drivers/vl53l1x: VL53L1X ToF ranging sensor added as package #10363
Conversation
4afebb6 to
f54431c
Compare
f54431c to
14043fc
Compare
|
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. |
|
It's waiting for review. |
|
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. |
|
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! |
|
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. |
| @@ -0,0 +1,11881 @@ | |||
| /* | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This file is part of the ST VL53L1X driver package. Maybe we could pull the package even if it is not used.
There was a problem hiding this comment.
I support this idea, that would instantly reduce the Lines of Code from 14k to 3k.
drivers/include/vl53l1x.h
Outdated
| #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 |
There was a problem hiding this comment.
Would the be a downside to just unconditionally using the constants which are compatible with the ST API in any case?
There was a problem hiding this comment.
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.
drivers/include/vl53l1x.h
Outdated
| */ | ||
| int vl53l1x_read_mm(vl53l1x_t *dev, int16_t *mm); | ||
|
|
||
| #if !MODULE_VL53L1X_BASIC |
There was a problem hiding this comment.
| #if !MODULE_VL53L1X_BASIC | |
| #if !MODULE_VL53L1X_BASIC || DOXYGEN |
There was a problem hiding this comment.
It shouldn't be necessary because MODULE_VL53L1X_BASIC isn't defined when doxygen is executed so that the condition is true.
|
@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 I would have to touch the code anyway because of the conflicts and the changes in autoinitialization. |
d52ab8f to
f96a505
Compare
| * @warning The ST STSW-IMG007 VL53L1X API package functions use a significant | ||
| * amount of memory in the stack. If you have crashes due to memory | ||
| * access errors, try increasing the default thread stack size | ||
| * \ref THREAD_STACKSIZE_DEFAULT. |
There was a problem hiding this comment.
Maybe you can give an estimation on how big the stack size should be.
| #define VL53L1X_RANGESTATUS_RANGE_VALID 0 /**< range is valid */ | ||
| #define VL53L1X_RANGESTATUS_SIGMA_FAIL 1 /**< sigma failure */ | ||
| #define VL53L1X_RANGESTATUS_SIGNAL_FAIL 2 /**< signal failure */ | ||
| #define VL53L1X_RANGESTATUS_RANGE_VALID_MIN_RANGE_CLIPPED 3 /**< target is below minimum | ||
| detection threshold */ | ||
| #define VL53L1X_RANGESTATUS_OUTOFBOUNDS_FAIL 4 /**< phase out of valid limits, | ||
| different to a wrap exit. */ | ||
| #define VL53L1X_RANGESTATUS_HARDWARE_FAIL 5 /**< hardware failure */ | ||
| #define VL53L1X_RANGESTATUS_RANGE_VALID_NO_WRAP_CHECK_FAIL 6 /**< range is valid but the wrap | ||
| around check has not been | ||
| done */ | ||
| #define VL53L1X_RANGESTATUS_WRAP_TARGET_FAIL 7 /**< wrapped target, no matching | ||
| phase in other VCSEL period | ||
| timing */ | ||
| #define VL53L1X_RANGESTATUS_XTALK_SIGNAL_FAIL 9 /**< specific to lite ranging */ | ||
| #define VL53L1X_RANGESTATUS_SYNCRONISATION_INT 10 /**< first interrupt when starting | ||
| ranging in back to back mode */ | ||
| #define VL53L1X_RANGESTATUS_MIN_RANGE_FAIL 13 /**< user ROI input is not valid */ | ||
| #define VL53L1X_RANGESTATUS_NONE 255 /**< No Update. */ |
There was a problem hiding this comment.
You can also put the documentation in the previous line, that would save some line breaks I guess.
| * @brief Interrupt mode | ||
| * | ||
| * It defines when an interrupt is triggered. Interrupts can be triggered | ||
| * either on new data in each measurement cycly or when the threshold condition |
There was a problem hiding this comment.
| * either on new data in each measurement cycly or when the threshold condition | |
| * either on new data in each measurement cycle or when the threshold condition |
| uint32_t period; /**< Inter-measurement period in ms, has to be | ||
| vl53l1x_params_t::t_budget + 4ms, | ||
| default: 100 ms */ |
There was a problem hiding this comment.
But the budget is 50ms, and 50+4ms != 100ms 🤔
| * - Since reading any data resets the interrupt as well as the data-ready | ||
| * flag, either function #vl53l1x_read_mm, function #vl53l1x_read_data, or | ||
| * function #vl53l1x_read_details can be used in one measurement cycle. | ||
| * - This function is ONLY available when module \ref vl53l1x_basic is used. |
There was a problem hiding this comment.
| * - This function is ONLY available when module \ref vl53l1x_basic is used. | |
| * - This function is ONLY available when module \ref vl53l1x_st_api is used. |
However this part of the note is redundant, because in the normal description block, the same information is already given. So you can also remove the sentence I guess.
| #include "vl53l1x.h" | ||
| #include "vl53l1x_params.h" | ||
|
|
||
| static void isr (void *arg) |
There was a problem hiding this comment.
| static void isr (void *arg) | |
| static void isr(void *arg) |
| #include "vl53l1x.h" | ||
| #include "vl53l1x_params.h" | ||
|
|
||
| static void isr (void *arg) |
There was a problem hiding this comment.
| static void isr (void *arg) | |
| static void isr(void *arg) |
| * The test application demonstrates the usage of different functions | ||
| * dependent on the used driver variant: | ||
| * | ||
| * - vl53l1x Standard driver with most functionality | ||
| * - vl53l1x_basic Basic driver with only very basic functionality | ||
| * - vl53l1x_st_api ST VL53L1X API driver with complete functionality | ||
| * | ||
| * The driver variant used is defined by my variable DRIVER, which is set to | ||
| * vl53l1x by default. In this case, it is not necessary to specify the DRIVER | ||
| * variable in the make command: | ||
| * | ||
| * make flash -C tests/driver_vl53l1x BOARD=... | ||
| * | ||
| * To use other driver variants, module vl53l1x_st_api or module vl53l1x_basic | ||
| * have to be specified at make command line | ||
| * | ||
| * DRIVER=vl53l1x_st_api make flash -C tests/driver_vl53l1x BOARD=... | ||
| * | ||
| * or | ||
| * | ||
| * DRIVER=vl53l1x_basic make flash -C tests/driver_vl53l1x BOARD=... | ||
| * | ||
| * If the configuration parameter VL53L1X_PARAM_PIN_INT is defined, 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. Otherwise | ||
| * only data interrupts are used. | ||
| * | ||
| * In all cases, the sensor is configured with following default parameters: | ||
| * | ||
| * - timing budget of 50 ms | ||
| * - intermeasurement period of 100 ms |
There was a problem hiding this comment.
Not sure if it makes sense to replicate the content of the README.md here?
There was a problem hiding this comment.
I think this is also the case for other .c files.
|
|
||
| ## Usage | ||
|
|
||
| The driver variant used is defined by my variable `DRIVER`, which is set to |
There was a problem hiding this comment.
IMO the "defined by my variable" sounds a bit weird.
| @@ -0,0 +1,33 @@ | |||
| /* | |||
| * SPDX-FileCopyrightText: 20215 Gunar Schorcht | |||
There was a problem hiding this comment.
| * SPDX-FileCopyrightText: 20215 Gunar Schorcht | |
| * SPDX-FileCopyrightText: 2025 Gunar Schorcht |
Oh sorry, I forgot to set the WIP label. I picked up this PR after it was not touched for 4 years and just started with rebasing it. Now, I'm going through step by step to see what have to be done do update this PR. It#s definitly too early to review it for the moment. |
|
Closed because of a complete rework in PR #21648. |
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:
vl53l1xvl53l1x_st_apivl53l1x_basicThis
vl53l1x_st_apivariant 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_stddriver 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_basicdriver 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_apidriver variant allows to use threshold interrupts.vl53l1x_basicvl53l1x_std(default)vl53l1x_st_api[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_stdis used by default:To use the other driver variants, module
vl53l1x_st_apior modulevl53l1x_basichave to be specified at make command line
or
If the configuration parameter
VL53L1X_PARAM_PIN_INTis defined, e.g.,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:
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