Bridge as a gz sim system #501

caguero · 2024-02-21T21:29:23Z

🎉 New feature

Summary

This PR runs the ROS<->Gz bridge as a gz sim system.

Test it

ros2 launch ros_gz_sim_demos ros_gz.launch.py

Verify the output to confirm that two topics are bridged:

[ruby $(which ign) gazebo-1] [INFO] [1708634732.070359776] [ros_gz_bridge]: Creating ROS->GZ Bridge: [ros_chatter (std_msgs/msg/String) -> gz_chatter (ignition.msgs.StringMsg)] (Lazy 1)
[ruby $(which ign) gazebo-1] [INFO] [1708634732.071384449] [ros_gz_bridge]: Creating GZ->ROS Bridge: [gz_chatter (ignition.msgs.StringMsg) -> ros_chatter (std_msgs/msg/String)] (Lazy 0)

Then, you should see a ROS topic /ros_chatter:

caguero@cold:~/ros_gz_ws$ ros2 topic list
/parameter_events
/ros_chatter
/rosout

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.

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

ahcorde · 2024-02-21T21:34:24Z

ros_gz_sim/CMakeLists.txt

@@ -12,6 +12,8 @@ endif()

 find_package(ament_cmake REQUIRED)
 find_package(rclcpp REQUIRED)
+find_package(rclcpp_components REQUIRED)


you don't need to include this dependency, with rclcpp should be enough, you are not adding any component

If I don't add it, the code doesn't compile. Maybe a dependency isn't exporting it correctly.

I think we determined that ros_gz_bridge isn't correctly exporting the dependency.

ros_gz_sim/src/ros_gz.cpp

ahcorde · 2024-02-21T21:35:26Z

ros_gz_sim/src/ros_gz.cpp

+  }
+
+  // Sanity check: Make sure that the config file exists and it's a file.
+  std::filesystem::path configFile = _sdf->Get<std::string>("config_file");


include <filesystem>

That was already there :)

ahcorde · 2024-02-21T21:35:44Z

ros_gz_sim/src/ros_gz.cpp

+  // Sanity check: Make sure that the config file exists and it's a file.
+  std::filesystem::path configFile = _sdf->Get<std::string>("config_file");
+  if (!std::filesystem::is_regular_file(configFile)) {
+    std::cerr << "[" << configFile << "] is not a regular file. Plugin disabled"


include <iostream>

ahcorde · 2024-02-21T21:36:25Z

ros_gz_sim/include/ros_gz_sim/ros_gz.hpp

+    gz::sim::EventManager & _eventMgr) override;
+
+
+  /// \brief Private data pointer.
+  std::unique_ptr<ROSGzPluginPrivate> dataPtr;


Suggested change

gz::sim::EventManager & _eventMgr) override;

/// \brief Private data pointer.

std::unique_ptr<ROSGzPluginPrivate> dataPtr;

gz::sim::EventManager & _eventMgr) override;

/// \brief Private data pointer.

std::unique_ptr<ROSGzPluginPrivate> dataPtr;

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

ahcorde · 2024-02-23T15:35:08Z

ros_gz_image/CMakeLists.txt

@@ -14,6 +14,7 @@ find_package(ament_cmake REQUIRED)
 find_package(image_transport REQUIRED)
 find_package(ros_gz_bridge REQUIRED)
 find_package(rclcpp REQUIRED)
+find_package(rclcpp_components REQUIRED)


This PR should fix this issue #501

Signed-off-by: Michael Carroll <[email protected]>

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

mjcarroll · 2024-02-23T18:37:07Z

Ah sorry, there was a bad merge there. Thanks for fixing that.

azeey

This looks great! I have a few comments that I think will make this more consistent with the ROS ecosystem as well as existing naming schemes in Gazebo.

One question I have is how this deals with models spawned at runtime. Would the user be able to add another bridge system as part of the model?

azeey · 2024-03-13T15:08:28Z

ros_gz_sim_demos/worlds/ros_gz.sdf

+      filename="ignition-gazebo-physics-system"
+      name="gz::sim::systems::Physics">
+    </plugin>
+    <plugin
+      filename="ignition-gazebo-user-commands-system"
+      name="gz::sim::systems::UserCommands">
+    </plugin>
+    <plugin
+      filename="ignition-gazebo-scene-broadcaster-system"
+      name="gz::sim::systems::SceneBroadcaster">
+    </plugin>
+    <plugin
+      filename="ignition-gazebo-contact-system"
+      name="gz::sim::systems::Contact">


Use gz-sim instead of ignition-gazebo.

Changed in #511

azeey · 2024-03-13T15:09:17Z

ros_gz_sim_demos/config/full.yaml

+  gz_type_name: "ignition.msgs.StringMsg"
+  subscriber_queue: 5
+  publisher_queue: 6
+  lazy: true
+  direction: ROS_TO_GZ
+
+- ros_topic_name: "ros_chatter"
+  gz_topic_name: "gz_chatter"
+  ros_type_name: "std_msgs/msg/String"
+  gz_type_name: "ignition.msgs.StringMsg"


I believe we can use gz.msgs for the gz_type_name.

Changed in #511.

azeey · 2024-03-13T15:19:15Z

ros_gz_sim/src/ros_gz.cpp

+  std::string path = gz::common::findFile(filename);
+  if (!gz::common::isFile(path)) {
+    std::cerr << "Unable to open YAML file [" << filename
+              << "], check your GAZEBO_RESOURCE_PATH settings." << std::endl;


Do we use GAZEBO_RESOURCE_PATH? I thought that was for Gazebo-classic.

Sorry, that was a typo. Changed in #511.

azeey · 2024-03-13T15:25:34Z

ros_gz_sim_demos/worlds/ros_gz.sdf

+    <plugin name="ros_gz_sim::ROSGzPlugin"
+            filename="libROSGzPlugin.so">


Can we rename this to be name=ros_gz_sim::ROSGzBridge and filename=ros-gz-bridge-system to have bridge in the name and to be more consistent with our other systems (eg. we haven't used Plugin in any of our existing systems). For the filename, the shared library would need to be libros-gz-bridge-system.so. In the <plugin> tag the lib and .so can be dropped.

Updated in #511.

azeey · 2024-03-13T15:35:24Z

ros_gz_sim/src/ros_gz.cpp

+
+  // Sanity check: Make sure that the config file exists and it's a file.
+  std::string filename = _sdf->Get<std::string>("config_file");
+  std::string path = gz::common::findFile(filename);


I think it would be better to use package:// URIs for the file name since this is all happening in the ROS ecosystem. Then, instead of calling gz::common::findFile, we can pass the file directly to RosGzBridge (so no need to call findFile here) and update RosGzBridge so it can use resource_retriever to find the file.

resource_retriever is functional but the API is slightly different because as far as I know it can't resolve the path only. Instead, it resolves the path and read the content of the file, returning it as a byte array.

However I figured that we still can use the package:// approach. I guess findFile() is solving it for us?

Yes, the API is different. That's why I was suggesting to pass the filename directly to RosGzBridge and use resource_retriever there where we are currently opening the file:

ros_gz/ros_gz_bridge/src/bridge_config.cpp

Line 176 in ef9cd5c

std::ifstream in(filename);

I don't know how common::findFile is handling package://. My guess is it ignores the package:// part and tries to find the path based on Gazebo environment variables, which is not what we want.

In any case I can tackle it in a separate PR as it affects mainly ros_gz_bridge. I already played with it.

caguero · 2024-03-15T15:06:48Z

Replaced with #511 targeting ROS 2 rolling.

caguero added 17 commits February 16, 2024 21:41

Bridge as a gz system

5d3d406

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

Extra files

2882c87

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

Rename

4921a07

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

Tweaks

9ddd652

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

Tweaks

758aa21

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

Tweaks

f1f5fa0

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

Tweaks

be153b6

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

Tweaks

ac76877

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

Tests

3bc36bd

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

Manual PIMPL

6f05a12

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

Include

ebb6cab

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

no gzerr

bdcc825

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

tweak

42aa826

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

tweak

8d767e9

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

tweak

49c8811

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

tweak

e2a5db0

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

tweak

a195207

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

ahcorde requested changes Feb 21, 2024

View reviewed changes

Tweaks

fe132a1

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

caguero requested a review from ahcorde February 22, 2024 17:21

caguero added 4 commits February 22, 2024 20:44

Find yaml file.

8c5b845

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

Tweak

0ff8633

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

Tweak

161f8ee

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

Style

ddf084c

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

ahcorde reviewed Feb 23, 2024

View reviewed changes

mjcarroll and others added 4 commits February 23, 2024 10:12

Merge branch 'humble' into caguero/ros_gz_system

62b212c

Signed-off-by: Michael Carroll <[email protected]>

Tweak

15cbccb

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

Tweak

93a0836

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

Tweak

f232b4b

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

caguero marked this pull request as ready for review February 23, 2024 17:35

azeey reviewed Mar 13, 2024

View reviewed changes

caguero closed this Mar 15, 2024

caguero mentioned this pull request Mar 20, 2024

Use resource_retriever in the bridge #515

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bridge as a gz sim system #501

Bridge as a gz sim system #501

caguero commented Feb 21, 2024 •

edited

Loading

ahcorde Feb 21, 2024

caguero Feb 22, 2024

mjcarroll Feb 22, 2024

caguero Feb 23, 2024

ahcorde Feb 21, 2024

caguero Feb 22, 2024

ahcorde Feb 21, 2024

caguero Feb 22, 2024

ahcorde Feb 21, 2024

caguero Feb 22, 2024

ahcorde Feb 23, 2024

caguero Feb 23, 2024

mjcarroll commented Feb 23, 2024

azeey left a comment

azeey Mar 13, 2024

caguero Mar 15, 2024

azeey Mar 13, 2024

caguero Mar 15, 2024

azeey Mar 13, 2024

caguero Mar 15, 2024

azeey Mar 13, 2024

caguero Mar 15, 2024

azeey Mar 13, 2024

caguero Mar 15, 2024 •

edited

Loading

azeey Mar 15, 2024

caguero Mar 15, 2024

caguero Mar 20, 2024

caguero commented Mar 15, 2024

		<plugin name="ros_gz_sim::ROSGzPlugin"
		filename="libROSGzPlugin.so">

Bridge as a gz sim system #501

Bridge as a gz sim system #501

Conversation

caguero commented Feb 21, 2024 • edited Loading

🎉 New feature

Summary

Test it

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjcarroll commented Feb 23, 2024

azeey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caguero Mar 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caguero commented Mar 15, 2024

caguero commented Feb 21, 2024 •

edited

Loading

caguero Mar 15, 2024 •

edited

Loading