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

AP_Airspeed: ND210 pressure sensor, airspeed library #26859

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zzaurak
Copy link

@zzaurak zzaurak commented Apr 21, 2024

I've rebased this pull request #23300, did some fixes and testet it with real hardware. It seems to work well on the bench, I will do a few real word flight test & comparisons next week and report as well.

@rmackay9
Copy link
Contributor

Hi @zzaurak,

Thanks for this. Just a small admin thing, it would be good to change the commit title (not the PR title but the individual commit titles) to start with "AP_Airspeed: ".

@zzaurak zzaurak force-pushed the nd210-pressure-sensor branch from 8088a65 to b78b8cc Compare April 22, 2024 06:44
@zzaurak
Copy link
Author

zzaurak commented Apr 22, 2024

Hi @rmackay9 thanks for the feedback, I changed the commit titles :)

@@ -71,3 +71,7 @@
#ifndef AP_AIRSPEED_EXTERNAL_ENABLED
#define AP_AIRSPEED_EXTERNAL_ENABLED AP_AIRSPEED_ENABLED && HAL_EXTERNAL_AHRS_ENABLED
#endif

#ifndef AP_AIRSPEED_ND210_ENABLED
#define AP_AIRSPEED_ND210_ENABLED AP_AIRSPEED_ENABLED
Copy link
Author

Choose a reason for hiding this comment

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

should this be changed to AP_AIRSPEED_BACKEND_DEFAULT_ENABLED instead of AP_AIRSPEED_ENABLED like it is in line 26 with the DLVR sensor? I don't really understand why there is this distinction, as they are both set to the same value in line 13.

@zzaurak zzaurak changed the title ND210 pressure sensor, airspeed library AP_Airspeed: ND210 pressure sensor, airspeed library Apr 22, 2024
@zzaurak zzaurak force-pushed the nd210-pressure-sensor branch 3 times, most recently from 4de9f82 to a48f40e Compare April 30, 2024 18:10
@zzaurak
Copy link
Author

zzaurak commented Apr 30, 2024

As promised I did a real-world comparison test flight with the ND210 airspeed sensor and this PR.
The top tube is the ND210, the bottom one is the Matek ASPD-DLVR-CAN.
Setup:
Screenshot 2024-04-30 at 20 12 39

I've attached the log 00000028-anon.log.zip.
In the flight before this logfile, airspeed autocal was on for the second airspeed sensor [1].

ASPD[0] is the DLVR
ASPD[1] is the ND210

I would say pretty much comparable, but especially in the lower speed ranges and while in QLoiter and FBWA it looks a bit smoother.
Screenshot 2024-04-30 at 20 16 15
Screenshot 2024-04-30 at 20 17 24
Screenshot 2024-04-30 at 20 17 51

@zzaurak zzaurak force-pushed the nd210-pressure-sensor branch from a48f40e to 8ae6f35 Compare May 3, 2024 10:36
@zzaurak
Copy link
Author

zzaurak commented May 3, 2024

@rmackay9 anything else that would be blocking a merge?

@zzaurak zzaurak force-pushed the nd210-pressure-sensor branch 3 times, most recently from 081fc23 to 5a380be Compare May 11, 2024 07:29
@zzaurak zzaurak force-pushed the nd210-pressure-sensor branch from 5a380be to a71dc45 Compare May 17, 2024 07:29
dwh and others added 2 commits June 24, 2024 09:14
adding ND210 Airspeed library
Tuning ND210
refactored _send_configuration_command and renamed member variables
Cleaning up printouts.
Changing cast to uint.
@zzaurak zzaurak force-pushed the nd210-pressure-sensor branch from a71dc45 to b6cb9bc Compare June 24, 2024 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants