feat(motor): implement GUI and tests for motor swap

Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>

Author: OmkarSarkar204

ardupilot_methodic_configurator/data_model_motor_test.py

@@ -457,6 +457,58 @@ def set_parameter(  # pylint: disable=too-many-arguments, too-many-positional-ar
             logging_error(error_msg)
             raise ParameterError(error_msg) from e
 
+    def swap_motor_functions(self, expected_label: str, detected_label: str) -> None:
+        """
+        Swap the SERVOx_FUNCTION parameters between two motors.
+
+        Args:
+            expected_label: The letter of the motor that was tested (e.g., 'A').
+            detected_label: The letter of the motor that actually spun (e.g., 'C').
+
+        Raises:
+            ValidationError: If the labels are invalid.
+            ParameterError: If reading or writing the parameters fails.
+
+        """
+        if expected_label == detected_label:
+            return  # Nothing to swap
+
+        try:
+            # 1. Convert the letters (A, B, C...) into their test sequence index (0, 1, 2...)
+            expected_idx = self._motor_labels.index(expected_label)
+            detected_idx = self._motor_labels.index(detected_label)
+
+            # 2. Get the actual physical motor numbers (1, 2, 3, 4...)
+            expected_servo_num = self._motor_numbers[expected_idx]
+            detected_servo_num = self._motor_numbers[detected_idx]
+
+            # 3. Construct the parameter names (e.g., "SERVO1_FUNCTION", "SERVO3_FUNCTION")
+            expected_param = f"SERVO{expected_servo_num}_FUNCTION"
+            detected_param = f"SERVO{detected_servo_num}_FUNCTION"
+
+            # 4. Read the current values from the flight controller
+            expected_val = self.get_parameter(expected_param)
+            detected_val = self.get_parameter(detected_param)
+
+            if expected_val is None or detected_val is None:
+                raise ParameterError(
+                    _("Could not read parameters %(p1)s or %(p2)s from flight controller.")
+                    % {"p1": expected_param, "p2": detected_param}
+                )
+
+            # 5. Swap them
+            logging_info(
+                _("Swapping motor %(exp)s (%(p1)s) with motor %(det)s (%(p2)s)"),
+                {"exp": expected_label, "det": detected_label, "p1": expected_param, "p2": detected_param},
+            )
+
+            # Set one at a time
+            self.set_parameter(expected_param, detected_val)
+            self.set_parameter(detected_param, expected_val)
+
+        except ValueError as e:
+            raise ValidationError(_("Invalid motor labels provided for swapping.")) from e
+
     def get_parameter(self, param_name: str) -> Optional[float]:
         """
         Get a parameter value from the flight controller.

ardupilot_methodic_configurator/frontend_tkinter_motor_test.py

@@ -27,10 +27,10 @@
 from logging import error as logging_error
 from logging import info as logging_info
 from logging import warning as logging_warning
-from tkinter import Frame, Label, ttk
-from tkinter.messagebox import askyesno, showerror, showwarning
+from tkinter import Event, Frame, Label, ttk
+from tkinter.messagebox import askyesno, showerror, showinfo, showwarning
 from tkinter.simpledialog import askfloat
-from typing import Callable, Optional, Union
+from typing import Callable, Optional, Union, cast
 
 from ardupilot_methodic_configurator import _
 from ardupilot_methodic_configurator.__main__ import (
@@ -244,7 +244,18 @@ def _create_widgets(self) -> None:  # pylint: disable=too-many-statements # noqa
         ).pack(side="left", padx=5)
 
     def _create_motor_buttons(self, parent: Union[Frame, ttk.Frame]) -> None:
-        """Create the motor test buttons and detection comboboxes."""
+        """
+        Create per-motor test controls and configure detection selection handling.
+
+        For each configured motor, this builds the test button, motor information label,
+        detection combobox, and status label. It also binds each detection combobox's
+        selection event to `_on_combobox_selected` so swap/detection UI updates are
+        handled when the user changes a detected motor assignment.
+
+        Args:
+            parent: The container that will receive the per-motor control frames.
+
+        """
         motor_labels = self.model.motor_labels
         motor_numbers = self.model.motor_numbers
         motor_directions = self.model.motor_directions
@@ -274,12 +285,65 @@ def make_test_command(test_sequence_nr: int, motor_output_nr: int) -> Callable[[
             combo = ttk.Combobox(motor_frame, values=self.model.motor_labels, width=5)
             combo.pack()
             self.detected_comboboxes.append(combo)
+            # Bind the selection event to the new handler
+            combo.bind("<<ComboboxSelected>>", self._on_combobox_selected)
 
             # Add status label for visual feedback
             status_label = ttk.Label(motor_frame, text=_("Ready"), foreground="blue")
             status_label.pack()
             self.motor_status_labels.append(status_label)
 
+    def _on_combobox_selected(self, event: Event) -> None:
+        """Handle combobox selection by looking up the triggering widget."""
+        # Cast the generic widget to a ttk.Combobox so the type checker knows it has a .get() method
+        combobox = cast("ttk.Combobox", event.widget)
+
+        # Find which row this combobox belongs to in our saved list
+        if combobox in self.detected_comboboxes:
+            index = self.detected_comboboxes.index(combobox)
+            expected_label = self.model.motor_labels[index]
+            selected_label = combobox.get()
+
+            # Pass the data to the swap logic
+            self._on_motor_swapped(expected_label, selected_label)
+
+    def _on_motor_swapped(self, expected_label: str, detected_label: str) -> None:
+        """Handle the user selecting a different motor from the dropdown."""
+        if expected_label == detected_label:
+            # If they select 'A' for motor 'A', just clear the box and idle
+            for cb in self.detected_comboboxes:
+                cb.set("")
+            return
+
+        # Ask for confirmation
+        confirm = askyesno(
+            _("Swapping Motors?"),
+            _("Are you sure you want to swap the functions of Motor %(exp)s and Motor %(det)s?")
+            % {"exp": expected_label, "det": detected_label},
+        )
+
+        if not confirm:
+            for cb in self.detected_comboboxes:
+                cb.set("")
+            return
+
+        try:
+            self.model.swap_motor_functions(expected_label, detected_label)
+
+            showinfo(
+                _("Success"),
+                _("Motor %(exp)s and Motor %(det)s have been successfully swapped.")
+                % {"exp": expected_label, "det": detected_label},
+            )
+
+        except (ParameterError, ValidationError) as e:
+            showerror(_("Swap Failed"), str(e))
+
+        finally:
+            # To make the UI looks clean always clear the comboboxes after the action is done
+            for cb in self.detected_comboboxes:
+                cb.set("")
+
     def _update_view(self) -> None:
         """Update the view with data from the model."""
         # Update diagram only when needed (not every second)

tests/test_data_model_motor_test.py

@@ -3267,3 +3267,86 @@ def test_frame_options_include_all_defined_classes(self, motor_test_model) -> No
         assert "Y6" in frame_options
         assert "SCRIPTINGMATRIX" in frame_options
         assert len(frame_options) == 3
+
+
+class TestMotorTestDataModelMotorSwapping:
+    """Test motor parameter swapping functionality."""
+
+    def test_swap_motor_functions_exits_early_when_labels_match(self, motor_test_model) -> None:
+        """
+        Swapping a motor with itself does nothing.
+
+        GIVEN: A request to swap motor 'A' with 'A'
+        WHEN: Calling swap_motor_functions
+        THEN: Returns immediately without reading or writing parameters
+        """
+        # Arrange: set parameter to ensure they aren't called
+        with (
+            patch.object(motor_test_model, "get_parameter") as mock_get,
+            patch.object(motor_test_model, "set_parameter") as mock_set,
+        ):
+            # Act: Attempt to swap
+            motor_test_model.swap_motor_functions("A", "A")
+
+            # Assert: No backend interactions occurred
+            mock_get.assert_not_called()
+            mock_set.assert_not_called()
+
+    def test_swap_motor_functions_swaps_parameters_successfully(self, motor_test_model) -> None:
+        """
+        Successfully swaps SERVO_FUNCTION parameters for two valid motors.
+
+        GIVEN: A request to swap motor 'A' (SERVO1) and 'C' (SERVO3)
+        WHEN: Calling swap_motor_functions and the FC returns valid data
+        THEN: The parameters are read and written back swapped
+        """
+
+        # Arrange: Set up mock parameter reading
+        def mock_get_param(param_name: str) -> Optional[float]:
+            if param_name == "SERVO1_FUNCTION":
+                return 33.0  # e.g, Motor 1 output function code
+            if param_name == "SERVO3_FUNCTION":
+                return 35.0  # e.g, Motor 3 output function code
+            return None
+
+        with (
+            patch.object(motor_test_model, "get_parameter", side_effect=mock_get_param) as mock_get,
+            patch.object(motor_test_model, "set_parameter") as mock_set,
+        ):
+            # Act: Execute swap
+            motor_test_model.swap_motor_functions("A", "C")
+
+            # Assert: Verify correct parameters were read
+            mock_get.assert_any_call("SERVO1_FUNCTION")
+            mock_get.assert_any_call("SERVO3_FUNCTION")
+
+            # Assert: Verify parameters were written back inverted
+            mock_set.assert_any_call("SERVO1_FUNCTION", 35.0)
+            mock_set.assert_any_call("SERVO3_FUNCTION", 33.0)
+            assert mock_set.call_count == 2
+
+    def test_swap_motor_functions_raises_validation_error_for_invalid_labels(self, motor_test_model) -> None:
+        """
+        Raises ValidationError when an invalid motor label is provided.
+
+        GIVEN: An invalid motor label not in the current frame
+        WHEN: Calling swap_motor_functions
+        THEN: Raises ValidationError preventing the swap
+        """
+        with pytest.raises(ValidationError, match="Invalid motor labels provided for swapping"):
+            motor_test_model.swap_motor_functions("A", "Z")
+
+    def test_swap_motor_functions_raises_parameter_error_on_read_failure(self, motor_test_model) -> None:
+        """
+        Raises ParameterError when failing to read existing parameter values.
+
+        GIVEN: Valid labels but the flight controller returns None for a parameter
+        WHEN: Calling swap_motor_functions
+        THEN: Raises ParameterError indicating the read failure
+        """
+        # Arrange, Act & Assert: Combine the mock and the exception check into one statement
+        with (
+            patch.object(motor_test_model, "get_parameter", return_value=None),
+            pytest.raises(ParameterError, match="Could not read parameters"),
+        ):
+            motor_test_model.swap_motor_functions("A", "B")

tests/test_frontend_tkinter_motor_test.py

@@ -13,7 +13,7 @@
 from __future__ import annotations
 
 from importlib import import_module
-from tkinter import ttk
+from tkinter import Event, ttk
 from types import SimpleNamespace
 from typing import TYPE_CHECKING, Any, Callable, cast  # pylint: disable=unused-import
 from unittest.mock import MagicMock, patch
@@ -834,3 +834,111 @@ def register(self, name: str, creator: Callable) -> None:
             assert dummy_factory.calls
         finally:
             module_with_attr.plugin_factory = original_factory
+
+
+class TestMotorTestViewMotorSwapping:
+    """Test the frontend GUI logic specifically for motor swapping."""
+
+    def test_on_combobox_selected_routes_correctly(self, motor_view: MotorTestView, mocker) -> None:
+        """
+        Combobox selection routes the expected and detected labels to the swap logic.
+
+        GIVEN: A combobox selection event triggered by the first combobox
+        WHEN: _on_combobox_selected is called
+        THEN: It looks up the correct labels and routes data to _on_motor_swapped
+        """
+        cb = motor_view.detected_comboboxes[0]
+        cb.set("B")
+
+        mock_event = MagicMock(spec=Event)
+        mock_event.widget = cb
+
+        swap_spy = mocker.patch.object(motor_view, "_on_motor_swapped")
+
+        # Act
+        motor_view._on_combobox_selected(mock_event)
+
+        swap_spy.assert_called_once_with("A", "B")
+
+    def test_on_motor_swapped_early_exit(
+        self, motor_view: MotorTestView, fake_model: FakeMotorTestModel, dialog_spies: SimpleNamespace
+    ) -> None:
+        """
+        Swapping a motor with itself exits early without prompting.
+
+        GIVEN: A swap request where expected and detected labels are identical
+        WHEN: _on_motor_swapped is called
+        THEN: It clears the comboboxes and returns immediately without prompting the user
+        """
+        fake_model.swap_motor_functions = MagicMock()
+        motor_view.detected_comboboxes[0].set("A")
+
+        motor_view._on_motor_swapped("A", "A")
+
+        dialog_spies.askyesno.assert_not_called()
+        fake_model.swap_motor_functions.assert_not_called()
+        assert motor_view.detected_comboboxes[0].get() == ""
+
+    def test_on_motor_swapped_cancelled_by_user(
+        self, motor_view: MotorTestView, fake_model: FakeMotorTestModel, dialog_spies: SimpleNamespace
+    ) -> None:
+        """
+        Swap requests cancelled by the user reset the UI safely.
+
+        GIVEN: A valid swap request
+        WHEN: The user clicks 'No' on the confirmation dialog
+        THEN: It clears the comboboxes and aborts the swap
+        """
+        dialog_spies.askyesno.return_value = False
+        fake_model.swap_motor_functions = MagicMock()
+        motor_view.detected_comboboxes[0].set("B")
+
+        motor_view._on_motor_swapped("A", "B")
+
+        dialog_spies.askyesno.assert_called_once()
+        fake_model.swap_motor_functions.assert_not_called()
+        assert motor_view.detected_comboboxes[0].get() == ""
+
+    def test_on_motor_swapped_success(
+        self, motor_view: MotorTestView, fake_model: FakeMotorTestModel, dialog_spies: SimpleNamespace, mocker
+    ) -> None:
+        """
+        Successful swap requests apply parameters and show a success dialog.
+
+        GIVEN: A valid swap request confirmed by the user
+        WHEN: The data model successfully swaps the parameters
+        THEN: A success info dialog is shown and comboboxes are cleared
+        """
+        dialog_spies.askyesno.return_value = True
+        mock_showinfo = mocker.patch("ardupilot_methodic_configurator.frontend_tkinter_motor_test.showinfo")
+        fake_model.swap_motor_functions = MagicMock()
+        motor_view.detected_comboboxes[0].set("B")
+
+        motor_view._on_motor_swapped("A", "B")
+
+        fake_model.swap_motor_functions.assert_called_once_with("A", "B")
+        mock_showinfo.assert_called_once()
+        assert motor_view.detected_comboboxes[0].get() == ""
+
+    def test_on_motor_swapped_handles_exception(
+        self, motor_view: MotorTestView, fake_model: FakeMotorTestModel, dialog_spies: SimpleNamespace
+    ) -> None:
+        """
+        Swap requests that fail at the backend show an error dialog.
+
+        GIVEN: A valid swap request confirmed by the user
+        WHEN: The data model raises an exception during the swap
+        THEN: An error dialog is shown and comboboxes are cleared
+        """
+        dialog_spies.askyesno.return_value = True
+        fake_model.swap_motor_functions = MagicMock(side_effect=ParameterError("Mock FC read error"))
+        motor_view.detected_comboboxes[0].set("B")
+
+        motor_view._on_motor_swapped("A", "B")
+
+        fake_model.swap_motor_functions.assert_called_once_with("A", "B")
+        dialog_spies.showerror.assert_called_once()
+
+        # Verify the actual error message was passed to the UI
+        assert "Mock FC read error" in dialog_spies.showerror.call_args[0][1]
+        assert motor_view.detected_comboboxes[0].get() == ""