Skip to content

Conversation

@ruehlchris
Copy link

Version 2 Pull request for Bosch bmp180 pressure sensor.

  • merge multiple commits
  • fixed code style if() {}
  • fixed camel case
  • use k_sleep() not k_wait_busy()
  • i2c.dtsi tests/../sensors/ add the device tree information on the tail with increment number
  • add comment to make clear why use the custom byte swap for int16_t in bmp180.h

Thank you for your review.

@kartben
Copy link
Contributor

kartben commented Aug 17, 2024

@ruehlchris there is already #73218 that is about ready to get in. I would suggest you check it out and maybe provide feedback there if you feel there are things that could be improved . Thanks!

@mariopaja
Copy link
Contributor

BMP180 is the newer version of BMP085 and they use the same registers.
Unfortunately, it got blocked and noone replied although all the issues were addressed.

#67612

@ruehlchris
Copy link
Author

I apply the changes , rebase and force update the branch.

@ruehlchris ruehlchris force-pushed the bmp180_sensor_v2 branch 2 times, most recently from ad5efe0 to 62a58b2 Compare August 18, 2024 05:23
@ruehlchris ruehlchris requested a review from faxe1008 August 19, 2024 01:19
@ruehlchris ruehlchris force-pushed the bmp180_sensor_v2 branch 2 times, most recently from 39a7a5d to 87994ec Compare August 20, 2024 01:15
@ruehlchris ruehlchris closed this Aug 20, 2024
@ruehlchris
Copy link
Author

Follow the advice to reopen this branch, rebase the fixups.

@ruehlchris
Copy link
Author

@faxe1008 I'm very sorry - but I synced the branch here i the web interface cause of unresolved issue in the i2c.dtsi, now I rebased and push forced to where I started. But the workflow test will still fail for the one bad 0x8e not my commit. Lets see how I can fix this up later.

@ruehlchris
Copy link
Author

@faxe1008 I stuck with the build process because
tests/drivers/build_all/sensor/i2c.dtsi has conflict with this branch.
I can rebase and delete the commit for i2c.dtsi only for now.

@ruehlchris
Copy link
Author

Received a bmp390l today and run side to side test, realized that the bmp180 has the pressure in Pa, and the kPa conversion was wrong. Fixed it.

faxe1008
faxe1008 previously approved these changes Aug 26, 2024
@ruehlchris
Copy link
Author

Tested the bmp180 against the new arrived bmp390 and the accuracy wasn't great. Find the problem in an
int32_t overrun in my optimized conversion formula. Fixed it and push the fix.

faxe1008
faxe1008 previously approved these changes Aug 28, 2024
Chris Ruehl added 3 commits August 29, 2024 08:58
Add driver support for Bosch bmp180 pressure sensor running on i2c bus.
It pushes the driver files to the bmp180 directory under
drivers/sensor/bosch. The driver implement the Sensor API
and support channel DIE_TEMP and PRESSURE.
Driver had been tested with the NRF5340DK.

V2 fixup
---------
Drivers: Bosch bmp180 fixups for PR v2
fix BMP180 in CMakeLists.txt
rename regControl to ctrlreg
simplify the logic in set_attribute
use BMP180_GET_CMD_TEMP_DELAY

Drivers: Bosch bmp180 use system byteorder macros
To aware of ENDIANESS use the macros from byteorders.h

Drivers: Bosch bmp180 comments added to #endif
Add a comment to the #endif /* CONFIG_FOO */

Drivers: Bosch bmp180 , fix twister run error
twister report an error related to using floor() with a float
fix this using floorf() function.

Drivers: Bosch bmp180, rework review v2
fix code style issues
simplify the structure for bus and i2c.
prepare to remove the empty bmp180_i2c.c

Drivers: Bosch bmp180, remove bmp180_i2c.c
bmp180: update CMakeList.txt remove bmp180_i2c.c
bmp180: remove unused variable

fixups for complient checks

replace homebrew bitshift with byteorder.h macro
sys_get_be16 and sys_get_be24
cleanup remove unused spi.h
change struct reg16/24 to uint8_t[]
remove channel AMBIENT_TEMP
bmp180/Kconfig add missing help text
fix indent issue in bmp180_pm_action
merge BMP180_CONFIG_I2C into BMP180_INST
Move DT_DRV_COMPAT out of the header file
drop LOG_DBG() from read calibration data
remove bus_io implementation and header cleanup
move headers and structs out from bmp180.h
fix typo in comment
apply clang format changes, only if not line lenght violation
fix indent code error after joined lines
conversion from Pa to kPh was wrong, fixed it
Remove default=n from Kconfig, it is the default.
Move the conversion ready test into a function (duplicate code)
and apply retry policy
parameter naming with indicate the usage
change BMP180_INST(inst) to clang format
fix int32_t overflow in convension formular
fix wrong devider in convension formula
move BSWAP_s16 BSWAP_u16 from .h to .c

Signed-off-by: Chris Ruehl <[email protected]>
Add the yaml files needed for the dts binding of the Bosch bmp180 sensor.

V2 Fixup
--------
yaml fixup for complient checks
merge bmp180-i2c.yaml and bmp180.yaml

Signed-off-by: Chris Ruehl <[email protected]>
Add bmp180 test case to tests/drivers/build_all/sensor/i2c.dtsi

Signed-off-by: Chris Ruehl <[email protected]>
@ruehlchris
Copy link
Author

@faxe1008 thank you for your reviews! It was very helpful. Have send an other pull request for merge bmp390 into bmp388 driver. You might have a look if you have time. #77724

@nashif nashif merged commit 8a154da into zephyrproject-rtos:main Aug 30, 2024
@github-actions
Copy link

Hi @ruehlchris!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants