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 ms5837 driver #18213

Merged
merged 8 commits into from
Feb 12, 2022
Merged

add ms5837 driver #18213

merged 8 commits into from
Feb 12, 2022

Conversation

xn365
Copy link
Contributor

@xn365 xn365 commented Sep 9, 2021

Please use PX4 Discuss or Slack to align on pull requests if necessary. You can then open draft pull requests to get early feedback.

Describe problem solved by this pull request
A clear and concise description of the problem this proposed change will solve.
E.g. For this use case I ran into...

Describe your solution
A clear and concise description of what you have implemented.

Describe possible alternatives
A clear and concise description of alternative solutions or features you've considered.

Test data / coverage
How was it tested? What cases were covered? Logs uploaded to https://review.px4.io/ and screenshots of the important plot parts.

Additional context
Add any other related context or media.

@xn365
Copy link
Contributor Author

xn365 commented Sep 9, 2021

Because of the hand slip, the following contents have been deleted:
in file "src/drivers/drv_sensor.h "
#define DRV_IMU_DEVTYPE_ICM42670P 0x2A
PLZ check.

src/drivers/drv_sensor.h Outdated Show resolved Hide resolved
src/drivers/drv_sensor.h Outdated Show resolved Hide resolved
@xn365
Copy link
Contributor Author

xn365 commented Sep 10, 2021

Modify as recommended by Dagar.

@xn365 xn365 requested a review from dagar September 10, 2021 08:55
@dagar
Copy link
Member

dagar commented Sep 11, 2021

There are some minor formatting errors causing CI to fail, but this otherwise looks good to go from my perspective.
https://github.com/PX4/PX4-Autopilot/pull/18213/checks?check_run_id=3568147845
Screenshot from 2021-09-11 11-31-34

Has this driver actually been tested underwater?

@xn365
Copy link
Contributor Author

xn365 commented Sep 15, 2021

Has this driver actually been tested underwater?

Yes.
Ms5837 can be used in both air and water.

@xn365
Copy link
Contributor Author

xn365 commented Sep 15, 2021

This drive only gives the absolute pressure reading of the sensor. If it is used in liquid, it is also necessary to calculate the water depth according to the initial conditions (liquid density, air pressure on the liquid surface, etc.).

@xn365
Copy link
Contributor Author

xn365 commented Sep 16, 2021

Test:

@dagar dagar changed the base branch from master to pr-ms5837_squash February 12, 2022 19:17
@dagar dagar merged commit 25bb577 into PX4:pr-ms5837_squash Feb 12, 2022
dagar added a commit that referenced this pull request Feb 12, 2022
Co-authored-by: xn365 <xn_365@163.com>
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.

2 participants