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

Adding a new differential pressure sensor ASP5033 #21568

Merged
merged 14 commits into from
Sep 4, 2023
Merged

Adding a new differential pressure sensor ASP5033 #21568

merged 14 commits into from
Sep 4, 2023

Conversation

DenisPGH
Copy link
Contributor

@DenisPGH DenisPGH commented May 9, 2023

Solved Problem

Just added a new driver for differential pressure sensor QioTeck ASP5033
https://www.qio-tek.com/index.php/product/qiotek-asp5033-dronecan-airspeed-and-compass-module/

Solution

  • created a new directory in src/drivers/differential_pressure with required files

Context

simple test video with blowing:
https://mega.nz/file/BnI00RrB#4qJGrwMDJpCevAnu8O7x4A8T5kPWacTT3g5sZol5G7E

@junwoo091400 junwoo091400 added Sensors All the sensors! Drivers 🔧 Sensors, Actuators, etc Admin: Needs Review ✋ labels May 17, 2023
@junwoo091400
Copy link
Contributor

Thanks for the contribution!

julianoes
julianoes requested changes May 30, 2023
View reviewed changes
Copy link
Contributor

@julianoes julianoes 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 the contribution! In order for this to be merged it needs some fixing as commented inline.

boards/px4/fmu-v6c/default.px4board Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/ASP5033Driver.cpp Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/ASP5033Driver.cpp Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/ASP5033Driver.hpp Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/ASP5033Driver.hpp Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/ASP5033Driver.cpp Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/ASP5033Driver.cpp Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/CMakeLists.txt Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033driver/parameters.c Outdated Show resolved Hide resolved
src/drivers/drv_sensor.h Outdated Show resolved Hide resolved
@DenisPGH
Copy link
Contributor Author

In the tests with my Pixhawk 6c after 50-60 min work I got this message : "Baro sensor #0 failure: High Error count . Land immediately and check the system". The ASP5033 still work normally after that. Also by Jenkins tests two hardware checks(cuav_x7pro_test and cubepilot_cubeorange_test) fail with error: "ERROR [gps] timeout, forcing stop". Any idea how to solve this problem and finish the request?

julianoes
julianoes requested changes Jun 21, 2023
View reviewed changes
Copy link
Contributor

@julianoes julianoes 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 the fixes! I think this is good to be merged soon. Can you make sure style is fixed? make format, and check my inline comments.

.vscode/settings.json Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033/ASP5033.hpp Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033/ASP5033.cpp Outdated Show resolved Hide resolved
julianoes
julianoes requested changes Jun 21, 2023
View reviewed changes
src/drivers/drv_sensor.h Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033/ASP5033.hpp Outdated Show resolved Hide resolved
julianoes
julianoes reviewed Jun 24, 2023
View reviewed changes
Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

Two small changes that shouldn't be there, otherwise this is good to go, thanks for iterating.

boards/px4/fmu-v6c/default.px4board Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
src/drivers/differential_pressure/asp5033/ASP5033.cpp Show resolved Hide resolved
@julianoes
Copy link
Contributor

@dagar any objections to get this in?

nano and others added 14 commits September 4, 2023 06:59
@julianoes
added a new driver ASP5033 for measuring the differential pressure an…
a171110 
…d airspeed
@julianoes
fixed missing variable with the address of the sensor
fb70509
@julianoes
changed class name, added right license message, added script to star…
7fbd0df 
…t the driver with parameter value, removed DDEBUG_BUILD from COMPILE_FLAGS
@julianoes
removed advertise, fixed timestamp replication, used elapsed time , m…
77c823f 
…ade variables private
@julianoes
no fake data from the sensor any more
ce589d8
@DenisPGH @julianoes
added more information in the description and added author info
f52aeff
@DenisPGH @julianoes
removed unnecessary if and the sensor from default configuration for 6c
084a6db
@julianoes
fixed formatting errors and ASP5033_BASEADDR definition
fa3c630
@julianoes
improved measuring
869c639
@julianoes
removed probe()
ce85770
@julianoes
changed probe()-check the device
c7e0559
@julianoes
changed probe()-check the device-2
ed4de7a
@julianoes
changed check device implementation and fixed address
c50b5bb
@julianoes
fixet last issues
43ed4ad
@julianoes
Copy link
Contributor

I've rebased and force pushed this PR. Let's check CI again now.

@julianoes
Copy link
Contributor

CI looks unrelated. Let's get this in.

@julianoes julianoes merged commit ece338c into PX4:main Sep 4, 2023
85 of 86 checks passed
@DenisPGH
Copy link
Contributor Author

DenisPGH commented Sep 5, 2023

Thank you very much, Julian!

@QioTek
Copy link

QioTek commented Sep 5, 2023

Thank you very much, Julian!

Thank you for your contribution. Could you like to post a new PR to correct the K value to 7? Its correct K value should be 7. If you need sensor testing, you can contact me jasonsiu@qio-tek.com to provide one.

I hope this fix PR will be submitted by the author. Thank you.

@DenisPGH
Copy link
Contributor Author

DenisPGH commented Sep 5, 2023

Hello @QioTek , I am not sure, if it was from my sensor. I have only once. I did many tests, and with k=8, the output value was much more stable.

royratcliffe pushed a commit to royratcliffe/PX4-Autopilot that referenced this pull request Sep 6, 2023
@DenisPGH @royratcliffe
Adding a new differential pressure sensor ASP5033 (PX4#21568)
899c02d 
* added a new driver ASP5033 for measuring the differential pressure and airspeed

---------

Co-authored-by: nano <nanobotzhe@abv.bg>
Co-authored-by: Denislav Petrov <you@example.com>
@julianoes
Copy link
Contributor

Follow-up where I changed it to 7: #22048

antbre pushed a commit to BioMorphic-Intelligence-Lab/PX4-Autopilot that referenced this pull request Sep 14, 2023
@DenisPGH @antbre
Adding a new differential pressure sensor ASP5033 (PX4#21568)
e953045 
* added a new driver ASP5033 for measuring the differential pressure and airspeed

---------

Co-authored-by: nano <nanobotzhe@abv.bg>
Co-authored-by: Denislav Petrov <you@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnedev tnedev tnedev left review comments

@hamishwillee hamishwillee hamishwillee left review comments

@julianoes julianoes julianoes approved these changes

Assignees
No one assigned
Labels
Admin: Needs Review ✋ Drivers 🔧 Sensors, Actuators, etc Sensors All the sensors!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@DenisPGH @junwoo091400 @julianoes @QioTek @tnedev @hamishwillee