Updates to log viewer for 4.3

[ctzsnooze]

Adds log header support for the new dynamic notch, rc_smoothing, and feedforward fields in 4.3 after inclusion of firmware PR #10723

A 4.2 or earlier log will display values in the appropriate boxes where possible, or in the 'unknown fields' box at the bottom.

This shows how the feedforward part of a log header appears with a 4.3 log

And here how the dynamic notch values look for 4.3

And rcSmoothing:

This supersedes the individual PRs #510 and #511.

[McGiverGim]

I'm not too sure what you want to do: you have "removed" some code that lets old versions show the values in a correct manner, and only the new one is shown ok. Is difficult to me to follow the changes with all of this code removed...

We must get to a solution where 4.2 logs are shown, at least, as good as in the current blackbox version, and 4.3 logs are shown in a correct way.

The first to know, to fix that, is what are the differences between versions. Are the fields simply "renamed" or some of them have different values? I see some differences in the enumerations. Can you explain it here? I think I can point you to the correct changes then.

If some values have simply being renamed, you can "mark" this in the Blackbox here:

blackbox-log-viewer/js/flightlog_parser.js

Line 316 in 7da5b00

translationValues = {

In this way we "rename" some values from the header to the internal var, so you can make two different headers behave exactly the same.

And about the fielddefs definition, we have a method at the end where we "adapt" them to old/new versions:

blackbox-log-viewer/js/flightlog_fielddefs.js

Line 462 in 7da5b00

function adjustFieldDefsList(firmwareType, firmwareVersion) {

Now I think you understand better the nightmare of backwards compatibility, when you work only in the firmware part you are unaware of all of that...

[klutvott123]

I think we should wait with the dynamic notch changes. @KarateBrot says we're going back to using Q instead of bandwidth as there weren't any benefits to using constant bandwidth.
@KarateBrot What's the status on this?

[KarateBrot]

@klutvott123 Yes, thanks for your objection.

==> Bandwidth can be ignored in this PR <==

Everything else is fine. I need to get the dynamic notch PR ready asap (probably tomorrow) to inform everyone.
I talked with @asizon and it looks like it's no big deal to revert the change for the configurator as well.

---

Edit:
Here is the PR - betaflight/betaflight#10767

[ctzsnooze]

@McGiverGim Thanks for your advice! I have tried to do the right things and apologise if it is not done right. I checked that a 4.3 and a 4.2 log would open values in the right places, that is the limit of my testing.

The following fields are added in this PR. They are mostly new from 4.3, but some existed before and have not been added previously.

ff_transition
ff_averaging
ff_smooth_factor
ff_jitter_factor
ff_boost
ff_max_rate_limit

rc_smoothing_mode
rc_smoothing_feedforward_hz
rc_smoothing_setpoint_hz
rc_smoothing_auto_factor_setpoint
rc_smoothing_throttle_hz
rc_smoothing_auto_factor_throttle
rc_smoothing_active_cutoffs_ff_sp_thr

rpm_notch_lpf

dyn_notch_count

These fields are not used in 4.3, and have no specific translation as they aren't really updating a previous version.

rc_smoothing_auto_factor  (now two separate factors, one for throttle, one for set point)
rc_smoothing_type (not used at all in 4.3)
rc_smoothing_filter_type (not used at all in 4.3)
dyn_notch_width_percent (not used at all in 4.3.  We now have dyn_notch_count in a kind of analogous role)

The adjustFieldDefsList concept in flightlog_fielddefs.js seems really clever, but I don't know how to use it.

In 4.3 the enums for RC_SMOOTHING_TYPE, RC_SMOOTHING_INPUT_TYPE, RC_SMOOTHING_DERIVATIVE_TYPE and RC_INTERPOLATION are not required.

The enum DYN_NOTCH_RANGE hasn't been used for a while.

The enum for RC_SMOOTHING_MODE only needs to be OFF and ON, and the FILTER_TYPE member FIR hasn't been used in years, and we now will use PT2 and PT3.

I don't know how to do version control for those enums. Should the old ones be left there?

I've removed dyn_notch_bandwidth from the original proposal now we are going to stay with dyn_notch_q.

I attempted to resolve the Soundcloud 'bugs'.

Please let me know what I need to do now.

Thanks

[McGiverGim]

It seems something is wrong with this PR, the changes have been lost...
I will try to help you when you have the code up again.

[McGiverGim]

The base is wrong. A first review asking for changes in the basic, once we have this right, we can go for the rest.

[haslinghuis]

Commenting on code quality only

[ctzsnooze]

@haslinghuis - thanks for your suggestions and checks. I've done those whitespace tidy ups.
I'd really be grateful if this could be merged soon, it fixes issues for 4.2 users and is helpful for all 4.3 developers.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

No Coverage information
0.0% Duplication

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within a week.

[ctzsnooze]

Thanks @blckmn this process should be really good - I'm confused as to why github identifies PR as mergeable -> FAIL happens?

[haslinghuis]

@ctzsnooze first conditions fails as one of the others fail. We could suppress output in this case but helps identify what's holding the auto-merge process.

[mikeller]

@McGiverGim, @asizon: Are you ok with this, or are there still problems with backwards compatibility?

[asizon]

Looks really better now @mikeller

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

No Coverage information
0.0% Duplication

[ctzsnooze]

rebased and tested

Requested changes have been implemented

[ctzsnooze]

This is how the info screen looks for a 4.3 log:

[McGiverGim]

Why the transition has been removed? Betaflight 4.2 uses it if I'm not wrong...

[ctzsnooze]

Thanks for checking.
ptermSRateWeight is removed, but Transition is still there, and still shows up normally when looking at 4.2 logs, and I think 4.1 also.
The name has been changed from feedforward_transition to the camelCase form feedforwardTransition, to match the form of all the other feedforward parameters.

[blckmn]

AUTOMERGE: (FAIL)

• github identifies PR as mergeable -> PASS
• assigned to a milestone -> PASS
• cooling off period lapsed -> PASS
• commit count less or equal to three -> PASS
• Don't merge label NOT found -> PASS
• at least one RN: label found -> PASS
• Tested label found -> PASS
• assigned to an approver -> PASS
• approver count at least three -> FAIL