refactor(parameter editor): do not unnecessarily reset the FC (#1348)

No need to reset the FC if SID_AXIS changes, only when it changes from 0 to something else

Signed-off-by: Amilcar Lucas <amilcar.lucas@iav.de>

Author: amilcarlucas

ardupilot_methodic_configurator/data_model_parameter_editor.py

@@ -711,7 +711,12 @@ def upload_parameters_that_require_reset_workflow(  # pylint: disable=too-many-l
                             logging_info(_("Parameter %s changed to %f, reset required"), param_name, param.value)
                         reset_required = True
                     # Check if any of the selected parameters have a _TYPE, _EN, or _ENABLE suffix
-                    elif param_name.endswith(("_TYPE", "_EN", "_ENABLE", "SID_AXIS")):
+                    # SID_AXIS only requires a possible reset when changing from 0 to a non-zero value
+                    elif param_name.endswith(("_TYPE", "_EN", "_ENABLE")) or (
+                        param_name == "SID_AXIS"
+                        and float(self._flight_controller.fc_parameters.get("SID_AXIS", 0.0)) == 0.0
+                        and float(param.value) != 0.0
+                    ):
                         success, error_msg = self._flight_controller.set_param(param_name, float(param.value))
                         if not success:
                             logging_error(_("Failed to set parameter %s: %s"), param_name, error_msg)

tests/test_data_model_parameter_editor.py

@@ -602,10 +602,124 @@ def test_user_handles_parameter_upload_errors_gracefully(self, parameter_editor)
         # Assert: Errors handled via callback
         assert reset_required is False  # No successful uploads
         assert len(uploaded_params) == 0  # No parameters were uploaded
+        mock_ask_confirmation.assert_not_called()
         mock_show_error.assert_called_once()
         error_call_args = mock_show_error.call_args[0]
         assert "Failed to set parameter" in error_call_args[1]  # Second argument is the error message
 
+    def test_user_uploads_sid_axis_from_zero_to_nonzero_triggers_possible_reset(self, parameter_editor) -> None:
+        """
+        Uploading SID_AXIS from 0 to a non-zero value triggers a possible reset confirmation.
+
+        GIVEN: SID_AXIS is currently 0 on the flight controller
+        WHEN: The user uploads SID_AXIS with a new non-zero value
+        THEN: A possible reset should be triggered (parameter added to reset_unsure_params)
+        """
+        # Arrange: SID_AXIS currently 0 on FC, new value is non-zero
+        selected_params = {"SID_AXIS": Par(1.0, "Sys ID axis")}
+        parameter_editor._flight_controller.fc_parameters = {"SID_AXIS": 0}
+        parameter_editor._local_filesystem.doc_dict = {"SID_AXIS": {"RebootRequired": False}}
+        parameter_editor._reset_and_reconnect_flight_controller = MagicMock(return_value=None)
+
+        mock_ask_confirmation = MagicMock(return_value=True)
+        mock_show_error = MagicMock()
+
+        # Act
+        reset_happened, uploaded_params = parameter_editor.upload_parameters_that_require_reset_workflow(
+            selected_params, mock_ask_confirmation, mock_show_error
+        )
+
+        # Assert: Possible reset triggered and parameter uploaded
+        assert reset_happened is True
+        assert "SID_AXIS" in uploaded_params
+        mock_ask_confirmation.assert_called_once()
+        mock_show_error.assert_not_called()
+
+    def test_user_uploads_sid_axis_from_nonzero_to_different_nonzero_skips_reset(self, parameter_editor) -> None:
+        """
+        Uploading SID_AXIS from one non-zero value to another does not trigger a reset.
+
+        GIVEN: SID_AXIS is currently non-zero on the flight controller
+        WHEN: The user uploads SID_AXIS with a different non-zero value
+        THEN: No possible reset should be triggered
+        """
+        # Arrange: SID_AXIS currently 1 on FC, new value is 2
+        selected_params = {"SID_AXIS": Par(2.0, "Sys ID axis")}
+        parameter_editor._flight_controller.fc_parameters = {"SID_AXIS": 1.0}
+        parameter_editor._local_filesystem.doc_dict = {"SID_AXIS": {"RebootRequired": False}}
+        parameter_editor._reset_and_reconnect_flight_controller = MagicMock(return_value=None)
+
+        mock_ask_confirmation = MagicMock(return_value=False)
+        mock_show_error = MagicMock()
+
+        # Act
+        reset_happened, uploaded_params = parameter_editor.upload_parameters_that_require_reset_workflow(
+            selected_params, mock_ask_confirmation, mock_show_error
+        )
+
+        # Assert: No reset triggered; SID_AXIS is NOT in uploaded_params because it fell through
+        # to the normal upload path (not handled by this workflow)
+        assert reset_happened is False
+        assert "SID_AXIS" not in uploaded_params
+        mock_ask_confirmation.assert_not_called()
+        mock_show_error.assert_not_called()
+
+    def test_user_uploads_sid_axis_from_nonzero_to_zero_skips_reset(self, parameter_editor) -> None:
+        """
+        Uploading SID_AXIS from a non-zero value to 0 does not trigger a reset.
+
+        GIVEN: SID_AXIS is currently non-zero on the flight controller
+        WHEN: The user uploads SID_AXIS with value 0
+        THEN: No possible reset should be triggered
+        """
+        # Arrange: SID_AXIS currently 1 on FC, new value is 0
+        selected_params = {"SID_AXIS": Par(0.0, "Sys ID axis")}
+        parameter_editor._flight_controller.fc_parameters = {"SID_AXIS": 1.0}
+        parameter_editor._local_filesystem.doc_dict = {"SID_AXIS": {"RebootRequired": False}}
+        parameter_editor._reset_and_reconnect_flight_controller = MagicMock(return_value=None)
+
+        mock_ask_confirmation = MagicMock(return_value=False)
+        mock_show_error = MagicMock()
+
+        # Act
+        reset_happened, uploaded_params = parameter_editor.upload_parameters_that_require_reset_workflow(
+            selected_params, mock_ask_confirmation, mock_show_error
+        )
+
+        # Assert: No reset triggered
+        assert reset_happened is False
+        assert "SID_AXIS" not in uploaded_params
+        mock_ask_confirmation.assert_not_called()
+        mock_show_error.assert_not_called()
+
+    def test_user_uploads_sid_axis_absent_from_fc_to_nonzero_triggers_possible_reset(self, parameter_editor) -> None:
+        """
+        Uploading SID_AXIS to a non-zero value when it is absent from FC (treated as 0) triggers a possible reset.
+
+        GIVEN: SID_AXIS is not present in the flight controller parameter cache
+        WHEN: The user uploads SID_AXIS with a non-zero value
+        THEN: A possible reset should be triggered (absent value defaults to 0)
+        """
+        # Arrange: SID_AXIS not in FC params (defaults to 0), new value is non-zero
+        selected_params = {"SID_AXIS": Par(3.0, "Sys ID axis")}
+        parameter_editor._flight_controller.fc_parameters = {}
+        parameter_editor._local_filesystem.doc_dict = {"SID_AXIS": {"RebootRequired": False}}
+        parameter_editor._reset_and_reconnect_flight_controller = MagicMock(return_value=None)
+
+        mock_ask_confirmation = MagicMock(return_value=True)
+        mock_show_error = MagicMock()
+
+        # Act
+        reset_happened, uploaded_params = parameter_editor.upload_parameters_that_require_reset_workflow(
+            selected_params, mock_ask_confirmation, mock_show_error
+        )
+
+        # Assert: Possible reset triggered and parameter uploaded
+        assert reset_happened is True
+        assert "SID_AXIS" in uploaded_params
+        mock_ask_confirmation.assert_called_once()
+        mock_show_error.assert_not_called()
+
 
 class TestFileDownloadUrlWorkflows:
     """Test file download URL workflow business logic methods with callback injection."""