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

Copter: stop disarming as part of running rtl as mission item #6914

Merged
merged 1 commit into from Nov 6, 2017

Conversation

Projects
None yet
6 participants
@peterbarker
Copy link
Contributor

commented Sep 7, 2017

Fixed #6324

Note that the land-as-mission-item code is duplicated from the
rtl-as-mode code but omits the disarm part, so doesn't suffer from the
same bug.

Copter: stop disarming as part of running rtl as mission item
Fixed #6324

Note that the land-as-mission-item code is duplicated from the
rtl-as-mode code but omits the disarm part, so doesn't suffer from the
same bug.

@OXINARF OXINARF added the Copter label Sep 7, 2017

@OXINARF

OXINARF approved these changes Sep 7, 2017

Copy link
Member

left a comment

LGTM.

I'd like to point out though that this has a behavior change: AUTO won't disarm anymore after an RTL. I think that's OK, but should be noted in future release notes.

@Pedals2Paddles

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2017

If doesn't disarm, are you saying it will just stay running at minimum throttle? Or will it time out after the disarm_delay time expires? If it doesn't disarm at all, that leaves you no way to disarm the vehicle if the radio is disconnected or fails.

@tridge

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2017

I think most people would expect it to disarm. A behaviour change like this would need a parameter for the new behaviour

@peterbarker

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2017

@peterbarker

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2017

@Pedals2Paddles

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2017

You're talking the PILOT_THR_BHV parameter, right? (http://ardupilot.org/copter/docs/parameters.html#pilot-thr-bhv-throttle-stick-behavior)

So as long as the bitmask for Disarm on land detection is enabled, it will disarm as expected in auto mode RTL? What happens if it is not set? Does it run for the DISARM_DELAY duration, then disarm? Because I've flown with that not set before, and it always disarmed after the delay time regardless.

@OXINARF

This comment has been minimized.

Copy link
Member

commented Sep 8, 2017

I'm sorry I've made hell broke loose over here with my simple comment. Let me try to clarify everything:

  • when a mission ends, if the vehicle is landed, disarming motors will happen; this happens independently of the last command
  • automatic delayed disarm only occurs when your throttle is zero (or if you have the sprung stick bit set in PILOT_THR_BHV, when it is zero in Acro and Stabilize and mid-stick on all other modes)
  • with current code any command to RTL will disarm when it ends, meaning that any command after it won't be executed; with this change if there is a command after RTL it will still be executed <- this is the main difference I was referring to

Here is a simple mission example that shows the differences:

  • waypoint 1
  • land waypoint 2
  • nav delay of 20 seconds
  • takeoff command
  • waypoint 3
  • RTL
  • waypoint 4

With current code and throttle not zero: mission will end after RTL because a disarm happens

With current code and throttle at zero: mission will end after land waypoint 2, because the auto disarm will happen during the nav delay

With this PR code and throttle not zero: mission will end after waypoint 4, which means Copter will still be in the air and no disarm occurs

With this PR code and throttle at zero: mission will end after land waypoint 2, because the auto disarm will happen during the nav delay

@Pedals2Paddles the disarm on landing bit in PILOT_THR_BHV only affects modes that can be armed, which isn't the case with Auto.

@Pedals2Paddles

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2017

That is a great explanation and it all makes sense now.

@peterbarker

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2017

@OXINARF

This comment has been minimized.

Copy link
Member

commented Sep 9, 2017

So any suggestions on what we should do?

@peterbarker I believe this change is a correct one as is. I see the RTL command as a shortcut to waypoint at home + land command, so it makes sense it doesn't disarm if it isn't the last command. If a user puts commands after an RTL what is the purpose? Isn't it to continue after the RTL? If a user wants to end the mission with an RTL then put it at the end of the mission.

@amilcarlucas

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2017

ping

@OXINARF OXINARF removed the DevCallTopic label Nov 6, 2017

@OXINARF OXINARF merged commit 5b79325 into ArduPilot:master Nov 6, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details
@OXINARF

This comment has been minimized.

Copy link
Member

commented Nov 6, 2017

Merged, thanks!

@peterbarker peterbarker deleted the peterbarker:clear-mission-after-rtl branch Mar 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.