Break out server constructor #2638

nkoenig · 2024-10-02T13:12:16Z

🎉 New feature

Summary

Moves a section of the Server constructor code to a separate function. The primary reason is to make the eventual download models in parallel PR easier to review. There is a side benefit of breaking up a large function into two smaller functions.

I also replaced the static DefaultWorld with a default constructed sdf::World object. This removes a static object, and will make sure the default world uses the latest sdf version.

Commits:

Move the code to a new function. c48536b
Patches the moved code so that it builds. 3c7e333
Replace DefaultWorld with a default constructed sdf::World object. 21ccdf0

Test it

Tests should pass, and everything should work as normal.

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.

src/ServerPrivate.cc

mjcarroll · 2024-10-02T17:27:43Z

Have these Reset related failures been around a bit?

nkoenig · 2024-10-02T17:37:54Z

Have these Reset related failures been around a bit?

Was that reset error brought up in the last gazebo weekly meeting? This rings a bell:

/home/jenkins/workspace/gz_sim-ci-pr_any-jammy-amd64/gz-sim/test/integration/reset_sensors.cc:259
Expected equality of these values:
  kEndingPressure
    Which is: 100362.99
  pressureReceiver.Last().pressure()
    Which is: 100360.66

mjcarroll · 2024-10-02T17:47:17Z

Was that reset error brought up in the last gazebo weekly meeting?

Possibly, it rings a bell, that was over 24 hours ago, so it has been erased.

nkoenig · 2024-10-02T22:04:35Z

Was that reset error brought up in the last gazebo weekly meeting?

Possibly, it rings a bell, that was over 24 hours ago, so it has been erased.

Ran CI again, and the test failure went away. Looks like a flaky test.

azeey · 2024-10-03T20:53:39Z

The windows failure is strange. The last build on the gz-sim8 branch was green (https://build.osrfoundation.org/job/gz_sim-8-win/125/), so it might be caused by this PR.


C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,59): error C2143: syntax error: missing '}' before 'constant' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,59): error C2059: syntax error: 'constant' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,69): error C2143: syntax error: missing ';' before '}' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,69): error C2238: unexpected token(s) preceding ';' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(43,7): error C2059: syntax error: 'public' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(43,23): error C2059: syntax error: ')' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,7): error C2059: syntax error: 'public' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,31): error C2588: '::~MeshCSG': illegal global destructor [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,24): error C2575: 'MeshCSG': only member functions and bases can be virtual [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,33): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(54,7): error C2059: syntax error: 'public' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(59,7): error C2059: syntax error: 'private' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(65,7): error C2059: syntax error: 'private' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(68,1): error C2059: syntax error: '}' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(68,1): error C2143: syntax error: missing ';' before '}' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Line3.hh(25,1): error C2143: syntax error: missing ';' before '{' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Line3.hh(25,1): error C2447: '{': missing function header (old-style formal list?) [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(226,30): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(226,30): error C2143: syntax error: missing ',' before '&' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(238,55): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(238,55): error C2143: syntax error: missing ',' before '&' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(105,18): error C2143: syntax error: missing ';' before '<' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(271): message : see reference to class template instantiation 'gz::math::v7::Triangle3<T>' being compiled [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(105,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(106,1): error C2334: unexpected token(s) preceding '{'; skipping apparent function body [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(120,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(120,38): error C2143: syntax error: missing ',' before '<' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(182,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(182,18): error C2143: syntax error: missing ',' before '<' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

  gtest.vcxproj -> C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\lib\Release\gtest.lib

  gtest_main.vcxproj -> C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\lib\Release\gtest_main.lib

nkoenig · 2024-10-03T21:00:32Z

The windows failure is strange. The last build on the gz-sim8 branch was green (https://build.osrfoundation.org/job/gz_sim-8-win/125/), so it might be caused by this PR.


C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,59): error C2143: syntax error: missing '}' before 'constant' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,59): error C2059: syntax error: 'constant' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,69): error C2143: syntax error: missing ';' before '}' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(40,69): error C2238: unexpected token(s) preceding ';' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(43,7): error C2059: syntax error: 'public' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(43,23): error C2059: syntax error: ')' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,7): error C2059: syntax error: 'public' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,31): error C2588: '::~MeshCSG': illegal global destructor [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,24): error C2575: 'MeshCSG': only member functions and bases can be virtual [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(46,33): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(54,7): error C2059: syntax error: 'public' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(59,7): error C2059: syntax error: 'private' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(65,7): error C2059: syntax error: 'private' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(68,1): error C2059: syntax error: '}' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-common5\include\gz\common5\gz/common/MeshCSG.hh(68,1): error C2143: syntax error: missing ';' before '}' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Line3.hh(25,1): error C2143: syntax error: missing ';' before '{' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Line3.hh(25,1): error C2447: '{': missing function header (old-style formal list?) [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(226,30): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(226,30): error C2143: syntax error: missing ',' before '&' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(238,55): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/AxisAlignedBox.hh(238,55): error C2143: syntax error: missing ',' before '&' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(105,18): error C2143: syntax error: missing ';' before '<' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(271): message : see reference to class template instantiation 'gz::math::v7::Triangle3<T>' being compiled [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(105,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(106,1): error C2334: unexpected token(s) preceding '{'; skipping apparent function body [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(120,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(120,38): error C2143: syntax error: missing ',' before '<' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(182,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

C:\J\workspace\gz_sim-pr-win\ws\install\gz-math7\include\gz\math7\gz/math/Triangle3.hh(182,18): error C2143: syntax error: missing ',' before '<' [C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\src\gz-sim8.vcxproj]

  gtest.vcxproj -> C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\lib\Release\gtest.lib

  gtest_main.vcxproj -> C:\J\workspace\gz_sim-pr-win\ws\build\gz-sim8\lib\Release\gtest_main.lib

Yeah, I was looking at those. I'm not yet sure what the problem is, but I'm looking into it.

Signed-off-by: Nate Koenig <[email protected]>

nkoenig · 2024-10-04T18:36:53Z

Update in the windows error. Moving the location of the MeshInertiaCalculator header file fixed the windows build errors, which is strange.

Signed-off-by: Nate Koenig <[email protected]>

mjcarroll · 2024-10-09T14:18:19Z

Just re-run windows CI here, looks like a spurious jenkins error.

nkoenig · 2024-10-14T18:04:08Z

Just re-run windows CI here, looks like a spurious jenkins error.

@mjcarroll, looks like it was a spurious error. Could you please approve this PR? I think the CI checks are waiting for your approval.

nkoenig added 3 commits October 2, 2024 05:34

Copy the code to the new function - won't compile

c48536b

Fix the build

3c7e333

Replace static default world with sdf default world

21ccdf0

nkoenig requested a review from mjcarroll as a code owner October 2, 2024 13:12

github-actions bot added the 🎵 harmonic Gazebo Harmonic label Oct 2, 2024

ahcorde requested changes Oct 2, 2024

View reviewed changes

src/ServerPrivate.cc Show resolved Hide resolved

src/ServerPrivate.cc Show resolved Hide resolved

Added string and utility headers

219a79c

ahcorde approved these changes Oct 3, 2024

View reviewed changes

nkoenig added 2 commits October 4, 2024 06:07

test moving header file

dc8ee70

Signed-off-by: Nate Koenig <[email protected]>

Move header file back

c32ebee

Signed-off-by: Nate Koenig <[email protected]>

nkoenig added 4 commits October 4, 2024 14:37

Merge branch 'gz-sim8' into feature/break-out-server-constructor

bdf7522

Move meshinertiacalculator header

e9c547b

Signed-off-by: Nate Koenig <[email protected]>

Testing remove of literals

3fc3a68

Signed-off-by: Nate Koenig <[email protected]>

Undo tests, and just move the MeshInertiaCalculator.hh to top of file

d2f11dc

Signed-off-by: Nate Koenig <[email protected]>

mjcarroll approved these changes Oct 14, 2024

View reviewed changes

nkoenig merged commit d94c11e into gz-sim8 Oct 14, 2024
8 of 9 checks passed

nkoenig deleted the feature/break-out-server-constructor branch October 14, 2024 21:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Break out server constructor #2638

Break out server constructor #2638

nkoenig commented Oct 2, 2024 •

edited

Loading

mjcarroll commented Oct 2, 2024

nkoenig commented Oct 2, 2024

mjcarroll commented Oct 2, 2024

nkoenig commented Oct 2, 2024

azeey commented Oct 3, 2024

nkoenig commented Oct 3, 2024

nkoenig commented Oct 4, 2024

mjcarroll commented Oct 9, 2024

nkoenig commented Oct 14, 2024

Break out server constructor #2638

Break out server constructor #2638

Conversation

nkoenig commented Oct 2, 2024 • edited Loading

🎉 New feature

Summary

Test it

Checklist

mjcarroll commented Oct 2, 2024

nkoenig commented Oct 2, 2024

mjcarroll commented Oct 2, 2024

nkoenig commented Oct 2, 2024

azeey commented Oct 3, 2024

nkoenig commented Oct 3, 2024

nkoenig commented Oct 4, 2024

mjcarroll commented Oct 9, 2024

nkoenig commented Oct 14, 2024

nkoenig commented Oct 2, 2024 •

edited

Loading