refactor: making the log analysis frontend more nice and adding param… - #1939
refactor: making the log analysis frontend more nice and adding param…#1939OmkarSarkar204 wants to merge 2 commits into
Conversation
|
This will have some conflicts but it will be resolved once this is merged #1904 |
4c18b04 to
24287f1
Compare
24287f1 to
b5048be
Compare
|
This is looking good. |
…ter changes to respective steps Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
df3ee60 to
c7f8bed
Compare
There was a problem hiding this comment.
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.
Refactors log analysis “quality” results to carry more context (related config step + suggested parameter fixes) and updates the Tkinter UI to let users navigate to steps and apply recommended parameter changes from the log quality report.
Changes:
- Extend
LogQualityResult/QualityIssuewithrelated_step,param_name, andsuggested_valueto support parameter recommendations. - Thread
related_stepthrough multiple quality models and enhance bitmask diagnostics with computed suggested values. - Update the Tkinter log quality report UI to show “Fix” and “Go to Step” actions and integrate with the parameter editor workflow.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ardupilot_methodic_configurator/log_analysis/data_model_quality_vibe.py | Passes related_step into results for VIBE quality checks. |
| ardupilot_methodic_configurator/log_analysis/data_model_quality_pm.py | Passes related_step into results for PM quality checks. |
| ardupilot_methodic_configurator/log_analysis/data_model_quality_mode.py | Passes related_step into results and propagates it in absence diagnosis. |
| ardupilot_methodic_configurator/log_analysis/data_model_quality_imu.py | Passes related_step into results for IMU quality checks. |
| ardupilot_methodic_configurator/log_analysis/data_model_quality_gnss.py | Passes related_step into results for GNSS/GPS quality checks. |
| ardupilot_methodic_configurator/log_analysis/data_model_quality_fft.py | Adds parameter recommendation fields for INS batch logging and passes related_step. |
| ardupilot_methodic_configurator/log_analysis/data_model_quality_battery.py | Adds related_step to absence results and adjusts UI-facing text. |
| ardupilot_methodic_configurator/log_analysis/data_model_quality_base.py | Extends build_result() and adds suggested parameter value computation for LOG_BITMASK. |
| ardupilot_methodic_configurator/log_analysis/data_model_log_quality.py | Extends datamodels to carry parameter fix metadata and related step. |
| ardupilot_methodic_configurator/log_analysis/data_model_log_analysis.py | Collects “related parameter values” from issues to power fix recommendations in UI. |
| ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py | Adds “Analyse a .bin log” button and hooks navigation/upload flow back to report. |
| ardupilot_methodic_configurator/frontend_tkinter_log_quality.py | Adds “Fix” / “Go to Step” actions, review dialog, and step absorption logic in report. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if self.navigate_callback is not None: | ||
| self.navigate_callback(step) |
| 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) | ||
| show_tooltip(value_lbl, "\n".join(f"- {r}" for r in reasons)) |
| return self.build_result(issues, name, related_step=step) | ||
|
|
||
| def _diagnose_absence(self) -> LogQualityResult: | ||
| name = self.resolve_message_step("BAT", "Battery")[1] |
| return LogQualityResult( | ||
| available=False, state=LogQualityState.WARNING, reason=reason, issues=issues, name=name, related_step=step | ||
| ) |
| proposed |= int(issue.suggested_value) # type: ignore[arg-type] | ||
| proposed_value = float(proposed) | ||
| else: | ||
| proposed_value = param_issues[0].suggested_value # type: ignore[assignment] |
| absorbed_by_step: dict[str, list[StepValidationResult]] = {} | ||
| for step_result in self.summary.step_results: | ||
| for q in self.summary.quality_results: | ||
| if q.related_step and q.related_step == step_result.step: | ||
| absorbed_by_step.setdefault(q.related_step, []).append(step_result) | ||
| break |
Coverage Report for CI Build 32549897007Coverage decreased (-0.4%) to 90.385%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions233 previously-covered lines in 11 files lost coverage.
Coverage Stats💛 - Coveralls |

Log analysis frontend restructure and adding parameter recommendation
Description
Checklist
git commit --signoff)Testing
Describe how you tested these changes: