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

fix usage of ROS namespaces #39

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

damien-robotsix
Copy link
Contributor

Currently the library do not take into account the namespace if specified. The micro DDS client allows in PX4 to configure a custom namespace (multiple drones on the same network). To reflect this, it would be good to also allow this library to use custom namespaces. Removing "/" in front of topic definitions solve this. The topics will reflect the chosen namespace for the node.

@bkueng
Copy link
Collaborator

bkueng commented Jun 5, 2024

There's already a topic_namespace_prefix for that. You can pass that to the Mode's constructor.
That said I'm not sure whether that's the best solution though.

@damien-robotsix
Copy link
Contributor Author

damien-robotsix commented Jun 5, 2024

Ho, my bad, I was too fast on this, I didn't see the topic prefix in the constructor.

However that breaks the ability to set the namespace using ROS arguments when running the mode Node.

Anyway both approaches are compatible. With this PR, you can:

  • use "/" as prefix to avoid the node namespace to be propagated on the /fmu/...
  • use "/blabla/" to enforce the namespace for the fmu topics
  • use no prefix to get the node namespace propagated to the fmu topics

How does that sound?

@bkueng
Copy link
Collaborator

bkueng commented Jun 6, 2024

However that breaks the ability to set the namespace using ROS arguments when running the mode Node.

That is correct. It's separate, as someone might want to write a node that controls/consumes telemetry from multiple vehicles.

Anyway both approaches are compatible. With this PR, you can:

I'm fine with both approaches, just the default needs to work with the PX4 default, independent from the node name. Which looks like it's the case here, right?

@damien-robotsix
Copy link
Contributor Author

It will if the node has no namespace (default in ROS2)

Copy link
Collaborator

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Thanks for clarifying

@bkueng bkueng merged commit aced150 into Auterion:main Jun 6, 2024
9 checks passed
@damien-robotsix
Copy link
Contributor Author

Thanks for merging :)

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.

2 participants