Support SDFormat 1.8 Composition #542

brawner · 2021-01-09T00:13:47Z

This PR adds support for model composition via <include>'d models. This PR adds the following functionality related to nested models:

Supports loading nested models via includes
Support nested references to frames
Support the use of frames for a joint's parent or child
Placement frames with references to nested models
AABBs mostly take nested models into account. However, currently one needs to re-click on a nested entity to update its bounding box, since otherwise the bounding box just follows the canonical link. Since there is only one canonical link per model even if it includes many nested models, the bounding box's update is linked to the canonical link. @azeey's work on supporting one canonical link per nested model will help with this.
TransformControl of nested models

Work towards #479

brawner · 2021-02-01T20:15:15Z

Fixed a cppcheck error and rebased onto main

src/EntityComponentManager.cc

src/SdfEntityCreator.cc

brawner

Thanks @azeey!

src/SdfEntityCreator.cc

azeey

I left a few comments that should be easy to address. The one comment that might be more involved is the FreeGroup feature. We can tackle that in a follow up PR if you'd like. Otherwise, this is looking great!

src/systems/user_commands/UserCommands.cc

test/integration/sdf_frame_semantics.cc

azeey · 2021-02-02T03:18:46Z

test/integration/sdf_frame_semantics.cc

+TEST_F(SdfFrameSemanticsTest, IncludeNestedModelsRelativeTo)
+{
+  std::string path = std::string(PROJECT_SOURCE_PATH) + "/test/worlds/models";
+  setenv("IGN_GAZEBO_RESOURCE_PATH",


I'm not sure how well this works across multiple platforms. An alternative is to use sdf::setFindCallback

It seems to be the preferred way of setting the resource path in these test files. I got it from test/integration/physics_system.cc

azeey · 2021-02-02T04:09:30Z

src/systems/physics/Physics.cc

@@ -1458,14 +1458,6 @@ void PhysicsPrivate::UpdatePhysics(EntityComponentManager &_ecm)
        if (modelIt == this->entityModelMap.end())
          return true;

-        // world pose cmd currently not supported for nested models
-        if (_entity != topLevelModel(_entity, _ecm))


I'm not sure TPE supports moving nested models separately from their top level model. So, maybe we shouldn't remove this yet?

Hmm, if you have this check then a nested model won't transform correctly, even if it's fully connected by joints.

azeey · 2021-02-02T04:20:17Z

src/SdfEntityCreator.cc

@@ -280,6 +304,9 @@ Entity SdfEntityCreator::CreateEntities(const sdf::Model *_model,

  // Links
  bool canonicalLinkCreated = false;
+  const auto[canonicalLink, canonicalLinkName] =


nit: canonicalLinkName is not used, so can just be const auto *canonicalLink = _model->CanonicalLink();

azeey · 2021-02-02T04:37:10Z

test/worlds/models/nested_models/model.sdf

+        </link>
+
+        <joint name="joint_01" type="continuous">
+          <parent>frame_00</parent>


I was getting the error Failed to resolve parent link for joint 'joint_01' with parent name 'frame_00' when I ran INTEGRATION_sdf_frame_semantics. This is because frame_00 can't be referenced from this scope.

Moved the joint up one level.

src/SdfEntityCreator.cc

azeey · 2021-02-02T04:42:57Z

test/worlds/models/nested_models/model.sdf

+          </visual>
+        </link>
+
+        <joint name="joint_01" type="continuous">


DART doesn't support continuous joints yet. Can you change this to revolute?

Signed-off-by: Addisu Z. Taddese <[email protected]>

Signed-off-by: Stephen Brawner <[email protected]>

brawner · 2021-02-09T19:56:13Z

Rebased onto main, and resolved a nullptr issue which was showing up in the breadcrumbs test.

There appears to be a memory error with the user_commands integration test, which seems to be unrelated to this PR, but nonetheless I'll try to resolve it first.

Signed-off-by: Stephen Brawner <[email protected]>

codecov · 2021-02-09T23:50:16Z

Codecov Report

Merging #542 (bca2fe1) into main (4e692db) will decrease coverage by 0.00%.
The diff coverage is 54.83%.

@@            Coverage Diff             @@
##             main     #542      +/-   ##
==========================================
- Coverage   64.88%   64.87%   -0.01%     
==========================================
  Files         235      235              
  Lines       16889    16941      +52     
==========================================
+ Hits        10958    10991      +33     
- Misses       5931     5950      +19

Impacted Files	Coverage Δ
.../plugins/component_inspector/ComponentInspector.cc	`7.20% <0.00%> (-0.02%)`	⬇️
.../plugins/component_inspector/ComponentInspector.hh	`28.57% <ø> (ø)`
src/systems/user_commands/UserCommands.cc	`69.13% <0.00%> (-0.69%)`	⬇️
src/rendering/RenderUtil.cc	`42.64% <50.00%> (+0.04%)`	⬆️
src/SdfEntityCreator.cc	`88.23% <60.78%> (-5.08%)`	⬇️
src/systems/physics/Physics.cc	`68.81% <0.00%> (+0.71%)`	⬆️
src/ServerConfig.cc	`89.68% <0.00%> (+0.79%)`	⬆️

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 f1bc04b...bca2fe1. Read the comment docs.

Signed-off-by: Stephen Brawner <[email protected]>

brawner · 2021-02-12T00:36:25Z

Added integration test which fails without the nested model dartsim PR (gazebosim/gz-physics#189).

scpeters · 2021-02-12T02:05:33Z

I haven't looked at the code yet, just run some tests locally. The INTEGRATION_physics_system has two failing cases for me

[  FAILED  ] PhysicsSystemFixture.IncludeNestedModelDartsim
[  FAILED  ] PhysicsSystemFixture.IncludeNestedModelTPE

Signed-off-by: Stephen Brawner <[email protected]>

brawner · 2021-02-12T21:56:35Z

Failures in tests are expected until dependent ign-physics is merged and released

scpeters · 2021-02-16T19:38:05Z

I think the windows build is failing due to setenv calls

azeey · 2021-02-24T22:01:56Z

The transform control is not working for me. I also found out that it doesn't work for nested models in Dome with TPE neither since gazebosim/gz-physics#142. I will go ahead remove the changes related to that and fix it in a follow up PR.

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey · 2021-02-24T23:37:58Z

The transform control is not working for me. I also found out that it doesn't work for nested models in Dome with TPE neither since ignitionrobotics/ign-physics#142. I will go ahead remove the changes related to that and fix it in a follow up PR.

Removed in 023d4bb. Also created issue #649.

azeey · 2021-02-24T23:39:06Z

I think the windows build is failing due to setenv calls

Fixed in cf6f4f3

brawner · 2021-02-24T23:39:08Z

I'm going to unsubscribe from this PR. Thanks for taking over @azeey. Feel free to ping me if you have any questions.

…_sdf_18 Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey · 2021-03-10T18:45:30Z

I believe test failures on macOS are unrelated to this PR. Merging!

[Dome] Support <actor><pose> and <actor><plugin> Approved-by: Luca Della Vedova

brawner requested a review from azeey January 9, 2021 00:13

github-actions bot added the 🏢 edifice Ignition Edifice label Jan 9, 2021

brawner self-assigned this Jan 11, 2021

brawner mentioned this pull request Jan 11, 2021

Support SDFormat 1.8 composition #479

Closed

12 tasks

brawner force-pushed the brawner/composition_sdf_18 branch 3 times, most recently from 7d87eb2 to b342f6e Compare January 26, 2021 01:56

brawner changed the title ~~WIP: Support SDFormat 1.8 Composition~~ Support SDFormat 1.8 Composition Jan 26, 2021

brawner marked this pull request as ready for review January 26, 2021 01:57

brawner requested review from chapulina and iche033 as code owners January 26, 2021 01:57

brawner force-pushed the brawner/composition_sdf_18 branch from b342f6e to f51154c Compare February 1, 2021 20:14

azeey reviewed Feb 1, 2021

View reviewed changes

src/EntityComponentManager.cc Outdated Show resolved Hide resolved

src/SdfEntityCreator.cc Outdated Show resolved Hide resolved

src/SdfEntityCreator.cc Show resolved Hide resolved

brawner commented Feb 1, 2021

View reviewed changes

src/SdfEntityCreator.cc Show resolved Hide resolved

src/SdfEntityCreator.cc Outdated Show resolved Hide resolved

azeey reviewed Feb 2, 2021

View reviewed changes

brawner force-pushed the brawner/composition_sdf_18 branch from 2c6e372 to cbd49c6 Compare February 3, 2021 23:58

brawner requested a review from azeey February 3, 2021 23:59

azeey and others added 6 commits February 9, 2021 11:53

WIP: First pass at supporting SDFormat 1.8 Composition.

92d7b79

Signed-off-by: Addisu Z. Taddese <[email protected]>

Add tests and support transform control

1b70a35

Signed-off-by: Stephen Brawner <[email protected]>

cppcheck cleanup

56b915e

Signed-off-by: Stephen Brawner <[email protected]>

Fix some tests and address PR feedback

9a0f0e7

Signed-off-by: Stephen Brawner <[email protected]>

PR Fixup

500020f

Signed-off-by: Stephen Brawner <[email protected]>

Fix nullptr issue

f9b456e

Signed-off-by: Stephen Brawner <[email protected]>

brawner force-pushed the brawner/composition_sdf_18 branch from cbd49c6 to f9b456e Compare February 9, 2021 19:54

Fix whitespace

22de985

Signed-off-by: Stephen Brawner <[email protected]>

Add movement test for nested model physics

d02a5a7

Signed-off-by: Stephen Brawner <[email protected]>

Update physics_system test

694eec2

Signed-off-by: Stephen Brawner <[email protected]>

scpeters mentioned this pull request Feb 12, 2021

ignition-physics4: newer commit for SDFormat 1.8 osrf/homebrew-simulation#1335

Merged

Merge branch 'main' into brawner/composition_sdf_18

69cb299

Merge branch 'main' into brawner/composition_sdf_18

921a501

azeey added 2 commits February 24, 2021 16:12

Revert changes related to PoseCommand in UserCommands

023d4bb

Signed-off-by: Addisu Z. Taddese <[email protected]>

setenv -> ignition::common::setenv

cf6f4f3

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey force-pushed the brawner/composition_sdf_18 branch from e1dfbc4 to cf6f4f3 Compare February 24, 2021 23:36

Merge remote-tracking branch 'upstream/main' into brawner/composition…

9e4fcda

…_sdf_18 Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey approved these changes Mar 3, 2021

View reviewed changes

azeey added 2 commits March 9, 2021 09:51

Merge branch 'main' into brawner/composition_sdf_18

5348afa

Merge branch 'main' into brawner/composition_sdf_18

bca2fe1

azeey merged commit fb4080e into main Mar 10, 2021

azeey deleted the brawner/composition_sdf_18 branch March 10, 2021 18:47

nkoenig pushed a commit that referenced this pull request Nov 8, 2021

Merged in actor_pose_dome (pull request #542)

67d370e

[Dome] Support <actor><pose> and <actor><plugin> Approved-by: Luca Della Vedova

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support SDFormat 1.8 Composition #542

Support SDFormat 1.8 Composition #542

brawner commented Jan 9, 2021 •

edited

Loading

brawner commented Feb 1, 2021

brawner left a comment

azeey left a comment

azeey Feb 2, 2021

brawner Feb 3, 2021

azeey Feb 2, 2021

brawner Feb 3, 2021

brawner Feb 3, 2021

azeey Feb 2, 2021

brawner Feb 3, 2021

azeey Feb 2, 2021

brawner Feb 3, 2021

azeey Feb 2, 2021

brawner Feb 3, 2021

brawner commented Feb 9, 2021

codecov bot commented Feb 9, 2021 •

edited

Loading

brawner commented Feb 12, 2021

scpeters commented Feb 12, 2021

brawner commented Feb 12, 2021

scpeters commented Feb 16, 2021

azeey commented Feb 24, 2021

azeey commented Feb 24, 2021

azeey commented Feb 24, 2021

brawner commented Feb 24, 2021

azeey commented Mar 10, 2021

Support SDFormat 1.8 Composition #542

Support SDFormat 1.8 Composition #542

Conversation

brawner commented Jan 9, 2021 • edited Loading

brawner commented Feb 1, 2021

brawner left a comment

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brawner commented Feb 9, 2021

codecov bot commented Feb 9, 2021 • edited Loading

Codecov Report

brawner commented Feb 12, 2021

scpeters commented Feb 12, 2021

brawner commented Feb 12, 2021

scpeters commented Feb 16, 2021

azeey commented Feb 24, 2021

azeey commented Feb 24, 2021

azeey commented Feb 24, 2021

brawner commented Feb 24, 2021

azeey commented Mar 10, 2021

brawner commented Jan 9, 2021 •

edited

Loading

codecov bot commented Feb 9, 2021 •

edited

Loading