refactor(component editor): Use local functions instead of lamba functions

Author: amilcarlucas

ardupilot_methodic_configurator/frontend_tkinter_component_editor.py

@@ -18,7 +18,7 @@
 from logging import basicConfig as logging_basicConfig
 from logging import getLevelName as logging_getLevelName
 from tkinter import ttk
-from typing import Callable, Optional, Union
+from typing import Optional, Union
 
 from ardupilot_methodic_configurator import _, __version__
 from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem
@@ -94,20 +94,6 @@ def set_mcu_series(self, mcu: str) -> None:
             if mcu.upper() in ("STM32F4XX", "STM32F7XX", "STM32H7XX"):
                 self.data_model.schema.modify_schema_for_mcu_series(is_optional=True)
 
-    def set_vehicle_configuration_template(self, configuration_template: str) -> None:
-        """Set the configuration template name in the data."""
-        self.data_model.set_configuration_template(configuration_template)
-
-    def set_values_from_fc_parameters(self, fc_parameters: dict, doc: dict) -> None:
-        """
-        Process flight controller parameters and update the data model.
-
-        This delegates to the data model's process_fc_parameters method to handle
-        all the business logic of processing parameters.
-        """
-        # Delegate to the data model for parameter processing
-        self.data_model.process_fc_parameters(fc_parameters, doc)
-
     def update_component_protocol_combobox_entries(self, component_path: ComponentPath, connection_type: str) -> str:
         """Updates the Protocol combobox entries based on the selected component connection Type."""
         self.data_model.set_component_value(component_path, connection_type)
@@ -183,12 +169,15 @@ def add_entry_or_combobox(
         # Determine foreground color based on is_optional flag
         fg_color = "gray" if is_optional else "black"
 
+        def on_validate_combobox(event: tk.Event) -> bool:
+            return self._validate_combobox(event, path)
+
         if combobox_values:
             cb = ttk.Combobox(entry_frame, values=combobox_values, foreground=fg_color)
-            cb.bind("<FocusOut>", lambda event, path=path: self._validate_combobox(event, path))  # type: ignore[misc]
-            cb.bind("<KeyRelease>", lambda event, path=path: self._validate_combobox(event, path))  # type: ignore[misc]
-            cb.bind("<Return>", lambda event, path=path: self._validate_combobox(event, path))  # type: ignore[misc]
-            cb.bind("<ButtonRelease>", lambda event, path=path: self._validate_combobox(event, path))  # type: ignore[misc]
+            cb.bind("<FocusOut>", on_validate_combobox)
+            cb.bind("<KeyRelease>", on_validate_combobox)
+            cb.bind("<Return>", on_validate_combobox)
+            cb.bind("<ButtonRelease>", on_validate_combobox)
 
             # Prevent mouse wheel from changing value when dropdown is not open
             def handle_mousewheel(_event: tk.Event, widget: tk.Widget = cb) -> Optional[str]:
@@ -237,18 +226,15 @@ def dropdown_closed(_event: tk.Event, widget: tk.Widget = cb) -> None:
             return cb
 
         entry = ttk.Entry(entry_frame, foreground=fg_color)
-        update_if_valid_function = self.get_validate_function(entry, path)
-        entry.bind("<FocusOut>", update_if_valid_function)
-        entry.bind("<KeyRelease>", update_if_valid_function)
-        entry.bind("<Return>", update_if_valid_function)
-        entry.insert(0, str(value))
-        return entry
 
-    def get_validate_function(self, entry: ttk.Entry, path: ComponentPath) -> Union[Callable[[tk.Event], object], None]:
-        def validate_limits(event: tk.Event) -> bool:
-            return self.validate_entry_limits_ui(event, entry, path)
+        def on_validate_entry_limits_ui(event: tk.Event) -> bool:
+            return self._validate_entry_limits_ui(event, entry, path)
 
-        return validate_limits
+        entry.bind("<FocusOut>", on_validate_entry_limits_ui)
+        entry.bind("<KeyRelease>", on_validate_entry_limits_ui)
+        entry.bind("<Return>", on_validate_entry_limits_ui)
+        entry.insert(0, str(value))
+        return entry
 
     def _validate_combobox(self, event: tk.Event, path: ComponentPath) -> bool:
         """Validates the value of a combobox."""
@@ -285,7 +271,7 @@ def _validate_combobox(self, event: tk.Event, path: ComponentPath) -> bool:
         combobox.configure(style="comb_input_valid.TCombobox")
         return True
 
-    def validate_entry_limits_ui(self, event: Union[None, tk.Event], entry: ttk.Entry, path: ComponentPath) -> bool:
+    def _validate_entry_limits_ui(self, event: Union[None, tk.Event], entry: ttk.Entry, path: ComponentPath) -> bool:
         """UI wrapper for entry limits validation."""
         is_focusout_event = event and event.type in {
             tk.EventType.FocusOut,

ardupilot_methodic_configurator/frontend_tkinter_component_editor_base.py

@@ -318,6 +318,20 @@ def _set_component_value(self, path: ComponentPath, value: str) -> None:
         """Set component value in data model."""
         self.data_model.set_component_value(path, value)
 
+    def set_vehicle_configuration_template(self, configuration_template: str) -> None:
+        """Set the configuration template name in the data."""
+        self.data_model.set_configuration_template(configuration_template)
+
+    def set_values_from_fc_parameters(self, fc_parameters: dict, doc: dict) -> None:
+        """
+        Process flight controller parameters and update the data model.
+
+        This delegates to the data model's process_fc_parameters method to handle
+        all the business logic of processing parameters.
+        """
+        # Delegate to the data model for parameter processing
+        self.data_model.process_fc_parameters(fc_parameters, doc)
+
     def _update_widget_value(self, path: ComponentPath, value: str) -> None:
         """Update widget value in UI."""
         if path in self.entry_widgets:

tests/test_frontend_tkinter_component_editor.py

@@ -352,14 +352,13 @@ def test_add_entry_or_combobox_optional_field(self, editor_with_mocked_root: Com
             mock_entry = MagicMock()
             mock_entry_class.return_value = mock_entry
 
-            with patch.object(editor_with_mocked_root, "get_validate_function", return_value=MagicMock()):
-                result = editor_with_mocked_root.add_entry_or_combobox(value, mock_entry_frame, path, is_optional=True)
+            result = editor_with_mocked_root.add_entry_or_combobox(value, mock_entry_frame, path, is_optional=True)
 
-                # Should create entry with gray foreground for optional field
-                call_args = mock_entry_class.call_args[1]
-                assert call_args["foreground"] == "gray"
-                mock_entry.insert.assert_called_once_with(0, str(value))
-                assert result == mock_entry
+            # Should create entry with gray foreground for optional field
+            call_args = mock_entry_class.call_args[1]
+            assert call_args["foreground"] == "gray"
+            mock_entry.insert.assert_called_once_with(0, str(value))
+            assert result == mock_entry
 
     def test_add_entry_or_combobox_fc_connection_type(self, editor_with_mocked_root: ComponentEditorWindow) -> None:
         """Test adding combobox for FC connection type with special binding."""
@@ -401,21 +400,24 @@ def test_add_entry_or_combobox_battery_chemistry(self, editor_with_mocked_root:
             bound_events = [call[0][0] for call in bind_calls]  # Extract just the event names
             assert "<<ComboboxSelected>>" in bound_events
 
-    def test_get_validate_function(self, editor_with_mocked_root: ComponentEditorWindow) -> None:
-        """Test getting validation function for entry."""
+    def test_validate_entry_limits_ui_private_method(self, editor_with_mocked_root: ComponentEditorWindow) -> None:
+        """Test the private validation method for entry limits."""
+        mock_event = MagicMock()
+        mock_event.type = tk.EventType.FocusOut
         mock_entry = MagicMock()
+        mock_entry.get.return_value = "1000"
         path = ("Motor", "Specifications", "KV")  # Use proper 3-level path
 
-        validate_func = editor_with_mocked_root.get_validate_function(mock_entry, path)
+        editor_with_mocked_root.data_model.validate_entry_limits = MagicMock(return_value=("", None))
+        editor_with_mocked_root.data_model.set_component_value = MagicMock()
 
-        assert callable(validate_func)
+        result = editor_with_mocked_root._validate_entry_limits_ui(mock_event, mock_entry, path)
 
-        # Test the returned function
-        mock_event = MagicMock()
-        with patch.object(editor_with_mocked_root, "validate_entry_limits_ui", return_value=True) as mock_validate:
-            result = validate_func(mock_event)
-            mock_validate.assert_called_once_with(mock_event, mock_entry, path)
-            assert result is True
+        # Should validate and update data model
+        editor_with_mocked_root.data_model.validate_entry_limits.assert_called_once_with("1000", path)
+        editor_with_mocked_root.data_model.set_component_value.assert_called_once_with(path, "1000")
+        mock_entry.configure.assert_called_once_with(style="entry_input_valid.TEntry")
+        assert result is True
 
     def test_validate_combobox_valid_value(self, editor_with_mocked_root: ComponentEditorWindow) -> None:
         """Test combobox validation with valid value."""
@@ -487,7 +489,7 @@ def test_validate_entry_limits_ui_valid(self, editor_with_mocked_root: Component
         editor_with_mocked_root.data_model.validate_entry_limits = MagicMock(return_value=("", None))
         editor_with_mocked_root.data_model.set_component_value = MagicMock()
 
-        result = editor_with_mocked_root.validate_entry_limits_ui(mock_event, mock_entry, path)
+        result = editor_with_mocked_root._validate_entry_limits_ui(mock_event, mock_entry, path)
 
         editor_with_mocked_root.data_model.validate_entry_limits.assert_called_once_with("1000", path)
         editor_with_mocked_root.data_model.set_component_value.assert_called_once_with(path, "1000")
@@ -505,7 +507,7 @@ def test_validate_entry_limits_ui_invalid_with_correction(self, editor_with_mock
         editor_with_mocked_root.data_model.validate_entry_limits = MagicMock(return_value=("Value too high", "8000"))
 
         with patch("ardupilot_methodic_configurator.frontend_tkinter_component_editor.show_error_message") as mock_error:
-            result = editor_with_mocked_root.validate_entry_limits_ui(mock_event, mock_entry, path)
+            result = editor_with_mocked_root._validate_entry_limits_ui(mock_event, mock_entry, path)
 
             # Should correct the value and show error
             mock_entry.delete.assert_called_once_with(0, tk.END)
@@ -527,7 +529,7 @@ def test_validate_entry_limits_ui_voltage_path(self, editor_with_mocked_root: Co
         editor_with_mocked_root.data_model.validate_entry_limits = MagicMock(return_value=("", None))
         editor_with_mocked_root.data_model.set_component_value = MagicMock()
 
-        result = editor_with_mocked_root.validate_entry_limits_ui(mock_event, mock_entry, path)
+        result = editor_with_mocked_root._validate_entry_limits_ui(mock_event, mock_entry, path)
 
         editor_with_mocked_root.data_model.validate_entry_limits.assert_called_once_with("4.2", path)
         assert result is True