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.

Sign up for GitHub

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

Jump to bottom

sensor: Add support for INA219 #35538

Merged
merged 4 commits into from
Aug 23, 2021
Merged

sensor: Add support for INA219 #35538

merged 4 commits into from
Aug 23, 2021

Conversation

leonardp
Copy link
Contributor

@leonardp leonardp commented May 21, 2021 •
edited
Loading

This adds support and an example application for the TI INA219 zero-drift, bidirectional current/power monitor.

There already is a "quick and dirty" application example using this device here.

Improvements over the old example application are:

  • usage of the sensor API
  • configuration/calibration data via devicetree
  • reverse current now working
  • power management
  • triggered mode to save on power

Signed-off-by: Leonard Pollak leonardp@tr-host.de

@github-actions github-actions bot added area: API Changes to public APIs area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Documentation area: Samples Samples labels May 21, 2021
@zephyrbot zephyrbot added the area: Sensors Sensors label May 21, 2021
mbolivar-nordic
mbolivar-nordic suggested changes May 21, 2021
View reviewed changes
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.

Commenting on the binding only.

@leonardp leonardp force-pushed the ina219 branch 3 times, most recently from 0890844 to 26ac811 Compare May 21, 2021 19:43
@leonardp leonardp requested a review from mbolivar-nordic May 21, 2021 19:46
@leonardp leonardp force-pushed the ina219 branch 3 times, most recently from 9c0b5a6 to 41b1309 Compare May 22, 2021 10:34
mbolivar-nordic
mbolivar-nordic approved these changes Jun 9, 2021
View reviewed changes
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. The binding looks good to me now. I didn't review the rest in detail.

mbolivar-nordic
mbolivar-nordic reviewed Jun 10, 2021
View reviewed changes
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

While working on something else, I discovered samples/drivers/current_sensing, which is a pretty hacky example of getting this exact sensor working on a particular breakout board with a hardcoded I2C device name.

I think it's no longer necessary with this patch -- @leonardp could you please add a commit to this pull request removing the stale sample?

@carlescufi carlescufi requested a review from gmarull June 11, 2021 14:21
gmarull
gmarull reviewed Jun 14, 2021
View reviewed changes
@leonardp leonardp force-pushed the ina219 branch 2 times, most recently from 21cfa4d to 56c1dbf Compare June 25, 2021 13:00
galak
galak previously requested changes Jun 25, 2021
View reviewed changes
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 tests/drivers/build_all/sensor/

@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Jun 25, 2021
gmarull
gmarull requested changes Aug 4, 2021
View reviewed changes
@leonardp
Copy link
Contributor Author

leonardp commented Aug 15, 2021 •
edited
Loading

@gmarull @galak
I have removed the private channels and replaced them with the public ones introduced by #35753.
The channel for the shunt voltage was therefore removed completely as I could not think of a use case where one would want
to use this device to read that value.

@leonardp leonardp requested a review from gmarull August 15, 2021 10:35
gmarull
gmarull requested changes Aug 17, 2021
View reviewed changes
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.

Some more changes requested, please check my comments. I still see that samples comes before the actual driver, can you re-order commits in proper chronological order?

@leonardp leonardp force-pushed the ina219 branch 2 times, most recently from 5a2e09b to e46f881 Compare August 17, 2021 13:51
@leonardp leonardp requested a review from gmarull August 17, 2021 14:35
MaureenHelm
MaureenHelm requested changes Aug 20, 2021
View reviewed changes
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.

Please rebase and resolve the merge conflict

samples/sensor/ina219/CMakeLists.txt Outdated
@@ -0,0 +1,8 @@
# SPDX-License-Identifier: Apache-2.0

cmake_minimum_required(VERSION 3.13.1)
Copy link
Member

Choose a reason for hiding this comment

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

This should now be 3.20.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebased and updated

@leonardp leonardp requested a review from MaureenHelm August 20, 2021 20:32
Leonard Pollak added 4 commits August 23, 2021 12:17
drivers: sensor: INA219: added support
3dff3f6 
This adds support for the TI INA219 Zero-Drift, Bidirectional
Current/Power Monitor with I2C Interface

Signed-off-by: Leonard Pollak <leonardp@tr-host.de>
tests: drivers: added INA219 to build tests
f9c9c98 
This adds the INA219 driver to the build tests.

Signed-off-by: Leonard Pollak <leonardp@tr-host.de>
samples: sensor: added sample app for INA219
e23ad9a 
This adds an example application for the TI INA219 Zero-Drift,
Bidirectional Current/Power Monitor with I2C Interface

Signed-off-by: Leonard Pollak <leonardp@tr-host.de>
samples: drivers: removed old sample code for INA219
a4a99ab 
This removes the now stale sample code for the INA219

Signed-off-by: Leonard Pollak <leonardp@tr-host.de>
@MaureenHelm MaureenHelm dismissed galak’s stale review August 23, 2021 15:31

Requested changes were made

@MaureenHelm MaureenHelm merged commit a98ba2f into zephyrproject-rtos:main Aug 23, 2021
@leonardp leonardp deleted the ina219 branch August 27, 2021 14:28
kartben added a commit to kartben/zephyr that referenced this pull request Sep 7, 2023
@kartben
samples: remove leftovers from current_sensing sample
57c7ec2 
Remove leftovers of a code sample that was (only partially) deleted
with PR zephyrproject-rtos#35538

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
@kartben kartben mentioned this pull request Sep 7, 2023
Merged
fabiobaltieri pushed a commit that referenced this pull request Sep 8, 2023
@kartben @fabiobaltieri
samples: remove leftovers from current_sensing sample
faa8bb5 
Remove leftovers of a code sample that was (only partially) deleted
with PR #35538

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbolivar-nordic mbolivar-nordic mbolivar-nordic approved these changes

@MaureenHelm MaureenHelm MaureenHelm approved these changes

@gmarull gmarull gmarull approved these changes

@carlescufi carlescufi Awaiting requested review from carlescufi

@nashif nashif Awaiting requested review from nashif

@avisconti avisconti Awaiting requested review from avisconti

@galak galak Awaiting requested review from galak

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

Assignees

@MaureenHelm MaureenHelm

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@leonardp @mbolivar-nordic @MaureenHelm @jfischer-no @galak @gmarull @zephyrbot