Add a way to pass extra parameters to ros_gz_bridge #628

Amronos · 2024-10-30T07:34:24Z

🎉 New feature

Closes #624

Summary

Added a new argument bridge_params to allow users to pass extra parameters to ros_gz_bridge when using the launch file. An example of these parameters would be qos_overrides
I have changed the node declarations to an opaque function so that I can get the value of bridge_params.
bridge_params is converted from string to dictionary and then passed to the parameters of the ros_gz_bridge node.
The way I have done all of this is complicated and so please do tell me if there is an easier way to do this.

Test it

To test this something like the following command can be used:
ros2 launch ros_gz_bridge ros_gz_bridge.launch.py bridge_name:=ros_gz_bridge config_file:=<path_to_your_YAML_file> bridge_params:="'qos_overrides./topic_name.publisher.durability': 'transient_local', 'qos_overrides./another_topic_name.publisher.durability': 'transient_local'"

Then to check if the qos overrides have applied:
ros2 topic info /topic_name --verbose
ros2 topic info /another_topic_name --verbose

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.

caguero

There are some style errors. See https://github.com/gazebosim/ros_gz/actions/runs/11588839442/job/32263218643?pr=628

azeey · 2024-11-01T15:29:54Z

I haven't had time to dig into this, but it feels like a big code change for a fairly small feature. Is it the conversion from string to dictionary that requires the use of the opaque function? Another approach I would consider is moving more of the logic into the gz_server Action or even reverting the current architecture where the Action includes a launch file

ros_gz/ros_gz_bridge/ros_gz_bridge/actions/ros_gz_bridge.py

Lines 146 to 150 in 2658f27

    
           ros_gz_bridge_description = IncludeLaunchDescription( 
        
               PythonLaunchDescriptionSource( 
        
                   [PathJoinSubstitution([FindPackageShare('ros_gz_bridge'), 
        
                                          'launch', 
        
                                          'ros_gz_bridge.launch.py'])]),

and instead have the ros_gz_bridge.launch.py file use the Action. It seems like we have more flexibility there.

Amronos · 2024-11-01T16:44:39Z

Is it the conversion from string to dictionary that requires the use of the opaque function?

Yes.

or even reverting the current architecture where the Action includes a launch file and instead have the ros_gz_bridge.launch.py file use the Action. It seems like we have more flexibility there.

That does sound like a good idea.

Amronos · 2024-11-14T11:04:55Z

Any updates on this PR?

caguero

Could you merge from ros2. In particular, I want to make sure that this patch is consider in your refactor:

https://github.com/gazebosim/ros_gz/pull/620/files

caguero

Does this test work for you? (remember to update the path to the bridge config file).

ros2 launch ros_gz_sim ros_gz_sim.launch bridge_name:=ros_gz_bridge config_file:=/home/caguero/ros_gz_ws/src/ros_gz/ros_gz_bridge/test/config/full.yaml world_sdf_file:=empty.sdf use_composition:=True create_own_container:=True

Amronos · 2024-11-14T16:51:42Z

Does this test work for you? (remember to update the path to the bridge config file).

ros2 launch ros_gz_sim ros_gz_sim.launch bridge_name:=ros_gz_bridge config_file:=/home/caguero/ros_gz_ws/src/ros_gz/ros_gz_bridge/test/config/full.yaml world_sdf_file:=empty.sdf use_composition:=True create_own_container:=True

I haven't specifically run this command, but I have used the ros_gz_sim.launch.py launch file inside another python launch file with composition enabled.

caguero · 2024-11-14T17:01:48Z

Does this test work for you? (remember to update the path to the bridge config file).
ros2 launch ros_gz_sim ros_gz_sim.launch bridge_name:=ros_gz_bridge config_file:=/home/caguero/ros_gz_ws/src/ros_gz/ros_gz_bridge/test/config/full.yaml world_sdf_file:=empty.sdf use_composition:=True create_own_container:=True
I haven't specifically run this command, but I have used the ros_gz_sim.launch.py launch file inside another python launch file with composition enabled.

I'm getting an error that I don't get with the ros2 branch:

[INFO] [launch]: All log files can be found below /home/caguero/.ros/log/2024-11-14-17-40-26-288785-cold-1915140
[INFO] [launch]: Default logging verbosity is set to INFO
[ERROR] [launch]: Caught exception in launch (see debug for traceback): 'str' object has no attribute 'perform'

Allows one to pass extra arguments to ros_gz_bridge Signed-off-by: Aarav Gupta <[email protected]>

Signed-off-by: Aarav Gupta <[email protected]>

Signed-off-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Aarav Gupta <[email protected]>

Signed-off-by: Aarav Gupta <[email protected]>

…action using the launch file Signed-off-by: Aarav Gupta <[email protected]>

Signed-off-by: Aarav Gupta <[email protected]>

…arams Signed-off-by: Aarav Gupta <[email protected]>

Signed-off-by: Wiktor Bajor <[email protected]> Signed-off-by: Aarav Gupta <[email protected]>

Signed-off-by: Aarav Gupta <[email protected]>

Amronos · 2024-11-15T05:35:24Z

The XML launch file should work now.
Should I add the bridge_params argument to the other launch files?

caguero · 2024-11-15T15:44:24Z

The XML launch file should work now. Should I add the bridge_params argument to the other launch files?

That'd be great!

Signed-off-by: Aarav Gupta <[email protected]>

Amronos · 2024-11-15T16:18:54Z

Should be ready to merge now!

caguero

I think there's still something not working for me. If I run:

ros2 launch ros_gz_sim ros_gz_sim.launch.py bridge_name:=ros_gz_bridge config_file:=/home/caguero/ros_gz_ws/src/ros_gz/ros_gz_bridge/test/config/full.yaml world_sdf_file:=empty.sdf use_composition:=True create_own_container:=True

And then:

caguero@cold:~/ros_gz_ws$ ros2 node list
WARNING: Be aware that there are nodes in the graph that share an exact name, which can have unintended side effects.
/gz_server
/gzserver
/gzserver
/gzserver
/launch_ros_2128322
/ros_gz_bridge
/ros_gz_bridge
/ros_gz_container

It shows multiple instances of the bridge and gzserver. This issue was fixed in #620 but it's resurfacing here.

Signed-off-by: Aarav Gupta <[email protected]>

caguero · 2024-11-18T18:56:55Z

@Amronos , out of curiosity, were you able to solve the latest issue (duplicated nodes)?

Amronos · 2024-11-19T02:55:24Z

@Amronos , out of curiosity, were you able to solve the latest issue (duplicated nodes)?

Yes, it was fixed in commit f2d6cda. The PR should be ready to merge.

caguero

All looks good to me now, thanks!

Amronos · 2024-11-21T02:20:18Z

@caguero Can this also be backported to Jazzy? If yes, #629 should be backported first.

ahcorde · 2024-11-21T12:26:18Z

https://github.com/Mergifyio backport jazzy

mergify · 2024-11-21T12:26:43Z

backport jazzy

✅ Backports have been created

#648 Add a way to pass extra parameters to ros_gz_bridge (backport #628) has been created for branch jazzy

* Add bridge_params argument to ros_gz_bridge Signed-off-by: Aarav Gupta <[email protected]> Signed-off-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Wiktor Bajor <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> Co-authored-by: Wiktor Bajor <[email protected]> (cherry picked from commit 558a1cf)

* Add bridge_params argument to ros_gz_bridge Signed-off-by: Aarav Gupta <[email protected]> Signed-off-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Wiktor Bajor <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> Co-authored-by: Wiktor Bajor <[email protected]> (cherry picked from commit 558a1cf) Co-authored-by: Aarav Gupta <[email protected]>

Amronos requested a review from ahcorde as a code owner October 30, 2024 07:34

caguero requested changes Oct 30, 2024

View reviewed changes

Amronos requested a review from caguero October 30, 2024 09:30

caguero reviewed Nov 14, 2024

View reviewed changes

Amronos and others added 10 commits November 15, 2024 10:58

Add bridge_params argument to ros_gz_bridge.launch.py

e4dcd91

Allows one to pass extra arguments to ros_gz_bridge Signed-off-by: Aarav Gupta <[email protected]>

Pass tests

7cce8e5

Signed-off-by: Aarav Gupta <[email protected]>

Fix errors that happen when no parameters are passed

9fe6485

Signed-off-by: Aarav Gupta <[email protected]>

Changelog

8834dbc

Signed-off-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Aarav Gupta <[email protected]>

2.1.2

0b9ee84

Signed-off-by: Aarav Gupta <[email protected]>

For ros_gz_bridge make the launch file use the action instead of the …

8f289b9

…action using the launch file Signed-off-by: Aarav Gupta <[email protected]>

Fix style errors

8643b0e

Signed-off-by: Aarav Gupta <[email protected]>

Fix errors that happen when an empty dictionary is passed to bridge_p…

4e27320

…arams Signed-off-by: Aarav Gupta <[email protected]>

Add remove entity node (gazebosim#629)

9eb7e3e

Signed-off-by: Wiktor Bajor <[email protected]> Signed-off-by: Aarav Gupta <[email protected]>

Fix errors occuring when using XML launch files

fb01c8f

Signed-off-by: Aarav Gupta <[email protected]>

Amronos force-pushed the bridge-params branch from 50ad6ca to fb01c8f Compare November 15, 2024 05:29

Merge branch 'ros2' into bridge-params

5282373

Amronos requested a review from caguero November 15, 2024 05:35

Add bridge_params argument to other launch files

9aa4a6a

Signed-off-by: Aarav Gupta <[email protected]>

caguero reviewed Nov 15, 2024

View reviewed changes

Fix multiple instances of nodes getting created

f2d6cda

Signed-off-by: Aarav Gupta <[email protected]>

Amronos requested a review from caguero November 16, 2024 04:41

Use the ros_gz_bridge action in other launch files also

c60f810

Signed-off-by: Aarav Gupta <[email protected]>

Amronos mentioned this pull request Nov 17, 2024

Add documentation on QOS Overrides and the bridge_params argument gazebosim/docs#539

Open

9 tasks

ahcorde assigned Amronos Nov 18, 2024

caguero approved these changes Nov 20, 2024

View reviewed changes

caguero merged commit 558a1cf into gazebosim:ros2 Nov 20, 2024
4 checks passed

caguero mentioned this pull request Nov 20, 2024

Move gzserver logic to its action #646

Merged

8 tasks

Amronos deleted the bridge-params branch November 21, 2024 02:06

mergify bot mentioned this pull request Nov 21, 2024

Add a way to pass extra parameters to ros_gz_bridge (backport #628) #648

Merged

9 tasks

mergify bot mentioned this pull request Nov 21, 2024

Move gzserver logic to its action (backport #646) #649

Merged

8 tasks

caguero mentioned this pull request Nov 21, 2024

Error in launch file using <ros_gz_bridge> #650

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a way to pass extra parameters to ros_gz_bridge #628

Add a way to pass extra parameters to ros_gz_bridge #628

Amronos commented Oct 30, 2024 •

edited

Loading

caguero left a comment

azeey commented Nov 1, 2024

Amronos commented Nov 1, 2024

Amronos commented Nov 14, 2024

caguero left a comment

caguero left a comment •

edited

Loading

Amronos commented Nov 14, 2024 •

edited

Loading

caguero commented Nov 14, 2024

Amronos commented Nov 15, 2024 •

edited

Loading

caguero commented Nov 15, 2024

Amronos commented Nov 15, 2024

caguero left a comment

caguero commented Nov 18, 2024

Amronos commented Nov 19, 2024 •

edited

Loading

caguero left a comment

Amronos commented Nov 21, 2024 •

edited

Loading

ahcorde commented Nov 21, 2024

mergify bot commented Nov 21, 2024 •

edited

Loading

Add a way to pass extra parameters to ros_gz_bridge #628

Add a way to pass extra parameters to ros_gz_bridge #628

Conversation

Amronos commented Oct 30, 2024 • edited Loading

🎉 New feature

Summary

Test it

Checklist

caguero left a comment

Choose a reason for hiding this comment

azeey commented Nov 1, 2024

Amronos commented Nov 1, 2024

Amronos commented Nov 14, 2024

caguero left a comment

Choose a reason for hiding this comment

caguero left a comment • edited Loading

Choose a reason for hiding this comment

Amronos commented Nov 14, 2024 • edited Loading

caguero commented Nov 14, 2024

Amronos commented Nov 15, 2024 • edited Loading

caguero commented Nov 15, 2024

Amronos commented Nov 15, 2024

caguero left a comment

Choose a reason for hiding this comment

caguero commented Nov 18, 2024

Amronos commented Nov 19, 2024 • edited Loading

caguero left a comment

Choose a reason for hiding this comment

Amronos commented Nov 21, 2024 • edited Loading

ahcorde commented Nov 21, 2024

mergify bot commented Nov 21, 2024 • edited Loading

✅ Backports have been created

Amronos commented Oct 30, 2024 •

edited

Loading

caguero left a comment •

edited

Loading

Amronos commented Nov 14, 2024 •

edited

Loading

Amronos commented Nov 15, 2024 •

edited

Loading

Amronos commented Nov 19, 2024 •

edited

Loading

Amronos commented Nov 21, 2024 •

edited

Loading

mergify bot commented Nov 21, 2024 •

edited

Loading