Skip to content

Commit 47f6f81

Browse files
amilcarlucasamilcarlucas
committed
feat(config-steps): conditionally add, delete, and derive parameters per vehicle/FC state
Parameters in each configuration step can now be fully tailored at the JSON level to match the connected flight controller, its current parameter set, and vehicle component data — without requiring Python changes for each new vehicle type. New JSON capabilities in configuration_steps_*.json: - `derived_parameters` entries accept an optional `"if"` expression; the entry is skipped when the condition is false. An entry with only `"if"` and no `"New Value"` is an "add-from-FC" shorthand: the parameter is added at its current FC value when the condition holds. - New `delete_parameters` section removes parameters from a file when an optional `"if"` expression evaluates to true, or unconditionally when no condition is given. Engine changes (backend_filesystem_configuration_steps.py): - `compute_parameters` refactored into focused helpers: `_condition_passes`, `_eval_new_value`, `_resolve_string_result`, `_compute_single_parameter`. - New `compute_deletions` method evaluates the `delete_parameters` section. - `fc_parameters` detection uses a word-boundary regex to prevent false positives. - `StopIteration` from doc-dict lookups now produces a descriptive error message instead of propagating silently. - `SyntaxError` in `"if"` conditions logs a warning; `NameError`/`KeyError` are silent (expected when the FC is not yet connected). Integration: - `LocalFilesystem.calculate_derived_and_forced_param_changes` accepts an optional `fc_parameters` dict that is injected into the eval context. - `ParameterEditor._repopulate_configuration_step_parameters` applies add-from-FC entries and deletions to the live UI domain model. - `configuration_steps_schema.json` updated to validate the new structure. - Applied in `configuration_steps_ArduCopter.json` for IMU temperature calibration parameters (INS_TCAL2/3_ENABLE, BRD_HEAT_TARG) and battery monitor parameters (BATT_I2C_BUS, BATT_AMP_OFFSET, BATT_AMP_PERVLT, BATT_VOLT_MULT). - Tests added for `_condition_passes`, `compute_deletions`, and the add-from-FC shorthand.
1 parent 8a51c5b commit 47f6f81

8 files changed

+538
-101
lines changed

‎ardupilot_methodic_configurator/__main__.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,7 @@ def process_component_editor_results(
576576
try:
577577
component_dependent_param_changes = local_filesystem.calculate_derived_and_forced_param_changes(
578578
fc_param_names=fc_param_names,
579+
fc_parameters=flight_controller.fc_parameters or None, # convert empty dict {} to None to indicate no FC connected
579580
)
580581
except ValueError as e:
581582
error_msg = str(e)

‎ardupilot_methodic_configurator/backend_filesystem.py‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -829,24 +829,22 @@ def get_eval_variables(self) -> dict[str, dict[str, Any]]:
829829
def calculate_derived_and_forced_param_changes(
830830
self,
831831
fc_param_names: list[str],
832+
fc_parameters: Optional[dict[str, float]] = None,
832833
) -> dict[str, ParDict]:
833834
"""
834835
Compute updated parameter values without mutating the in-memory data model.
835836
836-
This method performs a purely read-only computation phase:
837-
1. For each parameter file, deep-copies the current in-memory state.
838-
2. Applies any values received from the flight controller to the copy.
839-
3. Computes forced and derived parameters and merges them into the copy.
840-
4. Compares each copy against the unmodified original in ``self.file_parameters``.
841-
5. Returns only copies that differ - ``self.file_parameters`` is never mutated.
842-
843837
To apply accepted changes to the data model call
844838
:meth:`apply_computed_changes` with the returned dict. To persist them to
845839
disk call :meth:`save_vehicle_params_to_files` afterwards.
846840
847841
Args:
848842
fc_param_names: List of parameter names that exist in the FC.
849843
If empty or None all parameters are assumed to exist.
844+
fc_parameters: Optional dictionary mapping parameter names to their current FC values.
845+
When provided, enables evaluation of conditions referencing ``fc_parameters``
846+
and allows add-from-FC shorthand ``derived_parameters`` entries
847+
(those without a ``New Value``) to be populated with current FC values.
850848
851849
Returns:
852850
dict[str, ParDict]: Mapping of filenames to their fully-computed ``ParDict``
@@ -872,8 +870,10 @@ def calculate_derived_and_forced_param_changes(
872870
873871
"""
874872
eval_variables = self.get_eval_variables()
875-
# the eval variables do not contain fc_parameter values
876-
# and that is intentional, the fc_parameters are not to be used in here
873+
if fc_parameters:
874+
eval_variables["fc_parameters"] = fc_parameters
875+
# fc_parameters are intentionally kept in eval_variables only when provided,
876+
# so add-from-FC derived entries without fc_parameters silently skip.
877877

878878
computed_changes: dict[str, ParDict] = {}
879879

@@ -900,6 +900,10 @@ def calculate_derived_and_forced_param_changes(
900900
param_filename, self.derived_parameters, fc_param_names, target=working
901901
)
902902

903+
# Apply deletions from delete_parameters
904+
for param_name in self.compute_deletions(param_filename, step_dict, eval_variables):
905+
working.pop(param_name, None)
906+
903907
# Include in computed_changes if the working copy differs from the loaded in-memory state
904908
if working.differs_from(param_dict):
905909
computed_changes[param_filename] = working

‎ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py‎

Lines changed: 225 additions & 70 deletions
Large diffs are not rendered by default.

‎ardupilot_methodic_configurator/configuration_steps_ArduCopter.json‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,14 @@
1717
"LOG_DISARMED": { "New Value": 1, "Change Reason": "Gather data for the offline IMU temperature calibration while the FC is disarmed" }
1818
},
1919
"derived_parameters": {
20-
"INS_TCAL2_ENABLE": { "New Value": "2", "Change Reason": "Activates the temperature calibration for IMU 2 at the next start" },
21-
"INS_TCAL3_ENABLE": { "New Value": "2", "Change Reason": "Activates the temperature calibration for IMU 3 at the next start" },
22-
"BRD_HEAT_TARG": { "New Value": "65", "Change Reason": "Reasonable for most places on this planet" }
20+
"INS_TCAL2_ENABLE": { "if": "'INS_TCAL2_ENABLE' in fc_parameters", "New Value": "2", "Change Reason": "Activates the temperature calibration for IMU 2 at the next start" },
21+
"INS_TCAL3_ENABLE": { "if": "'INS_TCAL3_ENABLE' in fc_parameters", "New Value": "2", "Change Reason": "Activates the temperature calibration for IMU 3 at the next start" },
22+
"BRD_HEAT_TARG": { "if": "'BRD_HEAT_TARG' in fc_parameters", "New Value": "65", "Change Reason": "Reasonable for most places on this planet" }
23+
},
24+
"delete_parameters": {
25+
"INS_TCAL2_ENABLE": { "if": "fc_parameters and ('INS_TCAL2_ENABLE' not in fc_parameters)" },
26+
"INS_TCAL3_ENABLE": { "if": "fc_parameters and ('INS_TCAL3_ENABLE' not in fc_parameters)" },
27+
"BRD_HEAT_TARG": { "if": "fc_parameters and ('BRD_HEAT_TARG' not in fc_parameters)" }
2328
},
2429
"jump_possible": {"04_board_orientation.param": "IMU temperature calibration reduces the number of possible 'Accel inconsistent' and 'Gyro inconsistent' errors.\nIMU temperature calibration is optional.\n\nDo you want to skip it?"},
2530
"old_filenames": []
@@ -134,9 +139,18 @@
134139
"BATT_CRT_VOLT": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell crit'])*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Critical voltage + 0.0) x no. of cells" },
135140
"BATT_LOW_VOLT": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell low'])*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Low voltage + 0.0) x no. of cells" },
136141
"BATT_MONITOR": { "New Value": "vehicle_components['Battery Monitor']['FC Connection']['Protocol']", "Change Reason": "Selected in component editor window" },
137-
"BATT_I2C_BUS": { "New Value": "1 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C2' else 2 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C3' else 3 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C4' else 0", "Change Reason": "Selected in component editor window" },
142+
"BATT_I2C_BUS": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Type'] in ['I2C1', 'I2C2', 'I2C3', 'I2C4']", "New Value": "1 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C2' else 2 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C3' else 3 if vehicle_components['Battery Monitor']['FC Connection']['Type'] == 'I2C4' else 0", "Change Reason": "Selected in component editor window" },
138143
"MOT_BAT_VOLT_MAX": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell max']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Max voltage + 0.0) x no. of cells" },
139-
"MOT_BAT_VOLT_MIN": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell crit']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Critical voltage + 0.0) x no. of cells" }
144+
"MOT_BAT_VOLT_MIN": { "New Value": "(vehicle_components['Battery']['Specifications']['Volt per cell crit']+0.0)*vehicle_components['Battery']['Specifications']['Number of cells']", "Change Reason": "(Critical voltage + 0.0) x no. of cells" },
145+
"BATT_AMP_OFFSET": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Protocol'] == 'Analog'" },
146+
"BATT_AMP_PERVLT": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Protocol'] == 'Analog'" },
147+
"BATT_VOLT_MULT": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Protocol'] == 'Analog'" }
148+
},
149+
"delete_parameters": {
150+
"BATT_I2C_BUS": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Type'] not in ['I2C1', 'I2C2', 'I2C3', 'I2C4']" },
151+
"BATT_AMP_OFFSET": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Protocol'] != 'Analog'" },
152+
"BATT_AMP_PERVLT": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Protocol'] != 'Analog'" },
153+
"BATT_VOLT_MULT": { "if": "vehicle_components['Battery Monitor']['FC Connection']['Protocol'] != 'Analog'" }
140154
},
141155
"rename_connection": "vehicle_components['Battery Monitor']['FC Connection']['Type']",
142156
"old_filenames": [],

‎ardupilot_methodic_configurator/configuration_steps_schema.json‎

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,56 @@
8585
"patternProperties": {
8686
"^[A-Z0-9_]+$": {
8787
"type": "object",
88-
"required": ["New Value", "Change Reason"],
89-
"properties": {
90-
"New Value": {
91-
"type": "string",
92-
"description": "Expression to derive new parameter value"
88+
"oneOf": [
89+
{
90+
"description": "Add-from-FC: include this parameter at its current FC value when the condition holds",
91+
"required": ["if"],
92+
"properties": {
93+
"if": {
94+
"type": "string",
95+
"description": "Python expression; the parameter is added from the FC only when this evaluates to true"
96+
}
97+
},
98+
"additionalProperties": false
9399
},
94-
"Change Reason": {
100+
{
101+
"description": "Computed value: evaluate New Value expression and set the parameter",
102+
"required": ["New Value", "Change Reason"],
103+
"properties": {
104+
"if": {
105+
"type": "string",
106+
"description": "Optional Python expression; if present, the parameter is only applied when this expression evaluates to true"
107+
},
108+
"New Value": {
109+
"type": "string",
110+
"description": "Expression to derive new parameter value"
111+
},
112+
"Change Reason": {
113+
"type": "string",
114+
"description": "Reason for the derived parameter"
115+
}
116+
}
117+
}
118+
]
119+
}
120+
},
121+
"description": "Parameters whose values are derived from vehicle component data or FC state. An entry with only 'if' means: add the parameter from the FC at its current value when the condition is true."
122+
},
123+
"delete_parameters": {
124+
"type": "object",
125+
"patternProperties": {
126+
"^[A-Z0-9_]+$": {
127+
"type": "object",
128+
"properties": {
129+
"if": {
95130
"type": "string",
96-
"description": "Reason for the derived parameter"
131+
"description": "Optional Python expression; if present, the parameter is only deleted when this expression evaluates to true"
97132
}
98-
}
133+
},
134+
"additionalProperties": false
99135
}
100-
}
136+
},
137+
"description": "Parameters to remove from the configuration file, optionally conditioned on a Python expression"
101138
},
102139
"jump_possible": {
103140
"type": "object",

‎ardupilot_methodic_configurator/configuration_steps_strings.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,8 @@ def configuration_steps_descriptions() -> None:
339339
340340
For pygettext to extract them, they have no other function
341341
"""
342+
_config_steps_descriptions = _("Add-from-FC: include this parameter at its current FC value when the condition holds")
343+
_config_steps_descriptions = _("Computed value: evaluate New Value expression and set the parameter")
342344
_config_steps_descriptions = _("Description of the phase")
343345
_config_steps_descriptions = _("Explanation of why this step is needed")
344346
_config_steps_descriptions = _("Explanation of why this step needs to be done at this point")
@@ -348,8 +350,21 @@ def configuration_steps_descriptions() -> None:
348350
_config_steps_descriptions = _("Name of tool/process that automatically changes these parameters")
349351
_config_steps_descriptions = _("Name/description of external tool needed")
350352
_config_steps_descriptions = _("New value for the parameter")
353+
_config_steps_descriptions = _(
354+
"Optional Python expression; if present, the parameter is only applied when this expression evaluates to true"
355+
)
356+
_config_steps_descriptions = _(
357+
"Optional Python expression; if present, the parameter is only deleted when this expression evaluates to true"
358+
)
359+
_config_steps_descriptions = _(
360+
"Parameters to remove from the configuration file, optionally conditioned on a Python expression"
361+
)
362+
_config_steps_descriptions = _(
363+
"Parameters whose values are derived from vehicle component data or FC state. An entry with only 'if' means: add the parameter from the FC at its current value when the condition is true."
364+
)
351365
_config_steps_descriptions = _("Phases of the configuration process")
352366
_config_steps_descriptions = _("Previous filenames for this step")
367+
_config_steps_descriptions = _("Python expression; the parameter is added from the FC only when this evaluates to true")
353368
_config_steps_descriptions = _("Reason for changing the parameter")
354369
_config_steps_descriptions = _("Reason for the derived parameter")
355370
_config_steps_descriptions = _("Short description for blog reference")

‎ardupilot_methodic_configurator/data_model_parameter_editor.py‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1661,7 +1661,7 @@ def create_forum_help_zip_workflow(
16611661

16621662
# frontend_tkinter_parameter_editor_table.py API start
16631663

1664-
def _repopulate_configuration_step_parameters(
1664+
def _repopulate_configuration_step_parameters( # pylint: disable=too-many-locals, too-many-branches
16651665
self,
16661666
) -> tuple[list[tuple[str, str]], list[tuple[str, str]]]:
16671667
"""
@@ -1731,6 +1731,30 @@ def _repopulate_configuration_step_parameters(
17311731
if old_name in self.current_step_parameters:
17321732
del self.current_step_parameters[old_name]
17331733

1734+
# Process delete_parameters and add-from-FC derived entries from configuration steps
1735+
step_info = self._local_filesystem.configuration_steps.get(self.current_file, {})
1736+
if step_info:
1737+
variables = self._config_step_processor.variables.copy()
1738+
variables["fc_parameters"] = self.fc_parameters
1739+
1740+
# Apply add-from-FC: derived entries whose condition passed are already stored in derived_parameters;
1741+
# add any that are not yet in current_step_parameters
1742+
fc_parameters = self.fc_parameters
1743+
for param_name, param in self._local_filesystem.derived_parameters.get(self.current_file, ParDict()).items():
1744+
if param_name not in self.current_step_parameters and param_name in fc_parameters:
1745+
self.current_step_parameters[param_name] = self._config_step_processor.create_ardupilot_parameter(
1746+
param_name, param, self.current_file, fc_parameters
1747+
)
1748+
self._added_parameters.add(param_name)
1749+
1750+
# Apply deletions - remove parameters from domain model and track them.
1751+
# Note: compute_deletions is also called in calculate_derived_and_forced_param_changes to
1752+
# update the pre-computed filesystem working copy; this separate call updates the live UI domain model.
1753+
for param_name in self._local_filesystem.compute_deletions(self.current_file, step_info, variables):
1754+
if param_name in self.current_step_parameters:
1755+
self._deleted_parameters.add(param_name)
1756+
del self.current_step_parameters[param_name]
1757+
17341758
return ui_errors, ui_infos
17351759

17361760
def update_parameter_value(

0 commit comments

Comments
 (0)