3 to 6 05-02-2023 #1989

nkoenig · 2023-05-11T13:12:50Z

➡️ Forward port

Port ign-gazebo3 to ign-gazbeo6

Branch comparison: ign-gazebo6...ign-gazebo3

Note to maintainers: Remember to Merge with commit (not squash-merge or rebase)

* Migrate headers Signed-off-by: methylDragon <methylDragon@gmail.com> * Add redirection headers Signed-off-by: methylDragon <methylDragon@gmail.com> * Migrate include statements Signed-off-by: methylDragon <methylDragon@gmail.com> * Leave ignition as primary in headers to fix ABI Signed-off-by: methylDragon <methylDragon@gmail.com> * Generate MOC files from gz headers Signed-off-by: Louise Poubel <louise@openrobotics.org> * Migrate msgs include usage Signed-off-by: methylDragon <methylDragon@gmail.com> * Add plugin aliases Signed-off-by: methylDragon <methylDragon@gmail.com> * Fix tests and code check Signed-off-by: Nate Koenig <nate@openrobotics.org> * Added fuel.gazebosim.org test dir Signed-off-by: Nate Koenig <nate@openrobotics.org> * Fix SdfGenerator_TEST Signed-off-by: Nate Koenig <nate@openrobotics.org> * require rendering 3.7 Signed-off-by: Nate Koenig <nate@openrobotics.org> * Fix namespaces Signed-off-by: Nate Koenig <nate@openrobotics.org> * Clarify messages Signed-off-by: Nate Koenig <natekoenig@gmail.com> * More debugging Signed-off-by: Nate Koenig <natekoenig@gmail.com> * Fix linter Signed-off-by: Nate Koenig <natekoenig@gmail.com> * More testing Signed-off-by: Nate Koenig <natekoenig@gmail.com> * More debug Signed-off-by: Nate Koenig <natekoenig@gmail.com> * fix build Signed-off-by: Nate Koenig <natekoenig@gmail.com> * More debug Signed-off-by: Nate Koenig <natekoenig@gmail.com> * linter Signed-off-by: Nate Koenig <natekoenig@gmail.com> * Fix build Signed-off-by: Nate Koenig <natekoenig@gmail.com> * Fix build Signed-off-by: Nate Koenig <natekoenig@gmail.com> * More debug Signed-off-by: Nate Koenig <natekoenig@gmail.com> * More testing Signed-off-by: Nate Koenig <natekoenig@gmail.com> * More tests Signed-off-by: Nate Koenig <natekoenig@gmail.com> * Fix gui.config Signed-off-by: Nate Koenig <natekoenig@gmail.com> * Remove debugging Signed-off-by: Nate Koenig <natekoenig@gmail.com> --------- Signed-off-by: methylDragon <methylDragon@gmail.com> Signed-off-by: Louise Poubel <louise@openrobotics.org> Signed-off-by: Nate Koenig <nate@openrobotics.org> Signed-off-by: Nate Koenig <natekoenig@gmail.com> Co-authored-by: Louise Poubel <louise@openrobotics.org> Co-authored-by: Nate Koenig <nate@openrobotics.org> Co-authored-by: Nate Koenig <natekoenig@gmail.com>

Not all of the needed include paths are exported with the gz-sim target, so including the gz/sim.hh header doesn't work easily. This test fails to build and illustrates the problem. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Needed since the auto-generated header is gz/gazebo.hh Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Fixes macOS build. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

nkoenig · 2023-05-11T16:03:33Z

This needs a release of gz-gui.

nkoenig · 2023-05-12T18:57:07Z

@osrf-jenkins run tests

codecov · 2023-05-12T20:28:44Z

Codecov Report

Merging #1989 (661e18c) into ign-gazebo6 (1a18071) will increase coverage by 0.08%.
The diff coverage is 93.05%.

❗ Current head 661e18c differs from pull request most recent head 2390595. Consider uploading reports for the commit 2390595 to get more accurate results

@@               Coverage Diff               @@
##           ign-gazebo6    #1989      +/-   ##
===============================================
+ Coverage        65.29%   65.38%   +0.08%     
===============================================
  Files              327      327              
  Lines            26893    26932      +39     
===============================================
+ Hits             17560    17609      +49     
+ Misses            9333     9323      -10

Impacted Files	Coverage Δ
include/gz/sim/detail/BaseView.hh	`100.00% <ø> (ø)`
include/gz/sim/detail/EntityComponentManager.hh	`93.86% <ø> (ø)`
include/gz/sim/detail/View.hh	`100.00% <ø> (ø)`
include/gz/sim/gui/GuiEvents.hh	`0.00% <0.00%> (ø)`
include/gz/sim/gui/GuiSystem.hh	`0.00% <0.00%> (ø)`
include/gz/sim/components/Visual.hh	`12.50% <12.50%> (ø)`
include/gz/sim/EventManager.hh	`79.16% <79.16%> (ø)`
include/gz/sim/components/Model.hh	`93.10% <93.10%> (ø)`
include/gz/sim/components/Serialization.hh	`95.55% <95.55%> (ø)`
include/gz/sim/components/Factory.hh	`98.03% <98.03%> (ø)`
... and 118 more

... and 173 files with indirect coverage changes

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

azeey

Overall, the changes look good to me. I did a merge myself and compared mine with yours and most differences were pretty minor. I've left comments in some of the places I found ignition headers, but not everywhere. I wasn't sure if they were missed or you were intentionally leaving them out for a follow-up PR.

We also need redirection headers in include/ignition/gazebo for the all the new headers in ign-gazebo6:

include/gz/sim/components/BatteryPowerLoad.hh
include/gz/sim/components/BoundingBoxCamera.hh
include/gz/sim/components/CustomSensor.hh
include/gz/sim/components/ForceTorque.hh
include/gz/sim/components/HaltMotion.hh
include/gz/sim/components/JointTransmittedWrench.hh
include/gz/sim/components/LightCmd.hh
include/gz/sim/components/LightType.hh
include/gz/sim/components/NavSat.hh
include/gz/sim/components/ParticleEmitter.hh
include/gz/sim/components/Recreate.hh
include/gz/sim/components/RenderEngineServerHeadless.hh
include/gz/sim/components/SegmentationCamera.hh
include/gz/sim/components/SemanticLabel.hh
include/gz/sim/components/SphericalCoordinates.hh
include/gz/sim/components/SystemPluginInfo.hh
include/gz/sim/components/TemperatureRange.hh
include/gz/sim/components/Visibility.hh
include/gz/sim/components/VisualCmd.hh
include/gz/sim/components/WheelSlipCmd.hh
include/gz/sim/comms/Broker.hh
include/gz/sim/comms/ICommsModel.hh
include/gz/sim/comms/MsgManager.hh
include/gz/sim/Actor.hh
include/gz/sim/Joint.hh
include/gz/sim/Light.hh
include/gz/sim/Primitives.hh
include/gz/sim/Sensor.hh

include/gz/sim/comms/Broker.hh

include/gz/sim/components/CMakeLists.txt

include/gz/sim/components/BatteryPowerLoad.hh

include/gz/sim/components/CanonicalLink.hh

include/gz/sim/components/CustomSensor.hh

src/gui/plugins/scene3d/Scene3D.cc

src/network/PeerTracker_TEST.cc

src/systems/detachable_joint/DetachableJoint.cc

src/systems/log/LogPlayback.cc

src/EntityComponentManager.cc

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

nkoenig · 2023-05-25T12:08:07Z

Overall, the changes look good to me. I did a merge myself and compared mine with yours and most differences were pretty minor. I've left comments in some of the places I found ignition headers, but not everywhere. I wasn't sure if they were missed or you were intentionally leaving them out for a follow-up PR.

We also need redirection headers in include/ignition/gazebo for the all the new headers in ign-gazebo6:

include/gz/sim/components/BatteryPowerLoad.hh

include/gz/sim/components/BoundingBoxCamera.hh

include/gz/sim/components/CustomSensor.hh

include/gz/sim/components/ForceTorque.hh

include/gz/sim/components/HaltMotion.hh

include/gz/sim/components/JointTransmittedWrench.hh

include/gz/sim/components/LightCmd.hh

include/gz/sim/components/LightType.hh

include/gz/sim/components/NavSat.hh

include/gz/sim/components/ParticleEmitter.hh

include/gz/sim/components/Recreate.hh

include/gz/sim/components/RenderEngineServerHeadless.hh

include/gz/sim/components/SegmentationCamera.hh

include/gz/sim/components/SemanticLabel.hh

include/gz/sim/components/SphericalCoordinates.hh

include/gz/sim/components/SystemPluginInfo.hh

include/gz/sim/components/TemperatureRange.hh

include/gz/sim/components/Visibility.hh

include/gz/sim/components/VisualCmd.hh

include/gz/sim/components/WheelSlipCmd.hh

include/gz/sim/comms/Broker.hh

include/gz/sim/comms/ICommsModel.hh

include/gz/sim/comms/MsgManager.hh

include/gz/sim/Actor.hh

include/gz/sim/Joint.hh

include/gz/sim/Light.hh

include/gz/sim/Primitives.hh

include/gz/sim/Sensor.hh

Redirects added in aee9f0d

mjcarroll

Thanks for iterating, since this is so huge, I skimmed it and nothing jumped out. Green CI should be sufficient from my point of view.

nkoenig · 2023-05-25T13:15:18Z

Any ideas regarding this windows build error: https://build.osrfoundation.org/job/ign_gazebo-pr-win/5706/consoleFull#13067844058ff58640-3599-4406-a210-216932f1748c?

nkoenig · 2023-05-25T22:39:17Z

CI is mostly green. The windows failure looks weird, especially since this PR is not touching python. The ABI checker is red, but Jenkins also says no failures were found. Homebrew builds, but has some test failures.

I could use someone else to check what, if anything, needs to be fixed here. Thanks.

azeey

A few more comments. There are still a lot of instances of ignition::gazebo, but I think it's better to change them in a separate PR. I've only commented on the ones I thought were updated in the ign-gazebo3 branch but not forward ported.

Also, include/ignition/gazebo/detail/BaseView.hh needs to be removed since it's also in include/gz/sim/detail/BaseView.hh.

include/gz/sim/components/BatteryPowerLoad.hh

include/gz/sim/components/Factory.hh

include/gz/sim/detail/BaseView.hh

src/gui/plugins/shapes/Shapes.cc

src/gui/plugins/scene3d/Scene3D.cc

src/Server_TEST.cc

include/gz/sim/rendering/MarkerManager.hh

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

azeey

Looks great. The only thing left is to delete include/ignition/gazebo/detail/BaseView.hh . Also, a friendly reminder to squash everything to a single commit since we're merging via a merge commit. I tried doing the following and the resulting history seems correct:

git rebase -i --rebase-merges ign-gazebo6

Mark everything that comes after the merge commit to f:

reset onto
merge -C b615a82da Forward-port-3-to-6 # Forward port 3 to 6
f 1b1ac04c6 Use gui 6.8
f bd5353f62 Fix visibility
f a0234e946 Fixed headers and most comments
f aee9f0d99 Fixed header redirects
f 5f1458ac6 fixed comments

azeey · 2023-05-26T15:34:23Z

The windows failure also happens on the stable CI, although inconsistently (gazebo-tooling/release-tools#724 (comment)). The ABI checker issue could be related to gazebo-tooling/release-tools#939, but it's also reporting an error due to the duplicated BaseView.hh

In file included from /tmp/of3phyEMhD/dump2.h:417:
/usr/local/source_branch/include/ignition/gazebo6/ignition/gazebo/detail/BaseView.hh:46:8: error: redefinition of â€˜struct ignition::gazebo::v6::detail::ComponentTypeHasherâ€™
   46 | struct ComponentTypeHasher
      |        ^~~~~~~~~~~~~~~~~~~
In file included from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/detail/View.hh:32,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/EntityComponentManager.hh:42,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/System.hh:23,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/comms/MsgManager.hh:30,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/comms/Broker.hh:25,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/gazebo.hh:23,
                 from /tmp/of3phyEMhD/dump2.h:5:
/usr/local/source_branch/include/ignition/gazebo6/gz/sim/detail/BaseView.hh:46:8: note: previous definition of â€˜struct ignition::gazebo::v6::detail::ComponentTypeHasherâ€™
   46 | struct ComponentTypeHasher
      |        ^~~~~~~~~~~~~~~~~~~

The macOS issue is probably due to an outdated ign-math bottle (osrf/homebrew-simulation#2269).

nkoenig · 2023-05-26T15:49:34Z

The windows failure also happens on the stable CI, although inconsistently (gazebo-tooling/release-tools#724 (comment)). The ABI checker issue could be related to gazebo-tooling/release-tools#939, but it's also reporting an error due to the duplicated BaseView.hh

In file included from /tmp/of3phyEMhD/dump2.h:417:
/usr/local/source_branch/include/ignition/gazebo6/ignition/gazebo/detail/BaseView.hh:46:8: error: redefinition of â€˜struct ignition::gazebo::v6::detail::ComponentTypeHasherâ€™
   46 | struct ComponentTypeHasher
      |        ^~~~~~~~~~~~~~~~~~~
In file included from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/detail/View.hh:32,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/EntityComponentManager.hh:42,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/System.hh:23,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/comms/MsgManager.hh:30,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/sim/comms/Broker.hh:25,
                 from /usr/local/source_branch/include/ignition/gazebo6/gz/gazebo.hh:23,
                 from /tmp/of3phyEMhD/dump2.h:5:
/usr/local/source_branch/include/ignition/gazebo6/gz/sim/detail/BaseView.hh:46:8: note: previous definition of â€˜struct ignition::gazebo::v6::detail::ComponentTypeHasherâ€™
   46 | struct ComponentTypeHasher
      |        ^~~~~~~~~~~~~~~~~~~

The macOS issue is probably due to an outdated ign-math bottle (osrf/homebrew-simulation#2269).

Right, I forgot to commit the BaseView update. I'll do soon.

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

…/ign-gazebo into nkoenig/3-to-6-05022023

nkoenig · 2023-05-31T00:15:33Z

@azeey, can I get one last LGTM from you when you're good with this? Or, just merge whenever you're comfortable.

azeey · 2023-05-31T19:10:24Z

I'm trying to figure out why the abichecker is failing. It might be a false-positive, but I'm not sure.

azeey

Alright, I'm giving up on the abichecker issue. I'm pretty sure it's a false positive, but I can't prove it even though I spent quite a bit of time comparing the headers.

nkoenig · 2023-06-01T00:22:11Z

Well, it's done. Now I can start the next forward port. Yah!

Requires: https://github.com/snapcore/snapcraft/pull/4193/files gazebosim/gz-sim#1989

methylDragon and others added 5 commits April 13, 2023 13:32

Add redirection header gz/sim.hh (#1983)

875de1b

Needed since the auto-generated header is gz/gazebo.hh Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Use ignition::gazebo:: in class instantiation (#1967)

a2a2c85

Fixes macOS build. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Forward port 3 to 6

b615a82

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

nkoenig requested review from jennuine, iche033, azeey and mjcarroll as code owners May 11, 2023 13:12

github-actions bot added the 🏯 fortress Ignition Fortress label May 11, 2023

osrf-triage added this to Inbox in Core development May 11, 2023

nkoenig added the needs upstream release Blocked by a release of an upstream library label May 11, 2023

nkoenig changed the title ~~Nkoenig/3 to 6 05022023~~ 3 to 6 05-02-2023 May 12, 2023

nkoenig mentioned this pull request May 12, 2023

6.8.0 release gazebosim/gz-gui#539

Merged

7 tasks

nkoenig removed the needs upstream release Blocked by a release of an upstream library label May 12, 2023

Use gui 6.8

1b1ac04

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

azeey moved this from Inbox to In review in Core development May 15, 2023

azeey mentioned this pull request May 15, 2023

🧑‍🌾 Flaky test INTEGRATION_ackermann_steering_system on gz-gazebo3 and gz-gazebo6 #1993

Closed

Blast545 requested a review from Crola1702 May 16, 2023 16:17

Fix visibility

bd5353f

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

azeey reviewed May 24, 2023

View reviewed changes

nkoenig added 2 commits May 24, 2023 05:57

Fixed headers and most comments

a0234e9

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

Fixed header redirects

aee9f0d

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

nkoenig requested a review from azeey May 25, 2023 12:07

mjcarroll reviewed May 25, 2023

View reviewed changes

azeey reviewed May 25, 2023

View reviewed changes

fixed comments

5f1458a

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

nkoenig requested review from azeey and mjcarroll May 26, 2023 04:01

azeey reviewed May 26, 2023

View reviewed changes

azeey approved these changes May 26, 2023

View reviewed changes

azeey mentioned this pull request May 26, 2023

Tests that use ign-math6 python bindings are failing in CI osrf/homebrew-simulation#2269

Open

nkoenig added 2 commits May 26, 2023 09:49

Forward port 3 to 6

ce1e531

Signed-off-by: Nate Koenig <natekoenig@gmail.com>

Merge branch 'nkoenig/3-to-6-05022023' of github.com:ignitionrobotics…

2390595

…/ign-gazebo into nkoenig/3-to-6-05022023

This was referenced May 26, 2023

abi-checker needs -fPIC for libraries that use Qt gazebo-tooling/release-tools#939

Closed

Enable -fPIC for ign-gazebo6 gazebo-tooling/release-tools#941

Merged

mjcarroll approved these changes May 31, 2023

View reviewed changes

azeey approved these changes May 31, 2023

View reviewed changes

nkoenig merged commit e37ff8f into ign-gazebo6 Jun 1, 2023
11 of 14 checks passed

Core development automation moved this from In review to Done Jun 1, 2023

nkoenig deleted the nkoenig/3-to-6-05022023 branch June 1, 2023 00:21

MirkoFerrati added a commit to canonical/gazebo_snap that referenced this pull request Jun 8, 2023

Fixes after upstream changes

06d1e77

Requires: https://github.com/snapcore/snapcraft/pull/4193/files gazebosim/gz-sim#1989

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

3 to 6 05-02-2023 #1989

3 to 6 05-02-2023 #1989

nkoenig commented May 11, 2023 •

edited

Loading

nkoenig commented May 11, 2023

nkoenig commented May 12, 2023

codecov bot commented May 12, 2023 •

edited

Loading

azeey left a comment

nkoenig commented May 25, 2023

mjcarroll left a comment

nkoenig commented May 25, 2023

nkoenig commented May 25, 2023

azeey left a comment

azeey left a comment

azeey commented May 26, 2023

nkoenig commented May 26, 2023

nkoenig commented May 31, 2023

azeey commented May 31, 2023

azeey left a comment

nkoenig commented Jun 1, 2023

3 to 6 05-02-2023 #1989

3 to 6 05-02-2023 #1989

Conversation

nkoenig commented May 11, 2023 • edited Loading

➡️ Forward port

nkoenig commented May 11, 2023

nkoenig commented May 12, 2023

codecov bot commented May 12, 2023 • edited Loading

Codecov Report

azeey left a comment

Choose a reason for hiding this comment

nkoenig commented May 25, 2023

mjcarroll left a comment

Choose a reason for hiding this comment

nkoenig commented May 25, 2023

nkoenig commented May 25, 2023

azeey left a comment

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

azeey commented May 26, 2023

nkoenig commented May 26, 2023

nkoenig commented May 31, 2023

azeey commented May 31, 2023

azeey left a comment

Choose a reason for hiding this comment

nkoenig commented Jun 1, 2023

nkoenig commented May 11, 2023 •

edited

Loading

codecov bot commented May 12, 2023 •

edited

Loading