Skip to content
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

drivers: sensor: Add driver for TI INA23x #35753

Merged
merged 2 commits into from
Jul 27, 2021

Conversation

sambhurst
Copy link
Contributor

This driver supports the TI INA230 and INA231 Bidirectional Current
and Power Monitors. The devices work on the I2C interface and are
created from DT nodes with a compatible property matching "ti,ina23x".

The following datasheets were referenced while developing the driver:
https://www.ti.com/product/INA230
https://www.ti.com/product/INA231

Testing was performed on the stm32g071b_disco board:
twister --west-flash --device-testing --device-serial /dev/ttyACM0
-p stm32g071b_disco -T samples/sensor/ina23x

Signed-off-by: Sam Hurst sbh1187@gmail.com

@martinjaeger
Copy link
Member

@carlescufi, @sambhurst I'm on vacation for about 2 weeks, but I'm happy to review this PR afterwards.

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

Please add to the tests/drivers/build_all/sensor

galak
galak previously requested changes Jun 11, 2021
Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

lets remove the sample as it doesn't do anything interesting

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.

Thanks for this addition, please check comments

@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Jun 18, 2021
@sambhurst sambhurst force-pushed the add_ina2xx branch 2 times, most recently from f05c700 to b21fa83 Compare June 18, 2021 22:45
@sambhurst sambhurst requested a review from gmarull June 19, 2021 00:41
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.

LGTM

@sambhurst
Copy link
Contributor Author

@galak when you have time, could you take another look at this pull request? Thanks

Copy link
Member

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

As the sample was removed, the commit message should be adjusted.

@sambhurst sambhurst force-pushed the add_ina2xx branch 4 times, most recently from 9d111f8 to c579784 Compare June 26, 2021 04:40
@sambhurst sambhurst requested a review from martinjaeger June 26, 2021 15:20
@sambhurst
Copy link
Contributor Author

@galak I've made the changes you've requested.

Please add to the tests/drivers/build_all/sensor
lets remove the sample as it doesn't do anything interesting

Are there any further changes you think I need to make? Thank you

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

This review is limited mainly to the DT portions.

@sambhurst
Copy link
Contributor Author

@galak
@mbolivar-nordic

I've made the changes you've requested. Are there any further changes you think I need to make? Thank you

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Thanks. My comments are addressed.

Moving the BIT macros from <sys/util.h> to <sys/util_macro.h>
allows the BIT macros to be used in device trees.

Testing: twister -T tests/drivers/build_all/sensor/

Signed-off-by: Sam Hurst <sbh1187@gmail.com>
This driver supports the TI INA230 and INA231 Bidirectional Current
and Power Monitors. The devices work on the I2C interface and are
created from DT nodes with a compatible property matching "ti,ina23x".

The following datasheets were referenced while developing the driver:
https://www.ti.com/product/INA230
https://www.ti.com/product/INA231

Twister passed:
twister -T tests/drivers/build_all/sensor/

Testing was performed on the stm32g071b_disco board with the following:
Load:    ~170 ohms
Voltage: 5V

Measured Values:
Voltage: 5.1 V
Current: 0.032 A
Power:   0.157 W

Signed-off-by: Sam Hurst <sbh1187@gmail.com>
@sambhurst
Copy link
Contributor Author

@galak
@nashif

Could you take another look at your earliest convenience? Thank you

@mbolivar-nordic mbolivar-nordic requested a review from cfriedt July 27, 2021 21:30
@cfriedt cfriedt self-requested a review July 27, 2021 21:58
@cfriedt cfriedt merged commit 1382d64 into zephyrproject-rtos:main Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Documentation area: Samples Samples area: Sensors Sensors area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants