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

Enable GzWeb visualization of markers by republishing service requests on a topic #1994

Merged
merged 5 commits into from
May 23, 2023

Conversation

nkoenig
Copy link
Contributor

@nkoenig nkoenig commented May 18, 2023

🎉 New feature

The MarkerManager utilizes services for updating visual markers. GzWeb can't listen into services, and instead requires topics. This PR enables GzWeb visualization of markers by republishing service requests on a topic.

Test it

An integration test has been added.

Checklist

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

Signed-off-by: Nate Koenig <natekoenig@gmail.com>
Signed-off-by: Nate Koenig <natekoenig@gmail.com>
Signed-off-by: Nate Koenig <natekoenig@gmail.com>
Signed-off-by: Nate Koenig <natekoenig@gmail.com>
Signed-off-by: Nate Koenig <natekoenig@gmail.com>
@osrf-triage osrf-triage added this to Inbox in Core development May 18, 2023
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label May 18, 2023
Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

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

Looks good to me with green CI, one comment.

include/gz/sim/rendering/MarkerManager.hh Show resolved Hide resolved
Core development automation moved this from Inbox to In review May 18, 2023
@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #1994 (3ddbbde) into ign-gazebo3 (2673c01) will decrease coverage by 7.30%.
The diff coverage is 0.00%.

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

@@               Coverage Diff               @@
##           ign-gazebo3    #1994      +/-   ##
===============================================
- Coverage        78.04%   70.75%   -7.30%     
===============================================
  Files              255      262       +7     
  Lines            15080    16759    +1679     
===============================================
+ Hits             11769    11857      +88     
- Misses            3311     4902    +1591     
Impacted Files Coverage Δ
include/gz/sim/gui/TmpIface.hh 0.00% <ø> (ø)
include/gz/sim/rendering/MarkerManager.hh 100.00% <ø> (ø)
src/EntityComponentManager.cc 89.09% <ø> (ø)
src/Link.cc 93.54% <ø> (ø)
src/Model.cc 95.83% <ø> (ø)
src/SdfEntityCreator.cc 90.44% <ø> (ø)
src/ServerConfig.cc 90.28% <ø> (ø)
src/SystemLoader.cc 77.04% <ø> (-4.92%) ⬇️
src/TestFixture.cc 100.00% <ø> (ø)
src/World.cc 96.87% <ø> (ø)
... and 37 more

... and 2 files with indirect coverage changes

@azeey azeey changed the title Nkoenig/marker topic Enable GzWeb visualization of markers by republishing service requests on a topic May 18, 2023
@nkoenig
Copy link
Contributor Author

nkoenig commented May 22, 2023

The ABI checker is unhappy because I made the MarkerManager have the correct visibility. This okay because it resolves an error case.

@nkoenig nkoenig merged commit 3dd77a2 into ign-gazebo3 May 23, 2023
6 of 9 checks passed
Core development automation moved this from In review to Done May 23, 2023
@nkoenig nkoenig deleted the nkoenig/marker_topic branch May 23, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants