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.

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 option to change material color from ROS. #486

Merged
merged 21 commits into from
Feb 1, 2024

Conversation

bperseghetti
Copy link
Collaborator

@bperseghetti bperseghetti commented Jan 14, 2024

🎉 New feature

Summary

Allows for setting a GZ entities visual material color from ROS. This is highly valuable for creating noticeable LED status lighting. And depends on associated gz-msgs.

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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.

This allows bridging MaterialColor from ROS to GZ and is
important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

can you add a test?

ros_gz_interfaces/msg/MaterialColor.msg Outdated Show resolved Hide resolved
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
@bperseghetti
Copy link
Collaborator Author

bperseghetti commented Jan 15, 2024

can you add a test?

Yes, I was planning to wait and make sure this change was desired in upstream before spending any extra time to add a test.
Also to make testing work properly with CI will need to get gazebosim/gz-msgs#414 in first anyhow. This was more just to show the interdependent changes.

@bperseghetti
Copy link
Collaborator Author

@ahcorde Test added.

Test for the MaterialColor message ROS<->GZ.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
@ahcorde ahcorde added the needs upstream release Blocked by a release of an upstream library label Jan 16, 2024
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

you included some changes related with renaming ign to gz. Do you mind to remove them?

@bperseghetti
Copy link
Collaborator Author

you included some changes related with renaming ign to gz. Do you mind to remove them?

So that readme is in the ros_gz_bridge instead of the ros_ign_bridge, should it not be changed to match the naming?

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
@azeey
Copy link
Contributor

azeey commented Jan 17, 2024

So that readme is in the ros_gz_bridge instead of the ros_ign_bridge, should it not be changed to match the naming?

I think we should stick with ign since Humble is officially paired with Fortress. It's confusing for users since trying to use gz doesn't work in Humble/Fortress.

@bperseghetti
Copy link
Collaborator Author

So that readme is in the ros_gz_bridge instead of the ros_ign_bridge, should it not be changed to match the naming?

I think we should stick with ign since Humble is officially paired with Fortress. It's confusing for users since trying to use gz doesn't work in Humble/Fortress.

But to be clear this is not the main README but this is the one inside ros_gz_bridge. The one inside ros_ign_bridge should just say it's a shim on the other and have ign listed in it, right?

I guess I should just swap them.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
@azeey
Copy link
Contributor

azeey commented Jan 17, 2024

But to be clear this is not the main README but this is the one inside ros_gz_bridge. The one inside ros_ign_bridge should just say it's a shim on the other and have ign listed in it, right?

https://github.com/gazebosim/ros_gz/tree/humble/ros_ign_bridge does say it's a shim

@bperseghetti
Copy link
Collaborator Author

But to be clear this is not the main README but this is the one inside ros_gz_bridge. The one inside ros_ign_bridge should just say it's a shim on the other and have ign listed in it, right?

https://github.com/gazebosim/ros_gz/tree/humble/ros_ign_bridge does say it's a shim

Well I did also just change that so you know it's a shim but it has different interface and that gives you the interface of it.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
azeey added a commit to gazebosim/gz-sim that referenced this pull request Jan 25, 2024
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](gazebosim/ros_gz#486). This is especially useful for simulating status lighting.


---------

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Co-authored-by: Addisu Z. Taddese <addisuzt@intrinsic.ai>
@bperseghetti
Copy link
Collaborator Author

@osrf-jenkins run tests please

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
@azeey
Copy link
Contributor

azeey commented Jan 26, 2024

This PR is targeting humble which needs to work with Fortress, but the gz-msgs and gz-sim that landed recently all targeted Harmonic. I recommend retargeting this to rolling.

@azeey
Copy link
Contributor

azeey commented Jan 26, 2024

Alternatively, you can do something like

// Dataframe is available from versions 8.4.0 (fortress) forward
// This can be removed when the minimum supported version passes 8.4.0
#if (IGNITION_MSGS_MAJOR_VERSION > 8) || \
((IGNITION_MSGS_MAJOR_VERSION == 8) && (IGNITION_MSGS_MINOR_VERSION >= 4))
#define HAVE_DATAFRAME true
#endif
#if (GZ_MSGS_MAJOR_VERSION > 8) || \
((GZ_MSGS_MAJOR_VERSION == 8) && (GZ_MSGS_MINOR_VERSION >= 4))
#define HAVE_DATAFRAME true
#endif

Co-authored-by: Addisu Z. Taddese <addisuzt@intrinsic.ai>
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
@azeey azeey removed the needs upstream release Blocked by a release of an upstream library label Feb 1, 2024
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
azeey and others added 3 commits January 31, 2024 20:28
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Co-authored-by: Addisu Z. Taddese <addisuzt@intrinsic.ai>
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

linters

"ros_gz_bridge" "--output-file" "/github/home/ws/build/ros_gz_bridge/ament_flake8/flake8.txt" "--command" "/opt/ros/rolling/bin/ament_flake8" "--xunit-file" "/github/home/ws/build/ros_gz_bridge/test_results/ros_gz_bridge/flake8.xunit.xml"
2024-02-01T03:46:02.0622501Z 10: Test timeout computed to be: 60
2024-02-01T03:46:02.1111404Z 10: -- run_test.py: invoking following command in '/github/home/ws/src/ros_gz/ros_gz_bridge':
2024-02-01T03:46:02.1112566Z 10:  - /opt/ros/rolling/bin/ament_flake8 --xunit-file /github/home/ws/build/ros_gz_bridge/test_results/ros_gz_bridge/flake8.xunit.xml
2024-02-01T03:46:02.4909897Z 10: from ros_gz_bridge.mappings import MAPPINGS, MAPPINGS_8_4_0, MAPPINGS_10_1_0
2024-02-01T03:46:02.4911413Z 10: ^
2024-02-01T03:46:02.4913079Z 10: 1     I101 Imported names are in the wrong order. Should be MAPPINGS, MAPPINGS_10_1_0, MAPPINGS_8_4_0
2024-02-01T03:46:02.4914637Z 10: 
2024-02-01T03:46:02.4916618Z 10: ./ros_gz_bridge/__init__.py:19:1: I101 Imported names are in the wrong order. Should be MAPPINGS, MAPPINGS_10_1_0, MAPPINGS_8_4_0
2024-02-01T03:46:02.4918587Z 10: 
2024-02-01T03:46:02.4919517Z 10: 
2024-02-01T03:46:02.4921177Z 10: 5 files checked
2024-02-01T03:46:02.4922615Z 10: 1 errors
2024-02-01T03:46:02.4924329Z 10: 
2024-02-01T03:46:02.4926578Z 10: 'I'-type errors: 1
2024-02-01T03:46:02.4928273Z 10: 
2024-02-01T03:46:02.4929977Z 10: Checked files:

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Co-authored-by: Addisu Z. Taddese <addisuzt@intrinsic.ai>
Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

@azeey any other comment?

@azeey azeey merged commit b5dffde into gazebosim:humble Feb 1, 2024
9 checks passed
@azeey
Copy link
Contributor

azeey commented Feb 1, 2024

@bperseghetti could you please cherry-pick this into iron and ros2 branches?

@bperseghetti
Copy link
Collaborator Author

@bperseghetti could you please cherry-pick this into iron and ros2 branches?

Yep, I can do it later today!

@ahcorde
Copy link
Collaborator

ahcorde commented Feb 19, 2024

friendly ping @bperseghetti do you have any plans to forward port these changes?

@bperseghetti
Copy link
Collaborator Author

Sorry, my bad, I let this one slip through the cracks on my side, will create the PRs after my meetings today.

bperseghetti added a commit to rudislabs/ros_gz that referenced this pull request Mar 27, 2024
Forward port of gazebosim#486

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
bperseghetti added a commit to rudislabs/ros_gz that referenced this pull request Mar 27, 2024
Forward port of gazebosim#486

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
bperseghetti added a commit to rudislabs/ros_gz that referenced this pull request Mar 27, 2024
Forward port of gazebosim#486

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
bperseghetti added a commit to rudislabs/ros_gz that referenced this pull request Mar 27, 2024
Forward port of gazebosim#486.

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
@bperseghetti
Copy link
Collaborator Author

friendly ping @bperseghetti do you have any plans to forward port these changes?

Randomly remembered to do this, sorry about the dealy, please see associated "forward feature ports":
#520
#521

ahcorde pushed a commit that referenced this pull request Mar 27, 2024
Forward port of #486

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
azeey pushed a commit that referenced this pull request May 6, 2024
Forward port of #486.

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
mergify bot pushed a commit that referenced this pull request Jun 25, 2024
Forward port of #486.

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
(cherry picked from commit 78dc482)
azeey pushed a commit that referenced this pull request Jun 25, 2024
Forward port of #486.

    * Message and bridge for MaterialColor.

    This allows bridging MaterialColor from ROS to GZ and is
    important for allowing simulation users to create status lights.

Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
(cherry picked from commit 78dc482)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants