Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preset parameter visualizer #2886

Closed
MythicLionMan opened this issue Apr 9, 2022 · 6 comments
Closed

Preset parameter visualizer #2886

MythicLionMan opened this issue Apr 9, 2022 · 6 comments

Comments

@MythicLionMan
Copy link

Is your feature request related to a problem? Please describe

Presets can interact in complex ways. Because a preset can overwrite a parameter that was set by another preset the order in which presets are applied is significant, as well as complex. Understanding these relationships can be complicated for a user when configuring presets, and for an author who is creating a preset. This feature aims to make it more clear exactly how multiple presets interact with one another. It also allows the preset queue to be visualized and edited.

For example, consider a simple setup that includes the default_rpm_clean filter preset and the SupaflyFPV_Freestyle_5_Inch_EasyTune tune preset. If we examine the lifecycle of the 'dshot_bidir' parameter we can see the following:

default_rpm_clean includes presets/4.3/filters/defaults.txt which sets dshot_bidir to OFF
default_rpm_clean sets dshot_bidir to ON

SupaflyFPV_Freestyle_5_Inch_EasyTune includes presets/4.3/filters/defaults.txt which sets dshot_bidir to OFF
SupaflyFPV_Freestyle_5_Inch_EasyTune sets dshot_bidir to ON or OFF based on a user supplied option.

Applying default_rpm_clean before SupaflyFPV_Freestyle_5_Inch_EasyTune will result in dshot_bidir being configured as specified by the user, but applying them in the opposite order will not, and the reason why isn't completely clear. In this case the complexity is resolved by having a convention that filter presets are always applied before tune presets, but it is quite easy to inadvertently create a preset that has an assumption about other parameters.

Describe the solution you'd like

I'm proposing a table that lists parameters as rows and presets as columns. Each row tracks the lifecycle of a parameter as different presets are applied. This is inspired by the 'Build Settings > Levels' view in Xcode (and is a feature I'd love to have in Cura).

The table would have a column for each 'root level' preset in the preset queue. The columns would be ordered the way that the queue would apply them. The left most column would be the flight controller default for a parameter. The rightmost column would be the final value of the parameter after applying the preset. The options list would appear at the top of each preset column to allow it to be customized.

Initially each preset would have a single column. More detail could be obtained by 'expanding' a column to display the presets that were included to make up the root preset that was selected by the user. When a column is expanded it would display columns for the settings defined by the 'parent' preset, interspersed with columns for the included presets. Included columns could be expanded as well to determine exactly why a value is changing.

To simplify the table there would be no need to display a row for a parameter that is only ever assigned the default value (though if the value changes to a different value and then back to the default it would be useful to display it to indicate why the value is still the default).

The preset options menu would appear above each root preset column, so that changes to the preset options could be inspected live. Options to delete a staged preset as well as to re-order the order of the presets by dragging the columns would also be useful stretch goals.

The table could either be permanently positioned at the bottom of the presets tab, updating as the user made choices, or it could appear in a popup to confirm changes before "Save and Reboot". An example UI would look something like this:

Screen Shot 2022-04-09 at 12 59 35 AM

One interesting observation about this example: it looks like the Supafly tune is resetting the simplified_dterm_filter_multiplier that is setup by default_rpm_clean. Whether this is intentional or not, I didn't notice it until I made this table. As a user my expectation was that all filter settings would be supplied by default_rpm_clean unless Supafly specifically overrode one. But in this case the default value has been restored without Supafly setting a new custom value specific to its tune, so the value set by default_rpm_clean is 'lost' without a replacement.

Describe alternatives you've considered

The "Show CLI…" and "View Online…" buttons in the preset info dialog are the closest approximation to this functionality. Neither of these options makes it easy to determine what the value of a parameter will end up being after applying a preset let alone how it gets to that value. This proposal essentially provides a diff between each stage in the configuration and the last, and only shows one row for each parameter.

I also considered not having a full table but having a list of all parameters modified by the preset queue, and clicking on each one would return a list of the transformations applied to the value. This would be easier to implement, but I believe that it would be more difficult to visualize.

Other information

Algorithm

One possible algorithm to compute the diff is to create difference lists for each parameter and a collapse index for columns. The difference lists are linked lists of different values for each parameter, and include the values for all columns collapsed or not. The collapse index indicates which columns are collapsed when a column is not expanded. Either the parent column is displayed, or the list of collapsed columns, but not both.

int currentColumnID = 0;
struct ColumnValue {
    Object value;
    int columnID;
    ColumnValue nextColumn;
}
Dictionary<ParameterName, ColumnValue> parameters;
Dictionary<ParameterName, ColumnValue> latestValues;
Dictionary<int, int> collapsesUntil;

To generate the lists, iterate through each preset line by line, respecting the user supplied options. Each time that a new preset is opened (or included from another preset) increase the currentColumnID and recurse into the included file. When recursing pass in currentColumnID as parentColumnID. When the recursion ends increase currentColumnID again and set the ColumnValue of all parameters with parentColumnID to the most recent value for the parameter (this will be the value displayed when the child columns are collapsed). Set collapseUntil[ParentColumnID] = currentColumnID to define how the columns are collapsed.

Whenever a property set command is encountered retrieve the most recent value from latestValues. If the parameter is not found get the default value for the parameter and store it as columnID 0 in parameters. If the existing ColumnValue has the same ID then update the value. Otherwise create a new ColumnValue, link it to the existing value, and store it in latestValues.

When complete the parameters dictionary will have the values needed to populate a table of substitutions and collapseUntil will define which columns should be displayed based on the users collapsing input. Initially all columns would be collapsed, but they could be expanded to reveal more detail.

An example using the dshot_bidir parameter from the table above:

columnID 0 1 2 3 4 5 6
dshot_bidir Default:Off default_rpm_clean:On default:Off default_rpm_clean:On SupaflyFPV:On default:Off SupaflyFPV:On
collapseUntil {
    1 => 3,
    4 => 6
}

Column 1 is the parent of columns 2 - 3, so either column 1 is displayed or 2 and 3 are displayed.
Likewise column 4 is the parent of columns 5 - 6, so either column 4 is displayed or 5 and 6 are displayed.
(In more complicated examples the collapsing will continue deeper).

Other Proposals

Pitch #2806 proposes a wizard or guide that suggests preset categories to make the initial setup easier. This isn't really related to the issues that this proposal is trying to solve, but the proposed UI is similar enough that it bears considering if both proposals were to be pursued. It might make sense to implement one view that could do both jobs and could be configured differently for different locations.

Pitch #2867 proposes using existing Betaflight parameters to configure optional blocks in presets. This introduces a problem, since if preset A performs some work conditional on parameter X, and later another preset B changes the value of X, then the configuration done by A is incorrect. The difference system could track reads as well as writes, and if a read were ever followed by a write it could highlight the parameter in red to indicate that it is being used inconsistently.

@github-actions
Copy link
Contributor

This issue 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.

@haslinghuis
Copy link
Member

ping @limonspb

@github-actions github-actions bot removed the Inactive label May 11, 2022
@github-actions
Copy link
Contributor

This issue 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.

@limonspb
Copy link
Member

limonspb commented Jun 14, 2022

Sorry for the late response. Now we are in BF 4.4 and 10.9 development so can consider new features.

Basic Tune and Filter presets are wiping the previous tune to defaults before applying the actual preset.
That's the purpose of #$ INCLUDE: ....defaults.txt in the presets code.

It means when applying two TUNE presets one after another, only the second preset will matter.
This is simple and made that way so that people could try different presets without being worried about which preset they tried previously.

It becomes a lil more complicated with FIlter + Tune presets. Because Filters can be included in TUNE or may not depends on what the author wanted. But either way it's still made for the users that don't want to dig through parameters, but rather just click try and fly.

If someone wants to track a specific parameter in the preset, I guess he is an advanced user and can open the presets code and see what's happening.

Making comprehensive parameters tables would be cool, but really benefits only for a few people out there.
But if someone wants to implement that without overwhelming the basic interface, I'm ok with that.

@github-actions github-actions bot removed the Inactive label Jun 15, 2022
@github-actions
Copy link
Contributor

This issue 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.

@github-actions
Copy link
Contributor

Issue closed automatically as inactive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants