Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add QML Debugging support #1503

Merged
merged 2 commits into from
Jun 20, 2022
Merged

Add QML Debugging support #1503

merged 2 commits into from
Jun 20, 2022

Conversation

darksylinc
Copy link
Contributor

The project must be rebuilt defining the QT_QML_DEBUG macro.

Once that's done, loading the GUI will wait until the debugger is
attached to port 40000.

QtCreator does this automatically when setting:
Projects -> Build -> QML debugging and profiling -> Enable

Tested with QtCreator (Qt Quick plugin enabled) and working.

Signed-off-by: Matias N. Goldberg dark_sylinc@yahoo.com.ar

🦟 Bug fix

No ticket was created for this issue.

Summary

Troubleshooting QML errors in Javascript code is a PITA without proper tools.
This PR addresses that.

Usually QML debugging is handled automatically by appending the right command line entry.

However IGN is special because it's a so loaded by a ruby script; hence all command line arguments added by QtCreator are lost (and may also result in failure to start).

A quick solution is to hardcode those commands when compiling with QT_QML_DEBUG.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@darksylinc
Add QML Debugging support
ecec91c 
The project must be rebuilt defining the QT_QML_DEBUG macro.

Once that's done, loading the GUI will wait until the debugger is
attached to port 40000.

QtCreator does this automatically when setting:
  Projects -> Build -> QML debugging and profiling -> Enable

Tested with QtCreator (Qt Quick plugin enabled) and working.

Signed-off-by: Matias N. Goldberg <dark_sylinc@yahoo.com.ar>
@darksylinc darksylinc requested a review from chapulina as a code owner May 20, 2022 23:11
@codecov
Copy link

codecov bot commented May 21, 2022 •
edited
Loading

Codecov Report

Merging #1503 (ecec91c) into main (a3cbd8c) will decrease coverage by 28.61%.
The diff coverage is n/a.

❗ Current head ecec91c differs from pull request most recent head c9140d9. Consider uploading reports for the commit c9140d9 to get more accurate results

@@             Coverage Diff             @@
##             main    #1503       +/-   ##
===========================================
- Coverage   63.62%   35.01%   -28.62%     
===========================================
  Files         330       44      -286     
  Lines       25787     2356    -23431     
===========================================
- Hits        16408      825    -15583     
+ Misses       9379     1531     -7848
Impacted Files Coverage Δ
src/ign.cc
include/gz/sim/components/LevelBuffer.hh
src/systems/battery_plugin/LinearBatteryPlugin.cc
src/network/NetworkManagerPrimary.cc
src/Barrier.cc
include/gz/sim/components/SemanticLabel.hh
src/systems/elevator/utils/DoorTimer.hh
include/gz/sim/components/VisualCmd.hh
src/gui/plugins/copy_paste/CopyPaste.cc
include/gz/sim/components/JointType.hh
... and 364 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9993ea...c9140d9. Read the comment docs.

@chapulina chapulina added the 🌱 garden Ignition Garden label May 23, 2022
@chapulina chapulina added the GUI Gazebo's graphical interface (not pure Ignition GUI) label May 23, 2022
@chapulina chapulina requested a review from mayman99 May 23, 2022 18:27
@mayman99
Copy link
Contributor

Thanks for the PR!
I've a question, would it possible to expose the debug option thru the starting ruby script?

mayman99
mayman99 requested changes May 31, 2022
View reviewed changes
Copy link
Contributor

@mayman99 mayman99 left a comment

Choose a reason for hiding this comment

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

I think exposing this feature thru a tutorial or an example would be a better idea for usability, because if merged like this the only way users would know about the feature is by reading the source code.

@darksylinc
Copy link
Contributor Author

I've a question, would it possible to expose the debug option thru the starting ruby script?

I agree but I am not familiar with the Ruby script. This was a quick and dirty solution that works.

The Ruby script rejects cmd arguments it doesn't expect instead of passing them to the application

tutorial or an example would be a better idea for usability,

Updating docs sounds ok. There's no much to it: rebuild gazebo with -DQT_QML_DEBUG flag passed to colcon. (Note: this flag is a security risk because it exposes a debug port and hence out of the box gazebo should not be built with that flag)

When launching the gui, stdio will print it's waiting for the debugger to connect to port 40000.
From QtCreator, Debug-> Start Debugging -> Attach to QML

@darksylinc darksylinc mentioned this pull request May 31, 2022
Merged
8 tasks
@darksylinc
Copy link
Contributor Author

OK I added gazebosim/docs#251 as documentation that should accompany this new feature.

Is it satisfactory?

@mayman99
Copy link
Contributor

mayman99 commented Jun 3, 2022

Thank you, the doc looks clean, I plan to install QT creator this weekend and test it 👍

mayman99
mayman99 approved these changes Jun 6, 2022
View reviewed changes
Copy link
Contributor

@mayman99 mayman99 left a comment

Choose a reason for hiding this comment

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

lgtm!
thanks for the PR!

@chapulina
merged from main
c9140d9 
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina merged commit f5f753e into gazebosim:main Jun 20, 2022
@j-rivero j-rivero mentioned this pull request Sep 16, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mayman99 mayman99 mayman99 approved these changes

@chapulina chapulina Awaiting requested review from chapulina

Assignees
No one assigned
Labels
🌱 garden Ignition Garden GUI Gazebo's graphical interface (not pure Ignition GUI)
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@darksylinc @mayman99 @chapulina