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

Jump to bottom

Particle system - Part2 #562

Merged
merged 16 commits into from
Feb 18, 2021
Merged

Particle system - Part2 #562

merged 16 commits into from
Feb 18, 2021

Conversation

caguero
Copy link
Contributor

@caguero caguero commented Jan 15, 2021 •
edited
Loading

It depends on gazebosim/gz-msgs#127 .

This pull request introduces the particle emitter system accepting SDF. It should parse the SDF properly and create the appropriate components.

@caguero
Parsin SDF and component creation with tests.
71ea2b9 
Signed-off-by: Carlos Agüero <[email protected]>
@caguero
Update test
23d0c62 
Signed-off-by: Carlos Agüero <[email protected]>
@caguero
Tweaks
9374df7 
Signed-off-by: Carlos Agüero <[email protected]>
@caguero
Doc
e0510a2 
Signed-off-by: Carlos Agüero <[email protected]>
@caguero caguero requested a review from chapulina as a code owner January 15, 2021 16:52
@chapulina chapulina added 🔮 dome Ignition Dome enhancement New feature or request rendering Involves Ignition Rendering labels Jan 15, 2021
chapulina
chapulina reviewed Jan 15, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Just some preliminary comments.

src/systems/CMakeLists.txt Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.cc Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.cc Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.hh Outdated Show resolved Hide resolved
test/worlds/particle_emitter.sdf Outdated Show resolved Hide resolved
test/worlds/particle_emitter.sdf Show resolved Hide resolved
iche033
iche033 reviewed Jan 20, 2021
View reviewed changes
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

I think you still need to create the PR introducing the new particle emitter proto msg in ign-msgs?

test/worlds/particle_emitter.sdf Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.cc Outdated Show resolved Hide resolved
@caguero
Copy link
Contributor Author

caguero commented Jan 29, 2021

I think you still need to create the PR introducing the new particle emitter proto msg in ign-msgs?

It's now ready in gazebosim/gz-msgs#127.

@caguero
Unique name if name not provided.
ea61288 
Signed-off-by: Carlos Agüero <[email protected]>
@caguero
Document default values.
10988c9 
Signed-off-by: Carlos Agüero <[email protected]>
@adlarkin adlarkin self-requested a review February 8, 2021 22:34
adlarkin
adlarkin reviewed Feb 8, 2021
View reviewed changes
src/systems/particle_emitter/ParticleEmitter.hh Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.hh Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.hh Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.cc Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.cc Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.cc Show resolved Hide resolved
test/worlds/particle_emitter.sdf Outdated Show resolved Hide resolved
test/worlds/particle_emitter.sdf Outdated Show resolved Hide resolved
test/integration/particle_emitter.cc Outdated Show resolved Hide resolved
@iche033
allow renaming
62646ea 
Signed-off-by: Ian Chen <[email protected]>
@iche033
remove unique test dir env
ec532c9 
Signed-off-by: Ian Chen <[email protected]>
adlarkin
adlarkin reviewed Feb 17, 2021
View reviewed changes
Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

@iche033 I left a few comments about the allow_renaming functionality.

src/systems/particle_emitter/ParticleEmitter.hh Outdated Show resolved Hide resolved
src/systems/particle_emitter/ParticleEmitter.cc Show resolved Hide resolved
@nkoenig nkoenig mentioned this pull request Feb 17, 2021
Merged
7 tasks
caguero and others added 2 commits February 17, 2021 20:44
@caguero @adlarkin
Particle system - Part3 (#593)
18b0194 
Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Ashton Larkin <[email protected]>

Signed-off-by: Carlos Agüero <[email protected]>
@adlarkin
update note about adding material check to integration test
2a6549e 
Signed-off-by: Ashton Larkin <[email protected]>
@adlarkin adlarkin force-pushed the particle_system_part2 branch from db0a633 to 2a6549e Compare February 18, 2021 01:45
@adlarkin adlarkin merged commit fdb02c2 into particle_system Feb 18, 2021
@adlarkin adlarkin deleted the particle_system_part2 branch February 18, 2021 01:49
adlarkin added a commit that referenced this pull request Feb 18, 2021
@caguero @adlarkin @iche033
Particle system - Part2 (#562)
e77e376 
Signed-off-by: Carlos Agüero <[email protected]>

Co-authored-by: Ashton Larkin <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
nkoenig added a commit that referenced this pull request Feb 18, 2021
@nkoenig @adlarkin @iche033
Particle system - Part 1 (#516)
deddb6f 
* New ParticleEmitter component.

Signed-off-by: Carlos Agüero <[email protected]>

* Remove attribute name.

Signed-off-by: Carlos Agüero <[email protected]>

* Update particle emitter component to use ignition::msgs::ParticleEmitter.

Signed-off-by: Carlos Agüero <[email protected]>

* Simplify particle emitter component.

Signed-off-by: Carlos Agüero <[email protected]>

* Particle system - Part2 (#562)

Signed-off-by: Carlos Agüero <[email protected]>

Co-authored-by: Ashton Larkin <[email protected]>
Co-authored-by: Ian Chen <[email protected]>

* bump msgs version to 6.3.0

Signed-off-by: Ian Chen <[email protected]>

* Remove extra main

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

* Added missing <set>

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

Co-authored-by: Ashton Larkin <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Ashton Larkin <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
nkoenig pushed a commit that referenced this pull request Nov 8, 2021
@maryaB-osr
Merged in maryaB/pointcloudtutorial (pull request #562)
343b99a 
Point cloud to mesh tutorial

Approved-by: Ian Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adlarkin adlarkin adlarkin left review comments

@chapulina chapulina chapulina left review comments

@iche033 iche033 iche033 left review comments

Assignees
No one assigned
Labels
🔮 dome Ignition Dome enhancement New feature or request rendering Involves Ignition Rendering
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@caguero @iche033 @chapulina @adlarkin