Autotune: Add switch to test gains after tune is complete #27754
Conversation
| update_gcs(AUTOTUNE_MESSAGE_TESTING_END); | ||
| break; | ||
| case RC_Channel::AuxSwitchPos::MIDDLE: | ||
| // Middle position is unused for now |
There was a problem hiding this comment.
This is where we could change to mode AutoTune if the tune has not been completed.
********* For Discussion *********
|
Confirmed that this showed a problem in master :) |
|
We decided to wait for @bnsgeyer to test this |
IamPete1
force-pushed
the
autotune_test_switch
branch
from
92b8ed3
to
2b1615f
Compare
There was a problem hiding this comment.
there is a bug in the ordering of the hal.util->set_soft_armed(false); and the copter.mode_autotune.save_tuning_gains(); as otherwise AP_Param will only save the first 30 gains as we queue parameters when soft_armed is true, and queue length is 30 - we probably get away with it for now, but if autotune changed 32 params, only 30 would save
randy has asked the fix to be in a separate PR, does need fixing
| const bool tune_complete_no_testing = !have_pilot_testing_command && in_autotune_mode; | ||
|
|
||
| if (tune_complete_no_testing || testing_tuned) { | ||
| save_tuning_gains(); |
There was a problem hiding this comment.
do you intend to save_tuning_gains() if switch is in lower position so we currently have the old gains loaded?
There was a problem hiding this comment.
If you have used the switch to test the gains and you have the switch in the "original gains" position then it should not save the tuned gains.
If you have not used the switch to test the gains and are in AutoTune mode then it will save the gains. The is should be the same as the current behaviour.
There was a problem hiding this comment.
I'm open to better suggestions but I could not think of anything else while preserving the current behaviour, we can't make it a requirement that everyone must setup the new switch to run autotune.
Now we can send aux functions from the gcs we can't really check if the user has setup a switch until they first trigger it.
There was a problem hiding this comment.
How about reverse the switch?
- low = new gains (test/save on disarm),
- high = original gains (discard)
There was a problem hiding this comment.
I'm not sure how that is less confusing, the pattern (there are one or two exceptions) for functions is that high is to take some action. In this case the action is the thing we have added namely the ability to test gains in any mode. Original gains and discarding in other modes is the existing behavior, it would be abit odd to require the user to flip a switch to high for that.
Don't forget that aux functions are edge triggered, it does not matter where the switch is until autotune has completed, only changes after that (and before you disarm) will do anything.
…leted successfully
…r disarm and aux function
IamPete1
force-pushed
the
autotune_test_switch
branch
from
2b1615f
to
caaf27e
Compare
This adds a switch allowing the user to test gains in any mode once the autotune has been completed. If the user has used the switch they can then disarm in any mode and the gains will be saved if they are active. If the user does not use the switch then they must disarm in autotune to save gains per the current behavior.
This allows easy testing of the tune in any mode and makes the procedure to save a little simpler.
The load test gains functions have been updated to ensure they use the same criteria as the saving of gains for the load. Currently they were not checking if a axis had completed, only that the axis was enabled.