Tempcal plugin #1022

amilcarlucas · 2025-11-05T20:41:16Z

No description provided.

Copilot

Pull Request Overview

This PR introduces a comprehensive plugin architecture for the ArduPilot Methodic Configurator, enabling modular extension of functionality through UI plugins and workflow plugins. The implementation follows clean architecture principles with strict separation of concerns between business logic (data models), UI coordination (views/workflows), and infrastructure (factories).

Key Changes

Plugin factory pattern with dependency injection to avoid circular imports
Separation of UI plugins (persistent views like motor test panel) from workflow plugins (one-time actions like IMU calibration)
Migration of IMU temperature calibration from monolithic to plugin-based architecture
JSON schema updates for plugin configuration in parameter files

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
`plugin_factory_ui.py`	UI plugin factory for registering and creating persistent plugin views
`plugin_factory_workflow.py`	Workflow plugin factory for one-time triggered actions
`plugin_protocol.py`	Protocol definition for plugin view interface contract
`plugin_constants.py`	Centralized plugin name constants
`business_logic_tempcal_imu.py`	Business logic for IMU calibration extracted from ConfigurationManager
`frontend_tkinter_tempcal_imu.py`	UI workflow coordinator for IMU calibration
`frontend_tkinter_motor_test.py`	Motor test plugin with lifecycle hooks
`frontend_tkinter_parameter_editor.py`	Plugin integration in parameter editor with dynamic layout
`configuration_manager.py`	Plugin data model creation and refactored API
`backend_filesystem_configuration_steps.py`	Plugin configuration retrieval
`backend_filesystem.py`	Generalized file path helper
`__main__.py`	Plugin registration and validation at startup
`configuration_steps_*.json`	JSON schema and example configurations
`ARCHITECTURE_plugins.md`	Comprehensive plugin architecture documentation
`test_*.py`	Comprehensive test coverage for all plugin components

Copilot · 2025-11-05T20:57:57Z

ardupilot_methodic_configurator/plugin_factory_ui.py

+
+import tkinter as tk
+from tkinter import ttk
+from typing import Callable, Optional, Union


Use from __future__ import annotations and modern union syntax (|) instead of importing Optional and Union from typing. Replace Optional[object] with object | None and Union[tk.Frame, ttk.Frame] with tk.Frame | ttk.Frame to align with the project's modern Python type hinting style as seen in other new files.

Copilot · 2025-11-05T20:57:58Z

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py

+            finally:
+                self.parameter_area_paned = None
+
+    def __update_plugin_layout(self, plugin: Optional[dict]) -> None:  # noqa: UP045


The # noqa: UP045 comment suppresses the ruff rule for using modern union syntax. Since the project uses from __future__ import annotations (line 13), you should use dict | None instead of Optional[dict] and remove the noqa comment to maintain consistency with project standards.

Suggested change

def __update_plugin_layout(self, plugin: Optional[dict]) -> None: # noqa: UP045

def __update_plugin_layout(self, plugin: dict | None) -> None:

Copilot · 2025-11-05T20:57:58Z

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py

            self.file_upload_progress_window.destroy()

-    def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: bool = False) -> None:
+    def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: bool = False) -> None:  # noqa: UP007


Use modern union syntax tk.Event | None instead of Union[None, tk.Event] and remove the # noqa: UP007 comments. The file already has from __future__ import annotations so the modern syntax is available and should be used consistently.

Suggested change

def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: bool = False) -> None: # noqa: UP007

def on_param_file_combobox_change(self, _event: tk.Event | None, forced: bool = False) -> None:

Copilot · 2025-11-05T20:57:58Z

ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py

            self.skip_button.configure(state=skip_button_state)

-    def on_skip_click(self, _event: Union[None, tk.Event] = None) -> None:
+    def on_skip_click(self, _event: Union[None, tk.Event] = None) -> None:  # noqa: UP007


Use modern union syntax tk.Event | None instead of Union[None, tk.Event] and remove the # noqa: UP007 comments. The file already has from __future__ import annotations so the modern syntax is available and should be used consistently.

Suggested change

def on_skip_click(self, _event: Union[None, tk.Event] = None) -> None: # noqa: UP007

def on_skip_click(self, _event: tk.Event | None = None) -> None:

Copilot · 2025-11-05T20:57:59Z

ardupilot_methodic_configurator/frontend_tkinter_motor_test.py

+def _create_motor_test_view(
+    parent: Union[tk.Frame, ttk.Frame],
+    model: object,
+    base_window: object,
+) -> MotorTestView:


Use modern union syntax tk.Frame | ttk.Frame instead of Union[tk.Frame, ttk.Frame]. Since this file doesn't currently have from __future__ import annotations, it should be added at the top of the file to enable modern type hinting syntax consistently.

Copilot · 2025-11-05T20:58:00Z

ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py


        return sorted_phases
+
+    def get_plugin(self, selected_file: str) -> Optional[dict]:


Use modern union syntax dict | None instead of Optional[dict]. Add from __future__ import annotations at the top of the file to enable this syntax.

Copilot · 2025-11-05T20:58:00Z

ardupilot_methodic_configurator/configuration_steps_strings.py

    _config_steps_descriptions = _("Short description for blog reference")
    _config_steps_descriptions = _("Short description for wiki reference")
    _config_steps_descriptions = _("Starting step number of this phase")
    _config_steps_descriptions = _("Text indicating if step is mandatory or optional with percentages")


This assignment to '_config_steps_descriptions' is unnecessary as it is redefined before this value is used.

Copilot · 2025-11-05T20:58:00Z

ardupilot_methodic_configurator/configuration_steps_strings.py

    _config_steps_descriptions = _("Short description for wiki reference")
    _config_steps_descriptions = _("Starting step number of this phase")
    _config_steps_descriptions = _("Text indicating if step is mandatory or optional with percentages")
+    _config_steps_descriptions = _("The name of the plugin to load")


This assignment to '_config_steps_descriptions' is unnecessary as it is redefined before this value is used.

Copilot · 2025-11-05T20:58:00Z

ardupilot_methodic_configurator/configuration_steps_strings.py

+    _config_steps_descriptions = _("The name of the plugin to load")
    _config_steps_descriptions = _("URL to blog documentation")
    _config_steps_descriptions = _("URL to external tool")
    _config_steps_descriptions = _("URL to wiki documentation")


This assignment to '_config_steps_descriptions' is unnecessary as it is redefined before this value is used.

Copilot · 2025-11-05T20:58:01Z

ardupilot_methodic_configurator/configuration_steps_strings.py

    _config_steps_descriptions = _("URL to blog documentation")
    _config_steps_descriptions = _("URL to external tool")
    _config_steps_descriptions = _("URL to wiki documentation")
+    _config_steps_descriptions = _("Where to place the plugin: 'left' for left of scrollable frame, 'top' for above contents")


This assignment to '_config_steps_descriptions' is unnecessary as it is redefined before this value is used.

github-actions · 2025-11-05T21:01:02Z

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
9221	7679	83%	80%	🟢

New Files

File	Coverage	Status
ardupilot_methodic_configurator/business_logic_tempcal_imu.py	100%	🟢
ardupilot_methodic_configurator/frontend_tkinter_tempcal_imu.py	100%	🟢
ardupilot_methodic_configurator/plugin_factory_workflow.py	100%	🟢
TOTAL	100%	🟢

Modified Files

File	Coverage	Status
ardupilot_methodic_configurator/main.py	76%	🟢
ardupilot_methodic_configurator/backend_filesystem.py	89%	🟢
ardupilot_methodic_configurator/configuration_manager.py	77%	🟢
ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py	64%	🟢
ardupilot_methodic_configurator/plugin_constants.py	100%	🟢
TOTAL	81%	🟢

updated for commit: 9be901b by action🐍

github-actions · 2025-11-05T21:02:10Z

Test Results

3 files ± 0 3 suites ±0 2m 54s ⏱️ -4s
2 072 tests + 42 2 070 ✅ + 42 2 💤 ±0 0 ❌ ±0
6 216 runs +126 6 210 ✅ +126 6 💤 ±0 0 ❌ ±0

Results for commit 9be901b. ± Comparison against base commit 252d4ad.

This pull request removes 6 and adds 48 tests. Note that renamed tests count towards both.

tests.test_backend_filesystem.TestLocalFilesystem ‑ test_tempcal_imu_result_param_tuple
tests.test_configuration_manager.TestIMUTemperatureCalibrationMethods ‑ test_handle_imu_temperature_calibration_workflow_calibration_fails
tests.test_configuration_manager.TestIMUTemperatureCalibrationMethods ‑ test_handle_imu_temperature_calibration_workflow_success
tests.test_configuration_manager.TestIMUTemperatureCalibrationMethods ‑ test_handle_imu_temperature_calibration_workflow_user_cancels_file_selection
tests.test_configuration_manager.TestIMUTemperatureCalibrationMethods ‑ test_handle_imu_temperature_calibration_workflow_user_declines_confirmation
tests.test_configuration_manager.TestIMUTemperatureCalibrationMethods ‑ test_handle_imu_temperature_calibration_workflow_with_non_matching_file

tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_configuration_manager_can_create_tempcal_imu_data_model
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_configuration_manager_returns_none_for_invalid_plugin_data_model_request
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_configuration_manager_returns_none_when_partial_callbacks_provided
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_end_to_end_plugin_workflow_execution_via_configuration_manager
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_plugin_factories_maintain_independence_between_ui_and_workflow_plugins
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_plugin_registration_during_app_startup_registers_all_plugins
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_plugin_registration_populates_factory_with_correct_creators
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_plugin_system_handles_missing_registrations_gracefully
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_workflow_execution_handles_exceptions_gracefully
tests.integration_plugin_system.TestPluginSystemIntegration ‑ test_workflow_plugin_execution_flow_from_factory_to_run_workflow
…

♻️ This comment has been updated with latest results.

The big advantage is that it allows for easier testing and development of the tempcal IMU (and other similar) feature in isolation from the rest of the codebase.

Copilot AI review requested due to automatic review settings November 5, 2025 20:41

Copilot AI reviewed Nov 5, 2025

View reviewed changes

feat(tempcal IMU): Move the tempcal IMU feature to a plugin

9be901b

The big advantage is that it allows for easier testing and development of the tempcal IMU (and other similar) feature in isolation from the rest of the codebase.

amilcarlucas force-pushed the tempcal_plugin branch from 20d548c to 9be901b Compare November 10, 2025 18:49

	def __update_plugin_layout(self, plugin: Optional[dict]) -> None: # noqa: UP045
	def __update_plugin_layout(self, plugin: dict \| None) -> None:

	def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: bool = False) -> None: # noqa: UP007
	def on_param_file_combobox_change(self, _event: tk.Event \| None, forced: bool = False) -> None:

	def on_skip_click(self, _event: Union[None, tk.Event] = None) -> None: # noqa: UP007
	def on_skip_click(self, _event: tk.Event \| None = None) -> None:


		return sorted_phases

		def get_plugin(self, selected_file: str) -> Optional[dict]:

Tempcal plugin #1022

Are you sure you want to change the base?

Tempcal plugin #1022

Uh oh!

Conversation

amilcarlucas commented Nov 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☂️ Python Coverage

Overall Coverage

New Files

Modified Files

Uh oh!

github-actions bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Nov 5, 2025 •

edited

Loading

github-actions bot commented Nov 5, 2025 •

edited

Loading