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

New setting for disabling motor_stop in NAV modes #3349

Merged
merged 1 commit into from Jul 15, 2018

Conversation

Projects
None yet
10 participants
@digitalentity
Copy link
Member

commented Jun 10, 2018

As discussed in #3195
@fiam @DzikuVx @shellixyz @ABLomas please review and comment

@digitalentity digitalentity added this to the 2.0 milestone Jun 10, 2018

@fiam

This comment has been minimized.

Copy link
Member

commented Jun 10, 2018

Seems OK to me as long as motor stop is ignored by navigation modes by default.

@DzikuVx

This comment has been minimized.

Copy link
Member

commented Jun 14, 2018

TBH I find this setting, as well as a MOTOR_STOP feature a harmful one. It should not be configurable and behavior should be hardcoded.

  1. For MR there should be no MOTOR_STOP. Never. Armed = spinning. Chance of someone putting a hand somewhere thinking it's disabled but it is not and accidently raising a throttle (happens when TX is hanging on a strap around a neck) is quite a big one!
  2. For FW, MOTOR_STOP should be always enabled but NAV mechanism is allowed to use it only during landing phase. Only. User is not allowed to override it in any way. We can not control stall.

From my perspective, every other combination is risky and might be potentialy harmful.

@shellixyz

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2018

I agree. MOTOR_STOP is not really useful for MRs and can be dangerous. For fixed wing I don't see any advantage not using MOTOR_STOP so it can always be enabled.

@digitalentity digitalentity force-pushed the de_nav_motor_stop branch from 9f0e53f to 1354cce Jun 30, 2018

@digitalentity

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

Rebased

@K-Suhas-Bharadwaj

This comment has been minimized.

Copy link

commented Jun 30, 2018

@digitalentity

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

@K-Suhas-Bharadwaj sorry, this is not a school - checkout the code and get yourself familiar with it. Read open issues, make meaningful comments, submit some patches and you would be warmly welcomed to the community.

@OlivierC-FR

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2018

Its automated spam , ban the fucker !

@digitalentity

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

@OlivierC-FR let's not judge by a single comment 😆

@ABLomas

This comment has been minimized.

Copy link

commented Jul 15, 2018

Because this didn't made to 2.0, we can try to refine logic and do everything correctly.
I believe, that nav_overrides_motor_stop should accept 3 values in total:

  • OFF - current, default, logic (use throttle as needed by autopilot, ignore battery/alt/anything)
  • MANUAL - do not use motor if user initiated RTH using switch (like current patch does) - if mode was activated with throttle low
  • ALLWAYS - do not use motor in RTH modes (Switch activated or failsafe)

I made simple logic flowchart:
setting_flowchart

@digitalentity

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2018

Did anybody actually test this PR?

@ABLomas

This comment has been minimized.

Copy link

commented Jul 15, 2018

Yes, i flew several times, AFAIK others too, so smth like 10 flights or so.
Current logic allow glide with motor off in RTH mode, but on failsafe (TX OFF) INAV still uses throttle, no matter what throttle value is.

@digitalentity

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2018

Ok, thanks @ABLomas. Since it's in 2.0 milestone and works as implemented, I'm merging this (it will be included in 2.0 final). We can improve it in 2.1.

@digitalentity

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2018

@ABLomas can you open a new issue with your suggestion and diagram?

@digitalentity digitalentity merged commit 6cc3d29 into development Jul 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@digitalentity digitalentity deleted the de_nav_motor_stop branch Jul 15, 2018

@ABLomas

This comment has been minimized.

Copy link

commented Jul 15, 2018

Sure, thanks!

@romanas

This comment has been minimized.

Copy link

commented Jul 15, 2018

Great, im in! new RTH with 0 thrt works great, with new option it will just safe some wings!

@Saxin

This comment has been minimized.

Copy link

commented Sep 16, 2018

@digitalentity Had esc failure yesterday while 2.6 km out and 600 m alt., glided back best I could - around 800 m glide... at 10 m alt. lost video and engaged rth (swith) found the plane 100 meters off last gps cordinate headed directly to home as expected...

I wasn't sure if I should have engaged rth when discovered esc loss and how this option could affect rth without avaliable thrust - would appriciate your comments

@rk-tipro

This comment has been minimized.

Copy link

commented Sep 16, 2018

if wind is fine, wing will try to go straight home descending by 45° or try to keep the alt if hot wings accrues. rth with 0 throttle is my best settings (another is cruise). if right settings FS wont kick in with rth zero throttle. and wing will just try to go home, if it balanced well it will glide, and wont stall.

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.