-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
base: main
Are you sure you want to change the base?
Conversation
… active, and has the Mission Require Takeoff waypoint active
I should also mention this is in response to #10657 |
src/modules/commander/Arming/PreFlightCheck/checks/preArmCheck.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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.
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); |
There was a problem hiding this comment.
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:
- The triplet is quite big and hence this needs CPU to copy and RAM to store it.
- 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:
- 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?
- 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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good idea!
@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? |
@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. |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
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.