Skip to content

Same timer and on-demand popup for all OS#1533

Closed
OmkarSarkar204 wants to merge 5 commits into
ArduPilot:masterfrom
OmkarSarkar204:parameter-builder
Closed

Same timer and on-demand popup for all OS#1533
OmkarSarkar204 wants to merge 5 commits into
ArduPilot:masterfrom
OmkarSarkar204:parameter-builder

Conversation

@OmkarSarkar204
Copy link
Copy Markdown
Contributor

@OmkarSarkar204 OmkarSarkar204 commented Apr 20, 2026 •
edited
Loading

Description

Fix: #1520

Parameter ediotr builder is slow on windows and linux, applying the same logic for popups as on MacOS to the other OS's

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

WIP

I want to test this on all the three OS's to see any edge cases.

Copilot AI review requested due to automatic review settings April 20, 2026 05:39
@OmkarSarkar204 OmkarSarkar204 marked this pull request as draft April 20, 2026 05:40
Copilot AI reviewed Apr 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR makes tooltip popups use the same macOS-style “on-demand creation with a timer” behavior across Windows/Linux to avoid slow UI construction.

Changes:

  • Removed OS-specific tooltip binding logic and applied delayed tooltip creation universally.
  • Stopped pre-creating tooltips on non-macOS during initialization.
  • Standardized tooltip teardown behavior to destroy the tooltip on hide/leave.

Comment thread ardupilot_methodic_configurator/frontend_tkinter_show.py
Comment thread ardupilot_methodic_configurator/frontend_tkinter_show.py
Comment thread ardupilot_methodic_configurator/frontend_tkinter_show.py Outdated
@OmkarSarkar204 OmkarSarkar204 force-pushed the parameter-builder branch 3 times, most recently from cf6d013 to 37c4503 Compare April 22, 2026 06:34
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 22, 2026 •
edited
Loading

Coverage Report for CI Build 24790957227

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 →

Coverage increased (+0.8%) to 95.782%

Details

  • Coverage increased (+0.8%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats

Coverage Status
Relevant Lines: 13111
Covered Lines: 12558
Line Coverage: 95.78%
Relevant Branches: 2223
Covered Branches: 2141
Branch Coverage: 96.31%
Branches in Coverage %: No
Coverage Strength: 3.54 hits per line
💛 - Coveralls

@OmkarSarkar204
Copy link
Copy Markdown
Contributor Author

OmkarSarkar204 commented Apr 22, 2026 •
edited
Loading

@amilcarlucas ,
I found that the tooltips were not the main problem of the slow load. The true issue is positively the sheer volume of parameters being loaded into the Tkinter UI at the exact same time. The data itself loads very fast, but the Tkinter rendering completely chokes when trying to instantiate and .grid() hundreds of complex row widgets (Comboboxes, Entries, Labels) simultaneously.

I used a scroll method to load a small amount of parameters. The scroll is currently the most efficient method i have explored and tested to make it fast till now

Though i think its a bypass and will have a few bugs itself, but works perfectly for the time being, will test other methods also but it is much faster than the original way.

Working perfectly on Windows and Mac, Linux testing and updating tests are still left , ill update it shortly

Please let me know if anything can be improved.

@OmkarSarkar204 OmkarSarkar204 marked this pull request as ready for review April 24, 2026 15:14
@OmkarSarkar204
Copy link
Copy Markdown
Contributor Author

OmkarSarkar204 commented Apr 28, 2026

@amilcarlucas , any changes needed in this?

@amilcarlucas amilcarlucas requested a review from Copilot April 28, 2026 08:01
@amilcarlucas
Copy link
Copy Markdown
Collaborator

amilcarlucas commented Apr 28, 2026

Thanks, it looks good, I will need some time to review and I want to benchmark it a bit.

In the future I plan to use the new python 3.15 unintrusive profiler to find bottlenecks without requiring code instrumentation.

Copilot AI reviewed Apr 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread ardupilot_methodic_configurator/frontend_tkinter_show.py Outdated
Comment thread tests/bdd_frontend_tkinter_show.py Outdated
Comment thread tests/bdd_frontend_tkinter_show.py Outdated
Comment thread ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py Outdated
Comment thread ardupilot_methodic_configurator/frontend_tkinter_parameter_editor_table.py Outdated
Comment thread ardupilot_methodic_configurator/frontend_tkinter_scroll_frame.py Outdated
@amilcarlucas
Copy link
Copy Markdown
Collaborator

amilcarlucas commented Apr 28, 2026

Using copilot in VS code you can add this github pull request as reference and tell it to validate the review comments and address the valid ones.

@OmkarSarkar204
Copy link
Copy Markdown
Contributor Author

OmkarSarkar204 commented Apr 28, 2026

Thanks, I have never used copilot in VS but ill try it

@OmkarSarkar204 @amilcarlucas
fix: timer and on-demand load along with fast loading on large parame…
71c110c 
…ter table

Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
@OmkarSarkar204 @amilcarlucas
fix: cleanup and scroll noatt fixed
a1bb4d9 
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
@OmkarSarkar204 @amilcarlucas
fix: MacOs tooltip destroy
eb4b640 
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
@OmkarSarkar204 @amilcarlucas
fix: updated test and pylint issue
d2e9c58 
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
@amilcarlucas
Copy link
Copy Markdown
Collaborator

amilcarlucas commented Apr 28, 2026

I rebased it, force pushed it and tested it. It works great!!!!
Good work

@OmkarSarkar204
Copy link
Copy Markdown
Contributor Author

OmkarSarkar204 commented Apr 28, 2026

Thank You so much.

@OmkarSarkar204
Copy link
Copy Markdown
Contributor Author

OmkarSarkar204 commented Apr 28, 2026 •
edited
Loading

Adressed copilot review.
Tested Mac, Linux

@OmkarSarkar204
fix: adressed copilot
ef4326f 
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
@amilcarlucas
Copy link
Copy Markdown
Collaborator

amilcarlucas commented Apr 28, 2026

I simplified this massively in #1572 and added the rest to #1573. I did an extra second commit on #1573 on top of your original changes

@OmkarSarkar204
Copy link
Copy Markdown
Contributor Author

OmkarSarkar204 commented Apr 29, 2026 •
edited
Loading

@amilcarlucas , Glad this worked well and also thank you very much for the appreciation on discord, is it ok to close this one.

@amilcarlucas
Copy link
Copy Markdown
Collaborator

amilcarlucas commented Apr 29, 2026

Yes, we'll continue this on #1573

I can only test it on linux and windows. Can you test it on macOS?

@OmkarSarkar204
Copy link
Copy Markdown
Contributor Author

OmkarSarkar204 commented Apr 29, 2026

Yes, we'll continue this on #1573

I can only test it on linux and windows. Can you test it on macOS?

Sure ill do it and let you know :)

@amilcarlucas amilcarlucas mentioned this pull request Apr 30, 2026
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@amilcarlucas amilcarlucas Awaiting requested review from amilcarlucas amilcarlucas is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Parameter editor is slow to build when there are many parameters in the current step

4 participants

@OmkarSarkar204 @coveralls @amilcarlucas