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

Add support for unsetting parameters (track default values again) #26075

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

Conversation

peterbarker
Copy link
Contributor

@peterbarker peterbarker commented Jan 24, 2024

The general concept is to go through all of the same things we do when saving values, but instead of updating the value reset the key to a flag value. Once done searching for that parameter in storage will fail. The type is NOT changed so when scanning we know the size of the variable.

I'm quite aware this leaks storage. Future improvements would have us remember suitable empty slots for each type we might store and use them when storing. This is a major complication, IMO.

  • need to add a sanity check so we never store under the key (we don't have one for the sentinel value!)
  • valgrind
  • check Vector3 still works appropriately

@peterbarker
Copy link
Contributor Author

Message PR is here: ArduPilot/mavlink#350

Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

let's do a new COMMAND_INT which says to "unset" all values that are currently in storage and equal to the default

@tridge tridge removed the DevCallEU label Jan 24, 2024
@peterbarker
Copy link
Contributor Author

I've discovered this is an absolutely terrible idea.

If we remove from storage a value which is at its default value and there exists in storage a value from an old, unused storage key which has some conversion code then we may execute that conversion code again, leading to a non-default value.

We need to purge from storage old, unused keys from storage. Which is probably impossible as we don't know what scripts are going to do, and when.

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

Successfully merging this pull request may close these issues.

2 participants