Skip to content

Commit ec11f63

Browse files
azeeyjennuine
andauthored
Fix bug where //include/pose was ignored when using the Interface API (#853)
Pose overrides made using `//include/pose` were not working if the parser did not set the pose of the `sdf::InterfaceModel` object it returns to the value in `//include/pose`. See #852 for more details. Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org> Co-authored-by: Jenn Nguyen <jenn@openrobotics.org>
1 parent ad61117 commit ec11f63

File tree

3 files changed

+39
-13
lines changed

3 files changed

+39
-13
lines changed

src/FrameSemantics.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,8 @@ struct ModelWrapper : public WrapperBase
514514
: WrapperBase{_ifaceModel.Name(), "Interface Model",
515515
_ifaceModel.Static() ? FrameType::STATIC_MODEL
516516
: FrameType::MODEL},
517-
rawPose(_ifaceModel.ModelFramePoseInParentFrame()),
517+
rawPose(_nestedInclude.IncludeRawPose().value_or(
518+
_ifaceModel.ModelFramePoseInParentFrame())),
518519
rawRelativeTo(_nestedInclude.IncludePoseRelativeTo().value_or("")),
519520
relativeTo(rawRelativeTo),
520521
canonicalLinkName(_ifaceModel.CanonicalLinkName()),

test/integration/interface_api.cc

+36-11
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,14 @@ sdf::InterfaceModelPtr parseModel(toml::Value &_doc,
8383

8484
class CustomTomlParser
8585
{
86-
public: CustomTomlParser(bool _supportsMergeInclude = true)
87-
: supportsMergeInclude(_supportsMergeInclude)
86+
/// \brief Constructor
87+
/// \param[in] _supportsMergeInclude Whether the parser supports merge include
88+
/// \param[in] _overridePoseInParser Whether the parser should apply pose
89+
/// overrides from //include/pose
90+
public: CustomTomlParser(bool _supportsMergeInclude = true,
91+
bool _overridePoseInParser = true)
92+
: supportsMergeInclude(_supportsMergeInclude),
93+
overridePoseInParser(_overridePoseInParser)
8894
{
8995
}
9096

@@ -104,7 +110,7 @@ class CustomTomlParser
104110
param.Set(*_include.IsStatic());
105111
doc["static"] = {param};
106112
}
107-
if (_include.IncludeRawPose().has_value())
113+
if (this->overridePoseInParser && _include.IncludeRawPose().has_value())
108114
{
109115
// if //include/static is set, override the value in the inluded model
110116
sdf::Param poseParam("pose", "pose", "", false);
@@ -120,6 +126,7 @@ class CustomTomlParser
120126
}
121127

122128
public: bool supportsMergeInclude;
129+
public: bool overridePoseInParser{true};
123130
};
124131

125132
bool endsWith(const std::string &_str, const std::string &_suffix)
@@ -552,16 +559,34 @@ TEST_F(InterfaceAPI, FrameSemantics)
552559
const std::string testFile = sdf::testing::TestFile(
553560
"sdf", "include_with_interface_api_frame_semantics.sdf");
554561
this->config.RegisterCustomModelParser(this->customTomlParser);
555-
sdf::Root root;
556-
sdf::Errors errors = root.Load(testFile, config);
557-
EXPECT_TRUE(errors.empty()) << errors;
562+
{
563+
sdf::Root root;
564+
sdf::Errors errors = root.Load(testFile, config);
565+
EXPECT_TRUE(errors.empty()) << errors;
558566

559-
const sdf::World *world = root.WorldByIndex(0);
560-
ASSERT_NE(nullptr, world);
561-
EXPECT_EQ(1u, world->InterfaceModelCount());
567+
const sdf::World *world = root.WorldByIndex(0);
568+
ASSERT_NE(nullptr, world);
569+
EXPECT_EQ(1u, world->InterfaceModelCount());
562570

563-
SCOPED_TRACE("InterfaceAPI.FrameSemantics");
564-
this->CheckFrameSemantics(world);
571+
SCOPED_TRACE("InterfaceAPI.FrameSemantics");
572+
this->CheckFrameSemantics(world);
573+
}
574+
{
575+
// Check without //include/pose override applied in parser.
576+
sdf::Root root;
577+
sdf::ParserConfig newConfig = this->config;
578+
CustomTomlParser parserWithoutPoseOverride(true, false);
579+
newConfig.RegisterCustomModelParser(parserWithoutPoseOverride);
580+
sdf::Errors errors = root.Load(testFile, newConfig);
581+
EXPECT_TRUE(errors.empty()) << errors;
582+
583+
const sdf::World *world = root.WorldByIndex(0);
584+
ASSERT_NE(nullptr, world);
585+
EXPECT_EQ(1u, world->InterfaceModelCount());
586+
587+
SCOPED_TRACE("InterfaceAPI.FrameSemantics_NoPoseOverrideInParser");
588+
this->CheckFrameSemantics(world);
589+
}
565590
}
566591

567592
/////////////////////////////////////////////////

test/integration/model/double_pendulum.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# very limited toml parser that is not capable of parsing the full toml syntax.
33
# Specifically, arrays are not supported.
44
name = "double_pendulum"
5-
pose = "1 0 0 0 0 0"
5+
pose = "1 2 3 0 0 0"
66
canonical_link = "base"
77

88
[links.base]

0 commit comments

Comments
 (0)