drivers: sensor: add nooploop tofsens ToF sensor #88494

AlexFabre · 2025-04-11T11:14:20Z

Description

Nooploop TOFSense is a TOF-based (Time of Flight) laser ranging sensor that offers UART and CAN communication.
It's quite common in university projects and some pre-industrial prototypes.

Nooploop is specialized in the development of sensor systems and solutions for navigation and positioning.

Official documentation:

Product descriptions:

Implementation remark

I took inspiration from the various sensors that offer two bus communication (most of the time it's SPI & I2C). But because CAN is not declared as a bus in zephyr's dts binding, I had to do differently to handle the bus detection and property handling.
I'm not so proud of it, and it would have been much easier and cleaner if CAN still had it's bus: can property in can-controller.yaml and a can-device.yaml removed with Zephyr 3.0.0 (#42934 (comment)).

Also I provided a demo code for a nucleo board, but I'm not sure what is the common practice regarding samples. I let you tell me if the sample should be kept or should be dropped.

Copilot

Copilot reviewed 11 out of 21 changed files in this pull request and generated 1 comment.

Files not reviewed (10)

drivers/sensor/CMakeLists.txt: Language not supported
drivers/sensor/Kconfig: Language not supported
drivers/sensor/nooploop/CMakeLists.txt: Language not supported
drivers/sensor/nooploop/Kconfig: Language not supported
drivers/sensor/nooploop/tofsense/CMakeLists.txt: Language not supported
drivers/sensor/nooploop/tofsense/Kconfig: Language not supported
samples/sensor/tofsense/CMakeLists.txt: Language not supported
samples/sensor/tofsense/README.rst: Language not supported
samples/sensor/tofsense/boards/nucleo_u575zi_q.overlay: Language not supported
samples/sensor/tofsense/prj.conf: Language not supported

Comments suppressed due to low confidence (2)

drivers/sensor/nooploop/tofsense/tofsense.h:21

[nitpick] The type name 'tosense_id_t' is inconsistent with the sensor's naming ('tofsense'). Consider renaming it to 'tofsense_id_t' for clarity.

typedef uint8_t tosense_id_t;

drivers/sensor/nooploop/tofsense/tofsense_uart.h:54

[nitpick] Consider renaming 'tosense_id_t' to 'tofsense_id_t' to align with the sensor’s naming convention.

tosense_id_t id;     /* Sensor ID */

Copilot · 2025-04-11T11:35:45Z

samples/sensor/tofsense/src/main.c

+		}
+
+		ret = sensor_channel_get(dev, SENSOR_CHAN_DISTANCE, &value);
+		printk("distance is %.3lld mm\n", sensor_value_to_milli(&value));


The format specifier '%.3lld' is unconventional for an integer value. Consider using '%lld' for proper formatting.

Suggested change

printk("distance is %.3lld mm\n", sensor_value_to_milli(&value));

printk("distance is %lld mm\n", sensor_value_to_milli(&value));

Copilot

Copilot reviewed 11 out of 21 changed files in this pull request and generated no comments.

Files not reviewed (10)

drivers/sensor/CMakeLists.txt: Language not supported
drivers/sensor/Kconfig: Language not supported
drivers/sensor/nooploop/CMakeLists.txt: Language not supported
drivers/sensor/nooploop/Kconfig: Language not supported
drivers/sensor/nooploop/tofsense/CMakeLists.txt: Language not supported
drivers/sensor/nooploop/tofsense/Kconfig: Language not supported
samples/sensor/tofsense/CMakeLists.txt: Language not supported
samples/sensor/tofsense/README.rst: Language not supported
samples/sensor/tofsense/boards/nucleo_u575zi_q.overlay: Language not supported
samples/sensor/tofsense/prj.conf: Language not supported

Comments suppressed due to low confidence (2)

samples/sensor/tofsense/src/main.c:31

Verify that the return type of sensor_value_to_milli is a 64-bit integer. If it returns a different type, adjust the printf format specifier accordingly.

printk("distance is %.3lld mm\n", sensor_value_to_milli(&value));

drivers/sensor/nooploop/tofsense/tofsense_uart.h:44

[nitpick] Consider renaming 'dummy1' to 'reserved' to improve code clarity and better indicate that these bits are unused.

uint8_t dummy1: 3;

AlexFabre · 2025-04-12T07:59:07Z

Récent force-push to fix Copilote remakes.

include/zephyr/dt-bindings/sensor/tofsense.h

dts/bindings/sensor/nooploop,tofsense-common.yaml

rruuaanng · 2025-04-15T03:02:54Z

dts/bindings/sensor/nooploop,tofsense-common.yaml

+      - 0 # TOFSENSE_MODE_ACTIVE
+      - 1 # TOFSENSE_MODE_QUERY
+
+    enum: [0, 1]


Suggested change

enum: [0, 1]

enum:

- 0

- 1

drivers/sensor/nooploop/tofsense/tofsense.c

rruuaanng · 2025-05-05T15:10:44Z

drivers/sensor/nooploop/tofsense/tofsense.c

+
+LOG_MODULE_REGISTER(TOFSense, CONFIG_SENSOR_LOG_LEVEL);
+
+#ifdef CONFIG_TOFSENSE_BUS_UART


Suggested change

#ifdef CONFIG_TOFSENSE_BUS_UART

For large-scale conditional compilation, the code may appear less clean.

You can refer to the approach used in the display/ssd1306.c, where a union is employed to encapsulate the device object.

I did use union where it makes sense to have one structure to represent both bus, but I encapsulated into #ifdef functions that are specific to one. (ex UART flush/clear). For I2C and SPI there are probably more functions that can be unioned... Can you make a suggestion, maybe I don't see what you mean.

drivers/sensor/nooploop/tofsense/tofsense.h

drivers/sensor/nooploop/tofsense/tofsense_can.h

AlexFabre · 2025-05-05T15:39:56Z

Fixed the CI compliance check and twister tests

AlexFabre · 2025-05-06T08:34:46Z

@kartben I would like to get more reviews, and I' don't know who to ask.

This driver is the first of its kind with both UART and CAN support.
As CAN has lost its bus: can property in Zephyr 3, I had to trick to get the bus it's instanced on.

It's working great, and this driver is quite popular in China. The full support of Noopoop lineup could be added easily from then.

rruuaanng · 2025-05-09T02:17:24Z

@AlexFabre You can refer to the review suggestions I proposed, as they appear not to have been addressed.

AlexFabre · 2025-05-09T08:29:56Z

Hi @rruuaanng sorry I had completely missed your comments... Thanks !

MaureenHelm · 2025-05-09T15:04:13Z

drivers/sensor/nooploop/tofsense/Kconfig

+menuconfig TOFSENSE
+	bool "Nooploop TOFSense ToF sensor"
+	default y
+	depends on DT_HAS_NOOPLOOP_TOFSENSE_ENABLED
+	help
+	  Enable UART/CAN-based driver for the Nooploop TOFSense,
+	  time of flight sensor.
+
+if TOFSENSE
+
+config TOFSENSE_BUS_UART
+	bool "UART TOFSense driver"
+	default y
+	depends on $(dt_compat_on_bus,$(DT_COMPAT_NOOPLOOP_TOFSENSE),uart)
+	depends on UART_INTERRUPT_DRIVEN
+	select UART
+
+config TOFSENSE_BUS_CAN
+	bool "CAN TOFSense driver"
+	default y
+	depends on $(dt_compat_any_has_prop,$(DT_COMPAT_NOOPLOOP_TOFSENSE),can-bus)
+	depends on CAN


This isn't the only ToF sensor; need to namespace these Kconfig symbols to indicate it's the Nooploop sensor specifically.

It should also select the bus it needs, not depend on.

TOFSENSE is the product name, and NoopLoop the manufacturer.
Do you suggest to rename the menuconfig entry to NOOPLOOP_TOFSENSE ?
In other sensor's Config, it's usually only the product name.
I took inspiration form this VL53 TOF from ST

drivers/sensor/nooploop/tofsense/tofsense_can.h

MaureenHelm · 2025-05-09T15:15:15Z

dts/bindings/sensor/nooploop,tofsense-can.yaml

+    default: 1
+    required: true


Need to explain in the description why the default value was selected
https://docs.zephyrproject.org/latest/build/dts/bindings-upstream.html#dt-bindings-default-rules

The property also isn't required if there's a default specified.

I removed the default entry, to force the user to input the configured id value.

MaureenHelm · 2025-05-09T15:16:41Z

dts/bindings/sensor/nooploop,tofsense-common.yaml

+    default: 0
+    required: true


Need to explain in the description why the default value was selected (not what the value is)
https://docs.zephyrproject.org/latest/build/dts/bindings-upstream.html#dt-bindings-default-rules

The property also isn't required if there's a default specified.

I removed the default entry, to force the user to input the configured mode of operation value.

MaureenHelm · 2025-05-09T15:17:17Z

dts/bindings/sensor/nooploop,tofsense-common.yaml

+
+  active-mode-frequency:
+    type: int
+    default: 30


MaureenHelm · 2025-05-09T15:17:43Z

dts/bindings/sensor/nooploop,tofsense-uart.yaml

+    default: 0
+    required: true


Need to explain in the description why the default value was selected
https://docs.zephyrproject.org/latest/build/dts/bindings-upstream.html#dt-bindings-default-rules

The property also isn't required if there's a default specified.

I removed the default entry, to force the user to input the configured id value.

MaureenHelm · 2025-05-09T15:21:35Z

samples/sensor/tofsense/README.rst

+This sample uses the TOFSense sensor with its factory default settings, on the UART interface.
+
+References
+**********
+
+ - Datasheet: https://ftp.nooploop.com/downloads/tofsense/TOFSense_Datasheet_V3.0_en.pdf
+ - User manual: https://ftp.nooploop.com/downloads/tofsense/TOFSense_User_Manual_V3.0_en.pdf


Sensor samples should be agnostic to the specific driver implementation. It's ok to mention specific examples, but the README as currently written assumes the nooploop is the only type of ToF sensor.

I don't see where I suggest that the TOFSense is the only type of TOF sensor.
It's only one of the NoopLoop products lineup.
I took inspiration from this VL53 TOF sample.

MaureenHelm · 2025-05-09T15:22:57Z

samples/sensor/tofsense/prj.conf

+CONFIG_SENSOR=y
+CONFIG_UART_INTERRUPT_DRIVEN=y
+
+CONFIG_CAN=y


This can be removed if the driver selects CAN

Thanks, removed

MaureenHelm · 2025-05-09T15:24:02Z

samples/sensor/tofsense/sample.yaml

+      - uart
+    tags:
+      - sensors
+    filter: dt_compat_enabled("nooploop,tofsense")


Use a generic dt alias instead

MaureenHelm · 2025-05-09T15:24:31Z

samples/sensor/tofsense/src/main.c

+
+int main(void)
+{
+	const struct device *const dev = DEVICE_DT_GET_ONE(nooploop_tofsense);


Use a generic dt alias instead of the nooploop compatible.

henrikbrixandersen

Blocking until I have reviewed the CAN part.

This is not usually how the CAN bus is supposed to be used for integration in Zephyr.

henrikbrixandersen · 2025-05-09T21:26:26Z

I took inspiration from the various sensors that offer two bus communication (most of the time it's SPI & I2C). But because CAN is not declared as a bus in zephyr's dts binding, I had to do differently to handle the bus detection and property handling.
I'm not so proud of it, and it would have been much easier and cleaner if CAN still had it's bus: can property in can-controller.yaml and a can-device.yaml removed with Zephyr 3.0.0 (#42934 (comment)).

This is similar to how other networking peripherals (e.g. Ethernet) work in Zephyr.

Nooploop TOFSense is a packaged TOF (Time of Flight) laser ranging sensor with a UART or CAN interface. It supports two modes of communication: - ACTIVE: The sensor actively output its measured distance at a maximum frequency of 30Hz. - QUERY: The user has to make a request to read the measured distance. It offers 1cm to 8m detection range, with as low as 1mm resolution, ± 1.5cm error, and supports 15 ° ~ 27 ° adjustable FOV. Signed-off-by: Alex Fabre <alex.fabre@rtone.fr>

Provide a demo of the Nooploop TOFSense sensor on a nucleo_u575zi_q board. Signed-off-by: Alex Fabre <alex.fabre@rtone.fr>

AlexFabre · 2025-05-13T13:49:49Z

Recent push are related to @MaureenHelm comments.

sonarqubecloud · 2025-05-13T14:16:46Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

AlexFabre · 2025-06-02T11:01:55Z

@henrikbrixandersen do you still plan to make a review ? I'm waiting for it to open another PR for a CAN compatible device, relatively similar to this one on the CAN part.

github-actions bot added area: Devicetree area: Samples area: Sensors labels Apr 11, 2025

github-actions bot requested review from asemjonovs, avisconti, decsny, galak, jeppenodgaard, kartben, MaureenHelm, nashif, rruuaanng, teburd, tristan-google, ubieda and yperess April 11, 2025 11:15

github-actions bot assigned MaureenHelm Apr 11, 2025

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch from ad927aa to ff663fc Compare April 11, 2025 11:25

kartben requested a review from Copilot April 11, 2025 11:35

Copilot AI reviewed Apr 11, 2025

View reviewed changes

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch 2 times, most recently from d785929 to 4fc5204 Compare April 11, 2025 11:55

kartben requested a review from Copilot April 11, 2025 12:11

Copilot AI reviewed Apr 11, 2025

View reviewed changes

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch from 4fc5204 to b3ccc76 Compare April 12, 2025 07:52

rruuaanng reviewed Apr 13, 2025

View reviewed changes

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch 2 times, most recently from 3dc7198 to 3cc390a Compare April 14, 2025 08:27

decsny removed their request for review April 14, 2025 17:00

rruuaanng reviewed Apr 15, 2025

View reviewed changes

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch from 3cc390a to e0a6d66 Compare April 15, 2025 14:35

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch 3 times, most recently from bd5140f to 03aa60e Compare May 5, 2025 09:08

rruuaanng reviewed May 5, 2025

View reviewed changes

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch from 03aa60e to 8a1b100 Compare May 5, 2025 15:28

kartben requested review from alexanderwachter and henrikbrixandersen May 6, 2025 08:41

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch from 8a1b100 to 3bdacf2 Compare May 9, 2025 08:30

MaureenHelm requested changes May 9, 2025

View reviewed changes

henrikbrixandersen requested changes May 9, 2025

View reviewed changes

henrikbrixandersen added the area: CAN label May 9, 2025

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch from 3bdacf2 to 2882fe3 Compare May 12, 2025 15:21

AlexFabre added 2 commits May 13, 2025 15:25

samples: sensor: nooploop: add TOFSense uart demo

5f01dc9

Provide a demo of the Nooploop TOFSense sensor on a nucleo_u575zi_q board. Signed-off-by: Alex Fabre <alex.fabre@rtone.fr>

AlexFabre force-pushed the drivers-sensor-nooploop-add-tofsens-sensor branch from 2882fe3 to 5f01dc9 Compare May 13, 2025 13:48

	printk("distance is %.3lld mm\n", sensor_value_to_milli(&value));
	printk("distance is %lld mm\n", sensor_value_to_milli(&value));


		LOG_MODULE_REGISTER(TOFSense, CONFIG_SENSOR_LOG_LEVEL);

		#ifdef CONFIG_TOFSENSE_BUS_UART

drivers: sensor: add nooploop tofsens ToF sensor #88494

Are you sure you want to change the base?

drivers: sensor: add nooploop tofsens ToF sensor #88494

Conversation

AlexFabre commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Implementation remark

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

AlexFabre commented Apr 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexFabre commented May 5, 2025

Uh oh!

AlexFabre commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rruuaanng commented May 9, 2025

Uh oh!

AlexFabre commented May 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henrikbrixandersen left a comment

Choose a reason for hiding this comment

AlexFabre commented Apr 11, 2025 •

edited

Loading

AlexFabre commented May 6, 2025 •

edited

Loading