add feedforward max_rate_limit and jitter_factor to msp for 4.3 #11017
Conversation
src/main/msp/msp.c
Outdated
| @@ -2836,6 +2843,14 @@ static mspResult_e mspProcessInCommand(mspDescriptor_t srcDesc, int16_t cmdMSP, | |||
| #else | |||
| sbufReadU8(src); | |||
| #endif | |||
| #if defined(USE_FEEDFORWARD) | |||
There was a problem hiding this comment.
Please move and add this logic to line 2819 (2826)
There was a problem hiding this comment.
Then the configurator and lua scripts and every other client have to be updated.
There was a problem hiding this comment.
But's it's within the same #if defined(USE_FEEDFORWARD) statement, but maybe I'm missing order of other settings. New statements should be added at the end of existing ones,
There was a problem hiding this comment.
The new parameters have to be added to the end of the MSP message. If they were added before thrust linear and vbat sag compensation, any client not aware of this change would end up changing max rate limit and jitter factor instead.
There was a problem hiding this comment.
I kind of agree with @haslinghuis here - the way MSP works is that it does so by only allowing incremental changes between versions - the above conditional for USE_FEEDFORWARD is targeting the same version as this, so I think we may as well keep it as clean as possible within the convention.
It is true that this will require some additional changes in clients, but in the end changes will be needed anyway for these clients to support 4.3, as clients implementing the intermediate state are not sending enough bytes to get the firmware to process this block.
|
The addition of feedforward_max_rate_limit is fine, but feedforward_jitter_factor was added for BF 4.3 and shouldn't be added to MSP until 4.4. I can see why @ctzsnooze wants to add this parameter early as it's a possibly better alternative to feedforward_transition. |
src/main/msp/msp.c
Outdated
| @@ -2836,6 +2843,14 @@ static mspResult_e mspProcessInCommand(mspDescriptor_t srcDesc, int16_t cmdMSP, | |||
| #else | |||
| sbufReadU8(src); | |||
| #endif | |||
| #if defined(USE_FEEDFORWARD) | |||
There was a problem hiding this comment.
I kind of agree with @haslinghuis here - the way MSP works is that it does so by only allowing incremental changes between versions - the above conditional for USE_FEEDFORWARD is targeting the same version as this, so I think we may as well keep it as clean as possible within the convention.
It is true that this will require some additional changes in clients, but in the end changes will be needed anyway for these clients to support 4.3, as clients implementing the intermediate state are not sending enough bytes to get the firmware to process this block.
|
I'm happy to put these new Feedforward parameters up into the one Both are easy enough to read, and there is less work for others if we leave it as is. @mikeller your call, I don't mind either way. |
|
@ctzsnooze Just put them in the same block. I'll get the lua scripts in sync again and you or @haslinghuis can do the configurator. 👍 |
ctzsnooze
dismissed stale reviews from klutvott123 and haslinghuis
via
d45653c
|
I've rebased to master, and put the changes into the one Please confirm it is done correctly, if so I'll squash to a single commit. |
ctzsnooze
force-pushed
the
include-feedforward-max_rate_limit-and-jitter_factor-in-MSP
branch
from
d45653c to
4e138a9
Compare
move params to one block, remove whitespace
ctzsnooze
force-pushed
the
include-feedforward-max_rate_limit-and-jitter_factor-in-MSP
branch
from
4e138a9 to
4a485b4
Compare
|
Checked and tested. |
ctzsnooze
deleted the
include-feedforward-max_rate_limit-and-jitter_factor-in-MSP
branch
Although
feedforward_max_rate_limitandfeedforward_ jitter_factorare new variables for 4.3, it would be great if they could be included in the MSP so we can set and review them in Configurator 1.44.In particular, the
feedforward_ jitter_factorparameter can, and should, be user editable without need to go to the CLI. Higher values give a strong 'transition-like' effect for freestyle at lower or Rx link rates, and with fast links, a value around 5 works better than defaults.feedforward_max_rate_limitaffects the onset time of the pull-back in feedforward when the sticks are rapidly heading towards max, eg at the start of a flip or roll. A value of 90 is default. On heavier setups with a stronger tendency to overshoot, a lower value, eg 85, may work better, and on a highly tuned racer, 95 may be better.