VL53L1X Region of Interest & Distance Mode Setting #16695

dirksavage88 · 2021-01-30T00:16:24Z

Describe problem solved by this pull request
Distance sensor driver for the VL53L1X has region of interest (ROI) that can be programmed, but the set of functions were not implemented in the VL53L1X ul driver integrated into PX4. The ROI settings allow better resolution when changing the ranging area within the field of view. Distance is decreased but resolution is increased allowing the sensor to pick up smaller objects at sub-1 meter ranges.

Describe your solution
Utilized the ST vl53l1x ultra lite driver manual to implement two functions that allow a programmable region of interest, also created a distance mode class data member that allows easier setting, this defaults to a short distance mode for better resolution. These two settings lay the ground work for using multiple simultaneous zone detection with multiple distance sensors (2d lidar)

Describe possible alternatives
Utilize code from a larger driver that allows Lidar sensing with multiple sensors. However, this has larger code footprint.

Test data / coverage
Tested on bench setting with real hardware ITL. Plot from flight review:

attached

dagar · 2021-01-30T00:38:09Z

Interesting, I was considering directly carrying the (big ugly) ST driver to make more of these features accessible.

dagar · 2021-01-30T00:40:23Z

src/modules/mavlink/mavlink_receiver.cpp

@@ -765,6 +765,17 @@ MavlinkReceiver::handle_message_distance_sensor(mavlink_message_t *msg)
 	// quality value. Also it comes normalised between 1 and 100 while the uORB
 	// signal quality is normalised between 0 and 100.
 	ds.signal_quality = dist_sensor.signal_quality == 0 ? -1 : 100 * (dist_sensor.signal_quality - 1) / 99;
+        qt = const_cast<float*>(ds.q);
+	//mavlink clients
+        mavlink_msg_distance_sensor_send(MAVLINK_COMM_0, ds.timestamp, ds.min_distance, ds.max_distance, ds.current_distance, ds.type, ds.device_id, ds.orientation, ds.variance, ds.h_fov, ds.v_fov, qt, ds.signal_quality);


Crossing mavlink channels (owned by separate mavlink instances) doesn't look quite right, could you explain what you're trying to accomplish here?

I'll be honest, I was troubleshooting mavlink messages issues. Turns out the sensor itself didn't like 5 volts and would refuse to work when connected to a LiPO, but would work perfectly fine in USB. Thought it was a telemetry vs usb issue. Can remove, unnecessary.

Please remove yes.

dagar · 2021-01-30T00:41:32Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

@@ -222,35 +232,55 @@ void VL53L1X::RunImpl()
 {
 	uint8_t dataReady = 0;

+	uint8_t roiCenter[] = {247, 215, 183, 167, 151}; 


Comment explain these magic numbers?

Is it desirable to configure a specific ROI for different usage?

logic to these numbers can be found in vl53l1x multiple detection zone pdf.

Please use defines to encode them.

dagar · 2021-01-30T00:41:52Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

-
-	if ((ret != PX4_OK) | (rangeStatus != PX4_OK)) {
+	
+	if ((ret != PX4_OK) | (rangeStatus == 13)) {


Is there a range status define you can use instead of the magic number (13)?

this is a range status that indicates ROI is 'out of bounds', also referenced in the ST multiple zone manaul

Please create const values for each of those, e.g. RANGE_STATUS_OUT_OF_BOUNDS for this one and then use it.

LorenzMeier · 2021-01-31T10:23:06Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

-	_px4_rangefinder.set_max_distance(2.f);
-	_px4_rangefinder.set_fov(math::radians(25.f));
+
+	if (distance_mode == 1) {


Please use defines instead of magic numbers.

LorenzMeier · 2021-01-31T10:23:19Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

@@ -166,16 +176,16 @@ VL53L1X::~VL53L1X()
 int VL53L1X::collect()
 {
 	uint8_t ret = 0;
-	uint8_t rangeStatus;
+	uint8_t rangeStatus = 0;


Needs a define here as well.

LorenzMeier · 2021-01-31T10:23:41Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

@@ -191,7 +201,7 @@ int VL53L1X::collect()
 	}

 	perf_end(_sample_perf);
-
+        


Please remove this whitespace change (run make format)

LorenzMeier · 2021-01-31T10:24:25Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.hpp

@@ -123,13 +123,16 @@ class VL53L1X : public device::I2C, public I2CSPIDriver<VL53L1X>
 	void stop();

 	virtual int init() override;
-
+          


Unnecessary whitespace change.

LorenzMeier · 2021-01-31T10:25:19Z

boards/nxp/fmuk66-v3/default.cmake

@@ -22,7 +22,7 @@ px4_add_board(
 		camera_trigger
 		differential_pressure # all available differential pressure drivers
 		distance_sensor # all available distance sensor drivers
-		distance_sensor/srf05 # Specific driver
+		distance_sensor/srf05 # Specific driver		


Unnecessary whitespace change

LorenzMeier · 2021-01-31T10:25:53Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

-{
-	// VL53L1X typical range 0-2 meters with 25 degree field of view
+{       
+	//Set distance mode (1 for ~2m ranging, 2 for ~4m ranging


Please use one space after // for every comment

LorenzMeier · 2021-01-31T10:26:52Z

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

 	VL53L1X_CheckForDataReady(&dataReady);

 	if (dataReady == 1) {
-		collect();
+                collect();


Whitespace / indentation change that should be removed (wrong indentation and it adds noise to the diff)

mrpollo · 2021-02-01T18:01:41Z

@dirksavage88 thanks for putting in the work to push this upstream, we would love to get this in soon, can you please check the CI errors and comments from the reviewers above? once you are ready for a new review please ping me

dirksavage88 · 2021-02-01T19:24:00Z

@dirksavage88 thanks for putting in the work to push this upstream, we would love to get this in soon, can you please check the CI errors and comments from the reviewers above? once you are ready for a new review please ping me

Hello @mrpollo ,

I have incorporated the changes in the latest commit, to include format changes. I have successfully built for my specific hardware target but not tested (SITL, HITL, etc).

mrpollo · 2021-02-01T20:06:57Z

Thanks @dirksavage88

Looks like the CI timeout is non-related to your changeset, I will ask for a final review from the team before merging.

boards/nxp/fmuk66-v3/default.cmake

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

dirksavage88 · 2021-02-07T19:25:53Z

@mrpollo Wasn't sure if the CI failures are related, but the SITL tests /build (tailsitter) and MAVROS mission tests / build rover, are those fails that can be remedied?

mrpollo · 2021-02-15T17:44:09Z

@dirksavage88 I don't think either of those failed tests has anything to do with your PR, I think we should proceed with a merge as soon as a reviewer with commit access has time to get this in.

Note: I restarted the continuous-integration/jenkins/pr-head action and is now passing 💚

I'm tracking the CI failures here in case anyone is interested in following up #16859 and #16745

igalloway · 2021-02-19T20:20:28Z

Thanks @dirksavage88
@LorenzMeier @dagar @mrpollo FYI - This is a HoverGames2 contestant
https://www.hackster.io/savagecabbage/smart-scan-delivery-drone-ed5c54

boards/nxp/fmuk66-v3/default.cmake

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

src/drivers/distance_sensor/vl53l1x/vl53l1x.hpp

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

src/drivers/distance_sensor/vl53l1x/vl53l1x.hpp

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp

…ns (on NuttX)

…on a bus This also simplifies the API a bit, since we anyway have to change the drivers to pass additional information (the bus device index). The orientation flag is merged with the rotation.

Some targets don't build it (e.g. ark can bootloader)

…anging

… interest center setting and size Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

…enge 2, Team SavageCabbage Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

…agic #'s with global constants Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

dirksavage88 changed the base branch from master to beta January 30, 2021 00:32

dirksavage88 changed the base branch from beta to master January 30, 2021 00:34

dagar reviewed Jan 30, 2021

View reviewed changes

dagar added Drivers 🔧 Sensors, Actuators, etc Admin: Enhancement (improvement) 💡 labels Jan 30, 2021

LorenzMeier reviewed Jan 31, 2021

View reviewed changes

dagar reviewed Feb 2, 2021

View reviewed changes

boards/nxp/fmuk66-v3/default.cmake Outdated Show resolved Hide resolved

dagar reviewed Feb 2, 2021

View reviewed changes

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp Outdated Show resolved Hide resolved

dagar reviewed Feb 23, 2021

View reviewed changes

boards/nxp/fmuk66-v3/default.cmake Outdated Show resolved Hide resolved

dagar reviewed Feb 23, 2021

View reviewed changes

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp Outdated Show resolved Hide resolved

dagar reviewed Feb 23, 2021

View reviewed changes

src/drivers/distance_sensor/vl53l1x/vl53l1x.hpp Outdated Show resolved Hide resolved

dagar reviewed Feb 23, 2021

View reviewed changes

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp Show resolved Hide resolved

dagar reviewed Feb 23, 2021

View reviewed changes

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp Outdated Show resolved Hide resolved

dagar reviewed Feb 23, 2021

View reviewed changes

src/drivers/distance_sensor/vl53l1x/vl53l1x.hpp Show resolved Hide resolved

dagar reviewed Feb 23, 2021

View reviewed changes

src/drivers/distance_sensor/vl53l1x/vl53l1x.cpp Outdated Show resolved Hide resolved

Igor-Misic and others added 25 commits August 2, 2021 18:41

navigator: reduce traffic alert messages

7f54f0d

mixers: exclude FX79.main.mix from px4_fmu-v2

1fcdc13

px4_platform_common: atomic support larger types with critical sectio…

dbbbc6a

…ns (on NuttX)

i2c_spi_buses: add support for multiple instances of the same device …

cdfdabc

…on a bus This also simplifies the API a bit, since we anyway have to change the drivers to pass additional information (the bus device index). The orientation flag is merged with the rotation.

drivers: use updated I2C SPI driver interface

7da76c2

github actions: fix airframe_metadata

71af34e

monocypher submodule point to px4 branch

44529cb

fix github deploy_all.yml: explicitly call events_json

a07a8f3

Some targets don't build it (e.g. ark can bootloader)

metadata.cmake: ensure events directory exists & add px4 enums

2613bd0

Initial prototype for vl53l1x uORB activate and publish range

92b9fef

Enabled orb topic sub for distance sensor, value read in mm with 1m r…

b05bc9c

…anging

Created mavlink message send method to receiver for distance_sensor msg

30c90ea

Changed return value statement on range status to allow range warnings

92074d2

Removed erroneous uorb methods, refined distance mode selection

f3e9a40

removed struct handled by rangefinder driver

07fa1bc

cleaned up merge conflicts w/ upstream remote master, added region of…

4419092

… interest center setting and size Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

lowered timing budget and inter measurement period, limited to 5 ROI's

e92977a

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Implemented Region of Interest prototype, submission hovergames Chall…

afe9996

…enge 2, Team SavageCabbage Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

removed unecessary whitespace in default.cmake, vl53l1x.*, replaced m…

c294fca

…agic #'s with global constants Signed-off-by: Andrew Brahim <dirksavage88@gmail.com>

format checks passed, ran 'make format'

208f3b0

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Fixed unindent in default.cmake, removed variable declaration redundancy

2391f18

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Apply suggestions from code review

f9fa397

Update boards/nxp/fmuk66-v3/default.cmake

18134f6

Adjusted ROI, added sizeof operator to get num of array elements

df7ef85

Signed-off-by: dirksavage88 <dirksavage88@gmail.com>

Updated submodules

8ab295e

dirksavage88 force-pushed the master branch from 9208d89 to 8ab295e Compare August 3, 2021 02:07

dirksavage88 deleted the branch PX4:master August 10, 2021 00:33

dirksavage88 closed this Aug 10, 2021

dirksavage88 deleted the master branch August 10, 2021 00:33

dirksavage88 mentioned this pull request Aug 14, 2021

Distance Sensor vl53l1x Enhancements (Added multiple Region of Interest settings) #18073

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VL53L1X Region of Interest & Distance Mode Setting #16695

VL53L1X Region of Interest & Distance Mode Setting #16695

dirksavage88 commented Jan 30, 2021

dagar commented Jan 30, 2021

dagar Jan 30, 2021

dirksavage88 Jan 30, 2021

LorenzMeier Jan 31, 2021

dagar Jan 30, 2021

dirksavage88 Jan 30, 2021

LorenzMeier Jan 31, 2021

dagar Jan 30, 2021

dirksavage88 Jan 30, 2021

LorenzMeier Jan 31, 2021

LorenzMeier Jan 31, 2021

LorenzMeier Jan 31, 2021

LorenzMeier Jan 31, 2021

LorenzMeier Jan 31, 2021

LorenzMeier Jan 31, 2021

LorenzMeier Jan 31, 2021

LorenzMeier Jan 31, 2021

mrpollo commented Feb 1, 2021

dirksavage88 commented Feb 1, 2021

mrpollo commented Feb 1, 2021

dirksavage88 commented Feb 7, 2021

mrpollo commented Feb 15, 2021 •

edited

Loading

igalloway commented Feb 19, 2021


		if ((ret != PX4_OK) \| (rangeStatus != PX4_OK)) {

		if ((ret != PX4_OK) \| (rangeStatus == 13)) {

		@@ -191,7 +201,7 @@ int VL53L1X::collect()
		}

		perf_end(_sample_perf);

		@@ -123,13 +123,16 @@ class VL53L1X : public device::I2C, public I2CSPIDriver<VL53L1X>
		void stop();

		virtual int init() override;

VL53L1X Region of Interest & Distance Mode Setting #16695

VL53L1X Region of Interest & Distance Mode Setting #16695

Conversation

dirksavage88 commented Jan 30, 2021

dagar commented Jan 30, 2021

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

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

mrpollo commented Feb 1, 2021

dirksavage88 commented Feb 1, 2021

mrpollo commented Feb 1, 2021

dirksavage88 commented Feb 7, 2021

mrpollo commented Feb 15, 2021 • edited Loading

igalloway commented Feb 19, 2021

mrpollo commented Feb 15, 2021 •

edited

Loading