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

Plane: disallow mavlink disarm while flying #20234

Merged
merged 4 commits into from Mar 9, 2022

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Mar 7, 2022

this relies on is_flying(), and we will need to watch for reports of
the heuristics failing
in simple SITL tests the is_flying() checks seem to be good for VTOL modes

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Looks right

Copy link
Member

@IamPete1 IamPete1 left a comment

Choose a reason for hiding this comment

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

LGTM. If users really want to disarm from mavlink they can still force.

@IamPete1
Copy link
Member

IamPete1 commented Mar 7, 2022

CI failures look genuine. Can no longer disarm in airmode since that forces flying. Just a matter of updating the tests to reflect the new behavior.

@tridge
Copy link
Contributor Author

tridge commented Mar 7, 2022

CI failures look genuine. Can no longer disarm in airmode since that forces flying. Just a matter of updating the tests to reflect the new behavior.

fixed

Copy link
Collaborator

@davidbuzz davidbuzz left a comment

Choose a reason for hiding this comment

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

seems quite reasonable.

@WickedShell
Copy link
Contributor

WickedShell commented Mar 8, 2022

I really think this needs an opt in (or at least have an opt out) check. I'd prefer not to have this. I've had to disarm aircraft in the past from the GCS and the flying estimate has been quite wrong before. This definitely doesn't look like a check I'd like to have on, as disarming is used to safe the aircraft in the event of a crash or anomaly.

Random log I have here shows that after crashing is flying held true for 5 seconds before falling down to be not true, and that only happened because it was disarmed.

@tridge
Copy link
Contributor Author

tridge commented Mar 8, 2022

@WickedShell as discussed, I'd be happy to add a FLIGHT_OPTIONS bit to disable the new behaviour, but we do need to change the default because of the way the mission planner arm/disarm button is implemented

this relies on is_flying(), and we will need to watch for reports of
the heuristics failing
expect disarm to fail when airmode on
@peterbarker peterbarker merged commit 6fcf85e into ArduPilot:master Mar 9, 2022
@tridge tridge added this to pending in Plane 4.2 Mar 9, 2022
@tridge tridge moved this from pending to merged in Plane 4.2 Mar 9, 2022
@timtuxworth
Copy link
Contributor

What about RC disarm? I accidentally disarmed my plane today about 200m up in the air. The log is quite spectacular and crazy that the plane is still in one piece. Will this PR prevent this?
This is the log: https://www.dropbox.com/s/78uk9sdjyswfw2e/log_81_2022-3-10-15-17-00.bin?dl=0

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

6 participants