diff --git a/ardupilot_methodic_configurator/configuration_manager.py b/ardupilot_methodic_configurator/configuration_manager.py index 25a12747b..d9b4b88df 100644 --- a/ardupilot_methodic_configurator/configuration_manager.py +++ b/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) diff --git a/ardupilot_methodic_configurator/data_model_ardupilot_parameter.py b/ardupilot_methodic_configurator/data_model_ardupilot_parameter.py index 9d00cbb20..6375f6795 100644 --- a/ardupilot_methodic_configurator/data_model_ardupilot_parameter.py +++ b/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.""" diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py index cfca60ebe..0e3d853c1 100755 --- a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py +++ b/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() diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py index 5b957ca22..c26d32719 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py +++ b/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)) diff --git a/tests/test_frontend_tkinter_parameter_editor_table.py b/tests/test_frontend_tkinter_parameter_editor_table.py index 47ef3b5df..55e74283f 100755 --- a/tests/test_frontend_tkinter_parameter_editor_table.py +++ b/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: """