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

Topics and frame ids for compatibility with Nav2 #49

Closed
finani opened this issue Aug 8, 2024 · 5 comments
Closed

Topics and frame ids for compatibility with Nav2 #49

finani opened this issue Aug 8, 2024 · 5 comments

Comments

@finani
Copy link
Contributor

finani commented Aug 8, 2024

As I mentioned about the rule of Nav2 (PR) before, topics are separated by namespace and each topic should have the same frame id.
I thought this rule is only for tf (PR), but every topics including sensors should follow this.

I want to add compatibility with Nav2 to mvsim, but I worry about I will break the policy and projects using mvsim.
If there is any rule to follow, I will consider to add additional parameter.

I think there are three options.
a) Make every frame id of each topic the same. (radical)
b) Add parameter to control frame ids. (safe)
c) This changes are not considered in mvsim. (abort)

I will follow the decision.

before (now)

  • r1
topic name frame id
cam1 cam1
  • r1, r2
topic name frame id
r1/cam1 r1/cam1
r2/cam1 r2/cam1

after (compatible with Nav2)

  • r1
topic name frame id
cam1 cam1
  • r1, r2
topic name frame id
r1/cam1 cam1
r2/cam1 cam1
@jlblancoc
Copy link
Member

So, what I see is that topics are ok: in Nav2 they use as prefix the robot name and mvsim does too, and I think it all makes sense.

On /tf: what I understand in Steve's answer in ros-navigation/navigation2#4594 is that each robot listen to the tf topic tf so it becomes /tf or /r1/tf, etc. depending on the case, right?

What I don't understand is how to avoid collisions between frame_ids (not the /tf topics themselves).

Maybe the point is that multi-robot systems are expected not to share /tf, right?? cc: @SteveMacenski 🙏

If the answer is yes, ok, let's follow the same convention here, and we'll fix tutorials, and examples,etc. in mvsim.

@SteveMacenski
Copy link

SteveMacenski commented Aug 8, 2024

Maybe the point is that multi-robot systems are expected not to share /tf, right?? cc: @SteveMacenski 🙏

For that setup, yea. You don't need to avoid collisions when there are no collisions. You could share 1 massive TF system, but that would not be good for real, hardware situations

@jlblancoc
Copy link
Member

Alright, @SteveMacenski , thanks, it makes sense.

@finani Then, go on and feel free of PR for making things compatible with multi-robot Nav2. 👍

@finani
Copy link
Contributor Author

finani commented Aug 9, 2024

Thank you all.

Yes, Exactly I worried about the frame id collisions, too.
Because I was already using the entire tf tree for monitoring, and tf tree works only using frame ids.
But, after splitting by namespace, even rviz, It has no problem. (Thanks Steve)

Thank you again.
I will go update this feature.

@finani finani mentioned this issue Aug 11, 2024
@finani
Copy link
Contributor Author

finani commented Aug 11, 2024

#51 is merged
Close this issue.

@finani finani closed this as completed Aug 11, 2024
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

No branches or pull requests

3 participants