Small fixes

ARCHITECTURE_log_analysis.md

@@ -12,13 +12,14 @@ The main components are:
 
 1. **Log Analysis Backend** - Loads and validates ArduPilot `.bin` logs and prepares the data required by the analysis layer.
 
-   * [`backend_log_analysis.py`](ardupilot_methodic_configurator/backend_log_analysis.py)
+   * [`backend_log_analysis.py`](ardupilot_methodic_configurator/log_analysis/backend_log_analysis.py)
    * [`backend_log_extraction.py`](ardupilot_methodic_configurator/log_analysis/backend_log_extraction.py)
 
 2. **Log Analysis Data Models** - Contains the analysis pipeline, shared context, quality models, analysis models, and result structures.
 
    * [`data_model_log_analysis.py`](ardupilot_methodic_configurator/log_analysis/data_model_log_analysis.py)
    * [`data_model_log_analysis_context.py`](ardupilot_methodic_configurator/log_analysis/data_model_log_analysis_context.py)
+   * [`data_model_parameter_derivation.py`](ardupilot_methodic_configurator/log_analysis/data_model_parameter_derivation.py)
    * [`data_model_log_quality.py`](ardupilot_methodic_configurator/log_analysis/data_model_log_quality.py)
    * [`data_model_log_quality_check.py`](ardupilot_methodic_configurator/log_analysis/data_model_log_quality_check.py)
    * [`data_model_quality_base.py`](ardupilot_methodic_configurator/log_analysis/data_model_quality_base.py)
@@ -48,14 +49,41 @@ The main components are:
 
 The log extraction backend reads an ArduPilot `.bin` flight log and creates the internal `LogData` representation.
 
-The extraction layer is responsible for parsing the log and providing the data required by the analysis models. Individual analysis models do not parse the `.bin` file themselves.
+The extraction layer is responsible for parsing the log and providing the data required by the analysis models. Individual analysis models do not parse the `.bin` file
+themselves.
 
 The extraction backend can also report progress through a callback so that the frontend can display parsing progress without depending on the parser implementation.
 
+### Numeric Storage and Scaling
+
+`LogData` keeps one compact NumPy structured array for each log message type. The stored representation is selected to limit the permanent memory cost of long flight
+logs; conversion to analysis units happens only when required.
+
+ArduPilot DataFlash fixed-point format characters `c`, `C`, `e`, `E`, and `L` are stored as their original integer values. Although pymavlink exposes scaled values
+through normal attribute access and `to_dict()`, extraction reads the corresponding `DFMessage._elements` entry for these fields. `_elements` is a pymavlink private API,
+but it is maintained by the ArduPilot project and is isolated to the extraction adapter with fixture-based regression coverage.
+
+`LogData.get_field(..., scaled=True)` applies the fixed-point multiplier with a vectorized `float64` NumPy operation. The temporary scaled array is not cached, so
+analyses that do not request a field do not pay its memory cost.
+
+FMTU multipliers use a width-aware policy:
+
+* `f` and `d` fields apply their dynamic FMTU multiplier while being ingested, retaining their original floating-point dtype and avoiding repeated scaling for common
+  telemetry fields.
+* Integer fields whose multiplier would require a wider or fractional representation retain their compact stored value and scale lazily.
+* Multipliers equal to one leave values unchanged.
+
+Each `MessageSchema` records `stored_units`, `scaled_units`, `multipliers`, and `multipliers_applied_at_ingest`. These fields make the storage-to-analysis conversion
+explicit and prevent a multiplier from being applied twice.
+
+Analysis code always uses `LogData`'s default scaled representation. The `scaled=False` option is retained for low-level diagnostics and regression tests, not for
+production analysis. A result timestamp is converted to microseconds only when populating `LogAnalysis.timestamp_us`; parameter values with different documented units
+are converted explicitly before comparison.
+
 ## Log Analysis Backend
 
-[`backend_log_analysis.py`](ardupilot_methodic_configurator/backend_log_analysis.py) acts as the orchestration layer between log extraction, Methodic Configurator context,
-and the analysis data models.
+[`backend_log_analysis.py`](ardupilot_methodic_configurator/log_analysis/backend_log_analysis.py) acts as the orchestration layer between log extraction, Methodic
+Configurator context, and the analysis data models.
 
 Its responsibilities are:
 
@@ -98,12 +126,18 @@ The context contains:
 * Methodic Configurator configuration steps
 * Vehicle component information
 * ArduPilot parameter documentation
+* A parameter-derivation service
 
 The backend constructs this context after extracting the log.
 
 The analysis models receive the context instead of independently loading these resources. This keeps data loading outside the analysis models and avoids duplicated
 filesystem and configuration logic.
 
+Detailed analysis models use the parameter-derivation service to evaluate forced
+and derived configuration parameters. The default adapter reuses the shared
+configuration-step expression evaluator with only the already-loaded context
+data; tests can supply a small replacement service without a vehicle directory.
+
 ## Analysis Pipeline
 
 [`data_model_log_analysis.py`](ardupilot_methodic_configurator/log_analysis/data_model_log_analysis.py) contains the main domain-level analysis pipeline.

ardupilot_methodic_configurator/frontend_tkinter_log_analysis.py

@@ -11,29 +11,23 @@
 """
 
 import tkinter as tk
+from collections.abc import Callable
 from enum import Enum
+from functools import partial
 from pathlib import Path
 from tkinter import messagebox, ttk
 from typing import Any
 
 from ardupilot_methodic_configurator import _
 from ardupilot_methodic_configurator.backend_internet import webbrowser_open_url
+from ardupilot_methodic_configurator.data_model_par_dict import Par
 from ardupilot_methodic_configurator.frontend_tkinter_autoresize_combobox import AutoResizeCombobox
 from ardupilot_methodic_configurator.frontend_tkinter_base_window import BaseWindow
 from ardupilot_methodic_configurator.frontend_tkinter_scroll_frame import ScrollFrame
 from ardupilot_methodic_configurator.frontend_tkinter_show import show_tooltip
 from ardupilot_methodic_configurator.frontend_tkinter_tuning_report import TuningReportWindow
-from ardupilot_methodic_configurator.log_analysis.data_model_log_analysis import QUALITY_AND_ANALYSIS_MODELS, LogSummary
-from ardupilot_methodic_configurator.log_analysis.data_model_log_analysis_result import LogAnalysis, LogAnalysisResult
-from ardupilot_methodic_configurator.log_analysis.data_model_log_quality import LogQualityResult
-
-_SUBSYSTEM_TO_COMPONENT_KEYS: dict[str, tuple[str, ...]] = {
-    "Battery": ("Battery", "Battery Monitor"),
-    "ESC telemetry": ("ESC", "Motors"),
-    "IMU": ("Flight Controller",),
-    "VIBE": ("Flight Controller",),
-    "GPS": ("GNSS Receiver",),
-}
+from ardupilot_methodic_configurator.log_analysis.data_model_log_analysis import LogSummary
+from ardupilot_methodic_configurator.log_analysis.data_model_log_analysis_result import LogAnalysis
 
 
 class Severity(Enum):
@@ -63,28 +57,6 @@ class Severity(Enum):
 }
 
 
-def paired_quality_and_analysis_results(summary: LogSummary) -> list[tuple[LogQualityResult, LogAnalysisResult | None]]:
-    offset = len(summary.quality_results) - len(QUALITY_AND_ANALYSIS_MODELS)
-    if offset not in (0, 1):
-        msg = (
-            f"Unexpected quality_results length ({len(summary.quality_results)}) vs "
-            f"QUALITY_AND_ANALYSIS_MODELS length ({len(QUALITY_AND_ANALYSIS_MODELS)})."
-        )
-        raise AssertionError(msg)
-
-    analysis_iter = iter(summary.analysis_results)
-    paired: list[tuple[LogQualityResult, LogAnalysisResult | None]] = []
-
-    for i, (_quality_cls, analysis_cls) in enumerate(QUALITY_AND_ANALYSIS_MODELS):
-        if analysis_cls is None:
-            continue
-        quality_result = summary.quality_results[i + offset]
-        analysis_result = next(analysis_iter) if quality_result.available else None
-        paired.append((quality_result, analysis_result))
-
-    return paired
-
-
 def _collect_links(quality_dict: dict[str, Any] | None, analysis_dict: dict[str, Any] | None) -> list[dict[str, Any]]:
     seen: set[tuple[str | None, str | None]] = set()
     links: list[dict[str, Any]] = []
@@ -146,20 +118,24 @@ def _format_component(component: dict[str, Any]) -> list[str]:  # pylint: disabl
 class LogAnalysisReportWindow(BaseWindow):  # pylint: disable=too-many-instance-attributes
     """Log analysis window."""
 
-    def __init__(
+    def __init__(  # pylint: disable=too-many-arguments, too-many-positional-arguments
         self,
         root_tk: tk.Tk | tk.Toplevel,
         summary: LogSummary,
         vehicle_dir: str,
         report: dict[str, Any] | None = None,
+        is_fc_connected: bool = False,
+        upload_callback: Callable[[dict[str, Par]], bool | None] | None = None,
     ) -> None:
         super().__init__(root_tk)
         self.summary = summary
         self.vehicle_dir = vehicle_dir
         self.report = report
+        self.is_fc_connected = is_fc_connected
+        self.upload_callback = upload_callback
         self._ai_panel_visible = False
 
-        self.pairs = paired_quality_and_analysis_results(summary)
+        self.pairs = summary.paired_quality_and_analysis_results()
         self.subsystem_names = [q.name for q, _a in self.pairs]
 
         self._report_quality_by_name: dict[str, dict[str, Any]] = {}
@@ -279,7 +255,7 @@ def _render_subsystem(self, name: str) -> None:  # pylint: disable=too-many-loca
                     self._section_link(_("Guide"), link.get("blog_text") or link["blog_url"], link["blog_url"])
 
         vehicle_components = (self.report or {}).get("vehicle_components") or {}
-        component_keys = _SUBSYSTEM_TO_COMPONENT_KEYS.get(name, ())
+        component_keys = self.summary.component_keys_for_subsystem(quality_result.subsystem_key)
         hardware_lines: list[tuple[str, list[str]]] = []
         for key in component_keys:
             component = vehicle_components.get(key)
@@ -331,13 +307,73 @@ def _bullet_line(self, text: str) -> None:
 
     def _outcome_line(self, outcome: LogAnalysis) -> None:
         timestamp_text = f" ({outcome.timestamp_us / 1e6:.1f}s)" if outcome.timestamp_us is not None else ""
+        row = ttk.Frame(self.body_frame)
+        row.pack(anchor=tk.W, padx=(10, 0), pady=3, fill=tk.X)
         ttk.Label(
-            self.body_frame,
+            row,
             text=f"{outcome.message}{timestamp_text}",
             font=("TkDefaultFont", 14),
             wraplength=950,
             justify=tk.LEFT,
-        ).pack(anchor=tk.W, padx=(10, 0), pady=3, fill=tk.X)
+        ).pack(side=tk.LEFT, fill=tk.X, expand=True)
+
+        fixes = self._fix_for_outcome(outcome)
+        if fixes:
+            fix_button = ttk.Button(row, text=_("Fix"), command=partial(self._open_review_dialog, fixes))
+            fix_button.pack(side=tk.RIGHT, padx=(8, 0))
+
+    def _fix_for_outcome(self, outcome: LogAnalysis) -> list[tuple[str, float, float, list[str]]]:
+        if not isinstance(outcome.param_name, str) or not isinstance(outcome.suggested_value, (int, float)):
+            return []
+        current = self.summary.related_parameter_values.get(outcome.param_name)
+        if current is None or float(outcome.suggested_value) == current:
+            return []
+        return [(outcome.param_name, current, float(outcome.suggested_value), [outcome.message])]
+
+    def _open_review_dialog(self, fixes: list[tuple[str, float, float, list[str]]]) -> None:
+        dialog = tk.Toplevel(self.root)
+        dialog.title(_("Review Parameter Changes"))
+        dialog.geometry(self.calculate_scaled_geometry(520, 140 + 60 * len(fixes)))
+        self.center_window(dialog, self.root)
+        dialog.transient(self.root)
+        dialog.grab_set()
+
+        ttk.Label(dialog, text=_("The following parameter change(s) are proposed:"), font=("TkDefaultFont", 11, "bold")).pack(
+            anchor=tk.W, padx=14, pady=(14, 6)
+        )
+        rows_frame = ttk.Frame(dialog)
+        rows_frame.pack(fill=tk.BOTH, expand=True, padx=14, pady=(0, 6))
+        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(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(proposed), foreground="darkgreen", font=("TkDefaultFont", 11, "bold"))
+            value_lbl.pack(side=tk.LEFT)
+            show_tooltip(value_lbl, "\n".join(f"- {reason}" for reason in reasons))
+
+        button_row = ttk.Frame(dialog)
+        button_row.pack(fill=tk.X, padx=14, pady=(6, 14))
+        ttk.Button(button_row, text=_("Cancel"), command=dialog.destroy).pack(side=tk.RIGHT, padx=(6, 0))
+        upload_button = ttk.Button(
+            button_row,
+            text=_("Apply & Upload"),
+            command=partial(self._apply_param_fixes, fixes, dialog),
+        )
+        upload_button.configure(state="normal" if self.is_fc_connected else "disabled")
+        upload_button.pack(side=tk.RIGHT)
+        if not self.is_fc_connected:
+            show_tooltip(upload_button, _("No flight controller connected, upload not available"))
+
+    def _apply_param_fixes(self, fixes: list[tuple[str, float, float, list[str]]], dialog: tk.Toplevel) -> None:
+        changes = {param_name: Par(proposed, "") for param_name, _current, proposed, _reasons in fixes}
+        if self.upload_callback is not None:
+            upload_result = self.upload_callback(changes)
+            if upload_result is False:
+                return
+        self.summary.related_parameter_values.update({name: par.value for name, par in changes.items()})
+        dialog.destroy()
 
     def _section_link(self, tag: str, text: str, url: str) -> None:
         row = ttk.Frame(self.body_frame)

ardupilot_methodic_configurator/frontend_tkinter_log_quality.py

@@ -23,10 +23,7 @@
 from ardupilot_methodic_configurator.data_model_par_dict import Par
 from ardupilot_methodic_configurator.formatting import format_filesize
 from ardupilot_methodic_configurator.frontend_tkinter_base_window import BaseWindow
-from ardupilot_methodic_configurator.frontend_tkinter_log_analysis import (
-    LogAnalysisReportWindow,
-    paired_quality_and_analysis_results,
-)
+from ardupilot_methodic_configurator.frontend_tkinter_log_analysis import LogAnalysisReportWindow
 from ardupilot_methodic_configurator.frontend_tkinter_log_hardware_quality import build_hardware_tab
 from ardupilot_methodic_configurator.frontend_tkinter_scroll_frame import ScrollFrame
 from ardupilot_methodic_configurator.frontend_tkinter_show import show_tooltip
@@ -45,6 +42,11 @@
 )
 
 
+def _format_parameter_value(value: float) -> str:
+    """Format a parameter value without hiding fractional changes."""
+    return str(int(value)) if value.is_integer() else str(value)
+
+
 class LogQualityReportWindow(BaseWindow):  # pylint: disable=too-many-instance-attributes
     """Displays log analysis results as a beginner-friendly, detailed dashboard."""
 
@@ -55,7 +57,7 @@ def __init__(  # pylint: disable=too-many-arguments, too-many-positional-argumen
         summary: LogSummary,
         vehicle_dir: str,
         is_fc_connected: bool = False,
-        upload_callback: Callable[[dict], bool] | None = None,
+        upload_callback: Callable[[dict[str, Par]], bool | None] | None = None,
         navigate_callback: Callable[[str], None] | None = None,
         report: dict | None = None,
     ) -> None:
@@ -146,7 +148,7 @@ def _build_footer(self) -> None:
     def _on_continue_to_analysis(self) -> None:
         pending_names = [
             quality_result.name
-            for quality_result, analysis_result in paired_quality_and_analysis_results(self.summary)
+            for quality_result, analysis_result in self.summary.paired_quality_and_analysis_results()
             if analysis_result is None
         ]
         if pending_names:
@@ -157,7 +159,14 @@ def _on_continue_to_analysis(self) -> None:
                 ),
                 parent=self.root,
             )
-        self._analysis_window = LogAnalysisReportWindow(self.root, self.summary, self.vehicle_dir, report=self.report)
+        self._analysis_window = LogAnalysisReportWindow(
+            self.root,
+            self.summary,
+            self.vehicle_dir,
+            is_fc_connected=self.is_fc_connected,
+            upload_callback=self.upload_callback,
+            report=self.report,
+        )
 
     def _open_review_dialog(self, fixes: list[tuple[str, float, float, list[str]]]) -> None:
         dialog = tk.Toplevel(self.root)
@@ -178,9 +187,14 @@ def _open_review_dialog(self, fixes: list[tuple[str, float, float, list[str]]])
             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=_format_parameter_value(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 = ttk.Label(
+                row,
+                text=_format_parameter_value(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))
 
@@ -200,9 +214,12 @@ def _open_review_dialog(self, fixes: list[tuple[str, float, float, list[str]]])
 
     def _apply_param_fixes(self, fixes: list[tuple[str, float, float, list[str]]], dialog: tk.Toplevel) -> None:
         changes = {param_name: Par(proposed, "") for param_name, _current, proposed, _reasons in fixes}
-        self.summary.related_parameter_values.update({name: par.value for name, par in changes.items()})
         if self.upload_callback is not None:
-            self.upload_callback(changes)
+            upload_result = self.upload_callback(changes)
+            if upload_result is False:
+                return
+
+        self.summary.related_parameter_values.update({name: par.value for name, par in changes.items()})
         dialog.destroy()
 
     @staticmethod
@@ -348,8 +365,8 @@ def _build_quality_tab(self, parent: ttk.Frame) -> None:  # pylint: disable=too-
             for kind, item in needs_attention:
                 if kind == "quality":
                     quality_item = cast("LogQualityResult", item)
-                    absorbed_steps = absorbed_by_step.get(quality_item.related_step, [])
-                    self._quality_result_card(inner, quality_item, absorbed_steps)
+                    quality_absorbed_steps = absorbed_by_step.get(quality_item.related_step, [])
+                    self._quality_result_card(inner, quality_item, quality_absorbed_steps)
                 else:
                     self._step_result_card(inner, item)  # type: ignore[arg-type]
             ttk.Separator(inner, orient=tk.HORIZONTAL).pack(fill=tk.X, padx=14, pady=(14, 14))
@@ -361,8 +378,8 @@ def _build_quality_tab(self, parent: ttk.Frame) -> None:  # pylint: disable=too-
             for kind, item in passed_checks:
                 if kind == "quality":
                     quality_item = cast("LogQualityResult", item)
-                    absorbed_steps = absorbed_by_step.get(quality_item.related_step, [])
-                    self._quality_result_card(inner, quality_item, absorbed_steps)
+                    quality_absorbed_steps = absorbed_by_step.get(quality_item.related_step, [])
+                    self._quality_result_card(inner, quality_item, quality_absorbed_steps)
                 else:
                     self._step_result_card(inner, item)  # type: ignore[arg-type]