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

🌐 Spherical coordinates #1008

Merged
merged 13 commits into from
Sep 22, 2021
Merged

🌐 Spherical coordinates #1008

merged 13 commits into from
Sep 22, 2021

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Sep 4, 2021

🎉 New feature

Closes #981

Requires

Summary

Add support for spherical coordinates (latitude / longitude). Users are able to set the lat / lon for the world origin, and query the lat / lon for other entities with respect to that.

  • Set from SDF, see example world
  • See on component inspector
  • Plot world origin coordinates
  • Change world origin coordinates from plugins
  • Change world origin coordinates from a transport service

spherical_coors

TODO

  • Tutorial

Under consideration:

  • Support spawning entities at a given spherical location through the create service
  • Move entities to a given spherical coordinate through the set pose service

I'll open this PR for review when the above have been sorted out.

Test it

Try changing it through the SDF, the component inspector, and the service.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • 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

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina added close the gap Features from Gazebo-classic needs upstream release Blocked by a release of an upstream library labels Sep 4, 2021
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Sep 4, 2021
@chapulina chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2021
@chapulina chapulina marked this pull request as ready for review September 15, 2021 04:21
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Sep 17, 2021
@codecov
Copy link

codecov bot commented Sep 18, 2021

Codecov Report

Merging #1008 (1fa8bae) into main (4be711c) will decrease coverage by 0.09%.
The diff coverage is 62.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1008      +/-   ##
==========================================
- Coverage   63.47%   63.37%   -0.10%     
==========================================
  Files         241      242       +1     
  Lines       19577    19715     +138     
==========================================
+ Hits        12426    12495      +69     
- Misses       7151     7220      +69     
Impacted Files Coverage Δ
include/ignition/gazebo/Util.hh 100.00% <ø> (ø)
include/ignition/gazebo/World.hh 100.00% <ø> (ø)
.../plugins/component_inspector/ComponentInspector.cc 6.22% <0.00%> (-0.52%) ⬇️
.../plugins/component_inspector/ComponentInspector.hh 28.57% <ø> (ø)
src/SdfEntityCreator.cc 84.14% <33.33%> (-0.40%) ⬇️
...nclude/ignition/gazebo/components/Serialization.hh 94.11% <60.00%> (-5.89%) ⬇️
src/Util.cc 93.10% <80.00%> (-0.60%) ⬇️
src/systems/user_commands/UserCommands.cc 60.88% <81.05%> (+2.93%) ⬆️
...ignition/gazebo/components/SphericalCoordinates.hh 100.00% <100.00%> (ø)
src/LevelManager.cc 89.14% <100.00%> (+0.11%) ⬆️
... and 5 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 4be711c...1fa8bae. Read the comment docs.

@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Sep 18, 2021
Signed-off-by: Louise Poubel <[email protected]>
Copy link
Contributor

@caguero caguero left a comment

Choose a reason for hiding this comment

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

Everything worked as expected. I just found a tiny typo in the tutorial. I have just one comment:

Let's say I follow the tutorial and spawn an entity. If I now set the world origin's spherical coordinates with the GUI or the service to a different value, shouldn't all entities be moved as a side effect?

tutorials/spherical_coordinates.md Outdated Show resolved Hide resolved
tutorials/spherical_coordinates.md Show resolved Hide resolved
Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

Let's say I follow the tutorial and spawn an entity. If I now set the world origin's spherical coordinates with the GUI or the service to a different value, shouldn't all entities be moved as a side effect?

I added a note about this to the tutorial in 76495a7. It would be tough to move the entire world, and I'm not sure this is something that the user may always want. I think in the future, if there's need for it, we could add an option that enables this. For now, changing the world origin only affects subsequent commands, like spawns and moves.

@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Sep 22, 2021
@chapulina chapulina merged commit 2b141ac into main Sep 22, 2021
@chapulina chapulina deleted the chapulina/6/spherical_coords branch September 22, 2021 05:29
WilliamLewww pushed a commit to WilliamLewww/ign-gazebo that referenced this pull request Dec 7, 2021
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: William Lew <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection close the gap Features from Gazebo-classic 🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants