diff --git a/ARCHITECTURE_3_directory_selection.md b/ARCHITECTURE_3_directory_selection.md index b8de2f87d..7dc7cb585 100644 --- a/ARCHITECTURE_3_directory_selection.md +++ b/ARCHITECTURE_3_directory_selection.md @@ -2,10 +2,9 @@ ## Overview -The Vehicle Configuration Directory Selection sub-application allows users to either create a new vehicle -configuration project or open an existing one. It manages the selection and creation of vehicle directories, -handles template selection, and downloads parameter documentation metadata corresponding to the flight -controller firmware version to the project directory. +The Vehicle Configuration Directory Selection sub-application allows users to create a new vehicle +configuration project, import one from a configured flight controller or `.bin` log, or open an existing +project. It manages directory selection and downloads parameter documentation for the project firmware. The architecture follows a clean layered design with dependency injection, where the frontend components depend on the VehicleProjectManager factory/container class, which provides a unified interface to all @@ -116,7 +115,7 @@ This architecture ensures: ![frontend_tkinter_project_opener](images/App_screenshot_Vehicle_directory10.png) - **File**: `frontend_tkinter_project_opener.py` -- **Purpose**: Main interface for opening existing vehicle projects and launching new project creation +- **Purpose**: Main interface for opening existing projects and launching new project creation - **Responsibilities**: - Present three main options: Create New, Open Existing, Re-open Last Used - Handle user interactions and directory selection through callback patterns @@ -130,9 +129,10 @@ This architecture ensures: ![frontend_tkinter_project_creator](images/App_screenshot_Vehicle_directory11.png) - **File**: `frontend_tkinter_project_creator.py` -- **Purpose**: Dedicated interface for creating new vehicle projects from templates +- **Purpose**: Dedicated interface for creating new vehicle projects from templates or a configured flight controller - **Responsibilities**: - - Present template selection and project configuration options + - Present template selection and project configuration options for template-based creation + - Present destination controls for configured-flight-controller creation - Handle new project settings and customization dynamically based on flight controller connection state - Coordinate template selection through TemplateOverviewWindow - Delegate project creation to VehicleProjectManager @@ -179,18 +179,17 @@ This architecture ensures: #### Project Creation Services - **File**: `data_model_vehicle_project_creator.py` -- **Purpose**: Handle creation of new vehicle projects from templates +- **Purpose**: Handle creation of new vehicle projects from templates and imported FC data - **Responsibilities**: - - Template copying and customization (with optional transformations in a single pass) + - Template copying and customization - Project directory initialization - - Optional one-time import of FC parameter values when `use_fc_params=True` + - Optional substitution of FC parameter values and import of values not represented by the template - Configuration file setup - Project metadata creation - **Access**: Through VehicleProjectManager factory methods -When `use_fc_params=True` and FC parameters are available, `LocalFilesystem.copy_template_files_to_new_vehicle_dir()` -applies both `blank_change_reason` and FC value substitution in a single `ParDict`-based parse-modify-write -pass per `.param` file, before `re_init` loads the directory. +When FC values are supplied, template parameter files are transformed during copying; the manager then persists +firmware metadata and writes any remaining values to a numbered import file before opening the project. #### Project Opening Services @@ -237,50 +236,27 @@ The data flow follows the layered architecture pattern with clear separation of 2. **Project Selection Flow** - User interacts with VehicleProjectOpenerWindow main interface - - Three options presented: Create New, Open Existing, Re-open Last Used - - For new projects: VehicleProjectOpenerWindow launches VehicleProjectCreatorWindow + - New-project choices include template, configured-flight-controller, and `.bin` log creation + - VehicleProjectOpenerWindow launches VehicleProjectCreatorWindow for template or configured-FC creation - For existing projects: callback functions handle directory selection through VehicleDirectorySelectionWidgets 3. **New Project Creation Flow** - VehicleProjectCreatorWindow instantiated with project_manager reference - - User optionally enables `use_fc_params` - - VehicleProjectCreator creates the directory from template - - During the copy step, FC source values and/or blank-change-reason are applied in a single - `ParDict`-based pass so that `re_init` reads fully-initialized files with no further writes - - Frontend presents template selection and project configuration options - - User selects template through TemplateOverviewWindow integration - - Frontend delegates to `project_manager.create_new_vehicle_from_template()` - - VehicleProjectManager coordinates with project creator services - - Project creation delegated to specialized creator services - - Success/failure feedback provided through manager interface + - Template creation presents a selectable template and optional settings + - Configured-FC creation resolves the matching empty firmware template automatically and uses live FC values + - VehicleProjectManager delegates copying to VehicleProjectCreator, persists project metadata, imports any + remaining FC values, opens the project, and updates recent-directory history + - Success and failure feedback is provided through the manager interface 4. **.bin Log Import Flow** - User clicks *Create a vehicle project from a .bin log file* in VehicleProjectOpenerWindow (option-1 panel, via `BinLogSelectionWidgets`) - Frontend opens a file-picker; user selects a `.bin` ArduPilot log file - Frontend delegates to `project_manager.create_new_vehicle_from_bin_log(bin_file)` - - `VehicleProjectManager` orchestrates the following steps via `VehicleProjectCreator`: - 1. `extract_firmware_version_from_bin_log()` — reads the `VER` (or `MSG`) record from the - log to determine vehicle type (e.g. `ArduCopter`) and firmware version (e.g. `4.6.3`); - `pymavlink` is imported lazily at call time so it does not slow application startup - 2. `template_dir_for_bin_import()` — resolves and validates the matching template directory - (e.g. `ArduCopter/empty_4.6.x`); raises a user-friendly error if none is installed - 3. `create_new_vehicle_from_template()` — copies the template with `fc_connected=False` - so no live FC values are injected - 4. `extract_param_files_from_bin_log()` — extracts default and current parameter values - from the log's `PARM` messages; also imported lazily - 5. `LocalFilesystem.fw_version` is set to `"major.minor.patch"` before `re_init()` is - called, preventing the template's placeholder version from being used - 6. `re_init()` — points the filesystem at the new vehicle directory - 7. `set_fc_fw_version_and_type_in_components_json()` — persists the detected firmware - version and type into `vehicle_components.json` - 8. `write_param_default_values_to_file()` — overwrites `00_default.param` with the - log-extracted defaults - 9. Parameters present in the log but absent/different in the template files are exported - to `xx_imported_bin_log_parameters.param`; `re_init()` is called again to pick up - the new file - 10. Manager state (`_settings`, `configuration_template`, recent-dir history) is updated - only after `open_vehicle_directory()` succeeds, ensuring manager metadata - is committed only on success + - `VehicleProjectManager` asks `VehicleProjectCreator` to extract firmware/default/current parameters, + resolve the matching empty template, and copy it without live-FC injection + - The shared import finalization persists firmware metadata, replaces `00_default.param` with extracted + defaults, and exports remaining values to a numbered import file + - The project is opened and manager history/state are updated only after file preparation succeeds - Success/failure feedback provided through manager interface; on failure the new directory is not registered in session history and manager in-memory state is not updated, though filesystem changes to the new project directory are not rolled back @@ -294,14 +270,7 @@ The data flow follows the layered architecture pattern with clear separation of - Project state is reconstructed and validated - Error handling managed through consistent interface -6. **Architecture Benefits** - - Frontend never directly accesses backend services - - All business logic centralized in VehicleProjectManager - - Easy to test with mock VehicleProjectManager - - Changes to backend services don't affect frontend code - - Clean separation between project opening and project creation concerns - -7. **Recent Directories History Flow** +6. **Recent Directories History Flow** - On application startup, `ProgramSettings.get_recent_vehicle_dirs()` loads history from settings.json; the history is passed through the manager - History is passed to VehicleProjectOpenerWindow to populate the combobox widget - User selects a directory from the combobox dropdown @@ -571,34 +540,3 @@ The architecture implements dependency injection where: - **Better Testing**: Each layer can be tested in isolation with appropriate mocks - **Code Reuse**: VehicleProjectManager can be used by multiple frontend components - **Consistent Interface**: All vehicle project operations go through unified interface - -## Template System Features - -### Template Categories - -- **Vehicle Type Based**: Organized by ArduPilot vehicle type -- **Size Categories**: Small, medium, large vehicle templates -- **Application Specific**: Racing, photography, mapping, etc. -- **Hardware Specific**: Specific flight controller or component combinations - -### Template Validation - -- Schema validation for all template configuration files -- Parameter file syntax checking -- Dependency verification between configuration steps -- Compatibility checking with different firmware versions - -### Template Customization - -- User can modify templates after copying -- Support for local template libraries -- Template versioning and update mechanisms -- Template sharing and import/export functionality - -## Performance Optimization - -- Lazy loading of template metadata -- Efficient directory scanning algorithms -- Parallel file operations where safe -- Caching of frequently accessed templates -- Progress reporting for long operations diff --git a/TUNING_GUIDE_ArduCopter.md b/TUNING_GUIDE_ArduCopter.md index d3ff05629..05e455137 100644 --- a/TUNING_GUIDE_ArduCopter.md +++ b/TUNING_GUIDE_ArduCopter.md @@ -133,8 +133,8 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl 1. Connect the flight controller to the PC via a USB cable and wait 7 seconds. 1. Open *ArduPilot Methodic Configurator*, and [connect it to the vehicle](USERMANUAL.md#step-1-flight-controller-connection).
![FC connection and parameter download](images/App_screenshot_FC_info_and_param_download.png) -1. Press the *Create a vehicle configuration directory from template* button.
- ![Create a vehicle configuration directory from template](images/App_screenshot_Vehicle_directory_create_from_template.png) +1. Press the *Create a vehicle project from a template* button.
+ ![Create a vehicle project from a template](images/App_screenshot_Vehicle_directory_create_from_template.png) 1. Now using [New vehicle](USERMANUAL.md#create-a-new-vehicle-configuration-directory) subsection
![New vehicle](images/App_screenshot_Vehicle_directory_create_from_template_source.png) 1. From the existing templates, select the one most similar to your vehicle:
@@ -152,7 +152,7 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl creating a new vehicle configuration directory from a template. Only makes sense if your FC has already been correctly configured. This option is only available when a flight controller is connected. - *Blank parameter change reason* - Do not use the parameters change reason from the template. -1. Select the destination directory, give it a name, and press the *Create a vehicle configuration directory from template* button. +1. Select the destination directory, give it a name, and press the *Create a vehicle project from a template* button. 1. On the component editor window, **add all the details of the components of your system** as we did in [Section 1.2](#12-our-example-vehicle):
![Component editor window](images/App_screenshot_Component_Editor.png) @@ -516,6 +516,11 @@ The `Change Reason` field is extremely important because: In our setup, we used an advanced RC receiver that cannot be fully configured using Mission Planner's `SETUP >> Mandatory Hardware >> Radio Calibration` menu. +On some flight controllers, it might be necessary to add and change the `BRD_ALT_CONFIG` +parameter, in order to be able to connect the RC receiver or ESC telemetry to a pin that can decode it. +This is necessary if not enough serial ports are available when `BRD_ALT_CONFIG==0` or +when DMA-capable servo-outputs conflict with serial ports. + Repeat the steps from [Section 6.1.1](#611-use-ardupilot-methodic-configurator-to-edit-the-parameter-file-and-upload-it-to-the-flight-controller) to edit and upload the `06_remote_controller_receiver.param` file ### 6.2.2 Configure the RC controller @@ -547,6 +552,11 @@ Once this is operating we no longer need the USB connection to the vehicle. We c In our setup, we used a [Bi-directional Dshot ESC](https://ardupilot.org/copter/docs/common-dshot-escs.html) that cannot be fully configured using Mission Planner's `SETUP >> Mandatory Hardware >> Servo Output` menu. +On some flight controllers, it might be necessary to add and change the `BRD_ALT_CONFIG` +parameter, in order to be able to connect the RC receiver or ESC telemetry to a pin that can decode it. +This is necessary if not enough serial ports are available when `BRD_ALT_CONFIG==0` or +when DMA-capable servo-outputs conflict with serial ports. + Repeat the steps from [Section 6.1.1](#611-use-ardupilot-methodic-configurator-to-edit-the-parameter-file-and-upload-it-to-the-flight-controller) to edit and upload the `09_esc_telemetry.param` file The step above configured ESC communication passthrough. diff --git a/TUNING_GUIDE_ArduPlane.md b/TUNING_GUIDE_ArduPlane.md index fb24f5073..bcc5c025d 100644 --- a/TUNING_GUIDE_ArduPlane.md +++ b/TUNING_GUIDE_ArduPlane.md @@ -133,8 +133,8 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl 1. Connect the flight controller to the PC via a USB cable and wait 7 seconds. 1. Open *ArduPilot Methodic Configurator*, and [connect it to the vehicle](USERMANUAL.md#step-1-flight-controller-connection).
![FC connection and parameter download](images/App_screenshot_FC_info_and_param_download.png) -1. Press the *Create a vehicle configuration directory from template* button.
- ![Create a vehicle configuration directory from template](images/App_screenshot_Vehicle_directory_create_from_template.png) +1. Press the *Create a vehicle project from a template* button.
+ ![Create a vehicle project from a template](images/App_screenshot_Vehicle_directory_create_from_template.png) 1. Now using [New](USERMANUAL.md#new) subsection
![New vehicle](images/App_screenshot_Vehicle_directory_create_from_template_source.png) 1. From the existing templates, select the one most similar to your vehicle:
@@ -152,7 +152,7 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl creating a new vehicle configuration directory from a template. Only makes sense if your FC has already been correctly configured. This option is only available when a flight controller is connected. - *Blank parameter change reason* - Do not use the parameters change reason from the template. -1. Select the destination directory, give it a name, and press the *Create a vehicle configuration directory from template* button. +1. Select the destination directory, give it a name, and press the *Create a vehicle project from a template* button. 1. On the component editor window, **add all the details of the components of your system** as we did in [Section 1.2](#12-our-example-vehicle):
![Component editor window](images/App_screenshot_Component_Editor.png) diff --git a/TUNING_GUIDE_Heli.md b/TUNING_GUIDE_Heli.md index 09c700fce..b2e68c851 100644 --- a/TUNING_GUIDE_Heli.md +++ b/TUNING_GUIDE_Heli.md @@ -132,8 +132,8 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl 1. Connect the flight controller to the PC via a USB cable and wait 7 seconds. 1. Open *ArduPilot Methodic Configurator*, and [connect it to the vehicle](USERMANUAL.md#step-1-flight-controller-connection).
![FC connection and parameter download](images/App_screenshot_FC_info_and_param_download.png) -1. Press the *Create a vehicle configuration directory from template* button.
- ![Create a vehicle configuration directory from template](images/App_screenshot_Vehicle_directory_create_from_template.png) +1. Press the *Create a vehicle project from a template* button.
+ ![Create a vehicle project from a template](images/App_screenshot_Vehicle_directory_create_from_template.png) 1. Now using [New](USERMANUAL.md#new) subsection
![New vehicle](images/App_screenshot_Vehicle_directory_create_from_template_source.png) 1. From the existing templates, select the one most similar to your vehicle:
@@ -151,7 +151,7 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl creating a new vehicle configuration directory from a template. Only makes sense if your FC has already been correctly configured. This option is only available when a flight controller is connected. - *Blank parameter change reason* - Do not use the parameters change reason from the template. -1. Select the destination directory, give it a name, and press the *Create a vehicle configuration directory from template* button. +1. Select the destination directory, give it a name, and press the *Create a vehicle project from a template* button. 1. On the component editor window, **add all the details of the components of your system** as we did in [Section 1.2](#12-our-example-vehicle):
![Component editor window](images/App_screenshot_Component_Editor.png) diff --git a/TUNING_GUIDE_Rover.md b/TUNING_GUIDE_Rover.md index 2aa64e532..4177f2d0d 100644 --- a/TUNING_GUIDE_Rover.md +++ b/TUNING_GUIDE_Rover.md @@ -133,8 +133,8 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl 1. Connect the flight controller to the PC via a USB cable and wait 7 seconds. 1. Open *ArduPilot Methodic Configurator*, and [connect it to the vehicle](USERMANUAL.md#step-1-flight-controller-connection).
![FC connection and parameter download](images/App_screenshot_FC_info_and_param_download.png) -1. Press the *Create a vehicle configuration directory from template* button.
- ![Create a vehicle configuration directory from template](images/App_screenshot_Vehicle_directory_create_from_template.png) +1. Press the *Create a vehicle project from a template* button.
+ ![Create a vehicle project from a template](images/App_screenshot_Vehicle_directory_create_from_template.png) 1. Now using [New](USERMANUAL.md#new) subsection
![New vehicle](images/App_screenshot_Vehicle_directory_create_from_template_source.png) 1. From the existing templates, select the one most similar to your vehicle:
@@ -152,7 +152,7 @@ So, [start the ArduPilot Methodic Configurator and select a vehicle that resembl creating a new vehicle configuration directory from a template. Only makes sense if your FC has already been correctly configured. This option is only available when a flight controller is connected. - *Blank parameter change reason* - Do not use the parameters change reason from the template. -1. Select the destination directory, give it a name, and press the *Create a vehicle configuration directory from template* button. +1. Select the destination directory, give it a name, and press the *Create a vehicle project from a template* button. 1. On the component editor window, **add all the details of the components of your system** as we did in [Section 1.2](#12-our-example-vehicle):
![Component editor window](images/App_screenshot_Component_Editor.png) diff --git a/USECASES.md b/USECASES.md index 148573bda..15c154068 100644 --- a/USECASES.md +++ b/USECASES.md @@ -34,8 +34,8 @@ as this would require reconfiguring everything from scratch. 1. Open the *ArduPilot Methodic Configurator* software. 1. The software should now automatically detect and connect to your flight controller.
![AMC FC info and parameter download](images/App_screenshot_FC_info_and_param_download.png) -1. Press the *Create a vehicle configuration directory from template* button.
- ![Create a vehicle configuration directory from template](images/App_screenshot_Vehicle_directory_create_from_template.png) +1. Press the *Create a vehicle project from a template* button.
+ ![Create a vehicle project from a template](images/App_screenshot_Vehicle_directory_create_from_template.png) 1. Select source template directory to use.
![AMC template selection1](images/App_screenshot_Vehicle_directory_create_from_template_source.png) 1. Select the vehicle template that better resembles your vehicle, it does not need to fully match your vehicle.
@@ -55,7 +55,7 @@ as this would require reconfiguring everything from scratch. - *Blank parameter change reason* - Do not use the parameters change reason from the template. 1. Give a name to your vehicle.
![AMC new vehicle name](images/App_screenshot_Vehicle_directory_create_from_template_name.png) -1. Press *Create a vehicle configuration directory from template*.
+1. Press *Create a vehicle project from a template*.
![AMC new vehicle create](images/App_screenshot_Vehicle_directory_create_from_template_create.png) 1. Edit all the components of your vehicle to match your own in the *Vehicle Component Editor* window. Please scroll down and make sure you do not miss a property.
@@ -137,26 +137,15 @@ If something is not clear, read the [ArduPilot Methodic Configurator user manual 1. Open the *ArduPilot Methodic Configurator* software. 1. The software should now automatically detect and connect to your flight controller.
![AMC FC info and parameter download](images/App_screenshot_FC_info_and_param_download.png) -1. Press the *Create a vehicle configuration directory from template* button.
- ![Create a vehicle configuration directory from template](images/App_screenshot_Vehicle_directory_create_from_template.png) -1. Select the vehicle template that better resembles your vehicle.
- ![AMC template selection1](images/App_screenshot_Vehicle_directory_vehicle_params0.png) - ![AMC template selection2](images/App_screenshot_Vehicle_overview.png) -1. **select the `Infer component specifications and FC connections from FC Parameters, not from template files` - and the `Use parameter values from connected FC, not from template files` checkboxes** - - *Infer component specifications and FC connections from FC parameters, not from template files* - When creating a new vehicle configuration, - extract component specifications and connection information directly from the connected flight controller instead of using the specifications - defined in the template files. - This helps ensure the configuration accurately matches your actual hardware. - Note: you will not see the information from the correctly configured vehicle template. This option is only available when a flight controller is connected. - - *Use parameter values from connected FC, not from template files* - Use the parameter values from the connected flight controller instead of - the template files when creating a new vehicle configuration directory from a template. - Only makes sense if your FC has already been correctly configured. This option is only available when a flight controller is connected.
- ![AMC template options](images/App_screenshot_Vehicle_directory_create_from_configured_options.png) -1. Give a name to your vehicle.
- ![AMC new vehicle name](images/App_screenshot_Vehicle_directory_create_from_configured_name.png) -1. Press *Create a vehicle configuration directory from template*.
- ![AMC new vehicle create](images/App_screenshot_Vehicle_directory_create_from_configured_create.png) +1. In the **New** panel, click **Create a vehicle project from an already configured flight controller**.
+ ![Create a vehicle project from an already configured flight controller](images/App_screenshot_Vehicle_directory_create_from_flight_controller.png) +1. Select the **Destination base directory** and enter the **Destination new vehicle name**.
+ ![Create vehicle project from an already configured flight controller](images/App_screenshot_Vehicle_directory_create_from_flight_controller_creator.png) +1. Click **Create a vehicle project from an already configured flight controller**. + The software automatically selects the empty template matching the flight controller's vehicle type and + firmware version, and imports the component information and parameter values from the connected vehicle. + The FC's default values are written to `00_default.param`; values that differ from the template/default + baseline are written to `xx_imported_flight_controller_parameters.param` for review. 1. Edit all the components of your vehicle to match your own in the *Vehicle Component Editor* window.
![AMC component editor](images/App_screenshot_Component_Editor.png) 1. Press *Save data and start configuration*. @@ -165,7 +154,7 @@ If something is not clear, read the [ArduPilot Methodic Configurator user manual Correct those entries and press the `Save data and start configuration` button again. 1. You should now see the *Parameter file editor and uploader* window.
![AMC parameter file editor and uploader](images/App_screenshot2.png) -1. Follow the procedure to [configure the vehicle parameters](USERMANUAL.md#step-4-parameter-file-editor-and-uploader-interface) +1. Follow the procedure to [configure the vehicle parameters](USERMANUAL.md#step-4-parameter-file-editor-and-uploader-interface). If something is not clear, read the [ArduPilot Methodic Configurator user manual](USERMANUAL.md) diff --git a/USERMANUAL.md b/USERMANUAL.md index ed89f0106..60f27a53a 100644 --- a/USERMANUAL.md +++ b/USERMANUAL.md @@ -43,33 +43,36 @@ flowchart TD subgraph "Step 2: Select Project" C --> E{Existing Project?} E -->|Yes| F[Open Vehicle Directory] - E -->|No| G[Select Template] - G --> H[Create New Project] - F --> I[Component Editor] - H --> I + E -->|No| G{Configured FC?} + G -->|Yes| H[Create from Configured FC] + G -->|No| I[Select Template] + I --> J[Create New Project] + F --> K[Component Editor] + H --> K + J --> K end subgraph "Step 3: Edit FC Components" - I --> J[Validate Components] - J --> K{Valid?} - K -->|No| I - K -->|Yes| L[Parameter Editor] + K --> L[Validate Components] + L --> M{Valid?} + M -->|No| K + M -->|Yes| N[Parameter Editor] end subgraph "Step 4: Edit FC Parameters" - L --> M[Configure Parameters] - M --> N[Upload to FC] - N --> O{Experiment Required?} - O -->|Yes| P[Close AMC] - P --> Q[Perform Experiment/Flight] - Q --> R[Start AMC] - R --> S[Read Results from FC] - S --> T[Write Results to File] - T --> U{More Files?} - O -->|No| U - U -->|Yes| L - U -->|No| V[Generate Summary] - V --> W[Configuration Complete] + N --> O[Configure Parameters] + O --> P[Upload to FC] + P --> Q{Experiment Required?} + Q -->|Yes| R[Close AMC] + R --> S[Perform Experiment/Flight] + S --> T[Start AMC] + T --> U[Read Results from FC] + U --> V[Write Results to File] + V --> W{More Files?} + Q -->|No| W + W -->|Yes| N + W -->|No| X[Generate Summary] + X --> Y[Configuration Complete] end ``` @@ -168,7 +171,23 @@ It provides three main options for selecting a vehicle directory: #### New -Create a new vehicle configuration directory, either from a template or from a `.bin` log file. +Create a new vehicle configuration directory from a template, an already configured flight controller, or a `.bin` log file. + +When a correctly configured flight controller is connected, you can create a project directly from it: + +- Click **Create a vehicle project from an already configured flight controller** in the **New** panel. +- In the creator window, select the **Destination base directory** and enter the **Destination new vehicle name**. +- The software automatically selects the empty template matching the flight controller's vehicle type and major/minor firmware version. +- Component information and parameter values are taken from the connected flight controller. +- The FC's default parameter values are written to `00_default.param`. +- If live FC values differ from the selected template/default baseline, they are written to an + `xx_imported_flight_controller_parameters.param` file for review. + +![Create vehicle from an already configured flight controller](images/App_screenshot_Vehicle_directory_create_from_flight_controller.png) +
+
+ Create vehicle from an already configured flight controller +
#### Open @@ -200,7 +219,26 @@ It's useful for setting up a new vehicle configuration quickly. will use the parameter values from the flight controller instead. - Use the "Destination base directory" `...` button to select the existing directory where the new vehicle directory will be created. - Enter the name for the new vehicle directory in the "Destination new vehicle name" field. -- Click the "Create vehicle directory from template" button to create the new vehicle directory on the base directory and copy the template files to it. +- Click the "Create a vehicle project from a template" button to create the new vehicle directory on the base directory and copy the template files to it. + +### Create a New Vehicle Configuration Directory from an Already Configured Flight Controller + +This workflow is intended for a vehicle whose flight controller is already configured. +It creates a new project using the live flight controller values and hardware information, while selecting the matching empty firmware template automatically. + +![Create vehicle project from an already configured flight controller](images/App_screenshot_Vehicle_directory_create_from_flight_controller_creator.png) +
+
+ Create vehicle project from an already configured flight controller +
+ +1. In the **New** panel, click **Create a vehicle project from an already configured flight controller**. +1. Select the destination base directory. +1. Enter a name for the new vehicle directory. +1. Click the create button. AMC copies the matching empty template, substitutes the downloaded + FC parameter values, and derives component specifications and connections from those values. +1. AMC writes the FC's default values to `00_default.param`. Values that differ from the template + baseline are written to `xx_imported_flight_controller_parameters.param`. ### Create a New Vehicle Configuration Directory from a .bin Log File diff --git a/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json b/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json index a8580bf20..1de6cf8b7 100644 --- a/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json +++ b/ardupilot_methodic_configurator/configuration_steps_ArduCopter.json @@ -11,7 +11,7 @@ "external_tool_url": "", "mandatory_text": "80% mandatory (20% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["BRD_HEAT_.*", "INS_TCAL_OPTIONS", "TCAL_ENABLED"], + "autoimport_nondefault_regexp": ["BRD_HEAT_.*", "INS_RAW_LOG_OPT", "INS_TCAL[1-3]_(ENABLE|TMAX|TMIN)", "INS_TCAL_OPTIONS", "TCAL_ENABLED"], "forced_parameters": { "INS_TCAL1_ENABLE": { "New Value": 2, "Change Reason": "Activates the temperature calibration for IMU 1 at the next start" }, "LOG_BITMASK": { "New Value": 524416, "Change Reason": "Only for IMU and Raw-IMU" }, @@ -138,6 +138,7 @@ "04_imu_temperature_calibration_finish.param": { "why": "After calibrating the IMU temperature bias, disable disarmed logging (no longer needed) and set board target temperature.", "why_now": "Can only be done after IMU temperature compensation results are available. Future steps will use a drift-compensated, constant temperature and only log when armed.", + "autoimport_nondefault_regexp": ["BRD_HEAT_LOWMGN"], "blog_text": "Finish IMU (Inertial Measurement Unit) temperature calibration", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#42-calculate-imu-temperature-calibration", "wiki_text": "IMU Temperature Calibration", @@ -191,7 +192,7 @@ "mandatory_text": "100% mandatory (0% optional)", "component": "RC Receiver", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["FS_THR_VALUE", "RC_.*"], + "autoimport_nondefault_regexp": ["BRD_ALT_CONFIG", "FS_THR_VALUE", "RC_.*", "RSSI_TYPE"], "derived_parameters": { "RC_PROTOCOLS": { "New Value": "vehicle_components['RC Receiver']['FC Connection']['Protocol']", "Change Reason": "Selected in the component editor" }, "FLTMODE_CH": { "if": "vehicle_components['RC Receiver']['FC Connection']['Protocol'] == 'ExpressLRS'", "New Value": "6", "Change Reason": "ExpressLRS requires FLTMODE_CH != 5" } @@ -202,6 +203,7 @@ "07_remote_controller_controller.param": { "why": "Configuring RC controller options ensures correct arming behavior and channel function assignments for the specific RC system in use", "why_now": "After the RC receiver protocol is configured, the controller-specific options such as arming method and channel function can be set", + "autoimport_nondefault_regexp": ["ARMING_RUDDER", "PILOT_THR_BHV", "RC[0-9]+_.+"], "blog_text": "Configure RC controller options including the arming method and RC channel option assignments", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#622-configure-the-rc-controller", "wiki_text": "Radio Control Systems", @@ -288,6 +290,7 @@ "mandatory_text": "100% mandatory (0% optional)", "component": "Battery Monitor", "auto_changed_by": "", + "autoimport_nondefault_regexp": ["BATT_AMP_OFFSET", "BATT_AMP_PERVLT", "BATT_CURR_PIN", "BATT_MONITOR", "BATT_VLT_OFFSET", "BATT_VOLT_MULT", "BATT_VOLT_PIN", "BATT[2-9]_AMP_OFFSET", "BATT[2-9]_AMP_PERVLT", "BATT[2-9]_CURR_PIN", "BATT[2-9]_MONITOR", "BATT[2-9]_VLT_OFFSET", "BATT[2-9]_VOLT_MULT", "BATT[2-9]_VOLT_PIN"], "derived_parameters": { "BATT_MONITOR": { "New Value": "vehicle_components['Battery Monitor']['FC Connection']['Protocol']", "Change Reason": "Selected in component editor window" }, "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" } @@ -321,7 +324,7 @@ "mandatory_text": "100% mandatory (0% optional)", "component": "Battery", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["BATT.*"], + "autoimport_nondefault_regexp": ["BATT.*", "MOT_BAT_VOLT_(MAX|MIN)"], "forced_parameters": { "BATT_FS_CRT_ACT": { "New Value": 1, "Change Reason": "Land ASAP" }, "BATT_FS_LOW_ACT": { "New Value": 2, "Change Reason": "Return and land at home or rally point" } @@ -339,7 +342,14 @@ "BATT_AMP_PERVLT": {}, "BATT_I2C_BUS": {}, "BATT_MONITOR": {}, - "BATT_VOLT_MULT": {} + "BATT_VOLT_MULT": {}, + "BATT_VLT_OFFSET": {}, + "BATT2_AMP_OFFSET": {}, + "BATT2_AMP_PERVLT": {}, + "BATT2_I2C_BUS": {}, + "BATT2_MONITOR": {}, + "BATT2_VOLT_MULT": {}, + "BATT2_VLT_OFFSET": {} }, "rename_connection": "vehicle_components['Battery Monitor']['FC Connection']['Type']", "old_filenames": ["08_batt1.param"] @@ -356,7 +366,7 @@ "mandatory_text": "100% mandatory (0% optional)", "component": "GNSS Receiver", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["BRD_BOOT_DELAY", "GPS.*", "GNSS.*"], + "autoimport_nondefault_regexp": ["BRD_BOOT_DELAY", "BRD_SAFETY_DEFLT", "CAN_.*", "GPS.*", "GNSS.*", "NTF_LED_TYPES", "SERIAL[34]_PROTOCOL", "(WPNAV_RADIUS|WP_RADIUS_M)$"], "derived_parameters": { "GPS_TYPE": { "if": "Version(vehicle_components['Flight Controller']['Firmware']['Version'].split(' ')[0]) < Version('4.6')", "New Value": "vehicle_components['GNSS Receiver']['FC Connection']['Protocol']", "Change Reason": "Defined in component editor" }, "GPS1_TYPE": { "if": "Version(vehicle_components['Flight Controller']['Firmware']['Version'].split(' ')[0]) >= Version('4.6')","New Value": "vehicle_components['GNSS Receiver']['FC Connection']['Protocol']", "Change Reason": "Defined in component editor" } @@ -379,8 +389,13 @@ } }, "13_initial_atc.param": { + "instructions_popup": { + "type": "warning", + "msg": "Only do this step once. It sets initial values that later steps will improve." + }, "why": "Propeller size has a big influence on the vehicle dynamics, this adapts the attitude controller response to it", "why_now": "Done before sensor calibration in Mission Planner to minimize the changes the user has to do in mission planner", + "autoimport_nondefault_regexp": ["ATC_ACC(EL)?_[PRY]_MAX$", "ATC_ANG_(RLL|PIT|YAW)_P$", "ATC_RAT_(PIT|RLL|YAW)_FLT[DET]$", "INS_ACCEL_FILTER$", "MOT_THST_(EXPO|HOVER)$"], "blog_text": "Initial attitude controller configuration depends on the vehicle's propeller size defined in the component editor window", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#67-initial-attitude-pid-gains-vehicle-size-dependent", "wiki_text": "Initial parameters calculator", @@ -450,6 +465,7 @@ "15_general_configuration.param": { "why": "The parameter defaults of some parameters are not suitable for the flight tests that will follow", "why_now": "Most parameters have been configured in previous steps. These are parameters that did not fit in any of the categories of the previous steps.", + "autoimport_nondefault_regexp": ["(ATC_)?ANGLE_MAX$", "AUTO_OPTIONS$", "BRD_RTC_TZ_MIN$", "EK3_.*", "INITIAL_MODE$", "INS_ACCEL_FILTER$", "INS_POS[12]_X$", "RTL_.*", "SCHED_LOOP_RATE$", "SERIAL[5-9]_.*"], "blog_text": "General configuration parameters for the vehicle, including flight modes and safety settings", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#69-general-configuration", "wiki_text": "", @@ -466,6 +482,7 @@ "16_safety_setup.param": { "why": "Safety parameters protect the vehicle and its surroundings by enabling arming checks, geofencing and failsafe actions. ESC slew rate limits protect the ESCs from desync events.", "why_now": "Before the first flight so that arming checks are enforced and the vehicle responds safely to abnormal conditions", + "autoimport_nondefault_regexp": ["ARMING_.*", "ATC_RATE_[PRY]_MAX", "ATC_RAT_(PIT|RLL|YAW)_(PDMX|SMAX)", "ATC_(SLEW_YAW|RATE_WPY_MAX)$", "BRD_SAFETYOPTION$", "FENCE_.*", "FS_.*", "RTL_ALT(_M)?$"], "blog_text": "Configure safety parameters including arming checks, geofence, failsafe actions and ESC slew rate limits", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#691-safety-setup", "wiki_text": "Arming Checks", @@ -483,6 +500,7 @@ "17_remote_id.param": { "why": "Some countries require a remote ID for drones to be flown legally.", "why_now": "Remote ID requires GNSS and air pressure to be configured before, and it must be set up before the first flight", + "autoimport_nondefault_regexp": ["ADSB_.*", "DID_.*"], "blog_text": "Set the remote ID for the vehicle, to comply with local laws if applicable", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#692-remote-id-aka-drone-id-optional", "wiki_text": "Remote ID (aka Drone ID)", @@ -537,7 +555,7 @@ "external_tool_url": "https://firmware.ardupilot.org/Tools/WebTools/ThrustExpo/", "mandatory_text": "100% mandatory (0% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["BRD_IO_DSHOT", "BRD_IO_ENABLE", "SERVO_.*"], + "autoimport_nondefault_regexp": ["BRD_IO_DSHOT", "BRD_IO_ENABLE", "MOT_(PWM_(MAX|MIN)|SPIN_(ARM|MAX|MIN)|THST_EXPO)", "NTF_.*", "SERVO[0-9]+_.+", "SERVO_.*"], "derived_parameters": { "MOT_PWM_MAX": { "if": "vehicle_components['ESC']['FC->ESC Connection']['Protocol'] not in ['Normal', 'Brushed', 'PWMAngle', 'PWMRange']", "New Value": "2000", "Change Reason": "Digital ESC protocol maximum is per definition 2000" }, "MOT_PWM_MIN": { "if": "vehicle_components['ESC']['FC->ESC Connection']['Protocol'] not in ['Normal', 'Brushed', 'PWMAngle', 'PWMRange']", "New Value": "1000", "Change Reason": "Digital ESC protocol minimum is per definition 1000" } @@ -579,7 +597,7 @@ "external_tool_url": "https://discuss.ardupilot.org/t/new-fft-filter-setup-and-review-web-tool/102572", "mandatory_text": "100% mandatory (0% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["FFT_.*", "INS_HNTCH[0-9]*_.*"], + "autoimport_nondefault_regexp": ["FFT_.*", "INS_HNTC[H2-4]_.*"], "forced_parameters": { "INS_HNTCH_ENABLE": { "New Value": 1, "Change Reason": "Use the first notch filter to filter the noise created by the motors/propellers" } }, @@ -624,10 +642,11 @@ "23_optional_pid_adjustment.param": { "instructions_popup": { "type": "info", - "msg": "This step is optional, only perform it if your vehicle is tiny, huge, or its motor outputs oscillate" + "msg": "This step is optional, only perform it if your vehicle is tiny, huge, or its motor outputs oscillate.\nIt must be repeated multiple times until outputs no longer oscillate." }, "why": "With very large or very small vehicles the default PID values are not suitable for the first flight", "why_now": "Most other parameters are done and these need to be corrected (depending on the vehicle size) before the first flight", + "autoimport_nondefault_regexp": ["ATC_INPUT_TC", "ATC_RAT_(PIT|RLL|YAW)_[DIP]$"], "blog_text": "Adjust the Proportional-Integral-Derivative (PID) controllers based on the vehicle size before the first flight", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#615-optional-pid-adjustment", "wiki_text": "Manual tuning of Roll and Pitch", @@ -641,6 +660,7 @@ "24_throttle_controller.param": { "why": "The throttle controller is crucial for maintaining altitude and controlling the vehicle's vertical movement.", "why_now": "After the first flight because it depends on the MOT_THST_HOVER parameter, before the second flight so that it can safely use the altitude controller", + "autoimport_nondefault_regexp": ["ATC_THR_MIX_MAN$", "MOT_BAT_VOLT_(MAX|MIN)$", "MOT_SPOOL_TIME$", "PSC_ACCZ_([IP]|SMAX)$", "PSC_D_ACC_([IP]|SMAX)$", "TKOFF_RPM_MIN$", "TKOFF_SLEW_TIME$"], "blog_text": "Use MOT_THST_HOVER value calculated during the first flight to set throttle controller PIDs", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#82-configure-the-throttle-controller", "wiki_text": "Test AltHold", @@ -648,7 +668,7 @@ "external_tool_text": "", "external_tool_url": "", "mandatory_text": "100% mandatory (0% optional)", - "auto_changed_by": "", + "auto_changed_by": "First flight in ALT_HOLD mode for at least 30 seconds. If you have not done it yet, close this application and go fly", "derived_parameters": { "ATC_THR_MIX_MAN": { "New Value": "0.5", "Change Reason": "Because ALTHOLD flight mode was used for more than 30 seconds to correctly learn the MOT_THST_HOVER value" }, "PSC_ACCZ_I": { "if": "Version(vehicle_components['Flight Controller']['Firmware']['Version'].split(' ')[0]) < Version('4.7')", "New Value": "2*fc_parameters['MOT_THST_HOVER']", "Change Reason": "Use 2 * MOT_THST_HOVER assuming MOT_THST_HOVER has been correctly learned" }, @@ -662,6 +682,10 @@ "old_filenames": ["19_throttle_controller.param", "20_throttle_controller.param"] }, "25_motor_notch_filter_results.param": { + "instructions_popup": { + "type": "info", + "msg": "After the flight, analyze the .bin in the filter review tool, change the values here and upload to the FC.\nRepeat the procedure until all post-filter noise peaks are below -50dB." + }, "why": "The notch filter(s) configuration depends on real-flight data.", "why_now": "real-flight data is only available after the first flight", "blog_text": "Configure the notch filter(s) based on the data collected from the first flight", @@ -672,12 +696,13 @@ "external_tool_url": "https://firmware.ardupilot.org/Tools/WebTools/FilterReview/", "mandatory_text": "100% mandatory (0% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["FFT_.*", "INS_GYRO_FILTER"], + "autoimport_nondefault_regexp": ["FFT_.*", "INS_GYRO_FILTER", "INS_HNTC[H2-4]_.*"], "old_filenames": ["18_notch_filter_results.param", "19_notch_filter_results.param"] }, "26_ekf_config.param": { "why": "Sometimes the weights of the barometer vs. GNSS altitude need to be adjusted.", "why_now": "Before the second flight so that the EKF can be used to estimate the vehicle's position", + "autoimport_nondefault_regexp": ["EK3_ACC_P_NSE", "EK3_ALT_M_NSE"], "blog_text": "Configure Extended Kalman Filter (EKF) noise weights", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#83-configure-the-ekf-altitude-source-weights", "wiki_text": "Extended Kalman filter tuning", @@ -698,7 +723,7 @@ "external_tool_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#612-motorpropeller-order-and-direction-test", "mandatory_text": "100% mandatory (0% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["INS_LOG_BAT_.+", "LOG_FILE_.+"], + "autoimport_nondefault_regexp": ["INS_LOG_BAT_.+", "LOG_BITMASK", "LOG_FILE_.+"], "forced_parameters": { "INS_LOG_BAT_OPT": { "New Value": "4", "Change Reason": "PID notch filters require batch pre- and post- filters logging" }, "INS_RAW_LOG_OPT": { "New Value": "0", "Change Reason": "PID notch filters require batch logging, not raw logging" }, @@ -720,7 +745,7 @@ "external_tool_url": "https://firmware.ardupilot.org/Tools/WebTools/FilterReview/", "mandatory_text": "100% mandatory (0% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["FILT.+", "ATC_RAT_.+_N[ET]F", "PSC_ACCZ_N[ET]F"], + "autoimport_nondefault_regexp": ["FILT.+", "ATC_RAT_.+_N[ET]F", "PSC_(ACCZ|D_ACC)_N[ET]F"], "add_parameters": { "ATC_RAT_RLL_NEF": { "if": "Version(vehicle_components['Flight Controller']['Firmware']['Version'].split(' ')[0]) >= Version('4.5')", "New Value": "0", "Change Reason": "" }, "ATC_RAT_RLL_NTF": { "if": "Version(vehicle_components['Flight Controller']['Firmware']['Version'].split(' ')[0]) >= Version('4.5')", "New Value": "0", "Change Reason": "" }, @@ -746,7 +771,7 @@ "external_tool_url": "https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Scripting/applets/VTOL-quicktune.md", "mandatory_text": "80% mandatory (20% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["QUIK_.*"], + "autoimport_nondefault_regexp": ["QUIK_.*", "SCR_ENABLE"], "forced_parameters": { "SCR_ENABLE": { "New Value": 1, "Change Reason": "Use lua scripting for VTOL-Quicktune" }, "QUIK_ENABLE": { "New Value": 1, "Change Reason": "Use VTOL-Quicktune lua script to estimate a good PID starting values" } @@ -758,6 +783,7 @@ "30_quick_tune_results.param": { "why": "The VTOL-quicktune lua script can safely estimate good PID starting values for the vehicle.", "why_now": "Before the second flight so that the vehicle can be safely tuned.", + "autoimport_nondefault_regexp": ["ATC_RAT_(PIT|RLL|YAW)_.*"], "blog_text": "Results of the in-flight VTOL-quicktune lua script PIDs before the MAGFit flight.", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#85-second-flight-pid-vtol-quiktune-lua-script-or-manual-pid-tune", "wiki_text": "If lua scripting is not possible, do a manual tune instead", @@ -800,7 +826,7 @@ "external_tool_url": "https://firmware.ardupilot.org/Tools/WebTools/MAGFit/", "mandatory_text": "80% mandatory (20% optional)", "auto_changed_by": "", - "autoimport_nondefault_regexp": ["MAGH_.*"], + "autoimport_nondefault_regexp": ["MAGH_.*", "SCR_ENABLE"], "forced_parameters": { "MAGH_LOG_ENABLE": { "New Value": 1, "Change Reason": "Activates the logging of the MAGH.Active message" }, "QUIK_ENABLE": { "New Value": 0, "Change Reason": "Quiktune is now completed. Disable it so that we can use the same RC switch to operate the wp-advance script" }, @@ -900,6 +926,7 @@ "33_evaluate_the_aircraft_tune_ff_disable.param": { "why": "Evaluating the aircraft's PID tuning and flight characteristics is best done with feed-forward disabled", "why_now": "Before the autotune process to estimate if autotune can safely operate the vehicle", + "autoimport_nondefault_regexp": ["ATC_RATE_FF_ENAB", "INS_LOG_BAT_.+", "LOG_BITMASK"], "blog_text": "Evaluate the aircraft's tuning with feed-forward control disabled", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#93-fifth-flight-evaluate-the-aircraft-tune---part-1", "wiki_text": "Evaluating the aircraft tune", @@ -922,6 +949,7 @@ "34_evaluate_the_aircraft_tune_ff_enable.param": { "why": "Evaluate the aircraft's PID tuning and flight characteristics with feed-forward enable to test faster dynamics", "why_now": "Before the autotune process to estimate if autotune can safely operate the vehicle", + "autoimport_nondefault_regexp": ["ATC_RATE_FF_ENAB"], "blog_text": "Evaluate the aircraft's tuning with feed-forward control enabled", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#94-sixth-flight-evaluate-the-aircraft-tune---part-2", "wiki_text": "Evaluating the aircraft tune", @@ -938,6 +966,7 @@ "35_autotune_roll_setup.param": { "why": "To optimize roll step response.", "why_now": "Because roll is usually the axis that has the highest dynamic, and we start with the highest dynamic axis", + "autoimport_nondefault_regexp": ["AUTOTUNE_AGGR", "AUTOTUNE_AXES"], "blog_text": "Set up parameters for the roll axis autotuning process", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#951-roll-axis-autotune", "wiki_text": "AutoTune", @@ -954,6 +983,7 @@ "36_autotune_roll_results.param": { "why": "To record autotune results after roll step response optimization", "why_now": "Because roll is usually the axis that has the highest dynamic, and we start with the highest dynamic axis", + "autoimport_nondefault_regexp": ["ATC_ACC(EL)?_R_MAX", "ATC_RAT_RLL_.*"], "blog_text": "Record the results of the roll axis autotuning, providing data for analysis and adjustment.", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#951-roll-axis-autotune", "wiki_text": "AutoTune", @@ -967,6 +997,7 @@ "37_autotune_pitch_setup.param": { "why": "To optimize pitch step response.", "why_now": "Because pitch is usually the axis that has the second highest dynamic, and we continue with the second highest dynamic axis", + "autoimport_nondefault_regexp": ["AUTOTUNE_AGGR", "AUTOTUNE_AXES"], "blog_text": "Set up parameters for the pitch axis autotuning process", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#952-pitch-axis-autotune", "wiki_text": "AutoTune", @@ -983,6 +1014,7 @@ "38_autotune_pitch_results.param": { "why": "To record autotune results after pitch step response optimization", "why_now": "Because pitch is usually the axis that has the second highest dynamic, and we continue with the second highest dynamic axis", + "autoimport_nondefault_regexp": ["ATC_ACC(EL)?_P_MAX", "ATC_RAT_PIT_.*"], "blog_text": "Record the results of the pitch axis autotuning", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#952-pitch-axis-autotune", "wiki_text": "AutoTune", @@ -996,6 +1028,7 @@ "39_autotune_yaw_setup.param": { "why": "To optimize yaw step response.", "why_now": "Because yaw is usually the axis that has the third highest dynamic, and we continue with the third highest dynamic axis", + "autoimport_nondefault_regexp": ["ATC_RAT_YAW_FLTD", "AUTOTUNE_AGGR", "AUTOTUNE_AXES"], "blog_text": "Set up parameters for the yaw axis autotuning process", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#953-yaw-axis-autotune", "wiki_text": "AutoTune", @@ -1015,6 +1048,7 @@ "40_autotune_yaw_results.param": { "why": "To record autotune results after yaw step response optimization", "why_now": "Because yaw is usually the axis that has the third highest dynamic, and we continue with the third highest dynamic axis", + "autoimport_nondefault_regexp": ["ATC_ACC(EL)?_Y_MAX", "ATC_RAT_YAW_.*"], "blog_text": "Record the results of the yaw axis autotuning, providing data for analysis and adjustment.", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#953-yaw-axis-autotune", "wiki_text": "AutoTune", @@ -1028,6 +1062,7 @@ "41_autotune_yawd_setup.param": { "why": "To optimize yaw D step response.", "why_now": "Because yaw D can only be done after yaw", + "autoimport_nondefault_regexp": ["AUTOTUNE_AGGR", "AUTOTUNE_AXES", "AUTOTUNE_MIN_D"], "blog_text": "Set up parameters for the yaw rate autotuning process", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#954-yaw-d-axis-autotune-optional", "wiki_text": "AutoTune", @@ -1294,6 +1329,7 @@ "60_position_controller.param": { "why": "Position controller parameters are crucial for waypoint navigation and precision flying", "why_now": "Because the position controller PIDs depend on the attitude and attitude-rate PIDs tuned in previous steps", + "autoimport_nondefault_regexp": ["(ATC_)?ANGLE_MAX", "FHLD_.*", "LOG_BITMASK", "LOIT_.*", "PHLD_.*", "PILOT_.*", "WP_YAW_BEHAVIOR", "(WPNAV_|WP_).*"], "blog_text": "Configure the position controller(s)", "blog_url": "https://ardupilot.github.io/MethodicConfigurator/TUNING_GUIDE_ArduCopter#121-position-controller", "wiki_text": "", diff --git a/ardupilot_methodic_configurator/data_model_ardupilot_parameter.py b/ardupilot_methodic_configurator/data_model_ardupilot_parameter.py index 6b1540a61..e9de3034f 100644 --- a/ardupilot_methodic_configurator/data_model_ardupilot_parameter.py +++ b/ardupilot_methodic_configurator/data_model_ardupilot_parameter.py @@ -34,6 +34,10 @@ class ParameterOutOfRangeError(Exception): """ +class ParameterForcedOrDerivedError(ValueError): + """Raised when attempting to change a forced or derived parameter without a manual override.""" + + class ArduPilotParameter: # pylint: disable=too-many-instance-attributes, too-many-public-methods """Domain model representing an ArduPilot parameter with all its attributes.""" @@ -397,6 +401,8 @@ def set_new_value(self, value: str, ignore_out_of_range: bool = False) -> float: Raises: TypeError: if the provided value is not a string. + ParameterForcedOrDerivedError: if the parameter is forced or derived and + has no manual override. ValueError: if the value is invalid for this parameter (not in choices, invalid bitmask bits, invalid numeric format, etc.). ParameterOutOfRangeError: if the value is outside min/max limits and @@ -406,7 +412,7 @@ def set_new_value(self, value: str, ignore_out_of_range: bool = False) -> float: """ if (self._is_forced or self._is_derived) and not self._is_manual_override: - raise ValueError(_("This parameter is forced or derived and cannot be changed.")) + raise ParameterForcedOrDerivedError(_("This parameter is forced or derived and cannot be changed.")) if not isinstance(value, str): raise TypeError(_("Parameter value must be provided as a string.")) diff --git a/ardupilot_methodic_configurator/data_model_configuration_step.py b/ardupilot_methodic_configurator/data_model_configuration_step.py index 004f6bc34..21c25981d 100644 --- a/ardupilot_methodic_configurator/data_model_configuration_step.py +++ b/ardupilot_methodic_configurator/data_model_configuration_step.py @@ -67,6 +67,7 @@ def process_configuration_step( # pylint: disable=too-many-locals set[str], list[tuple[str, str]], ParDict, + set[str], ]: """ Process a configuration step including parameter computation and domain model creation. @@ -83,6 +84,7 @@ def process_configuration_step( # pylint: disable=too-many-locals - Set of parameter names to remove (duplicates from rename operations) - List of (old_name, new_name) pairs to rename - ParDict of derived parameters to apply to domain model + - Set of parameter names auto-imported from the flight controller """ ui_errors: list[tuple[str, str]] = [] @@ -146,8 +148,11 @@ def process_configuration_step( # pylint: disable=too-many-locals # Create domain model parameters current_step_parameters = self._create_domain_model_parameters(selected_file, fc_parameters) - # Apply auto-imports for the current step - self._apply_auto_imports(selected_file, fc_parameters, current_step_parameters, parameters_to_delete) + # Apply auto-imports for the current step. The editor uses these names to + # track parameters that were added to the in-memory model and must be saved. + autoimported_parameters = self._apply_auto_imports( + selected_file, fc_parameters, current_step_parameters, parameters_to_delete + ) # Check for ExpressLRS and add FLTMODE_CH warning if current_step_parameters.get("RC_OPTIONS") is not None or current_step_parameters.get("FLTMODE_CH") is not None: @@ -166,7 +171,15 @@ def process_configuration_step( # pylint: disable=too-many-locals ) ) - return current_step_parameters, ui_errors, ui_infos, duplicates_to_remove, renames_to_apply, derived_params_to_apply + return ( + current_step_parameters, + ui_errors, + ui_infos, + duplicates_to_remove, + renames_to_apply, + derived_params_to_apply, + autoimported_parameters, + ) def _apply_auto_imports( self, @@ -174,17 +187,18 @@ def _apply_auto_imports( fc_parameters: dict[str, float], current_step_parameters: dict[str, ArduPilotParameter], parameters_to_delete: set[str] | None = None, - ) -> None: - """Automatically import non-default FC parameters matching regex rules into the domain model.""" + ) -> set[str]: + """Automatically import non-default FC parameters and return their names.""" step_dict = self.local_filesystem.configuration_steps.get(selected_file, {}) if "autoimport_nondefault_regexp" not in step_dict or not fc_parameters: - return + return set() # Parameters that will be deleted take priority; skip auto-importing them if parameters_to_delete is None: parameters_to_delete = set() regex_rules = step_dict["autoimport_nondefault_regexp"] + imported_parameters: set[str] = set() for live_key, live_value in fc_parameters.items(): if live_key in current_step_parameters: continue @@ -202,6 +216,9 @@ def _apply_auto_imports( current_step_parameters[live_key] = self.create_ardupilot_parameter( live_key, param, selected_file, fc_parameters ) + imported_parameters.add(live_key) + + return imported_parameters def _handle_connection_renaming( self, selected_file: str, variables: dict diff --git a/ardupilot_methodic_configurator/data_model_parameter_editor.py b/ardupilot_methodic_configurator/data_model_parameter_editor.py index 2fae426e7..bbdb616f1 100644 --- a/ardupilot_methodic_configurator/data_model_parameter_editor.py +++ b/ardupilot_methodic_configurator/data_model_parameter_editor.py @@ -36,6 +36,7 @@ from ardupilot_methodic_configurator.backend_internet import download_file_from_url, webbrowser_open_url from ardupilot_methodic_configurator.data_model_ardupilot_parameter import ( ArduPilotParameter, + ParameterForcedOrDerivedError, ParameterOutOfRangeError, ParameterUnchangedError, ) @@ -345,10 +346,23 @@ def _update_parameters_from_fc_values(self, relevant_fc_params: dict[str, float] continue # Expected, not an error except ParameterOutOfRangeError: # Log warning but accept FC value anyway since it came from FC - logging_warning(_("Parameter %s value %s is out of range but accepted from FC"), param_name, value) + logging_warning( + _("Parameter {parameter} value {value} is out of range but accepted from FC").format( + parameter=param_name, value=value + ) + ) params_copied += 1 + except ParameterForcedOrDerivedError as exc: + logging_warning( + _("Parameter {parameter} could not be updated because it is forced or derived: {error}").format( + parameter=param_name, error=exc + ) + ) + continue except (ValueError, TypeError): - logging_exception(_("Failed to update in-memory value for %s after FC copy"), param_name) + logging_exception( + _("Failed to update in-memory value for {parameter} after FC copy").format(parameter=param_name) + ) continue return bool(params_copied) @@ -391,6 +405,11 @@ def handle_copy_fc_values_workflow( _("Parameters copied"), _("FC values have been copied to {selected_file}").format(selected_file=selected_file), ) + else: + show_info( + _("No parameters copied"), + _("No FC values could be copied to {selected_file}.").format(selected_file=selected_file), + ) return user_choice return False @@ -1881,9 +1900,16 @@ def _repopulate_configuration_step_parameters( # pylint: disable=too-many-local self._connection_renames.clear() # Process configuration step and get operations to apply - self.current_step_parameters, ui_errors, ui_infos, duplicates_to_remove, renames_to_apply, derived_params = ( - self._config_step_processor.process_configuration_step(self.current_file, self.fc_parameters) - ) + ( + self.current_step_parameters, + ui_errors, + ui_infos, + duplicates_to_remove, + renames_to_apply, + derived_params, + autoimported_parameters, + ) = self._config_step_processor.process_configuration_step(self.current_file, self.fc_parameters) + self._added_parameters.update(autoimported_parameters) # Apply derived parameters to domain model using specialized setters for param_name, derived_par in derived_params.items(): diff --git a/ardupilot_methodic_configurator/data_model_vehicle_project.py b/ardupilot_methodic_configurator/data_model_vehicle_project.py index 844221b18..cdccb407e 100644 --- a/ardupilot_methodic_configurator/data_model_vehicle_project.py +++ b/ardupilot_methodic_configurator/data_model_vehicle_project.py @@ -18,8 +18,12 @@ from ardupilot_methodic_configurator import _ from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem -from ardupilot_methodic_configurator.data_model_par_dict import is_within_tolerance -from ardupilot_methodic_configurator.data_model_vehicle_project_creator import NewVehicleProjectSettings, VehicleProjectCreator +from ardupilot_methodic_configurator.data_model_par_dict import ParamFileError, ParDict, is_within_tolerance +from ardupilot_methodic_configurator.data_model_vehicle_project_creator import ( + NewVehicleProjectSettings, + VehicleProjectCreationError, + VehicleProjectCreator, +) from ardupilot_methodic_configurator.data_model_vehicle_project_opener import VehicleProjectOpener if TYPE_CHECKING: @@ -176,19 +180,178 @@ def create_new_vehicle_from_template( """ fc_connected = self.is_flight_controller_connected() + # The filesystem holds the defaults downloaded from the connected FC. + # Preserve them before opening the newly copied template, whose + # re_init() would otherwise replace them with template defaults. + fc_default_params = self._local_filesystem.param_default_dict.deep_copy() if settings.use_fc_params is True else None new_path = self._creator.create_new_vehicle_from_template( template_dir, new_base_dir, new_vehicle_name, settings, fc_connected, self.fc_parameters() ) if new_path: - self._settings = settings - self.configuration_template = self.get_directory_name_from_path(template_dir) - # History updates belong in the manager/facade layer so they are - # performed consistently for both project creation and opening. - self.store_recently_used_template_dirs(template_dir, new_base_dir) - self.open_vehicle_directory(new_path) + try: + if fc_default_params: + self._local_filesystem.re_init(new_path, self._local_filesystem.vehicle_type) + self._local_filesystem.write_param_default_values_to_file(fc_default_params) + self._settings = settings + self.configuration_template = self.get_directory_name_from_path(template_dir) + # History updates belong in the manager/facade layer so they are + # performed consistently for both project creation and opening. + self.store_recently_used_template_dirs(template_dir, new_base_dir) + self.open_vehicle_directory(new_path) + except (OSError, ParamFileError, ValueError, TypeError, SystemExit) as exc: + raise VehicleProjectCreationError( + _("Vehicle project creation"), + _("Could not finish creating the vehicle project: {error}").format(error=exc), + ) from exc return new_path - def create_new_vehicle_from_bin_log( # pylint: disable=too-many-locals + def create_new_vehicle_from_flight_controller(self, new_base_dir: str, new_vehicle_name: str) -> str: + """Create a vehicle project from the connected flight controller's configuration.""" + if not self.is_flight_controller_connected(): + raise VehicleProjectCreationError( + _("Flight controller"), + _("Cannot create a vehicle project: no flight controller is connected."), + ) + + fc_parameters = self.fc_parameters() + if fc_parameters is None or not NewVehicleProjectSettings.has_fc_parameters(fc_parameters): + raise VehicleProjectCreationError( + _("Flight controller"), + _("Cannot create a vehicle project: no flight controller parameters are available."), + ) + + template_dir = self._get_fc_template_dir_for_project_creation() + + settings = NewVehicleProjectSettings( + infer_comp_specs_and_conn_from_fc_params=True, + use_fc_params=True, + ) + new_path = self._creator.create_new_vehicle_from_template( + template_dir, + new_base_dir, + new_vehicle_name, + settings, + fc_connected=True, + fc_parameters=fc_parameters, + ) + + flight_controller = self._flight_controller + if flight_controller is None: # pragma: no cover - guarded by the connection check above + raise VehicleProjectCreationError( + _("Flight controller"), + _("Cannot create a vehicle project: no flight controller is connected."), + ) + fc_info = flight_controller.info + vehicle_type = fc_info.vehicle_type + fw_version = fc_info.flight_sw_version + try: + self._complete_imported_vehicle_project_creation( + template_dir, + new_base_dir, + settings, + new_path, + vehicle_type, + fw_version, + ParDict.from_fc_parameters(fc_parameters), + import_source="flight_controller", + ) + except (OSError, ParamFileError, ValueError, TypeError, SystemExit) as exc: + raise VehicleProjectCreationError( + _("Vehicle project creation"), + _("Could not finish creating the vehicle project: {error}").format(error=exc), + ) from exc + return new_path + + def _get_fc_template_dir_for_project_creation(self) -> str: + """Resolve the exact empty template matching the connected FC firmware.""" + if self._flight_controller is None: # pragma: no cover - guarded by the public method + raise VehicleProjectCreationError( + _("Flight controller"), + _("Cannot create a vehicle project: no flight controller is connected."), + ) + + fc_info = getattr(self._flight_controller, "info", None) + vehicle_type = getattr(fc_info, "vehicle_type", "") + firmware_version = getattr(fc_info, "flight_sw_version", "") + version_parts = firmware_version.split(".") if isinstance(firmware_version, str) else [] + if not isinstance(vehicle_type, str) or not vehicle_type or len(version_parts) < 2: + raise VehicleProjectCreationError( + _("Vehicle template directory"), + _( + "Could not determine the connected flight controller's vehicle type and firmware version " + "needed to select an empty template." + ), + ) + + try: + major, minor = int(version_parts[0]), int(version_parts[1]) + except ValueError as exc: + raise VehicleProjectCreationError( + _("Vehicle template directory"), + _( + "Could not determine the connected flight controller's vehicle type and firmware version " + "needed to select an empty template." + ), + ) from exc + + try: + return self._creator.template_dir_for_bin_import(vehicle_type, major, minor) + except VehicleProjectCreationError as exc: + raise VehicleProjectCreationError( + _("Vehicle template directory"), + _( + "Could not find an empty vehicle template matching the connected flight controller's " + "vehicle type and firmware version.\n" + ) + + exc.message, + ) from exc + + def _complete_imported_vehicle_project_creation( # pylint: disable=too-many-arguments,too-many-positional-arguments + self, + template_dir: str, + new_base_dir: str, + settings: NewVehicleProjectSettings, + new_path: str, + vehicle_type: str, + fw_version: str, + current_params: ParDict, + default_params: ParDict | None = None, + import_source: str = "bin_log", + ) -> str: + """Persist imported project metadata, any remaining parameters, and manager state.""" + # Capture the source defaults before re_init() points the filesystem at the + # destination. The destination currently contains the template's defaults, + # which would otherwise become the baseline for FC project creation. + if default_params is None: + default_params = self._local_filesystem.param_default_dict.deep_copy() + + # Point the filesystem at the new vehicle before reading or writing files. Set the + # firmware first so re_init() does not replace it with the template's placeholder. + self._local_filesystem.fw_version = fw_version + self._local_filesystem.re_init(new_path, vehicle_type) + self._local_filesystem.set_fc_fw_version_and_type_in_components_json(fw_version, vehicle_type, new_path) + + if default_params is not None: + self._local_filesystem.write_param_default_values_to_file(default_params) + + compounded_step_params, _first_config_step = self._local_filesystem.compound_params(skip_default=True) + baseline_params = default_params.deep_copy() + baseline_params.update(compounded_step_params) + if imported_params := current_params.get_missing_or_different(baseline_params, is_within_tolerance): + self._local_filesystem.export_to_param( + imported_params, + self._creator.next_import_filename(new_path, source=import_source), + annotate_doc=False, + ) + self._local_filesystem.re_init(new_path, vehicle_type) + + self.open_vehicle_directory(new_path) + self._settings = settings + self.configuration_template = self.get_directory_name_from_path(template_dir) + self.store_recently_used_template_dirs(template_dir, new_base_dir) + return new_path + + def create_new_vehicle_from_bin_log( self, bin_file: str, progress_callback: Callable[[int, int], None] | None = None, @@ -238,47 +401,25 @@ def create_new_vehicle_from_bin_log( # pylint: disable=too-many-locals fc_parameters=fc_parameters, ) - # Point the filesystem at the new vehicle directory before any reads or writes. - # write_param_default_values_to_file() and compound_params() rely on vehicle_dir, - # so re_init() must be called here to avoid accidentally operating on the previously-open project. - # Set fw_version first so re_init() does not override it with the template's placeholder version. - self._local_filesystem.fw_version = fw_version - self._local_filesystem.re_init(new_path, vehicle_type) - # Persist the correct firmware version and type into vehicle_components.json so subsequent - # re_init calls (and the user when they inspect the project) see the actual recorded firmware. - self._local_filesystem.set_fc_fw_version_and_type_in_components_json(fw_version, vehicle_type, new_path) - - self._local_filesystem.write_param_default_values_to_file(default_params) - - # Build the baseline from log-extracted defaults plus compounded AMC step files. - # This avoids exporting params that merely match 00_default.param but are absent - # from the numbered step files. - compounded_step_params, _first_config_step = self._local_filesystem.compound_params(skip_default=True) - baseline_params = default_params.deep_copy() - baseline_params.update(compounded_step_params) - - if imported_params := current_params.get_missing_or_different(baseline_params, is_within_tolerance): - self._local_filesystem.export_to_param( - imported_params, - self._creator.next_import_filename(new_path), - annotate_doc=False, + try: + self._complete_imported_vehicle_project_creation( + template_dir, + new_base_dir, + settings, + new_path, + vehicle_type, + fw_version, + current_params, + default_params, ) - self._local_filesystem.re_init(new_path, vehicle_type) - - # Open the vehicle directory only after all file modifications are complete and filesystem state is synced. - # This ensures the UI/session operates on the authoritative filesystem state, not stale in-memory cache. - # Also note: infer_comp_specs_and_conn_from_fc_params and use_fc_params are reused for log-derived params - # because the semantics align: we're supplying external parameter values for template substitution. - self.open_vehicle_directory(new_path) + except (OSError, ParamFileError, ValueError, TypeError, SystemExit) as exc: + raise VehicleProjectCreationError( + _("Vehicle project creation"), + _("Could not finish creating the vehicle project: {error}").format(error=exc), + ) from exc if self._flight_controller is not None: self._flight_controller.fc_parameters = fc_parameters - - # Store manager settings only after the whole import succeeds, so failed imports do not - # pollute the recently-used template history with an incomplete project. - self._settings = settings - self.configuration_template = self.get_directory_name_from_path(template_dir) - self.store_recently_used_template_dirs(template_dir, new_base_dir) return new_path # Vehicle project opening operations diff --git a/ardupilot_methodic_configurator/data_model_vehicle_project_creator.py b/ardupilot_methodic_configurator/data_model_vehicle_project_creator.py index f8e043c44..2b2837e12 100644 --- a/ardupilot_methodic_configurator/data_model_vehicle_project_creator.py +++ b/ardupilot_methodic_configurator/data_model_vehicle_project_creator.py @@ -425,8 +425,8 @@ def vehicle_name_from_bin_log(bin_file: str) -> str: return Path(bin_file).stem @staticmethod - def next_import_filename(vehicle_dir: str) -> str: - """Return the next available numbered parameter filename for imported log parameters.""" + def next_import_filename(vehicle_dir: str, source: str = "bin_log") -> str: + """Return the next available numbered parameter filename for imported parameters.""" highest_prefix = 0 try: for file_path in Path(vehicle_dir).iterdir(): @@ -444,7 +444,7 @@ def next_import_filename(vehicle_dir: str) -> str: if next_prefix > 99: msg = _("Could not create an import parameter file because no numbered slot is available in {vehicle_dir}") raise VehicleProjectCreationError(_("Parameter import"), msg.format(vehicle_dir=vehicle_dir)) - return f"{next_prefix:02d}_imported_bin_log_parameters.param" + return f"{next_prefix:02d}_imported_{source}_parameters.param" @staticmethod def extract_bin_log_data( diff --git a/ardupilot_methodic_configurator/frontend_tkinter_project_creator.py b/ardupilot_methodic_configurator/frontend_tkinter_project_creator.py index b95c92bf2..3fe699de8 100755 --- a/ardupilot_methodic_configurator/frontend_tkinter_project_creator.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_project_creator.py @@ -29,6 +29,7 @@ NewVehicleProjectSettings, VehicleProjectCreationError, ) +from ardupilot_methodic_configurator.data_model_vehicle_project_opener import VehicleProjectOpenError from ardupilot_methodic_configurator.frontend_tkinter_base_window import BaseWindow from ardupilot_methodic_configurator.frontend_tkinter_directory_selection import ( DirectorySelectionWidgets, @@ -46,7 +47,7 @@ class VehicleProjectCreatorWindow(BaseWindow): destination directory, and project options. Integrates with VehicleProjectManager for project creation. """ - def __init__(self, project_manager: VehicleProjectManager) -> None: + def __init__(self, project_manager: VehicleProjectManager, from_flight_controller: bool = False) -> None: super().__init__() self.project_manager = project_manager self.root.title( @@ -61,9 +62,16 @@ def __init__(self, project_manager: VehicleProjectManager) -> None: # Initialize settings variables dynamically from data model self.new_project_settings_vars: dict[str, tk.BooleanVar] = {} self.new_project_settings_widgets: dict[str, ttk.Checkbutton] = {} + # Created only for the regular template workflow; initialize the attribute here so + # static analysis also recognizes it when the flight-controller workflow is used. + self.template_dir: DirectorySelectionWidgets recent_template_dir, new_base_dir, vehicle_dir = self.project_manager.get_recently_used_dirs() - template_dir = self.project_manager.get_fc_default_template_dir() if fc_connected else recent_template_dir + template_dir = ( + self.project_manager.get_fc_default_template_dir() + if fc_connected and not from_flight_controller + else recent_template_dir + ) logging_debug("template_dir: %s", template_dir) # this string is intentionally left untranslated logging_debug("new_base_dir: %s", new_base_dir) # this string is intentionally left untranslated logging_debug("vehicle_dir: %s", vehicle_dir) # this string is intentionally left untranslated @@ -74,6 +82,7 @@ def __init__(self, project_manager: VehicleProjectManager) -> None: fc_connected, fc_parameters, project_manager.get_vehicle_type(), + from_flight_controller=from_flight_controller, ) # Bind the close_connection_and_quit function to the window close event @@ -82,7 +91,7 @@ def __init__(self, project_manager: VehicleProjectManager) -> None: def close_and_quit(self) -> None: sys_exit(0) - def create_option1_widgets( # pylint: disable=too-many-locals,too-many-arguments,too-many-positional-arguments + def create_option1_widgets( # pylint: disable=too-many-arguments,too-many-positional-arguments self, initial_template_dir: str, initial_base_dir: str, @@ -90,11 +99,76 @@ def create_option1_widgets( # pylint: disable=too-many-locals,too-many-argument fc_connected: bool, fc_parameters: dict[str, float] | None, connected_fc_vehicle_type: str, + from_flight_controller: bool = False, ) -> None: - # Option 1 - Create a new vehicle configuration directory based on an existing template - option1_label = ttk.Label(self.main_frame, text=_("New vehicle"), style="Bold.TLabel") + option1_label = ttk.Label( + self.main_frame, + text=_("New vehicle"), + style="Bold.TLabel", + ) option1_label_frame = ttk.LabelFrame(self.main_frame, labelwidget=option1_label) option1_label_frame.pack(expand=True, fill=tk.X, padx=6, pady=6) + + if from_flight_controller: + window_height = 200 + else: + self._create_template_selection_widgets(option1_label_frame, initial_template_dir, connected_fc_vehicle_type) + window_height = self._create_settings_widgets(option1_label_frame, fc_connected, fc_parameters) + self.root.geometry(self.calculate_scaled_geometry(800, window_height)) # Set the window size + + self.center_window_on_screen(self.root) + new_base_dir_edit_tooltip = _("Existing directory where the new vehicle configuration directory will be created") + new_base_dir_btn_tooltip = _("Select the directory where the new vehicle configuration directory will be created") + self.new_base_dir = DirectorySelectionWidgets( + parent=self, + parent_frame=option1_label_frame, + initialdir=initial_base_dir, + label_text=_("Destination base directory:"), + autoresize_width=False, + dir_tooltip=new_base_dir_edit_tooltip, + button_tooltip=new_base_dir_btn_tooltip, + on_directory_selected_callback=None, # Use default file dialog behavior + ) + self.new_base_dir.container_frame.pack(expand=False, fill=tk.X, padx=3, pady=5, anchor=tk.NW) + new_dir_edit_tooltip = _( + "A new vehicle configuration directory with this name will be created at the (destination) base directory" + ) + self.new_dir = PathEntryWidget( + option1_label_frame, initial_new_dir, _("Destination new vehicle name:"), new_dir_edit_tooltip + ) + self.new_dir.container_frame.pack(expand=False, fill=tk.X, padx=3, pady=5, anchor=tk.NW) + create_vehicle_button = ttk.Button( + option1_label_frame, + text=( + _("Create a vehicle project from an already configured flight controller") + if from_flight_controller + else _("Create a vehicle project from a template") + ), + command=( + self.create_new_vehicle_from_flight_controller + if from_flight_controller + else self.create_new_vehicle_from_template + ), + ) + create_vehicle_button.pack(expand=False, fill=tk.X, padx=20, pady=5, anchor=tk.CENTER) + show_tooltip( + create_vehicle_button, + _( + "Create a new vehicle configuration directory using the connected flight controller's\n" + "parameters and component information." + ) + if from_flight_controller + else _( + "Create a new vehicle configuration directory on the (destination) base directory,\n" + "copy the template files from the (source) template directory to it and\n" + "load the newly created files into the application" + ), + ) + + def _create_template_selection_widgets( + self, parent_frame: ttk.LabelFrame, initial_template_dir: str, connected_fc_vehicle_type: str + ) -> None: + """Create the template directory selector and its template overview callback.""" template_dir_edit_tooltip = _( "Existing vehicle template directory containing the intermediate\n" "parameter files to be copied to the new vehicle configuration directory" @@ -123,7 +197,7 @@ def template_selection_callback(_widget: "DirectorySelectionWidgets") -> str: self.template_dir = DirectorySelectionWidgets( parent=self, - parent_frame=option1_label_frame, + parent_frame=parent_frame, initialdir=initial_template_dir, label_text=_("Source Template directory:"), autoresize_width=False, @@ -133,21 +207,19 @@ def template_selection_callback(_widget: "DirectorySelectionWidgets") -> str: ) self.template_dir.container_frame.pack(expand=False, fill=tk.X, padx=3, pady=5, anchor=tk.NW) - # Create checkboxes dynamically from settings metadata + def _create_settings_widgets( + self, parent_frame: ttk.LabelFrame, fc_connected: bool, fc_parameters: dict[str, float] | None + ) -> int: + """Create the dynamic project-setting checkboxes and return the required window height.""" settings_metadata = NewVehicleProjectSettings.get_all_settings_metadata(fc_connected, fc_parameters) new_project_settings_default_values = NewVehicleProjectSettings.get_default_values() for setting_name in settings_metadata: default_value = new_project_settings_default_values.get(setting_name, False) self.new_project_settings_vars[setting_name] = tk.BooleanVar(value=default_value) - # Set dynamic window size based on number of settings - window_height = 250 + (len(settings_metadata) * 23) - self.root.geometry(self.calculate_scaled_geometry(800, window_height)) # Set the window size - self.center_window_on_screen(self.root) - for setting_name, metadata in settings_metadata.items(): checkbox = ttk.Checkbutton( - option1_label_frame, + parent_frame, variable=self.new_project_settings_vars[setting_name], text=metadata.label, state=tk.NORMAL if metadata.enabled else tk.DISABLED, @@ -156,40 +228,7 @@ def template_selection_callback(_widget: "DirectorySelectionWidgets") -> str: show_tooltip(checkbox, metadata.tooltip) self.new_project_settings_widgets[setting_name] = checkbox - new_base_dir_edit_tooltip = _("Existing directory where the new vehicle configuration directory will be created") - new_base_dir_btn_tooltip = _("Select the directory where the new vehicle configuration directory will be created") - self.new_base_dir = DirectorySelectionWidgets( - parent=self, - parent_frame=option1_label_frame, - initialdir=initial_base_dir, - label_text=_("Destination base directory:"), - autoresize_width=False, - dir_tooltip=new_base_dir_edit_tooltip, - button_tooltip=new_base_dir_btn_tooltip, - on_directory_selected_callback=None, # Use default file dialog behavior - ) - self.new_base_dir.container_frame.pack(expand=False, fill=tk.X, padx=3, pady=5, anchor=tk.NW) - new_dir_edit_tooltip = _( - "A new vehicle configuration directory with this name will be created at the (destination) base directory" - ) - self.new_dir = PathEntryWidget( - option1_label_frame, initial_new_dir, _("Destination new vehicle name:"), new_dir_edit_tooltip - ) - self.new_dir.container_frame.pack(expand=False, fill=tk.X, padx=3, pady=5, anchor=tk.NW) - create_vehicle_directory_from_template_button = ttk.Button( - option1_label_frame, - text=_("Create a vehicle configuration directory from template"), - command=self.create_new_vehicle_from_template, - ) - create_vehicle_directory_from_template_button.pack(expand=False, fill=tk.X, padx=20, pady=5, anchor=tk.CENTER) - show_tooltip( - create_vehicle_directory_from_template_button, - _( - "Create a new vehicle configuration directory on the (destination) base directory,\n" - "copy the template files from the (source) template directory to it and\n" - "load the newly created files into the application" - ), - ) + return 250 + (len(settings_metadata) * 23) def create_new_vehicle_from_template(self) -> None: # Get the selected template directory and new vehicle configuration directory name @@ -207,7 +246,17 @@ def create_new_vehicle_from_template(self) -> None: try: self.project_manager.create_new_vehicle_from_template(template_dir, new_base_dir, new_vehicle_name, settings) self.root.destroy() - except VehicleProjectCreationError as e: + except (VehicleProjectCreationError, VehicleProjectOpenError) as e: + messagebox.showerror(e.title, e.message) + + def create_new_vehicle_from_flight_controller(self) -> None: + """Create a vehicle project using the connected flight controller's configuration.""" + new_base_dir = self.new_base_dir.get_selected_directory() + new_vehicle_name = self.new_dir.get_selected_directory() + try: + self.project_manager.create_new_vehicle_from_flight_controller(new_base_dir, new_vehicle_name) + self.root.destroy() + except (VehicleProjectCreationError, VehicleProjectOpenError) as e: messagebox.showerror(e.title, e.message) diff --git a/ardupilot_methodic_configurator/frontend_tkinter_project_opener.py b/ardupilot_methodic_configurator/frontend_tkinter_project_opener.py index 4c15fade9..8ccebfd79 100755 --- a/ardupilot_methodic_configurator/frontend_tkinter_project_opener.py +++ b/ardupilot_methodic_configurator/frontend_tkinter_project_opener.py @@ -57,7 +57,7 @@ def __init__(self, project_manager: VehicleProjectManager) -> None: + _(" - Select vehicle configuration directory") ) - self.root.geometry(self.calculate_scaled_geometry(600, 450)) # Set the window size + self.root.geometry(self.calculate_scaled_geometry(600, 470)) # Set the window size self.center_window_on_screen(self.root) # Explain why we are here @@ -89,7 +89,7 @@ def create_option1_widgets(self) -> None: create_vehicle_directory_from_template_button = ttk.Button( option1_label_frame, - text=_("Create a vehicle configuration directory from template"), + text=_("Create a vehicle project from a template"), command=self.create_new_vehicle_from_template, ) create_vehicle_directory_from_template_button.pack(expand=False, fill=tk.X, padx=20, pady=5, anchor=tk.CENTER) @@ -98,6 +98,25 @@ def create_option1_widgets(self) -> None: _("Create a new vehicle configuration directory, choose this option when using the software for the first time"), ) + create_vehicle_from_fc_button = ttk.Button( + option1_label_frame, + text=_("Create a vehicle project from an already configured flight controller"), + command=self.create_new_vehicle_from_flight_controller, + state=( + tk.NORMAL + if self.project_manager.is_flight_controller_connected() and self.project_manager.fc_parameters() + else tk.DISABLED + ), + ) + create_vehicle_from_fc_button.pack(expand=False, fill=tk.X, padx=20, pady=5, anchor=tk.CENTER) + show_tooltip( + create_vehicle_from_fc_button, + _( + "Create a new vehicle configuration directory using the connected flight controller's " + "parameters and component information." + ), + ) + def on_bin_log_selected(bin_file: str) -> None: progress_window = ProgressWindow( self.root, @@ -246,6 +265,11 @@ def create_new_vehicle_from_template(self) -> None: self.root.destroy() VehicleProjectCreatorWindow(self.project_manager) + def create_new_vehicle_from_flight_controller(self) -> None: + """Open the minimal project creator for a connected, already configured FC.""" + self.root.destroy() + VehicleProjectCreatorWindow(self.project_manager, from_flight_controller=True) + def open_last_vehicle_directory(self, last_vehicle_dir: str) -> None: # Attempt to open the last opened vehicle configuration directory try: diff --git a/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.5.x/06_remote_controller_receiver.param b/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.5.x/06_remote_controller_receiver.param index 58b06e769..496c80f0e 100644 --- a/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.5.x/06_remote_controller_receiver.param +++ b/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.5.x/06_remote_controller_receiver.param @@ -1,4 +1,3 @@ -BRD_ALT_CONFIG,0 RC_OPTIONS,32 RC_PROTOCOLS,1 # Selected in the component editor RSSI_TYPE,0 diff --git a/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.6.x/06_remote_controller_receiver.param b/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.6.x/06_remote_controller_receiver.param index 58b06e769..496c80f0e 100644 --- a/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.6.x/06_remote_controller_receiver.param +++ b/ardupilot_methodic_configurator/vehicle_templates/ArduCopter/empty_4.6.x/06_remote_controller_receiver.param @@ -1,4 +1,3 @@ -BRD_ALT_CONFIG,0 RC_OPTIONS,32 RC_PROTOCOLS,1 # Selected in the component editor RSSI_TYPE,0 diff --git a/images/App_screenshot_Parameter_file_editor_and_uploader4_4.png b/images/App_screenshot_Parameter_file_editor_and_uploader4_4.png index 893ee5e2e..b0593e516 100644 Binary files a/images/App_screenshot_Parameter_file_editor_and_uploader4_4.png and b/images/App_screenshot_Parameter_file_editor_and_uploader4_4.png differ diff --git a/images/App_screenshot_Parameter_file_editor_and_uploader4_4_simple.png b/images/App_screenshot_Parameter_file_editor_and_uploader4_4_simple.png index b74a8d26a..4befa8c46 100644 Binary files a/images/App_screenshot_Parameter_file_editor_and_uploader4_4_simple.png and b/images/App_screenshot_Parameter_file_editor_and_uploader4_4_simple.png differ diff --git a/images/App_screenshot_Vehicle_directory.png b/images/App_screenshot_Vehicle_directory.png index 26718a671..4cf39e20e 100644 Binary files a/images/App_screenshot_Vehicle_directory.png and b/images/App_screenshot_Vehicle_directory.png differ diff --git a/images/App_screenshot_Vehicle_directory10.png b/images/App_screenshot_Vehicle_directory10.png index 26718a671..4cf39e20e 100644 Binary files a/images/App_screenshot_Vehicle_directory10.png and b/images/App_screenshot_Vehicle_directory10.png differ diff --git a/images/App_screenshot_Vehicle_directory4.png b/images/App_screenshot_Vehicle_directory4.png index 023d9e8c9..07e2e0eab 100644 Binary files a/images/App_screenshot_Vehicle_directory4.png and b/images/App_screenshot_Vehicle_directory4.png differ diff --git a/images/App_screenshot_Vehicle_directory_create_from_bin.png b/images/App_screenshot_Vehicle_directory_create_from_bin.png index ad19f47d1..e9432490e 100644 Binary files a/images/App_screenshot_Vehicle_directory_create_from_bin.png and b/images/App_screenshot_Vehicle_directory_create_from_bin.png differ diff --git a/images/App_screenshot_Vehicle_directory_create_from_configured_create.png b/images/App_screenshot_Vehicle_directory_create_from_configured_create.png deleted file mode 100644 index 7debf4117..000000000 Binary files a/images/App_screenshot_Vehicle_directory_create_from_configured_create.png and /dev/null differ diff --git a/images/App_screenshot_Vehicle_directory_create_from_configured_name.png b/images/App_screenshot_Vehicle_directory_create_from_configured_name.png deleted file mode 100644 index 93e43af7f..000000000 Binary files a/images/App_screenshot_Vehicle_directory_create_from_configured_name.png and /dev/null differ diff --git a/images/App_screenshot_Vehicle_directory_create_from_configured_options.png b/images/App_screenshot_Vehicle_directory_create_from_configured_options.png deleted file mode 100644 index 5a6467e3f..000000000 Binary files a/images/App_screenshot_Vehicle_directory_create_from_configured_options.png and /dev/null differ diff --git a/images/App_screenshot_Vehicle_directory_create_from_flight_controller.png b/images/App_screenshot_Vehicle_directory_create_from_flight_controller.png new file mode 100644 index 000000000..3ff9929c7 Binary files /dev/null and b/images/App_screenshot_Vehicle_directory_create_from_flight_controller.png differ diff --git a/images/App_screenshot_Vehicle_directory_create_from_flight_controller_creator.png b/images/App_screenshot_Vehicle_directory_create_from_flight_controller_creator.png new file mode 100644 index 000000000..722561023 Binary files /dev/null and b/images/App_screenshot_Vehicle_directory_create_from_flight_controller_creator.png differ diff --git a/images/App_screenshot_Vehicle_directory_create_from_template.png b/images/App_screenshot_Vehicle_directory_create_from_template.png index 03486c9ab..f92c76ed5 100644 Binary files a/images/App_screenshot_Vehicle_directory_create_from_template.png and b/images/App_screenshot_Vehicle_directory_create_from_template.png differ diff --git a/images/App_screenshot_motor_test.png b/images/App_screenshot_motor_test.png index acdca8990..397fc86b8 100644 Binary files a/images/App_screenshot_motor_test.png and b/images/App_screenshot_motor_test.png differ diff --git a/scripts/regenerate_app_screenshots_fully_automated.py b/scripts/regenerate_app_screenshots_fully_automated.py index 82e267ba4..56d3c448a 100755 --- a/scripts/regenerate_app_screenshots_fully_automated.py +++ b/scripts/regenerate_app_screenshots_fully_automated.py @@ -34,13 +34,16 @@ import pyautogui from PIL import Image, ImageDraw +from ardupilot_methodic_configurator import _ as translate from ardupilot_methodic_configurator import __version__ +from ardupilot_methodic_configurator.__main__ import register_plugins from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem from ardupilot_methodic_configurator.backend_filesystem_program_settings import ProgramSettings from ardupilot_methodic_configurator.backend_flightcontroller import FlightController from ardupilot_methodic_configurator.data_model_par_dict import ParDict from ardupilot_methodic_configurator.data_model_parameter_editor import ParameterEditor from ardupilot_methodic_configurator.frontend_tkinter_about_popup_window import AboutWindow +from ardupilot_methodic_configurator.frontend_tkinter_base_window import BaseWindow from ardupilot_methodic_configurator.frontend_tkinter_connection_selection import ConnectionSelectionWindow from ardupilot_methodic_configurator.frontend_tkinter_flightcontroller_info import FlightControllerInfoWindow from ardupilot_methodic_configurator.frontend_tkinter_parameter_editor import ParameterEditorWindow @@ -48,8 +51,8 @@ from ardupilot_methodic_configurator.frontend_tkinter_project_opener import VehicleProjectOpenerWindow from ardupilot_methodic_configurator.frontend_tkinter_template_overview import TemplateOverviewWindow from ardupilot_methodic_configurator.frontend_tkinter_usage_popup_windows import display_parameter_editor_usage_popup -from ardupilot_methodic_configurator.plugins.data_model_motor_test import MotorTestDataModel -from ardupilot_methodic_configurator.plugins.frontend_tkinter_motor_test import MotorTestView, MotorTestWindow +from ardupilot_methodic_configurator.plugins.plugin_constants import PLUGIN_MOTOR_TEST +from ardupilot_methodic_configurator.plugins.plugin_factory import PluginModelContext, plugin_factory if TYPE_CHECKING: from collections.abc import Callable, Iterator @@ -85,28 +88,38 @@ class CaptureTarget: "param_04_simple", scale=0.666, gui_complexity="simple", - current_file="04_board_orientation.param", + current_file="05_board_orientation.param", ), CaptureTarget( "App_screenshot_Parameter_file_editor_and_uploader4_4.png", "param_04_normal", scale=0.666, gui_complexity="normal", - current_file="04_board_orientation.param", + current_file="05_board_orientation.param", ), CaptureTarget( "App_screenshot_Parameter_file_editor_and_uploader4.png", "param_20_normal", scale=0.666, gui_complexity="normal", - current_file="20_throttle_controller.param", + current_file="24_throttle_controller.param", ), CaptureTarget("App_screenshot_Vehicle_directory.png", "vehicle_opener"), CaptureTarget("App_screenshot_Vehicle_directory10.png", "vehicle_opener"), CaptureTarget("App_screenshot_Vehicle_directory_create_from_template.png", "vehicle_opener_from_template", scale=0.8), + CaptureTarget( + "App_screenshot_Vehicle_directory_create_from_flight_controller.png", + "vehicle_opener_from_flight_controller", + scale=0.8, + ), CaptureTarget("App_screenshot_Vehicle_directory_create_from_bin.png", "vehicle_opener_from_bin", scale=0.8), CaptureTarget("App_screenshot_Vehicle_directory4.png", "vehicle_opener_legacy4", scale=0.8), CaptureTarget("App_screenshot_Vehicle_directory11.png", "vehicle_creator"), + CaptureTarget( + "App_screenshot_Vehicle_directory_create_from_flight_controller_creator.png", + "vehicle_creator_from_flight_controller", + scale=0.8, + ), CaptureTarget( "App_screenshot_Vehicle_directory_create_from_template_source.png", "create_from_template_source", @@ -131,31 +144,13 @@ class CaptureTarget: scale=0.8, variant="from_configured_source", ), - CaptureTarget( - "App_screenshot_Vehicle_directory_create_from_configured_options.png", - "vehicle_creator_options", - scale=0.8, - variant="from_configured_options", - ), - CaptureTarget( - "App_screenshot_Vehicle_directory_create_from_configured_name.png", - "vehicle_creator_name", - scale=0.8, - variant="from_configured_name", - ), - CaptureTarget( - "App_screenshot_Vehicle_directory_create_from_configured_create.png", - "vehicle_creator_create", - scale=0.8, - variant="from_configured_create", - ), CaptureTarget("App_screenshot_Vehicle_overview.png", "template_overview"), CaptureTarget( "App_screenshot1.png", "param_20_normal", scale=0.666, gui_complexity="normal", - current_file="20_throttle_controller.param", + current_file="24_throttle_controller.param", ), ) @@ -261,6 +256,9 @@ def get_introduction_message(self) -> str: def get_recently_used_dirs(self) -> tuple[str, str, str]: return self._template_dir, self._base_dir, self._vehicle_dir + def get_fc_default_template_dir(self) -> str: + return self._template_dir + def get_recent_vehicle_dirs(self) -> list[str]: return [self._vehicle_dir] @@ -375,6 +373,20 @@ def _find_descendant(widget: tk.Misc, predicate: Callable[[tk.Misc], bool]) -> t return None +def _press_no_on_fc_copy_prompt(root: tk.Tk) -> None: + """Dismiss the file-24 FC-copy prompt by choosing its No button.""" + prompt_title = translate("Update file with values from FC?") + for child in root.winfo_children(): + if not isinstance(child, tk.Toplevel) or not child.winfo_exists(): + continue + if child.title() != prompt_title: + continue + no_button = _find_descendant(child, lambda widget: _widget_text(widget) == translate("No")) + if no_button is not None: + no_button.invoke() + return + + def _widget_screen_box(widget: tk.Misc, margin: int = 2) -> tuple[int, int, int, int]: """Return widget bounds in screen coordinates as (left, top, right, bottom).""" left = max(widget.winfo_rootx() - margin, 0) @@ -589,13 +601,23 @@ def _vehicle_opener_highlight_box(window: VehicleProjectOpenerWindow, action: st if action == "vehicle_opener_from_template": template_button = _find_descendant( window.main_frame, - lambda w: _widget_text(w).startswith("Create a vehicle configuration directory from template"), + lambda w: _widget_text(w).startswith("Create a vehicle project from a template"), ) if template_button is None: msg = "Could not find create from template button" raise RuntimeError(msg) return _widget_screen_box(template_button, margin=2) + if action == "vehicle_opener_from_flight_controller": + fc_button = _find_descendant( + window.main_frame, + lambda w: _widget_text(w).startswith("Create a vehicle project from an already configured flight controller"), + ) + if fc_button is None: + msg = "Could not find create from flight controller button" + raise RuntimeError(msg) + return _widget_screen_box(fc_button, margin=2) + if action == "vehicle_opener_from_bin": bin_button = _find_descendant( window.main_frame, @@ -617,8 +639,16 @@ def _capture_vehicle_opener_with_highlight( # pylint: disable=too-many-argument vehicle_dir: Path, action: str = "vehicle_opener_legacy4", scale: float = 1.0, + fc_connected: bool = False, ) -> None: - manager = FakeProjectManager(vehicle_dir, vehicle_dir.parent, vehicle_dir) + fc_params = _load_fc_params_from_file(vehicle_dir) if fc_connected else None + manager = FakeProjectManager( + vehicle_dir, + vehicle_dir.parent, + vehicle_dir, + fc_connected=fc_connected, + fc_parameters=fc_params, + ) window = VehicleProjectOpenerWindow(manager) # type: ignore[arg-type] try: settle_tk(window.root, cycles=6, delay=0.05) @@ -639,7 +669,9 @@ def _capture_vehicle_creator(output_path: Path, delay: float, padding: int, vehi window.root.destroy() -def _create_vehicle_creator_window(vehicle_dir: Path, fc_connected: bool = False) -> VehicleProjectCreatorWindow: +def _create_vehicle_creator_window( + vehicle_dir: Path, fc_connected: bool = False, from_flight_controller: bool = False +) -> VehicleProjectCreatorWindow: fc_params = _load_fc_params_from_file(vehicle_dir) if fc_connected else {} manager = FakeProjectManager( vehicle_dir, @@ -648,11 +680,21 @@ def _create_vehicle_creator_window(vehicle_dir: Path, fc_connected: bool = False fc_connected=fc_connected, fc_parameters=fc_params if fc_connected else None, ) - window = VehicleProjectCreatorWindow(manager) # type: ignore[arg-type] + window = VehicleProjectCreatorWindow(manager, from_flight_controller=from_flight_controller) # type: ignore[arg-type] settle_tk(window.root, cycles=6, delay=0.05) return window +def _capture_vehicle_creator_from_flight_controller(output_path: Path, delay: float, padding: int, vehicle_dir: Path) -> None: + """Capture the minimal creator dialog for an already configured flight controller.""" + window = _create_vehicle_creator_window(vehicle_dir, fc_connected=True, from_flight_controller=True) + try: + capture_widget(window.root, output_path, delay, padding) + finally: + if window.root.winfo_exists(): + window.root.destroy() + + def _find_template_browse_button(window: VehicleProjectCreatorWindow) -> tuple[int, int, int, int]: """Find and return bounding box for template browse button.""" browse_button = _find_descendant( @@ -683,7 +725,7 @@ def _find_create_button(window: VehicleProjectCreatorWindow) -> tuple[int, int, """Find and return bounding box for create vehicle directory button.""" create_button = _find_descendant( window.main_frame, - lambda w: _widget_text(w).startswith("Create a vehicle configuration directory"), + lambda w: _widget_text(w).startswith("Create a vehicle project from a template"), ) if create_button is None: msg = "Could not find create vehicle directory button" @@ -786,6 +828,26 @@ def _load_fc_params_from_file(vehicle_dir: Path) -> dict[str, float]: return {name: param.value for name, param in pardict.items()} +def _configure_fake_flight_controller(flight_controller: FlightController, fc_params: dict[str, float]) -> None: + """Configure a connected, non-blocking FC double for screenshot-only GUI workflows.""" + flight_controller.set_master_for_testing(MagicMock()) + flight_controller.fc_parameters = fc_params + flight_controller.request_scaled_imu_messages = MagicMock(return_value=(True, "")) + flight_controller.poll_scaled_imu = MagicMock(return_value=None) + flight_controller.request_periodic_battery_status = MagicMock(return_value=(True, "")) + flight_controller.get_battery_status = MagicMock(return_value=(None, "")) + + +def _cleanup_plugin_view(plugin_view: object) -> None: + """Call optional plugin cleanup hooks on a dynamically-created view.""" + on_deactivate = getattr(plugin_view, "on_deactivate", None) + if callable(on_deactivate): + on_deactivate() + destroy = getattr(plugin_view, "destroy", None) + if callable(destroy): + destroy() + + def _build_parameter_editor( current_file: str, vehicle_dir: Path, @@ -810,19 +872,28 @@ def _build_parameter_editor( flight_controller = FlightController() # Fake an FC connection so the table renders FC values instead of "N/A". - flight_controller.set_master_for_testing(MagicMock()) # make master non-None - flight_controller.fc_parameters = fc_params # pre-populate parameter cache + # Stub telemetry requests as well, because plugin activation runs during Tk event settling. + _configure_fake_flight_controller(flight_controller, fc_params) # Patch download_params so the window startup download returns our fake data # without attempting any real MAVLink communication. try: - with patch.object( - FlightController, - "download_params", - return_value=(fc_params, ParDict()), + with ( + patch.object( + FlightController, + "download_params", + return_value=(fc_params, ParDict()), + ), + patch.object(ParameterEditor, "open_documentation_in_browser"), ): editor = ParameterEditor(current_file, flight_controller, filesystem) window = ParameterEditorWindow(editor) + if current_file == "24_throttle_controller.param": + # This step asks whether the FC values should be copied into the + # file. Choose No before the startup workflow can block on it. + # Leave enough time for the dialog to finish its own setup + # (including focus_set()) before destroying it. + window.root.after(500, lambda: _press_no_on_fc_copy_prompt(window.root)) settle_tk(window.root, cycles=8, delay=0.05) finally: # Restore prior settings to avoid persistent side effects on the user's config. @@ -852,16 +923,13 @@ def _capture_parameter_editor( # pylint: disable=too-many-arguments, too-many-p try: capture_widget(window.root, output_path, delay, padding, scale) finally: + if window.current_plugin_view is not None: + _cleanup_plugin_view(window.current_plugin_view) if window.root.winfo_exists(): window.root.destroy() flight_controller.disconnect() -def _suppress_motor_view_periodic_updates(_view: MotorTestView) -> None: - """Disable periodic updates to keep capture deterministic and non-blocking.""" - return - - def _capture_motor_test(output_path: Path, delay: float, padding: int, vehicle_dir: Path) -> None: fc_params = _load_fc_params_from_file(vehicle_dir) fc_params["FRAME_CLASS"] = 1.0 @@ -881,17 +949,36 @@ def _capture_motor_test(output_path: Path, delay: float, padding: int, vehicle_d save_component_to_system_templates=False, ) flight_controller = FlightController() - flight_controller.set_master_for_testing(MagicMock()) - flight_controller.fc_parameters = fc_params + _configure_fake_flight_controller(flight_controller, fc_params) flight_controller.stop_all_motors = MagicMock(return_value=(True, "")) - model = MotorTestDataModel(flight_controller, filesystem) - with patch.object(MotorTestView, "_update_view", _suppress_motor_view_periodic_updates): - window = MotorTestWindow(model) + model = plugin_factory.create_model( + PLUGIN_MOTOR_TEST, + PluginModelContext( + flight_controller=flight_controller, + local_filesystem=filesystem, + parameter_editor=cast("ParameterEditor", MagicMock()), + ), + ) + if model is None: + msg = f"Could not create {PLUGIN_MOTOR_TEST} plugin model" + raise RuntimeError(msg) + + window = BaseWindow() + window.root.title(translate("ArduPilot Motor Test")) + window.root.geometry(window.calculate_scaled_geometry(400, 610)) + plugin_view = plugin_factory.create(PLUGIN_MOTOR_TEST, window.main_frame, model, window) + if plugin_view is None: + window.root.destroy() + msg = f"Could not create {PLUGIN_MOTOR_TEST} plugin view" + raise RuntimeError(msg) + plugin_view.pack(fill="both", expand=True) try: capture_widget(window.root, output_path, delay, padding) finally: - window.on_close() + _cleanup_plugin_view(plugin_view) + window.root.destroy() + flight_controller.disconnect() def capture_target(target: CaptureTarget, output_path: Path, args: argparse.Namespace) -> None: # pylint: disable=too-many-branches @@ -925,12 +1012,25 @@ def capture_target(target: CaptureTarget, output_path: Path, args: argparse.Name ) elif action == "vehicle_opener": _capture_vehicle_opener(output_path, args.delay, args.padding, args.vehicle_dir) - elif action in ("vehicle_opener_from_template", "vehicle_opener_legacy4", "vehicle_opener_from_bin"): + elif action in ( + "vehicle_opener_from_template", + "vehicle_opener_from_flight_controller", + "vehicle_opener_legacy4", + "vehicle_opener_from_bin", + ): _capture_vehicle_opener_with_highlight( - output_path, args.delay, args.padding, args.vehicle_dir, action=action, scale=target.scale + output_path, + args.delay, + args.padding, + args.vehicle_dir, + action=action, + scale=target.scale, + fc_connected=action == "vehicle_opener_from_flight_controller", ) elif action == "vehicle_creator": _capture_vehicle_creator(output_path, args.delay, args.padding, args.vehicle_dir) + elif action == "vehicle_creator_from_flight_controller": + _capture_vehicle_creator_from_flight_controller(output_path, args.delay, args.padding, args.vehicle_dir) elif action.startswith("vehicle_creator_"): if target.variant is None: msg = f"variant required for {action}" @@ -970,6 +1070,9 @@ def main() -> int: """Program entrypoint.""" args = parse_args() configure_logging(args.log_level) + # Screenshot windows are created directly rather than through application + # startup, so explicitly initialize the same plugin registry first. + register_plugins() pyautogui.FAILSAFE = True diff --git a/tests/test_data_model_ardupilot_parameter.py b/tests/test_data_model_ardupilot_parameter.py index 538a47dfe..be5d51a9a 100755 --- a/tests/test_data_model_ardupilot_parameter.py +++ b/tests/test_data_model_ardupilot_parameter.py @@ -18,6 +18,7 @@ from ardupilot_methodic_configurator.data_model_ardupilot_parameter import ( ArduPilotParameter, BitmaskHelper, + ParameterForcedOrDerivedError, ParameterOutOfRangeError, ParameterUnchangedError, ) @@ -284,7 +285,7 @@ def test_set_new_value(param_fixture) -> None: original_value = param_fixture["forced_param"]._new_value # Act & Assert: User attempts to change forced parameter - with pytest.raises(ValueError, match="forced or derived"): + with pytest.raises(ParameterForcedOrDerivedError, match="forced or derived"): param_fixture["forced_param"].set_new_value(new_value) # Assert: Forced parameter value unchanged @@ -294,7 +295,7 @@ def test_set_new_value(param_fixture) -> None: original_value = param_fixture["derived_param"]._new_value # Act & Assert: User attempts to change derived parameter - with pytest.raises(ValueError, match="forced or derived"): + with pytest.raises(ParameterForcedOrDerivedError, match="forced or derived"): param_fixture["derived_param"].set_new_value(new_value) # Assert: Derived parameter value unchanged diff --git a/tests/test_data_model_configuration_step.py b/tests/test_data_model_configuration_step.py index 2c0fcf2e8..6008c91ea 100755 --- a/tests/test_data_model_configuration_step.py +++ b/tests/test_data_model_configuration_step.py @@ -128,7 +128,7 @@ def test_user_can_process_basic_configuration_step_without_special_operations(se selected_file = "test_file.param" # Act: Process the configuration step - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step(selected_file, fc_parameters) + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step(selected_file, fc_parameters) # Assert: Basic processing completed successfully assert isinstance(parameters, dict) @@ -154,7 +154,7 @@ def test_user_can_process_configuration_step_with_derived_parameters( processor.local_filesystem.merge_forced_or_derived_parameters.return_value = True # Act: Process configuration step with derived parameters - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step( + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) # Assert: Derived parameters were processed # compute_parameters is now called twice: once for "forced", once for "derived" @@ -185,7 +185,7 @@ def test_user_receives_error_feedback_when_derived_parameter_computation_fails( ) # Act: Process configuration step with failing computation - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step( + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) # Assert: Error feedback provided to UI layer # Both forced and derived computation report errors since compute_parameters always returns error @@ -227,7 +227,9 @@ def test_user_can_auto_import_nondefault_parameters_matching_regex(self, process } # Act: Process configuration step - parameters, ui_errors, _, _, _, _ = processor.process_configuration_step(selected_file, test_fc_parameters) + parameters, ui_errors, _, _, _, _, imported_parameters = processor.process_configuration_step( + selected_file, test_fc_parameters + ) # Assert: No errors assert ui_errors == [] @@ -236,6 +238,7 @@ def test_user_can_auto_import_nondefault_parameters_matching_regex(self, process assert "BATT_OPTIONS" in parameters assert parameters["BATT_OPTIONS"].get_new_value() == 5.0 assert parameters["BATT_OPTIONS"].change_reason == "" + assert imported_parameters == {"BATT_OPTIONS"} # BATT_MONITOR should remain untouched (protecting the user comment) assert "BATT_MONITOR" in parameters @@ -265,7 +268,7 @@ def test_user_can_rename_connection_parameters_successfully(self, processor, fc_ processor.local_filesystem.configuration_steps = {selected_file: {"rename_connection": "selected_can"}} # Act: Process configuration step with connection renaming - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step( + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) # Assert: Connection renaming completed successfully assert len(ui_infos) > 0 # Should have info messages about renaming @@ -286,7 +289,7 @@ def test_optional_connection_lookup_missing_does_not_crash(self, processor, fc_p selected_file: {"rename_connection": "vehicle_components['GNSS Receiver']['FC Connection']['Type']"} } - parameters, ui_errors, ui_infos, duplicates_to_remove, renames_to_apply, _ = processor.process_configuration_step( + parameters, ui_errors, ui_infos, duplicates_to_remove, renames_to_apply, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) @@ -313,7 +316,7 @@ def test_user_receives_feedback_about_duplicate_parameter_removal(self, processo # Act: Process configuration step with potential duplicates with patch.object(processor, "calculate_connection_rename_operations") as mock_apply: mock_apply.return_value = ({"CAN_P2_DRIVER"}, [("CAN_P1_DRIVER", "CAN_P2_DRIVER")]) - _parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step( + _parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) # Assert: User informed about duplicate removal assert len(ui_infos) > 0 # Should have info about parameter removal @@ -340,7 +343,7 @@ def test_user_can_process_configuration_step_without_connection_renaming(self, p processor.local_filesystem.merge_forced_or_derived_parameters.return_value = True # Act: Process configuration step without connection renaming - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step( + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) # Assert: Only derived parameters processed, no connection renaming # compute_parameters is now called twice: once for "forced", once for "derived" @@ -376,6 +379,7 @@ def test_connection_renaming_state_does_not_leak_between_steps(self, processor, _duplicates_first, renames_first, _derived_first, + _autoimported_first, ) = processor.process_configuration_step(rename_step, fc_parameters) assert renames_first # Sanity check that renaming took place @@ -387,6 +391,7 @@ def test_connection_renaming_state_does_not_leak_between_steps(self, processor, _duplicates_second, renames_second, _derived_second, + _autoimported_second, ) = processor.process_configuration_step(later_step, fc_parameters) assert renames_second == [] assert ui_infos_second == [] @@ -874,7 +879,7 @@ def test_processor_handles_missing_configuration_steps_gracefully(self, processo processor.local_filesystem.file_parameters[selected_file] = {} # Add empty file entry # Act: Process configuration step - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step(selected_file, fc_parameters) + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step(selected_file, fc_parameters) # Assert: Processing completed without errors assert isinstance(parameters, dict) @@ -896,7 +901,7 @@ def test_processor_handles_empty_parameter_files_gracefully(self, processor, fc_ processor.local_filesystem.file_parameters[selected_file] = {} # Act: Process configuration step - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step(selected_file, fc_parameters) + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step(selected_file, fc_parameters) # Assert: Empty file handled gracefully assert isinstance(parameters, dict) @@ -952,7 +957,7 @@ def test_processor_handles_complex_connection_renaming_edge_cases(self, processo processor.local_filesystem.configuration_steps = {selected_file: {"rename_connection": "selected_can"}} # Act: Process complex connection renaming - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step( + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) # Assert: Complex scenarios handled correctly assert isinstance(parameters, dict) @@ -974,7 +979,7 @@ def test_processor_handles_empty_variables_dictionary(self, processor, fc_parame selected_file = "test_file.param" # Act: Process with empty variables - parameters, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step( + parameters, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step( selected_file, fc_parameters ) # Assert: Processing completed successfully assert isinstance(parameters, dict) @@ -1003,7 +1008,7 @@ def test_user_receives_expresslrs_warning_when_fltmode_ch_is_5_and_expresslrs_de test_fc_params["FLTMODE_CH"] = 5 # Act: Process configuration step - _, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) + _, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) # Assert: ExpressLRS warning is present assert len(ui_infos) == 1 @@ -1034,7 +1039,7 @@ def test_user_does_not_receive_expresslrs_warning_when_fltmode_ch_is_not_5_and_e test_fc_params["FLTMODE_CH"] = 6 # Act: Process configuration step - _, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) + _, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) # Assert: No ExpressLRS warning assert ui_infos == [] @@ -1060,7 +1065,7 @@ def test_user_does_not_receive_expresslrs_warning_when_expresslrs_not_detected(s test_fc_params["FLTMODE_CH"] = 5 # Act: Process configuration step - _, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) + _, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) # Assert: No ExpressLRS warning assert ui_infos == [] @@ -1086,7 +1091,7 @@ def test_user_receives_expresslrs_warning_with_both_bits_set_and_fltmode_ch_5(se test_fc_params["FLTMODE_CH"] = 5 # Act: Process configuration step - _, ui_errors, ui_infos, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) + _, ui_errors, ui_infos, _, _, _, _ = processor.process_configuration_step(selected_file, test_fc_params) # Assert: ExpressLRS warning is present assert len(ui_infos) == 1 @@ -1125,9 +1130,15 @@ def test_derived_parameters_filtered_by_fc_keys_when_fc_provided(self, processor # FC only has SERIAL1_PROTOCOL (not CAN_P1_DRIVER) limited_fc_params = {"SERIAL1_PROTOCOL": 4.0} - _params, ui_errors, _ui_infos, _duplicates, _renames, derived_to_apply = processor.process_configuration_step( - selected_file, limited_fc_params - ) + ( + _params, + ui_errors, + _ui_infos, + _duplicates, + _renames, + derived_to_apply, + _autoimported, + ) = processor.process_configuration_step(selected_file, limited_fc_params) assert ui_errors == [] # SERIAL1_PROTOCOL is in both file and FC, so it should be in derived_to_apply @@ -1165,9 +1176,15 @@ def test_derived_parameters_not_in_file_are_included_for_adding_to_gui(self, pro fc_params = {"SERIAL1_PROTOCOL": 4.0, "NEW_PARAM": 99.0} - _params, ui_errors, _ui_infos, _duplicates, _renames, derived_to_apply = processor.process_configuration_step( - selected_file, fc_params - ) + ( + _params, + ui_errors, + _ui_infos, + _duplicates, + _renames, + derived_to_apply, + _autoimported, + ) = processor.process_configuration_step(selected_file, fc_params) assert ui_errors == [] # Parameters in file should be included @@ -1202,9 +1219,15 @@ def test_derived_parameters_all_included_when_no_fc_parameters(self, processor, } # No FC parameters (empty dict simulates offline mode) - _params, ui_errors, _ui_infos, _duplicates, _renames, derived_to_apply = processor.process_configuration_step( - selected_file, {} - ) + ( + _params, + ui_errors, + _ui_infos, + _duplicates, + _renames, + derived_to_apply, + _autoimported, + ) = processor.process_configuration_step(selected_file, {}) assert ui_errors == [] # Both should be included since fc_param_keys is empty (no FC filter) @@ -1298,7 +1321,15 @@ def test_deleted_parameter_is_not_auto_imported_from_fc(self, processor) -> None # Act (When): process the step with a non-default FC value for the to-be-deleted param fc_params = {"BATT_OPTIONS": 5.0} - parameters, ui_errors, _ui_infos, _dup, _ren, _derived = processor.process_configuration_step(selected_file, fc_params) + ( + parameters, + ui_errors, + _ui_infos, + _dup, + _ren, + _derived, + _autoimported, + ) = processor.process_configuration_step(selected_file, fc_params) # Assert (Then): BATT_OPTIONS absent because it is in the delete set assert "BATT_OPTIONS" not in parameters @@ -1337,12 +1368,15 @@ def test_apply_auto_imports_skips_parameters_in_delete_set(self, processor) -> N current_step_parameters: dict = {} # Act (When): run auto-import with the delete set - processor._apply_auto_imports(selected_file, fc_params, current_step_parameters, parameters_to_delete) + imported_parameters = processor._apply_auto_imports( + selected_file, fc_params, current_step_parameters, parameters_to_delete + ) # Assert (Then): imported param present with correct value; deleted param absent assert "BATT_OPTIONS" in current_step_parameters assert current_step_parameters["BATT_OPTIONS"].get_new_value() == 5.0 assert "BATT_MONITOR" not in current_step_parameters + assert imported_parameters == {"BATT_OPTIONS"} def test_apply_auto_imports_with_none_delete_set_behaves_as_empty_set(self, processor) -> None: """ diff --git a/tests/test_data_model_parameter_editor.py b/tests/test_data_model_parameter_editor.py index aa3b3d949..076376e2e 100755 --- a/tests/test_data_model_parameter_editor.py +++ b/tests/test_data_model_parameter_editor.py @@ -17,6 +17,7 @@ from ardupilot_methodic_configurator.data_model_ardupilot_parameter import ( ArduPilotParameter, + ParameterForcedOrDerivedError, ParameterOutOfRangeError, ParameterUnchangedError, ) @@ -3519,6 +3520,7 @@ def test_user_can_apply_valid_derived_parameters(self, parameter_editor) -> None [], # duplicates_to_remove [], # renames_to_apply derived_params, # derived_params + set(), # autoimported_parameters ), ): parameter_editor._repopulate_configuration_step_parameters() @@ -3564,6 +3566,7 @@ def test_user_receives_error_when_derived_param_is_readonly(self, parameter_edit [], # duplicates_to_remove [], # renames_to_apply derived_params, # derived_params + set(), # autoimported_parameters ), ), patch("ardupilot_methodic_configurator.data_model_parameter_editor.logging_error") as mock_log_error, @@ -3612,6 +3615,7 @@ def test_user_receives_error_when_derived_param_not_marked_as_forced_or_derived( [], [], derived_params, + set(), ), ), patch("ardupilot_methodic_configurator.data_model_parameter_editor.logging_error") as mock_log_error, @@ -3651,6 +3655,7 @@ def test_user_can_see_derived_param_added_when_not_in_file(self, parameter_edito [], [], derived_params, + set(), ), ): parameter_editor._repopulate_configuration_step_parameters() @@ -3680,7 +3685,7 @@ def test_user_can_see_forced_param_added_when_not_in_file(self, parameter_editor patch.object( parameter_editor._config_step_processor, "process_configuration_step", - return_value=({}, [], [], set(), [], ParDict()), + return_value=({}, [], [], set(), [], ParDict(), set()), ), patch.object( parameter_editor._config_step_processor, @@ -3694,6 +3699,31 @@ def test_user_can_see_forced_param_added_when_not_in_file(self, parameter_editor assert parameter_editor.current_step_parameters["NEW_FORCED"] is mock_ap_param assert "NEW_FORCED" in parameter_editor._added_parameters + def test_autoimported_parameter_is_tracked_for_saving(self, parameter_editor) -> None: + """Auto-imported parameters trigger the normal save workflow.""" + parameter_editor.current_file = "test_file.param" + parameter_editor._last_time_asked_to_save = 0.0 + autoimported = ArduPilotParameter("AUTO_IMPORTED", Par(2.0), fc_value=2.0) + with patch.object( + parameter_editor._config_step_processor, + "process_configuration_step", + return_value=({"AUTO_IMPORTED": autoimported}, [], [], set(), [], ParDict(), {"AUTO_IMPORTED"}), + ): + parameter_editor._repopulate_configuration_step_parameters() + + assert parameter_editor._has_unsaved_changes() + + with patch.object(parameter_editor, "_export_current_file") as mock_export: + assert ( + parameter_editor.handle_write_changes_workflow( + annotate_params_into_files=False, + ask_user_confirmation=MagicMock(return_value=True), + ) + is True + ) + + mock_export.assert_called_once_with(annotate_doc=False) + def test_connected_editor_delegates_plugin_model_creation_to_registry(self, parameter_editor) -> None: """The editor supplies shared dependencies without importing concrete plugin models.""" parameter_editor._flight_controller.master = MagicMock() @@ -4501,7 +4531,7 @@ def test_system_tracks_renamed_parameters_during_repopulation(self, parameter_ed patch.object( parameter_editor._config_step_processor, "process_configuration_step", - return_value=({"OLD": MagicMock()}, [], [], [], [("OLD", "NEW")], ParDict()), + return_value=({"OLD": MagicMock()}, [], [], [], [("OLD", "NEW")], ParDict(), set()), ), patch.object(parameter_editor._config_step_processor, "create_ardupilot_parameter", return_value=mock_new_param), ): @@ -4529,7 +4559,7 @@ def test_system_applies_derived_parameter_reason_during_repopulation(self, param with patch.object( parameter_editor._config_step_processor, "process_configuration_step", - return_value=({"DER": mock_der}, [], [], [], [], ParDict({"DER": Par(2.0, "because math")})), + return_value=({"DER": mock_der}, [], [], [], [], ParDict({"DER": Par(2.0, "because math")}), set()), ): parameter_editor._repopulate_configuration_step_parameters() @@ -6149,8 +6179,35 @@ def test_system_skips_a_flight_controller_value_that_cannot_be_converted(self, p good = ArduPilotParameter("GOOD", Par(1.0)) parameter_editor.current_step_parameters = {"BAD": bad, "GOOD": good} - assert parameter_editor._update_parameters_from_fc_values({"BAD": 1.0, "GOOD": 2.0}) is True + with patch("ardupilot_methodic_configurator.data_model_parameter_editor.logging_exception") as mock_exception: + assert parameter_editor._update_parameters_from_fc_values({"BAD": 1.0, "GOOD": 2.0}) is True + assert good.get_new_value() == 2.0 + mock_exception.assert_called_once_with("Failed to update in-memory value for BAD after FC copy") + + def test_system_warns_without_a_traceback_when_a_parameter_is_forced_or_derived( + self, parameter_editor: ParameterEditor + ) -> None: + """ + A forced or derived parameter cannot be replaced by a value from the flight controller. + + GIVEN: A parameter that rejects an FC value because it is forced or derived + WHEN: The FC value is copied into the current file + THEN: Its error message is logged as a warning without an exception traceback + """ + param = MagicMock() + error_message = "This parameter is forced or derived and cannot be changed." + error = ParameterForcedOrDerivedError(error_message) + param.set_new_value.side_effect = error + parameter_editor.current_step_parameters = {"FORCED": param} + + with patch("ardupilot_methodic_configurator.data_model_parameter_editor.logging_warning") as mock_warning: + assert parameter_editor._update_parameters_from_fc_values({"FORCED": 1.0}) is False + + mock_warning.assert_called_once_with( + "Parameter FORCED could not be updated because it is forced or derived: " + "This parameter is forced or derived and cannot be changed." + ) def test_system_skips_a_flight_controller_value_absent_from_the_current_step( self, parameter_editor: ParameterEditor @@ -6299,7 +6356,7 @@ def test_step_declared_parameters_are_added_to_the_current_step(self, parameter_ with patch.object( parameter_editor._config_step_processor, "process_configuration_step", - return_value=({}, [], [], [], [], {}), + return_value=({}, [], [], [], [], {}, set()), ): parameter_editor._repopulate_configuration_step_parameters() @@ -6324,7 +6381,7 @@ def test_step_declared_deletions_remove_parameters_from_the_current_step(self, p with patch.object( parameter_editor._config_step_processor, "process_configuration_step", - return_value=({"OLD_PARAM": MagicMock()}, [], [], [], [], {}), + return_value=({"OLD_PARAM": MagicMock()}, [], [], [], [], {}, set()), ): parameter_editor._repopulate_configuration_step_parameters() @@ -6746,7 +6803,7 @@ def test_a_duplicate_present_in_the_file_is_tracked_as_deleted(self, parameter_e with patch.object( parameter_editor._config_step_processor, "process_configuration_step", - return_value=({"DUP_PARAM": MagicMock()}, [], [], {"DUP_PARAM"}, [], {}), + return_value=({"DUP_PARAM": MagicMock()}, [], [], {"DUP_PARAM"}, [], {}, set()), ): parameter_editor._repopulate_configuration_step_parameters() @@ -6772,7 +6829,7 @@ def test_a_duplicate_absent_from_the_file_is_removed_without_a_deletion_record( with patch.object( parameter_editor._config_step_processor, "process_configuration_step", - return_value=({"DUP_PARAM": MagicMock()}, [], [], {"DUP_PARAM"}, [], {}), + return_value=({"DUP_PARAM": MagicMock()}, [], [], {"DUP_PARAM"}, [], {}, set()), ): parameter_editor._repopulate_configuration_step_parameters() diff --git a/tests/test_data_model_vehicle_project.py b/tests/test_data_model_vehicle_project.py index 3a0c80795..484e00c15 100755 --- a/tests/test_data_model_vehicle_project.py +++ b/tests/test_data_model_vehicle_project.py @@ -1,1584 +1,1868 @@ -#!/usr/bin/env python3 - -""" -Tests for data_model_vehicle_project.py module. - -This module tests the VehicleProjectManager class which provides a unified interface -for all vehicle project operations, acting as a facade that coordinates between -different data models. - -This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator - -SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas - -SPDX-License-Identifier: GPL-3.0-or-later -""" - -from unittest.mock import MagicMock, patch - -import pytest - -from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem -from ardupilot_methodic_configurator.data_model_par_dict import ParDict -from ardupilot_methodic_configurator.data_model_vehicle_project import VehicleProjectManager -from ardupilot_methodic_configurator.data_model_vehicle_project_creator import ( - NewVehicleProjectSettings, - VehicleProjectCreationError, -) -from ardupilot_methodic_configurator.data_model_vehicle_project_opener import VehicleProjectOpenError - -# pylint: disable=protected-access, too-many-lines - - -class TestVehicleProjectManagerInitialization: - """Test VehicleProjectManager initialization and basic properties.""" - - def test_user_can_initialize_project_manager_without_flight_controller(self) -> None: - """ - User can create project manager without flight controller. - - GIVEN: A user wants to manage vehicle projects without a flight controller - WHEN: User initializes VehicleProjectManager with only local filesystem - THEN: Manager should be created with all internal components initialized - """ - # Arrange: Create filesystem instance - mock_filesystem = MagicMock(spec=LocalFilesystem) - - # Act: Initialize project manager without flight controller - manager = VehicleProjectManager(mock_filesystem) - - # Assert: Manager is properly initialized - assert manager._local_filesystem is mock_filesystem - assert manager._flight_controller is None - assert manager._creator is not None - assert manager._opener is not None - assert manager._settings is None - assert manager.configuration_template == "" - - def test_user_can_initialize_project_manager_with_flight_controller(self) -> None: - """ - User can create project manager with flight controller. - - GIVEN: A user wants to manage vehicle projects with a connected flight controller - WHEN: User initializes VehicleProjectManager with filesystem and flight controller - THEN: Manager should be created with flight controller reference stored - """ - # Arrange: Create filesystem and flight controller instances - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() - - # Act: Initialize project manager with flight controller - manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) - - # Assert: Manager is properly initialized with flight controller - assert manager._local_filesystem is mock_filesystem - assert manager._flight_controller is mock_flight_controller - assert manager._creator is not None - assert manager._opener is not None - - def test_user_can_get_fc_parameters_when_fc_connected(self) -> None: - """ - User can retrieve FC parameters when flight controller is connected. - - GIVEN: A project manager with a connected flight controller that has parameters - WHEN: User requests FC parameters - THEN: Should return the flight controller's parameters dictionary - """ - # Arrange: Create manager with FC that has parameters - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() - mock_flight_controller.fc_parameters = {"PARAM1": 1.0, "PARAM2": 2.0} - manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) - - # Act: Get FC parameters - fc_params = manager.fc_parameters() - - # Assert: FC parameters are returned - assert fc_params == {"PARAM1": 1.0, "PARAM2": 2.0} - - def test_user_gets_none_when_fc_not_connected(self) -> None: - """ - User receives None when no flight controller is connected. - - GIVEN: A project manager without a flight controller - WHEN: User requests FC parameters - THEN: Should return None - """ - # Arrange: Create manager without FC - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Get FC parameters - fc_params = manager.fc_parameters() - - # Assert: None is returned - assert fc_params is None - - def test_user_gets_fc_parameters_even_if_empty(self) -> None: - """ - User receives empty dict when FC is connected but has no parameters yet. - - GIVEN: A project manager with FC that hasn't loaded parameters yet - WHEN: User requests FC parameters - THEN: Should return empty dictionary - """ - # Arrange: Create manager with FC that has empty parameters - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() - mock_flight_controller.fc_parameters = {} - manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) - - # Act: Get FC parameters - fc_params = manager.fc_parameters() - - # Assert: Empty dict is returned - assert fc_params == {} - - -class TestDirectoryAndPathOperations: - """Test directory and path related operations.""" - - def test_user_can_get_recently_used_directories(self) -> None: - """ - User can retrieve recently used directories. - - GIVEN: A project manager with stored directory preferences - WHEN: User requests recently used directories - THEN: Should return tuple of template, base, and vehicle directories - """ - # Arrange: Mock filesystem and recently used directories - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "get_recently_used_dirs") as mock_get_dirs: - mock_get_dirs.return_value = ("/templates", "/base", "/vehicle") - - # Act: Get recently used directories - template_dir, new_base_dir, vehicle_dir = manager.get_recently_used_dirs() - - # Assert: Correct directories returned - assert template_dir == "/templates" - assert new_base_dir == "/base" - assert vehicle_dir == "/vehicle" - mock_get_dirs.assert_called_once() - - def test_creation_stores_template_and_base_dirs_in_history(self) -> None: - """ - Manager stores the template and base directories in history after creation. - - GIVEN: A user creates a new vehicle from a template - WHEN: Creation succeeds - THEN: The template directory and base directory are stored in recently-used history - """ - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with ( - patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - patch.object(LocalFilesystem, "store_recently_used_template_dirs") as mock_store_template, - patch.object(LocalFilesystem, "store_recently_used_vehicle_dir"), - ): - mock_create.return_value = "/new/vehicle/path" - mock_open.return_value = "/new/vehicle/path" - settings = MagicMock(spec=NewVehicleProjectSettings) - - # Act - manager.create_new_vehicle_from_template("/templates/ArduCopter", "/vehicles", "Name", settings) - - # Assert: only the template/base history is the concern of this test - mock_store_template.assert_called_once_with("/templates/ArduCopter", "/vehicles") - - def test_creation_opens_new_directory_and_updates_vehicle_history(self) -> None: - """ - Manager opens the newly created directory and records it in vehicle history. - - GIVEN: A user creates a new vehicle from a template - WHEN: Creation succeeds - THEN: open_vehicle_directory is called with the new path - AND: The new path is stored exactly once in recent-vehicle history - """ - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with ( - patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - patch.object(LocalFilesystem, "store_recently_used_template_dirs"), - patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store_vehicle, - ): - mock_create.return_value = "/new/vehicle/path" - mock_open.return_value = "/new/vehicle/path" - settings = MagicMock(spec=NewVehicleProjectSettings) - - # Act - manager.create_new_vehicle_from_template("/templates/ArduCopter", "/vehicles", "Name", settings) - - # Assert: the new directory is opened and the vehicle dir stored exactly once - mock_open.assert_called_once_with("/new/vehicle/path") - mock_store_vehicle.assert_called_once_with("/new/vehicle/path") - - def test_user_can_get_current_working_directory(self) -> None: - """ - User can get current working directory. - - GIVEN: A project manager in any state - WHEN: User requests current working directory - THEN: Should return the current working directory path - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "getcwd") as mock_getcwd: - mock_getcwd.return_value = "/current/working/dir" - - # Act: Get current working directory - result = manager.get_current_working_directory() - - # Assert: Correct directory returned - assert result == "/current/working/dir" - mock_getcwd.assert_called_once() - - def test_user_can_extract_directory_name_from_path(self) -> None: - """ - User can extract directory name from full path. - - GIVEN: A project manager and a full path - WHEN: User requests directory name extraction - THEN: Should return just the directory name - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "get_directory_name_from_full_path") as mock_get_name: - mock_get_name.return_value = "vehicle_name" - - # Act: Extract directory name - result = manager.get_directory_name_from_path("/path/to/vehicle_name") - - # Assert: Correct name returned - assert result == "vehicle_name" - mock_get_name.assert_called_once_with("/path/to/vehicle_name") - - def test_user_can_check_if_directory_exists(self) -> None: - """ - User can check if a directory exists. - - GIVEN: A project manager and a directory path - WHEN: User checks if directory exists - THEN: Should return boolean indicating existence - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "directory_exists") as mock_exists: - mock_exists.return_value = True - - # Act: Check directory existence - result = manager.directory_exists("/test/path") - - # Assert: Correct existence status returned - assert result is True - mock_exists.assert_called_once_with("/test/path") - - def test_user_can_validate_directory_name(self) -> None: - """ - User can validate directory name. - - GIVEN: A project manager and a directory name - WHEN: User validates directory name - THEN: Should return boolean indicating validity - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "valid_directory_name") as mock_valid: - mock_valid.return_value = True - - # Act: Validate directory name - result = manager.valid_directory_name("valid_name") - - # Assert: Correct validation result returned - assert result is True - mock_valid.assert_called_once_with("valid_name") - - -class TestVehicleProjectCreation: - """Test vehicle project creation operations.""" - - def test_user_can_create_new_vehicle_from_template_successfully(self) -> None: - """ - User can create new vehicle from template successfully. - - GIVEN: A project manager with valid template and settings - WHEN: User creates new vehicle from template - THEN: Should create vehicle directory and update manager state - """ - # Arrange: Mock filesystem and components - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() - mock_flight_controller.master = MagicMock() # FC is connected - manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) - - # Mock the creator and opener - with ( - patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - patch.object(LocalFilesystem, "store_recently_used_template_dirs"), - patch.object(LocalFilesystem, "store_recently_used_vehicle_dir"), - ): - mock_create.return_value = "/new/vehicle/path" - mock_open.return_value = "/new/vehicle/path" - - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - - # Act: Create new vehicle from template - result = manager.create_new_vehicle_from_template("/template/path", "/base/path", "NewVehicle", mock_settings) - - # Assert: correct path returned and manager state updated - assert result == "/new/vehicle/path" - assert manager._settings is mock_settings - assert manager.configuration_template == "path" # last component of template path - - # Assert: creator called with fc_connected=True (FC master is set) and opener called afterwards - fc_connected = True # FC master is set in the fixture above - mock_create.assert_called_once_with( - "/template/path", "/base/path", "NewVehicle", mock_settings, fc_connected, mock_flight_controller.fc_parameters - ) - mock_open.assert_called_once_with("/new/vehicle/path") - - def test_user_sees_error_when_vehicle_creation_fails(self) -> None: - """ - User sees error when vehicle creation fails. - - GIVEN: A project manager with invalid settings - WHEN: User attempts to create vehicle from template - THEN: Should raise VehicleProjectCreationError - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Mock the creator to raise an exception - with patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create: - mock_create.side_effect = VehicleProjectCreationError("Creation Error", "Creation failed") - - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - - # Act & Assert: Creation should raise error - with pytest.raises(VehicleProjectCreationError, match="Creation failed"): - manager.create_new_vehicle_from_template("/template/path", "/base/path", "NewVehicle", mock_settings) - - def test_opener_not_called_when_creation_fails(self) -> None: - """ - Opener must not be invoked when the creator raises an error. - - GIVEN: A project manager whose creator raises a VehicleProjectCreationError - WHEN: User attempts to create a vehicle from a template - THEN: The opener is never called - AND: The VehicleProjectCreationError propagates to the caller - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with ( - patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - ): - mock_create.side_effect = VehicleProjectCreationError("Creation Error", "Creation failed") - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - - # Act & Assert: error propagates and opener is never touched - with pytest.raises(VehicleProjectCreationError): - manager.create_new_vehicle_from_template("/template/path", "/base/path", "NewVehicle", mock_settings) - - mock_open.assert_not_called() - - -class TestVehicleProjectOpening: - """Test vehicle project opening operations.""" - - def test_user_can_open_vehicle_directory_successfully(self) -> None: - """ - User can open existing vehicle directory successfully. - - GIVEN: A project manager with valid vehicle directory - WHEN: User opens vehicle directory - THEN: Should open directory, update history and return path - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Mock the opener and history store - with ( - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, - ): - mock_open.return_value = "/opened/vehicle/path" - - # Act: Open vehicle directory - result = manager.open_vehicle_directory("/vehicle/path") - - # Assert: Directory opened successfully and history recorded - assert result == "/opened/vehicle/path" - mock_open.assert_called_once_with("/vehicle/path") - mock_store.assert_called_once_with("/opened/vehicle/path") - - def test_user_sees_error_when_vehicle_directory_opening_fails(self) -> None: - """ - User sees error when vehicle directory opening fails. - - GIVEN: A project manager with invalid vehicle directory - WHEN: User attempts to open vehicle directory - THEN: Should raise VehicleProjectOpenError and not update history - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Mock the opener to raise an exception - with ( - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, - ): - mock_open.side_effect = VehicleProjectOpenError("Open Error", "Opening failed") - - # Act & Assert: Opening should raise error - with pytest.raises(VehicleProjectOpenError, match="Opening failed"): - manager.open_vehicle_directory("/invalid/path") - - mock_store.assert_not_called() - - def test_user_can_open_last_vehicle_directory_successfully(self) -> None: - """ - User can open last used vehicle directory successfully. - - GIVEN: A project manager with last used vehicle directory - WHEN: User opens last vehicle directory - THEN: Should open directory, update history and return path - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Mock the opener and history storage - with ( - patch.object(manager._opener, "open_last_vehicle_directory") as mock_open, - patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, - ): - mock_open.return_value = "/last/vehicle/path" - - # Act: Open last vehicle directory - result = manager.open_last_vehicle_directory("/last/path") - - # Assert: Directory opened successfully and history recorded - assert result == "/last/vehicle/path" - mock_open.assert_called_once_with("/last/path") - mock_store.assert_called_once_with("/last/vehicle/path") - - def test_user_sees_error_when_opening_last_vehicle_directory_fails(self) -> None: - """ - User sees error when opening last vehicle directory fails. - - GIVEN: A project manager with invalid last vehicle directory - WHEN: User attempts to open last vehicle directory - THEN: Should raise VehicleProjectOpenError and not update history - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Mock the opener to raise an exception - with ( - patch.object(manager._opener, "open_last_vehicle_directory") as mock_open, - patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, - ): - mock_open.side_effect = VehicleProjectOpenError("Last Open Error", "Last directory opening failed") - - # Act & Assert: Opening should raise error - with pytest.raises(VehicleProjectOpenError, match="Last directory opening failed"): - manager.open_last_vehicle_directory("/invalid/last/path") - - mock_store.assert_not_called() - - -class TestFilesystemStateManagement: - """Test filesystem state management operations.""" - - def test_user_can_get_current_vehicle_directory(self) -> None: - """ - User can get current vehicle directory from filesystem. - - GIVEN: A project manager with filesystem containing vehicle directory - WHEN: User requests current vehicle directory - THEN: Should return filesystem's vehicle directory - """ - # Arrange: Mock filesystem with vehicle directory - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_filesystem.vehicle_dir = "/current/vehicle" - manager = VehicleProjectManager(mock_filesystem) - - # Act: Get vehicle directory - result = manager.get_vehicle_directory() - - # Assert: Correct vehicle directory returned - assert result == "/current/vehicle" - - def test_user_can_store_recently_used_template_directories(self) -> None: - """ - User can store recently used template and base directories. - - GIVEN: A project manager and template/base directories - WHEN: User stores recently used template directories - THEN: Should delegate to LocalFilesystem for storage - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "store_recently_used_template_dirs") as mock_store: - # Act: Store template directories - manager.store_recently_used_template_dirs("/template", "/base") - - # Assert: Storage delegated correctly - mock_store.assert_called_once_with("/template", "/base") - - def test_user_can_store_recently_used_vehicle_directory(self) -> None: - """ - User can store recently used vehicle directory. - - GIVEN: A project manager and vehicle directory - WHEN: User stores recently used vehicle directory - THEN: Should delegate to LocalFilesystem for storage - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store: - # Act: Store vehicle directory - manager.store_recently_used_vehicle_dir("/vehicle") - - # Assert: Storage delegated correctly - mock_store.assert_called_once_with("/vehicle") - - -class TestProjectSettingsProperties: - """Test project settings property access.""" - - def test_user_can_access_reset_fc_parameters_property_when_settings_exist(self) -> None: - """ - User can access reset FC parameters property when settings exist. - - GIVEN: A project manager with settings configured - WHEN: User accesses reset_fc_parameters_to_their_defaults property - THEN: Should return the setting value from project settings - """ - # Arrange: Mock filesystem and settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - mock_settings.reset_fc_parameters_to_their_defaults = True - manager._settings = mock_settings - - # Act: Access property - result = manager.reset_fc_parameters_to_their_defaults - - # Assert: Correct value returned - assert result is True - - def test_user_gets_false_for_reset_fc_parameters_when_no_settings(self) -> None: - """ - User gets False for reset FC parameters when no settings exist. - - GIVEN: A project manager without settings configured - WHEN: User accesses reset_fc_parameters_to_their_defaults property - THEN: Should return False - """ - # Arrange: Mock filesystem without settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Access property - result = manager.reset_fc_parameters_to_their_defaults - - # Assert: False returned for missing settings - assert result is False - - def test_user_can_access_blank_component_data_property_when_settings_exist(self) -> None: - """ - User can access blank component data property when settings exist. - - GIVEN: A project manager with settings configured - WHEN: User accesses blank_component_data property - THEN: Should return the setting value from project settings - """ - # Arrange: Mock filesystem and settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - mock_settings.blank_component_data = True - manager._settings = mock_settings - - # Act: Access property - result = manager.blank_component_data - - # Assert: Correct value returned - assert result is True - - def test_user_gets_false_for_blank_component_data_when_no_settings(self) -> None: - """ - User gets False for blank component data when no settings exist. - - GIVEN: A project manager without settings configured - WHEN: User accesses blank_component_data property - THEN: Should return False - """ - # Arrange: Mock filesystem without settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Access property - result = manager.blank_component_data - - # Assert: False returned for missing settings - assert result is False - - def test_user_can_access_infer_comp_specs_property_when_settings_exist(self) -> None: - """ - User can access infer component specs property when settings exist. - - GIVEN: A project manager with settings configured - WHEN: User accesses infer_comp_specs_and_conn_from_fc_params property - THEN: Should return the setting value from project settings - """ - # Arrange: Mock filesystem and settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - mock_settings.infer_comp_specs_and_conn_from_fc_params = True - manager._settings = mock_settings - - # Act: Access property - result = manager.infer_comp_specs_and_conn_from_fc_params - - # Assert: Correct value returned - assert result is True - - def test_user_gets_false_for_infer_comp_specs_when_no_settings(self) -> None: - """ - User gets False for infer component specs when no settings exist. - - GIVEN: A project manager without settings configured - WHEN: User accesses infer_comp_specs_and_conn_from_fc_params property - THEN: Should return False - """ - # Arrange: Mock filesystem without settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Access property - result = manager.infer_comp_specs_and_conn_from_fc_params - - # Assert: False returned for missing settings - assert result is False - - def test_user_can_access_use_fc_params_property_when_settings_exist(self) -> None: - """ - User can access use FC params property when settings exist. - - GIVEN: A project manager with settings configured - WHEN: User accesses use_fc_params property - THEN: Should return the setting value from project settings - """ - # Arrange: Mock filesystem and settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - mock_settings.use_fc_params = True - manager._settings = mock_settings - - # Act: Access property - result = manager.use_fc_params - - # Assert: Correct value returned - assert result is True - - def test_user_gets_false_for_use_fc_params_when_no_settings(self) -> None: - """ - User gets False for use FC params when no settings exist. - - GIVEN: A project manager without settings configured - WHEN: User accesses use_fc_params property - THEN: Should return False - """ - # Arrange: Mock filesystem without settings - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Access property - result = manager.use_fc_params - - # Assert: False returned for missing settings - assert result is False - - -class TestFlightControllerOperations: - """Test flight controller related operations.""" - - def test_user_can_check_flight_controller_connection_when_connected(self) -> None: - """ - User can check flight controller connection when connected. - - GIVEN: A project manager with connected flight controller - WHEN: User checks flight controller connection - THEN: Should return True - """ - # Arrange: Mock filesystem and connected flight controller - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() - mock_flight_controller.master = MagicMock() # Connected - manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) - - # Act: Check connection - result = manager.is_flight_controller_connected() - - # Assert: Connection detected - assert result is True - - def test_user_can_check_flight_controller_connection_when_disconnected(self) -> None: - """ - User can check flight controller connection when disconnected. - - GIVEN: A project manager with disconnected flight controller - WHEN: User checks flight controller connection - THEN: Should return False - """ - # Arrange: Mock filesystem and disconnected flight controller - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() - mock_flight_controller.master = None # Disconnected - manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) - - # Act: Check connection - result = manager.is_flight_controller_connected() - - # Assert: No connection detected - assert result is False - - def test_user_can_check_flight_controller_connection_when_no_controller(self) -> None: - """ - User can check flight controller connection when no controller exists. - - GIVEN: A project manager without flight controller - WHEN: User checks flight controller connection - THEN: Should return False - """ - # Arrange: Mock filesystem without flight controller - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Check connection - result = manager.is_flight_controller_connected() - - # Assert: No connection detected - assert result is False - - def test_user_can_check_if_last_vehicle_directory_can_be_opened_when_exists(self) -> None: - """ - User can check if last vehicle directory can be opened when it exists. - - GIVEN: A project manager with existing last vehicle directory - WHEN: User checks if last vehicle directory can be opened - THEN: Should return True - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(manager, "directory_exists") as mock_exists: - mock_exists.return_value = True - - # Act: Check if can open last directory - result = manager.can_open_last_vehicle_directory("/existing/path") - - # Assert: Can open existing directory - assert result is True - mock_exists.assert_called_once_with("/existing/path") - - def test_user_can_check_if_last_vehicle_directory_can_be_opened_when_not_exists(self) -> None: - """ - User can check if last vehicle directory can be opened when it doesn't exist. - - GIVEN: A project manager with non-existing last vehicle directory - WHEN: User checks if last vehicle directory can be opened - THEN: Should return False - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(manager, "directory_exists") as mock_exists: - mock_exists.return_value = False - - # Act: Check if can open last directory - result = manager.can_open_last_vehicle_directory("/nonexistent/path") - - # Assert: Cannot open non-existent directory - assert result is False - mock_exists.assert_called_once_with("/nonexistent/path") - - def test_user_can_check_if_last_vehicle_directory_can_be_opened_when_empty_path(self) -> None: - """ - User can check if last vehicle directory can be opened when path is empty. - - GIVEN: A project manager with empty last vehicle directory path - WHEN: User checks if last vehicle directory can be opened - THEN: Should return False - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Check if can open empty path - result = manager.can_open_last_vehicle_directory("") - - # Assert: Cannot open empty path - assert result is False - - -class TestIntroductionMessageAndFileOperations: - """Test introduction message generation and file operations.""" - - def test_user_gets_working_directory_message_when_in_current_directory(self) -> None: - """ - User gets working directory message when in current directory. - - GIVEN: A project manager where vehicle directory equals working directory - WHEN: User requests introduction message - THEN: Should return current working directory message - """ - # Arrange: Mock filesystem with equal directories - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_filesystem.vehicle_dir = "/working/dir" - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(manager, "get_current_working_directory") as mock_getcwd: - mock_getcwd.return_value = "/working/dir" - - # Act: Get introduction message - result = manager.get_introduction_message() - - # Assert: Current working directory message returned - assert "current working directory" in result - - def test_user_gets_vehicle_dir_message_when_in_different_directory(self) -> None: - """ - User gets vehicle dir message when in different directory. - - GIVEN: A project manager where vehicle directory differs from working directory - WHEN: User requests introduction message - THEN: Should return vehicle directory specified message - """ - # Arrange: Mock filesystem with different directories - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_filesystem.vehicle_dir = "/vehicle/dir" - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(manager, "get_current_working_directory") as mock_getcwd: - mock_getcwd.return_value = "/working/dir" - - # Act: Get introduction message - result = manager.get_introduction_message() - - # Assert: Vehicle directory specified message returned - assert "--vehicle-dir specified directory" in result - - def test_user_can_get_file_parameters_list(self) -> None: - """ - User can get list of intermediate parameter files. - - GIVEN: A project manager with filesystem containing parameter files - WHEN: User requests file parameters list - THEN: Should return list of parameter file names - """ - # Arrange: Mock filesystem with parameter files - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_filesystem.file_parameters = { - "01_first.param": {}, - "02_second.param": {}, - "03_third.param": {}, - } - manager = VehicleProjectManager(mock_filesystem) - - # Act: Get file parameters list - result = manager.get_file_parameters_list() - - # Assert: Correct list of parameter files returned - assert len(result) == 3 - assert "01_first.param" in result - assert "02_second.param" in result - assert "03_third.param" in result - - def test_user_can_get_default_vehicle_name(self) -> None: - """ - User can get default name for new vehicle directory. - - GIVEN: A project manager in any state - WHEN: User requests default vehicle name - THEN: Should return localized default vehicle name - """ - # Arrange: Mock filesystem - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - # Act: Get default vehicle name - result = manager.get_default_vehicle_name() - - # Assert: Default name returned (should be translatable) - assert result == "MyVehicleName" # This should be localized in actual use - - -class TestIntegrationScenarios: - """Test complete integration scenarios.""" - - def test_user_can_complete_new_vehicle_creation_workflow(self) -> None: - """ - User can complete full new vehicle creation workflow. - - GIVEN: A project manager with all components configured - WHEN: User completes vehicle creation from template to storage - THEN: Should create vehicle, update state, and store preferences - """ - # Arrange: Mock all components - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() - mock_flight_controller.master = MagicMock() - manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) - - with ( - patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - patch.object(LocalFilesystem, "store_recently_used_template_dirs") as mock_store_template, - patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store_vehicle, - ): - mock_create.return_value = "/new/vehicle/MyVehicle" - mock_open.return_value = "/new/vehicle/MyVehicle" - mock_settings = MagicMock(spec=NewVehicleProjectSettings) - - # Act: Complete workflow - manager orchestrates creation, opening, and history - vehicle_path = manager.create_new_vehicle_from_template( - "/templates/ArduCopter", "/vehicles", "MyVehicle", mock_settings - ) - - # Assert: all layers of the workflow were triggered in the correct order - assert vehicle_path == "/new/vehicle/MyVehicle" - assert manager._settings is mock_settings - assert manager.configuration_template == "ArduCopter" - # step 1: creator receives all necessary arguments - fc_connected = True # FC master is set in the fixture above - mock_create.assert_called_once_with( - "/templates/ArduCopter", - "/vehicles", - "MyVehicle", - mock_settings, - fc_connected, - mock_flight_controller.fc_parameters, - ) - # step 2: opener receives the path returned by the creator - mock_open.assert_called_once_with("/new/vehicle/MyVehicle") - # step 3: both history records are written - mock_store_template.assert_called_once_with("/templates/ArduCopter", "/vehicles") - mock_store_vehicle.assert_called_once_with("/new/vehicle/MyVehicle") - - def test_user_can_complete_vehicle_opening_workflow(self) -> None: - """ - User can complete full vehicle opening workflow. - - GIVEN: A project manager with existing vehicle directory - WHEN: User completes vehicle opening and preference storage - THEN: Should open vehicle and store preferences - """ - # Arrange: Mock all components - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with ( - patch.object(manager._opener, "open_vehicle_directory") as mock_open, - patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store, - ): - mock_open.return_value = "/opened/vehicle/path" - - # Act: Complete workflow - the manager method is responsible for - # updating the history, so we don't call store_recently_used_vehicle_dir - # explicitly here. - vehicle_path = manager.open_vehicle_directory("/vehicle/path") - - # Assert: Complete workflow executed - assert vehicle_path == "/opened/vehicle/path" - mock_open.assert_called_once_with("/vehicle/path") - mock_store.assert_called_once_with("/opened/vehicle/path") - - -class TestCreateNewVehicleFromBinLog: - """Test the create_new_vehicle_from_bin_log orchestration method.""" - - def _make_manager(self, with_fc: bool = False) -> "VehicleProjectManager": - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_flight_controller = MagicMock() if with_fc else None - return VehicleProjectManager(mock_filesystem, mock_flight_controller) - - def test_user_can_create_project_from_bin_log_successfully(self) -> None: - """ - User can create a new vehicle project from a valid .bin log file. - - GIVEN: A project manager and a valid .bin log file - WHEN: create_new_vehicle_from_bin_log is called - THEN: The vehicle directory is created, defaults replaced, and the path returned - """ - # Arrange - manager = self._make_manager() - - fake_defaults = ParDict.from_float_dict({"PARAM_A": 1.0}) - fake_current = ParDict.from_float_dict({"PARAM_A": 1.0, "PARAM_B": 2.0}) - empty_compound = ParDict.from_float_dict({}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl/ArduCopter/empty_4.6.x"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="my_flight"), - patch.object( - manager._creator, - "extract_bin_log_data", - return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), - ), - patch.object( - manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/my_flight" - ) as mock_create, - patch.object(manager._creator, "next_import_filename", return_value="02_imported_bin_log_parameters.param"), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory") as mock_open, - patch.object(manager._local_filesystem, "write_param_default_values_to_file") as mock_write, - patch.object(manager._local_filesystem, "compound_params", return_value=(empty_compound, "00_default.param")), - patch.object(manager._local_filesystem, "export_to_param"), - patch.object(manager._local_filesystem, "re_init"), - ): - # Act - result = manager.create_new_vehicle_from_bin_log("/logs/my_flight.bin") - - # Assert: correct path returned - assert result == "/vehicles/my_flight" - # Assert: template creation called with fc_connected=False (key difference from normal flow) - _args, kwargs = mock_create.call_args - assert kwargs.get("fc_connected") is False - # Assert: vehicle directory opened immediately after creation - mock_open.assert_called_once_with("/vehicles/my_flight") - # Assert: extracted defaults written (target path/filename come from LocalFilesystem state after re_init) - mock_write.assert_called_once_with(fake_defaults) - - def test_bin_log_defaults_are_written_to_vehicle_directory(self) -> None: - """ - The defaults extracted from the .bin log replace the template's 00_default.param. - - GIVEN: A valid .bin log file with a known defaults snapshot - WHEN: create_new_vehicle_from_bin_log is called - THEN: write_param_default_values_to_file is called with the extracted defaults ParDict - """ - # Arrange - manager = self._make_manager() - - fake_defaults = ParDict.from_float_dict({"BARO_ALT_OFFSET": 0.0}) - fake_current = ParDict.from_float_dict({"BARO_ALT_OFFSET": 0.5}) - empty_compound = ParDict.from_float_dict({}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), - patch.object( - manager._creator, - "extract_bin_log_data", - return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), - ), - patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory"), - patch.object(manager._local_filesystem, "compound_params", return_value=(empty_compound, "00_default.param")), - patch.object(manager._creator, "next_import_filename", return_value="02_imported_bin_log_parameters.param"), - patch.object(manager._local_filesystem, "export_to_param"), - patch.object(manager._local_filesystem, "re_init"), - patch.object(manager._local_filesystem, "write_param_default_values_to_file") as mock_write, - ): - manager.create_new_vehicle_from_bin_log("/logs/flight.bin") - - # Assert: the extracted defaults — not the template's — are written - mock_write.assert_called_once_with(fake_defaults) - - def test_missing_params_exported_to_import_file(self) -> None: - """ - Parameters present in the .bin log but absent from the AMC files are exported. - - GIVEN: A .bin log where current params include entries not covered by AMC files - WHEN: create_new_vehicle_from_bin_log is called - THEN: export_to_param is called for the difference, and the filesystem is re-initialised - """ - # Arrange - manager = self._make_manager() - - fake_defaults = ParDict.from_float_dict({"PARAM_A": 1.0}) - fake_current = ParDict.from_float_dict({"PARAM_A": 1.0, "EXTRA_PARAM": 99.0}) - # compound_params covers only PARAM_A — EXTRA_PARAM is missing - compound = ParDict.from_float_dict({"PARAM_A": 1.0}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), - patch.object( - manager._creator, - "extract_bin_log_data", - return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), - ), - patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), - patch.object(manager._creator, "next_import_filename", return_value="02_imported_bin_log_parameters.param"), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory"), - patch.object(manager._local_filesystem, "write_param_default_values_to_file"), - patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), - patch.object(manager._local_filesystem, "export_to_param") as mock_export, - patch.object(manager._local_filesystem, "re_init") as mock_reinit, - ): - manager.create_new_vehicle_from_bin_log("/logs/flight.bin") - - # Assert: the import file is created and the filesystem is re-initialised - mock_export.assert_called_once() - exported_params, export_filename = mock_export.call_args.args[:2] - assert export_filename == "02_imported_bin_log_parameters.param" - assert "EXTRA_PARAM" in exported_params - assert "PARAM_A" not in exported_params - assert mock_export.call_args.kwargs.get("annotate_doc") is False - # re_init is called once unconditionally (to point filesystem at new_path) and - # once more after exporting imported params (to reload the new file). - assert mock_reinit.call_count == 2 - - def test_no_import_file_when_all_params_covered_by_amc_files(self) -> None: - """ - No extra import file is created when all current params are already in AMC files. - - GIVEN: A .bin log where all current params match the AMC param files - WHEN: create_new_vehicle_from_bin_log is called - THEN: export_to_param and re_init are NOT called - """ - # Arrange - manager = self._make_manager() - - params = ParDict.from_float_dict({"PARAM_A": 1.0, "PARAM_B": 2.0}) - compound = ParDict.from_float_dict({"PARAM_A": 1.0, "PARAM_B": 2.0}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), - patch.object(manager._creator, "extract_bin_log_data", return_value=(("ArduCopter", 4, 6, 3), params, params)), - patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory"), - patch.object(manager._local_filesystem, "write_param_default_values_to_file"), - patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), - patch.object(manager._local_filesystem, "export_to_param") as mock_export, - patch.object(manager._local_filesystem, "re_init") as mock_reinit, - ): - manager.create_new_vehicle_from_bin_log("/logs/flight.bin") - - # Assert: no extra import file written; re_init called exactly once (the unconditional - # initial call to point the filesystem at the new vehicle directory). - mock_export.assert_not_called() - mock_reinit.assert_called_once_with("/vehicles/flight", "ArduCopter") - # Assert: fw_version is set to the full "major.minor.patch" string from the log, - # not just "major.minor" or whatever the template's vehicle_components.json contained. - assert manager._local_filesystem.fw_version == "4.6.3" - # Assert: the correct firmware version and type are written into vehicle_components.json. - manager._local_filesystem.set_fc_fw_version_and_type_in_components_json.assert_called_once_with( - "4.6.3", "ArduCopter", "/vehicles/flight" - ) - - def test_no_import_file_for_params_matching_defaults_but_missing_from_step_files(self) -> None: - """ - Params equal to extracted defaults are not exported just because step files omit them. - - GIVEN: Current log params include a value that equals the extracted default - and no numbered step file defines that parameter - WHEN: create_new_vehicle_from_bin_log is called - THEN: No import file is written for that parameter - """ - # Arrange - manager = self._make_manager() - - default_params = ParDict.from_float_dict({"PARAM_A": 10.0}) - current_params = ParDict.from_float_dict({"PARAM_A": 10.0}) - empty_step_compound = ParDict.from_float_dict({}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), - patch.object( - manager._creator, - "extract_bin_log_data", - return_value=(("ArduCopter", 4, 6, 3), default_params, current_params), - ), - patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory"), - patch.object(manager._local_filesystem, "write_param_default_values_to_file"), - patch.object(manager._local_filesystem, "compound_params", return_value=(empty_step_compound, "00_default.param")), - patch.object(manager._local_filesystem, "export_to_param") as mock_export, - patch.object(manager._local_filesystem, "re_init") as mock_reinit, - ): - manager.create_new_vehicle_from_bin_log("/logs/flight.bin") - - # Assert: nothing to export because current value equals default baseline - mock_export.assert_not_called() - mock_reinit.assert_called_once_with("/vehicles/flight", "ArduCopter") - - def test_fc_parameters_synced_when_flight_controller_connected(self) -> None: - """ - When a flight controller is connected, its fc_parameters are updated. - - GIVEN: A project manager with an active flight controller - WHEN: create_new_vehicle_from_bin_log completes successfully - THEN: The flight controller's fc_parameters are set to the current log params - """ - # Arrange - manager = self._make_manager(with_fc=True) - - fake_defaults = ParDict.from_float_dict({"PARAM_A": 1.0}) - fake_current = ParDict.from_float_dict({"PARAM_A": 1.0}) - compound = ParDict.from_float_dict({"PARAM_A": 1.0}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), - patch.object( - manager._creator, - "extract_bin_log_data", - return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), - ), - patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory"), - patch.object(manager._local_filesystem, "write_param_default_values_to_file"), - patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), - patch.object(manager._local_filesystem, "export_to_param"), - patch.object(manager._local_filesystem, "re_init"), - ): - manager.create_new_vehicle_from_bin_log("/logs/flight.bin") - - # Assert: FC parameters updated to the values extracted from the log - assert manager._flight_controller.fc_parameters == {"PARAM_A": 1.0} - - def test_creation_error_propagates_to_caller(self) -> None: - """ - VehicleProjectCreationError from param extraction propagates unchanged. - - GIVEN: A .bin log file that cannot have its params extracted - WHEN: create_new_vehicle_from_bin_log is called - THEN: VehicleProjectCreationError is raised with the original title/message - """ - # Arrange - manager = self._make_manager() - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="bad"), - patch.object( - manager._creator, - "extract_bin_log_data", - side_effect=VehicleProjectCreationError(".bin log import", "Corrupt log"), - ), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - pytest.raises(VehicleProjectCreationError) as exc_info, - ): - manager.create_new_vehicle_from_bin_log("/logs/bad.bin") - - assert exc_info.value.title == ".bin log import" - assert exc_info.value.message == "Corrupt log" - - def test_firmware_version_error_propagates_to_caller(self) -> None: - """ - VehicleProjectCreationError from firmware extraction propagates unchanged. - - GIVEN: A .bin log file with no firmware version information - WHEN: create_new_vehicle_from_bin_log is called - THEN: VehicleProjectCreationError is raised immediately - """ - # Arrange - manager = self._make_manager() - - with ( - patch.object( - manager._creator, - "extract_bin_log_data", - side_effect=VehicleProjectCreationError(".bin log import", "No VER or MSG found"), - ), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - pytest.raises(VehicleProjectCreationError) as exc_info, - ): - manager.create_new_vehicle_from_bin_log("/logs/no_ver.bin") - - assert exc_info.value.title == ".bin log import" - assert "No VER or MSG found" in exc_info.value.message - - def test_template_creation_always_called_with_fc_connected_false(self) -> None: - """ - create_new_vehicle_from_template is always called with fc_connected=False. - - This is the key difference from the normal template-creation flow: the vehicle - is scaffolded without a live FC connection, using log-extracted params instead. - - GIVEN: A project manager that even has a flight controller connected - WHEN: create_new_vehicle_from_bin_log is called - THEN: create_new_vehicle_from_template receives fc_connected=False - """ - # Arrange: manager WITH a connected flight controller - manager = self._make_manager(with_fc=True) - - params = ParDict.from_float_dict({"PARAM_A": 1.0}) - compound = ParDict.from_float_dict({"PARAM_A": 1.0}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), - patch.object(manager._creator, "extract_bin_log_data", return_value=(("ArduCopter", 4, 6, 3), params, params)), - patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight") as mock_create, - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory"), - patch.object(manager._local_filesystem, "write_param_default_values_to_file"), - patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), - patch.object(manager._local_filesystem, "export_to_param"), - patch.object(manager._local_filesystem, "re_init"), - ): - manager.create_new_vehicle_from_bin_log("/logs/flight.bin") - - # Assert: regardless of FC connection, fc_connected must be False - _args, kwargs = mock_create.call_args - assert kwargs.get("fc_connected") is False - - def test_manager_state_updated_after_bin_log_import(self) -> None: - """ - Manager internal state is updated correctly after a successful .bin log import. - - GIVEN: A project manager in its initial state - WHEN: create_new_vehicle_from_bin_log completes successfully - THEN: _settings carries the bin-log import options and configuration_template - is set to the template directory name - """ - # Arrange - manager = self._make_manager() - - params = ParDict.from_float_dict({"PARAM_A": 1.0}) - compound = ParDict.from_float_dict({"PARAM_A": 1.0}) - - with ( - patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl/ArduCopter/empty_4.6.x"), - patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), - patch.object(manager._creator, "extract_bin_log_data", return_value=(("ArduCopter", 4, 6, 3), params, params)), - patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), - patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), - patch.object(manager, "store_recently_used_template_dirs"), - patch.object(manager, "open_vehicle_directory"), - patch.object(manager._local_filesystem, "write_param_default_values_to_file"), - patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), - patch.object(manager._local_filesystem, "export_to_param"), - patch.object(manager._local_filesystem, "re_init"), - ): - manager.create_new_vehicle_from_bin_log("/logs/flight.bin") - - # Assert: settings reflect the bin-log import defaults - assert manager._settings is not None - assert manager._settings.blank_change_reason is True - assert manager._settings.infer_comp_specs_and_conn_from_fc_params is True - assert manager._settings.use_fc_params is True - # Assert: configuration_template is the leaf directory name of the template path - assert manager.configuration_template == "empty_4.6.x" - - -class TestGetFcDefaultTemplateDir: - """Test VehicleProjectManager.get_fc_default_template_dir.""" - - def _make_connected_manager(self, vehicle_type: str = "ArduCopter", fw_version: str = "4.6.0") -> "VehicleProjectManager": - """Return a manager whose FC is connected with the given vehicle type and firmware version.""" - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_fc = MagicMock() - mock_fc.master = MagicMock() # marks as connected - mock_fc.info.vehicle_type = vehicle_type - mock_fc.info.flight_sw_version = fw_version - return VehicleProjectManager(mock_filesystem, mock_fc) - - def test_user_gets_fc_derived_template_when_directory_exists(self) -> None: - """ - User gets a template directory derived from the FC's vehicle type and firmware version. - - GIVEN: An FC is connected reporting ArduCopter 4.6.0 - AND: The directory ArduCopter/empty_4.6.x exists in the templates base - WHEN: get_fc_default_template_dir is called - THEN: The path to that directory is returned - """ - manager = self._make_connected_manager("ArduCopter", "4.6.0") - - with ( - patch.object(LocalFilesystem, "get_templates_base_dir", return_value="/templates"), - patch("ardupilot_methodic_configurator.data_model_vehicle_project.Path.is_dir", return_value=True), - ): - result = manager.get_fc_default_template_dir() - - assert result.replace("\\", "/").endswith("ArduCopter/empty_4.6.x") - - def test_user_gets_fallback_when_fc_derived_directory_does_not_exist(self) -> None: - """ - User gets the recently-used fallback when the FC-derived template directory is missing. - - GIVEN: An FC is connected reporting Rover 4.5.7 - AND: The directory Rover/empty_4.5.x does NOT exist - WHEN: get_fc_default_template_dir is called - THEN: The recently-used template directory is returned instead - """ - manager = self._make_connected_manager("Rover", "4.5.7") - - with ( - patch.object(LocalFilesystem, "get_templates_base_dir", return_value="/templates"), - patch("ardupilot_methodic_configurator.data_model_vehicle_project.Path.is_dir", return_value=False), - patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")), - ): - result = manager.get_fc_default_template_dir() - - assert result == "/fallback/template" - - def test_user_gets_fallback_when_fc_vehicle_type_is_empty(self) -> None: - """ - User gets the recently-used fallback when the FC has no vehicle type information. - - GIVEN: An FC is connected but vehicle_type is an empty string - WHEN: get_fc_default_template_dir is called - THEN: The recently-used template directory is returned - """ - manager = self._make_connected_manager(vehicle_type="", fw_version="4.6.0") - - with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): - result = manager.get_fc_default_template_dir() - - assert result == "/fallback/template" - - def test_user_gets_fallback_when_fc_firmware_version_is_empty(self) -> None: - """ - User gets the recently-used fallback when the FC has no firmware version information. - - GIVEN: An FC is connected but flight_sw_version is an empty string - WHEN: get_fc_default_template_dir is called - THEN: The recently-used template directory is returned - """ - manager = self._make_connected_manager(vehicle_type="ArduCopter", fw_version="") - - with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): - result = manager.get_fc_default_template_dir() - - assert result == "/fallback/template" - - def test_user_gets_fallback_when_firmware_version_has_no_dot(self) -> None: - """ - User gets the recently-used fallback when the firmware version string is unparsable. - - GIVEN: An FC is connected but flight_sw_version contains no '.' separator - WHEN: get_fc_default_template_dir is called - THEN: The recently-used template directory is returned - """ - manager = self._make_connected_manager(vehicle_type="ArduCopter", fw_version="46") - - with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): - result = manager.get_fc_default_template_dir() - - assert result == "/fallback/template" - - def test_user_gets_fallback_when_firmware_version_is_non_numeric(self) -> None: - """ - User gets the recently-used fallback when firmware version parts are non-numeric. - - GIVEN: An FC is connected but flight_sw_version contains non-integer parts - WHEN: get_fc_default_template_dir is called - THEN: The recently-used template directory is returned - """ - manager = self._make_connected_manager(vehicle_type="ArduCopter", fw_version="X.Y.Z") - - with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): - result = manager.get_fc_default_template_dir() - - assert result == "/fallback/template" - - def test_user_gets_fallback_when_fc_is_disconnected(self) -> None: - """ - User gets the recently-used fallback when the FC is present but not connected. - - GIVEN: A project manager with a flight controller whose master is None - WHEN: get_fc_default_template_dir is called - THEN: The recently-used template directory is returned - """ - mock_filesystem = MagicMock(spec=LocalFilesystem) - mock_fc = MagicMock() - mock_fc.master = None # disconnected - manager = VehicleProjectManager(mock_filesystem, mock_fc) - - with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): - result = manager.get_fc_default_template_dir() - - assert result == "/fallback/template" - - def test_user_gets_fallback_when_no_fc_is_present(self) -> None: - """ - User gets the recently-used fallback when no flight controller is attached. - - GIVEN: A project manager initialised without any flight controller - WHEN: get_fc_default_template_dir is called - THEN: The recently-used template directory is returned - """ - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = VehicleProjectManager(mock_filesystem) - - with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): - result = manager.get_fc_default_template_dir() - - assert result == "/fallback/template" - - def test_fallback_uses_manager_wrapper_not_direct_localfilesystem_call(self) -> None: - """ - get_fc_default_template_dir falls back via self.get_recently_used_dirs(). - - GIVEN: A subclass of VehicleProjectManager that overrides get_recently_used_dirs - AND: No FC is connected (so the FC-derived path is not attempted) - WHEN: get_fc_default_template_dir is called - THEN: The subclass override is used for the fallback, not the base LocalFilesystem method - """ - - class _ManagerWithOverride(VehicleProjectManager): - def get_recently_used_dirs(self) -> tuple[str, str, str]: - return ("/overridden/template", "/base", "/vehicle") - - mock_filesystem = MagicMock(spec=LocalFilesystem) - manager = _ManagerWithOverride(mock_filesystem) - - # Ensure the base LocalFilesystem is NOT patched — if the code still calls - # LocalFilesystem.get_recently_used_dirs() directly the override would be bypassed. - result = manager.get_fc_default_template_dir() - - assert result == "/overridden/template" +#!/usr/bin/env python3 + +""" +Tests for data_model_vehicle_project.py module. + +This module tests the VehicleProjectManager class which provides a unified interface +for all vehicle project operations, acting as a facade that coordinates between +different data models. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem +from ardupilot_methodic_configurator.data_model_par_dict import Par, ParDict +from ardupilot_methodic_configurator.data_model_vehicle_project import VehicleProjectManager +from ardupilot_methodic_configurator.data_model_vehicle_project_creator import ( + NewVehicleProjectSettings, + VehicleProjectCreationError, +) +from ardupilot_methodic_configurator.data_model_vehicle_project_opener import VehicleProjectOpenError + +# pylint: disable=protected-access, too-many-lines + + +class TestVehicleProjectManagerInitialization: + """Test VehicleProjectManager initialization and basic properties.""" + + def test_user_can_initialize_project_manager_without_flight_controller(self) -> None: + """ + User can create project manager without flight controller. + + GIVEN: A user wants to manage vehicle projects without a flight controller + WHEN: User initializes VehicleProjectManager with only local filesystem + THEN: Manager should be created with all internal components initialized + """ + # Arrange: Create filesystem instance + mock_filesystem = MagicMock(spec=LocalFilesystem) + + # Act: Initialize project manager without flight controller + manager = VehicleProjectManager(mock_filesystem) + + # Assert: Manager is properly initialized + assert manager._local_filesystem is mock_filesystem + assert manager._flight_controller is None + assert manager._creator is not None + assert manager._opener is not None + assert manager._settings is None + assert manager.configuration_template == "" + + def test_user_can_initialize_project_manager_with_flight_controller(self) -> None: + """ + User can create project manager with flight controller. + + GIVEN: A user wants to manage vehicle projects with a connected flight controller + WHEN: User initializes VehicleProjectManager with filesystem and flight controller + THEN: Manager should be created with flight controller reference stored + """ + # Arrange: Create filesystem and flight controller instances + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + + # Act: Initialize project manager with flight controller + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + # Assert: Manager is properly initialized with flight controller + assert manager._local_filesystem is mock_filesystem + assert manager._flight_controller is mock_flight_controller + assert manager._creator is not None + assert manager._opener is not None + + def test_user_can_get_fc_parameters_when_fc_connected(self) -> None: + """ + User can retrieve FC parameters when flight controller is connected. + + GIVEN: A project manager with a connected flight controller that has parameters + WHEN: User requests FC parameters + THEN: Should return the flight controller's parameters dictionary + """ + # Arrange: Create manager with FC that has parameters + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.fc_parameters = {"PARAM1": 1.0, "PARAM2": 2.0} + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + # Act: Get FC parameters + fc_params = manager.fc_parameters() + + # Assert: FC parameters are returned + assert fc_params == {"PARAM1": 1.0, "PARAM2": 2.0} + + def test_user_gets_none_when_fc_not_connected(self) -> None: + """ + User receives None when no flight controller is connected. + + GIVEN: A project manager without a flight controller + WHEN: User requests FC parameters + THEN: Should return None + """ + # Arrange: Create manager without FC + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Get FC parameters + fc_params = manager.fc_parameters() + + # Assert: None is returned + assert fc_params is None + + def test_user_gets_fc_parameters_even_if_empty(self) -> None: + """ + User receives empty dict when FC is connected but has no parameters yet. + + GIVEN: A project manager with FC that hasn't loaded parameters yet + WHEN: User requests FC parameters + THEN: Should return empty dictionary + """ + # Arrange: Create manager with FC that has empty parameters + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.fc_parameters = {} + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + # Act: Get FC parameters + fc_params = manager.fc_parameters() + + # Assert: Empty dict is returned + assert fc_params == {} + + +class TestDirectoryAndPathOperations: + """Test directory and path related operations.""" + + def test_user_can_get_recently_used_directories(self) -> None: + """ + User can retrieve recently used directories. + + GIVEN: A project manager with stored directory preferences + WHEN: User requests recently used directories + THEN: Should return tuple of template, base, and vehicle directories + """ + # Arrange: Mock filesystem and recently used directories + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "get_recently_used_dirs") as mock_get_dirs: + mock_get_dirs.return_value = ("/templates", "/base", "/vehicle") + + # Act: Get recently used directories + template_dir, new_base_dir, vehicle_dir = manager.get_recently_used_dirs() + + # Assert: Correct directories returned + assert template_dir == "/templates" + assert new_base_dir == "/base" + assert vehicle_dir == "/vehicle" + mock_get_dirs.assert_called_once() + + def test_creation_stores_template_and_base_dirs_in_history(self) -> None: + """ + Manager stores the template and base directories in history after creation. + + GIVEN: A user creates a new vehicle from a template + WHEN: Creation succeeds + THEN: The template directory and base directory are stored in recently-used history + """ + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with ( + patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + patch.object(LocalFilesystem, "store_recently_used_template_dirs") as mock_store_template, + patch.object(LocalFilesystem, "store_recently_used_vehicle_dir"), + ): + mock_create.return_value = "/new/vehicle/path" + mock_open.return_value = "/new/vehicle/path" + settings = MagicMock(spec=NewVehicleProjectSettings) + + # Act + manager.create_new_vehicle_from_template("/templates/ArduCopter", "/vehicles", "Name", settings) + + # Assert: only the template/base history is the concern of this test + mock_store_template.assert_called_once_with("/templates/ArduCopter", "/vehicles") + + def test_creation_opens_new_directory_and_updates_vehicle_history(self) -> None: + """ + Manager opens the newly created directory and records it in vehicle history. + + GIVEN: A user creates a new vehicle from a template + WHEN: Creation succeeds + THEN: open_vehicle_directory is called with the new path + AND: The new path is stored exactly once in recent-vehicle history + """ + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with ( + patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + patch.object(LocalFilesystem, "store_recently_used_template_dirs"), + patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store_vehicle, + ): + mock_create.return_value = "/new/vehicle/path" + mock_open.return_value = "/new/vehicle/path" + settings = MagicMock(spec=NewVehicleProjectSettings) + + # Act + manager.create_new_vehicle_from_template("/templates/ArduCopter", "/vehicles", "Name", settings) + + # Assert: the new directory is opened and the vehicle dir stored exactly once + mock_open.assert_called_once_with("/new/vehicle/path") + mock_store_vehicle.assert_called_once_with("/new/vehicle/path") + + def test_user_can_get_current_working_directory(self) -> None: + """ + User can get current working directory. + + GIVEN: A project manager in any state + WHEN: User requests current working directory + THEN: Should return the current working directory path + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "getcwd") as mock_getcwd: + mock_getcwd.return_value = "/current/working/dir" + + # Act: Get current working directory + result = manager.get_current_working_directory() + + # Assert: Correct directory returned + assert result == "/current/working/dir" + mock_getcwd.assert_called_once() + + def test_user_can_extract_directory_name_from_path(self) -> None: + """ + User can extract directory name from full path. + + GIVEN: A project manager and a full path + WHEN: User requests directory name extraction + THEN: Should return just the directory name + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "get_directory_name_from_full_path") as mock_get_name: + mock_get_name.return_value = "vehicle_name" + + # Act: Extract directory name + result = manager.get_directory_name_from_path("/path/to/vehicle_name") + + # Assert: Correct name returned + assert result == "vehicle_name" + mock_get_name.assert_called_once_with("/path/to/vehicle_name") + + def test_user_can_check_if_directory_exists(self) -> None: + """ + User can check if a directory exists. + + GIVEN: A project manager and a directory path + WHEN: User checks if directory exists + THEN: Should return boolean indicating existence + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "directory_exists") as mock_exists: + mock_exists.return_value = True + + # Act: Check directory existence + result = manager.directory_exists("/test/path") + + # Assert: Correct existence status returned + assert result is True + mock_exists.assert_called_once_with("/test/path") + + def test_user_can_validate_directory_name(self) -> None: + """ + User can validate directory name. + + GIVEN: A project manager and a directory name + WHEN: User validates directory name + THEN: Should return boolean indicating validity + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "valid_directory_name") as mock_valid: + mock_valid.return_value = True + + # Act: Validate directory name + result = manager.valid_directory_name("valid_name") + + # Assert: Correct validation result returned + assert result is True + mock_valid.assert_called_once_with("valid_name") + + +class TestVehicleProjectCreation: + """Test vehicle project creation operations.""" + + def test_user_is_told_when_creating_from_flight_controller_without_connection(self) -> None: + """ + User receives a clear error when no flight controller is connected. + + GIVEN: A project manager without a flight controller + WHEN: The user requests a project from a flight controller + THEN: VehicleProjectCreationError explains that no controller is connected + """ + manager = VehicleProjectManager(MagicMock(spec=LocalFilesystem)) + + with pytest.raises(VehicleProjectCreationError, match="no flight controller is connected"): + manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + def test_user_is_told_when_connected_flight_controller_has_no_parameters(self) -> None: + """ + User receives a clear error when the connected flight controller has no parameters. + + GIVEN: A connected flight controller with an empty parameter set + WHEN: The user requests a project from the flight controller + THEN: VehicleProjectCreationError explains that parameters are unavailable + """ + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {} + manager = VehicleProjectManager(MagicMock(spec=LocalFilesystem), mock_flight_controller) + + with pytest.raises(VehicleProjectCreationError, match="no flight controller parameters are available"): + manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + def test_user_can_create_new_vehicle_from_connected_flight_controller(self) -> None: + """A configured FC project preserves its exact identity and parameters not in the template.""" + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"IN_TEMPLATE": 1.0, "FC_ONLY": 2.0} + mock_flight_controller.info.vehicle_type = "ArduCopter" + mock_flight_controller.info.flight_sw_version = "4.6.0" + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + mock_filesystem.param_default_dict = ParDict.from_float_dict({"DEFAULT_ONLY": 0.0}) + mock_filesystem.compound_params.return_value = (ParDict.from_float_dict({"IN_TEMPLATE": 1.0}), None) + + with ( + patch.object( + manager._creator, + "template_dir_for_bin_import", + return_value="/templates/ArduCopter/empty_4.6.x", + ) as mock_template_lookup, + patch.object( + manager._creator, + "create_new_vehicle_from_template", + return_value="/base/ConfiguredVehicle", + ) as mock_create, + patch.object( + manager._creator, + "next_import_filename", + return_value="67_imported_flight_controller_parameters.param", + ) as mock_import_filename, + patch.object(manager, "open_vehicle_directory"), + patch.object(manager, "store_recently_used_template_dirs"), + ): + result = manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + assert result == "/base/ConfiguredVehicle" + mock_template_lookup.assert_called_once_with("ArduCopter", 4, 6) + template_dir, new_base_dir, vehicle_name, settings = mock_create.call_args.args[:4] + assert template_dir == "/templates/ArduCopter/empty_4.6.x" + assert new_base_dir == "/base" + assert vehicle_name == "ConfiguredVehicle" + assert settings.infer_comp_specs_and_conn_from_fc_params is True + assert settings.use_fc_params is True + assert mock_create.call_args.kwargs == { + "fc_connected": True, + "fc_parameters": {"IN_TEMPLATE": 1.0, "FC_ONLY": 2.0}, + } + mock_filesystem.re_init.assert_any_call("/base/ConfiguredVehicle", "ArduCopter") + mock_filesystem.set_fc_fw_version_and_type_in_components_json.assert_called_once_with( + "4.6.0", "ArduCopter", "/base/ConfiguredVehicle" + ) + mock_import_filename.assert_called_once_with("/base/ConfiguredVehicle", source="flight_controller") + imported_params = mock_filesystem.export_to_param.call_args.args[0] + assert {name: param.value for name, param in imported_params.items()} == {"FC_ONLY": 2.0} + assert mock_filesystem.export_to_param.call_args.kwargs == {"annotate_doc": False} + + def test_configured_fc_creation_does_not_create_redundant_import_file(self) -> None: + """ + A configured-FC project does not create an import file for already represented values. + + GIVEN: The live FC parameters are all present in the template baseline + WHEN: A project is created from the connected FC + THEN: No additional parameter import file is written + """ + # Arrange: configure a connected FC and a baseline containing its only parameter + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"PARAM1": 1.0} + mock_flight_controller.info.vehicle_type = "ArduCopter" + mock_flight_controller.info.flight_sw_version = "4.6.0" + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + mock_filesystem.param_default_dict = ParDict.from_float_dict({"PARAM1": 1.0}) + mock_filesystem.compound_params.return_value = (ParDict.from_float_dict({}), None) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/templates/empty_4.6.x"), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/base/ConfiguredVehicle"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager._local_filesystem, "export_to_param") as mock_export, + ): + # Act: create the project from the already configured FC + manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + # Assert: the default baseline is sufficient; no redundant import file is needed + mock_export.assert_not_called() + + def test_configured_fc_creation_translates_persistence_errors(self) -> None: + """Persistence failures are reported as translated creation errors.""" + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"PARAM1": 1.0} + mock_flight_controller.info.vehicle_type = "ArduCopter" + mock_flight_controller.info.flight_sw_version = "4.6.0" + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/templates/empty_4.6.x"), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/base/ConfiguredVehicle"), + patch.object( + manager, + "_complete_imported_vehicle_project_creation", + side_effect=PermissionError("read-only"), + ), + pytest.raises(VehicleProjectCreationError, match="Could not finish creating") as exc_info, + ): + manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + assert "read-only" in exc_info.value.message + + def test_configured_fc_defaults_survive_destination_reinitialization(self) -> None: + """FC defaults remain the import baseline when destination re_init reloads template defaults.""" + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"FC_DEFAULT": 0.0} + mock_flight_controller.info.vehicle_type = "ArduCopter" + mock_flight_controller.info.flight_sw_version = "4.6.0" + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + fc_defaults = ParDict.from_float_dict({"FC_DEFAULT": 0.0}) + mock_filesystem.param_default_dict = fc_defaults + mock_filesystem.compound_params.return_value = (ParDict.from_float_dict({}), None) + + def reload_template_defaults(_new_path: str, _vehicle_type: str) -> None: + mock_filesystem.param_default_dict = ParDict.from_float_dict({"FC_DEFAULT": 1.0}) + + mock_filesystem.re_init.side_effect = reload_template_defaults + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/templates/empty_4.6.x"), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/base/ConfiguredVehicle"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file") as mock_write, + patch.object(manager._local_filesystem, "export_to_param") as mock_export, + ): + manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + mock_write.assert_called_once_with(fc_defaults) + mock_export.assert_not_called() + + def test_real_filesystem_fc_defaults_are_preserved_across_project_creation(self, tmp_path) -> None: + """ + A real destination re_init preserves FC defaults and exports changed values. + + This reproduces the original ordering bug: the template default for + ``SERIAL5_BAUD`` is 57, while the connected FC reports 57 as a user + setting against its own default of 115. Reloading the destination + template must not lose the FC baseline, otherwise that setting would + be omitted from every generated parameter file. + """ + template_dir = ( + Path(__file__).parents[1] / "ardupilot_methodic_configurator" / "vehicle_templates" / "ArduCopter" / "empty_4.6.x" + ) + local_filesystem = LocalFilesystem( + str(template_dir), + "ArduCopter", + "4.6.0", + allow_editing_template_files=False, + save_component_to_system_templates=False, + ) + fc_defaults = local_filesystem.param_default_dict.deep_copy() + fc_defaults["SERIAL5_BAUD"] = Par(115.0) + local_filesystem.set_param_default_values_if_different(fc_defaults) + + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"SERIAL5_BAUD": 57.0} + mock_flight_controller.info.vehicle_type = "ArduCopter" + mock_flight_controller.info.flight_sw_version = "4.6.0" + manager = VehicleProjectManager(local_filesystem, mock_flight_controller) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value=str(template_dir)), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager, "store_recently_used_template_dirs"), + ): + project_dir = Path(manager.create_new_vehicle_from_flight_controller(str(tmp_path), "ConfiguredVehicle")) + + assert ParDict.from_file(str(project_dir / "00_default.param"))["SERIAL5_BAUD"].value == 115.0 + imported_files = list(project_dir.glob("*_imported_flight_controller_parameters.param")) + assert len(imported_files) == 1 + assert ParDict.from_file(str(imported_files[0]))["SERIAL5_BAUD"].value == 57.0 + + def test_template_project_with_fc_params_uses_fc_defaults(self, tmp_path) -> None: + """Creating from a template writes FC defaults when FC values are selected.""" + template_dir = ( + Path(__file__).parents[1] / "ardupilot_methodic_configurator" / "vehicle_templates" / "ArduCopter" / "empty_4.6.x" + ) + local_filesystem = LocalFilesystem( + str(template_dir), + "ArduCopter", + "4.6.0", + allow_editing_template_files=False, + save_component_to_system_templates=False, + ) + fc_defaults = local_filesystem.param_default_dict.deep_copy() + fc_defaults["SERIAL5_BAUD"] = Par(115.0) + local_filesystem.set_param_default_values_if_different(fc_defaults) + + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"SERIAL5_BAUD": 57.0} + manager = VehicleProjectManager(local_filesystem, mock_flight_controller) + + with ( + patch.object(manager, "open_vehicle_directory"), + patch.object(manager, "store_recently_used_template_dirs"), + ): + project_dir = Path( + manager.create_new_vehicle_from_template( + str(template_dir), str(tmp_path), "TemplateVehicle", NewVehicleProjectSettings(use_fc_params=True) + ) + ) + + assert ParDict.from_file(str(project_dir / "00_default.param"))["SERIAL5_BAUD"].value == 115.0 + + def test_creation_from_flight_controller_rejects_missing_matching_template(self, tmp_path) -> None: + """A configured FC must not fall back to an unrelated recently-used template.""" + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"PARAM1": 1.0} + mock_flight_controller.info.vehicle_type = "ArduCopter" + mock_flight_controller.info.flight_sw_version = "4.6.0" + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + with ( + patch.object(LocalFilesystem, "get_templates_base_dir", return_value=str(tmp_path)), + patch.object( + LocalFilesystem, + "get_recently_used_dirs", + return_value=("/fallback/empty_4.5.x", "/base", "/vehicle"), + ), + pytest.raises(VehicleProjectCreationError) as exc_info, + ): + manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + assert exc_info.value.title == "Vehicle template directory" + assert "empty_4.6.x" in exc_info.value.message + + def test_creation_from_flight_controller_rejects_invalid_firmware_version(self) -> None: + """A configured FC without a parseable firmware version fails clearly.""" + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + mock_flight_controller.fc_parameters = {"PARAM1": 1.0} + mock_flight_controller.info.vehicle_type = "ArduCopter" + mock_flight_controller.info.flight_sw_version = "unknown" + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + with pytest.raises(VehicleProjectCreationError, match="Could not determine"): + manager.create_new_vehicle_from_flight_controller("/base", "ConfiguredVehicle") + + def test_user_can_create_new_vehicle_from_template_successfully(self) -> None: + """ + User can create new vehicle from template successfully. + + GIVEN: A project manager with valid template and settings + WHEN: User creates new vehicle from template + THEN: Should create vehicle directory and update manager state + """ + # Arrange: Mock filesystem and components + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() # FC is connected + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + # Mock the creator and opener + with ( + patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + patch.object(LocalFilesystem, "store_recently_used_template_dirs"), + patch.object(LocalFilesystem, "store_recently_used_vehicle_dir"), + ): + mock_create.return_value = "/new/vehicle/path" + mock_open.return_value = "/new/vehicle/path" + + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + + # Act: Create new vehicle from template + result = manager.create_new_vehicle_from_template("/template/path", "/base/path", "NewVehicle", mock_settings) + + # Assert: correct path returned and manager state updated + assert result == "/new/vehicle/path" + assert manager._settings is mock_settings + assert manager.configuration_template == "path" # last component of template path + + # Assert: creator called with fc_connected=True (FC master is set) and opener called afterwards + fc_connected = True # FC master is set in the fixture above + mock_create.assert_called_once_with( + "/template/path", "/base/path", "NewVehicle", mock_settings, fc_connected, mock_flight_controller.fc_parameters + ) + mock_open.assert_called_once_with("/new/vehicle/path") + + def test_user_sees_error_when_vehicle_creation_fails(self) -> None: + """ + User sees error when vehicle creation fails. + + GIVEN: A project manager with invalid settings + WHEN: User attempts to create vehicle from template + THEN: Should raise VehicleProjectCreationError + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Mock the creator to raise an exception + with patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create: + mock_create.side_effect = VehicleProjectCreationError("Creation Error", "Creation failed") + + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + + # Act & Assert: Creation should raise error + with pytest.raises(VehicleProjectCreationError, match="Creation failed"): + manager.create_new_vehicle_from_template("/template/path", "/base/path", "NewVehicle", mock_settings) + + def test_opener_not_called_when_creation_fails(self) -> None: + """ + Opener must not be invoked when the creator raises an error. + + GIVEN: A project manager whose creator raises a VehicleProjectCreationError + WHEN: User attempts to create a vehicle from a template + THEN: The opener is never called + AND: The VehicleProjectCreationError propagates to the caller + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with ( + patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + ): + mock_create.side_effect = VehicleProjectCreationError("Creation Error", "Creation failed") + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + + # Act & Assert: error propagates and opener is never touched + with pytest.raises(VehicleProjectCreationError): + manager.create_new_vehicle_from_template("/template/path", "/base/path", "NewVehicle", mock_settings) + + mock_open.assert_not_called() + + +class TestVehicleProjectOpening: + """Test vehicle project opening operations.""" + + def test_user_can_open_vehicle_directory_successfully(self) -> None: + """ + User can open existing vehicle directory successfully. + + GIVEN: A project manager with valid vehicle directory + WHEN: User opens vehicle directory + THEN: Should open directory, update history and return path + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Mock the opener and history store + with ( + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, + ): + mock_open.return_value = "/opened/vehicle/path" + + # Act: Open vehicle directory + result = manager.open_vehicle_directory("/vehicle/path") + + # Assert: Directory opened successfully and history recorded + assert result == "/opened/vehicle/path" + mock_open.assert_called_once_with("/vehicle/path") + mock_store.assert_called_once_with("/opened/vehicle/path") + + def test_user_sees_error_when_vehicle_directory_opening_fails(self) -> None: + """ + User sees error when vehicle directory opening fails. + + GIVEN: A project manager with invalid vehicle directory + WHEN: User attempts to open vehicle directory + THEN: Should raise VehicleProjectOpenError and not update history + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Mock the opener to raise an exception + with ( + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, + ): + mock_open.side_effect = VehicleProjectOpenError("Open Error", "Opening failed") + + # Act & Assert: Opening should raise error + with pytest.raises(VehicleProjectOpenError, match="Opening failed"): + manager.open_vehicle_directory("/invalid/path") + + mock_store.assert_not_called() + + def test_user_can_open_last_vehicle_directory_successfully(self) -> None: + """ + User can open last used vehicle directory successfully. + + GIVEN: A project manager with last used vehicle directory + WHEN: User opens last vehicle directory + THEN: Should open directory, update history and return path + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Mock the opener and history storage + with ( + patch.object(manager._opener, "open_last_vehicle_directory") as mock_open, + patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, + ): + mock_open.return_value = "/last/vehicle/path" + + # Act: Open last vehicle directory + result = manager.open_last_vehicle_directory("/last/path") + + # Assert: Directory opened successfully and history recorded + assert result == "/last/vehicle/path" + mock_open.assert_called_once_with("/last/path") + mock_store.assert_called_once_with("/last/vehicle/path") + + def test_user_sees_error_when_opening_last_vehicle_directory_fails(self) -> None: + """ + User sees error when opening last vehicle directory fails. + + GIVEN: A project manager with invalid last vehicle directory + WHEN: User attempts to open last vehicle directory + THEN: Should raise VehicleProjectOpenError and not update history + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Mock the opener to raise an exception + with ( + patch.object(manager._opener, "open_last_vehicle_directory") as mock_open, + patch.object(manager, "store_recently_used_vehicle_dir") as mock_store, + ): + mock_open.side_effect = VehicleProjectOpenError("Last Open Error", "Last directory opening failed") + + # Act & Assert: Opening should raise error + with pytest.raises(VehicleProjectOpenError, match="Last directory opening failed"): + manager.open_last_vehicle_directory("/invalid/last/path") + + mock_store.assert_not_called() + + +class TestFilesystemStateManagement: + """Test filesystem state management operations.""" + + def test_user_can_get_current_vehicle_directory(self) -> None: + """ + User can get current vehicle directory from filesystem. + + GIVEN: A project manager with filesystem containing vehicle directory + WHEN: User requests current vehicle directory + THEN: Should return filesystem's vehicle directory + """ + # Arrange: Mock filesystem with vehicle directory + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_filesystem.vehicle_dir = "/current/vehicle" + manager = VehicleProjectManager(mock_filesystem) + + # Act: Get vehicle directory + result = manager.get_vehicle_directory() + + # Assert: Correct vehicle directory returned + assert result == "/current/vehicle" + + def test_user_can_store_recently_used_template_directories(self) -> None: + """ + User can store recently used template and base directories. + + GIVEN: A project manager and template/base directories + WHEN: User stores recently used template directories + THEN: Should delegate to LocalFilesystem for storage + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "store_recently_used_template_dirs") as mock_store: + # Act: Store template directories + manager.store_recently_used_template_dirs("/template", "/base") + + # Assert: Storage delegated correctly + mock_store.assert_called_once_with("/template", "/base") + + def test_user_can_store_recently_used_vehicle_directory(self) -> None: + """ + User can store recently used vehicle directory. + + GIVEN: A project manager and vehicle directory + WHEN: User stores recently used vehicle directory + THEN: Should delegate to LocalFilesystem for storage + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store: + # Act: Store vehicle directory + manager.store_recently_used_vehicle_dir("/vehicle") + + # Assert: Storage delegated correctly + mock_store.assert_called_once_with("/vehicle") + + +class TestProjectSettingsProperties: + """Test project settings property access.""" + + def test_user_can_access_reset_fc_parameters_property_when_settings_exist(self) -> None: + """ + User can access reset FC parameters property when settings exist. + + GIVEN: A project manager with settings configured + WHEN: User accesses reset_fc_parameters_to_their_defaults property + THEN: Should return the setting value from project settings + """ + # Arrange: Mock filesystem and settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + mock_settings.reset_fc_parameters_to_their_defaults = True + manager._settings = mock_settings + + # Act: Access property + result = manager.reset_fc_parameters_to_their_defaults + + # Assert: Correct value returned + assert result is True + + def test_user_gets_false_for_reset_fc_parameters_when_no_settings(self) -> None: + """ + User gets False for reset FC parameters when no settings exist. + + GIVEN: A project manager without settings configured + WHEN: User accesses reset_fc_parameters_to_their_defaults property + THEN: Should return False + """ + # Arrange: Mock filesystem without settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Access property + result = manager.reset_fc_parameters_to_their_defaults + + # Assert: False returned for missing settings + assert result is False + + def test_user_can_access_blank_component_data_property_when_settings_exist(self) -> None: + """ + User can access blank component data property when settings exist. + + GIVEN: A project manager with settings configured + WHEN: User accesses blank_component_data property + THEN: Should return the setting value from project settings + """ + # Arrange: Mock filesystem and settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + mock_settings.blank_component_data = True + manager._settings = mock_settings + + # Act: Access property + result = manager.blank_component_data + + # Assert: Correct value returned + assert result is True + + def test_user_gets_false_for_blank_component_data_when_no_settings(self) -> None: + """ + User gets False for blank component data when no settings exist. + + GIVEN: A project manager without settings configured + WHEN: User accesses blank_component_data property + THEN: Should return False + """ + # Arrange: Mock filesystem without settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Access property + result = manager.blank_component_data + + # Assert: False returned for missing settings + assert result is False + + def test_user_can_access_infer_comp_specs_property_when_settings_exist(self) -> None: + """ + User can access infer component specs property when settings exist. + + GIVEN: A project manager with settings configured + WHEN: User accesses infer_comp_specs_and_conn_from_fc_params property + THEN: Should return the setting value from project settings + """ + # Arrange: Mock filesystem and settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + mock_settings.infer_comp_specs_and_conn_from_fc_params = True + manager._settings = mock_settings + + # Act: Access property + result = manager.infer_comp_specs_and_conn_from_fc_params + + # Assert: Correct value returned + assert result is True + + def test_user_gets_false_for_infer_comp_specs_when_no_settings(self) -> None: + """ + User gets False for infer component specs when no settings exist. + + GIVEN: A project manager without settings configured + WHEN: User accesses infer_comp_specs_and_conn_from_fc_params property + THEN: Should return False + """ + # Arrange: Mock filesystem without settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Access property + result = manager.infer_comp_specs_and_conn_from_fc_params + + # Assert: False returned for missing settings + assert result is False + + def test_user_can_access_use_fc_params_property_when_settings_exist(self) -> None: + """ + User can access use FC params property when settings exist. + + GIVEN: A project manager with settings configured + WHEN: User accesses use_fc_params property + THEN: Should return the setting value from project settings + """ + # Arrange: Mock filesystem and settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + mock_settings.use_fc_params = True + manager._settings = mock_settings + + # Act: Access property + result = manager.use_fc_params + + # Assert: Correct value returned + assert result is True + + def test_user_gets_false_for_use_fc_params_when_no_settings(self) -> None: + """ + User gets False for use FC params when no settings exist. + + GIVEN: A project manager without settings configured + WHEN: User accesses use_fc_params property + THEN: Should return False + """ + # Arrange: Mock filesystem without settings + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Access property + result = manager.use_fc_params + + # Assert: False returned for missing settings + assert result is False + + +class TestFlightControllerOperations: + """Test flight controller related operations.""" + + def test_user_can_check_flight_controller_connection_when_connected(self) -> None: + """ + User can check flight controller connection when connected. + + GIVEN: A project manager with connected flight controller + WHEN: User checks flight controller connection + THEN: Should return True + """ + # Arrange: Mock filesystem and connected flight controller + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() # Connected + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + # Act: Check connection + result = manager.is_flight_controller_connected() + + # Assert: Connection detected + assert result is True + + def test_user_can_check_flight_controller_connection_when_disconnected(self) -> None: + """ + User can check flight controller connection when disconnected. + + GIVEN: A project manager with disconnected flight controller + WHEN: User checks flight controller connection + THEN: Should return False + """ + # Arrange: Mock filesystem and disconnected flight controller + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = None # Disconnected + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + # Act: Check connection + result = manager.is_flight_controller_connected() + + # Assert: No connection detected + assert result is False + + def test_user_can_check_flight_controller_connection_when_no_controller(self) -> None: + """ + User can check flight controller connection when no controller exists. + + GIVEN: A project manager without flight controller + WHEN: User checks flight controller connection + THEN: Should return False + """ + # Arrange: Mock filesystem without flight controller + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Check connection + result = manager.is_flight_controller_connected() + + # Assert: No connection detected + assert result is False + + def test_user_can_check_if_last_vehicle_directory_can_be_opened_when_exists(self) -> None: + """ + User can check if last vehicle directory can be opened when it exists. + + GIVEN: A project manager with existing last vehicle directory + WHEN: User checks if last vehicle directory can be opened + THEN: Should return True + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(manager, "directory_exists") as mock_exists: + mock_exists.return_value = True + + # Act: Check if can open last directory + result = manager.can_open_last_vehicle_directory("/existing/path") + + # Assert: Can open existing directory + assert result is True + mock_exists.assert_called_once_with("/existing/path") + + def test_user_can_check_if_last_vehicle_directory_can_be_opened_when_not_exists(self) -> None: + """ + User can check if last vehicle directory can be opened when it doesn't exist. + + GIVEN: A project manager with non-existing last vehicle directory + WHEN: User checks if last vehicle directory can be opened + THEN: Should return False + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(manager, "directory_exists") as mock_exists: + mock_exists.return_value = False + + # Act: Check if can open last directory + result = manager.can_open_last_vehicle_directory("/nonexistent/path") + + # Assert: Cannot open non-existent directory + assert result is False + mock_exists.assert_called_once_with("/nonexistent/path") + + def test_user_can_check_if_last_vehicle_directory_can_be_opened_when_empty_path(self) -> None: + """ + User can check if last vehicle directory can be opened when path is empty. + + GIVEN: A project manager with empty last vehicle directory path + WHEN: User checks if last vehicle directory can be opened + THEN: Should return False + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Check if can open empty path + result = manager.can_open_last_vehicle_directory("") + + # Assert: Cannot open empty path + assert result is False + + +class TestIntroductionMessageAndFileOperations: + """Test introduction message generation and file operations.""" + + def test_user_gets_working_directory_message_when_in_current_directory(self) -> None: + """ + User gets working directory message when in current directory. + + GIVEN: A project manager where vehicle directory equals working directory + WHEN: User requests introduction message + THEN: Should return current working directory message + """ + # Arrange: Mock filesystem with equal directories + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_filesystem.vehicle_dir = "/working/dir" + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(manager, "get_current_working_directory") as mock_getcwd: + mock_getcwd.return_value = "/working/dir" + + # Act: Get introduction message + result = manager.get_introduction_message() + + # Assert: Current working directory message returned + assert "current working directory" in result + + def test_user_gets_vehicle_dir_message_when_in_different_directory(self) -> None: + """ + User gets vehicle dir message when in different directory. + + GIVEN: A project manager where vehicle directory differs from working directory + WHEN: User requests introduction message + THEN: Should return vehicle directory specified message + """ + # Arrange: Mock filesystem with different directories + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_filesystem.vehicle_dir = "/vehicle/dir" + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(manager, "get_current_working_directory") as mock_getcwd: + mock_getcwd.return_value = "/working/dir" + + # Act: Get introduction message + result = manager.get_introduction_message() + + # Assert: Vehicle directory specified message returned + assert "--vehicle-dir specified directory" in result + + def test_user_can_get_file_parameters_list(self) -> None: + """ + User can get list of intermediate parameter files. + + GIVEN: A project manager with filesystem containing parameter files + WHEN: User requests file parameters list + THEN: Should return list of parameter file names + """ + # Arrange: Mock filesystem with parameter files + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_filesystem.file_parameters = { + "01_first.param": {}, + "02_second.param": {}, + "03_third.param": {}, + } + manager = VehicleProjectManager(mock_filesystem) + + # Act: Get file parameters list + result = manager.get_file_parameters_list() + + # Assert: Correct list of parameter files returned + assert len(result) == 3 + assert "01_first.param" in result + assert "02_second.param" in result + assert "03_third.param" in result + + def test_user_can_get_default_vehicle_name(self) -> None: + """ + User can get default name for new vehicle directory. + + GIVEN: A project manager in any state + WHEN: User requests default vehicle name + THEN: Should return localized default vehicle name + """ + # Arrange: Mock filesystem + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + # Act: Get default vehicle name + result = manager.get_default_vehicle_name() + + # Assert: Default name returned (should be translatable) + assert result == "MyVehicleName" # This should be localized in actual use + + +class TestIntegrationScenarios: + """Test complete integration scenarios.""" + + def test_user_can_complete_new_vehicle_creation_workflow(self) -> None: + """ + User can complete full new vehicle creation workflow. + + GIVEN: A project manager with all components configured + WHEN: User completes vehicle creation from template to storage + THEN: Should create vehicle, update state, and store preferences + """ + # Arrange: Mock all components + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() + mock_flight_controller.master = MagicMock() + manager = VehicleProjectManager(mock_filesystem, mock_flight_controller) + + with ( + patch.object(manager._creator, "create_new_vehicle_from_template") as mock_create, + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + patch.object(LocalFilesystem, "store_recently_used_template_dirs") as mock_store_template, + patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store_vehicle, + ): + mock_create.return_value = "/new/vehicle/MyVehicle" + mock_open.return_value = "/new/vehicle/MyVehicle" + mock_settings = MagicMock(spec=NewVehicleProjectSettings) + + # Act: Complete workflow - manager orchestrates creation, opening, and history + vehicle_path = manager.create_new_vehicle_from_template( + "/templates/ArduCopter", "/vehicles", "MyVehicle", mock_settings + ) + + # Assert: all layers of the workflow were triggered in the correct order + assert vehicle_path == "/new/vehicle/MyVehicle" + assert manager._settings is mock_settings + assert manager.configuration_template == "ArduCopter" + # step 1: creator receives all necessary arguments + fc_connected = True # FC master is set in the fixture above + mock_create.assert_called_once_with( + "/templates/ArduCopter", + "/vehicles", + "MyVehicle", + mock_settings, + fc_connected, + mock_flight_controller.fc_parameters, + ) + # step 2: opener receives the path returned by the creator + mock_open.assert_called_once_with("/new/vehicle/MyVehicle") + # step 3: both history records are written + mock_store_template.assert_called_once_with("/templates/ArduCopter", "/vehicles") + mock_store_vehicle.assert_called_once_with("/new/vehicle/MyVehicle") + + def test_user_can_complete_vehicle_opening_workflow(self) -> None: + """ + User can complete full vehicle opening workflow. + + GIVEN: A project manager with existing vehicle directory + WHEN: User completes vehicle opening and preference storage + THEN: Should open vehicle and store preferences + """ + # Arrange: Mock all components + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with ( + patch.object(manager._opener, "open_vehicle_directory") as mock_open, + patch.object(LocalFilesystem, "store_recently_used_vehicle_dir") as mock_store, + ): + mock_open.return_value = "/opened/vehicle/path" + + # Act: Complete workflow - the manager method is responsible for + # updating the history, so we don't call store_recently_used_vehicle_dir + # explicitly here. + vehicle_path = manager.open_vehicle_directory("/vehicle/path") + + # Assert: Complete workflow executed + assert vehicle_path == "/opened/vehicle/path" + mock_open.assert_called_once_with("/vehicle/path") + mock_store.assert_called_once_with("/opened/vehicle/path") + + +class TestCreateNewVehicleFromBinLog: + """Test the create_new_vehicle_from_bin_log orchestration method.""" + + def _make_manager(self, with_fc: bool = False) -> "VehicleProjectManager": + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_flight_controller = MagicMock() if with_fc else None + return VehicleProjectManager(mock_filesystem, mock_flight_controller) + + def test_user_can_create_project_from_bin_log_successfully(self) -> None: + """ + User can create a new vehicle project from a valid .bin log file. + + GIVEN: A project manager and a valid .bin log file + WHEN: create_new_vehicle_from_bin_log is called + THEN: The vehicle directory is created, defaults replaced, and the path returned + """ + # Arrange + manager = self._make_manager() + + fake_defaults = ParDict.from_float_dict({"PARAM_A": 1.0}) + fake_current = ParDict.from_float_dict({"PARAM_A": 1.0, "PARAM_B": 2.0}) + empty_compound = ParDict.from_float_dict({}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl/ArduCopter/empty_4.6.x"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="my_flight"), + patch.object( + manager._creator, + "extract_bin_log_data", + return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), + ), + patch.object( + manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/my_flight" + ) as mock_create, + patch.object(manager._creator, "next_import_filename", return_value="02_imported_bin_log_parameters.param"), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory") as mock_open, + patch.object(manager._local_filesystem, "write_param_default_values_to_file") as mock_write, + patch.object(manager._local_filesystem, "compound_params", return_value=(empty_compound, "00_default.param")), + patch.object(manager._local_filesystem, "export_to_param"), + patch.object(manager._local_filesystem, "re_init"), + ): + # Act + result = manager.create_new_vehicle_from_bin_log("/logs/my_flight.bin") + + # Assert: correct path returned + assert result == "/vehicles/my_flight" + # Assert: template creation called with fc_connected=False (key difference from normal flow) + _args, kwargs = mock_create.call_args + assert kwargs.get("fc_connected") is False + # Assert: vehicle directory opened immediately after creation + mock_open.assert_called_once_with("/vehicles/my_flight") + # Assert: extracted defaults written (target path/filename come from LocalFilesystem state after re_init) + mock_write.assert_called_once_with(fake_defaults) + + def test_bin_log_defaults_are_written_to_vehicle_directory(self) -> None: + """ + The defaults extracted from the .bin log replace the template's 00_default.param. + + GIVEN: A valid .bin log file with a known defaults snapshot + WHEN: create_new_vehicle_from_bin_log is called + THEN: write_param_default_values_to_file is called with the extracted defaults ParDict + """ + # Arrange + manager = self._make_manager() + + fake_defaults = ParDict.from_float_dict({"BARO_ALT_OFFSET": 0.0}) + fake_current = ParDict.from_float_dict({"BARO_ALT_OFFSET": 0.5}) + empty_compound = ParDict.from_float_dict({}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), + patch.object( + manager._creator, + "extract_bin_log_data", + return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), + ), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager._local_filesystem, "compound_params", return_value=(empty_compound, "00_default.param")), + patch.object(manager._creator, "next_import_filename", return_value="02_imported_bin_log_parameters.param"), + patch.object(manager._local_filesystem, "export_to_param"), + patch.object(manager._local_filesystem, "re_init"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file") as mock_write, + ): + manager.create_new_vehicle_from_bin_log("/logs/flight.bin") + + # Assert: the extracted defaults — not the template's — are written + mock_write.assert_called_once_with(fake_defaults) + + def test_missing_params_exported_to_import_file(self) -> None: + """ + Parameters present in the .bin log but absent from the AMC files are exported. + + GIVEN: A .bin log where current params include entries not covered by AMC files + WHEN: create_new_vehicle_from_bin_log is called + THEN: export_to_param is called for the difference, and the filesystem is re-initialised + """ + # Arrange + manager = self._make_manager() + + fake_defaults = ParDict.from_float_dict({"PARAM_A": 1.0}) + fake_current = ParDict.from_float_dict({"PARAM_A": 1.0, "EXTRA_PARAM": 99.0}) + # compound_params covers only PARAM_A — EXTRA_PARAM is missing + compound = ParDict.from_float_dict({"PARAM_A": 1.0}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), + patch.object( + manager._creator, + "extract_bin_log_data", + return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), + ), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), + patch.object(manager._creator, "next_import_filename", return_value="02_imported_bin_log_parameters.param"), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file"), + patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), + patch.object(manager._local_filesystem, "export_to_param") as mock_export, + patch.object(manager._local_filesystem, "re_init") as mock_reinit, + ): + manager.create_new_vehicle_from_bin_log("/logs/flight.bin") + + # Assert: the import file is created and the filesystem is re-initialised + mock_export.assert_called_once() + exported_params, export_filename = mock_export.call_args.args[:2] + assert export_filename == "02_imported_bin_log_parameters.param" + assert "EXTRA_PARAM" in exported_params + assert "PARAM_A" not in exported_params + assert mock_export.call_args.kwargs.get("annotate_doc") is False + # re_init is called once unconditionally (to point filesystem at new_path) and + # once more after exporting imported params (to reload the new file). + assert mock_reinit.call_count == 2 + + def test_no_import_file_when_all_params_covered_by_amc_files(self) -> None: + """ + No extra import file is created when all current params are already in AMC files. + + GIVEN: A .bin log where all current params match the AMC param files + WHEN: create_new_vehicle_from_bin_log is called + THEN: export_to_param and re_init are NOT called + """ + # Arrange + manager = self._make_manager() + + params = ParDict.from_float_dict({"PARAM_A": 1.0, "PARAM_B": 2.0}) + compound = ParDict.from_float_dict({"PARAM_A": 1.0, "PARAM_B": 2.0}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), + patch.object(manager._creator, "extract_bin_log_data", return_value=(("ArduCopter", 4, 6, 3), params, params)), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file"), + patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), + patch.object(manager._local_filesystem, "export_to_param") as mock_export, + patch.object(manager._local_filesystem, "re_init") as mock_reinit, + ): + manager.create_new_vehicle_from_bin_log("/logs/flight.bin") + + # Assert: no extra import file written; re_init called exactly once (the unconditional + # initial call to point the filesystem at the new vehicle directory). + mock_export.assert_not_called() + mock_reinit.assert_called_once_with("/vehicles/flight", "ArduCopter") + # Assert: fw_version is set to the full "major.minor.patch" string from the log, + # not just "major.minor" or whatever the template's vehicle_components.json contained. + assert manager._local_filesystem.fw_version == "4.6.3" + # Assert: the correct firmware version and type are written into vehicle_components.json. + manager._local_filesystem.set_fc_fw_version_and_type_in_components_json.assert_called_once_with( + "4.6.3", "ArduCopter", "/vehicles/flight" + ) + + def test_no_import_file_for_params_matching_defaults_but_missing_from_step_files(self) -> None: + """ + Params equal to extracted defaults are not exported just because step files omit them. + + GIVEN: Current log params include a value that equals the extracted default + and no numbered step file defines that parameter + WHEN: create_new_vehicle_from_bin_log is called + THEN: No import file is written for that parameter + """ + # Arrange + manager = self._make_manager() + + default_params = ParDict.from_float_dict({"PARAM_A": 10.0}) + current_params = ParDict.from_float_dict({"PARAM_A": 10.0}) + empty_step_compound = ParDict.from_float_dict({}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), + patch.object( + manager._creator, + "extract_bin_log_data", + return_value=(("ArduCopter", 4, 6, 3), default_params, current_params), + ), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file"), + patch.object(manager._local_filesystem, "compound_params", return_value=(empty_step_compound, "00_default.param")), + patch.object(manager._local_filesystem, "export_to_param") as mock_export, + patch.object(manager._local_filesystem, "re_init") as mock_reinit, + ): + manager.create_new_vehicle_from_bin_log("/logs/flight.bin") + + # Assert: nothing to export because current value equals default baseline + mock_export.assert_not_called() + mock_reinit.assert_called_once_with("/vehicles/flight", "ArduCopter") + + def test_fc_parameters_synced_when_flight_controller_connected(self) -> None: + """ + When a flight controller is connected, its fc_parameters are updated. + + GIVEN: A project manager with an active flight controller + WHEN: create_new_vehicle_from_bin_log completes successfully + THEN: The flight controller's fc_parameters are set to the current log params + """ + # Arrange + manager = self._make_manager(with_fc=True) + + fake_defaults = ParDict.from_float_dict({"PARAM_A": 1.0}) + fake_current = ParDict.from_float_dict({"PARAM_A": 1.0}) + compound = ParDict.from_float_dict({"PARAM_A": 1.0}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), + patch.object( + manager._creator, + "extract_bin_log_data", + return_value=(("ArduCopter", 4, 6, 3), fake_defaults, fake_current), + ), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file"), + patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), + patch.object(manager._local_filesystem, "export_to_param"), + patch.object(manager._local_filesystem, "re_init"), + ): + manager.create_new_vehicle_from_bin_log("/logs/flight.bin") + + # Assert: FC parameters updated to the values extracted from the log + assert manager._flight_controller.fc_parameters == {"PARAM_A": 1.0} + + def test_creation_error_propagates_to_caller(self) -> None: + """ + VehicleProjectCreationError from param extraction propagates unchanged. + + GIVEN: A .bin log file that cannot have its params extracted + WHEN: create_new_vehicle_from_bin_log is called + THEN: VehicleProjectCreationError is raised with the original title/message + """ + # Arrange + manager = self._make_manager() + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="bad"), + patch.object( + manager._creator, + "extract_bin_log_data", + side_effect=VehicleProjectCreationError(".bin log import", "Corrupt log"), + ), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + pytest.raises(VehicleProjectCreationError) as exc_info, + ): + manager.create_new_vehicle_from_bin_log("/logs/bad.bin") + + assert exc_info.value.title == ".bin log import" + assert exc_info.value.message == "Corrupt log" + + def test_firmware_version_error_propagates_to_caller(self) -> None: + """ + VehicleProjectCreationError from firmware extraction propagates unchanged. + + GIVEN: A .bin log file with no firmware version information + WHEN: create_new_vehicle_from_bin_log is called + THEN: VehicleProjectCreationError is raised immediately + """ + # Arrange + manager = self._make_manager() + + with ( + patch.object( + manager._creator, + "extract_bin_log_data", + side_effect=VehicleProjectCreationError(".bin log import", "No VER or MSG found"), + ), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + pytest.raises(VehicleProjectCreationError) as exc_info, + ): + manager.create_new_vehicle_from_bin_log("/logs/no_ver.bin") + + assert exc_info.value.title == ".bin log import" + assert "No VER or MSG found" in exc_info.value.message + + def test_template_creation_always_called_with_fc_connected_false(self) -> None: + """ + create_new_vehicle_from_template is always called with fc_connected=False. + + This is the key difference from the normal template-creation flow: the vehicle + is scaffolded without a live FC connection, using log-extracted params instead. + + GIVEN: A project manager that even has a flight controller connected + WHEN: create_new_vehicle_from_bin_log is called + THEN: create_new_vehicle_from_template receives fc_connected=False + """ + # Arrange: manager WITH a connected flight controller + manager = self._make_manager(with_fc=True) + + params = ParDict.from_float_dict({"PARAM_A": 1.0}) + compound = ParDict.from_float_dict({"PARAM_A": 1.0}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), + patch.object(manager._creator, "extract_bin_log_data", return_value=(("ArduCopter", 4, 6, 3), params, params)), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight") as mock_create, + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file"), + patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), + patch.object(manager._local_filesystem, "export_to_param"), + patch.object(manager._local_filesystem, "re_init"), + ): + manager.create_new_vehicle_from_bin_log("/logs/flight.bin") + + # Assert: regardless of FC connection, fc_connected must be False + _args, kwargs = mock_create.call_args + assert kwargs.get("fc_connected") is False + + def test_manager_state_updated_after_bin_log_import(self) -> None: + """ + Manager internal state is updated correctly after a successful .bin log import. + + GIVEN: A project manager in its initial state + WHEN: create_new_vehicle_from_bin_log completes successfully + THEN: _settings carries the bin-log import options and configuration_template + is set to the template directory name + """ + # Arrange + manager = self._make_manager() + + params = ParDict.from_float_dict({"PARAM_A": 1.0}) + compound = ParDict.from_float_dict({"PARAM_A": 1.0}) + + with ( + patch.object(manager._creator, "template_dir_for_bin_import", return_value="/tpl/ArduCopter/empty_4.6.x"), + patch.object(manager._creator, "vehicle_name_from_bin_log", return_value="flight"), + patch.object(manager._creator, "extract_bin_log_data", return_value=(("ArduCopter", 4, 6, 3), params, params)), + patch.object(manager._creator, "create_new_vehicle_from_template", return_value="/vehicles/flight"), + patch.object(LocalFilesystem, "get_vehicles_default_dir", return_value="/vehicles"), + patch.object(manager, "store_recently_used_template_dirs"), + patch.object(manager, "open_vehicle_directory"), + patch.object(manager._local_filesystem, "write_param_default_values_to_file"), + patch.object(manager._local_filesystem, "compound_params", return_value=(compound, "00_default.param")), + patch.object(manager._local_filesystem, "export_to_param"), + patch.object(manager._local_filesystem, "re_init"), + ): + manager.create_new_vehicle_from_bin_log("/logs/flight.bin") + + # Assert: settings reflect the bin-log import defaults + assert manager._settings is not None + assert manager._settings.blank_change_reason is True + assert manager._settings.infer_comp_specs_and_conn_from_fc_params is True + assert manager._settings.use_fc_params is True + # Assert: configuration_template is the leaf directory name of the template path + assert manager.configuration_template == "empty_4.6.x" + + +class TestGetFcDefaultTemplateDir: + """Test VehicleProjectManager.get_fc_default_template_dir.""" + + def _make_connected_manager(self, vehicle_type: str = "ArduCopter", fw_version: str = "4.6.0") -> "VehicleProjectManager": + """Return a manager whose FC is connected with the given vehicle type and firmware version.""" + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_fc = MagicMock() + mock_fc.master = MagicMock() # marks as connected + mock_fc.info.vehicle_type = vehicle_type + mock_fc.info.flight_sw_version = fw_version + return VehicleProjectManager(mock_filesystem, mock_fc) + + def test_user_gets_fc_derived_template_when_directory_exists(self) -> None: + """ + User gets a template directory derived from the FC's vehicle type and firmware version. + + GIVEN: An FC is connected reporting ArduCopter 4.6.0 + AND: The directory ArduCopter/empty_4.6.x exists in the templates base + WHEN: get_fc_default_template_dir is called + THEN: The path to that directory is returned + """ + manager = self._make_connected_manager("ArduCopter", "4.6.0") + + with ( + patch.object(LocalFilesystem, "get_templates_base_dir", return_value="/templates"), + patch("ardupilot_methodic_configurator.data_model_vehicle_project.Path.is_dir", return_value=True), + ): + result = manager.get_fc_default_template_dir() + + assert result.replace("\\", "/").endswith("ArduCopter/empty_4.6.x") + + def test_user_gets_fallback_when_fc_derived_directory_does_not_exist(self) -> None: + """ + User gets the recently-used fallback when the FC-derived template directory is missing. + + GIVEN: An FC is connected reporting Rover 4.5.7 + AND: The directory Rover/empty_4.5.x does NOT exist + WHEN: get_fc_default_template_dir is called + THEN: The recently-used template directory is returned instead + """ + manager = self._make_connected_manager("Rover", "4.5.7") + + with ( + patch.object(LocalFilesystem, "get_templates_base_dir", return_value="/templates"), + patch("ardupilot_methodic_configurator.data_model_vehicle_project.Path.is_dir", return_value=False), + patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")), + ): + result = manager.get_fc_default_template_dir() + + assert result == "/fallback/template" + + def test_user_gets_fallback_when_fc_vehicle_type_is_empty(self) -> None: + """ + User gets the recently-used fallback when the FC has no vehicle type information. + + GIVEN: An FC is connected but vehicle_type is an empty string + WHEN: get_fc_default_template_dir is called + THEN: The recently-used template directory is returned + """ + manager = self._make_connected_manager(vehicle_type="", fw_version="4.6.0") + + with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): + result = manager.get_fc_default_template_dir() + + assert result == "/fallback/template" + + def test_user_gets_fallback_when_fc_firmware_version_is_empty(self) -> None: + """ + User gets the recently-used fallback when the FC has no firmware version information. + + GIVEN: An FC is connected but flight_sw_version is an empty string + WHEN: get_fc_default_template_dir is called + THEN: The recently-used template directory is returned + """ + manager = self._make_connected_manager(vehicle_type="ArduCopter", fw_version="") + + with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): + result = manager.get_fc_default_template_dir() + + assert result == "/fallback/template" + + def test_user_gets_fallback_when_firmware_version_has_no_dot(self) -> None: + """ + User gets the recently-used fallback when the firmware version string is unparsable. + + GIVEN: An FC is connected but flight_sw_version contains no '.' separator + WHEN: get_fc_default_template_dir is called + THEN: The recently-used template directory is returned + """ + manager = self._make_connected_manager(vehicle_type="ArduCopter", fw_version="46") + + with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): + result = manager.get_fc_default_template_dir() + + assert result == "/fallback/template" + + def test_user_gets_fallback_when_firmware_version_is_non_numeric(self) -> None: + """ + User gets the recently-used fallback when firmware version parts are non-numeric. + + GIVEN: An FC is connected but flight_sw_version contains non-integer parts + WHEN: get_fc_default_template_dir is called + THEN: The recently-used template directory is returned + """ + manager = self._make_connected_manager(vehicle_type="ArduCopter", fw_version="X.Y.Z") + + with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): + result = manager.get_fc_default_template_dir() + + assert result == "/fallback/template" + + def test_user_gets_fallback_when_fc_is_disconnected(self) -> None: + """ + User gets the recently-used fallback when the FC is present but not connected. + + GIVEN: A project manager with a flight controller whose master is None + WHEN: get_fc_default_template_dir is called + THEN: The recently-used template directory is returned + """ + mock_filesystem = MagicMock(spec=LocalFilesystem) + mock_fc = MagicMock() + mock_fc.master = None # disconnected + manager = VehicleProjectManager(mock_filesystem, mock_fc) + + with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): + result = manager.get_fc_default_template_dir() + + assert result == "/fallback/template" + + def test_user_gets_fallback_when_no_fc_is_present(self) -> None: + """ + User gets the recently-used fallback when no flight controller is attached. + + GIVEN: A project manager initialised without any flight controller + WHEN: get_fc_default_template_dir is called + THEN: The recently-used template directory is returned + """ + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = VehicleProjectManager(mock_filesystem) + + with patch.object(LocalFilesystem, "get_recently_used_dirs", return_value=("/fallback/template", "/base", "/vehicle")): + result = manager.get_fc_default_template_dir() + + assert result == "/fallback/template" + + def test_fallback_uses_manager_wrapper_not_direct_localfilesystem_call(self) -> None: + """ + get_fc_default_template_dir falls back via self.get_recently_used_dirs(). + + GIVEN: A subclass of VehicleProjectManager that overrides get_recently_used_dirs + AND: No FC is connected (so the FC-derived path is not attempted) + WHEN: get_fc_default_template_dir is called + THEN: The subclass override is used for the fallback, not the base LocalFilesystem method + """ + + class _ManagerWithOverride(VehicleProjectManager): + def get_recently_used_dirs(self) -> tuple[str, str, str]: + return ("/overridden/template", "/base", "/vehicle") + + mock_filesystem = MagicMock(spec=LocalFilesystem) + manager = _ManagerWithOverride(mock_filesystem) + + # Ensure the base LocalFilesystem is NOT patched — if the code still calls + # LocalFilesystem.get_recently_used_dirs() directly the override would be bypassed. + result = manager.get_fc_default_template_dir() + + assert result == "/overridden/template" diff --git a/tests/test_data_model_vehicle_project_creator.py b/tests/test_data_model_vehicle_project_creator.py index 8220c4e15..e5dd71f91 100755 --- a/tests/test_data_model_vehicle_project_creator.py +++ b/tests/test_data_model_vehicle_project_creator.py @@ -1238,6 +1238,23 @@ def test_next_import_filename_ignores_non_param_files(self, tmp_path) -> None: # Assert: txt file must not have inflated the counter assert result == "04_imported_bin_log_parameters.param" + def test_next_import_filename_labels_the_parameter_source(self, tmp_path) -> None: + """ + next_import_filename labels an import according to its source. + + GIVEN: A newly created vehicle directory + WHEN: A flight-controller import filename is requested + THEN: The filename identifies the flight-controller source + """ + # Arrange: use an empty vehicle directory + vehicle_dir = str(tmp_path) + + # Act: request a filename for live flight-controller parameters + result = VehicleProjectCreator.next_import_filename(vehicle_dir, source="flight_controller") + + # Assert: the source is visible in the generated filename + assert result == "01_imported_flight_controller_parameters.param" + def test_next_import_filename_converts_os_error_to_creation_error(self, tmp_path, monkeypatch) -> None: """ next_import_filename converts filesystem errors into VehicleProjectCreationError. diff --git a/tests/test_frontend_tkinter_project_creator.py b/tests/test_frontend_tkinter_project_creator.py index 3bb46b0cb..839754c80 100755 --- a/tests/test_frontend_tkinter_project_creator.py +++ b/tests/test_frontend_tkinter_project_creator.py @@ -19,6 +19,7 @@ from ardupilot_methodic_configurator.data_model_vehicle_project_creator import ( VehicleProjectCreationError, ) +from ardupilot_methodic_configurator.data_model_vehicle_project_opener import VehicleProjectOpenError from ardupilot_methodic_configurator.frontend_tkinter_base_window import BaseWindow from ardupilot_methodic_configurator.frontend_tkinter_project_creator import VehicleProjectCreatorWindow @@ -164,6 +165,69 @@ def test_user_can_create_new_vehicle_from_template_successfully(self, configured window.project_manager.create_new_vehicle_from_template.assert_called_once() window.root.destroy.assert_called_once() + def test_user_can_create_new_vehicle_from_flight_controller_successfully(self, configured_creator_window) -> None: + """The minimal creator forwards the base directory and name to the project manager.""" + window = configured_creator_window + window.new_base_dir = MagicMock() + window.new_base_dir.get_selected_directory.return_value = "/path/to/base" + window.new_dir = MagicMock() + window.new_dir.get_selected_directory.return_value = "ConfiguredVehicle" + + window.create_new_vehicle_from_flight_controller() + + window.project_manager.create_new_vehicle_from_flight_controller.assert_called_once_with( + "/path/to/base", "ConfiguredVehicle" + ) + window.root.destroy.assert_called_once() + + def test_user_sees_open_error_after_flight_controller_project_creation( + self, configured_creator_window, mock_messagebox + ) -> None: + """An error opening the newly created project is shown by the creator window.""" + window = configured_creator_window + window.new_base_dir = MagicMock() + window.new_base_dir.get_selected_directory.return_value = "/path/to/base" + window.new_dir = MagicMock() + window.new_dir.get_selected_directory.return_value = "ConfiguredVehicle" + error = VehicleProjectOpenError("Open failed", "The new project could not be opened.") + window.project_manager.create_new_vehicle_from_flight_controller.side_effect = error + + window.create_new_vehicle_from_flight_controller() + + mock_messagebox.showerror.assert_called_once_with(error.title, error.message) + window.root.destroy.assert_not_called() + + def test_flight_controller_mode_omits_template_and_settings_widgets(self, configured_creator_window) -> None: + """The FC workflow only builds controls for the destination and vehicle name.""" + window = configured_creator_window + window.main_frame = MagicMock() + window._create_template_selection_widgets = MagicMock() + window._create_settings_widgets = MagicMock() + window.calculate_scaled_geometry = MagicMock(return_value="800x200") + + with ( + patch.object(BaseWindow, "center_window_on_screen"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_creator.ttk.Label"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_creator.ttk.LabelFrame"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_creator.ttk.Button"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_creator.show_tooltip"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_creator.DirectorySelectionWidgets"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_creator.PathEntryWidget"), + ): + window.create_option1_widgets( + "/templates/ArduCopter/empty_4.6.x", + "/path/to/projects", + "ConfiguredVehicle", + fc_connected=True, + fc_parameters={"PARAM1": 1.0}, + connected_fc_vehicle_type="ArduCopter", + from_flight_controller=True, + ) + + window._create_template_selection_widgets.assert_not_called() + window._create_settings_widgets.assert_not_called() + window.calculate_scaled_geometry.assert_called_once_with(800, 200) + def test_user_sees_error_when_project_creation_fails(self, configured_creator_window, mock_messagebox) -> None: """ User receives clear error feedback when project creation fails. diff --git a/tests/test_frontend_tkinter_project_opener.py b/tests/test_frontend_tkinter_project_opener.py index 7f6700aea..19ab1082f 100755 --- a/tests/test_frontend_tkinter_project_opener.py +++ b/tests/test_frontend_tkinter_project_opener.py @@ -107,6 +107,19 @@ def mock_sys_exit() -> Generator[MagicMock, None, None]: yield mock +@pytest.fixture +def mocked_option1_widget_construction() -> Generator[MagicMock, None, None]: + """Fixture that captures option-one buttons without requiring real Tk widgets.""" + with ( + patch("ardupilot_methodic_configurator.frontend_tkinter_project_opener.ttk.Label"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_opener.ttk.LabelFrame"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_opener.ttk.Button") as mock_button, + patch("ardupilot_methodic_configurator.frontend_tkinter_project_opener.BinLogSelectionWidgets"), + patch("ardupilot_methodic_configurator.frontend_tkinter_project_opener.show_tooltip"), + ): + yield mock_button + + # ==================== TEST CLASSES ==================== @@ -127,7 +140,7 @@ def test_user_can_initialize_window_with_three_options(self, configured_opener_w # Assert: Window properties are set correctly window.root.title.assert_called_once() - window.root.geometry.assert_called_once_with("600x450") + window.root.geometry.assert_called_once_with("600x470") window.root.protocol.assert_called_once_with("WM_DELETE_WINDOW", window.close_and_quit) # Assert: Project manager methods were called for initialization @@ -195,6 +208,71 @@ def test_user_can_create_new_vehicle_with_flight_controller_connected( # Assert: New project window is created with project manager mock_create_new_project_window.assert_called_once_with(window.project_manager) + def test_user_can_create_new_vehicle_from_flight_controller( + self, configured_opener_window, mock_create_new_project_window + ) -> None: + """The FC project option opens the minimal creator window.""" + window = configured_opener_window + + window.create_new_vehicle_from_flight_controller() + + window.root.destroy.assert_called_once() + mock_create_new_project_window.assert_called_once_with(window.project_manager, from_flight_controller=True) + + def test_user_sees_vehicle_creation_options_in_requested_order( + self, configured_opener_window, mocked_option1_widget_construction + ) -> None: + """ + User sees template, configured-flight-controller, and bin-log options in order. + + GIVEN: The vehicle opener window is displayed + WHEN: The vehicle creation options are rendered + THEN: The configured-flight-controller option appears between template and bin-log controls + """ + window = configured_opener_window + window.project_manager.is_flight_controller_connected.return_value = True + window.project_manager.fc_parameters.return_value = {"PARAM1": 1.0} + + window.create_option1_widgets() + + button_texts = [call.kwargs["text"] for call in mocked_option1_widget_construction.call_args_list] + assert button_texts == [ + "Create a vehicle project from a template", + "Create a vehicle project from an already configured flight controller", + ] + + @pytest.mark.parametrize( + ("fc_connected", "fc_parameters", "expected_state"), + [ + (False, {"PARAM1": 1.0}, tk.DISABLED), + (True, {}, tk.DISABLED), + (True, {"PARAM1": 1.0}, tk.NORMAL), + ], + ) + def test_user_sees_flight_controller_option_only_when_fc_is_configured( # pylint: disable=too-many-arguments,too-many-positional-arguments + self, + configured_opener_window, + mocked_option1_widget_construction, + fc_connected: bool, + fc_parameters: dict[str, float], + expected_state: str, + ) -> None: + """ + User can select the configured-flight-controller option only when FC data is ready. + + GIVEN: The opener reports a flight-controller connection and parameter state + WHEN: The vehicle creation options are rendered + THEN: The configured-flight-controller button has the expected enabled state + """ + window = configured_opener_window + window.project_manager.is_flight_controller_connected.return_value = fc_connected + window.project_manager.fc_parameters.return_value = fc_parameters + + window.create_option1_widgets() + + fc_button_call = mocked_option1_widget_construction.call_args_list[1] + assert fc_button_call.kwargs["state"] == expected_state + def test_user_can_open_last_vehicle_directory_successfully(self, configured_opener_window) -> None: """ User can successfully open the last used vehicle configuration directory. diff --git a/tests/test_regenerate_app_screenshots_fully_automated.py b/tests/test_regenerate_app_screenshots_fully_automated.py new file mode 100755 index 000000000..dcc2969b8 --- /dev/null +++ b/tests/test_regenerate_app_screenshots_fully_automated.py @@ -0,0 +1,131 @@ +#!/usr/bin/env python3 + +""" +Tests for the fully automated application screenshot generator. + +This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator + +SPDX-FileCopyrightText: 2026 Amilcar do Carmo Lucas + +SPDX-License-Identifier: GPL-3.0-or-later +""" + +import argparse +import importlib.util +import sys +from pathlib import Path +from types import ModuleType, SimpleNamespace +from unittest.mock import ANY, MagicMock, patch + +from ardupilot_methodic_configurator.plugins.plugin_constants import PLUGIN_MOTOR_TEST + +# pylint: disable=protected-access + + +def _load_screenshot_generator() -> ModuleType: + """Load the standalone screenshot script as a test module.""" + script_path = Path(__file__).parents[1] / "scripts" / "regenerate_app_screenshots_fully_automated.py" + spec = importlib.util.spec_from_file_location("screenshot_generator", script_path) + if spec is None or spec.loader is None: + message = f"Could not load screenshot generator from {script_path}" + raise RuntimeError(message) + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +screenshot_generator = _load_screenshot_generator() + + +def test_cleanup_plugin_view_ignores_non_callable_optional_hook() -> None: + """Cleanup must not call an attribute that only happens to use the hook name.""" + destroy = MagicMock() + plugin_view = SimpleNamespace(on_deactivate="not callable", destroy=destroy) + + screenshot_generator._cleanup_plugin_view(plugin_view) + + destroy.assert_called_once_with() + + +def test_screenshot_generator_registers_application_plugins_before_capture(tmp_path) -> None: + """Screenshot generation initializes plugins just like normal application startup.""" + args = argparse.Namespace( + images_dir=tmp_path, + vehicle_dir=tmp_path, + delay=0.0, + padding=0, + overwrite=False, + log_level="WARNING", + ) + + with ( + patch.object(screenshot_generator, "parse_args", return_value=args), + patch.object(screenshot_generator, "configure_logging"), + patch.object(screenshot_generator, "register_plugins") as mock_register_plugins, + patch.object(screenshot_generator, "capture_target"), + ): + assert screenshot_generator.main() == 0 + + mock_register_plugins.assert_called_once_with() + + +def test_simple_parameter_editor_capture_suppresses_external_documentation(tmp_path) -> None: + """Simple-mode capture does not launch a browser while settling the editor.""" + screenshot_generator.register_plugins() + + with ( + patch("ardupilot_methodic_configurator.data_model_parameter_editor.webbrowser_open_url") as open_browser, + patch.object(screenshot_generator, "capture_widget"), + ): + screenshot_generator._capture_parameter_editor( + tmp_path / "parameter-editor.png", + delay=0.0, + padding=0, + vehicle_dir=screenshot_generator.DEFAULT_VEHICLE_DIR, + current_file="05_board_orientation.param", + gui_complexity="simple", + scale=0.666, + ) + + open_browser.assert_not_called() + + +def test_motor_test_capture_uses_registered_plugin_with_fake_connection(tmp_path) -> None: + """Motor-test screenshots use the registered plugin and a connected test FC.""" + fake_flight_controller = MagicMock() + fake_window = MagicMock() + fake_model = MagicMock() + fake_plugin_view = MagicMock() + fake_factory = MagicMock() + fake_factory.create_model.return_value = fake_model + fake_factory.create.return_value = fake_plugin_view + + with ( + patch.object(screenshot_generator, "FlightController", return_value=fake_flight_controller), + patch.object(screenshot_generator, "LocalFilesystem"), + patch.object(screenshot_generator, "BaseWindow", return_value=fake_window), + patch.object(screenshot_generator, "plugin_factory", fake_factory), + patch.object(screenshot_generator, "capture_widget"), + ): + screenshot_generator._capture_motor_test( + tmp_path / "motor-test.png", + delay=0.0, + padding=0, + vehicle_dir=screenshot_generator.DEFAULT_VEHICLE_DIR, + ) + + fake_flight_controller.set_master_for_testing.assert_called_once_with(ANY) + assert fake_flight_controller.fc_parameters["FRAME_CLASS"] == 1.0 + assert fake_flight_controller.fc_parameters["FRAME_TYPE"] == 1.0 + assert fake_flight_controller.request_scaled_imu_messages.return_value == (True, "") + assert fake_flight_controller.poll_scaled_imu.return_value is None + assert fake_flight_controller.request_periodic_battery_status.return_value == (True, "") + assert fake_flight_controller.get_battery_status.return_value == (None, "") + fake_factory.create_model.assert_called_once() + model_context = fake_factory.create_model.call_args.args[1] + assert fake_factory.create_model.call_args.args[0] == PLUGIN_MOTOR_TEST + assert model_context.flight_controller is fake_flight_controller + fake_factory.create.assert_called_once_with(PLUGIN_MOTOR_TEST, fake_window.main_frame, fake_model, fake_window) + fake_plugin_view.pack.assert_called_once_with(fill="both", expand=True) + fake_flight_controller.disconnect.assert_called_once_with()