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

Allow creation transformation functions over data lake variables #1706

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rafaellehmkuhl
Copy link
Member

@rafaellehmkuhl rafaellehmkuhl commented Feb 20, 2025

This PR adds a powerful new feature to Cockpit: being able to create transformation functions. What that means is that it basically allows the user to create new data lake variables based on existing ones.

Those functions can be as simple as summing two variables, or as complex as the user wants. Want if clauses? You have it. The transformation function is basically an eval over the expression input.

If the user does not include a return statement (as the video example below, where we use {{ var1 }} + {{ var2 }}, the system does it automatically for them. The user can have complex functions with their own returns as well.

Kapture.2025-02-20.at.16.25.55.mp4

If the expression fails to be evaluated (e.g.: math error, variable access error, etc), it will try again in 10ms, than 20ms, than 40, doubling on every sequential error, till each reaches 10 seconds. That heuristic was put there to prevent error spamming.

This functionality will be used with the Radcam joystick variables to create the final zoom and focus values to be sent to the autopilot.

Fix #1642

@rafaellehmkuhl rafaellehmkuhl force-pushed the allow-transforming-data-lake-variables branch from 4271164 to ed8ee45 Compare February 20, 2025 19:36
Copy link
Contributor

@ArturoManzoli ArturoManzoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome feature!

UI wise is good to change some stuff:
1- The save button is blue (probably as primary color). Change it to our white/transparent standard;
2- The idea of using the placeholder for some quick use guide is very nice! Add some usage examples for boolean and text variables;
3- The placeholder on the description could simply be "optional description" or something more direct. The current placeholder is too descriptive I think;
4- The click-to-copy button isn't working. This is important to have imo;

image

Code will be reviewed next

@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Feb 24, 2025
Copy link
Contributor

@ES-Alexander ES-Alexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It's confusing to have transforming functions in a separate menu to the data lake variable monitor, especially since the transforming functions are just compound variables
    • Can we merge them? I think they can even be in the same table
      • We could add a Source column (between ID and Type) that has icons to specify whether a variable is from MAVLink (logo), Cockpit internal (logo), Other (mdi-exit-run), or compound (mdi-database-settings-outline)
        • The compound ones could have the edit/delete icons included in the Source column, or we could use database-edit-outline as the icon and only allow deleting from inside the editor
        • In future clicking on an Other one could list the Actions and/or widgets that are known to set that variable, and possibly even jump to their configuration page
      • We could add a "New compound variable" button under the table
      • The ID column contents seem largely redundant with the Name (only humans are looking at the table)
        • we could regain some table width by removing the text for the IDs and just leaving the copy button
    • More generally, I feel like Joystick, Actions, Data Lake, and MAVLink belong together, separated from the rest of the settings stuff, because they're all related to things you can do with Cockpit - I think they can all go in the "Tools" menu, which would also balance the load with the "Settings" menu
      • It seems reasonable to have a shortcut from the MAVLink page to the Actions page, in case someone wants to manually send a message
  2. Having the variables accessible is a a great idea, but it's seemingly quite awkward to use at the moment - could we make it inline instead of a separate form field?
    • I'm thinking there could be an auto-complete dropdown where the cursor is, whenever it's at the end of a zero or more length set of valid ID characters (and searched using those characters), between either a pair of double curly braces, or an open double curly brace and the end of the input (e.g. when first typing)
      • This could support normal clicking to select and replace the currently typed characters with the selected variable's ID, and ideally also tab-completion
    • I would prefer for this to be implemented here, but it can be opened as an Issue and handled later if necessary, since it's polish / quality of life rather than fundamental to the feature

@rafaellehmkuhl
Copy link
Member Author

  1. It's confusing to have transforming functions in a separate menu to the data lake variable monitor, especially since the transforming functions are just compound variables

    • Can we merge them? I think they can even be in the same table

      • We could add a Source column (between ID and Type) that has icons to specify whether a variable is from MAVLink (logo), Cockpit internal (logo), Other (mdi-exit-run), or compound (mdi-database-settings-outline)

        • The compound ones could have the edit/delete icons included in the Source column, or we could use database-edit-outline as the icon and only allow deleting from inside the editor
        • In future clicking on an Other one could list the Actions and/or widgets that are known to set that variable, and possibly even jump to their configuration page
      • We could add a "New compound variable" button under the table

      • The ID column contents seem largely redundant with the Name (only humans are looking at the table)

        • we could regain some table width by removing the text for the IDs and just leaving the copy button
    • More generally, I feel like Joystick, Actions, Data Lake, and MAVLink belong together, separated from the rest of the settings stuff, because they're all related to things you can do with Cockpit - I think they can all go in the "Tools" menu, which would also balance the load with the "Settings" menu

      • It seems reasonable to have a shortcut from the MAVLink page to the Actions page, in case someone wants to manually send a message
  2. Having the variables accessible is a a great idea, but it's seemingly quite awkward to use at the moment - could we make it inline instead of a separate form field?

    • I'm thinking there could be an auto-complete dropdown where the cursor is, whenever it's at the end of a zero or more length set of valid ID characters (and searched using those characters), between either a pair of double curly braces, or an open double curly brace and the end of the input (e.g. when first typing)

      • This could support normal clicking to select and replace the currently typed characters with the selected variable's ID, and ideally also tab-completion
    • I would prefer for this to be implemented here, but it can be opened as an Issue and handled later if necessary, since it's polish / quality of life rather than fundamental to the feature

I liked almost all the suggestions, but I fear we can miss our deadline for the radcam support (this week) if we try to make this initial PR already the ideal one.

Let me check what can be done that does not involves too much extra work and I get back to you.

@rafaellehmkuhl rafaellehmkuhl force-pushed the allow-transforming-data-lake-variables branch 3 times, most recently from b6876c8 to dceec4d Compare February 24, 2025 22:05
A transforming function is a new DataLake variable in which the value is calculated based on a JavsScript expression, that can include other DataLake variables in it.
@rafaellehmkuhl rafaellehmkuhl force-pushed the allow-transforming-data-lake-variables branch from dceec4d to e07936d Compare February 24, 2025 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-needed Change needs to be documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow "transforming functions" on data-lake variables
3 participants