fix(param editor): Fix asking to save changes multiple times.

Author: amilcarlucas

ardupilot_methodic_configurator/configuration_manager.py

@@ -1418,10 +1418,11 @@ def export_current_file(self, annotate_doc: bool) -> None:
         # Update the filesystem's file_parameters to match what was saved
         self._local_filesystem.file_parameters[self.current_file] = export_params
 
-        # Note: We don't need to clear tracking sets or dirty flags on domain model parameters
-        # because after export, the user always navigates to a different file, which triggers
-        # repopulate_configuration_step_parameters() that clears tracking sets and rebuilds
-        # self.parameters fresh.
+        self._added_parameters.clear()
+        self._deleted_parameters.clear()
+        # copy parameters new values to their _values_on_file
+        for param in self.current_step_parameters.values():
+            param.copy_new_value_to_file()
 
     def open_documentation_in_browser(self, filename: str) -> None:
         _blog_text, blog_url = self.get_documentation_text_and_url("blog", filename)

ardupilot_methodic_configurator/data_model_ardupilot_parameter.py

@@ -478,6 +478,11 @@ def set_forced_or_derived_change_reason(self, change_reason: str) -> None:
 
         self._change_reason = change_reason
 
+    def copy_new_value_to_file(self) -> None:
+        """Copy the new value to the value on file, marking it as saved, resetting the is_dirty property."""
+        self._value_on_file = self._new_value
+        self._change_reason_on_file = self._change_reason
+
 
 class BitmaskHelper:
     """Helper class for working with ArduPilot parameter bitmasks."""

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py

@@ -318,7 +318,7 @@ def __create_parameter_area_widgets(self) -> None:
 
         # Create a Scrollable parameter editor table
         self.parameter_editor_table = ParameterEditorTable(self.main_frame, self.configuration_manager, self)
-        self.repopulate_parameter_table()
+        self.repopulate_parameter_table(regenerate_from_disk=True)
         self.parameter_editor_table.pack(side="top", fill="both", expand=True)
 
         # Create a frame for the buttons
@@ -654,7 +654,7 @@ def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: b
             self.configuration_manager.current_file = selected_file
             self.documentation_frame.refresh_documentation_labels()
             self.documentation_frame.update_why_why_now_tooltip()
-            self.repopulate_parameter_table()
+            self.repopulate_parameter_table(regenerate_from_disk=True)
             self._update_skip_button_state()
 
     def _update_progress_bar_from_file(self, selected_file: str) -> None:
@@ -676,14 +676,14 @@ def download_flight_controller_parameters(self, redownload: bool = False) -> Non
         if not redownload:
             self.on_param_file_combobox_change(None, forced=True)  # the initial param read will trigger a table update
 
-    def repopulate_parameter_table(self) -> None:
+    def repopulate_parameter_table(self, regenerate_from_disk: bool) -> None:
         if not self.configuration_manager.current_file:
             return  # no file was yet selected, so skip it
         # Re-populate the table with the new parameters
-        self.parameter_editor_table.repopulate(self.show_only_differences.get(), self.gui_complexity)
+        self.parameter_editor_table.repopulate(self.show_only_differences.get(), self.gui_complexity, regenerate_from_disk)
 
     def on_show_only_changed_checkbox_change(self) -> None:
-        self.repopulate_parameter_table()
+        self.repopulate_parameter_table(regenerate_from_disk=False)
 
     def on_upload_selected_click(self) -> None:
         self.write_changes_to_intermediate_parameter_file()

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py

@@ -127,7 +127,7 @@ def _create_headers_and_tooltips(self, show_upload_column: bool) -> tuple[tuple[
         return tuple(base_headers), tuple(base_tooltips)
 
     def repopulate(  # pylint: disable=too-many-locals
-        self, show_only_differences: bool, gui_complexity: str
+        self, show_only_differences: bool, gui_complexity: str, regenerate_from_disk: bool
     ) -> None:
         for widget in self.view_port.winfo_children():
             widget.destroy()
@@ -150,12 +150,13 @@ def repopulate(  # pylint: disable=too-many-locals
         self.upload_checkbutton_var = {}
 
         # Process configuration step and create domain model parameters
-        (ui_errors, ui_infos) = self.configuration_manager.repopulate_configuration_step_parameters()
+        if regenerate_from_disk:
+            (ui_errors, ui_infos) = self.configuration_manager.repopulate_configuration_step_parameters()
 
-        for title, msg in ui_errors:
-            messagebox.showerror(title, msg)
-        for title, msg in ui_infos:
-            messagebox.showinfo(title, msg)
+            for title, msg in ui_errors:
+                messagebox.showerror(title, msg)
+            for title, msg in ui_infos:
+                messagebox.showinfo(title, msg)
 
         if show_only_differences:
             # Filter to show only different parameters
@@ -718,7 +719,7 @@ def _on_parameter_delete(self, param_name: str) -> None:
 
             # Delete the parameter
             self.configuration_manager.delete_parameter_from_current_file(param_name)
-            self.parameter_editor.repopulate_parameter_table()
+            self.parameter_editor.repopulate_parameter_table(regenerate_from_disk=False)
 
             # Restore the scroll position
             self.canvas.yview_moveto(current_scroll_position)
@@ -769,7 +770,7 @@ def _confirm_parameter_addition(self, param_name: str) -> bool:
         try:
             if self.configuration_manager.add_parameter_to_current_file(param_name):
                 self._pending_scroll_to_bottom = True
-                self.parameter_editor.repopulate_parameter_table()
+                self.parameter_editor.repopulate_parameter_table(regenerate_from_disk=False)
                 return True
         except InvalidParameterNameError as exc:
             messagebox.showerror(_("Invalid parameter name."), str(exc))

tests/test_frontend_tkinter_parameter_editor_table.py

@@ -270,7 +270,7 @@ def test_repopulate_empty_parameters(parameter_editor_table: ParameterEditorTabl
     parameter_editor_table.configuration_manager.filesystem.file_parameters = ParDict({test_file: ParDict({})})
 
     # Act: Repopulate the table
-    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple")
+    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: No parameter rows were added
     parameter_editor_table.add_parameter_row.assert_not_called()
@@ -296,7 +296,7 @@ def test_repopulate_clears_existing_content(parameter_editor_table: ParameterEdi
     parameter_editor_table.configuration_manager.filesystem.param_default_dict = ParDict({"PARAM1": Par(0.0, "default")})
 
     # Act: Repopulate the table
-    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple")
+    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: Existing content was cleared
     assert not dummy_widget.winfo_exists()
@@ -318,7 +318,7 @@ def test_repopulate_handles_none_current_file(parameter_editor_table: ParameterE
     parameter_editor_table.configuration_manager.filesystem.param_default_dict = ParDict({})
 
     # Act: Attempt to repopulate with no current file
-    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple")
+    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: No parameter rows were added
     parameter_editor_table.add_parameter_row.assert_not_called()
@@ -344,7 +344,7 @@ def test_repopulate_single_parameter(parameter_editor_table: ParameterEditorTabl
 
     # Act: Repopulate with single parameter
     with patch.object(parameter_editor_table, "grid_slaves", return_value=[]):
-        parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple")
+        parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: Parameter row was added (implicitly tested through repopulate call)
 
@@ -387,7 +387,7 @@ def test_repopulate_multiple_parameters(parameter_editor_table: ParameterEditorT
 
     # Act: Repopulate with multiple parameters
     with patch.object(parameter_editor_table, "grid_slaves", return_value=[]):
-        parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple")
+        parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: All parameters were processed (implicitly tested through repopulate call)
 
@@ -418,7 +418,7 @@ def test_repopulate_preserves_checkbutton_states(parameter_editor_table: Paramet
     )
 
     # Act: Repopulate the table
-    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple")
+    parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: Checkbutton states were preserved (implicitly tested through repopulate call)
 
@@ -459,7 +459,7 @@ def test_repopulate_show_only_differences(parameter_editor_table: ParameterEdito
     )
 
     # Act: Repopulate showing only differences
-    parameter_editor_table.repopulate(show_only_differences=True, gui_complexity="simple")
+    parameter_editor_table.repopulate(show_only_differences=True, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: Only differing parameters were processed (implicitly tested through repopulate call)
 
@@ -485,7 +485,7 @@ def test_repopulate_uses_scroll_helper(parameter_editor_table: ParameterEditorTa
 
     # Act: Repopulate and check scroll behavior
     with patch.object(parameter_editor_table, "_apply_scroll_position") as mock_scroll:
-        parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple")
+        parameter_editor_table.repopulate(show_only_differences=False, gui_complexity="simple", regenerate_from_disk=False)
 
     # Assert: Scroll position was applied correctly
     mock_scroll.assert_called_once_with(pending_scroll)
@@ -918,7 +918,9 @@ def test_on_parameter_delete_confirmed(self, parameter_editor_table: ParameterEd
 
             # Assert: Parameter is deleted and table repopulated
             assert "TEST_PARAM" not in parameter_editor_table.configuration_manager.filesystem.file_parameters["test_file"]
-            parameter_editor_table.parameter_editor.repopulate_parameter_table.assert_called_once_with()
+            parameter_editor_table.parameter_editor.repopulate_parameter_table.assert_called_once_with(
+                regenerate_from_disk=False
+            )
 
     def test_on_parameter_delete_cancelled(self, parameter_editor_table: ParameterEditorTable) -> None:
         """
@@ -1650,7 +1652,7 @@ def test_user_can_add_parameter_to_configuration_file(self, parameter_editor_tab
         # Assert: Parameter addition was successful
         assert result is True
         parameter_editor_table.configuration_manager.add_parameter_to_current_file.assert_called_once_with("NEW_PARAM")
-        parameter_editor_table.parameter_editor.repopulate_parameter_table.assert_called_once()
+        parameter_editor_table.parameter_editor.repopulate_parameter_table.assert_called_once_with(regenerate_from_disk=False)
 
     def test_user_can_delete_parameter_from_configuration_file(self, parameter_editor_table: ParameterEditorTable) -> None:
         """
@@ -1674,7 +1676,9 @@ def test_user_can_delete_parameter_from_configuration_file(self, parameter_edito
             parameter_editor_table.configuration_manager.delete_parameter_from_current_file.assert_called_once_with(
                 "TEST_PARAM"
             )
-            parameter_editor_table.parameter_editor.repopulate_parameter_table.assert_called_once()
+            parameter_editor_table.parameter_editor.repopulate_parameter_table.assert_called_once_with(
+                regenerate_from_disk=False
+            )
 
     def test_user_cannot_delete_parameter_when_cancelled(self, parameter_editor_table: ParameterEditorTable) -> None:
         """