drivers: sensors: add ST VL53L1X TOF sensor #18772

chris-schra · 2019-08-29T11:05:58Z

Adds Zephyr platform driver for VL51L1X TOF sensor,
based on ST's driver API (see UM2356)

chris-schra · 2019-08-29T11:07:41Z

This is a driver for ST VL53L1X. I mostly took the code from vl53l0x driver but rewrote the platforms api to support 16bit register of vl53l1x (instead of 8bit of vl53l0x).
My most important question is: how do I create a pull request against the /hal/st module? how do I reference it here?
And what is the current best practice for allowing multiple device instances?

zephyrbot · 2019-08-29T11:09:58Z

Found the following issues, please fix and resubmit:

License issues

In most cases you do not need to do anything here, especially if the files
reported below are going into ext/ and if license was approved for inclusion
into ext/ already. Fix any missing license/copyright issues. The license
exception if a JFYI for the maintainers and can be overriden when merging the
pull request.

drivers/sensor/vl53l1x/vl53l1_platform.c is not apache-2.0 licensed: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform.c has non-permissive license: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform.c is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.c has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.c is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform.c is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.c has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.c is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform.c is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform.c is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform.h is not apache-2.0 licensed: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform.h has non-permissive license: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_log.h is not apache-2.0 licensed: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_log.h has non-permissive license: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_log.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_log.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_log.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_log.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_log.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_log.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_log.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_log.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h is not apache-2.0 licensed: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h has non-permissive license: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_config.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h is not apache-2.0 licensed: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h has non-permissive license: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_data.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h is not apache-2.0 licensed: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h has non-permissive license: free-unknown
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h is not apache-2.0 licensed: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h has non-permissive license: proprietary-license
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h is not apache-2.0 licensed: bsd-new
drivers/sensor/vl53l1x/vl53l1_platform_user_defines.h is not apache-2.0 licensed: bsd-new

erwango

First batch of comments, thanks for this submission!
Please update west.yml manifest file to use the PR you submitted in hal_st (cf https://docs.zephyrproject.org/latest/contribute/index.html?highlight=modules#contributions-to-external-modules)
Also add your sign-off in commit message

drivers/sensor/CMakeLists.txt

drivers/sensor/vl53l1x/vl53l1.c

drivers/sensor/vl53l1x/vl53l1_platform.c

erwango · 2019-08-30T13:14:54Z

drivers/sensor/vl53l1x/vl53l1_platform.h

erwango · 2019-08-30T13:15:19Z

drivers/sensor/vl53l1x/vl53l1_platform_log.h

erwango · 2019-08-30T13:15:45Z

drivers/sensor/vl53l1x/vl53l1_platform_user_config.h

erwango · 2019-08-30T13:16:59Z

drivers/sensor/vl53l1x/vl53l1_platform_user_data.h

Not sure all these files are actually needed here

they're included in the platform-folder of STSW-IMG007 and basically I just copied everything from there to the driver folder - I think it makes sense.
I just wonder why the driver got so much more files compared to VL53L0X.
Also, atm
VL53L1_WriteMulti
is not working (as far as I can tell the other I2C funcs work).

I'll push the updated files but they will still have most of the style errors.

chris-schra · 2019-08-30T13:23:26Z

well, that's why I added [DNM] and [RFC] - I know that there are code changes to do, I just knew no other place to start - code via mailing list is.. urgh

chris-schra · 2019-08-30T13:51:20Z

what about the licensing issues in the platform files? ST gives a choice of BSD or ST proprietary but not Apache-2.0

chris-schra · 2019-08-30T15:02:59Z

What is this issue about?

1b04962be0c88d2c2705e2c78e2ce4dd2ee77c31: author email (Chris Schramm [email protected]) needs to match one of the signed-off-by entries.

I tried different combinations of the signed-off message but none does work

And what‘s about this execute permission?

erwango · 2019-09-02T08:26:30Z

@chris-makaio, for sign-off, you'll find info here: https://docs.zephyrproject.org/latest/contribute/index.html
About Licence update, I'll check internally in ST and will get back to you?

And what‘s about this execute permission?

I'm not sure what you're referring to

erwango · 2019-09-03T08:00:21Z

@chris-makaio, for ST license update to Apache 2.0, we need first to check what files are requested for this. Can you confirm the names of the files that are actually requested to be in Zephyr repo? Specially, I'd like to understand the difference with files that were used for driver vl53l1x.

chris-schra · 2019-09-03T08:29:06Z

@erwango I'm confused. Aren't exactly those files listed under
#18772 (comment)
above?
The files are the boilerplate platform-specific files required to "bridge" to the ST driver in PR
zephyrproject-rtos/hal_st#2
I adopted these files to the Zephyr kernel and implemented the required functions (sleep -> k_sleep, i2c related driver functions, etc)

Concerning sign off: I think it was an old issue which already disappeared

erwango · 2019-09-03T08:46:08Z

@chris-makaio, there is already a driver for a similar sensor (vl53l0x) in zephyr tree.
For this sensor the files pushed in zephyr tree, coming from ST lib are the following:

- vl53l0x_platform.c 
- vl53l0x_platform.h
- vl53l0x_platform_log.h
- vl53l0x_types.h

I haven't check in details difference between ST lib for vl53l0x and vl53l1x, but I'd expect them to be similar. Hence I'd like to understand why the set of files you're pushing in this PR is different.

Adds Zephyr platform driver for VL51L1X TOF sensor, based on ST's driver API (see UM2356) Signed-off-by: Chris Schramm <[email protected]>

chris-schra · 2019-09-03T08:50:06Z

@erwango because it's a different and newer driver for a different device. The files are from ST's STSW-IMG007 and are just a boilerplate which one has to add the platform-specific implementation to. I did not want to mess with the files so I deliberately kept all files (also, for easier maintenance) and just added implementations where needed

chris-schra · 2019-09-03T08:51:40Z

changed the title and removed DNM/RFC because the driver is basically complete now

avisconti · 2019-09-03T09:25:47Z

@erwango @chris-makaio
I remembered now that there was a pending PR for the same component see #15566.
That PR was still using the old ext/hal path for external module.
I think it is correct to involve also @overheat in the discussion!

chris-schra · 2019-09-03T19:38:09Z

So any changes left for me to do?

erwango · 2019-09-05T12:07:19Z

@chris-makaio, before going on with in deep review, we need to check if it is possible to convert the files you propose to upstream in zephyr main tree to Apache 2.0 licence. I need to get this information internally from with ST. I'll get back to you on that

erwango · 2019-10-11T07:50:23Z

@chris-makaio, quick message to update you on the situation: this is progressing, I hope to have an answer next week.

erwango · 2019-10-16T13:54:10Z

@chris-makaio, I inform you that ST allows the change of License to Apache 2.0 in the files you're importing for x-cube package. Though, please keep STMicro copyright.

erwango · 2019-11-08T08:22:01Z

@chris-makaio, do you consider to complete work on this PR?

galak · 2020-04-17T09:42:53Z

Closing in favor of #22747

chris-schra requested review from MaureenHelm, avisconti and nashif as code owners August 29, 2019 11:05

zephyrbot added area: Devicetree area: Sensors Sensors area: Tests Issues related to a particular existing or missing test labels Aug 29, 2019

erwango requested changes Aug 30, 2019

View reviewed changes

chris-schra force-pushed the add-vl53l1x-driver branch 3 times, most recently from 4372172 to 9078dcb Compare August 30, 2019 14:00

zephyrbot added the area: Modules label Aug 30, 2019

chris-schra force-pushed the add-vl53l1x-driver branch 2 times, most recently from ade598c to 9f5a4ab Compare August 30, 2019 15:01

chris-schra force-pushed the add-vl53l1x-driver branch from 9f5a4ab to f1c3832 Compare September 3, 2019 08:34

drivers: sensors: add ST VL53L1X TOF sensor

d5483b1

Adds Zephyr platform driver for VL51L1X TOF sensor, based on ST's driver API (see UM2356) Signed-off-by: Chris Schramm <[email protected]>

chris-schra force-pushed the add-vl53l1x-driver branch from f1c3832 to d5483b1 Compare September 3, 2019 08:46

chris-schra changed the title ~~[RFC] [DNM] drivers: sensors: add ST VL53L1X TOF sensor~~ drivers: sensors: add ST VL53L1X TOF sensor Sep 3, 2019

avisconti mentioned this pull request Sep 3, 2019

Add vl53l1x support #15566

Closed

avisconti requested a review from erwango September 3, 2019 09:44

galak added area: Devicetree Binding PR modifies or adds a Device Tree binding and removed area: Devicetree labels Jan 30, 2020

galak closed this Apr 17, 2020

drivers: sensors: add ST VL53L1X TOF sensor #18772

drivers: sensors: add ST VL53L1X TOF sensor #18772

Uh oh!

Conversation

chris-schra commented Aug 29, 2019

Uh oh!

chris-schra commented Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zephyrbot commented Aug 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License issues

Uh oh!

erwango left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erwango Aug 30, 2019

Choose a reason for hiding this comment

Uh oh!

erwango Aug 30, 2019

Choose a reason for hiding this comment

Uh oh!

erwango Aug 30, 2019

Choose a reason for hiding this comment

Uh oh!

erwango Aug 30, 2019

Choose a reason for hiding this comment

Uh oh!

chris-schra Aug 30, 2019

Choose a reason for hiding this comment

Uh oh!

chris-schra commented Aug 30, 2019

Uh oh!

chris-schra commented Aug 30, 2019

Uh oh!

chris-schra commented Aug 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erwango commented Sep 2, 2019

Uh oh!

erwango commented Sep 3, 2019

Uh oh!

chris-schra commented Sep 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erwango commented Sep 3, 2019

Uh oh!

chris-schra commented Sep 3, 2019

Uh oh!

chris-schra commented Sep 3, 2019

Uh oh!

avisconti commented Sep 3, 2019

Uh oh!

chris-schra commented Sep 3, 2019

Uh oh!

erwango commented Sep 5, 2019

Uh oh!

erwango commented Oct 11, 2019

Uh oh!

erwango commented Oct 16, 2019

Uh oh!

erwango commented Nov 8, 2019

Uh oh!

galak commented Apr 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chris-schra commented Aug 29, 2019 •

edited

Loading

zephyrbot commented Aug 29, 2019 •

edited

Loading

erwango left a comment •

edited

Loading

chris-schra commented Aug 30, 2019 •

edited

Loading

chris-schra commented Sep 3, 2019 •

edited

Loading