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

drivers: sensor: stm32_temp #34673

Merged
merged 5 commits into from
May 8, 2021
Merged

drivers: sensor: stm32_temp #34673

merged 5 commits into from
May 8, 2021

Conversation

Delsian
Copy link
Contributor

@Delsian Delsian commented Apr 28, 2021

STM32 internal temperature sensor driver.
This sensor can be used to measure the temperature of the CPU
and its surroundings.
Related issue: #34538

Signed-off-by: Eug Krashtan eug.krashtan@gmail.com

@github-actions github-actions bot added area: ADC Analog-to-Digital Converter (ADC) area: API Changes to public APIs area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Documentation area: Samples Samples platform: STM32 ST Micro STM32 labels Apr 28, 2021
@Delsian Delsian force-pushed the temper branch 2 times, most recently from 1197068 to 79e41bf Compare April 28, 2021 16:07
@erwango erwango requested a review from ABOSTM April 28, 2021 16:52
@Delsian Delsian mentioned this pull request Apr 28, 2021
Closed
@Delsian
Copy link
Contributor Author

Delsian commented Apr 28, 2021

I'd like to make a change in sensor.h after merging #34679
Please review it first.

@Delsian Delsian requested a review from anangl as a code owner April 29, 2021 09:18
@zephyrbot zephyrbot added the area: Sensors Sensors label Apr 29, 2021
MaureenHelm
MaureenHelm requested changes Apr 29, 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.

Thanks for your contribution!

Please split the changes into separate commits:

  • drivers: adc: Define max acquisition time
  • drivers: sensor: Add helper function to convert double to struct sensor_value
  • drivers: sensor: Introduce stm32 die temperature driver
  • samples: sensor: Add stm32_temp sample application

Add samples/sensor/stm32_temp_sensor/sample.yaml to ensure the sample gets built in CI.

Add this sensor to tests/drivers/build_all/sensors_i_z.conf

@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Apr 29, 2021
@Delsian
Copy link
Contributor Author

Delsian commented Apr 29, 2021

@MaureenHelm Thank you. I did all the requested changes.

@erwango
Copy link
Member

erwango commented May 6, 2021

@MaureenHelm can you have a new look ?

@Delsian Delsian force-pushed the temper branch 2 times, most recently from 7332b35 to 4a28c66 Compare May 7, 2021 05:10
@Delsian
Copy link
Contributor Author

Delsian commented May 7, 2021

@FRASTM I have question about STM32G4 HAL. In this HAL there is no definition for ADC_CHANNEL_TEMPSENSOR, common for all other HALs. Is it possible to add

#define ADC_CHANNEL_TEMPSENSOR ADC_CHANNEL_TEMPSENSOR_ADC1

to file stm32cube\stm32g4xx\drivers\include\stm32g4xx_hal_adc.h ?

@Delsian Delsian force-pushed the temper branch 2 times, most recently from d6638d9 to 03dbb43 Compare May 7, 2021 08:42
ABOSTM
ABOSTM approved these changes May 7, 2021
View reviewed changes
Copy link
Collaborator

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

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

Minor remarkl, otherwise LGTM

drivers/adc/adc_stm32.c Show resolved Hide resolved
@Delsian
Copy link
Contributor Author

Delsian commented May 7, 2021

@ABOSTM Yes, fixed with ARRAY_SIZE() instead of sizeof(), as this table contains uint16 values.

@Delsian
Copy link
Contributor Author

Delsian commented May 7, 2021

@erwango Sorry for disturbing, but looks like I need your help again. Probably my change in file tests/drivers/build_all/sensors_i_z.conf crashes buildkite for some reason, but I have no idea why.
Can you take a short look, please?

@erwango
Copy link
Member

erwango commented May 7, 2021

@erwango Sorry for disturbing, but looks like I need your help again. Probably my change in file tests/drivers/build_all/sensors_i_z.conf crashes buildkite for some reason, but I have no idea why.
Can you take a short look, please?

Seems like a buildkite limitation, but that's ok.
What we miss is @MaureenHelm to revisit and approve.

@galak
Copy link
Collaborator

galak commented May 7, 2021

@erwango Sorry for disturbing, but looks like I need your help again. Probably my change in file tests/drivers/build_all/sensors_i_z.conf crashes buildkite for some reason, but I have no idea why.
Can you take a short look, please?

Don't worry about that, will look over the CI issue once we have @MaureenHelm's approval.

MaureenHelm
MaureenHelm requested changes May 7, 2021
View reviewed changes
drivers/sensor/stm32_temp/stm32_temp.c Show resolved Hide resolved
drivers/sensor/stm32_temp/stm32_temp.c Outdated Show resolved Hide resolved
@Delsian
drivers: sensor: Add sensor_value_from_double
6a1aa7b 
Add helper function to convert double to struct sensor_value.

Signed-off-by: Eug Krashtan <eug.krashtan@gmail.com>
@Delsian
drivers: adc: Define max acquisition time
3f7d88d 
Required in STM32 tempereture sensor asquisition.

Signed-off-by: Eug Krashtan <eug.krashtan@gmail.com>
@Delsian
drivers: adc: STM32 Add common path
9c46ff5 
Enable common path for internal voltage voltage
reference source.

Signed-off-by: Eug Krashtan <eug.krashtan@gmail.com>
@Delsian
drivers: sensor: STM32 die temperature driver
474cae2 
STM32 internal temperature sensor driver.
This sensor can be used to measure the temperature of the CPU
and its surroundings.

Signed-off-by: Eug Krashtan <eug.krashtan@gmail.com>
@Delsian
samples: sensor: Add stm32_temp sample application
1a15e32 
Demonstating usage of STM32 temperature sensor.

Signed-off-by: Eug Krashtan <eug.krashtan@gmail.com>
@galak galak merged commit 0c4e9d2 into zephyrproject-rtos:master May 8, 2021
@Delsian Delsian deleted the temper branch May 8, 2021 16:47
@erwango erwango mentioned this pull request Jun 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StefJar StefJar StefJar left review comments

@MaureenHelm MaureenHelm MaureenHelm approved these changes

@erwango erwango erwango approved these changes

@ABOSTM ABOSTM ABOSTM approved these changes

@FRASTM FRASTM FRASTM approved these changes

@avisconti avisconti Awaiting requested review from avisconti

@cybertale cybertale Awaiting requested review from cybertale

@nashif nashif Awaiting requested review from nashif

@anangl anangl Awaiting requested review from anangl

@galak galak Awaiting requested review from galak

@mbolivar-nordic mbolivar-nordic Awaiting requested review from mbolivar-nordic

@carlescufi carlescufi Awaiting requested review from carlescufi

Assignees

@MaureenHelm MaureenHelm

Labels
area: ADC Analog-to-Digital Converter (ADC) 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 platform: STM32 ST Micro STM32
Projects
None yet
Milestone
v2.6.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Delsian @erwango @StefJar @galak @MaureenHelm @ABOSTM @FRASTM @zephyrbot