Skip to content

drivers: sensors: Add driver for AHT20 temperature humidity sensor. #41702

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

Closed
wants to merge 1 commit into from

Conversation

RafaelLeeImg
Copy link
Contributor

Support AHT20 Temperature and Humidity Sensor with I2C bus

Webpage for the sensor is here:
https://asairsensors.com/product/aht20-integrated-temperature-and-humidity-sensor/

Data sheet v1.0.03 is here:
https://asairsensors.com/wp-content/uploads/2021/09/Data-Sheet-AHT20-Humidity-and-Temperature-Sensor-ASAIR-V1.0.03.pdf

Tested with physical sensor.
arm-none-eabi-gcc (15:10.3-2021.07-4) 10.3.1 20210621 (release)

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Did an initial review, there are quite a few issues that need to be addressed.

Copy link
Collaborator

@teburd teburd left a comment

Choose a reason for hiding this comment

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

Primarily the k_busy_wait concerns me

Copy link
Collaborator

@teburd teburd left a comment

Choose a reason for hiding this comment

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

This looks pretty good, the comments need to use /* */ rather than // though, and I believe checkpatch will fail on those.

If you plan on contributing a lot to zephyr I recommend setting up checkpatch as a git hook.
See https://docs.zephyrproject.org/latest/contribute/index.html#coding-style

@RafaelLeeImg RafaelLeeImg force-pushed the dev branch 2 times, most recently from bec19f6 to 406c47d Compare February 17, 2022 17:00
@RafaelLeeImg
Copy link
Contributor Author

I used both clang-format and uncrustify to format my source code, but the space before the colon is introduced by uncrustify, if there is no proper tool to format code. I'm using version Uncrustify-0.72.0_f, is my version too old? Otherwise, it's better to fix the uncrustify config instead of modify the code format by hand.

uncrustify --replace --no-backup -l C -c $ZEPHYR_BASE/.uncrustify.cfg aht20.c

@github-actions
Copy link

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 github-actions bot added the Stale label Apr 26, 2022
@github-actions github-actions bot closed this May 11, 2022
@ssekar15 ssekar15 reopened this Oct 23, 2022
@ssekar15
Copy link
Collaborator

@RafaelLeeImg we are interested on this driver, do you have plan to continue with the PR !!

@RafaelLeeImg
Copy link
Contributor Author

@RafaelLeeImg we are interested on this driver, do you have plan to continue with the PR !!

Hi.
I would like to continue with the current PR. I've already formatted the code according to the coding style proposed by

uncrustify --replace --no-backup -l C -c $ZEPHYR_BASE/.uncrustify.cfg

But, at the stage, the CI still complains coding formats error.
I think make every tiny format modification and let CI tell me where I'm wrong is very inefficient.
Have the .uncrustify.cfg or CI script updated?

@RafaelLeeImg
Copy link
Contributor Author

CI failed due to fail to fetch packages.
#52298

@RafaelLeeImg
Copy link
Contributor Author

How to generate this symbol DT_HAS_ASAIR_AHT20_ENABLED ?
I didn't find any clue.

@ssekar15
Copy link
Collaborator

DT_HAS_ASAIR_AHT20_ENABLED

^^ drop on kconfig.
please check example of dts binding in dts/bindings/sensor

add vendor prefix in dts/bindings/vendor-prefixes.txt
add dts entry for this sensor on tests/drivers/build_all/sensor/i2c.dtsi

It seems humidity value is needs to be corrected according to datasheet it has to be multiplied by 100

   	-val->val2 = (drv_data->humidity) * 1000000LL / (1 << AHT20_FULL_RANGE_BITS);
   	+val->val2 = ((drv_data->humidity) * 1000000LL / (1 << AHT20_FULL_RANGE_BITS)) * 100;

@RafaelLeeImg
Copy link
Contributor Author

I have wrote a test demo, the data format is right. Datasheet wrote RH[%]=(S_rh/2^20)*100%, 100% is 1.0.

@RafaelLeeImg RafaelLeeImg requested a review from galak as a code owner November 23, 2022 06:13
@zephyrbot zephyrbot added the area: Devicetree Binding PR modifies or adds a Device Tree binding label Nov 23, 2022
@RafaelLeeImg
Copy link
Contributor Author

H.val1 = 0, H.val2 = 335938, T.val1 = 14, T.val2 = 633667;
H = 0.335938, T=14.633667;
H = 33.59%, T = 14.63;
H = 33.5938%, T = 14.63;

Add support of AHT20 Temperature and Humidity Sensor

Signed-off-by: Rafael Lee <[email protected]>
@RafaelLeeImg
Copy link
Contributor Author

Is there any further modifications required?

Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

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

The compliance check failure still needs to be addressed

Comment on lines +114 to +115
__ASSERT_NO_MSG(chan == SENSOR_CHAN_ALL || chan == SENSOR_CHAN_AMBIENT_TEMP ||
chan == SENSOR_CHAN_HUMIDITY);
Copy link
Member

Choose a reason for hiding this comment

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

Return an errno


#define AHT20_INIT(n) \
static struct aht20_data aht20_data_##n = { \
.bus = I2C_DT_SPEC_INST_GET(n), \
Copy link
Member

Choose a reason for hiding this comment

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

Move this to a const config struct

@ssekar15
Copy link
Collaborator

ssekar15 commented Jan 6, 2023

@RafaelLeeImg ping

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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 github-actions bot added the Stale label Mar 8, 2023
@github-actions github-actions bot closed this Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System area: Devicetree Binding PR modifies or adds a Device Tree binding area: Sensors Sensors Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants