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

Change internal map topic from "map" to "stdr_map" #164

Open
wants to merge 1 commit into
base: hydro-devel
Choose a base branch
from

Conversation

mehditlili
Copy link

During the implementation of turtlebot_stdr simulator, there was no other way to make things work than changing the frame_id of the map to "world" (Because the frame_id map is actually representing the odometry frame and is not fixed) or running another map_server hosting the same map but with a different frame_id. I prefer the first solution as it consumes less ressources and I tested it on STDR, the frame_id doesn't seem to affect the good functionning of STDR.

@czalidis
Copy link
Member

Are you sure that everything works fine after this change? As I can see, you have to change map_server.cpp:94 also for transformations to work properly. Although, your approach might work accidentally if your map origin is zero [0, 0, 0] (this is the case for all maps in stdr_resources).

Conceptually, I agree that it is better to name the frame world as it is referring to the map representing the environment. But I don't think that this would resolve naming conflicts when it comes to frame_ids. If you are using a slam package that produces a map that has a different frame as world, you would probably end up with the same issues.

I am aware that the way stdr handles frame_ids is problematic when it comes to simulating robots that are doing some kind of localization. Actually, the above mentioned frames should be for internal use, for geometric calculations by stdr, and not exposed to the end user.

I am currently working to find a solution to that issue, maybe using a different master for internal stdr stuff. In the meantime, renaming map frame_id to world shouldn't affect other things, if done properly. If that helps your work, I am willing to proceed with that change.

@mehditlili
Copy link
Author

I think hardcoding the frame_id's is not a good solution. Why not do like in AMCL or Move_base and get all the needed parameters from am launch file?
The slam package from the navigation stack in ROS has all parameters defined this way so it is possible to tell slam to use world as frame_id.
And yeah having world instead of map as a frame_id would really help for my actual situation (just to avoid having to run an additional map server and use the internal stdr map server).
If The only change to be done has to be in map_server and map_loader I will commit those changes to this pull request.

@czalidis
Copy link
Member

Since that frame should only exist for internal stdr purposes there is no point to make it configurable, such as reading it from the param server.

If you are willing to proceed with the pull request, please make sure to test the functionality with different map origins. The changes that I suggested in my first comment should be applied also.

@czalidis
Copy link
Member

One more thing. As you can see, all the launchers in stdr_launchers spawn a static_trasnform_publisher that publishes a transform between world and map (ex. server_with_map_and_gui.launch:7). You have to exclude that from the launchers in order to test the changes.

@mehditlili mehditlili changed the title change map frame_id from map to world Change internal map topic from "map" to "stdr_map" Nov 24, 2014
@mehditlili
Copy link
Author

So what do you think of this last commit? I canceled other commits.

@czalidis
Copy link
Member

I see that you finally didn't change the frame_id, only the topic name. Is there a reason behind that decision?

I agree, the name of the map topic was a bit confusing, stdr_map fits better even if it is not intended for the end user.

@mehditlili
Copy link
Author

Yes I decided that using my own map server is better since you said that map_stdr is intended only for internal use. But I would suggest in the future to subscribe from stdr to a normal ros map_server.

@czalidis
Copy link
Member

So the separate map server you are using, has a different frame_id than map? If not, the issues discussed above remain.

The reasoning behind the change of the topic name form map to stdr_map is to avoid naming conflicts with other packages. Doesn't the same apply for the frame_id?

In my opinion either we change both the topic name and the frame_id or none of them.

I don't quite get what you are suggesting, but if you are saying that stdr should use the standard implementation of map_server, thats not possible due to extra functionality that is needed. Of course stdr uses the map_server's underlying library for image loading.

@etsardou
Copy link
Member

@czalidis what is the status of this PR? Should we merge it or..?

@czalidis
Copy link
Member

This PR partially addresses one of the many problems we have, regarding the internal STDR topics. I was holding this back because I was looking for a more general solution. Keep in mind that what is proposed here can be also resolved with a simple topic remap.

In general this PR was very useful because of the conversation that took place and clarified certain things. It can either be closed or left open, only for other people to read this conversation, util a solid solution has been found.

@etsardou
Copy link
Member

Ok, lets keep it open for reference, until we have a sound implementation.

@corot
Copy link
Contributor

corot commented May 5, 2016

Not sure if this helps, but... the fast, easy way I found to make things work the normal ROS way, with localization, move_base, and so on, is:

  • ignore world frame
  • use map instead of world as the static, global frame
  • use map_static as odom, being map -> map_static provided by amcl.
  • Of course, I need to comment out map_server publication of map -> map_static, so this tf doesn get duplicated.

It's dirty and names are inconsistent, but like this I can simulate navigation/localization same way as with the real robot, what is what many people wants of a 2D simulator!

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.

4 participants