sensor: drivers: Panasonic sngcja5 particle matter sensor #55765

pideu-sj · 2023-03-14T13:36:19Z

Support for Panasonic's SN-GCJA5 Particle Matter Sensor.
Added sensor drivers and sensor sample for example usage.

drivers/sensor/sngcja5/sngcja5.c

drivers/sensor/sngcja5/Kconfig

drivers/sensor/sngcja5/sngcja5.c

dts/bindings/sensor/panasonic,sngcja5.yaml

drivers/sensor/sngcja5/sngcja5.c

MaureenHelm · 2023-03-22T13:44:11Z

drivers/sensor/sngcja5/sngcja5.c

+		return -EIO;
+	}
+
+	*value = (buf[3] << 24) | (buf[2] << 16) | (buf[1] << 8) | buf[0];


sys_be32_to_cpu

See comment below

MaureenHelm · 2023-03-22T13:44:45Z

drivers/sensor/sngcja5/sngcja5.c

+		return -EIO;
+	}
+
+	*value = (buf[1] << 8) | buf[0];


sys_be16_to_cpu

When using sys_be16_to_cpu like in following snippet:

static int read_register_2(const struct i2c_dt_spec *dev, uint8_t addr, uint16_t *value) { if (i2c_burst_read_dt(dev, addr, (uint8_t *)value, sizeof(*value)) < 0) { LOG_ERR("i2c_burst_read_dt() @ i2c failed"); return -EIO; } *value = sys_be16_to_cpu(*value); return 0; }

Like it's done for example in drivers/sensor/tmp112/tmp112.c, i would get the wrong byte order for my read data. Should be:
new sample: pc0_5.1: 0x0003
But is:
new sample: pc0_5.1: 0x0300
So i guess i have to stick to the previous solution.

These are actually little-endian, not big-endian, so using *_le*. versions of the conversion macros should work as expected

drivers/sensor/sngcja5/sngcja5.c

include/zephyr/drivers/sensor.h

samples/sensor/sngcja5/src/main.c

github-actions · 2023-05-22T00:30:51Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

pideu-sj · 2023-05-25T11:38:03Z

Hello @MaureenHelm
sorry was caught on other projects on last weeks, but will go on here in the next days. Could you please remove the stale label?

drivers/sensor/Kconfig

drivers/sensor/CMakeLists.txt

pideu-sj · 2023-10-25T06:28:33Z

Hi @teburd and @avisconti
had to rebase to match the compliance check and update manifest to pass all checks after removal of stale label.

pideu-sj · 2023-11-01T07:21:28Z

Hi @MaureenHelm, @avisconti,
could you maybe find some minuets to check if we can merge the PR?
Thanks a lot.

pideu-sj · 2023-11-06T09:05:18Z

Hi to all listed reviewers,
if @MaureenHelm and @avisconti are not available, could someone else maybe review?
Thanks a lot!

pideu-sj · 2023-11-13T05:34:58Z

Hello to the reviewers in the list @avisconti, @nashif, @galak, @yperess, @kartben, @tristan-google,
the Pull-Request already got two times the stale label. Can maybe someone else review if @MaureenHelm is not available?
Thanks a lot!

kartben · 2023-11-13T09:20:34Z

drivers/sensor/sngcja5/sngcja5.c

+		return -EIO;
+	}
+
+	*value = (buf[1] << 8) | buf[0];


These are actually little-endian, not big-endian, so using *_le*. versions of the conversion macros should work as expected

kartben · 2023-11-13T09:44:59Z

include/zephyr/drivers/sensor.h

+	/**
+	 * Particle count array
+	 * value1[(0.3-0.5µm)],
+	 * value2[(0.5-1.0µm)],
+	 * value3[(1.0-2.5µm)],
+	 * value4[(2.5-5.0µm)],
+	 * value5[(5.0-7.5µm)],
+	 * value6[(7.5-10.0µm)]
+	 */
+	SENSOR_CHAN_PARTICLE_COUNT,


At a minimum this should be a different commit, yes.

But I am -1 on adding this new "array" channel as both your linked datasheets actually show that different sensors will likely use different "classes" of particle counts in this array i.e PM 0.5/1.0/2.5/4.0/10.0 for SPS30, and PM 0.3/0.5/1.0/2.5/5.0/10.0, none of which correspond to the ranges proposed here. HM3300/3600 sensors have yet another range (https://files.seeedstudio.com/wiki/Grove-Laser_PM2.5_Sensor-HM3301/res/HM-3300%263600_V2.1.pdf) and so on.

What would probably work though is adding SENSOR_CHAN_PC_{X_Y} channels.

…nnel Particle matter sensors like the SNGCJA5 are able to detect particle counts. This sensor channel array needs to be added. Signed-off-by: Steffen Jahnke <[email protected]>

Added driver for Panasonic's "particulate matters (PM) sensor" laser type. PM sensor is able to detect smoke, environmental dust and other unwanted dangerous pollutant in the air that surrounds us. Signed-off-by: Steffen Jahnke <[email protected]>

Added sample for Panasonic's "particulate matters (PM) sensor" laser type. The sample detects values for PM1, PM2.5, PM10 and the particle counts periodically and prints them once a second to the console. Signed-off-by: Steffen Jahnke <[email protected]>

pideu-sj · 2023-11-14T06:18:34Z

@MaureenHelm and other core contributors to the sensor API,
could you confirm, that I should change sensor channels for particle count, like @kartben suggested here: https://github.com/zephyrproject-rtos/zephyr/pull/55765/files#r1391162757
Thanks a lot.

yperess · 2023-11-14T16:30:42Z

Ack, will review today

yperess · 2023-11-15T04:59:43Z

drivers/sensor/sngcja5/sngcja5.c

+	if (read_register_4(&device_config->i2c, SNGCJA5_PM10_LL, &(device_data->pm1_0))) {
+		return -EIO;
+	}
+	if (read_register_4(&device_config->i2c, SNGCJA5_PM25_LL, &(device_data->pm2_5))) {
+		return -EIO;
+	}
+	if (read_register_4(&device_config->i2c, SNGCJA5_PM100_LL, &(device_data->pm10_0))) {
+		return -EIO;
+	}
+	if (read_register_2(&device_config->i2c, SNGCJA5_05_L, &(device_data->pc0_5))) {
+		return -EIO;
+	}
+	if (read_register_2(&device_config->i2c, SNGCJA5_10_L, &(device_data->pc1_0))) {
+		return -EIO;
+	}
+	if (read_register_2(&device_config->i2c, SNGCJA5_25_L, &(device_data->pc2_5))) {
+		return -EIO;
+	}
+	if (read_register_2(&device_config->i2c, SNGCJA5_50_L, &(device_data->pc5_0))) {
+		return -EIO;
+	}
+	if (read_register_2(&device_config->i2c, SNGCJA5_75_L, &(device_data->pc7_5))) {
+		return -EIO;
+	}
+	if (read_register_2(&device_config->i2c, SNGCJA5_100_L, &(device_data->pc10_0))) {
+		return -EIO;
+	}


These all seem to be sequential registers, can we just do 1 burst i2c read and then convert to CPU endianess one by one?

yperess · 2023-11-15T05:05:31Z

include/zephyr/drivers/sensor.h

+	/**
+	 * Particle count array
+	 * value1[(0.3-0.5µm)],
+	 * value2[(0.5-1.0µm)],
+	 * value3[(1.0-2.5µm)],
+	 * value4[(2.5-5.0µm)],
+	 * value5[(5.0-7.5µm)],
+	 * value6[(7.5-10.0µm)]
+	 */
+	SENSOR_CHAN_PARTICLE_COUNT,


I'm not really seeing the need for this to be a part of sensor.h, why can't this highly specific 6 value channel be added in sngcja5.h? I would also like to add that with the new async API you're able to have an index along with a channel which would service this model much better. SENSOR_CHAN_PARTICLE_COUNT would be used along with an index to provide any number of dimensions to the measurement. We would still need a way to describe what each index means.

yperess · 2023-11-15T05:07:15Z

samples/sensor/particulate_matter/sample.yaml

I'm going to let @MaureenHelm decide the path on this, but I'm not seeing a strong benefit to adding yet another sensor specific sample to sensors. The sensor shell sample can do this just fine.

@yperess, thanks for reviewing.
Okay, so I will wait for decision of @MaureenHelm, before I change this in the driver.

github-actions · 2024-01-15T00:30:05Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

github-actions · 2024-03-16T00:29:49Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

pideu-sj requested review from nashif, MaureenHelm and galak as code owners March 14, 2023 13:36

zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Sensors Sensors labels Mar 14, 2023

zephyrbot requested review from avisconti, teburd and yperess March 14, 2023 13:36

zephyrbot assigned MaureenHelm Mar 14, 2023

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch 2 times, most recently from 457efe5 to 7d1f0e5 Compare March 14, 2023 14:43

pideu-sj changed the title ~~Sensor driver and sample for panasonic sngcja5~~ sensor: drivers: Panasonic sngcja5 particle matter sensor Mar 14, 2023

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch 2 times, most recently from 6a81df6 to 371b209 Compare March 15, 2023 08:09

pideu-sj commented Mar 15, 2023

View reviewed changes

drivers/sensor/sngcja5/sngcja5.c Show resolved Hide resolved

MaureenHelm requested changes Mar 17, 2023

View reviewed changes

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch 2 times, most recently from f48dfc2 to 63a5936 Compare March 21, 2023 10:53

MaureenHelm requested changes Mar 22, 2023

View reviewed changes

github-actions bot added the Stale label May 22, 2023

github-actions bot removed the Stale label May 26, 2023

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch 2 times, most recently from cd1d396 to cb41cc2 Compare June 1, 2023 12:22

MaureenHelm reviewed Jun 1, 2023

View reviewed changes

drivers/sensor/Kconfig Outdated Show resolved Hide resolved

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch from cb41cc2 to 30f227d Compare June 1, 2023 13:42

MaureenHelm reviewed Jun 2, 2023

View reviewed changes

drivers/sensor/CMakeLists.txt Outdated Show resolved Hide resolved

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch 2 times, most recently from 015b3d9 to bf62427 Compare June 5, 2023 06:22

teburd reopened this Oct 24, 2023

teburd removed the Stale label Oct 24, 2023

zephyrbot added the area: Samples Samples label Oct 24, 2023

zephyrbot requested review from kartben and tristan-google October 24, 2023 17:27

teburd previously approved these changes Oct 24, 2023

View reviewed changes

pideu-sj dismissed stale reviews from teburd and avisconti via 36359a4 October 25, 2023 05:16

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch from 4c0324a to 36359a4 Compare October 25, 2023 05:16

teburd previously approved these changes Oct 25, 2023

View reviewed changes

teburd requested a review from MaureenHelm October 25, 2023 12:24

kartben requested changes Nov 13, 2023

View reviewed changes

pideu-sj added 3 commits November 13, 2023 13:26

include: zephyr: drivers: sensor: Add particle count array sensor cha…

4cc12ed

…nnel Particle matter sensors like the SNGCJA5 are able to detect particle counts. This sensor channel array needs to be added. Signed-off-by: Steffen Jahnke <[email protected]>

pideu-sj dismissed teburd’s stale review via 31b0606 November 13, 2023 13:34

pideu-sj force-pushed the sensor_driver_panasonic_sngcja5 branch from 36359a4 to 31b0606 Compare November 13, 2023 13:34

yperess requested changes Nov 15, 2023

View reviewed changes

github-actions bot added the Stale label Jan 15, 2024

kartben removed the Stale label Jan 15, 2024

github-actions bot added the Stale label Mar 16, 2024

github-actions bot closed this Mar 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sensor: drivers: Panasonic sngcja5 particle matter sensor #55765

sensor: drivers: Panasonic sngcja5 particle matter sensor #55765

pideu-sj commented Mar 14, 2023

MaureenHelm Mar 22, 2023

pideu-sj Jun 1, 2023 •

edited

Loading

MaureenHelm Mar 22, 2023

pideu-sj Jun 1, 2023

kartben Nov 13, 2023

github-actions bot commented May 22, 2023

pideu-sj commented May 25, 2023

pideu-sj commented Oct 25, 2023

pideu-sj commented Nov 1, 2023

pideu-sj commented Nov 6, 2023

pideu-sj commented Nov 13, 2023

kartben Nov 13, 2023

kartben Nov 13, 2023

pideu-sj commented Nov 14, 2023

yperess commented Nov 14, 2023

yperess Nov 15, 2023

yperess Nov 15, 2023

yperess Nov 15, 2023

pideu-sj Nov 15, 2023

github-actions bot commented Jan 15, 2024

github-actions bot commented Mar 16, 2024

sensor: drivers: Panasonic sngcja5 particle matter sensor #55765

sensor: drivers: Panasonic sngcja5 particle matter sensor #55765

Conversation

pideu-sj commented Mar 14, 2023

Choose a reason for hiding this comment

pideu-sj Jun 1, 2023 • edited Loading

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

github-actions bot commented May 22, 2023

pideu-sj commented May 25, 2023

pideu-sj commented Oct 25, 2023

pideu-sj commented Nov 1, 2023

pideu-sj commented Nov 6, 2023

pideu-sj commented Nov 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pideu-sj commented Nov 14, 2023

yperess commented Nov 14, 2023

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

github-actions bot commented Jan 15, 2024

github-actions bot commented Mar 16, 2024

pideu-sj Jun 1, 2023 •

edited

Loading