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

Make ROS/ROS2 node name configurable via launch file #58

Merged
merged 3 commits into from
Apr 8, 2024

Conversation

lukaszanger
Copy link
Collaborator

Implementation to make ROS/ROS2 node name configurable via launch file.

To be considered when using ROS2:

The node name has to be considered here in the params file (e.g. params.ros2.yaml). Thus, when renaming the ROS2 node via the launch argument, a new config file has to be applied (with a modified entry considering the updated node name).

@lukaszanger lukaszanger requested a review from lreiher April 3, 2024 13:07
@lukaszanger lukaszanger self-assigned this Apr 3, 2024
Copy link
Member

@lreiher lreiher 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 please also add a second argument for the namespace?

And the ROS2 config file first line can be changed to "/**/*" such that it's usable independent of the actual node name.

@lukaszanger
Copy link
Collaborator Author

Can you please also add a second argument for the namespace?

And the ROS2 config file first line can be changed to "/**/*" such that it's usable independent of the actual node name.

Done. I configured mqtt_client as the default namespace. In ROS2, it would work to provide an empty string as default argument for namespace. In ROS1, an error is thrown. Alternative to the current implementation would be to only provide a non-empty default argument for namespace for ROS1 and an empty string for ROS2.

@lukaszanger
Copy link
Collaborator Author

Can you please also add a second argument for the namespace?
And the ROS2 config file first line can be changed to "/**/*" such that it's usable independent of the actual node name.

Done. I configured mqtt_client as the default namespace. In ROS2, it would work to provide an empty string as default argument for namespace. In ROS1, an error is thrown. Alternative to the current implementation would be to only provide a non-empty default argument for namespace for ROS1 and an empty string for ROS2.

Update on previous comment: Using "/" works for ROS1 to choose root as default namespace. This is implemented now.

@lreiher lreiher merged commit b0e6c06 into main Apr 8, 2024
18 checks passed
@lreiher lreiher deleted the feature/configure_node_name branch April 8, 2024 11:25
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

2 participants