-
Notifications
You must be signed in to change notification settings - Fork 44
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.
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
Add plotting to common widget pose #439
Conversation
Signed-off-by: youhy <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-gui6 #439 +/- ##
=========================================
Coverage 66.76% 66.76%
=========================================
Files 39 39
Lines 5247 5247
=========================================
Hits 3503 3503
Misses 1744 1744 Help us with your feedback. Take ten seconds to tell us how you rate us. |
Signed-off-by: youhy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some issues that need to be worked out.
This is how the plotting worked before:
Currently, it's not able to plot any new data with the same component. For example, if plotting the z
for the box and I want to add the z
for the sphere nothing happens. I can only drag in a new component but the entity is incorrect (The box entity is 8
and the cylinder entity is 12
but in the legend it's showing 8
).
Signed-off-by: youhy <[email protected]>
Signed-off-by: youhy <[email protected]>
Signed-off-by: youhy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on the bug fixes, left some comments. Also, lets rename GzPlot.qml
to GzPlotIcon.qml
for clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some investigation, I figured out what mimeData
needs to have (see comments). Let's add docs to this to help future developers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to renaming to GzPlotIcon
Signed-off-by: youhy <[email protected]>
Signed-off-by: youhy <[email protected]>
Signed-off-by: Jenn Nguyen <[email protected]>
Thank you for iterating 4b89afe I updated the optional technique to use As a quick test for the new optional plotting, you can use this PR gazebosim/gz-sim#1607 and replace this line in Pose3d.qml with: gzPlotEnabled: {
return entityType == "model"
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me!
Signed-off-by: youhy <[email protected]>
Signed-off-by: youhy [email protected]
New feature
As mentioned here gazebosim/gz-sim#1571. This PR adds plotting to the common widget pose, then we can forward-port gz-sim citadel (3) to fortress (6) without a recession of features.
Summary
Should be exactly the same as before:
Test it
Checklist
codecheck
passed (See contributing)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.