Small fixes by amilcarlucas · Pull Request #1974 · ArduPilot/MethodicConfigurator

amilcarlucas · 2026-08-24T12:43:54Z

Description

Fixes on top of Omkar's work.

Checklist

Run pre-commit checks locally
Verified by a human programmer
All commits are signed off (use git commit --signoff)
Code follows our coding standards
Documentation updated if needed
No breaking changes or properly documented

Testing

Describe how you tested these changes:

Unit tests pass
Integration tests pass
Manual testing performed
Tested on flight controller hardware

Copilot

Pull request overview

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.

Extends the log-quality workflow into a richer “quality + analysis” pipeline, adds tuning-report visualization, and updates models/tests to support parameter-fix and navigation flows.

Changes:

Introduces analysis result types and per-subsystem analysis models (Battery/ESC/IMU/VIBE) and wires them into analyze_log().
Adds new Tkinter windows for detailed analysis and tuning-report plotting, and integrates them with the parameter editor + log quality UI.
Updates configuration-step parameter computation plumbing and expands/adjusts tests accordingly.

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_frontend_tkinter_parameter_editor.py	Updates test expectations for the new log report window constructor + internal state.
tests/test_frontend_tkinter_log_analysis.py	Adds coverage for pairing logic and the new LogAnalysisReportWindow behaviors.
tests/test_data_model_tuning_report.py	Adds parsing test for truncating malformed CSV rows to header length.
tests/test_data_model_quality_models.py	Adds targeted tests for new ESC analysis checks.
tests/test_data_model_parameter_editor.py	Aligns context-input structure with updated vehicle_components shape.
tests/test_data_model_log_analysis.py	Updates tests to new model registry API (`quality_and_analysis_models`).
tests/test_data_model_imu_analysis.py	Adds comprehensive IMU quality + analysis tests.
tests/test_data_model_configuration_step.py	Updates mocks/assertions for combined forced+derived computation API.
tests/test_data_model_battery_analysis.py	Adds comprehensive Battery quality + analysis tests.
tests/acceptance_only_changed_get_uploaded_usage.py	Updates acceptance wiring for new log report window state fields.
ardupilot_methodic_configurator/log_analysis/utils.py	Extends step lookup to include `add_parameters`.
ardupilot_methodic_configurator/log_analysis/data_model_tuning_report.py	Adds tuning_report.csv parsing + forward-fill model.
ardupilot_methodic_configurator/log_analysis/data_model_quality_vibe.py	Adds VIBE analysis model and related-step propagation.
ardupilot_methodic_configurator/log_analysis/data_model_quality_pm.py	Migrates PM model to shared base and propagates related-step.
ardupilot_methodic_configurator/log_analysis/data_model_quality_mode.py	Migrates MODE model to shared base and propagates related-step.
ardupilot_methodic_configurator/log_analysis/data_model_quality_imu.py	Adds IMU analysis model and related-step propagation.
ardupilot_methodic_configurator/log_analysis/data_model_quality_gnss.py	Migrates GPS model to shared base and propagates related-step.
ardupilot_methodic_configurator/log_analysis/data_model_quality_fft.py	Improves FFT absence issue with actionable suggested_value and related-step.
ardupilot_methodic_configurator/log_analysis/data_model_quality_esc.py	Adds ESC analysis model and new parameter/telemetry checks.
ardupilot_methodic_configurator/log_analysis/data_model_quality_err.py	Migrates ERR model to shared base.
ardupilot_methodic_configurator/log_analysis/data_model_quality_battery.py	Adds Battery analysis model and moves “analysis-like” checks out of quality.
ardupilot_methodic_configurator/log_analysis/data_model_quality_base.py	Introduces `BaseLogModel` with shared expected-value derivation helpers.
ardupilot_methodic_configurator/log_analysis/data_model_quality_arm.py	Migrates ARM model to shared base.
ardupilot_methodic_configurator/log_analysis/data_model_log_quality.py	Extends `QualityIssue` with param + suggested_value, and results with related_step.
ardupilot_methodic_configurator/log_analysis/data_model_log_analysis_result.py	Adds structured analysis outcome/result dataclasses.
ardupilot_methodic_configurator/log_analysis/data_model_log_analysis.py	Wires quality+analysis model registry and tracks related_parameter_values.
ardupilot_methodic_configurator/log_analysis/backend_log_extraction.py	Adjusts multiplier resolution to avoid double-scaling for fixed-point fields.
ardupilot_methodic_configurator/frontend_tkinter_tuning_report.py	Adds interactive tuning parameter graph window (Matplotlib + Tkinter).
ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py	Adds “Analyse .bin log” entrypoint and return-to-report workflow.
ardupilot_methodic_configurator/frontend_tkinter_log_quality.py	Adds “Continue to Analysis” and “Fix/Go to Step” flows from quality report.
ardupilot_methodic_configurator/frontend_tkinter_log_analysis.py	Adds detailed analysis window, link aggregation, hardware formatting, fix workflow.
ardupilot_methodic_configurator/data_model_parameter_editor.py	Fixes vehicle_components snapshot shape for log analysis inputs.
ardupilot_methodic_configurator/data_model_configuration_step.py	Switches to combined forced+derived computation method.
ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py	Adds `compute_forced_and_derived_parameters()` helper.
ardupilot_methodic_configurator/backend_filesystem.py	Uses combined forced+derived computation and improves error reporting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        for param_name, current, proposed, reasons in fixes:
+            row = ttk.Frame(rows_frame)
+            row.pack(fill=tk.X, pady=4)
+            ttk.Label(row, text=param_name, width=18, font=("TkDefaultFont", 11, "bold")).pack(side=tk.LEFT)
+            ttk.Label(row, text=str(int(current)), foreground="gray").pack(side=tk.LEFT, padx=(0, 6))
+            ttk.Label(row, text="->").pack(side=tk.LEFT, padx=(0, 6))
+            value_lbl = ttk.Label(row, text=str(int(proposed)), foreground="darkgreen", font=("TkDefaultFont", 11, "bold"))
+            value_lbl.pack(side=tk.LEFT)


Copilot

Pull request overview

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

Handle empty or malformed tuning_report.csv files without allowing uncaught exceptions to escape from the tuning graph button callback. Only update displayed parameter values and close the review dialog after a parameter upload succeeds. Failed uploads now preserve the proposed changes for retry and keep the report state accurate. Remove the duplicate unattached Skip button construction from the parameter editor. Signed-off-by: Dr.-Ing. Amilcar do Carmo Lucas <amilcar.lucas@iav.de>

… mismatches Collect current parameter values from every quality-model result so the log-quality report can offer Fix actions for battery, IMU, ESC, FFT, and other parameter-related issues. Also ensure MOT_BAT_VOLT_MAX derivation mismatches are appended as analysis outcomes instead of being silently discarded. Signed-off-by: Dr.-Ing. Amilcar do Carmo Lucas <amilcar.lucas@iav.de>

Suppress zero-error ESC reports and require meaningful telemetry coverage before reporting zero RPM during armed periods. Expose actionable analysis recommendations through the parameter review and upload workflow, with regression coverage for both ESC detection paths and parameter fixes. Signed-off-by: Dr.-Ing. Amilcar do Carmo Lucas <amilcar.lucas@iav.de>

Restrict ESC current imbalance analysis to armed telemetry windows and skip DShot output-rate checks when the vehicle uses ordinary PWM. Add regression coverage for PWM configurations and ESC analysis behavior. Signed-off-by: Dr.-Ing. Amilcar do Carmo Lucas <amilcar.lucas@iav.de>

Truncate tuning_report.csv rows that contain more cells than the header defines so every parameter series remains aligned with configuration steps and can be plotted safely.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Amilcar Lucas <amilcar.lucas@iav.de>

…logs Format parameter values without truncating decimals in the log-quality parameter-change review dialog. Add coverage for integral and fractional display values. Signed-off-by: Dr.-Ing. Amilcar do Carmo Lucas <amilcar.lucas@iav.de>

Treat upload callbacks returning None as successful side-effect operations and abort only when they explicitly return False. Tighten callback type annotations, remove obsolete StopIteration handling, and add regression coverage for quality and analysis parameter fixes.

Remove unnecessary type casts from log-analysis frontend tests and clean up the unused typing import.

Add the missing BaseLogModel analyse interface, avoid variable type shadowing in the quality report, and document untyped Matplotlib calls with narrow mypy suppressions.

…ring Introduce dedicated quality and analysis base classes to reduce coupling with configuration-step infrastructure. Register subsystems with stable keys and pair quality results with analysis results by key instead of list position. Update frontend pairing tests accordingly.

Treat NaN and infinite numeric telemetry as invalid data during quality and performance checks. Prevent invalid values from being reported as healthy or causing numeric conversion errors. Add regression tests for battery and PM telemetry.

Store DataFlash fixed-point values in their compact raw representation and scale them lazily for analysis. Apply compatible FMTU multipliers during ingestion, while recording stored and scaled units explicitly. Migrate analysis models to canonical scaled values, convert timestamps only at result boundaries, and document the storage and scaling policy.

…teps Inject parameter derivation through the analysis context and move subsystem component metadata into the model registry. Update architecture documentation and regression coverage.

Extract PM validation, log record conversion, and subsystem execution helpers. Bundle parameter-derivation inputs to remove wide method signatures.

github-actions · 2026-08-25T00:41:21Z

☂️ Code Coverage

current status: ✅

Overall Coverage

Statements	Covered	Coverage	Threshold	Status
18644	16660	89%	89%	🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: ee46bce by action🐍

coveralls · 2026-08-25T00:42:32Z

Coverage Report for CI Build 32757700414

Coverage increased (+0.4%) to 89.391%

Details

Coverage increased (+0.4%) from the base build.
Patch coverage: No coverable lines changed in this PR.
404 coverage regressions across 14 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

404 previously-covered lines in 14 files lost coverage.

Top 10 Files by Coverage Loss	Lines Losing Coverage	Coverage
frontend_tkinter_log_quality.py	81	24.0%
log_analysis/data_model_quality_esc.py	74	45.37%
frontend_tkinter_log_analysis.py	57	73.44%
frontend_tkinter_parameter_editor.py	37	93.86%
frontend_tkinter_tuning_report.py	29	14.66%
log_analysis/backend_log_extraction.py	29	86.01%
log_analysis/data_model_quality_battery.py	24	83.95%
log_analysis/data_model_quality_vibe.py	15	26.47%
log_analysis/data_model_quality_imu.py	15	78.57%
log_analysis/data_model_log_analysis.py	13	89.26%

Coverage Stats


Relevant Lines:	18644
Covered Lines:	16666
Line Coverage:	89.39%
Relevant Branches:	5576
Covered Branches:	4608
Branch Coverage:	82.64%
Branches in Coverage %:	No
Coverage Strength:	2.66 hits per line

💛 - Coveralls

github-actions · 2026-08-25T05:10:42Z

Test Results

4 files ± 0 4 suites ±0 45m 38s ⏱️ -28s
4 858 tests +22 4 851 ✅ +22 7 💤 ±0 0 ❌ ±0
19 214 runs +88 19 169 ✅ +88 45 💤 ±0 0 ❌ ±0

Results for commit ee46bce. ± Comparison against base commit 6980784.

This pull request removes 3 and adds 25 tests. Note that renamed tests count towards both.

tests.test_frontend_tkinter_log_analysis.TestPairedQualityAndAnalysisResults ‑ test_handles_one_prepended_quality_result_via_offset
tests.test_frontend_tkinter_log_analysis.TestPairedQualityAndAnalysisResults ‑ test_raises_when_offset_is_not_zero_or_one
tests.unit_backend_log_extraction.TestNumpyAllocation ‑ test_integer_scaling_promotes_dtype

tests.test_data_model_log_analysis ‑ test_base_analysis_model_delegates_parameter_derivation_to_context_service
tests.test_data_model_log_analysis ‑ test_subsystem_component_metadata_is_declared_by_the_registry
tests.test_data_model_log_quality_check ‑ test_pm_error_signals_preserve_report_order
tests.test_data_model_log_quality_check ‑ test_pm_non_finite_values_are_reported_as_invalid
tests.test_data_model_quality_models ‑ test_battery_capacity_uses_canonical_amp_hours
tests.test_data_model_quality_models ‑ test_battery_model_checks_failsafes_for_each_enabled_monitor
tests.test_data_model_quality_models ‑ test_battery_model_rejects_non_finite_telemetry_values
tests.test_data_model_quality_models ‑ test_esc_analysis_does_not_report_zero_error_rates_as_findings
tests.test_data_model_quality_models ‑ test_esc_analysis_ignores_a_single_zero_rpm_sample
tests.test_data_model_quality_models ‑ test_esc_analysis_skips_dshot_rate_for_pwm_output
…

amilcarlucas · 2026-08-25T10:09:59Z

@OmkarSarkar204 can you review and test this?

Copilot AI lite review requested due to automatic review settings August 24, 2026 12:43

Copilot AI reviewed Aug 24, 2026

View reviewed changes

amilcarlucas force-pushed the small_fixes branch 2 times, most recently from 181e1f2 to 0346356 Compare August 24, 2026 13:26

amilcarlucas requested a lite review from Copilot August 24, 2026 13:29

Copilot AI reviewed Aug 24, 2026

View reviewed changes

amilcarlucas force-pushed the small_fixes branch 3 times, most recently from 6614be4 to f0dd065 Compare August 24, 2026 15:40

amilcarlucas and others added 13 commits August 24, 2026 17:44

fix(log-analysis): normalize malformed tuning report rows

5917daf

Truncate tuning_report.csv rows that contain more cells than the header defines so every parameter series remains aligned with configuration steps and can be plotted safely.

fix(log-analysis): Apply suggestions from code review

ac7f425

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Amilcar Lucas <amilcar.lucas@iav.de>

test(log-analysis): remove redundant window casts

5b8f42d

Remove unnecessary type casts from log-analysis frontend tests and clean up the unused typing import.

fix(log-analysis): resolve static type-checking errors

871889d

Add the missing BaseLogModel analyse interface, avoid variable type shadowing in the quality report, and document untyped Matplotlib calls with narrow mypy suppressions.

fix(log-analysis): fix pylint

163c0bc

amilcarlucas force-pushed the small_fixes branch from f0dd065 to 34d6075 Compare August 24, 2026 15:44

amilcarlucas added 4 commits August 24, 2026 19:05

refactor(log-analysis): decouple analysis models from configuration s…

9496174

…teps Inject parameter derivation through the analysis context and move subsystem component metadata into the model registry. Update architecture documentation and regression coverage.

refactor(log-analysis): reduce analysis complexity

69aee69

Extract PM validation, log record conversion, and subsystem execution helpers. Bundle parameter-derivation inputs to remove wide method signatures.

fix(lint): fix linter issues

ee46bce

Copilot started reviewing on behalf of amilcarlucas August 24, 2026 17:58 View session

Copilot started reviewing on behalf of amilcarlucas August 24, 2026 19:58 View session

Navigation Menu

Uh oh!

Small fixes - #1974

Small fixes#1974
amilcarlucas wants to merge 17 commits into
masterfrom
small_fixes

amilcarlucas commented Aug 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 25, 2026 •

edited

Loading

Uh oh!

coveralls commented Aug 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 25, 2026

Uh oh!

amilcarlucas commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

amilcarlucas commented Aug 24, 2026

Description

Checklist

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☂️ Code Coverage

Overall Coverage

New Files

Modified Files

Uh oh!

coveralls commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 32757700414

Coverage increased (+0.4%) to 89.391%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

github-actions Bot commented Aug 25, 2026

Test Results

Uh oh!

amilcarlucas commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Aug 25, 2026 •

edited

Loading

coveralls commented Aug 25, 2026 •

edited

Loading