fix: confirm before saving derived param changes#1247
fix: confirm before saving derived param changes#1247ryanbasic1 wants to merge 5 commits intoArduPilot:masterfrom
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
This pull request adds user confirmation before saving automatically recalculated derived parameter changes to disk. Previously, the application would silently update parameter files when derived parameters were recalculated, which could lead to unexpected file modifications. This change ensures users are aware of and approve these modifications before they are persisted.
Changes:
- Adds a new
ask_yesno_messagedialog function following the established pattern for user interaction - Introduces an optional
ask_user_confirmationcallback parameter toupdate_and_export_vehicle_params_from_fc - Implements tracking and conditional prompting for derived parameter changes during parameter export
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ardupilot_methodic_configurator/frontend_tkinter_show.py | Adds ask_yesno_message function for yes/no confirmation dialogs, consistent with existing error and warning message functions |
| ardupilot_methodic_configurator/backend_filesystem.py | Adds derived parameter change tracking and optional user confirmation before file export, allowing users to decline saving derived parameter changes |
| ardupilot_methodic_configurator/main.py | Passes ask_yesno_message as the confirmation callback to ensure users are prompted when derived parameters change |
amilcarlucas
left a comment
amilcarlucas
left a comment
There was a problem hiding this comment.
Thanks, this looks really clean.
| eval_variables = self.get_eval_variables() | ||
| # the eval variables do not contain fc_parameter values | ||
| # and that is intentional, the fc_parameters are not to be used in here | ||
| derived_changed_by_file: dict[str, bool] = {} |
There was a problem hiding this comment.
We fully process one file after the other. I think there is no need for this dict.
derived_changed: bool = false
should be enough
There was a problem hiding this comment.
That makes sense, thanks.
I’ll replace it with a boolean flag.
|
replaced by #1262 |
Title :
Confirm before saving derived parameter changes
Summary
Prevents file writes from occurring without user awareness when derived parameters are recalculated.
Adds an optional confirmation callback to control disk write operations.
Prompts are shown only when derived values have actually changed.
Why
Automatic parameter corrections were creating saved changes to derived parameters without user approval.
This update ensures user consent before modifying .param files and prevents unintended writes.
How it works
Derived parameters are calculated during the update_and_export_vehicle_params_from_fc process.
When a file contains a derived value that has changed, the system calls ask_user_confirmation(title, message).
The user must confirm before the system writes the file.
If the user declines, the file is not created or modified.
If no confirmation callback is provided, the system continues operating with its existing behavior.