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

Qplane auto disarm #6475

Closed
wants to merge 1 commit into from
Closed

Conversation

magicrub
Copy link
Contributor

Implement LAND_DISARMDELAY param for quadplanes instead of always immediately disarming on land.

Fixes: http://discuss.ardupilot.org/t/quadplane-multiple-vtol-lands-vtol-take-offs-in-a-single-mission/18561/4

Needs some testing by someone who know how to use quadplanes in SITL. The only gotcha on this is that a quadplane is now using is_flying() before it auto-disarms.

@@ -925,7 +925,7 @@ class Plane : public AP_HAL::HAL::Callbacks {
bool geofence_stickmixing(void);
void geofence_send_status(mavlink_channel_t chan);
bool geofence_breached(void);
void disarm_if_autoland_complete();
void disarm_if_autoland_complete(uint32_t timer);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this just have a default argument value of 0? It's making the stuff in AP_Landing look really confusing, or at least in need of a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about that but wasn't sure how the function pointers would handle it. Do you know?

@magicrub
Copy link
Contributor Author

Well, it's failing Travis. I think it's stuck right here.

I don't know what unit "timeout=1200" is but that sounds like a long timeout. Before it was a hardcoded 5s but now it uses LAND_DISARMDELAY which defaults to 20s. Either that or the other checks are failing.

@magicrub
Copy link
Contributor Author

@peterbarker how do I get the .bin logs from Travis from this PR? I can only find the nightly's at autotest.ardupilot.org

@WickedShell
Copy link
Contributor

@magicrub you can run the same test travis does locally for easier debugging.

@peterbarker
Copy link
Contributor

peterbarker commented Jun 21, 2017 via email

@magicrub
Copy link
Contributor Author

Does -j8 and -S99 go in front or behind those arguments? Also, shouldn't I be doing fly.Quadplane?

@peterbarker
Copy link
Contributor

peterbarker commented Jun 21, 2017 via email

@OXINARF
Copy link
Member

OXINARF commented Jun 21, 2017

Just to clarify: Travis is failing because DISARMED never appears.

@magicrub
Copy link
Contributor Author

Correct

@magicrub
Copy link
Contributor Author

I'll be on a long plane flight tomorrow, I'll take a stab at this again.

@aperez24
Copy link

aperez24 commented Jul 3, 2017

Hi. I was curious if you had any further progress with this particular item?

@magicrub magicrub force-pushed the qplane_auto_disarm branch 2 times, most recently from ac41e58 to 39820f5 Compare July 9, 2017 19:43
@magicrub
Copy link
Contributor Author

magicrub commented Jul 9, 2017

This should be fixed now. Would like @tridge 's review before merge.

@tridge
Copy link
Contributor

tridge commented Jul 10, 2017

to actually be useful it will need more work than this. It would need changes to the mission logic so that it can continue the mission after a VTOL landing.
In that case I think it would be better not to use the disarm delay, and instead just move to the next mission item on land complete

@magicrub
Copy link
Contributor Author

Which is empty in most cases so it will trigger qrtl? Then what..?

@magicrub
Copy link
Contributor Author

And normal fixed wing NAV_LAND doesn't increment the index. If it is allowed to move on to another item I'm afraid that will case more accidents with it immediately taking off. Only a delay command makes any sense after the qland command.

@tridge
Copy link
Contributor

tridge commented Jul 12, 2017

And normal fixed wing NAV_LAND doesn't increment the index. If it is allowed to move on to another item I'm afraid that will case more accidents with it immediately taking off. Only a delay command makes any sense after the qland command.

indeed, and that is why the reasoning behind this arming change needs a re-think. If the aim is to get multi-takeoff missions then we need quite a large change in the code, both for fixed wing and quadplanes

@magicrub
Copy link
Contributor Author

Perhaps, but meanwhile I don't see any disadvantage to allowing a delayed auto-disarm instead of forcing an instant disarm.

if (plane.landing.get_disarm_delay() <= 0 ||
(now - landing_detect.lower_limit_start_ms >= plane.landing.get_disarm_delay()*1000UL)) {
// disarm immediately or after user-defined delay
plane.disarm_motors();
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we need to keep the load() of airspeed_cruise_cm, otherwise landing missions with airspeed changes won't reset correct airspeed

Copy link
Contributor

Choose a reason for hiding this comment

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

what stops this triggering when landing_detect.land_start_ms == 0?
have you flown this?

@mrthomasbarnard
Copy link

Hi,

this is a very useful change for me as we would like to do drops of fragile cargo and would ideally like to be "Landed" to drop the cargo and then take-off again at 3 or 4 different locations.

Is this something that is still being progressed and would you need anybody to test it either in SITL or for real, we would be happy to help...

Thanks
Tom

@magicrub
Copy link
Contributor Author

@mrthomasbarnard glad to hear you're interested in this! I ended up ignoring this because I couldn't argue for a good use-case so it was shot down in a dev call. However, the PR is still alive and I just rebased it so if you can flight test and/or share your use-case we'd love to hear about it!

@magicrub
Copy link
Contributor Author

this is failing autotest because.. it's a change in behavior that isn't scripted.

@magicrub magicrub closed this Feb 10, 2019
@magicrub magicrub deleted the qplane_auto_disarm branch February 10, 2019 19:52
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.

None yet

7 participants