Skip to content

Commit 211d0b1

Browse files
OmkarSarkar204amilcarlucas
authored andcommitted
feat: add optional step instruction popup
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
1 parent 867e329 commit 211d0b1

8 files changed

Lines changed: 99 additions & 0 deletions

ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,3 +335,9 @@ def get_plugin(self, selected_file: str) -> Optional[dict]:
335335
if selected_file in self.configuration_steps:
336336
return self.configuration_steps[selected_file].get("plugin")
337337
return None
338+
339+
def get_instructions_popup(self, selected_file: str) -> Optional[dict]:
340+
"""Get the instructions popup configuration for the selected file."""
341+
if selected_file in self.configuration_steps:
342+
return self.configuration_steps[selected_file].get("instructions_popup")
343+
return None

ardupilot_methodic_configurator/configuration_steps_ArduCopter.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,10 @@
283283
}
284284
},
285285
"16_pid_adjustment.param": {
286+
"instructions_popup": {
287+
"type": "info",
288+
"msg": "This step is optional, only perform it if your vehicle is tiny, huge, or its motor outputs oscillate"
289+
},
286290
"why": "With very large or very small vehicles the default PID values are not suitable for the first flight",
287291
"why_now": "Most other parameters are done and these need to be corrected (depending on the vehicle size) before the first flight",
288292
"blog_text": "Adjust the Proportional-Integral-Derivative (PID) controllers based on the vehicle size before the first flight",

ardupilot_methodic_configurator/configuration_steps_schema.json

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,23 @@
168168
"description": "Where to place the plugin: 'left' for left of scrollable frame, 'top' for above contents"
169169
}
170170
}
171+
},
172+
"instructions_popup": {
173+
"type": "object",
174+
"description": "Optional instructions to display as a popup when entering this step",
175+
"properties": {
176+
"type": {
177+
"type": "string",
178+
"enum": ["info", "warning"],
179+
"description": "The type of popup dialog to display (info or warning)"
180+
},
181+
"msg": {
182+
"type": "string",
183+
"description": "The instruction message to display to the user"
184+
}
185+
},
186+
"required": ["type", "msg"],
187+
"additionalProperties": false
171188
}
172189
}
173190
}

ardupilot_methodic_configurator/configuration_steps_strings.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ def configuration_steps_descriptions() -> None:
356356
_config_steps_descriptions = _("Name of tool/process that automatically changes these parameters")
357357
_config_steps_descriptions = _("Name/description of external tool needed")
358358
_config_steps_descriptions = _("New value for the parameter")
359+
_config_steps_descriptions = _("Optional instructions to display as a popup when entering this step")
359360
_config_steps_descriptions = _("Phases of the configuration process")
360361
_config_steps_descriptions = _("Previous filenames for this step")
361362
_config_steps_descriptions = _("Reason for changing the parameter")
@@ -364,7 +365,9 @@ def configuration_steps_descriptions() -> None:
364365
_config_steps_descriptions = _("Short description for wiki reference")
365366
_config_steps_descriptions = _("Starting step number of this phase")
366367
_config_steps_descriptions = _("Text indicating if step is mandatory or optional with percentages")
368+
_config_steps_descriptions = _("The instruction message to display to the user")
367369
_config_steps_descriptions = _("The name of the plugin to load")
370+
_config_steps_descriptions = _("The type of popup dialog to display (info or warning)")
368371
_config_steps_descriptions = _("URL to blog documentation")
369372
_config_steps_descriptions = _("URL to external tool")
370373
_config_steps_descriptions = _("URL to wiki documentation")

ardupilot_methodic_configurator/data_model_parameter_editor.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,6 +2170,10 @@ def parse_mandatory_level_percentage(self, text: str) -> tuple[int, str]:
21702170
def get_plugin(self, filename: str) -> Optional[dict]:
21712171
return self._local_filesystem.get_plugin(filename)
21722172

2173+
def get_instructions_popup(self, filename: str) -> Optional[dict]:
2174+
"""Get the optional instructions popup data for a given configuration step."""
2175+
return self._local_filesystem.get_instructions_popup(filename)
2176+
21732177
def create_plugin_data_model(self, plugin_name: str) -> Optional[object]:
21742178
"""
21752179
Create and return a data model for the specified plugin.

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,17 @@ def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: b
10281028
self.repopulate_parameter_table()
10291029
self._update_skip_button_state()
10301030

1031+
# Display optional instruction step
1032+
popup_data = self.parameter_editor.get_instructions_popup(final_file)
1033+
if popup_data:
1034+
msg = popup_data.get("msg", "")
1035+
popup_type = popup_data.get("type", "info")
1036+
1037+
if popup_type == "warning":
1038+
self.ui.show_warning(_("Step Instructions"), _(msg))
1039+
else:
1040+
self.ui.show_info(_("Step Instructions"), _(msg))
1041+
10311042
def _update_progress_bar_from_file(self, selected_file: str) -> None:
10321043
if self.parameter_editor.configuration_phases():
10331044
try:

tests/test_backend_filesystem_configuration_steps.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,5 +641,21 @@ def test_multiple_forced_param_errors_collected() -> None:
641641
assert config_steps.forced_parameters["test_file"]["GOOD2"].value == 20
642642

643643

644+
def test_get_instructions_popup() -> None:
645+
"""Test that the filesystem correctly retrieves the instructions popup dictionary."""
646+
config_steps = ConfigurationSteps("dummy_dir", "dummy_type")
647+
config_steps.configuration_steps = {
648+
"16_pid_adjustment.param": {"instructions_popup": {"type": "info", "msg": "Test message"}},
649+
"other.param": {},
650+
}
651+
652+
# Test when it exists
653+
assert config_steps.get_instructions_popup("16_pid_adjustment.param") == {"type": "info", "msg": "Test message"}
654+
# Test when it doesn't exist
655+
assert config_steps.get_instructions_popup("other.param") is None
656+
# Test when file doesn't exist
657+
assert config_steps.get_instructions_popup("missing.param") is None
658+
659+
644660
if __name__ == "__main__":
645661
unittest.main()

tests/test_frontend_tkinter_parameter_editor.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2002,3 +2002,41 @@ def test_multiple_button_clicks_invoke_workflow_each_time(self, parameter_editor
20022002

20032003
# Assert - workflow was called three times
20042004
assert param_editor_mock.create_forum_help_zip_workflow.call_count == 3
2005+
2006+
2007+
class TestStepInstructionsPopupWorkflows:
2008+
"""Cover the step instructions popup behaviors when navigating files."""
2009+
2010+
def test_user_sees_info_popup_when_instructions_exist(self, parameter_editor_window: ParameterEditorWindow) -> None:
2011+
"""
2012+
User receives an informational popup for steps with info instructions.
2013+
2014+
GIVEN: Instructions exist with type 'info'
2015+
WHEN: The parameter file changes to that step
2016+
THEN: An info popup is displayed to the user
2017+
"""
2018+
param_editor_mock = cast("MagicMock", parameter_editor_window.parameter_editor)
2019+
param_editor_mock.get_instructions_popup.return_value = {"type": "info", "msg": "Test info message"}
2020+
2021+
parameter_editor_window.on_param_file_combobox_change(None)
2022+
2023+
ui_mock = cast("MagicMock", parameter_editor_window.ui)
2024+
ui_mock.show_info.assert_called_once_with("Step Instructions", "Test info message")
2025+
ui_mock.show_warning.assert_not_called()
2026+
2027+
def test_user_sees_warning_popup_when_instructions_exist(self, parameter_editor_window: ParameterEditorWindow) -> None:
2028+
"""
2029+
User receives a warning popup for steps with warning instructions.
2030+
2031+
GIVEN: Instructions exist with type 'warning'
2032+
WHEN: The parameter file changes to that step
2033+
THEN: A warning popup is displayed to the user
2034+
"""
2035+
param_editor_mock = cast("MagicMock", parameter_editor_window.parameter_editor)
2036+
param_editor_mock.get_instructions_popup.return_value = {"type": "warning", "msg": "Test warning message"}
2037+
2038+
parameter_editor_window.on_param_file_combobox_change(None)
2039+
2040+
ui_mock = cast("MagicMock", parameter_editor_window.ui)
2041+
ui_mock.show_warning.assert_called_once_with("Step Instructions", "Test warning message")
2042+
ui_mock.show_info.assert_not_called()

0 commit comments

Comments
 (0)