Dynamic ros gz bridges - #910
Conversation
Signed-off-by: Davide Iafrate <dvde.iafrate98@gmail.com>
Generated-by: OpenAI Codex (GPT-5) Signed-off-by: Davide Iafrate <dvde.iafrate98@gmail.com>
497e09e to
2be4169
Compare
|
@C88-YQ will be reviewing this |
| ASSERT_TRUE(gzNode.Subscribe<gz::msgs::StringMsg>( | ||
| topic, | ||
| [&mutex, &gzMessages](const gz::msgs::StringMsg & _msg) | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
| gzMessages.push_back(_msg.data()); | ||
| })); |
There was a problem hiding this comment.
| ASSERT_TRUE(gzNode.Subscribe<gz::msgs::StringMsg>( | |
| topic, | |
| [&mutex, &gzMessages](const gz::msgs::StringMsg & _msg) | |
| { | |
| std::lock_guard<std::mutex> lock(mutex); | |
| gzMessages.push_back(_msg.data()); | |
| })); | |
| std::function<void(const gz::msgs::StringMsg &)> gzCallback = | |
| [&mutex, &gzMessages](const gz::msgs::StringMsg & _msg) | |
| { | |
| std::lock_guard<std::mutex> lock(mutex); | |
| gzMessages.push_back(_msg.data()); | |
| }; | |
| ASSERT_TRUE(gzNode.Subscribe(topic, gzCallback)); |
I hit a build failure with gz-transport16 here. The migration notes mention that Node::Subscribe changed to a forwarding wrapper in newer gz-transport versions: Migration.md.
Perhaps we can change it like this to make it compatible across different gz-transport versions
There was a problem hiding this comment.
Good catch, I was mistakenly building against Jetty
There was a problem hiding this comment.
The regression is tracked in gazebosim/gz-transport#880
| bool create_dynamic_bridges = false; | ||
| bridge_node->get_parameter("create_dynamic_bridges", create_dynamic_bridges); | ||
| if (filteredArgs.empty() && !create_dynamic_bridges) { | ||
| usage(); | ||
| return -1; | ||
| } | ||
|
|
There was a problem hiding this comment.
This change seems to break the existing config-file-only usage:
ros2 run ros_gz_bridge parameter_bridge --ros-args -p config_file:=$WORKSPACE/ros_gz/ros_gz_bridge/test/config/full.yamlThere was a problem hiding this comment.
Adding a check for empty config file string.
| const auto direction_str = this->get_parameter("dynamic_bridge_direction").as_string(); | ||
| const auto direction = parse_bridge_direction(direction_str); |
There was a problem hiding this comment.
I wonder if we should consider determining the bridge direction automatically instead. Since Gazebo and ROS topics may have different directions depending on the specific topic, relying on a single dynamic_bridge_direction for all topics might not be flexible enough.
There was a problem hiding this comment.
We could potentially infer the direction based on the publishers and subscribers of each topic. For example, if a topic only has a publisher on the Gazebo side, it could be bridged as GZ_TO_ROS; if it only has subscribers on the Gazebo side, it could be ROS_TO_GZ; and if both exist, we could fall back to BIDIRECTIONAL for now.
There was a problem hiding this comment.
Do you suggest to automatically bridge all compatible ROS topics to Gazebo?
There was a problem hiding this comment.
Or just supporting a mix of GZ_TO_ROS and ROS_TO_GZ based on the gazebo topic direction?
| std::set<std::string> gz_type_names; | ||
| for (const auto & publisher : publishers) { | ||
| const auto gz_type_name = publisher.MsgTypeName(); | ||
| if (!gz_type_name.empty()) { | ||
| gz_type_names.insert(gz_type_name); | ||
| } | ||
| } |
There was a problem hiding this comment.
It looks like the current implementation only supports automatically bridging topics published from Gazebo, while topics subscribed by Gazebo are not handled in the same way. This might be another case we need to consider.
There was a problem hiding this comment.
We could potentially apply the same logic to the subscribers information from TopicInfo and get the gz_type_name from there as well.
| bool lazy; | ||
| this->get_parameter("lazy", lazy); |
There was a problem hiding this comment.
I think the default lazy state for dynamically created bridges should be true. Otherwise, it may cause unnecessary overhead, especially when there are many topics.
🎉 New feature
Closes #849
Summary
Adds opt-in dynamic discovery of Gazebo Transport topics to
ros_gz_bridge.When
create_dynamic_bridgesis enabled, the bridge periodically:TopicListandTopicInfo.Dynamic bridges default to
GZ_TO_ROS. The direction can be changed usingdynamic_bridge_direction, includingBIDIRECTIONAL.parameter_bridgecan now run without positional topic arguments when dynamicbridging is enabled.
Usage
Publish from Gazebo:
gz topic -t /hello_dynamic_bridge \ -m gz.msgs.StringMsg \ -p 'data: "hello from Gazebo"'Observe from ROS:
ros2 topic echo /hello_dynamic_bridge std_msgs/msg/StringTests
Added launch integration tests covering:
The tests use separate
GZ_PARTITIONandROS_DOMAIN_IDvalues.Local result:
Backport Policy
This is a Jetty roadmap feature introducing new parameters and behavior.
Checklist
codecheckpassedGenerated-bytrailers are includedNote to maintainers: Please use Squash-Merge and retain the
Signed-off-byandGenerated-bytrailers.