Add feedforward to OSD PID element preview #2208
Conversation
src/js/tabs/osd.js
Outdated
| const i = pidDefaults[axis * 5 + 1].toString().padStart(3); | ||
| const d = pidDefaults[axis * 5 + 2].toString().padStart(3); | ||
| const f = pidDefaults[axis * 5 + 4].toString().padStart(3); | ||
| if (semver.lt(FC.CONFIG.apiVersion, "1.44.0")) { |
There was a problem hiding this comment.
| if (semver.lt(FC.CONFIG.apiVersion, "1.44.0")) { | |
| if (semver.lt(FC.CONFIG.apiVersion, API_VERSION_1_44) { |
There was a problem hiding this comment.
I'm not really sure how that's an improvement. There are 54 occurrences of a version check in just this file alone and only 6 of them use the suggested constant style. It seems to me that this adds unnecessary fragmentation. If a follow-on PR wants to address this and change to a consistent style throughout the code then that's fine with me, but out of scope for this PR.
There was a problem hiding this comment.
The constant was introduced with API_VERSION_1_43
There was a problem hiding this comment.
We are adding the constants to any new code. I can do later a PR to change all of the old occurrences, but if we are adding new code is better to add the constant at this PR.
There was a problem hiding this comment.
It's just avoidance of magic numbers - in general they are bad coding style.
etracer65
force-pushed
the
update_osd_pid_element_preview
branch
from
b02f629 to
9c48acb
Compare
Also change the element preview to use the default PIDs for the firmware version (as defined in the Configurator) so that the display is more representative of the actual values (and not display 20 for D on yaw!).
etracer65
force-pushed
the
update_osd_pid_element_preview
branch
from
9c48acb to
ed292a9
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
4 participants
Also change the element preview to use the default PIDs for the firmware version (as defined in the Configurator) so that the display is more representative of the actual values (and not display 20 for D on yaw!).
Related to betaflight/betaflight#10257