Skip to content

Commit 5d8eb62

Browse files
OmkarSarkar204amilcarlucas
authored andcommitted
fix: Auto import params
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
1 parent c332e27 commit 5d8eb62

6 files changed

Lines changed: 101 additions & 3 deletions

USERMANUAL.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,10 @@ These files are crucial for the tool's operation and are organized in a specific
421421
The `configuration_steps_ArduCopter.json` documentation file is first searched in the selected vehicle-specific directory,
422422
and if not found, in the directory where the script is located.
423423

424+
- **Configuration Steps File**: The `configuration_steps_*.json` files (like `configuration_steps_ArduCopter.json`) define the workflow,
425+
for each intermediate parameter file. They provide the documentation links, explanations("why" and "why now"), mandatory/optional percentages,
426+
and advanced logic rules like `autoimport_nondefault_regexp` to automatically pull specific non-default parameters from the live flight controller into the GUI.
427+
424428
- **Default Parameter Values File**: The `00_defaults.param` file is located in the vehicle-specific directory.
425429
If the file does not exist or is invalid, use this command to regenerate it
426430

ardupilot_methodic_configurator/configuration_steps_ArduCopter.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@
124124
"external_tool_url": "",
125125
"mandatory_text": "100% mandatory (0% optional)",
126126
"auto_changed_by": "",
127+
"autoimport_nondefault_regexp": ["BATT.*"],
127128
"forced_parameters": {
128129
"BATT_FS_CRT_ACT": { "New Value": 1, "Change Reason": "Land ASAP" },
129130
"BATT_FS_LOW_ACT": { "New Value": 2, "Change Reason": "Return and land at home or rally point" }
@@ -169,6 +170,7 @@
169170
"external_tool_url": "",
170171
"mandatory_text": "100% mandatory (0% optional)",
171172
"auto_changed_by": "",
173+
"autoimport_nondefault_regexp": ["GPS.*", "GNSS.*"],
172174
"derived_parameters": {
173175
"GPS_TYPE": { "New Value": "vehicle_components['GNSS Receiver']['FC Connection']['Protocol']", "Change Reason": "Defined in component editor" },
174176
"GPS1_TYPE": { "New Value": "vehicle_components['GNSS Receiver']['FC Connection']['Protocol']", "Change Reason": "Defined in component editor" }

ardupilot_methodic_configurator/configuration_steps_schema.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@
2323
"type": "string",
2424
"description": "Explanation of why this step is needed"
2525
},
26+
"autoimport_nondefault_regexp": {
27+
"type": "array",
28+
"items": {
29+
"type": "string"
30+
},
31+
"description": "A list of regular expressions to match parameters for automatic import if they have non-default values."
32+
},
2633
"why_now": {
2734
"type": "string",
2835
"description": "Explanation of why this step needs to be done at this point"

ardupilot_methodic_configurator/configuration_steps_strings.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,9 @@ def configuration_steps_descriptions() -> None:
344344
345345
For pygettext to extract them, they have no other function
346346
"""
347+
_config_steps_descriptions = _(
348+
"A list of regular expressions to match parameters for automatic import if they have non-default values."
349+
)
347350
_config_steps_descriptions = _("Description of the phase")
348351
_config_steps_descriptions = _("Explanation of why this step is needed")
349352
_config_steps_descriptions = _("Explanation of why this step needs to be done at this point")

ardupilot_methodic_configurator/data_model_configuration_step.py

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,14 @@
1111
SPDX-License-Identifier: GPL-3.0-or-later
1212
"""
1313

14+
import re
1415
from logging import info as logging_info
1516
from typing import Any, Optional
1617

1718
from ardupilot_methodic_configurator import _
1819
from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem
1920
from ardupilot_methodic_configurator.data_model_ardupilot_parameter import ArduPilotParameter
20-
from ardupilot_methodic_configurator.data_model_par_dict import Par, ParDict
21+
from ardupilot_methodic_configurator.data_model_par_dict import Par, ParDict, is_within_tolerance
2122
from ardupilot_methodic_configurator.data_model_safe_evaluator import safe_evaluate
2223

2324

@@ -100,10 +101,10 @@ def process_configuration_step( # pylint: disable=too-many-locals
100101
# Collect derived parameter values to apply later in domain model
101102
elif selected_file in self.local_filesystem.derived_parameters:
102103
# Filter derived parameters that exist in FC (if fc_parameters provided)
103-
fc_param_names = set(fc_parameters.keys()) if fc_parameters else set()
104+
fc_param_keys = set(fc_parameters.keys()) if fc_parameters else set()
104105
for param_name, param in self.local_filesystem.derived_parameters[selected_file].items():
105106
# Only include if no FC filter OR parameter exists in FC
106-
if not fc_param_names or param_name in fc_param_names:
107+
if not fc_param_keys or param_name in fc_param_keys:
107108
derived_params_to_apply[param_name] = param
108109

109110
# Populate new_connection_prefix from rename_connection configuration step (per-step scope)
@@ -123,6 +124,9 @@ def process_configuration_step( # pylint: disable=too-many-locals
123124
# Create domain model parameters
124125
current_step_parameters = self._create_domain_model_parameters(selected_file, fc_parameters)
125126

127+
# Call the _apply_auto_imports import
128+
self._apply_auto_imports(selected_file, fc_parameters, current_step_parameters)
129+
126130
# Check for ExpressLRS and add FLTMODE_CH warning
127131
if current_step_parameters.get("RC_OPTIONS") is not None or current_step_parameters.get("FLTMODE_CH") is not None:
128132
rc_options = int(fc_parameters.get("RC_OPTIONS", 32))
@@ -142,6 +146,34 @@ def process_configuration_step( # pylint: disable=too-many-locals
142146

143147
return current_step_parameters, ui_errors, ui_infos, duplicates_to_remove, renames_to_apply, derived_params_to_apply
144148

149+
def _apply_auto_imports(
150+
self,
151+
selected_file: str,
152+
fc_parameters: dict[str, float],
153+
current_step_parameters: dict[str, ArduPilotParameter],
154+
) -> None:
155+
"""Automatically import non-default FC parameters matching regex rules into the domain model."""
156+
step_dict = self.local_filesystem.configuration_steps.get(selected_file, {})
157+
if "autoimport_nondefault_regexp" not in step_dict or not fc_parameters:
158+
return
159+
160+
regex_rules = step_dict["autoimport_nondefault_regexp"]
161+
for live_key, live_value in fc_parameters.items():
162+
if live_key in current_step_parameters:
163+
continue
164+
165+
is_matching_regex = any(re.match(pattern, live_key) for pattern in regex_rules)
166+
is_in_defaults = live_key in self.local_filesystem.param_default_dict
167+
168+
if is_matching_regex and is_in_defaults:
169+
default_value = self.local_filesystem.param_default_dict[live_key].value
170+
if not is_within_tolerance(live_value, default_value):
171+
# Create the parameter with a blank reason and inject it into the UI domain model
172+
param = Par(float(live_value), "")
173+
current_step_parameters[live_key] = self.create_ardupilot_parameter(
174+
live_key, param, selected_file, fc_parameters
175+
)
176+
145177
def _handle_connection_renaming(
146178
self, selected_file: str, variables: dict
147179
) -> tuple[list[tuple[str, str]], set[str], list[tuple[str, str]]]:

tests/test_data_model_configuration_step.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,56 @@ def test_user_receives_error_feedback_when_derived_parameter_computation_fails(
190190
# Should have info messages about parameter renaming since configuration includes rename_connection
191191
assert len(ui_infos) > 0
192192

193+
def test_user_can_auto_import_nondefault_parameters_matching_regex(self, processor) -> None:
194+
"""
195+
User can automatically import non-default flight controller parameters that match regex rules.
196+
197+
GIVEN: A configuration step with auto-import regex rules
198+
WHEN: The step is processed with live FC parameters
199+
THEN: Non-default matching parameters are added to the domain model with a blank reason
200+
AND: Parameters already in the file or matching firmware defaults are not overridden
201+
"""
202+
# Arrange: Set up configuration with regex rules and mixed FC states
203+
selected_file = "test_file.param"
204+
processor.local_filesystem.configuration_steps = {selected_file: {"autoimport_nondefault_regexp": ["BATT.*", "GPS.*"]}}
205+
processor.local_filesystem.param_default_dict = {
206+
"BATT_OPTIONS": Par(value=0.0, comment="Default option"),
207+
"BATT_MONITOR": Par(value=0.0, comment="Default monitor"),
208+
"GPS_TYPE": Par(value=0.0, comment="Default type"),
209+
}
210+
processor.local_filesystem.file_parameters = {
211+
selected_file: {
212+
"BATT_MONITOR": Par(value=4.0, comment="User comment") # Already exists in file
213+
}
214+
}
215+
test_fc_parameters = {
216+
"BATT_OPTIONS": 5.0, # Non-default, matching regex -> MUST IMPORT
217+
"BATT_MONITOR": 4.0, # Already in file -> MUST SKIP
218+
"GPS_TYPE": 0.0, # Default value -> MUST SKIP
219+
"SERIAL1_BAUD": 115200.0, # Doesn't match regex -> MUST SKIP
220+
}
221+
222+
# Act: Process configuration step
223+
parameters, ui_errors, _, _, _, _ = processor.process_configuration_step(selected_file, test_fc_parameters)
224+
225+
# Assert: No errors
226+
assert ui_errors == []
227+
228+
# BATT_OPTIONS should be auto-imported with a blank reason
229+
assert "BATT_OPTIONS" in parameters
230+
assert parameters["BATT_OPTIONS"].get_new_value() == 5.0
231+
assert parameters["BATT_OPTIONS"].change_reason == ""
232+
233+
# BATT_MONITOR should remain untouched (protecting the user comment)
234+
assert "BATT_MONITOR" in parameters
235+
assert parameters["BATT_MONITOR"].change_reason == "User comment"
236+
237+
# GPS_TYPE should NOT be imported (it is at default value)
238+
assert "GPS_TYPE" not in parameters
239+
240+
# SERIAL1_BAUD should NOT be imported (does not match regex)
241+
assert "SERIAL1_BAUD" not in parameters
242+
193243

194244
class TestConfigurationStepProcessorConnectionRenaming:
195245
"""Test connection renaming workflows and edge cases."""

0 commit comments

Comments
 (0)