fix(codepilot AI): pylint fixes and fixes suggested by codepilot AI

Author: amilcarlucas

ardupilot_methodic_configurator/data_model_motor_test.py

@@ -22,8 +22,10 @@
 from ardupilot_methodic_configurator.backend_filesystem_program_settings import ProgramSettings
 from ardupilot_methodic_configurator.backend_flightcontroller import FlightController
 
+# pylint: disable=too-many-lines
 
-class MotorTestDataModel:  # pylint: disable=too-many-public-methods
+
+class MotorTestDataModel:  # pylint: disable=too-many-public-methods, too-many-instance-attributes
     """
     Data model for motor test functionality.
 
@@ -537,7 +539,7 @@ def set_test_duration(self, duration: float) -> bool:
         except ValueError as exc:
             logging_error(_("Invalid motor test duration setting: %(error)s"), {"error": str(exc)})
             return False
-        except Exception as exc:
+        except Exception as exc:  # pylint: disable=broad-exception-caught
             logging_error(_("Failed to save duration setting: %(error)s"), {"error": str(exc)})
             return False
 
@@ -584,7 +586,7 @@ def set_test_throttle_pct(self, throttle_pct: int) -> bool:
         except ValueError as exc:
             logging_error(_("Invalid motor test throttle percentage setting: %(error)s"), {"error": str(exc)})
             return False
-        except Exception as exc:
+        except Exception as exc:  # pylint: disable=broad-exception-caught
             logging_error(_("Failed to save throttle percentage setting: %(error)s"), {"error": str(exc)})
             return False
 
@@ -646,7 +648,7 @@ def update_frame_configuration(self, frame_class: int, frame_type: int) -> tuple
             logging_error(error_msg)
             return False, error_msg
 
-    def get_frame_options(self) -> dict[str, dict[int, str]]:
+    def get_frame_options(self) -> dict[str, dict[int, str]]:  # pylint: disable=too-many-branches
         """
         Get all available frame configuration options.
 
@@ -689,7 +691,7 @@ def get_frame_options(self) -> dict[str, dict[int, str]]:
                 )
 
         # Fallback: Use doc_dict only if motor data loader didn't provide data
-        if not frame_options and hasattr(self.filesystem, "doc_dict") and self.filesystem.doc_dict:
+        if not frame_options and hasattr(self.filesystem, "doc_dict") and self.filesystem.doc_dict:  # pylint: disable=too-many-nested-blocks
             logging_debug(_("Motor data unavailable, using doc_dict as fallback for frame options"))
 
             # Get FRAME_TYPE options (these contain both class and type info)

ardupilot_methodic_configurator/data_model_vehicle_components_base.py

@@ -261,6 +261,7 @@ def update_json_structure(self) -> None:
 
         # Merge existing data onto default structure (preserves existing values)
         self._data = self._deep_merge_dicts(default_structure, self._data)
+        self._data["Program version"] = __version__
 
     def _deep_merge_dicts(self, default: dict[str, Any], existing: dict[str, Any]) -> dict[str, Any]:
         """

ardupilot_methodic_configurator/data_model_vehicle_components_import.py

@@ -11,6 +11,7 @@
 # from logging import debug as logging_debug
 from logging import error as logging_error
 from math import log2
+from typing import Any
 
 from ardupilot_methodic_configurator import _
 from ardupilot_methodic_configurator.data_model_vehicle_components_base import ComponentDataModelBase
@@ -57,7 +58,9 @@ def _reverse_key_search(
         logging_error(_("No values found for %s in the metadata"), param_name)
         return fallbacks
 
-    def _verify_dict_is_uptodate(self, doc: dict, dict_to_check: dict, doc_key: str, doc_dict: str) -> bool:
+    def _verify_dict_is_uptodate(
+        self, doc: dict[str, Any], dict_to_check: dict[str, dict[str, Any]], doc_key: str, doc_dict: str
+    ) -> bool:
         """
         Verify that a dictionary is up-to-date with the apm.pdef.xml documentation metadata.
 
@@ -80,8 +83,8 @@ def _verify_dict_is_uptodate(self, doc: dict, dict_to_check: dict, doc_key: str,
 
     def process_fc_parameters(
         self,
-        fc_parameters: dict,
-        doc: dict,
+        fc_parameters: dict[str, Any],
+        doc: dict[str, Any],
     ) -> None:
         """
         Process flight controller parameters and update the data model accordingly.
@@ -214,7 +217,7 @@ def _set_serial_type_from_fc_parameters(self, fc_parameters: dict) -> bool:  # p
 
         return esc >= 2
 
-    def _set_esc_type_from_fc_parameters(self, fc_parameters: dict, doc: dict) -> None:
+    def _set_esc_type_from_fc_parameters(self, fc_parameters: dict[str, Any], doc: dict[str, Any]) -> None:
         """Process ESC parameters and update the data model."""
         mot_pwm_type = fc_parameters.get("MOT_PWM_TYPE", 0)
         try:
@@ -240,7 +243,7 @@ def _set_esc_type_from_fc_parameters(self, fc_parameters: dict, doc: dict) -> No
             protocol = str(MOT_PWM_TYPE_DICT[str(mot_pwm_type)]["protocol"])
             self.set_component_value(("ESC", "FC Connection", "Protocol"), protocol)
 
-    def _set_battery_type_from_fc_parameters(self, fc_parameters: dict) -> None:
+    def _set_battery_type_from_fc_parameters(self, fc_parameters: dict[str, Any]) -> None:
         """Process battery monitor parameters and update the data model."""
         if "BATT_MONITOR" in fc_parameters:
             try:
@@ -258,7 +261,7 @@ def _set_battery_type_from_fc_parameters(self, fc_parameters: dict) -> None:
             except (ValueError, KeyError, TypeError) as e:
                 logging_error(_("Error processing BATT_MONITOR parameter: %s"), str(e))
 
-    def _set_motor_poles_from_fc_parameters(self, fc_parameters: dict) -> None:
+    def _set_motor_poles_from_fc_parameters(self, fc_parameters: dict[str, Any]) -> None:
         """Process motor parameters and update the data model."""
         if "MOT_PWM_TYPE" in fc_parameters:
             mot_pwm_type_str = str(fc_parameters["MOT_PWM_TYPE"])

ardupilot_methodic_configurator/frontend_tkinter_component_editor_base.py

@@ -19,7 +19,7 @@
 from logging import info as logging_info
 from sys import exit as sys_exit
 from tkinter import messagebox, ttk
-from typing import Optional, Union, cast
+from typing import Any, Optional, Union, cast
 from unittest.mock import patch
 
 from ardupilot_methodic_configurator import _, __version__
@@ -324,7 +324,7 @@ def set_vehicle_configuration_template(self, configuration_template: str) -> Non
         """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:
+    def set_values_from_fc_parameters(self, fc_parameters: dict[str, Any], doc: dict[str, Any]) -> None:
         """
         Process flight controller parameters and update the data model.
 

tests/test_data_model_vehicle_components_base.py

@@ -770,7 +770,7 @@ def test_safe_cast_value_none_handling(self, model_with_datatypes) -> None:
 
         GIVEN: A model with type casting capability
         WHEN: Casting None values to different datatypes
-        THEN: Should return appropriate default values (missing lines 134, 137-143)
+        THEN: Should return appropriate default values
         """
         path = ("Battery", "Specifications", "Test")
 
@@ -800,17 +800,17 @@ def test_safe_cast_value_none_handling_edge_cases(self, model_with_datatypes) ->
 
         GIVEN: A model with type casting capability
         WHEN: Casting None values to unusual datatypes
-        THEN: Should handle gracefully (covers lines 141-143)
+        THEN: Should handle gracefully
         """
         path = ("Battery", "Specifications", "Test")
 
-        # Test None to dict - covers line 141
+        # Test None to dict
         result = model_with_datatypes._safe_cast_value(None, dict, path)
         assert result == {}
 
-        # Test None with unknown datatype - covers line 143 (fallback case)
-        class CustomType:
-            pass
+        # Test None with unknown datatype
+        class CustomType:  # pylint: disable=too-few-public-methods
+            """Dummy, just for testing."""
 
         result = model_with_datatypes._safe_cast_value(None, CustomType, path)
         assert result == ""  # Should return empty string for unknown types
@@ -821,16 +821,16 @@ def test_get_component_datatype_isinstance_comprehensive_coverage(self, model_wi
 
         GIVEN: A model with component datatypes
         WHEN: Accessing datatypes with various path scenarios
-        THEN: Should execute isinstance check paths (covers lines 113-114)
+        THEN: Should execute isinstance check paths
         """
         # Set up component datatypes with both type objects and non-type values
         model_with_datatypes._component_datatypes = {
             "Battery": {
                 "Specifications": {
-                    "ValidType": int,  # This is a type - line 113
-                    "InvalidType": "not_a_type",  # This is not a type - line 114
-                    "AnotherValidType": str,  # Another type - line 113
-                    "NonCallable": 42,  # Not callable - line 114
+                    "ValidType": int,  # This is a type
+                    "InvalidType": "not_a_type",  # This is not a type
+                    "AnotherValidType": str,  # Another type
+                    "NonCallable": 42,  # Not callable
                 }
             }
         }
@@ -839,15 +839,15 @@ def test_get_component_datatype_isinstance_comprehensive_coverage(self, model_wi
         datatype = model_with_datatypes._get_component_datatype(("Battery", "Specifications", "ValidType"))
         assert datatype is int
 
-        # Test another valid type (covers line 113 again)
+        # Test another valid type
         datatype = model_with_datatypes._get_component_datatype(("Battery", "Specifications", "AnotherValidType"))
         assert datatype is str
 
-        # Test invalid type - string (covers line 114: else case)
+        # Test invalid type - string
         datatype = model_with_datatypes._get_component_datatype(("Battery", "Specifications", "InvalidType"))
         assert datatype is None
 
-        # Test invalid type - number (covers line 114: else case)
+        # Test invalid type - number
         datatype = model_with_datatypes._get_component_datatype(("Battery", "Specifications", "NonCallable"))
         assert datatype is None
 
@@ -857,18 +857,18 @@ def test_safe_cast_value_list_dict_special_handling_direct(self, model_with_data
 
         GIVEN: A model with type casting capability
         WHEN: Attempting to cast to list or dict types
-        THEN: Should execute special handling code and log error (covers lines 152-154)
+        THEN: Should execute special handling code and log error
         """
         path = ("Battery", "Specifications", "TestField")
 
-        # Test list datatype - should hit line 152 check, log error, and fall back to _process_value
+        # Test list datatype - log error, and fall back to _process_value
         result = model_with_datatypes._safe_cast_value("some_value", list, path)
         assert result == "some_value"  # Falls back to _process_value which returns the original value
         assert "Failed to cast value" in caplog.text
 
         caplog.clear()
 
-        # Test dict datatype - should hit line 152 check, log error, and fall back to _process_value
+        # Test dict datatype - log error, and fall back to _process_value
         result = model_with_datatypes._safe_cast_value("another_value", dict, path)
         assert result == "another_value"  # Falls back to _process_value which returns the original value
         assert "Failed to cast value" in caplog.text
@@ -879,20 +879,22 @@ def test_safe_cast_value_attribute_error_handling(self, model_with_datatypes, ca
 
         GIVEN: A model with type casting capability
         WHEN: A callable datatype raises AttributeError during instantiation
-        THEN: Should catch AttributeError and fall back to _process_value (covers line 159)
+        THEN: Should catch AttributeError and fall back to _process_value
         """
         path = ("Battery", "Specifications", "TestField")
 
         # Create a mock class that raises AttributeError when called
         class AttributeErrorType(type):
+            """Dummy, just for testing."""
+
             def __call__(cls, *args, **kwargs) -> None:
                 msg = "Mock AttributeError for testing"
                 raise AttributeError(msg)
 
-        class MockDatatype(metaclass=AttributeErrorType):
-            pass
+        class MockDatatype(metaclass=AttributeErrorType):  # pylint: disable=too-few-public-methods
+            """Dummy, just for testing."""
 
-        # This should trigger the AttributeError handling at line 159
+        # This should trigger the AttributeError handling
         result = model_with_datatypes._safe_cast_value("test_value", MockDatatype, path)
 
         # Should log the error and fall back to _process_value
@@ -906,9 +908,9 @@ def test_deep_merge_dicts_recursive_comprehensive(self, model_with_datatypes) ->
 
         GIVEN: A model instance with _deep_merge_dicts method
         WHEN: Merging nested dictionaries with various structures
-        THEN: Should handle all recursive paths (covers lines 259-260)
+        THEN: Should handle all recursive paths
         """
-        # Test deep recursive merging (covers line 259: recursive call)
+        # Test deep recursive merging
         default = {
             "level1": {
                 "level2": {
@@ -936,7 +938,7 @@ def test_deep_merge_dicts_recursive_comprehensive(self, model_with_datatypes) ->
 
         result = model_with_datatypes._deep_merge_dicts(default, existing)
 
-        # Verify deep recursive merging occurred (line 259)
+        # Verify deep recursive merging occurred
         assert result["level1"]["level2"]["level3"]["default_value"] == "from_default"
         assert result["level1"]["level2"]["level3"]["existing_value"] == "from_existing"
         assert result["level1"]["level2"]["level3"]["shared_key"] == "existing_shared"
@@ -947,7 +949,7 @@ def test_deep_merge_dicts_recursive_comprehensive(self, model_with_datatypes) ->
         assert result["top_level_default"] == "default"
         assert result["top_level_existing"] == "existing"
 
-        # Test case where existing value is not a dict (covers line 260: else case)
+        # Test case where existing value is not a dict
         default_with_dict = {"mixed_key": {"nested": "should_not_appear"}}
 
         existing_with_string = {
@@ -956,7 +958,7 @@ def test_deep_merge_dicts_recursive_comprehensive(self, model_with_datatypes) ->
 
         result = model_with_datatypes._deep_merge_dicts(default_with_dict, existing_with_string)
 
-        # existing value should be preserved (line 260: result[key] = existing[key])
+        # existing value should be preserved
         assert result["mixed_key"] == "string_value"
         assert not isinstance(result["mixed_key"], dict)
 
@@ -966,14 +968,14 @@ def test_process_value_none_and_version_handling(self, model_with_datatypes) ->
 
         GIVEN: A model instance
         WHEN: Processing None values and Version fields
-        THEN: Should handle appropriately (covers line 173)
+        THEN: Should handle appropriately
         """
-        # Test None value handling (covers line 173: if value is None)
+        # Test None value handling
         path = ("Battery", "Specifications", "TestField")
         result = model_with_datatypes._process_value(path, None)
         assert result == ""
 
-        # Test Version field special handling (covers version check)
+        # Test Version field special handling
         version_path = ("Battery", "Product", "Version")
         result = model_with_datatypes._process_value(version_path, "1.2.3-beta")
         assert result == "1.2.3-beta"