Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]: ST VL53L1X ToF driver #12575

Closed
wants to merge 1 commit into from
Closed

[WIP]: ST VL53L1X ToF driver #12575

wants to merge 1 commit into from

Conversation

dagar
Copy link
Member

@dagar dagar commented Jul 29, 2019

This PR introduces a new driver for the ST VL53L1X long distance ranging Time-of-Flight sensor. https://www.st.com/en/imaging-and-photonics-solutions/vl53l1x.html

What's a little unusual here is that I used ST's VL53L1X API (https://www.st.com/content/st_com/en/products/embedded-software/proximity-sensors-software/stsw-img007.html) rather than directly talking to the sensor. This wasn't my first choice, and I initially implemented it from scratch, but it was massive with a fairly complex initialization routine (that failed sporadically) and still only implemented a fraction of what ST provided. For reference here's the Arduino library provided by Pololu for this device - https://github.com/pololu/vl53l1x-arduino. There are over 1000 registers.

The ST library is BSD 3-clause.

@davids5
Copy link
Member

davids5 commented Jul 29, 2019

@dagar - it will be a while until I can look a this.

@TSC21
Copy link
Member

TSC21 commented Aug 16, 2019

I should get some units in the next 2 weeks, so maybe I can test it.

Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of unknown code in this driver.

I did a quick pass looking for {while|for} I stopped looking when I got to
-VL53L1_wait_for_range_completion
-- VL53L1_poll_for_range_completion
---VL53L1_WaitValueMaskEx

VL53L1_Error VL53L1_WaitValueMaskEx(
	VL53L1_Dev_t *pdev,
	uint32_t      timeout_ms,
	uint16_t      index,
	uint8_t       value,
	uint8_t       mask,
	uint32_t      poll_delay_ms)
{
	VL53L1_Error status  = VL53L1_ERROR_NONE;
	return status;
}

I think the only options are: testing it, and use code coverage and profiling to verify it is not going to hang the system or rely on bad code, like above.

@@ -0,0 +1,68 @@
#include <string.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Title block?

@dagar
Copy link
Member Author

dagar commented Aug 17, 2019

Standby, ST has a slightly less enormous "ultra lite" driver (ULD) for this thing I'm looking at.

Screen Shot 2019-08-17 at 10 20 04 AM

@dagar dagar changed the title ST VL53L1X ToF driver [WIP]: ST VL53L1X ToF driver Aug 17, 2019
@dagar dagar mentioned this pull request Aug 27, 2019
@TSC21
Copy link
Member

TSC21 commented Oct 15, 2019

Standby, ST has a slightly less enormous "ultra lite" driver (ULD) for this thing I'm looking at.

Screen Shot 2019-08-17 at 10 20 04 AM

@dagar is this still feasible before a release? Or should we aim to a further patch release?

@zbynekwinkler
Copy link

How can I help with this?

On a related note - any thoughts on connecting multiple VL53L1X sensors to Pixhawk 4? The I2C address of the sensor can be changed but AFAIK the change does not survive the reset. The suggested way is to either connect the reset pin of the sensor to mcu and start the sensors one by one or use some kind of I2C switch to separate them from each other. I am thinking about using PCF8574 to drive the reset pins. Would approach like this be welcomed as a PR?

@zbynekwinkler
Copy link

I am thinking about using PCF8574 to drive the reset pins. Would approach like this be welcomed as a PR?

I believe that is the solution used in bitcraze multiranger deck: bitcraze/crazyflie-firmware@bbf485a and also in https://github.com/ipmgroup/RangeFinder_9xVL53LXX_hardware

@davids5
Copy link
Member

davids5 commented Oct 21, 2019

@zwn

If adding an IC; Why not use something like http://www.ti.com/product/PCA9548A

These downstream channels can be used to resolve I2C slave address conflicts.
For example, if eight identical digital temperature sensors are needed in the 
application, one sensor can be connected at each channel: 0-7.

@stale
Copy link

stale bot commented Jan 19, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Jan 19, 2020
@copterspace
Copy link
Contributor

copterspace commented Feb 17, 2020

Today I have tried to build https://github.com/dagar/PX4-Firmware/tree/pr-vl53l1x and setup my test Pixracer + vl53l1x sensor with it.
Driver does not start up automatically, but it starts OK manually.
But although I see the driver reports it works OK:
изображение

I stil can not see any Distance sensor measures in Widgets-Analyse in QGC:
изображение

How can I obtain Distance Sensor uorb messages from this driver, to test it in flight?

@stale stale bot removed the stale label Feb 17, 2020
@dagar
Copy link
Member Author

dagar commented Feb 17, 2020

@copterspace I've been using this driver in private for various things, but it never merged in PX4/Firmware because the enormous ST driver seemed questionable. I can take another look at bringing it in using the smaller version of the ST driver before the next PX4/Firmware stable release.

@copterspace
Copy link
Contributor

@dagar yes, please consider include the driver for vl53l1x to the PX4/Firmware release.
I currently use https://github.com/okalachev/vl53l1x_ros with this sensor, it works OK, does not seem much bigger then any other i2c device driver.
vl53l1x rangefinder is also supported in latest Arducopter, it works OK.
But I'd like to use it with PX4 flight stack, connected directly to FC, not via onboard computer+mavros.
I can help with any hardware tests of vl53l1x + Pixracer

@dagar
Copy link
Member Author

dagar commented Mar 31, 2020

Accidentally overwrote the branch with a new PR cleaning up the pr-vl53l0x. I'll open a new PR for the vl53l1x.

@koby08
Copy link
Contributor

koby08 commented Apr 19, 2020

@dagar Have you got a chance to open such a PR? Worth to mention just for the sake of continuity in this thread.. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants