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.

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

use uint64_t for ComponentInspector Entity IDs #1144

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

adlarkin
Copy link
Contributor

@adlarkin adlarkin commented Oct 26, 2021

Signed-off-by: Ashton Larkin [email protected]

🦟 Bug fix

Summary

On the server, an Entity ID is defined as a uint64_t. This PR updates the component inspector to use the same type for entity IDs in the GUI plugin.

This should help with some of the issues seen in #1138.

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

@adlarkin adlarkin requested a review from iche033 October 26, 2021 21:49
@adlarkin adlarkin requested a review from chapulina as a code owner October 26, 2021 21:49
@github-actions github-actions bot added 🌱 garden Ignition Garden 🏯 fortress Ignition Fortress labels Oct 26, 2021
@adlarkin adlarkin changed the title use uint64_t for Entity IDs use uint64_t for ComponentInspector Entity IDs Oct 26, 2021
@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #1144 (41f4732) into ign-gazebo6 (a42cac4) will decrease coverage by 13.97%.
The diff coverage is 93.47%.

❗ Current head 41f4732 differs from pull request most recent head 36081cd. Consider uploading reports for the commit 36081cd to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           ign-gazebo6    #1144       +/-   ##
================================================
- Coverage        77.70%   63.73%   -13.98%     
================================================
  Files              222      256       +34     
  Lines            12716    20117     +7401     
================================================
+ Hits              9881    12821     +2940     
- Misses            2835     7296     +4461     
Impacted Files Coverage Δ
include/ignition/gazebo/EntityComponentManager.hh 100.00% <ø> (ø)
include/ignition/gazebo/Model.hh 100.00% <ø> (ø)
include/ignition/gazebo/SdfEntityCreator.hh 100.00% <ø> (ø)
include/ignition/gazebo/ServerConfig.hh 100.00% <ø> (ø)
include/ignition/gazebo/Util.hh 100.00% <ø> (ø)
include/ignition/gazebo/World.hh 100.00% <ø> (ø)
...nclude/ignition/gazebo/components/ChildLinkName.hh 100.00% <ø> (ø)
include/ignition/gazebo/components/Name.hh 100.00% <ø> (ø)
...clude/ignition/gazebo/components/ParentLinkName.hh 100.00% <ø> (ø)
...de/ignition/gazebo/components/PerformerAffinity.hh 100.00% <ø> (ø)
... and 167 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 a42cac4...36081cd. Read the comment docs.

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.

LGTM, this could be targeted at Citadel and merged forward.

Signed-off-by: Ashton Larkin <[email protected]>
@adlarkin adlarkin force-pushed the adlarkin/compInspector_64bit_ID branch from 41f4732 to 36081cd Compare October 27, 2021 18:52
@adlarkin adlarkin changed the base branch from ign-gazebo6 to ign-gazebo3 October 27, 2021 18:53
@adlarkin
Copy link
Contributor Author

LGTM, this could be targeted at Citadel and merged forward.

I have retargeted to Citadel and will merge if/when CI comes back green.

@adlarkin adlarkin added 🏰 citadel Ignition Citadel and removed 🏯 fortress Ignition Fortress 🌱 garden Ignition Garden labels Oct 27, 2021
@adlarkin adlarkin merged commit 63decda into ign-gazebo3 Oct 27, 2021
@adlarkin adlarkin deleted the adlarkin/compInspector_64bit_ID branch October 27, 2021 19:49
srmainwaring pushed a commit to srmainwaring/gz-sim that referenced this pull request Nov 11, 2021
WilliamLewww pushed a commit to WilliamLewww/ign-gazebo that referenced this pull request Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants