ardupilot
ardupilot copied to clipboard
Plane: NAV_ALTITUDE_WAIT apply wiggle to all control surfaces
This is two commits. First is an NFC refactor to bring NAV_ALTITUDE_WAIT mission and logic code paths to line up with NAV_DELAY. It is now owned by ModeAuto instead of global access via plane.auto_state. Second is to wiggle all control surfaces instead of just the 3 main ones. I wrote this in the Holybro booth at AUVSI Exponential 2024 /w inspiration from @ohitstarik to improve his balloon drop servo wiggles.
This code hasn't been changed in many years so I thought it deserved a refactor before anyone added new functionality to it.
I've compiled and bench tested to verify the above changes as well:
https://www.youtube.com/watch?v=uB6nyUTwIfc
We do have a open issue in this area: https://github.com/ArduPilot/ardupilot/issues/25851
I do like the refactor. I don't understand why the fix works. Your outputting to all control surfaces, but the existing output mixers are still running, so all of those surfaces other than aileron, elevator, rudder and flap will just be overwritten with values derived from those four when the output mixers run later in the loop.
@ohitstarik I would have expected the existing wiggle function to work for the right elevon but not left, this is not show in your testing?
This is blocked by PR https://github.com/ArduPilot/ardupilot/pull/26946
@IamPete1 I only tested with the Left Elevon, so yes theres a real possibility that the right elevon wouldve worked fine.
I noted that the control surfaces had less throws while wiggling marked as an elevon. They had noticeably more throw while wiggling marked as Ailerons. This might have something to do with the old output mixers still running.
PR https://github.com/ArduPilot/ardupilot/pull/26946 has merged so this PR is just the control surface change
@IamPete1 I see what you mean about the mixer handling many of the flight surfaces, and the mixer also cancelling some of the movements in your Issue https://github.com/ArduPilot/ardupilot/issues/25851. I'm open to suggestions on how to do this, lets discuss in the next devcall
Possible fix concept https://github.com/ArduPilot/ardupilot/compare/master...ohitstarik:ardupilot:wiggle_staging_fix
result from devcal:
- The change from @ohitstarik looks OK but it's extending the existing messy code so a better way-forward is to do it right with the modern functions we have today. The existing code is 8 years old... many helpers have been added to the Servo library since. A better solution is a time-based thing that is not using the magic 4500 type numbers
- This is generally better handled in scripting so I'll add a Vehicle::is_wiggling_servo() flag that has LUA hooks.
- Nice-to-have: add support for MAV_CMD_NAV_ALTITUDE_WAIT to all vehicles
I missed the original dev call, apologies. I've created a separate PR for this fix: #27275
I agree that this could be fixed better but the situation is:
1.) something doesnt work as intended in the code 2.) we have a fix for it
i propose that we merge this as an interim fix until someone picks up the more intensive fix in the future. Otherwise, we'll knowingly be leaving the feature broken until whenever that fix happens. happy to talk about this on the next dev call.
replaced by https://github.com/ArduPilot/ardupilot/pull/27275