Skip to content

Commit 5b9d1c6

Browse files
committed
feat(plugin): Implement motor test plugin
Uses dependency injection via plugin factory to avoid circular dependencies Adds tests
1 parent f9fed24 commit 5b9d1c6

14 files changed

+1307
-7
lines changed

ardupilot_methodic_configurator/__main__.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,50 @@
4848
from ardupilot_methodic_configurator.frontend_tkinter_parameter_editor import ParameterEditorWindow
4949
from ardupilot_methodic_configurator.frontend_tkinter_project_opener import VehicleProjectOpenerWindow
5050
from ardupilot_methodic_configurator.frontend_tkinter_show import show_error_message
51+
from ardupilot_methodic_configurator.plugin_constants import PLUGIN_MOTOR_TEST
52+
from ardupilot_methodic_configurator.plugin_factory import plugin_factory
53+
54+
55+
def register_plugins() -> None:
56+
"""
57+
Register all available plugins with the factory.
58+
59+
This function explicitly imports and registers plugins, avoiding
60+
side-effect imports and potential race conditions.
61+
"""
62+
# Import and register motor test plugin
63+
# pylint: disable=import-outside-toplevel, cyclic-import
64+
from ardupilot_methodic_configurator.frontend_tkinter_motor_test import register_motor_test_plugin # noqa: PLC0415
65+
# pylint: enable=import-outside-toplevel, cyclic-import
66+
67+
register_motor_test_plugin()
68+
69+
# Add more plugin registrations here in the future
70+
71+
72+
def validate_plugin_registry(local_filesystem: LocalFilesystem) -> None:
73+
"""
74+
Validate that all plugins configured in configuration steps are registered.
75+
76+
Args:
77+
local_filesystem: The filesystem interface to access configuration steps
78+
79+
"""
80+
# Get all configured plugins
81+
configured_plugins = set()
82+
# configuration_steps is a dict, not an object with configuration_steps attribute
83+
for file_info in local_filesystem.configuration_steps.values():
84+
plugin = file_info.get("plugin")
85+
if plugin and plugin.get("name"):
86+
configured_plugins.add(plugin["name"])
87+
88+
# Verify each configured plugin is registered
89+
for plugin_name in configured_plugins:
90+
if not plugin_factory.is_registered(plugin_name):
91+
logging_error(
92+
_("Plugin '%(plugin_name)s' is configured but not registered. Available plugins: %(available)s"),
93+
{"plugin_name": plugin_name, "available": PLUGIN_MOTOR_TEST},
94+
)
5195

5296

5397
class ApplicationState: # pylint: disable=too-few-public-methods
@@ -507,6 +551,9 @@ def main() -> None:
507551
"""
508552
args = create_argument_parser().parse_args()
509553

554+
# Register plugins early, before any UI creation
555+
register_plugins()
556+
510557
# Create desktop icon if needed (only on first run in venv)
511558
FreeDesktop.create_desktop_icon_if_needed()
512559

@@ -518,6 +565,10 @@ def main() -> None:
518565
if check_updates(state):
519566
sys_exit(0) # user asked to update, exit the old version
520567

568+
# Validate that all configured plugins are registered
569+
if state.local_filesystem:
570+
validate_plugin_registry(state.local_filesystem)
571+
521572
if bool(ProgramSettings.get_setting("auto_open_doc_in_browser")):
522573
display_first_use_documentation()
523574

ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from logging import info as logging_info
1515
from logging import warning as logging_warning
1616
from os import path as os_path
17-
from typing import TypedDict
17+
from typing import Optional, TypedDict
1818

1919
# from sys import exit as sys_exit
2020
# from logging import debug as logging_debug
@@ -293,3 +293,18 @@ def get_sorted_phases_with_end_and_weight(self, total_files: int) -> dict[str, P
293293
sorted_phases[phase_name]["weight"] = max(2, phase_end - phase_start)
294294

295295
return sorted_phases
296+
297+
def get_plugin(self, selected_file: str) -> Optional[dict]:
298+
"""
299+
Get the plugin configuration for the selected file.
300+
301+
Args:
302+
selected_file: The filename to get plugin info for
303+
304+
Returns:
305+
The plugin dict with 'name' and 'placement' if exists, None otherwise
306+
307+
"""
308+
if selected_file in self.configuration_steps:
309+
return self.configuration_steps[selected_file].get("plugin")
310+
return None

ardupilot_methodic_configurator/configuration_manager.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727
from ardupilot_methodic_configurator.backend_internet import download_file_from_url
2828
from ardupilot_methodic_configurator.data_model_ardupilot_parameter import ArduPilotParameter
2929
from ardupilot_methodic_configurator.data_model_configuration_step import ConfigurationStepProcessor
30+
from ardupilot_methodic_configurator.data_model_motor_test import MotorTestDataModel
3031
from ardupilot_methodic_configurator.data_model_par_dict import Par, ParDict, is_within_tolerance
32+
from ardupilot_methodic_configurator.plugin_constants import PLUGIN_MOTOR_TEST
3133
from ardupilot_methodic_configurator.tempcal_imu import IMUfit
3234

3335
# Type aliases for callback functions used in workflow methods
@@ -1593,3 +1595,28 @@ def parse_mandatory_level_percentage(self, text: str) -> tuple[int, str]:
15931595
return 0, tooltip.format(current_file=current_file)
15941596

15951597
# frontend_tkinter_parameter_editor_documentation_frame.py API end
1598+
1599+
# plugin API begin
1600+
1601+
def get_plugin(self, filename: str) -> Optional[dict]:
1602+
return self._local_filesystem.get_plugin(filename)
1603+
1604+
def create_plugin_data_model(self, plugin_name: str) -> Optional[object]:
1605+
"""
1606+
Create and return a data model for the specified plugin.
1607+
1608+
Args:
1609+
plugin_name: The name of the plugin to create a data model for
1610+
1611+
Returns:
1612+
The data model instance, or None if plugin not supported or requirements not met
1613+
1614+
"""
1615+
if plugin_name == PLUGIN_MOTOR_TEST:
1616+
if not self.is_fc_connected:
1617+
return None
1618+
return MotorTestDataModel(self._flight_controller, self._local_filesystem)
1619+
# Add more plugins here in the future
1620+
return None
1621+
1622+
# plugin API end

ardupilot_methodic_configurator/configuration_steps_ArduCopter.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,11 @@
269269
"external_tool_url": "https://firmware.ardupilot.org/Tools/WebTools/ThrustExpo/",
270270
"mandatory_text": "40% mandatory (60% optional)",
271271
"auto_changed_by": "",
272-
"old_filenames": ["14_motor.param"]
272+
"old_filenames": ["14_motor.param"],
273+
"plugin": {
274+
"name": "motor_test",
275+
"placement": "left"
276+
}
273277
},
274278
"16_pid_adjustment.param": {
275279
"why": "With very large or very small vehicles the default PID values are not suitable for the first flight",

ardupilot_methodic_configurator/configuration_steps_schema.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,22 @@
145145
"pattern": "^/APM/"
146146
}
147147
}
148+
},
149+
"plugin": {
150+
"type": "object",
151+
"required": ["name", "placement"],
152+
"properties": {
153+
"name": {
154+
"type": "string",
155+
"enum": ["motor_test"],
156+
"description": "The name of the plugin to load"
157+
},
158+
"placement": {
159+
"type": "string",
160+
"enum": ["left", "top"],
161+
"description": "Where to place the plugin: 'left' for left of scrollable frame, 'top' for above contents"
162+
}
163+
}
148164
}
149165
}
150166
}

ardupilot_methodic_configurator/configuration_steps_strings.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,9 @@ def configuration_steps_descriptions() -> None:
356356
_config_steps_descriptions = _("Short description for wiki reference")
357357
_config_steps_descriptions = _("Starting step number of this phase")
358358
_config_steps_descriptions = _("Text indicating if step is mandatory or optional with percentages")
359+
_config_steps_descriptions = _("The name of the plugin to load")
359360
_config_steps_descriptions = _("URL to blog documentation")
360361
_config_steps_descriptions = _("URL to external tool")
361362
_config_steps_descriptions = _("URL to wiki documentation")
363+
_config_steps_descriptions = _("Where to place the plugin: 'left' for left of scrollable frame, 'top' for above contents")
362364
_config_steps_descriptions = _("Whether this phase is optional")

ardupilot_methodic_configurator/frontend_tkinter_motor_test.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@
5858
from ardupilot_methodic_configurator.frontend_tkinter_pair_tuple_combobox import PairTupleCombobox
5959
from ardupilot_methodic_configurator.frontend_tkinter_progress_window import ProgressWindow
6060
from ardupilot_methodic_configurator.frontend_tkinter_scroll_frame import ScrollFrame
61+
from ardupilot_methodic_configurator.plugin_constants import PLUGIN_MOTOR_TEST
62+
from ardupilot_methodic_configurator.plugin_factory import plugin_factory
6163

6264

6365
class DelayedProgressCallback: # pylint: disable=too-few-public-methods
@@ -756,6 +758,42 @@ def _setup_keyboard_shortcuts(self) -> None:
756758
# Focus root window to ensure it can capture key events
757759
self.root_window.focus_set()
758760

761+
def on_activate(self) -> None:
762+
"""
763+
Called when the plugin becomes active (visible).
764+
765+
Refreshes the frame configuration from the flight controller
766+
to ensure the display is up-to-date.
767+
"""
768+
# Refresh frame configuration when becoming active
769+
if not self.model.refresh_from_flight_controller():
770+
logging_warning(_("Could not refresh frame configuration from flight controller"))
771+
self._update_view()
772+
773+
def on_deactivate(self) -> None:
774+
"""
775+
Called when the plugin becomes inactive (hidden).
776+
777+
Stops all running motor tests for safety when switching away from this plugin.
778+
"""
779+
# Critical safety requirement: stop all motors when user navigates away
780+
# to prevent motors running unattended in the background
781+
try:
782+
self.model.stop_all_motors()
783+
self._reset_all_motor_status()
784+
except (MotorTestExecutionError, ParameterError) as e:
785+
# Motor stop failed - this could indicate a communication issue or unsupported frame type.
786+
# We log as warning (not debug) because failed motor stop is a safety concern
787+
# that operators should be aware of, even if it's expected for some configurations.
788+
logging_warning(
789+
_("Motor stop failed during deactivation: %(error)s. Please verify motors are stopped."), {"error": str(e)}
790+
)
791+
except Exception as e:
792+
# Unexpected errors during motor stop are critical safety issues.
793+
# We log as error and re-raise to prevent silently continuing with motors potentially running.
794+
logging_error(_("Critical error during motor stop at deactivation: %(error)s"), {"error": str(e)})
795+
raise
796+
759797

760798
class MotorTestWindow(BaseWindow):
761799
"""
@@ -849,5 +887,36 @@ def main() -> None:
849887
state.flight_controller.disconnect() # Disconnect from the flight controller
850888

851889

890+
# Register this plugin with the factory for dependency injection
891+
def _create_motor_test_view(
892+
parent: Union[tk.Frame, ttk.Frame],
893+
model: object,
894+
base_window: object,
895+
) -> MotorTestView:
896+
"""
897+
Factory function to create MotorTestView instances.
898+
899+
This function trusts that the caller provides the correct types
900+
as per the plugin protocol (duck typing approach).
901+
902+
Args:
903+
parent: The parent frame
904+
model: The MotorTestDataModel instance (passed as object for protocol compliance)
905+
base_window: The BaseWindow instance (passed as object for protocol compliance)
906+
907+
Returns:
908+
A new MotorTestView instance
909+
910+
"""
911+
# Trust the caller to provide correct types (protocol-based duck typing)
912+
# Type checker will verify this at static analysis time
913+
return MotorTestView(parent, model, base_window) # type: ignore[arg-type]
914+
915+
916+
def register_motor_test_plugin() -> None:
917+
"""Register the motor test plugin with the factory."""
918+
plugin_factory.register(PLUGIN_MOTOR_TEST, _create_motor_test_view)
919+
920+
852921
if __name__ == "__main__":
853922
main()

0 commit comments

Comments
 (0)