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

Added MarkerManager Plugin #247

Merged
merged 14 commits into from
Jul 20, 2021
Merged

Added MarkerManager Plugin #247

merged 14 commits into from
Jul 20, 2021

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jul 1, 2021

Signed-off-by: ahcorde ahcorde@gmail.com

🎉 New feature

Summary

This PR is part of the consolidation between the Scene3D in ign-gui and ign-gazebo. This PR add a marker manager plugin

Test it

  • Compile the example marker inside the repo ign-gazebo
  • Add this lines to your gui.config file
<plugin filename="MarkerManager" name="Marker manager">
  <ignition-gui>
    <title>Marker Manager</title>
    <property type="bool" key="showTitleBar">false</property>
    <property type="string" key="state">docked</property>
    <property key="resizable" type="bool">false</property>
    <property key="width" type="double">5</property>
    <property key="height" type="double">5</property>
    <property key="state" type="string">floating</property>
    <property key="showTitleBar" type="bool">false</property>
  </ignition-gui>
</plugin>
  • launch gazebo ign gazebo --gui-config src/ign-gazebo/src/gui/gui.config -v 4
  • Run the marker example

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

@ahcorde ahcorde added the 🏯 fortress Ignition Fortress label Jul 1, 2021
@ahcorde ahcorde requested a review from chapulina July 1, 2021 12:08
@ahcorde ahcorde self-assigned this Jul 1, 2021
@ahcorde ahcorde requested a review from jennuine as a code owner July 1, 2021 12:08
Base automatically changed from chapulina/6/new_scene to main July 1, 2021 18:23
@codecov
Copy link

codecov bot commented Jul 6, 2021

Codecov Report

Merging #247 (cd052eb) into main (1190ed0) will increase coverage by 3.53%.
The diff coverage is 56.57%.

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

@@            Coverage Diff             @@
##             main     #247      +/-   ##
==========================================
+ Coverage   61.57%   65.10%   +3.53%     
==========================================
  Files          21       28       +7     
  Lines        2803     4276    +1473     
==========================================
+ Hits         1726     2784    +1058     
- Misses       1077     1492     +415     
Impacted Files Coverage Δ
include/ignition/gui/GuiEvents.hh 100.00% <ø> (ø)
src/Conversions.cc 100.00% <ø> (ø)
src/plugins/image_display/ImageDisplay.cc 30.32% <0.00%> (+7.05%) ⬆️
src/Plugin.cc 57.44% <17.64%> (-2.83%) ⬇️
src/plugins/screenshot/Screenshot.cc 33.33% <33.33%> (ø)
...s/transport_scene_manager/TransportSceneManager.cc 52.29% <52.29%> (ø)
src/plugins/minimal_scene/MinimalScene.cc 53.51% <53.51%> (ø)
src/plugins/marker_manager/MarkerManager.cc 60.60% <60.60%> (ø)
src/plugins/camera_tracking/CameraTracking.cc 68.45% <68.45%> (ø)
src/plugins/scene3d/Scene3D.cc 48.89% <71.51%> (+36.11%) ⬆️
... and 12 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 f107bd1...76bae05. 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.

Works well for me 👍

How about moving the marker.cc example from ign-gazebo here? I also think we should add this plugin to scene3d.config.

src/plugins/marker_manager/MarkerManager.hh Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Show resolved Hide resolved
src/plugins/marker_manager/MarkerManager.cc Outdated Show resolved Hide resolved
@ahcorde
Copy link
Contributor Author

ahcorde commented Jul 7, 2021

  • moved marker.cc example
  • added plugin to scene3d.config
  • Added world statistic topic to scene provider

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde force-pushed the ahcorde/6/markermanagerplugin branch from e0d4f0b to 43bd730 Compare July 12, 2021 14:07
@ahcorde
Copy link
Contributor Author

ahcorde commented Jul 12, 2021

Added MarkerManager_TEST

Signed-off-by: ahcorde <ahcorde@gmail.com>
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.

Note that the test is segfaulting

examples/standalone/marker/README.md Outdated Show resolved Hide resolved
examples/standalone/marker/README.md Outdated Show resolved Hide resolved
examples/standalone/scene_provider/scene_provider.cc Outdated Show resolved Hide resolved
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
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.

🚀

@chapulina chapulina merged commit 973cbcc into main Jul 20, 2021
@chapulina chapulina deleted the ahcorde/6/markermanagerplugin branch July 20, 2021 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants