ServerConfig accepts an sdf::Root DOM object #1333

nkoenig · 2022-02-11T05:08:15Z

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

🎉 New feature

Summary

I have a downstream program that is using Gazebo as a library. This program runs a world with a number of models. Each model can have different starting conditions, which is mostly just pose information for now but can be expanded to joint positions and forces.

A key aspect is consistent and reproducible behavior. To meet this goal, I would like to start Gazebo with all models at their specified locations, and not rely on asynchronous spawning. This goal can be accomplished by sending the Server a complete SDF DOM.

I have modified the ServerConfig to accept an sdf::Root object as an alternative to the file and string options.

Checklist

Signed all commits for DCO
Added tests
Updated documentation (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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

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

nkoenig · 2022-02-16T16:48:28Z

The findFuelResourceSdf function in Util.cc is a direct copy from Server.cc, and the new resolveSdfWorldFile function is a copy of the code removed from Server.cc's constructor.

mjcarroll · 2022-02-16T16:54:16Z

Since we are expanding the number of ways that sdf can be injected into the server config, do you think it would make sense to introduce an enum like:

enum class SdfSource {
  kSdfString,
  kSdfFile,
  kSdfRoot
}

To make it easier to check and make switch statements around the branching logic?

mjcarroll · 2022-02-16T16:57:14Z

Also, not for gazebo6, since it would screw up API, but for main I think we could remove the "SdfString" and "SdfFile" accessors/mutators and make ServerConfig users always parse the Sdf into a root before construction the server config.

mjcarroll

Two nits and a question

src/Util_TEST.cc

include/ignition/gazebo/Util.hh

include/ignition/gazebo/ServerConfig.hh

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

nkoenig · 2022-02-16T21:36:18Z

Since we are expanding the number of ways that sdf can be injected into the server config, do you think it would make sense to introduce an enum like:
enum class SdfSource {
  kSdfString,
  kSdfFile,
  kSdfRoot
}
To make it easier to check and make switch statements around the branching logic?

Added the enum in f32555b

include/ignition/gazebo/ServerConfig.hh

src/ServerConfig_TEST.cc

src/Util.cc

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

…ign-gazebo into server-config-root-dom

nkoenig · 2022-02-16T22:36:25Z

Needs a release of sdformat12.

ahcorde · 2022-03-16T09:06:47Z

@osrf-jenkins retest this please

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

nkoenig · 2022-03-16T16:50:08Z

@osrf-jenkins retest this please

nkoenig · 2022-03-16T16:51:56Z

@osrf-jenkins rerun tests please

chapulina · 2022-03-16T16:56:51Z

@osrf-jenkins run tests ⬅️ I believe that's the magic incantation

nkoenig · 2022-03-16T16:57:23Z

I love magic

chapulina · 2022-03-16T16:57:57Z

humm no, that didn't work. It could be because GitHub is having webhook issues right now:

https://www.githubstatus.com/

nkoenig · 2022-03-16T16:58:58Z

That also explains why github actions didn't start when I triggered a rerun.

chapulina · 2022-03-16T17:00:56Z

I manually clicked retry on the Jenkins builds. GitHub Actions seems to be down too.

Here's where the incantation is documented, for future reference:

https://ignitionrobotics.org/docs/all/ci#triggering-ci

nkoenig · 2022-03-17T12:16:17Z

@osrf-jenkins run tests

codecov · 2022-03-17T13:37:23Z

Codecov Report

Merging #1333 (72958cb) into ign-gazebo6 (4121a5c) will increase coverage by 0.05%.
The diff coverage is 86.41%.

@@               Coverage Diff               @@
##           ign-gazebo6    #1333      +/-   ##
===============================================
+ Coverage        63.04%   63.10%   +0.05%     
===============================================
  Files              303      303              
  Lines            24385    24419      +34     
===============================================
+ Hits             15374    15410      +36     
+ Misses            9011     9009       -2

Impacted Files	Coverage Δ
include/ignition/gazebo/ServerConfig.hh	`100.00% <ø> (ø)`
include/ignition/gazebo/Util.hh	`100.00% <ø> (ø)`
src/Server.cc	`89.72% <81.48%> (+3.58%)`	⬆️
src/Util.cc	`92.16% <83.33%> (-1.38%)`	⬇️
src/ServerConfig.cc	`90.79% <100.00%> (+0.43%)`	⬆️

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 4121a5c...72958cb. Read the comment docs.

chapulina

LGTM, just some small nits about documentation

chapulina · 2022-03-30T18:57:49Z

include/ignition/gazebo/Util.hh

+    /// The provided SDF filename may be a Fuel URI, relative path, name
+    /// of an installed Gazebo world filename, or an absolute path.
+    /// \param[in] _sdfFile An SDF world filename such as:
+    ///    1. "shapes.sdf" - This is referencing an installed world file.


It could also be a relative path to a file in the current directory.

chapulina · 2022-03-30T18:58:51Z

include/ignition/gazebo/Util.hh

+    /// of an installed Gazebo world filename, or an absolute path.
+    /// \param[in] _sdfFile An SDF world filename such as:
+    ///    1. "shapes.sdf" - This is referencing an installed world file.
+    ///    2. "../shapes.sdf" - This is referencing a relative world file.


It's worth mentioning what it's relative to. It could be either the running directory or any path in IGN_GAZEBO_RESOURCE_PATH

chapulina · 2022-04-01T20:02:54Z

Merging it with my documentation comments not addressed, they're minor.

* GzSceneManager: Prevent crash 💥 when inserted from menu (#1371) Signed-off-by: Louise Poubel <[email protected]> Signed-off-by: ahcorde <[email protected]> Co-authored-by: ahcorde <[email protected]> * Prepare version 6.7.0 (#1373) * Prepare version 6.7.0 Signed-off-by: Jose Luis Rivero <[email protected]> * Populate GUI plugins that are empty (#1375) Signed-off-by: Louise Poubel <[email protected]> * Fix visualization python tutorial (#1377) Signed-off-by: ahcorde <[email protected]> * Add xyz and rpy offset to published odometry pose (#1341) * Added xyz and rpy offset to published pose Signed-off-by: Aditya <[email protected]> * Added headless rendering tutorial (#1386) * Added headless rendering tutorial Signed-off-by: Nate Koenig <[email protected]> * Update tutorials/headless_rendering.md Co-authored-by: Louise Poubel <[email protected]> Signed-off-by: Nate Koenig <[email protected]> * Update tutorials/headless_rendering.md Co-authored-by: Louise Poubel <[email protected]> Signed-off-by: Nate Koenig <[email protected]> * Mention ogre2 Signed-off-by: Nate Koenig <[email protected]> * Added note about software rendering Signed-off-by: Nate Koenig <[email protected]> * add 'on linux systems' Signed-off-by: Nate Koenig <[email protected]> * Add xyz and rpy offset to published odometry pose (#1341) * Added xyz and rpy offset to published pose Signed-off-by: Aditya <[email protected]> Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Louise Poubel <[email protected]> Co-authored-by: Aditya Pande <[email protected]> * Allow to turn on/off lights (#1343) Signed-off-by: ahcorde <[email protected]> Co-authored-by: Louise Poubel <[email protected]> * Add gazebo Entity id to rendering sensor's user data (#1381) Adds the gazebo::Entity id to a rendering::Sensor's UserData object. The main use case is so that we can get back the corresponding gazebo Entity in the rendering thread when processing sensors. Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> * Don't mark entities with a ComponentState::NoChange component as modified (#1391) Signed-off-by: Ashton Larkin <[email protected]> * Disable ModelCommandAPI_TEST.RgbdCameraSensor on macOS (#1397) Disabling test since it's very flaky. Signed-off-by: Addisu Z. Taddese <[email protected]> * Disable PeerTracker.PeerTrackerStale on macOS (#1398) Signed-off-by: Addisu Z. Taddese <[email protected]> * Toggle Light visuals (#1387) Signed-off-by: ahcorde <[email protected]> * Prevent hanging when world has only non-world plugins (#1383) Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> * Component inspector: refactor Pose3d C++ code into a separate class (#1400) Signed-off-by: Louise Poubel <[email protected]> * add initial_position param to joint controller system (#1406) Similar to the <initial_position> parameter in the JointTrajectoryController system, this PR adds an <initial_position> parameter to the JointPositionController to let users specifies the initial target pos for a joint. Signed-off-by: Ian Chen <[email protected]> * Fix JointStatePublisher topic name for nested models (#1405) The joint-state-publisher system currently assumes it's always attached to the top level model and hence generates an incorrect topic name for nested models. This PR updates it to generate the correct topic name. Signed-off-by: Ian Chen <[email protected]> * Added user command to set multiple entities (#1394) Signed-off-by: ahcorde <[email protected]> Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: Louise Poubel <[email protected]> * Disable tests that are expected to fail on Windows (#1408) Signed-off-by: Louise Poubel <[email protected]> * Fortress: Install Ogre 2.2, simplify docker (#1395) Signed-off-by: Louise Poubel <[email protected]> * SceneBroadcaster: only send changed state information for change events (#1392) Signed-off-by: Ashton Larkin <[email protected]> Co-authored-by: Louise Poubel <[email protected]> * Add wheel slip user command (#1241) Signed-off-by: Ivan Santiago Paunovic <[email protected]> * Distortion camera integration test (#1374) Signed-off-by: William Lew <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> Co-authored-by: Louise Poubel <[email protected]> * Add the Model Photo Shoot system, port of Modelpropshop plugin from Gazebo classic (#1331) This system can be used to generate thumbnails of models. In comparison to the Modelpropshop plugin in Gazebo classic, this adds the ability to generate thumbnails after randomizing the joint positions of the model. Signed-off-by: Marco A. Gutierrez <[email protected]> Signed-off-by: Addisu Z. Taddese <[email protected]> * Referring to Fuel assets within a heightmap (#1419) Signed-off-by: Louise Poubel <[email protected]> * Disable sensors in sensors system when battery is drained (#1385) Added a new parameter <disable_on_drained_battery> to the sensors system. If set to true, sensors will be disabled if the model is out of battery. It listens to the battery state of charge from the battery plugin, and if the charge reaches zero, all child sensors and nested sensors are set to be inactive. Signed-off-by: Ian Chen <[email protected]> * 🏁🎈 5.4.0 (#1420) Signed-off-by: Louise Poubel <[email protected]> * ServerConfig accepts an sdf::Root DOM object (#1333) Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Michael Carroll <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> * Preparing 6.8.0 release (#1425) * Prepareing 6.8.0 release Signed-off-by: Jose Luis Rivero <[email protected]> * Update changelog after merging forward port Signed-off-by: Jose Luis Rivero <[email protected]> * Add Gaussian noise to Odometry Publisher (#1393) * Added gaussian noise and odometry with covariance publisher Signed-off-by: Aditya <[email protected]> Co-authored-by: Louise Poubel <[email protected]> * Fix deprecation warnings for ModelPhotoShoot (#1437) Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: ahcorde <[email protected]> Co-authored-by: Jose Luis Rivero <[email protected]> Co-authored-by: Aditya Pande <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Ian Chen <[email protected]> Co-authored-by: Ashton Larkin <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]> Co-authored-by: Ivan Santiago Paunovic <[email protected]> Co-authored-by: William Lew <[email protected]> Co-authored-by: Marco A. Gutiérrez <[email protected]> Co-authored-by: Michael Carroll <[email protected]>

osrf-triage · 2022-04-13T21:29:41Z

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1

Added more tests

0896ac1

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

nkoenig requested a review from chapulina as a code owner February 11, 2022 05:08

github-actions bot added the 🏯 fortress Ignition Fortress label Feb 11, 2022

merged with ign-gazebo6

60be532

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

Change clone

2770692

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

mjcarroll reviewed Feb 16, 2022

View reviewed changes

src/Util_TEST.cc Outdated Show resolved Hide resolved

include/ignition/gazebo/Util.hh Outdated Show resolved Hide resolved

include/ignition/gazebo/ServerConfig.hh Outdated Show resolved Hide resolved

Address review comments

f32555b

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

Merge branch 'ign-gazebo6' into server-config-root-dom

171d745

mjcarroll requested changes Feb 16, 2022

View reviewed changes

include/ignition/gazebo/ServerConfig.hh Outdated Show resolved Hide resolved

include/ignition/gazebo/ServerConfig.hh Outdated Show resolved Hide resolved

src/ServerConfig_TEST.cc Show resolved Hide resolved

src/Util.cc Outdated Show resolved Hide resolved

Nate Koenig added 2 commits February 16, 2022 14:17

Fix linter, and added tests

7d33cf0

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

Merge branch 'server-config-root-dom' of github.com:ignitionrobotics/…

7fedd8d

…ign-gazebo into server-config-root-dom

mjcarroll approved these changes Feb 16, 2022

View reviewed changes

Merge branch 'ign-gazebo6' into server-config-root-dom

087225e

chapulina added the needs upstream release Blocked by a release of an upstream library label Mar 1, 2022

nkoenig and others added 5 commits March 8, 2022 10:35

Merge branch 'ign-gazebo6' into server-config-root-dom

713f3ba

Merge branch 'ign-gazebo6' into server-config-root-dom

bb3d2a6

Merge branch 'ign-gazebo6' into server-config-root-dom

0402553

Merge branch 'ign-gazebo6' into server-config-root-dom

146ecf9

Merge branch 'ign-gazebo6' into server-config-root-dom

71ee327

use sdf 12.4

d0695a4

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

nkoenig mentioned this pull request Mar 16, 2022

Enable preleases on ign-gazebo6 gazebo-tooling/gzdev#55

Merged

nkoenig and others added 3 commits March 18, 2022 11:15

Merge branch 'ign-gazebo6' into server-config-root-dom

977847e

Merge branch 'ign-gazebo6' into server-config-root-dom

3af47f1

Merge branch 'ign-gazebo6' into server-config-root-dom

61f286a

chapulina approved these changes Mar 30, 2022

View reviewed changes

Merge branch 'ign-gazebo6' into server-config-root-dom

72958cb

chapulina merged commit 39a0ce3 into ign-gazebo6 Apr 1, 2022

chapulina deleted the server-config-root-dom branch April 1, 2022 20:03

chapulina mentioned this pull request Apr 27, 2022

Disable prereleases on ign-gazebo6 gazebo-tooling/gzdev#58

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ServerConfig accepts an sdf::Root DOM object #1333

ServerConfig accepts an sdf::Root DOM object #1333

nkoenig commented Feb 11, 2022 •

edited

Loading

nkoenig commented Feb 16, 2022

mjcarroll commented Feb 16, 2022

mjcarroll commented Feb 16, 2022

mjcarroll left a comment

nkoenig commented Feb 16, 2022

nkoenig commented Feb 16, 2022

ahcorde commented Mar 16, 2022

nkoenig commented Mar 16, 2022

nkoenig commented Mar 16, 2022

chapulina commented Mar 16, 2022

nkoenig commented Mar 16, 2022

chapulina commented Mar 16, 2022

nkoenig commented Mar 16, 2022

chapulina commented Mar 16, 2022

nkoenig commented Mar 17, 2022

codecov bot commented Mar 17, 2022 •

edited

Loading

chapulina left a comment

chapulina Mar 30, 2022

chapulina Mar 30, 2022

chapulina commented Apr 1, 2022

osrf-triage commented Apr 13, 2022

ServerConfig accepts an sdf::Root DOM object #1333

ServerConfig accepts an sdf::Root DOM object #1333

Conversation

nkoenig commented Feb 11, 2022 • edited Loading

🎉 New feature

Summary

Checklist

nkoenig commented Feb 16, 2022

mjcarroll commented Feb 16, 2022

mjcarroll commented Feb 16, 2022

mjcarroll left a comment

Choose a reason for hiding this comment

nkoenig commented Feb 16, 2022

nkoenig commented Feb 16, 2022

ahcorde commented Mar 16, 2022

nkoenig commented Mar 16, 2022

nkoenig commented Mar 16, 2022

chapulina commented Mar 16, 2022

nkoenig commented Mar 16, 2022

chapulina commented Mar 16, 2022

nkoenig commented Mar 16, 2022

chapulina commented Mar 16, 2022

nkoenig commented Mar 17, 2022

codecov bot commented Mar 17, 2022 • edited Loading

Codecov Report

chapulina left a comment

Choose a reason for hiding this comment

chapulina Mar 30, 2022

Choose a reason for hiding this comment

chapulina Mar 30, 2022

Choose a reason for hiding this comment

chapulina commented Apr 1, 2022

osrf-triage commented Apr 13, 2022

nkoenig commented Feb 11, 2022 •

edited

Loading

codecov bot commented Mar 17, 2022 •

edited

Loading