refactor(filesystem): Only re-init after filesystem contents are correct

Author: amilcarlucas

ardupilot_methodic_configurator/__main__.py

@@ -371,7 +371,9 @@ def initialize_filesystem(state: ApplicationState) -> None:
 
     # Write parameter default values if available
     if state.param_default_values:
-        state.param_default_values_dirty = state.local_filesystem.write_param_default_values(state.param_default_values)
+        state.param_default_values_dirty = state.local_filesystem.set_param_default_values_if_different(
+            state.param_default_values
+        )
 
 
 def initialize_flight_controller_and_filesystem(state: ApplicationState) -> None:

ardupilot_methodic_configurator/backend_filesystem.py

@@ -984,17 +984,27 @@ def merge_forced_or_derived_parameters(
                 dest[param_name] = param
         return at_least_one_param_changed
 
-    def write_param_default_values(self, param_default_values: ParDict) -> bool:
+    def set_param_default_values_if_different(self, param_default_values: ParDict) -> bool:
+        """
+        Set the default parameter values if they are different from the current ones.
+
+        Args:    param_default_values: A ParDict containing the default parameter values to compare against the current ones.
+        Returns: bool: True if the default parameter values were updated, False if they were the same as the current ones.
+        """
         param_default_values = ParDict(dict(sorted(param_default_values.items())))
         if self.param_default_dict != param_default_values:
             self.param_default_dict = param_default_values
             return True
         return False
 
-    def write_param_default_values_to_file(self, param_default_values: ParDict, filename: str = "00_default.param") -> None:
-        if self.write_param_default_values(param_default_values):
-            self.file_parameters[filename] = param_default_values
-            self.param_default_dict.export_to_param(os_path.join(self.vehicle_dir, filename))
+    def write_param_default_values_to_file(
+        self, param_default_values: ParDict, filename: str = "00_default.param", vehicle_dir: str = ""
+    ) -> None:
+        if self.set_param_default_values_if_different(param_default_values):
+            # self.file_parameters[filename] = param_default_values
+            if len(vehicle_dir) == 0:
+                vehicle_dir = self.vehicle_dir
+            self.param_default_dict.export_to_param(os_path.join(vehicle_dir, filename))
 
     @staticmethod
     def _safe_path_join(base_dir: str, untrusted_path: str) -> str:

ardupilot_methodic_configurator/data_model_vehicle_project.py

@@ -198,17 +198,13 @@ def create_new_vehicle_from_bin_log(self, bin_file: str) -> str:
             fc_parameters=fc_parameters,
         )
 
-        # Point the filesystem at the new vehicle directory before any reads or writes.
-        # write_param_default_values_to_file() and compound_params() rely on vehicle_dir,
-        # so re_init() must be called here to avoid accidentally operating on the previously-open project.
-        # Set fw_version first so re_init() does not override it with the template's placeholder version.
+        # Update flight controller firmware version in the vehicle_components.json file
         self._local_filesystem.fw_version = fw_version
-        self._local_filesystem.re_init(new_path, vehicle_type)
-        # Persist the correct firmware version and type into vehicle_components.json so subsequent
-        # re_init calls (and the user when they inspect the project) see the actual recorded firmware.
         self._local_filesystem.set_fc_fw_version_and_type_in_components_json(fw_version, vehicle_type, new_path)
 
-        self._local_filesystem.write_param_default_values_to_file(default_params)
+        # replace the "00_default.param" from the template with the .bin log-extracted default parameter values
+        self._local_filesystem.write_param_default_values_to_file(default_params, "00_default.param", new_path)
+        self._local_filesystem.re_init(new_path, vehicle_type)
 
         # Build the baseline from log-extracted defaults plus compounded AMC step files.
         # This avoids exporting params that merely match 00_default.param but are absent

tests/test__main__.py

@@ -354,7 +354,7 @@ def test_user_can_initialize_with_connected_hardware(
             mock_fc_info = MagicMock()
             mock_fc_info.get_param_default_values.return_value = default_params
             mock_fc_window.return_value = mock_fc_info
-            mock_local_filesystem.write_param_default_values.return_value = True
+            mock_local_filesystem.set_param_default_values_if_different.return_value = True
 
             # Act: User initializes with connected hardware
             initialize_flight_controller_and_filesystem(application_state)
@@ -364,6 +364,7 @@ def test_user_can_initialize_with_connected_hardware(
             assert application_state.vehicle_type == "ArduCopter"
             assert application_state.local_filesystem is mock_local_filesystem
             assert application_state.param_default_values == default_params
+            mock_local_filesystem.set_param_default_values_if_different.assert_called_once_with(default_params)
             assert application_state.param_default_values_dirty is True
 
     def test_user_can_work_in_simulation_mode(self, application_state: ApplicationState) -> None:

tests/test_backend_filesystem.py

@@ -580,19 +580,19 @@ def test_tolerance_check_with_zero_values(self) -> None:
             assert result == test_filename
             mock_file.assert_called_once_with(expected_path, encoding="utf-8")
 
-    def test_write_param_default_values(self) -> None:
+    def test_set_param_default_values_if_different(self) -> None:
         lfs = LocalFilesystem(
             "vehicle_dir", "vehicle_type", None, allow_editing_template_files=False, save_component_to_system_templates=False
         )
 
         # Test with new values
         new_values = {"PARAM1": MagicMock(value=1.0)}
-        result = lfs.write_param_default_values(new_values)
+        result = lfs.set_param_default_values_if_different(new_values)
         assert result is True
         assert lfs.param_default_dict == new_values
 
         # Test with same values (no change)
-        result = lfs.write_param_default_values(new_values)
+        result = lfs.set_param_default_values_if_different(new_values)
         assert result is False
 
     def test_get_start_file(self) -> None:

tests/test_data_model_vehicle_project.py

@@ -1081,7 +1081,7 @@ def test_user_can_create_project_from_bin_log_successfully(self) -> None:
         # Assert: vehicle directory opened immediately after creation
         mock_open.assert_called_once_with("/vehicles/my_flight")
         # Assert: extracted defaults written (replacing the template's 00_default.param)
-        mock_write.assert_called_once_with(fake_defaults)
+        mock_write.assert_called_once_with(fake_defaults, "00_default.param", "/vehicles/my_flight")
 
     def test_bin_log_defaults_are_written_to_vehicle_directory(self) -> None:
         """
@@ -1116,7 +1116,7 @@ def test_bin_log_defaults_are_written_to_vehicle_directory(self) -> None:
             manager.create_new_vehicle_from_bin_log("/logs/flight.bin")
 
         # Assert: the extracted defaults — not the template's — are written
-        mock_write.assert_called_once_with(fake_defaults)
+        mock_write.assert_called_once_with(fake_defaults, "00_default.param", "/vehicles/flight")
 
     def test_missing_params_exported_to_import_file(self) -> None:
         """