fix(parameter-editor): keep bottom controls visible on short screens

[amilcarlucas]

Description

Cap the parameter editor window height to the available screen height and size the parameter area to the space remaining after fixed controls, including the bottom button row.

Recalculate the parameter area when the window is resized or an embedded component editor is shown or hidden, allowing the parameter table to scroll instead of pushing the buttons off-screen.

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]

🟡 Changes recommended

Multi-monitor sizing, geometry preservation, and existing initialization tests must be corrected.

2 open findings

• Update mocked initialization tests for parameter area container setup · New
• Use the hosting monitor bounds for the window height cap · New

What changed in this PR

Caps the parameter editor height and dynamically resizes its scrollable table.

Changes:

• Adds screen-height fitting.
• Recalculates table space during resizing and component-editor visibility changes.

File	Description
frontend_tkinter_parameter_editor.py	Adds responsive window and parameter-area sizing.

🧠 Review effort: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

☂️ Code Coverage

current status: ✅

Overall Coverage

Statements	Covered	Coverage	Threshold	Status
26702	24438	92%	89%	🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: 9ca1bac by action🐍

[github-actions]

Test Results

     3 files       3 suites   36m 12s ⏱️
 6 644 tests  6 635 ✅  9 💤 0 ❌
19 813 runs  19 780 ✅ 33 💤 0 ❌

Results for commit 9ca1bac.

♻️ This comment has been updated with latest results.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-10)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: REQUEST CHANGES

Reviewed at head 4632a414bd.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2190/1.html#prMethodicConfigurator-2190

I ran the real parameter editor window on this commit (4632a41) and on its base under Xvfb (Python 3.14.4, Tk 8.6.17, no window manager), walking through the steps of the diatone_taycan_mxc 4.5.x template. On a 1366x768 screen it does what the title says: the Skip button bottom moves from y=891 (off screen) to y=729. Four things need fixing first, though.

Scalar pady is counted once. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1654: Tk returns a single number from pack_info() when top and bottom padding are equal, so pady=(10, 10) comes back as 10 and means 10 px on each side. With the button row's (10, 10) and the two (2, 2) paddings that under-counts the fixed height by 14 px. Measured on 1920x1080: the button row gets 31 of its 45 px at startup, so the two-line buttons lose their second line, and because the table height is now pinned the window grows 14 px on each step change (854, 868, 882, 896, 910 across steps 14 to 18), leaving the Skip button at y=1103 on a 1080 px screen. Base stays at 854 with Skip at y=1047. Counting the scalar twice fixes both.

The height cap ignores where the window is. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1626 limits the height to the screen height minus 80 px, but the top edge stays where center_window_on_screen() put the initial 630 px high window. On 1600x900 the window is 817 px high at y=121, so the Skip button (y=926) is still off screen on every step. On 1920x1080, with the padding fixed, the window still grows to 931 px on 11_battery.param and 930 px on 19_motor.param, putting Skip at y=1124 and y=1123; base keeps it on screen at y=1047 on all steps. The cap needs the window's top offset, or the window needs moving up.

Nine existing tests fail. tests/test_frontend_tkinter_parameter_editor.py gives 9 failed, 90 passed here and 99 passed on the base, which matches the 9 failures per platform in the CI report. Five initialisation tests reach the new call at ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:389 with a mocked main_frame and fail with TypeError at line 1657; four build the editor without BaseWindow.__init__ and fail on dpi_scaling_factor at line 1622. Copilot's comment on this is right in substance (it is nine tests, not four). A test for the new sizing code would have caught the two problems above.

The window size is reset on every step. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1628 now runs on each parameter-file change and sets the default width and a recomputed height. A window I resized to 1400x700 went back to 1031x854 on the next step, with or without a component editor; the base keeps the size. Applying the default size only at startup, and clamping later only when needed, would avoid that.

Smaller points, none blocking. On a 768 px screen the Battery step leaves the parameter table 43 px (29 px once the padding is fixed) next to a 272 px component editor (ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1657); that is the trade-off this change chooses, but it may be worth letting the component editor scroll instead. Copilot's multi-monitor comment (ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1623) I could not confirm, as I could not set up two monitors here; by the code a larger winfo_screenheight() only makes the cap ineffective, and get_monitor_bounds() would give both the right height and the top offset needed above. The commit has no Signed-off-by, which CONTRIBUTING.md:208 asks for.

Not tested: any real window manager (title bars, taskbars, maximised windows), Windows, macOS, real multi-monitor setups, HiDPI, and the rest of the test suite beyond this module's file.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-10)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: REQUEST CHANGES

Reviewed at head 3da33c445d.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2190/2.html#prMethodicConfigurator-2190

This covers 3da33c4, which replaces the 4632a41 I reviewed before; nothing newer has been looked at. All four blockers from that review are fixed. I ran the real parameter editor window on this commit and on its base (66cf608) under Xvfb (Python 3.14.4, Tk 8.6.17), walking through the steps of the diatone_taycan_mxc 4.5.x template. With no window manager the Skip button is on screen on every step I measured (bottom at y=707 on 1366x768, 839 on 1600x900, 1019 on 1920x1080; base 891, 957, 1047), the button row keeps its full 45 px, the window no longer grows from step to step, and a window resized to 1400x700 or 1100x500 keeps its size. tests/test_frontend_tkinter_parameter_editor.py gives 114 passed here and 99 on the base.

Running it under a window manager (jwm 2.4.6: 23 px title bar, 5 px borders, 40 px bottom panel) showed two new problems in the repositioning code, and both need fixing first.

The reposition mixes frame and client coordinates on X11. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1636 passes winfo_x() back to wm geometry, and ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1635 treats winfo_y() as the top of the window-manager frame. On X11 Tk reports the client area from winfo_x()/winfo_y() as well as from winfo_rootx()/winfo_rooty(), while wm geometry positions the frame. A bare Tk window placed at +200+100 reports winfo_x=205 and winfo_y=123, and re-applying those moves it by (5, 23) each time. In the editor on 1366x768 the window moves 5 px to the right on every step change (x=177 at startup, then 182 up to 217 over eight steps; the base stays at 167), and the same on 1600x900 (294 to 334). It never settles, because the client top ends at y=65 rather than the intended 42, so the next step finds it out of place again. For the same reason the window bottom is at y=749 on a 768 px screen instead of 726, and the Skip button bottom (730) sits 2 px under the panel. On 1920x1080 the height is not capped and nothing drifts. In the same bare-Tk probe, re-applying the +x+y that wm geometry itself returns leaves the window where it is, so taking the frame position from self.root.geometry() would be one fix. The new tests set winfo_y equal to winfo_rooty (tests/test_frontend_tkinter_parameter_editor.py:370), so they model only an undecorated window.

A maximised window is un-maximised on the next step. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1632 issues wm geometry whenever the window is taller than the monitor minus the two margins, or its client top is inside the top margin, and nothing checks the zoomed state. Under jwm a maximised 1910x1012 window on 1920x1080 is a plain 1031x854 window after the next step change (1356x700 becomes 1031x684 on 1366x768); the base stays maximised. A window moved to the top edge of the screen is pushed down the same way (+100+0 becomes +105+42 on 1366x768), because its client top is 23 px from the edge and the margin is 42. Skipping the adjustment while the window is zoomed or full screen, and otherwise acting only when the window really extends past the monitor, would avoid both.

Two smaller points, neither blocking. With gui_complexity set to normal, where the stage bar shows every phase, narrowing the window from 1031 to 950 px wraps a phase label, the stage bar grows 15 px and the table keeps its old height, so the button row is left 31 of its 46 px until the next resize or step change (and again at 800 px); the base shows the full row at the same sizes. The <Configure> callback at ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1643 measures before the labels have re-wrapped, and nothing re-fits when a fixed widget changes height afterwards; re-fitting from after_idle, or on the fixed widgets' own <Configure>, would cover it. In the default simple mode I only saw this below 300 px wide, and I resized in single jumps, not by dragging. Separately, and carried over from last time: on a 768 px screen the Battery step leaves the parameter table 28 px beside a 272 px component editor (ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1669). That is the trade-off this change chooses and it beats the base, where the buttons are off screen, but a minimum table height with a scrolling component editor would make that step usable.

CI is red on this commit, but pytest on macOS 3.14 and markdown-link-check fail on the base commit too, and the Ubuntu 3.10 pytest job appears to be seven 30 s setup timeouts in tests/acceptance_template_import_from_params.py; I did not confirm that last cause myself. Ubuntu 3.14 and Windows pytest, ruff, mypy, pylint and pyright pass.

Not tested: any window manager other than jwm (the per-step drift is the frame's left border width, so it should be zero where that is zero), Windows, macOS, real multi-monitor setups, HiDPI beyond the 1.04 factor Xvfb reports, interactive drag resizing, translated phase labels, and the rest of the test suite beyond this module's file.

[AP-Review]

Deprecated — see below for the updated review.

Previous review (2026-10-11)

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: REQUEST CHANGES

Reviewed at head a2831431c2.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2190/3.html#prMethodicConfigurator-2190

This covers a283143, which replaces the 3da33c4 I reviewed before; nothing newer has been looked at. Both blockers from that review are fixed. I ran the real parameter editor window on this commit, on its base (66cf608) and on the previous head under Xvfb (Python 3.14.4, Tk 8.6.17), walking through the steps of the diatone_taycan_mxc 4.5.x template. Under jwm and icewm the window now stays put over eight step changes on 1366x768 and 1600x900 (the previous head, run the same way, still drifts 5 px a step), with the Skip button bottom at y=704 to 707 on a 768 px screen. A maximised window stays maximised and a window moved to the top edge stays there. The two smaller points are dealt with too: in normal mode the table refits when the phase labels wrap, and the Battery step on a 768 px screen has a 125 px table beside a 175 px scrolling component editor. tests/test_frontend_tkinter_parameter_editor.py gives 128 passed here and 99 on the base.

The scrolling component editor brings one new problem, which needs fixing first.

A rebuilt component editor is clipped to a 28 px strip when its height has not changed. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1738 only calls configure(height=...) on the inline component container when the wanted height differs from the one already configured. But ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1612 turns pack propagation back on for every rebuild, so by the time of the refit the packer has shrunk the container's requested height to the label frame plus the scrollbar's minimum, 49 px. If the new step wants the same height as the last component editor shown, the guard skips the configure, ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1737 switches propagation off, and the container is left at 49 px. Walking the template in order, 09_esc_telemetry.param and 10_battery_monitor.param after 08_telemetry.param show 28 of 87 px of content, 19_motor.param after 13_initial_atc.param (with component-less steps in between) shows 28 of 59 px, and 05_board_orientation.param revisited shows 28 of 115 px. It is the same at 1920x1080 with no window manager, at 1366x768 and 1600x900 under jwm, under icewm and at 144 dpi. The base and the previous head show the whole editor on each of those steps (104, 104, 76 and 132 px). The fields can still be reached by scrolling the strip, but on the Motors step the Poles entry is not visible at all, and nothing re-applies the height until a step with a different one comes along. Making the guard also re-apply the height when winfo_reqheight() disagrees removed every case in the same walk; that is the only remedy I tried. The new real-Tk test at tests/test_frontend_tkinter_parameter_editor.py:669 does rebuild three editors of equal height, but its assertions at tests/test_frontend_tkinter_parameter_editor.py:713 and tests/test_frontend_tkinter_parameter_editor.py:715 are lower bounds that a clipped container also meets. A test that builds two editors small enough to fit and checks the second gets the same height as the first fails on this commit (container 98 then 49 px).

One smaller point, not blocking. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1733 gives the component editor only what is left once the table has its 120 px, down to 1 px. In a window resized to 1100x500 the Battery step leaves the component container 1 px high beside a 115 px table (the previous head gave it 181 px), and on a 1024x600 screen, where the window is 516 px high, every component step is asked for 7 px. From 1280x720 up it is fine: Battery gets 127 px there and 175 px on 1366x768. This still beats the base, where the buttons are off screen at these sizes, but a small minimum for the component editor, or sharing what is left between the two, would keep both usable in a short window.

CI on this commit is red only for pytest on macOS 3.14 and the Publish Tests Results job that depends on it, and both fail on the base commit too: the macOS job dies with exit code 133 at tests/test_regenerate_app_screenshots_fully_automated.py::test_simple_parameter_editor_capture_suppresses_external_documentation on both, after passing all 128 tests of the changed file here. Ubuntu 3.10, Ubuntu 3.14 and Windows pytest, ruff, mypy, pylint and pyright pass.

Not tested: Windows and macOS at run time (the state() == "zoomed" branch is covered only by the mocked test), any full desktop environment or Wayland, real multi-monitor setups, HiDPI other than 144 dpi, interactive drag resizing, translated labels, other vehicle templates, and the rest of the test suite beyond this module's file, for which I relied on the CI jobs above.

[AP-Review]

Automated review note — AI-generated (Claude+Codex), validated against the live diff. Please sanity-check before acting.
Verdict: REQUEST CHANGES

Reviewed at head 0fc73ac8fe.
Full report: https://firmware.ardupilot.org/Tools/APReview/DevCallReviews/PRReviews/ardupilot/methodicconfigurator/2190/4.html#prMethodicConfigurator-2190

This covers 0fc73ac, which replaces the a283143 I reviewed before (rebased onto 80134be); nothing newer has been looked at. Both points from that review are fixed. I ran the real parameter editor window on this commit, on its base and on the previous head under Xvfb (Python 3.14.4, Tk 8.6.17), with no window manager and under jwm, walking steps of the diatone_taycan_mxc 4.5.x template. The clipped component editor is gone: 10_battery_monitor.param after 08_telemetry.param now gets a 112 px container showing all 83 px of content, where the previous head still gives the 49 px strip. Short windows now share the space: 68 px each for the component editor and the table on a 1024x600 screen, 58 px each in a 1100x500 window. The Skip button is on screen on every step at 1080, 768 and 600 px, and under jwm the window does not move over nine step changes. tests/test_frontend_tkinter_parameter_editor.py gives 143 passed here and 99 on the base.

There is one problem that needs fixing first, and I missed it last time: it is on the previous head too, because my earlier walk did not register the plugins.

On steps with a left-hand plugin pane the parameter table is squeezed to 2 px. Going to 07_remote_controller_controller.param or 10_battery_monitor.param leaves the plugin pane 983 px wide and the table pane 2 px wide. It is the same on 1920x1080, 1366x768 and 1024x600 screens and in a 1100x500 window, with no flight controller (the left pane then holds only the 'Plugin requires flight controller connection' label) and with a faked connection, with no window manager and under jwm, and when the step changes are driven from inside root.mainloop(). The base shows the table in every one of those runs: 552 and 433 px at the default window width, 603 and 382 with the RC calibration plugin shown. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1128 (_rebuild_plugin_layout, not in the diff) destroys parameter_area_container and builds a new one holding a fresh tk.PanedWindow. ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1753 freezes that new container the first time a refit reaches it, and the refit is also queued with after_idle at ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1696. A PanedWindow shares its width between its panes at its first layout pass, in proportion to their sizes at that moment, and does not follow their requested widths afterwards. With the refit in play that first pass now happens before the table pane has reported its size, so the split is 500 px of minsize against 1 px. A Tk-only model shows the same two outcomes: panes filled before the first layout give 552/433, a layout pass first gives 983/2 for good. I did not pin down the exact order of idle handlers in the editor that brings the pass forward. What I did confirm is that one update_idletasks() before freezing a container whose height is still unset restores 552/433 at 1080, 768 and 600 px, keeps the Skip button on screen and leaves the 143 tests passing; that is the only remedy I ran. The table can be brought back by dragging the sash at the right-hand edge, but the split collapses again each time the step is left and revisited. I did not run 09_esc_telemetry.param or 20_esc.param, which use the same layout. None of the PR's tests takes real widgets through _rebuild_plugin_layout; a test that rebuilds a left-plugin layout with a real PanedWindow and checks the table pane's width would catch this.

One smaller point, not blocking. The plugin shares the height that ardupilot_methodic_configurator/frontend_tkinter_parameter_editor.py:1754 gives the table container, and the AHRS orientation assistant on 05_board_orientation.param cannot scroll. With a faked connection it gets 164 of the 369 px it asks for on a 1366x768 screen and 120 px at 1280x720. Most of that predates this PR: the base gives it 304 of 369 px even at 1920x1080, and under jwm on a 768 px screen the same area is 199 px on the base against 164 here. Still, on plugin steps it may be worth letting the component editor give up some height before the plugin is cut. The battery monitor plugin is in its own scroll frame and stays reachable.

CI on this commit is red only for pytest on macOS 3.14 and the Publish Tests Results job that depends on it. The macOS job passes all 143 tests of the changed file and then dies with exit code 133 at tests/test_regenerate_app_screenshots_fully_automated.py::test_simple_parameter_editor_capture_suppresses_external_documentation, and so does the macOS job on base ancestor a12a847, so that is not from this change. Ubuntu 3.10, Ubuntu 3.14 and Windows pytest, ruff, mypy, pylint and pyright pass.

Not tested: Windows and macOS at run time (the state() == "zoomed" branch is covered only by the mocked test), window managers other than jwm, full desktop environments and Wayland, maximised windows on this commit, real multi-monitor setups, HiDPI, interactive drag resizing, a real flight controller or SITL, the ESC, motor test and autotune plugins, translated labels, other vehicle templates, and the rest of the test suite beyond this module's file, for which I relied on the CI jobs above.

[coveralls]

Coverage Report for CI Build 38180608229

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Warning

No base build found for commit d3871bc on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 91.562%

Details

• Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

---

Coverage Stats

Relevant Lines:	26702
Covered Lines:	24449
Line Coverage:	91.56%
Relevant Branches:	8052
Covered Branches:	6807
Branch Coverage:	84.54%
Branches in Coverage %:	No
Coverage Strength:	2.73 hits per line

---

💛 - Coveralls