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

Copter: improve landing detection by using filtered acceleration #13639

Merged
merged 3 commits into from
Feb 25, 2020

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented Feb 21, 2020

This PR modifies the update_throttle_mix() method so that it uses filtered accelerations (filtered at 1hz) instead of the raw accelerations. This is consistent with the update_land_detector() function and makes the landing detector more resistant to high vibrations.

High vibrations leading to a failed landing detector has been seen in the logs of at least one user.

Perhaps @lthall can say whether there's any downsides to the additional filtering.

Below is a test with this change applied to Copter-3.6.12 (see below for why I didn't use master) after setting SIM_ACC_RND = 20
before-after

This PR also makes some minor formatting and function name changes to both Copter and Plane. These are non-functional changes.

Note: we should probably make the same change in Plane but I could not immediately figure out where to introduce the change

Note2: I attempted to test with master but apparently there are no longer any vibrations in master? @andyp1per can you check that your changes to SITL haven't disabled vibration by default?

was called update_throttle_thr_mix
also minor formatting fixes
was called update_throttle_thr_mix
also minor format fixes
Copy link
Contributor

@lthall lthall left a comment

Choose a reason for hiding this comment

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

I think this is a safe change that will handle high accel noise better.

@andyp1per
Copy link
Collaborator

@rmackay9 I'm still waiting for #13169 to go in as there are known problems with the vibration support as it stands 🙂. That said the noise is now somewhat proportional to throttle - could it be that you throttle is zero in this test?

@rmackay9
Copy link
Contributor Author

@andyp1per, no, the throttle is definitely not zero. The issue is quite obvious by graphing the RCOU message values in master. At least in my tests they produced perfectly identical outputs for each motor. It's as if there's no noise in the IMU at all.

@andyp1per
Copy link
Collaborator

Ok, do you have a test I can try?

@rmackay9
Copy link
Contributor Author

@andyp1per,

Thanks very much for looking into this. So this is the test I did using the Copter-3.6 branch (which seems OK to me) vs master.

  • param set SIM_ACC_RND 5
  • param set SIM_ACC2_RND 5
  • loiter
  • arm throttle
  • rc 3 1800 (and wait a few seconds)
  • rc 3 1200 (wait for land)
  • rc 3 1000
  • disarm

Then I downloaded the log and graphed the RCOU messages. Below is a screen shot.
sim-acc-rnd-rcout-before-after

@andyp1per
Copy link
Collaborator

#13649 should fix the noise issues in master

@rmackay9
Copy link
Contributor Author

@andyp1per, awesome, thanks very much for the fix related to (missing) accel noise in master!

@andyp1per
Copy link
Collaborator

Did it work?

@rmackay9 rmackay9 merged commit 7565e9d into ArduPilot:master Feb 25, 2020
@rmackay9 rmackay9 deleted the copter-throttle-mix-filter branch February 25, 2020 07:55
@rmackay9
Copy link
Contributor Author

I've gone ahead and merged this but as discussed on the dev call I'll attempt to make a similar fix for plane. #13664

@andyp1per, yes, it's looking much better now, thanks!
image

@rmackay9
Copy link
Contributor Author

rmackay9 commented Mar 5, 2020

This has been successfully flight tested now and seems to greatly improve the landing detection for the one vehicle in question at least.

@rmackay9
Copy link
Contributor Author

rmackay9 commented May 6, 2020

This is included in Copter-4.0.4-rc1

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

Successfully merging this pull request may close these issues.

None yet

4 participants