drivers: sensor: STCC4 Add Driver #98434

jfaeh · 2025-10-28T15:59:05Z

This adds support for Sensirion's STCC4 co2 sensor.

In addition to the driver, we would like to add a driver core that handles basic functions relevant to Sensirion sensors. With the help of this core, we will be able to add and unify many new drivers quickly and easily in the near future.

jfaeh · 2025-11-03T08:15:37Z

@MaureenHelm Have you had a chance to take a look at this pull request?

jfaeh · 2025-11-18T14:15:22Z

Can someone please have a look?
@MaureenHelm @jeppenodgaard @avisconti @nashif @ubieda @teburd

jeppenodgaard · 2025-11-18T15:02:42Z

Hi @jfaeh .
Do you know why the CI fails?
I prefer to review when the CI succeeds.

jfaeh · 2025-11-18T16:32:19Z

@jeppenodgaard Tests are now successfull

jeppenodgaard

Thanks you for this PR!

I did not do a full review as it seems like usage of any zephyr helper functions absent.

I think a lot of code can be removed and some be more concise by becoming familiar and using zephyr functions.

drivers/sensor/sensirion/sensirion_core/crc_tables.c

jeppenodgaard · 2025-11-18T19:25:44Z

drivers/sensor/sensirion/sensirion_core/sensirion_common.h

+
+#include <stdint.h>
+
+#define NO_ERROR              0


Why not just use standard errnos?

Isn't 0 the errno for no error?

A define is not required. Please use errno directly without wrapping them in another define.

drivers/sensor/sensirion/sensirion_core/CMakeLists.txt

drivers/sensor/sensirion/sensirion_core/sensirion_common.c

drivers/sensor/sensirion/sensirion_core/sensirion_i2c.c

drivers/sensor/sensirion/sensirion_core/sensirion_i2c.h

jeppenodgaard · 2025-11-18T19:47:52Z

Related:
#97129

jfaeh · 2025-11-20T10:03:06Z

Hi @jeppenodgaard
Thank you for your review. I have updated the code based on your suggested changes.

This adds support for Sensirion's STCC4 co2 sensor. Signed-off-by: Jan Faeh <[email protected]>

sonarqubecloud · 2025-11-20T10:53:51Z

Quality Gate passed

Issues
9 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

jfaeh · 2025-11-27T12:06:25Z

Hi @jeppenodgaard I wanted to ask if you have already had a chance to look at my changes.

jeppenodgaard

After reading the issue I think I understand what is desired:
A driver that is OS agnostic. I do not think sensiron_core does or can do that.

I was not part of #71374 but I think it could be used as a guideline. See #71374 (review).

Again I did not do a full review - I think we need another input, maybe from @teburd ?

jeppenodgaard · 2025-11-27T14:50:06Z

drivers/sensor/sensirion/stcc4/stcc4.c

+		return local_error;
+	}
+	*product_id = sensirion_common_bytes_to_uint32_t(&buffer_ptr[0]);
+	sensirion_common_to_integer(&buffer_ptr[4], (uint8_t *)serial_number, LONG_INTEGER, 8);


I would prefer memcpy, since it is faster, should use less memory and is a non-custom function.

Suggested change

sensirion_common_to_integer(&buffer_ptr[4], (uint8_t *)serial_number, LONG_INTEGER, 8);

memcpy(serial_number, &buffer_ptr[4], 8);

drivers/sensor/sensirion/sensirion_core/sensirion_common.c

jeppenodgaard · 2025-11-27T14:52:02Z

drivers/sensor/sensirion/sensirion_core/sensirion_common.h

+
+#include <stdint.h>
+
+#define NO_ERROR              0


A define is not required. Please use errno directly without wrapping them in another define.

jeppenodgaard · 2025-11-27T14:52:46Z

drivers/sensor/sensirion/sensirion_core/sensirion_i2c.h

+
+#define SENSIRION_COMMAND_SIZE     2
+#define SENSIRION_WORD_SIZE        2
+#define SENSIRION_NUM_WORDS(x)     (sizeof(x) / SENSIRION_WORD_SIZE)


jeppenodgaard · 2025-11-27T14:57:54Z

drivers/sensor/sensirion/sensirion_core/sensirion_i2c.h

+ * @param count:      Number of bytes in the data buffer (excluding checksum).
+ * @param checksum:   The expected CRC checksum byte received from the sensor.
+ *
+ * @return  0 if the checksum is valid, a negative error code otherwise.


Consider using boolean instead:
https://www.kernel.org/doc/html/latest/process/coding-style.html#function-return-values-and-names

If it can return multiple types of errors it should return type int and return an errno.

drivers/sensor/sensirion/sensirion_core/sensirion_common.h

jeppenodgaard · 2025-11-27T15:02:39Z

drivers/sensor/sensirion/sensirion_core/sensirion_common.h

+/**
+ * Enum to describe the type of an integer
+ */
+typedef enum {


Remove this:

An integer is not always 4 bytes

Names might clash with other defines since they are very generic.

Seems like only LONG_INTEGER is used.

jfaeh · 2025-12-01T09:57:00Z

Hello @jeppenodgaard

Thank you again for your review, and sorry that I didn’t explain our goal more clearly.

Our intention is to make the standard drivers of Sensirion available on Zephyr. These standard drivers are automatically generated by a code generator. We generate these drivers for various other platforms, like, e.g., Raspberry Pi. The drivers are published on GitHub.
To make the drivers independent from the underlying platform, we have introduced a thin adapter layer.
The adapter layer consists of the files sensirion_common.* sensirion_i2c.* and sensirion_hal.* . (For the core that we would like to publish in Zephyr, we have combined sensirion_i2c.* and sensirion_hal.* into sensirion_i2c.* .)
An example of our stcc4 driver for an other platform: https://github.com/Sensirion/raspberry-pi-i2c-stcc4

Since this generated code depends on the interface of the adapter layer, we cannot change this interface. This would break the generated driver. Furthermore, we also cannot narrow the interface, as further drivers with other features will not work.

What you see in the PR is one driver instance that may not access all functions and features of the adapter layer.

While we can easily change the implementation of the adapter layer, we cannot easily change the interface of the adapter layer. That's also why we initially proposed to provide the adapter layer as a module.
If it is not an option to include this adapter layer as a module, we should at least be able to keep the interface unchanged. But we are happy to receive inputs on how to improve and simplify the implementation of the adapter layer.

jeppenodgaard · 2025-12-08T06:45:44Z

Thank you for the thorough explanation.

The goal makes a lot of sense and it must be possible to achieve somehow.
I think the solution requires additional discussion.
It might not make sense modifying this PR without a clear course, as it might be a waste of time.

Some thoughts:

Ideally the OS agnostic drivers resides in another repo and is referenced by a SHA. Copy/pasting from one common source to multiple OS repos does not seem scalable.
I guess there's no way around a core/common library when C std lib does not have stdbytes.h. Zephyr utils, C std lib function or maybe POSIX functions should be used whenever possible to prevent reimplementing a function that already exists. I do not have a complete overview of what is available.
It looks like sensiron_i2c.c also needs to be updated with utils/stdlib functions (if this approach is viable).
I think generated code should always have a "generated" notice in the top.

Please await another review before continuing - and sorry for the delays.

jfaeh marked this pull request as ready for review October 28, 2025 16:00

zephyrbot added area: Sensors Sensors area: Boards/SoCs labels Oct 28, 2025

jfaeh closed this Oct 28, 2025

zephyrbot added area: Devicetree Bindings area: Tests Issues related to a particular existing or missing test labels Oct 28, 2025

zephyrbot requested review from MaureenHelm, asemjonovs, avisconti, jeppenodgaard, nashif, teburd, tristan-google, ubieda and yperess October 28, 2025 16:02

zephyrbot assigned MaureenHelm Oct 28, 2025

jfaeh reopened this Oct 28, 2025

jfaeh force-pushed the main branch 10 times, most recently from 2c0be47 to 729e16d Compare October 29, 2025 17:29

jfaeh force-pushed the main branch 2 times, most recently from 2a70286 to f40a06c Compare November 14, 2025 11:04

jfaeh force-pushed the main branch 2 times, most recently from 216099a to a7b6ea6 Compare November 14, 2025 11:17

jfaeh force-pushed the main branch 2 times, most recently from 8a0ed58 to 452ba30 Compare November 18, 2025 15:53

jeppenodgaard requested changes Nov 18, 2025

View reviewed changes

jfaeh force-pushed the main branch from 452ba30 to d49bbe0 Compare November 20, 2025 09:57

drivers: sensor: STCC4 Add Driver

535aa15

This adds support for Sensirion's STCC4 co2 sensor. Signed-off-by: Jan Faeh <[email protected]>

jfaeh force-pushed the main branch from d49bbe0 to 535aa15 Compare November 20, 2025 10:26

jfaeh requested a review from jeppenodgaard November 25, 2025 08:36

jeppenodgaard requested changes Nov 27, 2025

View reviewed changes

	sensirion_common_to_integer(&buffer_ptr[4], (uint8_t *)serial_number, LONG_INTEGER, 8);
	memcpy(serial_number, &buffer_ptr[4], 8);


		#include <stdint.h>

		#define NO_ERROR 0


		#include <stdint.h>

		#define NO_ERROR 0

drivers: sensor: STCC4 Add Driver #98434

Are you sure you want to change the base?

drivers: sensor: STCC4 Add Driver #98434

Conversation

jfaeh commented Oct 28, 2025

Uh oh!

jfaeh commented Nov 3, 2025

Uh oh!

jfaeh commented Nov 18, 2025

Uh oh!

jeppenodgaard commented Nov 18, 2025

Uh oh!

jfaeh commented Nov 18, 2025

Uh oh!

jeppenodgaard left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeppenodgaard commented Nov 18, 2025

Uh oh!

jfaeh commented Nov 20, 2025

Uh oh!

sonarqubecloud bot commented Nov 20, 2025

Quality Gate passed

Uh oh!

jfaeh commented Nov 27, 2025

Uh oh!

jeppenodgaard left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfaeh commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeppenodgaard commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jfaeh commented Dec 1, 2025 •

edited

Loading

jeppenodgaard commented Dec 8, 2025 •

edited

Loading