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

WIP: Fix FMU MC failsafe on RC loss #5388

Closed
wants to merge 5 commits into from

Conversation

kd0aij
Copy link
Contributor

@kd0aij kd0aij commented Aug 26, 2016

This fixes some problems with RC loss failsafe when in manual modes on fmu-v4, but may break other things since it's not clear to me how it was intended to work.

It appears that failsafe would never be entered if in manual mode with a GPS connected.
Also, px4fmu quit running the output mixer when RC was lost.

@kd0aij kd0aij changed the title WIP: Fix fmu mc failsafe on RC loss WIP: Fix FMU MC failsafe on RC loss Aug 26, 2016
@LorenzMeier
Copy link
Member

Please rebase on master to fix CI.

@LorenzMeier
Copy link
Member

@julianoes Can you have a look?

@@ -889,6 +891,12 @@ MulticopterAttitudeControl::task_main()
vehicle_status_poll();
vehicle_motor_limits_poll();

/* Simple handling of failsafe: kill motors if failsafe is on */
if (_termination_enabled && _v_control_mode.flag_control_termination_enabled) {
_armed.lockdown = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't have any effect. The _armed topic is subscribed here, not published.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I guess all we can do here is deploy a parachute, as in the FW case.

@kd0aij
Copy link
Contributor Author

kd0aij commented Aug 26, 2016

The remaining open question on this fix is how to repair the commander logic which sets flags.gps_failure. Since this is never set if there is no GPS module (one known problem case), there is currently no way to configure RC loss to cause motor shutdown (termination). My proposed fix works, but precludes a controlled descent on RC loss. Of course at this point (line 2703 of commander.cpp) we're already in termination state, not land or descend...

Also, even if GPS were available, would it be wise to execute a powered descent, given that we have no idea what is below the vehicle?

@mhkabir
Copy link
Member

mhkabir commented Aug 27, 2016

Even without GPS, the failsafe configuration should have an openloop descent as default. With GPS available, we can just close the altitude control loop. Even if we have no idea about what's below / exactly how high we are, it's safer than killing the motors.

@kd0aij
Copy link
Contributor Author

kd0aij commented Aug 27, 2016

@mhkabir That's debatable. The FAI rules (http://www.fai.org/downloads/ciam/SC4_F3FPV_2016) for FPV racers state

The model must be equipped with a fail-safe device, the triggering of which stops the motorization.

and

In case of emergency, the helper is authorised to shut off the transmitter in order to trigger the fail-safe
device.

@mhkabir
Copy link
Member

mhkabir commented Aug 27, 2016

I was talking about the general config (for most users). For racers, killing motors is obviously something we will have to support, as an extension of the flight termination state.

@kd0aij
Copy link
Contributor Author

kd0aij commented Aug 27, 2016

Right now, all users will see a fly-away, and at least one user (besides me) would like to be able to prevent such out-of-control situations:
#5357 (comment)

@kd0aij
Copy link
Contributor Author

kd0aij commented Aug 28, 2016

I'll close this in another day or two unless there is further useful discussion

@mhkabir
Copy link
Member

mhkabir commented Aug 28, 2016

@kd0aij, I will be joining this effort to fix failsafe handling. Just reviewing the state machine code now.
Thanks a lot for the initiative!

@julianoes
Copy link
Contributor

Sorry @kd0aij for the lack of work and communication here. This is something that needs to be fixed for sure. I'll assign myself to keep it on the radar.

@julianoes julianoes self-assigned this Aug 31, 2016
@kd0aij
Copy link
Contributor Author

kd0aij commented Sep 20, 2016

@mhkabir @julianoes any progress on this?

@julianoes
Copy link
Contributor

@kd0aij sorry I'm pretty busy. Will tackle it when I get a chance.

@LorenzMeier
Copy link
Member

@julianoes Any chance to get back to this?

@julianoes
Copy link
Contributor

@kd0aij Ok I started debugging this with a Pixracer and a FrSky D4RII receiver and the first thing of master I tested was:
Flying in manual without GPS connected and switching off RC. It successfully enters "Land" mode and lowers overall PWM trying to descend. At some point it switches to min PWM and detects landed.
Does this fit with your observation?

@kd0aij
Copy link
Contributor Author

kd0aij commented Oct 10, 2016

I believe so. My PR addressed the case with GPS valid:
#5388 (comment)

On Mon, Oct 10, 2016 at 2:45 PM Julian Oes notifications@github.com wrote:

@kd0aij https://github.com/kd0aij Ok I started debugging this with a
Pixracer and a FrSky D4RII receiver and the first thing of master I tested
was:
Flying in manual without GPS connected and switching off RC. It
successfully enters "Land" mode and lowers overall PWM trying to descend.
At some point it switches to min PWM and detects landed.
Does this fit with your observation?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#5388 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACMZPQixkuz254gnE9IBNYJF5xkhNWhzks5qyqPkgaJpZM4Jtn_m
.

@kd0aij
Copy link
Contributor Author

kd0aij commented Oct 10, 2016

I'm sure I never (intentionally) flight tested RC loss under any
circumstances since I wasn't satisfied with the documentation.

On Mon, Oct 10, 2016 at 2:52 PM Mark Whitehorn kd0aij@gmail.com wrote:

I believe so. My PR addressed the case with GPS valid:
#5388 (comment)

On Mon, Oct 10, 2016 at 2:45 PM Julian Oes notifications@github.com
wrote:

@kd0aij https://github.com/kd0aij Ok I started debugging this with a
Pixracer and a FrSky D4RII receiver and the first thing of master I tested
was:
Flying in manual without GPS connected and switching off RC. It
successfully enters "Land" mode and lowers overall PWM trying to descend.
At some point it switches to min PWM and detects landed.
Does this fit with your observation?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#5388 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACMZPQixkuz254gnE9IBNYJF5xkhNWhzks5qyqPkgaJpZM4Jtn_m
.

@julianoes
Copy link
Contributor

Ok, I'll test with GPS next up. Unfortunately, I don't have the cable to wire the GPS up with the Pixracer. I'll have to do the test with a Pixhawk.

@julianoes
Copy link
Contributor

@kd0aij: This fixes some problems with RC loss failsafe when in manual modes on fmu-v4, but may break other things since it's not clear to me how it was intended to work.

Which behaviour did you want and test?? RTL or all props off?

@kd0aij
Copy link
Contributor Author

kd0aij commented Oct 11, 2016

In non-assisted modes, this PR ensures that flight_termination state is entered, and that results in "lockdown", which is all motors stopped. No RTL.

@kd0aij
Copy link
Contributor Author

kd0aij commented Oct 11, 2016

The remaining open question on this fix is how to repair the commander logic which sets flags.gps_failure. Since this is never set if there is no GPS module (one known problem case), there is currently no way to configure RC loss to cause motor shutdown (termination). My proposed fix works, but precludes a controlled descent on RC loss. Of course at this point (line 2703 of commander.cpp) we're already in termination state, not land or descend...

Also, even if GPS were available, would it be wise to execute a powered descent, given that we have no idea what is below the vehicle?

@julianoes
Copy link
Contributor

Also, even if GPS were available, would it be wise to execute a powered descent, given that we have no idea what is below the vehicle?

According to the commander state machine that's what we want to do. And I think it's what most other drones would do as well. If a drone comes down slowly it gives people the chance to step away. If it falls like a brick that's harder.

I think I now understood that the RC loss issue you're addressing, is for the case where you want termination for racers. I think this is not properly implemented (and you're fixes might be the missing piece). I'll look closer at this case but I need to find the correct params first. As you said, documentation is hard to find about this 😢.

@PX4BuildBot
Copy link
Collaborator

Can one of the admins verify this patch?

@LorenzMeier
Copy link
Member

Jenkins add to whitelist.

@LorenzMeier
Copy link
Member

@kd0aij Can you rebase?

@kd0aij
Copy link
Contributor Author

kd0aij commented Nov 11, 2016

The remaining open question on this fix is how to repair the commander logic which sets flags.gps_failure. Since this is never set if there is no GPS module (one known problem case), there is currently no way to configure RC loss to cause motor shutdown (termination). My proposed fix works, but precludes a controlled descent on RC loss. Of course at this point (line 2703 of commander.cpp) we're already in termination state, not land or descend...

And for FPV racers, RC loss MUST result in motor shutdown, if I understand FAI rules correctly.

@gregd72002
Copy link
Contributor

I would also like to see motor shutdown option on RC loss +1

@kd0aij
Copy link
Contributor Author

kd0aij commented Nov 11, 2016

retesting: enters "LAND" mode on RC loss (Pixracer, no GPS, FrSky RX)
Required a mod to commander to force lockdown in addition to failsafe (not sure what the force_failsafe flag accomplishes).
Note that the default value for CBRK_FLIGHTTERM disables this behavior.

Needs testing on fmu-v2/v3

@kd0aij
Copy link
Contributor Author

kd0aij commented Nov 11, 2016

Jenkins test this please

@kd0aij
Copy link
Contributor Author

kd0aij commented Nov 13, 2016

@julianoes What do you think of this as an interim workaround for motor shutdown? If flight termination is disabled, the copter should just switch to LAND mode without shutting down the motors.

@kd0aij
Copy link
Contributor Author

kd0aij commented Dec 17, 2016

superseded by #5863 ?

@kd0aij kd0aij closed this Dec 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants