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

Block arming if takeoff waypoint is inactive and MIS_TKOFF_REQ is set to 1 #13824

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

Conversation

Kjkinney
Copy link
Contributor

@Kjkinney Kjkinney commented Dec 31, 2019

Describe problem solved by this pull request
Currently PX4 has no pre-arm checks that check to see if a takeoff waypoint is active. It checks to make sure that a mission has a takeoff waypoint, but that's the extent of it . This can be an issue for fixed wings, especially hand launched ones as arming the vehicle without the first waypoint active will not activate launch detection. This will lead to failed takeoffs. This PR adds a check to determine if the takeoff waypoint is active or not.

Describe your solution
I added a check in the prearm checks to determine if the current active waypoint is a takeoff waypoint from the position setpoint. This is also tied to the MIS_TAKEOFF_REQ parameter. My thought process is that if you have this parameter active, then you probably want added protection against potentially arming the vehicle in a non-takeoff state.

Describe possible alternatives
Currently if the position setpoint is not a takeoff setpoint, the vehicle will not arm. This means that you can not arm in "Manual" modes if the parameter is set. This could be changed to allow this check to function in only "Auto" modes.

Test data / coverage
A version of this has been in use on our fixed-wing vehicles for the past 3 months. This version has been ground tested with the parameter active and inactive.

Additional Context
This was added after we had a number of users arming the airplane with a non-takeoff waypoint active.

… active, and has the Mission Require Takeoff waypoint active
@Antiheavy
Copy link
Contributor

FYI @TSC21 this builds on the PR you did about a year ago: #11531 that added the parameter MIS_TAKEOFF_REQ

@Kjkinney
Copy link
Contributor Author

Kjkinney commented Jan 2, 2020

I should also mention this is in response to #10657

@Kjkinney Kjkinney changed the title Block arming if takeoff waypoint is inactive and MIS_TKOFF_REQ is set to1 Block arming if takeoff waypoint is inactive and MIS_TKOFF_REQ is set to 1 Jan 2, 2020
@Antiheavy Antiheavy requested a review from dagar January 8, 2020 17:36
Copy link
Contributor

@Antiheavy Antiheavy left a comment

Choose a reason for hiding this comment

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

Looks good, but I'd like another reviewer to comment too.

@julianoes
Copy link
Contributor

@Kjkinney

Currently if the position setpoint is not a takeoff setpoint, the vehicle will not arm. This means that you can not arm in "Manual" modes if the parameter is set. This could be changed to allow this check to function in only "Auto" modes.

If there is no mission set at all, can you arm in a manual mode or neither?

// This requires a valid takeodd waypoint to be set
uORB::Subscription setpoint_sub(ORB_ID(position_setpoint_triplet));
position_setpoint_triplet_s setpoint = {};
setpoint_sub.update(&setpoint);
Copy link
Contributor

Choose a reason for hiding this comment

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

I have two concerns here:

  1. The triplet is quite big and hence this needs CPU to copy and RAM to store it.
  2. The flow is generally commander -> navigator but here the commander is subscribing to navigator again to check what it is doing when really navigator should just be executing as told by commander.

Alternatives I see:

  1. Add this check into the mission feasibility checker. So when it checks that the mission is valid it also checks if it is set to the beginning?
  2. Don't start a mission in the controller even if armed if the item is not a takeoff item.

@MaEtUgR do you have other ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

@julianoes How about using the topic mission_result for this? To me this seems to be a topic which navigator uses to report various information regarding the current mission.
Wouldn't it be as simple as checking if mission_result.seq_current == 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, good idea!

@RomanBapst
Copy link
Contributor

@Kjkinney Thanks for coming up with this proposal! How does it actually happen that the takeoff waypoint is not selected when starting the mission, e.g. right before the launch?
Is this related to the user clicking on another waypoint by mistake?

@Kjkinney
Copy link
Contributor Author

@RomanBapst precisely. We have had users accidentally activate another waypoint before takeoff and then arming the vehicle. This almost always leads to crashes if the user does not catch this.

@stale
Copy link

stale bot commented Apr 16, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

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

Successfully merging this pull request may close these issues.

4 participants