-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
Move REQUIRE_POSITION_FOR_ARMING bit to AP_arming #29076
base: master
Are you sure you want to change the base?
Move REQUIRE_POSITION_FOR_ARMING bit to AP_arming #29076
Conversation
while Copter is the only user at the moment, will be useful on Rover shortly
while Copter is the only user at the moment, will be useful on Rover shortly
while Copter is the only user at the moment, will be useful on Rover shortly
@@ -146,6 +146,7 @@ const AP_Param::GroupInfo AP_Arming::var_info[] = { | |||
// @DisplayName: Arming options | |||
// @Description: Options that can be applied to change arming behaviour | |||
// @Bitmask: 0:Disable prearm display,1:Do not send status text on state change | |||
// @Bitmask{Copter}: 0:Disable prearm display,1:Do not send status text on state change,2:Require position for arming |
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.
// @Bitmask{Copter}: 0:Disable prearm display,1:Do not send status text on state change,2:Require position for arming | |
// @Bitmask{Copter}: 0:Disable prearm display,1:Do not send status text on state change,2:Require location for arming |
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.
Why not add it to plane also?
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.
Plane already requires position for arming, all the time.
That's actually why we put this into Copter's flight options in the first place - we're going to have an awkward time when Plane no longer requires position for arming....
But we want this bit for Rover, so @rmackay9 asked for it to be moved (#29073)
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.
Thanks for this. I'm not sure we really needed to replace "position" with "location" but it's OK in either case.
More importantly I guess the reason we put it in FLIGHT_OPTIONS in the first place was because once someone sets this bit they will never get the default changes to the ARMING_OPTIONS parameter? that hardly seems like a good enough reason so I suspect I'm missing something
I should have taken better notes :-) |
... also rename it to
REQUIRE_LOCATION_FROM_ARMING
to be more in-line with ArduPilot's distinction between "position" (local position) and "location" (absolute lat/lng)