feat(param-editor): split non-default params into IDs + non-IDs categories

Author: amilcarlucas

ARCHITECTURE_5_parameter_editor.md

@@ -359,7 +359,8 @@ At completion, the system generates several summary files:
 - `complete.param`: All parameters with change reasons
 - `non-default_read-only.param`: Read-only parameters that differ from defaults
 - `non-default_writable_calibrations.param`: Vehicle-specific calibrations
-- `non-default_writable_non-calibrations.param`: Reusable configuration parameters
+- `non-default_writable_ids.param`: Vehicle-specific IDs
+- `non-default_writable_non-calibrations_non-ids.param`: Reusable configuration parameters
 
 ### Configuration Steps Definition
 

SYSTEM_REQUIREMENTS.md

@@ -169,7 +169,8 @@ To semi-automate the steps and processes on that guide the following *system des
   - Complete flight controller *reason changed* annotated parameters in `complete.param` file
   - Non-default, read-only *reason changed* annotated parameters in, `non-default_read-only.param` file
   - Non-default, writable calibrations *reason changed* annotated parameters in `non-default_writable_calibrations.param` file
-  - Non-default, writable non-calibrations *reason changed* annotated parameters in `non-default_writable_non-calibrations.param` file
+  - Non-default, writable IDs *reason changed* annotated parameters in `non-default_writable_ids.param` file
+  - Non-default, writable non-calibrations, non-IDs *reason changed* annotated parameters in `non-default_writable_non-calibrations_non-ids.param` file
 - automatically create a parameter backup before the first usage of the software to change parameters [PR #173](https://github.com/ArduPilot/MethodicConfigurator/pull/173)
   - Only backs up the parameters if a backup file does not exist and only if AMC has not yet been used to write parameters to the FC
 

USERMANUAL.md

@@ -365,7 +365,8 @@ pie title Summary files example
     "Unchanged parameters" : 728
     "Non-default read-only parameters - non-default_read-only.param" : 8
     "Non-default writable sensor calibrations - non-default_writable_calibrations.param" : 71
-    "Non-default writable non-sensor-calibrations - non-default_writable_non-calibrations.param" : 217
+    "Non-default writable IDs - non-default_writable_ids.param" : 3
+    "Non-default writable non-sensor-calibrations, non-IDs - non-default_writable_non-calibrations_non-ids.param" : 217
 ```
 
 If the diagram above does not display correctly [look here](https://github.com/ArduPilot/MethodicConfigurator/blob/master/USERMANUAL.md#9-completing-the-configuration-process)
@@ -378,24 +379,25 @@ If the diagram above does not display correctly [look here](https://github.com/A
 - **Non-default writable sensor calibrations**: These parameters are vehicle-instance dependent and cannot be reused between similar vehicles.
   They are typically related to sensor calibration and should be adjusted for each vehicle and are displayed on a yellow background 🟨.
 
-- **Non-default writable non-sensor calibrations**: These parameters can be reused between similar vehicles.
+- **Non-default writable IDs**: These parameters are vehicle-instance dependent and cannot be reused between similar vehicles.
+
+- **Non-default writable non-sensor, non-IDs calibrations**: These parameters can be reused between similar vehicles.
   They are not related to sensor calibration and are generally applicable to a range of vehicles with the same configuration.
 
 After the summary message box is displayed, the application will write the summary information into separate files for easy reference and documentation. These files include:
 
 - `complete.param`: Contains all parameters contained in the flight controller.
 - `non-default_read-only.param`: Contains all non-default read-only 🟥 parameters. You can ignore these.
 - `non-default_writable_calibrations.param`: Contains all non-default writable sensor calibration 🟨 parameters. These are non-reusable.
-- `non-default_writable_non-calibrations.param`: Contains all non-default writable non-sensor-calibration parameters. These are reusable across similar vehicles.
+- `non-default_writable_ids.param`: Contains all non-default writable IDs. These are non-reusable.
+- `non-default_writable_non-calibrations_non-ids.param`: Contains all non-default writable non-sensor-calibration, non-IDs parameters. These are reusable across similar vehicles.
 
 The summary files provide a clear overview of the changes made.
 
 The files are also automatically zipped into a file with the same name as the vehicle directory, inside the vehicle directory.
 
 ![Parameter files zipped message box](images/Parameter_files_zipped.png)
 
-You should upload this `.zip` file or the `non-default_writable_non-calibrations.param` file to the [How to methodically tune any ArduCopter Blog post](https://discuss.ardupilot.org/t/how-to-methodically-configure-and-tune-any-arducopter/110842/)
-
 Once the summary files are written, the application will close the connection to the flight controller and terminate.
 
 ## Configuration Details

ardupilot_methodic_configurator/backend_filesystem.py

@@ -489,20 +489,21 @@ def annotate_intermediate_comments_to_param_dict(self, param_dict: dict[str, flo
         ip_comments = self.__all_intermediate_parameter_file_comments()
         return ParDict.from_float_dict(param_dict).annotate_with_comments(ip_comments)
 
-    def categorize_parameters(self, param: ParDict) -> tuple[ParDict, ParDict, ParDict]:
+    def categorize_parameters(self, param: ParDict) -> tuple[ParDict, ParDict, ParDict, ParDict]:
         """
-        Categorize parameters into three categories based on their default values and documentation attributes.
+        Categorize parameters into four categories based on their default values and documentation attributes.
 
-        This method iterates through the provided ParDict of parameters and categorizes them into three groups:
+        This method iterates through the provided ParDict of parameters and categorizes them into four groups:
         - Non-default, read-only parameters
         - Non-default, writable calibrations
-        - Non-default, writable non-calibrations
+        - Non-default, writable IDs
+        - Non-default, writable non-calibrations, non-IDs
 
         Args:
           param (ParDict): A ParDict mapping parameter names to their 'Par' objects.
 
         Returns:
-          Tuple[ParDict, ParDict, ParDict]: A tuple of three ParDict objects.
+          Tuple[ParDict, ParDict, ParDict, ParDict]: A tuple of four ParDict objects.
                                   Each ParDict represents one of the categories mentioned above.
 
         """

ardupilot_methodic_configurator/data_model_par_dict.py

@@ -591,43 +591,64 @@ def _filter_by_calibration(self, doc_dict: dict) -> "ParDict":
             }
         )
 
+    def _filter_by_id(self, _doc_dict: dict) -> "ParDict":
+        """
+        Filter parameters that are marked as ID parameters in the documentation.
+
+        Args:
+            doc_dict: Documentation dictionary containing parameter metadata.
+
+        Returns:
+            A new ParDict containing only ID parameters.
+
+        """
+        return ParDict(
+            {
+                param_name: param_info
+                for param_name, param_info in self.items()
+                if param_name in ["SYSID_THISMAV", "SYSID_MYGCS", "FOLL_SYSID"]
+            }
+        )
+
     def categorize_by_documentation(
         self,
         doc_dict: dict,
         default_params: "ParDict",
         tolerance_func: Optional[Callable[[float, float], bool]] = None,
-    ) -> tuple["ParDict", "ParDict", "ParDict"]:
+    ) -> tuple["ParDict", "ParDict", "ParDict", "ParDict"]:
         """
-        Categorize parameters into read-only, calibration, and other non-default parameters.
+        Categorize parameters into read-only, calibration, IDs and other non-default parameters.
 
         Args:
             doc_dict: Documentation dictionary containing parameter metadata.
             default_params: ParDict containing default parameter values.
             tolerance_func: Function to check if values are within tolerance.
 
         Returns:
-            A tuple of three ParDict objects:
+            A tuple of four ParDict objects:
             - Non-default read-only parameters
             - Non-default writable calibration parameters
-            - Non-default writable non-calibration parameters
+            - Non-default writable ID parameters
+            - Non-default writable non-calibration, non-ID parameters
 
         """
         non_default_params = self._filter_by_defaults(default_params, tolerance_func)
 
         # there are protected members from a locally created object, so it is OK to access them like this
         read_only_params = non_default_params._filter_by_readonly(doc_dict)  # pylint: disable=protected-access # noqa: SLF001
         calibration_params = non_default_params._filter_by_calibration(doc_dict)  # pylint: disable=protected-access # noqa: SLF001
+        id_params = non_default_params._filter_by_id(doc_dict)  # pylint: disable=protected-access # noqa: SLF001
 
-        # Non-calibration parameters are those that are not read-only and not calibration
+        # Non-calibration parameters are those that are not read-only, not ID, and not calibration
         other_params = ParDict(
             {
                 param_name: param_info
                 for param_name, param_info in non_default_params.items()
-                if param_name not in read_only_params and param_name not in calibration_params
+                if param_name not in read_only_params and param_name not in calibration_params and param_name not in id_params
             }
         )
 
-        return read_only_params, calibration_params, other_params
+        return read_only_params, calibration_params, id_params, other_params
 
     def annotate_with_comments(self, comment_lookup: dict[str, str]) -> "ParDict":
         """

ardupilot_methodic_configurator/data_model_parameter_editor.py

@@ -1297,7 +1297,7 @@ def _generate_parameter_summary(self) -> dict[str, ParDict]:
 
         Returns:
             dict: Dictionary with parameter categories and their ParDict objects.
-                Keys: "complete", "read_only", "calibrations", "non_calibrations"
+                Keys: "complete", "read_only", "calibrations", "ids", "non_calibrations_non_ids"
 
         """
         # Get annotated FC parameters
@@ -1309,17 +1309,18 @@ def _generate_parameter_summary(self) -> dict[str, ParDict]:
 
         # Categorize parameters using filesystem logic
         categorized = self._local_filesystem.categorize_parameters(annotated_fc_parameters)
-        if not categorized or len(categorized) != 3:
+        if not categorized or len(categorized) != 4:
             # Return empty dict if categorization fails or returns empty tuple
             return {}
 
-        non_default__read_only_params, non_default__writable_calibrations, non_default__writable_non_calibrations = categorized
+        non_default__read_only_params, non_default__writable_calibrations, non_default__ids, non_default__writable_non_calibrations_non_ids = categorized
 
         return {
             "complete": annotated_fc_parameters,
             "read_only": non_default__read_only_params,
             "calibrations": non_default__writable_calibrations,
-            "non_calibrations": non_default__writable_non_calibrations,
+            "ids": non_default__ids,
+            "non_calibrations_non_ids": non_default__writable_non_calibrations_non_ids,
         }
 
     def _get_parameter_summary_msg(self, parameter_summary: dict[str, ParDict]) -> str:
@@ -1340,12 +1341,14 @@ def _get_parameter_summary_msg(self, parameter_summary: dict[str, ParDict]) -> s
         nr_total_params = len(parameter_summary.get("complete", {}))
         nr_non_default__read_only_params = len(parameter_summary.get("read_only", {}))
         nr_non_default__writable_calibrations = len(parameter_summary.get("calibrations", {}))
-        nr_non_default__writable_non_calibrations = len(parameter_summary.get("non_calibrations", {}))
+        nr_non_default__ids = len(parameter_summary.get("ids", {}))
+        nr_non_default__writable_non_calibrations_non_ids = len(parameter_summary.get("non_calibrations_non_ids", {}))
         nr_unchanged_params = (
             nr_total_params
             - nr_non_default__read_only_params
             - nr_non_default__writable_calibrations
-            - nr_non_default__writable_non_calibrations
+            - nr_non_default__ids
+            - nr_non_default__writable_non_calibrations_non_ids
         )
 
         # Format the summary message
@@ -1356,7 +1359,9 @@ def _get_parameter_summary_msg(self, parameter_summary: dict[str, ParDict]) -> s
             "ignore these, you can not change them\n\n"
             "{nr_non_default__writable_calibrations} non-default writable sensor-calibrations - "
             "non-reusable between vehicles\n\n"
-            "{nr_non_default__writable_non_calibrations} non-default writable non-sensor-calibrations - "
+            "{nr_non_default__ids} non-default ID parameters - "
+            "these can be reused but often require manual review due to vehicle-specific values\n\n"
+            "{nr_non_default__writable_non_calibrations_non_ids} non-default writable non-sensor-calibrations non-IDs - "
             "these can be reused between similar vehicles"
         )
 
@@ -1365,7 +1370,8 @@ def _get_parameter_summary_msg(self, parameter_summary: dict[str, ParDict]) -> s
             nr_unchanged_params=nr_unchanged_params,
             nr_non_default__read_only_params=nr_non_default__read_only_params,
             nr_non_default__writable_calibrations=nr_non_default__writable_calibrations,
-            nr_non_default__writable_non_calibrations=nr_non_default__writable_non_calibrations,
+            nr_non_default__ids=nr_non_default__ids,
+            nr_non_default__writable_non_calibrations_non_ids=nr_non_default__writable_non_calibrations_non_ids,
         )
 
     def write_summary_files_workflow(
@@ -1405,7 +1411,8 @@ def write_summary_files_workflow(
         complete_params = parameter_summary["complete"]
         read_only_params = parameter_summary["read_only"]
         calibration_params = parameter_summary["calibrations"]
-        non_calibration_params = parameter_summary["non_calibrations"]
+        id_params = parameter_summary["ids"]
+        non_calibration_non_ids_params = parameter_summary["non_calibrations_non_ids"]
 
         # Write individual summary files
         wrote_complete = self._write_single_summary_file_workflow(
@@ -1420,9 +1427,15 @@ def write_summary_files_workflow(
             annotate_doc=False,
             ask_confirmation=ask_confirmation,
         )
-        wrote_non_calibrations = self._write_single_summary_file_workflow(
-            non_calibration_params,
-            "non-default_writable_non-calibrations.param",
+        wrote_ids = self._write_single_summary_file_workflow(
+            id_params,
+            "non-default_writable_ids.param",
+            annotate_doc=False,
+            ask_confirmation=ask_confirmation,
+        )
+        wrote_non_calibrations_non_ids = self._write_single_summary_file_workflow(
+            non_calibration_non_ids_params,
+            "non-default_writable_non-calibrations_non-ids.param",
             annotate_doc=False,
             ask_confirmation=ask_confirmation,
         )
@@ -1432,7 +1445,8 @@ def write_summary_files_workflow(
             (wrote_complete, "complete.param"),
             (wrote_read_only, "non-default_read-only.param"),
             (wrote_calibrations, "non-default_writable_calibrations.param"),
-            (wrote_non_calibrations, "non-default_writable_non-calibrations.param"),
+            (wrote_ids, "non-default_writable_ids.param"),
+            (wrote_non_calibrations_non_ids, "non-default_writable_non-calibrations_non-ids.param"),
         ]
 
         # Write zip file with user confirmation

tests/test_backend_filesystem.py

@@ -634,11 +634,12 @@ def test_categorize_parameters(self) -> None:
 
         test_params = ParDict({"PARAM1": Par(2.0), "PARAM2": Par(2.0), "PARAM3": Par(2.0)})
 
-        readonly, calibration, other = lfs.categorize_parameters(test_params)
+        readonly, calibration, ids, other = lfs.categorize_parameters(test_params)
 
         assert "PARAM1" in readonly
         assert "PARAM2" in calibration
         assert "PARAM3" in other
+        assert not ids
 
     def test_calculate_derived_and_forced_param_changes(self) -> None:
         lfs = LocalFilesystem(

tests/test_data_model_par_dict.py

@@ -884,13 +884,14 @@ def test_user_can_categorize_parameters_by_documentation(
         # Arrange: Parameters, defaults, and documentation provided by fixtures
 
         # Act: Categorize parameters
-        readonly, calibration, other = parameter_dict.categorize_by_documentation(documentation_dict, default_parameters)
+        readonly, calibration, ids, other = parameter_dict.categorize_by_documentation(documentation_dict, default_parameters)
 
         # Assert: Parameters categorized correctly
         # Note: Only non-default parameters are categorized
         assert "PILOT_SPEED_UP" in readonly  # Read-only and non-default
         assert "BATT_CAPACITY" in calibration  # Calibration and non-default
         assert "GPS_TYPE" in other  # Neither read-only nor calibration, non-default
+        assert not ids
 
 
 class TestParameterUtilities:
@@ -1061,7 +1062,7 @@ def test_user_can_categorize_parameters_by_documentation_type(
         # Arrange: Parameter dictionary with documentation
 
         # Act: Categorize parameters by documentation
-        readonly, calibration, other = parameter_dict.categorize_by_documentation(documentation_dict, default_parameters)
+        readonly, calibration, ids, other = parameter_dict.categorize_by_documentation(documentation_dict, default_parameters)
 
         # Assert: Parameters correctly categorized
         assert "PILOT_SPEED_UP" in readonly  # Marked as ReadOnly=True
@@ -1070,6 +1071,7 @@ def test_user_can_categorize_parameters_by_documentation_type(
         assert "ACRO_YAW_P" not in readonly  # Not read-only
         assert "ACRO_YAW_P" not in calibration  # Not calibration
         assert "GPS_TYPE" in other  # Non-default, non-readonly, non-calibration
+        assert not ids
 
     def test_user_can_categorize_parameters_with_tolerance_function(
         self, parameter_dict, documentation_dict, default_parameters
@@ -1087,7 +1089,7 @@ def always_within_tolerance(_value1: float, _value2: float) -> bool:
             return True
 
         # Act: Categorize with tolerance function
-        readonly, calibration, other = parameter_dict.categorize_by_documentation(
+        readonly, calibration, ids, other = parameter_dict.categorize_by_documentation(
             documentation_dict, default_parameters, always_within_tolerance
         )
 
@@ -1099,6 +1101,7 @@ def always_within_tolerance(_value1: float, _value2: float) -> bool:
         assert "BATT_CAPACITY" in calibration  # Still non-default, still calibration
         assert "COMPASS_ENABLE" in calibration  # Still non-default, still calibration
         assert "GPS_TYPE" in other  # Still non-default, not readonly, not calibration
+        assert not ids
 
 
 class TestParameterCommentAnnotation: