diff --git a/ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py b/ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py index 130f50e29..f89dc466d 100644 --- a/ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py +++ b/ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py @@ -41,6 +41,7 @@ class ConfigurationSteps: def __init__(self, _vehicle_dir: str, vehicle_type: str) -> None: self.configuration_steps_filename = "configuration_steps_" + vehicle_type + ".json" self.configuration_steps: dict[str, dict] = {} + self.configuration_phases: dict[str, dict] = {} self.forced_parameters: dict[str, dict] = {} self.derived_parameters: dict[str, dict] = {} self.log_loaded_file = False @@ -96,6 +97,11 @@ def re_init(self, vehicle_dir: str, vehicle_type: str) -> None: # pylint: disab self.__validate_parameters_in_configuration_steps(filename, file_info, "derived") else: logging_warning(_("No configuration steps documentation and no forced and derived parameters will be available.")) + + if file_found and "phases" in json_content: + self.configuration_phases = json_content["phases"] + else: + logging_warning(_("No configuration phases documentation will be available.")) self.log_loaded_file = True def __validate_parameters_in_configuration_steps(self, filename: str, file_info: dict, parameter_type: str) -> None: diff --git a/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json b/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json index 77499bda0..4599e6738 100644 --- a/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json +++ b/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json @@ -826,5 +826,65 @@ "auto_changed_by": "", "old_filenames": ["45_everyday_use.param", "47_everyday_use.param", "50_everyday_use.param"] } + }, + "phases": { + "IMU temperature calibration": { + "description": "Calibrate the IMU sensors for at different temperatures", + "optional": true, + "start": 2 + }, + "Assemble all components except the propellers": { + "description": "Assemble all components except the propellers" + }, + "Basic mandatory configuration": { + "description": "Set up the basic parameters for the vehicle", + "start": 4 + }, + "Assemble the propellers and perform the first flight": { + "description": "Assemble the propellers and perform the first flight" + }, + "Minimalistic mandatory tuning": { + "description": "Minimalistic mandatory tuning using test flight data", + "start": 19 + }, + "Standard tuning": { + "description": "Improve tuning with more test flight data", + "optional": true, + "start": 24 + }, + "Improve altitude control": { + "description": "Improve altitude under windy conditions", + "optional": true, + "start": 40 + }, + "Analytical PID optimization": { + "description": "System identification and analytical PID optimization", + "optional": true, + "start": 42 + }, + "Position controller tuning": { + "description": "Position controller tuning", + "optional": true, + "start": 47 + }, + "Guided operation": { + "description": "Guided operation", + "optional": true, + "start": 48 + }, + "Precision landing": { + "description": "Precision landing", + "optional": true, + "start": 49 + }, + "Optical flow calibration": { + "description": "Optical flow sensor calibration", + "optional": true, + "start": 50 + }, + "Everyday use": { + "description": "Everyday use", + "start": 53 + } } } \ No newline at end of file diff --git a/ardupilot_methodic_configurator/configuration_steps_schema.json b/ardupilot_methodic_configurator/configuration_steps_schema.json index 0078d4531..c5817603e 100644 --- a/ardupilot_methodic_configurator/configuration_steps_schema.json +++ b/ardupilot_methodic_configurator/configuration_steps_schema.json @@ -1,6 +1,6 @@ { "type": "object", - "required": ["steps"], + "required": ["steps", "phases"], "properties": { "steps": { "type": "object", @@ -149,6 +149,31 @@ } } } + }, + "phases": { + "type": "object", + "patternProperties": { + "^.*$": { + "type": "object", + "required": ["description"], + "properties": { + "description": { + "type": "string", + "description": "Description of the phase" + }, + "optional": { + "type": "boolean", + "description": "Whether this phase is optional" + }, + "start": { + "type": "integer", + "minimum": 1, + "description": "Starting step number of this phase" + } + } + } + }, + "description": "Phases of the configuration process" } }, "additionalProperties": false diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py index 874e48591..864b38e04 100755 --- a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py @@ -44,6 +44,7 @@ from ardupilot_methodic_configurator.frontend_tkinter_directory_selection import VehicleDirectorySelectionWidgets from ardupilot_methodic_configurator.frontend_tkinter_parameter_editor_documentation_frame import DocumentationFrame from ardupilot_methodic_configurator.frontend_tkinter_parameter_editor_table import ParameterEditorTable +from ardupilot_methodic_configurator.frontend_tkinter_stage_progress import StageProgressBar from ardupilot_methodic_configurator.tempcal_imu import IMUfit @@ -153,7 +154,7 @@ def __init__(self, current_file: str, flight_controller: FlightController, local self.root.title( _("Amilcar Lucas's - ArduPilot methodic configurator ") + __version__ + _(" - Parameter file editor and uploader") ) - self.root.geometry("990x550") # Set the window width + self.root.geometry("990x610") # Set the window width # Bind the close_connection_and_quit function to the window close event self.root.protocol("WM_DELETE_WINDOW", self.close_connection_and_quit) @@ -169,8 +170,19 @@ def __init__(self, current_file: str, flight_controller: FlightController, local self.__create_conf_widgets(__version__) + if self.local_filesystem.configuration_phases: + # Get the first two characters of the last configuration step filename + last_step_filename = next(reversed(self.local_filesystem.file_parameters.keys())) + last_step_nr = int(last_step_filename[:2]) + 1 if len(last_step_filename) >= 2 else 1 + + self.stage_progress_bar = StageProgressBar( + self.main_frame, self.local_filesystem.configuration_phases, last_step_nr + ) + self.stage_progress_bar.pack(side=tk.TOP, fill="x", expand=False, pady=(2, 2), padx=(4, 4)) + # Create a DocumentationFrame object for the Documentation Content self.documentation_frame = DocumentationFrame(self.main_frame, self.local_filesystem, self.current_file) + self.documentation_frame.documentation_frame.pack(side=tk.TOP, fill="x", expand=False, pady=(2, 2), padx=(4, 4)) self.__create_parameter_area_widgets() @@ -333,7 +345,9 @@ def __create_parameter_area_widgets(self) -> None: "Upload selected parameters to the flight controller and advance to the next " "intermediate parameter file\nIf changes have been made to the current file it will ask if you want " "to save them\nIt will reset the FC if necessary, re-download all parameters and validate their value" - ), + ) + if self.flight_controller.master + else _("No flight controller connected, without it this is not available"), ) # Create skip button @@ -494,6 +508,7 @@ def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: b return self.parameter_editor_table.generate_edit_widgets_focus_out() selected_file = self.file_selection_combobox.get() + self._update_progress_bar_from_file(selected_file) if self.current_file != selected_file or forced: self.write_changes_to_intermediate_parameter_file() self.__do_tempcal_imu(selected_file) @@ -509,6 +524,15 @@ def on_param_file_combobox_change(self, _event: Union[None, tk.Event], forced: b self.documentation_frame.update_why_why_now_tooltip(selected_file) self.repopulate_parameter_table(selected_file) + def _update_progress_bar_from_file(self, selected_file: str) -> None: + if self.local_filesystem.configuration_phases: + try: + step_nr = int(selected_file[:2]) + self.stage_progress_bar.update_progress(step_nr) + except ValueError as _e: + msg = _("Failed to update progress bar, {selected_file} does not start with two digits like it should: {_e}") + logging_error(msg.format(**locals())) + def download_flight_controller_parameters(self, redownload: bool = False) -> None: operation_string = _("Re-downloading FC parameters") if redownload else _("Downloading FC parameters") self.param_download_progress_window = ProgressWindow( diff --git a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_documentation_frame.py b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_documentation_frame.py index df5211158..a61a441a6 100644 --- a/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_documentation_frame.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_documentation_frame.py @@ -42,7 +42,6 @@ def __init__(self, root: tk.Widget, local_filesystem: LocalFilesystem, current_f def __create_documentation_frame(self) -> None: self.documentation_frame = ttk.LabelFrame(self.root, text=_("Documentation")) - self.documentation_frame.pack(side=tk.TOP, fill="x", expand=False, pady=(4, 4), padx=(4, 4)) # Create a grid structure within the documentation_frame documentation_grid = ttk.Frame(self.documentation_frame) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_stage_progress.py b/ardupilot_methodic_configurator/frontend_tkinter_stage_progress.py new file mode 100755 index 000000000..33496bf9d --- /dev/null +++ b/ardupilot_methodic_configurator/frontend_tkinter_stage_progress.py @@ -0,0 +1,251 @@ +#!/usr/bin/env python3 + +""" +Configuration stage progress UI. + +This module implements a graphical progress indicator for the ArduPilot Methodic +Configurator that visualizes the configuration process across multiple phases. + +The UI consists of a horizontal sequence of progress bars, each representing a distinct +configuration phase. The phases are arranged left-to-right and are defined in a +configuration JSON file. Each phase: +- Shows its progress based on the current configuration file being processed +- Displays its name in a label below the progress bar +- Provides a tooltip with detailed phase description +- Can be marked as optional (shown in gray) +- Has a defined start point, with its end being the start of the next phase + (or total files for the last phase) + +The component is implemented as a ttk.LabelFrame containing individual frames for +each phase. Progress is tracked by file number (1 to N) across .param files being +processed. Phases without a start position are treated as milestones and are not +displayed. + +Each phase's progress bar automatically: +- Remains empty (0%) before its start file is reached +- Shows relative progress while its files are being processed +- Fills completely (100%) after all its files are processed + +The UI dynamically adjusts to window resizing while maintaining proportional spacing +between phases. + +This file is part of Ardupilot methodic configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2025 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +import argparse +import tkinter as tk +from logging import basicConfig as logging_basicConfig +from logging import error as logging_error +from logging import getLevelName as logging_getLevelName +from tkinter import ttk +from typing import Union + +from ardupilot_methodic_configurator import _ +from ardupilot_methodic_configurator.backend_filesystem_configuration_steps import ConfigurationSteps +from ardupilot_methodic_configurator.common_arguments import add_common_arguments +from ardupilot_methodic_configurator.frontend_tkinter_base import show_tooltip + + +class StageProgressBar(ttk.LabelFrame): # pylint: disable=too-many-ancestors + """Stage-segmented Configuration sequence progress UI.""" + + def __init__(self, master: Union[tk.Widget, tk.Tk], phases: dict[str, dict], total_steps: int, **kwargs) -> None: + super().__init__(master, text=_("Configuration sequence progress"), **kwargs) + self.phases = phases + self.total_files = total_steps + self.phase_frames: dict[str, ttk.Frame] = {} + self.phase_bars: list[dict[str, Union[ttk.Progressbar, int]]] = [] + + self.grid_columnconfigure(0, weight=1) + self.grid_rowconfigure(0, weight=1) + + self.create_phase_frames() + self.bind("", self._on_resize) + show_tooltip( + self, + _( + "This bar shows your progress through the configuration phases. " + "Each phase contains configuration steps that must be completed in sequence." + ), + position_below=False, + ) + + def create_phase_frames(self) -> None: + """Create frames for each phase with progress bars and labels.""" + # Get phases with start positions + active_phases = {k: v for k, v in self.phases.items() if "start" in v} + + # Sort phases by start position + sorted_phases = dict(sorted(active_phases.items(), key=lambda x: x[1]["start"])) + + num_phases = len(sorted_phases) + + # Create container frame that will expand + container = ttk.Frame(self) + container.grid(row=0, column=0, sticky="nsew", padx=5, pady=5) + + # Configure container columns to expand equally + for i in range(num_phases): + container.grid_columnconfigure(i, weight=1, uniform="phase") + + for i, (phase_name, phase_data) in enumerate(sorted_phases.items()): + start = phase_data["start"] + end = list(sorted_phases.values())[i + 1]["start"] if i < num_phases - 1 else self.total_files + self.phase_frames[phase_name] = self._create_phase_frame(container, i, phase_name, phase_data, (start, end)) + + def _create_phase_frame( # pylint: disable=too-many-arguments, too-many-positional-arguments + self, container: ttk.Frame, i: int, phase_name: str, phase_data: dict[str, Union[str, bool]], limits: tuple[int, int] + ) -> ttk.Frame: + frame = ttk.Frame(container) + frame.grid(row=0, column=i, sticky="nsew", padx=1) + frame.grid_columnconfigure(0, weight=1) + frame.grid_rowconfigure(0, weight=0) # Progress bar row + frame.grid_rowconfigure(1, weight=1) # Label row + + progress = ttk.Progressbar(frame, orient=tk.HORIZONTAL, mode="determinate", maximum=limits[1] - limits[0]) + progress.grid(row=0, column=0, sticky="ew", pady=2) + self.phase_bars.append({"bar": progress, "start": limits[0], "end": limits[1]}) + + label_text = phase_name + first_space = label_text.find(" ") + if "\n" not in label_text: + if 6 <= first_space < 20: + label_text = label_text.replace(" ", "\n", 1) + if len(label_text) < 20: + label_text += "\n " + + # Use gray text color if phase is marked as optional + is_optional = False + if "optional" in phase_data and isinstance(phase_data["optional"], bool): + is_optional = phase_data.get("optional", False) # type: ignore[assignment] + label_fg = "gray" if is_optional else "black" + + label = ttk.Label( + frame, + text=label_text, + wraplength=0, # Will be updated in _on_resize + justify=tk.CENTER, + anchor="center", + foreground=label_fg, + ) + label.grid(row=1, column=0, sticky="ew") + + if "description" in phase_data and isinstance(phase_data["description"], str): + tooltip_msg = _(phase_data.get("description", "")) # type: ignore[arg-type] + if is_optional: + tooltip_msg += "\n" + _("This phase is optional.") + show_tooltip(frame, tooltip_msg) + return frame + + def _on_resize(self, _event: Union[tk.Event, None] = None) -> None: + """Update progress bar and label widths when window is resized.""" + if not self.phase_frames: + return + + # Calculate new width per phase + num_phases = len(self.phase_frames) + padding = 4 # Account for frame padding + new_width = max(1, (self.winfo_width() - padding) // num_phases) + + # Update wraplength for all labels + for frame in self.phase_frames.values(): + for child in frame.winfo_children(): + if isinstance(child, ttk.Label): + child.configure(wraplength=new_width - padding) + + def update_progress(self, current_file: int) -> None: + """ + Update progress bars based on current file number. + + Args: + current_file: Current configuration file number + + Each bar will be: + - Empty (0%) if current_file < start + - Full (100%) if current_file > end + - Show progress between start-end otherwise + + """ + if not 0 < current_file < self.total_files: + msg = _("Out of expected range [0 .. {self.total_files}] current file number: {current_file}") + msg = msg.format(self=self, current_file=current_file) + logging_error(msg) + return + + for phase in self.phase_bars: + if ( # pylint: disable=too-many-boolean-expressions + "start" in phase + and "end" in phase + and "bar" in phase + and isinstance(phase["start"], int) + and isinstance(phase["end"], int) + and isinstance(phase["bar"], ttk.Progressbar) + ): + if phase["start"] <= current_file <= phase["end"]: + # Calculate progress within this phase + progress = current_file - phase["start"] + phase["bar"]["value"] = progress + elif current_file > phase["end"]: + # Phase complete + phase["bar"]["value"] = phase["end"] - phase["start"] + else: + # Phase not started + phase["bar"]["value"] = 0 + + +def argument_parser() -> argparse.Namespace: + """ + Parses command-line arguments for the script. + + This function sets up an argument parser to handle the command-line arguments for the script. + + Returns: + argparse.Namespace: An object containing the parsed arguments. + + """ + parser = argparse.ArgumentParser( + description=_( + "ArduPilot methodic configurator is a Wizard-style GUI tool to configure " + "ArduPilot parameters. This module shows configuration sequence progress in " + "the form of a configuration-stage-segmented progress bar." + ) + ) + return add_common_arguments(parser).parse_args() + + +def main() -> None: + args = argument_parser() + + logging_basicConfig(level=logging_getLevelName(args.loglevel), format="%(asctime)s - %(levelname)s - %(message)s") + + root = tk.Tk() + root.title("Configuration Progress") + + # Load phases from configuration + config_steps = ConfigurationSteps("", "ArduCopter") + config_steps.re_init("", "ArduCopter") + + progress = StageProgressBar(root, config_steps.configuration_phases, 54) + progress.pack(padx=10, pady=10, fill="both", expand=True) + + # Demo update function + current_file = 2 + + def update_demo() -> None: + nonlocal current_file + progress.update_progress(current_file) + current_file = 2 if current_file > 54 else current_file + 1 + root.after(1000, update_demo) + + # Start demo updates + update_demo() + + root.mainloop() + + +if __name__ == "__main__": + main() diff --git a/tests/test_frontend_tkinter_stage_progress.py b/tests/test_frontend_tkinter_stage_progress.py new file mode 100755 index 000000000..453b3cf32 --- /dev/null +++ b/tests/test_frontend_tkinter_stage_progress.py @@ -0,0 +1,166 @@ +#!/usr/bin/env python3 + +""" +Tests for the StageProgressBar class. + +This file is part of Ardupilot methodic configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2025 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +import tkinter as tk +import unittest +from unittest.mock import patch + +from ardupilot_methodic_configurator.frontend_tkinter_stage_progress import StageProgressBar + + +class TestStageProgressBar(unittest.TestCase): + """Test cases for the StageProgressBar class.""" + + def setUp(self) -> None: + """Set up test fixtures.""" + self.root = tk.Tk() + self.test_phases = { + "Phase 1": {"start": 1, "description": "First phase"}, + "Phase 2": {"start": 10, "description": "Second phase", "optional": True}, + "Phase 3": {"start": 20}, + "Milestone": {"description": "A milestone without start"}, + } + self.total_steps = 30 + self.progress_bar = StageProgressBar(self.root, self.test_phases, self.total_steps) + + def tearDown(self) -> None: + """Clean up after tests.""" + self.root.destroy() + + def test_initialization(self) -> None: + """Test proper initialization of StageProgressBar.""" + assert self.progress_bar.total_files == self.total_steps + assert self.progress_bar.phases == self.test_phases + + # Should only create frames for phases with start positions + assert len(self.progress_bar.phase_frames) == 3 + assert "Milestone" not in self.progress_bar.phase_frames + + def test_phase_ordering(self) -> None: + """Test that phases are ordered correctly by start position.""" + phase_names = list(self.progress_bar.phase_frames.keys()) + assert phase_names == ["Phase 1", "Phase 2", "Phase 3"] + + def test_progress_updates(self) -> None: + """Test progress bar updates for different file positions.""" + test_cases = [ + (5, [4, 0, 0]), # In Phase 1 (1-9) + (15, [9, 5, 0]), # In Phase 2 (10-19) + (25, [9, 10, 5]), # In Phase 3 (20-30) + (29, [9, 10, 9]), # Still in Phase 3, but not beyond total + ] + + for current_file, expected_values in test_cases: + self.progress_bar.update_progress(current_file) + for test_bar, expected in zip(self.progress_bar.phase_bars, expected_values): + assert test_bar["bar"]["value"] == expected + + def test_invalid_progress_update(self) -> None: + """Test handling of invalid progress updates.""" + with self.assertLogs(level="ERROR"): + self.progress_bar.update_progress(0) + self.progress_bar.update_progress(31) + + @patch("tkinter.ttk.Label.configure") + def test_resize_handling(self, mock_configure) -> None: + """Test window resize handling.""" + self.root.geometry("400x300") + self.progress_bar._on_resize() # pylint: disable=protected-access + + # Should update wraplength for all phase labels + expected_calls = len(self.progress_bar.phase_frames) + assert mock_configure.call_count == expected_calls + + def test_optional_phase_styling(self) -> None: + """Test that optional phases are styled correctly.""" + optional_frame = self.progress_bar.phase_frames["Phase 2"] + normal_frame = self.progress_bar.phase_frames["Phase 1"] + + # Find labels in frames + optional_label = None + normal_label = None + for child in optional_frame.winfo_children(): + if isinstance(child, tk.ttk.Label): + optional_label = child + for child in normal_frame.winfo_children(): + if isinstance(child, tk.ttk.Label): + normal_label = child + + assert str(optional_label.cget("foreground")) == "gray" + assert str(normal_label.cget("foreground")) == "black" + + def test_phase_frame_creation(self) -> None: + """Test that phase frames are created with correct structure.""" + test_frame = self.progress_bar.phase_frames["Phase 1"] + + # Check frame structure + children = test_frame.winfo_children() + progressbar = None + label = None + for child in children: + if isinstance(child, tk.ttk.Progressbar): + progressbar = child + elif isinstance(child, tk.ttk.Label): + label = child + + assert progressbar is not None, "Progressbar should exist" + assert label is not None, "Label should exist" + assert str(progressbar.cget("orient")) == "horizontal" + assert str(progressbar.cget("mode")) == "determinate" + + def test_progress_bar_limits(self) -> None: + """Test progress bar maximum values are set correctly.""" + for i, phase in enumerate(self.progress_bar.phase_bars): + test_bar = phase["bar"] + start = phase["start"] + end = phase["end"] + + # Check maximum is correctly set to phase range + assert test_bar.cget("maximum") == end - start, f"Phase {i + 1} maximum should be {end - start}" + + def test_label_text_splitting(self) -> None: + """Test text splitting behavior for different label lengths.""" + test_texts = { + "A": {"start": 1}, # Very short + "Short": {"start": 2}, # Short + "Medium Text": {"start": 3}, # Medium with space + "MediumNoSpace": {"start": 4}, # Medium without space + "Very Long Phase Name Example": {"start": 5}, # Long with spaces + } + + test_progress = StageProgressBar(self.root, test_texts, 10) + + for phase_name, frame in test_progress.phase_frames.items(): + label = None + for child in frame.winfo_children(): + if isinstance(child, tk.ttk.Label): + label = child + break + + assert label is not None + text = label.cget("text") + + if len(phase_name) <= 1: + # Very short text should have padding newline + assert text.endswith("\n "), f"Very short text '{phase_name}' missing padding newline" + elif len(phase_name) < 6: + # Short text should have newline + assert "\n" in text, f"Short text '{phase_name}' missing newline" + elif " " in phase_name and len(phase_name) < 20: + # Medium text with space should split at first space + first_space_pos = phase_name.find(" ") + expected_newline_pos = text.find("\n") + assert expected_newline_pos == first_space_pos, f"Medium text '{phase_name}' not split at first space" + + +if __name__ == "__main__": + unittest.main()