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

Adjust pose decimals based on element width #1089

Merged
merged 3 commits into from
Oct 14, 2021
Merged

Adjust pose decimals based on element width #1089

merged 3 commits into from
Oct 14, 2021

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented Oct 5, 2021

Signed-off-by: Nate Koenig nate@openrobotics.org

🦟 Bug fix

Summary

Pose values would be truncated because they didn't fit in the spin box. This PR adjusts the number of decimals based on the available width.

pose_decimals

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

Adjust pose decimals based on element width
8caa8fe 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
@nkoenig nkoenig requested a review from chapulina as a code owner October 5, 2021 17:52
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Oct 5, 2021
@chapulina chapulina added the GUI Gazebo's graphical interface (not pure Ignition GUI) label Oct 5, 2021
@codecov
Copy link

codecov bot commented Oct 5, 2021 •
edited
Loading

Codecov Report

Merging #1089 (b405876) into ign-gazebo3 (d9195c4) will increase coverage by 0.28%.
The diff coverage is 78.03%.

❗ Current head b405876 differs from pull request most recent head 0ce4e5c. Consider uploading reports for the commit 0ce4e5c to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo3    #1089      +/-   ##
===============================================
+ Coverage        77.56%   77.85%   +0.28%     
===============================================
  Files              208      221      +13     
  Lines            11385    12687    +1302     
===============================================
+ Hits              8831     9877    +1046     
- Misses            2554     2810     +256
Impacted Files Coverage Δ
include/ignition/gazebo/EntityComponentManager.hh 100.00% <ø> (ø)
include/ignition/gazebo/Link.hh 100.00% <ø> (ø)
include/ignition/gazebo/Server.hh 100.00% <ø> (ø)
...nclude/ignition/gazebo/components/ContactSensor.hh 100.00% <ø> (ø)
include/ignition/gazebo/components/Factory.hh 96.05% <ø> (ø)
...nclude/ignition/gazebo/components/LogicalCamera.hh 100.00% <ø> (ø)
include/ignition/gazebo/components/Sensor.hh 100.00% <ø> (ø)
...nclude/ignition/gazebo/components/Serialization.hh 90.32% <ø> (ø)
include/ignition/gazebo/gui/GuiSystem.hh 0.00% <0.00%> (ø)
.../plugins/component_inspector/ComponentInspector.cc 8.58% <0.00%> (-0.90%) ⬇️
... and 110 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 e9be884...0ce4e5c. Read the comment docs.

chapulina
chapulina approved these changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

I can't reproduce the original issue on my machine, because all digits fit with the inspector's minimum width. I tried changing the display resolution and still couldn't reproduce it... But it looks good to me.

My only request would be to do the same for Vector3d:

https://github.com/ignitionrobotics/ign-gazebo/blob/fc2a98ec5e1b149d773eeb6cae4a407bd82a81a2/src/gui/plugins/component_inspector/Vector3d.qml#L76

@chapulina chapulina mentioned this pull request Oct 11, 2021
Merged
7 tasks
@chapulina
Merge branch 'ign-gazebo3' into pose_decimals_adj_citadel
0fe1c89 
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Do the same for Vector3d
0ce4e5c 
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor

My only request would be to do the same for Vector3d

Done in 0ce4e5c

@chapulina chapulina merged commit 4f460fb into ign-gazebo3 Oct 14, 2021
@chapulina chapulina deleted the pose_decimals_adj_citadel branch October 14, 2021 20:24
WilliamLewww pushed a commit to WilliamLewww/ign-gazebo that referenced this pull request Dec 7, 2021
@nkoenig @chapulina @WilliamLewww
Adjust pose decimals based on element width (gazebosim#1089)
7eb2f9f 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>

Co-authored-by: Nate Koenig <nate@openrobotics.org>
Co-authored-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

Assignees
No one assigned
Labels
🏰 citadel Ignition Citadel GUI Gazebo's graphical interface (not pure Ignition GUI)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nkoenig @chapulina