Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scatter ratio parameter to Particle Emitter DOM #547

Merged
merged 4 commits into from
May 6, 2021
Merged

Add scatter ratio parameter to Particle Emitter DOM #547

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81381f0
add particle scatter ratio?
iche033 Apr 26, 2021
2256147
update field name
iche033 Apr 26, 2021
b89e8b3
update doc
iche033 May 5, 2021
67defd7
Merge branch 'sdf10' into particle_scatter_ratio
iche033 May 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions include/sdf/ParticleEmitter.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,17 @@ namespace sdf
/// \param[in] _topic The topic used to update the particle emitter.
public: void SetTopic(const std::string &_topic);

/// \brief Get the particle scatter ratio. This is used to determine the
/// ratio of particles that will be detected by sensors.
/// \return Particle scatter ratio
/// \sa SetScatterRatio
public: float ScatterRatio() const;

/// \brief Set the particle scatter ratio. This is used to determine the
/// ratio of particles that will be detected by sensors.
/// \param[in] _ratio Scatter raito. This value should be > 0.
iche033 marked this conversation as resolved.
Show resolved Hide resolved
public: void SetScatterRatio(float _ratio);

/// \brief Get the pose of the particle emitter. This is the pose of the
/// emitter as specified in SDF
/// (<particle_emitter><pose> ... </pose></particle_emitter>).
Expand Down
10 changes: 10 additions & 0 deletions sdf/1.7/particle_emitter.sdf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@
</description>
</element>

<element name="particle_scatter_ratio" type="float" default="0.65" required="0">
<description>
This is used to determine the ratio of particles that will be detected
by sensors. Increasing the ratio means there is a higher chance of
particles reflecting and interfering with depth sensing, making the
emitter appear more dense. Decreasing the ratio decreases making it
appear less dense. This value should be > 0.
iche033 marked this conversation as resolved.
Show resolved Hide resolved
</description>
</element>

<include filename="pose.sdf" required="0"/>
<include filename="material.sdf" required="0"/>
</element>
23 changes: 23 additions & 0 deletions src/ParticleEmitter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ class sdf::ParticleEmitterPrivate
/// \brief Topic used to update particle emitter properties at runtime.
public: std::string topic = "";

/// \brief Particle scatter ratio. This is used to determine the ratio of
/// particles that will be detected by sensors. Increasing the ratio
/// means there is a higher chance of particles reflecting and interfering
/// with depth sensing, making the emitter appear more dense. Decreasing the
/// ratio decreases making it appear less dense. This value should be > 0.
iche033 marked this conversation as resolved.
Show resolved Hide resolved
public: float scatterRatio = 0.65f;

/// \brief Pose of the emitter
public: ignition::math::Pose3d pose = ignition::math::Pose3d::Zero;

Expand Down Expand Up @@ -136,6 +143,7 @@ ParticleEmitterPrivate::ParticleEmitterPrivate(
colorEnd(_private.colorEnd),
colorRangeImage(_private.colorRangeImage),
topic(_private.topic),
scatterRatio(_private.scatterRatio),
pose(_private.pose),
poseRelativeTo(_private.poseRelativeTo),
xmlParentName(_private.xmlParentName),
Expand Down Expand Up @@ -272,6 +280,9 @@ Errors ParticleEmitter::Load(ElementPtr _sdf)
this->dataPtr->topic = _sdf->Get<std::string>(
"topic", this->dataPtr->topic).first;

this->dataPtr->scatterRatio = _sdf->Get<float>(
"particle_scatter_ratio", this->dataPtr->scatterRatio).first;

if (_sdf->HasElement("material"))
{
this->dataPtr->material.reset(new sdf::Material());
Expand Down Expand Up @@ -487,6 +498,18 @@ void ParticleEmitter::SetTopic(const std::string &_topic)
this->dataPtr->topic = _topic;
}

/////////////////////////////////////////////////
void ParticleEmitter::SetScatterRatio(float _ratio)
{
this->dataPtr->scatterRatio = _ratio;
Copy link
Contributor

@adlarkin adlarkin May 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this->dataPtr->scatterRatio = _ratio;
if (_ratio <= 0.0f)
{
ignwarn << "Scatter ratio must be > 0, but a value of " << _ratio << " was given. Using a value of " << this->dataPtr->scatterRatio << " instead" << std::endl;
return;
}
this->dataPtr->scatterRatio = _ratio;

Since the scatter ratio must be > 0, should we only set it if the value passed in is > 0? If so, we should also update the documentation to indicate this in ParticleEmitter.hh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after thinking about it more, I removed the requirement on the value needs to be > 0. It's more of an implementation limitation on the ignition end. Since we should keep the spec as general as possible, I think someone could decide to use 0 to simulate 0 scattering for certain sensors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. Everything looks good to me, so feel free to merge unless you're waiting for someone else to look over this as well 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, will merge after CI builds are done

}

/////////////////////////////////////////////////
float ParticleEmitter::ScatterRatio() const
{
return this->dataPtr->scatterRatio;
}

/////////////////////////////////////////////////
const ignition::math::Pose3d &ParticleEmitter::RawPose() const
{
Expand Down
4 changes: 4 additions & 0 deletions src/ParticleEmitter_TEST.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ TEST(DOMParticleEmitter, Construction)
emitter.SetTopic("/test/topic");
EXPECT_EQ("/test/topic", emitter.Topic());

EXPECT_FLOAT_EQ(0.65f, emitter.ScatterRatio());
emitter.SetScatterRatio(0.5f);
EXPECT_FLOAT_EQ(0.5f, emitter.ScatterRatio());

EXPECT_EQ(ignition::math::Pose3d::Zero, emitter.RawPose());
emitter.SetRawPose(ignition::math::Pose3d(1, 2, 3, 0, 0, 1.5707));
EXPECT_EQ(ignition::math::Pose3d(1, 2, 3, 0, 0, 1.5707), emitter.RawPose());
Expand Down
1 change: 1 addition & 0 deletions test/integration/particle_emitter_dom.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ TEST(DOMWorld, LoadParticleEmitter)
EXPECT_DOUBLE_EQ(0.2, linkEmitter->MaxVelocity());
EXPECT_DOUBLE_EQ(0.5, linkEmitter->ScaleRate());
EXPECT_DOUBLE_EQ(5, linkEmitter->Rate());
EXPECT_FLOAT_EQ(0.2f, linkEmitter->ScatterRatio());

sdf::Material *mat = linkEmitter->Material();
ASSERT_NE(nullptr, mat);
Expand Down
1 change: 1 addition & 0 deletions test/sdf/world_complete.sdf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
</pbr>
</material>
<color_range_image>materials/textures/fogcolors.png</color_range_image>
<particle_scatter_ratio>0.2</particle_scatter_ratio>
</particle_emitter>

<light name="spot_light" type="spot">
Expand Down