Renesas HS300x Sensor Support #63016

iandmorris · 2023-09-23T23:52:23Z

Added driver and sample application for the Renesas HS3001 and HS3003 temperature/humidity sensors.

Tested using a Nucleo-F401RE, Click shield for Nucleo-64 and Temp&Hum Click 18 (HS3003) and Temp& Hum Click 17 (HS3001). The only difference between the HS3001 and HS3003 is the accuracy of the relative humidity measurements.

drivers/sensor/hs300x/hs300x.c

kartben · 2023-09-24T08:01:09Z

drivers/sensor/hs300x/hs300x.c

+		return -ENOTSUP;
+	}
+
+	rc = hs300x_start_measurement(dev);


Not sure it's worth a dedicated function?

samples/sensor/hs300x/README.rst

iandmorris · 2023-09-28T21:14:50Z

I messed up when rebasing this branch with the main branch (very new to the Zephyr Git workflow) and can't seem to recover. I'm going to close this pull request, go away and learn/practice the work flow and then create a new PR that also addresses the comments made. The time you spent reviewing was not a waste, I'll be sure to incorporate the comments.

kartben · 2023-09-28T21:20:07Z

I messed up when rebasing this branch with the main branch (very new to the Zephyr Git workflow) and can't seem to recover. I'm going to close this pull request, go away and learn/practice the work flow and then create a new PR that also addresses the comments made. The time you spent reviewing was not a waste, I'll be sure to incorporate the comments.

git rebase origin/main --interactive and then just delete that commit that's not yours?

iandmorris · 2023-09-29T02:43:18Z

I messed up when rebasing this branch with the main branch (very new to the Zephyr Git workflow) and can't seem to recover. I'm going to close this pull request, go away and learn/practice the work flow and then create a new PR that also addresses the comments made. The time you spent reviewing was not a waste, I'll be sure to incorporate the comments.

git rebase origin/main --interactive and then just delete that commit that's not yours?

Thanks for the suggestion, however I have compounded the problem while trying to fix it (new to using git rebase). Quicker if i just create a new branch and PR (once I have added the features requested in the comments).

kartben · 2023-09-29T02:49:01Z

I see. FWIW you should also be able to do a hard reset to any of your previous commits, which SHAs still show up on this PR or which you could also find via git reflog. Just thought it might be worthwhile for you to explore a bit more as there is almost always a solution that doesn't involve reopening a PR :)

iandmorris · 2023-09-30T21:08:29Z

I see. FWIW you should also be able to do a hard reset to any of your previous commits, which SHAs still show up on this PR or which you could also find via git reflog. Just thought it might be worthwhile for you to explore a bit more as there is almost always a solution that doesn't involve reopening a PR :)

Thanks for the tip about git reflog (and for your encouragement). I think I have resolved the issues I created...

MaureenHelm · 2023-10-20T21:09:12Z

drivers/sensor/hs300x/hs300x.c

+{
+	const struct hs300x_config *cfg = dev->config;
+
+	if (!device_is_ready(cfg->bus.bus)) {


Suggested change

if (!device_is_ready(cfg->bus.bus)) {

if (!i2c_is_ready_dt(cfg->bus)) {

MaureenHelm · 2023-10-20T21:10:05Z

drivers/sensor/hs300x/hs300x.h

+struct hs300x_config {
+	struct i2c_dt_spec bus;
+};
+
+struct hs300x_data {
+	int16_t t_sample;
+	uint16_t rh_sample;
+};


Not enough here to justify having a separate private include file. Please move into hs300x.c

MaureenHelm · 2023-10-20T21:10:51Z

tests/drivers/build_all/sensor/i2c.dtsi

+test_i2c_hs300x: hs300x@44 {
+	compatible = "renesas,hs300x";
+	reg = <0x44>;
+};


Address should be incremented by one from the last one. Note this is just a build test that will not execute on hardware so the address doesn't need to match the sensor datasheet

thanks, hadn't realized the address should be incremented.

MaureenHelm · 2023-10-20T21:13:49Z

samples/sensor/hs300x/README.rst

@@ -0,0 +1,65 @@
+.. _hs300x_sample:
+
+HS300X Sample


We want to stop introducing new driver-specific sensor samples, and instead use more generic ones for each sensor type. See samples/sensor/accel_polling and samples/sensor/magn_polling for example

makes sense. do you want me to create a generic sample for temperature and humidity sensors, based on one of these samples? something like samples/sensor/temp_hum_polling perhaps?

That would be great, thank you

Maybe simply repurpose/rename the dht sample?

ok, I will look at creating a generic temperature/humidity sample based on the existing dhtsample.

MaureenHelm

Needs a rebase

MaureenHelm · 2023-10-25T14:44:46Z

samples/sensor/hs300x/README.rst

@@ -0,0 +1,65 @@
+.. _hs300x_sample:
+
+HS300X Sample


That would be great, thank you

MaureenHelm · 2023-11-08T21:01:59Z

@kartben please revisit

kartben · 2023-11-08T21:42:50Z

samples/sensor/dht_polling/src/main.c

+				break;
+			}
+
+			printk("%16s: temp is %d.%d oC humidity is %d.%d %%RH\n", dev->name,


Suggested change

printk("%16s: temp is %d.%d oC humidity is %d.%d %%RH\n", dev->name,

printk("%16s: temp is %d.%d °C humidity is %d.%d %%RH\n", dev->name,

Also, maybe the format should be %d.%02d, having so many digits feels odd.

I changed oC to °C, however when I tried the %d.%02d modification I found it did not do what I thought it would... After some reading it seems that (unless I have missed something) the width format specifier defines the minimum number of characters to be printed but it does not truncate the value if it is larger. Therefore there was no change in the output (in most cases) and so I left formatting as it was...

Ah! of course... my bad. So also need to divide val2 by 10000, right?

zephyr/samples/shields/npm1300_ek/src/main.c

Line 96 in 191e47b

printk("T: %d.%02d\n", temp.val1, temp.val2 / 10000);

kartben · 2023-11-08T21:43:40Z

samples/sensor/dht_polling/README.rst

+       hs300x@44: temp is 25.31129 oC humidity is 30.391259 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.446194 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.372947 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.391259 %RH
+       hs300x@44: temp is 25.31129 oC humidity is 30.372947 %RH
+       hs300x@44: temp is 25.31129 oC humidity is 30.354635 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.372947 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.372947 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.391259 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.446194 %RH
+       hs300x@44: temp is 25.51272 oC humidity is 30.531648 %RH


update to use "°" and possibly the newer format as per my other comment.

done, but see comment above.

kartben · 2023-11-08T21:44:02Z

samples/sensor/dht_polling/sample.yaml

+    harness_config:
+      type: one_line
+      regex:
+        - "[0-9A-Za-z_,+-.]*@[0-9A-Fa-f]*: temp is (.*) oC humidity is (.*) %RH"


drivers/sensor/hs300x/hs300x.c

kartben

Sorry for the delay! The new generic sample is great, thanks for this!
A few remaining comments that are hopefully not too nitpicky :)

kartben

Thanks! My latest comment is not asking you for yet another update, to be clear :)

@MaureenHelm we should probably now just remove the existing /dht sample, right?

samples/sensor/dht_polling/src/main.c

MaureenHelm

we should probably now just remove the existing /dht sample, right?

Yes, but it would be nice to port boards/nrf52dk_nrf52832.overlay in the process

kartben · 2023-11-13T15:26:00Z

we should probably now just remove the existing /dht sample, right?

Yes, but it would be nice to port boards/nrf52dk_nrf52832.overlay in the process

Ya although tbh I am not a big fan of overlay files that imply manual wiring and breadboarding, especially if the README does not really provide guidance for beginners re: wiring for beginners (as this is virtually the only audience for which such overlay could really be useful IMO). OTH the overlay still kind of make sense in this particular example of the DHT22/11, I guess, as it showcases the use of the dht22 binding property.

In any case, the deletion of the legacy dht sample is for another PR to address which I'll likely look into at some point :)

carlescufi · 2023-11-14T08:21:31Z

@iandmorris please rebase to solve the conflict.

Adds support for Renesas HS3001 and HS3003 temperature/humidity sensors connected via an I2C bus. Signed-off-by: Ian Morris <ian.d.morris@outlook.com>

This simple application periodically prints the temperature and humidity measured by one or more digital humidity/temperature sensors. Signed-off-by: Ian Morris <ian.d.morris@outlook.com>

iandmorris requested review from nashif, MaureenHelm and galak as code owners September 23, 2023 23:52

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

zephyrbot requested review from avisconti, teburd, tristan-google and yperess September 23, 2023 23:52

zephyrbot assigned MaureenHelm Sep 23, 2023

iandmorris force-pushed the renesas_hs300x_sensor branch from 8b4b168 to 98be9d2 Compare September 24, 2023 01:07

kartben requested changes Sep 24, 2023

View reviewed changes

iandmorris force-pushed the renesas_hs300x_sensor branch 2 times, most recently from ebc8239 to 12a8b3b Compare September 27, 2023 02:40

iandmorris requested a review from nordic-krch as a code owner September 27, 2023 02:40

zephyrbot added the area: Logging label Sep 27, 2023

zephyrbot requested a review from dcpleung September 27, 2023 02:41

iandmorris force-pushed the renesas_hs300x_sensor branch from 12a8b3b to e9aa919 Compare September 30, 2023 20:26

MaureenHelm requested changes Oct 20, 2023

View reviewed changes

iandmorris force-pushed the renesas_hs300x_sensor branch from e9aa919 to 8e4f72a Compare October 24, 2023 18:08

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

iandmorris force-pushed the renesas_hs300x_sensor branch 2 times, most recently from 25f1fc3 to 5634408 Compare October 24, 2023 22:03

MaureenHelm reviewed Oct 25, 2023

View reviewed changes

samples/sensor/hs300x/README.rst Outdated

@@ -0,0 +1,65 @@

.. _hs300x_sample:

HS300X Sample

Copy link

Member

MaureenHelm Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great, thank you

iandmorris force-pushed the renesas_hs300x_sensor branch from 5634408 to 9029c32 Compare October 27, 2023 22:09

iandmorris force-pushed the renesas_hs300x_sensor branch from 9029c32 to 2b310a7 Compare October 27, 2023 23:40

iandmorris requested a review from MaureenHelm October 28, 2023 17:12

MaureenHelm previously approved these changes Oct 30, 2023

View reviewed changes

iandmorris requested a review from kartben October 31, 2023 02:45

kartben reviewed Nov 8, 2023

View reviewed changes

kartben requested changes Nov 8, 2023

View reviewed changes

iandmorris dismissed MaureenHelm’s stale review via 4e99762 November 10, 2023 00:19

iandmorris force-pushed the renesas_hs300x_sensor branch 2 times, most recently from 4e99762 to 8ee123d Compare November 10, 2023 00:34

iandmorris requested a review from kartben November 10, 2023 02:36

MaureenHelm previously approved these changes Nov 10, 2023

View reviewed changes

iandmorris dismissed MaureenHelm’s stale review via bae5576 November 13, 2023 00:40

iandmorris force-pushed the renesas_hs300x_sensor branch 2 times, most recently from bae5576 to 91cabaa Compare November 13, 2023 00:54

kartben previously approved these changes Nov 13, 2023

View reviewed changes

samples/sensor/dht_polling/src/main.c Show resolved Hide resolved

MaureenHelm previously approved these changes Nov 13, 2023

View reviewed changes

iandmorris added 2 commits November 14, 2023 08:49

drivers: sensor: hs300x: Add driver for Renesas HS300x sensors

1d1e6b4

Adds support for Renesas HS3001 and HS3003 temperature/humidity sensors connected via an I2C bus. Signed-off-by: Ian Morris <ian.d.morris@outlook.com>

samples: sensor: dht_polling: Add generic dht sample application

ce99f40

This simple application periodically prints the temperature and humidity measured by one or more digital humidity/temperature sensors. Signed-off-by: Ian Morris <ian.d.morris@outlook.com>

iandmorris dismissed stale reviews from MaureenHelm and kartben via ce99f40 November 14, 2023 16:50

iandmorris force-pushed the renesas_hs300x_sensor branch from 91cabaa to ce99f40 Compare November 14, 2023 16:50

kartben approved these changes Nov 14, 2023

View reviewed changes

iandmorris requested a review from MaureenHelm November 14, 2023 22:14

MaureenHelm approved these changes Nov 17, 2023

View reviewed changes

MaureenHelm merged commit dfc747d into zephyrproject-rtos:main Nov 17, 2023
19 checks passed

iandmorris deleted the renesas_hs300x_sensor branch November 17, 2023 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renesas HS300x Sensor Support #63016

Renesas HS300x Sensor Support #63016

iandmorris commented Sep 23, 2023

kartben Sep 24, 2023

iandmorris Sep 30, 2023

iandmorris commented Sep 28, 2023

kartben commented Sep 28, 2023

iandmorris commented Sep 29, 2023

kartben commented Sep 29, 2023

iandmorris commented Sep 30, 2023

MaureenHelm Oct 20, 2023

iandmorris Oct 25, 2023

MaureenHelm Oct 20, 2023

iandmorris Oct 25, 2023

MaureenHelm Oct 20, 2023

iandmorris Oct 25, 2023

MaureenHelm Oct 20, 2023

iandmorris Oct 25, 2023

MaureenHelm Oct 25, 2023

kartben Oct 25, 2023 •

edited

iandmorris Oct 25, 2023

MaureenHelm left a comment

MaureenHelm Oct 25, 2023

MaureenHelm commented Nov 8, 2023

kartben Nov 8, 2023

iandmorris Nov 10, 2023

kartben Nov 10, 2023

kartben Nov 8, 2023

iandmorris Nov 10, 2023

kartben Nov 8, 2023

iandmorris Nov 10, 2023

kartben left a comment

kartben left a comment

MaureenHelm left a comment

kartben commented Nov 13, 2023

carlescufi commented Nov 14, 2023

	if (!device_is_ready(cfg->bus.bus)) {
	if (!i2c_is_ready_dt(cfg->bus)) {

	printk("%16s: temp is %d.%d oC humidity is %d.%d %%RH\n", dev->name,
	printk("%16s: temp is %d.%d °C humidity is %d.%d %%RH\n", dev->name,

Renesas HS300x Sensor Support #63016

Renesas HS300x Sensor Support #63016

Conversation

iandmorris commented Sep 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iandmorris commented Sep 28, 2023

kartben commented Sep 28, 2023

iandmorris commented Sep 29, 2023

kartben commented Sep 29, 2023

iandmorris commented Sep 30, 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

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

kartben Oct 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaureenHelm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaureenHelm commented Nov 8, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartben left a comment

Choose a reason for hiding this comment

kartben left a comment

Choose a reason for hiding this comment

MaureenHelm left a comment

Choose a reason for hiding this comment

kartben commented Nov 13, 2023

carlescufi commented Nov 14, 2023

kartben Oct 25, 2023 •

edited