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

Add support for BMP388 barometer #13190

Merged
merged 2 commits into from
Oct 16, 2019
Merged

Add support for BMP388 barometer #13190

merged 2 commits into from
Oct 16, 2019

Conversation

modaltb
Copy link
Contributor

@modaltb modaltb commented Oct 14, 2019

Describe problem solved by this pull request
Originally this was submitted here: #13128
It was recommended to issue this PR directly on master instead of the v5x branch as this driver is needed by an upcoming PR for 'modalai_fmu-v5m' BSP.

Describe your solution
Flight tested using the I2C interface. Our HW doesn't have the SPI interface so this has been left in but is 'untested'. The noise and update rates were compared to a Pixhawk 4 Mini and it's comparable if not less noisy.

Describe possible alternatives
NA

Test data / coverage
We have been flight testing this with our hardware for the last months. Recently made some measurement improvements by oversampling and altitude hold is stable.

Additional context
#13128

If this merges to master, I'd suggest getting rid of these stale branches:
https://github.com/PX4/Firmware/commits/pr-bmp388
https://github.com/PX4/Firmware/commits/pr-bmp388_initial

Copy link
Member

@LorenzMeier LorenzMeier left a comment

Choose a reason for hiding this comment

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

Please rename the fmuv5m board target - all FMUvAb names are reserved for industry standard targets that are binary-compatible across manufacturers. A recent example for a non-standard board that we added was “durandal”. I would recommend a similar naming pattern.

The only targeted v5 series boards are v5 and v5X and there is no plan to add a third series, which would lead to additional fragmentation of our test matrix.

@dagar
Copy link
Member

dagar commented Oct 15, 2019

If possible I'd prefer we carry this without the Bosch header (bmp3_defs.h). It looks like a lot of the registers and structures aren't used. Thoughts?

@dagar dagar dismissed LorenzMeier’s stale review October 15, 2019 14:44

This PR doesn't introduce the board support, but the name change request has been noted.

@modaltb
Copy link
Contributor Author

modaltb commented Oct 15, 2019

@dagar Agreed, let me yank out bmp3_defs.h as that's adding a lot of unused code.

Also, I'll refactor the 'fmu_v5m' to something else before getting to the board support PR.

- Remove the Bosch bmp3_defs.h, updating bmp388.h with selected variables only to keep things cleaner
- Updated the fmu-v5 stackcheck to only use ms5611 barometer
@modaltb
Copy link
Contributor Author

modaltb commented Oct 16, 2019

@dagar per the slack chat, I changed the fmu-v5 stackcheck to only use the ms5611. Let me know if I mis-interpreted!

@dagar dagar merged commit 1e1549a into PX4:master Oct 16, 2019
@modaltb modaltb deleted the pr-bmp388-support branch October 22, 2019 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants