-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
samples: sensor: update dht_polling to new API #72176
samples: sensor: update dht_polling to new API #72176
Conversation
d107d80
to
ab27525
Compare
ab27525
to
bbcf964
Compare
bbcf964
to
78d975c
Compare
78d975c
to
1c496f6
Compare
dev->name, temp.val1, temp.val2 / 10000, | ||
hum.val1, hum.val2 / 10000); | ||
uint32_t temp_fit = 0; | ||
uint8_t temp_buf[64]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems too large for a single sample? Same question about hum_buf[64]
below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@teburd @laurenmurphyx64 Is there a scenario where temp_buf
and hum_buf
won't be struct sensor_q31_data
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use sensor_q31_data struct instead of a buffer, take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Looks good, assuming rebasing it's done.
I've tried it on my end (on a single instance DHT sensor) and it works as expected:
*** Booting Zephyr OS build v3.6.0-4878-g1c496f6b97f3 ***
hts221@5f: temp is 24.87 °C humidity is 44.49 RH
hts221@5f: temp is 25.12 °C humidity is 43.99 RH
hts221@5f: temp is 25.12 °C humidity is 43.99 RH
hts221@5f: temp is 25.24 °C humidity is 43.99 RH
hts221@5f: temp is 25.24 °C humidity is 43.99 RH
hts221@5f: temp is 25.24 °C humidity is 43.99 RH
hts221@5f: temp is 25.37 °C humidity is 43.99 RH
hts221@5f: temp is 25.37 °C humidity is 43.49 RH
hts221@5f: temp is 25.37 °C humidity is 43.49 RH
hts221@5f: temp is 25.49 °C humidity is 43.49 RH
hts221@5f: temp is 25.49 °C humidity is 43.49 RH
hts221@5f: temp is 25.49 °C humidity is 43.49 RH
hts221@5f: temp is 25.49 °C humidity is 43.49 RH
hts221@5f: temp is 25.62 °C humidity is 43.49 RH
hts221@5f: temp is 25.62 °C humidity is 43.49 RH
hts221@5f: temp is 25.74 °C humidity is 43.49 RH
@laurenmurphyx64 Please rebase on main when you can, the sample changes look good. The buffer size concerns @MaureenHelm has should be addressed with when both #71235 and #73031 are addressed. The buffer is large because bme280's compensation parameters are included in the buffer today and that's what @laurenmurphyx64 was testing with. We need to resolve #73031 in order to fix this part. The buffer is large as well because we don't have a way of deciding, at compile time, how big the buffer should be without the metadata #71235 would presumably provide which @yperess is working on. |
1c496f6
to
7b095c9
Compare
Rebased, will standby. |
Updates dht_polling sample to new sensor API. Signed-off-by: Lauren Murphy <[email protected]>
7b095c9
to
bd19142
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remaining task of updating the read buffer size can be done later IMO once we've sorted out how to obtain the metadata needed, which we are working on in #71235
Updates dht_polling sample to new sensor API.
Depends on #71093 (merged) and #71160 (still waiting) (commits are included in this PR, so this is DNM until both PRs are merged and I can drop the commits)