[LAUNCH] Fix a bug with throttling up in launch mode before throttle is raised

[digitalentity]

Fixes #6312, also some code style changes.
@dragnea, PTAL, this is what I ended up yesterday. Tested, working as intended.

[dragnea]

@digitalentity yes. Thanks, not my daily coding style, so I will try to follow the inav style in the future. Regarding the fix, it's exactly what I've done here. b78ab30 and later I had a proposal in this discussion #6312 to force with min_command. But as you wish if you want to spin the motors at low throttle in launch mode. Now the users will update the inav and will end up with a throttle idle in autolaunch with low throttle. We must make them aware of that to avoid propeller injuries, or perhaps change the default for the nav_overrides_motor_stop to OFF.

[digitalentity]

nav_overrides_motor_stop default to OFF was deemed unsafe. If a pilot was gliding at zero throttle and hits a failsafe - the airplane is doomed, hence the safer default.

Aha, gotcha. In the 2.5.2 implementation we didn't set the throttle value at all at zero throttle - therefore actual stick value was used: https://github.com/iNavFlight/inav/blob/release_2.5.2/src/main/navigation/navigation_fw_launch.c#L140-L160

So 2.6 will actually change the behavior in this case. Indeed, it makes sense then to force min_throttle.

PTAL at fd73511

[stronnag]

So 2.6 will actually change the behavior in this case. Indeed, it makes sense then to force min_throttle.

Added Rel Note flag for change of behaviour.

[digitalentity]

@stronnag fd73511 will actually revert to the old behavior:

MOTOR_STOP on:

• Motor stays stopped until launch is detected regardless of the stick position
• Motor goes to launch_throttle after launch is detected

MOTOR_STOP off:

• Motor is idling at low RPM until throttle stick is raised
• Motor spins up to launch_idle when the stick is raised and stays there until launch is detected
• Motor goes to launch_throttle after launch is detected

[dragnea]

In the 2.5.2 implementation we didn't set the throttle value at all at zero throttle - therefore actual stick value was used

Exactly. What's odd, I've tested last night and the motor it spins even with MOTOR_STOP ON while using

ENABLE_STATE(NAV_MOTOR_STOP_OR_IDLE); 
rcCommand[THROTTLE] = getThrottleIdleValue();

I usually arm with autolaunch while the wing is on the ground, perhaps others doing the same. I don't think fd73511 will fix the 2.6 behaviour, maybe just if you set nav_overrides_motor_stop = off

[digitalentity]

Tested for both MOTOR_STOP on and off. Default and non-default nav_fw_launch_idle_thr, works as described in #6319 (comment)

[0crap]

OK, I tried to follow all this, but now it's a bit of a mess in my head.

I have motor stop is ON, on my wing.
I have launch mode permanently enabled with the switch in the configurator.
When I arm the wing it's always on the ground with throttle stick low, it now starts beeping at me, I pick up the wing and raise the throttle so it uses "set nav_fw_launch_idle_thr = 1255", I toss the wing and the motor spins up automagically to get it climbing, when autolaunch is finished it uses the current throttle stick value from my radio.

Is this (iNav v2.5) behaviour still the same after this PR?

Relevant settings I use:
feature MOTOR_STOP
feature FW_LAUNCH
set nav_fw_launch_thr = 1600
set nav_fw_launch_idle_thr = 1255

[digitalentity]

@0crap yes, the behavior would be this (just verified):

• Arm the wing on the ground with throttle stick low
• It starts beeping at you. Beeping is a bit different than in 2.5 to warn you that the throttle is low
• You pick up the wing and raise the throttle stick
• Motor starts spinning at nav_fw_launch_idle_thr = 1255. The beeping changes to a standard 2.5 launch beeping
• You toss the wing and the motor spins up automagically to get it climbing
• When autolaunch is finished it uses the current throttle stick value from the radio

All this discussion was to get the behavior back to what users are used to in 2.5 and earlier.

[dragnea]

As I tested yesterday,using MOTOR_STOP ON will still spin the motor at throttle idle. The hex is made with your bug fix

[dragnea]

Autolaunch always on, Motor_stop on, Low Throttle, Arm -> motor is at idle

Here's the diff: https://pastebin.com/zDtTR8cD

[0crap]

@digitalentity Awesome, it works great as it is in 2.5
The extra beeps are welcome, makes sure you don't forget to raise throttle a bit before the toss.
(Otherwise it just nosedives into the ground and nothing will happen. Yep did happen to me as well.)

[0crap]

Autolaunch always on, Motor_stop on, Low Throttle, Arm -> motor is at idle

I expect on Low Throttle and Arm, the motor is NOT spinning, only the buzzer is beeping.
Then raise the throttle it starts spinning slowly when nav_fw_launch_idle_thr = 1255 is used.
Then "full" power after the toss.

[dragnea]

Exactly. The motor should not spin with low throttle and armed, but it seems it does, even with MOTOR_STOP set.
The following code it seems that will spin the motor at idle, so it's not good anymore in 2.6:
ENABLE_STATE(NAV_MOTOR_STOP_OR_IDLE); rcCommand[THROTTLE] = getThrottleIdleValue();

[0crap]

If that is really the case I'm sure it will be fixed.
Otherwise a lot of fixed wing users will see props spinning unexpected, which is a safety risk.
(For your precious body parts but also for the ESC/Motor, My wing is in this case always on the ground.
I pick it up just before I raise the throttle.)

[b14ckyy]

Just as reminder: if nav_overrides_motor_stop is set to OFF, the motor will NOT spin when armed and throttle low + autolaunch active. Beep comes.
if nav_overrides_motor_stop is set to ALL_NAV or AUTO_ONLY, then the motor WILL spin when armed and throttle low + autolaunch active. Beep comes (unwanted behavior)

[0crap]

Interesting, I'm not aware of that setting, so it is at it's default value.
nav_overrides_motor_stop = ALL_NAV
This is a unwanted change vs iNav 2.5 behaviour if you asked me.
(Safety risk!)

[dragnea]

@0crap I had a proposal to fix this but it was closed by @digitalentity #6313 (it may be other solutions, but this works). It can cause injuries or damages for people with a fresh 2.6 and also for many of us, we can't arm a wing on the ground
@b14ckyy I know, but ALL_NAV is good to have, so we have to fix it on the Autolaunch side

[0crap]

Whatever the devs decide, please make the Autolaunch DEFAULT behavior like in 2.5
Just to prevent any safety risks.

[digitalentity]

Autolaunch always on, Motor_stop on, Low Throttle, Arm -> motor is at idle
From your diff:

# INAV/MATEKF722SE 2.6.0 Nov 19 2020 / 13:55:32 (02e1fb8d)

02e1fb8 does not belong to this branch. You used some other firmware build where this fix is NOT applied yet.

[digitalentity]

@0crap, it appears @dragnea did his tests on a firmware which doesn't include fixes from this pull request.

I've tested this several times - see #6319 (comment) and #6319 (comment) (yes, I've actually tested your relevant settings).

[digitalentity]

My test (used settings from #6319 (comment)) tweaked for OMNIBUSF4 board I'm using to test this. Settings diff for folks who're interested in comparing - https://pastebin.com/NbMxEyFh

Armed, throttle stick at zero:

Armed, throttle stick at 50%:

After a toss:

[dragnea]

02e1fb8 does not belong to this branch

@digitalentity this is another branch which is already merged and closed. No more work to do here. There was another branch #6313 where I've fixed the issue from #6312. The build I've made and used in my comment #6319 (comment) is with your changes from the actual pull request. Appearently I have throttle idle with it. Please, let me to do another test and capture what happens

[dragnea]

it appears @dragnea did his tests on a firmware which doesn't include fixes from this pull request.

Actually it includes this pull request

[digitalentity]

> git log

commit fd73511053ca9edd6502a6a6d60b16d23af7628e (HEAD -> de_launch_fixes, origin/de_launch_fixes)
Author: Konstantin (DigitalEntity) Sharlaimov <konstantin.sharlaimov@gmail.com>
Date:   Thu Nov 19 09:55:08 2020 +0100

    Force mixer idle in launch mode while throttle stick is low

commit a3e7e3cb15ea85cc38cb946c059410a0cc526490
Author: Konstantin (DigitalEntity) Sharlaimov <konstantin.sharlaimov@gmail.com>
Date:   Thu Nov 19 08:44:30 2020 +0100

    [LAUNCH] Some readability refactoring; Fix bug with throttling up at zero throttle

commit 02e1fb8d1380cc116fa06fb9d15a595611864e3c (origin/master, origin/HEAD, master)
Merge: 1a490f4d4 7df04133b
Author: Konstantin Sharlaimov <konstantin.sharlaimov@gmail.com>
Date:   Thu Nov 19 09:09:07 2020 +0100

    Merge pull request #6318 from dragnea/autolaunch_no_control_during_finish
    
    No control during Autolaunch FINISHING sequence

Your firmware was built from 02e1fb8 which is two commits behind this PR. Please re-build and re-test.

[dragnea]

ah I did quick test and I copy-pasted the autolaunch file source :D from here https://github.com/iNavFlight/inav/blob/de_launch_fixes/src/main/navigation/navigation_fw_launch.c just to test

[b14ckyy]

@digitalentity as nav-overrides_motor_stop is not in your diff it seems to be default ALL_ON. So your setup is as mine and it works as expected. Looking good.
If someone can build be a F405SE or F722WPX or F411WSE target I can test tomorrow. I'm not at home in a few minutes.

[digitalentity]

@b14ckyy yes, my test has nav_overrides_motor_stop = ALL. Here's the F405SE firmware: inav_2.6.0_MATEKF405SE.zip

[digitalentity]

Just FYI - the same firmware for F405SE was tested on my wing (on the bench as it's raining at the moment)

[b14ckyy]

@b14ckyy yes, my test has nav_overrides_motor_stop = ALL. Here's the F405SE firmware: inav_2.6.0_MATEKF405SE.zip

That was fast. Bench testing now with different settings as soon as I have GPS fix. It's raining here too and it is pitch black night :D Will flight test RC-2 on the weeken dif the weather allows it.

[digitalentity]

Be aware that RC2 doesn't include this fix. Better to use custom builds from this branch.

[b14ckyy]

Be aware that RC2 doesn't include this fix. Better to use custom builds from this branch.

yeah I mean RC2 with the fix :D
One quick question: should the I-Term be active before the throw detect in Autolaunch? If I remember correctly and even demonstrated in a video, I-Term should only kick in when throw is detected and plane is in the air. But right now it kicks in as soon as I raise the throttle stick.

But motor stops works as expected when stick is low.

[dragnea]

@digitalentity Please take a look at this video: https://www.youtube.com/watch?v=ZzWLTdqXtJU
Not sure why, but something's not right with on my side. The build is made with you implementation: https://github.com/iNavFlight/inav/blob/de_launch_fixes/src/main/navigation/navigation_fw_launch.c
After arming, and zero throttle, I have AutoThr at 12% in the OSD and in the Configurator it's 0%. When I raise the throttle, is not going from zero to nav_fw_idle, but it jumps directly from throttle_idle(12%) to nav_fw_idle. And it's obvious why:
rcCommand[THROTTLE] = scaleRangef(elapsedTimeMs, 0.0f, LAUNCH_MOTOR_IDLE_SPINUP_TIME, getThrottleIdleValue(), navConfig()->fw.launch_idle_throttle); because nav_overrides_motor_stop is forcing to idle, not zero given by the stick
Then at the end I shake de fc and the launch is activated as it should.

The build is made 15mins ago

[digitalentity]

@dragnea did you check the actual motor behavior? To me this looks like an OSD bug.

As for the jump - this is WAI. Motor just won't spin below min_throttle, so launch code correctly gradually raises the throttle from min_throttle (minimal possible value to get the motor spinning, you can think of it as <1% throttle) to launch_idle. Until we migrate the whole code from working in PWM to working in percentages we'll have to cope with this.

[b14ckyy]

That throttle behavior is fine. The PWM Output is between min check 1050-1850 by default. The outputs tab scales this from 0-100%. This is why your Idle throttle of 15% is shown as 12% in the output due to the scaling. That's totally fine. Maybe can be optimized in the Configurator but that's not a issue.

[dragnea]

I don't have the wing at my desk this week. I've flashed a quad fc. So the osd and configurator misled me. When I implemented the autolaunch, past months, at least the osd thr was the same with the wing motor(physical)... now different behaviour in every side and I know it's scaled, but I've thrusted them so well. Next week I'll have the chance to see my hangar and I will test it live. I'm still suspicious about some things...
Thank guys

[avsaase]

I just tested this on the bench, with launch mode permanently enabled, and it now works again as in 2.5. #6318 also works.

[0crap]

I just tested this on the bench, with launch mode permanently enabled, and it now works again as in 2.5. #6318 also works.

Awesome!
Did you leave below setting at the default? (ALL_NAV)
nav_overrides_motor_stop = ALL_NAV

[0crap]

I'm horrible at baking hex files, so when this PR is merged I'll grab one from here and test myself as well.

[stronnag]

I'm horrible at baking hex files, so when this PR is merged I'll grab one from here and test myself as well.

Funnily enough, that repo is built on de_launch_fixes just now ...

[0crap]

I'm horrible at baking hex files, so when this PR is merged I'll grab one from here and test myself as well.

Funnily enough, that repo is built on de_launch_fixes just now ...

To my understanding that can't be until this PR is merged into master, to trigger a new build cycle.

[stronnag]

I'm horrible at baking hex files, so when this PR is merged I'll grab one from here and test myself as well.

Funnily enough, that repo is built on de_launch_fixes just now ...

To my understanding that can't be until this PR is merged into master, to trigger a new build cycle.

I can build my personal hex set on whatever I wish, choosing when I wish to update it. That's why there's a git log there, so you can see what you're getting.

[0crap]

I'm horrible at baking hex files, so when this PR is merged I'll grab one from here and test myself as well.

Funnily enough, that repo is built on de_launch_fixes just now ...

To my understanding that can't be until this PR is merged into master, to trigger a new build cycle.

I can build my personal hex set on whatever I wish, choosing when I wish to update it. That's why there's a git log there, so you can see what you're getting.

Ah, you're running that site, now I see!
Thanks for the effort, use it a lot.
I'll grab a hex and flash my wing. :-)

[digitalentity]

Seems that all is WAI and tested a few times now. Merging.

[digitalentity]

Thanks everybody!

[0crap]

And tested on a MATEKF405 target with my "default" settings below.
Confimed it works as in 2.5
Thx!

# version
# INAV/MATEKF405 2.6.0 Nov 19 2020 / 17:10:17 (fd735110)
# GCC-10.2.0
feature MOTOR_STOP
feature FW_LAUNCH
set nav_fw_launch_thr = 1600
set nav_fw_launch_idle_thr = 1255
set nav_overrides_motor_stop = ALL_NAV

[avsaase]

Thanks for fixing this guys! Will this be RC3?

[0crap]

Thanks for fixing this guys! Will this be RC3?

It's merged and has the Milestone 2.6
So yes this will be in the next RC.