Misc fixes - #2022
Conversation
6b41562 to
85ae549
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
This PR broadens parameter file handling to support multiple Unicode encodings, improves ArduPlane-specific frame-class behavior, and synchronizes UI state after external parameter uploads. It also introduces/upgrades ArduPilot device-ID decoding assets and updates templates/config to reflect ArduPlane quad-plane parameter semantics.
Changes:
- Add UTF-16/UTF-32 (with/without BOM) support when reading
.paramfiles and extend tests accordingly. - Adjust ArduPlane frame-class logic (allow “Undefined”; prefer
Q_FRAME_*) and update import/config/validation + tests. - Refresh FC-value snapshots and parent tables after successful external uploads; document the workflow and add tests.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_data_model_vehicle_components_validation_constants.py | Updates expectations around “Undefined” frame class for ArduPlane. |
| tests/test_param_pid_adjustment_update.py | Adds coverage for UTF-16/UTF-32 parameter file loading in PID adjustment tool. |
| tests/test_frontend_tkinter_parameter_compare_and_upload.py | Adds regression tests for post-upload FC snapshot refresh and table repopulation. |
| tests/test_data_model_vehicle_components_validation.py | Adds validation coverage for ArduPlane accepting “Undefined” frame class. |
| tests/test_data_model_par_dict.py | Adds broad encoding/BOM coverage for parameter file parsing. |
| tests/test_backend_flightcontroller_business_logic.py | Adds ArduPlane-specific preference for Q_FRAME_* in frame info extraction. |
| tests/test_annotate_params.py | Adds annotation coverage for UTF-16/UTF-32 parameter inputs. |
| tests/acceptance_use_fc_params_from_file.py | Improves acceptance test by using real filesystem + monkeypatch.chdir. |
| pyproject.toml | Excludes generated decode_devid.py from linting. |
| ardupilot_methodic_configurator/vehicle_templates/ArduPlane/normal_plane/00_default.param | Updates default ArduPlane template parameters (large refresh). |
| ardupilot_methodic_configurator/param_pid_adjustment_update.py | Switches to shared Unicode-aware line reader and updates error messaging. |
| ardupilot_methodic_configurator/log_analysis/decode_devid_lib.py | Updates device-id decoding logic/types/CLI to match upstream. |
| ardupilot_methodic_configurator/log_analysis/data_model_vehicle_overview_sensor_rules.py | Switches to generated decode_devid module and handles decode failure. |
| ardupilot_methodic_configurator/frontend_tkinter_parameter_compare_and_upload.py | Synchronizes FC snapshots post-upload and refreshes parent table on close. |
| ardupilot_methodic_configurator/data_model_vehicle_components_validation.py | Allows “Undefined” frame class only for ArduPlane. |
| ardupilot_methodic_configurator/data_model_vehicle_components_import.py | Passes firmware type into frame-info extraction to support ArduPlane behavior. |
| ardupilot_methodic_configurator/data_model_par_dict.py | Adds encoding detection + Unicode-aware line reader used across loaders. |
| ardupilot_methodic_configurator/configuration_steps_ArduPlane.json | Adds derived Q_FRAME_CLASS handling and autoimport patterns. |
| ardupilot_methodic_configurator/backend_flightcontroller_commands.py | Supplies vehicle type into frame-info extraction. |
| ardupilot_methodic_configurator/backend_flightcontroller_business_logic.py | Makes frame-info extraction vehicle-type aware (ArduPlane prefers Q_FRAME_*). |
| ardupilot_methodic_configurator/annotate_params.py | Reads input via Unicode-aware line reader. |
| ardupilot_methodic_configurator/main.py | Sets vehicle type/version earlier so widgets/imports use correct frame-class semantics. |
| ARCHITECTURE_parameter_upload.md | Documents post-upload synchronization and table refresh behavior. |
| .github/workflows/update_flightcontroller_ids.yml | Starts syncing upstream decode_devid.py into the repo and stages it. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ACRO_PITCH_RATE,180 | ||
| ACRO_ROLL_RATE,180 | ||
| ACRO_YAW_RATE,0 | ||
| ADSB_TYPE,0 |
| with open("temp.param", "wb") as f: | ||
| f.write("# HEADER\r\nPARAM1,1.0 # Comment\r\n".encode("utf-16")) | ||
|
|
||
| params, content = load_param_file_with_content("temp.param") | ||
|
|
||
| assert params == {"PARAM1": Par(1.0, "Comment")} | ||
| assert content == ["# HEADER", "PARAM1,1.0 # Comment"] |
| with open("temp.param", "wb") as f: | ||
| f.write("# HEADER\r\nPARAM1,1.0 # Comment\r\n".encode("utf-32")) | ||
|
|
||
| params, content = load_param_file_with_content("temp.param") |
Replace the local device ID decoder with ArduPilot's upstream Tools/scripts/decode_devid.py. Update sensor-rule integration for the upstream decoder API, preserve the copied-file disclaimer in the update workflow, and exclude the synchronized file from project-specific Ruff checks.
ArduPlane stores multicopter frame settings in Q_FRAME_CLASS and Q_FRAME_TYPE, unlike ArduCopter. Pass the vehicle type through frame extraction so ArduPlane explicitly prefers those parameters. Initialize the detected firmware type before importing parameters and creating component-editor widgets, ensuring the correct frame-class choices are used. Allow Undefined as a valid displayable frame class for normal fixed-wing ArduPlane configurations without a multicopter frame. Add regression coverage for parameter precedence and Plane validation.
…n parameter tables
1c8e866 to
63dae3c
Compare
Use per-test temporary directories for parameter loader, export, and PID adjustment tests. Update path-sensitive assertions and remove manual cleanup of repository-relative test artifacts.
6e19117 to
07e3bd4
Compare
Coverage Report for CI Build 33817969144Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.08%) to 89.316%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions39 previously-covered lines in 5 files lost coverage.
Coverage Stats
💛 - Coveralls |
☂️ Code Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
Description
Small improvements
Checklist
git commit --signoff)Testing
Describe how you tested these changes: