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

ign -> gz Migrate Ignition Headers : gz-rendering #705

Merged
merged 10 commits into from
Nov 22, 2022
Merged

ign -> gz Migrate Ignition Headers : gz-rendering #705

merged 10 commits into from
Nov 22, 2022

Conversation

methylDragon
Copy link
Contributor

@methylDragon methylDragon requested a review from iche033 as a code owner August 12, 2022 22:37
@methylDragon methylDragon mentioned this pull request Aug 12, 2022
Open
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Aug 12, 2022
@codecov
Copy link

codecov bot commented Aug 12, 2022 •
edited
Loading

Codecov Report

Merging #705 (c65a4cf) into ign-rendering3 (10627f6) will not change coverage.
The diff coverage is 74.42%.

@@               Coverage Diff               @@
##           ign-rendering3     #705   +/-   ##
===============================================
  Coverage           83.40%   83.40%           
===============================================
  Files                 100      100           
  Lines                7816     7816           
===============================================
  Hits                 6519     6519           
  Misses               1297     1297
Impacted Files Coverage Δ
include/gz/rendering/GizmoVisual.hh 0.00% <0.00%> (ø)
include/gz/rendering/GpuRays.hh 0.00% <0.00%> (ø)
include/gz/rendering/Scene.hh 0.00% <0.00%> (ø)
include/gz/rendering/Text.hh 0.00% <0.00%> (ø)
include/gz/rendering/base/BaseScene.hh 0.00% <0.00%> (ø)
include/gz/rendering/base/BaseText.hh 0.00% <0.00%> (ø)
include/gz/rendering/Storage.hh 25.00% <25.00%> (ø)
include/gz/rendering/base/BaseDepthCamera.hh 33.33% <33.33%> (ø)
include/gz/rendering/base/BaseRayQuery.hh 36.84% <36.84%> (ø)
include/gz/rendering/base/BaseMaterial.hh 64.16% <64.16%> (ø)
... and 76 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chapulina chapulina added the ign to gz Renaming Ignition to Gazebo. label Aug 15, 2022
@methylDragon methylDragon force-pushed the citadel_header_redirects branch from c411fbb to 1bb6749 Compare August 18, 2022 22:25
@methylDragon
Migrate headers
9ef2da1 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the citadel_header_redirects branch 6 times, most recently from 2c533ac to 43b5e40 Compare August 19, 2022 03:00
@methylDragon
Add redirection headers
dc50423 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the citadel_header_redirects branch 2 times, most recently from d7e384d to 1c4ca3e Compare August 19, 2022 03:47
@methylDragon
Migrate include statements
a8b31e9 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the citadel_header_redirects branch from 1c4ca3e to a8b31e9 Compare August 19, 2022 19:26
@methylDragon methylDragon changed the title ign -> gz Reverse Ignition Headers : gz-rendering ign -> gz Migrate Ignition Headers : gz-rendering Aug 22, 2022
@methylDragon methylDragon force-pushed the citadel_header_redirects branch from c71704f to 5f89991 Compare August 30, 2022 20:54
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Sep 3, 2022
chapulina
chapulina reviewed Sep 3, 2022
View reviewed changes
examples/gazebo_scene_viewer/CameraWindow.hh Outdated Show resolved Hide resolved
examples/gazebo_scene_viewer/GazeboDemo.cc
using namespace rendering;

void Connect()
{
gazebo::common::Console::SetQuiet(false);
gazebo::transport::init();
gazebo::transport::run();
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this is actually Gazebo classic

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I think these lines should not be removed. This demo shows integration with gazebo classic so the old gazebo-classic calls are expected

examples/gazebo_scene_viewer/GazeboWorldDemo.cc Outdated
using namespace rendering;

void Connect()
{
common::Console::SetVerbosity(4);
gazebo::transport::init();
gazebo::transport::run();
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too Gazebo classic

Copy link
Contributor

Choose a reason for hiding this comment

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

same here, we should keep these calls

examples/gazebo_scene_viewer/SceneManagerPrivate.hh Outdated Show resolved Hide resolved
@methylDragon
Leave ignition as primary in headers to fix ABI
655698d 
Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon methylDragon force-pushed the citadel_header_redirects branch from 5f89991 to 655698d Compare September 5, 2022 23:25
Nate Koenig added 3 commits November 15, 2022 12:00
Fixed build
ec5c869 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
Fix test
fd9f69c 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
Removed some gazeb classic code
0154c59 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
@nkoenig
Copy link
Contributor

nkoenig commented Nov 15, 2022

@iche033 , I think this is ready.

iche033
iche033 reviewed Nov 15, 2022
View reviewed changes
examples/gazebo_scene_viewer/GazeboDemo.cc
using namespace rendering;

void Connect()
{
gazebo::common::Console::SetQuiet(false);
gazebo::transport::init();
gazebo::transport::run();
Copy link
Contributor

Choose a reason for hiding this comment

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

oh I think these lines should not be removed. This demo shows integration with gazebo classic so the old gazebo-classic calls are expected

examples/gazebo_scene_viewer/SceneManagerPrivate.hh Outdated
@@ -114,27 +114,27 @@ namespace ignition

private: NewSceneManager *newSceneManager;

private: gazebo::transport::NodePtr transportNode;
private: sim::transport::NodePtr transportNode;
Copy link
Contributor

Choose a reason for hiding this comment

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

these should stay gazebo as this demo uses gazebo-classic transport to get its scene info and render the scene using gz-rendering

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh. I reverted these changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still see sim::, maybe the changes were not pushed?

Copy link
Contributor

Choose a reason for hiding this comment

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

more sim:: to gazebo:: changes in df38c4e

examples/gazebo_scene_viewer/GazeboWorldDemo.cc Outdated
using namespace rendering;

void Connect()
{
common::Console::SetVerbosity(4);
gazebo::transport::init();
gazebo::transport::run();
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, we should keep these calls

Nate Koenig and others added 3 commits November 15, 2022 15:50
Revert changes
69ce1c3 
Signed-off-by: Nate Koenig <nate@openrobotics.org>
@iche033
revert to gazebo in gazebo scene viewer
df38c4e 
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033
fix build errors
c65a4cf 
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@iche033
Copy link
Contributor

iche033 commented Nov 17, 2022

added more changes in c65a4cf to fix the gazebo_scene_viewer demo

@nkoenig nkoenig merged commit c6798a0 into ign-rendering3 Nov 22, 2022
@nkoenig nkoenig deleted the citadel_header_redirects branch November 22, 2022 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina left review comments

@nkoenig nkoenig nkoenig approved these changes

@iche033 iche033 iche033 approved these changes

Assignees
No one assigned
Labels
🏰 citadel Ignition Citadel ign to gz Renaming Ignition to Gazebo. needs upstream release Blocked by a release of an upstream library
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@methylDragon @nkoenig @iche033 @chapulina