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

Change an entities visual material color by topic. #2286

Merged
merged 12 commits into from
Jan 25, 2024
Merged

Change an entities visual material color by topic. #2286

merged 12 commits into from
Jan 25, 2024

Conversation

bperseghetti
Copy link
Member

@bperseghetti bperseghetti commented Jan 14, 2024 •
edited
Loading

🎉 New feature

Summary

This change allows for a simpler mapping from MaterialColor msg in gz to the much more cumbersome Visual msg. By doing this and adding a topic listener it allows users to rapidly change the colors of a entities visual material color components, including from the ROS side using the gz bridge. This is especially useful for simulating status lighting.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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.

@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Jan 14, 2024
This was referenced Jan 14, 2024
Merged
Merged
@bperseghetti bperseghetti requested a review from ahcorde January 16, 2024 05:30
@ahcorde ahcorde added the needs upstream release Blocked by a release of an upstream library label Jan 16, 2024
@bperseghetti bperseghetti force-pushed the pr-material_color branch 3 times, most recently from 43578d8 to fc67f1d Compare January 17, 2024 04:36
@azeey azeey mentioned this pull request Jan 17, 2024
Merged
7 tasks
@azeey azeey mentioned this pull request Jan 18, 2024
Merged
7 tasks
@azeey
Copy link
Contributor

azeey commented Jan 18, 2024

@osrf-jenkins run tests please

@bperseghetti bperseghetti force-pushed the pr-material_color branch 2 times, most recently from 7511b61 to 7e62928 Compare January 19, 2024 15:08
@bperseghetti
Copy link
Member Author

@osrf-jenkins run tests please

bperseghetti and others added 5 commits January 19, 2024 22:49
@bperseghetti
Change an entities visual material color by topic.
d49b820 
This change allows for a simpler mapping from MaterialColor msg
to the much more cumbersome Visual msg, allong for users to
rapidly change the colors of a entities visual material color
components. This is especially useful for simulating status lighting.

Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti @ahcorde
Update src/systems/user_commands/UserCommands.cc
9ffb05c 
Co-authored-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti
Add UserCommands MaterialColor test.
0d153bd 
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti
Use MaterialColor Apply Enum and Entity.
1cba71d 
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti
Update MaterialCommand to use EntityMatch.
f8f23f9 
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti
MaterialColor test world and line fix.
0e23d42 
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti bperseghetti requested a review from ahcorde January 20, 2024 03:55
@codecov Codecov
Copy link

codecov bot commented Jan 20, 2024 •
edited
Loading

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (cc4914a) 65.79% compared to head (22f1c02) 65.92%.
Report is 32 commits behind head on gz-sim8.

Files Patch % Lines
src/systems/user_commands/UserCommands.cc 82.89% 13 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           gz-sim8    #2286      +/-   ##
===========================================
+ Coverage    65.79%   65.92%   +0.13%     
===========================================
  Files          327      327              
  Lines        31212    31285      +73     
===========================================
+ Hits         20536    20625      +89     
+ Misses       10676    10660      -16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bperseghetti
MaterialColor increase codecov.
60cace1 
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti bperseghetti mentioned this pull request Jan 22, 2024
Merged
8 tasks
@azeey azeey mentioned this pull request Jan 22, 2024
Merged
7 tasks
@azeey
Copy link
Contributor

azeey commented Jan 22, 2024

@osrf-jenkins run tests please

@azeey azeey removed the needs upstream release Blocked by a release of an upstream library label Jan 22, 2024
azeey
azeey requested changes Jan 22, 2024
View reviewed changes
Copy link
Contributor

@azeey azeey 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 the main issue is that the ECM is being used from a transport thread. Otherwise, just minor issues.

src/systems/user_commands/UserCommands.cc Outdated Show resolved Hide resolved
src/systems/user_commands/UserCommands.cc Outdated Show resolved Hide resolved
src/systems/user_commands/UserCommands.cc Outdated Show resolved Hide resolved
test/integration/user_commands.cc Show resolved Hide resolved
test/integration/user_commands.cc Show resolved Hide resolved
bperseghetti and others added 2 commits January 22, 2024 16:04
@bperseghetti @azeey
Update src/systems/user_commands/UserCommands.cc
ed1b31c 
Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti
Update user_commands test check all spheres.
89a6367 
Signed-off-by: Benjamin Perseghetti <[email protected]>
bperseghetti and others added 2 commits January 25, 2024 08:42
@bperseghetti @azeey
Update UserCommands MaterialColor to thread safe method.
15ecb18 
Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti
UserCommands fix whitespace.
b5e08dc 
Signed-off-by: Benjamin Perseghetti <[email protected]>
@bperseghetti bperseghetti requested a review from azeey January 25, 2024 14:46
@bperseghetti
Add shininess TODO.
22f1c02 
Signed-off-by: Benjamin Perseghetti <[email protected]>
azeey
azeey approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Thanks for iterating!

@azeey
Copy link
Contributor

azeey commented Jan 25, 2024

@ahcorde mind taking a quick look?

@bperseghetti
Copy link
Member Author

Thanks for iterating!

Thanks for the code review!

@azeey azeey merged commit 08b5cfe into gazebosim:gz-sim8 Jan 25, 2024
8 of 10 checks passed
@bperseghetti bperseghetti deleted the pr-material_color branch January 26, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahcorde ahcorde ahcorde approved these changes

@azeey azeey azeey approved these changes

@mjcarroll mjcarroll Awaiting requested review from mjcarroll mjcarroll is a code owner

Assignees

@bperseghetti bperseghetti

Labels
🎵 harmonic Gazebo Harmonic
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bperseghetti @azeey @ahcorde